From 0ce97c322a3c8a14350f7c6bfbd093ec4e45cb36 Mon Sep 17 00:00:00 2001 From: Thando Mini Date: Thu, 11 Jun 2026 12:59:09 +0200 Subject: [PATCH] fix(web): fail-fast on crypto/rand + drop bare auth-token log line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two web-server security hardening fixes from the 2026-06-11 audit, bundled because they sit two lines apart and share one Start path. - crypto/rand failure used to fall back to `time.Now().UnixNano()` for the 16-byte session token. A nanosecond timestamp has ~30 bits of entropy in the low-order bits and is bruteforceable in milliseconds if an attacker knows roughly when the server started. Replace the fallback with log.Fatalf so OS entropy starvation surfaces instead of silently degrading to insecure tokens. - The `log.Printf("[web] auth token: %s", token)` line duplicated the token into log aggregators (Datadog/Splunk/syslog) at INFO level — far wider read surface than the operator console. The URL line immediately above already carries the token for operator discovery. Drop the bare line. New TestServer_Start_DoesNotLogBareAuthToken captures log output during a Start/Shutdown cycle and asserts the `[web] auth token:` prefix never appears. Surfaced by the 2026-06-11 security audit (SEC-M1, SEC-M2). --- internal/web/server.go | 15 ++++++++---- internal/web/start_lifecycle_test.go | 36 ++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/internal/web/server.go b/internal/web/server.go index cfda780..686bbb3 100644 --- a/internal/web/server.go +++ b/internal/web/server.go @@ -56,11 +56,14 @@ func NewServer(es state.EventStore, ps *state.SQLiteStore, port int, filter stat // C1: generate a random per-session token. Required on every WebSocket // upgrade and on the static-asset HTTP handlers, preventing localhost // cross-process CSRF and unauthenticated dashboard access. + // + // crypto/rand failures are extremely rare on a healthy host but signal + // OS entropy starvation — the previous fallback to a nanosecond + // timestamp gave ~30 bits of entropy and was bruteforceable in + // milliseconds. Fail fast instead so the operator notices. tokenBytes := make([]byte, 16) if _, err := rand.Read(tokenBytes); err != nil { - // extremely unlikely on a healthy host; fall back to time-based. - log.Printf("[web] crypto/rand error, using insecure fallback: %v", err) - copy(tokenBytes, []byte(fmt.Sprintf("%d", time.Now().UnixNano()))) + log.Fatalf("[web] crypto/rand failed (OS entropy starvation?): %v", err) } token := hex.EncodeToString(tokenBytes) @@ -165,10 +168,12 @@ func (s *Server) Start(ctx context.Context) error { ReadHeaderTimeout: 5 * time.Second, // S3-7-adjacent: prevent slowloris } - // Open browser with the auth-gated URL. + // Open browser with the auth-gated URL. The URL contains the token — + // don't log the bare token separately so it doesn't end up duplicated + // in log aggregators with a wider read surface than the operator + // console. url := fmt.Sprintf("http://%s/?token=%s", addr, s.authToken) log.Printf("Dashboard server running at %s", url) - log.Printf("[web] auth token: %s (required as ?token=)", s.authToken) openBrowser(url) // Start hub broadcast loop diff --git a/internal/web/start_lifecycle_test.go b/internal/web/start_lifecycle_test.go index 8f91e6f..b6d8d8a 100644 --- a/internal/web/start_lifecycle_test.go +++ b/internal/web/start_lifecycle_test.go @@ -1,7 +1,9 @@ package web import ( + "bytes" "context" + "log" "net" "net/http" "strings" @@ -215,3 +217,37 @@ func itoa(i int) string { } return string(b[pos:]) } + +// TestServer_Start_DoesNotLogBareAuthToken guards the SEC-M1 fix: the +// auth token used to be log.Printf'd as its own line, duplicating it +// into any log aggregator that captured stderr. The URL line still +// contains the token (operator UX), but no bare `[web] auth token:` +// line should appear. +func TestServer_Start_DoesNotLogBareAuthToken(t *testing.T) { + var buf bytes.Buffer + prev := log.Writer() + log.SetOutput(&buf) + t.Cleanup(func() { log.SetOutput(prev) }) + + s := newTestServer(t) + s.port = freePort(t) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + errCh := make(chan error, 1) + go func() { errCh <- s.Start(ctx) }() + + if !waitForPort(t, s.port, 2*time.Second) { + t.Fatal("server did not bind within 2s") + } + cancel() + if err := <-errCh; err != nil && err != http.ErrServerClosed { + t.Fatalf("Start: %v", err) + } + + logged := buf.String() + if strings.Contains(logged, "[web] auth token:") { + t.Fatalf("bare auth token leaked to logs:\n%s", logged) + } +}