doctor: surface security-posture drift in banger doctor
`docs/privileges.md` now documents what the install promises (helper +
daemon services active, sockets at 0600 ownerUID, units carrying the
hardening directives, firecracker root-owned + non-writable). Doctor
verifies the running install matches: drift between the doc and the
filesystem would silently weaken the trust model otherwise.
In system mode (install.toml present):
* helper service / owner daemon service: `systemctl is-active`.
* helper socket / daemon socket: stat-and-compare mode + uid against
the registered owner.
* helper unit hardening / daemon unit hardening: scan the rendered
unit for NoNewPrivileges, ProtectSystem=strict, ProtectHome
(=yes for the helper, =read-only for the daemon), RestrictSUIDSGID,
LockPersonality, and the helper's CapabilityBoundingSet line. The
daemon unit also pins User=<registered owner>.
* firecracker binary ownership: regular file, not a symlink, mode
not group/world writable, executable, owned by uid 0 — same
constraints validateRootExecutable enforces at launch, surfaced
once at doctor time so a misconfigured binary fails fast with a
clearer error than the helper's open-time rejection.
In non-system mode (no /etc/banger/install.toml) doctor emits a single
WARN row pointing at docs/privileges.md > 'Running outside the system
install'. A PASS would imply guarantees the install isn't actually
providing.
Tests cover both branches: the non-system warn pins its message
substrings; system-mode pins that every check name shows up; and the
helpers (socket-perms, unit-hardening, executable-ownership) have
direct table-style negative tests.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
853249dec2
commit
3e6d0cee89
2 changed files with 386 additions and 0 deletions
|
|
@ -4,17 +4,25 @@ import (
|
|||
"context"
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"strings"
|
||||
"syscall"
|
||||
|
||||
"banger/internal/config"
|
||||
"banger/internal/imagecat"
|
||||
"banger/internal/installmeta"
|
||||
"banger/internal/model"
|
||||
"banger/internal/paths"
|
||||
"banger/internal/store"
|
||||
"banger/internal/system"
|
||||
)
|
||||
|
||||
// systemdSystemDir is the path systemd reads enabled units from. Pulled
|
||||
// out as a var (not a const) so the security-posture tests can swap it
|
||||
// for a tempdir without faking /etc/systemd/system on the test host.
|
||||
var systemdSystemDir = "/etc/systemd/system"
|
||||
|
||||
func Doctor(ctx context.Context) (system.Report, error) {
|
||||
userLayout, err := paths.Resolve()
|
||||
if err != nil {
|
||||
|
|
@ -83,10 +91,204 @@ func (d *Daemon) doctorReport(ctx context.Context, storeErr error, storeMissing
|
|||
d.addVMDefaultsCheck(&report)
|
||||
d.addSSHShortcutCheck(&report)
|
||||
d.addCapabilityDoctorChecks(ctx, &report)
|
||||
d.addSecurityPostureChecks(ctx, &report)
|
||||
|
||||
return report
|
||||
}
|
||||
|
||||
// addSecurityPostureChecks verifies the install matches what
|
||||
// docs/privileges.md describes: helper + owner-daemon units active,
|
||||
// sockets at the expected mode/owner, unit files carrying the
|
||||
// hardening directives, and the firecracker binary owned by root +
|
||||
// non-writable. Drift between the doc and the running install would
|
||||
// silently weaken the trust model; surfacing it here makes the doc
|
||||
// load-bearing rather than aspirational.
|
||||
//
|
||||
// In non-system mode (no /etc/banger/install.toml) emits a single
|
||||
// warn pointing at the docs section that explains the looser dev-mode
|
||||
// trust model — a doctor PASS row in that mode would imply guarantees
|
||||
// the install isn't actually providing.
|
||||
func (d *Daemon) addSecurityPostureChecks(ctx context.Context, report *system.Report) {
|
||||
d.addSecurityPostureChecksAt(ctx, report, installmeta.DefaultPath, systemdSystemDir)
|
||||
}
|
||||
|
||||
// addSecurityPostureChecksAt is the seam tests use: pass a fake
|
||||
// install.toml + systemd dir to exercise the system-mode branch
|
||||
// without writing to /etc.
|
||||
func (d *Daemon) addSecurityPostureChecksAt(ctx context.Context, report *system.Report, installPath, systemdDir string) {
|
||||
meta, err := installmeta.Load(installPath)
|
||||
if err != nil {
|
||||
report.AddWarn("security posture",
|
||||
"running outside the system install (no "+installPath+")",
|
||||
"helper SO_PEERCRED, narrow CapabilityBoundingSet, NoNewPrivileges, and ProtectSystem=strict are bypassed in this mode",
|
||||
"see docs/privileges.md > 'Running outside the system install'; install via `sudo banger system install --owner $USER` for the supported trust model")
|
||||
return
|
||||
}
|
||||
addServiceActiveCheck(ctx, d.runner, report, "helper service", installmeta.DefaultRootHelperService)
|
||||
addServiceActiveCheck(ctx, d.runner, report, "owner daemon service", installmeta.DefaultService)
|
||||
addSocketPermsCheck(report, "helper socket", installmeta.DefaultRootHelperSocketPath, meta.OwnerUID, 0o600)
|
||||
addSocketPermsCheck(report, "daemon socket", installmeta.DefaultSocketPath, meta.OwnerUID, 0o600)
|
||||
addUnitHardeningCheck(report, "helper unit hardening",
|
||||
filepath.Join(systemdDir, installmeta.DefaultRootHelperService),
|
||||
[]string{
|
||||
"NoNewPrivileges=yes",
|
||||
"ProtectSystem=strict",
|
||||
"ProtectHome=yes",
|
||||
"RestrictSUIDSGID=yes",
|
||||
"LockPersonality=yes",
|
||||
"CapabilityBoundingSet=",
|
||||
})
|
||||
addUnitHardeningCheck(report, "daemon unit hardening",
|
||||
filepath.Join(systemdDir, installmeta.DefaultService),
|
||||
[]string{
|
||||
"User=" + meta.OwnerUser,
|
||||
"NoNewPrivileges=yes",
|
||||
"ProtectSystem=strict",
|
||||
"ProtectHome=read-only",
|
||||
"RestrictSUIDSGID=yes",
|
||||
"LockPersonality=yes",
|
||||
})
|
||||
addExecutableOwnershipCheck(report, "firecracker binary ownership", d.config.FirecrackerBin)
|
||||
}
|
||||
|
||||
// addServiceActiveCheck shells `systemctl is-active <svc>` and surfaces
|
||||
// the result. is-active exits non-zero for inactive/failed states but
|
||||
// always prints the state on stdout, so we read the trimmed output and
|
||||
// ignore the exit code. Anything other than "active" is a fail with a
|
||||
// systemctl-restart hint.
|
||||
func addServiceActiveCheck(ctx context.Context, runner system.CommandRunner, report *system.Report, name, service string) {
|
||||
out, _ := runner.Run(ctx, "systemctl", "is-active", service)
|
||||
state := strings.TrimSpace(string(out))
|
||||
if state == "" {
|
||||
state = "unknown"
|
||||
}
|
||||
if state == "active" {
|
||||
report.AddPass(name, fmt.Sprintf("%s is active", service))
|
||||
return
|
||||
}
|
||||
report.AddFail(name,
|
||||
fmt.Sprintf("%s is %s, not active", service, state),
|
||||
fmt.Sprintf("run `sudo systemctl restart %s` and re-run `banger doctor`", service))
|
||||
}
|
||||
|
||||
// addSocketPermsCheck stat()s the socket path and compares mode +
|
||||
// owner against the values the install promises. Both daemon and
|
||||
// helper sockets are 0600 chowned to the registered owner UID; any
|
||||
// drift means filesystem perms aren't gating access the way the docs
|
||||
// describe.
|
||||
func addSocketPermsCheck(report *system.Report, name, path string, expectedUID int, expectedMode os.FileMode) {
|
||||
info, err := os.Stat(path)
|
||||
if err != nil {
|
||||
report.AddFail(name,
|
||||
fmt.Sprintf("%s: %v", path, err),
|
||||
"is the service running? `sudo systemctl status` and check the runtime dir")
|
||||
return
|
||||
}
|
||||
stat, ok := info.Sys().(*syscall.Stat_t)
|
||||
if !ok {
|
||||
report.AddWarn(name, fmt.Sprintf("%s: cannot read ownership metadata on this platform", path))
|
||||
return
|
||||
}
|
||||
actualMode := info.Mode().Perm()
|
||||
var problems []string
|
||||
if actualMode != expectedMode {
|
||||
problems = append(problems, fmt.Sprintf("mode is %#o, want %#o", actualMode, expectedMode))
|
||||
}
|
||||
if int(stat.Uid) != expectedUID {
|
||||
problems = append(problems, fmt.Sprintf("uid is %d, want %d", stat.Uid, expectedUID))
|
||||
}
|
||||
if len(problems) > 0 {
|
||||
problems = append(problems, "restart the service so the socket gets recreated with correct perms")
|
||||
report.AddFail(name, fmt.Sprintf("%s: %s", path, strings.Join(problems, "; ")))
|
||||
return
|
||||
}
|
||||
report.AddPass(name, fmt.Sprintf("%s: mode %#o, uid %d", path, actualMode, expectedUID))
|
||||
}
|
||||
|
||||
// addUnitHardeningCheck reads the systemd unit file and confirms
|
||||
// every required directive is present as a literal substring. Brittle
|
||||
// to formatting changes (a comment-out would slip through), but
|
||||
// strong enough to catch the "someone hand-edited the unit and
|
||||
// dropped NoNewPrivileges" failure mode that motivates this check.
|
||||
// The directives list captures the security-relevant subset of the
|
||||
// renderer in commands_system.go; everything else (Description,
|
||||
// ExecStart, etc.) is operational and not worth pinning here.
|
||||
func addUnitHardeningCheck(report *system.Report, name, path string, required []string) {
|
||||
data, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
report.AddFail(name,
|
||||
fmt.Sprintf("%s: %v", path, err),
|
||||
"reinstall via `sudo banger system install` to refresh the unit")
|
||||
return
|
||||
}
|
||||
content := string(data)
|
||||
var missing []string
|
||||
for _, directive := range required {
|
||||
if !strings.Contains(content, directive) {
|
||||
missing = append(missing, directive)
|
||||
}
|
||||
}
|
||||
if len(missing) > 0 {
|
||||
report.AddFail(name,
|
||||
fmt.Sprintf("%s missing directives: %s", path, strings.Join(missing, ", ")),
|
||||
"reinstall via `sudo banger system install` to refresh the unit")
|
||||
return
|
||||
}
|
||||
report.AddPass(name, fmt.Sprintf("%s: %d hardening directives present", path, len(required)))
|
||||
}
|
||||
|
||||
// addExecutableOwnershipCheck mirrors validateRootExecutable's runtime
|
||||
// check at doctor time: regular file, root-owned, executable, not
|
||||
// group/world writable, not a symlink. Doctor catching this once at
|
||||
// install time beats the helper failing every launch with a less
|
||||
// helpful message.
|
||||
func addExecutableOwnershipCheck(report *system.Report, name, path string) {
|
||||
if strings.TrimSpace(path) == "" {
|
||||
report.AddWarn(name, "no firecracker binary path configured")
|
||||
return
|
||||
}
|
||||
info, err := os.Lstat(path)
|
||||
if err != nil {
|
||||
report.AddFail(name, fmt.Sprintf("%s: %v", path, err))
|
||||
return
|
||||
}
|
||||
if info.Mode()&os.ModeSymlink != 0 {
|
||||
report.AddFail(name,
|
||||
fmt.Sprintf("%s is a symlink", path),
|
||||
"the helper opens the binary with O_NOFOLLOW; resolve the symlink and update firecracker_bin in the daemon config")
|
||||
return
|
||||
}
|
||||
if !info.Mode().IsRegular() {
|
||||
report.AddFail(name, fmt.Sprintf("%s is not a regular file", path))
|
||||
return
|
||||
}
|
||||
mode := info.Mode().Perm()
|
||||
if mode&0o111 == 0 {
|
||||
report.AddFail(name,
|
||||
fmt.Sprintf("%s mode %#o is not executable", path, mode),
|
||||
"chmod +x the binary")
|
||||
return
|
||||
}
|
||||
if mode&0o022 != 0 {
|
||||
report.AddFail(name,
|
||||
fmt.Sprintf("%s mode %#o is group/world writable", path, mode),
|
||||
"chmod g-w,o-w the binary so the helper accepts it")
|
||||
return
|
||||
}
|
||||
stat, ok := info.Sys().(*syscall.Stat_t)
|
||||
if !ok {
|
||||
report.AddWarn(name, fmt.Sprintf("%s: cannot read ownership metadata on this platform", path))
|
||||
return
|
||||
}
|
||||
if stat.Uid != 0 {
|
||||
report.AddFail(name,
|
||||
fmt.Sprintf("%s is owned by uid %d, want 0", path, stat.Uid),
|
||||
"`sudo chown root` the firecracker binary")
|
||||
return
|
||||
}
|
||||
report.AddPass(name, fmt.Sprintf("%s: regular, root-owned, mode %#o", path, mode))
|
||||
}
|
||||
|
||||
// addSSHShortcutCheck surfaces a gentle warning when banger maintains
|
||||
// an ssh_config file but the user hasn't wired it into ~/.ssh/config.
|
||||
// This is intentionally a warn, not a fail — the shortcut is opt-in
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue