mirror of
https://github.com/jmcorgan/fips.git
synced 2026-10-05 19:18:25 +00:00
Rotate the FreeBSD daemon log with newsyslog
The FreeBSD package wrote the daemon's output to /var/log/fips.log and never rotated it, so the log grew without bound. Ship a newsyslog entry in /usr/local/etc/newsyslog.conf.d, which the stock newsyslog.conf includes: five bzip2-compressed generations of 1000 KB, mode 600 to match the file daemon(8) creates. An entry alone would not work, because daemon(8) keeps the -o file open and reopens it on SIGHUP only when started with -H, and only the supervisor handles that signal. The rc script now starts daemon(8) with -H and records the supervisor's pid in /var/run/fips/daemon.pid, which is the pid file the entry signals. The child pidfile that rc.subr uses for stop and status is unchanged. Add a lib test that checks the rc script, the newsyslog entry and the package staging agree on the log path, the signalled pid file and the installed location. The FreeBSD package smoke install now also confirms the stock newsyslog configuration picks up the shipped entry, starts the daemon, forces a rotation of its log, and checks that daemon(8) closes the rotated file and holds the new one open. This exercises the SIGHUP reopen that the static check of the packaging files cannot.
This commit is contained in:
Johnathan Corgan
@@ -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"
|
||||
|
||||
|
||||
@@ -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 <user>`, then re-login).
|
||||
On `pkg upgrade` the services are stopped before the binaries are
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
@@ -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
|
||||
|
||||
@@ -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<String> {
|
||||
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<String, String> {
|
||||
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, String>) -> 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<String> {
|
||||
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:?}"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user