prod/AWG2_harness
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 2 Wiki Activity

fix(swap): never touch an unowned swapfile, activate before deactivating [P0]

Browse Source 
Two more release blockers in the swap module.

1. Foreign files could be destroyed.

   swap.path was validated against a blacklist of exact strings. /etc/../etc/fstab is not
   string-equal to /etc/fstab but resolves to it, and nothing normalized the path or
   checked the parent chain for symlinks. ensure_desired_swapfile then removed any
   existing regular file whose size did not match the configured one, so a mistyped or
   hostile path meant deleting /etc/passwd, an application config, or any other regular
   file. disable_foreign_swaps additionally deleted foreign swapfiles outright.

   Ownership is now explicit. A sidecar marker <swap.path>.awg2-harness-owned sits next to
   the managed swapfile, and inspect_swapfile_ownership classifies the path as Missing,
   ManagedValid, ManagedWrongSize or Foreign. Foreign is always a hard error - the file is
   never resized, rewritten or removed. Foreign active swaps are now only swapped off;
   deleting an administrator's swapfile is not this package's call, and swapoff plus the
   fstab rewrite already produce the exclusive runtime state.

   Path validation is split in two. The lexical half rejects relative paths, '.' and '..'
   components, whitespace, control characters and backslashes, the system path list, and
   anything under /proc, /sys or /dev; it touches no filesystem, so validate-config stays
   side-effect free on a host where the file does not exist yet. It runs after '..' is
   rejected, so the string is already normalized and the blacklist cannot be bypassed. The
   filesystem half runs just before use and refuses a symlink anywhere in the parent chain,
   not only at the final component.

   One deliberate deviation from the audit: arbitrary absolute paths stay allowed rather
   than being restricted to a fixed managed location. Pinning the path would break
   deployments that already configured their own, and the ownership marker is the guard
   that actually prevents data loss - a wrong path can now only fail, never destroy.

2. The migration order left the host swapless on failure.

   ensure_swap ran disable_foreign_swaps first, then created the swapfile, then activated
   it. Any failure in fallocate, dd, mkswap or swapon after that first step left the host
   with no swap at all - on a box provisioned with swap precisely because it is short on
   memory.

   The sequence is now a transaction: preflight, prepare the managed swapfile, swapon,
   verify it actually appears in the active list, and only then swapoff the others,
   rewrite fstab, apply swappiness and verify. Nothing destructive happens until the
   replacement is confirmed live. The one unavoidable gap is deactivating our own swap to
   resize it, which cannot be done in place; foreign swap stays up throughout.

   chmod 0600 now also happens before mkswap, so the file is never briefly world-readable
   while it already holds swap structure.

config.rs no longer carries its own copy of the path blacklist and calls the shared
validator.

Twelve tests: the four ownership states, refusal to recreate a foreign file with its
content asserted intact afterwards, '..' traversal, dangerous and virtual paths,
whitespace and control characters, relative and directory paths, a missing parent
directory, an accepted chain, and a symlinked parent (unix only).

Docs updated: IMPLEMENTATION_SPEC section 17 gained the ownership table, the ordered
transaction and the path rules; CONFIGURATION and the acceptance checklist gained the
operator-facing constraints and nine checks.

cargo test --workspace: 136 + 4 passed. clippy -D warnings clean.
This commit is contained in:
Crimson
2026-08-06 21:07:55 +05:00
parent 549fbcae31
commit a895295701
5 changed files with 461 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files
traffic_core/src/swap.rs
+406 -50
View File
@@ -46,6 +46,31 @@ pub struct SwapStatus {
pub drift_reason: Option<String>,
}
/// Sidecar that marks a swapfile as created and owned by this package.
///
/// 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";
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum SwapfileOwnership {
/// Nothing at the path: safe to create.
Missing,
/// Ours, and already the requested size.
ManagedValid,
/// Ours, but the size differs: safe to recreate.
ManagedWrongSize,
/// Exists without our marker. Never touched.
Foreign,
}
/// Brings host swap to the desired state as a transaction.
///
/// Ordering is the point: the managed swap is created, activated and verified *before*
/// any foreign swap is disabled. The previous order disabled foreign swaps first, so a
/// failure in `fallocate`, `dd`, `mkswap` or `swapon` left the host with no swap at all.
pub fn ensure_swap(cfg: &Config) -> anyhow::Result<()> {
if !cfg.swap.enabled {
return Ok(());
@@ -53,17 +78,162 @@ pub fn ensure_swap(cfg: &Config) -> anyhow::Result<()> {
let _lock = FileLock::acquire_timeout("/run/awg2-harness/swap.lock", Duration::from_secs(30))?;
// 1. Preflight: nothing is modified here.
let desired = DesiredSwap::from_config(cfg)?;
assert_swap_path_is_safe(&desired.path)?;
let active_before = read_active_swaps()?;
disable_foreign_swaps(&desired, &active_before)?;
ensure_desired_swapfile(&desired)?;
rewrite_fstab_exclusive(&desired)?;
// 2-3. Prepare and activate the managed swap.
ensure_desired_swapfile(&desired, &active_before)?;
enable_desired_swap(&desired)?;
// 4. Confirm it is really live before touching anything else.
let active_after = read_active_swaps()?;
if !active_after.iter().any(|s| s.name == desired.path) {
anyhow::bail!(
"managed swap {} did not become active; foreign swap left untouched",
desired.path.display()
);
}
// 5. Only now is it safe to take the others down.
disable_foreign_swaps(&desired, &active_after)?;
// 6. Persist and verify.
rewrite_fstab_exclusive(&desired)?;
apply_swappiness(desired.swappiness)?;
verify_exclusive_swap_state(&desired)?;
Ok(())
}
/// Lexical validation of `swap.path`, with no filesystem access.
///
/// Kept side-effect free so `validate-config` can run it on a host where the file does not
/// exist yet. Operates on the string rather than [`Path`] because the target is always a
/// Linux path, regardless of where validation runs.
pub fn validate_swap_path_syntax(raw: &str) -> anyhow::Result<()> {
if !raw.starts_with('/') {
anyhow::bail!("swap.path must be absolute: {raw}");
}
if raw.ends_with('/') {
anyhow::bail!("swap.path must not be a directory path: {raw}");
}
if let Some(bad) = raw
.chars()
.find(|c| c.is_whitespace() || c.is_control() || *c == '\\')
{
anyhow::bail!(
"swap.path must not contain whitespace or control characters: {raw:?} contains {bad:?}"
);
}
for component in raw.split('/') {
if component == "." || component == ".." {
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}");
}
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}");
}
Ok(())
}
/// Filesystem-level checks performed just before the path is used.
///
/// A symlink anywhere in the chain, not only at the final component, can redirect the
/// whole operation at a file we never intended to touch.
fn assert_swap_path_is_safe(path: &Path) -> anyhow::Result<()> {
validate_swap_path_syntax(&path.to_string_lossy())?;
assert_path_chain_is_safe(path)
}
/// Walks the parent chain and refuses any symlink or missing directory in it.
///
/// Split from the syntax check so it can be exercised independently of the Linux path
/// grammar.
fn assert_path_chain_is_safe(path: &Path) -> anyhow::Result<()> {
let mut walked = PathBuf::new();
let components: Vec<_> = path.components().collect();
for component in components.iter().take(components.len().saturating_sub(1)) {
walked.push(component);
match fs::symlink_metadata(&walked) {
Ok(md) if md.file_type().is_symlink() => {
anyhow::bail!(
"swap.path traverses a symlink at {}; refusing to continue",
walked.display()
);
}
Ok(_) => {}
Err(e) if e.kind() == std::io::ErrorKind::NotFound => {
anyhow::bail!(
"swap.path parent directory does not exist: {}",
walked.display()
);
}
Err(e) => {
anyhow::bail!("failed to stat {}: {e}", walked.display());
}
}
}
Ok(())
}
fn ownership_marker_path(path: &Path) -> anyhow::Result<PathBuf> {
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 inspect_swapfile_ownership(path: &Path, desired_size: u64) -> anyhow::Result<SwapfileOwnership> {
let md = match fs::symlink_metadata(path) {
Ok(v) => v,
Err(e) if e.kind() == std::io::ErrorKind::NotFound => {
return Ok(SwapfileOwnership::Missing);
}
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()),
};
if !owned {
return Ok(SwapfileOwnership::Foreign);
}
if md.len() == desired_size {
Ok(SwapfileOwnership::ManagedValid)
} else {
Ok(SwapfileOwnership::ManagedWrongSize)
}
}
pub fn inspect_swap(cfg: &Config) -> anyhow::Result<SwapStatus> {
let desired_path = PathBuf::from(cfg.swap.path.clone());
let active_entries = read_active_swaps()?;
@@ -193,69 +363,84 @@ fn parse_swapon_output(raw: &str) -> anyhow::Result<Vec<ActiveSwap>> {
Ok(out)
}
/// Takes foreign swap offline. The file itself is never deleted.
///
/// Destroying a swapfile an administrator created is not this package's call: `swapoff`
/// plus removal from `/etc/fstab` already achieves the exclusive runtime state, and the
/// file can be removed by hand once the operator agrees.
fn disable_foreign_swaps(desired: &DesiredSwap, active: &[ActiveSwap]) -> anyhow::Result<()> {
for entry in active {
if entry.name != desired.path {
run("swapoff", &[entry.name.to_string_lossy().as_ref()])?;
maybe_delete_foreign_swapfile(entry, &desired.path)?;
} else if !active_swap_size_matches(entry.size_bytes, desired.size_bytes)
|| entry.priority != desired.priority
{
run("swapoff", &[desired.path.to_string_lossy().as_ref()])?;
if entry.name == desired.path {
continue;
}
run("swapoff", &[entry.name.to_string_lossy().as_ref()])?;
}
Ok(())
}
fn maybe_delete_foreign_swapfile(entry: &ActiveSwap, desired_path: &Path) -> anyhow::Result<()> {
if entry.name == desired_path {
return Ok(());
}
let md = match fs::symlink_metadata(&entry.name) {
Ok(v) => v,
Err(_) => return Ok(()),
};
if md.file_type().is_symlink() {
return Ok(());
}
if md.file_type().is_file() {
let _ = fs::remove_file(&entry.name);
}
Ok(())
}
fn ensure_desired_swapfile(desired: &DesiredSwap) -> anyhow::Result<()> {
/// Creates or validates the managed swapfile without ever touching an unowned one.
///
/// `active` is needed because a live swapfile cannot be resized in place: if the managed
/// swap is currently active with the wrong geometry it has to be deactivated first. That
/// is our own swap, so the brief gap is acceptable; foreign swap stays up throughout.
fn ensure_desired_swapfile(desired: &DesiredSwap, active: &[ActiveSwap]) -> anyhow::Result<()> {
let p = &desired.path;
if !p.is_absolute() {
anyhow::bail!("swap.path must be absolute: {}", p.display());
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",
p.display()
);
}
let mut need_create = true;
if p.exists() {
let md = fs::symlink_metadata(p)?;
if md.file_type().is_symlink() {
anyhow::bail!("desired swap path is symlink: {}", p.display());
}
if !md.file_type().is_file() {
anyhow::bail!("desired swap path is not regular file: {}", p.display());
}
let desired_active = active.iter().find(|s| s.name == *p);
let geometry_ok = desired_active
.map(|s| {
active_swap_size_matches(s.size_bytes, desired.size_bytes)
&& s.priority == desired.priority
})
.unwrap_or(false);
if md.len() == desired.size_bytes {
need_create = false;
} else {
fs::remove_file(p)?;
}
if ownership == SwapfileOwnership::ManagedValid && (desired_active.is_none() || geometry_ok) {
write_ownership_marker(p)?;
set_swapfile_mode(p)?;
return Ok(());
}
if need_create {
create_swapfile(p, desired.size_bytes)?;
run("mkswap", &[p.to_string_lossy().as_ref()])?;
if desired_active.is_some() {
run("swapoff", &[p.to_string_lossy().as_ref()])?;
}
#[cfg(unix)]
{
fs::set_permissions(p, fs::Permissions::from_mode(0o600))?;
if ownership != SwapfileOwnership::Missing {
fs::remove_file(p).map_err(|e| anyhow::anyhow!("failed to remove {}: {e}", p.display()))?;
}
create_swapfile(p, desired.size_bytes)?;
// Restrictive mode before mkswap, so the file is never briefly world-readable while
// it already contains swap structure.
set_swapfile_mode(p)?;
run("mkswap", &[p.to_string_lossy().as_ref()])?;
set_swapfile_mode(p)?;
write_ownership_marker(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))
.map_err(|e| anyhow::anyhow!("failed to chmod 0600 {}: {e}", path.display()))
}
#[cfg(not(unix))]
fn set_swapfile_mode(_path: &Path) -> anyhow::Result<()> {
Ok(())
}
@@ -517,6 +702,177 @@ fn run(program: &str, args: &[&str]) -> anyhow::Result<()> {
mod tests {
use super::*;
fn scratch(name: &str) -> PathBuf {
let dir = std::env::temp_dir().join(format!("awg2_swap_{}_{}", name, std::process::id()));
let _ = fs::remove_dir_all(&dir);
fs::create_dir_all(&dir).expect("scratch dir");
dir
}
#[test]
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());
}
/// The blacklist used to compare exact strings only, so a path that is lexically
/// different but resolves to a forbidden file slipped through.
#[test]
fn swap_path_syntax_rejects_dot_dot_traversal() {
for bad in [
"/etc/../etc/fstab",
"/var/../etc/passwd",
"/./swapfile",
"/var/lib/../../etc/fstab",
] {
assert!(
validate_swap_path_syntax(bad).is_err(),
"must reject traversal: {bad}"
);
}
}
#[test]
fn swap_path_syntax_rejects_dangerous_and_virtual_paths() {
for bad in [
"/",
"/etc",
"/etc/fstab",
"/etc/passwd",
"/etc/shadow",
"/dev",
"/proc",
"/sys",
"/run",
"/boot",
"/proc/self/mem",
"/sys/kernel",
"/dev/sda",
] {
assert!(
validate_swap_path_syntax(bad).is_err(),
"must reject dangerous path: {bad}"
);
}
}
/// Unescaped whitespace would shift the fstab columns; control characters and
/// backslashes have no legitimate use in a swapfile path.
#[test]
fn swap_path_syntax_rejects_whitespace_and_control_characters() {
for bad in [
"/var/my swap",
"/var/swap\tfile",
"/var/swap\nfile",
"/var/swap\\file",
] {
assert!(
validate_swap_path_syntax(bad).is_err(),
"must reject: {bad:?}"
);
}
}
#[test]
fn swap_path_syntax_rejects_relative_and_directory_paths() {
assert!(validate_swap_path_syntax("swapfile").is_err());
assert!(validate_swap_path_syntax("var/lib/swapfile").is_err());
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);
}
/// 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);
}
#[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");
assert_eq!(
inspect_swapfile_ownership(&p, 1024).expect("inspect"),
SwapfileOwnership::ManagedValid
);
assert_eq!(
inspect_swapfile_ownership(&p, 2048).expect("inspect"),
SwapfileOwnership::ManagedWrongSize
);
}
#[test]
fn ownership_rejects_non_regular_file() {
let dir = scratch("own_dir");
let p = dir.join("swapfile");
fs::create_dir(&p).expect("seed dir");
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");
let err =
assert_path_chain_is_safe(&dir.join("nested").join("swapfile")).expect_err("must fail");
assert!(err.to_string().contains("parent directory does not exist"));
}
#[test]
fn safe_path_check_accepts_existing_parent_chain() {
let dir = scratch("parent_ok");
assert!(assert_path_chain_is_safe(&dir.join("swapfile")).is_ok());
}
/// A symlink anywhere in the chain, not only at the final component, can redirect the
/// operation at a file that was never configured.
#[cfg(unix)]
#[test]
fn safe_path_check_rejects_symlinked_parent() {
let dir = scratch("symlink_parent");
let real = dir.join("real");
fs::create_dir(&real).expect("real dir");
let link = dir.join("link");
std::os::unix::fs::symlink(&real, &link).expect("symlink");
let err = assert_path_chain_is_safe(&link.join("swapfile")).expect_err("must fail");
assert!(err.to_string().contains("traverses a symlink"));
}
#[test]
fn parse_swapon_empty() {
let got = parse_swapon_output("").expect("parse");