From 44f826a9d327c70a058ea0aa31ec181338bda6c4 Mon Sep 17 00:00:00 2001 From: LeChatP Date: Sun, 8 Nov 2026 11:36:52 +0100 Subject: [PATCH 1/2] fix: chsr does not rewrite a policy for a new version alone, and writes the version of the format * fix: chsr upgrade rewrote every file whose version was not the version of the package, even when no migration changed it: at each release, 4.3.1 included, the postinst of the package ran chsr upgrade, which failed through dosr (t_root, without CAP_LINUX_IMMUTABLE) and printed "run dosr chsr upgrade" for nothing; decided by the maintainer: the version of a file does not change when nothing else does * fix: a file that this version reads as it is (from UNCHANGED_SINCE, 4.1.0) is up to date: chsr upgrade and --check leave it with its version; an older one is migrated as before * feat: FORMAT_VERSION (4.3.0), the last version whose migration changes the documents, up or down: chsr writes it in a file that it saves and in the data that it creates, where it wrote its own version; the dosr of 4.3.0, which refuses a newer version, then reads a file saved by 4.3.1, a downgrade between them included * fix: chsr upgrade --check only reads the policy, and chsr upgrade reads it first and opens it for writing only when a file is to upgrade: both work on an up to date immutable policy without CAP_LINUX_IMMUTABLE, where chsr always opened it for writing and failed with "Operation not permitted" * fix: chsr upgrade --to the version that a file already has reports it, where it gave "its version is not known, or is already" as an error * chore: the report says ": , up to date" and "upgraded from to 4.3.0", the editor "migrated to 4.3.0" * tests: test_format_version: the migrations after FORMAT_VERSION change nothing both ways, a document of FORMAT_VERSION or of the current version is not migrated, new data is written for FORMAT_VERSION; 4.1.0 and 4.2.0 documents are read as they are; test_upgrade_access; test_upgrade_reads_an_up_to_date_policy_only: a 4.1.0 file that cannot be written is checked and upgraded through read-only settings and stays as it was, a 4.0.3 one is to upgrade, which fails on the former code; test_upgrade_to_an_older_version: once written for 4.1.0, the file is up to date and keeps 4.1.0; the tests that read PACKAGE_VERSION in a saved file read FORMAT_VERSION * tests: e2e.sh upgrade_without_cap_linux_immutable: under capsh --drop=cap_linux_immutable, chsr upgrade and --check succeed on the immutable policy, and fail with a 4.0.3 file planted Co-Authored-By: Claude --- core/src/database/migrations.rs | 110 +++++++++++++++++++------- core/src/database/versionning.rs | 16 ++-- core/src/file.rs | 31 ++++++-- src/chsr/cli/editor.rs | 12 ++- src/chsr/cli/locality.rs | 7 +- src/chsr/cli/mod.rs | 127 +++++++++++++++++++++++++++++-- src/chsr/cli/process/upgrade.rs | 19 +++-- src/chsr/main.rs | 20 ++++- tests/vagrant/e2e.sh | 20 +++++ 9 files changed, 303 insertions(+), 59 deletions(-) diff --git a/core/src/database/migrations.rs b/core/src/database/migrations.rs index c8d526a0..7514e8eb 100644 --- a/core/src/database/migrations.rs +++ b/core/src/database/migrations.rs @@ -542,14 +542,38 @@ fn describe_changes(before: &Value, after: &Value, pointer: &str, changes: &mut } } -/// Migrate a raw policy document to the current version, before it is deserialized. +/// Migrate a raw policy document written by an older version, before it is deserialized: to +/// [`FORMAT_VERSION`], the version that chsr writes. /// -/// A document without a parsable version, or already at the current version, is left unchanged. +/// A document without a parsable version, or one that the current version reads as it is (see +/// [`reads_as_is`]), is left unchanged, its version included: no migration would change it. /// Returns what the migration changed, if the document was migrated. /// # Errors /// Returns an error if a migration fails or if there is no migration path to the current version. pub fn migrate_document(document: &mut Value) -> Result, MigrationError> { - migrate_document_to(document, &PACKAGE_VERSION) + if document_version(document).is_some_and(|version| reads_as_is(&version)) { + return Ok(None); + } + let migration = migrate_document_to(document, &PACKAGE_VERSION)?; + // the migrations after FORMAT_VERSION change nothing + if migration.is_some() + && let Value::Object(map) = document + { + map.insert( + "version".to_string(), + Value::String(FORMAT_VERSION.to_string()), + ); + } + Ok(migration) +} + +/// The version of a raw policy document, under its long or its short key. +fn document_version(document: &Value) -> Option { + document + .get("version") + .or_else(|| document.get("v")) + .and_then(Value::as_str) + .and_then(|version| Version::parse(version).ok()) } /// Migrate a raw policy document to the version `to`, newer or older than its own. @@ -562,12 +586,7 @@ pub fn migrate_document_to( document: &mut Value, to: &Version, ) -> Result, MigrationError> { - let Some(version) = document - .get("version") - .or_else(|| document.get("v")) - .and_then(Value::as_str) - .and_then(|version| Version::parse(version).ok()) - else { + let Some(version) = document_version(document) else { return Ok(None); }; if version == *to { @@ -614,6 +633,15 @@ pub fn writable_version(text: &str) -> Result { /// migration that rewrites the documents on the way up raises it to its `to`. pub const UNCHANGED_SINCE: Version = Version::new(4, 1, 0); +/// The version that chsr writes in a policy file that it saves: the last version whose migration +/// changes the documents, on the way up or down. +/// +/// The versions from it to the current one read and write the same documents: chsr does not +/// rewrite a file for the current version alone, and a package of one of these versions reads +/// what a later one wrote, a downgrade among them included. A migration that changes the +/// documents, in either direction, raises it to its `to`. +pub const FORMAT_VERSION: Version = Version::new(4, 3, 0); + /// Why the current version does not read a policy document as it is. #[derive(Debug, Clone, PartialEq, Eq)] pub enum UnreadableVersion { @@ -662,15 +690,25 @@ pub fn read_as_is(version: Option<&str>) -> Result<(), UnreadableVersion> { .map_err(|_| UnreadableVersion::Invalid(text.escape_debug().to_string()))?; // the build metadata does not change the format version.build = semver::BuildMetadata::EMPTY; - if version < UNCHANGED_SINCE { + if reads_as_is(&version) { + Ok(()) + } else if version < UNCHANGED_SINCE { Err(UnreadableVersion::Older(version)) - } else if version > PACKAGE_VERSION { - Err(UnreadableVersion::Newer(version)) } else { - Ok(()) + Err(UnreadableVersion::Newer(version)) } } +/// Whether the current version reads a document of `version` as it is: from [`UNCHANGED_SINCE`] +/// to the current version. dosr reads it, and chsr does not rewrite it, as it is. +#[must_use] +pub fn reads_as_is(version: &Version) -> bool { + let mut version = version.clone(); + // the build metadata does not change the format + version.build = semver::BuildMetadata::EMPTY; + UNCHANGED_SINCE <= version && version <= PACKAGE_VERSION +} + #[cfg(test)] mod tests { use serde_json::json; @@ -714,7 +752,8 @@ mod tests { r#"/roles/0/tasks/0/options/env/delete/0: "XDG_.*_DIR" -> "XDG_*_DIR""#, ] ); - assert_eq!(document["version"], PACKAGE_VERSION.to_string()); + // written for the format, which the current version reads + assert_eq!(document["version"], FORMAT_VERSION.to_string()); assert_eq!(document["options"]["env"]["check"], json!(["LC_*", "TZ"])); assert_eq!(document["options"]["env"]["keep"], json!(["HOME"])); // values of the set list are not patterns @@ -1257,8 +1296,13 @@ mod tests { "options": {"env": {"default": "delete", "check": ["LC_*"]}, "path": {"default": "keep-safe"}}, "roles": [{"name": "r", "tasks": [{"name": "t", "commands": ["/usr/bin/ls ^.*$"]}]}] }); + // the current version reads it as it is: chsr does not rewrite it let mut document = original.clone(); - let migration = migrate_document(&mut document).unwrap().unwrap(); + assert_eq!(migrate_document(&mut document).unwrap(), None); + assert_eq!(document, original); + let migration = migrate_document_to(&mut document, &PACKAGE_VERSION) + .unwrap() + .unwrap(); assert_eq!(migration.from, Version::new(4, 1, 0)); assert_eq!(document["version"], PACKAGE_VERSION.to_string()); let mut unchanged = original.clone(); @@ -1287,8 +1331,13 @@ mod tests { "options": {"password": {"tries": 2}, "tty": {"owner": "target"}}, "roles": [{"name": "r", "tasks": [{"name": "t", "cred": {"setgid": {"from": "caller"}}, "commands": ["/usr/bin/ls ^a|b$"]}]}] }); + // the current version reads it as it is: chsr does not rewrite it let mut document = original.clone(); - let migration = migrate_document(&mut document).unwrap().unwrap(); + assert_eq!(migrate_document(&mut document).unwrap(), None); + assert_eq!(document, original); + let migration = migrate_document_to(&mut document, &PACKAGE_VERSION) + .unwrap() + .unwrap(); assert_eq!(migration.from, Version::new(4, 2, 0)); assert_eq!(document["version"], PACKAGE_VERSION.to_string()); let mut unchanged = original.clone(); @@ -1309,21 +1358,30 @@ mod tests { assert_eq!(document, original); } - // 4.3.1 only changes how dosr reads an older policy, not the format + // the versions from FORMAT_VERSION to the current one read and write the same documents: a + // document of one of them is not migrated, and goes from one to another unchanged #[test] - fn test_4_3_0_documents_up_and_down() { + fn test_format_version() { + assert!(UNCHANGED_SINCE <= FORMAT_VERSION && FORMAT_VERSION <= PACKAGE_VERSION); let original = json!({ - "version": "4.3.0", - "options": {"env": {"default": "delete", "keep": ["HOME"]}, "password": {"tries": 2}, "tty": {"owner": "target"}}, - "roles": [{"name": "r", "tasks": [{"name": "t", "cred": {"setgid": {"from": "caller"}}, "commands": ["/usr/bin/ls"]}]}] + "version": FORMAT_VERSION.to_string(), + "o": {"e": {"d": "delete", "k": ["HOME"]}}, + "options": {"password": {"tries": 2}, "tty": {"owner": "target"}, "path": {"default": "keep-safe"}}, + "roles": [{"name": "r", "tasks": [{"name": "t", "cred": {"setgid": {"from": "caller"}}, "commands": ["/usr/bin/ls ^a|b$", "/usr/bin/cat"]}]}] }); + for version in [FORMAT_VERSION, PACKAGE_VERSION] { + let mut document = original.clone(); + document["version"] = json!(version.to_string()); + let read = document.clone(); + assert_eq!(migrate_document(&mut document).unwrap(), None, "{version}"); + assert_eq!(document, read, "{version}"); + } let mut document = original.clone(); - let migration = migrate_document(&mut document).unwrap().unwrap(); - assert_eq!(migration.from, Version::new(4, 3, 0)); - assert!(migration.changes.is_empty(), "{:?}", migration.changes); - assert_eq!(document["version"], PACKAGE_VERSION.to_string()); - migrate_document_to(&mut document, &Version::new(4, 3, 0)).unwrap(); + migrate_document_to(&mut document, &PACKAGE_VERSION).unwrap(); + migrate_document_to(&mut document, &FORMAT_VERSION).unwrap(); assert_eq!(document, original); + // the data that chsr creates or saves is written for it + assert_eq!(Versioning::new(()).version, FORMAT_VERSION); } #[test] diff --git a/core/src/database/versionning.rs b/core/src/database/versionning.rs index c1c99c4f..708262d0 100644 --- a/core/src/database/versionning.rs +++ b/core/src/database/versionning.rs @@ -3,9 +3,9 @@ use semver::Version; use serde::{Deserialize, Serialize, Serializer, ser::SerializeMap}; use std::{cell::RefCell, fmt::Debug, rc::Rc}; -use crate::{ - PACKAGE_VERSION, - database::migration::{Migration, MigrationError}, +use crate::database::{ + migration::{Migration, MigrationError}, + migrations::{FORMAT_VERSION, reads_as_is}, }; #[derive(Deserialize, Debug, Clone)] @@ -62,7 +62,7 @@ impl Serialize for Versioning { impl Versioning { pub const fn new(data: T) -> Self { Self { - version: PACKAGE_VERSION, + version: FORMAT_VERSION, data, } } @@ -71,7 +71,7 @@ impl Versioning { impl Default for Versioning { fn default() -> Self { Self { - version: PACKAGE_VERSION, + version: FORMAT_VERSION, data: T::default(), } } @@ -81,8 +81,12 @@ impl Versioning { /// # Errors /// Returns an error if the migration process fails. pub fn upgrade_version(&mut self, migrations: &[Migration]) -> Result { + // read as it is: it keeps its version until it is saved, written for FORMAT_VERSION + if reads_as_is(&self.version) { + return Ok(false); + } let res = Migration::migrate(&self.version, &mut self.data, migrations)?; - self.version = PACKAGE_VERSION; + self.version = FORMAT_VERSION; Ok(res) } } diff --git a/core/src/file.rs b/core/src/file.rs index 70f1b8f9..4b7dae9f 100644 --- a/core/src/file.rs +++ b/core/src/file.rs @@ -21,7 +21,10 @@ use crate::{ PACKAGE_VERSION, SettingsContent, database::{ migration::{Migration, MigrationError}, - migrations::{DocumentMigration, migrate_document, migrate_document_to, writable_version}, + migrations::{ + DocumentMigration, FORMAT_VERSION, migrate_document, migrate_document_to, + writable_version, + }, ser::field, structs::SPolicy, versionning::{VersionedData, Versioning}, @@ -630,9 +633,15 @@ impl FileSettings { // // only the files that changed: the others are not rewritten (in the typed form of the // actors, without their immutable flag for a while...) + // + // a file that is written gets the version of the format, which its content follows; the + // others keep theirs let mut has_errors = if !self.root.changed() { false - } else if let Err(e) = self.root.save(RAR_CFG_TYPE, immutable) { + } else if let Err(e) = { + self.root.data.version = FORMAT_VERSION; + self.root.save(RAR_CFG_TYPE, immutable) + } { error!("Failed to save root settings: {e}"); true } else { @@ -641,6 +650,7 @@ impl FileSettings { }; for config in self.map.iter_mut().filter(|config| config.changed()) { + config.data.version = FORMAT_VERSION; if let Err(e) = config.save(self.root.data.data.storage.method, immutable) { error!( "Failed to save settings for {}: {}", @@ -713,8 +723,12 @@ impl FileSettings { ) -> FileDowngrade { let mut write = || -> Result> { let mut document = file.document(method)?; + // its version is known, the one of the loaded data: only already `to` let Some(migration) = migrate_document_to(&mut document, to)? else { - return Err(format!("its version is not known, or is already {to}").into()); + return Ok(DocumentMigration { + from: to.clone(), + changes: Vec::new(), + }); }; if !check { file.save_document(&document, method, immutable)?; @@ -728,6 +742,12 @@ impl FileSettings { } } + /// Whether a loaded file was migrated, and is to be saved by `save_migrated`. + #[must_use] + pub fn has_migration(&self) -> bool { + self.root.migration.is_some() || self.map.iter().any(|config| config.migration.is_some()) + } + /// The version of each loaded file, root settings first. #[must_use] pub fn get_versions(&self) -> Vec> { @@ -861,7 +881,8 @@ mod tests { ) .unwrap(); let policy = FileSettings::read_policy(&path, StorageMethod::JSON).unwrap(); - assert_eq!(policy.data.version, PACKAGE_VERSION); + // migrated for the format, which the current version reads + assert_eq!(policy.data.version, FORMAT_VERSION); let options = policy.data.data.as_ref().borrow().options.clone().unwrap(); let check = options .as_ref() @@ -1454,7 +1475,7 @@ mod tests { file.read_to_end(&mut content).unwrap(); let deserialized: Versioning>> = cbor4ii::serde::from_reader(&content[..]).unwrap(); - assert_eq!(deserialized.version, PACKAGE_VERSION); + assert_eq!(deserialized.version, FORMAT_VERSION); } #[test] diff --git a/src/chsr/cli/editor.rs b/src/chsr/cli/editor.rs index b861fdb3..3e21f73b 100644 --- a/src/chsr/cli/editor.rs +++ b/src/chsr/cli/editor.rs @@ -10,7 +10,9 @@ use log::{debug, warn}; use rootasrole_core::{ PACKAGE_VERSION, database::{ - migrations::{DocumentMigration, UnreadableVersion, migrate_document, read_as_is}, + migrations::{ + DocumentMigration, FORMAT_VERSION, UnreadableVersion, migrate_document, read_as_is, + }, structs::SPolicy, warn::Warn, }, @@ -326,7 +328,7 @@ where if let Some(migration) = migration { writeln!( output, - "Written for RootAsRole {}, migrated to {PACKAGE_VERSION}:", + "Written for RootAsRole {}, migrated to {FORMAT_VERSION}:", migration.from )?; // the values are JSON, and the keys escaped @@ -888,9 +890,11 @@ echo '{ "storage": { "method": "json" }, "unknown_config_field": "foo" }' > "$fi &serde_json::json!({"version": "4.0.3", "options": {"env": {"check": ["LC_.*"]}}}), b"y\n", ); + // written for the format, which this version reads + let format = rootasrole_core::database::migrations::FORMAT_VERSION.to_string(); assert!( output.contains(&format!( - "Written for RootAsRole 4.0.3, migrated to {version}:" + "Written for RootAsRole 4.0.3, migrated to {format}:" )), "{output}" ); @@ -899,7 +903,7 @@ echo '{ "storage": { "method": "json" }, "unknown_config_field": "foo" }' > "$fi "{output}" ); let saved: Value = serde_json::from_str(&saved).unwrap(); - assert_eq!(saved["version"], version.as_str()); + assert_eq!(saved["version"], format.as_str()); assert_eq!( saved["options"]["env"]["check"], serde_json::json!(["LC_*"]) diff --git a/src/chsr/cli/locality.rs b/src/chsr/cli/locality.rs index 70900368..9b6ec800 100644 --- a/src/chsr/cli/locality.rs +++ b/src/chsr/cli/locality.rs @@ -7,7 +7,9 @@ use std::{fs, os::unix::fs::PermissionsExt, path::PathBuf}; -use rootasrole_core::{PACKAGE_VERSION, file::FileSettings, util::StorageMethod}; +use rootasrole_core::{ + database::migrations::FORMAT_VERSION, file::FileSettings, util::StorageMethod, +}; use serde_json::{Value, json}; use super::main; @@ -112,7 +114,8 @@ fn generated_policy(rng: &mut Rng, path: &str) -> Value { }) .collect(); json!({ - "version": PACKAGE_VERSION.to_string(), + // as chsr writes it + "version": FORMAT_VERSION.to_string(), "storage": {"method": "json", "settings": {"path": path, "immutable": false}}, "options": options(rng), "roles": roles, diff --git a/src/chsr/cli/mod.rs b/src/chsr/cli/mod.rs index f4d4a398..a098bf08 100644 --- a/src/chsr/cli/mod.rs +++ b/src/chsr/cli/mod.rs @@ -47,6 +47,47 @@ where }) } +/// How `chsr upgrade` opens the policy: it writes nothing with `--check`, nor when it upgrades +/// for the current version and every file is up to date (see [`upgrade_access`]). +#[derive(Debug, PartialEq, Eq, Clone, Copy)] +pub enum UpgradeAccess { + /// `--check`: the files are only read. + Read, + /// For the current version: read first, opened for writing only if a file is to upgrade. + ReadFirst, + /// `--to` an older version: the files are written. + Write, +} + +/// How the command `args` opens the policy, when it is `chsr upgrade`: an upgrade of the package +/// that runs without `CAP_LINUX_IMMUTABLE`, through dosr, cannot open an immutable policy for +/// writing, and needs not when it is up to date. `None` for another command, or invalid +/// arguments, which `main` reports. +pub fn upgrade_access(args: I) -> Option +where + I: IntoIterator, + S: AsRef, +{ + let args = escape_parser_string_vec(args); + let pairs = Cli::parse(Rule::cli, &args).ok()?; + let mut inputs = Inputs::default(); + for pair in pairs { + recurse_pair(pair, &mut inputs).ok()?; + } + if inputs.action != InputAction::Upgrade { + return None; + } + // the same choice as process::upgrade between the current version and an older one + let current = rootasrole_core::PACKAGE_VERSION.to_string(); + Some(if inputs.upgrade_check { + UpgradeAccess::Read + } else if inputs.upgrade_to.as_ref().is_none_or(|to| *to == current) { + UpgradeAccess::ReadFirst + } else { + UpgradeAccess::Write + }) +} + #[builder] pub fn main( #[builder(start_fn)] storage: &mut FileSettings, @@ -2819,9 +2860,10 @@ mod tests { // upgrade saves the file itself, nothing is left for save_all assert!(!upgrade(&path, "upgrade").unwrap()); let saved = read_json(&path); + // written for the format, which this version reads assert_eq!( saved["version"], - rootasrole_core::PACKAGE_VERSION.to_string() + rootasrole_core::database::migrations::FORMAT_VERSION.to_string() ); assert_eq!( saved["options"]["env"]["check"], @@ -2835,6 +2877,71 @@ mod tests { assert_eq!(fs::metadata(&path).unwrap().modified().unwrap(), modified); } + // chsr upgrade writes nothing with --check, nor when the policy is up to date: an upgrade of + // the package through dosr, without CAP_LINUX_IMMUTABLE, cannot open an immutable policy for + // writing + #[test] + fn test_upgrade_access() { + let current = rootasrole_core::PACKAGE_VERSION.to_string(); + for (command, access) in [ + ("upgrade --check".to_string(), Some(UpgradeAccess::Read)), + ( + "upgrade --check --to 4.1.0".into(), + Some(UpgradeAccess::Read), + ), + ("upgrade".into(), Some(UpgradeAccess::ReadFirst)), + ( + format!("upgrade --to {current}"), + Some(UpgradeAccess::ReadFirst), + ), + ("upgrade --to 4.1.0".into(), Some(UpgradeAccess::Write)), + ("r r1 add".into(), None), + ("upgrade --unknown".into(), None), + ] { + assert_eq!(upgrade_access(command.split(' ')), access, "{command}"); + } + } + + #[test] + fn test_upgrade_reads_an_up_to_date_policy_only() { + let path = format!("{RAR_CFG_PATH}.upgrade_read"); + let _cleanup = defer(|| { + let _ = fs::remove_file(format!("{RAR_CFG_PATH}.upgrade_read")); + }); + let upgrade_read = |command: &str| { + let mut settings = + FileSettings::read_all(path.as_str(), path.as_str(), StorageMethod::JSON).unwrap(); + let has_migration = settings.has_migration(); + ( + has_migration, + main(&mut settings, command.split(' ')).call(), + ) + }; + // read as it is: 4.1.0, and not writable, like an immutable file without + // CAP_LINUX_IMMUTABLE + fs::write( + &path, + format!( + r#"{{"version":"4.1.0","storage":{{"method":"json","settings":{{"path":"{path}","immutable":false}}}},"roles":[]}}"# + ), + ) + .unwrap(); + fs::set_permissions(&path, std::os::unix::fs::PermissionsExt::from_mode(0o400)).unwrap(); + let before = fs::read_to_string(&path).unwrap(); + for command in ["upgrade --check", "upgrade"] { + let (has_migration, result) = upgrade_read(command); + assert!(!has_migration, "{command}"); + assert!(!result.unwrap(), "{command}"); + } + assert_eq!(fs::read_to_string(&path).unwrap(), before); + // an older one is to upgrade: --check tells it, chsr opens it for writing + fs::set_permissions(&path, std::os::unix::fs::PermissionsExt::from_mode(0o600)).unwrap(); + fs::write(&path, before.replace("4.1.0", "4.0.3")).unwrap(); + let (has_migration, result) = upgrade_read("upgrade --check"); + assert!(has_migration); + assert!(result.is_err()); + } + #[test] fn test_upgrade_policy_directory() { let path = format!("{RAR_CFG_PATH}.upgrade_dir"); @@ -2873,7 +2980,10 @@ mod tests { // the file that cannot be loaded is reported, the other one is upgraded assert!(upgrade(&path, "upgrade").is_err()); let saved = read_json(&policy); - assert_eq!(saved["version"], version.to_string()); + assert_eq!( + saved["version"], + rootasrole_core::database::migrations::FORMAT_VERSION.to_string() + ); assert_eq!(saved["options"]["env"]["keep"], serde_json::json!(["HOME"])); assert_eq!( read_json(&invalid)["options"]["env"]["keep"], @@ -2931,14 +3041,17 @@ mod tests { task["commands"], serde_json::json!(["/usr/bin/systemctl ^(?:start|stop)$"]) ); - // this version reads it again, and upgrades it: what 4.1.0 does not know is lost - assert!(upgrade(&path, "upgrade --check").is_err()); + // this version reads it as it is: chsr upgrade has nothing to write, the file keeps + // its version, and what 4.1.0 does not know is lost + let modified = fs::metadata(&path).unwrap().modified().unwrap(); + assert!(!upgrade(&path, "upgrade --check").unwrap()); assert!(!upgrade(&path, "upgrade").unwrap()); - assert_eq!(read_json(&path)["version"], version); + assert_eq!(fs::metadata(&path).unwrap().modified().unwrap(), modified); + assert_eq!(read_json(&path)["version"], "4.1.0"); assert_eq!(read_json(&path)["options"], saved["options"]); - // below 4.1.0, the words change: dosr of this version refuses the policy until the - // older version is installed + // below 4.1.0, the words change: dosr of this version reads the policy only through + // its migrations until the older version is installed assert!(!upgrade(&path, "upgrade --to 4.0.3").unwrap()); let saved = read_json(&path); assert_eq!(saved["version"], "4.0.3"); diff --git a/src/chsr/cli/process/upgrade.rs b/src/chsr/cli/process/upgrade.rs index de05973a..d7c0c3be 100644 --- a/src/chsr/cli/process/upgrade.rs +++ b/src/chsr/cli/process/upgrade.rs @@ -2,7 +2,11 @@ use rootasrole_core::{eprintln_ignore_io_error, println_ignore_io_error}; use std::error::Error; use log::debug; -use rootasrole_core::{PACKAGE_VERSION, database::migrations::read_as_is, file::FileSettings}; +use rootasrole_core::{ + PACKAGE_VERSION, + database::migrations::{FORMAT_VERSION, read_as_is, reads_as_is}, + file::FileSettings, +}; /// Escape control characters: paths, entries and errors come from the policy files. fn printable(text: &str) -> String { @@ -54,12 +58,12 @@ fn upgrade_to_current(settings: &mut FileSettings, check: bool) -> Result Result Result { - report.push(if check { + report.push(if migration.from.to_string() == to { + format!("{path}: already written for {to}") + } else if check { format!("{path}: {}, can be written for {to}", migration.from) } else { format!("{path}: written for {to}, from {}", migration.from) diff --git a/src/chsr/main.rs b/src/chsr/main.rs index 0bd01b25..b136008d 100644 --- a/src/chsr/main.rs +++ b/src/chsr/main.rs @@ -49,7 +49,7 @@ fn main() -> Result> { use std::{path::Path, process::ExitCode}; use crate::{ - cli::editor::defer, + cli::{UpgradeAccess, editor::defer}, util::{CHSR_RUN_DIR, RAR_CFG_DATA_PATH, RAR_CFG_PATH, secure_dir}, }; use ::landlock::{RestrictionStatus, RulesetStatus}; @@ -106,8 +106,22 @@ fn main() -> Result> { let _ = std::fs::remove_dir_all(&folder); }); - let mut settings = match FileSettings::write_all(RAR_CFG_PATH, RAR_CFG_DATA_PATH, RAR_CFG_TYPE) - { + let write = || FileSettings::write_all(RAR_CFG_PATH, RAR_CFG_DATA_PATH, RAR_CFG_TYPE); + let read = || FileSettings::read_all(RAR_CFG_PATH, RAR_CFG_DATA_PATH, RAR_CFG_TYPE); + let settings = match cli::upgrade_access(&args) { + Some(UpgradeAccess::Read) => read(), + // the read files are unlocked before they are opened again for writing + Some(UpgradeAccess::ReadFirst) => read().and_then(|settings| { + if settings.has_migration() { + drop(settings); + write() + } else { + Ok(settings) + } + }), + Some(UpgradeAccess::Write) | None => write(), + }; + let mut settings = match settings { Ok(settings) => settings, Err(e) => return Ok(failure(&format!("cannot read the policy: {e}"))), }; diff --git a/tests/vagrant/e2e.sh b/tests/vagrant/e2e.sh index d29910cf..d9b7d8a9 100755 --- a/tests/vagrant/e2e.sh +++ b/tests/vagrant/e2e.sh @@ -331,6 +331,25 @@ old_policy_is_read_as_upgraded() { [ "$out" = 0 ] && [ "$logged" -eq 0 ] && ! grep -qE 'version|4\.0\.3|journalctl' <<<"$terminal" } +# chsr upgrade only reads the policy when it writes nothing: without CAP_LINUX_IMMUTABLE, like an +# upgrade of the package through dosr, it checks and upgrades the up to date immutable policy, and +# only fails when a file is to upgrade +upgrade_without_cap_linux_immutable() { + immutable_policy || return 77 + local without=(capsh --drop=cap_linux_immutable --) check upgrade old_check old_upgrade + "${without[@]}" -c 'chsr upgrade --check' >/dev/null 2>&1 + check=$? + "${without[@]}" -c 'chsr upgrade' >/dev/null 2>&1 + upgrade=$? + write_policy "$PLANTED_POLICY" "$(planted_policy 4.0.3)" + "${without[@]}" -c 'chsr upgrade --check' >/dev/null 2>&1 + old_check=$? + "${without[@]}" -c 'chsr upgrade' >/dev/null 2>&1 + old_upgrade=$? + remove_policy "$PLANTED_POLICY" + [ "$check" -eq 0 ] && [ "$upgrade" -eq 0 ] && [ "$old_check" -ne 0 ] && [ "$old_upgrade" -ne 0 ] +} + # dosr ends like the command: killed by SIGTERM, the shell sees 143 signal_is_forwarded() { as_user dosr -r e2e -t shell /usr/bin/sh -c 'kill -TERM $$' @@ -1837,6 +1856,7 @@ check "the policy, its directory included, is immutable" policy_is_immutable check "a policy file without the immutable flag is refused, the user told where the reason is" planted_policy_is_refused check "a policy file without a version is refused, the user told where the reason is" policy_without_version_is_refused check "a policy file of an older version is read as chsr upgrade writes it, with a warning in the log" old_policy_is_read_as_upgraded +check "chsr upgrade without CAP_LINUX_IMMUTABLE succeeds on an up to date policy, fails on a file to upgrade" upgrade_without_cap_linux_immutable check "dosr ends like a command killed by a signal" signal_is_forwarded check "the command gets the umask of the policy, else the build-time one" umask_of_the_policy check "a regular expression without quotes" unquoted_regex From cd76aeb2dc422028f9adbde8ffd8d07f30b7bb3f Mon Sep 17 00:00:00 2001 From: LeChatP Date: Sun, 8 Nov 2026 11:43:53 +0100 Subject: [PATCH 2/2] docs: update behavior explainations --- book/src/cli/chsr.md | 4 +--- book/src/cli/dosr.md | 2 +- book/src/configuration.md | 3 ++- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/book/src/cli/chsr.md b/book/src/cli/chsr.md index 316f253f..26e59083 100644 --- a/book/src/cli/chsr.md +++ b/book/src/cli/chsr.md @@ -290,7 +290,7 @@ Read the resulting configuration before you answer yes: what chsr s ## `chsr upgrade` -A new version of RootAsRole may change how the policy is written. After an upgrade, `chsr upgrade` rewrites the policy files in the format of the installed version. Packages run it for you during their updates. +A new version of RootAsRole may change how the policy is written. After an upgrade, `chsr upgrade` might do necessary migrations. ```bash chsr upgrade --check @@ -304,8 +304,6 @@ Going back to an older version needs the same care, the other way. Before instal chsr upgrade --to 4.1.0 ``` -`chsr` writes every policy file for that version, and changes what that version would read with another meaning. With `--check`, it writes nothing. For a version older than 4.1.0, the installed `dosr` refuses the files written this way until the older version is installed. - ## File Format Conversion `chsr convert` converts the policy between JSON and CBOR. JSON is easy to read and to review. CBOR is a binary format, much faster to load: when your policy is ready, you can convert it. In CBOR, `chsr` writes each key of the policy with one character, which makes the file smaller. `dosr` and `chsr` read both the short and the long keys, in both formats, but `dosr` 4.2.0 does not: before going back to it with a CBOR policy, run `chsr upgrade --to 4.2.0`. diff --git a/book/src/cli/dosr.md b/book/src/cli/dosr.md index 65e2a7a5..25147a4a 100644 --- a/book/src/cli/dosr.md +++ b/book/src/cli/dosr.md @@ -197,7 +197,7 @@ Today, the CLI of `dosr` follows the one of sudo, to keep old scripts working. W `Permission denied` --> Two equivalent tasks match: the policy must be fixed. The log has the details. -`Configuration error` --> The policy was refused: for example a file written by hand without its `version`, or written by a version older than 4.1.0 that `chsr upgrade` did not migrate. `dosr` never shows on the terminal what is wrong in the policy, but it says where to read it: ``dosr: see the reason with `journalctl -t dosr -p info`, or ask your system administrator``. The same line follows any error when a file of the policy was refused, like a file of a policy directory that other users can write. The command comes from the build of `dosr` (`RAR_LOG_COMMAND`, see [Build-time default configuration](../install/build.md)). +`Configuration error` --> The policy was refused: for example a file written by hand without its `version`. `dosr` never shows on the terminal what is wrong in the policy, but it says where to read it: ``dosr: see the reason with `journalctl -t dosr -p info`, or ask your system administrator``. The same line follows any error when a file of the policy was refused, like a file of a policy directory that other users can write. The command comes from the build of `dosr` (`RAR_LOG_COMMAND`, see [Build-time default configuration](../install/build.md)). `dosr: the task needs capabilities that this session cannot obtain anymore: …` --> The task needs capabilities that are out of the bounding set of your session, for example in a container. diff --git a/book/src/configuration.md b/book/src/configuration.md index 367e873d..036ce691 100644 --- a/book/src/configuration.md +++ b/book/src/configuration.md @@ -569,7 +569,8 @@ For `sys-admin`, the most critical role, the team requires authentication on eve ### Migrating configurations from version 4.0 -Upgrading from version 4.0 requires running `chsr upgrade` as root. Package maintenance scripts run it automatically during updates. +Upgrading from breaking changes version requires running `chsr upgrade`. +Package maintenance scripts run it automatically during updates, but if you ran the package upgrade with `dosr`, you might not have `CAP_LINUX_IMMUTABLE`, blocking the file modification. In that case, run `dosr chsr upgrade` manually to migrate the configuration. We'll try to avoid breaking changes in the future, but security-default changes are sometimes unavoidable. This utility addresses differences in pattern syntax and list inheritance: