diff --git a/.github/workflows/package-freebsd.yml b/.github/workflows/package-freebsd.yml index cec6276c..fe27838c 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/packaging/freebsd/README.md b/packaging/freebsd/README.md index 197c6239..e9466ccf 100644 --- a/packaging/freebsd/README.md +++ b/packaging/freebsd/README.md @@ -41,6 +41,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 27f78e83..2ab9dfa5 100755 --- a/packaging/freebsd/build-pkg.sh +++ b/packaging/freebsd/build-pkg.sh @@ -73,6 +73,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/" @@ -148,6 +152,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/src/packaging_tests.rs b/src/packaging_tests.rs index b926096f..b026bb52 100644 --- a/src/packaging_tests.rs +++ b/src/packaging_tests.rs @@ -6,6 +6,7 @@ //! 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 @@ -87,6 +88,79 @@ fn bash_array(pkgbuild: &str, name: &str) -> Vec { 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 +} + #[test] fn deb_and_aur_packages_declare_nftables_for_the_firewall_units_nft() { let unit = repo_file("packaging/debian/fips-firewall.service"); @@ -147,3 +221,98 @@ fn deb_and_aur_packages_declare_nftables_for_the_firewall_units_nft() { 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:?}" + ); +}