From 0386d23870bcc21f4512f705ec77b2c28850c8cb Mon Sep 17 00:00:00 2001 From: DevomB Date: Wed, 7 Oct 2026 09:12:47 -0700 Subject: [PATCH] kryptikd's broker, launch daemon, update, zone and command comments are shorter, with their reasons kept: cleanup-held's cut of kryptikd's sources, probes and cli suite, redone on main by hand, comments only Only comment regions are taken from 65a6fbb. Where the cut dropped a reason a shorter one is kept: the broker's peer uid is the kernel's, a zone reaches only its own clipboard and a move crosses once, None refuses every transfer, the destination is looked up again as it may have restarted, the identity switch is restored on every path; update.rs never resolves under what the net zone reports, never stages past the signed size, and its channel file is the image's; serve.rs's socket directory admits the group alone; stop signals a launcher only while its pid and start time match; and the suites' charters and orderings. Kept where the cut corrects main: the broker's request table gains transfer and clipboard-move, cli.sh's header its exit codes. --- compartments/kryptikd/Cargo.toml | 3 +- .../kryptikd/probes/boundary-checks.sh | 49 +++----- compartments/kryptikd/probes/fixtures.sh | 32 ++---- compartments/kryptikd/src/broker.rs | 105 +++++++----------- compartments/kryptikd/src/broker/tests.rs | 50 +++------ compartments/kryptikd/src/main.rs | 81 +++++--------- compartments/kryptikd/src/serve.rs | 83 +++++--------- compartments/kryptikd/src/update.rs | 77 +++++-------- compartments/kryptikd/src/update/tests.rs | 2 +- compartments/kryptikd/src/zone.rs | 63 ++++------- compartments/tests/cli.sh | 40 +++---- 11 files changed, 209 insertions(+), 376 deletions(-) diff --git a/compartments/kryptikd/Cargo.toml b/compartments/kryptikd/Cargo.toml index c3220108..44f40443 100644 --- a/compartments/kryptikd/Cargo.toml +++ b/compartments/kryptikd/Cargo.toml @@ -5,8 +5,7 @@ edition = "2021" description = "Kryptik compartment manager: zone lifecycle and isolation" license = "GPL-2.0-or-later" -# kryptikd is the most privileged process on the system, so it depends on -# `libc` alone (ADR-010); anything else, TOML parsing included, is written here. +# The most privileged process on the system depends on libc alone; even TOML parsing is ours (ADR-010). [dependencies] libc = "0.2" diff --git a/compartments/kryptikd/probes/boundary-checks.sh b/compartments/kryptikd/probes/boundary-checks.sh index d1912d59..980df64f 100755 --- a/compartments/kryptikd/probes/boundary-checks.sh +++ b/compartments/kryptikd/probes/boundary-checks.sh @@ -1,10 +1,8 @@ #!/usr/bin/env bash -# Regression probes for the kryptikd zone boundary, through the real launcher -# (`kryptikd run`). Exit status is the number of failures. +# Regression probes for the zone boundary, through `kryptikd run`; the exit status counts failures. +# Unprivileged, what needs root or the target kernel is reported as not run; zones-test.sh also +# runs it as root on the installed system. # compartments/kryptikd/probes/boundary-checks.sh [path/to/kryptikd] -# Unprivileged on a developer host, where what needs root and the target kernel -# is reported as not run; also run as root on the installed system -# (tools/image/zones-test.sh). set -u HERE="$(cd "$(dirname "$0")" && pwd)" @@ -15,9 +13,7 @@ K="$(cd "$(dirname "$K")" && pwd)/$(basename "$K")" # shellcheck source=fixtures.sh source "$HERE/fixtures.sh" -# As root, zones need an explicit identity (kryptikd will not map a zone's root -# to uid 0), and the fixture roots must belong to it: setup drops to that uid -# before it opens the data directory. +# As root, zones need an identity other than uid 0, and setup opens their data as that uid. IDFLAGS=() if [[ "$(id -u)" -eq 0 ]]; then IDFLAGS=(--zone-uid 100000 --zone-gid 100000) @@ -33,8 +29,7 @@ fail() { echo "FAIL $1"; shift; [[ $# -gt 0 ]] && printf '%s\n' "$1" | sed 's/^ skip() { echo "SKIP $1"; SKIPS=$((SKIPS+1)); } head_() { echo; echo "== $1"; } -# Noise every launch prints that no check is about: the experimental hint, -# the swap caveat, inherited groups an unprivileged launcher cannot drop. +# Lines every launch prints that no check is about. denoise() { grep -v 'KRYPTIK_EXPERIMENTAL=1 -\|supplementary host group\|not secure erasure\|auto-approve-transfers: every\|NOT encrypted at rest' } @@ -43,9 +38,7 @@ denoise() { zrun() { local zone="$1"; shift [[ "${1:-}" == "--" ]] && shift - # No pipe, so ZRC is the launcher's status. stdout and stderr are captured - # apart and joined after, zone output first: on one pipe they interleave - # mid-line. + # Files, not a pipe: ZRC stays the launcher's status, and the two streams cannot interleave. local out="$F/zrun.out" err="$F/zrun.err" timeout 60 "$K" run "$zone" "${ZFLAGS[@]}" "${IDFLAGS[@]}" --zones "$F/zones" --rootfs "$F/roots" -- "$@" > "$out" 2> "$err" ZRC=$? @@ -134,8 +127,7 @@ import socket for n,f,t in [('AF_VSOCK',40,1),('AF_ALG',38,5),('AF_PACKET',17,2)]: try: socket.socket(f,t); print(n,'OPENED') except OSError as e: print(n,'refused',e.errno)" -# The kernel runs a family's create code before it refuses a pair (EOPNOTSUPP, -# 95), so the filter refuses every family but AF_UNIX first (97). +# The kernel runs a family's create code before refusing a pair, so the filter refuses first (97). MATCH="^unix-pair inet 97$" check "socketpair(2) makes AF_UNIX pairs and refuses AF_INET by family" 0 /usr/bin/python3 -c " import socket a,b=socket.socketpair(); a.close(); b.close() @@ -162,8 +154,7 @@ MATCH="owns the NIC" checkz routedraw "a zone that does not own the NIC may not MATCH="policy" checkz nopolicy "a policy file that does not exist is a refusal, not a fallback" 1 /bin/sh -c "echo RAN-ANYWAY" MATCH="ptrace" checkz badpolicy "a policy may not re-allow something on the denied list" 1 /bin/sh -c "echo RAN-ANYWAY" -# Landlock: a zone policy is a second layer, and layers intersect, so it can -# only take access away. +# A Landlock policy is a second layer, and layers intersect, so it can only take access away. MATCH="^ok$" checkz narrowed "a zone with a Landlock policy still runs and can read its root" 0 /bin/sh -c "test -r /usr/bin/env && echo ok" MATCH="^ok$" checkz narrowed "... and writes where the policy grants write" 0 /bin/sh -c "echo x > /tmp/f && echo x > /dev/null && echo ok" MATCH="^denied$" checkz narrowed "... but NOT its own HOME, which the base rules alone would allow" 0 /bin/sh -c "echo x > \$HOME/f 2>/dev/null && echo WROTE || echo denied" @@ -177,8 +168,7 @@ MATCH="symbolic link" checkz swapped "... which its next start refuses rather th # --------------------------------------------------------------------------- head_ "F. Guarantees a build cannot give are refused, not implied" -# Unprivileged, the refusal is that a LUKS2 volume needs a root launch; as -# root, that the passphrase is missing. Both say "is encrypted:". +# Unprivileged it needs a root launch, as root a passphrase; both refusals say "is encrypted:". MATCH="is encrypted:" checkz sealed "a zone declaring encrypted storage does not start on a plain directory" 1 /bin/sh -c "echo RAN-ANYWAY" if "$K" run capped "${ZFLAGS[@]}" "${IDFLAGS[@]}" --zones "$F/zones" --rootfs "$F/roots" -- /bin/sh -c "echo LIMITS-RAN" 2>&1 | grep -q LIMITS-RAN; then pass "[limits] is enforced here: cgroups are creatable and the zone ran" @@ -214,16 +204,13 @@ MATCH="^kryptik-broker 1 zone=probe$" check "a zone reaches its own broker and i " MATCH="^error: unknown verb$" check "an unknown verb is refused" 0 /usr/bin/python3 -c "$BRK" "steal " -# The clock (docs/design/time.md): only the zone that holds the network may -# claim a time. A malformed claim is refused at parse time, before the caller -# is checked. +# Only the zone holding the network may claim a time (docs/design/time.md); parsing comes first. MATCH="does not hold the network" check "the clock's verb is refused from a zone that does not hold the network" 0 /usr/bin/python3 -c "$BRK" "time-offset 5 4 " MATCH="is not an offset in seconds" check "a time claim outside the grammar is refused at parse time" 0 /usr/bin/python3 -c "$BRK" "time-offset 1e9 4 " -# The update channel (docs/design/update-channel.md): only the zone that holds -# the network may bring a release. Other zones are refused before any payload -# is read, and a malformed request at parse time, before that. +# Only the zone holding the network may bring a release (docs/design/update-channel.md); other +# zones are refused before any payload is read. MATCH="does not hold the network" check "a statement of what is current is refused from a zone that does not hold the network" 0 /usr/bin/python3 -c "$BRK" "update-latest 5 3 helloabc" MATCH="does not hold the network" check "asking whether a release is wanted is refused from a zone that does not hold the network" 0 /usr/bin/python3 -c "$BRK" "update-poll @@ -255,10 +242,7 @@ s=socket.socket(socket.AF_UNIX); s.connect("/run/kryptik/broker") s.sendmsg([("transfer %s %s\n"%(dest,name)).encode()],[(socket.SOL_SOCKET,socket.SCM_RIGHTS,array.array("i",[fd]))]) print(s.recv(300).decode().strip())' -# A real transfer between two running zones: packet announces itself, waits -# for the file and reports it; probe then sends one. Waits are bounded polls, -# not sleeps, as zone start-up time varies widely. The broker creates the file -# under its final name before filling it, so an empty file is still landing. +# packet polls for a non-empty file: the broker creates it under its final name before filling it. "$K" run packet "${ZFLAGS[@]}" "${IDFLAGS[@]}" --zones "$F/zones" --rootfs "$F/roots" -- /usr/bin/python3 -u -c " import os,time print('PACKET-UP') @@ -293,8 +277,7 @@ MATCH="not on the zone" check "a file from the zone's tmpfs, not its data mount, MATCH="transfer limit is 16" check "a file over the destination's [transfer] max_bytes is refused" 0 /bin/sh -c "printf '%017d' 0 > /home/probe/big && python3 -c '$TX' packet big /home/probe/big 0" MATCH="not running" check "a destination that is not running is refused" 0 /bin/sh -c "echo x > /home/probe/f && python3 -c '$TX' packet f /home/probe/f 0" ZFLAGS=() -# Consent is asked only for a running destination. Nobody answers here, so a -# short deadline makes that a refusal rather than a timeout. +# Consent is asked only for a running destination; a short deadline makes no answer a refusal. "$K" run packet "${IDFLAGS[@]}" --zones "$F/zones" --rootfs "$F/roots" -- /usr/bin/python3 -u -c " import time print('PACKET-UP') @@ -309,9 +292,7 @@ MATCH="single path component" check "a name carrying a path separator is refused # --------------------------------------------------------------------------- head_ "H. The zone dies with its launcher" -# MARK is argv[0] of the zone's command, so `pgrep -f "^$MARK"` matches it and -# not the launcher, whose command line carries it further along. The zone must -# be up before its launcher is signalled; both waits are bounded polls. +# MARK is argv[0] of the zone's command, so "^$MARK" matches it and not the launcher. MARK="kryptik-probe-sleep-$$" zone_up() { for _ in $(seq 1 300); do pgrep -f "^$MARK" >/dev/null && return 0; kill -0 "$1" 2>/dev/null || return 1; sleep 0.1; done; return 1; } zone_gone() { for _ in $(seq 1 100); do pgrep -f "^$MARK" >/dev/null || return 0; sleep 0.1; done; return 1; } diff --git a/compartments/kryptikd/probes/fixtures.sh b/compartments/kryptikd/probes/fixtures.sh index 39369cd0..0899775d 100755 --- a/compartments/kryptikd/probes/fixtures.sh +++ b/compartments/kryptikd/probes/fixtures.sh @@ -1,7 +1,5 @@ #!/usr/bin/env bash -# Synthetic zones for the kryptikd boundary probes, sourced by boundary-checks.sh. -# Exports $F, a temporary root (removed on exit) holding zones/, zones/policy/ -# and roots/. Nothing here is secret; payloads are the strings a check greps for. +# Synthetic zones for boundary-checks.sh, under $F (zones/, zones/policy/, roots/; removed on exit). set -u F="$(mktemp -d -t kryptik-probe-XXXXXX)" @@ -17,8 +15,7 @@ zone() { # zone NAME BODY... mkdir -p "$F/roots/$name" } -# probe: the ordinary zone most checks run in. No policy file: base seccomp -# allowlist and CAP_NET_BIND_SERVICE only. +# probe: where most checks run, with no policy file: the base allowlist and CAP_NET_BIND_SERVICE. zone probe \ '[zone]' 'name = "probe"' \ '[network]' 'mode = "routed"' \ @@ -26,8 +23,7 @@ zone probe \ '[transfer]' 'to = "packet"' \ '[ui]' 'border_color = "#123456"' -# packet: the transfer destination. Its policy allows AF_PACKET but keeps no -# capability, so the socket passes seccomp and the kernel refuses it. +# packet: the transfer destination, allowed AF_PACKET but no capability, so the kernel refuses it. zone packet \ '[zone]' 'name = "packet"' \ '[network]' 'mode = "none"' \ @@ -39,8 +35,7 @@ printf '%s\n' \ '# Synthetic: the socket family is allowed, no capability is kept.' \ 'allow-socket AF_PACKET' > "$F/zones/policy/packet.seccomp" -# capped: declares [limits]. Where cgroups cannot be created it must be -# refused, naming [limits]. +# capped: declares [limits], which must be refused where cgroups cannot be created. zone capped \ '[zone]' 'name = "capped"' \ '[network]' 'mode = "none"' \ @@ -55,16 +50,14 @@ zone keeper \ '[storage]' 'mode = "persistent"' \ '[ui]' 'border_color = "#fedcba"' -# sealed: encrypted storage must refuse to start, never fall back to a plain -# directory. +# sealed: encrypted storage must refuse to start, never fall back to a plain directory. zone sealed \ '[zone]' 'name = "sealed"' \ '[network]' 'mode = "none"' \ '[storage]' 'mode = "encrypted"' 'volume = "/dev/null"' \ '[ui]' 'border_color = "#0f0f0f"' -# nicholder: the NIC owner every zone set needs, and the zone NIC-only policy -# rules apply to. It names no interface, so none is moved. +# nicholder: the NIC owner every zone set needs; it names no interface, so none is moved. zone nicholder \ '[zone]' 'name = "nicholder"' \ '[network]' 'mode = "nic"' \ @@ -77,8 +70,7 @@ printf '%s\n' \ 'keep-capability CAP_NET_RAW' \ 'keep-capability CAP_NET_ADMIN' > "$F/zones/policy/nic.seccomp" -# routedraw: a zone that does not own the NIC may not keep CAP_NET_RAW, -# whatever its policy says. +# routedraw: does not own the NIC, so it may not keep CAP_NET_RAW whatever its policy says. zone routedraw \ '[zone]' 'name = "routedraw"' \ '[network]' 'mode = "routed"' \ @@ -86,8 +78,7 @@ zone routedraw \ '[policy]' 'seccomp = "policy/nic.seccomp"' \ '[ui]' 'border_color = "#ff0088"' -# nopolicy: a missing policy file is a refusal, never a fallback to the base -# rules. +# nopolicy: a missing policy file is a refusal, never a fallback to the base rules. zone nopolicy \ '[zone]' 'name = "nopolicy"' \ '[network]' 'mode = "none"' \ @@ -95,8 +86,7 @@ zone nopolicy \ '[policy]' 'seccomp = "policy/absent.seccomp"' \ '[ui]' 'border_color = "#333333"' -# narrowed: Landlock grants read everywhere and write only in /tmp and /dev, -# so its HOME becomes read-only; the second layer can only subtract. +# narrowed: its Landlock policy grants write only in /tmp and /dev, so its HOME is read-only. zone narrowed \ '[zone]' 'name = "narrowed"' \ '[network]' 'mode = "none"' \ @@ -109,9 +99,7 @@ printf '%s\n' \ 'read-write /tmp' \ 'read-write /dev' > "$F/zones/policy/narrowed.landlock" -# swapped: persistent, and its policy keeps HOME read-only but for work, with -# exec only in work/bin. Write in work lets it swap work/bin for a link to -# HOME, which a rule must refuse at the next start, not follow. +# swapped: can replace work/bin with a link to HOME, which its next start must refuse, not follow. zone swapped \ '[zone]' 'name = "swapped"' \ '[network]' 'mode = "none"' \ diff --git a/compartments/kryptikd/src/broker.rs b/compartments/kryptikd/src/broker.rs index 2caf58d6..eb3c2c37 100755 --- a/compartments/kryptikd/src/broker.rs +++ b/compartments/kryptikd/src/broker.rs @@ -1,9 +1,7 @@ -//! The zone broker (docs/design/broker.md): each zone's one socket into zone 0, -//! serving the clipboard, transfer, time and update verbs. -//! -//! A peer is known by its `SO_PEERCRED` uid, which the kernel asserts and the -//! zone cannot choose; each zone has its own `[identity]` uid range. The pid -//! is never used, since it may be reused. +//! The zone broker (docs/design/broker.md): each zone's socket into zone 0, for the clipboard, +//! transfer, time and update verbs. A peer is known by its `SO_PEERCRED` uid, which the kernel +//! asserts and the zone cannot choose, each zone having its own uid range; never by its pid, +//! which may be reused. use std::cell::Cell; use std::ffi::CString; @@ -19,8 +17,7 @@ use crate::zone::{NetworkMode, Zone}; /// The socket's name in a registry entry, and in the zone's /run/kryptik. pub const SOCKET_NAME: &str = "broker"; -/// Listen on an AF_UNIX socket at `path` that only the zone identity can -/// connect to. A stale file is removed first: this launcher owns the entry. +/// Listen at `path` for the zone identity alone, replacing a stale file: the entry is ours. pub fn listen_at(path: &std::path::Path, uid: u32, gid: u32) -> io::Result { let _ = std::fs::remove_file(path); let c = std::ffi::CString::new(path.as_os_str().as_encoded_bytes()) @@ -60,9 +57,8 @@ pub fn listen_at(path: &std::path::Path, uid: u32, gid: u32) -> io::Result kryptik-broker 1 zone=NAME\n /// clipboard-set \n -> ok\n /// clipboard-get\n -> ok \n | empty\n -/// time-offset \n -> ok ignored | slewed | stepped | stepped after consent\n -/// (from the zone that holds the network, and no other) -/// update-latest \n -> ok current | ok available \n +/// transfer \n (+1 fd) -> ok \n +/// clipboard-move ... -> error: clipboard-move is a zone 0 act, not a zone verb\n +/// time-offset \n -> ok ignored | slewed | stepped | stepped after consent\n +/// update-latest \n +/// -> ok current | ok available \n /// update-poll\n -> idle | fetch need ...\n -/// update-put \n -> ok / | ok complete\n -/// (the same zone, and no other) +/// update-put \n +/// -> ok / | ok complete\n /// anything else -> error: \n /// ``` +/// +/// The time and update verbs are taken from the nic zone alone. #[derive(Debug, PartialEq)] pub enum Request { Version, @@ -107,8 +106,6 @@ pub enum Request { /// The nic zone's claim of the clock's offset from network time (docs/design/time.md). TimeOffset(crate::time::Claim), /// The update channel (docs/design/update-channel.md), nic zone only. - /// `update-latest` is followed by the pointer and its signature, - /// `update-put` by `len` bytes of the named file. UpdateLatest { plen: usize, slen: usize }, UpdatePoll, UpdatePut { name: String, offset: u64, len: usize }, @@ -141,8 +138,7 @@ fn check_zone_name(s: &str) -> Result<(), String> { Ok(()) } -/// A transferred file's name: one path component of 1 to 255 printable ASCII -/// bytes, with no leading dot, so a zone cannot plant dotfiles. +/// A transferred file's name: one component of 1 to 255 printable ASCII bytes, not a dotfile. pub fn check_transfer_name(n: &str) -> Result<(), String> { if n.is_empty() || n.len() > 255 { return Err("name must be 1 to 255 bytes".into()); @@ -215,8 +211,7 @@ pub fn parse_request(line: &str) -> Result { } } -/// `time-offset`, taken only from the nic zone: no other zone has a network of -/// its own to measure with. `time::consider` decides what to believe. +/// `time-offset`, nic zone only: no other zone has a network of its own to measure with. fn handle_time_offset(zone: &Zone, claim: &crate::time::Claim, asking: &dyn Fn() -> bool) -> crate::time::Outcome { time_offset_in( zone, @@ -228,8 +223,7 @@ fn handle_time_offset(zone: &Zone, claim: &crate::time::Claim, asking: &dyn Fn() ) } -/// Why `zone` may not bring an update: only the nic zone can have fetched one. -/// What it sends is still untrusted; `update.rs` judges it. +/// Only the nic zone can have fetched an update; what it sends is still judged by update.rs. fn update_refusal(zone: &Zone) -> Option { (zone.network != NetworkMode::Nic) .then(|| format!("zone {:?} does not hold the network; only the zone that does may bring an update", zone.name)) @@ -304,8 +298,8 @@ pub struct Served<'a> { pub entry: &'a Path, /// The zone directory, to load a destination's zone file. pub zones_dir: &'a Path, - /// st_dev of the zone's /home/, looked up per request because the - /// zone's root is built after its pid 1 starts. None refuses every transfer. + /// st_dev of the zone's /home/, per request: its root is built after pid 1 starts. + /// None refuses every transfer. pub home_dev: &'a dyn Fn() -> Option, /// The development stand-in for the zone 0 prompt. pub auto_approve: bool, @@ -313,21 +307,18 @@ pub struct Served<'a> { pub max_bytes: u64, /// Finds a running destination's root and identity: the registry, or a test directory. pub resolve_dest: &'a dyn Fn(&str) -> Result, - /// Called ten times a second while the user is being asked; the launcher - /// pumps zone output there, and `false` withdraws the question. + /// Called ten times a second while asking: the launcher pumps zone output; `false` withdraws. pub asking: &'a dyn Fn() -> bool, /// Where the broker's lines go; the launcher bounds them per launch. pub log: &'a dyn Fn(&str), - /// Until when this launch asks nothing more after a refusal: every - /// question takes focus in zone 0, so a zone may not raise them in a loop. + /// After a refusal, no new question until then: each one takes focus in zone 0. pub refused_until: &'a Cell>, } /// How long a refused zone waits before it may ask again. pub const REFUSAL_PAUSE: Duration = Duration::from_secs(60); -/// The launcher's `resolve_dest`: the registry says whether the zone runs and -/// as whom, and its pid 1's root leads into its mount namespace. +/// The launcher's `resolve_dest`, from the registry and the zone's pid 1 root. pub fn registry_target(dest: &str) -> Result { let st = match registry::state(dest) { Ok(registry::State::Running { init: Some(st), .. }) if st.still_alive() => st, @@ -448,10 +439,9 @@ fn handle_transfer(s: &Served, dest: &str, name: &str, fds: &[OwnedFd]) -> Resul if st.st_size as u64 > limit { return Err(format!("file is {} bytes; the transfer limit is {limit}{whose}", st.st_size)); } - /* Ask last, once even the destination is known to be running: a question - * whose answer changes nothing teaches people to say yes. Look it up again - * afterwards; holding its root through the wait would pin its mounts, and - * the zone may have restarted meanwhile. */ + /* Ask last, once the destination is known to run: a pointless question teaches people to say + * yes. Then look it up again: holding its root through the wait would pin its mounts, and + * the zone may have restarted. */ if !s.auto_approve { drop((s.resolve_dest)(dest)?); if s.refused_until.get().is_some_and(|t| Instant::now() < t) { @@ -470,8 +460,7 @@ fn handle_transfer(s: &Served, dest: &str, name: &str, fds: &[OwnedFd]) -> Resul deliver(&target, name, src, st.st_size as u64) } -/// Copy into the destination's `incoming/`. Every path is resolved from its -/// root with openat2 and no symlinks, so nothing written leaves its tree. +/// Copy into the destination's `incoming/`, every path resolved in its root without symlinks. fn deliver(target: &Target, name: &str, src: RawFd, size: u64) -> Result<(String, u64), String> { let home = openat2( target.root_fd.as_raw_fd(), @@ -481,10 +470,9 @@ fn deliver(target: &Target, name: &str, src: RawFd, size: u64) -> Result<(String RESOLVE_IN_ROOT | RESOLVE_NO_SYMLINKS | RESOLVE_NO_MAGICLINKS, ) .map_err(|e| format!("destination home is not reachable: {e}"))?; - /* Create as the destination identity: an ephemeral zone's home is a tmpfs - * mounted in its user namespace, which refuses (EOVERFLOW) to create an - * inode for an unmapped uid such as host root. Restored on every path; - * the broker serves one request at a time. */ + /* As the destination identity: an ephemeral home is a tmpfs in the zone's user namespace, + * which refuses (EOVERFLOW) an inode for an unmapped uid such as host root. Restored on + * every path; the broker serves one request at a time. */ let switched = unsafe { libc::geteuid() } == 0; if switched { unsafe { @@ -532,8 +520,7 @@ fn deliver_into(home: RawFd, target: &Target, name: &str, src: RawFd, size: u64) if st.st_uid != target.uid { return Err("incoming/ is not owned by the destination zone".into()); } - /* O_EXCL picks the name: a collision, or a planted symlink (also EEXIST), - * moves on to the next number. */ + // O_EXCL picks the name: a collision or a planted symlink (EEXIST) moves to the next number. for i in 1..=100u32 { let cand = if i == 1 { name.to_string() } else { format!("{name}-{i}") }; match openat2( @@ -575,10 +562,8 @@ fn fill(out: RawFd, src: RawFd, size: u64, target: &Target) -> Result Result { let mut total: u64 = 0; let mut fallback = false; @@ -646,8 +631,7 @@ fn write_all(fd: RawFd, mut data: &[u8]) -> io::Result<()> { Ok(()) } -/// Accept one connection and answer one request. Returns the request's name -/// (Request::name), for logging. +/// Accept one connection and answer its request; returns the request's name for the log. pub fn serve_one(listen_fd: RawFd, s: &Served) -> io::Result> { let fd = unsafe { libc::accept4(listen_fd, std::ptr::null_mut(), std::ptr::null_mut(), libc::SOCK_CLOEXEC) }; if fd < 0 { @@ -662,8 +646,7 @@ pub fn serve_one(listen_fd: RawFd, s: &Served) -> io::Result io::Result> { let zone = s.zone.name.as_str(); let entry = s.entry; @@ -793,8 +776,7 @@ fn recv_first(fd: RawFd, buf: &mut Vec, started: Instant) -> io::Result<(boo } } -/// Receive straight into `buf` until it holds `want` bytes (a length the -/// header was checked against) or the peer reaches EOF, within the deadline. +/// Receive into `buf` until it holds `want` bytes (a checked length) or EOF, by the deadline. fn read_more(fd: RawFd, buf: &mut Vec, want: usize, started: Instant) -> io::Result<()> { let mut filled = buf.len(); if filled >= want { @@ -900,8 +882,7 @@ pub fn clipboard_read(entry: &Path) -> io::Result)>> { Ok(Some((mime, bytes))) } -/// Replace the zone's payload whole, 0600; a planted symlink is replaced, -/// never followed. +/// Replace the zone's payload whole, 0600; a planted symlink is replaced, never followed. pub fn clipboard_write(entry: &Path, mime: &str, bytes: &[u8]) -> io::Result<()> { if !MIME_TYPES.contains(&mime) { return Err(io::Error::new(io::ErrorKind::InvalidInput, format!("unsupported MIME type {mime:?}"))); @@ -912,9 +893,8 @@ pub fn clipboard_write(entry: &Path, mime: &str, bytes: &[u8]) -> io::Result<()> crate::files::write_atomic(&entry.join(CLIPBOARD_FILE), &[mime.as_bytes(), b"\n", bytes], 0o600, None) } -/// `kryptikd clipboard move`: `to` takes `from`'s payload, which leaves -/// `from`: one payload crosses, once. Both are registry entries of running -/// zones. Returns what moved. +/// Move `from`'s payload to `to`, leaving `from` empty: one payload crosses, once. Both are +/// registry entries of running zones; returns what moved. pub fn clipboard_move(from: &Path, to: &Path) -> io::Result<(String, usize)> { let Some((mime, bytes)) = clipboard_read(from)? else { return Err(io::Error::new(io::ErrorKind::NotFound, "nothing on the source zone's clipboard")); @@ -931,8 +911,7 @@ pub fn clipboard_move(from: &Path, to: &Path) -> io::Result<(String, usize)> { Ok((mime, bytes.len())) } -/// The zone 0 gesture, for `kryptikd clipboard move` and serve alike: both -/// zones must be running. Ok is the line to show, Err why it was refused. +/// The zone 0 gesture, for `kryptikd clipboard move` and serve: Ok is the line to show. pub fn move_between(from: &str, to: &str) -> Result { use crate::registry::{self, State}; for z in [from, to] { diff --git a/compartments/kryptikd/src/broker/tests.rs b/compartments/kryptikd/src/broker/tests.rs index a5222b51..a8003e67 100644 --- a/compartments/kryptikd/src/broker/tests.rs +++ b/compartments/kryptikd/src/broker/tests.rs @@ -258,8 +258,7 @@ struct Lab { dev: u64, } -/// Zones a (sender), b and c (plain) and n (nic); a destination root with -/// homes for b and c; the lab directory's filesystem as the data mount. +/// Zones a (sender), b, c and n (nic); homes for b and c; the lab's filesystem as data mount. fn lab(tag: &str, to: &str) -> Lab { use std::os::unix::fs::MetadataExt; let dir = entry(&format!("transfer-{tag}")); @@ -302,8 +301,7 @@ fn resolver(root: std::path::PathBuf) -> impl Fn(&str) -> Result } } -/// How many of this process's descriptors point at `p`; tests run in -/// parallel, so a total count would be noise. +/// How many of our descriptors point at `p` (tests run in parallel: a total would be noise). fn fds_pointing_at(p: &Path) -> usize { std::fs::read_dir("/proc/self/fd") .unwrap() @@ -381,8 +379,7 @@ fn transfer_lands_in_incoming() { let _ = std::fs::remove_dir_all(&lab.dir); } -/// One transfer of notes.txt ("hello transfer", 14 bytes) from a descriptor -/// the sender left at byte 6; returns the reply. +/// Transfer notes.txt (14 bytes) from a descriptor the sender left at byte 6; returns the reply. fn send_notes(sv: &Served, file: &Path) -> String { std::fs::write(file, b"hello transfer").unwrap(); let src = open_flags(file, libc::O_RDONLY); @@ -496,8 +493,7 @@ fn transfer_refusals_precede_copy() { let text = String::from_utf8_lossy(&r); assert!(text.starts_with("error: ") && text.contains(want), "request {req:?}: got {text:?}, wanted {want:?}"); } - /* Every refusal closed what it was handed. Checked after the loop, - * since the cases open all their descriptors up front. */ + // Every refusal closed what it was handed; checked here, as the cases open theirs up front. let left = fds_pointing_at(&file) + fds_pointing_at(&lab.dir) + fds_pointing_at(&big); assert_eq!(left, 0, "a refusal leaked a descriptor in the broker"); // A file on another filesystem than the zone's data mount. @@ -511,16 +507,15 @@ fn transfer_refusals_precede_copy() { } else { eprintln!("/dev/shm is on the same filesystem as the lab; the st_dev case is not exercised here"); } - /* Consent and an unknown data mount each refuse on their own. This sets - * the variable consent's tests set, so it takes their lock. */ + // Consent and an unknown data mount each refuse on their own. This sets the variable + // consent's tests set, so it takes their lock. sv.auto_approve = false; { let _env = crate::consent::ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); std::env::set_var("KRYPTIK_CONSENT_DIR", "/nonexistent/kryptik-consent"); let (_, r) = ask_with(&sv, "transfer b f.txt\n", &[ro()], false); assert!(String::from_utf8_lossy(&r).contains("no consent channel"), "{}", String::from_utf8_lossy(&r)); - /* A destination that is not running is refused before any question: - * even with no consent channel, the error is about the destination. */ + // A destination not running is refused before any question, even with no consent channel. sv.resolve_dest = ¬_running; let (_, r) = ask_with(&sv, "transfer b f.txt\n", &[ro()], false); let text = String::from_utf8_lossy(&r); @@ -584,8 +579,7 @@ fn zone_limits_bound_transfer() { #[test] fn dest_resolved_after_consent() { - /* The first lookup finds the zone under `before`; by the answer it runs - * under the lab root, as a zone restarted during the wait would. */ + // Found under `before` first, then under the lab root: a zone restarted during the wait. let lab = lab("again", "b"); let entry_dir = lab.dir.join("entry"); let before = lab.dir.join("before"); @@ -655,8 +649,7 @@ fn dest_resolved_after_consent() { #[test] fn refusal_pauses_questions() { - /* Every question takes focus in zone 0: after a refusal the same launch - * may raise no other for a minute, and is told so without one. */ + // After a refusal the launch raises no question for a minute, and is told so without one. let lab = lab("pause", "b"); let entry_dir = lab.dir.join("entry"); std::fs::create_dir_all(&entry_dir).unwrap(); @@ -719,9 +712,8 @@ fn refusal_pauses_questions() { let second = send(); done.store(true, std::sync::atomic::Ordering::SeqCst); let asked = person.join().unwrap(); - /* Once the pause is over the next request is asked again; with no - * channel, it says so. That asked nobody, so the one right after it is - * not paused: it reaches the channel check again. */ + /* After the pause the next request is asked again (no channel, so it says so); that asked + * nobody, so the one after it is not paused either. */ refused.set(Some(Instant::now())); std::env::set_var("KRYPTIK_CONSENT_DIR", "/nonexistent/kryptik-consent"); let third = send(); @@ -814,8 +806,7 @@ fn parse_request_wire_format() { } } -/// From the nic zone the claim reaches the decision, which refuses for want -/// of a floor; no clock is touched either way. +/// The nic zone's claim reaches the decision (refused: no floor); no clock is touched. #[test] fn only_nic_zone_reports_time() { let claim = crate::time::Claim { offset: 2.0, sources: 3 }; @@ -874,11 +865,8 @@ fn only_nic_zone_brings_update() { assert_eq!(update_refusal(&zone_of("nic", "")), None); } -/// Requests from fuzz-corpus/broker-requests (add any that ever breaks the -/// broker), damaged by a fixed-seed generator and sent down a real -/// connection, each with a fresh descriptor of `attach` when given. No -/// panic, no overrun of the deadline, one well-formed reply, and nothing -/// accepted outside the grammar. Returns (sent, accepted). +/// Serve each fuzz-corpus/broker-requests line damaged by a seeded generator, with a fresh +/// `attach` fd if given: no panic or overrun, one well-formed reply. Returns (sent, accepted). fn fuzz_pass(s: &Served, hello: &str, attach: Option<&Path>) -> (u32, u32) { const CORPUS: &str = include_str!("../../fuzz-corpus/broker-requests"); // xorshift64*: small, seeded, the same sequence everywhere. @@ -961,9 +949,7 @@ fn broker_survives_any_request() { assert!(sent > 2000 && accepted > 50 && accepted < sent, "sent {sent}, accepted {accepted}"); } -/// The same damage past the transfer path's descriptor check: a sender -/// whose policy names b, its data mount the lab's, b running under the lab -/// root, and every request carrying a file of that mount. +/// The same damage on the transfer path: b allowed and running, and a file on every request. #[test] fn broker_survives_any_transfer() { let lab = lab("fuzz", "b"); @@ -997,10 +983,8 @@ fn broker_survives_any_transfer() { assert!(sent > 2000, "sent {sent}"); } -/// The nic zone reaches time-offset's judgement and the update verbs' -/// payloads, and through them this host's clock and update state. As root -/// that would be the real thing, so it runs only unprivileged, where every -/// such write is refused, and with no consent channel for a question. +/// The nic zone's verbs reach this host's clock and update state, so this runs unprivileged +/// only, where every such write is refused, and with no consent channel. #[test] fn broker_survives_any_request_from_the_nic_zone() { if unsafe { libc::geteuid() } == 0 { diff --git a/compartments/kryptikd/src/main.rs b/compartments/kryptikd/src/main.rs index 490bafd9..464c9ebd 100644 --- a/compartments/kryptikd/src/main.rs +++ b/compartments/kryptikd/src/main.rs @@ -1,8 +1,6 @@ -//! kryptikd, the Kryptik compartment manager. -//! -//! Owns zone lifecycle. Runs privileged in zone 0 (ADR-003) and is the only -//! process that creates zones or moves data between them. Anything not built -//! is refused with an error, never a silent no-op. +//! kryptikd, the Kryptik compartment manager: runs privileged in zone 0 (ADR-003) and is the +//! only process that creates zones or moves data between them. Anything not built is refused +//! with an error, never a silent no-op. // cargo fuzz builds this file with cfg(fuzzing), and libFuzzer brings its own main. #![cfg_attr(fuzzing, no_main, allow(dead_code))] @@ -133,9 +131,8 @@ fn main() -> ExitCode { ExitCode::from(2) } }, - /* confine-test ROOTFS TARGET: Landlock-confine to ROOTFS, then read - * TARGET. Exit 0: read succeeded (confinement failed); 4: blocked; - * 1: could not confine. Used by the isolation exit test. */ + /* confine-test ROOTFS TARGET: confine to ROOTFS with Landlock, then read TARGET. Exit 0: + * the read worked, so confinement failed; 4: blocked; 1: could not confine. */ "confine-test" => { let Some(root) = args.get(1) else { eprintln!("confine-test: expected ROOTFS TARGET"); @@ -171,10 +168,7 @@ fn main() -> ExitCode { } } } - /* seccomp-test SYSCALL|PROBE: a forked child installs the zone filter, - * then makes the call (probes: cmd_seccomp_probe). Exit 0: completed; - * 5: SIGSYS; 6: another signal; 7: refused with the intended errno; - * 1: filter not installed. */ + // seccomp-test SYSCALL|PROBE: try it under the zone filter (exit codes: under_zone_filter). "seccomp-test" => { let Some(name) = args.get(1) else { eprintln!("seccomp-test: expected a syscall name"); @@ -197,9 +191,6 @@ fn main() -> ExitCode { } }, "run" => cmd_run(&zone_dir, &args), - /* seccomp-trace [--zone NAME] -- CMD: run CMD under the base filter, or - * NAME's widened by its policy file, and name every call it refuses - * (cmd_seccomp_trace). For writing a policy file. */ "seccomp-trace" => { let Some(sep) = args.iter().position(|a| a == "--") else { eprintln!("seccomp-trace: expected `-- COMMAND`"); @@ -262,9 +253,8 @@ fn main() -> ExitCode { } } -/// Stop a running zone by signalling its launcher, which forwards the signal -/// to pid 1 and escalates to SIGKILL after 5 s. The launcher is signalled only -/// while its pid and start time both still match: pids are reused. +/// Stop a zone; its launcher forwards the signal to pid 1 and escalates to SIGKILL after 5 s. +/// The launcher is signalled only while its pid and start time both match: pids are reused. fn cmd_stop(name: &str, now: bool, base: &str) -> ExitCode { let mut st = match registry::state(name) { Ok(s) => s, @@ -274,8 +264,7 @@ fn cmd_stop(name: &str, now: bool, base: &str) -> ExitCode { } }; - /* "Still starting" lasts a few ms, between claim() and the fork that - * records the launcher pid; `run &` then `stop` often lands in it. */ + // "Still starting" lasts a few ms after claim(); `run &` then `stop` often lands in it. if matches!(st, registry::State::Running { launcher: None, .. }) { std::thread::sleep(std::time::Duration::from_millis(100)); st = match registry::state(name) { @@ -317,10 +306,8 @@ fn cmd_stop(name: &str, now: bool, base: &str) -> ExitCode { println!("zone {name:?} exited while stopping it"); return ExitCode::SUCCESS; } - /* --now kills the zone's pid 1, which takes the pid namespace with - * it, not the launcher: the launcher outlives its zone to unmount - * and close the zone's volume. During setup pid 1 may not yet be - * recorded; signal the launcher gracefully in that case. */ + /* --now kills pid 1 and so the pid namespace, not the launcher, which outlives the + * zone to close its volume. Before pid 1 is recorded the launcher gets SIGTERM. */ let (pid, sig) = match init.filter(|i| now && i.still_alive()) { Some(i) => (i.pid, libc::SIGKILL), None => (l.pid, libc::SIGTERM), @@ -357,9 +344,7 @@ fn cmd_stop(name: &str, now: bool, base: &str) -> ExitCode { } } -/// Close the volume of a zone whose launcher died without closing it, as -/// `gc` does for every zone: otherwise the plaintext stays mounted and its -/// key in the kernel after the zone is reported stopped. +/// Close a volume a dead launcher left open, as `gc` does, so no plaintext outlives a stop. fn close_left_volume(name: &str, base: &str) { if !volume::mappings().iter().any(|z| z == name) || matches!(registry::state(name), Ok(registry::State::Running { .. })) { return; @@ -371,9 +356,8 @@ fn close_left_volume(name: &str, base: &str) { } } -/// Cross-zone paste, a zone 0 gesture: no zone's socket has a verb to fetch -/// another zone's payload. Both zones must be running; the payload lives in -/// one zone's registry entry at a time. +/// Cross-zone paste, a zone 0 gesture: no zone's socket has a verb to fetch another's payload, +/// which lives in one zone's registry entry at a time. fn cmd_clipboard(args: &[String]) -> ExitCode { let (from, to) = match (args.get(1).map(|s| s.as_str()), args.get(2), args.get(3)) { (Some("move"), Some(f), Some(t)) if !f.starts_with("--") && !t.starts_with("--") => (f.as_str(), t.as_str()), @@ -413,8 +397,8 @@ fn cmd_status(name: &str) -> ExitCode { ExitCode::SUCCESS } Ok(registry::State::Running { launcher, init, cgroup, started }) => { - /* core-sched is asked of the kernel (nothing in /proc shows it): own, - * none, no-smt (no sibling thread online) or unavailable. */ + // core-sched is asked of the kernel, as nothing in /proc shows it: own, none, no-smt or + // unavailable. println!( "{name} running launcher {} init {} since {}{}{}", launcher.map(|l| l.pid.to_string()).unwrap_or_else(|| "starting".into()), @@ -542,8 +526,7 @@ fn cmd_check(dir: &Path, target: bool) -> ExitCode { None => println!(" landlock NO"), } - /* Install the filter in a child and make an allowed call. A self-test that - * cannot run is a failure: this command gates a kernel as fit for zones. */ + // A self-test that cannot run is a failure: this command gates a kernel as fit for zones. let self_test = std::env::current_exe() .map_err(|e| format!("current_exe: {e}")) .and_then(|exe| { @@ -566,8 +549,7 @@ fn cmd_check(dir: &Path, target: bool) -> ExitCode { } } - /* On the target only CAP_SYS_ADMIN in the initial namespace may create a - * user namespace. Read the knob, then prove it by trying. */ + // On the target only CAP_SYS_ADMIN may create a user namespace: read the knob, then try. let knob = isolate::userns_restriction_sysctl(); match isolate::probe_userns_restriction() { Ok(true) => println!( @@ -650,8 +632,7 @@ fn cmd_check(dir: &Path, target: bool) -> ExitCode { } } } - /* Parsed as the launcher parses it (spawn.rs), so a file that - * would stop a launch fails the check. */ + // Parsed as spawn.rs does, so a file that would stop a launch fails the check. if let Some(rel) = &z.landlock { let path = policy::resolve(dir, rel); match std::fs::read_to_string(&path) @@ -787,8 +768,7 @@ fn cmd_explain(dir: &Path, name: &str, args: &[String]) -> ExitCode { ExitCode::SUCCESS } -/// Argument-rule and soft-refusal probes for `seccomp-test`; None when `name` -/// is not a probe. +/// Argument-rule and soft-refusal probes for `seccomp-test`; None if `name` is not one. fn cmd_seccomp_probe(name: &str) -> Option { // Runs in the filtered child: 7 if refused with the intended errno, else 0. let probe: fn() -> i32 = match name { @@ -874,8 +854,8 @@ fn run_options_from(args: &[String]) -> Result { }) } -/// An encrypted zone's LUKS2 container, outside any launch -/// (docs/design/encrypted-volumes.md). init and passwd need root. +/// An encrypted zone's LUKS2 container, outside any launch (docs/design/encrypted-volumes.md). +/// init, passwd and destroy need root. /// /// volume init NAME [--size 512M] --passphrase-file F [--zone-uid N --zone-gid N] /// volume passwd NAME --passphrase-file OLD --new-passphrase-file NEW @@ -1058,9 +1038,7 @@ fn cmd_time(args: &[String]) -> ExitCode { } } -/// `kryptikd wifi list | add SSID | forget SSID`: the net zone's Wi-Fi networks, -/// for root and the tests (the session goes through the launch daemon). The -/// passphrase is one line on stdin, never an argument. +/// The net zone's Wi-Fi networks, for root and the tests; a passphrase comes on stdin, never argv. fn cmd_wifi(zones_dir: &Path, args: &[String]) -> ExitCode { let dir = wifi_dir_from(args); let sub = args.get(1).map(String::as_str).unwrap_or(""); @@ -1183,10 +1161,8 @@ fn trace_filter(dir: &Path, name: &str) -> Result<(Vec, seccomp::S Ok((seccomp::widened(&p.extra_syscalls).map_err(|e| e.to_string())?, p.sockets)) } -/* Run CMD under a zone filter and name every call it refuses. Refused calls - * come here by seccomp user notification (Kryptik forbids ptrace) and fail - * with ENOSYS, so one run lists them all. The child shares our descriptor - * table until exec: the zone filter has no sendmsg to pass its listener. */ +/// Run CMD under a zone filter, naming every call it refuses. Refusals arrive by user +/// notification (Kryptik forbids ptrace) and fail with ENOSYS, so one run lists them all. fn cmd_seccomp_trace(cmd: &[String], allow: &[libc::c_long], sockets: &seccomp::SocketPolicy) -> ExitCode { use std::ffi::CString; @@ -1205,7 +1181,7 @@ fn cmd_seccomp_trace(cmd: &[String], allow: &[libc::c_long], sockets: &seccomp:: return ExitCode::FAILURE; } - // A fork that shares the descriptor table; the child's exec unshares it. + // Shares the descriptor table until exec: the zone filter has no sendmsg to pass the listener. let pid = unsafe { libc::syscall(libc::SYS_clone, libc::CLONE_FILES | libc::SIGCHLD, 0, 0, 0, 0) } as libc::pid_t; if pid < 0 { eprintln!("seccomp-trace: clone: {}", std::io::Error::last_os_error()); @@ -1291,9 +1267,8 @@ fn cmd_seccomp_test(name: &str, nr: libc::c_long) -> ExitCode { }) } -/// Run `probe` in a child under the zone filter and say how it ended. Exit 5: -/// killed by SIGSYS; 6: another signal; 7: refused with the intended errno; -/// 1: no filter; 0: completed. +/// Run `probe` in a child under the zone filter. Exit 5: killed by SIGSYS; 6: another signal; +/// 7: refused with the intended errno; 1: no filter; 0: completed. fn under_zone_filter(name: &str, probe: impl FnOnce() -> i32) -> ExitCode { // SAFETY: fork in a program that does no threading before this point. let pid = unsafe { libc::fork() }; diff --git a/compartments/kryptikd/src/serve.rs b/compartments/kryptikd/src/serve.rs index 6495b834..bab41eb0 100644 --- a/compartments/kryptikd/src/serve.rs +++ b/compartments/kryptikd/src/serve.rs @@ -1,8 +1,5 @@ -//! `kryptikd serve`: the launch daemon the desktop session talks to. -//! -//! Zones are created by root and the session is an ordinary user; this socket -//! is the one door between them. The daemon checks the peer (SO_PEERCRED) and -//! the request, then runs `kryptikd run`, whose every refusal still applies. +//! `kryptikd serve`: the root daemon the unprivileged desktop session asks to launch zones. +//! It checks the peer (SO_PEERCRED) and the request; `kryptikd run` still applies its refusals. //! //! socket /run/kryptik-launch/launch.sock (root:kryptik 0660) //! request one per connection, NUL-free text lines: @@ -59,8 +56,7 @@ fn clear_cloexec(fd: RawFd) { } } -/// One recvmsg into `buf`, owning the descriptors that came with it. Control -/// data cut short is refused, and what did arrive is closed. +/// One recvmsg into `buf`, owning any descriptors; truncated control data is refused. pub fn recv_with_fds(fd: RawFd, buf: &mut [u8], flags: libc::c_int) -> std::io::Result<(usize, Vec)> { // Aligned for cmsghdr, with room for twelve, so an excess is refused by count. #[repr(C, align(8))] @@ -117,8 +113,7 @@ fn gid_of_uid(uid: u32) -> Option { } } -/// Is `uid` root, or a member (primary or supplementary) of `group`? Member -/// names are copied out first in case getpwuid(3) reuses getgrnam(3)'s buffer. +/// Is `uid` root, or a member (primary or supplementary) of `group`? fn in_group(uid: u32, group: &str) -> bool { if uid == 0 { return true; @@ -129,6 +124,7 @@ fn in_group(uid: u32, group: &str) -> bool { return false; } let gid = unsafe { (*g).gr_gid }; + // Copied out first: getpwuid(3) may reuse getgrnam(3)'s buffer. let mut members: Vec = Vec::new(); let mut mem = unsafe { (*g).gr_mem }; unsafe { @@ -161,8 +157,7 @@ pub fn peer_cred(fd: RawFd) -> std::io::Result { // --- the request ------------------------------------------------------------- -/// Whether the bytes so far are a whole request. `run`, `wifi-add` and -/// `wifi-forget` end at an `end` line, other verbs at a newline. +/// `run`, `wifi-add` and `wifi-forget` end at an `end` line, other verbs at a newline. fn request_complete(text: &[u8]) -> bool { if !text.ends_with(b"\n") { return false; @@ -174,8 +169,7 @@ fn request_complete(text: &[u8]) -> bool { } } -/// Read the request text and any descriptor that came with it, within -/// `deadline`. At most one descriptor, and only with the first bytes. +/// Read the request, and at most one descriptor (with its first bytes), within `deadline`. fn recv_request(fd: RawFd, deadline: Instant) -> Result<(Vec, Option), String> { let mut text = Vec::new(); let mut carried: Option = None; @@ -294,8 +288,7 @@ impl std::fmt::Debug for WifiRequest { } } -/// Parse `wifi-add` or `wifi-forget`. A value is all after the first space; a -/// refusal never repeats a line, which may hold the passphrase. +/// Parse `wifi-add` or `wifi-forget`. A refusal never repeats a line: it may hold the passphrase. fn parse_wifi(text: &str) -> Result { let mut lines = text.lines(); let verb = lines.next().unwrap_or(""); @@ -354,9 +347,7 @@ fn openat_component(dir: RawFd, name: &str, flags: libc::c_int) -> Result Result { if !path.is_absolute() { return Err(format!("{}: not an absolute path", path.display())); @@ -430,9 +420,8 @@ pub fn open_nofollow(path: &Path, want_socket: bool) -> Result Ok(dir) } -/// Accept only `/run/user//kryptik//wayland-0`, walked without -/// following symlinks. `/`, `kryptik/`, `/` and the socket must be -/// the session's, the last two directories private, and the listener its proxy. +/// Accept only `/run/user//kryptik//wayland-0`, walked without following symlinks: +/// the session's from `/` down, `kryptik/` and `/` private, its proxy listening. fn verify_proxy_socket(p: &Path, uid: u32, zone: &str, proxy_exe: Option<&Path>) -> Result { let want = PathBuf::from(format!("/run/user/{uid}/kryptik/{zone}/wayland-0")); if p != want { @@ -467,14 +456,10 @@ fn verify_proxy_socket(p: &Path, uid: u32, zone: &str, proxy_exe: Option<&Path>) Ok(ProxySocket { _fd: sock, path: want, inode: InodeId::of(&st) }) } -/// Ask the kernel who listens on the socket's inode. SO_PEERCRED names the -/// caller of listen(), which must be the session's uid running kryptik-wlproxy -/// for this zone (the probe shows in its log as a client disconnect). A process -/// handed the socket later goes unseen, but only that uid could hand it over, -/// and it reaches the compositor directly anyway. +/// SO_PEERCRED names whoever called listen(), which must be the session's kryptik-wlproxy for +/// this zone. Only the session could pass the socket on, and it reaches the compositor anyway. fn verify_proxy_listener(sock: &OwnedFd, uid: u32, zone: &str, proxy_exe: Option<&Path>) -> Result<(), String> { - /* Non-blocking: the listener is the session's and may never accept; a - * full backlog must refuse at once, not block the root daemon. */ + // Non-blocking: a full backlog on the session's listener must not block the root daemon. let s = unsafe { libc::socket(libc::AF_UNIX, libc::SOCK_STREAM | libc::SOCK_CLOEXEC | libc::SOCK_NONBLOCK, 0) }; if s < 0 { return Err(format!("socket: {}", std::io::Error::last_os_error())); @@ -524,8 +509,7 @@ struct Launch { log: PathBuf, } -/// Start `kryptikd run` for the request; returns the readiness pipe's read end. -/// The passphrase descriptor goes to the child; ours all close on every path. +/// Start `kryptikd run` for the request; our copies of its descriptors close on every path. fn spawn_launcher( req: &Request, cfg: &ServeConfig, @@ -546,9 +530,8 @@ fn spawn_launcher( "--ready-fd".into(), ready_w.as_raw_fd().to_string(), ]; - /* The path, not a descriptor: one opened in this mount namespace cannot be - * bind-mounted from the zone's (EINVAL). The launcher reopens the path and - * refuses any other inode (spawn.rs, StagedSocket). */ + /* The path, not a descriptor: one opened in this mount namespace cannot be bind-mounted + * from the zone's (EINVAL). The launcher refuses any other inode (spawn.rs, StagedSocket). */ if let Some(w) = &wayland { args.push("--wayland-socket".into()); args.push(w.path.display().to_string()); @@ -569,9 +552,7 @@ fn spawn_launcher( .chain(args.iter().map(|a| CString::new(a.as_str()).unwrap_or_else(|_| CString::new("?").unwrap()))) .collect(); let path = CString::new("PATH=/usr/bin:/usr/sbin").unwrap(); - /* A developer instance's registry is under XDG_RUNTIME_DIR (registry::base), - * and its launcher must use the same one or `status` and `stop` would not - * find the zone. Nothing else of the environment crosses. */ + // Of our environment only XDG_RUNTIME_DIR crosses: a developer launcher's registry is under it. let runtime_dir = if unsafe { libc::geteuid() } != 0 { std::env::var("XDG_RUNTIME_DIR").ok().and_then(|v| CString::new(format!("XDG_RUNTIME_DIR={v}")).ok()) } else { @@ -691,9 +672,8 @@ impl Pending { } } -/// A slow request (`stop`, `update-apply`), answered when `reap` sees its -/// command end so the daemon keeps serving meanwhile. Not a thread: `reap` -/// collects every child and would take the status a thread waited for. +/// A slow request (`stop`, `update-apply`), answered once `reap` sees its command end. Not a +/// thread: `reap` collects every child and would take the status a thread waited for. struct Job { conn: UnixStream, pid: libc::pid_t, @@ -731,7 +711,6 @@ fn start_job(conn: UnixStream, what: JobKind, cmd: &mut std::process::Command) - _ => return Err((conn, "could not hold the command's output".into())), }; match cmd.stdin(std::process::Stdio::null()).stdout(o2).stderr(e2).spawn() { - // The Child is dropped without a wait: `reap` collects it. Ok(child) => Ok(Job { conn, pid: child.id() as libc::pid_t, what, out, err, exited: None }), Err(e) => Err((conn, e.to_string())), } @@ -789,8 +768,7 @@ fn reply(mut c: &UnixStream, text: &str) { let _ = c.flush(); } -/// The write end of the pipe SIGCHLD writes to, so a launcher or job that -/// ends is reaped at once rather than at the next request. +/// Write end of the SIGCHLD wake pipe, so a launcher or job that ends is reaped at once. static CHILD_WAKE: std::sync::atomic::AtomicI32 = std::sync::atomic::AtomicI32::new(-1); extern "C" fn on_child(_sig: libc::c_int) { @@ -823,8 +801,7 @@ fn reap(pending: &mut [Pending], jobs: &mut [Job]) { } } -/// Wait up to 500 ms for a launcher's status once its pipe has closed: the -/// close can reach us before the exit does. +/// Wait up to 500 ms for a launcher's status after its pipe closed: the close can come first. fn wait_exit(p: &mut Pending) -> Option { if p.exited.is_some() { return p.exited; @@ -925,8 +902,8 @@ fn bind(cfg: &ServeConfig) -> Result { } let gid = gid_of_group(&cfg.group).ok_or_else(|| format!("no group {:?}; nobody could connect", cfg.group))?; let dir = cfg.socket.parent().map(Path::to_path_buf).unwrap_or_else(|| PathBuf::from(SOCKET_DIR)); - /* Only the default directory is made root:kryptik 0750, so the group alone - * reaches the socket; a directory named with --socket is left as found. */ + // Only the default directory becomes root:kryptik 0750, so the group alone reaches the socket; + // a --socket one is left as found. if dir == Path::new(SOCKET_DIR) { let _ = std::fs::create_dir_all(&dir); let _ = std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o750)); @@ -942,8 +919,7 @@ fn bind(cfg: &ServeConfig) -> Result { Ok(l) } -/// Create the session's /run/user/ (0700, the user's). With no logind, -/// the daemon is the one root process the session can ask. +/// Create the session's /run/user/ (0700, the user's): with no logind, nothing else can. fn runtime_dir(cfg: &ServeConfig, uid: u32) -> Result { let dir = if cfg.developer { cfg.log_dir.join(format!("run-user-{uid}")) @@ -986,8 +962,7 @@ fn zone_running(name: &str) -> bool { matches!(crate::registry::state(name), Ok(crate::registry::State::Running { .. })) } -/// Serve one connection. A started launch is returned to be watched and a -/// slow command pushed to `jobs`; anything else is answered here. +/// Serve one connection; a started launch is returned and a slow command pushed to `jobs`. fn handle(cfg: &ServeConfig, conn: UnixStream, jobs: &mut Vec) -> Option { let fd = conn.as_raw_fd(); let Ok(peer) = peer_cred(fd) else { diff --git a/compartments/kryptikd/src/update.rs b/compartments/kryptikd/src/update.rs index 82ca7c8e..f25f2059 100644 --- a/compartments/kryptikd/src/update.rs +++ b/compartments/kryptikd/src/update.rs @@ -1,10 +1,5 @@ -//! The update channel's rules (docs/design/update-channel.md): which pointers -//! zone 0 accepts, and which bytes it takes from the net zone for a release -//! the user asked for. -//! -//! Signatures are checked by `kryptik-update` (`Checks`), and only verified -//! text reaches the parsers. Decided here: role, replay, staleness, and that -//! each staged byte is one the signed manifest provides for, at its place. +//! Zone 0's side of the update channel (docs/design/update-channel.md): which pointers it accepts +//! and which bytes it stages; signatures are checked first, by `kryptik-update` (`Checks`). use std::cmp::Ordering; use std::io::Write as _; @@ -12,14 +7,13 @@ use std::os::unix::fs::OpenOptionsExt; use std::path::{Path, PathBuf}; pub const POINTER_MAGIC: &str = "KRYPTIK-LATEST-1"; -/// The pointer and its signature, each. +/// Byte limit for the pointer and for its signature. pub const POINTER_MAX: usize = 8 * 1024; -/// The manifest and its signature, each. +/// Byte limit for the manifest and for its signature. pub const MANIFEST_MAX: u64 = 64 * 1024; /// Past this age a pointer is reported stale: it is re-issued on a schedule. pub const STALE_AFTER_SECS: i64 = 30 * 86400; -/// How far ahead of this machine's clock a statement may be dated, allowing -/// for skew. One dated later would make every genuine statement a replay. +/// Skew allowed in a statement's date; one dated later would make every genuine one a replay. pub const MAX_AHEAD_SECS: i64 = 86400; /// One pointer is considered per hour; the rest are refused unread. pub const POINTER_INTERVAL_SECS: u64 = 3600; @@ -39,8 +33,7 @@ fn is_version(s: &str) -> bool { !s.is_empty() && s.len() <= 32 && s.bytes().all(|b| b.is_ascii_alphanumeric() || matches!(b, b'.' | b'+' | b'-' | b'~')) } -/// The pointer's text: the magic line, then each key exactly once. An unknown -/// key is refused, so an older system never half-understands a newer format. +/// The magic line, then each key once; an unknown key is refused, never half-understood. pub fn parse_pointer(text: &str) -> Result { let mut lines = text.lines(); if lines.next() != Some(POINTER_MAGIC) { @@ -77,9 +70,8 @@ pub fn parse_pointer(text: &str) -> Result { Ok(Pointer { role, version, issued, manifest_sha256: sha, base }) } -/// Where a release's files are fetched from: an absolute base as is, a relative -/// one under the `CONF` channel address, never under what the net zone reports. -/// Plain http only for a development image, a privacy matter: the hash is signed. +/// A release's URL: an absolute base as is, a relative one under the `CONF` channel, never under +/// what the net zone reports. pub fn resolve_base(channel: &str, base: &str, role: &str) -> Result { let mut url = if base.contains("://") { base.to_string() @@ -94,6 +86,7 @@ pub fn resolve_base(channel: &str, base: &str, role: &str) -> Result Ok(url), + // Plain http costs privacy, not integrity: the hash is signed. Some("http") if role == "development" => Ok(url), Some("http") => Err("a production image does not fetch over plain http".into()), _ => Err(format!("{url:?} is neither https nor http")), @@ -126,17 +119,15 @@ pub fn version_cmp(a: &str, b: &str) -> Ordering { (a.len() - i).cmp(&(b.len() - j)) } -/// What an accepted pointer says about this machine. +/// Where this machine stands against an accepted pointer. #[derive(Debug, PartialEq)] pub enum Standing { - /// It names the running release, or an older one. + /// The pointer's release is the running one, or older. Current, Available(String), } -/// Whether zone 0 accepts a verified pointer: it must be for this image's role, -/// not issued before the newest already accepted (a replay), and not dated more -/// than `MAX_AHEAD_SECS` after `now`. +/// Accept a verified pointer for this image's role unless it is a replay or dated too far ahead. pub fn accept_pointer(p: &Pointer, required_role: &str, running: &str, newest_issued: Option, now: i64) -> Result { if p.role != required_role { return Err(format!("the pointer's role is '{}'; this image requires '{required_role}'", p.role)); @@ -152,8 +143,7 @@ pub fn accept_pointer(p: &Pointer, required_role: &str, running: &str, newest_is Ok(if version_cmp(&p.version, running) == Ordering::Greater { Standing::Available(p.version.clone()) } else { Standing::Current }) } -/// How many whole days old the newest accepted pointer is, and whether that -/// is past the bound. A clock behind the pointer reads as zero days. +/// Whole days since `issued`, and whether that is stale; a clock behind it reads as zero. pub fn staleness(now: i64, issued: i64) -> (i64, bool) { let age = (now - issued).max(0); (age / 86400, age > STALE_AFTER_SECS) @@ -166,8 +156,7 @@ pub struct Entry { pub size: u64, } -/// Parses `check-manifest`'s output: `file ` lines, others -/// ignored. Names must pass the broker's transfer-name check. +/// Parse `check-manifest`'s `file ` lines; names must pass the broker's check. pub fn parse_file_list(text: &str) -> Result, String> { let mut out: Vec = Vec::new(); for line in text.lines() { @@ -190,10 +179,9 @@ pub fn total_bytes(files: &[Entry]) -> u64 { files.iter().fold(0, |sum, e| sum.saturating_add(e.size)) } -/// Whether `len` bytes for `name` at `offset` may be written, `held` being -/// there already. Until the manifest verifies (`files` is `None`) only it and -/// its signature are taken, whole and small; then only listed names, at exactly -/// `held` (resumable, never out of order), never past the signed size. +/// Whether `len` bytes of `name` may be written at `offset`, with `held` already there. +/// Until the manifest verifies, only it and its signature, whole and small; then listed files, +/// appended in order, never past the signed size. pub fn may_put(files: Option<&[Entry]>, name: &str, offset: u64, len: u64, held: u64) -> Result<(), String> { if len == 0 { return Err("nothing to put".into()); @@ -219,7 +207,7 @@ pub fn may_put(files: Option<&[Entry]>, name: &str, offset: u64, len: u64, held: Ok(()) } -/// What is still missing and from which byte: the answer to `update-poll`. +/// Each missing file and the byte it resumes at, for `update-poll`. pub fn still_needed(files: &[Entry], held: impl Fn(&str) -> u64) -> Vec<(String, u64)> { files.iter().filter_map(|e| { let h = held(&e.name); (h < e.size).then(|| (e.name.clone(), h)) }).collect() } @@ -236,20 +224,17 @@ pub fn still_needed(files: &[Entry], held: impl Fn(&str) -> u64) -> Vec<(String, * * Functions take the directory and the checks, so tests supply their own. */ - - pub const STATE_DIR: &str = "/var/lib/kryptik/update"; pub const TOOL: &str = "/usr/sbin/kryptik-update"; pub const ROLE_FILE: &str = "/usr/share/kryptik/trust/required-role"; pub const CONF: &str = "/etc/kryptik/update.conf"; -/// The most one `update-put` carries. The launcher answers each between two -/// looks at its zone, so supervision is never more than one piece away. +/// Largest `update-put`, so the launcher never goes long without checking its zone. pub const PUT_MAX: usize = 1 << 20; -/// The signature checks, as functions so a test can stand in for -/// `kryptik-update`. `manifest` returns what `check-manifest` printed. +/// The signature checks, as functions so a test can stand in for `kryptik-update`. pub struct Checks<'a> { pub pointer: &'a dyn Fn(&Path, &Path) -> Result<(), String>, + /// Returns what `check-manifest` printed. pub manifest: &'a dyn Fn(&Path) -> Result, } @@ -268,8 +253,7 @@ fn run_tool(args: &[&std::ffi::OsStr]) -> Result { Err(err.lines().last().unwrap_or("refused").trim_start_matches("kryptik-update: ").to_string()) } -/// The checks the installed system uses: `kryptik-update`, with the trust -/// anchor on the verified root and nothing from this process's environment. +/// The installed system's checks: `kryptik-update`, run with a cleared environment. pub fn tool_checks() -> Checks<'static> { Checks { pointer: &|p, s| run_tool(&["check-pointer".as_ref(), p.as_os_str(), s.as_os_str()]).map(|_| ()), @@ -300,9 +284,8 @@ pub fn running_version() -> String { text.lines().find_map(|l| l.strip_prefix("VERSION_ID=")).map(|v| v.trim_matches('"').to_string()).unwrap_or_default() } -/// `channel =
` from `CONF`. A copy written under /etc does not -/// survive the next boot (sysinit.sh, `prune_etc_upper`), and the address is -/// only where to ask: answers are believed on the trust anchor's signature. +/// `channel =
` from `CONF`, the image's own: sysinit prunes a copy written under /etc. +/// The address is only where to ask; answers stand on their signatures. pub fn channel_from(conf: &str) -> Option { conf.lines().find_map(|l| { let (k, v) = l.split_once('=')?; @@ -341,9 +324,7 @@ fn verified_files(dir: &Path, version: &str) -> Option> { (text.lines().next() == Some(&format!("version: {version}"))).then(|| parse_file_list(&text).ok()).flatten() } -/// `update-latest`: a pointer and its signature from the net zone. One is -/// looked at per interval, whatever its fate, so a hostile zone cannot keep -/// zone 0 verifying signatures. +/// `update-latest`: one pointer per interval, so a hostile zone cannot keep zone 0 verifying. pub fn latest(dir: &Path, checks: &Checks, now: i64, role: &str, running: &str, pointer: &[u8], sig: &[u8]) -> Result { private_dir(dir)?; let last: Option = std::fs::read_to_string(dir.join("considered")).ok().and_then(|s| s.trim().parse().ok()); @@ -377,6 +358,7 @@ pub fn want(dir: &Path, channel: Option<&str>, role: &str, running: &str) -> Res return Err(format!("{} is the newest release known, and this machine runs {running}", p.version)); } let channel = channel.ok_or_else(|| format!("this image names no update channel ({CONF})"))?; + // Refused here if this image would not fetch it: the poll would only say `idle`. resolve_base(channel, &p.base, role).map_err(|e| format!("{} cannot be fetched: {e}", p.version))?; if wanted(dir).as_deref() != Some(p.version.as_str()) { let _ = std::fs::remove_file(dir.join("files")); @@ -460,9 +442,7 @@ fn free_bytes(path: &Path) -> Option { (unsafe { libc::statvfs(c.as_ptr(), &mut st) } == 0).then(|| st.f_bavail as u64 * st.f_frsize as u64) } -/// `update-put`: bytes for the wanted release, under `may_put`'s rule. Once -/// both manifest and signature are in, they must verify, match the pointer's -/// hash and fit the free space before any file they list is accepted. +/// `update-put`: bytes for the wanted release, under `may_put`'s rule. pub fn put(dir: &Path, checks: &Checks, now: i64, name: &str, offset: u64, bytes: &[u8]) -> Result { let version = wanted(dir).ok_or("no release has been asked for")?; let p = stored_pointer(dir).filter(|p| p.version == version).ok_or("the release asked for is not the one the newest statement names")?; @@ -487,6 +467,7 @@ pub fn put(dir: &Path, checks: &Checks, now: i64, name: &str, offset: u64, bytes if held(&stage, "manifest") == 0 || held(&stage, "manifest.sig") == 0 { return Ok(format!("{name} complete")); } + // The pair is in: it must verify, match the pointer's hash and fit the free space. let refuse = |why: String| -> Result { let _ = std::fs::remove_dir_all(&stage); Err(why) @@ -579,7 +560,7 @@ pub fn complete_stage(dir: &Path) -> Result { } } -/// Once the machine runs what was staged, the staging area has no job. +/// Clear the staged release once the machine runs it or a newer one. pub fn forget_if_installed(dir: &Path, running: &str) { if wanted(dir).is_some_and(|v| version_cmp(&v, running) != Ordering::Greater) { for f in ["wanted", "files"] { diff --git a/compartments/kryptikd/src/update/tests.rs b/compartments/kryptikd/src/update/tests.rs index 91c76ce4..c7a59354 100644 --- a/compartments/kryptikd/src/update/tests.rs +++ b/compartments/kryptikd/src/update/tests.rs @@ -133,7 +133,7 @@ fn poll_names_missing_files_and_offsets() { assert!(still_needed(&files(), |_| u64::MAX).is_empty()); } -// --- the state, against a directory of the test's own --- +// --- the state, in a scratch directory per test --- fn scratch(tag: &str) -> PathBuf { let d = std::env::temp_dir().join(format!("kryptik-update-test-{}-{tag}", std::process::id())); diff --git a/compartments/kryptikd/src/zone.rs b/compartments/kryptikd/src/zone.rs index 0acd60e9..7c489938 100644 --- a/compartments/kryptikd/src/zone.rs +++ b/compartments/kryptikd/src/zone.rs @@ -1,7 +1,5 @@ -//! Zone definitions: parsing and validation. -//! -//! A hand-written parser rather than a TOML crate, to keep dependencies to -//! `libc` (ADR-010). A malformed file is an error, never a weaker zone. +//! Zone definitions, parsed by hand to keep dependencies to `libc` (ADR-010). +//! A malformed file is an error, never a weaker zone. use std::collections::HashMap; use std::fmt; @@ -42,8 +40,7 @@ pub enum StorageMode { Encrypted, /// tmpfs overlay, destroyed at teardown. Ephemeral, - /// A plain directory on the host filesystem, kept between launches. Not - /// encrypted at rest, and every report of this mode says so. + /// A plain host directory kept between launches; not encrypted at rest, and reports say so. Persistent, } @@ -67,8 +64,7 @@ pub struct Zone { pub name: String, pub description: String, pub network: NetworkMode, - /// Interface a `nic` zone takes (`nic = "eth0"`), or `"*"` for every - /// physical one (`netzone::physical_interfaces`). Refused for other modes. + /// The `nic` zone's interface (`nic = "eth0"`), or `"*"` for every physical one. pub nic: Option, /// `[network] local = true`: the nic zone lets this routed zone reach the /// networks its uplinks sit on; every other routed zone is refused them @@ -78,31 +74,25 @@ pub struct Zone { pub volume: Option, pub seccomp: Option, pub landlock: Option, - /// Upper bound on an ephemeral zone's tmpfs: required for ephemeral, - /// refused otherwise. Unbounded, a zone could fill host memory with files. + /// Required bound on an ephemeral zone's tmpfs, which could otherwise fill host memory. pub size: Option, pub memory_max: Option, pub pids_max: Option, - /// A share of CPU time as a percentage of one CPU (`"150%"`), enforced by - /// cgroup cpu.max; and bytes per second each way on the zone's volume, by - /// io.max, so only an encrypted zone may set it. + /// A percentage of one CPU (`"150%"`), enforced by cgroup cpu.max. pub cpu_max: Option, + /// Bytes per second each way on the zone's volume (io.max), so encrypted zones only. pub io_max: Option, pub border_color: String, - /// Identity without colour: `glyph` and `label` name the zone in the - /// chrome's menu, whose f names the last zone window's. Nothing draws - /// `border_pattern`; it is only checked here, and `zoneid audit` gives it - /// no weight. + /// Never drawn: only checked here, and `zoneid audit` gives it no weight. pub border_pattern: Option, + /// Glyph and label identify the zone without colour, in the chrome menu. pub glyph: Option, pub label: Option, - /// Host identity range, `[identity] uid_base = N`: a privileged launch maps - /// root to N and nobody to N + 65534. Declared, not derived from zone order, - /// so adding a zone never changes who owns another's files. `None`: a root - /// launch needs `--zone-uid/--zone-gid`, and `check --target` refuses it. + /// Host uid the zone's root maps to (nobody to it + 65534); declared, not derived from zone + /// order, so adding a zone never changes who owns another's files. None: a root launch needs + /// `--zone-uid/--zone-gid`, and `check --target` refuses it. pub uid_base: Option, - /// `[transfer] to = "work personal"`: zones this one may send files to via - /// the broker. Never the nic zone (`check_invariants`). + /// `[transfer] to = "work personal"`: zones this one may send files to; never the nic zone. pub transfer_to: Vec, /// `[transfer] max_bytes = N`: the largest file this zone sends or /// receives through the broker, at most its cap. None: the cap alone. @@ -148,8 +138,7 @@ pub const KNOWN_KEYS: &[&str] = &[ "ui.border_color", "ui.border_pattern", "ui.glyph", "ui.label", ]; -/// A CPU limit as a zone file and cgroup cpu.max take it: a percentage of one -/// CPU, digits then `%`, at least 1. None for anything else. +/// A percentage of one CPU, digits then `%`, at least 1; None for anything else. pub fn parse_cpu_max(s: &str) -> Option { let digits = s.strip_suffix('%')?; if digits.is_empty() || !digits.bytes().all(|b| b.is_ascii_digit()) { @@ -158,9 +147,7 @@ pub fn parse_cpu_max(s: &str) -> Option { digits.parse::().ok().filter(|n| *n > 0) } -/// A byte size as a zone file, cgroup memory.max and `volume init --size` -/// take it: digits with an optional K, M, G or T. None for zero, for -/// anything else (so not "max": omit the key), and on overflow. +/// Bytes as digits with an optional K, M, G or T; None for zero, "max", other text or overflow. pub fn parse_size(s: &str) -> Option { let (digits, shift) = match s.as_bytes().last()? { b'K' | b'k' => (&s[..s.len() - 1], 10), @@ -175,8 +162,8 @@ pub fn parse_size(s: &str) -> Option { digits.parse::().ok().filter(|&n| n > 0)?.checked_mul(1 << shift) } -/// Minimal TOML reader: `[section]`, `key = value` (quoted string, integer or -/// boolean) and `#` comments. Arrays and nested tables are errors, not ignored. +/// Flat TOML: `[section]`, `key = value` (quoted string, integer or boolean) and `#` comments; +/// arrays and nested tables are errors, not ignored. fn parse_flat_toml(text: &str) -> Result, ZoneError> { let mut out = HashMap::new(); let mut section = String::new(); @@ -321,7 +308,6 @@ impl Zone { if parse_size(v).is_none() { return Err(bad("limits.io_max", v, "bytes per second such as 20M")); } - // The limit is on the volume's device; a zone without one has nothing to bound. if storage != StorageMode::Encrypted { return Err(ZoneError::Invalid(format!( "zone {name:?}: limits.io_max bounds reads and writes of the zone's volume, \ @@ -373,7 +359,6 @@ impl Zone { } } StorageMode::Persistent => { - // Nothing would enforce it: a host directory with no quota. if kv.contains_key("storage.size") { return Err(ZoneError::Invalid(format!( "zone {:?}: storage.size is only meaningful for storage.mode = \ @@ -538,7 +523,6 @@ impl Zone { self.name ))); } - // A persistent zone is a directory and opens no volume. if self.storage == StorageMode::Persistent && self.volume.is_some() { return Err(ZoneError::Invalid(format!( "zone {:?}: storage.volume is only meaningful for storage.mode = \ @@ -558,8 +542,7 @@ impl Zone { self.name, self.border_color ))); } - /* Shape only, as zoneid checks it. A printable-ASCII label rules out - * bidi controls and homographs. */ + // Shape only, as zoneid checks it. if let Some(p) = &self.border_pattern { const PATTERNS: [&str; 6] = ["solid", "dashed", "dotted", "double", "dash-dot", "notched"]; if !PATTERNS.contains(&p.as_str()) { @@ -577,6 +560,7 @@ impl Zone { ))); } } + // Printable ASCII rules out bidi controls and homographs. if let Some(l) = &self.label { if l.is_empty() || l.len() > 12 || !l.chars().all(|c| matches!(c, ' '..='~')) || l.starts_with(' ') || l.ends_with(' ') { return Err(ZoneError::Invalid(format!( @@ -609,8 +593,7 @@ pub fn load_all(dir: &Path) -> Result, ZoneError> { continue; } let zone = Zone::from_file(&path)?; - /* The broker and the net zone open a zone as .toml, so a file - * named otherwise would launch but never receive a transfer. */ + // The broker and the net zone open .toml; a file named otherwise gets no transfer. if path.file_stem().and_then(|s| s.to_str()) != Some(zone.name.as_str()) { return Err(ZoneError::Invalid(format!( "{}: holds zone {:?}; a zone's file is named {}.toml", @@ -629,8 +612,7 @@ pub fn load_all(dir: &Path) -> Result, ZoneError> { /// Invariants that hold across the whole zone set, not within one file. pub fn check_invariants(zones: &[Zone]) -> Result<(), ZoneError> { - /* [transfer] to must name configured zones, never the nic zone. First - * occurrence wins; duplicate names are refused below. */ + // Transfer targets must be configured zones, never the nic zone; duplicates are refused below. let mut by_name: HashMap<&str, &Zone> = HashMap::with_capacity(zones.len()); for z in zones { by_name.entry(z.name.as_str()).or_insert(z); @@ -688,8 +670,7 @@ pub fn check_invariants(zones: &[Zone]) -> Result<(), ZoneError> { } } - /* Zones sharing a range could not be told apart by uid (broker, compositor - * proxy). Bases are stride-aligned, so distinct bases never overlap. */ + // The broker and compositor proxy tell zones apart by uid; aligned bases never overlap. let mut bases: HashMap = HashMap::new(); for z in zones { if let Some(b) = z.uid_base { diff --git a/compartments/tests/cli.sh b/compartments/tests/cli.sh index 195211d3..6a869886 100755 --- a/compartments/tests/cli.sh +++ b/compartments/tests/cli.sh @@ -1,12 +1,11 @@ #!/usr/bin/env bash -# Tests for `kryptik`, the user-facing command: mostly that convenience has not -# cost safety. No zone starts with its guarantees unmet, the config file is -# never executed, and nothing is offered that does not work. +# Tests for `kryptik`, the user-facing command: its conveniences must not cost safety. No zone +# starts with its guarantees unmet, the config file never runs, and nothing is offered that fails. +# Exit 1 if a check failed, 2 if kryptik or kryptikd is missing. set -uo pipefail REPO="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" -# tools/kryptik in a checkout; on PATH in an image, where this suite lives -# under /usr/lib/kryptik. +# tools/kryptik in a checkout; on PATH in an image, which installs this suite in /usr/lib/kryptik. KRYPTIK="${KRYPTIK:-}" if [[ -z "$KRYPTIK" ]]; then if [[ -x "$REPO/tools/kryptik" ]]; then KRYPTIK="$REPO/tools/kryptik" @@ -43,9 +42,8 @@ trap cleanup EXIT ZONES="$WORK/zones"; ROOTFS="$WORK/data"; mkdir -p "$ZONES" "$ROOTFS" -# As root a zone maps to host uid 100000 and must traverse into its data -# directory, which `mktemp -d` makes 0700 and root-owned. Otherwise every -# launch fails with a misleading "mount(root tmpfs)...: Permission denied". +# As root, zones map to uid 100000, which must traverse mktemp's 0700 directory to its data; +# otherwise every launch fails with a misleading "Permission denied" from mount. if [[ "$(id -u)" -eq 0 ]]; then chmod 0755 "$WORK" "$ZONES" "$ROOTFS" chown -R 100000:100000 "$ROOTFS" @@ -70,8 +68,7 @@ mkzone() { # name storage-mode colour [network-mode] printf '[ui]\nborder_color = "%s"\n' "$3" } > "$ZONES/$1.toml" } -# kryptikd refuses a zone set in which no zone holds the physical NIC, hence -# `carrier`. +# kryptikd refuses a zone set in which no zone holds the NIC, hence `carrier`. mkzone plain ephemeral "#101010" mkzone sealed encrypted "#202020" mkzone carrier ephemeral "#303030" nic @@ -124,9 +121,6 @@ else fi # --- an encrypted zone needs its passphrase; there is no way to skip that ---- -# It starts only with its passphrase (a descriptor from the launch daemon, or a -# root-owned file). Without one the command never runs, and no flag may fall -# back to a plain directory. out="$(K run sealed -- /bin/sh -c 'echo CLI_STARTED' 2>&1)"; rc=$? if [[ "$out" == *CLI_STARTED* ]]; then fail "a zone claiming ENCRYPTED storage ran without its passphrase" @@ -162,9 +156,8 @@ else info "output: $(printf '%s' "$out" | tr '\n' '|' | cut -c1-240)" fi -# It must run in the zone, not on the host: the zone's hostname is its name. -# Read from a sentinel line, since an error such as "no zone named plain" also -# contains the name. +# It must run in the zone, not on the host: the zone's hostname is its name. A sentinel line, +# since an error such as "no zone named plain" also contains the name. out="$(K run plain -- /bin/sh -c 'echo "D2HOST=$(hostname)"' 2>&1)" got="$(printf '%s' "$out" | sed -n 's/^D2HOST=//p' | head -1)" if [[ "$got" == "plain" ]]; then @@ -204,9 +197,9 @@ else fi # --- no transfer or clipboard commands --------------------------------------- -# Files cross zones only through the sending zone's broker after the user's yes, -# and the clipboard only by the chrome's gesture. From zone 0 either command -# would go around the user, so both are refused, naming the real path. +# Files cross zones only through the sending zone's broker after the user's yes, and the +# clipboard only by the chrome's gesture: from zone 0 either would go around the user, so both +# are refused, naming the real path. for c in transfer clipboard; do out="$(K "$c" 2>&1)"; rc=$? if (( rc != 0 )) && [[ "$out" == *"not a"*"command"* && "$out" == *"broker"* && "$out" == *"chrome"* ]]; then @@ -246,9 +239,8 @@ else fi # --- wifi: the net zone's credentials ---------------------------------------- -# A passphrase never goes on a command line, where any process could read it: -# `kryptik wifi add` reads it (terminal with echo off, or a pipe) and passes it -# on stdin to kryptik-launch or, as here, to kryptikd. +# A passphrase never goes on a command line, where any process could read it: `kryptik wifi add` +# takes it from the terminal or a pipe and passes it on stdin. out="$(K wifi 2>&1)"; rc=$? if (( rc != 0 )) && [[ "$out" == *"list, add SSID or forget SSID"* ]]; then pass "wifi without a subcommand fails and names the subcommands" @@ -336,9 +328,7 @@ sys.stdout.write(out.decode("utf-8", "replace")) PY cat > "$WORK/launch-shim" <