diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b42f1f10..6ebfb3a6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1221,11 +1221,12 @@ jobs: - type: deb-install scenario: ubuntu26 arch: amd64 - # The arm64 package on the oldest supported distribution: a fresh - # install and a daemon start. Deliberately GitHub-only (the local host - # is x86_64), and deliberately one leg: the upgrade, purge and - # conffile paths run under debian12 on amd64 only and stay - # unexercised on arm64. + # The arm64 package on the oldest supported distribution: the install + # scenario, which covers a fresh install, a daemon start and a purge + # of the DNS routing. Deliberately GitHub-only (the local host is + # x86_64), and deliberately one leg: the upgrade and conffile paths, + # including the upgrade scenario's own purge, run under debian12 on + # amd64 only and stay unexercised on arm64. - type: deb-install scenario: ubuntu22 arch: arm64 diff --git a/.github/workflows/package-freebsd.yml b/.github/workflows/package-freebsd.yml index 222c0f83..34d95302 100644 --- a/.github/workflows/package-freebsd.yml +++ b/.github/workflows/package-freebsd.yml @@ -159,6 +159,63 @@ jobs: # post-install must create the control-socket access group. pw groupshow fips >/dev/null || { echo "FAIL: fips group missing"; exit 1; } test -x /usr/local/libexec/fips/fips-dns-setup + + # The package's newsyslog entry must rotate the daemon log and + # make daemon(8) reopen it. The rc script starts daemon(8) with + # -H and records the supervisor's pid in daemon.pid, which the + # entry signals; without -H the supervisor keeps writing into + # the rotated file. + LOG=/var/log/fips.log + ENTRY=/usr/local/etc/newsyslog.conf.d/fips.conf + test -f "$ENTRY" || { echo "FAIL: missing $ENTRY"; exit 1; } + # Dry run of the stock configuration: the entry is reached only + # through newsyslog.conf's include of newsyslog.conf.d. + if ! newsyslog -nv 2>&1 | grep -q "$LOG"; then + echo "FAIL: the stock newsyslog configuration does not cover $LOG" + newsyslog -nv 2>&1 || true + exit 1 + fi + service fips onestart + i=0 + until [ -s /var/run/fips/daemon.pid ] && [ -s "$LOG" ]; do + i=$((i + 1)) + if [ "$i" -gt 30 ]; then + echo "FAIL: no daemon.pid or empty $LOG 30s after start" + ls -l /var/run/fips "$LOG" 2>&1 || true + cat "$LOG" 2>&1 || true + exit 1 + fi + sleep 1 + done + sup_pid=$(cat /var/run/fips/daemon.pid) + # Inode numbers a process holds open, from fstat's INUM column. + open_inodes() { fstat -p "$1" 2>/dev/null | awk 'NR > 1 && $6 ~ /^[0-9]+$/ { print $6 }'; } + before=$(stat -f %i "$LOG") + # -F rotates regardless of size; -f reads the shipped entry alone. + newsyslog -F -f "$ENTRY" + after=$(stat -f %i "$LOG") + if [ "$after" = "$before" ]; then + echo "FAIL: newsyslog -F did not rotate $LOG"; ls -li "$LOG"*; exit 1 + fi + if ! ls "$LOG".0* >/dev/null 2>&1; then + echo "FAIL: no rotated generation of $LOG"; ls -l "$LOG"*; exit 1 + fi + i=0 + until open_inodes "$sup_pid" | grep -qx "$after"; do + i=$((i + 1)) + if [ "$i" -gt 10 ]; then + echo "FAIL: daemon(8) pid $sup_pid did not reopen $LOG after rotation" + fstat -p "$sup_pid" || true + exit 1 + fi + sleep 1 + done + if open_inodes "$sup_pid" | grep -qx "$before"; then + echo "FAIL: daemon(8) pid $sup_pid still holds the rotated $LOG open"; exit 1 + fi + service fips onestop + echo "==> log rotation PASSED" + pkg info fips echo "==> pkg smoke-install PASSED" diff --git a/CHANGELOG.md b/CHANGELOG.md index cc89e31d..9a68ff73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1068,6 +1068,19 @@ with v0.5.x or earlier peers. built. It picked the most recently modified `fips_*.deb` in the output directory that sorted last by name, so a package with a higher version left there by an earlier run was returned instead. +- Purging the `.deb`, or running `uninstall.sh` from the tarball, now removes + the `.fips` DNS routing when `fips-dns` was not running at the time. The + cleanup removed the dns-delegate file from the wrong directory, never removed + the systemd-resolved global drop-in, and restarted no resolver, so the host + kept sending `.fips` queries to `[::1]:5354`, where nothing listens any more, + and `.fips` lookups timed out. Both scripts now remove all four files + `fips-dns-setup` can write, and restart systemd-resolved or reload dnsmasq or + NetworkManager when they removed that resolver's file and it is running. A + failed restart is reported and does not fail the removal. +- The `.deb` now recommends `nftables`. `fips-firewall.service` runs + `/usr/sbin/nft`, so enabling it on a host without nftables failed at start. + It is a recommendation rather than a dependency because the firewall unit is + opt-in and the daemon itself does not need `nft`. - The Linux `.deb` and the systemd tarball now install and run on Debian 12 and Ubuntu 22.04. Every Linux artifact from v0.3.0 through v0.5.0 was built on the @@ -1091,6 +1104,44 @@ with v0.5.x or earlier peers. - The release `PKGBUILD` now lists `dbus` as a runtime dependency. The `fips` binary links `libdbus-1`, and the `fips-git` package already declared it. +- Both `PKGBUILD` files list `nftables` as an optional dependency, for + `fips-firewall.service`. + +#### Packaging (FreeBSD) + +- The daemon's log, `/var/log/fips.log`, is now rotated. The package ships a + newsyslog entry that keeps five compressed generations of 1000 KB, and the rc + script starts `daemon(8)` with `-H` so it reopens the log after a rotation. + The log used to grow without bound. + +#### Windows + +- Windows now keeps its config, key, hosts and peer ACL files in + `C:\ProgramData\fips`, the directory the service installer writes to. The + config search, the key directory and the peer ACL defaults disagreed: the + service found none of the installed files after a reboot and ran on + defaults, `fipsctl keygen` wrote to the per-user `%APPDATA%\fips`, and a + `peers.deny` placed beside the hosts file was never read, so the ACL failed + open. A key left in `%APPDATA%\fips` is still used when the new directory + has none, with a note to move it. For one release, a `peers.allow` or + `peers.deny` left at the old `\etc\fips` location is still read when the + new directory has no such file, with a warning naming both paths. +- The Windows service now writes its log to `C:\ProgramData\fips\fips.log`, + rolled at 10 MiB with four old files kept. A service has no standard output, + so everything the daemon logged in service mode was lost, including + config-load failures and panic messages. A foreground run still logs to the + console. +- The Windows service installer now restricts `C:\ProgramData\fips` to + SYSTEM and Administrators. The directory inherited `C:\ProgramData`'s + default ACL, which lets any local user read the files in it and create new + ones, so any local account could read the node's key, or create a missing + `fips.key`, `fips.yaml` or `hosts` that the service then used. The installer + creates the directory with the restricted ACL, or replaces the ACL of an + existing one, resets the files already in it to inherit it, and refuses to + continue if the directory or anything in it is a link or a folder, or if the + directory is owned by another account. Rerun the installer after moving files + into the directory. A foreground run from an unelevated prompt can no longer + read the files there. ### Security diff --git a/Cargo.toml b/Cargo.toml index 0e47eee2..70a5d296 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -95,7 +95,8 @@ priority = "optional" # ahead of what the binaries need. # systemd stays by hand: nothing links it, so nothing can derive it. depends = "$auto, libgcc-s1 (>= 4.2), systemd" -recommends = "bluez" +# nftables is recommended, not required: only the opt-in fips-firewall.service runs nft. +recommends = "bluez, nftables" extended-description = """\ FIPS is a distributed, decentralized network routing protocol for mesh \ nodes connecting over arbitrary transports including UDP, TCP, Ethernet, \ diff --git a/docs/reference/cli-fips.md b/docs/reference/cli-fips.md index aca69f28..653291a1 100644 --- a/docs/reference/cli-fips.md +++ b/docs/reference/cli-fips.md @@ -66,26 +66,31 @@ highest-priority value wins. | Priority | Path | Purpose | | -------- | ---- | ------- | -| 1 | `/usr/local/etc/fips/fips.yaml` (macOS, FreeBSD), `/etc/fips/fips.yaml` (other Unix) | System-wide defaults | -| 2 | `~/.config/fips/fips.yaml` | User preferences | +| 1 | `/usr/local/etc/fips/fips.yaml` (macOS, FreeBSD), `C:\ProgramData\fips\fips.yaml` (Windows), `/etc/fips/fips.yaml` (other Unix) | System-wide defaults | +| 2 | `~/.config/fips/fips.yaml` (`%APPDATA%\fips\fips.yaml` on Windows) | User preferences | | 3 | `~/.fips.yaml` | Legacy user config | | 4 | `./fips.yaml` | Deployment-specific overrides | On macOS and FreeBSD both system directories are probed: `/etc/fips` first, then `/usr/local/etc/fips`, so the packaged file wins over a -leftover `/etc/fips` copy from an earlier install. +leftover `/etc/fips` copy from an earlier install. Windows likewise +probes `\etc\fips` on the current drive, then `C:\ProgramData\fips`. Adjacent to the highest-priority config file the daemon reads (or writes, on first start) the identity files: | File | Mode | Purpose | | ---- | ---- | ------- | -| `fips.key` | `0600` | Bech32 nsec for the persistent identity (Unix only; Windows inherits parent ACLs). | +| `fips.key` | `0600` | Bech32 nsec for the persistent identity (Unix; on Windows the file takes its directory's ACL, which `install-service.ps1` restricts to SYSTEM and Administrators). | | `fips.pub` | `0644` | Bech32 npub corresponding to `fips.key`. | When `node.identity.persistent` is `false` (the default), a fresh keypair is written to these files on every start. +On Windows the service writes its log to `C:\ProgramData\fips\fips.log`, +rolled at 10 MiB with four old files kept; a foreground run logs to the +console. + The control socket path is derived per [control-socket.md](control-socket.md). diff --git a/docs/reference/cli-fipsctl.md b/docs/reference/cli-fipsctl.md index 3818669e..29c336c2 100644 --- a/docs/reference/cli-fipsctl.md +++ b/docs/reference/cli-fipsctl.md @@ -90,7 +90,7 @@ daemon. | Flag | Argument | Default | Description | | ---- | -------- | ------- | ----------- | -| `-d`, `--dir` | `DIR` | `/usr/local/etc/fips` (macOS, FreeBSD), `/etc/fips` (other Unix), `%APPDATA%\fips` (Windows) | Output directory for `fips.key` and `fips.pub`. Matches the directory the platform's packaging installs config into, which is where the daemon derives the key paths from. | +| `-d`, `--dir` | `DIR` | `/usr/local/etc/fips` (macOS, FreeBSD), `/etc/fips` (other Unix), `C:\ProgramData\fips` (Windows) | Output directory for `fips.key` and `fips.pub`. Matches the directory the platform's packaging installs config into, which is where the daemon derives the key paths from. | | `-f`, `--force` | — | off | Overwrite an existing `fips.key`. | | `-s`, `--stdout` | — | off | Print `nsec` then `npub` to stdout instead of writing files. | diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 2967c39a..8fdd0bef 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -13,21 +13,24 @@ locations, lowest to highest priority: | Priority | Path | Purpose | |----------|------|---------| -| 1 (lowest) | `/usr/local/etc/fips/fips.yaml` (macOS, FreeBSD), `/etc/fips/fips.yaml` (other Unix) | System-wide defaults | -| 2 | `~/.config/fips/fips.yaml` | User preferences | +| 1 (lowest) | `/usr/local/etc/fips/fips.yaml` (macOS, FreeBSD), `C:\ProgramData\fips\fips.yaml` (Windows), `/etc/fips/fips.yaml` (other Unix) | System-wide defaults | +| 2 | `~/.config/fips/fips.yaml` (`%APPDATA%\fips\fips.yaml` on Windows) | User preferences | | 3 | `~/.fips.yaml` | Legacy user config | | 4 (highest) | `./fips.yaml` | Deployment-specific overrides | All found files are loaded and merged in priority order. Values from higher priority files override those from lower priority files. This allows a system administrator to set site-wide defaults in the priority 1 path above, -`/usr/local/etc/fips/fips.yaml` on macOS and FreeBSD and +`/usr/local/etc/fips/fips.yaml` on macOS and FreeBSD, +`C:\ProgramData\fips\fips.yaml` on Windows and `/etc/fips/fips.yaml` on other Unix systems, while individual deployments override specific values in `./fips.yaml`. On macOS and FreeBSD both directories are probed: `/etc/fips` first, then `/usr/local/etc/fips`, so the packaged file wins over a leftover -`/etc/fips` copy from an earlier install. +`/etc/fips` copy from an earlier install. Windows likewise probes +`\etc\fips` on the current drive, then `C:\ProgramData\fips`, which is +the only directory the Windows service reads. ### CLI Option diff --git a/packaging/aur/PKGBUILD b/packaging/aur/PKGBUILD index 9cdbc749..032be11f 100644 --- a/packaging/aur/PKGBUILD +++ b/packaging/aur/PKGBUILD @@ -8,7 +8,7 @@ license=('MIT') arch=('x86_64') depends=('dbus' 'gcc-libs' 'glibc') makedepends=('cargo' 'clang') -optdepends=('systemd-resolved: .fips DNS resolution') +optdepends=('systemd-resolved: .fips DNS resolution' 'nftables: fips-firewall.service ruleset') conflicts=('fips-git' 'fips-git-debug') backup=('etc/fips/fips.yaml' 'etc/fips/hosts' 'etc/fips/fips.nft') install=fips.install diff --git a/packaging/aur/PKGBUILD-git b/packaging/aur/PKGBUILD-git index 44069765..7248d46b 100644 --- a/packaging/aur/PKGBUILD-git +++ b/packaging/aur/PKGBUILD-git @@ -8,7 +8,7 @@ license=('MIT') arch=('x86_64') depends=('dbus' 'gcc-libs' 'glibc') makedepends=('cargo' 'clang' 'git') -optdepends=('systemd-resolved: .fips DNS resolution') +optdepends=('systemd-resolved: .fips DNS resolution' 'nftables: fips-firewall.service ruleset') provides=('fips') conflicts=('fips' 'fips-debug') backup=('etc/fips/fips.yaml' 'etc/fips/hosts' 'etc/fips/fips.nft') diff --git a/packaging/debian/postrm b/packaging/debian/postrm index 70efcfd6..8f1afd5d 100755 --- a/packaging/debian/postrm +++ b/packaging/debian/postrm @@ -13,10 +13,45 @@ case "$1" in # Remove runtime directory rm -rf /run/fips/ - # Remove DNS config files that fips-dns-setup may have created + # Remove the DNS routing fips-dns-setup may have written, in case + # fips-dns-teardown did not run (prerm's stop runs it only when + # fips-dns.service was active), and make the resolver drop it. The + # paths match packaging/common/fips-dns-teardown, which dpkg has + # already removed, so it cannot be called from here. + restart_resolved=0 + if [ -f /etc/systemd/dns-delegate.d/fips.dns-delegate ]; then + rm -f /etc/systemd/dns-delegate.d/fips.dns-delegate + restart_resolved=1 + fi + if [ -f /etc/systemd/resolved.conf.d/fips.conf ]; then + rm -f /etc/systemd/resolved.conf.d/fips.conf + restart_resolved=1 + fi + # Only pre-v0.3.0 development builds wrote this path, and systemd + # never read it. rm -f /etc/systemd/dns-delegate/fips.dns-delegate - rm -f /etc/dnsmasq.d/fips.conf - rm -f /etc/NetworkManager/dnsmasq.d/fips.conf + if [ "$restart_resolved" = 1 ] && [ -d /run/systemd/system ] \ + && systemctl is-active --quiet systemd-resolved.service; then + systemctl restart systemd-resolved \ + || echo "fips: warning: could not restart systemd-resolved; restart it to drop the .fips route" + fi + if [ -f /etc/dnsmasq.d/fips.conf ]; then + rm -f /etc/dnsmasq.d/fips.conf + if [ -d /run/systemd/system ] \ + && systemctl is-active --quiet dnsmasq.service; then + systemctl reload dnsmasq \ + || echo "fips: warning: could not reload dnsmasq; reload it to drop the .fips route" + fi + fi + if [ -f /etc/NetworkManager/dnsmasq.d/fips.conf ]; then + rm -f /etc/NetworkManager/dnsmasq.d/fips.conf + if [ -d /run/systemd/system ] \ + && systemctl is-active --quiet NetworkManager.service \ + && command -v nmcli >/dev/null 2>&1; then + nmcli general reload \ + || echo "fips: warning: could not reload NetworkManager; reload it to drop the .fips route" + fi + fi # Remove fips system group if getent group fips >/dev/null 2>&1; then diff --git a/packaging/freebsd/README.md b/packaging/freebsd/README.md index 5b289472..135a3e6a 100644 --- a/packaging/freebsd/README.md +++ b/packaging/freebsd/README.md @@ -46,6 +46,14 @@ scripts), so an edited `fips.yaml` survives upgrade/removal. `/var/run/fips/fips.pid` and logs to `/var/log/fips.log` (rc.conf knobs: `fips_config`, `fips_flags`, `fips_logfile`). +The log is rotated by `/usr/local/etc/newsyslog.conf.d/fips.conf`: five +bzip2-compressed generations of 1000 KB. daemon(8) runs with `-H` and +records its own pid in `/var/run/fips/daemon.pid`; newsyslog signals that +pid after the rename, and daemon(8) reopens the log. The entry covers +the default `fips_logfile` only, so a log moved elsewhere needs its own +newsyslog entry. The entry belongs to the package and is replaced on +upgrade. + The package creates a `fips` group; members can run `fipsctl` and `fipstop` without root (`pw groupmod fips -m `, then re-login). On `pkg upgrade` the services are stopped before the binaries are diff --git a/packaging/freebsd/build-pkg.sh b/packaging/freebsd/build-pkg.sh index aa0f4c2f..77bc0045 100755 --- a/packaging/freebsd/build-pkg.sh +++ b/packaging/freebsd/build-pkg.sh @@ -61,6 +61,10 @@ install -m 0644 "${PROJECT_ROOT}/packaging/common/hosts" \ install -m 0755 "${SCRIPT_DIR}/fips.rc" "${STAGE}/usr/local/etc/rc.d/fips" install -m 0755 "${SCRIPT_DIR}/fips-dns.rc" "${STAGE}/usr/local/etc/rc.d/fips_dns" +install -d "${STAGE}/usr/local/etc/newsyslog.conf.d" +install -m 0644 "${SCRIPT_DIR}/fips.newsyslog" \ + "${STAGE}/usr/local/etc/newsyslog.conf.d/fips.conf" + install -m 0755 "${SCRIPT_DIR}/fips-dns-setup" \ "${SCRIPT_DIR}/fips-dns-teardown" \ "${STAGE}/usr/local/libexec/fips/" @@ -116,6 +120,7 @@ bin/fipsctl bin/fipstop etc/fips/fips.yaml.sample etc/fips/hosts.sample +etc/newsyslog.conf.d/fips.conf etc/rc.d/fips etc/rc.d/fips_dns libexec/fips/fips-dns-setup diff --git a/packaging/freebsd/fips.newsyslog b/packaging/freebsd/fips.newsyslog new file mode 100644 index 00000000..6a7aad2b --- /dev/null +++ b/packaging/freebsd/fips.newsyslog @@ -0,0 +1,13 @@ +# newsyslog(8) rotation for the FIPS daemon log. Installed as +# /usr/local/etc/newsyslog.conf.d/fips.conf, which the stock +# /etc/newsyslog.conf includes. +# +# 5 generations, rotate at 1000 KB, bzip2-compressed (J), created if +# missing (C), mode 600 as daemon(8) itself creates the log. The pid file +# is the daemon(8) supervisor's (-P in the rc script), not the fips +# process's: newsyslog sends it SIGHUP after the rename, and the rc script +# starts daemon(8) with -H, which reopens the log on that signal. Without +# -H the daemon keeps writing into the rotated file. Covers the default +# fips_logfile only. +# logfilename [owner:group] mode count size when flags [/pid_file] [sig_num] +/var/log/fips.log 600 5 1000 * JC /var/run/fips/daemon.pid diff --git a/packaging/freebsd/fips.rc b/packaging/freebsd/fips.rc index 81269d38..5f492caf 100755 --- a/packaging/freebsd/fips.rc +++ b/packaging/freebsd/fips.rc @@ -8,7 +8,8 @@ # fips_enable (bool): Set YES to run the FIPS daemon. Default NO. # fips_config (path): Config file. Default /usr/local/etc/fips/fips.yaml. # fips_flags (str): Extra arguments passed to the fips daemon. -# fips_logfile (path): Daemon stdout/stderr log. Default /var/log/fips.log. +# fips_logfile (path): Daemon stdout/stderr log. Default /var/log/fips.log; +# rotated by newsyslog only at the default path. . /etc/rc.subr @@ -24,9 +25,12 @@ load_rc_config $name runtime_dir="/var/run/fips" pidfile="${runtime_dir}/fips.pid" +# daemon(8) records its own pid here (-P) so newsyslog can signal it after +# rotating the log: with -H the supervisor reopens its output on SIGHUP. +supervisor_pidfile="${runtime_dir}/daemon.pid" procname="/usr/local/bin/fips" command="/usr/sbin/daemon" -command_args="-p ${pidfile} -t fips -o ${fips_logfile} ${procname} --config ${fips_config} ${fips_flags}" +command_args="-H -p ${pidfile} -P ${supervisor_pidfile} -t fips -o ${fips_logfile} ${procname} --config ${fips_config} ${fips_flags}" start_precmd="fips_precmd" # The daemon resolves its control socket to /var/run/fips when the diff --git a/packaging/systemd/uninstall.sh b/packaging/systemd/uninstall.sh index 251bb897..be77d8c2 100755 --- a/packaging/systemd/uninstall.sh +++ b/packaging/systemd/uninstall.sh @@ -42,10 +42,54 @@ rm -rf /usr/lib/fips/ systemctl daemon-reload echo "systemd units and DNS scripts removed." -# Clean up DNS config files that fips-dns-setup may have created +# --- Remove DNS routing --- +# fips-dns-setup may have written one of these, and stopping fips-dns.service +# above runs fips-dns-teardown only when the unit was active. The paths match +# packaging/common/fips-dns-teardown. + +restart_resolved=false +if [ -f /etc/systemd/dns-delegate.d/fips.dns-delegate ]; then + rm -f /etc/systemd/dns-delegate.d/fips.dns-delegate + echo "Removed /etc/systemd/dns-delegate.d/fips.dns-delegate." + restart_resolved=true +fi +if [ -f /etc/systemd/resolved.conf.d/fips.conf ]; then + rm -f /etc/systemd/resolved.conf.d/fips.conf + echo "Removed /etc/systemd/resolved.conf.d/fips.conf." + restart_resolved=true +fi +# Only pre-v0.3.0 development builds wrote this path, and systemd never read it. rm -f /etc/systemd/dns-delegate/fips.dns-delegate -rm -f /etc/dnsmasq.d/fips.conf -rm -f /etc/NetworkManager/dnsmasq.d/fips.conf +if $restart_resolved && systemctl is-active --quiet systemd-resolved.service 2>/dev/null; then + if systemctl restart systemd-resolved; then + echo "systemd-resolved restarted." + else + echo "Warning: could not restart systemd-resolved; restart it to drop the .fips route." >&2 + fi +fi +if [ -f /etc/dnsmasq.d/fips.conf ]; then + rm -f /etc/dnsmasq.d/fips.conf + echo "Removed /etc/dnsmasq.d/fips.conf." + if systemctl is-active --quiet dnsmasq.service 2>/dev/null; then + if systemctl reload dnsmasq; then + echo "dnsmasq reloaded." + else + echo "Warning: could not reload dnsmasq; reload it to drop the .fips route." >&2 + fi + fi +fi +if [ -f /etc/NetworkManager/dnsmasq.d/fips.conf ]; then + rm -f /etc/NetworkManager/dnsmasq.d/fips.conf + echo "Removed /etc/NetworkManager/dnsmasq.d/fips.conf." + if systemctl is-active --quiet NetworkManager.service 2>/dev/null \ + && command -v nmcli >/dev/null 2>&1; then + if nmcli general reload; then + echo "NetworkManager reloaded." + else + echo "Warning: could not reload NetworkManager; reload it to drop the .fips route." >&2 + fi + fi +fi # --- Remove tmpfiles.d entry --- diff --git a/packaging/windows/build-zip.ps1 b/packaging/windows/build-zip.ps1 index 1b7d6c1a..532cca75 100644 --- a/packaging/windows/build-zip.ps1 +++ b/packaging/windows/build-zip.ps1 @@ -100,8 +100,39 @@ Control Socket: fipsctl and fipstop connect to this port automatically. Configuration: - Edit fips.yaml before starting. Place it in the same directory - as fips.exe, or in %APPDATA%\fips\, or set FIPS_CONFIG. + The service reads C:\ProgramData\fips\fips.yaml, where + install-service.ps1 puts it, and keeps fips.key, hosts, + peers.allow and peers.deny beside it. Edit fips.yaml there + before starting the service. + + install-service.ps1 restricts C:\ProgramData\fips to SYSTEM + and Administrators before writing into it. Reading or editing + files there, fipsctl keygen, fipsctl address with no argument, + and a foreground fips.exe run that relies on + C:\ProgramData\fips\fips.yaml all need an elevated prompt. + Unelevated, a foreground run may skip that file without + saying so, or may fail with an access error. + + A foreground run takes -c , or reads + C:\ProgramData\fips\fips.yaml and then, as per-user overrides + the service does not read, %APPDATA%\fips\fips.yaml, + %USERPROFILE%\.fips.yaml and .\fips.yaml. The key file sits + beside the last config loaded. + + fipsctl keygen writes to C:\ProgramData\fips by default and + needs an elevated prompt. Run install-service.ps1 before it: + a directory that keygen creates first carries + C:\ProgramData's access until the installer restricts it. + + A file moved into C:\ProgramData\fips keeps its old + permissions. That includes a fips.yaml or fips.key moved from + %APPDATA%\fips as the daemon's warning suggests, so run + install-service.ps1 again after moving files there. + +Logs: + The service logs to C:\ProgramData\fips\fips.log, rolled at + 10 MiB with four old files kept. A foreground run logs to the + console. "@ | Out-File -FilePath "$StagingDir\README.txt" -Encoding UTF8 # Create ZIP diff --git a/packaging/windows/install-service.ps1 b/packaging/windows/install-service.ps1 index 1306ba94..8b4bf214 100644 --- a/packaging/windows/install-service.ps1 +++ b/packaging/windows/install-service.ps1 @@ -19,9 +19,116 @@ $ConfigDir = "$env:ProgramData\fips" Write-Host "Installing FIPS service..." -# Create directories +# Create the install directory New-Item -ItemType Directory -Force -Path $InstallDir | Out-Null -New-Item -ItemType Directory -Force -Path $ConfigDir | Out-Null + +# Create the config directory, or restrict an existing one, so that only +# SYSTEM and Administrators can use it before anything is written into it. +# The service keeps its identity key, config, hosts file, peer ACL files and +# log there, and the ACL inherited from C:\ProgramData lets any local user +# read those files and create missing ones. A user may also have created the +# directory, or a link in its place, before this script ran, so a link, a +# directory owned by another account, and a link or folder inside it are +# refused rather than acted on. A user who created the directory keeps full +# control of it through an inherited entry even after an administrator takes +# ownership, so the only recovery offered for a refused directory is to +# delete it. Well-known SIDs are used because account names are translated on +# non-English Windows. +$icacls = "$env:SystemRoot\System32\icacls.exe" + +# A new object, so only the DACL is written and any explicit entry an +# existing directory carried is dropped; inheritance from C:\ProgramData is +# turned off. +$acl = New-Object System.Security.AccessControl.DirectorySecurity +$acl.SetAccessRuleProtection($true, $false) +$inherit = [System.Security.AccessControl.InheritanceFlags]::ContainerInherit -bor [System.Security.AccessControl.InheritanceFlags]::ObjectInherit +foreach ($sid in @("S-1-5-18", "S-1-5-32-544")) { + $rule = [System.Security.AccessControl.FileSystemAccessRule]::new( + [System.Security.Principal.SecurityIdentifier]::new($sid), + [System.Security.AccessControl.FileSystemRights]::FullControl, + $inherit, + [System.Security.AccessControl.PropagationFlags]::None, + [System.Security.AccessControl.AccessControlType]::Allow) + $acl.AddAccessRule($rule) +} + +# A new directory is created with this ACL in one step. Created first and +# restricted afterwards, it would carry C:\ProgramData's access in between, +# and a user could turn the empty directory into a junction in that time. +# Both calls leave an existing directory, or a link at the path, as it is. +# Windows PowerShell's .NET Framework takes the ACL in +# Directory.CreateDirectory; PowerShell 7 takes it in +# FileSystemAclExtensions.CreateDirectory instead. +if ($PSVersionTable.PSEdition -eq "Core") { + [System.IO.FileSystemAclExtensions]::CreateDirectory($acl, $ConfigDir) | Out-Null +} else { + [System.IO.Directory]::CreateDirectory($ConfigDir, $acl) | Out-Null +} + +$dirItem = Get-Item -LiteralPath $ConfigDir -Force +if ($dirItem.Attributes -band [System.IO.FileAttributes]::ReparsePoint) { + Write-Error "$ConfigDir is a link or junction, not a directory. Remove it, then run install-service.ps1 again." + exit 1 +} + +try { + $ownerSid = (Get-Acl -LiteralPath $ConfigDir).GetOwner([System.Security.Principal.SecurityIdentifier]).Value +} catch { + Write-Error "Cannot read the owner of $ConfigDir. Another account may have created it and placed files in it. Copy out anything you need, delete the directory, then run install-service.ps1 again. If Windows refuses the deletion, take ownership first, but still delete it: taking ownership leaves its creator full control of it." + exit 1 +} +$trustedOwners = @("S-1-5-18", "S-1-5-32-544", [System.Security.Principal.WindowsIdentity]::GetCurrent().User.Value) +if ($trustedOwners -notcontains $ownerSid) { + Write-Error "$ConfigDir is owned by $ownerSid, not by SYSTEM, Administrators or this account. Another account may have created it and placed files in it. Copy out anything you need, delete the directory, then run install-service.ps1 again. If Windows refuses the deletion, take ownership first, but still delete it: taking ownership leaves its creator full control of it." + exit 1 +} + +& $icacls $ConfigDir /setowner "*S-1-5-32-544" /L /Q +if ($LASTEXITCODE -ne 0) { + Write-Error "icacls could not set the owner of $ConfigDir (exit code $LASTEXITCODE). Another account may have changed its permissions. Copy out anything you need, delete the directory, then run install-service.ps1 again." + exit 1 +} + +# FIPS keeps only files in the directory. Applying the new ACL propagates +# into existing entries, so links and folders are refused before it as well +# as after it and after each file is reset. An existing directory that is +# still empty can be turned into a junction by any user until the ACL is +# applied, so each check also refuses the directory itself if it has become +# a link. +$refuseEntries = { + if ((Get-Item -LiteralPath $ConfigDir -Force).Attributes -band [System.IO.FileAttributes]::ReparsePoint) { + Write-Error "$ConfigDir became a link or junction while the installer ran. Another account may have converted it, and the permissions of the folder it points to may have been changed. Remove the link, then run install-service.ps1 again." + exit 1 + } + foreach ($item in @(Get-ChildItem -LiteralPath $ConfigDir -Force)) { + if (($item.Attributes -band [System.IO.FileAttributes]::ReparsePoint) -or $item.PSIsContainer) { + Write-Error "$($item.FullName) is a link or a folder, and FIPS keeps only files in $ConfigDir. Inspect and remove that entry, then run install-service.ps1 again." + exit 1 + } + } +} +& $refuseEntries + +Set-Acl -LiteralPath $ConfigDir -AclObject $acl + +& $refuseEntries + +foreach ($item in @(Get-ChildItem -LiteralPath $ConfigDir -Force)) { + & $icacls $item.FullName /setowner "*S-1-5-32-544" /L /Q + if ($LASTEXITCODE -ne 0) { + Write-Error "icacls could not set the owner of $($item.FullName) (exit code $LASTEXITCODE). Another account may have placed or changed this file. Delete the file if it is still there, then run install-service.ps1 again." + exit 1 + } + & $icacls $item.FullName /reset /L /Q + if ($LASTEXITCODE -ne 0) { + Write-Error "icacls could not reset the ACL of $($item.FullName) (exit code $LASTEXITCODE). Another account may have placed or changed this file. Delete the file if it is still there, then run install-service.ps1 again." + exit 1 + } +} + +& $refuseEntries + +Write-Host " Restricted $ConfigDir to SYSTEM and Administrators" # Copy binaries $Binaries = @("fips.exe", "fipsctl.exe", "fipstop.exe") @@ -83,6 +190,7 @@ Write-Host "" Write-Host "FIPS service installed successfully." Write-Host "" Write-Host "Edit config: notepad $ConfigDir\fips.yaml" +Write-Host "Logs: $ConfigDir\fips.log" Write-Host "Start: sc start fips" Write-Host "Stop: sc stop fips" Write-Host "Status: sc query fips" diff --git a/src/bin/fips.rs b/src/bin/fips.rs index 1fb474be..8b9572bf 100644 --- a/src/bin/fips.rs +++ b/src/bin/fips.rs @@ -56,11 +56,14 @@ async fn run_daemon( match Config::load_file(config_path) { Ok(config) => (config, vec![config_path.clone()]), Err(e) => { - eprintln!( + let msg = format!( "Failed to load configuration from {}: {}", config_path.display(), e ); + #[cfg(windows)] + service::startup_error(&msg); + eprintln!("{msg}"); std::process::exit(1); } } @@ -68,7 +71,10 @@ async fn run_daemon( match Config::load() { Ok(result) => result, Err(e) => { - eprintln!("Failed to load configuration: {}", e); + let msg = format!("Failed to load configuration: {}", e); + #[cfg(windows)] + service::startup_error(&msg); + eprintln!("{msg}"); std::process::exit(1); } } @@ -108,12 +114,15 @@ async fn run_daemon( // stderr fails too — and the shipped supervisor configs point stdout and // stderr at the same place, so one full disk satisfies both. A worker // thread killed that way takes its share of the peer space with it. - fmt() + let builder = fmt() .with_env_filter(filter) .with_target(true) .with_ansi(std::io::IsTerminal::is_terminal(&std::io::stdout())) - .log_internal_errors(false) - .init(); + .log_internal_errors(false); + // A Windows service has no stdout to log to; it writes a file instead. + #[cfg(windows)] + let builder = builder.with_writer(service::log_writer()); + builder.init(); info!("FIPS {} starting", version::short_version()); @@ -130,6 +139,12 @@ async fn run_daemon( #[cfg(any(target_os = "macos", target_os = "freebsd"))] fips::node::warn_on_legacy_config_paths(); + // Windows moved its config, key and ACL files from %APPDATA%\fips and + // /etc/fips to C:\ProgramData\fips; flag a config left behind. The peer + // ACL reloader reports ACL files left at the old location. + #[cfg(windows)] + fips::config::warn_legacy(&loaded_paths); + // Identity provisioning: config nsec > key file > generate ephemeral let mut resolved = match resolve_identity(&config, &loaded_paths) { Ok(r) => r, @@ -279,9 +294,12 @@ fn main() { #[cfg(windows)] mod service { + use fips::utils::logfile::{ROLL_BYTES, ROLL_KEEP, RollingFile, SharedLog}; use std::ffi::OsString; - use std::path::PathBuf; + use std::path::{Path, PathBuf}; + use std::sync::OnceLock; use std::time::Duration; + use tracing_subscriber::fmt::writer::BoxMakeWriter; use windows_service::{ define_windows_service, service::{ @@ -301,6 +319,48 @@ mod service { define_windows_service!(ffi_service_main, service_main); + /// The service's log file. Set at the start of `service_main`, and never + /// in a foreground run, which logs to the console. + static LOG: OnceLock = OnceLock::new(); + + /// Open the service log in the config directory and send panics to it. + /// + /// A failure leaves the log unset and the daemon runs on without one: + /// with no console and logging not yet up, nothing could report it. + fn open_log() { + let path = Path::new(fips::config::SYSTEM_CONFIG_DIR).join("fips.log"); + let Ok(file) = RollingFile::open(&path, ROLL_BYTES, ROLL_KEEP) else { + return; + }; + let log = SharedLog::new(file); + if LOG.set(log.clone()).is_err() { + return; + } + // The default hook writes to stderr, which a service does not have. + let previous = std::panic::take_hook(); + std::panic::set_hook(Box::new(move |info| { + previous(info); + log.try_line(&info.to_string()); + })); + } + + /// Where the daemon's tracing output goes: the service log when one is + /// open, otherwise stdout. + pub fn log_writer() -> BoxMakeWriter { + match LOG.get() { + Some(log) => BoxMakeWriter::new(log.clone()), + None => BoxMakeWriter::new(std::io::stdout), + } + } + + /// Record an error raised before logging is set up in the service log. + /// Does nothing in a foreground run, where stderr carries it. + pub fn startup_error(msg: &str) { + if let Some(log) = LOG.get() { + log.line(&format!("ERROR {msg}")); + } + } + /// Start the service dispatcher, which blocks until the service stops. pub fn run_as_service() -> Result<(), windows_service::Error> { service_dispatcher::start(SERVICE_NAME, ffi_service_main) @@ -308,8 +368,13 @@ mod service { /// Entry point called by the Windows service control manager. fn service_main(arguments: Vec) { + open_log(); if let Err(e) = run_service(arguments) { - eprintln!("Service error: {:?}", e); + let msg = format!("Service error: {:?}", e); + match LOG.get() { + Some(log) => log.line(&msg), + None => eprintln!("{msg}"), + } } } @@ -414,10 +479,16 @@ mod service { println!("Service '{}' installed successfully.", SERVICE_NAME); println!("Start it with: sc start {}", SERVICE_NAME); println!(); - println!("Configuration: place fips.yaml in one of:"); - println!(" - Current directory"); - println!(" - %APPDATA%\\fips\\fips.yaml"); - println!(" - Set FIPS_CONFIG environment variable"); + let dir = std::path::Path::new(fips::config::SYSTEM_CONFIG_DIR); + println!( + "Configuration: the service reads {}", + dir.join("fips.yaml").display() + ); + println!(" keep fips.key, hosts, peers.allow and peers.deny beside it."); + println!( + "Logs: the service writes {}", + dir.join("fips.log").display() + ); Ok(()) } diff --git a/src/bin/fipsctl.rs b/src/bin/fipsctl.rs index 440c52ea..5ee737db 100644 --- a/src/bin/fipsctl.rs +++ b/src/bin/fipsctl.rs @@ -353,16 +353,7 @@ fn print_response(value: &serde_json::Value) { /// Must match the platform's config dir, since the daemon derives key /// paths from the config file's location. fn default_key_dir() -> PathBuf { - #[cfg(unix)] - { - PathBuf::from(fips::config::SYSTEM_CONFIG_DIR) - } - #[cfg(windows)] - { - dirs::config_dir() - .map(|d| d.join("fips")) - .unwrap_or_else(|| PathBuf::from("C:\\ProgramData\\fips")) - } + PathBuf::from(fips::config::SYSTEM_CONFIG_DIR) } /// Check if `address` is an IPv6 literal in `fd00::/8` (FIPS mesh ULA range). @@ -442,10 +433,42 @@ fn mesh_address(identity: Option<&str>, key: Option<&Path>) -> Result address_from_npub(&resolve_peer(peer)), (None, Some(path)) => address_from_file(path), + #[cfg(windows)] + (None, None) => own_address(&default_key_dir()), + #[cfg(not(windows))] (None, None) => address_from_key_dir(&default_key_dir()), } } +/// Derive this node's own mesh address on Windows, where the default key +/// directory moved from the per-user `%APPDATA%\fips`. +/// +/// A `fips.key` or `fips.pub` in `dir` always answers first. Only when +/// neither does is a key left at the previous default used, with a note +/// saying so, which is the same rule the daemon applies at startup. A lookup +/// there that could not be made is reported rather than read as an absence. +#[cfg(windows)] +fn own_address(dir: &Path) -> Result { + let own = match address_from_key_dir(dir) { + Ok(addr) => return Ok(addr), + Err(e) => e, + }; + match fips::config::legacy_key_fallback(&dir.join("fips.key"), dir, &fips::config::legacy_dir()) + { + Ok(Some(key)) => { + eprintln!( + "note: no key in {}; using {}, the previous default — move it to {}", + dir.display(), + key.display(), + dir.display() + ); + address_from_file(&key) + } + Ok(None) => Err(own), + Err(e) => Err(format!("{own}\n{e}")), + } +} + /// Derive a mesh address from a bech32 npub. fn address_from_npub(npub: &str) -> Result { let peer = PeerIdentity::from_npub(npub).map_err(|e| format!("invalid npub: {e}"))?; @@ -525,6 +548,23 @@ fn main() { ); } + // The Windows default key directory moved from %APPDATA%\fips to + // C:\ProgramData\fips; point at a key left at the old one. + #[cfg(windows)] + { + let legacy = fips::config::legacy_dir().join("fips.key"); + if legacy.exists() && !key_path.exists() { + eprintln!( + "note: {} exists but the default key directory", + legacy.display() + ); + eprintln!( + " is now {}; that key is no longer used by default.", + dir.display() + ); + } + } + // symlink_metadata rather than exists: a dangling symlink at the key // path reports exists() == false and would slip past the guard. if key_path.symlink_metadata().is_ok() && !force { @@ -1652,6 +1692,13 @@ mod tests { assert_eq!(default_key_dir(), PathBuf::from("/etc/fips")); } + // Windows keeps keys beside the service's config in C:\ProgramData\fips. + #[cfg(windows)] + #[test] + fn test_default_key_dir_follows_windows_layout() { + assert_eq!(default_key_dir(), PathBuf::from(r"C:\ProgramData\fips")); + } + /// Build a key file for `identity` in `dir` and return its path. fn write_identity_key(dir: &Path, identity: &Identity) -> PathBuf { let mut keypair = identity.keypair(); diff --git a/src/config/mod.rs b/src/config/mod.rs index eab80957..1576cd88 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -52,13 +52,16 @@ pub use transport::{ const CONFIG_FILENAME: &str = "fips.yaml"; /// System-wide config directory, following the platform's packaging layout -/// (`/usr/local/etc/fips` on macOS and FreeBSD, `/etc/fips` otherwise). The -/// daemon derives identity key paths from the config file's location, so -/// anything that reads or writes config-adjacent files should use this one -/// constant. +/// (`/usr/local/etc/fips` on macOS and FreeBSD, `C:\ProgramData\fips` on +/// Windows, where the service installer puts the config and the hosts file +/// already lived, `/etc/fips` otherwise). The daemon derives identity key +/// paths from the config file's location, so anything that reads or writes +/// config-adjacent files should use this one constant. #[cfg(any(target_os = "macos", target_os = "freebsd"))] pub const SYSTEM_CONFIG_DIR: &str = "/usr/local/etc/fips"; -#[cfg(not(any(target_os = "macos", target_os = "freebsd")))] +#[cfg(windows)] +pub const SYSTEM_CONFIG_DIR: &str = r"C:\ProgramData\fips"; +#[cfg(not(any(target_os = "macos", target_os = "freebsd", windows)))] pub const SYSTEM_CONFIG_DIR: &str = "/etc/fips"; /// Default key filename, placed alongside the config file. @@ -100,9 +103,75 @@ pub fn key_file_path(config_path: &Path) -> PathBuf { /// /// Equal to [`SYSTEM_CONFIG_DIR`] everywhere except where packaging installs /// outside `/etc`, which makes [`legacy_key_fallback`] inert on those -/// platforms without needing a `cfg` of its own. +/// platforms without needing a `cfg` of its own. On Windows the previous +/// default was the per-user `%APPDATA%\fips` instead; [`legacy_dir`] gives +/// the directory that applies on the running platform. const LEGACY_SYSTEM_CONFIG_DIR: &str = "/etc/fips"; +/// The previous default config directory, with the per-user config directory +/// passed in rather than looked up, so the Windows rule can be tested on any +/// platform. +/// +/// `user` is the per-user config directory (`%APPDATA%` on Windows). Without +/// one there is nothing per-user to fall back to, and the historic system +/// directory stands in. +#[cfg(any(windows, test))] +fn legacy_from(user: Option<&Path>) -> PathBuf { + match user { + Some(dir) => dir.join("fips"), + None => PathBuf::from(LEGACY_SYSTEM_CONFIG_DIR), + } +} + +/// The directory an identity key lived in before the current default. +/// +/// On Windows that is the per-user `%APPDATA%\fips`, which `fipsctl keygen` +/// wrote to before the move to [`SYSTEM_CONFIG_DIR`]. Everywhere else it is +/// the historic `/etc/fips`. +pub fn legacy_dir() -> PathBuf { + #[cfg(windows)] + { + legacy_from(dirs::config_dir().as_deref()) + } + #[cfg(not(windows))] + { + PathBuf::from(LEGACY_SYSTEM_CONFIG_DIR) + } +} + +/// The per-user config that loaded in place of the system one, if any. +/// +/// Returns `legacy/fips.yaml` when it is among `loaded` and +/// `system/fips.yaml` is not: the node is running on a config the Windows +/// service never reads. When both loaded, the per-user file is an ordinary +/// override merged over the system one, and nothing is reported. +#[cfg(any(windows, test))] +fn stranded_config(loaded: &[PathBuf], system: &Path, legacy: &Path) -> Option { + let old = legacy.join(CONFIG_FILENAME); + let new = system.join(CONFIG_FILENAME); + (loaded.contains(&old) && !loaded.contains(&new)).then_some(old) +} + +/// Warn about a config loaded from the location Windows used before +/// [`SYSTEM_CONFIG_DIR`] became `C:\ProgramData\fips`. +/// +/// `loaded` is the list of config files the daemon loaded, in order. ACL +/// files left at their old location are reported, and still enforced for +/// now, by the peer ACL reloader. +#[cfg(windows)] +pub fn warn_legacy(loaded: &[PathBuf]) { + let system = Path::new(SYSTEM_CONFIG_DIR); + if let Some(stranded) = stranded_config(loaded, system, &legacy_dir()) { + tracing::warn!( + legacy = %stranded.display(), + current = %system.join(CONFIG_FILENAME).display(), + "Config loaded from %APPDATA%\\fips but not from C:\\ProgramData\\fips; \ + the service reads only C:\\ProgramData\\fips — move fips.yaml and fips.key \ + there to run this node as the service" + ); + } +} + /// Find an identity key stranded at the legacy system config directory. /// /// Adding a second system config directory to the search path moves the @@ -124,7 +193,7 @@ const LEGACY_SYSTEM_CONFIG_DIR: &str = "/etc/fips"; /// /// Returns an error when either location cannot be examined, which the caller /// aborts on: a lookup that failed is not evidence that no key is there. -fn legacy_key_fallback( +pub fn legacy_key_fallback( key_path: &Path, system_dir: &Path, legacy_dir: &Path, @@ -473,9 +542,11 @@ fn warn_unmanaged_key_file(path: &Path) { /// only) before any key material is written, so an existing file at a looser /// mode is corrected rather than inherited. /// -/// Coverage gap: on Windows the file inherits default ACLs from the parent +/// Coverage gap: on Windows the file takes the ACL inherited from its /// directory, and neither the mode enforcement nor the symlink protection -/// applies. The exclusion is deliberate. +/// applies. `install-service.ps1` restricts `C:\ProgramData\fips` to SYSTEM +/// and Administrators, but a key written anywhere else gets whatever that +/// directory grants. pub fn write_key_file(path: &Path, nsec: &str) -> Result<(), ConfigError> { use std::io::Write; @@ -598,11 +669,9 @@ pub fn resolve_identity( // check whether one is stranded at the legacy system config directory: // generating here would silently change the node's npub, routing // address and mesh IPv6. - if let Some(legacy) = legacy_key_fallback( - &key_path, - Path::new(SYSTEM_CONFIG_DIR), - Path::new(LEGACY_SYSTEM_CONFIG_DIR), - )? { + if let Some(legacy) = + legacy_key_fallback(&key_path, Path::new(SYSTEM_CONFIG_DIR), &legacy_dir())? + { // Guarded for the same reason as the current-path read above. let nsec = Zeroizing::new(read_key_file(&legacy)?); let identity = Identity::from_secret_str(&nsec)?; @@ -968,12 +1037,13 @@ impl Config { // keep working after an upgrade. paths.push(PathBuf::from("/etc/fips").join(CONFIG_FILENAME)); - // macOS and FreeBSD packaging install config under /usr/local/etc/fips; - // probe it after /etc/fips so the packaged file wins over a stale - // /etc/fips leftover. Read from SYSTEM_CONFIG_DIR rather than a second - // literal, so this path and the directory `fipsctl keygen` writes into - // cannot drift apart. - #[cfg(any(target_os = "macos", target_os = "freebsd"))] + // macOS and FreeBSD packaging install config under /usr/local/etc/fips, + // and the Windows service installer under C:\ProgramData\fips; probe + // it after /etc/fips so the packaged file wins over a stale /etc/fips + // leftover. Read from SYSTEM_CONFIG_DIR rather than a second literal, + // so this path and the directory `fipsctl keygen` writes into cannot + // drift apart. + #[cfg(any(target_os = "macos", target_os = "freebsd", windows))] paths.push(PathBuf::from(SYSTEM_CONFIG_DIR).join(CONFIG_FILENAME)); // User config directory @@ -1859,6 +1929,77 @@ node: ); } + // --- the Windows move from %APPDATA%\fips to C:\ProgramData\fips --- + // + // The helpers take every directory as an argument, so the Windows rules + // run here on any platform against synthetic or temporary paths. + + #[test] + fn stranded_config_names_a_config_loaded_only_from_the_old_windows_dir() { + let system = Path::new("/sys-cfg/fips"); + let legacy = Path::new("/user-cfg/fips"); + let old = legacy.join(CONFIG_FILENAME); + let new = system.join(CONFIG_FILENAME); + + assert_eq!( + stranded_config(std::slice::from_ref(&old), system, legacy), + Some(old.clone()), + "a config loaded only from the old directory must be reported" + ); + assert_eq!( + stranded_config(&[new, old], system, legacy), + None, + "a per-user config merged over the system one is an override, not stranded" + ); + assert_eq!(stranded_config(&[], system, legacy), None); + assert_eq!( + stranded_config(&[PathBuf::from("./fips.yaml")], system, legacy), + None + ); + } + + #[test] + fn windows_key_fallback_finds_a_key_in_the_old_appdata_dir() { + let root = TempDir::new().unwrap(); + let appdata = root.path().join("appdata"); + let system = root.path().join("system"); + fs::create_dir_all(appdata.join("fips")).unwrap(); + fs::create_dir_all(&system).unwrap(); + let identity = crate::Identity::generate(); + let nsec = crate::encode_nsec(&identity.keypair().secret_key()); + let legacy_key = appdata.join("fips").join(KEY_FILENAME); + fs::write(&legacy_key, format!("{nsec}\n")).unwrap(); + + assert_eq!( + legacy_key_fallback( + &system.join(KEY_FILENAME), + &system, + &legacy_from(Some(&appdata)) + ) + .unwrap(), + Some(legacy_key), + "a key left in the per-user directory must be found, not regenerated" + ); + assert_eq!(legacy_from(None), PathBuf::from("/etc/fips")); + } + + #[cfg(windows)] + #[test] + fn search_paths_probe_programdata_before_the_user_config_dir() { + let paths = Config::search_paths(); + let system = PathBuf::from(r"C:\ProgramData\fips").join(CONFIG_FILENAME); + let at = |want: &Path| paths.iter().position(|p| p == want); + let system_at = at(&system).expect("C:\\ProgramData\\fips\\fips.yaml is probed"); + if let Some(user) = dirs::config_dir() { + let user_at = at(&user.join("fips").join(CONFIG_FILENAME)) + .expect("the per-user config is probed"); + assert!( + system_at < user_at, + "the per-user config overrides the system one" + ); + } + } + #[test] fn test_to_yaml() { let mut config = Config::new(); diff --git a/src/lib.rs b/src/lib.rs index b4eac67c..6f32de3b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -25,6 +25,8 @@ pub mod native; pub mod node; pub mod noise; pub mod nostr; +#[cfg(test)] +mod packaging_tests; pub mod peer; pub mod perf_profile; pub(crate) mod proto; diff --git a/src/node/acl.rs b/src/node/acl.rs index 1c53647f..d079cf3a 100644 --- a/src/node/acl.rs +++ b/src/node/acl.rs @@ -27,21 +27,26 @@ use tracing::{debug, error, info, warn}; /// On macOS (`packaging/macos/`) and FreeBSD (`packaging/freebsd/`) the /// install layout ships config under `/usr/local/etc/fips/` rather than /// `/etc/fips/`; the default follows the platform's packaging so the daemon -/// reads the file the operator was told to edit. Linux and other Unix keep -/// the historic `/etc/fips/` location. -#[cfg(not(any(target_os = "macos", target_os = "freebsd")))] +/// reads the file the operator was told to edit. Windows uses +/// `C:\ProgramData\fips\`, beside the config and hosts file the service +/// installer sets up. Linux and other Unix keep the historic `/etc/fips/` +/// location. +#[cfg(not(any(target_os = "macos", target_os = "freebsd", windows)))] pub const DEFAULT_PEERS_ALLOW_PATH: &str = "/etc/fips/peers.allow"; #[cfg(any(target_os = "macos", target_os = "freebsd"))] pub const DEFAULT_PEERS_ALLOW_PATH: &str = "/usr/local/etc/fips/peers.allow"; +#[cfg(windows)] +pub const DEFAULT_PEERS_ALLOW_PATH: &str = r"C:\ProgramData\fips\peers.allow"; /// Default path for the peer deny list. /// -/// See [`DEFAULT_PEERS_ALLOW_PATH`] for the `/usr/local/etc/fips/` -/// rationale. -#[cfg(not(any(target_os = "macos", target_os = "freebsd")))] +/// See [`DEFAULT_PEERS_ALLOW_PATH`] for the per-platform rationale. +#[cfg(not(any(target_os = "macos", target_os = "freebsd", windows)))] pub const DEFAULT_PEERS_DENY_PATH: &str = "/etc/fips/peers.deny"; #[cfg(any(target_os = "macos", target_os = "freebsd"))] pub const DEFAULT_PEERS_DENY_PATH: &str = "/usr/local/etc/fips/peers.deny"; +#[cfg(windows)] +pub const DEFAULT_PEERS_DENY_PATH: &str = r"C:\ProgramData\fips\peers.deny"; /// Warn about config files stranded at the pre-move default location. /// @@ -70,6 +75,145 @@ pub fn warn_on_legacy_config_paths() { } } +/// Which of an ACL file's two locations the reloader reads. +/// +/// Windows read `peers.allow` and `peers.deny` from `\etc\fips` on the current +/// drive before they moved to `C:\ProgramData\fips`. Ignoring a deny list +/// left at the old location would fail open, so for one release a file found +/// only there is still enforced, with a warning to move it. This fallback, with +/// [`default_legacy_paths`], is meant to be removed in a later release. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum AclPathChoice { + /// Read the current default; nothing is at the legacy location. + Current, + /// Read the legacy location: the file is there and not at the current + /// default. + Legacy, + /// Read the current default; a file also left at the legacy location is + /// ignored. + LegacyIgnored, +} + +/// Choose where to read one ACL file from: its current default path, or the +/// location an earlier release read it from. +/// +/// `exists` is passed in so the rule can be tested on any platform. The +/// current path wins whenever it exists; the legacy one is read only when it +/// is the sole file present. +fn select_acl_path(current: &Path, legacy: &Path, exists: impl Fn(&Path) -> bool) -> AclPathChoice { + match (exists(current), exists(legacy)) { + (true, true) => AclPathChoice::LegacyIgnored, + (false, true) => AclPathChoice::Legacy, + (_, false) => AclPathChoice::Current, + } +} + +/// Whether a path exists, counting one that cannot be checked as present. +/// +/// A current file that is there but inaccessible then stays selected, and +/// the loader reports it as unreadable, rather than an older legacy file +/// being enforced in its place without a word. +fn path_present(path: &Path) -> bool { + path.try_exists().unwrap_or(true) +} + +/// Locations an earlier release read the ACL files from. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct LegacyAclPaths { + pub(crate) allow: PathBuf, + pub(crate) deny: PathBuf, +} + +/// The legacy ACL locations honoured on this platform. +/// +/// On Windows that is `/etc/fips`, the exact path the `/etc/fips/peers.*` +/// defaults named before the move to `C:\ProgramData\fips`. It is kept +/// drive-relative on purpose: Windows resolves it against the current +/// drive, as it did then, so a run started from another drive still finds +/// the file an earlier release read there. Other platforms have +/// none; see [`AclPathChoice`] for why the fallback exists and that it is +/// meant to be removed in a later release. +#[cfg(windows)] +fn default_legacy_paths() -> Option { + Some(LegacyAclPaths { + allow: PathBuf::from("/etc/fips/peers.allow"), + deny: PathBuf::from("/etc/fips/peers.deny"), + }) +} + +/// The legacy ACL locations honoured on this platform: none. +#[cfg(not(windows))] +fn default_legacy_paths() -> Option { + None +} + +/// One ACL file's current default and legacy location, with the choice the +/// last selection made between them. +struct AclFallback { + current: PathBuf, + legacy: PathBuf, + /// `None` until the first selection, so the startup choice is logged. + choice: Option, +} + +impl AclFallback { + /// Pair a current default path with its legacy location. + fn new(current: PathBuf, legacy: PathBuf) -> Self { + Self { + current, + legacy, + choice: None, + } + } + + /// Select the location to read, logging the choice when it changes. + fn select(&mut self) -> &Path { + let choice = select_acl_path(&self.current, &self.legacy, path_present); + if self.choice != Some(choice) { + match choice { + AclPathChoice::Legacy => warn!( + legacy = %self.legacy.display(), + current = %self.current.display(), + "Peer ACL file found only at its legacy path; enforcing it from there \ + for now — move it to the current path, the legacy path will stop \ + being read in a later release" + ), + AclPathChoice::LegacyIgnored => warn!( + legacy = %self.legacy.display(), + current = %self.current.display(), + "Peer ACL file found at both its legacy and current paths; the legacy \ + file is ignored — remove it" + ), + AclPathChoice::Current if self.choice.is_some() => info!( + legacy = %self.legacy.display(), + current = %self.current.display(), + "Peer ACL file no longer at its legacy path; reading the current path" + ), + AclPathChoice::Current => {} + } + self.choice = Some(choice); + } + match choice { + AclPathChoice::Legacy => &self.legacy, + AclPathChoice::Current | AclPathChoice::LegacyIgnored => &self.current, + } + } +} + +/// Point `path` at the location `fallback` selects, if there is a fallback, +/// and report whether it moved. +fn reselect(fallback: &mut Option, path: &mut PathBuf) -> bool { + let Some(fallback) = fallback else { + return false; + }; + let selected = fallback.select(); + if selected == path.as_path() { + return false; + } + *path = selected.to_path_buf(); + true +} + /// Result of evaluating a peer against the ACL. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum PeerAclDecision { @@ -390,8 +534,14 @@ pub struct PeerAclReloader { /// Reader-facing effective ACL snapshot. acl: arc_swap::ArcSwap, hosts: HostMapReloader, + /// The allow and deny files read on the last load: the current defaults, + /// or a legacy location a fallback below selected instead. allow_path: PathBuf, deny_path: PathBuf, + /// Legacy locations re-checked on every reload, so the choice follows + /// the files as the operator moves them. `None` off Windows. + allow_fallback: Option, + deny_fallback: Option, last_allow_mtime: Option, last_deny_mtime: Option, /// Set while a reload input is unreadable. Forces the next reload @@ -407,12 +557,7 @@ impl PeerAclReloader { /// Create a reloader using the standard ACL file locations. #[allow(dead_code)] pub fn new() -> Self { - Self::with_alias_sources( - PathBuf::from(DEFAULT_PEERS_ALLOW_PATH), - PathBuf::from(DEFAULT_PEERS_DENY_PATH), - HostMap::new(), - PathBuf::from(DEFAULT_HOSTS_PATH), - ) + Self::with_default_paths(HostMap::new(), PathBuf::from(DEFAULT_HOSTS_PATH)) } /// Create a reloader for explicit ACL file paths. @@ -426,13 +571,49 @@ impl PeerAclReloader { ) } + /// Create the node's reloader: the platform's default ACL paths, plus + /// the legacy locations still honoured there (Windows only). + pub(crate) fn with_default_paths(base_hosts: HostMap, hosts_path: PathBuf) -> Self { + Self::with_legacy_sources( + PathBuf::from(DEFAULT_PEERS_ALLOW_PATH), + PathBuf::from(DEFAULT_PEERS_DENY_PATH), + default_legacy_paths(), + base_hosts, + hosts_path, + ) + } + /// Create a reloader with explicit ACL paths and alias sources. + #[cfg(test)] pub(crate) fn with_alias_sources( allow_path: PathBuf, deny_path: PathBuf, base_hosts: HostMap, hosts_path: PathBuf, ) -> Self { + Self::with_legacy_sources(allow_path, deny_path, None, base_hosts, hosts_path) + } + + /// Create a reloader with explicit ACL paths and alias sources that + /// reads each ACL file from its `legacy` location while the file is + /// absent from its current one. + pub(crate) fn with_legacy_sources( + mut allow_path: PathBuf, + mut deny_path: PathBuf, + legacy: Option, + base_hosts: HostMap, + hosts_path: PathBuf, + ) -> Self { + let (mut allow_fallback, mut deny_fallback) = match legacy { + Some(legacy) => ( + Some(AclFallback::new(allow_path.clone(), legacy.allow)), + Some(AclFallback::new(deny_path.clone(), legacy.deny)), + ), + None => (None, None), + }; + reselect(&mut allow_fallback, &mut allow_path); + reselect(&mut deny_fallback, &mut deny_path); + let last_allow_mtime = file_mtime(&allow_path); let last_deny_mtime = file_mtime(&deny_path); let hosts = HostMapReloader::new(base_hosts, hosts_path); @@ -459,6 +640,8 @@ impl PeerAclReloader { hosts, allow_path, deny_path, + allow_fallback, + deny_fallback, last_allow_mtime, last_deny_mtime, retry_pending, @@ -512,6 +695,10 @@ impl Reloadable for PeerAclReloader { type Snapshot = PeerAcl; async fn reload(&mut self) -> bool { + // A switch between a legacy and a current location forces the load: + // a copied file can carry the same mtime as the one it replaces. + let allow_moved = reselect(&mut self.allow_fallback, &mut self.allow_path); + let deny_moved = reselect(&mut self.deny_fallback, &mut self.deny_path); let allow_mtime = file_mtime(&self.allow_path); let deny_mtime = file_mtime(&self.deny_path); let hosts_changed = match self.hosts.try_check_reload() { @@ -527,6 +714,8 @@ impl Reloadable for PeerAclReloader { && deny_mtime == self.last_deny_mtime && !hosts_changed && !self.retry_pending + && !allow_moved + && !deny_moved { return false; } @@ -707,6 +896,277 @@ mod tests { assert_eq!(DEFAULT_PEERS_DENY_PATH, "/etc/fips/peers.deny"); } + // Windows keeps config, hosts and the ACL files in C:\ProgramData\fips, + // where the service installer puts them. + #[cfg(windows)] + #[test] + fn test_default_acl_paths_follow_windows_layout() { + assert_eq!(DEFAULT_PEERS_ALLOW_PATH, r"C:\ProgramData\fips\peers.allow"); + assert_eq!(DEFAULT_PEERS_DENY_PATH, r"C:\ProgramData\fips\peers.deny"); + } + + // The hosts file, both ACL files and the config all belong in one + // directory on every platform. A default that sits anywhere else is read + // from a directory the operator was never told about, and a missing deny + // list fails open. + #[test] + fn default_hosts_and_acl_paths_sit_in_the_system_config_dir() { + let system = Some(Path::new(crate::config::SYSTEM_CONFIG_DIR)); + for path in [ + DEFAULT_HOSTS_PATH, + DEFAULT_PEERS_ALLOW_PATH, + DEFAULT_PEERS_DENY_PATH, + ] { + assert_eq!( + Path::new(path).parent(), + system, + "{path} is outside the system config directory" + ); + } + } + + // Before the move to C:\ProgramData\fips, Windows resolved the + // `/etc/fips/peers.*` defaults against the root of the current drive, so + // that is where an existing deny list sits after an upgrade. The legacy + // path stays drive-relative so a run from another drive finds it too. + #[cfg(windows)] + #[test] + fn windows_honours_acl_files_at_the_old_drive_relative_location() { + assert_eq!( + default_legacy_paths(), + Some(LegacyAclPaths { + allow: PathBuf::from("/etc/fips/peers.allow"), + deny: PathBuf::from("/etc/fips/peers.deny"), + }) + ); + } + + // Only Windows moved its ACL files in a way that is still honoured; the + // other platforms read exactly the one default path they always did. + #[cfg(not(windows))] + #[test] + fn non_windows_platforms_have_no_legacy_acl_location() { + assert_eq!(default_legacy_paths(), None); + } + + /// Existence check over a fixed set of paths, for the selection tests. + fn exists_among<'a>(present: &'a [&'a Path]) -> impl Fn(&Path) -> bool + 'a { + move |p| present.contains(&p) + } + + #[test] + fn acl_path_selection_reads_the_legacy_file_when_only_it_exists() { + let current = Path::new(r"C:\ProgramData\fips\peers.deny"); + let legacy = Path::new("/etc/fips/peers.deny"); + assert_eq!( + select_acl_path(current, legacy, exists_among(&[legacy])), + AclPathChoice::Legacy + ); + } + + #[test] + fn acl_path_selection_prefers_the_current_file_and_flags_the_legacy_one_when_both_exist() { + let current = Path::new(r"C:\ProgramData\fips\peers.deny"); + let legacy = Path::new("/etc/fips/peers.deny"); + assert_eq!( + select_acl_path(current, legacy, exists_among(&[current, legacy])), + AclPathChoice::LegacyIgnored + ); + } + + #[test] + fn acl_path_selection_reads_the_current_path_when_nothing_is_at_the_legacy_one() { + let current = Path::new(r"C:\ProgramData\fips\peers.deny"); + let legacy = Path::new("/etc/fips/peers.deny"); + assert_eq!( + select_acl_path(current, legacy, exists_among(&[current])), + AclPathChoice::Current + ); + // Absent from both: the current path is read, and its absence is the + // ordinary "no deny list" it has always been. + assert_eq!( + select_acl_path(current, legacy, exists_among(&[])), + AclPathChoice::Current + ); + } + + /// Current and legacy ACL paths under a temporary root, as + /// `(allow, deny, legacy)`, with both directories created. + fn legacy_layout(root: &Path) -> (PathBuf, PathBuf, LegacyAclPaths) { + let new = root.join("new"); + let old = root.join("old"); + std::fs::create_dir_all(&new).unwrap(); + std::fs::create_dir_all(&old).unwrap(); + let legacy = LegacyAclPaths { + allow: old.join("peers.allow"), + deny: old.join("peers.deny"), + }; + (new.join("peers.allow"), new.join("peers.deny"), legacy) + } + + fn legacy_reloader( + root: &Path, + allow: &Path, + deny: &Path, + legacy: &LegacyAclPaths, + ) -> PeerAclReloader { + PeerAclReloader::with_legacy_sources( + allow.to_path_buf(), + deny.to_path_buf(), + Some(legacy.clone()), + HostMap::new(), + root.join("hosts"), + ) + } + + #[test] + fn a_deny_list_left_only_at_the_legacy_location_is_still_enforced() { + let dir = tempfile::tempdir().unwrap(); + let (allow, deny, legacy) = legacy_layout(dir.path()); + let denied = test_npub(); + let allowed = test_npub(); + // The two files are chosen independently: the allow list is at its + // current path, the deny list only at the legacy one. + write_file(&allow, &format!("{allowed}\n")); + write_file(&legacy.deny, &format!("{denied}\n")); + + let reloader = legacy_reloader(dir.path(), &allow, &deny, &legacy); + + assert_eq!( + reloader.acl().check(&test_peer(&denied)), + PeerAclDecision::DenyList + ); + assert_eq!( + reloader.acl().check(&test_peer(&allowed)), + PeerAclDecision::AllowList + ); + let status = reloader.status(); + assert_eq!(status.deny_file, legacy.deny.display().to_string()); + assert_eq!(status.allow_file, allow.display().to_string()); + } + + #[test] + fn a_current_acl_file_wins_over_one_left_at_the_legacy_location() { + let dir = tempfile::tempdir().unwrap(); + let (allow, deny, legacy) = legacy_layout(dir.path()); + let old_denied = test_npub(); + let new_denied = test_npub(); + write_file(&legacy.deny, &format!("{old_denied}\n")); + write_file(&deny, &format!("{new_denied}\n")); + + let reloader = legacy_reloader(dir.path(), &allow, &deny, &legacy); + + assert_eq!( + reloader.acl().check(&test_peer(&new_denied)), + PeerAclDecision::DenyList + ); + assert_eq!( + reloader.acl().check(&test_peer(&old_denied)), + PeerAclDecision::DefaultAllow + ); + assert_eq!(reloader.status().deny_file, deny.display().to_string()); + } + + #[tokio::test] + async fn acl_reload_re_evaluates_the_legacy_fallback_as_files_come_and_go() { + let dir = tempfile::tempdir().unwrap(); + let (allow, deny, legacy) = legacy_layout(dir.path()); + let old_denied = test_npub(); + let new_denied = test_npub(); + write_file(&legacy.deny, &format!("{old_denied}\n")); + + let mut reloader = legacy_reloader(dir.path(), &allow, &deny, &legacy); + assert_eq!( + reloader.acl().check(&test_peer(&old_denied)), + PeerAclDecision::DenyList + ); + assert!(!reloader.reload().await, "nothing changed on disk"); + + // The operator puts a deny list at the current path: it takes over. + std::thread::sleep(std::time::Duration::from_millis(5)); + write_file(&deny, &format!("{new_denied}\n")); + assert!(reloader.reload().await); + assert_eq!( + reloader.acl().check(&test_peer(&new_denied)), + PeerAclDecision::DenyList + ); + assert_eq!( + reloader.acl().check(&test_peer(&old_denied)), + PeerAclDecision::DefaultAllow + ); + assert_eq!(reloader.status().deny_file, deny.display().to_string()); + + // The current file goes away again: the legacy one is back in force. + std::fs::remove_file(&deny).unwrap(); + assert!(reloader.reload().await); + assert_eq!( + reloader.acl().check(&test_peer(&old_denied)), + PeerAclDecision::DenyList + ); + assert_eq!( + reloader.status().deny_file, + legacy.deny.display().to_string() + ); + } + + // A copy keeps its source's modification time on Windows, so a switch + // between the two locations can leave the tracked mtime unchanged. The + // switch itself must force the reload. + #[tokio::test] + async fn acl_reload_follows_a_switch_of_location_even_when_the_mtimes_match() { + let dir = tempfile::tempdir().unwrap(); + let (allow, deny, legacy) = legacy_layout(dir.path()); + let old_denied = test_npub(); + let new_denied = test_npub(); + write_file(&legacy.deny, &format!("{old_denied}\n")); + + let mut reloader = legacy_reloader(dir.path(), &allow, &deny, &legacy); + assert_eq!( + reloader.acl().check(&test_peer(&old_denied)), + PeerAclDecision::DenyList + ); + + write_file(&deny, &format!("{new_denied}\n")); + let old_mtime = std::fs::metadata(&legacy.deny).unwrap().modified().unwrap(); + std::fs::File::options() + .write(true) + .open(&deny) + .unwrap() + .set_modified(old_mtime) + .unwrap(); + assert_eq!(file_mtime(&deny), file_mtime(&legacy.deny)); + + assert!(reloader.reload().await); + assert_eq!( + reloader.acl().check(&test_peer(&new_denied)), + PeerAclDecision::DenyList + ); + } + + // A current file whose existence cannot be checked must not hand the + // decision to an older legacy file: it stays selected, and the loader + // reports it as unreadable. Root bypasses the directory's mode bits, so + // the test skips there rather than passing vacuously. + #[cfg(unix)] + #[test] + fn an_acl_path_that_cannot_be_checked_counts_as_present() { + use std::os::unix::fs::PermissionsExt; + let dir = tempfile::tempdir().unwrap(); + let sealed = dir.path().join("sealed"); + std::fs::create_dir(&sealed).unwrap(); + let file = sealed.join("peers.deny"); + write_file(&file, ""); + std::fs::set_permissions(&sealed, std::fs::Permissions::from_mode(0o000)).unwrap(); + let checkable = file.try_exists().is_ok(); + let present = path_present(&file); + std::fs::set_permissions(&sealed, std::fs::Permissions::from_mode(0o755)).unwrap(); + if checkable { + eprintln!("skipping: the effective uid can search a mode-000 directory"); + return; + } + assert!(present); + } + #[test] fn test_acl_decision_allowed_and_display() { assert!(PeerAclDecision::AllowList.allowed()); diff --git a/src/node/mod.rs b/src/node/mod.rs index 6722cf7d..743b0020 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -870,12 +870,7 @@ impl Node { let hosts_path = std::path::PathBuf::from(crate::upper::hosts::DEFAULT_HOSTS_PATH); let host_map = reloadable::HostMapReloadable::new(base_host_map.clone(), hosts_path.clone()); - let peer_acl = acl::PeerAclReloader::with_alias_sources( - std::path::PathBuf::from(acl::DEFAULT_PEERS_ALLOW_PATH), - std::path::PathBuf::from(acl::DEFAULT_PEERS_DENY_PATH), - base_host_map, - hosts_path, - ); + let peer_acl = acl::PeerAclReloader::with_default_paths(base_host_map, hosts_path); #[cfg(unix)] let (decrypt_fallback_tx, decrypt_fallback_rx) = @@ -1051,12 +1046,7 @@ impl Node { let hosts_path = std::path::PathBuf::from(crate::upper::hosts::DEFAULT_HOSTS_PATH); let host_map = reloadable::HostMapReloadable::new(base_host_map.clone(), hosts_path.clone()); - let peer_acl = acl::PeerAclReloader::with_alias_sources( - std::path::PathBuf::from(acl::DEFAULT_PEERS_ALLOW_PATH), - std::path::PathBuf::from(acl::DEFAULT_PEERS_DENY_PATH), - base_host_map, - hosts_path, - ); + let peer_acl = acl::PeerAclReloader::with_default_paths(base_host_map, hosts_path); #[cfg(unix)] let (decrypt_fallback_tx, decrypt_fallback_rx) = diff --git a/src/packaging_tests.rs b/src/packaging_tests.rs new file mode 100644 index 00000000..6716cac1 --- /dev/null +++ b/src/packaging_tests.rs @@ -0,0 +1,760 @@ +//! Checks that the shipped packaging files agree with each other and with the +//! code that consumes them. +//! +//! Every file is read at run time from the source tree rather than with +//! `include_str!`, so a file that is missing is a named test failure instead of +//! a compile error. Lines are trimmed at the end before matching, so a CRLF +//! checkout reads the same as an LF one. + +use std::collections::HashMap; +use std::path::Path; + +/// Reads `rel`, a path relative to the crate root, panicking with the path on +/// failure. +fn repo_file(rel: &str) -> String { + let path = Path::new(env!("CARGO_MANIFEST_DIR")).join(rel); + std::fs::read_to_string(&path).unwrap_or_else(|e| panic!("{rel}: {e}")) +} + +/// Returns the lines of a TOML document after the line `header`, up to the +/// next line that starts (untrimmed) with `[`. +/// +/// Array entries indented under a key begin with spaces, so they do not end +/// the section. +fn toml_section<'a>(text: &'a str, header: &str) -> Vec<&'a str> { + let mut lines = text.lines().map(str::trim_end); + assert!( + lines.by_ref().any(|l| l == header), + "no {header} section found" + ); + lines.take_while(|l| !l.starts_with('[')).collect() +} + +/// Returns the package names in a comma-separated `key = "..."` value of +/// `[package.metadata.deb]`, each cut at its first space or `(` so a version +/// constraint is dropped. +fn deb_list(cargo_toml: &str, key: &str) -> Vec { + let prefix = format!("{key} = \""); + let value = toml_section(cargo_toml, "[package.metadata.deb]") + .into_iter() + .find_map(|l| l.strip_prefix(prefix.as_str())) + .unwrap_or_else(|| panic!("no `{key} = \"...\"` line in [package.metadata.deb]")); + let value = value + .strip_suffix('"') + .unwrap_or_else(|| panic!("[package.metadata.deb] {key} is not a one-line string")); + value + .split(',') + .map(|item| { + let item = item.trim(); + let end = item.find([' ', '(']).unwrap_or(item.len()); + item[..end].to_string() + }) + .collect() +} + +/// Returns the single-quoted items of the bash array `name=( ... )` in a +/// PKGBUILD. +/// +/// The opening `name=(` must start a line, so `depends` does not match +/// `makedepends=(` or `optdepends=(`. The array may span lines; unquoted `#` +/// starts a comment that runs to the end of the line. +fn bash_array(pkgbuild: &str, name: &str) -> Vec { + let open = format!("{name}=("); + let mut lines = pkgbuild.lines().map(str::trim_end); + let first = lines + .by_ref() + .find_map(|l| l.strip_prefix(open.as_str())) + .unwrap_or_else(|| panic!("no line starting `{open}`")); + let mut items = Vec::new(); + let mut quoted: Option = None; + for line in std::iter::once(first).chain(lines) { + for c in line.chars() { + match quoted.as_mut() { + Some(item) if c == '\'' => { + items.push(std::mem::take(item)); + quoted = None; + } + Some(item) => item.push(c), + None if c == '\'' => quoted = Some(String::new()), + None if c == ')' => return items, + None if c == '#' => break, + None => {} + } + } + if let Some(item) = quoted.as_mut() { + item.push('\n'); + } + } + panic!("`{open}` is never closed"); +} + +/// Returns the variables a FreeBSD rc script sets: `name="value"` assignments +/// at column 0 and `: ${name:="value"}` defaults. +/// +/// `${var}` references in a value are expanded from the variables set on +/// earlier lines; an unset variable expands to nothing, as in sh. +fn rc_vars(rc: &str) -> HashMap { + let mut vars = HashMap::new(); + for line in rc.lines().map(str::trim_end) { + let assignment = line + .strip_prefix(": ${") + .and_then(|rest| rest.strip_suffix('}')) + .and_then(|rest| rest.split_once(":=")) + .or_else(|| line.split_once('=')); + let Some((name, value)) = assignment else { + continue; + }; + let is_name = !name.is_empty() + && name.chars().all(|c| c == '_' || c.is_ascii_alphanumeric()) + && !name.starts_with(|c: char| c.is_ascii_digit()); + let Some(value) = value + .strip_prefix('"') + .and_then(|v| v.strip_suffix('"')) + .filter(|_| is_name) + else { + continue; + }; + let expanded = expand_vars(value, &vars); + vars.insert(name.to_string(), expanded); + } + vars +} + +/// Expands each `${name}` in `value` from `vars`, an unset name giving the +/// empty string. +fn expand_vars(value: &str, vars: &HashMap) -> String { + let mut out = String::new(); + let mut rest = value; + while let Some(start) = rest.find("${") { + out.push_str(&rest[..start]); + let after = &rest[start + 2..]; + let end = after + .find('}') + .unwrap_or_else(|| panic!("unclosed ${{ in {value:?}")); + out.push_str(vars.get(&after[..end]).map_or("", String::as_str)); + rest = &after[end + 1..]; + } + out.push_str(rest); + out +} + +/// Returns the lines of a shell script with trailing-backslash continuations +/// joined into one line each. +fn logical_lines(sh: &str) -> Vec { + let mut out = Vec::new(); + let mut pending = String::new(); + for line in sh.lines().map(str::trim_end) { + match line.strip_suffix('\\') { + Some(head) => { + pending.push_str(head); + pending.push(' '); + } + None => { + pending.push_str(line); + out.push(std::mem::take(&mut pending)); + } + } + } + if !pending.is_empty() { + out.push(pending); + } + out +} + +/// Returns the logical lines of a shell script, trimmed at both ends, without +/// the lines that are comments. +fn code_lines(sh: &str) -> Vec { + logical_lines(sh) + .into_iter() + .map(|l| l.trim().to_string()) + .filter(|l| !l.starts_with('#')) + .collect() +} + +/// Returns the code lines of the `case` branch `label)` in a shell script: +/// those after the line that trims to `label)`, up to the next line that trims +/// to `;;`. +fn case_branch(sh: &str, label: &str) -> Vec { + let open = format!("{label})"); + let lines = code_lines(sh); + let start = 1 + lines + .iter() + .position(|l| *l == open) + .unwrap_or_else(|| panic!("no `{open}` branch found")); + let len = lines[start..] + .iter() + .position(|l| l == ";;") + .unwrap_or_else(|| panic!("`{open}` branch is never closed with `;;`")); + lines[start..start + len].to_vec() +} + +#[test] +fn deb_and_aur_packages_declare_nftables_for_the_firewall_units_nft() { + let unit = repo_file("packaging/debian/fips-firewall.service"); + assert!( + unit.lines() + .map(str::trim_end) + .any(|l| l.starts_with("ExecStart=") && l.contains("/usr/sbin/nft")), + "fips-firewall.service no longer starts /usr/sbin/nft; revisit whether the \ + packages still need to declare nftables" + ); + + let mut undeclared = Vec::new(); + + let cargo = repo_file("Cargo.toml"); + let depends = deb_list(&cargo, "depends"); + let recommends = deb_list(&cargo, "recommends"); + assert!( + recommends.iter().any(|d| d == "bluez") && depends.iter().any(|d| d == "systemd"), + "control: expected bluez in recommends and systemd in depends, \ + read depends {depends:?}, recommends {recommends:?}" + ); + if !depends.iter().chain(&recommends).any(|d| d == "nftables") { + undeclared.push(format!( + "Cargo.toml [package.metadata.deb]: depends {depends:?}, recommends {recommends:?}" + )); + } + + for rel in ["packaging/aur/PKGBUILD", "packaging/aur/PKGBUILD-git"] { + let text = repo_file(rel); + let depends = bash_array(&text, "depends"); + let optdepends: Vec = bash_array(&text, "optdepends") + .iter() + .map(|item| { + item.split(':') + .next() + .unwrap_or_default() + .trim() + .to_string() + }) + .collect(); + assert!( + optdepends.iter().any(|d| d == "systemd-resolved") + && depends.iter().any(|d| d == "glibc"), + "{rel} control: expected systemd-resolved in optdepends and glibc in depends, \ + read depends {depends:?}, optdepends {optdepends:?}" + ); + if !depends.iter().chain(&optdepends).any(|d| d == "nftables") { + undeclared.push(format!( + "{rel}: depends {depends:?}, optdepends {optdepends:?}" + )); + } + } + + assert!( + undeclared.is_empty(), + "fips-firewall.service runs /usr/sbin/nft, but nftables is declared in neither \ + the required nor the optional dependencies of:\n {}", + undeclared.join("\n ") + ); +} + +#[test] +fn freebsd_newsyslog_entry_signals_the_daemon8_supervisor_started_with_sighup_reopen() { + let rc = repo_file("packaging/freebsd/fips.rc"); + let vars = rc_vars(&rc); + let args = vars + .get("command_args") + .unwrap_or_else(|| panic!("fips.rc sets no command_args")); + let procname = vars + .get("procname") + .unwrap_or_else(|| panic!("fips.rc sets no procname")); + let tokens: Vec<&str> = args.split_whitespace().collect(); + // daemon(8)'s own options are the tokens before the command it runs. + let daemon_opts = tokens + .iter() + .position(|t| t == procname) + .map(|i| &tokens[..i]) + .unwrap_or_else(|| panic!("fips.rc command_args does not run {procname}: {args}")); + let operand = |flag: &str, what: &str| -> String { + daemon_opts + .iter() + .position(|t| *t == flag) + .and_then(|i| daemon_opts.get(i + 1)) + .map(|s| s.to_string()) + .unwrap_or_else(|| panic!("fips.rc starts daemon(8) without {flag} <{what}>: {args}")) + }; + let child_pidfile = operand("-p", "child pidfile"); + let supervisor_pidfile = operand("-P", "supervisor pidfile"); + let logfile = operand("-o", "log file"); + assert!( + daemon_opts.contains(&"-H"), + "fips.rc starts daemon(8) without -H, so a SIGHUP from newsyslog does not \ + reopen {logfile} and the daemon keeps writing into the rotated file: {args}" + ); + assert_ne!( + child_pidfile, supervisor_pidfile, + "fips.rc gives daemon(8) the same pidfile for -p and -P" + ); + + let rel = "packaging/freebsd/fips.newsyslog"; + let entry = repo_file(rel); + let entries: Vec<&str> = entry + .lines() + .map(str::trim_end) + .filter(|l| !l.trim_start().is_empty() && !l.trim_start().starts_with('#')) + .collect(); + let [line] = entries[..] else { + panic!("{rel}: expected exactly one entry, found {entries:?}"); + }; + let mut fields = line.split_whitespace().peekable(); + let entry_logfile = fields.next().unwrap_or_default(); + fields.next_if(|f| f.contains(':')); + let mode = fields.next().unwrap_or_default(); + let entry_pidfile = fields.find(|f| f.starts_with('/')); + assert_eq!( + entry_logfile, logfile, + "{rel} rotates a different file from the one fips.rc passes to daemon(8) -o" + ); + assert_eq!( + mode, "600", + "{rel} creates the rotated log with a mode other than daemon(8)'s 600" + ); + assert_eq!( + entry_pidfile, + Some(supervisor_pidfile.as_str()), + "{rel} must signal the daemon(8) supervisor (-P), the only process that \ + reopens the log on SIGHUP; the child pidfile (-p) is {child_pidfile}" + ); + + let build = repo_file("packaging/freebsd/build-pkg.sh"); + let installed = "/usr/local/etc/newsyslog.conf.d/fips.conf"; + assert!( + logical_lines(&build) + .iter() + .any(|l| l.starts_with("install") + && l.contains("fips.newsyslog") + && l.contains(installed)), + "build-pkg.sh does not install fips.newsyslog as {installed}" + ); + let plist: Vec<&str> = build + .lines() + .map(str::trim_end) + .skip_while(|l| *l != r#"cat > "${STAGE}/pkg-plist" <<'EOF'"#) + .skip(1) + .take_while(|l| *l != "EOF") + .collect(); + assert!( + plist.contains(&"etc/rc.d/fips"), + "control: build-pkg.sh pkg-plist heredoc not found or lacks etc/rc.d/fips: {plist:?}" + ); + assert!( + plist.contains(&"etc/newsyslog.conf.d/fips.conf"), + "build-pkg.sh pkg-plist does not list etc/newsyslog.conf.d/fips.conf: {plist:?}" + ); +} + +/// Pins the DNS cleanup in `postrm purge` and `uninstall.sh` to the files +/// `fips-dns-setup` writes, so a purge after a `fips-dns` that never ran its +/// teardown does not leave the resolver sending `.fips` to a dead responder. +/// +/// This is a text test. Each path must appear on an `rm -f` line, but a +/// resolver command passes wherever it appears on a code line, including in a +/// message. What `postrm` actually does is covered by the deb-install purge +/// check. No suite runs `uninstall.sh`: its two resolved paths were run once, +/// by hand in a container, and its dnsmasq and NetworkManager paths by nothing. +#[test] +fn dns_cleanup_in_postrm_purge_and_uninstall_removes_every_file_fips_dns_setup_writes_and_restarts_its_resolver() + { + let setup = rc_vars(&repo_file("packaging/common/fips-dns-setup")); + let teardown = rc_vars(&repo_file("packaging/common/fips-dns-teardown")); + let paths: Vec<&str> = [ + "DNS_DELEGATE_FILE", + "RESOLVED_DROPIN_FILE", + "DNSMASQ_CONF", + "NM_DNSMASQ_CONF", + ] + .into_iter() + .map(|name| { + let path = setup + .get(name) + .filter(|p| p.starts_with('/')) + .unwrap_or_else(|| panic!("fips-dns-setup sets no absolute {name}")); + assert_eq!( + teardown.get(name), + Some(path), + "fips-dns-teardown's {name} is not the file fips-dns-setup writes" + ); + path.as_str() + }) + .collect(); + let commands = [ + "restart systemd-resolved", + "reload dnsmasq", + "nmcli general reload", + ]; + + let scripts = [ + ( + "packaging/debian/postrm purge)", + case_branch(&repo_file("packaging/debian/postrm"), "purge"), + ), + ( + "packaging/systemd/uninstall.sh", + code_lines(&repo_file("packaging/systemd/uninstall.sh")), + ), + ]; + let mut missing = Vec::new(); + for (script, lines) in &scripts { + for path in &paths { + if !lines + .iter() + .any(|l| l.contains("rm -f") && l.contains(path)) + { + missing.push(format!("{script}: no `rm -f` of {path}")); + } + } + for command in commands { + if !lines.iter().any(|l| l.contains(command)) { + missing.push(format!("{script}: never runs `{command}`")); + } + } + } + assert!( + missing.is_empty(), + "DNS cleanup does not match the files fips-dns-setup writes and the resolvers \ + fips-dns-teardown restarts:\n {}", + missing.join("\n ") + ); +} + +/// Returns the lines of a PowerShell script, trimmed, without blank lines and +/// without lines that are only a `#` comment. +fn ps_lines(ps1: &str) -> Vec { + ps1.lines() + .map(str::trim) + .filter(|l| !l.is_empty() && !l.starts_with('#')) + .map(str::to_string) + .collect() +} + +/// Guards the order in which install-service.ps1 secures `C:\ProgramData\fips`. +/// +/// The directory inherits `C:\ProgramData`'s access, under which any local +/// user can read the files in it and create missing ones, and a user can +/// create the directory, or a junction in its place, before the installer +/// runs, or turn an empty one into a junction. So the installer must build +/// the restricted ACL and create a new directory with it in one step, refuse +/// a link and a directory owned by another account, take ownership, check the +/// directory and the entries inside for links and folders before replacing +/// the ACL (applying it propagates into them) and again after it, reset each +/// file, check once more, and only then name any path inside the directory. +#[test] +fn windows_installer_restricts_config_dir_before_any_path_inside_it() { + let lines = ps_lines(&repo_file("packaging/windows/install-service.ps1")); + let nth = |what: &str, n: usize, pred: &dyn Fn(&str) -> bool| -> usize { + lines + .iter() + .enumerate() + .filter(|(_, l)| pred(l)) + .nth(n) + .map(|(i, _)| i) + .unwrap_or_else(|| { + panic!( + "install-service.ps1: no {} line {what}", + ["first", "second", "third"][n] + ) + }) + }; + let is_check = |l: &str| l == "& $refuseEntries"; + let is_create = |l: &str| l.contains("CreateDirectory(") && l.contains("$ConfigDir"); + + let order = [ + ( + "SetAccessRuleProtection", + nth( + "containing SetAccessRuleProtection($true, $false)", + 0, + &|l| l.contains("SetAccessRuleProtection($true, $false)"), + ), + ), + ( + "creation of $ConfigDir", + nth("containing CreateDirectory( and $ConfigDir", 0, &is_create), + ), + ( + "ReparsePoint check on $ConfigDir", + nth("containing ReparsePoint", 0, &|l| { + l.contains("ReparsePoint") + }), + ), + ( + "owner check", + nth("containing GetOwner", 0, &|l| l.contains("GetOwner")), + ), + ( + "directory /setowner", + nth("containing /setowner", 0, &|l| l.contains("/setowner")), + ), + ( + "$refuseEntries definition", + nth("starting $refuseEntries =", 0, &|l| { + l.starts_with("$refuseEntries =") + }), + ), + ( + "check before the lock", + nth("that is & $refuseEntries", 0, &is_check), + ), + ( + "Set-Acl of $ConfigDir", + nth("containing Set-Acl and $ConfigDir", 0, &|l| { + l.contains("Set-Acl") && l.contains("$ConfigDir") + }), + ), + ( + "check after the lock", + nth("that is & $refuseEntries", 1, &is_check), + ), + ( + "child /setowner", + nth("containing /setowner", 1, &|l| l.contains("/setowner")), + ), + ( + "child /reset", + nth("containing /reset", 0, &|l| l.contains("/reset")), + ), + ( + "check after the reset", + nth("that is & $refuseEntries", 2, &is_check), + ), + ( + "first path inside $ConfigDir", + nth("containing $ConfigDir\\", 0, &|l| { + l.contains("$ConfigDir\\") + }), + ), + ]; + for pair in order.windows(2) { + let [(a, ia), (b, ib)] = pair else { + unreachable!("windows(2) yields pairs") + }; + assert!( + ia < ib, + "install-service.ps1: {a} (code line {ia}) must come before {b} (code line {ib})" + ); + } + + let (protect, create) = (order[0].1, order[1].1); + for needle in [ + "S-1-5-18", + "S-1-5-32-544", + "ContainerInherit", + "ObjectInherit", + ] { + assert!( + lines[protect..create].iter().any(|l| l.contains(needle)), + "install-service.ps1: the ACL built between SetAccessRuleProtection and \ + the creation of $ConfigDir does not name {needle}" + ); + } + for l in lines.iter().filter(|l| l.contains("CreateDirectory")) { + assert!( + l.contains("$acl") && l.contains("$ConfigDir"), + "install-service.ps1: $ConfigDir must be created with $acl in one step: {l}" + ); + } + if let Some(l) = lines + .iter() + .find(|l| l.contains("New-Item") && l.contains("$ConfigDir")) + { + panic!("install-service.ps1: New-Item creates $ConfigDir without its ACL: {l}"); + } + let set_acl = &lines[order[7].1]; + assert!( + set_acl.contains("-AclObject $acl"), + "install-service.ps1: Set-Acl does not apply the ACL built for creation: {set_acl}" + ); + + let (def, first_check) = (order[5].1, order[6].1); + let block = &lines[def..first_check]; + assert!( + block + .iter() + .any(|l| l.contains("Get-Item -LiteralPath $ConfigDir") && l.contains("ReparsePoint")), + "install-service.ps1: the $refuseEntries script block does not test whether \ + $ConfigDir itself has become a link" + ); + assert!( + block.iter().any(|l| l.contains("Get-ChildItem -LiteralPath $ConfigDir") + && !l.contains("-Recurse")), + "install-service.ps1: the $refuseEntries script block does not list $ConfigDir's entries" + ); + for needle in ["ReparsePoint", "PSIsContainer"] { + assert!( + block + .iter() + .any(|l| l.contains("$item") && l.contains(needle)), + "install-service.ps1: the $refuseEntries script block does not test {needle} \ + on each entry" + ); + } + + let checks = lines.iter().filter(|l| is_check(l)).count(); + assert_eq!( + checks, 3, + "install-service.ps1: expected exactly three & $refuseEntries lines, found {checks}" + ); + let setowners = lines.iter().filter(|l| l.contains("/setowner")).count(); + assert_eq!( + setowners, 2, + "install-service.ps1: expected exactly two /setowner lines, found {setowners}" + ); + if let Some(l) = lines.iter().find(|l| l.contains("-Recurse")) { + panic!("install-service.ps1: -Recurse is not allowed: {l}"); + } +} + +/// Guards the conditions under which install-service.ps1 refuses its config +/// directory, not only their position. +/// +/// Each refusal must be an `if` whose body is `Write-Error` then `exit 1`: an +/// owner outside SYSTEM, Administrators and the installing account; the +/// directory being a link, both at the start and inside every recheck; and an +/// entry that is a link or a folder, either of which is enough. A condition +/// that can never hold, or that needs both a link and a folder, would pass an +/// ordering check while refusing nothing. +#[test] +fn windows_installer_refusal_conditions_stop_the_install() { + let lines = ps_lines(&repo_file("packaging/windows/install-service.ps1")); + let refuses_at = |i: usize, what: &str| { + let cond = &lines[i]; + assert!( + cond.starts_with("if (") && cond.ends_with('{'), + "install-service.ps1: the {what} is not an if statement: {cond}" + ); + let body = lines.get(i + 1..i + 3).unwrap_or_default(); + assert!( + body.len() == 2 && body[0].starts_with("Write-Error ") && body[1] == "exit 1", + "install-service.ps1: the {what} is not followed by Write-Error then exit 1: \ + {cond}\n then: {body:?}" + ); + }; + let find = |what: &str, pred: &dyn Fn(&str) -> bool| -> Vec { + let found: Vec = lines + .iter() + .enumerate() + .filter(|(_, l)| pred(l)) + .map(|(i, _)| i) + .collect(); + assert!( + !found.is_empty(), + "install-service.ps1: no line found for the {what}" + ); + found + }; + + let trusted = find("list of trusted owners", &|l| { + l.starts_with("$trustedOwners = @(") + }); + for needle in [ + "\"S-1-5-18\"", + "\"S-1-5-32-544\"", + "WindowsIdentity]::GetCurrent().User.Value", + ] { + assert!( + lines[trusted[0]].contains(needle), + "install-service.ps1: the trusted owners do not include {needle}: {}", + lines[trusted[0]] + ); + } + for i in find("owner refusal", &|l| { + l == "if ($trustedOwners -notcontains $ownerSid) {" + }) { + refuses_at(i, "owner refusal"); + } + + let dir_links = find("refusal of $ConfigDir as a link", &|l| { + l.starts_with("if (") && l.contains("ReparsePoint") && !l.contains("$item") + }); + assert_eq!( + dir_links.len(), + 2, + "install-service.ps1: expected the directory's link check once at the start and \ + once in $refuseEntries, found {}", + dir_links.len() + ); + for i in dir_links { + refuses_at(i, "refusal of $ConfigDir as a link"); + } + + for i in find("refusal of a link or folder entry", &|l| { + l.starts_with("if (") && l.contains("$item") + }) { + let cond = &lines[i]; + assert!( + cond.contains("ReparsePoint") + && cond.contains("PSIsContainer") + && cond.contains(" -or ") + && !cond.contains(" -and "), + "install-service.ps1: an entry must be refused if it is a link or a folder, \ + either one: {cond}" + ); + refuses_at(i, "refusal of a link or folder entry"); + } +} + +/// Guards the recovery install-service.ps1 gives for a directory it refuses. +/// +/// A user who created `C:\ProgramData\fips` holds full control of it through +/// an inherited entry for their own account. Taking ownership changes only the +/// owner, so the installer's owner check would then pass while that user could +/// still swap the directory for a junction. The recovery must be to delete the +/// directory, and the installer must not offer `takeown` as a way through. +#[test] +fn windows_installer_refusals_never_offer_takeown_as_the_recovery() { + let lines = ps_lines(&repo_file("packaging/windows/install-service.ps1")); + if let Some(l) = lines + .iter() + .find(|l| l.to_ascii_lowercase().contains("takeown")) + { + panic!("install-service.ps1: a refusal offers takeown as the recovery: {l}"); + } + let owner_refusals = lines + .iter() + .filter(|l| l.contains("Write-Error") && l.contains("delete the directory")) + .count(); + assert!( + owner_refusals >= 2, + "install-service.ps1: expected the owner refusals to say to delete the directory, \ + found {owner_refusals} such lines" + ); +} + +/// Guards every icacls call in install-service.ps1: each acts on a link itself +/// rather than its target (`/L`), never walks a tree (`/T`), and has its exit +/// code checked on the next line, since `$ErrorActionPreference = "Stop"` does +/// not cover a native command's exit code in Windows PowerShell 5.1. +#[test] +fn windows_installer_icacls_calls_act_on_links_and_check_exit_codes() { + let lines = ps_lines(&repo_file("packaging/windows/install-service.ps1")); + let calls: Vec = lines + .iter() + .enumerate() + .filter(|(_, l)| l.contains("& $icacls")) + .map(|(i, _)| i) + .collect(); + assert!( + calls.len() >= 3, + "install-service.ps1: expected at least three & $icacls calls, found {}", + calls.len() + ); + for i in calls { + let call = &lines[i]; + let tokens: Vec<&str> = call.split_whitespace().collect(); + assert!( + tokens.iter().any(|t| t.eq_ignore_ascii_case("/L")), + "install-service.ps1: icacls call without /L follows a link: {call}" + ); + assert!( + !tokens.iter().any(|t| t.eq_ignore_ascii_case("/T")), + "install-service.ps1: icacls call with /T walks the tree: {call}" + ); + let next = lines.get(i + 1).map(String::as_str).unwrap_or_default(); + assert!( + next.contains("$LASTEXITCODE"), + "install-service.ps1: icacls call not followed by a $LASTEXITCODE check: \ + {call}\n next line: {next}" + ); + } +} diff --git a/src/utils/logfile.rs b/src/utils/logfile.rs new file mode 100644 index 00000000..25f1065a --- /dev/null +++ b/src/utils/logfile.rs @@ -0,0 +1,415 @@ +//! A size-rolling log file for the Windows service. +//! +//! A process started by the service control manager has no standard handles, +//! and writes to its absent stdout report success, so the daemon's log is lost +//! unless it goes to a file. The file is rolled by size, which bounds the disk +//! it can take: `ROLL_KEEP` old files of about `ROLL_BYTES` each, plus the +//! current one. +//! +//! Nothing here may emit a tracing event. The writer runs inside the +//! subscriber with the `SharedLog` lock held, so an event raised from here +//! would re-enter that lock on the same thread and deadlock. A roll that fails +//! is therefore silent; the file growing past its cap is the only sign. + +use std::fs::{self, File, OpenOptions}; +use std::io::{self, Write}; +use std::path::{Path, PathBuf}; +use std::sync::{Arc, Mutex, MutexGuard, PoisonError, TryLockError}; +use tracing_subscriber::fmt::MakeWriter; + +/// Size at which the service log is rolled. +pub const ROLL_BYTES: u64 = 10 * 1024 * 1024; + +/// Number of rolled service log files kept beside the current one. +pub const ROLL_KEEP: u32 = 4; + +/// An append-only file that is renamed aside once it would pass `max` bytes. +/// +/// Old files are `path.1` (newest) to `path.{keep}` (oldest). Writes are +/// unbuffered, so a `process::exit` right after a log line loses nothing. +pub struct RollingFile { + path: PathBuf, + max: u64, + keep: u32, + file: Option, + len: u64, + limit: u64, +} + +/// Open `path` for appending, creating it if absent. +fn append(path: &Path) -> io::Result { + OpenOptions::new().create(true).append(true).open(path) +} + +impl RollingFile { + /// Open or create the log at `path`, keeping `keep` old files (at least + /// one) and rolling at `max` bytes. + /// + /// Bytes already in the file from earlier runs count toward the cap. + pub fn open(path: &Path, max: u64, keep: u32) -> io::Result { + if let Some(parent) = path.parent().filter(|p| !p.as_os_str().is_empty()) { + fs::create_dir_all(parent)?; + } + let file = append(path)?; + let len = file.metadata()?.len(); + Ok(Self { + path: path.to_path_buf(), + max, + keep: keep.max(1), + file: Some(file), + len, + limit: max, + }) + } + + /// The path of the `n`th rolled file. + fn numbered(&self, n: u32) -> PathBuf { + let mut name = self.path.clone().into_os_string(); + name.push(format!(".{n}")); + PathBuf::from(name) + } + + /// The open handle, reopening the file if an earlier roll or open left + /// none. A reopen takes the length from the file itself. + fn handle(&mut self) -> io::Result<&mut File> { + let file = match self.file.take() { + Some(file) => file, + None => { + let file = append(&self.path)?; + self.len = file.metadata()?.len(); + file + } + }; + Ok(self.file.insert(file)) + } + + /// Rename the current file aside and shift the older ones down, dropping + /// the oldest. The next write reopens `path` as a new, empty file. + /// + /// The handle is dropped first so that a failed roll reopens cleanly and + /// nothing is left pointing at `path.1`. Renaming a file this process + /// holds open is allowed on Windows too (std opens with + /// `FILE_SHARE_DELETE`); a rename that fails there is one another process + /// blocks, such as a viewer holding the file without delete sharing. + fn roll(&mut self) -> io::Result<()> { + self.file = None; + for n in (1..self.keep).rev() { + match fs::rename(self.numbered(n), self.numbered(n + 1)) { + Err(e) if e.kind() != io::ErrorKind::NotFound => return Err(e), + _ => {} + } + } + fs::rename(&self.path, self.numbered(1))?; + self.len = 0; + self.limit = self.max; + Ok(()) + } + + /// Append `buf` to the current file and count it. + fn append_buf(&mut self, buf: &[u8]) -> io::Result { + let n = self.handle()?.write(buf)?; + self.len += n as u64; + Ok(n) + } +} + +impl Write for RollingFile { + /// Write all of `buf` to one file, rolling first if it would take the + /// file past its limit. A file that is still empty is never rolled, so a + /// write larger than the cap lands whole instead of leaving an empty + /// rolled file behind. + /// + /// A roll that fails leaves the current file in use: `buf` is appended to + /// it and the next attempt waits until another `max` bytes have been + /// written, so a file held by another process does not turn every write + /// into a rename attempt. Only a failure to open the file is an error. + fn write(&mut self, buf: &[u8]) -> io::Result { + let size = buf.len() as u64; + if self.len > 0 && self.len + size > self.limit && self.roll().is_err() { + let n = self.append_buf(buf)?; + self.limit = self.len + self.max; + return Ok(n); + } + self.append_buf(buf) + } + + fn flush(&mut self) -> io::Result<()> { + match self.file.as_mut() { + Some(file) => file.flush(), + None => Ok(()), + } + } +} + +/// A [`RollingFile`] shared between the tracing subscriber, startup error +/// reporting and the panic hook. +#[derive(Clone)] +pub struct SharedLog(Arc>); + +/// A [`SharedLog`] held locked for the length of one tracing event. +pub struct LogGuard<'a>(MutexGuard<'a, RollingFile>); + +impl Write for LogGuard<'_> { + fn write(&mut self, buf: &[u8]) -> io::Result { + self.0.write(buf) + } + + fn flush(&mut self) -> io::Result<()> { + self.0.flush() + } +} + +impl<'a> MakeWriter<'a> for SharedLog { + type Writer = LogGuard<'a>; + + fn make_writer(&'a self) -> Self::Writer { + LogGuard(self.0.lock().unwrap_or_else(PoisonError::into_inner)) + } +} + +impl SharedLog { + /// Share `file`. + pub fn new(file: RollingFile) -> Self { + Self(Arc::new(Mutex::new(file))) + } + + /// Write `text` as one line, waiting for the lock. + pub fn line(&self, text: &str) { + let mut file = self.0.lock().unwrap_or_else(PoisonError::into_inner); + let _ = file.write_all(format!("{text}\n").as_bytes()); + } + + /// Write `text` as one line if the lock is free, for the panic hook. + /// + /// A panic raised while this thread already holds the lock, inside a + /// tracing event, must not wait for it, so a held lock drops the line. So + /// does one held by another thread at that moment. + pub fn try_line(&self, text: &str) { + let mut file = match self.0.try_lock() { + Ok(file) => file, + Err(TryLockError::Poisoned(poisoned)) => poisoned.into_inner(), + Err(TryLockError::WouldBlock) => return, + }; + let _ = file.write_all(format!("{text}\n").as_bytes()); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::mpsc; + use std::thread; + use std::time::Duration; + use tempfile::TempDir; + + /// A temporary directory and the log path inside it. + fn setup() -> (TempDir, PathBuf) { + let dir = TempDir::new().unwrap(); + let path = dir.path().join("fips.log"); + (dir, path) + } + + fn read(path: &Path) -> String { + fs::read_to_string(path).unwrap() + } + + /// The names in `dir`, sorted. + fn names(dir: &TempDir) -> Vec { + let mut names: Vec = fs::read_dir(dir.path()) + .unwrap() + .map(|e| e.unwrap().file_name().to_string_lossy().into_owned()) + .collect(); + names.sort(); + names + } + + fn rolled(path: &Path, n: u32) -> PathBuf { + PathBuf::from(format!("{}.{n}", path.display())) + } + + fn shared(path: &Path) -> SharedLog { + SharedLog::new(RollingFile::open(path, ROLL_BYTES, ROLL_KEEP).unwrap()) + } + + /// Poison the lock the way a real failure would: a thread panics while + /// holding it. + fn poison(log: &SharedLog) { + let held = log.clone(); + let joined = thread::spawn(move || { + let _guard = held.0.lock().unwrap(); + panic!("a thread panicked holding the log lock"); + }) + .join(); + assert!(joined.is_err()); + assert!( + log.0.is_poisoned(), + "the lock must be poisoned for this test" + ); + } + + /// Emit one `info!` through a real fmt subscriber writing to `log`. + fn emit(log: &SharedLog, text: &str) { + let subscriber = tracing_subscriber::fmt() + .with_writer(log.clone()) + .with_ansi(false) + .finish(); + tracing::subscriber::with_default(subscriber, || tracing::info!("{text}")); + } + + #[test] + fn writes_below_the_cap_stay_in_one_file() { + let (dir, path) = setup(); + let mut file = RollingFile::open(&path, 16, 2).unwrap(); + for chunk in [b"aaaaa", b"bbbbb", b"ccccc"] { + assert_eq!(file.write(chunk).unwrap(), 5); + } + assert_eq!(names(&dir), ["fips.log"]); + assert_eq!(read(&path), "aaaaabbbbbccccc"); + } + + #[test] + fn a_write_that_would_cross_the_cap_rolls_first_and_is_not_split() { + let (dir, path) = setup(); + let mut file = RollingFile::open(&path, 16, 2).unwrap(); + file.write_all(b"aaaaaaaaaa").unwrap(); + file.write_all(b"bbbbbbbbbb").unwrap(); + assert_eq!(names(&dir), ["fips.log", "fips.log.1"]); + assert_eq!(read(&rolled(&path, 1)), "aaaaaaaaaa"); + assert_eq!(read(&path), "bbbbbbbbbb"); + } + + #[test] + fn rolling_more_than_keep_times_discards_the_oldest() { + let (dir, path) = setup(); + let mut file = RollingFile::open(&path, 4, 2).unwrap(); + // Every write after the first fills the file past the cap: five rolls. + for chunk in [b"aaaa", b"bbbb", b"cccc", b"dddd", b"eeee", b"ffff"] { + file.write_all(chunk).unwrap(); + } + assert_eq!(names(&dir), ["fips.log", "fips.log.1", "fips.log.2"]); + assert_eq!(read(&path), "ffff"); + assert_eq!(read(&rolled(&path, 1)), "eeee"); + assert_eq!(read(&rolled(&path, 2)), "dddd"); + } + + #[test] + fn reopening_counts_bytes_already_in_the_file() { + let (dir, path) = setup(); + let mut file = RollingFile::open(&path, 10, 2).unwrap(); + file.write_all(b"aaaaaaaaa").unwrap(); + drop(file); + + let mut file = RollingFile::open(&path, 10, 2).unwrap(); + file.write_all(b"bb").unwrap(); + assert_eq!(names(&dir), ["fips.log", "fips.log.1"]); + assert_eq!(read(&rolled(&path, 1)), "aaaaaaaaa"); + assert_eq!(read(&path), "bb"); + } + + #[test] + fn a_write_larger_than_the_cap_lands_whole_in_a_fresh_file() { + let (dir, path) = setup(); + let mut file = RollingFile::open(&path, 16, 2).unwrap(); + let big = "x".repeat(40); + file.write_all(big.as_bytes()).unwrap(); + // No roll from the empty file: rolling it would leave an empty + // fips.log.1 behind. + assert_eq!(names(&dir), ["fips.log"]); + assert_eq!(read(&path), big); + } + + #[test] + fn a_failed_roll_keeps_writing_to_the_current_file() { + let (_dir, path) = setup(); + let mut file = RollingFile::open(&path, 16, 1).unwrap(); + file.write_all(b"aaaaaaaaaa").unwrap(); + // A non-empty directory where the roll must rename to: the rename + // fails on every platform. + let blocker = rolled(&path, 1); + fs::create_dir(&blocker).unwrap(); + fs::write(blocker.join("x"), "").unwrap(); + + assert_eq!(file.write(b"bbbbbbbbbb").unwrap(), 10); + assert_eq!(read(&path), "aaaaaaaaaabbbbbbbbbb"); + assert!(blocker.is_dir()); + assert!(blocker.join("x").exists()); + } + + #[test] + fn a_failed_roll_is_retried_only_after_another_cap_of_bytes() { + let (_dir, path) = setup(); + let mut file = RollingFile::open(&path, 16, 1).unwrap(); + file.write_all(b"aaaaaaaaaa").unwrap(); + let blocker = rolled(&path, 1); + fs::create_dir(&blocker).unwrap(); + fs::write(blocker.join("x"), "").unwrap(); + file.write_all(b"bbbbbbbbbb").unwrap(); + fs::remove_dir_all(&blocker).unwrap(); + + // 30 bytes: under the 20 + 16 the failed roll set, so no new attempt. + file.write_all(b"cccccccccc").unwrap(); + assert!(!blocker.exists(), "a roll was retried before another cap"); + + // 40 bytes: past it, so the roll runs and now succeeds. + file.write_all(b"dddddddddd").unwrap(); + assert!(blocker.is_file()); + assert_eq!(read(&blocker), "aaaaaaaaaabbbbbbbbbbcccccccccc"); + assert_eq!(read(&path), "dddddddddd"); + } + + #[test] + fn shared_log_receives_tracing_events() { + let (_dir, path) = setup(); + let log = shared(&path); + emit(&log, "probe line"); + let text = read(&path); + assert!(text.contains("probe line"), "log was: {text:?}"); + assert!(!text.contains("\x1b["), "log has ANSI codes: {text:?}"); + } + + #[test] + fn tracing_still_reaches_the_file_after_a_thread_panicked_holding_the_lock() { + let (_dir, path) = setup(); + let log = shared(&path); + poison(&log); + emit(&log, "after poison"); + assert!(read(&path).contains("after poison")); + } + + #[test] + fn line_still_writes_after_a_thread_panicked_holding_the_lock() { + let (_dir, path) = setup(); + let log = shared(&path); + poison(&log); + log.line("line after poison"); + assert_eq!(read(&path), "line after poison\n"); + } + + #[test] + fn try_line_still_writes_after_a_thread_panicked_holding_the_lock() { + let (_dir, path) = setup(); + let log = shared(&path); + poison(&log); + log.try_line("try after poison"); + assert_eq!(read(&path), "try after poison\n"); + } + + #[test] + fn try_line_returns_without_writing_while_the_lock_is_held() { + let (_dir, path) = setup(); + let log = shared(&path); + let guard = log.0.lock().unwrap(); + + let (tx, rx) = mpsc::channel(); + let other = log.clone(); + let worker = thread::spawn(move || { + other.try_line("blocked"); + let _ = tx.send(()); + }); + let returned = rx.recv_timeout(Duration::from_secs(5)); + drop(guard); + assert!(returned.is_ok(), "try_line blocked on a held lock"); + worker.join().unwrap(); + assert!(!read(&path).contains("blocked")); + } +} diff --git a/src/utils/mod.rs b/src/utils/mod.rs index b961292e..0b5d0187 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -5,6 +5,8 @@ //! primitives, and other cross-cutting concerns. pub mod index; +#[cfg(any(windows, test))] +pub mod logfile; pub mod sockbind; #[cfg(unix)] pub mod sockperm; diff --git a/testing/deb-install/test.sh b/testing/deb-install/test.sh index 31f421a2..9b46c0b4 100755 --- a/testing/deb-install/test.sh +++ b/testing/deb-install/test.sh @@ -8,13 +8,16 @@ # to come up, and verifies that `dig @127.0.0.53 AAAA .fips` # returns a non-empty AAAA answer through the resolver backend that # fips-dns-setup configured. Then exercises fips-gateway against the -# same daemon to verify the gateway/daemon default-pairing. +# same daemon to verify the gateway/daemon default-pairing. Finally it +# purges the package with the DNS routing file planted and fips-dns +# stopped, and checks the file is removed and systemd-resolved restarted. # # This is the most thorough test surface — it exercises: # - cargo deb packaging (binary stripping, dependency declaration) # - dpkg conffile placement (/etc/fips/fips.yaml) # - postinst maintainer scripts (systemd unit enablement, # fips-dns.service running fips-dns-setup) +# - postrm purge (removing the DNS routing fips-dns-setup wrote) # - The fips, fips-dns, and (optionally) fips-gateway systemd units # - End-to-end .fips resolution as a real user would experience it # @@ -301,6 +304,111 @@ EOF return } +# Purge the package with the DNS routing file planted and fips-dns stopped, and +# check that postrm removes the file and restarts systemd-resolved. +# +# Stopping fips-dns runs fips-dns-teardown, which removes the file; putting it +# back gives the state in which removal leaves it behind: a live delegation and +# an inactive fips-dns, so prerm's stop runs no teardown. Only postrm purge is +# left to clean up, and a file it misses keeps the resolver sending .fips to +# [::1]:5354 after nothing listens there. +# +# Args: , the backend the scenario expects +# fips-dns-setup to pick (dns-delegate or global-drop-in). +check_purge_clears_dns() { + local name="$1" backend="$2" file + case "$backend" in + dns-delegate) file=/etc/systemd/dns-delegate.d/fips.dns-delegate ;; + global-drop-in) file=/etc/systemd/resolved.conf.d/fips.conf ;; + *) + fail "purge: no DNS routing file known for backend '$backend'" + return + ;; + esac + + # The gateway-enable restart of fips.service is passed on to fips-dns + # (Requires=fips.service), whose setup waits for fips0 before it writes the + # file, and nothing since has waited for it. After=fips.service stops the + # old instance before the daemon, so active here means the new setup ran. + if ! wait_for_service_active "$name" fips-dns.service; then + fail "purge: fips-dns.service not active again after the gateway-enable restart" + echo " --- fips-dns.service journal ---" + docker exec "$name" journalctl -u fips-dns.service --no-pager 2>&1 | tail -20 + return + fi + if ! cexec "$name" test -f "$file"; then + fail "purge: $file not written by fips-dns-setup before the purge" + return + fi + # Saved inside the container: cexec runs docker exec without -i, so a + # copy piped back from the host would arrive empty. + if ! cexec "$name" cp "$file" /root/fips-dns.saved; then + fail "purge: could not save $file" + return + fi + + cexec "$name" systemctl stop fips-dns.service >/dev/null 2>&1 + cexec "$name" cp /root/fips-dns.saved "$file" + cexec "$name" systemctl restart systemd-resolved >/dev/null 2>&1 + + local ok=1 status + if ! cexec "$name" sh -c "test -s '$file' && cmp -s '$file' /root/fips-dns.saved"; then + fail "purge: $file not restored before the purge" + ok=0 + fi + if cexec "$name" systemctl is-active --quiet fips-dns.service; then + fail "purge: fips-dns.service still active before the purge" + ok=0 + fi + # Captured rather than piped into grep -q: under pipefail, grep closing the + # pipe early can fail the pipeline on a match. + if ! status=$(cexec "$name" resolvectl status 2>&1); then + fail "purge: resolvectl status failed before the purge" + echo "$status" | tail -10 + ok=0 + elif ! grep -q ':5354' <<<"$status"; then + fail "purge: resolvectl status does not show $file in effect before the purge" + echo "$status" | tail -25 + ok=0 + fi + [ "$ok" = 1 ] || return + local before after + before=$(cexec "$name" systemctl show -p InvocationID --value systemd-resolved) + + run_apt "$name" "$UPGRADE_APT_TIMEOUT" purge -y fips + echo " purge took ${APT_SECS}s" + if [ "$APT_RC" -ne 0 ]; then + fail "purge: apt-get purge exited $APT_RC" + echo "$APT_OUT" | tail -20 + return + fi + + # test exits 1 for a missing file; any other failure is docker exec's. + local rc=0 + cexec "$name" test -e "$file" || rc=$? + case "$rc" in + 1) pass "purge removed $file" ;; + 0) fail "purge left $file behind" ;; + *) fail "purge: could not check for $file (exit $rc)" ;; + esac + after=$(cexec "$name" systemctl show -p InvocationID --value systemd-resolved) + if [ -n "$before" ] && [ -n "$after" ] && [ "$before" != "$after" ]; then + pass "purge restarted systemd-resolved" + else + fail "purge did not restart systemd-resolved (InvocationID '$before' -> '$after')" + fi + if ! status=$(cexec "$name" resolvectl status 2>&1); then + fail "purge: resolvectl status failed after the purge" + echo "$status" | tail -10 + elif grep -q ':5354' <<<"$status"; then + fail "purge: resolvectl status still routes to port 5354" + echo "$status" | tail -25 + else + pass "purge: resolvectl status no longer routes to port 5354" + fi + return +} + # ───────────────────────────────────────────────────────────────────── # Scenario runner # @@ -601,6 +709,8 @@ DOCKERFILE docker exec "$name" journalctl -u fips-gateway.service --no-pager 2>&1 | tail -15 fi + check_purge_clears_dns "$name" "$expected_backend" + cleanup_container "$name" }