feat(config): SEC-013 — advisory WARN on external sslmode=disable

Acquisition-audit SEC-013 closure (Sprint 2 ACQ, 2026-05-16). Add a post-Validate advisory WARN (NOT fail-closed) that fires when `CERTCTL_DATABASE_URL` parses as a Postgres URL with `sslmode=disable` AND the host is outside the local safelist. 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. Safelist (silenced): - localhost, 127.0.0.1, ::1 - postgres (compose default service name) - certctl-postgres (compose / Helm service name) - *.svc.cluster.local (K8s in-cluster service-name convention) Anything else → `slog.Warn` with structured `host=` + `sslmode=` fields plus a pointer to docs/operator/database-tls.md for the verify-full upgrade procedure. Tests: - TestWarnExternalSslmodeDisable_FiresOnExternalHost - TestWarnExternalSslmodeDisable_QuietForLocalSafelist (6 subtests) - TestWarnExternalSslmodeDisable_QuietWithoutDisable (3 subtests) - TestWarnExternalSslmodeDisable_QuietOnUnparseableOrEmpty (3 subtests) Docs: docs/operator/security.md gains a Postgres transport encryption subsection covering both SEC-013 (this commit) and SEC-014 (loopback host-port bind, prior commit); the deep procedure remains at docs/operator/database-tls.md.
2026-06-07 12:21:31 +00:00 · 2026-05-16 17:12:58 +00:00
parent 7e2481b225
commit 578ac4ec68
3 changed files with 225 additions and 1 deletions
@@ -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 <name>.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=<ca-path>).",
+		"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 != "" {
@@ -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())
+			}
+		})
+	}
+}