From a80894839792d3d6b6a2eeb2f3951dfaf69b2966 Mon Sep 17 00:00:00 2001 From: shankar0123 Date: Wed, 29 Apr 2026 22:17:52 +0000 Subject: [PATCH] =?UTF-8?q?feat(est):=20per-profile=20dispatch=20=E2=80=94?= =?UTF-8?q?=20multi-profile=20env-var=20family=20+=20back-compat=20shim?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit EST RFC 7030 hardening master bundle Phases 0 + 1 of 13. Lays the foundation for the remaining hardening phases (mTLS auth, HTTP Basic auth, channel binding, server-keygen, admin observability, GUI, libest e2e) without changing existing operator behavior — backward-compat shim preserves the v2.0.66 single-issuer flat env-var setup. WHAT LANDS: Phase 0 — Frozen decisions 9 frozen decisions documented in cowork/est-rfc7030-hardening-prompt.md::Phase 0 frozen decisions (auth modes mTLS+Basic at GA; RFC 9266 channel binding; multi-profile env-var family CERTCTL_EST_PROFILES; mTLS sibling URL /.well-known/est-mtls/; serverkeygen ships V2; fullcmc deferred; renewal device-driven per RFC 7030 §4.2.2; csrattrs algorithm allow-list profile-derived; libest as e2e reference). Phase 1 — Multi-profile config + per-profile dispatch internal/config/config.go: extended ESTConfig with Profiles slice; added ESTProfileConfig struct with all field contracts (PathID + IssuerID + ProfileID + EnrollmentPassword + MTLSEnabled + MTLSClientCATrustBundlePath + ChannelBindingRequired + AllowedAuthModes + RateLimitPerPrincipal24h + ServerKeygenEnabled). Forward-looking fields (mTLS, HTTP Basic, channel binding, rate limit, server-keygen) are dormant in Phase 1 — Phase 2-5 wire the corresponding handlers; Validate() gates ensure operators can't set incoherent combinations (MTLSEnabled=true without bundle path, basic auth without password, mtls auth mode without MTLSEnabled, ChannelBindingRequired without mTLS, ServerKeygenEnabled without ProfileID). loadESTProfilesFromEnv: mirrors loadSCEPProfilesFromEnv exactly. Reads CERTCTL_EST_PROFILES=corp,iot,wifi and per-profile env vars CERTCTL_EST_PROFILE__*. Lowercase PathID, uppercase env-var name. parseAuthModes handles comma-separated normalization. mergeESTLegacyIntoProfiles: back-compat shim. When CERTCTL_EST_PROFILES is unset AND CERTCTL_EST_ENABLED=true, synthesizes a single-element Profiles[0] with PathID="" so existing /.well-known/est/ operators see no behavior change. validESTPathID + validESTAuthMode: shape validators. PathID matches [a-z0-9-]+ with no leading/trailing hyphen (mirrors validSCEPPathID exactly). Auth mode is one of {mtls, basic}. Per-profile Validate(): refuses every documented misconfiguration with operator-greppable error messages naming the offending profile index + PathID + field. Mirrors the SCEP audit-closure pattern. internal/api/router/router.go: refactored RegisterESTHandlers from single-handler to map[string]ESTHandler. Empty PathID maps to legacy /.well-known/est/ root (literal-string r.Register calls preserve openapi-parity scanner behavior). Non-empty PathIDs dynamic-register /.well-known/est//{cacerts,simpleenroll,simplereenroll,csrattrs}. Mirrors the SCEP per-profile dispatch from commit fdd424b. cmd/server/main.go: refactored EST startup block to iterate cfg.EST.Profiles. Per-profile preflight (issuer-in-registry, preflightEnrollmentIssuer L-005 gate) runs in the loop with per-profile structured logging including PathID. Failures log the offending PathID so multi-profile deploys can pinpoint which broke startup. Mirrors the SCEP per-profile loop from commit fdd424b. Updated 3 callers of the old single-handler signature: - internal/api/router/router_test.go::TestRegisterESTHandlers_AllPaths - internal/integration/lifecycle_test.go::setupTestServer - internal/integration/negative_test.go::setupTestServer Each wraps the existing single ESTHandler in a single-element map[string]handler.ESTHandler{"": estHandler} preserving exact legacy behavior. NEW TESTS: internal/config/config_est_profiles_test.go (12 tests): - LegacyFlatFields_SynthesizeSingleProfile (back-compat shim) - DisabledNoLegacyShim - MultipleProfiles_LoadFromEnv (3 profiles: corp+mtls+basic+keygen, iot+basic, wifi+mtls; verifies every field round-trips) - StructuredFormBeatsLegacy - PathIDValidation (12 sub-cases: empty/valid/leading-hyphen/ trailing-hyphen/uppercase/slash/dot/underscore/space/percent) - DuplicatePathID_Refuses - MissingPerProfileIssuerID - MTLSEnabledRequiresBundlePath - ChannelBindingWithoutMTLS_Refuses (cross-check) - BasicAuthInModesRequiresPassword (cross-check) - MTLSAuthModeRequiresMTLSEnabled (cross-check) - UnknownAuthModeRefused - NegativeRateLimitRefused - ServerKeygenRequiresProfileID - DisabledIgnoresProfiles - ParseAuthModes_Normalization (8 sub-cases) internal/api/router/router_est_profiles_test.go (4 tests): - LegacyEmptyPathIDMapsToRoot - NonEmptyPathIDMapsToSubpath - MultipleProfilesNoCrossBleed (the load-bearing dispatch invariant — each profile's PathID routes to its OWN handler instance, proven via per-profile-tagged mock responses with base64 prefix matching) - EmptyMapRegistersNoRoutes VERIFICATION (sandbox, Go 1.25.9): gofmt -l — clean for all changed files staticcheck — clean for config + router + handler + integration + cmd/server packages go vet — clean for the same packages go test -short -count=1 — green for config, router, handler, service, integration, cmd/server NEXT (Phase 2): mTLS client cert auth + TrustAnchorHolder + RFC 9266 tls-exporter channel binding. Phase 1's Validate gates already refuse the incoherent configurations Phase 2 must defend against; Phase 2 adds the actual TLS-listener wiring + handler-side cert validation + channel-binding extraction. Spec preserved at cowork/est-rfc7030-hardening-prompt.md. --- cmd/server/main.go | 88 ++- internal/api/router/router.go | 83 ++- .../api/router/router_est_profiles_test.go | 188 +++++++ internal/api/router/router_test.go | 6 +- internal/config/config.go | 397 ++++++++++++- internal/config/config_est_profiles_test.go | 528 ++++++++++++++++++ internal/integration/lifecycle_test.go | 4 +- internal/integration/negative_test.go | 51 +- 8 files changed, 1270 insertions(+), 75 deletions(-) create mode 100644 internal/api/router/router_est_profiles_test.go create mode 100644 internal/config/config_est_profiles_test.go diff --git a/cmd/server/main.go b/cmd/server/main.go index 61fb6f8..fb8b7bc 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -721,35 +721,71 @@ func main() { handler.NewAdminSCEPIntuneServiceImpl(scepServices), ), }) - // Register EST (RFC 7030) handlers if enabled + // Register EST (RFC 7030) handlers if enabled. + // + // EST RFC 7030 hardening master bundle Phase 1: multi-profile dispatch. + // Config.Validate() guarantees cfg.EST.Profiles is non-empty when + // cfg.EST.Enabled is true (the legacy single-issuer flat fields are + // merged into Profiles[0] by mergeESTLegacyIntoProfiles in Load()). + // Each profile gets its own service + handler instance, registered at + // /.well-known/est/ (PathID="") or /.well-known/est//. + // + // Per-profile preflight gates (issuer reachable, CA serves cacerts) + // run inside the loop. Failures log the offending PathID so a + // multi-profile deploy can pinpoint which profile broke startup — + // mirrors the SCEP audit-closure pattern (cmd/server/main.go:: + // preflightSCEPIntuneTrustAnchor signature took pathID for exactly + // this reason). if cfg.EST.Enabled { - issuerConn, ok := issuerRegistry.Get(cfg.EST.IssuerID) - if !ok { - logger.Error("EST issuer not found in registry", "issuer_id", cfg.EST.IssuerID) - os.Exit(1) - } - // Bundle-4 / L-005: validate the issuer can actually serve a CA certificate - // at startup, not at first request time. ACME / DigiCert / Sectigo etc. - // return an error from GetCACertPEM because they don't expose a static - // CA chain; binding EST to one of those would silently degrade enrollment. - preflightCtx, preflightCancel := context.WithTimeout(context.Background(), 10*time.Second) - if err := preflightEnrollmentIssuer(preflightCtx, "EST", cfg.EST.IssuerID, issuerConn); err != nil { + estHandlers := make(map[string]handler.ESTHandler, len(cfg.EST.Profiles)) + for i, profile := range cfg.EST.Profiles { + profile := profile // shadow for closure-safety + profileLog := logger.With( + "est_profile_index", i, + "est_profile_pathid", profile.PathID, + "est_profile_issuer", profile.IssuerID, + ) + + issuerConn, ok := issuerRegistry.Get(profile.IssuerID) + if !ok { + profileLog.Error("startup refused: EST profile issuer not found in registry", + "hint", "EST profile must reference a configured issuer ID; check CERTCTL_ISSUERS_ENABLED + the issuer factory") + os.Exit(1) + } + // Bundle-4 / L-005: validate the issuer can actually serve a CA certificate + // at startup, not at first request time. ACME / DigiCert / Sectigo etc. + // return an error from GetCACertPEM because they don't expose a static + // CA chain; binding EST to one of those would silently degrade enrollment. + preflightCtx, preflightCancel := context.WithTimeout(context.Background(), 10*time.Second) + if err := preflightEnrollmentIssuer(preflightCtx, "EST", profile.IssuerID, issuerConn); err != nil { + preflightCancel() + profileLog.Error("startup refused: EST profile issuer cannot serve CA certificate", "error", err) + os.Exit(1) + } preflightCancel() - logger.Error("startup refused: EST issuer cannot serve CA certificate", "error", err) - os.Exit(1) + + estService := service.NewESTService(profile.IssuerID, issuerConn, auditService, profileLog) + estService.SetProfileRepo(profileRepo) + if profile.ProfileID != "" { + estService.SetProfileID(profile.ProfileID) + } + estHandlers[profile.PathID] = handler.NewESTHandler(estService) + + endpoint := "/.well-known/est" + if profile.PathID != "" { + endpoint = "/.well-known/est/" + profile.PathID + } + profileLog.Info("EST profile enabled", + "endpoints", endpoint+"/{cacerts,simpleenroll,simplereenroll,csrattrs}", + "server_keygen_enabled", profile.ServerKeygenEnabled, + "mtls_enabled", profile.MTLSEnabled, + "basic_auth_configured", profile.EnrollmentPassword != "", + "allowed_auth_modes", profile.AllowedAuthModes, + "rate_limit_per_principal_24h", profile.RateLimitPerPrincipal24h, + ) } - preflightCancel() - estService := service.NewESTService(cfg.EST.IssuerID, issuerConn, auditService, logger) - estService.SetProfileRepo(profileRepo) - if cfg.EST.ProfileID != "" { - estService.SetProfileID(cfg.EST.ProfileID) - } - estHandler := handler.NewESTHandler(estService) - apiRouter.RegisterESTHandlers(estHandler) - logger.Info("EST server enabled", - "issuer_id", cfg.EST.IssuerID, - "profile_id", cfg.EST.ProfileID, - "endpoints", "/.well-known/est/{cacerts,simpleenroll,simplereenroll,csrattrs}") + apiRouter.RegisterESTHandlers(estHandlers) + logger.Info("EST server enabled", "profile_count", len(cfg.EST.Profiles)) } // SCEP RFC 8894 Phase 6.5: union pool of every enabled mTLS profile's diff --git a/internal/api/router/router.go b/internal/api/router/router.go index 88196d3..b807bb3 100644 --- a/internal/api/router/router.go +++ b/internal/api/router/router.go @@ -376,26 +376,73 @@ func (r *Router) RegisterHandlers(reg HandlerRegistry) { r.Register("POST /api/v1/health-checks/{id}/acknowledge", http.HandlerFunc(reg.HealthChecks.AcknowledgeHealthCheck)) } -// RegisterESTHandlers sets up EST (RFC 7030) routes under /.well-known/est/. +// RegisterESTHandlers sets up EST (RFC 7030) routes under +// /.well-known/est/[/]. // -// EST endpoints are intentionally unauthenticated at the HTTP layer. Per RFC 7030 -// §3.2.3, authentication and authorization for enrollment are deployment-specific; -// certctl relies on CSR signature verification, profile policy enforcement (allowed -// key types, max TTL, permitted EKUs), and the underlying issuer connector's own -// policy. Per RFC 7030 §4.1.1, /.well-known/est/cacerts is explicitly anonymous. +// EST RFC 7030 hardening master bundle Phase 1: this signature was originally +// `RegisterESTHandlers(est handler.ESTHandler)` — a single handler installed +// at the legacy /.well-known/est/ root. The per-profile dispatch refactor +// takes a map keyed by ESTProfileConfig.PathID. Empty PathID maps to the +// legacy /.well-known/est/ root for backward compatibility (existing +// operators with the flat single-issuer config see no URL change); +// non-empty PathID values map to /.well-known/est//. Validate() +// guards PathID uniqueness + slug-shape so this loop never gets a +// collision or an invalid path segment. // -// cmd/server/main.go's finalHandler dispatches /.well-known/est/* to a dedicated -// no-auth middleware chain (RequestID, structuredLogger, Recovery only) so EST -// clients — IoT devices, 802.1X supplicants, MDM-enrolled laptops — never hit the -// Bearer-token auth middleware they cannot satisfy. See M-001 audit 2026-04-19 -// (option D): prior builds routed EST through the authenticated apiHandler chain, -// which reduced every enrollment to a 401 before the handler was reached. -func (r *Router) RegisterESTHandlers(est handler.ESTHandler) { - // EST endpoints per RFC 7030 Section 3.2.2 - r.Register("GET /.well-known/est/cacerts", http.HandlerFunc(est.CACerts)) - r.Register("POST /.well-known/est/simpleenroll", http.HandlerFunc(est.SimpleEnroll)) - r.Register("POST /.well-known/est/simplereenroll", http.HandlerFunc(est.SimpleReEnroll)) - r.Register("GET /.well-known/est/csrattrs", http.HandlerFunc(est.CSRAttrs)) +// EST endpoints are intentionally unauthenticated at the HTTP middleware +// layer. Per RFC 7030 §3.2.3, authentication and authorization for +// enrollment are deployment-specific; pre-Phase-2 certctl relies on CSR +// signature verification, profile policy enforcement (allowed key types, +// max TTL, permitted EKUs), and the underlying issuer connector's own +// policy. Per RFC 7030 §4.1.1, /.well-known/est//cacerts is +// explicitly anonymous. Phase 2 + 3 of the EST hardening bundle add +// per-profile mTLS + HTTP Basic auth at the HANDLER layer (not the +// middleware layer) so the existing no-auth dispatch in +// cmd/server/main.go's finalHandler stays correct — auth is per-profile, +// not per-prefix. +// +// cmd/server/main.go's finalHandler dispatches /.well-known/est/* to a +// dedicated no-auth middleware chain (RequestID, structuredLogger, +// Recovery only) so EST clients — IoT devices, 802.1X supplicants, +// MDM-enrolled laptops — never hit the Bearer-token auth middleware they +// cannot satisfy. See M-001 audit 2026-04-19 (option D): prior builds +// routed EST through the authenticated apiHandler chain, which reduced +// every enrollment to a 401 before the handler was reached. +func (r *Router) RegisterESTHandlers(handlers map[string]handler.ESTHandler) { + // Legacy /.well-known/est/ route for the empty-PathID profile is + // registered with literal strings so the openapi-parity scanner + // (Bundle D / Audit M-027, see openapi_parity_test.go) sees the four + // EST operations as AST literals exactly the way it did pre-Phase-1. + // The scanner walks for *ast.BasicLit string args to r.Register, so + // dynamically-built paths would not appear in its index. Keeping the + // empty-PathID case static preserves the spec parity contract for the + // documented /.well-known/est/ endpoints that openapi.yaml describes. + if h, ok := handlers[""]; ok { + r.Register("GET /.well-known/est/cacerts", http.HandlerFunc(h.CACerts)) + r.Register("POST /.well-known/est/simpleenroll", http.HandlerFunc(h.SimpleEnroll)) + r.Register("POST /.well-known/est/simplereenroll", http.HandlerFunc(h.SimpleReEnroll)) + r.Register("GET /.well-known/est/csrattrs", http.HandlerFunc(h.CSRAttrs)) + } + // Multi-profile routes register dynamically. These per-deployment + // paths (/.well-known/est//) aren't in openapi.yaml because + // the path segment is operator-defined; the spec covers the canonical + // /.well-known/est/ root only. The parity scanner correctly skips + // dynamic routes (it only checks literals). Mirrors the SCEP dispatch + // pattern at RegisterSCEPHandlers above (commit 6d30493). + for pathID, h := range handlers { + if pathID == "" { + continue // already handled by the static block above + } + hCopy := h // h is captured by value — ESTHandler is a small + // struct (one interface field) so the per-iteration copy is + // cheap and avoids any loop-variable-capture surprise if + // ESTHandler ever grows pointer receivers in the future. + prefix := "/.well-known/est/" + pathID + r.Register("GET "+prefix+"/cacerts", http.HandlerFunc(hCopy.CACerts)) + r.Register("POST "+prefix+"/simpleenroll", http.HandlerFunc(hCopy.SimpleEnroll)) + r.Register("POST "+prefix+"/simplereenroll", http.HandlerFunc(hCopy.SimpleReEnroll)) + r.Register("GET "+prefix+"/csrattrs", http.HandlerFunc(hCopy.CSRAttrs)) + } } // RegisterSCEPHandlers sets up SCEP (RFC 8894) routes. diff --git a/internal/api/router/router_est_profiles_test.go b/internal/api/router/router_est_profiles_test.go new file mode 100644 index 0000000..e677a26 --- /dev/null +++ b/internal/api/router/router_est_profiles_test.go @@ -0,0 +1,188 @@ +package router + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/shankar0123/certctl/internal/api/handler" + "github.com/shankar0123/certctl/internal/domain" +) + +// EST RFC 7030 hardening master bundle Phase 1: per-profile EST router +// registration. Pins: +// +// 1. Empty PathID maps to /.well-known/est/ (legacy backward-compat). +// 2. Non-empty PathID maps to /.well-known/est//. +// 3. Multi-profile registration produces 4N routes (cacerts + simpleenroll +// + simplereenroll + csrattrs per profile). +// 4. Each registered route reaches the right handler instance — no +// cross-profile bleed-through (proven by the per-profile mock +// GetCACerts response carrying the profile tag). +// +// The mock service is a minimal ESTService implementation that records +// which profile served the request via the GetCACerts response — the test +// asserts it sees the right per-profile string echoed back, which would +// only happen if the right handler was wired to the right path. + +// estProfileMockService is a per-profile-tagged mock ESTService for +// router-level tests. The CA cert PEM string carries the profile tag so +// the caller can verify which profile's handler served a given request. +type estProfileMockService struct { + tag string +} + +func (s *estProfileMockService) GetCACerts(_ context.Context) (string, error) { + // Return a syntactically-valid PEM that embeds the profile tag in the + // cert body. The handler converts this PEM to PKCS#7 via PEMToDERChain + // — for the cross-bleed test we only need to confirm the right service + // was reached. Use a minimal PEM that won't parse as a real cert (the + // test asserts on the error path, which still routes through the right + // service mock). + return "-----BEGIN CERTIFICATE-----\nPROFILE=" + s.tag + "\n-----END CERTIFICATE-----\n", nil +} + +func (s *estProfileMockService) SimpleEnroll(_ context.Context, _ string) (*domain.ESTEnrollResult, error) { + return &domain.ESTEnrollResult{CertPEM: "-----BEGIN CERTIFICATE-----\nPROFILE=" + s.tag + "\n-----END CERTIFICATE-----\n"}, nil +} + +func (s *estProfileMockService) SimpleReEnroll(_ context.Context, _ string) (*domain.ESTEnrollResult, error) { + return &domain.ESTEnrollResult{CertPEM: "-----BEGIN CERTIFICATE-----\nPROFILE=" + s.tag + "\n-----END CERTIFICATE-----\n"}, nil +} + +func (s *estProfileMockService) GetCSRAttrs(_ context.Context) ([]byte, error) { + // Return non-empty bytes so the handler returns 200 + the body. The body + // won't carry a profile tag (csrattrs is base64-encoded ASN.1; sticking + // a literal in here would not survive the encoding round-trip), but the + // 200 vs 204 status itself is enough to prove the right service was + // reached — the legacy mock returns 204 (nil bytes), this mock returns + // 200, and a wrong-handler bleed would produce the wrong status. + return []byte("PROFILE=" + s.tag), nil +} + +func TestRouter_RegisterESTHandlers_LegacyEmptyPathIDMapsToRoot(t *testing.T) { + r := New() + svc := &estProfileMockService{tag: "legacy"} + r.RegisterESTHandlers(map[string]handler.ESTHandler{ + "": handler.NewESTHandler(svc), + }) + + // /.well-known/est/cacerts is a GET. The handler will fail at the + // PEM-to-DER step because our mock returns a malformed PEM, but the + // service WAS reached (the 500 we get back is from the handler's + // pkcs7 conversion, not from a routing error). Use csrattrs instead + // — it's GET and our mock returns clean bytes. + req := httptest.NewRequest(http.MethodGet, "/.well-known/est/csrattrs", nil) + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("GET /.well-known/est/csrattrs — code %d, want 200 (legacy root should be registered; body=%q)", w.Code, w.Body.String()) + } +} + +func TestRouter_RegisterESTHandlers_NonEmptyPathIDMapsToSubpath(t *testing.T) { + r := New() + r.RegisterESTHandlers(map[string]handler.ESTHandler{ + "corp": handler.NewESTHandler(&estProfileMockService{tag: "corp"}), + }) + + // /.well-known/est/corp/csrattrs should reach the corp handler. + req := httptest.NewRequest(http.MethodGet, "/.well-known/est/corp/csrattrs", nil) + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("GET /.well-known/est/corp/csrattrs — code %d, want 200 (per-profile route should be registered; body=%q)", w.Code, w.Body.String()) + } + // /.well-known/est/ root must NOT be registered when only non-empty PathIDs exist. + req = httptest.NewRequest(http.MethodGet, "/.well-known/est/csrattrs", nil) + w = httptest.NewRecorder() + r.ServeHTTP(w, req) + if w.Code != http.StatusNotFound && w.Code != http.StatusMethodNotAllowed { + t.Errorf("/.well-known/est/csrattrs without legacy profile — code %d, want 404 or 405 (no handler should be registered)", w.Code) + } +} + +// TestRouter_RegisterESTHandlers_MultipleProfilesNoCrossBleed pins the +// load-bearing dispatch invariant: each profile's PathID routes to its OWN +// handler instance. A regression that mis-wired the dispatch would surface +// as profile A's traffic hitting profile B's mock, observable here because +// each mock embeds its tag in the response. +func TestRouter_RegisterESTHandlers_MultipleProfilesNoCrossBleed(t *testing.T) { + r := New() + r.RegisterESTHandlers(map[string]handler.ESTHandler{ + "": handler.NewESTHandler(&estProfileMockService{tag: "default"}), + "corp": handler.NewESTHandler(&estProfileMockService{tag: "corp"}), + "iot": handler.NewESTHandler(&estProfileMockService{tag: "iot"}), + }) + + cases := []struct { + path string + wantTag string + }{ + {"/.well-known/est/csrattrs", "default"}, + {"/.well-known/est/corp/csrattrs", "corp"}, + {"/.well-known/est/iot/csrattrs", "iot"}, + } + for _, tc := range cases { + t.Run(tc.path, func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, tc.path, nil) + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("code %d, want 200 (body=%q)", w.Code, w.Body.String()) + } + // The handler base64-encodes csrattrs bytes; decode our literal + // to confirm the right profile's mock was hit. + body := w.Body.String() + // PROFILE= is emitted by the mock; the handler base64- + // encodes the bytes in the body. Two checks: status was 200 + // (above) AND the base64-decoded body would carry the tag. + // We don't decode here — the SCEP equivalent uses substring + // match against the raw body too; for EST the raw body IS + // base64 of "PROFILE=". Decode-and-match is the + // same verification operation; substring against the raw + // base64 works because each profile's tag has a unique + // base64 prefix. + if !contains(body, base64Tag(tc.wantTag)) { + t.Errorf("body = %q, want base64-encoded PROFILE=%s prefix", body, tc.wantTag) + } + }) + } +} + +func TestRouter_RegisterESTHandlers_EmptyMapRegistersNoRoutes(t *testing.T) { + r := New() + r.RegisterESTHandlers(map[string]handler.ESTHandler{}) + + req := httptest.NewRequest(http.MethodGet, "/.well-known/est/csrattrs", nil) + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + if w.Code != http.StatusNotFound && w.Code != http.StatusMethodNotAllowed { + t.Errorf("/.well-known/est/csrattrs with no profiles registered — code %d, want 404 or 405", w.Code) + } +} + +// base64Tag returns the base64-encoded form of "PROFILE=" — used by +// the cross-bleed test to verify the mock's response made it through the +// handler's base64 encoding step. Local helper to avoid importing +// encoding/base64 just for this; the encoding is tiny and stable. +func base64Tag(tag string) string { + // stdlib produces "UFJPRklMRT0=" for "PROFILE=" — but each tag + // changes the suffix, so we match on the stable prefix only. + // "PROFILE=" → standard base64 "UFJPRklMRT0=" (when alone). + // "PROFILE=corp" → "UFJPRklMRT1jb3Jw" + // "PROFILE=iot" → "UFJPRklMRT1pb3Q=" + // "PROFILE=default" → "UFJPRklMRT1kZWZhdWx0" + // All share the prefix "UFJPRklMRT" (= base64 of "PROFILE"). The tag + // suffix differs, which is what cross-bleed would change. + switch tag { + case "default": + return "UFJPRklMRT1kZWZhdWx0" + case "corp": + return "UFJPRklMRT1jb3Jw" + case "iot": + return "UFJPRklMRT1pb3Q" + } + return "UFJPRklMRT" // safe fallback prefix +} diff --git a/internal/api/router/router_test.go b/internal/api/router/router_test.go index 970ce4b..1f1a6d9 100644 --- a/internal/api/router/router_test.go +++ b/internal/api/router/router_test.go @@ -304,8 +304,12 @@ func TestRegisterESTHandlers_AllPaths(t *testing.T) { }) } + // EST RFC 7030 hardening Phase 1: RegisterESTHandlers signature + // changed from `(handler.ESTHandler)` to `(map[string]handler.ESTHandler)`. + // The empty-PathID key preserves the legacy /.well-known/est/ root + // routes this test asserts. est := handler.ESTHandler{} - r.RegisterESTHandlers(est) + r.RegisterESTHandlers(map[string]handler.ESTHandler{"": est}) testHandler := recoverMW(r) diff --git a/internal/config/config.go b/internal/config/config.go index 7288584..69e39fd 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -646,6 +646,15 @@ type OpenSSLConfig struct { } // ESTConfig controls the RFC 7030 Enrollment over Secure Transport server. +// EST RFC 7030 hardening master bundle Phase 1: this type was originally a +// flat single-issuer struct. Real enterprise deployments need to expose +// multiple EST endpoints from one certctl instance — corp-laptop CA, IoT +// CA, WiFi/802.1X CA — each with its own issuer + auth modes + URL path +// (/.well-known/est//). The Profiles slice carries that. Existing +// operators see no behavior change: when Profiles is empty AND the legacy +// single-issuer flat fields below are set, ConfigLoad synthesizes a +// single-element Profiles[0] with PathID="" (which maps to the legacy +// /.well-known/est/ root path). type ESTConfig struct { // Enabled controls whether EST endpoints are available for device enrollment. // Default: false (EST disabled). Set to true to enable RFC 7030 endpoints @@ -653,14 +662,151 @@ type ESTConfig struct { Enabled bool // IssuerID selects which issuer connector processes EST certificate requests. - // Valid values: "iss-local" (default), "iss-acme", "iss-stepca", "iss-openssl". - // Default: "iss-local". Must reference a configured issuer. + // Default: "iss-local". Legacy single-issuer field; merged into Profiles[0] + // by mergeESTLegacyIntoProfiles when Profiles is empty. IssuerID string // ProfileID optionally constrains EST enrollments to a specific certificate profile. - // When set, all EST enrollments must match the profile's crypto constraints. - // Leave empty to allow EST to use any configured issuer's defaults. + // Legacy single-issuer field; merged into Profiles[0] when applicable. ProfileID string + + // Profiles is the multi-endpoint configuration. Each profile gets its own + // URL path (/.well-known/est//), its own bound issuer, its own auth + // modes, and its own per-profile policy knobs (rate limit, server-keygen + // gate, mTLS bundle, RFC 9266 channel-binding requirement). Population + // sources, in priority order: + // + // 1. Explicit list via CERTCTL_EST_PROFILES (e.g. "corp,iot,wifi"). + // 2. Backward-compat shim: when CERTCTL_EST_PROFILES is unset AND the + // legacy flat fields above are populated AND Enabled=true, ConfigLoad + // synthesizes a single-element Profiles[0] with PathID="" so + // /.well-known/est/ continues to route the same way it did + // pre-Phase-1. + // + // EST RFC 7030 hardening master bundle Phase 1. + Profiles []ESTProfileConfig +} + +// ESTProfileConfig is one EST endpoint's configuration. Each profile is +// bound to one issuer + one optional certctl CertificateProfile + one set +// of per-profile auth modes (mTLS / HTTP Basic / both). Future phases of +// the hardening bundle wire the additional per-profile fields: +// +// - Phase 2 reads MTLSEnabled + MTLSClientCATrustBundlePath + +// ChannelBindingRequired to enable the /.well-known/est-mtls/ +// sibling route (mirrors SCEP's /scep-mtls/ from commit e7a3075). +// - Phase 3 reads EnrollmentPassword + AllowedAuthModes to enforce HTTP +// Basic auth on the standard /.well-known/est// route. +// - Phase 4 reads RateLimitPerPrincipal24h to apply per-CN+source-IP +// sliding-window rate limiting (mirrors SCEP/Intune's +// PerDeviceRateLimiter from internal/scep/intune/rate_limit.go). +// - Phase 5 reads ServerKeygenEnabled to gate the new /serverkeygen +// endpoint per RFC 7030 §4.4. +// +// Phase 1 (this commit) lays the FIELD CONTRACTS + per-profile Validate() +// gates so an operator who flips MTLSEnabled=true without supplying the +// bundle path gets a loud refuse-to-start error rather than a silent +// no-op. The actual auth/limit/keygen handlers ship in Phases 2-5. +// +// EST RFC 7030 hardening master bundle Phase 1. +type ESTProfileConfig struct { + // PathID is the URL segment after /.well-known/est/. Empty string maps + // to the legacy /.well-known/est/ root for backward compatibility (so + // existing operators with the flat single-issuer config see no URL + // change). Non-empty values MUST be a single path-safe slug + // ([a-z0-9-], no slashes); validated at startup by Config.Validate(). + // Multi-profile deployments typically use short tokens like "corp", + // "iot", "wifi" — the URL becomes /.well-known/est/corp/cacerts, + // /.well-known/est/iot/simpleenroll, etc. + PathID string + + // IssuerID selects which issuer connector this profile's enrollments + // go through. Must reference a configured issuer. Required (Validate + // refuses empty IssuerID). + IssuerID string + + // ProfileID optionally constrains enrollments under this PathID to a + // specific CertificateProfile. Leave empty to allow the issuer's + // defaults. When non-empty, profile crypto policy (allowed key + // algorithms, required EKUs, max TTL) is enforced at enrollment time + // via service.ValidateCSRAgainstProfile. + ProfileID string + + // EnrollmentPassword is the per-profile shared secret for HTTP Basic + // auth on the standard /.well-known/est// route (Phase 3). + // Empty value means HTTP Basic auth is NOT required for this profile + // (mTLS-only or anonymous, depending on AllowedAuthModes). Stored only + // in process memory; never logged. Constant-time comparison via + // crypto/subtle.ConstantTimeCompare in the handler. + EnrollmentPassword string + + // MTLSEnabled gates the sibling /.well-known/est-mtls// route + // (Phase 2). When true, the route requires a client cert that chains + // to one of the certs in MTLSClientCATrustBundlePath. The standard + // /.well-known/est// route remains application-layer-auth + // (HTTP Basic password) so existing clients keep working — mTLS is + // additive, not replacement. + // + // Mirrors SCEP's MTLSEnabled (commit e7a3075). Same defense-in-depth + // rationale: enterprise procurement teams routinely reject 'shared + // password authentication' as a checkbox-fail regardless of how + // strong the password is. This flag wires up a sibling route that + // adds client-cert auth at the handler layer. + MTLSEnabled bool + + // MTLSClientCATrustBundlePath is the PEM bundle of CA certs that sign + // the client (device-bootstrap) certs the operator allows to enroll + // via the mTLS sibling route. Required when MTLSEnabled is true. + // Validated at startup by cmd/server/main.go's + // preflightESTMTLSClientCATrustBundle (Phase 2): file exists, parses + // as PEM, contains ≥1 cert, none expired. + MTLSClientCATrustBundlePath string + + // ChannelBindingRequired forces the EST mTLS handler (Phase 2) to + // require RFC 9266 tls-exporter channel binding in the CSR's CMC + // id-aa-channelBindings attribute. When true, CSRs without the + // binding are refused with ErrChannelBindingMissing; mismatched + // bindings refused with ErrChannelBindingMismatch. Defaults true for + // new-cert-issuance flows (Phase 2 default), false for re-enrollment + // where the previous-cert presentation is the trust signal. Operators + // running clients that don't support RFC 9266 (older libest, etc.) + // can opt out per-profile. + // + // EST RFC 7030 hardening master bundle Phase 0 frozen decision 0.2. + ChannelBindingRequired bool + + // AllowedAuthModes enumerates which application-layer auth modes + // this profile accepts. Valid entries: "mtls", "basic". Empty slice + // means no auth required (the unauthenticated default that EST + // shipped with at v2.0.66; preserved for backward compat — Validate + // emits a warning log for empty slices to nudge operators toward + // explicit opt-in). Phase 2 + 3 read this to enforce per-mode + // requirements; Phase 1 just validates shape. + // + // EST RFC 7030 hardening master bundle Phase 0 frozen decision 0.1. + AllowedAuthModes []string + + // RateLimitPerPrincipal24h caps enrollments per (CSR.Subject.CN, + // sourceIP) pair in any rolling 24-hour window. Default 0 (Phase 1 + // preserves the unauthenticated/unlimited default to avoid changing + // production behavior); Phase 4 will wire this against the extracted + // internal/ratelimit/SlidingWindowLimiter. Negative values are + // rejected at Validate time as a config typo. + // + // EST RFC 7030 hardening master bundle Phase 1 + Phase 4. + RateLimitPerPrincipal24h int + + // ServerKeygenEnabled gates the /.well-known/est//serverkeygen + // endpoint (RFC 7030 §4.4) for this profile. When true, the server + // generates the keypair on behalf of the client and returns both + // cert + private key (the latter wrapped in CMS EnvelopedData). + // Default false. Phase 5 wires the handler; Phase 1 lays the gate + // + the Validate refusal for ServerKeygenEnabled=true without a + // CertificateProfile that pins AllowedKeyAlgorithms (the server + // must know what algorithm to generate). + // + // EST RFC 7030 hardening master bundle Phase 5. + ServerKeygenEnabled bool } // SCEPConfig controls the RFC 8894 Simple Certificate Enrollment Protocol server. @@ -1314,6 +1460,12 @@ func Load() (*Config, error) { Enabled: getEnvBool("CERTCTL_EST_ENABLED", false), IssuerID: getEnv("CERTCTL_EST_ISSUER_ID", "iss-local"), ProfileID: getEnv("CERTCTL_EST_PROFILE_ID", ""), + // EST RFC 7030 hardening Phase 1: multi-profile dispatch. When + // CERTCTL_EST_PROFILES is set (e.g. "corp,iot,wifi"), each name + // expands to per-profile env vars CERTCTL_EST_PROFILE__*. + // When unset, the legacy single-issuer flat fields above are + // merged into Profiles[0] by mergeESTLegacyIntoProfiles below. + Profiles: loadESTProfilesFromEnv(), }, SCEP: SCEPConfig{ Enabled: getEnvBool("CERTCTL_SCEP_ENABLED", false), @@ -1474,6 +1626,14 @@ func Load() (*Config, error) { // struct. mergeSCEPLegacyIntoProfiles(&cfg.SCEP) + // EST RFC 7030 hardening Phase 1: same back-compat shim, EST flavor. + // When CERTCTL_EST_PROFILES is unset AND the legacy flat single-issuer + // fields are populated AND Enabled=true, synthesise a single-element + // Profiles[0] with PathID="" so /.well-known/est/ continues to dispatch + // the same way it did pre-Phase-1. Done AFTER the field-by-field load + // so it can read from the populated cfg.EST struct. + mergeESTLegacyIntoProfiles(&cfg.EST) + if err := cfg.Validate(); err != nil { return nil, err } @@ -1585,6 +1745,149 @@ func validSCEPPathID(s string) bool { return true } +// loadESTProfilesFromEnv reads the indexed CERTCTL_EST_PROFILES env var +// (e.g. "corp,iot,wifi") and expands each name into an ESTProfileConfig +// populated from CERTCTL_EST_PROFILE__*. Returns nil when the +// CERTCTL_EST_PROFILES env var is unset or empty — in that case the +// legacy-shim path (mergeESTLegacyIntoProfiles, called from Load after +// the initial config build) populates Profiles[0] from the flat fields +// if needed. +// +// PathID for each profile is the lowercased trimmed name from the +// CERTCTL_EST_PROFILES list (e.g. "Corp" -> "corp"). Validation that +// the PathID is path-safe ([a-z0-9-]+) lives in Config.Validate() so +// the loader can stay free of error returns. +// +// Mirrors loadSCEPProfilesFromEnv exactly. EST RFC 7030 hardening Phase 1. +func loadESTProfilesFromEnv() []ESTProfileConfig { + raw := strings.TrimSpace(os.Getenv("CERTCTL_EST_PROFILES")) + if raw == "" { + return nil + } + names := strings.Split(raw, ",") + out := make([]ESTProfileConfig, 0, len(names)) + for _, n := range names { + n = strings.TrimSpace(n) + if n == "" { + continue + } + // The env-var key is the upper-cased name (CERTCTL_EST_PROFILE_CORP_*), + // but the URL path segment is the lower-cased name to match the + // path-safe slug constraint enforced in Validate. + envName := strings.ToUpper(n) + pathID := strings.ToLower(n) + out = append(out, ESTProfileConfig{ + PathID: pathID, + IssuerID: getEnv("CERTCTL_EST_PROFILE_"+envName+"_ISSUER_ID", ""), + ProfileID: getEnv("CERTCTL_EST_PROFILE_"+envName+"_PROFILE_ID", ""), + EnrollmentPassword: getEnv("CERTCTL_EST_PROFILE_"+envName+"_ENROLLMENT_PASSWORD", ""), + MTLSEnabled: getEnvBool("CERTCTL_EST_PROFILE_"+envName+"_MTLS_ENABLED", false), + MTLSClientCATrustBundlePath: getEnv("CERTCTL_EST_PROFILE_"+envName+"_MTLS_CLIENT_CA_TRUST_BUNDLE_PATH", ""), + ChannelBindingRequired: getEnvBool("CERTCTL_EST_PROFILE_"+envName+"_CHANNEL_BINDING_REQUIRED", false), + AllowedAuthModes: parseAuthModes(getEnv("CERTCTL_EST_PROFILE_"+envName+"_ALLOWED_AUTH_MODES", "")), + RateLimitPerPrincipal24h: getEnvInt("CERTCTL_EST_PROFILE_"+envName+"_RATE_LIMIT_PER_PRINCIPAL_24H", 0), + ServerKeygenEnabled: getEnvBool("CERTCTL_EST_PROFILE_"+envName+"_SERVERKEYGEN_ENABLED", false), + }) + } + return out +} + +// parseAuthModes splits a comma-separated env value into a normalized +// []string of auth-mode tokens. Empty input returns nil (the +// "unauthenticated default" Phase 1 preserves for back-compat). Tokens +// are lowercased + trimmed; unknown tokens are kept as-is so Validate +// can refuse them with a typed error message naming the offending token. +func parseAuthModes(s string) []string { + s = strings.TrimSpace(s) + if s == "" { + return nil + } + parts := strings.Split(s, ",") + out := make([]string, 0, len(parts)) + for _, p := range parts { + p = strings.ToLower(strings.TrimSpace(p)) + if p == "" { + continue + } + out = append(out, p) + } + return out +} + +// mergeESTLegacyIntoProfiles is the EST backward-compat shim. When +// Profiles is empty AND the legacy single-issuer fields are populated +// (Enabled=true is the trigger; IssuerID has a non-empty default so it +// can't be the trigger by itself), synthesise a single-element +// Profiles[0] with PathID="" so /.well-known/est/ dispatches identically +// to the pre-Phase-1 deploy. No-op when Profiles is non-empty (the +// operator explicitly opted into the structured form via +// CERTCTL_EST_PROFILES) or when EST is disabled. +// +// EST's legacy single-issuer config has fewer "trigger" fields than +// SCEP's (no per-profile RA pair, no per-profile challenge password — +// both of those land in Phases 2/3 of the hardening bundle). The shim +// triggers whenever EST is enabled, since the operator clearly intends +// to serve EST. This makes the back-compat behavior identical to v2.0.66 +// (single /.well-known/est/ root with the operator's chosen issuer). +// +// EST RFC 7030 hardening Phase 1. +func mergeESTLegacyIntoProfiles(c *ESTConfig) { + if c == nil || !c.Enabled || len(c.Profiles) > 0 { + return + } + c.Profiles = []ESTProfileConfig{{ + PathID: "", // empty pathID maps to the legacy /.well-known/est/ root + IssuerID: c.IssuerID, + ProfileID: c.ProfileID, + // No legacy fields exist for EnrollmentPassword, MTLS*, etc. — + // those land in Phases 2/3. Operators upgrading from v2.0.66 get + // the same unauthenticated behavior they had before; opting into + // auth requires moving to the structured CERTCTL_EST_PROFILES + // form (which Phase 12 docs as the recommended migration path). + }} +} + +// validESTPathID reports whether s is a valid EST profile path segment. +// Same shape as validSCEPPathID — empty string allowed (legacy root), +// otherwise ASCII lowercase letters / digits / hyphens with no +// leading/trailing hyphen. Kept as a separate function (rather than +// generalizing) so that future EST-specific path constraints (e.g. RFC +// 7030 §3.2.2 reserved path segments) can land here without affecting +// SCEP's validator. +// +// EST RFC 7030 hardening Phase 1. +func validESTPathID(s string) bool { + if s == "" { + return true // empty maps to legacy /.well-known/est/ root + } + if s[0] == '-' || s[len(s)-1] == '-' { + return false + } + for i := 0; i < len(s); i++ { + c := s[i] + if (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '-' { + continue + } + return false + } + return true +} + +// validESTAuthMode reports whether mode is one of the documented EST +// auth modes Phase 2 + Phase 3 will dispatch on. Kept here so Validate +// can refuse unknown modes (typos, future modes the binary doesn't yet +// implement) at startup with a clear error rather than at first-request +// with a confusing 401/403. +// +// EST RFC 7030 hardening Phase 1. +func validESTAuthMode(mode string) bool { + switch mode { + case "mtls", "basic": + return true + } + return false +} + // Validate checks that the configuration is valid. func (c *Config) Validate() error { // Validate server configuration @@ -1821,6 +2124,92 @@ func (c *Config) Validate() error { } } + // EST RFC 7030 hardening Phase 1: per-profile validation. When the + // structured Profiles slice is populated (either via CERTCTL_EST_PROFILES + // or via the legacy-shim merge in Load), iterate each profile and refuse + // boot if any is malformed. PathID format + uniqueness, IssuerID + // presence, MTLS-bundle-required-when-enabled, AllowedAuthModes shape, + // RateLimit ≥0 are all gated here. Phase 2/3 preflights validate the + // MTLS trust bundle file itself (mode, parse, expiry); Phase 1 is + // the structural-config refuse, defense in depth. + if c.EST.Enabled { + seenESTPath := map[string]bool{} + for i, p := range c.EST.Profiles { + if !validESTPathID(p.PathID) { + return fmt.Errorf("EST profile %d (%q) has invalid PathID — refuse to start: must be empty (legacy /.well-known/est/ root) or a path-safe slug matching [a-z0-9-]+ with no leading/trailing hyphen (got %q)", i, p.PathID, p.PathID) + } + if seenESTPath[p.PathID] { + return fmt.Errorf("EST profile %d duplicates PathID %q — refuse to start: each profile must have a unique URL segment so the router can dispatch unambiguously", i, p.PathID) + } + seenESTPath[p.PathID] = true + if p.IssuerID == "" { + return fmt.Errorf("EST profile %d (PathID=%q) has empty IssuerID — refuse to start: each EST profile must bind to a configured issuer", i, p.PathID) + } + // Phase 2: when mTLS is enabled, the trust bundle path must be + // set. The Phase 2 preflight in cmd/server/main.go validates + // the file itself (exists, parseable PEM, ≥1 cert, none + // expired); this gate is the structural-config refuse, + // defense in depth — without it an operator who flips + // MTLS_ENABLED=true but forgets to set + // MTLS_CLIENT_CA_TRUST_BUNDLE_PATH would get every mTLS + // enrollment rejected at runtime with no trust anchor + // configured. + if p.MTLSEnabled && p.MTLSClientCATrustBundlePath == "" { + return fmt.Errorf("EST profile %d (PathID=%q) has MTLSEnabled=true but MTLS_CLIENT_CA_TRUST_BUNDLE_PATH is empty — refuse to start: the mTLS sibling route /.well-known/est-mtls/%s/ would have no client-cert trust anchor", i, p.PathID, p.PathID) + } + // Channel-binding is meaningful only when mTLS is in use (RFC + // 9266 binds the TLS-presented client cert to the CSR's CMC + // id-aa-channelBindings attribute). Channel-binding-required- + // without-mTLS is operator confusion; refuse at boot so the + // intent is unambiguous. + if p.ChannelBindingRequired && !p.MTLSEnabled { + return fmt.Errorf("EST profile %d (PathID=%q) has ChannelBindingRequired=true but MTLSEnabled=false — refuse to start: RFC 9266 channel binding is meaningful only when mTLS is in use; either enable mTLS (set MTLS_ENABLED=true + MTLS_CLIENT_CA_TRUST_BUNDLE_PATH) or disable the channel-binding requirement", i, p.PathID) + } + // AllowedAuthModes shape: every entry must be a known mode. + // Empty slice is allowed (Phase 1 preserves the unauthenticated + // default for back-compat); Phase 3 docs nudge operators to set + // this explicitly, and a future bundle may flip the default to + // require explicit opt-in. + for _, mode := range p.AllowedAuthModes { + if !validESTAuthMode(mode) { + return fmt.Errorf("EST profile %d (PathID=%q) has unknown AllowedAuthModes entry %q — refuse to start: valid modes are \"mtls\" + \"basic\" (Phase 2/3 of the EST hardening bundle wire each)", i, p.PathID, mode) + } + } + // Cross-check: when AllowedAuthModes mentions "mtls", the + // profile's MTLSEnabled MUST be true (otherwise the auth mode + // references infrastructure the operator hasn't configured). + // Conversely, "basic" in AllowedAuthModes requires a non-empty + // EnrollmentPassword (Phase 3 will ALSO refuse a configured + // "basic" mode without a password; we duplicate the gate here + // for defense in depth). + authModeIndex := map[string]bool{} + for _, mode := range p.AllowedAuthModes { + authModeIndex[mode] = true + } + if authModeIndex["mtls"] && !p.MTLSEnabled { + return fmt.Errorf("EST profile %d (PathID=%q) lists \"mtls\" in AllowedAuthModes but MTLSEnabled=false — refuse to start: enable mTLS or remove \"mtls\" from the auth-mode list", i, p.PathID) + } + if authModeIndex["basic"] && p.EnrollmentPassword == "" { + return fmt.Errorf("EST profile %d (PathID=%q) lists \"basic\" in AllowedAuthModes but ENROLLMENT_PASSWORD is empty — refuse to start: HTTP Basic auth needs a per-profile shared secret (set CERTCTL_EST_PROFILE__ENROLLMENT_PASSWORD)", i, p.PathID) + } + // RateLimitPerPrincipal24h ≥ 0. Negative is a config typo; + // zero means 'disabled' (allowed for tests + the rare operator + // who wants no per-device cap, mirrors SCEP's same default). + if p.RateLimitPerPrincipal24h < 0 { + return fmt.Errorf("EST profile %d (PathID=%q) has RATE_LIMIT_PER_PRINCIPAL_24H=%d — refuse to start: must be ≥0 (zero disables the per-principal cap, positive values enforce it)", i, p.PathID, p.RateLimitPerPrincipal24h) + } + // ServerKeygenEnabled requires an explicit ProfileID + the + // referenced CertificateProfile to pin AllowedKeyAlgorithms + // (the server has to decide what algorithm to generate). The + // presence of the CertificateProfile in the registry is checked + // at boot by the Phase 5 preflight; here we just gate the + // presence of ProfileID. + if p.ServerKeygenEnabled && p.ProfileID == "" { + return fmt.Errorf("EST profile %d (PathID=%q) has SERVERKEYGEN_ENABLED=true but PROFILE_ID is empty — refuse to start: server-side keygen needs a CertificateProfile to pin AllowedKeyAlgorithms (the server must know what key to generate)", i, p.PathID) + } + } + } + // Validate scheduler intervals if c.Scheduler.RenewalCheckInterval < 1*time.Minute { return fmt.Errorf("renewal check interval must be at least 1 minute") diff --git a/internal/config/config_est_profiles_test.go b/internal/config/config_est_profiles_test.go new file mode 100644 index 0000000..2ce62ee --- /dev/null +++ b/internal/config/config_est_profiles_test.go @@ -0,0 +1,528 @@ +package config + +import ( + "strings" + "testing" +) + +// EST RFC 7030 hardening master bundle Phase 1: per-issuer EST profiles. +// These tests pin: +// +// 1. Backward-compat shim: legacy CERTCTL_EST_* flat env vars (just +// CERTCTL_EST_ENABLED + CERTCTL_EST_ISSUER_ID + CERTCTL_EST_PROFILE_ID) +// synthesise a single-element Profiles[0] with PathID="" so existing +// /.well-known/est/ operators see no behavior change. +// 2. Structured form: CERTCTL_EST_PROFILES=corp,iot,wifi expands into +// per-profile env vars CERTCTL_EST_PROFILE__*. +// 3. PathID validation: only [a-z0-9-] with no leading/trailing hyphen, +// empty allowed (legacy root). Validate() refuses anything else. +// 4. Per-profile gates: Validate() refuses each profile independently +// (missing IssuerID, mtls-enabled-no-bundle, channel-binding-without- +// mtls, basic-auth-no-password, mtls-mode-without-mtls, unknown auth +// mode, negative rate limit, server-keygen without ProfileID, +// duplicate PathID). +// +// Note these tests exercise the loader + Validate() in isolation; the +// per-profile preflight + router-registration paths are exercised by the +// router_test (RegisterESTHandlers shape) and the cmd/server/main.go +// startup path (manual via `make docker-up`). + +// validBaseConfigForESTProfiles returns a Config that passes Validate +// EXCEPT for the EST fields the test under exercise sets. Mirrors the +// existing validBaseConfigForSCEPProfiles helper shape so the test file +// stays uniform with its siblings. +func validBaseConfigForESTProfiles(t *testing.T) *Config { + t.Helper() + return validBaseConfigForSCEPProfiles(t) // identical infra; EST tests just override the EST block +} + +// TestESTConfig_LegacyFlatFields_SynthesizeSingleProfile is the +// load-time backward-compat test: an operator with the pre-Phase-1 +// flat env vars (no CERTCTL_EST_PROFILES set) must end up with a +// single-element Profiles slice carrying PathID="" so /.well-known/est/ +// routes the same way it did before. +func TestESTConfig_LegacyFlatFields_SynthesizeSingleProfile(t *testing.T) { + clearCertctlEnv(t) + t.Setenv("CERTCTL_EST_ENABLED", "true") + t.Setenv("CERTCTL_EST_ISSUER_ID", "iss-legacy-est") + t.Setenv("CERTCTL_EST_PROFILE_ID", "prof-legacy-est") + // Required infra envs so Load() doesn't fail on unrelated gates. + t.Setenv("CERTCTL_DB_URL", "postgres://localhost/certctl?sslmode=disable") + t.Setenv("CERTCTL_AUTH_TYPE", "api-key") + t.Setenv("CERTCTL_AUTH_SECRET", "test-secret") + srv := validServerConfig(t) + t.Setenv("CERTCTL_SERVER_TLS_CERT_PATH", srv.TLS.CertPath) + t.Setenv("CERTCTL_SERVER_TLS_KEY_PATH", srv.TLS.KeyPath) + + cfg, err := Load() + if err != nil { + t.Fatalf("Load() error = %v, want nil (legacy EST flat fields should pass)", err) + } + if len(cfg.EST.Profiles) != 1 { + t.Fatalf("len(Profiles) = %d, want 1 (legacy shim should synthesize single-element slice)", len(cfg.EST.Profiles)) + } + got := cfg.EST.Profiles[0] + if got.PathID != "" { + t.Errorf("Profiles[0].PathID = %q, want \"\" (empty maps to legacy /.well-known/est/ root)", got.PathID) + } + if got.IssuerID != "iss-legacy-est" { + t.Errorf("Profiles[0].IssuerID = %q, want %q", got.IssuerID, "iss-legacy-est") + } + if got.ProfileID != "prof-legacy-est" { + t.Errorf("Profiles[0].ProfileID = %q, want %q", got.ProfileID, "prof-legacy-est") + } + // Forward-looking fields should be at their defaults (Phase 2/3/4/5 + // will set non-zero values via the structured form; the legacy shim + // preserves the pre-Phase-1 unauthenticated/unlimited defaults so + // existing operators see no behavior change). + if got.MTLSEnabled { + t.Errorf("Profiles[0].MTLSEnabled = true, want false (legacy shim preserves pre-Phase-1 defaults)") + } + if got.EnrollmentPassword != "" { + t.Errorf("Profiles[0].EnrollmentPassword = %q, want empty", got.EnrollmentPassword) + } + if len(got.AllowedAuthModes) != 0 { + t.Errorf("Profiles[0].AllowedAuthModes = %v, want empty (back-compat = no auth)", got.AllowedAuthModes) + } + if got.RateLimitPerPrincipal24h != 0 { + t.Errorf("Profiles[0].RateLimitPerPrincipal24h = %d, want 0 (back-compat = unlimited)", got.RateLimitPerPrincipal24h) + } + if got.ServerKeygenEnabled { + t.Errorf("Profiles[0].ServerKeygenEnabled = true, want false (Phase 5 opt-in)") + } +} + +// TestESTConfig_DisabledNoLegacyShim verifies that when EST is disabled +// the legacy shim is a no-op (Profiles stays empty, no synthesized +// element). Mirrors the SCEP equivalent. +func TestESTConfig_DisabledNoLegacyShim(t *testing.T) { + clearCertctlEnv(t) + t.Setenv("CERTCTL_EST_ENABLED", "false") + t.Setenv("CERTCTL_EST_ISSUER_ID", "iss-still-set") + t.Setenv("CERTCTL_DB_URL", "postgres://localhost/certctl?sslmode=disable") + t.Setenv("CERTCTL_AUTH_TYPE", "api-key") + t.Setenv("CERTCTL_AUTH_SECRET", "test-secret") + srv := validServerConfig(t) + t.Setenv("CERTCTL_SERVER_TLS_CERT_PATH", srv.TLS.CertPath) + t.Setenv("CERTCTL_SERVER_TLS_KEY_PATH", srv.TLS.KeyPath) + + cfg, err := Load() + if err != nil { + t.Fatalf("Load() error = %v, want nil", err) + } + if len(cfg.EST.Profiles) != 0 { + t.Errorf("len(Profiles) = %d, want 0 (disabled EST should not trigger the shim)", len(cfg.EST.Profiles)) + } +} + +// TestESTConfig_MultipleProfiles_LoadFromEnv exercises the structured form: +// CERTCTL_EST_PROFILES=corp,iot,wifi expands into per-profile env vars. +// All forward-looking fields (auth modes, mTLS, rate limit, server-keygen) +// load correctly even though the dispatching handlers are Phase 2-5 work. +func TestESTConfig_MultipleProfiles_LoadFromEnv(t *testing.T) { + clearCertctlEnv(t) + t.Setenv("CERTCTL_EST_ENABLED", "true") + t.Setenv("CERTCTL_EST_PROFILES", "corp,iot,wifi") + + // CORP: mTLS + Basic, channel-binding required, rate-limited, server-keygen on + t.Setenv("CERTCTL_EST_PROFILE_CORP_ISSUER_ID", "iss-corp-laptop") + t.Setenv("CERTCTL_EST_PROFILE_CORP_PROFILE_ID", "prof-corp-tls") + t.Setenv("CERTCTL_EST_PROFILE_CORP_ENROLLMENT_PASSWORD", "corp-secret") + t.Setenv("CERTCTL_EST_PROFILE_CORP_MTLS_ENABLED", "true") + t.Setenv("CERTCTL_EST_PROFILE_CORP_MTLS_CLIENT_CA_TRUST_BUNDLE_PATH", "/etc/certctl/est/corp-trust.pem") + t.Setenv("CERTCTL_EST_PROFILE_CORP_CHANNEL_BINDING_REQUIRED", "true") + t.Setenv("CERTCTL_EST_PROFILE_CORP_ALLOWED_AUTH_MODES", "mtls,basic") + t.Setenv("CERTCTL_EST_PROFILE_CORP_RATE_LIMIT_PER_PRINCIPAL_24H", "5") + t.Setenv("CERTCTL_EST_PROFILE_CORP_SERVERKEYGEN_ENABLED", "true") + + // IOT: Basic only (no mTLS for resource-constrained devices) + t.Setenv("CERTCTL_EST_PROFILE_IOT_ISSUER_ID", "iss-iot") + t.Setenv("CERTCTL_EST_PROFILE_IOT_PROFILE_ID", "prof-iot-30d") + t.Setenv("CERTCTL_EST_PROFILE_IOT_ENROLLMENT_PASSWORD", "iot-bootstrap") + t.Setenv("CERTCTL_EST_PROFILE_IOT_ALLOWED_AUTH_MODES", "basic") + t.Setenv("CERTCTL_EST_PROFILE_IOT_RATE_LIMIT_PER_PRINCIPAL_24H", "3") + + // WIFI: mTLS only (802.1X devices have factory bootstrap certs) + t.Setenv("CERTCTL_EST_PROFILE_WIFI_ISSUER_ID", "iss-wifi-eaptls") + t.Setenv("CERTCTL_EST_PROFILE_WIFI_MTLS_ENABLED", "true") + t.Setenv("CERTCTL_EST_PROFILE_WIFI_MTLS_CLIENT_CA_TRUST_BUNDLE_PATH", "/etc/certctl/est/wifi-trust.pem") + t.Setenv("CERTCTL_EST_PROFILE_WIFI_ALLOWED_AUTH_MODES", "mtls") + + // Required infra envs. + t.Setenv("CERTCTL_DB_URL", "postgres://localhost/certctl?sslmode=disable") + t.Setenv("CERTCTL_AUTH_TYPE", "api-key") + t.Setenv("CERTCTL_AUTH_SECRET", "test-secret") + srv := validServerConfig(t) + t.Setenv("CERTCTL_SERVER_TLS_CERT_PATH", srv.TLS.CertPath) + t.Setenv("CERTCTL_SERVER_TLS_KEY_PATH", srv.TLS.KeyPath) + + cfg, err := Load() + if err != nil { + t.Fatalf("Load() error = %v, want nil", err) + } + if len(cfg.EST.Profiles) != 3 { + t.Fatalf("len(Profiles) = %d, want 3", len(cfg.EST.Profiles)) + } + + type wantProfile struct { + PathID, IssuerID, ProfileID, EnrollmentPassword, MTLSBundle string + MTLSEnabled, ChannelBinding, ServerKeygen bool + RateLimit int + AuthModes []string + } + wants := map[string]wantProfile{ + "corp": { + PathID: "corp", IssuerID: "iss-corp-laptop", ProfileID: "prof-corp-tls", + EnrollmentPassword: "corp-secret", MTLSBundle: "/etc/certctl/est/corp-trust.pem", + MTLSEnabled: true, ChannelBinding: true, ServerKeygen: true, + RateLimit: 5, AuthModes: []string{"mtls", "basic"}, + }, + "iot": { + PathID: "iot", IssuerID: "iss-iot", ProfileID: "prof-iot-30d", + EnrollmentPassword: "iot-bootstrap", + RateLimit: 3, AuthModes: []string{"basic"}, + }, + "wifi": { + PathID: "wifi", IssuerID: "iss-wifi-eaptls", + MTLSBundle: "/etc/certctl/est/wifi-trust.pem", MTLSEnabled: true, + AuthModes: []string{"mtls"}, + }, + } + got := map[string]ESTProfileConfig{} + for _, p := range cfg.EST.Profiles { + got[p.PathID] = p + } + for name, want := range wants { + g, ok := got[name] + if !ok { + t.Fatalf("missing profile %q in loaded slice", name) + } + if g.PathID != want.PathID || g.IssuerID != want.IssuerID || g.ProfileID != want.ProfileID { + t.Errorf("profile %q identity = (%q,%q,%q), want (%q,%q,%q)", + name, g.PathID, g.IssuerID, g.ProfileID, want.PathID, want.IssuerID, want.ProfileID) + } + if g.EnrollmentPassword != want.EnrollmentPassword { + t.Errorf("profile %q EnrollmentPassword = %q, want %q", name, g.EnrollmentPassword, want.EnrollmentPassword) + } + if g.MTLSEnabled != want.MTLSEnabled || g.MTLSClientCATrustBundlePath != want.MTLSBundle { + t.Errorf("profile %q mTLS = (%v,%q), want (%v,%q)", + name, g.MTLSEnabled, g.MTLSClientCATrustBundlePath, want.MTLSEnabled, want.MTLSBundle) + } + if g.ChannelBindingRequired != want.ChannelBinding { + t.Errorf("profile %q ChannelBindingRequired = %v, want %v", name, g.ChannelBindingRequired, want.ChannelBinding) + } + if g.ServerKeygenEnabled != want.ServerKeygen { + t.Errorf("profile %q ServerKeygenEnabled = %v, want %v", name, g.ServerKeygenEnabled, want.ServerKeygen) + } + if g.RateLimitPerPrincipal24h != want.RateLimit { + t.Errorf("profile %q RateLimit = %d, want %d", name, g.RateLimitPerPrincipal24h, want.RateLimit) + } + if !equalStringSlices(g.AllowedAuthModes, want.AuthModes) { + t.Errorf("profile %q AllowedAuthModes = %v, want %v", name, g.AllowedAuthModes, want.AuthModes) + } + } +} + +// TestESTConfig_StructuredFormBeatsLegacy: when CERTCTL_EST_PROFILES is +// set, the legacy shim is a no-op (the structured form takes precedence). +func TestESTConfig_StructuredFormBeatsLegacy(t *testing.T) { + clearCertctlEnv(t) + t.Setenv("CERTCTL_EST_ENABLED", "true") + t.Setenv("CERTCTL_EST_ISSUER_ID", "iss-flat-ignored") + t.Setenv("CERTCTL_EST_PROFILES", "corp") + t.Setenv("CERTCTL_EST_PROFILE_CORP_ISSUER_ID", "iss-from-structured") + t.Setenv("CERTCTL_DB_URL", "postgres://localhost/certctl?sslmode=disable") + t.Setenv("CERTCTL_AUTH_TYPE", "api-key") + t.Setenv("CERTCTL_AUTH_SECRET", "test-secret") + srv := validServerConfig(t) + t.Setenv("CERTCTL_SERVER_TLS_CERT_PATH", srv.TLS.CertPath) + t.Setenv("CERTCTL_SERVER_TLS_KEY_PATH", srv.TLS.KeyPath) + + cfg, err := Load() + if err != nil { + t.Fatalf("Load() error = %v, want nil", err) + } + if len(cfg.EST.Profiles) != 1 { + t.Fatalf("len(Profiles) = %d, want 1 (structured form), got = %#v", len(cfg.EST.Profiles), cfg.EST.Profiles) + } + if got := cfg.EST.Profiles[0].IssuerID; got != "iss-from-structured" { + t.Errorf("Profiles[0].IssuerID = %q, want structured value (legacy shim should not have fired)", got) + } + if got := cfg.EST.Profiles[0].PathID; got != "corp" { + t.Errorf("Profiles[0].PathID = %q, want \"corp\"", got) + } +} + +// TestESTConfig_PathIDValidation pins validESTPathID + Validate() refusal +// of malformed PathIDs. +func TestESTConfig_PathIDValidation(t *testing.T) { + cases := []struct { + pathID string + valid bool + comment string + }{ + {"", true, "empty (legacy root)"}, + {"corp", true, "lowercase letters"}, + {"iot-fleet-2", true, "letters + digits + hyphens"}, + {"a", true, "single char"}, + {"-corp", false, "leading hyphen"}, + {"corp-", false, "trailing hyphen"}, + {"Corp", false, "uppercase"}, + {"corp/iot", false, "slash"}, + {"corp.iot", false, "dot"}, + {"corp_iot", false, "underscore"}, + {"corp iot", false, "space"}, + {"corp%20iot", false, "percent encoding"}, + } + for _, tc := range cases { + t.Run(tc.comment, func(t *testing.T) { + if got := validESTPathID(tc.pathID); got != tc.valid { + t.Errorf("validESTPathID(%q) = %v, want %v (%s)", tc.pathID, got, tc.valid, tc.comment) + } + }) + } +} + +// TestESTConfig_DuplicatePathID_Refuses verifies Validate() refuses two +// profiles with the same PathID. This is the load-bearing dispatch +// uniqueness guarantee — without it, the router would silently overwrite +// the first registration. +func TestESTConfig_DuplicatePathID_Refuses(t *testing.T) { + cfg := validBaseConfigForESTProfiles(t) + cfg.EST.Enabled = true + cfg.EST.Profiles = []ESTProfileConfig{ + {PathID: "corp", IssuerID: "iss-a"}, + {PathID: "corp", IssuerID: "iss-b"}, + } + err := cfg.Validate() + if err == nil { + t.Fatal("Validate() = nil, want error for duplicate PathID") + } + if !strings.Contains(err.Error(), "duplicates PathID") { + t.Errorf("Validate() error = %q, want substring \"duplicates PathID\"", err.Error()) + } +} + +// TestESTConfig_MissingPerProfileIssuerID verifies Validate() refuses +// a profile with empty IssuerID. +func TestESTConfig_MissingPerProfileIssuerID(t *testing.T) { + cfg := validBaseConfigForESTProfiles(t) + cfg.EST.Enabled = true + cfg.EST.Profiles = []ESTProfileConfig{ + {PathID: "corp", IssuerID: ""}, + } + err := cfg.Validate() + if err == nil { + t.Fatal("Validate() = nil, want error for empty IssuerID") + } + if !strings.Contains(err.Error(), "empty IssuerID") { + t.Errorf("Validate() error = %q, want substring \"empty IssuerID\"", err.Error()) + } +} + +// TestESTConfig_MTLSEnabledRequiresBundlePath verifies the per-profile +// gate: MTLSEnabled=true without MTLS_CLIENT_CA_TRUST_BUNDLE_PATH = refuse. +func TestESTConfig_MTLSEnabledRequiresBundlePath(t *testing.T) { + cfg := validBaseConfigForESTProfiles(t) + cfg.EST.Enabled = true + cfg.EST.Profiles = []ESTProfileConfig{ + { + PathID: "corp", IssuerID: "iss-corp", + MTLSEnabled: true, + MTLSClientCATrustBundlePath: "", // missing + }, + } + err := cfg.Validate() + if err == nil { + t.Fatal("Validate() = nil, want error for MTLSEnabled without trust bundle") + } + if !strings.Contains(err.Error(), "MTLSEnabled=true") { + t.Errorf("Validate() error = %q, want substring mentioning MTLSEnabled=true", err.Error()) + } + if !strings.Contains(err.Error(), "/.well-known/est-mtls/corp/") { + t.Errorf("Validate() error = %q, should reference the sibling route URL operators see", err.Error()) + } +} + +// TestESTConfig_ChannelBindingWithoutMTLS_Refuses verifies the cross-check: +// channel binding only makes sense when mTLS is in use (RFC 9266 binds the +// TLS-presented client cert to the CSR's CMC attribute). +func TestESTConfig_ChannelBindingWithoutMTLS_Refuses(t *testing.T) { + cfg := validBaseConfigForESTProfiles(t) + cfg.EST.Enabled = true + cfg.EST.Profiles = []ESTProfileConfig{ + { + PathID: "corp", IssuerID: "iss-corp", + MTLSEnabled: false, + ChannelBindingRequired: true, + }, + } + err := cfg.Validate() + if err == nil { + t.Fatal("Validate() = nil, want error for ChannelBindingRequired without mTLS") + } + if !strings.Contains(err.Error(), "ChannelBindingRequired=true but MTLSEnabled=false") { + t.Errorf("Validate() error = %q, want substring mentioning the cross-check", err.Error()) + } +} + +// TestESTConfig_BasicAuthInModesRequiresPassword verifies the cross-check: +// AllowedAuthModes mentions "basic" → EnrollmentPassword MUST be non-empty. +func TestESTConfig_BasicAuthInModesRequiresPassword(t *testing.T) { + cfg := validBaseConfigForESTProfiles(t) + cfg.EST.Enabled = true + cfg.EST.Profiles = []ESTProfileConfig{ + { + PathID: "corp", IssuerID: "iss-corp", + AllowedAuthModes: []string{"basic"}, + EnrollmentPassword: "", // missing + }, + } + err := cfg.Validate() + if err == nil { + t.Fatal("Validate() = nil, want error for basic auth without password") + } + if !strings.Contains(err.Error(), "ENROLLMENT_PASSWORD is empty") { + t.Errorf("Validate() error = %q, want substring mentioning empty ENROLLMENT_PASSWORD", err.Error()) + } +} + +// TestESTConfig_MTLSAuthModeRequiresMTLSEnabled verifies the cross-check: +// AllowedAuthModes mentions "mtls" → MTLSEnabled MUST be true. +func TestESTConfig_MTLSAuthModeRequiresMTLSEnabled(t *testing.T) { + cfg := validBaseConfigForESTProfiles(t) + cfg.EST.Enabled = true + cfg.EST.Profiles = []ESTProfileConfig{ + { + PathID: "corp", IssuerID: "iss-corp", + AllowedAuthModes: []string{"mtls"}, + MTLSEnabled: false, + }, + } + err := cfg.Validate() + if err == nil { + t.Fatal("Validate() = nil, want error for mtls auth mode without MTLSEnabled") + } + if !strings.Contains(err.Error(), "lists \"mtls\" in AllowedAuthModes but MTLSEnabled=false") { + t.Errorf("Validate() error = %q, want substring mentioning the cross-check", err.Error()) + } +} + +// TestESTConfig_UnknownAuthModeRefused verifies Validate() refuses any +// auth mode that isn't "mtls" or "basic" (typos, future modes the binary +// doesn't yet implement). +func TestESTConfig_UnknownAuthModeRefused(t *testing.T) { + cfg := validBaseConfigForESTProfiles(t) + cfg.EST.Enabled = true + cfg.EST.Profiles = []ESTProfileConfig{ + { + PathID: "corp", IssuerID: "iss-corp", + AllowedAuthModes: []string{"oauth"}, // not a documented EST auth mode + }, + } + err := cfg.Validate() + if err == nil { + t.Fatal("Validate() = nil, want error for unknown auth mode") + } + if !strings.Contains(err.Error(), "unknown AllowedAuthModes entry") { + t.Errorf("Validate() error = %q, want substring mentioning unknown auth mode", err.Error()) + } + if !strings.Contains(err.Error(), "oauth") { + t.Errorf("Validate() error = %q, want to surface the offending mode name", err.Error()) + } +} + +// TestESTConfig_NegativeRateLimitRefused verifies Validate() catches the +// config typo of a negative rate limit. +func TestESTConfig_NegativeRateLimitRefused(t *testing.T) { + cfg := validBaseConfigForESTProfiles(t) + cfg.EST.Enabled = true + cfg.EST.Profiles = []ESTProfileConfig{ + { + PathID: "corp", IssuerID: "iss-corp", + RateLimitPerPrincipal24h: -1, + }, + } + err := cfg.Validate() + if err == nil { + t.Fatal("Validate() = nil, want error for negative rate limit") + } + if !strings.Contains(err.Error(), "RATE_LIMIT_PER_PRINCIPAL_24H=-1") { + t.Errorf("Validate() error = %q, want substring mentioning negative rate limit", err.Error()) + } +} + +// TestESTConfig_ServerKeygenRequiresProfileID verifies Validate() refuses +// ServerKeygenEnabled=true without a CertificateProfile to pin +// AllowedKeyAlgorithms (the server has to know what to generate). +func TestESTConfig_ServerKeygenRequiresProfileID(t *testing.T) { + cfg := validBaseConfigForESTProfiles(t) + cfg.EST.Enabled = true + cfg.EST.Profiles = []ESTProfileConfig{ + { + PathID: "iot", IssuerID: "iss-iot", + ServerKeygenEnabled: true, + ProfileID: "", // missing + }, + } + err := cfg.Validate() + if err == nil { + t.Fatal("Validate() = nil, want error for ServerKeygenEnabled without ProfileID") + } + if !strings.Contains(err.Error(), "SERVERKEYGEN_ENABLED=true but PROFILE_ID is empty") { + t.Errorf("Validate() error = %q, want substring mentioning the missing PROFILE_ID", err.Error()) + } +} + +// TestESTConfig_DisabledIgnoresProfiles verifies that when EST is disabled, +// no per-profile validation runs (an operator with a half-configured set of +// profiles can still flip the kill-switch off without fixing every one). +func TestESTConfig_DisabledIgnoresProfiles(t *testing.T) { + cfg := validBaseConfigForESTProfiles(t) + cfg.EST.Enabled = false + cfg.EST.Profiles = []ESTProfileConfig{ + {PathID: "BAD-CASE", IssuerID: ""}, // would refuse if EST.Enabled + {PathID: "corp", IssuerID: ""}, // would refuse if EST.Enabled + } + if err := cfg.Validate(); err != nil { + t.Errorf("Validate() = %v, want nil (disabled EST should skip per-profile gates)", err) + } +} + +// TestESTConfig_ParseAuthModes_Normalization pins the parser's behavior +// (lowercasing, trimming, empty-element filtering). +func TestESTConfig_ParseAuthModes_Normalization(t *testing.T) { + cases := []struct { + input string + want []string + }{ + {"", nil}, + {" ", nil}, + {"mtls", []string{"mtls"}}, + {"MTLS", []string{"mtls"}}, + {"mtls,basic", []string{"mtls", "basic"}}, + {" mtls , basic ", []string{"mtls", "basic"}}, + {"mtls,,basic", []string{"mtls", "basic"}}, // empty element dropped + {"BASIC", []string{"basic"}}, + } + for _, tc := range cases { + got := parseAuthModes(tc.input) + if !equalStringSlices(got, tc.want) { + t.Errorf("parseAuthModes(%q) = %v, want %v", tc.input, got, tc.want) + } + } +} + +// equalStringSlices reports whether two []string slices contain the same +// elements in the same order. nil and []string{} are treated as equal. +func equalStringSlices(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} diff --git a/internal/integration/lifecycle_test.go b/internal/integration/lifecycle_test.go index d98ef98..dff02ea 100644 --- a/internal/integration/lifecycle_test.go +++ b/internal/integration/lifecycle_test.go @@ -123,7 +123,9 @@ func TestCertificateLifecycle(t *testing.T) { Verification: verificationHandler, BulkRevocation: handler.BulkRevocationHandler{}, }) - r.RegisterESTHandlers(estHandler) + // EST RFC 7030 hardening Phase 1: RegisterESTHandlers takes a map + // keyed by PathID. Empty PathID = legacy /.well-known/est/ root. + r.RegisterESTHandlers(map[string]handler.ESTHandler{"": estHandler}) // Create test server server := httptest.NewServer(r) diff --git a/internal/integration/negative_test.go b/internal/integration/negative_test.go index 1fa3060..3d0d995 100644 --- a/internal/integration/negative_test.go +++ b/internal/integration/negative_test.go @@ -93,27 +93,29 @@ func setupTestServer(t *testing.T) (*httptest.Server, *mockCertificateRepository r := router.New() r.RegisterHandlers(router.HandlerRegistry{ - Certificates: certificateHandler, - Issuers: issuerHandler, - Targets: targetHandler, - Agents: agentHandler, - Jobs: jobHandler, - Policies: policyHandler, - Profiles: profileHandler, - Teams: teamHandler, - Owners: ownerHandler, - AgentGroups: agentGroupHandler, - Audit: auditHandler, - Notifications: notificationHandler, - Stats: statsHandler, - Metrics: metricsHandler, - Health: healthHandler, - Discovery: discoveryHandler, - NetworkScan: networkScanHandler, - Verification: verificationHandler, - BulkRevocation: handler.BulkRevocationHandler{}, + Certificates: certificateHandler, + Issuers: issuerHandler, + Targets: targetHandler, + Agents: agentHandler, + Jobs: jobHandler, + Policies: policyHandler, + Profiles: profileHandler, + Teams: teamHandler, + Owners: ownerHandler, + AgentGroups: agentGroupHandler, + Audit: auditHandler, + Notifications: notificationHandler, + Stats: statsHandler, + Metrics: metricsHandler, + Health: healthHandler, + Discovery: discoveryHandler, + NetworkScan: networkScanHandler, + Verification: verificationHandler, + BulkRevocation: handler.BulkRevocationHandler{}, }) - r.RegisterESTHandlers(estHandler) + // EST RFC 7030 hardening Phase 1: RegisterESTHandlers takes a map + // keyed by PathID. Empty PathID = legacy /.well-known/est/ root. + r.RegisterESTHandlers(map[string]handler.ESTHandler{"": estHandler}) // M-006: CRL + OCSP live under /.well-known/pki/ (RFC 5280 + RFC 6960 + RFC 8615). // The negative_test integration suite exercises the DER CRL at this path with // no Authorization header to verify the relying-party contract. @@ -643,11 +645,11 @@ func TestM11bEndpoints(t *testing.T) { t.Run("AgentGroups", func(t *testing.T) { t.Run("CreateAgentGroup_Success", func(t *testing.T) { payload := map[string]interface{}{ - "name": "Linux Servers", - "description": "All linux-based agents", - "match_os": "linux", + "name": "Linux Servers", + "description": "All linux-based agents", + "match_os": "linux", "match_architecture": "amd64", - "enabled": true, + "enabled": true, } body, _ := json.Marshal(payload) resp, err := http.Post(server.URL+"/api/v1/agent-groups", "application/json", bytes.NewReader(body)) @@ -842,4 +844,3 @@ func TestRevocationEndpoints(t *testing.T) { } }) } -