From 20fd755d0db52206b99fd5d71bc44b035d6fc23b Mon Sep 17 00:00:00 2001
From: Fedor Korotkov <fkorotkov@openai.com>
Date: Tue, 21 Jul 2026 13:59:44 -0400
Subject: [PATCH] Reject superseded Softnet policy revisions

---
 README.md            |  2 +-
 lib/proxy/control.rs | 52 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/README.md b/README.md
index 7dd94ee..a9539a3 100644
--- a/README.md
+++ b/README.md
@@ -55,6 +55,6 @@ The supported methods are `softnet.policy.get` and `softnet.policy.set`. A compl
 {"jsonrpc":"2.0","id":"42","result":{"allow":["10.0.0.0/8","@host"],"block":["0.0.0.0/0"],"desiredRevision":"vm-uid:42","ruleCount":3,"bridgeIsolation":true}}
 ```
 
-Every request must include a non-null string (at most 256 bytes) or non-negative integer `id`; notifications are rejected so policy changes always have an acknowledgment. Policy updates are atomic: all targets are parsed and a new prefix map is built before the active policy changes. Longest-prefix matching and block precedence for identical prefixes are preserved. Targets are normalized and deduplicated, so retrying a revision with the same policy is idempotent; reusing a revision with a different policy returns a JSON-RPC conflict error. A policy may contain at most 4096 combined allow/block targets, and a request frame may not exceed 1 MiB.
+Every request must include a non-null string (at most 256 bytes) or non-negative integer `id`; notifications are rejected so policy changes always have an acknowledgment. Policy updates are atomic: all targets are parsed and a new prefix map is built before the active policy changes. Longest-prefix matching and block precedence for identical prefixes are preserved. Targets are normalized and deduplicated, so retrying the active revision with the same policy is idempotent; reusing it with a different policy or retrying a superseded revision returns a JSON-RPC conflict error. A policy may contain at most 4096 combined allow/block targets, and a request frame may not exceed 1 MiB.
 
 `--allow=0.0.0.0/0` additionally disables vmnet bridge isolation during interface creation. Bridge isolation cannot be changed for a running VM, so policy updates that would toggle it are rejected without changing the active policy. Use `block=["0.0.0.0/0"]` with specific allow targets for a default-deny policy. Closing the control socket leaves the last accepted policy active.
diff --git a/lib/proxy/control.rs b/lib/proxy/control.rs
index 5879633..1f3984c 100644
--- a/lib/proxy/control.rs
+++ b/lib/proxy/control.rs
@@ -12,6 +12,7 @@ use prefix_trie::{Prefix, PrefixMap};
 use serde::Deserialize;
 use serde_json::{Value, json};
 use smoltcp::wire::Ipv4Address;
+use std::collections::HashSet;
 use std::io::{self, ErrorKind, Read, Write};
 use std::mem::{size_of, zeroed};
 use std::os::fd::{AsRawFd, FromRawFd, RawFd};
@@ -31,6 +32,7 @@ pub(super) struct Policy {
     allow: Vec<Target>,
     block: Vec<Target>,
     desired_revision: Option<String>,
+    applied_revisions: HashSet<String>,
     bridge_isolation: bool,
     gateway_ip: Ipv4Address,
 }
@@ -47,6 +49,7 @@ impl Policy {
             allow,
             block,
             desired_revision: None,
+            applied_revisions: HashSet::new(),
             bridge_isolation,
             gateway_ip,
         }
@@ -88,6 +91,13 @@ impl Policy {
             ));
         }
 
+        if self.applied_revisions.contains(&desired_revision) {
+            return Err(rpc_error(
+                REVISION_CONFLICT,
+                "desiredRevision was already superseded by a newer policy",
+            ));
+        }
+
         if bridge_isolation != self.bridge_isolation {
             return Err(rpc_error(
                 BRIDGE_ISOLATION_CONFLICT,
@@ -100,6 +110,7 @@ impl Policy {
         self.rules = rules;
         self.allow = allow;
         self.block = block;
+        self.applied_revisions.insert(desired_revision.clone());
         self.desired_revision = Some(desired_revision);
 
         Ok(())
@@ -631,6 +642,47 @@ mod tests {
         assert_eq!(policy.result(), before);
     }
 
+    #[test]
+    fn superseded_revision_cannot_roll_back_the_active_policy() {
+        let mut policy = policy(&[], &[]);
+
+        let first = request(
+            &mut policy,
+            json!({
+                "jsonrpc": "2.0",
+                "id": 1,
+                "method": "softnet.policy.set",
+                "params": {"allow": ["10.0.0.0/8"], "block": [], "desiredRevision": "vm-uid:41"}
+            }),
+        );
+        assert_eq!(first["result"]["desiredRevision"], "vm-uid:41");
+
+        let second = request(
+            &mut policy,
+            json!({
+                "jsonrpc": "2.0",
+                "id": 2,
+                "method": "softnet.policy.set",
+                "params": {"allow": [], "block": ["0.0.0.0/0"], "desiredRevision": "vm-uid:42"}
+            }),
+        );
+        assert_eq!(second["result"]["desiredRevision"], "vm-uid:42");
+        let before = policy.result();
+
+        let stale = request(
+            &mut policy,
+            json!({
+                "jsonrpc": "2.0",
+                "id": 3,
+                "method": "softnet.policy.set",
+                "params": {"allow": ["10.0.0.0/8"], "block": [], "desiredRevision": "vm-uid:41"}
+            }),
+        );
+
+        assert_eq!(stale["error"]["code"], REVISION_CONFLICT);
+        assert_eq!(policy.result(), before);
+    }
+
     #[test]
     fn invalid_targets_limits_and_bridge_isolation_changes_leave_policy_unchanged() {
         let mut policy = policy(&["@host"], &["0.0.0.0/0"]);