Merge pull request #115 from multikernel/issue-111-deny-toctou-onbehalf-open

fs_deny: close the deny-bypass TOCTOU and namespace vectors

Author: congwang-mk

crates/sandlock-core/src/policy_fn.rs

@@ -147,22 +147,22 @@ pub struct PolicyContext {
     ceiling: LivePolicy,
     restricted: HashSet<&'static str>,
     pid_overrides: Arc<RwLock<HashMap<u32, HashSet<IpAddr>>>>,
-    denied_paths: Arc<RwLock<HashSet<String>>>,
+    denied: Arc<crate::seccomp::state::DeniedSet>,
 }
 
 impl PolicyContext {
     pub(crate) fn new(
         live: Arc<RwLock<LivePolicy>>,
         ceiling: LivePolicy,
         pid_overrides: Arc<RwLock<HashMap<u32, HashSet<IpAddr>>>>,
-        denied_paths: Arc<RwLock<HashSet<String>>>,
+        denied: Arc<crate::seccomp::state::DeniedSet>,
     ) -> Self {
         Self {
             live,
             ceiling,
             restricted: HashSet::new(),
             pid_overrides,
-            denied_paths,
+            denied,
         }
     }
 
@@ -246,16 +246,16 @@ impl PolicyContext {
     // ---- Filesystem restriction ----
 
     /// Deny access to a path (and all children). Checked by the supervisor
-    /// on openat/stat/access syscalls. Takes effect immediately.
+    /// on openat/stat/access syscalls. Takes effect immediately. The file's
+    /// inode identity is captured too, so the deny survives hardlinks and
+    /// renames to a non-denied name.
     pub fn deny_path(&self, path: &str) {
-        let mut denied = self.denied_paths.write().unwrap();
-        denied.insert(path.to_string());
+        self.denied.deny(path);
     }
 
     /// Remove a previously denied path.
     pub fn allow_path(&self, path: &str) {
-        let mut denied = self.denied_paths.write().unwrap();
-        denied.remove(path);
+        self.denied.allow(path);
     }
 
     // ---- Internal ----
@@ -337,14 +337,14 @@ pub(crate) fn spawn_policy_fn(
     live: Arc<RwLock<LivePolicy>>,
     ceiling: LivePolicy,
     pid_overrides: Arc<RwLock<HashMap<u32, HashSet<IpAddr>>>>,
-    denied_paths: Arc<RwLock<HashSet<String>>>,
+    denied: Arc<crate::seccomp::state::DeniedSet>,
 ) -> tokio::sync::mpsc::UnboundedSender<PolicyEvent> {
     let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel::<PolicyEvent>();
 
     std::thread::Builder::new()
         .name("sandlock-policy-fn".to_string())
         .spawn(move || {
-            let mut ctx = PolicyContext::new(live, ceiling, pid_overrides, denied_paths);
+            let mut ctx = PolicyContext::new(live, ceiling, pid_overrides, denied);
 
             while let Some(pe) = rx.blocking_recv() {
                 let verdict = callback(pe.event, &mut ctx);
@@ -389,8 +389,8 @@ mod tests {
         }));
         let ceiling = test_live();
         let pid_overrides = Arc::new(RwLock::new(HashMap::new()));
-        let denied_paths = Arc::new(RwLock::new(HashSet::new()));
-        let mut ctx = PolicyContext::new(live.clone(), ceiling, pid_overrides, denied_paths);
+        let denied = Arc::new(crate::seccomp::state::DeniedSet::default());
+        let mut ctx = PolicyContext::new(live.clone(), ceiling, pid_overrides, denied);
 
         let ip: IpAddr = "127.0.0.1".parse().unwrap();
         ctx.grant_network(&[ip]).unwrap();
@@ -406,8 +406,8 @@ mod tests {
         }));
         let ceiling = test_live();
         let pid_overrides = Arc::new(RwLock::new(HashMap::new()));
-        let denied_paths = Arc::new(RwLock::new(HashSet::new()));
-        let mut ctx = PolicyContext::new(live.clone(), ceiling, pid_overrides, denied_paths);
+        let denied = Arc::new(crate::seccomp::state::DeniedSet::default());
+        let mut ctx = PolicyContext::new(live.clone(), ceiling, pid_overrides, denied);
 
         // Try to grant an IP not in ceiling — should be silently ignored
         let foreign: IpAddr = "8.8.8.8".parse().unwrap();
@@ -420,8 +420,8 @@ mod tests {
         let live = Arc::new(RwLock::new(test_live()));
         let ceiling = test_live();
         let pid_overrides = Arc::new(RwLock::new(HashMap::new()));
-        let denied_paths = Arc::new(RwLock::new(HashSet::new()));
-        let mut ctx = PolicyContext::new(live, ceiling, pid_overrides, denied_paths);
+        let denied = Arc::new(crate::seccomp::state::DeniedSet::default());
+        let mut ctx = PolicyContext::new(live, ceiling, pid_overrides, denied);
 
         ctx.restrict_network(&[]);
         let ip: IpAddr = "127.0.0.1".parse().unwrap();
@@ -433,8 +433,8 @@ mod tests {
         let live = Arc::new(RwLock::new(test_live()));
         let ceiling = test_live();
         let pid_overrides = Arc::new(RwLock::new(HashMap::new()));
-        let denied_paths = Arc::new(RwLock::new(HashSet::new()));
-        let mut ctx = PolicyContext::new(live.clone(), ceiling, pid_overrides, denied_paths);
+        let denied = Arc::new(crate::seccomp::state::DeniedSet::default());
+        let mut ctx = PolicyContext::new(live.clone(), ceiling, pid_overrides, denied);
 
         ctx.restrict_max_memory(256 * 1024 * 1024);
         assert_eq!(live.read().unwrap().max_memory_bytes, 256 * 1024 * 1024);
@@ -445,8 +445,8 @@ mod tests {
         let live = Arc::new(RwLock::new(test_live()));
         let ceiling = test_live();
         let pid_overrides = Arc::new(RwLock::new(HashMap::new()));
-        let denied_paths = Arc::new(RwLock::new(HashSet::new()));
-        let ctx = PolicyContext::new(live, ceiling, pid_overrides.clone(), denied_paths);
+        let denied = Arc::new(crate::seccomp::state::DeniedSet::default());
+        let ctx = PolicyContext::new(live, ceiling, pid_overrides.clone(), denied);
 
         let localhost: IpAddr = "127.0.0.1".parse().unwrap();
         ctx.restrict_pid_network(1234, &[localhost]);
@@ -462,8 +462,8 @@ mod tests {
         let live = Arc::new(RwLock::new(test_live()));
         let ceiling = test_live();
         let pid_overrides = Arc::new(RwLock::new(HashMap::new()));
-        let denied_paths = Arc::new(RwLock::new(HashSet::new()));
-        let ctx = PolicyContext::new(live, ceiling, pid_overrides.clone(), denied_paths);
+        let denied = Arc::new(crate::seccomp::state::DeniedSet::default());
+        let ctx = PolicyContext::new(live, ceiling, pid_overrides.clone(), denied);
 
         let localhost: IpAddr = "127.0.0.1".parse().unwrap();
         ctx.restrict_pid_network(1234, &[localhost]);

crates/sandlock-core/src/sandbox.rs

@@ -1659,10 +1659,10 @@ impl Sandbox {
 
             let mut policy_fn_state = PolicyFnState::new();
 
-            if let Ok(mut denied) = policy_fn_state.denied_paths.write() {
-                for path in &self.fs_denied {
-                    denied.insert(path.to_string_lossy().into_owned());
-                }
+            for path in &self.fs_denied {
+                // Captures the path prefix and the file's inode identity, so
+                // the deny survives hardlinks/renames to a non-denied name.
+                policy_fn_state.denied.deny(&path.to_string_lossy());
             }
 
             if let Some(ref callback) = self.policy_fn {
@@ -1687,11 +1687,11 @@ impl Sandbox {
                 };
                 let ceiling = live.clone();
                 let live = std::sync::Arc::new(std::sync::RwLock::new(live));
-                let denied_paths = policy_fn_state.denied_paths.clone();
+                let denied = policy_fn_state.denied.clone();
                 let pid_overrides = net_state.pid_ip_overrides.clone();
                 policy_fn_state.live_policy = Some(live.clone());
                 let tx = crate::policy_fn::spawn_policy_fn(
-                    callback.clone(), live, ceiling, pid_overrides, denied_paths,
+                    callback.clone(), live, ceiling, pid_overrides, denied,
                 );
                 policy_fn_state.event_tx = Some(tx);
             }