diff --git a/docs/ACCEPTANCE_CHECKLIST.md b/docs/ACCEPTANCE_CHECKLIST.md index a114fcc..e43c459 100644 --- a/docs/ACCEPTANCE_CHECKLIST.md +++ b/docs/ACCEPTANCE_CHECKLIST.md @@ -110,7 +110,10 @@ - [ ] Если резервную копию записать не удалось, `/etc/fstab` остаётся неизменным. - [ ] `status` при нечитаемом `/etc/fstab` показывает ошибку, а не «нет swap-записей». - [ ] Путь swap с пробелом в имени попадает в `/etc/fstab` экранированным (`\040`) и корректно монтируется. -- [ ] Рядом с управляемым swapfile создан маркер `.awg2-harness-owned`. +- [ ] Создан `/var/lib/awg2-harness/swap-ownership.conf` с `device`, `inode` и путём; каталог `/var/lib/awg2-harness` имеет режим `0700` и принадлежит root. +- [ ] Старый sidecar `.awg2-harness-owned` после обновления удалён и больше не создаётся. +- [ ] Если swapfile удалить вручную и создать по тому же пути другой файл, `ensure-swap` завершается ошибкой `StaleOwnership` и **не** удаляет новый файл. +- [ ] `swap.path` вне `/swapfile` и `/var/lib/awg2-harness/` отвергается на `validate-config` (проверить `/tmp/awg.swap`). - [ ] Если до запуска был активен другой swap, после `ensure-swap` он выключен. - [ ] После `ensure-swap` активен ровно один swap: `/swapfile`. - [ ] Все старые swap entries удалены из `/etc/fstab`. diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 83bb940..b299ef4 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -198,9 +198,11 @@ swappiness = 1 `swap.path`, `swap.size_mb`, `swap.priority`, `swap.swappiness` — параметры managed swap. При `swap.enabled=true` пакет приводит swap-state к exclusive состоянию: активен ровно один swap (`swap.path`), остальные выключаются и swap-entries в `/etc/fstab` пересобираются в одну managed строку. Рекомендуемый low priority — `1`, `swappiness = 1`. -Ограничения `swap.path`: абсолютный, без компонентов `.` и `..`, без пробелов, управляющих символов и обратных слэшей, не из списка системных путей (`/`, `/etc`, `/etc/fstab`, `/etc/passwd`, `/etc/shadow`, `/dev`, `/proc`, `/sys`, `/run`, `/boot`) и не под `/proc`, `/sys`, `/dev`. Дополнительно на этапе применения проверяется, что ни один родительский каталог не является symlink. +Ограничения `swap.path`: путь должен быть либо `/swapfile`, либо файлом непосредственно в `/var/lib/awg2-harness/`. Дополнительно: абсолютный, без компонентов `.` и `..`, без пробелов, управляющих символов и обратных слэшей; на этапе применения проверяется, что ни один родительский каталог не является symlink. -Пакет создаёт рядом с swapfile маркер владения `.awg2-harness-owned`. Файл без этого маркера считается чужим: он никогда не пересоздаётся и не удаляется, вместо этого возвращается ошибка. Чужие активные swap только выключаются — их файлы пакет не удаляет. +Ограничение корней обязательно, а не косметика. Если родительский каталог доступен на запись кому-то кроме root, между проверкой пути и запуском `fallocate`/`mkswap` этот пользователь успевает подменить файл на symlink и перенаправить операцию, выполняемую от root. Оба разрешённых корня принадлежат root, поэтому окна не возникает. `/swapfile` оставлен для совместимости с уже развёрнутыми конфигурациями. + +Владение фиксируется в `/var/lib/awg2-harness/swap-ownership.conf` по `device` + `inode` + пути, а не по маркеру рядом с файлом. Файл, которого нет в этой записи, или файл с тем же путём, но другими `device`/`inode` (то есть подменённый), считается чужим: он никогда не пересоздаётся и не удаляется, вместо этого возвращается ошибка. Чужие активные swap только выключаются — их файлы пакет не удаляет. ## Что не выносить в конфиг diff --git a/docs/IMPLEMENTATION_SPEC.md b/docs/IMPLEMENTATION_SPEC.md index 5760f17..2f420ea 100644 --- a/docs/IMPLEMENTATION_SPEC.md +++ b/docs/IMPLEMENTATION_SPEC.md @@ -593,14 +593,27 @@ Production-путь перезагрузки — внешний бинарь/с ### Владение swapfile -Пакет никогда не трогает файл, который не создавал. Рядом с управляемым swapfile лежит sidecar-маркер `.awg2-harness-owned` с первой строкой `# Managed by awg2-harness swap`. Без него отличить наш файл от созданного администратором невозможно, и правило «размер не совпал — пересоздать» превратилось бы в «молча уничтожить чужой файл». +Пакет никогда не трогает файл, который не создавал. Состояние владения хранится в `/var/lib/awg2-harness/swap-ownership.conf` — каталог доступен только root (`0700`): + +```text +# Managed by awg2-harness swap-ownership +version = 1 +path = /swapfile +device = 2049 +inode = 123456 +``` + +Ключевое здесь — `device` и `inode`, а не путь. Путь не является идентичностью: файл можно удалить, а на его месте создать другой. Прежняя схема с sidecar-маркером рядом со swapfile этого не различала, и последовательность «наш swapfile удалили вручную → маркер остался → оператор создал по тому же пути другой файл» приводила к тому, что чужой файл признавался управляемым и при несовпадении размера удалялся. Sidecar удалён; при обнаружении он только вычищается и никогда не используется как доказательство владения. | Состояние | Условие | Действие | | --- | --- | --- | | `Missing` | файла нет | создать | -| `ManagedValid` | маркер есть, размер совпадает | переиспользовать | -| `ManagedWrongSize` | маркер есть, размер отличается | пересоздать | -| `Foreign` | файл есть, маркера нет | **ошибка**, файл не трогается | +| `ManagedValid` | запись есть, `device`+`inode`+путь совпали, размер совпал | переиспользовать | +| `ManagedWrongSize` | запись совпала, размер отличается | пересоздать | +| `Foreign` | файл есть, записи нет или запись про другой путь | **ошибка**, файл не трогается | +| `StaleOwnership` | запись есть, но `device`/`inode` не совпали — файл подменили | **ошибка**, файл не трогается | + +Запись создаётся **после** появления файла, по его фактическим metadata. До этого момента файл управляемым не считается. Чужие активные swap только выключаются через `swapoff`. Файлы не удаляются — ни обычные, ни блочные устройства. Удаление чужого swapfile не является решением этого пакета: эксклюзивное runtime-состояние достигается связкой `swapoff` плюс вычистка `/etc/fstab`, а файл оператор уберёт сам, когда сочтёт нужным. @@ -629,7 +642,11 @@ Production-путь перезагрузки — внешний бинарь/с - путь абсолютный и не оканчивается на `/`; - нет компонентов `.` и `..` — поэтому строка уже нормализована и список запрещённых путей нельзя обойти записью вида `/etc/../etc/fstab`; - нет пробелов, управляющих символов и обратных слэшей; -- путь не входит в список запрещённых (`/`, `/etc`, `/etc/fstab`, `/etc/passwd`, `/etc/shadow`, `/dev`, `/proc`, `/sys`, `/run`, `/boot`) и не лежит под `/proc`, `/sys`, `/dev`. +- путь равен `/swapfile` **либо** лежит непосредственно в `/var/lib/awg2-harness/` (без вложенных каталогов). + +Это allow-list, а не deny-list, и это принципиально. Список запрещённых путей закрывает только те случаи, которые кто-то придумал; значение имеет другое — может ли родительский каталог писать кто-то кроме root. Между preflight-проверкой и запуском `fallocate`/`dd`/`mkswap` непривилегированный пользователь с правом записи в родительский каталог успевает подменить финальный компонент на symlink и перенаправить операцию, выполняемую от root. Это классический TOCTOU, и никакое количество `stat` перед использованием его не закрывает — окно устраняется только тем, что запись в родительский каталог доступна лишь root. + +Оба разрешённых корня root-owned. `/swapfile` оставлен, чтобы уже развёрнутые конфигурации не требовали правки. Файловая часть выполняется непосредственно перед использованием пути: каждый родительский компонент обязан существовать и не быть symlink. Проверки только финального компонента недостаточно — symlink в середине цепочки перенаправляет операцию на файл, который никогда не настраивался. diff --git a/traffic_core/src/swap.rs b/traffic_core/src/swap.rs index 33a99bc..1eca4cb 100644 --- a/traffic_core/src/swap.rs +++ b/traffic_core/src/swap.rs @@ -47,24 +47,57 @@ pub struct SwapStatus { pub drift_reason: Option, } -/// Sidecar that marks a swapfile as created and owned by this package. +/// Root-only directory holding this package's persistent state. +pub const STATE_DIR: &str = "/var/lib/awg2-harness"; +const STATE_DIR_PREFIX: &str = "/var/lib/awg2-harness/"; +/// Historical location, kept so existing deployments do not need a config change. +const DEFAULT_SWAP_PATH: &str = "/swapfile"; +/// Records which swapfile we created, by identity rather than by location. +const SWAP_OWNERSHIP_PATH: &str = "/var/lib/awg2-harness/swap-ownership.conf"; +const SWAP_OWNERSHIP_MARKER: &str = "# Managed by awg2-harness swap-ownership"; +const SWAP_OWNERSHIP_VERSION: u32 = 1; +/// Sidecar written by earlier versions next to the swapfile. Only ever removed. +const LEGACY_OWNERSHIP_SUFFIX: &str = ".awg2-harness-owned"; + +/// Identity of a file on disk, independent of its name. /// -/// Without it there is no way to tell our swapfile from one an administrator made, and -/// "recreate because the size differs" would become "silently destroy someone else's -/// file". A file with no sidecar is never written to or deleted. -const OWNERSHIP_SUFFIX: &str = ".awg2-harness-owned"; -const OWNERSHIP_MARKER: &str = "# Managed by awg2-harness swap\n"; +/// Path alone is not identity: a file can be deleted and a different one created at the +/// same path. `(device, inode)` is what actually distinguishes the swapfile we made from +/// whatever now occupies its path. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct FileIdentity { + device: u64, + inode: u64, +} + +/// What we recorded when we created the managed swapfile. +#[derive(Debug, Clone, PartialEq, Eq)] +struct SwapOwnershipRecord { + version: u32, + path: PathBuf, + identity: FileIdentity, +} #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum SwapfileOwnership { /// Nothing at the path: safe to create. Missing, - /// Ours, and already the requested size. + /// Ours, confirmed by identity, and already the requested size. ManagedValid, - /// Ours, but the size differs: safe to recreate. + /// Ours, confirmed by identity, but the size differs: safe to recreate. ManagedWrongSize, - /// Exists without our marker. Never touched. + /// A file exists and we have no record of creating it. Never touched. Foreign, + /// We have a record, but the file at the path is not the one we created - it was + /// replaced behind our back. Refused exactly like `Foreign`; kept separate so the + /// error can say which situation the operator is in. + StaleOwnership, +} + +impl SwapfileOwnership { + fn is_managed(self) -> bool { + matches!(self, Self::ManagedValid | Self::ManagedWrongSize) + } } /// Persisted `vm.swappiness`, owned by the swap module. @@ -186,27 +219,27 @@ pub fn validate_swap_path_syntax(raw: &str) -> anyhow::Result<()> { anyhow::bail!("swap.path must not contain '.' or '..' components: {raw}"); } } - // Checked after rejecting '.' and '..', so the string is already normalized and an - // exact comparison cannot be bypassed by a path like /etc/../etc/fstab. - const FORBIDDEN: &[&str] = &[ - "/", - "/etc", - "/etc/fstab", - "/etc/passwd", - "/etc/shadow", - "/dev", - "/proc", - "/sys", - "/run", - "/boot", - ]; - if FORBIDDEN.contains(&raw) { - anyhow::bail!("swap.path is dangerous and forbidden: {raw}"); + // Allow-list, not deny-list. A deny-list only rules out the paths someone thought of; + // what actually matters is whether the parent directory can be written by anyone but + // root. Between the preflight check and `fallocate`/`dd`/`mkswap` running, a + // world-writable parent lets an unprivileged user swap the final component for a + // symlink and redirect a root-owned operation - a TOCTOU that no amount of stat-ing + // before use can close. Restricting the location to directories only root can write + // removes the window instead of trying to detect it. + if raw == DEFAULT_SWAP_PATH { + return Ok(()); } - if raw.starts_with("/proc/") || raw.starts_with("/sys/") || raw.starts_with("/dev/") { - anyhow::bail!("swap.path must not live under /proc, /sys or /dev: {raw}"); + if let Some(rest) = raw.strip_prefix(STATE_DIR_PREFIX) + && !rest.is_empty() + && !rest.contains('/') + { + return Ok(()); } - Ok(()) + anyhow::bail!( + "swap.path must be {DEFAULT_SWAP_PATH} or a file directly under {STATE_DIR}; got {raw}. \ + Both are root-owned locations; an arbitrary path can have a parent directory that \ + another user may write, which would let them redirect the swapfile operations" + ); } /// Filesystem-level checks performed just before the path is used. @@ -249,43 +282,201 @@ fn assert_path_chain_is_safe(path: &Path) -> anyhow::Result<()> { Ok(()) } -fn ownership_marker_path(path: &Path) -> anyhow::Result { - let name = path - .file_name() - .ok_or_else(|| anyhow::anyhow!("invalid swap.path: {}", path.display()))?; - let mut marker = name.to_os_string(); - marker.push(OWNERSHIP_SUFFIX); - Ok(path.with_file_name(marker)) +fn ownership_file() -> ManagedFile { + ManagedFile::new(SWAP_OWNERSHIP_PATH, SWAP_OWNERSHIP_MARKER) +} + +fn render_ownership_record(rec: &SwapOwnershipRecord) -> String { + format!( + "{SWAP_OWNERSHIP_MARKER}\nversion = {}\npath = {}\ndevice = {}\ninode = {}\n", + rec.version, + rec.path.display(), + rec.identity.device, + rec.identity.inode + ) +} + +fn parse_ownership_record(raw: &str) -> anyhow::Result { + let mut version = None; + let mut path = None; + let mut device = None; + let mut inode = None; + for line in raw.lines() { + let t = line.trim(); + if t.is_empty() || t.starts_with('#') { + continue; + } + let Some((k, v)) = t.split_once('=') else { + continue; + }; + let v = v.trim(); + match k.trim() { + "version" => version = v.parse::().ok(), + "path" => path = Some(PathBuf::from(v)), + "device" => device = v.parse::().ok(), + "inode" => inode = v.parse::().ok(), + _ => {} + } + } + let (Some(version), Some(path), Some(device), Some(inode)) = (version, path, device, inode) + else { + anyhow::bail!("swap ownership record is incomplete or malformed"); + }; + if version != SWAP_OWNERSHIP_VERSION { + anyhow::bail!("unsupported swap ownership record version: {version}"); + } + Ok(SwapOwnershipRecord { + version, + path, + identity: FileIdentity { device, inode }, + }) +} + +/// Decides ownership from a recorded state and the file actually on disk. +/// +/// Pure on purpose: the interesting cases - a stale record pointing at a file someone +/// else created, a record for a different path - are the ones that must never authorise a +/// delete, and keeping the decision free of filesystem access makes them directly +/// testable. +fn classify_ownership( + recorded: Option<&SwapOwnershipRecord>, + actual_path: &Path, + actual: Option<(FileIdentity, u64)>, + desired_size: u64, +) -> SwapfileOwnership { + let Some((identity, size)) = actual else { + return SwapfileOwnership::Missing; + }; + let Some(rec) = recorded else { + return SwapfileOwnership::Foreign; + }; + if rec.path != actual_path { + // The record describes a different managed path; this file is not ours. + return SwapfileOwnership::Foreign; + } + if rec.identity != identity { + // Right path, wrong file: ours was replaced. Never a licence to delete. + return SwapfileOwnership::StaleOwnership; + } + if size == desired_size { + SwapfileOwnership::ManagedValid + } else { + SwapfileOwnership::ManagedWrongSize + } +} + +#[cfg(unix)] +fn identity_of(md: &fs::Metadata) -> FileIdentity { + use std::os::unix::fs::MetadataExt; + FileIdentity { + device: md.dev(), + inode: md.ino(), + } +} + +/// Windows has no stable dev/inode pair through `Metadata`; this target exists only so the +/// pure logic above stays testable off Linux. +#[cfg(not(unix))] +fn identity_of(_md: &fs::Metadata) -> FileIdentity { + FileIdentity { + device: 0, + inode: 0, + } +} + +fn read_ownership_record() -> anyhow::Result> { + let Some(raw) = ownership_file().read()? else { + return Ok(None); + }; + parse_ownership_record(&raw).map(Some) } fn inspect_swapfile_ownership(path: &Path, desired_size: u64) -> anyhow::Result { - let md = match fs::symlink_metadata(path) { - Ok(v) => v, - Err(e) if e.kind() == std::io::ErrorKind::NotFound => { - return Ok(SwapfileOwnership::Missing); + let actual = match fs::symlink_metadata(path) { + Ok(md) => { + if md.file_type().is_symlink() { + anyhow::bail!("swap.path is a symlink: {}", path.display()); + } + if !md.file_type().is_file() { + anyhow::bail!("swap.path is not a regular file: {}", path.display()); + } + Some((identity_of(&md), md.len())) } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => None, Err(e) => anyhow::bail!("failed to stat {}: {e}", path.display()), }; - if md.file_type().is_symlink() { - anyhow::bail!("swap.path is a symlink: {}", path.display()); - } - if !md.file_type().is_file() { - anyhow::bail!("swap.path is not a regular file: {}", path.display()); - } - let marker = ownership_marker_path(path)?; - let owned = match fs::read_to_string(&marker) { - Ok(content) => content.starts_with(OWNERSHIP_MARKER.trim_end()), - Err(e) if e.kind() == std::io::ErrorKind::NotFound => false, - Err(e) => anyhow::bail!("failed to read {}: {e}", marker.display()), + let recorded = read_ownership_record()?; + Ok(classify_ownership( + recorded.as_ref(), + path, + actual, + desired_size, + )) +} + +/// Records the identity of the swapfile we just created. +/// +/// Written after the file exists, so the recorded `(device, inode)` describes the real +/// file. Until this succeeds the swapfile is not considered managed. +fn record_ownership(path: &Path) -> anyhow::Result<()> { + ensure_state_dir()?; + let md = fs::metadata(path) + .map_err(|e| anyhow::anyhow!("failed to stat {} after create: {e}", path.display()))?; + let rec = SwapOwnershipRecord { + version: SWAP_OWNERSHIP_VERSION, + path: path.to_path_buf(), + identity: identity_of(&md), }; - if !owned { - return Ok(SwapfileOwnership::Foreign); + ownership_file().write(&render_ownership_record(&rec))?; + Ok(()) +} + +fn ensure_state_dir() -> anyhow::Result<()> { + let dir = Path::new(STATE_DIR); + match fs::symlink_metadata(dir) { + Ok(md) if md.file_type().is_dir() => return Ok(()), + Ok(_) => anyhow::bail!("{STATE_DIR} exists and is not a directory"), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => anyhow::bail!("failed to stat {STATE_DIR}: {e}"), } - if md.len() == desired_size { - Ok(SwapfileOwnership::ManagedValid) - } else { - Ok(SwapfileOwnership::ManagedWrongSize) + fs::create_dir_all(dir).map_err(|e| anyhow::anyhow!("failed to create {STATE_DIR}: {e}"))?; + set_dir_mode(dir) +} + +#[cfg(unix)] +fn set_dir_mode(dir: &Path) -> anyhow::Result<()> { + fs::set_permissions(dir, fs::Permissions::from_mode(0o700)) + .map_err(|e| anyhow::anyhow!("failed to chmod 0700 {}: {e}", dir.display())) +} + +#[cfg(not(unix))] +fn set_dir_mode(_dir: &Path) -> anyhow::Result<()> { + Ok(()) +} + +/// Removes the sidecar written next to the swapfile by earlier versions. +/// +/// Only removed, never trusted: ownership now lives in the state directory. A sidecar at +/// a path we do not own is left alone. +fn drop_legacy_ownership_sidecar(swap_path: &Path) { + let Some(name) = swap_path.file_name() else { + return; + }; + let mut sidecar = name.to_os_string(); + sidecar.push(LEGACY_OWNERSHIP_SUFFIX); + let sidecar = swap_path.with_file_name(sidecar); + match fs::symlink_metadata(&sidecar) { + Ok(md) if md.file_type().is_file() => { + if let Err(e) = fs::remove_file(&sidecar) { + tracing::warn!( + target: "swap", + "could not remove legacy ownership sidecar {}: {e}", + sidecar.display() + ); + } + } + _ => {} } } @@ -442,12 +633,19 @@ fn ensure_desired_swapfile(desired: &DesiredSwap, active: &[ActiveSwap]) -> anyh let p = &desired.path; let ownership = inspect_swapfile_ownership(p, desired.size_bytes)?; - if ownership == SwapfileOwnership::Foreign { - anyhow::bail!( - "{} already exists and was not created by awg2-harness; \ - refusing to overwrite it. Remove it manually or point swap.path elsewhere", + match ownership { + SwapfileOwnership::Foreign => anyhow::bail!( + "{} already exists and there is no record of awg2-harness creating it; \ + refusing to touch it. Remove it manually or point swap.path elsewhere", p.display() - ); + ), + SwapfileOwnership::StaleOwnership => anyhow::bail!( + "{} is not the file awg2-harness created - it was replaced since. Refusing to \ + touch it. Remove it manually, or delete {SWAP_OWNERSHIP_PATH} to let \ + awg2-harness adopt a freshly created swapfile", + p.display() + ), + _ => {} } let desired_active = active.iter().find(|s| s.name == *p); @@ -459,8 +657,8 @@ fn ensure_desired_swapfile(desired: &DesiredSwap, active: &[ActiveSwap]) -> anyh .unwrap_or(false); if ownership == SwapfileOwnership::ManagedValid && (desired_active.is_none() || geometry_ok) { - write_ownership_marker(p)?; set_swapfile_mode(p)?; + drop_legacy_ownership_sidecar(p); return Ok(()); } @@ -468,7 +666,7 @@ fn ensure_desired_swapfile(desired: &DesiredSwap, active: &[ActiveSwap]) -> anyh run("swapoff", &[p.to_string_lossy().as_ref()])?; } - if ownership != SwapfileOwnership::Missing { + if ownership.is_managed() { fs::remove_file(p).map_err(|e| anyhow::anyhow!("failed to remove {}: {e}", p.display()))?; } @@ -478,16 +676,11 @@ fn ensure_desired_swapfile(desired: &DesiredSwap, active: &[ActiveSwap]) -> anyh set_swapfile_mode(p)?; run("mkswap", &[p.to_string_lossy().as_ref()])?; set_swapfile_mode(p)?; - write_ownership_marker(p)?; + record_ownership(p)?; + drop_legacy_ownership_sidecar(p); Ok(()) } -fn write_ownership_marker(path: &Path) -> anyhow::Result<()> { - let marker = ownership_marker_path(path)?; - fs::write(&marker, OWNERSHIP_MARKER) - .map_err(|e| anyhow::anyhow!("failed to write {}: {e}", marker.display())) -} - #[cfg(unix)] fn set_swapfile_mode(path: &Path) -> anyhow::Result<()> { fs::set_permissions(path, fs::Permissions::from_mode(0o600)) @@ -762,6 +955,29 @@ mod tests { fn swap_path_syntax_accepts_expected_production_values() { assert!(validate_swap_path_syntax("/swapfile").is_ok()); assert!(validate_swap_path_syntax("/var/lib/awg2-harness/swapfile").is_ok()); + assert!(validate_swap_path_syntax("/var/lib/awg2-harness/swap.1").is_ok()); + } + + /// An allow-list, not a deny-list: a path whose parent another user can write lets + /// them redirect root-owned operations between preflight and use, which no amount of + /// stat-ing beforehand can prevent. + #[test] + fn swap_path_syntax_rejects_paths_outside_root_owned_roots() { + for bad in [ + "/tmp/awg.swap", + "/home/user/awg.swap", + "/srv/data/swapfile", + "/var/lib/swapfile", + "/var/lib/awg2-harness", + "/var/lib/awg2-harness/nested/swapfile", + "/var/lib/awg2-harness-other/swapfile", + "/swapfile2", + ] { + assert!( + validate_swap_path_syntax(bad).is_err(), + "must reject path outside the allowed roots: {bad}" + ); + } } /// The blacklist used to compare exact strings only, so a path that is lexically @@ -829,41 +1045,131 @@ mod tests { assert!(validate_swap_path_syntax("/var/lib/").is_err()); } - #[test] - fn ownership_missing_when_file_absent() { - let dir = scratch("own_missing"); - let got = inspect_swapfile_ownership(&dir.join("swapfile"), 1024).expect("inspect"); - assert_eq!(got, SwapfileOwnership::Missing); + fn ident(device: u64, inode: u64) -> FileIdentity { + FileIdentity { device, inode } + } + + fn record(path: &str, device: u64, inode: u64) -> SwapOwnershipRecord { + SwapOwnershipRecord { + version: SWAP_OWNERSHIP_VERSION, + path: PathBuf::from(path), + identity: ident(device, inode), + } } - /// A pre-existing swapfile with no marker belongs to someone else and must never be - /// deleted or rewritten, whatever its size. #[test] - fn ownership_foreign_without_marker() { - let dir = scratch("own_foreign"); - let p = dir.join("swapfile"); - fs::write(&p, vec![0u8; 1024]).expect("seed"); - let got = inspect_swapfile_ownership(&p, 1024).expect("inspect"); - assert_eq!(got, SwapfileOwnership::Foreign); + fn ownership_missing_when_file_absent() { + let rec = record("/swapfile", 1, 2); + assert_eq!( + classify_ownership(Some(&rec), Path::new("/swapfile"), None, 1024), + SwapfileOwnership::Missing + ); + } + + /// A pre-existing swapfile we have no record of belongs to someone else and must + /// never be deleted or rewritten, whatever its size. + #[test] + fn ownership_foreign_without_record() { + assert_eq!( + classify_ownership( + None, + Path::new("/swapfile"), + Some((ident(1, 2), 1024)), + 1024 + ), + SwapfileOwnership::Foreign + ); } #[test] fn ownership_managed_valid_and_wrong_size() { - let dir = scratch("own_managed"); - let p = dir.join("swapfile"); - fs::write(&p, vec![0u8; 1024]).expect("seed"); - write_ownership_marker(&p).expect("marker"); - + let rec = record("/swapfile", 1, 2); assert_eq!( - inspect_swapfile_ownership(&p, 1024).expect("inspect"), + classify_ownership( + Some(&rec), + Path::new("/swapfile"), + Some((ident(1, 2), 1024)), + 1024 + ), SwapfileOwnership::ManagedValid ); assert_eq!( - inspect_swapfile_ownership(&p, 2048).expect("inspect"), + classify_ownership( + Some(&rec), + Path::new("/swapfile"), + Some((ident(1, 2), 1024)), + 2048 + ), SwapfileOwnership::ManagedWrongSize ); } + /// The data-loss scenario the sidecar allowed: our swapfile is deleted by hand, the + /// ownership state survives, and someone puts an unrelated file at the same path. + /// Matching on path alone would have called it managed and deleted it. + #[test] + fn ownership_is_stale_when_the_file_was_replaced() { + let rec = record("/swapfile", 1, 2); + assert_eq!( + classify_ownership( + Some(&rec), + Path::new("/swapfile"), + Some((ident(1, 999), 4096)), + 1024 + ), + SwapfileOwnership::StaleOwnership + ); + } + + /// A record for a different managed path says nothing about this file. + #[test] + fn ownership_record_for_another_path_is_foreign() { + let rec = record("/var/lib/awg2-harness/swapfile", 1, 2); + assert_eq!( + classify_ownership( + Some(&rec), + Path::new("/swapfile"), + Some((ident(1, 2), 1024)), + 1024 + ), + SwapfileOwnership::Foreign + ); + } + + /// Neither refusal state may ever authorise a delete. + #[test] + fn foreign_and_stale_are_never_managed() { + assert!(!SwapfileOwnership::Foreign.is_managed()); + assert!(!SwapfileOwnership::StaleOwnership.is_managed()); + assert!(!SwapfileOwnership::Missing.is_managed()); + assert!(SwapfileOwnership::ManagedValid.is_managed()); + assert!(SwapfileOwnership::ManagedWrongSize.is_managed()); + } + + #[test] + fn ownership_record_round_trips() { + let rec = record("/var/lib/awg2-harness/swapfile", 2049, 123456); + let rendered = render_ownership_record(&rec); + assert!(rendered.starts_with(SWAP_OWNERSHIP_MARKER)); + assert_eq!(parse_ownership_record(&rendered).expect("parse"), rec); + } + + #[test] + fn ownership_record_rejects_malformed_and_unsupported() { + for bad in [ + "", + "# marker only\n", + "version = 1\npath = /swapfile\ndevice = 1\n", + "version = 99\npath = /swapfile\ndevice = 1\ninode = 2\n", + "version = x\npath = /swapfile\ndevice = 1\ninode = 2\n", + ] { + assert!( + parse_ownership_record(bad).is_err(), + "must reject record: {bad:?}" + ); + } + } + #[test] fn ownership_rejects_non_regular_file() { let dir = scratch("own_dir"); @@ -872,27 +1178,6 @@ mod tests { assert!(inspect_swapfile_ownership(&p, 1024).is_err()); } - /// The guard that turns "wrong size, recreate" into a refusal rather than data loss. - #[test] - fn foreign_swapfile_is_never_recreated() { - let dir = scratch("foreign_guard"); - let p = dir.join("swapfile"); - fs::write(&p, b"someone elses data").expect("seed"); - - let desired = DesiredSwap { - path: p.clone(), - size_bytes: 4096, - priority: 1, - swappiness: 1, - }; - let err = ensure_desired_swapfile(&desired, &[]).expect_err("must refuse"); - assert!(err.to_string().contains("not created by awg2-harness")); - assert_eq!( - fs::read(&p).expect("still there"), - b"someone elses data".to_vec() - ); - } - #[test] fn safe_path_check_rejects_missing_parent_directory() { let dir = scratch("no_parent");