fix(config): align managed directory security
- commit
- 9f412ea1232eb402ccfaf4b9ddb468dda9eaff3b
- parent
- 06e2c3b40c542efabbbc77bf63c7050f2ed6dc51
- parent
- 7f4c8d27133572e851de2100a08876da47a609cd
- author
- FelineStateMachine <dami.etoile@gmail.com>
| M | crates/ | 38 | ++++++++++ |
| M | crates/ | 27 | ++++++- |
| M | crates/ | 115 | ++++++++++++++++++++++++++++-- |
| M | docs/ | 7 | ++ |
| M | docs/ | 8 | ++ |
| M | scripts/ | 32 | ++++++++- |
@@ -382,6 +382,17 @@ impl HostConfig {
382382 }383383 384384 pub fn load_document(path: &Path) -> Result<LoadedHostConfig> {385+ let parent = path386+ .parent()387+ .filter(|parent| !parent.as_os_str().is_empty())388+ .unwrap_or(Path::new("."));389+ crate::secure_state::validate_integrity_protected_directory(parent).with_context(|| {390+ format!(391+ "validating config directory {} before loading {}",392+ parent.display(),393+ path.display()394+ )395+ })?;385396 let mut options = OpenOptions::new();386397 options.read(true);387398 @@ -603,6 +614,33 @@ mod tests {
603614 assert!(HostConfig::load_document(&path).is_err());604615 }605616 617+ 618+ 619+ fn config_directory_protects_integrity_without_requiring_confidentiality() {620+ use std::os::unix::fs::symlink;621+622+ let outer = tempfile::tempdir().unwrap();623+ let directory = outer.path().join("config");624+ fs::create_dir(&directory).unwrap();625+ fs::set_permissions(&directory, fs::Permissions::from_mode(0o755)).unwrap();626+ let path = directory.join("host.toml");627+ fs::write(628+ &path,629+ b"identity_path = \"/tmp/host.key\"\nstate_path = \"/tmp/state\"\n",630+ )631+ .unwrap();632+ fs::set_permissions(&path, fs::Permissions::from_mode(0o600)).unwrap();633+ HostConfig::load_document(&path).unwrap();634+635+ fs::set_permissions(&directory, fs::Permissions::from_mode(0o775)).unwrap();636+ assert!(HostConfig::load_document(&path).is_err());637+ fs::set_permissions(&directory, fs::Permissions::from_mode(0o755)).unwrap();638+639+ let linked_directory = outer.path().join("linked-config");640+ symlink(&directory, &linked_directory).unwrap();641+ assert!(HostConfig::load_document(&linked_directory.join("host.toml")).is_err());642+ }643+606644 607645 fn rejects_unknown_fields() {608646 let input = r#"@@ -835,8 +835,13 @@ fn replace_config(config_path: &Path, bytes: &[u8]) -> ManagementResult<()> {
835835 anyhow::anyhow!("config filename is invalid"),836836 )837837 })?;838- secure_state::atomic_write_exact(parent, file_name, bytes, MAX_CONFIG_BYTES)839- .map_err(|error| ManagementError::new(ManagementErrorCode::UnsafeStorage, error))838+ secure_state::atomic_write_exact_in_integrity_protected_directory(839+ parent,840+ file_name,841+ bytes,842+ MAX_CONFIG_BYTES,843+ )844+ .map_err(|error| ManagementError::new(ManagementErrorCode::UnsafeStorage, error))840845 }841846 842847 fn write_artifact(state_directory: &Path, file_name: &str, bytes: &[u8]) -> ManagementResult<()> {@@ -1009,7 +1014,7 @@ mod tests {
10091014 10101015 {10111016 use std::os::unix::fs::PermissionsExt as _;1012- fs::set_permissions(directory.path(), fs::Permissions::from_mode(0o700)).unwrap();1017+ fs::set_permissions(directory.path(), fs::Permissions::from_mode(0o755)).unwrap();10131018 }10141019 let state_directory = directory.path().join("state");10151020 secure_state::ensure_private_directory(&state_directory).unwrap();@@ -1124,6 +1129,22 @@ mod tests {
11241129 assert!(!fixture.state_directory.join(CANDIDATE_FILE).exists());11251130 }11261131 1132+ 1133+ 1134+ fn writable_config_parent_fails_before_transaction_mutation() {1135+ use std::os::unix::fs::PermissionsExt as _;1136+1137+ let fixture = Fixture::new();1138+ let original = fs::read(&fixture.config_path).unwrap();1139+ fs::set_permissions(fixture._directory.path(), fs::Permissions::from_mode(0o775)).unwrap();1140+ let error = set(&fixture.config_path, &fixture.request(72), None).unwrap_err();1141+ assert_eq!(error.code, ManagementErrorCode::UnsafeStorage);1142+ assert_eq!(fs::read(&fixture.config_path).unwrap(), original);1143+ assert!(read_journal(&fixture.state_directory).unwrap().is_none());1144+ assert!(!fixture.state_directory.join(BASE_FILE).exists());1145+ assert!(!fixture.state_directory.join(CANDIDATE_FILE).exists());1146+ }1147+11271148 11281149 fn prepared_recovery_forward_applies_the_exact_candidate() {11291150 let fixture = Fixture::new();@@ -25,27 +25,51 @@ pub fn ensure_private_directory(path: &Path) -> Result<()> {
2525 }2626 2727 pub fn validate_private_directory(path: &Path) -> Result<()> {28- let metadata = fs::symlink_metadata(path)29- .with_context(|| format!("inspecting private state directory {}", path.display()))?;30- ensure!(31- !metadata.file_type().is_symlink(),32- "private state directory must not be a symlink"33- );34- ensure!(metadata.is_dir(), "private state path must be a directory");28+ let metadata = validate_owned_directory(path)?;3529 3630 {3731 ensure!(3832 metadata.mode() & 0o077 == 0,3933 "private state directory is accessible by group or others"4034 );35+ }36+ Ok(())37+}38+39+pub fn validate_integrity_protected_directory(path: &Path) -> Result<()> {40+ let metadata = validate_owned_directory(path)?;41+ 42+ {43+ ensure!(44+ metadata.mode() & 0o300 == 0o300,45+ "protected directory must be writable and searchable by its owner"46+ );4147 ensure!(42- metadata.uid() == unsafe { libc::geteuid() },43- "private state directory has the wrong owner"48+ metadata.mode() & 0o022 == 0,49+ "protected directory is writable by group or others"4450 );4551 }4652 Ok(())4753 }4854 55+fn validate_owned_directory(path: &Path) -> Result<fs::Metadata> {56+ let metadata = fs::symlink_metadata(path)57+ .with_context(|| format!("inspecting owned directory {}", path.display()))?;58+ ensure!(59+ !metadata.file_type().is_symlink(),60+ "owned directory must not be a symlink"61+ );62+ ensure!(metadata.is_dir(), "owned path must be a directory");63+ 64+ {65+ ensure!(66+ metadata.uid() == unsafe { libc::geteuid() },67+ "owned directory has the wrong owner"68+ );69+ }70+ Ok(metadata)71+}72+4973 5074 pub enum LockAcquireError {5175 Busy,@@ -162,6 +186,25 @@ pub fn atomic_write_exact(
162186 maximum_bytes: u64,163187 ) -> Result<()> {164188 validate_private_directory(directory)?;189+ atomic_write_exact_in_validated_directory(directory, file_name, bytes, maximum_bytes)190+}191+192+pub fn atomic_write_exact_in_integrity_protected_directory(193+ directory: &Path,194+ file_name: &str,195+ bytes: &[u8],196+ maximum_bytes: u64,197+) -> Result<()> {198+ validate_integrity_protected_directory(directory)?;199+ atomic_write_exact_in_validated_directory(directory, file_name, bytes, maximum_bytes)200+}201+202+fn atomic_write_exact_in_validated_directory(203+ directory: &Path,204+ file_name: &str,205+ bytes: &[u8],206+ maximum_bytes: u64,207+) -> Result<()> {165208 ensure!(166209 bytes.len() as u64 <= maximum_bytes,167210 "private state file exceeds its fixed size bound"@@ -315,6 +358,60 @@ mod tests {
315358 assert!(!remove_file_if_exists(directory.path(), "config.toml").unwrap());316359 }317360 361+ 362+ 363+ fn integrity_protected_atomic_write_accepts_readable_but_not_writable_directory() {364+ let directory = private_directory();365+ fs::set_permissions(directory.path(), fs::Permissions::from_mode(0o755)).unwrap();366+ assert!(367+ atomic_write_exact(directory.path(), "private.toml", b"framerate = 60\n", 64,).is_err()368+ );369+ atomic_write_exact_in_integrity_protected_directory(370+ directory.path(),371+ "host.toml",372+ b"framerate = 60\n",373+ 64,374+ )375+ .unwrap();376+ assert_eq!(377+ fs::read(directory.path().join("host.toml")).unwrap(),378+ b"framerate = 60\n"379+ );380+ assert_eq!(381+ fs::metadata(directory.path().join("host.toml"))382+ .unwrap()383+ .permissions()384+ .mode()385+ & 0o777,386+ 0o600387+ );388+389+ fs::set_permissions(directory.path(), fs::Permissions::from_mode(0o555)).unwrap();390+ assert!(391+ atomic_write_exact_in_integrity_protected_directory(392+ directory.path(),393+ "host.toml",394+ b"framerate = 72\n",395+ 64,396+ )397+ .is_err()398+ );399+ fs::set_permissions(directory.path(), fs::Permissions::from_mode(0o775)).unwrap();400+ assert!(401+ atomic_write_exact_in_integrity_protected_directory(402+ directory.path(),403+ "host.toml",404+ b"framerate = 72\n",405+ 64,406+ )407+ .is_err()408+ );409+ assert_eq!(410+ fs::read(directory.path().join("host.toml")).unwrap(),411+ b"framerate = 60\n"412+ );413+ }414+318415 319416 320417 fn rejects_symlink_and_permissive_state() {@@ -410,6 +410,13 @@ Use these locations consistently:
410410 411411 This avoids modifying Bazzite's immutable base image.412412 413+Sigil requires the configuration file and its immediate directory to be owned414+by the service user and not writable by group or other users. This appliance415+layout deliberately uses mode `0700`; a conventional owner-controlled mode416+`0755` configuration directory is also valid, while group- or world-writable417+directories fail during `config check` and daemon startup before any managed418+configuration transaction can begin.419+413420 ## 6. Prepare a containerized build environment414421 415422 Bazzite recommends Distrobox for development packages. The repository must@@ -17,6 +17,14 @@ sigil appliance config rollback --config ~/.config/sigil-spark/host.toml \
1717 --transaction <id> --json1818 ```1919 20+The configuration file must be owned by the effective user and not writable by21+group or other users. Its immediate directory must also be owned by that user,22+must retain owner write and search permission, must not be a symlink, and must23+not be writable by group or other users. A mode-`0755` configuration directory24+is supported; mode `0700` remains the recommended appliance layout because it25+also keeps filenames private. The daemon, `config check`, and every managed26+write enforce this same integrity boundary before doing work.27+2028 `set` and `commit` use the service's XDG runtime root to share its global2129 lifecycle lock and to bind baseline and candidate evidence to one exact runtime2230 directory. When `XDG_RUNTIME_DIR` is unavailable (for example, from a recovery@@ -40,8 +40,11 @@ state_directory="$temp_root/state"
4040 runtime_directory="$temp_root/runtime"4141 alternate_runtime_directory="$temp_root/runtime-alternate"4242 invalid_runtime_directory="$temp_root/invalid-runtime"43+config_directory="$temp_root/config"44+unsafe_config_directory="$temp_root/config-unsafe"4345 identity_path="$identity_directory/host.key"44-config_path="$temp_root/host.toml"46+config_path="$config_directory/host.toml"47+unsafe_config_path="$unsafe_config_directory/host.toml"4548 host_log="$temp_root/host.log"4649 second_log="$temp_root/second.log"4750 live_status="$temp_root/live-status.json"@@ -58,6 +61,9 @@ mkdir -m 0700 \
5861 "$state_directory" \5962 "$runtime_directory" \6063 "$alternate_runtime_directory"64+mkdir -m 0755 "$config_directory"65+mkdir -m 0775 "$unsafe_config_directory"66+chmod 0775 "$unsafe_config_directory"6167 mkdir -m 0755 "$invalid_runtime_directory"6268 "$sigil" identity init --output "$identity_path" >"$temp_root/identity.log"6369 host_node_id="$(sed -n 's/^node_id=//p' "$temp_root/identity.log")"@@ -77,6 +83,30 @@ printf '%s\n' \
7783 >"$config_path"7884 chmod 0600 "$config_path"7985 86+cp "$config_path" "$unsafe_config_path"87+chmod 0600 "$unsafe_config_path"88+unsafe_config_hash="$(python3 - "$unsafe_config_path" <<'PY'89+import hashlib90+import pathlib91+import sys92+print(hashlib.sha256(pathlib.Path(sys.argv[1]).read_bytes()).hexdigest())93+PY94+)"95+if XDG_RUNTIME_DIR="$runtime_directory" \96+ "$sigil" config check --config "$unsafe_config_path" \97+ >"$temp_root/unsafe-config-check.out" \98+ 2>"$temp_root/unsafe-config-check.error"; then99+ printf 'config check accepted a group-writable config directory\n' >&2100+ exit 1101+fi102+[[ "$unsafe_config_hash" == "$(python3 - "$unsafe_config_path" <<'PY'103+import hashlib104+import pathlib105+import sys106+print(hashlib.sha256(pathlib.Path(sys.argv[1]).read_bytes()).hexdigest())107+PY108+)" ]]109+80110 XDG_RUNTIME_DIR="$runtime_directory" \81111 "$sigil" serve \82112 --config "$config_path" \