Merge pull request #53 from NationalSecurityAgency/52-seabee-does-not-block-file-modification-with-sed

fix: inode_rename bug

Author: awandke

Cargo.toml

@@ -23,7 +23,7 @@ zerocopy = { version = "0.8", features = ["derive"] }
 edition = "2021"
 license = "Apache-2.0"
 readme = "README.md"
-rust-version = "1.79"
+rust-version = "1.83"
 version = "1.2.0"
 repository = "https://github.com/NationalSecurityAgency/seabee"
 homepage = "https://code.nsa.gov/seabee/"

bpf/src/seabee/seabee.bpf.c

@@ -435,15 +435,15 @@ int BPF_PROG(seabee_inode_setxattr, struct user_namespace *mnt_userns,
 /**
  * @brief prevent rename of a protected inode
  *
- @param old_dentry the old file
+ @param new_dentry the new file which will be replaced by old file
 */
 SEC("lsm/inode_rename")
 int BPF_PROG(seabee_inode_rename, struct inode *old_dir,
              struct dentry *old_dentry, struct inode *new_dir,
              struct dentry *new_dentry, unsigned int flags)
 {
-	return decide_inode_access(INODE_RENAME, old_dentry->d_inode,
-	                           old_dentry->d_name.name);
+	return decide_inode_access(INODE_RENAME, new_dentry->d_inode,
+	                           new_dentry->d_name.name);
 }
 
 /**

tests/policies/test_tool_debug_audit.yaml

@@ -5,6 +5,7 @@ scope:
   - ../target/debug/test_tool
 files:
   - /etc/seabee_test_tool
+  - /etc/seabee_test_tempfile
 config:
   map_access: audit
   file_write_access: audit

tests/policies/test_tool_debug_block.yaml

@@ -5,6 +5,7 @@ scope:
   - ../target/debug/test_tool
 files:
   - /etc/seabee_test_tool
+  - /etc/seabee_test_tempfile
 config:
   map_access: block
   file_write_access: block

tests/policies/test_tool_release_audit.yaml

@@ -5,6 +5,7 @@ scope:
   - ../target/release/test_tool
 files:
   - /etc/seabee_test_tool
+  - /etc/seabee_test_tempfile
 config:
   map_access: audit
   file_write_access: audit

tests/policies/test_tool_release_block.yaml

@@ -5,6 +5,7 @@ scope:
   - ../target/release/test_tool
 files:
   - /etc/seabee_test_tool
+  - /etc/seabee_test_tempfile
 config:
   map_access: block
   file_write_access: block

tests/src/bin/test_tool.rs

@@ -1,6 +1,5 @@
 // SPDX-License-Identifier: Apache-2.0
 
-use std::io::Write;
 use std::mem::MaybeUninit;
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::Arc;
@@ -12,10 +11,6 @@ use bpf::tests::test_tool::*;
 use tests::policy::test_constants;
 
 fn main() -> Result<()> {
-    // create file, TEST_DIR must already exist
-    let mut file = std::fs::File::create(test_constants::TEST_TOOL_FILE)?;
-    file.write_all(b"Hello, World!")?;
-
     // load ebpf skel
     let skel_builder = TestToolSkelBuilder::default();
     let mut open_object = MaybeUninit::uninit();
@@ -48,6 +43,7 @@ fn main() -> Result<()> {
 
     std::fs::remove_file(test_constants::TEST_TOOL_PIN_PATH)?;
     std::fs::remove_dir_all(test_constants::TEST_TOOL_DIR)?;
+    std::fs::remove_file(test_constants::TEST_TOOL_FILE)?;
 
     Ok(())
 }

tests/src/policy/protect_tool.rs

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use std::{
-    io::ErrorKind,
+    io::{ErrorKind, Write},
     path::Path,
     process::{Child, Command},
     sync::{
@@ -65,6 +65,14 @@ pub fn start_test_tool(level: SecurityLevel) -> Result<Child> {
             ));
         }
     }
+    // create file
+    let mut file = std::fs::File::create(test_constants::TEST_TOOL_FILE).map_err(|e| {
+        anyhow!(
+            "failed to create file {}: {e}",
+            test_constants::TEST_TOOL_FILE
+        )
+    })?;
+    file.write_all(b"Hello, World!")?;
 
     // add key
     Command::new(SEABEECTL_EXE)
@@ -276,6 +284,10 @@ fn deny_chmod_file() -> Result<(), Failed> {
     test_utils::try_chmod(test_constants::TEST_TOOL_FILE, false)
 }
 
+fn deny_rename_file() -> Result<(), Failed> {
+    test_utils::try_rename(test_constants::TEST_TOOL_FILE, false)
+}
+
 // check that protected directory attributes cannot be modified
 fn deny_chmod_dir() -> Result<(), Failed> {
     test_utils::try_chmod(test_constants::TEST_TOOL_DIR, false)
@@ -344,6 +356,7 @@ fn block_tests() -> Vec<Trial> {
         create_test!(deny_open_file),
         create_test!(deny_chmod_file),
         create_test!(deny_chmod_dir),
+        create_test!(deny_rename_file),
         create_test!(deny_policy_overwrite),
     ]
 }

tests/src/policy/test_constants.rs

@@ -2,6 +2,6 @@
 
 pub const TEST_TOOL_PIN_PATH: &str = "/sys/fs/bpf/test_tool_pin";
 pub const TEST_TOOL_DIR: &str = "/etc/seabee_test_tool";
-pub const TEST_TOOL_FILE: &str = "/etc/seabee_test_tool/tempfile";
+pub const TEST_TOOL_FILE: &str = "/etc/seabee_test_tempfile";
 pub const SHUTDOWN_REQUEST: &str = "shutdown_request.yaml";
 pub const SHUTDOWN_REQUEST_SIG: &str = "crypto/sigs/shutdown-request.sign";

tests/src/test_utils.rs

@@ -1,10 +1,43 @@
 // SPDX-License-Identifier: Apache-2.0
 
-use std::{fs, io::ErrorKind, os::unix::fs::PermissionsExt, path::Path};
+use anyhow::anyhow;
+use std::{fs, io::ErrorKind, os::unix::fs::PermissionsExt, path::Path, process::Command};
 
 use libtest_mimic::Failed;
 use nix::sys::{ptrace, signal::Signal::SIGCONT};
 
+pub fn try_rename(target: &str, expect_success: bool) -> Result<(), Failed> {
+    let output = Command::new("sed")
+        .arg("-i")
+        .arg("s/^/newtext/")
+        .arg(target)
+        .output()
+        .map_err(|e| anyhow!("failed to run sed: {e}"))?;
+
+    let code = match output.status.code() {
+        Some(c) => c,
+        None => {
+            return Err(format!(
+                "try_rename on {target} has no return code. possibly killed by signal unexpectedly"
+            )
+            .into())
+        }
+    };
+
+    // Check result
+    let stdout = String::from_utf8_lossy(&output.stdout);
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    if code == 0 && !expect_success {
+        return Err(format!("try_rename on {target} expected failure, but succeeded\nstdout: {stdout}\nstderr: {stderr}").into());
+    } else if code == 4 && expect_success {
+        // we see return code 4 on error
+        return Err(format!("try_rename on {target} expected success, but was denied\nstdout:{stdout}\nstderr: {stderr}").into());
+    } else if code != 0 && code != 4 {
+        return Err(format!("try_rename on {target}: unexpected return code: {code}\nstdout: {stdout}\nstderr:{stderr}").into());
+    }
+    Ok(())
+}
+
 // try chmod and expect permission denied
 pub fn try_chmod(path: &str, expect_success: bool) -> Result<(), Failed> {
     // Try to change permissions