diff --git a/docs/operator/security.md b/docs/operator/security.md index 466c65a..b5adb6c 100644 --- a/docs/operator/security.md +++ b/docs/operator/security.md @@ -58,7 +58,55 @@ For certificates issued to systems where revocation correctness matters: ## Postgres transport encryption -See [docs/database-tls.md](database-tls.md). +**Audit references:** SEC-013 (advisory) and SEC-014 (host-port bind), +both closed in Sprint 2 of the 2026-Q2 acquisition audit +(2026-05-16). + +The full upgrade procedure (sslmode flags, CA bundle paths, Helm chart +values, AWS RDS / Google Cloud SQL / Azure Database notes) lives at +[docs/operator/database-tls.md](database-tls.md). The summary of the +two operator-visible defenses certctl ships: + +### SEC-014 — Postgres host port is loopback-only + +`deploy/docker-compose.yml` and `deploy/docker-compose.test.yml` both +publish Postgres on `127.0.0.1:5432:5432` rather than `5432:5432`. +The default Docker port-binding behavior is to bind to `0.0.0.0`, +which exposes Postgres on every interface of the host — including any +public-facing NICs the operator did not realize were attached. The +loopback bind closes that footgun without breaking the +certctl-server hop (which goes over the `certctl-network` Docker +bridge, not over the host port). + +Operators who genuinely need to reach Postgres from another host — +e.g. a separate metrics box running `postgres_exporter` — should +either (1) attach that host into the same Docker network, (2) tunnel +through SSH (`ssh -L`), or (3) re-publish the port with explicit +`bind:` configuration and a documented network-layer access control. +Loosening the loopback bind without one of those is a regression. + +### SEC-013 — advisory WARN on external `sslmode=disable` + +`internal/config/config.go::Validate` emits an `slog.Warn` (NOT a +fail-closed error) when `CERTCTL_DATABASE_URL` parses as a Postgres +URL with `sslmode=disable` AND the host is outside the local +safelist (`localhost` / `127.0.0.1` / `::1` / `postgres` / +`certctl-postgres` / `*.svc.cluster.local`). The advisory exists +because the legitimate compose / Helm topology genuinely uses +`sslmode=disable` over the Docker bridge — failing closed would +break the production-shaped quickstart — but pointing +`CERTCTL_DATABASE_URL` at a managed-Postgres host (RDS, Cloud SQL, +Azure Database) without flipping `sslmode` to `verify-full` puts +the entire control plane's Postgres traffic on the wire in +cleartext. The WARN surfaces that landmine on every boot so the +operator notices it in the journal even if the rest of the boot +sequence looks healthy. + +To clear the WARN: set `CERTCTL_DATABASE_URL` to a URL with +`sslmode=verify-full` and `sslrootcert=`. The full +procedure (CA-bundle materialization, Helm chart values, secret- +manager wiring) is in +[docs/operator/database-tls.md](database-tls.md). ## Encryption at rest diff --git a/internal/config/config.go b/internal/config/config.go index d91176b..3aa4709 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "log/slog" + "net/url" "os" "strconv" "strings" @@ -1348,9 +1349,78 @@ func (c *Config) Validate() error { return fmt.Errorf("awaiting approval timeout must be at least 1 second") } + // Acquisition-audit SEC-013 closure (Sprint 2, 2026-05-16). + // Post-validate advisory WARN — NOT fail-closed — when + // CERTCTL_DATABASE_URL carries sslmode=disable AND the host is + // external (not loopback / not a known in-cluster service name). + // The compose bridge network legitimately uses sslmode=disable on + // the docker-internal hop to postgres:5432; failing closed would + // break the production-shaped quickstart. The WARN catches the + // real-world landmine: an operator who points CERTCTL_DATABASE_URL + // at an RDS / managed-Postgres host outside the bridge network + // without flipping sslmode to verify-full. + warnExternalSslmodeDisable(c.Database.URL, slog.Default()) + return nil } +// dbHostLocalSafelist is the set of hosts where sslmode=disable is an +// acceptable default (loopback + in-cluster service-name conventions). +// SEC-013 closure (Sprint 2 ACQ, 2026-05-16). Match is exact host +// equality except for the .svc.cluster.local suffix which is a +// substring match. Adding entries here is an operator-judgment call; +// keep the list tight (a too-permissive list silences a real +// landmine warning). +var dbHostLocalSafelist = map[string]struct{}{ + "localhost": {}, + "127.0.0.1": {}, + "::1": {}, + "postgres": {}, + "certctl-postgres": {}, +} + +// warnExternalSslmodeDisable emits an slog.Warn (matching the +// cmd/server/main.go demo-mode WARN shape) when the database URL +// parses as a Postgres URL with sslmode=disable AND the host is +// outside the local-safelist. The function is intentionally +// permissive on parse failures — if the URL is malformed, the +// downstream sql.Open will surface a clearer error than a noisy +// WARN here would. SEC-013 closure (Sprint 2 ACQ). +func warnExternalSslmodeDisable(rawURL string, logger *slog.Logger) { + if logger == nil { + logger = slog.Default() + } + if rawURL == "" { + return + } + u, err := url.Parse(rawURL) + if err != nil || u == nil { + return + } + if u.Scheme != "postgres" && u.Scheme != "postgresql" { + return + } + q := u.Query() + if q.Get("sslmode") != "disable" { + return + } + host := u.Hostname() + if _, ok := dbHostLocalSafelist[host]; ok { + return + } + // In-cluster service names of the form .svc.cluster.local + // (or longer K8s cluster-domain variants) are acceptable; the + // docker-bridge / pod-network hop is treated as trusted by the + // existing compose + Helm conventions. + if strings.HasSuffix(host, ".svc.cluster.local") { + return + } + logger.Warn("CERTCTL_DATABASE_URL points at a non-local Postgres host with sslmode=disable — Postgres traffic crosses an untrusted network in cleartext. Set sslmode=verify-full and provide a CA bundle. See docs/operator/database-tls.md for the full upgrade procedure. Override env var: CERTCTL_DATABASE_URL (set the URL with sslmode=verify-full + sslrootcert=).", + "host", host, + "sslmode", "disable", + ) +} + // getEnv reads a string environment variable with the given key and default value. func getEnv(key, defaultValue string) string { if value := os.Getenv(key); value != "" { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 2573f1f..3dc29ad 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1,6 +1,7 @@ package config import ( + "bytes" "crypto/ecdsa" "crypto/elliptic" "crypto/rand" @@ -2120,3 +2121,108 @@ func TestValidate_AgentKeygenIgnoresDemoAck(t *testing.T) { t.Errorf("Validate(KeygenMode=agent, DemoAck=false) = %v; want nil (production default must boot)", err) } } + +// newBufferLogger returns a slog.Logger that writes JSON records into the +// returned buffer, suitable for asserting WARN emission from +// warnExternalSslmodeDisable. SEC-013 closure (Sprint 2 ACQ). +func newBufferLogger() (*slog.Logger, *bytes.Buffer) { + var buf bytes.Buffer + h := slog.NewJSONHandler(&buf, &slog.HandlerOptions{Level: slog.LevelDebug}) + return slog.New(h), &buf +} + +// TestWarnExternalSslmodeDisable_FiresOnExternalHost asserts an external +// host (e.g. RDS) + sslmode=disable produces a WARN. SEC-013 closure +// (Sprint 2 ACQ, 2026-05-16). The advisory exists to surface the +// real-world landmine: an operator who points CERTCTL_DATABASE_URL at a +// managed-Postgres host outside the bridge network without flipping +// sslmode to verify-full. +func TestWarnExternalSslmodeDisable_FiresOnExternalHost(t *testing.T) { + t.Parallel() + logger, buf := newBufferLogger() + warnExternalSslmodeDisable("postgres://certctl:secret@db.internal.example.com:5432/certctl?sslmode=disable", logger) + + out := buf.String() + if !strings.Contains(out, `"level":"WARN"`) { + t.Fatalf("expected a WARN record, got: %s", out) + } + if !strings.Contains(out, "db.internal.example.com") { + t.Errorf("WARN should include the external host in structured fields; got: %s", out) + } + if !strings.Contains(out, "sslmode") { + t.Errorf("WARN should include the sslmode structured field; got: %s", out) + } +} + +// TestWarnExternalSslmodeDisable_QuietForLocalSafelist asserts the +// loopback + in-cluster service-name conventions stay silent. These are +// the legitimate sslmode=disable callers — compose bridge network +// (`postgres` / `certctl-postgres`), localhost dev loops, and K8s +// in-cluster service names (`*.svc.cluster.local`). SEC-013 closure. +func TestWarnExternalSslmodeDisable_QuietForLocalSafelist(t *testing.T) { + t.Parallel() + silentHosts := []string{ + "postgres://certctl@localhost:5432/certctl?sslmode=disable", + "postgres://certctl@127.0.0.1:5432/certctl?sslmode=disable", + "postgres://certctl@[::1]:5432/certctl?sslmode=disable", + "postgres://certctl@postgres:5432/certctl?sslmode=disable", + "postgres://certctl@certctl-postgres:5432/certctl?sslmode=disable", + "postgres://certctl@certctl-postgres.certctl.svc.cluster.local:5432/certctl?sslmode=disable", + } + for _, url := range silentHosts { + url := url + t.Run(url, func(t *testing.T) { + t.Parallel() + logger, buf := newBufferLogger() + warnExternalSslmodeDisable(url, logger) + if buf.Len() != 0 { + t.Errorf("expected silence for safelisted host (%s); got: %s", url, buf.String()) + } + }) + } +} + +// TestWarnExternalSslmodeDisable_QuietWithoutDisable asserts that any +// sslmode other than `disable` (the production-grade modes) stays +// silent even with an external host. SEC-013 closure. +func TestWarnExternalSslmodeDisable_QuietWithoutDisable(t *testing.T) { + t.Parallel() + for _, url := range []string{ + "postgres://certctl@db.internal.example.com:5432/certctl?sslmode=verify-full&sslrootcert=/etc/ssl/ca.pem", + "postgres://certctl@db.internal.example.com:5432/certctl?sslmode=require", + "postgres://certctl@db.internal.example.com:5432/certctl", // no sslmode at all + } { + url := url + t.Run(url, func(t *testing.T) { + t.Parallel() + logger, buf := newBufferLogger() + warnExternalSslmodeDisable(url, logger) + if buf.Len() != 0 { + t.Errorf("expected silence for non-disable sslmode (%s); got: %s", url, buf.String()) + } + }) + } +} + +// TestWarnExternalSslmodeDisable_QuietOnUnparseableOrEmpty asserts the +// helper is permissive on garbage input — downstream sql.Open surfaces +// the real parse error; the SEC-013 advisory must not become a noisy +// hot path. SEC-013 closure. +func TestWarnExternalSslmodeDisable_QuietOnUnparseableOrEmpty(t *testing.T) { + t.Parallel() + for _, url := range []string{ + "", + "not-a-url", + "mysql://certctl@db:3306/x?sslmode=disable", // non-postgres scheme + } { + url := url + t.Run(url, func(t *testing.T) { + t.Parallel() + logger, buf := newBufferLogger() + warnExternalSslmodeDisable(url, logger) + if buf.Len() != 0 { + t.Errorf("expected silence for unparseable/non-postgres input (%q); got: %s", url, buf.String()) + } + }) + } +}