Restart apt-update timer on upgrade and sandbox its service unit#79
Conversation
- install.sh: capture whether pimonitor-apt-update.timer was already active before enable --now, same as already done for pimonitor.service, and explicitly restart it afterwards. 'enable --now' is a no-op on an already-active unit, so without this a future change to the timer's schedule would install the new unit file but keep the old schedule loaded until an unrelated restart/reboot (closes #77). - pimonitor-apt-update.service: add cheap sandboxing directives now that the main pimonitor.service has them (closes #74). The capability bounding set and NoNewPrivileges were verified against a live 'apt-get update' run: apt drops to the _apt user for the actual download (CAP_SETUID/CAP_SETGID) and chowns/chmods the fetched index files back (CAP_CHOWN/CAP_FOWNER/CAP_DAC_OVERRIDE); all five are load-bearing. NoNewPrivileges does not interfere with this since it's a plain setuid()/setgid() syscall drop, not an execve of a setuid binary, contrary to the issue's initial assumption.
There was a problem hiding this comment.
Reviewed against the repo checklist. Verification on the PR branch: go build ./..., go vet ./..., go test ./... all pass (no Go changes), bash -n packaging/install.sh is clean, and systemd-analyze verify on the changed unit exits clean, confirming the PR's claim.
I also independently re-checked the sandboxing claims empirically (strace of a live apt-get update with apt 2.x): the five-capability analysis and the NoNewPrivileges reasoning in the PR body hold up — NNP only gates privilege gain across execve, not apt's own setuid() drop to _apt. The timer restart logic in install.sh correctly mirrors the #76 pattern.
Two findings inline: ReadWritePaths= is inert under ProtectSystem=full (misleading as written), and the || true on the initial refresh now silently hides exactly the failure mode this PR introduces risk of.
- pimonitor-apt-update.service: ProtectSystem=full made ReadWritePaths= a no-op since /var is already writable under 'full' - switch to 'strict' so it actually confines writes. Re-verified with strace against a live 'apt-get update' that only /var/lib/apt/lists and /tmp (already covered by PrivateTmp) are opened for writing; dropped /var/log/apt from ReadWritePaths since update never writes there. - install.sh: the initial cache refresh swallowed failures via '|| true'. With this PR adding new sandboxing to that service, a directive breaking apt-get update on some setup would now install cleanly and only surface as a silently stale update count. Print a warning on failure instead.
2 participants
Summary
install.sh: capture whetherpimonitor-apt-update.timerwas already active beforedaemon-reload/enable --now, and explicitlysystemctl restartit afterwards if so — same pattern already used forpimonitor.service(Restart pimonitor.service on upgrade if already running #76).enable --nowis a no-op on an already-active unit, so without this a future change topackaging/pimonitor-apt-update.timer(e.g. a different refresh interval) would install the new unit file on an upgrade but leave the old schedule loaded until an unrelated reboot/restart.pimonitor-apt-update.service: add the sandboxing directives suggested in Hardening: pimonitor-apt-update.service runs as root with no sandboxing #74 (ProtectSystem=fullwithReadWritePaths=,ProtectHome,PrivateTmp,ProtectKernelModules,ProtectKernelLogs,ProtectControlGroups, a trimmedCapabilityBoundingSet=, andNice=/IOSchedulingClass=idle), plusNoNewPrivileges=true.For the capability set and
NoNewPrivileges, I didn't want to guess — I ranapt-get updatelocally undersetprivwith a trimmed capability bounding set and with--no-new-privsto check what's actually load-bearing:_aptsandbox user for the network fetch (needsCAP_SETUID/CAP_SETGID), then chowns/chmods the fetched index files back (CAP_CHOWN/CAP_FOWNER), and needsCAP_DAC_OVERRIDEto replace lock/partial files it doesn't own as_apt. Dropping any one of these five causedapt-get updateto either fail outright (setuid/setgid/dac_override) or silently leave files root-owned instead of_apt-owned (chown/fowner).NoNewPrivileges=truedid not break anything in testing, contrary to the issue's initial assumption — it only blocks gaining privileges viaexecveof a setuid/capability-bearing binary, not a process's ownsetuid()/setgid()syscalls (which is how apt drops to_apt). So it's included.RestrictAddressFamiliesand syscall filtering weren't added; they're out of scope for what Hardening: pimonitor-apt-update.service runs as root with no sandboxing #74 asked for and would need more careful verification of apt's HTTPS/DNS code paths than I could do from this environment.Also ran
systemd-analyze verifyagainst the changed unit file (clean, no warnings).Related Issue
Closes #77
Closes #74
Checklist
go test ./...passes locally) — not applicable, no Go code changed; verified instead viasystemd-analyze verifyand liveapt-get updateruns undersetprivwith the trimmed capability set (see above)go vet ./...andgolangci-lint runare cleandocs/API.md), configuration (README.md,packaging/pimonitor.example.yaml), or installation/packaging (packaging/install.sh, systemd units) — not applicable, no user-facing config/API surface changed/api/v1/...response shapes, or a new API version was introduced instead — not applicable, no API changesVerification note
Hardware verification is still recommended: I tested
apt-get update's capability requirements empirically (viasetpriv) and validated the unit syntax withsystemd-analyze verify, but couldn't run this under a live systemd instance (this environment has no PID 1 systemd) or on real Pi hardware, so I'd suggest asystemctl start pimonitor-apt-update.service+systemctl statussanity check after deploying to a Pi.