diff --git a/crates/agent-artifact-admission/src/config.rs b/crates/agent-artifact-admission/src/config.rs index 59eeb748..e8ee38a5 100644 --- a/crates/agent-artifact-admission/src/config.rs +++ b/crates/agent-artifact-admission/src/config.rs @@ -1,8 +1,10 @@ use std::collections::BTreeSet; use std::fmt; -use std::fs::File; +use std::fs::{File, OpenOptions}; use std::io::Read; use std::net::SocketAddr; +#[cfg(target_os = "linux")] +use std::os::unix::fs::OpenOptionsExt; use std::path::Path; use serde::{Deserialize, Serialize}; @@ -19,6 +21,10 @@ const MAX_CREDENTIAL_FILE_BYTES: u64 = 16 * 1024; const MAX_REQUEST_BODY_BYTES: usize = 1024 * 1024; const MAX_ADMIN_TOKEN_BYTES: usize = 4096; const MIN_ADMIN_TOKEN_BYTES: usize = 32; +#[cfg(target_os = "linux")] +const LINUX_O_NOFOLLOW: i32 = 0o400000; +#[cfg(target_os = "linux")] +const LINUX_O_NONBLOCK: i32 = 0o4000; /// Immutable process configuration for the agent-artifact admission service. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -244,28 +250,43 @@ fn valid_executable(value: &str) -> bool { .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'-' | b'_' | b'.' | b'+')) } +/// Acquire security-sensitive local configuration through one Linux descriptor. +/// `O_NOFOLLOW` binds the decision to a non-symlink final component and +/// `O_NONBLOCK` prevents a FIFO from stalling startup before type validation. +#[cfg(target_os = "linux")] +fn open_local_authority_file(path: &Path) -> Result { + OpenOptions::new() + .read(true) + .custom_flags(LINUX_O_NOFOLLOW | LINUX_O_NONBLOCK) + .open(path) + .map_err(|_| ConfigError::Io) +} + +/// Fail closed on platforms where Wardnet has not implemented an equivalent +/// no-follow, nonblocking local-file authority contract. +#[cfg(not(target_os = "linux"))] +fn open_local_authority_file(_path: &Path) -> Result { + Err(ConfigError::Io) +} + fn read_credential_bounded(path: &Path, maximum_bytes: u64) -> Result, ConfigError> { - let file = File::open(path).map_err(|_| ConfigError::Io)?; + let file = open_local_authority_file(path)?; validate_credential_file_permissions(&file)?; read_open_file_bounded(file, maximum_bytes) } -#[cfg(unix)] +#[cfg(target_os = "linux")] fn validate_credential_file_permissions(file: &File) -> Result<(), ConfigError> { use std::os::unix::fs::PermissionsExt; - let mode = file - .metadata() - .map_err(|_| ConfigError::Io)? - .permissions() - .mode(); - if mode & 0o077 != 0 { + let metadata = file.metadata().map_err(|_| ConfigError::Io)?; + if !metadata.is_file() || metadata.permissions().mode() & 0o077 != 0 { return Err(ConfigError::InvalidCredential); } Ok(()) } -#[cfg(not(unix))] +#[cfg(not(target_os = "linux"))] fn validate_credential_file_permissions(_file: &File) -> Result<(), ConfigError> { Err(ConfigError::InvalidCredential) } @@ -276,24 +297,21 @@ fn validate_credential_file_permissions(_file: &File) -> Result<(), ConfigError> /// filesystem TOCTOU interval. See /// `docs/doctoring/agent-artifact-admission-configuration-integrity.md`. fn read_bounded(path: &Path, maximum_bytes: u64) -> Result, ConfigError> { - let file = File::open(path).map_err(|_| ConfigError::Io)?; + let file = open_local_authority_file(path)?; validate_config_file_permissions(&file)?; read_open_file_bounded(file, maximum_bytes) } -/// Reject Unix policy files writable by group or other principals while -/// preserving read-only visibility. Policy integrity, not confidentiality, is -/// the invariant at this boundary; credentials use a separate stricter check. -#[cfg(unix)] +/// Reject non-regular policy inputs and Linux policy files writable by group or +/// other principals while preserving read-only visibility. Policy integrity, +/// not confidentiality, is the invariant at this boundary; credentials use a +/// separate stricter check. +#[cfg(target_os = "linux")] fn validate_config_file_permissions(file: &File) -> Result<(), ConfigError> { use std::os::unix::fs::PermissionsExt; - let mode = file - .metadata() - .map_err(|_| ConfigError::Io)? - .permissions() - .mode(); - if mode & 0o022 != 0 { + let metadata = file.metadata().map_err(|_| ConfigError::Io)?; + if !metadata.is_file() || metadata.permissions().mode() & 0o022 != 0 { return Err(ConfigError::InvalidConfiguration); } Ok(()) @@ -303,7 +321,7 @@ fn validate_config_file_permissions(file: &File) -> Result<(), ConfigError> { /// configuration mutation authority. Adding a platform-specific ACL model is a /// separate compatibility change; silently accepting unverifiable authority is /// not an equivalent security boundary. -#[cfg(not(unix))] +#[cfg(not(target_os = "linux"))] fn validate_config_file_permissions(_file: &File) -> Result<(), ConfigError> { Err(ConfigError::InvalidConfiguration) } diff --git a/crates/agent-artifact-admission/tests/local_file_authority_contract.rs b/crates/agent-artifact-admission/tests/local_file_authority_contract.rs new file mode 100644 index 00000000..39cf9814 --- /dev/null +++ b/crates/agent-artifact-admission/tests/local_file_authority_contract.rs @@ -0,0 +1,160 @@ +#![cfg(unix)] + +use std::fs; +use std::os::unix::fs::{PermissionsExt, symlink}; +use std::path::PathBuf; +#[cfg(target_os = "linux")] +use std::process::{Command, Stdio}; +#[cfg(target_os = "linux")] +use std::thread; +#[cfg(target_os = "linux")] +use std::time::{Duration, Instant}; +use std::time::{SystemTime, UNIX_EPOCH}; + +use wardnet_agent_artifact_admission::{ + AdmissionPolicy, AdmissionServiceConfig, CredentialFile, load_admin_token, load_config, +}; + +#[cfg(target_os = "linux")] +const FIFO_HELPER_PATH: &str = "WARDNET_ADMISSION_FIFO_HELPER_PATH"; +#[cfg(target_os = "linux")] +const HELPER_DEADLINE: Duration = Duration::from_secs(3); + +fn temp_path(label: &str) -> PathBuf { + let nonce = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("clock must be after epoch") + .as_nanos(); + std::env::temp_dir().join(format!( + "wardnet-agent-local-file-{label}-{}-{nonce}", + std::process::id() + )) +} + +fn valid_config() -> AdmissionServiceConfig { + AdmissionServiceConfig { + configuration_version: "1".to_string(), + bind_address: "127.0.0.1:8787".to_string(), + max_request_body_bytes: 64 * 1024, + audit_log_path: "/var/lib/wardnet/agent-artifact-admission.ndjson".to_string(), + policy: AdmissionPolicy { + policy_id: "deny-all".to_string(), + policy_revision: "test".to_string(), + allowed_executables: Vec::new(), + approved_manifests: Vec::new(), + approved_artifacts: Vec::new(), + }, + } +} + +#[test] +fn credential_loader_rejects_final_symlink_even_to_owner_only_regular_file() { + let target = temp_path("credential-target.json"); + let link = temp_path("credential-link.json"); + let credential = CredentialFile { + admin_token: "0123456789abcdef0123456789abcdef".to_string(), + }; + + fs::write( + &target, + serde_json::to_vec(&credential).expect("credential fixture must serialize"), + ) + .expect("credential target must write"); + fs::set_permissions(&target, fs::Permissions::from_mode(0o600)) + .expect("credential target must be owner-only"); + symlink(&target, &link).expect("credential symlink must be created"); + + assert!( + load_admin_token(&link).is_err(), + "a symlink must not become credential-path authority even when its target is owner-only" + ); + + let _ = fs::remove_file(link); + let _ = fs::remove_file(target); +} + +#[test] +fn policy_loader_rejects_final_symlink_even_to_integrity_protected_regular_file() { + let target = temp_path("policy-target.json"); + let link = temp_path("policy-link.json"); + + fs::write( + &target, + serde_json::to_vec(&valid_config()).expect("policy fixture must serialize"), + ) + .expect("policy target must write"); + fs::set_permissions(&target, fs::Permissions::from_mode(0o644)) + .expect("policy target must not be group/other writable"); + symlink(&target, &link).expect("policy symlink must be created"); + + assert!( + load_config(&link).is_err(), + "a symlink must not become reviewed policy-path authority even when its target is not group/other writable" + ); + + let _ = fs::remove_file(link); + let _ = fs::remove_file(target); +} + +#[cfg(target_os = "linux")] +#[test] +fn loaders_reject_fifo_without_blocking_before_file_type_validation() { + let fifo_path = temp_path("authority-fifo"); + let mkfifo = Command::new("mkfifo") + .arg(&fifo_path) + .status() + .expect("mkfifo must be available on the Linux test runner"); + assert!(mkfifo.success(), "FIFO fixture must be created"); + + let executable = std::env::current_exe().expect("current test executable must resolve"); + let mut child = Command::new(executable) + .arg("--exact") + .arg("local_authority_fifo_open_helper") + .arg("--nocapture") + .env(FIFO_HELPER_PATH, &fifo_path) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .expect("isolated FIFO helper must start"); + + let deadline = Instant::now() + HELPER_DEADLINE; + let status = loop { + match child + .try_wait() + .expect("FIFO helper status must be readable") + { + Some(status) => break Some(status), + None if Instant::now() < deadline => thread::sleep(Duration::from_millis(25)), + None => break None, + } + }; + + let _ = fs::remove_file(&fifo_path); + + match status { + Some(status) => assert!( + status.success(), + "both local authority readers must reject a FIFO through their stable fail-closed errors" + ), + None => { + let _ = child.kill(); + let _ = child.wait(); + panic!( + "an admission local-file reader blocked while opening a FIFO; special inputs must fail closed promptly" + ); + } + } +} + +#[cfg(target_os = "linux")] +#[test] +fn local_authority_fifo_open_helper() { + let Ok(path) = std::env::var(FIFO_HELPER_PATH) else { + return; + }; + let path = PathBuf::from(path); + + assert!(load_admin_token(&path).is_err()); + assert!(load_config(&path).is_err()); +} diff --git a/docs/doctoring/agent-artifact-admission-configuration-integrity.md b/docs/doctoring/agent-artifact-admission-configuration-integrity.md index e32e1343..b093d2e7 100644 --- a/docs/doctoring/agent-artifact-admission-configuration-integrity.md +++ b/docs/doctoring/agent-artifact-admission-configuration-integrity.md @@ -4,21 +4,33 @@ Wardnet treats the Agent Artifact Admission configuration as policy authority, not as a secret. The file contains the reviewed policy revision, executable allowlist, workspace-manifest digests, and exact artifact coordinates. Read-only group or other visibility therefore does not change admission authority, but group or other write authority does: an unintended writer could replace an approved digest, artifact coordinate, or executable and thereby alter the result of a later admission decision. -On Unix, the loader opens the configured path once and inspects permissions through metadata obtained from that already-open `File` before reading and parsing its bytes. It rejects any group/other write bit (`mode & 0o022 != 0`) as `ConfigError::InvalidConfiguration`. This intentionally permits read-only modes such as `0644` while rejecting policy-mutation authority such as `0664` or `0666`. The separate credential loader remains stricter because credential confidentiality, unlike policy-file confidentiality, is itself a security requirement. +Credentials are a separate authority-bearing input. Their confidentiality and integrity both matter because the document carries the admission endpoint bearer token. A filesystem pathname is not itself sufficient evidence that either policy or credential bytes came from the intended local object. -The same-open-handle sequence is deliberate. A path-level permission check followed by a separate open would create a check/use interval in which the pathname could resolve to a different object. Dean and Hu (2004) formalize this class of filesystem TOCTOU race and show why a security decision separated from acquisition is unsafe under an adversarial pathname. Borisov et al. (2005) subsequently demonstrate that probabilistic attempts to make such path races difficult remain exploitable, reinforcing the preference for descriptor-bound checks rather than repeated pathname checks. Wardnet does not claim that descriptor metadata alone solves every filesystem replacement problem; it closes the narrower defect in this slice: deciding whether the bytes already opened as policy are writable by unintended Unix principals before those same opened bytes are materialized. +## Descriptor-bound acquisition -On non-Unix targets, this version fails closed because the product has not defined or tested a native ACL-equivalence contract for policy mutation authority. Silently accepting the configuration would assert a security property the implementation cannot currently verify. Adding Windows ACL or another platform-native authority model is a separate compatibility increment and must retain the same fail-closed invariant. +On Linux, both loaders now use one read-only open with `O_NOFOLLOW | O_NONBLOCK`, then inspect the resulting descriptor before materializing bytes. A final symbolic link is therefore rejected by the open operation rather than followed. A FIFO can be opened for inspection without waiting for a writer, after which Wardnet rejects it because the opened descriptor is not a regular file. Other special-file types are rejected by the same regular-file invariant. + +This order is deliberate. POSIX.1-2024 specifies that `O_NOFOLLOW` causes `open()` to fail when the final pathname component is a symbolic link and that a read-only FIFO opened with `O_NONBLOCK` returns without waiting for a writer. The same standard notes that no-follow behavior avoids races in which a pathname is substituted with a symbolic link to a sensitive object. MITRE CWE-59 classifies security-sensitive link following as improper link resolution before file access and identifies confidentiality, integrity and access-control consequences. + +After the descriptor is acquired, the policy loader rejects any group/other write bit (`mode & 0o022 != 0`) as `ConfigError::InvalidConfiguration`. This intentionally permits read-only modes such as `0644` while rejecting policy-mutation authority such as `0664` or `0666`. The credential loader is stricter and rejects any group/other permission bit (`mode & 0o077 != 0`) because credential confidentiality is itself a requirement. Both checks use metadata from the already-open regular-file descriptor. + +The same-open-handle sequence also preserves the earlier TOCTOU decision. A path-level permission or file-type check followed by a separate open would create a check/use interval in which the pathname could resolve to a different object. Dean and Hu (2004) formalize this class of filesystem race, while Borisov et al. (2005) show why probabilistic attempts to make such races difficult do not provide a sound authority boundary. Wardnet therefore does not add a pathname pre-check as a substitute for descriptor-bound acquisition. + +On targets other than Linux, this version fails closed because Wardnet has not defined and tested an equivalent no-follow, nonblocking open plus native ACL authority contract. The file-backed audit sink already follows the same compatibility boundary. Adding another platform is a separate compatibility increment and must preserve equivalent link, special-file, permission and bounded-read guarantees rather than silently weakening them. ## Executable acceptance contract -`config_file_permissions_contract.rs` creates one valid configuration, applies safe and unsafe Unix modes to the same fixture, and calls the public loader. Safe modes `0600`, `0640`, `0644`, and `0400` must load. Unsafe modes `0660`, `0606`, `0664`, `0646`, and `0666` must return exactly `ConfigError::InvalidConfiguration`; accepting any of them, or failing for a generic I/O/JSON reason, does not satisfy the security contract. +`config_file_permissions_contract.rs` creates one valid configuration, applies safe and unsafe Linux modes to the same fixture, and calls the public loader. Safe modes `0600`, `0640`, `0644`, and `0400` must load. Unsafe modes `0660`, `0606`, `0664`, `0646`, and `0666` must return exactly `ConfigError::InvalidConfiguration`; accepting any of them, or failing for a generic JSON reason, does not satisfy the security contract. -This maps directly to CWE-732: a security-critical configuration resource must not be modifiable by unintended actors. NIST SP 800-53 Rev. 5 CM-5 requires defined and enforced logical/physical restrictions on system changes; AC-6 provides the least-privilege principle for granting only the authorizations required for the task. Here, the smallest enforceable local boundary is write authority over the reviewed policy file. +`local_file_authority_contract.rs` covers the object-identity boundary independently of permission bits. A mode-`0600` credential target and a mode-`0644` policy target reached only through final symbolic links must both fail closed. Its Linux FIFO helper runs in a child process with a fixed deadline so a regression cannot hang the test job indefinitely; both loaders must reject the FIFO promptly before any attempt to parse it as JSON. -## Scope and residual risk +These tests complement rather than replace the append-only audit-path contracts. The audit sink and the two admission input readers now use the same Linux acquisition properties while retaining different write/read and confidentiality invariants appropriate to their bounded responsibilities. -This decision does not add runtime policy mutation, directory-ownership policy, secret distribution, sandbox execution, reusable egress control, LLM orchestration, or static package analysis. Those remain outside this bounded context. It also does not claim immutable storage, signature verification, or a complete cross-platform ACL model. The next independent audit-path hardening work is tracked separately and must not be folded into this configuration-loader slice. +## Control mapping and scope + +The permission portion maps directly to CWE-732: a security-critical configuration resource must not be modifiable by unintended actors. The pathname-object portion maps to CWE-59. NIST SP 800-53 Rev. 5 CM-5 requires defined and enforced restrictions on system changes, while AC-6 provides the least-privilege principle for granting only the authorizations required for the task. Here, the smallest enforceable local boundary is: acquire one non-symlink, nonblocking regular-file descriptor; verify the relevant local authority bits on that descriptor; then read only within the fixed byte budget. + +This decision does not add runtime policy mutation, directory-ownership policy, secret distribution, hostile workload execution, reusable egress control, LLM orchestration, or static package analysis. Those remain outside this bounded context. It does not claim immutable storage, signature verification, protection against a malicious privileged filesystem administrator, or a complete cross-platform ACL model. The USENIX papers are linked to their publisher copies rather than vendored into this repository. Their published reproduction terms are narrower than an unrestricted software-repository redistribution grant, so the repository preserves citation and traceability without copying the PDFs. @@ -28,6 +40,10 @@ Borisov, N., Johnson, R., Sastry, N., & Wagner, D. (2005). Fixing races for fun Dean, D., & Hu, A. J. (2004). Fixing races for fun and profit: How to use access(2). *13th USENIX Security Symposium*. https://www.usenix.org/conference/13th-usenix-security-symposium/fixing-races-fun-and-profit-how-use-access2 +IEEE & The Open Group. (2024). *open, openat — open file*. In *POSIX.1-2024*. https://pubs.opengroup.org/onlinepubs/9799919799/functions/open.html + +MITRE. (2026). *CWE-59: Improper link resolution before file access ('link following') (CWE 4.20)*. https://cwe.mitre.org/data/definitions/59.html + MITRE. (2026). *CWE-732: Incorrect permission assignment for critical resource (CWE 4.20)*. https://cwe.mitre.org/data/definitions/732.html National Institute of Standards and Technology. (2020). *Security and privacy controls for information systems and organizations* (NIST Special Publication 800-53 Rev. 5). https://doi.org/10.6028/NIST.SP.800-53r5