Skip to content

audit log: SIGHUP reopen so rotation doesn't drop records #278

Description

@WaylandYang

Follow-up from the packaging review in #276, kept out of that PR deliberately.

Problem

packaging/arch/forkd.logrotate rotates /var/log/forkd/audit.log with copytruncate. That is currently the only workable policy, and the choice is correct given today's code — but it has a failure mode worth recording rather than rediscovering.

copytruncate copies the file, then truncates the original. Any line the daemon writes between the copy and the truncate is silently lost. For an ordinary application log a small gap is a nuisance; for an append-only audit log it means a request can be served and its audit record can vanish, with nothing anywhere indicating a record ever existed. That is exactly the property an audit log is supposed to rule out.

Why copytruncate is currently forced

AuditSink::open opens the file once and the process holds the handle for its whole lifetime (crates/forkd-controller/src/audit.rs:36-53):

let file = OpenOptions::new()
    .create(true)
    .append(true)
    .open(&path)

spawn_shutdown_signal (crates/forkd-controller/src/lib.rs:204-228) installs handlers for SIGINT and SIGTERM only. There is no signal that tells the daemon to reopen its log, so a postrotate policy — the normal way to avoid the race — has nothing to call. logrotate would rename the file and the daemon would keep writing to the now-unlinked inode, which is worse than the copytruncate gap.

One thing that is not a problem here: because the handle is O_APPEND, writes after truncation resume at offset 0 rather than at the old offset, so this does not produce the NUL-padded sparse file that copytruncate causes with non-append writers. The lost-lines race is the whole issue.

Proposed fix

Add a SIGHUP handler that reopens the audit sink, then switch the logrotate policy to postrotate + systemctl reload forkd-controller.

The groundwork is already there — AuditInner stores path alongside the writer, so a reopen is roughly:

pub fn reopen(&self) -> Result<()> {
    let file = OpenOptions::new().create(true).append(true).open(&self.inner.path)?;
    let mut w = self.inner.writer.lock();
    let _ = w.flush();
    *w = BufWriter::new(file);
    Ok(())
}

Holding the same Mutex that write takes means a rotation cannot interleave with a line being written, which closes the race rather than narrowing it. The unit would also need ExecReload=/bin/kill -HUP $MAINPID.

Notes

  • Worth emitting an audit line on reopen (something like {"event":"log_reopened"}) so a rotation is visible in the log itself and a gap can be distinguished from a truncation.
  • Until this lands, copytruncate should stay — it is strictly better than the rename-and-keep-writing alternative. No change to packaging: add Arch forkd-bin recipe #276 is implied.
  • write() already flushes per line, so there is no additional buffered-data exposure beyond the race described above.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions