diff --git a/README.md b/README.md index 953e241..94913d8 100644 --- a/README.md +++ b/README.md @@ -187,20 +187,27 @@ documented in [`docs/advanced.md`](docs/advanced.md). ## Security Guest VMs are single-user development sandboxes, not multi-tenant -servers. Every provisioned image is configured with: +servers. Each guest's sshd is configured with: ``` -PermitRootLogin yes -StrictModes no +PermitRootLogin prohibit-password +PubkeyAuthentication yes +PasswordAuthentication no +KbdInteractiveAuthentication no +AuthorizedKeysFile /root/.ssh/authorized_keys ``` -The host SSH key is the only authentication mechanism, no password -auth is enabled, and VMs are reachable only through the host bridge -network (`172.16.0.0/24` by default). Do not expose the bridge -interface or guest IPs to an untrusted network. +The host SSH key is the only authentication mechanism. `StrictModes` +is on (sshd's default); banger normalises `/root`, `/root/.ssh`, and +`authorized_keys` perms at provisioning time so the default passes. -The web UI (when enabled) binds `127.0.0.1` by default. Do not -expose it to a shared network. +VMs are reachable only through the host bridge network +(`172.16.0.0/24` by default). Do not expose the bridge interface or +guest IPs to an untrusted network. + +The web UI is disabled by default. If you opt in via +`web_listen_addr`, it binds `127.0.0.1` — do not publish it to a +shared network. ## Further reading diff --git a/internal/daemon/image_seed.go b/internal/daemon/image_seed.go index 97f6c34..b0f47d3 100644 --- a/internal/daemon/image_seed.go +++ b/internal/daemon/image_seed.go @@ -34,6 +34,13 @@ func (d *Daemon) seedAuthorizedKeyOnExt4Image(ctx context.Context, imagePath str return "", err } + // Same rationale as in ensureAuthorizedKeyOnWorkDisk — the seed's + // filesystem root becomes /root inside the guest, and sshd's + // StrictModes check walks its ownership and mode. + if err := normaliseHomeDirPerms(ctx, d.runner, mountDir); err != nil { + return "", err + } + sshDir := filepath.Join(mountDir, ".ssh") if _, err := d.runner.RunSudo(ctx, "mkdir", "-p", sshDir); err != nil { return "", err @@ -41,6 +48,9 @@ func (d *Daemon) seedAuthorizedKeyOnExt4Image(ctx context.Context, imagePath str if _, err := d.runner.RunSudo(ctx, "chmod", "700", sshDir); err != nil { return "", err } + if _, err := d.runner.RunSudo(ctx, "chown", "0:0", sshDir); err != nil { + return "", err + } authorizedKeysPath := filepath.Join(sshDir, "authorized_keys") existing, err := d.runner.RunSudo(ctx, "cat", authorizedKeysPath) diff --git a/internal/daemon/sshd_config_test.go b/internal/daemon/sshd_config_test.go new file mode 100644 index 0000000..4135856 --- /dev/null +++ b/internal/daemon/sshd_config_test.go @@ -0,0 +1,59 @@ +package daemon + +import ( + "strings" + "testing" +) + +// TestSshdGuestConfig_Hardened is a regression guard for the guest +// SSH posture. An earlier version shipped `LogLevel DEBUG3` and +// `StrictModes no`; both are gone and must not come back without an +// explicit call-out. +func TestSshdGuestConfig_Hardened(t *testing.T) { + cfg := sshdGuestConfig() + + // Posture: key-only, root via pubkey, no password / keyboard- + // interactive fallback, pinned authorized_keys path. + mustContain := []string{ + "PermitRootLogin prohibit-password", + "PubkeyAuthentication yes", + "PasswordAuthentication no", + "KbdInteractiveAuthentication no", + "AuthorizedKeysFile /root/.ssh/authorized_keys", + } + for _, line := range mustContain { + if !strings.Contains(cfg, line) { + t.Errorf("sshd drop-in missing %q:\n%s", line, cfg) + } + } + + // Things that must NOT appear. Each has a history and a reason. + mustNotContain := map[string]string{ + "LogLevel DEBUG3": "was debug leftover; floods journald", + "StrictModes no": "masked a /root perm drift; real fix is in normaliseHomeDirPerms", + // Blanket "PermitRootLogin yes" (without prohibit-password) + // would re-enable password root login if something else + // flipped PasswordAuthentication back to yes. + "PermitRootLogin yes": "use prohibit-password instead", + } + for needle, why := range mustNotContain { + if strings.Contains(cfg, needle) { + t.Errorf("sshd drop-in contains %q (%s):\n%s", needle, why, cfg) + } + } +} + +func TestSshdGuestConfig_IsCompleteLines(t *testing.T) { + // Every directive should be a full line on its own. Trailing + // newline matters — sshd_config.d files without a newline sometimes + // get misparsed when concatenated with other drop-ins. + cfg := sshdGuestConfig() + if !strings.HasSuffix(cfg, "\n") { + t.Errorf("sshd drop-in should end with newline:\n%q", cfg) + } + for _, line := range strings.Split(strings.TrimRight(cfg, "\n"), "\n") { + if strings.TrimSpace(line) == "" { + t.Errorf("sshd drop-in has blank line:\n%s", cfg) + } + } +} diff --git a/internal/daemon/vm_authsync.go b/internal/daemon/vm_authsync.go index 9702083..ad21b46 100644 --- a/internal/daemon/vm_authsync.go +++ b/internal/daemon/vm_authsync.go @@ -47,6 +47,14 @@ func (d *Daemon) ensureAuthorizedKeyOnWorkDisk(ctx context.Context, vm *model.VM return err } + // Normalise the work-disk filesystem root: inside the guest this + // mounts at /root, which sshd inspects when StrictModes is on (the + // default after the hardening drop-in). Any drift — owner != root, + // group/other-writable — would make sshd silently reject the key. + if err := normaliseHomeDirPerms(ctx, d.runner, workMount); err != nil { + return err + } + sshDir := filepath.Join(workMount, ".ssh") if _, err := d.runner.RunSudo(ctx, "mkdir", "-p", sshDir); err != nil { return err @@ -54,6 +62,9 @@ func (d *Daemon) ensureAuthorizedKeyOnWorkDisk(ctx context.Context, vm *model.VM if _, err := d.runner.RunSudo(ctx, "chmod", "700", sshDir); err != nil { return err } + if _, err := d.runner.RunSudo(ctx, "chown", "0:0", sshDir); err != nil { + return err + } authorizedKeysPath := filepath.Join(sshDir, "authorized_keys") existing, err := d.runner.RunSudo(ctx, "cat", authorizedKeysPath) @@ -90,6 +101,25 @@ func (d *Daemon) ensureAuthorizedKeyOnWorkDisk(ctx context.Context, vm *model.VM return nil } +// normaliseHomeDirPerms forces the home-directory mount point to +// 0755 root:root. sshd's StrictModes (the default, re-enabled after +// banger stopped shipping "StrictModes no") rejects authorized_keys +// if the user's HOME — here the work-disk filesystem root — is +// group/other-writable or owned by anyone other than root. mkfs.ext4 +// normally creates an ext4 root dir at 0755 root:root, but older +// work-seed images may have drifted, and `cp -a` on a non-standard +// source can carry weird bits forward. Forcing a known-good state +// here is cheap insurance. +func normaliseHomeDirPerms(ctx context.Context, runner system.CommandRunner, workMount string) error { + if _, err := runner.RunSudo(ctx, "chown", "0:0", workMount); err != nil { + return err + } + if _, err := runner.RunSudo(ctx, "chmod", "0755", workMount); err != nil { + return err + } + return nil +} + func (d *Daemon) ensureGitIdentityOnWorkDisk(ctx context.Context, vm *model.VMRecord) error { runner := d.runner if runner == nil { diff --git a/internal/daemon/vm_disk.go b/internal/daemon/vm_disk.go index fb273c0..a5df6e7 100644 --- a/internal/daemon/vm_disk.go +++ b/internal/daemon/vm_disk.go @@ -30,14 +30,7 @@ func (d *Daemon) patchRootOverlay(ctx context.Context, vm model.VMRecord, image resolv := []byte(fmt.Sprintf("nameserver %s\n", d.config.DefaultDNS)) hostname := []byte(vm.Name + "\n") hosts := []byte(fmt.Sprintf("127.0.0.1 localhost\n127.0.1.1 %s\n", vm.Name)) - sshdConfig := []byte(strings.Join([]string{ - "LogLevel DEBUG3", - "PermitRootLogin yes", - "PubkeyAuthentication yes", - "AuthorizedKeysFile /root/.ssh/authorized_keys", - "StrictModes no", - "", - }, "\n")) + sshdConfig := []byte(sshdGuestConfig()) fstab, err := system.ReadDebugFSText(ctx, d.runner, vm.Runtime.DMDev, "/etc/fstab") if err != nil { fstab = "" @@ -136,6 +129,55 @@ func (d *Daemon) ensureWorkDisk(ctx context.Context, vm *model.VMRecord, image m return workDiskPreparation{}, nil } +// sshdGuestConfig is the banger-authored drop-in that lands at +// /etc/ssh/sshd_config.d/99-banger.conf inside every guest. +// +// Banger VMs are single-user root sandboxes reachable only through the +// host bridge (default 172.16.0.0/24). The drop-in sets the minimum +// needed to make that usable while keeping the posture tight enough +// that a misconfigured host bridge does not immediately hand over an +// unauthenticated root shell. +// +// Why each line is here: +// +// - PermitRootLogin prohibit-password +// The guest IS root — there's no other account. prohibit-password +// allows pubkey login and blocks password auth at the source even +// if some future config flips PasswordAuthentication on. +// +// - PubkeyAuthentication yes +// The only auth method we expect. Explicit in case a future +// Debian default or distro package flips it off. +// +// - PasswordAuthentication no +// +// - KbdInteractiveAuthentication no +// Belt-and-braces: every interactive auth path is off, not just +// the PermitRootLogin path. These are already Debian defaults but +// stating them here means the drop-in documents the intent. +// +// - AuthorizedKeysFile /root/.ssh/authorized_keys +// Pins the lookup path so the banger-written file always wins, +// regardless of distro default ($HOME/.ssh/authorized_keys) and +// regardless of any per-image weirdness. +// +// Previously this file also contained `LogLevel DEBUG3` and +// `StrictModes no`. DEBUG3 was a leftover from debugging the +// first-boot flow and flooded journald in normal use. StrictModes no +// was a workaround for perm drift on /root inside the work disk; the +// real fix — normalising /root permissions at provisioning time — is +// in ensureAuthorizedKeyOnWorkDisk / seedAuthorizedKeyOnExt4Image. +func sshdGuestConfig() string { + return strings.Join([]string{ + "PermitRootLogin prohibit-password", + "PubkeyAuthentication yes", + "PasswordAuthentication no", + "KbdInteractiveAuthentication no", + "AuthorizedKeysFile /root/.ssh/authorized_keys", + "", + }, "\n") +} + func (d *Daemon) flattenNestedWorkHome(ctx context.Context, workMount string) error { nestedHome := filepath.Join(workMount, "root") if !exists(nestedHome) { diff --git a/internal/daemon/vm_test.go b/internal/daemon/vm_test.go index 6ee16a8..26dbf98 100644 --- a/internal/daemon/vm_test.go +++ b/internal/daemon/vm_test.go @@ -1857,13 +1857,18 @@ func (r *filesystemRunner) RunSudo(ctx context.Context, args ...string) ([]byte, } return nil, os.WriteFile(dst, data, os.FileMode(mode)) case "chown": - // chown -R OWNER TARGET — owner is ignored under test; we - // already run as the test user and os.Chown would require - // CAP_CHOWN. - if len(args) != 4 || args[1] != "-R" { + // Recognised forms, both no-op under test (we run as the test + // user and os.Chown would need CAP_CHOWN): + // chown OWNER TARGET + // chown -R OWNER TARGET + switch { + case len(args) == 3: + return nil, nil + case len(args) == 4 && args[1] == "-R": + return nil, nil + default: return nil, fmt.Errorf("unexpected chown args: %v", args) } - return nil, nil default: return nil, fmt.Errorf("unexpected sudo command: %v", args) }