From 7a513dcea0ca068d118840e5cfc9e7002076b1c5 Mon Sep 17 00:00:00 2001 From: Kris Armstrong Date: Tue, 16 Jun 2026 17:08:19 -0400 Subject: [PATCH] refactor(api): extract TLS provisioning into internal/api/tlsutil leaf (ADR-0011) Move certificate provisioning, ACME manager construction, the TLS 1.3 config template, and the active-cert fingerprint cache out of the flat internal/api namespace into the internal/api/tlsutil leaf package (ADR-0011). The leaf depends only on stdlib, golang.org/x/crypto, and internal/logging; the api-tlsutil-isolated depguard rule statically forbids any upward import of the transport layer. The HTTP serving lifecycle stays in internal/api: startTLS / startTLSWithACME and the two Server methods that bridge the cache to the request path (activeCertPath, tlsFingerprintForResponse). Two constants that lived in the old tls.go const block but are unrelated to TLS are rehomed to the api layer rather than dragged into the leaf: refreshMultiplier (auth cookie lifetime -> handlers_auth.go) and acmeReadHeaderTimeoutSec (challenge-server timeout -> server.go). The ForTest ACME wrappers in test_helpers.go are dropped; the TLS tests now exercise the leaf's exported API directly from internal/api/tlsutil. No behaviour change: endpoints, /__version fingerprint, and cert paths are identical. --- .golangci.yml | 14 + ...-internal-api-sub-package-decomposition.md | 29 +- internal/api/handlers_auth.go | 5 + internal/api/server.go | 60 +++- internal/api/server_internal_test.go | 139 --------- internal/api/test_helpers.go | 19 -- internal/api/tls_test.go | 179 ----------- .../fingerprint.go} | 72 +---- internal/api/{tls.go => tlsutil/tlsutil.go} | 64 ++-- internal/api/tlsutil/tlsutil_internal_test.go | 37 +++ internal/api/tlsutil/tlsutil_test.go | 292 ++++++++++++++++++ 11 files changed, 483 insertions(+), 427 deletions(-) delete mode 100644 internal/api/tls_test.go rename internal/api/{tls_fingerprint.go => tlsutil/fingerprint.go} (57%) rename internal/api/{tls.go => tlsutil/tlsutil.go} (79%) create mode 100644 internal/api/tlsutil/tlsutil_internal_test.go create mode 100644 internal/api/tlsutil/tlsutil_test.go diff --git a/.golangci.yml b/.golangci.yml index 53d51414..1b833d02 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -277,6 +277,20 @@ linters: deny: - pkg: github.com/MustardSeedNetworks/stem/internal/api desc: "sse is a leaf — the broadcaster engine must not import the api transport layer; wire at the api layer" + # tlsutil is a leaf of internal/api (ADR-0011): it depends only on + # stdlib, golang.org/x/crypto (ACME), and internal/logging — never on + # the api transport layer itself. This depguard rule enforces that + # boundary so a future accidental upward import fails CI rather than + # silently coupling the leaf back into the transport layer. + # Test files are excluded: the _test.go external package imports the + # leaf itself, which is expected. + "api-tlsutil-isolated": + files: + - "**/internal/api/tlsutil/**" + - "!$test" + deny: + - pkg: github.com/MustardSeedNetworks/stem/internal/api + desc: "tlsutil is a leaf — cert provisioning, ACME, and TLS config take no api types, so it never imports the api transport layer" embeddedstructfieldcheck: # Checks that sync.Mutex and sync.RWMutex are not used as embedded fields. diff --git a/docs/adr/0011-internal-api-sub-package-decomposition.md b/docs/adr/0011-internal-api-sub-package-decomposition.md index d4ab7008..866dd14a 100644 --- a/docs/adr/0011-internal-api-sub-package-decomposition.md +++ b/docs/adr/0011-internal-api-sub-package-decomposition.md @@ -33,7 +33,8 @@ The api transport layer wires leaves at construction time; no leaf knows about | Slice | Package | PR | |-------|---------|-----| | Rate limiter | `internal/api/ratelimit` | #451 | -| SSE broadcaster | `internal/api/sse` | this ADR | +| SSE broadcaster | `internal/api/sse` | #452 | +| TLS utilities | `internal/api/tlsutil` | this ADR | ### SSE slice (this ADR) @@ -47,17 +48,39 @@ reference it without duplicating the value; the api-layer file (`sse.go`) re-derives its own constant from the same numeric literal to avoid an import dependency in the other direction. +### TLS slice (this ADR) + +`internal/api/tlsutil` holds the TLS material provisioning: the `Config`/ +`ACMEConfig` settings structs, `ServerConfig` (the TLS 1.3 [tls.Config] +template), `EnsureSelfSignedCert` (self-signed cert generation), the ACME +manager constructors (`NewACMEManager`, `ACMETLSConfig`), and the +`FingerprintCache` (active-cert SHA-256 fingerprint, exposed via `/__version`). +It depends only on the standard library, `golang.org/x/crypto` (ACME), and +`internal/logging`. + +The HTTP serving lifecycle stays in `internal/api`: `startTLS`/`startTLSWithACME` +(listener binding + the port-80 HTTP-01 challenge server) and the two `Server` +methods that bridge the cache to the request path — `activeCertPath` (resolves +the served cert path mirroring `startTLS`'s priority order) and +`tlsFingerprintForResponse`. `DefaultCertsDir` is exported so the api layer +resolves the self-signed default path without duplicating the literal. + +Two constants that lived in the old `tls.go` const block but are unrelated to +TLS were rehomed to the api layer rather than dragged into the leaf: +`refreshMultiplier` (auth cookie lifetime → `handlers_auth.go`) and +`acmeReadHeaderTimeoutSec` (transport-layer challenge-server timeout → +`server.go`). + ### Future slices (candidates) | Concern | Notes | |---------|-------| -| TLS utilities | `ensureSelfSignedCert`, `createTLSConfig`, ACME helpers | | CORS logic | RFC 1918 origin validation | ## Consequences - The leaf boundary is statically enforced by depguard (`api-sse-isolated`, - `api-ratelimit-isolated` rules in `.golangci.yml`). + `api-ratelimit-isolated`, `api-tlsutil-isolated` rules in `.golangci.yml`). - `go vet` + `golangci-lint` catch upward imports at CI time. - `internal/api` package size decreases incrementally with each slice. - No behaviour change: endpoints, event types, and publish sites are identical. diff --git a/internal/api/handlers_auth.go b/internal/api/handlers_auth.go index 0f137503..64aab275 100644 --- a/internal/api/handlers_auth.go +++ b/internal/api/handlers_auth.go @@ -10,6 +10,11 @@ import ( "github.com/MustardSeedNetworks/stem/internal/logging" ) +// refreshMultiplier sets how much longer the refresh-token cookie lives than +// the access token: the refresh cookie expires after sessionDuration * +// refreshMultiplier. Shared by the login, refresh, and MFA auth handlers. +const refreshMultiplier = 24 // Refresh token lasts 24x longer than access token + // handleAuthLogin issues JWT tokens for valid credentials. // Sets httpOnly cookies for browser auth and returns tokens for API clients. // diff --git a/internal/api/server.go b/internal/api/server.go index 77d35603..7745c75f 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -77,6 +77,7 @@ import ( "net/url" "os" "os/signal" + "path/filepath" "strings" "sync" "syscall" @@ -84,6 +85,7 @@ import ( "github.com/MustardSeedNetworks/stem/internal/api/ratelimit" "github.com/MustardSeedNetworks/stem/internal/api/sse" + "github.com/MustardSeedNetworks/stem/internal/api/tlsutil" "github.com/MustardSeedNetworks/stem/internal/auth" "github.com/MustardSeedNetworks/stem/internal/license" "github.com/MustardSeedNetworks/stem/internal/logging" @@ -157,7 +159,7 @@ type Server struct { currentModule string authLimiter *ratelimit.RateLimiter // Rate limiter for auth endpoints (5/min) apiLimiter *ratelimit.RateLimiter // Rate limiter for standard API endpoints (100/min) - tlsConfig TLSConfig // TLS configuration for HTTPS + tlsConfig tlsutil.Config // TLS configuration for HTTPS cookieConfig auth.CookieConfig // Cookie configuration for secure auth corsAllowPrivate bool // STEM_CORS_ALLOW_PRIVATE: reflect RFC1918 cross-origins (default off) routeManifest []route // capability registry: routes registered via register() (route.go) @@ -168,7 +170,7 @@ type Server struct { recoveryTokenManager *auth.RecoveryTokenManager // Recovery token manager for password recovery dataDir string // Application data directory for recovery files acmeChallengeServer *http.Server // HTTP-01 challenge server for ACME - tlsFingerprint tlsFingerprintCache // Cached SHA-256 fingerprint of the active TLS cert (exposed via /__version) + tlsFingerprint tlsutil.FingerprintCache // Cached SHA-256 fingerprint of the active TLS cert (exposed via /__version) background *BackgroundComponents // Run-scoped long-lived goroutines (reflector-stats SSE publisher); ordered Start/Stop (background.go) // executorResolver maps a module name to a factory producing a @@ -292,7 +294,7 @@ func NewServer(port int) (*Server, error) { s.currentModule = "" s.authLimiter = ratelimit.NewAuthRateLimiter() s.apiLimiter = ratelimit.NewAPIRateLimiter() - s.tlsConfig = TLSConfig{ + s.tlsConfig = tlsutil.Config{ Enabled: true, CertFile: os.Getenv("STEM_TLS_CERT"), KeyFile: os.Getenv("STEM_TLS_KEY"), @@ -814,6 +816,10 @@ func (s *Server) Run() error { } } +// acmeReadHeaderTimeoutSec is the timeout for reading ACME HTTP-01 challenge +// request headers on the port-80 challenge server. +const acmeReadHeaderTimeoutSec = 10 + // startTLS starts the server with TLS encryption on the already-bound // listener. Priority order: ACME → manual certificates → self-signed. func (s *Server) startTLS(ln net.Listener) error { @@ -832,14 +838,14 @@ func (s *Server) startTLS(ln net.Listener) error { // Priority 3: Self-signed certificate (fallback) if certFile == "" || keyFile == "" { var err error - certFile, keyFile, err = ensureSelfSignedCert(s.tlsConfig.CertsDir) + certFile, keyFile, err = tlsutil.EnsureSelfSignedCert(s.tlsConfig.CertsDir) if err != nil { return fmt.Errorf("failed to generate self-signed certificate: %w", err) } } // Configure TLS 1.3 minimum. - s.httpServer.TLSConfig = createTLSConfig() + s.httpServer.TLSConfig = tlsutil.ServerConfig() logging.Info("Starting HTTPS server", "addr", s.httpServer.Addr, @@ -858,13 +864,13 @@ func (s *Server) startTLS(ln net.Listener) error { // on the already-bound listener. Ported from Seed project for automatic // certificate management. func (s *Server) startTLSWithACME(ln net.Listener) error { - manager, err := createACMEManager(s.tlsConfig.ACME) + manager, err := tlsutil.NewACMEManager(s.tlsConfig.ACME) if err != nil { return fmt.Errorf("create ACME manager: %w", err) } // Configure TLS with ACME - s.httpServer.TLSConfig = createACMETLSConfig(manager) + s.httpServer.TLSConfig = tlsutil.ACMETLSConfig(manager) logging.Info("Starting HTTPS server with ACME", "addr", s.httpServer.Addr, @@ -892,6 +898,46 @@ func (s *Server) startTLSWithACME(ln net.Listener) error { return nil } +// activeCertPath returns the cert file path the server will use, or "" if the +// server is running in HTTP mode. Mirrors the priority order of startTLS so +// /__version reports the same cert that is actually served. +func (s *Server) activeCertPath() string { + if !s.tlsConfig.Enabled { + return "" + } + if s.tlsConfig.ACME.Enabled { + // ACME certs live in the autocert cache; they are not a single + // stable file path we can fingerprint here. Return empty rather + // than guessing. + return "" + } + if s.tlsConfig.CertFile != "" { + return s.tlsConfig.CertFile + } + // Fall back to the self-signed default path used by EnsureSelfSignedCert. + certsDir := s.tlsConfig.CertsDir + if certsDir == "" { + certsDir = tlsutil.DefaultCertsDir + } + return filepath.Join(certsDir, "server.crt") +} + +// tlsFingerprintForResponse returns the cached fingerprint (computing it on +// first call). Errors are swallowed and reported as an empty string so +// /__version always returns a stable shape even if the cert is missing or +// unreadable; an empty value is a signal to the operator to investigate. +func (s *Server) tlsFingerprintForResponse() string { + path := s.activeCertPath() + if path == "" { + return "" + } + fp, err := s.tlsFingerprint.Get(path) + if err != nil { + return "" + } + return strings.TrimSpace(fp) +} + // Shutdown gracefully shuts down the server. // Stops running tests and drains HTTP connections. func (s *Server) Shutdown() error { diff --git a/internal/api/server_internal_test.go b/internal/api/server_internal_test.go index 5cbc9676..310c6984 100644 --- a/internal/api/server_internal_test.go +++ b/internal/api/server_internal_test.go @@ -9,7 +9,6 @@ import ( "errors" "net/http" "net/http/httptest" - "os" "testing" "github.com/MustardSeedNetworks/stem/internal/api/ratelimit" @@ -922,108 +921,6 @@ func TestSecurityHeadersMiddleware_AllHeaders(t *testing.T) { } } -// TestDefaultTLSConfig tests the DefaultTLSConfig function. -func TestDefaultTLSConfig(t *testing.T) { - config := DefaultTLSConfig() - - if !config.Enabled { - t.Error("Expected TLS to be enabled by default") - } - if config.CertFile != "" { - t.Errorf("Expected empty CertFile, got '%s'", config.CertFile) - } - if config.KeyFile != "" { - t.Errorf("Expected empty KeyFile, got '%s'", config.KeyFile) - } - if config.CertsDir != defaultCertsDir { - t.Errorf("Expected CertsDir '%s', got '%s'", defaultCertsDir, config.CertsDir) - } -} - -// TestCreateTLSConfig tests the createTLSConfig function. -func TestCreateTLSConfig(t *testing.T) { - config := createTLSConfig() - - if config == nil { - t.Fatal("Expected non-nil TLS config") - } - - if config.MinVersion != tls.VersionTLS13 { - t.Errorf("Expected TLS 1.3 min version, got %d", config.MinVersion) - } -} - -// TestEnsureSelfSignedCert tests the ensureSelfSignedCert function. -func TestEnsureSelfSignedCert(t *testing.T) { - // Create a temporary directory for test certificates. - tempDir := t.TempDir() - - t.Run("generate new certificates", func(t *testing.T) { - certFile, keyFile, err := ensureSelfSignedCert(tempDir) - if err != nil { - t.Fatalf("ensureSelfSignedCert() error: %v", err) - } - - if certFile == "" { - t.Error("Expected non-empty certFile") - } - if keyFile == "" { - t.Error("Expected non-empty keyFile") - } - - // Verify files exist. - _, certStatErr := os.Stat(certFile) - if certStatErr != nil { - t.Errorf("Certificate file does not exist: %v", certStatErr) - } - _, keyStatErr := os.Stat(keyFile) - if keyStatErr != nil { - t.Errorf("Key file does not exist: %v", keyStatErr) - } - }) - - t.Run("use existing certificates", func(t *testing.T) { - // Should reuse existing certificates. - certFile, keyFile, err := ensureSelfSignedCert(tempDir) - if err != nil { - t.Fatalf("ensureSelfSignedCert() error: %v", err) - } - - if certFile == "" || keyFile == "" { - t.Error("Expected non-empty certificate paths") - } - }) - - t.Run("empty certs dir defaults to default", func(t *testing.T) { - // This would use the default certs dir. - // Skip if we don't want to pollute the filesystem. - t.Skip("Skipping to avoid creating files in default location") - }) -} - -// TestTLSConfigStruct tests the TLSConfig struct. -func TestTLSConfigStruct(t *testing.T) { - config := TLSConfig{ - Enabled: true, - CertFile: "/path/to/cert.pem", - KeyFile: "/path/to/key.pem", - CertsDir: "/path/to/certs", - } - - if !config.Enabled { - t.Error("Expected Enabled to be true") - } - if config.CertFile != "/path/to/cert.pem" { - t.Errorf("Unexpected CertFile: %s", config.CertFile) - } - if config.KeyFile != "/path/to/key.pem" { - t.Errorf("Unexpected KeyFile: %s", config.KeyFile) - } - if config.CertsDir != "/path/to/certs" { - t.Errorf("Unexpected CertsDir: %s", config.CertsDir) - } -} - // TestUpdateStats tests the UpdateStats function. func TestUpdateStats(t *testing.T) { t.Setenv("STEM_AUTH_USERNAME", "statsuser") @@ -2855,42 +2752,6 @@ func TestReflectorStatsResponse(t *testing.T) { } } -// TestEnsureSelfSignedCertExistingFiles tests ensureSelfSignedCert with existing files. -func TestEnsureSelfSignedCertExistingFiles(t *testing.T) { - tempDir := t.TempDir() - - // First call - generate. - certFile, keyFile, err := ensureSelfSignedCert(tempDir) - if err != nil { - t.Fatalf("First call error: %v", err) - } - - // Get file info. - certInfo, _ := os.Stat(certFile) - keyInfo, _ := os.Stat(keyFile) - - // Second call - should reuse. - certFile2, keyFile2, err := ensureSelfSignedCert(tempDir) - if err != nil { - t.Fatalf("Second call error: %v", err) - } - - if certFile != certFile2 || keyFile != keyFile2 { - t.Error("Expected same paths on second call") - } - - // Files should not have changed. - certInfo2, _ := os.Stat(certFile) - keyInfo2, _ := os.Stat(keyFile) - - if certInfo.ModTime() != certInfo2.ModTime() { - t.Error("Cert file should not have been regenerated") - } - if keyInfo.ModTime() != keyInfo2.ModTime() { - t.Error("Key file should not have been regenerated") - } -} - // TestBuildReflectorConfigUpdateWithExecutor tests buildReflectorConfigUpdate with executor. func TestBuildReflectorConfigUpdateWithExecutor(t *testing.T) { t.Setenv("STEM_AUTH_USERNAME", "buildcfgexecuser") diff --git a/internal/api/test_helpers.go b/internal/api/test_helpers.go index 9b30121c..b17c18f2 100644 --- a/internal/api/test_helpers.go +++ b/internal/api/test_helpers.go @@ -3,10 +3,6 @@ package api import ( - "crypto/tls" - - "golang.org/x/crypto/acme/autocert" - "github.com/MustardSeedNetworks/stem/internal/auth" ) @@ -62,21 +58,6 @@ func (s *Server) ResetTestStateForTest() { s.testResult = nil } -// CreateACMETLSConfigForTest exposes ACME TLS config creation for tests. -func CreateACMETLSConfigForTest(manager *autocert.Manager) *tls.Config { - return createACMETLSConfig(manager) -} - -// CreateACMEManagerForTest exposes ACME manager creation for tests. -func CreateACMEManagerForTest(config ACMEConfig) (*autocert.Manager, error) { - return createACMEManager(config) -} - -// DefaultACMECacheDirForTest exposes default ACME cache dir. -func DefaultACMECacheDirForTest() string { - return defaultACMECacheDir -} - // CSRFManagerForTest exposes the server's CSRFManager for tests that // need to assert directly on token-lifecycle state (e.g. rotation on // login, expiry handling). diff --git a/internal/api/tls_test.go b/internal/api/tls_test.go deleted file mode 100644 index 090ae9f8..00000000 --- a/internal/api/tls_test.go +++ /dev/null @@ -1,179 +0,0 @@ -// SPDX-License-Identifier: BUSL-1.1 - -package api_test - -import ( - "os" - "path/filepath" - "testing" - - "github.com/MustardSeedNetworks/stem/internal/api" -) - -// TestACMEConfig tests the ACME configuration struct. -func TestACMEConfig(t *testing.T) { - tests := []struct { - name string - config api.ACMEConfig - valid bool - }{ - { - name: "valid config", - config: api.ACMEConfig{ - Enabled: true, - Domain: "stem.example.com", - Email: "admin@example.com", - CacheDir: "certs/acme", - Staging: false, - }, - valid: true, - }, - { - name: "staging mode", - config: api.ACMEConfig{ - Enabled: true, - Domain: "test.example.com", - Email: "test@example.com", - CacheDir: "", - Staging: true, - }, - valid: true, - }, - { - name: "disabled config", - config: api.ACMEConfig{ - Enabled: false, - }, - valid: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if tt.config.Enabled && tt.config.Domain == "" { - t.Error("Expected domain to be required when ACME is enabled") - } - }) - } -} - -// TestCreateACMEManager tests the ACME manager creation. -func TestCreateACMEManager(t *testing.T) { - // Create temporary directory for cache - tmpDir := t.TempDir() - cacheDir := filepath.Join(tmpDir, "acme-cache") - - config := api.ACMEConfig{ - Enabled: true, - Domain: "test.example.com", - Email: "test@example.com", - CacheDir: cacheDir, - Staging: true, - } - - manager, err := api.CreateACMEManagerForTest(config) - if err != nil { - t.Fatalf("api.CreateACMEManagerForTest() error: %v", err) - } - - if manager == nil { - t.Fatal("Expected non-nil manager") - } - - // Verify cache directory was created - if _, statErr := os.Stat(cacheDir); os.IsNotExist(statErr) { - t.Error("Expected cache directory to be created") - } -} - -// TestCreateACMEManagerDefaultCache tests default cache directory. -func TestCreateACMEManagerDefaultCache(t *testing.T) { - // Skip if we can't create the default cache dir - if err := os.MkdirAll(api.DefaultACMECacheDirForTest(), 0o700); err != nil { - t.Skip("Cannot create default cache dir") - } - defer func() { _ = os.RemoveAll("certs") }() - - config := api.ACMEConfig{ - Enabled: true, - Domain: "test.example.com", - Email: "test@example.com", - CacheDir: "", // Use default - Staging: true, - } - - manager, err := api.CreateACMEManagerForTest(config) - if err != nil { - t.Fatalf("api.CreateACMEManagerForTest() error: %v", err) - } - - if manager == nil { - t.Fatal("Expected non-nil manager") - } -} - -// TestCreateACMETLSConfig tests TLS config creation with ACME. -func TestCreateACMETLSConfig(t *testing.T) { - tmpDir := t.TempDir() - - config := api.ACMEConfig{ - Enabled: true, - Domain: "test.example.com", - Email: "test@example.com", - CacheDir: tmpDir, - Staging: true, - } - - manager, err := api.CreateACMEManagerForTest(config) - if err != nil { - t.Fatalf("api.CreateACMEManagerForTest() error: %v", err) - } - - tlsConfig := api.CreateACMETLSConfigForTest(manager) - if tlsConfig == nil { - t.Fatal("Expected non-nil TLS config") - } - - // TLS 1.3 minimum should be set - if tlsConfig.MinVersion != 0x0304 { // tls.VersionTLS13 - t.Errorf("Expected MinVersion TLS 1.3, got %x", tlsConfig.MinVersion) - } -} - -// TestTLSConfigWithACME tests api.TLSConfig with ACME enabled. -func TestTLSConfigWithACME(t *testing.T) { - config := api.TLSConfig{ - Enabled: true, - CertFile: "", - KeyFile: "", - CertsDir: "certs", - ACME: api.ACMEConfig{ - Enabled: true, - Domain: "stem.example.com", - Email: "admin@example.com", - CacheDir: "certs/acme", - Staging: false, - }, - } - - // When ACME is enabled, CertFile/KeyFile should be ignored - if !config.Enabled { - t.Error("Expected TLS to be enabled") - } - if config.CertFile != "" { - t.Errorf("CertFile = %q, want empty", config.CertFile) - } - if config.KeyFile != "" { - t.Errorf("KeyFile = %q, want empty", config.KeyFile) - } - if config.CertsDir != "certs" { - t.Errorf("CertsDir = %q, want %q", config.CertsDir, "certs") - } - if !config.ACME.Enabled { - t.Error("Expected ACME to be enabled") - } - - if config.ACME.Domain == "" { - t.Error("Expected domain to be set") - } -} diff --git a/internal/api/tls_fingerprint.go b/internal/api/tlsutil/fingerprint.go similarity index 57% rename from internal/api/tls_fingerprint.go rename to internal/api/tlsutil/fingerprint.go index 1bcabd3c..04cd0b21 100644 --- a/internal/api/tls_fingerprint.go +++ b/internal/api/tlsutil/fingerprint.go @@ -1,12 +1,12 @@ // SPDX-License-Identifier: BUSL-1.1 -package api +package tlsutil -// tls_fingerprint.go computes and caches the SHA-256 fingerprint of the -// active TLS certificate. The fingerprint is exposed via /__version as -// `tlsFingerprint` so operators can verify the cert their browser sees -// matches the one the server is serving (matters for self-signed certs -// installed via `stem install-ca`). +// fingerprint.go computes and caches the SHA-256 fingerprint of the active TLS +// certificate. The fingerprint is exposed via /__version as `tlsFingerprint` +// so operators can verify the cert their browser sees matches the one the +// server is serving (matters for self-signed certs installed via +// `stem install-ca`). // // Lifted from the seed project (internal/api/tls_fingerprint.go); keep in sync. @@ -17,8 +17,6 @@ import ( "errors" "fmt" "os" - "path/filepath" - "strings" "sync" ) @@ -33,12 +31,14 @@ var errEmptyCertPath = errors.New("no certificate configured") // contain a CERTIFICATE block. var errNoCertificateBlock = errors.New("no CERTIFICATE block in PEM data") -// tlsFingerprintCache caches the active TLS certificate fingerprint so -// repeated /__version calls do not re-read disk. The cache key is the -// cert file path; this lets the cache stay valid across a server's -// lifetime (cert file is not rotated at runtime — a restart picks up a -// new fingerprint via cache miss on a different path or first access). -type tlsFingerprintCache struct { +// FingerprintCache caches the active TLS certificate fingerprint so repeated +// /__version calls do not re-read disk. The cache key is the cert file path; +// this lets the cache stay valid across a server's lifetime (cert file is not +// rotated at runtime — a restart picks up a new fingerprint via cache miss on a +// different path or first access). +// +// The zero value is ready to use. +type FingerprintCache struct { mu sync.RWMutex path string fingerprint string @@ -47,7 +47,7 @@ type tlsFingerprintCache struct { // Get returns the fingerprint for the given cert file path, computing // and caching it on first access. An empty path returns an empty // fingerprint without error (HTTP mode is a supported configuration). -func (c *tlsFingerprintCache) Get(path string) (string, error) { +func (c *FingerprintCache) Get(path string) (string, error) { if path == "" { return "", nil } @@ -80,7 +80,7 @@ func computeCertFingerprint(path string) (string, error) { if path == "" { return "", errEmptyCertPath } - // #nosec G304 -- path is server-controlled (TLSConfig.CertFile or the + // #nosec G304 -- path is server-controlled (Config.CertFile or the // self-signed default at certs/server.crt), not user input. data, err := os.ReadFile(path) if err != nil { @@ -129,43 +129,3 @@ func formatFingerprint(digest []byte) string { } return string(out) } - -// activeCertPath returns the cert file path the server will use, or "" if -// the server is running in HTTP mode. Mirrors the priority order of -// startTLS so /__version reports the same cert that is actually served. -func (s *Server) activeCertPath() string { - if !s.tlsConfig.Enabled { - return "" - } - if s.tlsConfig.ACME.Enabled { - // ACME certs live in the autocert cache; they are not a single - // stable file path we can fingerprint here. Return empty rather - // than guessing. - return "" - } - if s.tlsConfig.CertFile != "" { - return s.tlsConfig.CertFile - } - // Fall back to the self-signed default path used by ensureSelfSignedCert. - certsDir := s.tlsConfig.CertsDir - if certsDir == "" { - certsDir = defaultCertsDir - } - return filepath.Join(certsDir, "server.crt") -} - -// tlsFingerprintForResponse returns the cached fingerprint (computing it -// on first call). Errors are swallowed and reported as an empty string so -// /__version always returns a stable shape even if the cert is missing or -// unreadable; an empty value is a signal to the operator to investigate. -func (s *Server) tlsFingerprintForResponse() string { - path := s.activeCertPath() - if path == "" { - return "" - } - fp, err := s.tlsFingerprint.Get(path) - if err != nil { - return "" - } - return strings.TrimSpace(fp) -} diff --git a/internal/api/tls.go b/internal/api/tlsutil/tlsutil.go similarity index 79% rename from internal/api/tls.go rename to internal/api/tlsutil/tlsutil.go index daf3ae7a..fa7971ff 100644 --- a/internal/api/tls.go +++ b/internal/api/tlsutil/tlsutil.go @@ -1,6 +1,20 @@ // SPDX-License-Identifier: BUSL-1.1 -package api +// Package tlsutil provisions and configures the TLS material the api transport +// layer serves: self-signed certificate generation, ACME/Let's Encrypt manager +// construction, the secure [tls.Config] template, and the active-cert +// fingerprint cache. +// +// It is a leaf of internal/api (ADR-0011): it depends only on the standard +// library, golang.org/x/crypto (ACME), and internal/logging — never on the api +// transport layer itself. The boundary is enforced by depguard +// (api-tlsutil-isolated) so a future accidental upward import fails CI rather +// than silently coupling the leaf back into the transport layer. +// +// The HTTP serving lifecycle (binding the listener, the ACME HTTP-01 challenge +// server, /__version response assembly) lives in the api transport layer and +// calls into this package via the exported constructors. +package tlsutil import ( "crypto/rand" @@ -21,17 +35,19 @@ import ( "github.com/MustardSeedNetworks/stem/internal/logging" ) -// TLS configuration constants. +// TLS provisioning constants. const ( - rsaKeyBits = 4096 - certValidYears = 1 - defaultCertsDir = "certs" + rsaKeyBits = 4096 + certValidYears = 1 + + // DefaultCertsDir is the directory self-signed certificates are written to + // and read from when [Config.CertsDir] is empty. Exported so the api layer + // can resolve the active cert path for fingerprinting without duplicating + // the literal. + DefaultCertsDir = "certs" + defaultACMECacheDir = "certs/acme" serialNumberBitSize = 128 - refreshMultiplier = 24 // Refresh token lasts 24x longer than access token - - // acmeReadHeaderTimeoutSec is the timeout for reading ACME challenge request headers. - acmeReadHeaderTimeoutSec = 10 // privateKeyFileMode is the umask-restricted mode for new TLS key files // (owner read/write only, no group/other access). @@ -60,8 +76,8 @@ type ACMEConfig struct { Staging bool } -// TLSConfig holds TLS configuration options. -type TLSConfig struct { +// Config holds TLS configuration options for the server. +type Config struct { // Enabled enables HTTPS mode. Enabled bool @@ -82,29 +98,29 @@ type TLSConfig struct { ACME ACMEConfig } -// DefaultTLSConfig returns secure TLS defaults with auto-generated certificates. -func DefaultTLSConfig() TLSConfig { - return TLSConfig{ +// DefaultConfig returns secure TLS defaults with auto-generated certificates. +func DefaultConfig() Config { + return Config{ Enabled: true, CertFile: "", KeyFile: "", - CertsDir: defaultCertsDir, + CertsDir: DefaultCertsDir, } } -// createTLSConfig creates a [tls.Config] with secure settings. +// ServerConfig creates a [tls.Config] with secure settings. // Uses TLS 1.3 minimum for best security. -func createTLSConfig() *tls.Config { +func ServerConfig() *tls.Config { return &tls.Config{ MinVersion: tls.VersionTLS13, } } -// ensureSelfSignedCert generates a self-signed certificate if needed. +// EnsureSelfSignedCert generates a self-signed certificate if needed. // Returns paths to the certificate and key files. -func ensureSelfSignedCert(certsDir string) (string, string, error) { +func EnsureSelfSignedCert(certsDir string) (string, string, error) { if certsDir == "" { - certsDir = defaultCertsDir + certsDir = DefaultCertsDir } // Sanitize directory path to prevent directory traversal. @@ -253,9 +269,9 @@ func writePrivateKey(root *os.Root, keyFile string, privateKey *rsa.PrivateKey) return nil } -// createACMEManager creates an autocert.Manager for Let's Encrypt certificate management. +// NewACMEManager creates an autocert.Manager for Let's Encrypt certificate management. // Ported from Seed project for automatic certificate management. -func createACMEManager(config ACMEConfig) (*autocert.Manager, error) { +func NewACMEManager(config ACMEConfig) (*autocert.Manager, error) { cacheDir := config.CacheDir if cacheDir == "" { cacheDir = defaultACMECacheDir @@ -283,8 +299,8 @@ func createACMEManager(config ACMEConfig) (*autocert.Manager, error) { return manager, nil } -// createACMETLSConfig creates a TLS config using the ACME manager. -func createACMETLSConfig(manager *autocert.Manager) *tls.Config { +// ACMETLSConfig creates a TLS config using the ACME manager. +func ACMETLSConfig(manager *autocert.Manager) *tls.Config { tlsConfig := manager.TLSConfig() tlsConfig.MinVersion = tls.VersionTLS13 return tlsConfig diff --git a/internal/api/tlsutil/tlsutil_internal_test.go b/internal/api/tlsutil/tlsutil_internal_test.go new file mode 100644 index 00000000..045a8da0 --- /dev/null +++ b/internal/api/tlsutil/tlsutil_internal_test.go @@ -0,0 +1,37 @@ +// SPDX-License-Identifier: BUSL-1.1 + +package tlsutil + +import ( + "os" + "testing" +) + +// TestNewACMEManagerDefaultCache tests that an empty CacheDir falls back to the +// default cache directory. It lives in the internal test package so it can +// reference the unexported defaultACMECacheDir constant rather than hard-coding +// the literal path. +func TestNewACMEManagerDefaultCache(t *testing.T) { + // Skip if we can't create the default cache dir. + if err := os.MkdirAll(defaultACMECacheDir, 0o700); err != nil { + t.Skip("Cannot create default cache dir") + } + defer func() { _ = os.RemoveAll("certs") }() + + config := ACMEConfig{ + Enabled: true, + Domain: "test.example.com", + Email: "test@example.com", + CacheDir: "", // Use default + Staging: true, + } + + manager, err := NewACMEManager(config) + if err != nil { + t.Fatalf("NewACMEManager() error: %v", err) + } + + if manager == nil { + t.Fatal("Expected non-nil manager") + } +} diff --git a/internal/api/tlsutil/tlsutil_test.go b/internal/api/tlsutil/tlsutil_test.go new file mode 100644 index 00000000..3bfc08a5 --- /dev/null +++ b/internal/api/tlsutil/tlsutil_test.go @@ -0,0 +1,292 @@ +// SPDX-License-Identifier: BUSL-1.1 + +package tlsutil_test + +import ( + "crypto/tls" + "os" + "path/filepath" + "testing" + + "github.com/MustardSeedNetworks/stem/internal/api/tlsutil" +) + +// TestDefaultConfig tests the DefaultConfig function. +func TestDefaultConfig(t *testing.T) { + config := tlsutil.DefaultConfig() + + if !config.Enabled { + t.Error("Expected TLS to be enabled by default") + } + if config.CertFile != "" { + t.Errorf("Expected empty CertFile, got '%s'", config.CertFile) + } + if config.KeyFile != "" { + t.Errorf("Expected empty KeyFile, got '%s'", config.KeyFile) + } + if config.CertsDir != tlsutil.DefaultCertsDir { + t.Errorf("Expected CertsDir '%s', got '%s'", tlsutil.DefaultCertsDir, config.CertsDir) + } +} + +// TestServerConfig tests the ServerConfig function. +func TestServerConfig(t *testing.T) { + config := tlsutil.ServerConfig() + + if config == nil { + t.Fatal("Expected non-nil TLS config") + } + + if config.MinVersion != tls.VersionTLS13 { + t.Errorf("Expected TLS 1.3 min version, got %d", config.MinVersion) + } +} + +// TestEnsureSelfSignedCert tests the EnsureSelfSignedCert function. +func TestEnsureSelfSignedCert(t *testing.T) { + // Create a temporary directory for test certificates. + tempDir := t.TempDir() + + t.Run("generate new certificates", func(t *testing.T) { + certFile, keyFile, err := tlsutil.EnsureSelfSignedCert(tempDir) + if err != nil { + t.Fatalf("EnsureSelfSignedCert() error: %v", err) + } + + if certFile == "" { + t.Error("Expected non-empty certFile") + } + if keyFile == "" { + t.Error("Expected non-empty keyFile") + } + + // Verify files exist. + _, certStatErr := os.Stat(certFile) + if certStatErr != nil { + t.Errorf("Certificate file does not exist: %v", certStatErr) + } + _, keyStatErr := os.Stat(keyFile) + if keyStatErr != nil { + t.Errorf("Key file does not exist: %v", keyStatErr) + } + }) + + t.Run("use existing certificates", func(t *testing.T) { + // Should reuse existing certificates. + certFile, keyFile, err := tlsutil.EnsureSelfSignedCert(tempDir) + if err != nil { + t.Fatalf("EnsureSelfSignedCert() error: %v", err) + } + + if certFile == "" || keyFile == "" { + t.Error("Expected non-empty certificate paths") + } + }) + + t.Run("empty certs dir defaults to default", func(t *testing.T) { + // This would use the default certs dir. + // Skip if we don't want to pollute the filesystem. + t.Skip("Skipping to avoid creating files in default location") + }) +} + +// TestEnsureSelfSignedCertExistingFiles tests EnsureSelfSignedCert with existing files. +func TestEnsureSelfSignedCertExistingFiles(t *testing.T) { + tempDir := t.TempDir() + + // First call - generate. + certFile, keyFile, err := tlsutil.EnsureSelfSignedCert(tempDir) + if err != nil { + t.Fatalf("First call error: %v", err) + } + + // Get file info. + certInfo, _ := os.Stat(certFile) + keyInfo, _ := os.Stat(keyFile) + + // Second call - should reuse. + certFile2, keyFile2, err := tlsutil.EnsureSelfSignedCert(tempDir) + if err != nil { + t.Fatalf("Second call error: %v", err) + } + + if certFile != certFile2 || keyFile != keyFile2 { + t.Error("Expected same paths on second call") + } + + // Files should not have changed. + certInfo2, _ := os.Stat(certFile) + keyInfo2, _ := os.Stat(keyFile) + + if certInfo.ModTime() != certInfo2.ModTime() { + t.Error("Cert file should not have been regenerated") + } + if keyInfo.ModTime() != keyInfo2.ModTime() { + t.Error("Key file should not have been regenerated") + } +} + +// TestConfigStruct tests the Config struct. +func TestConfigStruct(t *testing.T) { + config := tlsutil.Config{ + Enabled: true, + CertFile: "/path/to/cert.pem", + KeyFile: "/path/to/key.pem", + CertsDir: "/path/to/certs", + } + + if !config.Enabled { + t.Error("Expected Enabled to be true") + } + if config.CertFile != "/path/to/cert.pem" { + t.Errorf("Unexpected CertFile: %s", config.CertFile) + } + if config.KeyFile != "/path/to/key.pem" { + t.Errorf("Unexpected KeyFile: %s", config.KeyFile) + } + if config.CertsDir != "/path/to/certs" { + t.Errorf("Unexpected CertsDir: %s", config.CertsDir) + } +} + +// TestACMEConfig tests the ACME configuration struct. +func TestACMEConfig(t *testing.T) { + tests := []struct { + name string + config tlsutil.ACMEConfig + valid bool + }{ + { + name: "valid config", + config: tlsutil.ACMEConfig{ + Enabled: true, + Domain: "stem.example.com", + Email: "admin@example.com", + CacheDir: "certs/acme", + Staging: false, + }, + valid: true, + }, + { + name: "staging mode", + config: tlsutil.ACMEConfig{ + Enabled: true, + Domain: "test.example.com", + Email: "test@example.com", + CacheDir: "", + Staging: true, + }, + valid: true, + }, + { + name: "disabled config", + config: tlsutil.ACMEConfig{ + Enabled: false, + }, + valid: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.config.Enabled && tt.config.Domain == "" { + t.Error("Expected domain to be required when ACME is enabled") + } + }) + } +} + +// TestNewACMEManager tests the ACME manager creation. +func TestNewACMEManager(t *testing.T) { + // Create temporary directory for cache + tmpDir := t.TempDir() + cacheDir := filepath.Join(tmpDir, "acme-cache") + + config := tlsutil.ACMEConfig{ + Enabled: true, + Domain: "test.example.com", + Email: "test@example.com", + CacheDir: cacheDir, + Staging: true, + } + + manager, err := tlsutil.NewACMEManager(config) + if err != nil { + t.Fatalf("tlsutil.NewACMEManager() error: %v", err) + } + + if manager == nil { + t.Fatal("Expected non-nil manager") + } + + // Verify cache directory was created + if _, statErr := os.Stat(cacheDir); os.IsNotExist(statErr) { + t.Error("Expected cache directory to be created") + } +} + +// TestACMETLSConfig tests TLS config creation with ACME. +func TestACMETLSConfig(t *testing.T) { + tmpDir := t.TempDir() + + config := tlsutil.ACMEConfig{ + Enabled: true, + Domain: "test.example.com", + Email: "test@example.com", + CacheDir: tmpDir, + Staging: true, + } + + manager, err := tlsutil.NewACMEManager(config) + if err != nil { + t.Fatalf("tlsutil.NewACMEManager() error: %v", err) + } + + tlsConfig := tlsutil.ACMETLSConfig(manager) + if tlsConfig == nil { + t.Fatal("Expected non-nil TLS config") + } + + // TLS 1.3 minimum should be set + if tlsConfig.MinVersion != tls.VersionTLS13 { + t.Errorf("Expected MinVersion TLS 1.3, got %x", tlsConfig.MinVersion) + } +} + +// TestConfigWithACME tests Config with ACME enabled. +func TestConfigWithACME(t *testing.T) { + config := tlsutil.Config{ + Enabled: true, + CertFile: "", + KeyFile: "", + CertsDir: "certs", + ACME: tlsutil.ACMEConfig{ + Enabled: true, + Domain: "stem.example.com", + Email: "admin@example.com", + CacheDir: "certs/acme", + Staging: false, + }, + } + + // When ACME is enabled, CertFile/KeyFile should be ignored + if !config.Enabled { + t.Error("Expected TLS to be enabled") + } + if config.CertFile != "" { + t.Errorf("CertFile = %q, want empty", config.CertFile) + } + if config.KeyFile != "" { + t.Errorf("KeyFile = %q, want empty", config.KeyFile) + } + if config.CertsDir != "certs" { + t.Errorf("CertsDir = %q, want %q", config.CertsDir, "certs") + } + if !config.ACME.Enabled { + t.Error("Expected ACME to be enabled") + } + + if config.ACME.Domain == "" { + t.Error("Expected domain to be set") + } +}