refactor(agent): keep private files with private-fs

.github/workflows/ci.yml

@@ -344,6 +344,9 @@ jobs:
         # A root-only test run without root skips and passes, so after changing this step, check that its log has no SKIP line.
         run: |
           cargo test --locked --target ${{ matrix.target }} --config "target.${{ matrix.target }}.runner='sudo'" -p device-key --lib root_only_
+          cargo test --locked --target ${{ matrix.target }} --config "target.${{ matrix.target }}.runner='sudo'" -p testsuite --test integration_tests private_fs::unix::root_only_
+        env:
+          AWS_LC_SYS_NO_ASM: true
 
       - name: Show sccache stats
         if: ${{ needs.preflight.outputs.sccache == 'true' && !cancelled() }}

crates/agent-identity/Cargo.toml

@@ -23,6 +23,7 @@ httpdate = "1"
 http-client-proxy = { path = "../http-client-proxy" }
 p256 = { version = "=0.14.0-rc.14", features = ["ecdsa", "pkcs8"] }
 p384 = { version = "=0.14.0-rc.14", features = ["ecdsa", "pkcs8"] }
+private-fs = { path = "../private-fs" }
 reqwest = { version = "0.12", default-features = false, features = ["rustls-tls-native-roots", "socks"] }
 rustls-pemfile = "2.2"
 serde = { version = "1", features = ["derive"] }
@@ -40,7 +41,6 @@ zeroize = "1"
 
 [target.'cfg(unix)'.dependencies]
 libc = "0.2"
-private-fs = { path = "../private-fs" }
 
 [target.'cfg(windows)'.dependencies]
 win-api-wrappers = { path = "../win-api-wrappers" }

crates/agent-identity/INTENT.md

@@ -52,7 +52,7 @@ The location and format of the files below are stable, because the conformance t
 - `<hex>` is the SHA-256 of the full token's UTF-8 bytes encoded as lowercase hexadecimal, and is the only encoding of a token hash the agent stores: file names and every `token_sha256` field use it.
 - One file per token, named `<hex>`:
   - Linux and macOS: `<data-dir>/identity/pending/<hex>.json`, mode `0600`, with content `{ "version": 1, "token": "<token>" }`.
-    The agent does not enforce the file's owner, and ignores and keeps a file with any group or other permission bit.
+    The file is owned by the agent's account or root; the agent ignores and keeps a file owned by any other account or with any group or other permission bit.
     The file holds the token in plaintext, protected only by its permissions, until the authority answers; that is accepted.
   - Windows: `<data-dir>\identity\pending\<hex>.dat`, whose content is DPAPI `CryptProtectData(json, entropy = "Devolutions.Agent.PendingEnrollment.v1", CRYPTPROTECT_LOCAL_MACHINE)` of the same JSON; `.dat` because the content is an opaque blob, not JSON.
     The agent accepts exactly two forms and ignores and keeps any other file:

crates/agent-identity/src/lib.rs

@@ -7,8 +7,6 @@ pub mod state;
 pub mod token;
 
 mod chain;
-#[cfg(target_os = "macos")]
-mod macos_acl;
 mod private_file;
 mod rejected;
 mod state_lock;

crates/agent-identity/src/pending.rs

@@ -80,7 +80,7 @@ pub fn write(data_dir: &Utf8Path, token: &Token, grant_current_user: bool) -> an
     let contents = plaintext;
     #[cfg(windows)]
     if administrator_writer && !grant_current_user {
-        crate::windows::submit_pending_as_administrator(data_dir, &path, &contents)?;
+        private_file::submit_pending_as_administrator(data_dir, &path, &contents)?;
         return Ok(path);
     }
     let _lock = state_lock::acquire(data_dir, grant_current_user)?;
@@ -110,7 +110,7 @@ pub fn list(data_dir: &Utf8Path) -> anyhow::Result<Vec<Utf8PathBuf>> {
     Ok(paths)
 }
 
-fn pending_hash_from_name(name: &str) -> Option<&str> {
+pub(crate) fn pending_hash_from_name(name: &str) -> Option<&str> {
     name.strip_suffix(&format!(".{EXTENSION}"))
         .filter(|hash| valid_hash_hex(hash))
 }
@@ -352,22 +352,19 @@ mod tests {
             URL_SAFE_NO_PAD.encode(r#"{"u":"https://example.test"}"#),
             "A".repeat(43)
         ))?;
-        let dir = Utf8Path::new(env!("CARGO_MANIFEST_DIR"))
-            .join("target")
-            .join("pending-tests")
-            .join(uuid::Uuid::new_v4().to_string());
-        fs::create_dir_all(&dir)?;
+        let test_dir = private_file::TestDir::new()?;
+        let dir = test_dir.path();
         #[cfg(unix)]
         {
             use std::os::unix::fs::PermissionsExt as _;
 
-            if fs::metadata(&dir)?.permissions().mode() & 0o022 != 0 {
-                fs::remove_dir_all(&dir)?;
+            if fs::metadata(dir)?.permissions().mode() & 0o022 != 0 {
+                fs::remove_dir_all(dir)?;
                 return Ok(());
             }
         }
-        private_file::prepare_directories(&dir, true)?;
-        let path = file_path(&dir, &token.sha256_hex())?;
+        private_file::prepare_directories(dir, true)?;
+        let path = file_path(dir, &token.sha256_hex())?;
         let body = format!(r#"{{"version":1,"token":"{}"}}"#, token.as_str());
         // JSON allows trailing white space, which pads the payload to an exact size.
         for (size, readable) in [(16 * 1024, true), (16 * 1024 + 1, false)] {
@@ -376,14 +373,14 @@ mod tests {
             let contents = crate::windows::protect(padded.as_bytes())?;
             #[cfg(not(windows))]
             let contents = padded.into_bytes();
-            private_file::write_atomic(&dir, &path, &contents, true)?;
+            private_file::write_atomic(dir, &path, &contents, true)?;
             assert_eq!(
                 matches!(read(&path, true)?, PendingRead::Ready(found) if found.as_str() == token.as_str()),
                 readable,
                 "{size}"
             );
         }
-        fs::remove_dir_all(&dir)?;
+        fs::remove_dir_all(dir)?;
         Ok(())
     }
 
@@ -396,23 +393,20 @@ mod tests {
             URL_SAFE_NO_PAD.encode(r#"{"u":"https://example.test"}"#),
             "A".repeat(43)
         );
-        let dir = Utf8Path::new(env!("CARGO_MANIFEST_DIR"))
-            .join("target")
-            .join("pending-tests")
-            .join(uuid::Uuid::new_v4().to_string());
-        fs::create_dir_all(&dir)?;
+        let test_dir = private_file::TestDir::new()?;
+        let dir = test_dir.path();
         #[cfg(unix)]
         {
             use std::os::unix::fs::PermissionsExt as _;
 
-            if fs::metadata(&dir)?.permissions().mode() & 0o022 != 0 {
-                fs::remove_dir_all(&dir)?;
+            if fs::metadata(dir)?.permissions().mode() & 0o022 != 0 {
+                fs::remove_dir_all(dir)?;
                 return Ok(());
             }
         }
         let hash = token::sha256_hex(&token);
-        let path = file_path(&dir, &hash)?;
-        private_file::prepare_directories(&dir, true)?;
+        let path = file_path(dir, &hash)?;
+        private_file::prepare_directories(dir, true)?;
         for oversized in [
             format!(
                 r#"{{"version":1,"token":"{token}","padding":"{}"}}"#,
@@ -429,7 +423,7 @@ mod tests {
             let contents = crate::windows::protect(oversized.as_bytes())?;
             #[cfg(not(windows))]
             let contents = oversized.into_bytes();
-            private_file::write_atomic(&dir, &path, &contents, true)?;
+            private_file::write_atomic(dir, &path, &contents, true)?;
             assert!(matches!(read(&path, true)?, PendingRead::Unusable));
         }
         #[cfg(windows)]
@@ -441,10 +435,10 @@ mod tests {
             .into_bytes();
             let ciphertext = crate::windows::protect(&oversized)?;
             assert!(ciphertext.len() > MAX_PENDING_BYTES + 4096);
-            private_file::write_atomic(&dir, &path, &ciphertext, true)?;
+            private_file::write_atomic(dir, &path, &ciphertext, true)?;
             assert!(matches!(read(&path, true)?, PendingRead::Unusable));
         }
-        fs::remove_dir_all(&dir)?;
+        fs::remove_dir_all(dir)?;
         Ok(())
     }
 
@@ -460,47 +454,44 @@ mod tests {
             "A".repeat(43)
         );
         let token = Token::parse(&raw)?;
-        let dir = Utf8Path::new(env!("CARGO_MANIFEST_DIR"))
-            .join("target")
-            .join("pending-tests")
-            .join(uuid::Uuid::new_v4().to_string());
-        fs::create_dir_all(&dir)?;
+        let test_dir = private_file::TestDir::new()?;
+        let dir = test_dir.path();
         let hash = hex::encode(Sha256::digest(raw.as_bytes()));
         assert_eq!(
-            file_path(&dir, &hash)?,
-            directory(&dir).join(format!("{hash}.{EXTENSION}"))
+            file_path(dir, &hash)?,
+            directory(dir).join(format!("{hash}.{EXTENSION}"))
         );
-        let result = write(&dir, &token, true);
+        let result = write(dir, &token, true);
         #[cfg(unix)]
-        if fs::metadata(&dir)?.permissions().mode() & 0o022 != 0 {
+        if fs::metadata(dir)?.permissions().mode() & 0o022 != 0 {
             assert!(
                 result.is_err(),
                 "pending file written to volume without owner-only permissions"
             );
-            assert!(list(&dir)?.is_empty());
-            fs::remove_dir_all(&dir)?;
+            assert!(list(dir)?.is_empty());
+            fs::remove_dir_all(dir)?;
             return Ok(());
         }
         let path = result?;
-        assert_eq!(path, file_path(&dir, &hash)?);
+        assert_eq!(path, file_path(dir, &hash)?);
         assert!(matches!(read(&path, true)?, PendingRead::Ready(found) if found.as_str() == raw));
-        let overwritten = write(&dir, &token, true)?;
+        let overwritten = write(dir, &token, true)?;
         assert_eq!(overwritten, path);
-        assert_eq!(list(&dir)?, vec![path.clone()]);
-        let token_named = directory(&dir).join(format!("{raw}.{EXTENSION}"));
+        assert_eq!(list(dir)?, vec![path.clone()]);
+        let token_named = directory(dir).join(format!("{raw}.{EXTENSION}"));
         fs::write(&token_named, b"not encrypted")?;
         #[cfg(unix)]
         fs::set_permissions(&token_named, fs::Permissions::from_mode(0o644))?;
-        assert_eq!(list(&dir)?, vec![path.clone()]);
+        assert_eq!(list(dir)?, vec![path.clone()]);
         let error = read(&token_named, true).expect_err("untrusted token-named file was read");
         assert!(!format!("{error:#}").contains(&raw));
         fs::remove_file(token_named)?;
-        private_file::write_atomic(&dir, &path, b"not a token", true)?;
+        private_file::write_atomic(dir, &path, b"not a token", true)?;
         #[cfg(windows)]
         {
             assert!(matches!(read(&path, true)?, PendingRead::Undecryptable));
             let encrypted = crate::windows::protect(b"not valid JSON")?;
-            private_file::write_atomic(&dir, &path, &encrypted, true)?;
+            private_file::write_atomic(dir, &path, &encrypted, true)?;
             assert!(matches!(read(&path, true)?, PendingRead::Unusable));
         }
         #[cfg(not(windows))]
@@ -522,26 +513,26 @@ mod tests {
                 URL_SAFE_NO_PAD.encode(r#"{"u":"https://other.test"}"#),
                 "A".repeat(43)
             ))?;
-            let independent = file_path(&dir, &other.sha256_hex())?;
+            let independent = file_path(dir, &other.sha256_hex())?;
             fs::write(&independent, b"independently supplied blob")?;
             assert!(
                 read(&independent, true).is_err(),
                 "files without a protected DACL must not be processed"
             );
             fs::remove_file(&independent)?;
-            crate::windows::assert_pending_protection(&path, true)?;
+            crate::windows::test::assert_pending_protection(&path, true)?;
             let protected_path = independent;
             if crate::windows::is_system()? {
-                assert_eq!(write(&dir, &other, false)?, protected_path);
-                assert_eq!(write(&dir, &other, false)?, protected_path);
-                crate::windows::assert_pending_protection(&protected_path, false)?;
+                assert_eq!(write(dir, &other, false)?, protected_path);
+                assert_eq!(write(dir, &other, false)?, protected_path);
+                crate::windows::test::assert_pending_protection(&protected_path, false)?;
                 fs::remove_file(protected_path)?;
             } else {
-                assert!(write(&dir, &other, false).is_err());
+                assert!(write(dir, &other, false).is_err());
                 assert!(!protected_path.exists());
             }
         }
-        fs::remove_dir_all(&dir)?;
+        fs::remove_dir_all(dir)?;
         Ok(())
     }
 }