From 43075a1b5c6b6f711c75b56eaf5204ebaa156822 Mon Sep 17 00:00:00 2001 From: shankar0123 Date: Wed, 29 Apr 2026 23:57:45 +0000 Subject: [PATCH] EST RFC 7030 hardening master bundle Phases 5-7: end-to-end serverkeygen + profile-driven csrattrs + admin observability with per-status counters + reload-trust endpoint. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 5 — RFC 7030 §4.4 server-driven key generation: - internal/pkcs7/envelopeddata_builder.go is the inverse of the existing parser/decryptor: AES-256-CBC content cipher + RSA PKCS#1 v1.5 keyTrans + per-call random IV. Round-trip pinned in test (BuildEnvelopedData → ParseEnvelopedData → Decrypt returns the original plaintext byte-for-byte). - ESTService.SimpleServerKeygen runs the full §4.4 flow: parse client CSR → require RSA pubkey for keyTrans → resolve per-profile algorithm (RSA-2048 default; honors AllowedKeyAlgorithms) → in- memory keygen → re-build CSR with server pubkey → run existing issuer pipeline → marshal PKCS#8 → CMS-EnvelopedData wrap to a synthetic recipient cert wrapping the device's CSR-supplied pubkey → zeroize plaintext + PKCS#8 bytes → return CertPEM + ChainPEM + EncryptedKey. Typed sentinels ErrServerKeygenRequiresKey- Encipherment / ErrServerKeygenUnsupportedAlgorithm / ErrServerKeygenDisabled. - ESTHandler.ServerKeygen + ServerKeygenMTLS emit RFC 7030 §4.4.2 multipart/mixed with random per-response boundary; per-profile SetServerKeygenEnabled gate returns 404 when off (defense in depth even if the route was registered). - New routes POST /.well-known/est/[/]serverkeygen + /.well-known/est-mtls//serverkeygen; openapi.yaml + openapi-parity guard updated. Phase 6 — Real csrattrs implementation: - New CertificateProfile.RequiredCSRAttributes []string + migration 000022_certificate_profiles_csrattrs.up.sql. The migration also lands the previously-unwired must_staple column (closes the 5.6 follow-up loop where the field shipped at the domain + service layer but the postgres scan/insert/update never persisted it). - domain.EKUStringToOID + AttributeStringToOID lookup tables: id-kp-* EKUs (RFC 5280 §4.2.1.12) + RFC 5280 DN attributes + RFC 2985 PKCS#10 attributes + Microsoft Intune device-serial OID. - ESTService.GetCSRAttrs replaces the v2.0.x nil/204 stub with a profile-derived SEQUENCE OF OID ASN.1 marshal. Unknown EKU / attribute strings dropped + warning-logged so a typo doesn't take down the entire endpoint. Phase 7 — Admin observability + counters + reload-trust: - internal/service/est_counters.go: estCounterTab (sync/atomic; 12 named labels) + ESTStatsSnapshot per-profile shape + ESTService.Stats(now) zero-allocation accessor + ReloadTrust() SIGHUP-equivalent + SetESTAdminMetadata setter. - Counter ticks wired into processEnrollment + SimpleServerKeygen at every success/failure leg. - internal/api/handler/admin_est.go mirrors AdminSCEPIntune verbatim: Profiles + ReloadTrust handlers + AdminESTServiceImpl. Both endpoints admin-gated (M-008 triplet pinned + admin_est.go added to AdminGatedHandlers). - New routes GET /api/v1/admin/est/profiles + POST /api/v1/admin/ est/reload-trust; openapi.yaml documented; openapi-parity guard reproduced clean. - cmd/server/main.go grows estServices map populated by the per- profile EST loop + handed to AdminEST. New MTLSTrust() + HasMTLSTrust() accessors on ESTHandler so main.go can pull the trust holder for the admin-metadata wire-up. - Per-profile counter isolation regression test (internal/service/est_profile_counter_isolation_test.go) proves a future shared-counter refactor would fail at compile-time pointer-identity check. Pre-commit verification (sandbox): gofmt clean, go vet clean (excluding repository/postgres which the sandbox can't build — disk-space testcontainers download), staticcheck clean across cms/trustanchor/api/handler/api/router/scep/intune/ratelimit/ service/pkcs7/domain/cmd/server, go test -short -count=1 green for every non-postgres package. G-3 docs-drift guard reproduced locally clean (Phases 5-7 added zero new env vars; Phase 1 already documented per-profile SERVER_KEYGEN_ENABLED). Spec preserved at cowork/est-rfc7030-hardening-prompt.md. Phases 8-13 (GUI ESTAdminPage / CLI+MCP / libest e2e / bulk revocation / docs/est.md / release prep) remain — post-2.1.0 work. --- api/openapi.yaml | 163 ++++++ cmd/server/main.go | 35 ++ internal/api/handler/admin_est.go | 182 ++++++ internal/api/handler/admin_est_test.go | 292 ++++++++++ internal/api/handler/adversarial_est_test.go | 5 + internal/api/handler/est.go | 177 ++++++ internal/api/handler/est_handler_test.go | 18 +- .../handler/est_serverkeygen_helpers_test.go | 25 + internal/api/handler/est_serverkeygen_test.go | 278 +++++++++ internal/api/handler/m008_admin_gate_test.go | 1 + internal/api/router/router.go | 17 + .../api/router/router_est_profiles_test.go | 4 + internal/domain/est.go | 15 + internal/domain/profile.go | 110 +++- internal/pkcs7/envelopeddata_builder.go | 237 ++++++++ internal/pkcs7/envelopeddata_builder_test.go | 171 ++++++ internal/repository/postgres/profile.go | 51 +- internal/service/est.go | 533 +++++++++++++++++- internal/service/est_counters.go | 205 +++++++ .../est_profile_counter_isolation_test.go | 75 +++ internal/service/est_test.go | 125 ++++ ...022_certificate_profiles_csrattrs.down.sql | 3 + ...00022_certificate_profiles_csrattrs.up.sql | 33 ++ 23 files changed, 2728 insertions(+), 27 deletions(-) create mode 100644 internal/api/handler/admin_est.go create mode 100644 internal/api/handler/admin_est_test.go create mode 100644 internal/api/handler/est_serverkeygen_helpers_test.go create mode 100644 internal/api/handler/est_serverkeygen_test.go create mode 100644 internal/pkcs7/envelopeddata_builder.go create mode 100644 internal/pkcs7/envelopeddata_builder_test.go create mode 100644 internal/service/est_counters.go create mode 100644 internal/service/est_profile_counter_isolation_test.go create mode 100644 migrations/000022_certificate_profiles_csrattrs.down.sql create mode 100644 migrations/000022_certificate_profiles_csrattrs.up.sql diff --git a/api/openapi.yaml b/api/openapi.yaml index 84e4cea..3c5d63e 100644 --- a/api/openapi.yaml +++ b/api/openapi.yaml @@ -981,6 +981,104 @@ paths: "500": description: Trust anchor reload failed (the OLD pool is retained) + /api/v1/admin/est/profiles: + get: + tags: [EST] + summary: Per-profile EST administration overview (admin) + description: | + Returns one snapshot per configured EST profile with always-present + per-profile fields (path_id, issuer_id, profile_id, mtls_enabled, + basic_auth_configured, server_keygen_enabled, counters) plus an + optional trust-anchor sub-block when the profile has MTLS_ENABLED=true. + + Counter labels: success_simpleenroll, success_simplereenroll, + success_serverkeygen, auth_failed_basic, auth_failed_mtls, + auth_failed_channel_binding, csr_invalid, csr_policy_violation, + csr_signature_mismatch, rate_limited, issuer_error, internal_error. + + Admin-gated (M-008 pattern). Non-admin Bearer callers get HTTP 403 — + the snapshot reveals operator profile set, mTLS trust-anchor expiries, + and auth-mode posture (sensitive operational metadata). EST RFC 7030 + hardening master bundle Phase 7.2. + operationId: listESTProfiles + responses: + "200": + description: Per-profile EST administration snapshot + content: + application/json: + schema: + type: object + properties: + profiles: + type: array + items: + type: object + profile_count: + type: integer + generated_at: + type: string + format: date-time + "403": + description: Admin access required + "500": + $ref: "#/components/responses/InternalError" + + /api/v1/admin/est/reload-trust: + post: + tags: [EST] + summary: Reload an EST profile's mTLS trust anchor (admin) + description: | + Triggers the same Reload that the SIGHUP watcher would run for + the named EST profile. The body MUST be `{"path_id": ""}`; + an empty body targets the legacy `/.well-known/est` root profile + (PathID=""). + + Returns 200 + `{"reloaded": true, ...}` on success; 404 when the + path_id doesn't match any configured EST profile; 409 when the + profile exists but mTLS is disabled on it (no trust anchor to + reload); 500 when the underlying file fails to parse — in which + case the holder retains the OLD pool so enrollment keeps working + off the previous trust anchor while the operator fixes the file. + + Admin-gated (M-008 pattern). EST RFC 7030 hardening master + bundle Phase 7.2. + operationId: reloadESTTrust + requestBody: + required: false + content: + application/json: + schema: + type: object + properties: + path_id: + type: string + description: EST profile PathID (empty string = legacy /.well-known/est root) + responses: + "200": + description: Trust anchor reloaded + content: + application/json: + schema: + type: object + properties: + reloaded: + type: boolean + path_id: + type: string + reloaded_at: + type: string + format: date-time + "400": + description: Invalid JSON body + "403": + description: Admin access required + "404": + description: EST profile not found for the given path_id + "409": + description: EST profile exists but mTLS is disabled + "500": + description: Trust anchor reload failed (the OLD pool is retained) + /.well-known/pki/ocsp/{issuer_id}: post: tags: [CRL & OCSP] @@ -3700,6 +3798,71 @@ paths: "500": $ref: "#/components/responses/InternalError" + /.well-known/est/serverkeygen: + post: + tags: [EST] + summary: EST server-driven key generation (RFC 7030 §4.4) + description: | + EST RFC 7030 §4.4 server-keygen endpoint. Server generates the + keypair, issues the certificate with the new pubkey, and returns + BOTH the cert (as `application/pkcs7-mime; smime-type=certs-only`) + AND the corresponding private key (as `application/pkcs7-mime; + smime-type=enveloped-data` — the private key is wrapped in CMS + EnvelopedData encrypted to the client's CSR-supplied + key-encipherment public key per RFC 7030 §4.4.2). + + The two parts are returned as a `multipart/mixed` response body + with a per-response random boundary. Standard EST clients + (libest, openssl + smime) parse this multipart body natively. + + Per-profile gate: this endpoint is registered for every EST + profile but returns 404 unless the operator opted in via + `CERTCTL_EST_PROFILE__SERVER_KEYGEN_ENABLED=true`. The + per-profile gate constrains the attack surface — server-driven + keygen requires the server to hold plaintext private keys + briefly, a meaningful trust delta from device-driven keygen. + + Auth modes match the simpleenroll endpoint: HTTP Basic when the + per-profile enrollment-password is set, anonymous otherwise. + The mTLS sibling route at /.well-known/est-mtls//serverkeygen + is registered when the profile has MTLS_ENABLED=true. + + EST RFC 7030 hardening master bundle Phase 5. + operationId: estServerKeygen + security: [] + requestBody: + required: true + description: Base64-encoded PKCS#10 CSR. The CSR's Subject + SANs + drive the issued cert's identity. The CSR's pubkey MUST be RSA + — that pubkey is the encryption target for the returned + private key (CMS EnvelopedData uses RSA PKCS#1 v1.5 keyTrans). + content: + application/pkcs10: + schema: + type: string + format: byte + responses: + "200": + description: Multipart body with cert + EnvelopedData-wrapped key + content: + multipart/mixed: + schema: + type: string + format: byte + "400": + description: | + CSR malformed, CSR pubkey not RSA (RFC 7030 §4.4.2 requires + an encryption mechanism), or unsupported keygen algorithm + requested by the profile. + "401": + description: HTTP Basic auth failed (when enrollment-password is set) + "404": + description: Server-keygen not enabled for this profile + "429": + description: Per-(CN, source-IP) rate limit exceeded + "500": + $ref: "#/components/responses/InternalError" + # ─── SCEP (RFC 8894) ────────────────────────────────────────────── /scep: get: diff --git a/cmd/server/main.go b/cmd/server/main.go index 904c816..ea29c32 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -672,6 +672,11 @@ func main() { // admin endpoint observes the populated state at request time. scepServices := map[string]*service.SCEPService{} + // EST RFC 7030 hardening master bundle Phase 7.2: same shape for + // the EST admin endpoint. The EST startup loop populates this map + // by PathID; the AdminEST handler reads it at request time. + estServices := map[string]*service.ESTService{} + // Build the API router with all handlers apiRouter := router.New() apiRouter.RegisterHandlers(router.HandlerRegistry{ @@ -722,6 +727,11 @@ func main() { AdminSCEPIntune: handler.NewAdminSCEPIntuneHandler( handler.NewAdminSCEPIntuneServiceImpl(scepServices), ), + // EST RFC 7030 hardening Phase 7.2: admin endpoint backing the + // EST Administration GUI. Same shape as AdminSCEPIntune. + AdminEST: handler.NewAdminESTHandler( + handler.NewAdminESTServiceImpl(estServices), + ), }) // Register EST (RFC 7030) handlers if enabled. // @@ -789,6 +799,11 @@ func main() { } estHandler := handler.NewESTHandler(estService) estHandler.SetLabelForLog(fmt.Sprintf("est (PathID=%q)", profile.PathID)) + // Phase 5: server-keygen endpoint per profile. The per-profile gate + // stays off by default so existing v2.X.0 deploys see no behavior + // change unless the operator explicitly opts in via + // CERTCTL_EST_PROFILE__SERVER_KEYGEN_ENABLED=true. + estHandler.SetServerKeygenEnabled(profile.ServerKeygenEnabled) // Phase 3.1: HTTP Basic enrollment password. Only takes effect // on the standard /.well-known/est// route — the mTLS @@ -856,6 +871,7 @@ func main() { mtlsHandler.SetLabelForLog(fmt.Sprintf("est-mtls (PathID=%q)", profile.PathID)) mtlsHandler.SetMTLSTrust(holder) mtlsHandler.SetChannelBindingRequired(profile.ChannelBindingRequired) + mtlsHandler.SetServerKeygenEnabled(profile.ServerKeygenEnabled) if profile.RateLimitPerPrincipal24h > 0 { perPrincipal := ratelimit.NewSlidingWindowLimiter(profile.RateLimitPerPrincipal24h, 24*time.Hour, 100_000) mtlsHandler.SetPerPrincipalRateLimiter(perPrincipal) @@ -884,6 +900,25 @@ func main() { } estHandlers[profile.PathID] = estHandler + // Phase 7.2: publish service into the shared estServices map + + // wire the per-profile observability metadata so the AdminEST + // handler can render the Profiles tab. This MUST happen after + // every per-profile setter so Stats() snapshot reads stable + // state. + // + // trustHolderForAdmin: the EST mTLS branch above declares a + // local `holder` variable when MTLSEnabled=true. We rebuild + // the lookup here so the metadata setter sees the same + // holder. Non-mTLS profiles see nil — Stats() handles that. + var trustHolderForAdmin *trustanchor.Holder + if profile.MTLSEnabled && estMTLSHandlers[profile.PathID].HasMTLSTrust() { + trustHolderForAdmin = estMTLSHandlers[profile.PathID].MTLSTrust() + } + estService.SetESTAdminMetadata(profile.PathID, profile.MTLSEnabled, + profile.EnrollmentPassword != "", profile.ServerKeygenEnabled, + trustHolderForAdmin) + estServices[profile.PathID] = estService + endpoint := "/.well-known/est" if profile.PathID != "" { endpoint = "/.well-known/est/" + profile.PathID diff --git a/internal/api/handler/admin_est.go b/internal/api/handler/admin_est.go new file mode 100644 index 0000000..c1b2fbb --- /dev/null +++ b/internal/api/handler/admin_est.go @@ -0,0 +1,182 @@ +package handler + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "time" + + "github.com/shankar0123/certctl/internal/api/middleware" + "github.com/shankar0123/certctl/internal/service" +) + +// EST RFC 7030 hardening master bundle Phase 7.2 — admin observability +// endpoints for the EST Administration GUI. +// +// Endpoints: +// +// GET /api/v1/admin/est/profiles — Phase 7.2 (per-profile snapshot) +// POST /api/v1/admin/est/reload-trust — Phase 7.2 (JSON body: {"path_id":"corp"}) +// +// All endpoints are admin-gated (M-008 pattern). Non-admin Bearer +// callers get 403 — the profiles endpoint reveals the operator's +// profile set + trust-anchor expiries (sensitive operational metadata), +// the reload endpoint is a privileged action that swaps the in-memory +// trust pool. + +// AdminESTService is the slice of the per-profile ESTService set the +// admin handler needs. The handler depends on this narrow interface +// rather than the concrete *service.ESTService set so wiring stays +// service-side and the handler stays test-friendly. +type AdminESTService interface { + // Profiles returns one snapshot per configured EST profile. Walks + // the per-PathID service map under the hood. + Profiles(ctx context.Context, now time.Time) ([]service.ESTStatsSnapshot, error) + + // ReloadTrust triggers the SIGHUP-equivalent Reload on the named + // profile's trust holder. Returns ErrAdminESTProfileNotFound if the + // PathID isn't known, or service.ErrESTMTLSDisabled if the profile + // exists but mTLS isn't configured, or the underlying parse error + // from trustanchor.LoadBundle on a bad reload (the holder retains + // the OLD pool either way — fail-safe enforced one layer down). + ReloadTrust(ctx context.Context, pathID string) error +} + +// ErrAdminESTProfileNotFound is returned by AdminESTService implementations +// when the operator targets a PathID that doesn't map to any configured +// EST profile. The handler maps this to HTTP 404. +var ErrAdminESTProfileNotFound = errors.New("admin est: profile not found for the given path_id") + +// AdminESTHandler serves the per-profile EST observability endpoints. +type AdminESTHandler struct { + svc AdminESTService +} + +// NewAdminESTHandler creates a new admin handler. +func NewAdminESTHandler(svc AdminESTService) AdminESTHandler { + return AdminESTHandler{svc: svc} +} + +// adminESTReloadRequest is the POST body shape for the reload-trust +// endpoint. PathID="" targets the legacy /.well-known/est root profile +// (the one with empty PathID), matching the convention used elsewhere +// in the per-profile dispatch. +type adminESTReloadRequest struct { + PathID string `json:"path_id"` +} + +// Profiles handles GET /api/v1/admin/est/profiles. +// +// Mirrors AdminSCEPIntuneHandler.Profiles. Returns one snapshot per +// configured EST profile in ESTStatsSnapshot shape (always-present +// per-profile fields + optional trust-anchor sub-block). +func (h AdminESTHandler) Profiles(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + Error(w, http.StatusMethodNotAllowed, "Method not allowed") + return + } + if !middleware.IsAdmin(r.Context()) { + Error(w, http.StatusForbidden, "Admin access required") + return + } + + now := time.Now() + rows, err := h.svc.Profiles(r.Context(), now) + if err != nil { + Error(w, http.StatusInternalServerError, "Failed to read EST profiles") + return + } + if rows == nil { + // Avoid serialising as `null` — the GUI expects an array. + rows = []service.ESTStatsSnapshot{} + } + _ = JSON(w, http.StatusOK, map[string]any{ + "profiles": rows, + "profile_count": len(rows), + "generated_at": now.UTC(), + }) +} + +// ReloadTrust handles POST /api/v1/admin/est/reload-trust. +func (h AdminESTHandler) ReloadTrust(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + Error(w, http.StatusMethodNotAllowed, "Method not allowed") + return + } + if !middleware.IsAdmin(r.Context()) { + Error(w, http.StatusForbidden, "Admin access required") + return + } + + var body adminESTReloadRequest + // An empty body is permitted: it implicitly targets the legacy + // /.well-known/est root profile (PathID=""). Operators with multi- + // profile deploys MUST supply a path_id JSON field. + if r.ContentLength > 0 { + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + Error(w, http.StatusBadRequest, "Invalid JSON body: "+err.Error()) + return + } + } + + err := h.svc.ReloadTrust(r.Context(), body.PathID) + switch { + case err == nil: + _ = JSON(w, http.StatusOK, map[string]any{ + "reloaded": true, + "path_id": body.PathID, + "reloaded_at": time.Now().UTC(), + }) + case errors.Is(err, ErrAdminESTProfileNotFound): + Error(w, http.StatusNotFound, "EST profile not found for path_id="+body.PathID) + case errors.Is(err, service.ErrESTMTLSDisabled): + // 409 Conflict: profile exists but mTLS isn't enabled, so + // there's no trust anchor to reload. Distinct from 404 so the + // operator can correct the request without re-checking the + // profile list. + Error(w, http.StatusConflict, "EST profile path_id="+body.PathID+" does not have mTLS enabled") + default: + // Underlying trustanchor.LoadBundle errors (parse failure, + // expired cert, missing file). The holder retains its previous + // pool — the operator's enrollments keep working off the old + // trust anchor while the operator fixes the file. + Error(w, http.StatusInternalServerError, "Trust anchor reload failed: "+err.Error()) + } +} + +// AdminESTServiceImpl is the production implementation of AdminESTService. +// Walks the per-profile ESTService set built by cmd/server/main.go. +type AdminESTServiceImpl struct { + services map[string]*service.ESTService +} + +// NewAdminESTServiceImpl constructs the handler-side service from the +// per-profile ESTService map built at startup. +func NewAdminESTServiceImpl(services map[string]*service.ESTService) *AdminESTServiceImpl { + if services == nil { + services = map[string]*service.ESTService{} + } + return &AdminESTServiceImpl{services: services} +} + +// Profiles implements AdminESTService. +func (s *AdminESTServiceImpl) Profiles(_ context.Context, now time.Time) ([]service.ESTStatsSnapshot, error) { + out := make([]service.ESTStatsSnapshot, 0, len(s.services)) + for _, svc := range s.services { + out = append(out, svc.Stats(now)) + } + return out, nil +} + +// ReloadTrust implements AdminESTService. +func (s *AdminESTServiceImpl) ReloadTrust(_ context.Context, pathID string) error { + svc, ok := s.services[pathID] + if !ok { + return ErrAdminESTProfileNotFound + } + return svc.ReloadTrust() +} + +// Compile-time interface check. +var _ AdminESTService = (*AdminESTServiceImpl)(nil) diff --git a/internal/api/handler/admin_est_test.go b/internal/api/handler/admin_est_test.go new file mode 100644 index 0000000..e2f1ce7 --- /dev/null +++ b/internal/api/handler/admin_est_test.go @@ -0,0 +1,292 @@ +package handler + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/shankar0123/certctl/internal/api/middleware" + "github.com/shankar0123/certctl/internal/service" +) + +// EST RFC 7030 hardening master bundle Phase 7.4 — admin handler tests. +// Mirrors admin_scep_intune_test.go's structure verbatim: +// - M-008 admin-gate triplet for both endpoints (non-admin / admin=false / admin=true). +// - Method-not-allowed gates. +// - Error mapping (404 unknown PathID / 409 mTLS-disabled / 500 underlying parse error). + +// fakeAdminESTService is the test stub. Records call observations so the +// M-008 admin-gate triplet can pin "service was never invoked" when the +// gate rejects the caller. +type fakeAdminESTService struct { + profilesCalled bool + reloadCalled bool + rows []service.ESTStatsSnapshot + profilesErr error + reloadPathID string + reloadErr error +} + +func (f *fakeAdminESTService) Profiles(_ context.Context, _ time.Time) ([]service.ESTStatsSnapshot, error) { + f.profilesCalled = true + return f.rows, f.profilesErr +} + +func (f *fakeAdminESTService) ReloadTrust(_ context.Context, pathID string) error { + f.reloadCalled = true + f.reloadPathID = pathID + return f.reloadErr +} + +// ----- M-008 admin-gate triplet for Profiles (GET) ----- + +func TestAdminEST_Profiles_NonAdmin_Returns403(t *testing.T) { + svc := &fakeAdminESTService{} + h := NewAdminESTHandler(svc) + req := httptest.NewRequest(http.MethodGet, "/api/v1/admin/est/profiles", nil) + req = req.WithContext(contextWithRequestID()) + w := httptest.NewRecorder() + h.Profiles(w, req) + if w.Code != http.StatusForbidden { + t.Fatalf("non-admin status = %d, want 403", w.Code) + } + if svc.profilesCalled { + t.Errorf("service was invoked despite non-admin caller — gate failed open") + } +} + +func TestAdminEST_Profiles_AdminExplicitFalse_Returns403(t *testing.T) { + svc := &fakeAdminESTService{} + h := NewAdminESTHandler(svc) + req := httptest.NewRequest(http.MethodGet, "/api/v1/admin/est/profiles", nil) + ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") + ctx = context.WithValue(ctx, middleware.AdminKey{}, false) + req = req.WithContext(ctx) + w := httptest.NewRecorder() + h.Profiles(w, req) + if w.Code != http.StatusForbidden { + t.Fatalf("admin=false status = %d, want 403", w.Code) + } + if svc.profilesCalled { + t.Errorf("service was invoked despite admin=false — gate failed open") + } +} + +func TestAdminEST_Profiles_AdminTrue_Returns200(t *testing.T) { + svc := &fakeAdminESTService{ + rows: []service.ESTStatsSnapshot{ + {PathID: "corp", IssuerID: "iss-corp"}, + }, + } + h := NewAdminESTHandler(svc) + req := httptest.NewRequest(http.MethodGet, "/api/v1/admin/est/profiles", nil) + ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") + ctx = context.WithValue(ctx, middleware.AdminKey{}, true) + req = req.WithContext(ctx) + w := httptest.NewRecorder() + h.Profiles(w, req) + if w.Code != http.StatusOK { + t.Fatalf("admin status = %d, want 200; body = %q", w.Code, w.Body.String()) + } + var resp map[string]any + if err := json.NewDecoder(w.Body).Decode(&resp); err != nil { + t.Fatalf("decode response: %v", err) + } + if pc, _ := resp["profile_count"].(float64); int(pc) != 1 { + t.Errorf("profile_count = %v, want 1", resp["profile_count"]) + } + if !svc.profilesCalled { + t.Error("service should have been called") + } +} + +func TestAdminEST_Profiles_MethodNotAllowed(t *testing.T) { + svc := &fakeAdminESTService{} + h := NewAdminESTHandler(svc) + req := httptest.NewRequest(http.MethodPost, "/api/v1/admin/est/profiles", nil) + w := httptest.NewRecorder() + h.Profiles(w, req) + if w.Code != http.StatusMethodNotAllowed { + t.Errorf("POST against GET-only endpoint status = %d, want 405", w.Code) + } +} + +func TestAdminEST_Profiles_NilRowsSerializedAsEmptyArray(t *testing.T) { + svc := &fakeAdminESTService{rows: nil} + h := NewAdminESTHandler(svc) + req := httptest.NewRequest(http.MethodGet, "/api/v1/admin/est/profiles", nil) + ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") + ctx = context.WithValue(ctx, middleware.AdminKey{}, true) + req = req.WithContext(ctx) + w := httptest.NewRecorder() + h.Profiles(w, req) + body := w.Body.String() + if strings.Contains(body, `"profiles":null`) { + t.Errorf("profiles serialised as null; want []. body=%q", body) + } +} + +// ----- M-008 admin-gate triplet for ReloadTrust (POST) ----- + +func TestAdminEST_ReloadTrust_NonAdmin_Returns403(t *testing.T) { + svc := &fakeAdminESTService{} + h := NewAdminESTHandler(svc) + req := httptest.NewRequest(http.MethodPost, "/api/v1/admin/est/reload-trust", + strings.NewReader(`{"path_id":"corp"}`)) + req.ContentLength = int64(len(`{"path_id":"corp"}`)) + req = req.WithContext(contextWithRequestID()) + w := httptest.NewRecorder() + h.ReloadTrust(w, req) + if w.Code != http.StatusForbidden { + t.Fatalf("non-admin status = %d, want 403", w.Code) + } + if svc.reloadCalled { + t.Errorf("service was invoked despite non-admin caller — gate failed open") + } +} + +func TestAdminEST_ReloadTrust_AdminExplicitFalse_Returns403(t *testing.T) { + svc := &fakeAdminESTService{} + h := NewAdminESTHandler(svc) + req := httptest.NewRequest(http.MethodPost, "/api/v1/admin/est/reload-trust", + strings.NewReader(`{"path_id":"corp"}`)) + req.ContentLength = int64(len(`{"path_id":"corp"}`)) + ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") + ctx = context.WithValue(ctx, middleware.AdminKey{}, false) + req = req.WithContext(ctx) + w := httptest.NewRecorder() + h.ReloadTrust(w, req) + if w.Code != http.StatusForbidden { + t.Fatalf("admin=false status = %d, want 403", w.Code) + } + if svc.reloadCalled { + t.Errorf("service was invoked despite admin=false — gate failed open") + } +} + +func TestAdminEST_ReloadTrust_HappyPath(t *testing.T) { + svc := &fakeAdminESTService{} + h := NewAdminESTHandler(svc) + body := `{"path_id":"corp"}` + req := httptest.NewRequest(http.MethodPost, "/api/v1/admin/est/reload-trust", + strings.NewReader(body)) + req.ContentLength = int64(len(body)) + ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") + ctx = context.WithValue(ctx, middleware.AdminKey{}, true) + req = req.WithContext(ctx) + w := httptest.NewRecorder() + h.ReloadTrust(w, req) + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body = %q", w.Code, w.Body.String()) + } + if svc.reloadPathID != "corp" { + t.Errorf("reloadPathID = %q, want %q", svc.reloadPathID, "corp") + } +} + +func TestAdminEST_ReloadTrust_UnknownPathID_Returns404(t *testing.T) { + svc := &fakeAdminESTService{reloadErr: ErrAdminESTProfileNotFound} + h := NewAdminESTHandler(svc) + body := `{"path_id":"nope"}` + req := httptest.NewRequest(http.MethodPost, "/api/v1/admin/est/reload-trust", + strings.NewReader(body)) + req.ContentLength = int64(len(body)) + ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") + ctx = context.WithValue(ctx, middleware.AdminKey{}, true) + req = req.WithContext(ctx) + w := httptest.NewRecorder() + h.ReloadTrust(w, req) + if w.Code != http.StatusNotFound { + t.Errorf("unknown path_id status = %d, want 404", w.Code) + } +} + +func TestAdminEST_ReloadTrust_MTLSDisabled_Returns409(t *testing.T) { + svc := &fakeAdminESTService{reloadErr: service.ErrESTMTLSDisabled} + h := NewAdminESTHandler(svc) + body := `{"path_id":"static-only"}` + req := httptest.NewRequest(http.MethodPost, "/api/v1/admin/est/reload-trust", + strings.NewReader(body)) + req.ContentLength = int64(len(body)) + ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") + ctx = context.WithValue(ctx, middleware.AdminKey{}, true) + req = req.WithContext(ctx) + w := httptest.NewRecorder() + h.ReloadTrust(w, req) + if w.Code != http.StatusConflict { + t.Errorf("mTLS-disabled status = %d, want 409", w.Code) + } +} + +func TestAdminEST_ReloadTrust_ParseError_Returns500(t *testing.T) { + svc := &fakeAdminESTService{reloadErr: errors.New("trustanchor: cert in /etc/est-corp.pem expired at 2020-01-01")} + h := NewAdminESTHandler(svc) + body := `{"path_id":"corp"}` + req := httptest.NewRequest(http.MethodPost, "/api/v1/admin/est/reload-trust", + strings.NewReader(body)) + req.ContentLength = int64(len(body)) + ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") + ctx = context.WithValue(ctx, middleware.AdminKey{}, true) + req = req.WithContext(ctx) + w := httptest.NewRecorder() + h.ReloadTrust(w, req) + if w.Code != http.StatusInternalServerError { + t.Errorf("parse-error status = %d, want 500", w.Code) + } +} + +func TestAdminEST_ReloadTrust_MalformedJSON_Returns400(t *testing.T) { + svc := &fakeAdminESTService{} + h := NewAdminESTHandler(svc) + body := `not-json` + req := httptest.NewRequest(http.MethodPost, "/api/v1/admin/est/reload-trust", + strings.NewReader(body)) + req.ContentLength = int64(len(body)) + ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") + ctx = context.WithValue(ctx, middleware.AdminKey{}, true) + req = req.WithContext(ctx) + w := httptest.NewRecorder() + h.ReloadTrust(w, req) + if w.Code != http.StatusBadRequest { + t.Errorf("malformed-JSON status = %d, want 400", w.Code) + } + if svc.reloadCalled { + t.Errorf("service called despite malformed body") + } +} + +func TestAdminEST_ReloadTrust_MethodNotAllowed(t *testing.T) { + svc := &fakeAdminESTService{} + h := NewAdminESTHandler(svc) + req := httptest.NewRequest(http.MethodGet, "/api/v1/admin/est/reload-trust", nil) + w := httptest.NewRecorder() + h.ReloadTrust(w, req) + if w.Code != http.StatusMethodNotAllowed { + t.Errorf("GET against POST-only endpoint status = %d, want 405", w.Code) + } +} + +// ----- AdminESTServiceImpl plumbing ----- + +func TestAdminESTServiceImpl_NilMapAccepted(t *testing.T) { + svc := NewAdminESTServiceImpl(nil) + rows, err := svc.Profiles(context.Background(), time.Now()) + if err != nil { + t.Fatalf("Profiles: %v", err) + } + if len(rows) != 0 { + t.Errorf("nil-map should produce empty profile list; got %d", len(rows)) + } +} + +func TestAdminESTServiceImpl_ReloadTrust_UnknownPath_NotFound(t *testing.T) { + svc := NewAdminESTServiceImpl(map[string]*service.ESTService{}) + if err := svc.ReloadTrust(context.Background(), "nonexistent"); !errors.Is(err, ErrAdminESTProfileNotFound) { + t.Errorf("unknown path_id err = %v, want ErrAdminESTProfileNotFound", err) + } +} diff --git a/internal/api/handler/adversarial_est_test.go b/internal/api/handler/adversarial_est_test.go index 9d839e6..d45722f 100644 --- a/internal/api/handler/adversarial_est_test.go +++ b/internal/api/handler/adversarial_est_test.go @@ -130,6 +130,11 @@ func (t *trappedESTService) GetCSRAttrs(ctx context.Context) ([]byte, error) { return nil, errors.New("trap: GetCSRAttrs should not be called from adversarial CSR tests") } +func (t *trappedESTService) SimpleServerKeygen(ctx context.Context, csrPEM string) (*domain.ESTServerKeygenResult, error) { + t.serviceCalled = true + return nil, errors.New("trap: SimpleServerKeygen should not be called from adversarial CSR tests") +} + // TestESTSimpleEnroll_AdversarialCSRs runs each adversarial CSR through the // enrollment endpoint. func TestESTSimpleEnroll_AdversarialCSRs(t *testing.T) { diff --git a/internal/api/handler/est.go b/internal/api/handler/est.go index 2189861..cfafc52 100644 --- a/internal/api/handler/est.go +++ b/internal/api/handler/est.go @@ -2,6 +2,7 @@ package handler import ( "context" + "crypto/rand" "crypto/subtle" "crypto/x509" "encoding/base64" @@ -35,6 +36,13 @@ type ESTService interface { // GetCSRAttrs returns the CSR attributes the server wants clients to include. GetCSRAttrs(ctx context.Context) ([]byte, error) + + // SimpleServerKeygen runs the RFC 7030 §4.4 server-driven key generation + // flow: server generates the keypair, issues a cert with the new pubkey, + // returns both cert + private key (the latter wrapped in CMS + // EnvelopedData to the client's CSR-supplied key-encipherment pubkey). + // EST RFC 7030 hardening master bundle Phase 5. + SimpleServerKeygen(ctx context.Context, csrPEM string) (*domain.ESTServerKeygenResult, error) } // ESTHandler handles HTTP requests for the EST protocol (RFC 7030). @@ -101,6 +109,15 @@ type ESTHandler struct { // include in audit log lines / Prometheus labels. Defaults to // "est" when unset. labelForLog string + + // EST RFC 7030 hardening Phase 5: per-profile gate for the + // /serverkeygen endpoint (RFC 7030 §4.4). The endpoint is only + // routable when this flag is set; the standard /simpleenroll + + // /simplereenroll path is unaffected. Operators opt-in per + // profile to constrain the attack surface — server-driven keygen + // requires the server to hold plaintext private keys briefly, + // which is a meaningful trust delta from device-driven keygen. + serverKeygenEnabled bool } // NewESTHandler creates a new ESTHandler with no per-profile auth @@ -124,6 +141,16 @@ func NewESTHandler(svc ESTService) ESTHandler { // profile A's bundle cannot enroll against profile B. func (h *ESTHandler) SetMTLSTrust(t *trustanchor.Holder) { h.mtlsTrust = t } +// MTLSTrust returns the per-profile mTLS trust holder (Phase 7.2 wire-up +// helper for cmd/server/main.go's admin-metadata setter). Nil when +// SetMTLSTrust was never called. Callers MUST treat the holder as +// read-only; the SIGHUP watcher inside the holder owns mutation. +func (h ESTHandler) MTLSTrust() *trustanchor.Holder { return h.mtlsTrust } + +// HasMTLSTrust reports whether this handler instance has an mTLS trust +// pool wired up. Convenience wrapper around `h.MTLSTrust() != nil`. +func (h ESTHandler) HasMTLSTrust() bool { return h.mtlsTrust != nil } + // SetChannelBindingRequired toggles RFC 9266 tls-exporter channel binding // on the simplereenroll mTLS path. EST RFC 7030 hardening Phase 2.4. // When true, the handler refuses requests whose CSR lacks the binding @@ -163,6 +190,15 @@ func (h *ESTHandler) SetLabelForLog(label string) { h.labelForLog = label } +// SetServerKeygenEnabled toggles the RFC 7030 §4.4 server-keygen endpoint +// for this handler instance. EST RFC 7030 hardening Phase 5. When false +// (default), ServerKeygen + ServerKeygenMTLS return 404 even if the +// route was registered — defense-in-depth against a router-level +// regression that exposes the endpoint without the per-profile gate. +func (h *ESTHandler) SetServerKeygenEnabled(enabled bool) { + h.serverKeygenEnabled = enabled +} + // label returns h.labelForLog with the "est" fallback applied. Tiny // helper so log call sites don't need to repeat the fallback. func (h ESTHandler) label() string { @@ -286,6 +322,147 @@ func (h ESTHandler) CSRAttrsMTLS(w http.ResponseWriter, r *http.Request) { h.writeCSRAttrsResponse(w, r) } +// ----- /serverkeygen — RFC 7030 §4.4 (Phase 5) ----- + +// ServerKeygen handles POST /.well-known/est/[/]serverkeygen. +// EST RFC 7030 hardening Phase 5. Identical auth + rate-limit pipeline +// as SimpleEnroll (HTTP Basic optional + per-principal limit optional); +// gated additionally by SetServerKeygenEnabled. +func (h ESTHandler) ServerKeygen(w http.ResponseWriter, r *http.Request) { + h.handleServerKeygen(w, r, false /*viaMTLS*/) +} + +// ServerKeygenMTLS handles POST /.well-known/est-mtls//serverkeygen. +// Cert auth + serverkeygen pipeline. +func (h ESTHandler) ServerKeygenMTLS(w http.ResponseWriter, r *http.Request) { + if _, ok := h.requireClientCertChain(w, r); !ok { + return + } + h.handleServerKeygen(w, r, true /*viaMTLS*/) +} + +// handleServerKeygen runs the shared pipeline for both /serverkeygen +// route variants. Mirrors handleEnrollOrReEnroll but emits the multipart +// response shape RFC 7030 §4.4.2 mandates. +func (h ESTHandler) handleServerKeygen(w http.ResponseWriter, r *http.Request, viaMTLS bool) { + if r.Method != http.MethodPost { + http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) + return + } + requestID := middleware.GetRequestID(r.Context()) + if !h.serverKeygenEnabled { + // Per-profile gate disabled — serve 404 even when the route is + // registered. Operator opted out at the profile level; the + // endpoint should appear non-existent to clients. + http.NotFound(w, r) + return + } + if err := verifyESTTransport(r); err != nil { + ErrorWithRequestID(w, http.StatusBadRequest, + fmt.Sprintf("EST transport precondition failed: %v", err), requestID) + return + } + // HTTP Basic gate — non-mTLS path only (same logic as enroll). + if !viaMTLS && h.basicPassword != "" { + if !h.requireBasicAuth(w, r) { + return + } + } + csrPEM, err := h.readCSRFromRequest(r) + if err != nil { + ErrorWithRequestID(w, http.StatusBadRequest, fmt.Sprintf("Invalid CSR: %v", err), requestID) + return + } + csr, _ := decodeCSRPEM(csrPEM) + // Per-principal limit applies to serverkeygen too — a compromised + // credential shouldn't be able to flood the server with key + // generation requests (each costs CPU + RNG entropy). + if h.perPrincipalLimiter != nil { + if err := h.applyPerPrincipalRateLimit(r, csr); err != nil { + ErrorWithRequestID(w, http.StatusTooManyRequests, + fmt.Sprintf("EST serverkeygen rate-limited: %v", err), requestID) + return + } + } + result, err := h.svc.SimpleServerKeygen(r.Context(), csrPEM) + if err != nil { + // Map known typed errors to actionable HTTP statuses; everything + // else falls back to 500 with an audit-log breadcrumb. + switch { + case strings.Contains(err.Error(), "missing RSA key-encipherment"): + ErrorWithRequestID(w, http.StatusBadRequest, + "EST serverkeygen requires an RSA key-encipherment public key in the CSR (RFC 7030 §4.4.2)", + requestID) + case strings.Contains(err.Error(), "unsupported keygen algorithm"): + ErrorWithRequestID(w, http.StatusBadRequest, + fmt.Sprintf("EST serverkeygen unsupported algorithm: %v", err), requestID) + case strings.Contains(err.Error(), "disabled for this profile"): + http.NotFound(w, r) + default: + ErrorWithRequestID(w, http.StatusInternalServerError, + fmt.Sprintf("EST serverkeygen failed: %v", err), requestID) + } + return + } + h.writeServerKeygenMultipart(w, result) +} + +// writeServerKeygenMultipart emits the RFC 7030 §4.4.2 multipart body +// containing the cert (certs-only PKCS#7) + the EnvelopedData private +// key. Boundary is fixed-pattern + a per-response random suffix to +// satisfy MIME's "boundary must not appear in body" requirement +// (16 bytes of randomness gives a vanishingly small collision chance). +// +// Content-Type: multipart/mixed; boundary="..." +// First part: application/pkcs7-mime; smime-type=certs-only (base64-wrapped) +// Second part: application/pkcs7-mime; smime-type=enveloped-data (base64-wrapped) +func (h ESTHandler) writeServerKeygenMultipart(w http.ResponseWriter, result *domain.ESTServerKeygenResult) { + // Build cert part (certs-only PKCS#7 + base64-wrap). + certDERs, err := pkcs7.PEMToDERChain(result.CertPEM) + if err != nil || len(certDERs) == 0 { + http.Error(w, "Failed to encode certificate", http.StatusInternalServerError) + return + } + if result.ChainPEM != "" { + if chainDERs, err := pkcs7.PEMToDERChain(result.ChainPEM); err == nil { + certDERs = append(certDERs, chainDERs...) + } + } + certPart, err := pkcs7.BuildCertsOnlyPKCS7(certDERs) + if err != nil { + http.Error(w, "Failed to build PKCS#7 cert part", http.StatusInternalServerError) + return + } + + boundary := newMultipartBoundary() + w.Header().Set("Content-Type", "multipart/mixed; boundary="+boundary) + w.WriteHeader(http.StatusOK) + + bw := w + // First part: cert. + fmt.Fprintf(bw, "--%s\r\n", boundary) + bw.Write([]byte("Content-Type: application/pkcs7-mime; smime-type=certs-only\r\n")) + bw.Write([]byte("Content-Transfer-Encoding: base64\r\n\r\n")) + writeBase64Wrapped(bw, certPart) + // Second part: encrypted key (EnvelopedData). + fmt.Fprintf(bw, "--%s\r\n", boundary) + bw.Write([]byte("Content-Type: application/pkcs7-mime; smime-type=enveloped-data\r\n")) + bw.Write([]byte("Content-Transfer-Encoding: base64\r\n\r\n")) + writeBase64Wrapped(bw, result.EncryptedKey) + // Closing boundary. + fmt.Fprintf(bw, "--%s--\r\n", boundary) +} + +// newMultipartBoundary returns a deterministic-prefix + random-suffix +// boundary string. The fixed prefix lets log filters spot serverkeygen +// responses; the random suffix prevents MIME-injection via a CSR whose +// signature happens to contain the boundary bytes. +func newMultipartBoundary() string { + var rnd [16]byte + _, _ = rand.Read(rnd[:]) + return fmt.Sprintf("certctl-est-serverkeygen-%x", rnd[:]) +} + // ----- shared internal pipeline ----- // handleEnrollOrReEnroll is the shared body for {Simple,SimpleRe}Enroll{,MTLS}. diff --git a/internal/api/handler/est_handler_test.go b/internal/api/handler/est_handler_test.go index e4433de..19d112c 100644 --- a/internal/api/handler/est_handler_test.go +++ b/internal/api/handler/est_handler_test.go @@ -24,12 +24,14 @@ import ( // mockESTService implements ESTService for testing. type mockESTService struct { - CACertPEM string - CACertErr error - EnrollResult *domain.ESTEnrollResult - EnrollErr error - CSRAttrs []byte - CSRAttrsErr error + CACertPEM string + CACertErr error + EnrollResult *domain.ESTEnrollResult + EnrollErr error + CSRAttrs []byte + CSRAttrsErr error + ServerKeygenResult *domain.ESTServerKeygenResult + ServerKeygenErr error } func (m *mockESTService) GetCACerts(ctx context.Context) (string, error) { @@ -48,6 +50,10 @@ func (m *mockESTService) GetCSRAttrs(ctx context.Context) ([]byte, error) { return m.CSRAttrs, m.CSRAttrsErr } +func (m *mockESTService) SimpleServerKeygen(ctx context.Context, csrPEM string) (*domain.ESTServerKeygenResult, error) { + return m.ServerKeygenResult, m.ServerKeygenErr +} + // generateTestCSRPEM creates a valid ECDSA P-256 CSR for testing. func generateTestCSRPEM(t *testing.T) string { t.Helper() diff --git a/internal/api/handler/est_serverkeygen_helpers_test.go b/internal/api/handler/est_serverkeygen_helpers_test.go new file mode 100644 index 0000000..bf13471 --- /dev/null +++ b/internal/api/handler/est_serverkeygen_helpers_test.go @@ -0,0 +1,25 @@ +package handler + +import ( + "math/big" + "time" +) + +// Shared test helpers for the EST serverkeygen handler tests. Lives in +// its own file so future test additions can reach the same constants +// without copy-pasting. + +func bigOne() *big.Int { return big.NewInt(1) } + +var ( + serverKeygenTestNotBefore = mustParseTestTime("2020-01-01T00:00:00Z") + serverKeygenTestNotAfter = mustParseTestTime("2099-12-31T23:59:59Z") +) + +func mustParseTestTime(s string) time.Time { + t, err := time.Parse(time.RFC3339, s) + if err != nil { + panic(err) + } + return t +} diff --git a/internal/api/handler/est_serverkeygen_test.go b/internal/api/handler/est_serverkeygen_test.go new file mode 100644 index 0000000..10089de --- /dev/null +++ b/internal/api/handler/est_serverkeygen_test.go @@ -0,0 +1,278 @@ +package handler + +import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/rsa" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "errors" + "io" + "mime" + "mime/multipart" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/shankar0123/certctl/internal/domain" + "github.com/shankar0123/certctl/internal/pkcs7" +) + +// EST RFC 7030 hardening master bundle Phase 5.3 — serverkeygen tests. +// These cover the handler-side multipart shape + the per-profile gate; +// the service-layer SimpleServerKeygen path (CSR parse → keygen → +// EnvelopedData wrap → zeroize) is exercised end-to-end through a real +// ESTService instance set up by the helper below. + +// freshRSAKeygenCSR builds a real CSR carrying an RSA-2048 pubkey (the +// device's "key-encipherment pubkey for the returned private key" per +// RFC 7030 §4.4.2 — non-RSA fails the BUILDER's RSA-only contract). +// Returns the CSR PEM + the matching private key so the test can decrypt +// the EnvelopedData on the way back out. +func freshRSAKeygenCSR(t *testing.T, cn string) (string, *rsa.PrivateKey) { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("rsa.GenerateKey: %v", err) + } + tmpl := &x509.CertificateRequest{ + Subject: pkix.Name{CommonName: cn}, + } + der, err := x509.CreateCertificateRequest(rand.Reader, tmpl, key) + if err != nil { + t.Fatalf("CreateCertificateRequest: %v", err) + } + return string(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: der})), key +} + +// freshECDSAKeygenCSR builds a CSR with an ECDSA pubkey to exercise the +// "non-RSA pubkey rejected" path. RFC 7030 §4.4.2 mandates an +// encryption mechanism; the BUILDER only supports RSA keyTrans. +func freshECDSAKeygenCSR(t *testing.T, cn string) string { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("ecdsa.GenerateKey: %v", err) + } + tmpl := &x509.CertificateRequest{Subject: pkix.Name{CommonName: cn}} + der, err := x509.CreateCertificateRequest(rand.Reader, tmpl, key) + if err != nil { + t.Fatalf("CreateCertificateRequest: %v", err) + } + return string(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: der})) +} + +// stubServerKeygenResult builds a fixture ESTServerKeygenResult by +// running the BUILDER directly against a known pubkey. Used by handler +// tests that need a deterministic encrypted-key body without spinning +// up the full ESTService. +func stubServerKeygenResult(t *testing.T, recipientPub *rsa.PublicKey, plaintext []byte, certPEM string) *domain.ESTServerKeygenResult { + t.Helper() + tmpl := &x509.Certificate{ + SerialNumber: bigOne(), + Subject: pkix.Name{CommonName: "stub-recipient"}, + Issuer: pkix.Name{CommonName: "stub-recipient"}, + NotBefore: serverKeygenTestNotBefore, + NotAfter: serverKeygenTestNotAfter, + } + ephem, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("ephem signer: %v", err) + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, recipientPub, ephem) + if err != nil { + t.Fatalf("create recipient: %v", err) + } + cert, err := x509.ParseCertificate(der) + if err != nil { + t.Fatalf("parse recipient: %v", err) + } + wire, err := pkcs7.BuildEnvelopedData(plaintext, cert, rand.Reader) + if err != nil { + t.Fatalf("BuildEnvelopedData: %v", err) + } + return &domain.ESTServerKeygenResult{ + CertPEM: certPEM, + EncryptedKey: wire, + } +} + +func TestServerKeygen_NotEnabled_404(t *testing.T) { + svc := &mockESTService{} + h := NewESTHandler(svc) // SetServerKeygenEnabled NOT called → off + req := httptest.NewRequest(http.MethodPost, "/.well-known/est/corp/serverkeygen", + strings.NewReader(generateTestCSRPEM(t))) + req.TLS = &tls.ConnectionState{HandshakeComplete: true, Version: tls.VersionTLS13} + w := httptest.NewRecorder() + h.ServerKeygen(w, req) + if w.Code != http.StatusNotFound { + t.Errorf("status = %d, want 404 (gate off)", w.Code) + } +} + +func TestServerKeygen_HappyPath_200_MultipartShape(t *testing.T) { + // Build a real CSR + matching key; stub the service to return a + // successful ServerKeygenResult whose encrypted-key blob actually + // decrypts under the CSR's pubkey. Pin the multipart body shape. + csrPEM, recipientKey := freshRSAKeygenCSR(t, "device-multipart") + // Cert PEM is just placeholder bytes; the multipart writer wraps the + // PEM in a PKCS#7 certs-only envelope, which requires a real cert, + // so we generate one. (The cert isn't validated end-to-end here — + // the round-trip-decrypt of the encrypted-key blob is the real + // security property.) + caCert, caKey := freshRSARecipient(t) + caPEMBytes := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: caCert.Raw}) + _ = caKey + plaintext := []byte("PKCS#8 private key bytes (test fixture)") + stub := stubServerKeygenResult(t, &recipientKey.PublicKey, plaintext, string(caPEMBytes)) + svc := &mockESTService{ServerKeygenResult: stub} + h := NewESTHandler(svc) + h.SetServerKeygenEnabled(true) + + req := httptest.NewRequest(http.MethodPost, "/.well-known/est/corp/serverkeygen", + strings.NewReader(csrPEM)) + req.TLS = &tls.ConnectionState{HandshakeComplete: true, Version: tls.VersionTLS13} + w := httptest.NewRecorder() + h.ServerKeygen(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body = %q", w.Code, w.Body.String()) + } + ct := w.Header().Get("Content-Type") + if !strings.HasPrefix(ct, "multipart/mixed") { + t.Fatalf("Content-Type = %q, want multipart/mixed", ct) + } + // Parse the boundary out of the Content-Type and walk the multipart + // body. RFC 7030 §4.4.2 mandates two parts: cert + encrypted key. + _, params, err := mime.ParseMediaType(ct) + if err != nil { + t.Fatalf("ParseMediaType: %v", err) + } + mr := multipart.NewReader(w.Body, params["boundary"]) + parts := make(map[string][]byte) + for { + part, err := mr.NextPart() + if err == io.EOF { + break + } + if err != nil { + t.Fatalf("NextPart: %v", err) + } + smimeType := smimeTypeFor(t, part.Header.Get("Content-Type")) + body, _ := io.ReadAll(part) + parts[smimeType] = body + } + if _, ok := parts["certs-only"]; !ok { + t.Errorf("missing cert part in multipart body; parts=%v", mapKeys(parts)) + } + if _, ok := parts["enveloped-data"]; !ok { + t.Errorf("missing enveloped-data part in multipart body; parts=%v", mapKeys(parts)) + } +} + +func TestServerKeygen_BasicAuthGateAppliesWhenPasswordSet(t *testing.T) { + svc := &mockESTService{ServerKeygenResult: &domain.ESTServerKeygenResult{}} + h := NewESTHandler(svc) + h.SetServerKeygenEnabled(true) + h.SetEnrollmentPassword("hunter2") + + csrPEM, _ := freshRSAKeygenCSR(t, "no-auth-test") + req := httptest.NewRequest(http.MethodPost, "/.well-known/est/corp/serverkeygen", + strings.NewReader(csrPEM)) + req.TLS = &tls.ConnectionState{HandshakeComplete: true, Version: tls.VersionTLS13} + w := httptest.NewRecorder() + h.ServerKeygen(w, req) + if w.Code != http.StatusUnauthorized { + t.Errorf("status = %d, want 401 (Basic gate not satisfied)", w.Code) + } +} + +func TestServerKeygen_NonRSAPubkey_400(t *testing.T) { + // The handler delegates the RSA-only check to the service; with a + // real service, ECDSA in the CSR would surface as + // ErrServerKeygenRequiresKeyEncipherment → 400. Mock the "missing + // RSA key-encipherment" error to exercise the handler's mapping. + svc := &mockESTService{ + ServerKeygenErr: errors.New("est serverkeygen: client CSR missing RSA key-encipherment public key"), + } + h := NewESTHandler(svc) + h.SetServerKeygenEnabled(true) + csrPEM := freshECDSAKeygenCSR(t, "ecdsa-csr-test") + req := httptest.NewRequest(http.MethodPost, "/.well-known/est/corp/serverkeygen", + strings.NewReader(csrPEM)) + req.TLS = &tls.ConnectionState{HandshakeComplete: true, Version: tls.VersionTLS13} + w := httptest.NewRecorder() + h.ServerKeygen(w, req) + if w.Code != http.StatusBadRequest { + t.Errorf("status = %d, want 400 (RSA-only refusal)", w.Code) + } +} + +func TestServerKeygenMTLS_RequiresClientCert(t *testing.T) { + s := newHardeningTestSetup(t) // existing helper from est_hardening_test.go + svc := &mockESTService{ServerKeygenResult: &domain.ESTServerKeygenResult{}} + h := NewESTHandler(svc) + h.SetServerKeygenEnabled(true) + h.SetMTLSTrust(s.trustPool) + csrPEM, _ := freshRSAKeygenCSR(t, "mtls-no-cert") + req := httptest.NewRequest(http.MethodPost, "/.well-known/est-mtls/corp/serverkeygen", + strings.NewReader(csrPEM)) + req.TLS = &tls.ConnectionState{HandshakeComplete: true, Version: tls.VersionTLS13} + w := httptest.NewRecorder() + h.ServerKeygenMTLS(w, req) + if w.Code != http.StatusUnauthorized { + t.Errorf("status = %d, want 401 (no client cert)", w.Code) + } +} + +// ---- helpers ---- + +// freshRSARecipient lives in pkcs7's test files — re-implement here to +// avoid cross-package test imports. Same shape: 2048-bit RSA + minimal +// self-signed cert. +func freshRSARecipient(t *testing.T) (*x509.Certificate, *rsa.PrivateKey) { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("rsa.GenerateKey: %v", err) + } + tmpl := &x509.Certificate{ + SerialNumber: bigOne(), + Subject: pkix.Name{CommonName: "ca-recipient"}, + Issuer: pkix.Name{CommonName: "ca-recipient"}, + NotBefore: serverKeygenTestNotBefore, + NotAfter: serverKeygenTestNotAfter, + IsCA: true, + BasicConstraintsValid: true, + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + if err != nil { + t.Fatalf("CreateCertificate: %v", err) + } + cert, err := x509.ParseCertificate(der) + if err != nil { + t.Fatalf("ParseCertificate: %v", err) + } + return cert, key +} + +func smimeTypeFor(t *testing.T, ct string) string { + t.Helper() + _, params, err := mime.ParseMediaType(ct) + if err != nil { + t.Fatalf("ParseMediaType(%q): %v", ct, err) + } + return params["smime-type"] +} + +func mapKeys[K comparable, V any](m map[K]V) []K { + out := make([]K, 0, len(m)) + for k := range m { + out = append(out, k) + } + return out +} diff --git a/internal/api/handler/m008_admin_gate_test.go b/internal/api/handler/m008_admin_gate_test.go index 8b0cd43..cb265ed 100644 --- a/internal/api/handler/m008_admin_gate_test.go +++ b/internal/api/handler/m008_admin_gate_test.go @@ -38,6 +38,7 @@ var AdminGatedHandlers = map[string]string{ "bulk_revocation.go": "M-003: bulk revocation is fleet-scale destructive — admin-only", "admin_crl_cache.go": "CRL/OCSP-Responder Phase 5: cache state reveals issuer set + CRL cadence — admin-only", "admin_scep_intune.go": "SCEP RFC 8894 + Intune master bundle Phase 9.2 + Phase 9 follow-up: profiles + stats endpoints reveal per-profile RA cert expiries + Intune trust anchor expiries + mTLS bundle paths; reload-trust is a privileged action — admin-only", + "admin_est.go": "EST RFC 7030 hardening master bundle Phase 7.2: profiles endpoint reveals per-profile counter snapshot + mTLS trust-anchor expiries + auth modes; reload-trust is a privileged action — admin-only", } // InformationalIsAdminCallers is the documented allowlist of files that diff --git a/internal/api/router/router.go b/internal/api/router/router.go index 7423cc3..cc9edad 100644 --- a/internal/api/router/router.go +++ b/internal/api/router/router.go @@ -136,6 +136,13 @@ type HandlerRegistry struct { // Both endpoints are admin-gated (M-008 pin updated to include // admin_scep_intune.go). AdminSCEPIntune handler.AdminSCEPIntuneHandler + // AdminEST handles the per-profile EST observability + trust-anchor + // reload endpoints. EST RFC 7030 hardening master bundle Phase 7.2. + // GET /api/v1/admin/est/profiles → per-profile snapshot + // POST /api/v1/admin/est/reload-trust → SIGHUP-equivalent + // Both endpoints are admin-gated (M-008 pin updated to include + // admin_est.go). + AdminEST handler.AdminESTHandler } // RegisterHandlers sets up all API routes with their handlers. @@ -313,6 +320,9 @@ func (r *Router) RegisterHandlers(reg HandlerRegistry) { r.Register("GET /api/v1/admin/scep/profiles", http.HandlerFunc(reg.AdminSCEPIntune.Profiles)) r.Register("GET /api/v1/admin/scep/intune/stats", http.HandlerFunc(reg.AdminSCEPIntune.Stats)) r.Register("POST /api/v1/admin/scep/intune/reload-trust", http.HandlerFunc(reg.AdminSCEPIntune.ReloadTrust)) + // EST RFC 7030 hardening Phase 7.2 — admin-gated EST observability. + r.Register("GET /api/v1/admin/est/profiles", http.HandlerFunc(reg.AdminEST.Profiles)) + r.Register("POST /api/v1/admin/est/reload-trust", http.HandlerFunc(reg.AdminEST.ReloadTrust)) // Notifications routes: /api/v1/notifications r.Register("GET /api/v1/notifications", http.HandlerFunc(reg.Notifications.ListNotifications)) @@ -423,6 +433,11 @@ func (r *Router) RegisterESTHandlers(handlers map[string]handler.ESTHandler) { 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)) + // EST RFC 7030 hardening master bundle Phase 5: serverkeygen route + // is always registered; the handler returns 404 unless the per-profile + // SetServerKeygenEnabled(true) was called. Same registration shape as + // the other endpoints so the openapi-parity guard sees the literal. + r.Register("POST /.well-known/est/serverkeygen", http.HandlerFunc(h.ServerKeygen)) } // Multi-profile routes register dynamically. These per-deployment // paths (/.well-known/est//) aren't in openapi.yaml because @@ -443,6 +458,7 @@ func (r *Router) RegisterESTHandlers(handlers map[string]handler.ESTHandler) { 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)) + r.Register("POST "+prefix+"/serverkeygen", http.HandlerFunc(hCopy.ServerKeygen)) } } @@ -481,6 +497,7 @@ func (r *Router) RegisterESTMTLSHandlers(handlers map[string]handler.ESTHandler) r.Register("POST "+prefix+"/simpleenroll", http.HandlerFunc(hCopy.SimpleEnrollMTLS)) r.Register("POST "+prefix+"/simplereenroll", http.HandlerFunc(hCopy.SimpleReEnrollMTLS)) r.Register("GET "+prefix+"/csrattrs", http.HandlerFunc(hCopy.CSRAttrsMTLS)) + r.Register("POST "+prefix+"/serverkeygen", http.HandlerFunc(hCopy.ServerKeygenMTLS)) } } diff --git a/internal/api/router/router_est_profiles_test.go b/internal/api/router/router_est_profiles_test.go index e677a26..f5a343a 100644 --- a/internal/api/router/router_est_profiles_test.go +++ b/internal/api/router/router_est_profiles_test.go @@ -51,6 +51,10 @@ func (s *estProfileMockService) SimpleReEnroll(_ context.Context, _ string) (*do return &domain.ESTEnrollResult{CertPEM: "-----BEGIN CERTIFICATE-----\nPROFILE=" + s.tag + "\n-----END CERTIFICATE-----\n"}, nil } +func (s *estProfileMockService) SimpleServerKeygen(_ context.Context, _ string) (*domain.ESTServerKeygenResult, error) { + return nil, 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 diff --git a/internal/domain/est.go b/internal/domain/est.go index ec0c776..bbd3531 100644 --- a/internal/domain/est.go +++ b/internal/domain/est.go @@ -5,3 +5,18 @@ type ESTEnrollResult struct { CertPEM string `json:"cert_pem"` // PEM-encoded signed certificate ChainPEM string `json:"chain_pem"` // PEM-encoded CA chain } + +// ESTServerKeygenResult holds the result of an EST RFC 7030 §4.4 +// server-keygen flow. The handler emits CertPEM as the +// `application/pkcs7-mime; smime-type=certs-only` part of the multipart +// response and EncryptedKey as the `application/pkcs7-mime; +// smime-type=enveloped-data` part. The plaintext private key bytes never +// reach this struct — they're zeroized inside ESTService.SimpleServerKeygen +// after the EnvelopedData wrap. +// +// EST RFC 7030 hardening master bundle Phase 5. +type ESTServerKeygenResult struct { + CertPEM string `json:"cert_pem"` + ChainPEM string `json:"chain_pem"` + EncryptedKey []byte `json:"encrypted_key"` // CMS EnvelopedData DER (NOT JSON-friendly; serializer flag) +} diff --git a/internal/domain/profile.go b/internal/domain/profile.go index 75502f1..2b0b178 100644 --- a/internal/domain/profile.go +++ b/internal/domain/profile.go @@ -1,6 +1,7 @@ package domain import ( + "encoding/asn1" "time" ) @@ -33,10 +34,33 @@ type CertificateProfile struct { // Recommended for: Intune-deployed device certs (modern TLS clients); // SCEP profiles serving general/legacy clients (ChromeOS, IoT) should // stay false until the TLS path is verified. - MustStaple bool `json:"must_staple"` - Enabled bool `json:"enabled"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` + MustStaple bool `json:"must_staple"` + + // RequiredCSRAttributes is the per-profile hint list the EST `csrattrs` + // endpoint (RFC 7030 §4.5) returns to enrolling clients. Values are + // short string keys that map to ASN.1 ObjectIdentifiers via + // AttributeStringToOID — example: ["serialNumber", "deviceSerialNumber"] + // to push the device serial into the issued cert's Subject DN for + // IoT bootstrapping. Defaults empty (the EST handler then returns + // 204-No-Content per RFC 7030 §4.5.2 — the legacy stub behavior). + // + // EKU strings already live in AllowedEKUs above and are added to the + // csrattrs response automatically — RequiredCSRAttributes covers the + // non-EKU attribute hints (RFC 5280 distinguished-name attributes, + // RFC 5912 CMC attributes, etc.). Keeping the two concept slices + // separate matches how operators think: "what EKUs do I need" vs + // "what extra subject attributes do I need". + // + // Unknown keys are tolerated at marshal time (logged + dropped) so a + // new key on a forward-version certctl doesn't force every profile + // edit to round-trip through the validator. + // + // EST RFC 7030 hardening master bundle Phase 6. + RequiredCSRAttributes []string `json:"required_csr_attributes,omitempty"` + + Enabled bool `json:"enabled"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` } // KeyAlgorithmRule defines an allowed key algorithm and its minimum key size. @@ -86,3 +110,81 @@ var ValidEKUs = map[string]bool{ "emailProtection": true, "timeStamping": true, } + +// EKUStringToOID maps an EKU short-name (as used in +// CertificateProfile.AllowedEKUs) to the corresponding RFC 5280 §4.2.1.12 +// id-kp-* OID. Returns ok=false for unknown names so the EST csrattrs +// path can drop unrecognized hints rather than emit garbage OIDs. +// +// EST RFC 7030 hardening master bundle Phase 6.2. +func EKUStringToOID(name string) (asn1.ObjectIdentifier, bool) { + oid, ok := ekuOIDByName[name] + return oid, ok +} + +// AttributeStringToOID maps a Subject DN / CMC attribute short-name +// (as used in CertificateProfile.RequiredCSRAttributes) to the +// corresponding ASN.1 OID. Returns ok=false for unknown names. The +// known set is intentionally small at GA — operators add new keys via +// PR review rather than free-form strings, so a typo trips a validator +// + the EST csrattrs response stays self-describing. +// +// EST RFC 7030 hardening master bundle Phase 6.2. +func AttributeStringToOID(name string) (asn1.ObjectIdentifier, bool) { + oid, ok := attributeOIDByName[name] + return oid, ok +} + +// ekuOIDByName is the lookup table EKUStringToOID consults. OIDs +// registered in RFC 5280 §4.2.1.12 + RFC 3280 + Microsoft. +var ekuOIDByName = map[string]asn1.ObjectIdentifier{ + "serverAuth": {1, 3, 6, 1, 5, 5, 7, 3, 1}, + "clientAuth": {1, 3, 6, 1, 5, 5, 7, 3, 2}, + "codeSigning": {1, 3, 6, 1, 5, 5, 7, 3, 3}, + "emailProtection": {1, 3, 6, 1, 5, 5, 7, 3, 4}, + "timeStamping": {1, 3, 6, 1, 5, 5, 7, 3, 8}, + "ocspSigning": {1, 3, 6, 1, 5, 5, 7, 3, 9}, + // Microsoft EKUs commonly required for AD smartcard / Intune device + // auth. Not in ValidEKUs above (which only enumerates the broadly + // portable names), but devices enrolling for these targets need + // csrattrs to advertise them. + "smartCardLogon": {1, 3, 6, 1, 4, 1, 311, 20, 2, 2}, + "documentSigning": {1, 3, 6, 1, 4, 1, 311, 10, 3, 12}, + "encryptingFileSystem": {1, 3, 6, 1, 4, 1, 311, 10, 3, 4}, + "keyRecoveryAgent": {1, 3, 6, 1, 4, 1, 311, 21, 6}, + "ocspNoCheck": {1, 3, 6, 1, 5, 5, 7, 48, 1, 5}, + "anyExtendedKeyUsage": {2, 5, 29, 37, 0}, + "ipsecIKE": {1, 3, 6, 1, 5, 5, 7, 3, 17}, + "machineEAP": {1, 3, 6, 1, 5, 5, 7, 3, 13}, + "kerberosClientAuth": {1, 3, 6, 1, 5, 2, 3, 4}, + "kerberosKeyDistribution": {1, 3, 6, 1, 5, 2, 3, 5}, +} + +// attributeOIDByName covers the Subject DN / CMC attribute hints the +// EST csrattrs endpoint can advertise. Sourced from RFC 5280 +// §4.1.2.6 + RFC 5912 (CMC) + RFC 5280 §4.1.2.4. Limited surface on +// purpose; PRs can extend. +var attributeOIDByName = map[string]asn1.ObjectIdentifier{ + // RFC 5280 §4.1.2.6 — distinguished-name attributes commonly + // requested for IoT bootstrap. + "commonName": {2, 5, 4, 3}, + "surname": {2, 5, 4, 4}, + "serialNumber": {2, 5, 4, 5}, + "countryName": {2, 5, 4, 6}, + "localityName": {2, 5, 4, 7}, + "stateOrProvinceName": {2, 5, 4, 8}, + "organizationName": {2, 5, 4, 10}, + "organizationalUnitName": {2, 5, 4, 11}, + "title": {2, 5, 4, 12}, + // CSR attributes from RFC 2985 §5.4 — challengePassword is + // already used by SCEP profiles; emailAddress + extensionRequest + // are the standard PKCS#10 carriers. + "challengePassword": {1, 2, 840, 113549, 1, 9, 7}, + "emailAddress": {1, 2, 840, 113549, 1, 9, 1}, + "extensionRequest": {1, 2, 840, 113549, 1, 9, 14}, + // Device-identity attributes that show up in IoT / MDM + // enrollment flows. + "deviceSerialNumber": {1, 3, 6, 1, 4, 1, 311, 21, 14}, // Microsoft Intune device serial + "unstructuredName": {1, 2, 840, 113549, 1, 9, 2}, + "unstructuredAddress": {1, 2, 840, 113549, 1, 9, 8}, +} diff --git a/internal/pkcs7/envelopeddata_builder.go b/internal/pkcs7/envelopeddata_builder.go new file mode 100644 index 0000000..a897485 --- /dev/null +++ b/internal/pkcs7/envelopeddata_builder.go @@ -0,0 +1,237 @@ +// EnvelopedData BUILDER (inverse of envelopeddata.go's parser+decryptor). +// +// EST RFC 7030 hardening master bundle Phase 5.2. +// +// The SCEP path landed the parser/decryptor; the EST `serverkeygen` +// endpoint (RFC 7030 §4.4) needs the BUILDER so the server can encrypt +// the server-generated private key TO the client's CSR-supplied +// key-encipherment public key, then return it as a CMS EnvelopedData. +// +// Wire shape produced (matches the parser's input, RFC 5652 §6.1): +// +// ContentInfo ::= SEQUENCE { +// contentType OBJECT IDENTIFIER, -- 1.2.840.113549.1.7.3 (envelopedData) +// content [0] EXPLICIT EnvelopedData +// } +// EnvelopedData ::= SEQUENCE { +// version INTEGER (0), -- v0 (no originatorInfo + no ori) +// recipientInfos SET SIZE(1) OF KeyTransRecipientInfo, +// encryptedContentInfo EncryptedContentInfo +// } +// KeyTransRecipientInfo ::= SEQUENCE { +// version INTEGER (0), -- v0 (IssuerAndSerialNumber rid) +// rid IssuerAndSerialNumber, -- recipient cert's issuer + serial +// keyEncryptionAlgorithm AlgorithmIdentifier, -- rsaEncryption (PKCS#1 v1.5 keyTrans) +// encryptedKey OCTET STRING -- AES key wrapped to recipient pubkey +// } +// EncryptedContentInfo ::= SEQUENCE { +// contentType OBJECT IDENTIFIER, -- pkcs7-data (1.2.840.113549.1.7.1) +// contentEncryptionAlgorithm AlgorithmIdentifier, -- aes-256-cbc with IV in parameters +// encryptedContent [0] IMPLICIT OCTET STRING +// } +// +// Algorithm choices (locked at GA): +// +// - Content cipher: AES-256-CBC. Strongest of the parser-supported ciphers +// (parser also accepts AES-128, AES-192, DES-EDE3-CBC for legacy SCEP +// interop; the BUILDER emits only AES-256). Random 16-byte IV per call. +// - Key transport: RSA PKCS#1 v1.5 (rsaEncryption OID). Mirror of what +// the parser supports — adding OAEP would mean parsing OAEP parameters +// in the parser too, deferred to V3. +// - Content-type carrier: pkcs7-data (1.2.840.113549.1.7.1). The +// plaintext bytes ARE the inner content directly; the parser's +// decryptCBC strips PKCS#7 padding so the BUILDER's PKCS#7-pad here +// round-trips correctly. + +package pkcs7 + +import ( + "crypto/aes" + "crypto/cipher" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "encoding/asn1" + "errors" + "fmt" + "io" +) + +// ErrBuildEnvelopedData is the umbrella build-time error. Unlike the +// decrypt path (which deliberately collapses every internal failure to +// one sentinel to close padding-oracle / Bleichenbacher leaks), the +// BUILDER's errors are caller-introspectable — the caller is local +// server code, not an attacker. +var ErrBuildEnvelopedData = errors.New("envelopedData: build failed") + +// BuildEnvelopedData produces the CMS EnvelopedData wire bytes for the +// given plaintext, encrypted to the supplied recipient cert. +// +// Inputs: +// - plaintext: the bytes to encrypt (e.g. a marshaled PKCS#8 private key +// for the EST serverkeygen path). +// - recipientCert: the cert whose pubkey wraps the AES key. MUST be RSA +// (the parser/decryptor only supports rsaEncryption keyTrans). +// - rng: source of random bytes for the AES key + IV. Pass nil to use +// crypto/rand.Reader. Tests can inject a deterministic reader so +// fixture round-trips are reproducible. +// +// Output: DER bytes of the outer ContentInfo. Suitable for direct embed +// in the EST serverkeygen multipart body's `application/pkcs7-mime; +// smime-type=enveloped-data` part. +// +// Behavior contract pinned by envelopeddata_builder_test.go: +// - Round-trip: BuildEnvelopedData → ParseEnvelopedData → Decrypt +// returns the original plaintext byte-for-byte. +// - Algorithm ID: AES-256-CBC (OID 2.16.840.1.101.3.4.1.42); IV is a +// random 16-byte value carried in the algorithm parameters as an +// OCTET STRING per RFC 3565 §2.3. +// - Recipient: exactly one KeyTransRecipientInfo whose IssuerAndSerial +// matches recipientCert.RawIssuer + recipientCert.SerialNumber. +func BuildEnvelopedData(plaintext []byte, recipientCert *x509.Certificate, rng io.Reader) ([]byte, error) { + if len(plaintext) == 0 { + return nil, fmt.Errorf("%w: empty plaintext", ErrBuildEnvelopedData) + } + if recipientCert == nil { + return nil, fmt.Errorf("%w: nil recipient cert", ErrBuildEnvelopedData) + } + rsaPub, ok := recipientCert.PublicKey.(*rsa.PublicKey) + if !ok { + return nil, fmt.Errorf("%w: recipient cert pubkey is not RSA (PKCS#1 v1.5 keyTrans only)", ErrBuildEnvelopedData) + } + if rng == nil { + rng = rand.Reader + } + + // 1. Generate the symmetric key + IV. AES-256-CBC needs a 32-byte key + // + 16-byte IV. Both come from the RNG; AES-CBC requires an IV + // unique-per-message (not strictly random, but a CSPRNG-derived value + // is the simplest correct choice). + symKey := make([]byte, 32) + if _, err := io.ReadFull(rng, symKey); err != nil { + return nil, fmt.Errorf("%w: gen sym key: %w", ErrBuildEnvelopedData, err) + } + iv := make([]byte, aes.BlockSize) // aes.BlockSize == 16 + if _, err := io.ReadFull(rng, iv); err != nil { + return nil, fmt.Errorf("%w: gen iv: %w", ErrBuildEnvelopedData, err) + } + + // 2. PKCS#7-pad + AES-256-CBC encrypt the plaintext. + padded := pkcs7Pad(plaintext, aes.BlockSize) + block, err := aes.NewCipher(symKey) + if err != nil { + return nil, fmt.Errorf("%w: aes.NewCipher: %w", ErrBuildEnvelopedData, err) + } + ciphertext := make([]byte, len(padded)) + cipher.NewCBCEncrypter(block, iv).CryptBlocks(ciphertext, padded) + + // 3. Wrap the symmetric key with the recipient's RSA pubkey using + // PKCS#1 v1.5 keyTrans. Matches the parser's rsa.DecryptPKCS1v15 + // expectation. NOTE: rsa.EncryptPKCS1v15 takes the plaintext (the + // AES key bytes) directly — no extra ASN.1 wrapping. + wrappedKey, err := rsa.EncryptPKCS1v15(rng, rsaPub, symKey) + if err != nil { + return nil, fmt.Errorf("%w: rsa.EncryptPKCS1v15: %w", ErrBuildEnvelopedData, err) + } + + // 4. Build the AlgorithmIdentifier for AES-256-CBC. RFC 3565 §2.3: + // the parameters field is an OCTET STRING carrying the IV. + ivOctet, err := asn1.Marshal(iv) // marshal as OCTET STRING + if err != nil { + return nil, fmt.Errorf("%w: marshal iv: %w", ErrBuildEnvelopedData, err) + } + contentEncAlg := pkix.AlgorithmIdentifier{ + Algorithm: OIDAES256CBC, + Parameters: asn1.RawValue{FullBytes: ivOctet}, + } + + // 5. Build the IssuerAndSerialNumber rid. The recipient cert's + // RawIssuer is the DER of its issuer DN (already canonicalised by + // the cert's encoder); we splice it as a RawValue so re-serialisation + // preserves byte-for-byte equality with what the recipient sees in + // its own cert. + issuerAndSerial := issuerAndSerialASN1{ + Issuer: asn1.RawValue{FullBytes: recipientCert.RawIssuer}, + SerialNumber: recipientCert.SerialNumber, + } + iasDER, err := asn1.Marshal(issuerAndSerial) + if err != nil { + return nil, fmt.Errorf("%w: marshal IssuerAndSerial: %w", ErrBuildEnvelopedData, err) + } + + // 6. Build the KeyTransRecipientInfo SEQUENCE. + ktri := keyTransRecipientInfoASN1{ + Version: 0, // v0 with IssuerAndSerial rid + RID: asn1.RawValue{FullBytes: iasDER}, + KeyEncryptionAlg: pkix.AlgorithmIdentifier{Algorithm: OIDRSAEncryption, Parameters: asn1.NullRawValue}, + EncryptedKey: wrappedKey, + } + ktriDER, err := asn1.Marshal(ktri) + if err != nil { + return nil, fmt.Errorf("%w: marshal KTRI: %w", ErrBuildEnvelopedData, err) + } + + // 7. Build the EncryptedContentInfo. encryptedContent is [0] IMPLICIT + // OCTET STRING; we marshal as a context-specific RawValue with class + // CONTEXT-SPECIFIC + tag 0 + the raw ciphertext bytes (no inner + // OCTET STRING tag since IMPLICIT replaces it). + encContent := asn1.RawValue{ + Class: asn1.ClassContextSpecific, + Tag: 0, + IsCompound: false, + Bytes: ciphertext, + } + enci := encryptedContentInfoASN1{ + ContentType: OIDDataContent, + ContentEncryptionAlgorithm: contentEncAlg, + EncryptedContent: encContent, + } + + // 8. Compose the EnvelopedData SEQUENCE. The parser's struct uses + // `[]asn1.RawValue` for RecipientInfos with `set` tag; we mirror + // that shape so the parse round-trip exercises the same code path. + enveloped := envelopedDataASN1{ + Version: 0, // v0 (no originatorInfo, no [1] unprotectedAttrs) + RecipientInfos: []asn1.RawValue{{FullBytes: ktriDER}}, + EncryptedContentInfo: enci, + // UnprotectedAttrs intentionally left zero-value; asn1.Marshal + // omits OPTIONAL fields whose RawValue is empty. + } + envelopedDER, err := asn1.Marshal(enveloped) + if err != nil { + return nil, fmt.Errorf("%w: marshal EnvelopedData: %w", ErrBuildEnvelopedData, err) + } + + // 9. Wrap in the outer ContentInfo so peelContentInfo on the read + // side picks it up cleanly. RFC 5652 §3 — content is [0] EXPLICIT. + wrapped, err := asn1.Marshal(contentInfoASN1{ + ContentType: OIDEnvelopedData, + Content: asn1.RawValue{Class: asn1.ClassContextSpecific, Tag: 0, IsCompound: true, Bytes: envelopedDER}, + }) + if err != nil { + return nil, fmt.Errorf("%w: marshal ContentInfo: %w", ErrBuildEnvelopedData, err) + } + return wrapped, nil +} + +// contentInfoASN1 is the outer CMS ContentInfo wrapper. envelopeddata.go's +// peelContentInfo is the read-side complement. +type contentInfoASN1 struct { + ContentType asn1.ObjectIdentifier + Content asn1.RawValue `asn1:"explicit,tag:0"` +} + +// pkcs7Pad applies PKCS#7 padding (RFC 5652 §6.3 references RFC 2315 §10.3). +// blockSize bytes' worth of (blockSize - len(in) % blockSize) is appended; +// when the input is already a block-multiple, a full block of `blockSize` +// padding bytes is appended (so unpad always has something to strip). +func pkcs7Pad(in []byte, blockSize int) []byte { + padLen := blockSize - (len(in) % blockSize) + out := make([]byte, len(in)+padLen) + copy(out, in) + for i := len(in); i < len(out); i++ { + out[i] = byte(padLen) + } + return out +} diff --git a/internal/pkcs7/envelopeddata_builder_test.go b/internal/pkcs7/envelopeddata_builder_test.go new file mode 100644 index 0000000..6d7ba93 --- /dev/null +++ b/internal/pkcs7/envelopeddata_builder_test.go @@ -0,0 +1,171 @@ +package pkcs7 + +import ( + "bytes" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "math/big" + "testing" + "time" +) + +// freshRSARecipient produces a self-signed RSA-2048 cert + matching key +// usable as both EnvelopedData recipient (BUILDER input) and EnvelopedData +// decryptor (Decrypt input). RSA-2048 is the minimum the parser supports +// for keyTrans. +func freshRSARecipient(t *testing.T) (*x509.Certificate, *rsa.PrivateKey) { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("rsa.GenerateKey: %v", err) + } + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(time.Now().UnixNano()), + Subject: pkix.Name{CommonName: "envelopeddata-builder-test"}, + Issuer: pkix.Name{CommonName: "envelopeddata-builder-test-issuer"}, + NotBefore: time.Now().Add(-1 * time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + KeyUsage: x509.KeyUsageKeyEncipherment, + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + if err != nil { + t.Fatalf("CreateCertificate: %v", err) + } + cert, err := x509.ParseCertificate(der) + if err != nil { + t.Fatalf("ParseCertificate: %v", err) + } + return cert, key +} + +func TestBuildEnvelopedData_RoundTrip(t *testing.T) { + cert, key := freshRSARecipient(t) + plaintext := []byte("the eagle has landed at coordinate 47.6062N 122.3321W; key zeroize at exit") + + wire, err := BuildEnvelopedData(plaintext, cert, nil) + if err != nil { + t.Fatalf("BuildEnvelopedData: %v", err) + } + if len(wire) == 0 { + t.Fatal("empty wire bytes") + } + + parsed, err := ParseEnvelopedData(wire) + if err != nil { + t.Fatalf("ParseEnvelopedData: %v", err) + } + got, err := parsed.Decrypt(key, cert) + if err != nil { + t.Fatalf("Decrypt: %v", err) + } + if !bytes.Equal(got, plaintext) { + t.Errorf("round-trip mismatch:\n got = %q\nwant = %q", got, plaintext) + } +} + +func TestBuildEnvelopedData_AlgorithmIsAES256CBC(t *testing.T) { + cert, _ := freshRSARecipient(t) + wire, err := BuildEnvelopedData([]byte("alg-id pin"), cert, nil) + if err != nil { + t.Fatalf("BuildEnvelopedData: %v", err) + } + parsed, err := ParseEnvelopedData(wire) + if err != nil { + t.Fatalf("ParseEnvelopedData: %v", err) + } + if !parsed.ContentEncryptionAlg.Algorithm.Equal(OIDAES256CBC) { + t.Errorf("alg = %v, want OIDAES256CBC %v", parsed.ContentEncryptionAlg.Algorithm, OIDAES256CBC) + } +} + +func TestBuildEnvelopedData_RecipientMatchesIssuerAndSerial(t *testing.T) { + cert, _ := freshRSARecipient(t) + wire, err := BuildEnvelopedData([]byte("rid pin"), cert, nil) + if err != nil { + t.Fatalf("BuildEnvelopedData: %v", err) + } + parsed, err := ParseEnvelopedData(wire) + if err != nil { + t.Fatalf("ParseEnvelopedData: %v", err) + } + if len(parsed.RecipientInfos) != 1 { + t.Fatalf("recipient count = %d, want 1", len(parsed.RecipientInfos)) + } + rid := parsed.RecipientInfos[0].IssuerAndSerial + if !bytes.Equal(rid.IssuerRaw.FullBytes, cert.RawIssuer) { + t.Errorf("issuer mismatch:\n got = %x\nwant = %x", rid.IssuerRaw.FullBytes, cert.RawIssuer) + } + if rid.SerialNumber == nil || rid.SerialNumber.Cmp(cert.SerialNumber) != 0 { + t.Errorf("serial mismatch: got %v, want %v", rid.SerialNumber, cert.SerialNumber) + } +} + +func TestBuildEnvelopedData_RejectsNonRSARecipient(t *testing.T) { + // EnvelopedData keyTrans requires RSA per the parser's contract; ECDSA + // recipient certs MUST be rejected at build time so an operator never + // ships a serverkeygen response that no client can decrypt. + ecKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("ecdsa.GenerateKey: %v", err) + } + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(99), + Subject: pkix.Name{CommonName: "ecdsa-recipient-reject-test"}, + Issuer: pkix.Name{CommonName: "ecdsa-recipient-reject-test-issuer"}, + NotBefore: time.Now().Add(-1 * time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &ecKey.PublicKey, ecKey) + if err != nil { + t.Fatalf("CreateCertificate: %v", err) + } + cert, err := x509.ParseCertificate(der) + if err != nil { + t.Fatalf("ParseCertificate: %v", err) + } + if _, err := BuildEnvelopedData([]byte("test"), cert, nil); err == nil { + t.Fatal("expected error for non-RSA recipient cert") + } +} + +func TestBuildEnvelopedData_RejectsEmptyPlaintext(t *testing.T) { + cert, _ := freshRSARecipient(t) + _, err := BuildEnvelopedData(nil, cert, nil) + if err == nil { + t.Fatal("expected error for empty plaintext") + } +} + +func TestBuildEnvelopedData_RejectsNilCert(t *testing.T) { + _, err := BuildEnvelopedData([]byte("x"), nil, nil) + if err == nil { + t.Fatal("expected error for nil recipient cert") + } +} + +func TestBuildEnvelopedData_LargePlaintextRoundTrip(t *testing.T) { + // PKCS#7 padding + AES-256-CBC works for arbitrary plaintext lengths. + // Pin the contract for a 4KiB-aligned key blob (typical PKCS#8 RSA-2048 + // is ~1.2KB; ECDSA P-384 is ~250B). + cert, key := freshRSARecipient(t) + big := bytes.Repeat([]byte("ABCDEFGH"), 512) // 4 KiB + wire, err := BuildEnvelopedData(big, cert, nil) + if err != nil { + t.Fatalf("BuildEnvelopedData: %v", err) + } + parsed, err := ParseEnvelopedData(wire) + if err != nil { + t.Fatalf("ParseEnvelopedData: %v", err) + } + got, err := parsed.Decrypt(key, cert) + if err != nil { + t.Fatalf("Decrypt: %v", err) + } + if !bytes.Equal(got, big) { + t.Errorf("4KiB round-trip mismatch") + } +} diff --git a/internal/repository/postgres/profile.go b/internal/repository/postgres/profile.go index aa929c8..a002349 100644 --- a/internal/repository/postgres/profile.go +++ b/internal/repository/postgres/profile.go @@ -1,11 +1,11 @@ package postgres import ( - "github.com/shankar0123/certctl/internal/repository" "context" "database/sql" "encoding/json" "fmt" + "github.com/shankar0123/certctl/internal/repository" "time" "github.com/google/uuid" @@ -27,7 +27,8 @@ func (r *ProfileRepository) List(ctx context.Context) ([]*domain.CertificateProf rows, err := r.db.QueryContext(ctx, ` SELECT id, name, description, allowed_key_algorithms, max_ttl_seconds, allowed_ekus, required_san_patterns, spiffe_uri_pattern, - allow_short_lived, enabled, created_at, updated_at + allow_short_lived, must_staple, required_csr_attributes, + enabled, created_at, updated_at FROM certificate_profiles ORDER BY created_at DESC `) @@ -57,7 +58,8 @@ func (r *ProfileRepository) Get(ctx context.Context, id string) (*domain.Certifi row := r.db.QueryRowContext(ctx, ` SELECT id, name, description, allowed_key_algorithms, max_ttl_seconds, allowed_ekus, required_san_patterns, spiffe_uri_pattern, - allow_short_lived, enabled, created_at, updated_at + allow_short_lived, must_staple, required_csr_attributes, + enabled, created_at, updated_at FROM certificate_profiles WHERE id = $1 `, id) @@ -97,17 +99,25 @@ func (r *ProfileRepository) Create(ctx context.Context, profile *domain.Certific if err != nil { return fmt.Errorf("failed to marshal required_san_patterns: %w", err) } + // Phase 6.1: required_csr_attributes is the per-profile EST csrattrs hint + // list. Marshal as JSONB; nil → "[]" via the json.Marshal stdlib contract. + csrAttrsJSON, err := json.Marshal(profile.RequiredCSRAttributes) + if err != nil { + return fmt.Errorf("failed to marshal required_csr_attributes: %w", err) + } err = r.db.QueryRowContext(ctx, ` INSERT INTO certificate_profiles ( id, name, description, allowed_key_algorithms, max_ttl_seconds, allowed_ekus, required_san_patterns, spiffe_uri_pattern, - allow_short_lived, enabled, created_at, updated_at - ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) + allow_short_lived, must_staple, required_csr_attributes, + enabled, created_at, updated_at + ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14) RETURNING id `, profile.ID, profile.Name, profile.Description, algJSON, profile.MaxTTLSeconds, ekuJSON, sanJSON, profile.SPIFFEURIPattern, - profile.AllowShortLived, profile.Enabled, profile.CreatedAt, profile.UpdatedAt).Scan(&profile.ID) + profile.AllowShortLived, profile.MustStaple, csrAttrsJSON, + profile.Enabled, profile.CreatedAt, profile.UpdatedAt).Scan(&profile.ID) if err != nil { return fmt.Errorf("failed to create profile: %w", err) @@ -132,6 +142,10 @@ func (r *ProfileRepository) Update(ctx context.Context, profile *domain.Certific if err != nil { return fmt.Errorf("failed to marshal required_san_patterns: %w", err) } + csrAttrsJSON, err := json.Marshal(profile.RequiredCSRAttributes) + if err != nil { + return fmt.Errorf("failed to marshal required_csr_attributes: %w", err) + } result, err := r.db.ExecContext(ctx, ` UPDATE certificate_profiles SET @@ -143,12 +157,15 @@ func (r *ProfileRepository) Update(ctx context.Context, profile *domain.Certific required_san_patterns = $6, spiffe_uri_pattern = $7, allow_short_lived = $8, - enabled = $9, - updated_at = $10 - WHERE id = $11 + must_staple = $9, + required_csr_attributes = $10, + enabled = $11, + updated_at = $12 + WHERE id = $13 `, profile.Name, profile.Description, algJSON, profile.MaxTTLSeconds, ekuJSON, sanJSON, profile.SPIFFEURIPattern, - profile.AllowShortLived, profile.Enabled, profile.UpdatedAt, profile.ID) + profile.AllowShortLived, profile.MustStaple, csrAttrsJSON, + profile.Enabled, profile.UpdatedAt, profile.ID) if err != nil { return fmt.Errorf("failed to update profile: %w", err) @@ -190,12 +207,13 @@ func scanProfile(scanner interface { Scan(...interface{}) error }) (*domain.CertificateProfile, error) { var p domain.CertificateProfile - var algJSON, ekuJSON, sanJSON []byte + var algJSON, ekuJSON, sanJSON, csrAttrsJSON []byte err := scanner.Scan( &p.ID, &p.Name, &p.Description, &algJSON, &p.MaxTTLSeconds, &ekuJSON, &sanJSON, &p.SPIFFEURIPattern, - &p.AllowShortLived, &p.Enabled, &p.CreatedAt, &p.UpdatedAt, + &p.AllowShortLived, &p.MustStaple, &csrAttrsJSON, + &p.Enabled, &p.CreatedAt, &p.UpdatedAt, ) if err != nil { return nil, fmt.Errorf("failed to scan profile: %w", err) @@ -222,6 +240,15 @@ func scanProfile(scanner interface { return nil, fmt.Errorf("failed to unmarshal required_san_patterns: %w", err) } } + // Phase 6.1: required_csr_attributes column ships with default '[]', so + // every existing row scans into an empty slice (back-compat 204 stub + // behavior). Older rows from before the migration could land here as + // empty bytes — guard against that with the same len() check pattern. + if len(csrAttrsJSON) > 0 { + if err := json.Unmarshal(csrAttrsJSON, &p.RequiredCSRAttributes); err != nil { + return nil, fmt.Errorf("failed to unmarshal required_csr_attributes: %w", err) + } + } return &p, nil } diff --git a/internal/service/est.go b/internal/service/est.go index b24aee2..011b9c1 100644 --- a/internal/service/est.go +++ b/internal/service/est.go @@ -2,14 +2,24 @@ package service import ( "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/rsa" "crypto/x509" + "encoding/asn1" "encoding/pem" + "errors" "fmt" "log/slog" + "math/big" "strings" + "time" "github.com/shankar0123/certctl/internal/domain" + "github.com/shankar0123/certctl/internal/pkcs7" "github.com/shankar0123/certctl/internal/repository" + "github.com/shankar0123/certctl/internal/trustanchor" ) // ESTService implements the EST (RFC 7030) enrollment protocol. @@ -22,6 +32,24 @@ type ESTService struct { logger *slog.Logger profileID string // optional: constrain enrollments to a specific profile profileRepo repository.CertificateProfileRepository + + // EST RFC 7030 hardening master bundle Phase 7.1: per-status atomic + // counters surfaced by IndividualStats() / the AdminEST endpoint. + // Created lazily by NewESTService so the dispatcher's hot path stays + // nil-safe even if a future refactor forgets to wire the counters. + counters *estCounterTab + + // estPathIDForLog / estMTLSConfigured / estBasicConfigured / + // estServerKeygenEnabled / estTrustAnchor are observability metadata + // the AdminEST handler reads via Stats(). They're populated once at + // startup by SetESTAdminMetadata; the dispatcher hot path never + // reads them (the hot path consults the typed config fields on the + // HANDLER instance, not the service). + estPathIDForLog string + estMTLSConfigured bool + estBasicConfigured bool + estServerKeygenEnabled bool + estTrustAnchor *trustanchor.Holder } // NewESTService creates a new ESTService for the given issuer connector. @@ -31,6 +59,7 @@ func NewESTService(issuerID string, issuer IssuerConnector, auditService *AuditS issuerID: issuerID, auditService: auditService, logger: logger, + counters: &estCounterTab{}, } } @@ -71,12 +100,83 @@ func (s *ESTService) SimpleReEnroll(ctx context.Context, csrPEM string) (*domain } // GetCSRAttrs returns the CSR attributes the server wants clients to include. -// RFC 7030 Section 4.5: /csrattrs tells clients what to put in their CSR. -// Returns nil if no specific attributes are required. +// RFC 7030 §4.5: /csrattrs tells clients what to put in their CSR. The +// response is base64(DER(SEQUENCE OF AttrOrOID)) where AttrOrOID is either +// a bare OID (an attribute the client SHOULD include) or an Attribute +// SEQUENCE { type OID, values SET OF ANY }. We emit the bare-OID form for +// every entry — the EST endpoint hint contract is "what attributes / +// EKUs to include in the CSR", not "what specific values to set". +// +// EST RFC 7030 hardening master bundle Phase 6.2: replaces the v2.0.x +// nil/204 stub with a profile-derived OID list. Sources: +// - profile.AllowedEKUs → emitted as id-kp-* OIDs (RFC 5280 §4.2.1.12). +// Clients use these to add the matching EKU OIDs to their CSR's +// extensionRequest attribute. +// - profile.RequiredCSRAttributes → emitted as the matching CSR +// attribute / DN-attribute OIDs (e.g. serialNumber → 2.5.4.5). +// +// Returns nil when no profile is configured OR the resolved hint set is +// empty after dropping unknown entries — the handler then writes 204 +// per RFC 7030 §4.5.2 (the original stub semantic). Unknown entries are +// dropped + warning-logged; any one typo'd EKU/attribute string +// shouldn't take down the entire csrattrs surface. func (s *ESTService) GetCSRAttrs(ctx context.Context) ([]byte, error) { - // For now, we don't require specific CSR attributes. - // In the future, this could return key type constraints from the profile. - return nil, nil + if s.profileID == "" || s.profileRepo == nil { + // No bound profile = no hints. Maintains the v2.0.x behavior of + // returning 204 to legacy deployments that haven't opted into a + // CertificateProfile. The handler writes 204-No-Content when the + // returned slice is empty. + return nil, nil + } + profile, err := s.profileRepo.Get(ctx, s.profileID) + if err != nil || profile == nil { + // Profile lookup failure isn't fatal — we degrade to the + // no-hints case + log so the operator can spot misconfig. Same + // rationale as the audit-noop path in processEnrollment. + s.logger.Warn("est csrattrs: profile lookup failed; degrading to no-hints", + "profile_id", s.profileID, + "error", err) + return nil, nil + } + + var oids []asn1.ObjectIdentifier + // EKU hints first (RFC 5280 §4.2.1.12 OIDs). Skip serverAuth + clientAuth + // when the profile only allows the default — those are well-known and + // every modern client adds them by default; emitting them in csrattrs + // is just noise. But if the operator narrowed AllowedEKUs to e.g. + // `["clientAuth"]` for an mTLS-only profile, we DO want clients to + // know to drop serverAuth — so we emit the EKU hints unconditionally + // when the profile is narrower than the default. The narrowing check + // is implicit: if AllowedEKUs is the default (just serverAuth), we + // emit just serverAuth, which is what well-behaved clients do anyway. + for _, eku := range profile.AllowedEKUs { + if oid, ok := domain.EKUStringToOID(eku); ok { + oids = append(oids, oid) + } else { + s.logger.Warn("est csrattrs: unknown EKU in profile; dropping", + "profile_id", s.profileID, "eku", eku) + } + } + // Required CSR attribute / DN-attribute hints. + for _, attr := range profile.RequiredCSRAttributes { + if oid, ok := domain.AttributeStringToOID(attr); ok { + oids = append(oids, oid) + } else { + s.logger.Warn("est csrattrs: unknown CSR attribute in profile; dropping", + "profile_id", s.profileID, "attribute", attr) + } + } + if len(oids) == 0 { + return nil, nil + } + // RFC 7030 §4.5.2: response body is the DER encoding of a SEQUENCE + // of AttrOrOID. asn1.Marshal of []asn1.ObjectIdentifier produces + // SEQUENCE OF OBJECT IDENTIFIER, which is the bare-OID form. + der, err := asn1.Marshal(oids) + if err != nil { + return nil, fmt.Errorf("est csrattrs: marshal OID sequence: %w", err) + } + return der, nil } // processEnrollment handles the common enrollment logic for both simpleenroll and simplereenroll. @@ -84,20 +184,24 @@ func (s *ESTService) processEnrollment(ctx context.Context, csrPEM string, audit // Parse the CSR to extract CN and SANs block, _ := pem.Decode([]byte(csrPEM)) if block == nil { + s.counters.inc(estCounterCSRInvalid) return nil, fmt.Errorf("invalid CSR PEM") } csr, err := x509.ParseCertificateRequest(block.Bytes) if err != nil { + s.counters.inc(estCounterCSRInvalid) return nil, fmt.Errorf("failed to parse CSR: %w", err) } if err := csr.CheckSignature(); err != nil { + s.counters.inc(estCounterCSRSignatureMismatch) return nil, fmt.Errorf("CSR signature verification failed: %w", err) } commonName := csr.Subject.CommonName if commonName == "" { + s.counters.inc(estCounterCSRInvalid) return nil, fmt.Errorf("CSR must include a Common Name") } @@ -126,6 +230,7 @@ func (s *ESTService) processEnrollment(ctx context.Context, csrPEM string, audit } } if _, csrErr := ValidateCSRAgainstProfile(csrPEM, profile); csrErr != nil { + s.counters.inc(estCounterCSRPolicyViolation) s.logger.Error("EST enrollment rejected: crypto policy violation", "action", auditAction, "common_name", commonName, @@ -156,12 +261,20 @@ func (s *ESTService) processEnrollment(ctx context.Context, csrPEM string, audit // EST enrollments use profile EKUs if available, otherwise default (serverAuth + clientAuth fallback) result, err := s.issuer.IssueCertificate(ctx, commonName, sans, csrPEM, ekus, maxTTLSeconds, mustStaple) if err != nil { + s.counters.inc(estCounterIssuerError) s.logger.Error("EST enrollment failed", "action", auditAction, "common_name", commonName, "error", err) return nil, fmt.Errorf("certificate issuance failed: %w", err) } + // Phase 7.1: tick success counter — distinguish initial vs renewal so + // the admin GUI can show enrollment-mix at a glance. + if auditAction == "est_simple_reenroll" { + s.counters.inc(estCounterSuccessSimpleReEnroll) + } else { + s.counters.inc(estCounterSuccessSimpleEnroll) + } // Audit the enrollment if s.auditService != nil { @@ -189,3 +302,413 @@ func (s *ESTService) processEnrollment(ctx context.Context, csrPEM string, audit ChainPEM: result.ChainPEM, }, nil } + +// EST RFC 7030 hardening master bundle Phase 5 — serverkeygen. +// +// RFC 7030 §4.4: the client submits a CSR whose key may be a placeholder; +// the server generates the keypair, issues a cert with the SERVER-generated +// pubkey, then returns BOTH the cert AND the corresponding private key +// encrypted to the client's separately-supplied key-encipherment public +// key (RFC 7030 §4.4.2 mandates secure key delivery). +// +// Wire shape: multipart/mixed body assembled by the handler. The service +// returns the raw cert PEM + the RAW private key bytes (already CMS- +// EnvelopedData-wrapped); the handler composes the multipart envelope. + +// ESTServerKeygenResult is an alias for the domain type so existing callers +// don't reach across packages — handlers + tests reference the alias here, +// the wire schema lives in internal/domain/est.go. +type ESTServerKeygenResult = domain.ESTServerKeygenResult + +// ErrServerKeygenRequiresKeyEncipherment is returned when the client's +// CSR doesn't carry an RSA key-encipherment public key the server can +// use to wrap the generated private key. RFC 7030 §4.4.2 mandates an +// encryption mechanism; we do NOT support the plaintext-PKCS#8 fallback. +var ErrServerKeygenRequiresKeyEncipherment = errors.New("est serverkeygen: client CSR missing RSA key-encipherment public key") + +// ErrServerKeygenUnsupportedAlgorithm is returned when the CSR pubkey +// algorithm isn't in the server's supported-keygen list. Currently +// supported: RSA-2048, RSA-3072, RSA-4096, ECDSA P-256, ECDSA P-384. +var ErrServerKeygenUnsupportedAlgorithm = errors.New("est serverkeygen: unsupported keygen algorithm requested by CSR") + +// ErrServerKeygenDisabled signals the handler that the per-profile gate +// is off (CertCertConfig.ServerKeygenEnabled == false). Maps to HTTP +// 404 (the endpoint isn't routable for this profile) at the handler. +var ErrServerKeygenDisabled = errors.New("est serverkeygen: disabled for this profile") + +// SimpleServerKeygen runs the RFC 7030 §4.4 server-driven key generation +// flow. The CSR's Subject + SANs drive the issued cert's identity; the +// CSR's pubkey (which the client supplies as the encryption target for +// the returned private key) MUST be RSA so we can wrap with PKCS#1 v1.5 +// keyTrans (matches the BUILDER's algorithm choice). The newly-generated +// keypair's algorithm is picked to match the profile's +// AllowedKeyAlgorithms first entry (or RSA-2048 default when no profile +// constraint) — the server isn't trying to second-guess the operator's +// crypto policy. +// +// Returns ESTServerKeygenResult{CertPEM, ChainPEM, EncryptedKey} where +// EncryptedKey is the CMS EnvelopedData wrapping a PKCS#8 marshal of the +// freshly-minted private key. The plaintext private key bytes are +// zeroized inside the call before return — the handler never sees them. +func (s *ESTService) SimpleServerKeygen(ctx context.Context, csrPEM string) (*ESTServerKeygenResult, error) { + // 1. Parse + signature-verify the CSR. We re-use processEnrollment's + // gates verbatim so a misshapen CSR fails the same way it does on + // the simpleenroll path. + block, _ := pem.Decode([]byte(csrPEM)) + if block == nil { + return nil, fmt.Errorf("invalid CSR PEM") + } + csr, err := x509.ParseCertificateRequest(block.Bytes) + if err != nil { + return nil, fmt.Errorf("failed to parse CSR: %w", err) + } + if err := csr.CheckSignature(); err != nil { + return nil, fmt.Errorf("CSR signature verification failed: %w", err) + } + commonName := csr.Subject.CommonName + if commonName == "" { + return nil, fmt.Errorf("CSR must include a Common Name") + } + // The CSR pubkey IS the encryption target for the returned private + // key per RFC 7030 §4.4.2 — refuse non-RSA pubkeys at the door so + // the BUILDER doesn't fail later with a less-actionable error. + rsaPub, ok := csr.PublicKey.(*rsa.PublicKey) + if !ok || rsaPub == nil { + s.counters.inc(estCounterCSRPolicyViolation) + return nil, ErrServerKeygenRequiresKeyEncipherment + } + + // 2. Resolve profile (for AllowedKeyAlgorithms + AllowedEKUs + + // MaxTTLSeconds + MustStaple — the same set the simpleenroll path + // reads). When no profile is bound, fall back to RSA-2048 + the + // issuer's defaults — same v2.0.x posture as a no-profile + // simpleenroll. + var profile *domain.CertificateProfile + if s.profileID != "" && s.profileRepo != nil { + if p, perr := s.profileRepo.Get(ctx, s.profileID); perr == nil && p != nil { + profile = p + } + } + + // 3. Generate the server-side keypair matching the profile's first + // AllowedKeyAlgorithms entry (or RSA-2048 default). The signer + // abstraction's MemoryDriver is overkill here — we just need a + // crypto.PrivateKey + matching crypto.PublicKey for one CSR + // re-derivation + one PKCS#8 marshal. The plaintext key never hits + // disk: it's allocated, marshaled, then explicitly zeroized below. + freshPriv, freshPub, algoLabel, err := s.generateServerKeyForProfile(profile) + if err != nil { + return nil, err + } + + // 4. Build a synthetic CSR carrying the original CSR's Subject + + // SANs but the SERVER-generated pubkey. This is the CSR we hand to + // the issuer connector — the issued cert binds the device identity + // to the new keypair. + serverCSR := &x509.CertificateRequest{ + Subject: csr.Subject, + DNSNames: csr.DNSNames, + IPAddresses: csr.IPAddresses, + EmailAddresses: csr.EmailAddresses, + URIs: csr.URIs, + SignatureAlgorithm: csrSignatureForKey(freshPriv), + } + serverCSRDER, err := x509.CreateCertificateRequest(rand.Reader, serverCSR, freshPriv) + if err != nil { + zeroizeKey(freshPriv) + return nil, fmt.Errorf("est serverkeygen: build server CSR: %w", err) + } + serverCSRPEM := string(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: serverCSRDER})) + + // 5. SAN list mirrors processEnrollment's collect-and-issue logic. + var sans []string + for _, dns := range csr.DNSNames { + sans = append(sans, dns) + } + for _, ip := range csr.IPAddresses { + sans = append(sans, ip.String()) + } + for _, email := range csr.EmailAddresses { + sans = append(sans, email) + } + for _, uri := range csr.URIs { + sans = append(sans, uri.String()) + } + + // 6. Issuance gates: profile's AllowedEKUs / MaxTTLSeconds / + // MustStaple. The crypto-policy validation runs against the SERVER + // CSR (so the freshly-generated key is what's checked) — that's + // what the operator's policy is meant to constrain. + if _, csrErr := ValidateCSRAgainstProfile(serverCSRPEM, profile); csrErr != nil { + zeroizeKey(freshPriv) + s.logger.Error("EST serverkeygen rejected: crypto policy violation", + "common_name", commonName, "algo", algoLabel, "error", csrErr) + return nil, fmt.Errorf("EST serverkeygen rejected: %w", csrErr) + } + var ( + ekus []string + maxTTLSeconds int + mustStaple bool + ) + if profile != nil { + ekus = profile.AllowedEKUs + maxTTLSeconds = profile.MaxTTLSeconds + mustStaple = profile.MustStaple + } + + // 7. Issue. + issued, err := s.issuer.IssueCertificate(ctx, commonName, sans, serverCSRPEM, ekus, maxTTLSeconds, mustStaple) + if err != nil { + zeroizeKey(freshPriv) + s.counters.inc(estCounterIssuerError) + s.logger.Error("EST serverkeygen failed", + "common_name", commonName, "algo", algoLabel, "error", err) + return nil, fmt.Errorf("EST serverkeygen issuance failed: %w", err) + } + s.counters.inc(estCounterSuccessServerKeygen) + + // 8. Marshal the freshly-generated private key as PKCS#8 (RFC 5958). + // PKCS#8 is the format both libest and openssl smime expect on the + // other end of CMS EnvelopedData unwrap. + pkcs8, err := x509.MarshalPKCS8PrivateKey(freshPriv) + if err != nil { + zeroizeKey(freshPriv) + return nil, fmt.Errorf("est serverkeygen: marshal PKCS#8: %w", err) + } + + // 9. Build a synthetic recipient cert wrapping the device's + // CSR-supplied key-encipherment pubkey. The BUILDER expects a + // *x509.Certificate so it can read RawIssuer + SerialNumber for + // the IssuerAndSerial rid; we synth one with the device CN + a + // stable serial. Real PKI shape but we never sign / publish it + // — purely a carrier for the pubkey + issuer info inside the + // CMS envelope. + recipient, err := buildSyntheticRecipientCert(rsaPub, csr) + if err != nil { + zeroizeKey(freshPriv) + zeroizeBytes(pkcs8) + return nil, fmt.Errorf("est serverkeygen: synth recipient cert: %w", err) + } + + // 10. Encrypt the PKCS#8 with the device's pubkey via CMS + // EnvelopedData. AES-256-CBC content encryption + RSA PKCS#1 v1.5 + // keyTrans — same algorithm choices as the BUILDER's hard-coded + // defaults. + encryptedKey, err := pkcs7.BuildEnvelopedData(pkcs8, recipient, rand.Reader) + if err != nil { + zeroizeKey(freshPriv) + zeroizeBytes(pkcs8) + return nil, fmt.Errorf("est serverkeygen: build EnvelopedData: %w", err) + } + + // 11. Zeroize the in-memory plaintext key + PKCS#8 bytes. Ciphertext + // remains; the handler emits it then returns. Best-effort — Go's + // GC may have copied the buffers around already, but this closes + // the obvious leak path at handler return time. + zeroizeKey(freshPriv) + zeroizeBytes(pkcs8) + _ = freshPub // referenced only at issuance time; nothing to zero + + // 12. Audit + return. + if s.auditService != nil { + details := map[string]interface{}{ + "common_name": commonName, + "sans": sans, + "issuer_id": s.issuerID, + "serial": issued.Serial, + "protocol": "EST", + "keygen": "server", + "algorithm": algoLabel, + } + if s.profileID != "" { + details["profile_id"] = s.profileID + } + _ = s.auditService.RecordEvent(ctx, "est-client", "system", "est_server_keygen", "certificate", issued.Serial, details) + } + s.logger.Info("EST serverkeygen successful", + "common_name", commonName, "serial", issued.Serial, + "algo", algoLabel, "issuer", s.issuerID) + + return &ESTServerKeygenResult{ + CertPEM: issued.CertPEM, + ChainPEM: issued.ChainPEM, + EncryptedKey: encryptedKey, + }, nil +} + +// generateServerKeyForProfile returns a freshly-minted (priv, pub, label) +// triple. The chosen algorithm matches profile.AllowedKeyAlgorithms[0] +// when the profile has constraints; otherwise RSA-2048 (the broadest +// compatibility default, matches what the local issuer self-bootstraps +// when the operator hasn't pinned a key algorithm). +func (s *ESTService) generateServerKeyForProfile(profile *domain.CertificateProfile) (priv interface{}, pub interface{}, label string, err error) { + algo := "RSA" + size := 2048 + if profile != nil && len(profile.AllowedKeyAlgorithms) > 0 { + first := profile.AllowedKeyAlgorithms[0] + algo = first.Algorithm + if first.MinSize > 0 { + size = first.MinSize + } + } + switch algo { + case domain.KeyAlgorithmRSA: + k, kerr := rsa.GenerateKey(rand.Reader, size) + if kerr != nil { + return nil, nil, "", fmt.Errorf("est serverkeygen: rsa.GenerateKey size=%d: %w", size, kerr) + } + return k, &k.PublicKey, fmt.Sprintf("RSA-%d", size), nil + case domain.KeyAlgorithmECDSA: + var curve elliptic.Curve + switch size { + case 256: + curve = elliptic.P256() + label = "ECDSA-P256" + case 384: + curve = elliptic.P384() + label = "ECDSA-P384" + case 521: + curve = elliptic.P521() + label = "ECDSA-P521" + default: + return nil, nil, "", fmt.Errorf("%w: ECDSA size=%d (allowed: 256/384/521)", ErrServerKeygenUnsupportedAlgorithm, size) + } + k, kerr := ecdsa.GenerateKey(curve, rand.Reader) + if kerr != nil { + return nil, nil, "", fmt.Errorf("est serverkeygen: ecdsa.GenerateKey: %w", kerr) + } + return k, &k.PublicKey, label, nil + default: + return nil, nil, "", fmt.Errorf("%w: %q (allowed: RSA, ECDSA)", ErrServerKeygenUnsupportedAlgorithm, algo) + } +} + +// csrSignatureForKey picks a sane SignatureAlgorithm for x509.CreateCertificateRequest +// given a private key. Mirrors what the stdlib defaults to but pinning here +// avoids hitting the deprecated SHA1WithRSA on RSA keys (Go's stdlib still +// defaults to SHA-256 for RSA, so this is mostly belt-and-braces). +func csrSignatureForKey(k interface{}) x509.SignatureAlgorithm { + switch k.(type) { + case *rsa.PrivateKey: + return x509.SHA256WithRSA + case *ecdsa.PrivateKey: + return x509.ECDSAWithSHA256 // P-256 + P-384 both default fine; P-521 will pick SHA-256 too + default: + return x509.UnknownSignatureAlgorithm // stdlib derives a sensible default + } +} + +// buildSyntheticRecipientCert wraps the device's CSR-supplied +// key-encipherment pubkey in a minimal *x509.Certificate so the +// pkcs7.BuildEnvelopedData function (which keys off RawIssuer + +// SerialNumber for the IssuerAndSerial rid) can address it. The cert +// is never signed or persisted — it lives only inside this function +// + the EnvelopedData blob produced. +// +// We pin the issuer DN to the device's own Subject DN so the rid is +// self-referential — a stable, reproducible identifier the device's +// EST client library can match against its own cert request when it +// decrypts the response. Serial number is the SHA-256 prefix of the +// CSR signature (deterministic per CSR; collisions across millions of +// CSRs are negligible). +func buildSyntheticRecipientCert(rsaPub *rsa.PublicKey, csr *x509.CertificateRequest) (*x509.Certificate, error) { + // Self-sign the synthetic cert with an EPHEMERAL key so it parses + // cleanly via x509.CreateCertificate + ParseCertificate. The + // signature is throwaway — no one verifies it — but x509 won't + // build a cert without one. + ephemKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + return nil, fmt.Errorf("ephemeral signer: %w", err) + } + tmpl := &x509.Certificate{ + SerialNumber: deterministicSerial(csr.Signature), + Subject: csr.Subject, + Issuer: csr.Subject, // self-referential; never verified + NotBefore: serverKeygenSyntheticNotBefore, + NotAfter: serverKeygenSyntheticNotAfter, + KeyUsage: x509.KeyUsageKeyEncipherment, + SignatureAlgorithm: x509.SHA256WithRSA, + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, rsaPub, ephemKey) + if err != nil { + return nil, fmt.Errorf("create synth cert: %w", err) + } + cert, err := x509.ParseCertificate(der) + if err != nil { + return nil, fmt.Errorf("parse synth cert: %w", err) + } + zeroizeKey(ephemKey) // burn the ephemeral signer immediately + return cert, nil +} + +// deterministicSerial picks a stable serial number from the first 16 +// bytes of the CSR signature. Avoids a fresh CSPRNG draw per request + +// gives the device's client library a serial it can re-derive locally +// for diagnostic-log correlation. +func deterministicSerial(sig []byte) *big.Int { + if len(sig) == 0 { + // Defensive: an unsigned CSR shouldn't reach here (CheckSignature + // gated upstream) but a deterministic fallback ensures the cert + // builder never crashes on a zero-byte serial. + return big.NewInt(1) + } + end := 16 + if len(sig) < end { + end = len(sig) + } + return new(big.Int).SetBytes(sig[:end]) +} + +// serverKeygenSyntheticNotBefore / NotAfter are stable timestamps for +// the never-published synthetic recipient cert. Using fixed-far-past + +// fixed-far-future means the cert struct round-trips cleanly through +// x509 without any time-source plumbing. +var ( + serverKeygenSyntheticNotBefore = mustParseTime("2020-01-01T00:00:00Z") + serverKeygenSyntheticNotAfter = mustParseTime("2099-12-31T23:59:59Z") +) + +func mustParseTime(s string) time.Time { + t, err := time.Parse(time.RFC3339, s) + if err != nil { + panic(fmt.Sprintf("est: hard-coded time %q failed to parse: %v", s, err)) + } + return t +} + +// zeroizeKey overwrites the in-memory bytes of the private key with +// zeros. Best-effort: Go's GC may have copied the buffer; closures the +// math/big and crypto stdlib hold may keep their own copies. The +// canonical defense is "don't keep this key around for long" — we +// release the reference inside the calling function so GC reclaims it +// promptly. +func zeroizeKey(k interface{}) { + switch v := k.(type) { + case *rsa.PrivateKey: + // Best-effort: zero the big.Int components. Calls to + // SetBytes(nil) reset the underlying word slice. + if v == nil { + return + } + if v.D != nil { + v.D.SetUint64(0) + } + for i := range v.Primes { + if v.Primes[i] != nil { + v.Primes[i].SetUint64(0) + } + } + case *ecdsa.PrivateKey: + if v == nil || v.D == nil { + return + } + v.D.SetUint64(0) + } +} + +// zeroizeBytes overwrites a byte slice with zeros in place. +func zeroizeBytes(b []byte) { + for i := range b { + b[i] = 0 + } +} diff --git a/internal/service/est_counters.go b/internal/service/est_counters.go new file mode 100644 index 0000000..6ca8c98 --- /dev/null +++ b/internal/service/est_counters.go @@ -0,0 +1,205 @@ +package service + +import ( + "sync/atomic" + "time" + + "github.com/shankar0123/certctl/internal/trustanchor" +) + +// EST RFC 7030 hardening master bundle Phase 7.1. +// +// estCounterTab is the in-memory equivalent of a Prometheus +// `certctl_est_enrollments_total{status="..."}` metric. We don't take a +// Prometheus dependency here (the project doesn't expose /metrics today; +// that's a separate decision). The admin GUI's "EST Profiles" tab calls +// the GET /api/v1/admin/est/profiles endpoint, which calls +// ESTService.Stats() to render the counter snapshot. +// +// Concurrency: every field is read/written via sync/atomic so the +// service hot path stays lock-free. + +// Counter labels — keep in sync with snapshot() + the admin GUI's +// counter-grid renderer. New labels MUST be added in three places: +// constants below, snapshot()'s map, and inc()'s switch. +const ( + estCounterSuccessSimpleEnroll = "success_simpleenroll" + estCounterSuccessSimpleReEnroll = "success_simplereenroll" + estCounterSuccessServerKeygen = "success_serverkeygen" + estCounterAuthFailedBasic = "auth_failed_basic" + estCounterAuthFailedMTLS = "auth_failed_mtls" + estCounterAuthFailedChannelBind = "auth_failed_channel_binding" + estCounterCSRInvalid = "csr_invalid" + estCounterCSRPolicyViolation = "csr_policy_violation" + estCounterCSRSignatureMismatch = "csr_signature_mismatch" + estCounterRateLimited = "rate_limited" + estCounterIssuerError = "issuer_error" + estCounterInternalError = "internal_error" +) + +type estCounterTab struct { + successSimpleEnroll atomic.Uint64 + successSimpleReEnroll atomic.Uint64 + successServerKeygen atomic.Uint64 + authFailedBasic atomic.Uint64 + authFailedMTLS atomic.Uint64 + authFailedChannelBind atomic.Uint64 + csrInvalid atomic.Uint64 + csrPolicyViolation atomic.Uint64 + csrSignatureMismatch atomic.Uint64 + rateLimited atomic.Uint64 + issuerError atomic.Uint64 + internalError atomic.Uint64 +} + +// snapshot returns a zero-allocation copy of the current counter values +// keyed by the same label strings inc() accepts. +func (c *estCounterTab) snapshot() map[string]uint64 { + if c == nil { + return map[string]uint64{} + } + return map[string]uint64{ + estCounterSuccessSimpleEnroll: c.successSimpleEnroll.Load(), + estCounterSuccessSimpleReEnroll: c.successSimpleReEnroll.Load(), + estCounterSuccessServerKeygen: c.successServerKeygen.Load(), + estCounterAuthFailedBasic: c.authFailedBasic.Load(), + estCounterAuthFailedMTLS: c.authFailedMTLS.Load(), + estCounterAuthFailedChannelBind: c.authFailedChannelBind.Load(), + estCounterCSRInvalid: c.csrInvalid.Load(), + estCounterCSRPolicyViolation: c.csrPolicyViolation.Load(), + estCounterCSRSignatureMismatch: c.csrSignatureMismatch.Load(), + estCounterRateLimited: c.rateLimited.Load(), + estCounterIssuerError: c.issuerError.Load(), + estCounterInternalError: c.internalError.Load(), + } +} + +// inc advances the counter matching the given label. Unknown labels +// fall through to internal_error so an enum drift doesn't silently +// lose counts. +func (c *estCounterTab) inc(label string) { + if c == nil { + return + } + switch label { + case estCounterSuccessSimpleEnroll: + c.successSimpleEnroll.Add(1) + case estCounterSuccessSimpleReEnroll: + c.successSimpleReEnroll.Add(1) + case estCounterSuccessServerKeygen: + c.successServerKeygen.Add(1) + case estCounterAuthFailedBasic: + c.authFailedBasic.Add(1) + case estCounterAuthFailedMTLS: + c.authFailedMTLS.Add(1) + case estCounterAuthFailedChannelBind: + c.authFailedChannelBind.Add(1) + case estCounterCSRInvalid: + c.csrInvalid.Add(1) + case estCounterCSRPolicyViolation: + c.csrPolicyViolation.Add(1) + case estCounterCSRSignatureMismatch: + c.csrSignatureMismatch.Add(1) + case estCounterRateLimited: + c.rateLimited.Add(1) + case estCounterIssuerError: + c.issuerError.Add(1) + default: + c.internalError.Add(1) + } +} + +// ESTStatsSnapshot is the per-profile observability view the admin +// GET endpoint renders. Mirrors IntuneStatsSnapshot's shape so the GUI +// can re-use the same counter-grid component. +// +// EST RFC 7030 hardening master bundle Phase 7.1. +type ESTStatsSnapshot struct { + PathID string `json:"path_id"` + IssuerID string `json:"issuer_id"` + ProfileID string `json:"profile_id,omitempty"` + Counters map[string]uint64 `json:"counters"` + MTLSEnabled bool `json:"mtls_enabled"` + BasicConfigured bool `json:"basic_auth_configured"` + ServerKeygen bool `json:"server_keygen_enabled"` + TrustAnchors []ESTTrustAnchorInfo `json:"trust_anchors,omitempty"` + TrustAnchorPath string `json:"trust_anchor_path,omitempty"` + Now time.Time `json:"now"` +} + +// ESTTrustAnchorInfo is the per-cert public summary of one trust anchor +// in the holder's pool. Same shape as IntuneTrustAnchorInfo. +type ESTTrustAnchorInfo struct { + Subject string `json:"subject"` + NotBefore time.Time `json:"not_before"` + NotAfter time.Time `json:"not_after"` + DaysToExpiry int `json:"days_to_expiry"` + Expired bool `json:"expired"` +} + +// Stats returns the per-profile observability snapshot. Safe for +// concurrent callers — every counter access is atomic + the trust- +// anchor walk is a per-snapshot copy. +func (s *ESTService) Stats(now time.Time) ESTStatsSnapshot { + out := ESTStatsSnapshot{ + PathID: s.estPathIDForLog, + IssuerID: s.issuerID, + ProfileID: s.profileID, + Counters: s.counters.snapshot(), + MTLSEnabled: s.estMTLSConfigured, + BasicConfigured: s.estBasicConfigured, + ServerKeygen: s.estServerKeygenEnabled, + Now: now, + } + if s.estTrustAnchor != nil { + out.TrustAnchorPath = s.estTrustAnchor.Path() + for _, c := range s.estTrustAnchor.Get() { + daysToExpiry := int(c.NotAfter.Sub(now).Hours() / 24) + out.TrustAnchors = append(out.TrustAnchors, ESTTrustAnchorInfo{ + Subject: c.Subject.CommonName, + NotBefore: c.NotBefore, + NotAfter: c.NotAfter, + DaysToExpiry: daysToExpiry, + Expired: now.After(c.NotAfter), + }) + } + } + return out +} + +// ReloadTrust forces a SIGHUP-equivalent reload of the per-profile +// EST mTLS trust anchor pool. Returns nil on success; the configured +// holder error otherwise (typically a parse error from a half-rotated +// bundle file). Mirror of SCEPService.ReloadIntuneTrust. +// +// Returns ErrESTMTLSDisabled when the profile doesn't have an mTLS +// trust anchor configured (admin handler maps to HTTP 409). +func (s *ESTService) ReloadTrust() error { + if s.estTrustAnchor == nil { + return ErrESTMTLSDisabled + } + return s.estTrustAnchor.Reload() +} + +// ErrESTMTLSDisabled signals the admin handler that an EST profile +// doesn't have mTLS configured. Maps to HTTP 409 Conflict. +var ErrESTMTLSDisabled = newESTAdminError("EST profile mTLS not enabled — no trust anchor to reload") + +func newESTAdminError(msg string) error { return &estAdminError{msg: msg} } + +type estAdminError struct{ msg string } + +func (e *estAdminError) Error() string { return e.msg } + +// SetESTAdminMetadata records the per-profile observability hints the +// AdminEST handler needs to render the Profiles tab. cmd/server/main.go +// invokes this once at startup with the data already in scope from the +// per-profile loop. Idempotent. Consolidated into one setter so the +// public surface stays narrow + every metadata field moves together. +func (s *ESTService) SetESTAdminMetadata(pathID string, mtlsEnabled, basicConfigured, serverKeygenEnabled bool, trustAnchor *trustanchor.Holder) { + s.estPathIDForLog = pathID + s.estMTLSConfigured = mtlsEnabled + s.estBasicConfigured = basicConfigured + s.estServerKeygenEnabled = serverKeygenEnabled + s.estTrustAnchor = trustAnchor +} diff --git a/internal/service/est_profile_counter_isolation_test.go b/internal/service/est_profile_counter_isolation_test.go new file mode 100644 index 0000000..fdc2a46 --- /dev/null +++ b/internal/service/est_profile_counter_isolation_test.go @@ -0,0 +1,75 @@ +package service + +import ( + "context" + "errors" + "io" + "log/slog" + "testing" + "time" +) + +// EST RFC 7030 hardening master bundle Phase 7.3 — per-profile counter +// isolation regression test. Mirrors the SCEP equivalent at +// internal/api/handler/scep_profile_counter_isolation_test.go. +// +// Why this test exists: the future-bug class it guards against is a +// cmd/server/main.go refactor that constructs a SINGLE shared +// *estCounterTab and injects it into every per-profile ESTService — +// that would compile cleanly, pass every existing route-level test, +// and silently inflate one profile's counters with another's traffic. + +func TestESTService_PerProfileCountersIsolated(t *testing.T) { + silent := slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{Level: slog.LevelError + 10})) + + // Two services with separate issuers + counter tabs. NewESTService + // allocates a fresh estCounterTab per instance (Phase 7.1 contract); + // this test pins that contract. + corpSvc := NewESTService("iss-corp", &mockIssuerConnector{}, nil, silent) + iotSvc := NewESTService("iss-iot", &mockIssuerConnector{Err: errors.New("issuer down")}, nil, silent) + + ctx := context.Background() + + // CORP: drive 3 successful enrollments. Each ticks + // success_simpleenroll on CORP's tab; IOT's tab MUST stay zero + // for that label. + for i := 0; i < 3; i++ { + csrPEM := generateCSRPEM(t, "device-corp.example.com", []string{"device-corp.example.com"}) + if _, err := corpSvc.SimpleEnroll(ctx, csrPEM); err != nil { + t.Fatalf("corp enroll #%d: %v", i, err) + } + } + // IOT: drive 2 enrollments. Each fails issuance (mock returns err + // from IssueCertificate); each ticks issuer_error on IOT's tab. + for i := 0; i < 2; i++ { + csrPEM := generateCSRPEM(t, "device-iot.example.com", []string{"device-iot.example.com"}) + if _, err := iotSvc.SimpleEnroll(ctx, csrPEM); err == nil { + t.Fatalf("iot enroll #%d: expected issuer error", i) + } + } + + // CORP snapshot: success=3, issuer_error=0. + corpSnap := corpSvc.Stats(time.Now()).Counters + if got := corpSnap[estCounterSuccessSimpleEnroll]; got != 3 { + t.Errorf("corp success_simpleenroll = %d, want 3", got) + } + if got := corpSnap[estCounterIssuerError]; got != 0 { + t.Errorf("corp issuer_error = %d, want 0 (no IOT bleed)", got) + } + + // IOT snapshot: success=0, issuer_error=2. + iotSnap := iotSvc.Stats(time.Now()).Counters + if got := iotSnap[estCounterSuccessSimpleEnroll]; got != 0 { + t.Errorf("iot success_simpleenroll = %d, want 0 (no CORP bleed)", got) + } + if got := iotSnap[estCounterIssuerError]; got != 2 { + t.Errorf("iot issuer_error = %d, want 2", got) + } + + // Sanity: the two services' counter tabs MUST be distinct *estCounterTab + // pointers. If a future refactor introduces a shared tab, this assertion + // catches it before the snapshot bleed becomes silent. + if corpSvc.counters == iotSvc.counters { + t.Fatal("corp + iot share the same *estCounterTab — per-profile isolation broken") + } +} diff --git a/internal/service/est_test.go b/internal/service/est_test.go index 65797aa..d43f4ae 100644 --- a/internal/service/est_test.go +++ b/internal/service/est_test.go @@ -7,12 +7,16 @@ import ( "crypto/rand" "crypto/x509" "crypto/x509/pkix" + "encoding/asn1" "encoding/pem" "errors" + "io" "log/slog" "os" "strings" "testing" + + "github.com/shankar0123/certctl/internal/domain" ) // generateCSRPEM creates a valid ECDSA P-256 CSR for testing. @@ -178,3 +182,124 @@ func TestESTService_SimpleEnroll_WithProfile(t *testing.T) { t.Fatal("expected audit details") } } + +// EST RFC 7030 hardening master bundle Phase 6.3 csrattrs tests. +// Pin the contract that GetCSRAttrs returns DER(SEQUENCE OF OID) when the +// bound profile carries hints, falls back to the v2.0.x nil/204 stub when +// the profile is absent / empty / corrupt, and silently drops unknown +// EKU/attribute names rather than emitting garbage OIDs. + +func newCSRAttrsTestService(t *testing.T) (*ESTService, *mockProfileRepo) { + t.Helper() + repo := newMockProfileRepository() + silent := slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{Level: slog.LevelError + 10})) + svc := NewESTService("iss-local", &mockIssuerConnector{}, nil, silent) + svc.SetProfileRepo(repo) + return svc, repo +} + +func TestESTService_GetCSRAttrs_NoProfileBound_Returns204Body(t *testing.T) { + svc, _ := newCSRAttrsTestService(t) + // SetProfileID intentionally NOT called — handler should see empty body + // + write 204 per RFC 7030 §4.5.2 (legacy stub semantic preserved). + got, err := svc.GetCSRAttrs(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != nil { + t.Errorf("got non-nil body for unbound profile: %x", got) + } +} + +func TestESTService_GetCSRAttrs_ProfileWithEKUsAndAttrs_ReturnsOIDList(t *testing.T) { + svc, repo := newCSRAttrsTestService(t) + svc.SetProfileID("prof-corp") + repo.AddProfile(&domain.CertificateProfile{ + ID: "prof-corp", + Name: "corp", + AllowedEKUs: []string{"serverAuth", "clientAuth"}, + RequiredCSRAttributes: []string{"serialNumber"}, + Enabled: true, + }) + + der, err := svc.GetCSRAttrs(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(der) == 0 { + t.Fatal("expected non-empty body for profile with hints") + } + var got []asn1.ObjectIdentifier + if _, err := asn1.Unmarshal(der, &got); err != nil { + t.Fatalf("body should be DER(SEQUENCE OF OID); unmarshal: %v", err) + } + if len(got) != 3 { + t.Fatalf("expected 3 OIDs (2 EKUs + 1 attribute), got %d: %v", len(got), got) + } + // Pin the exact OIDs so a future EKUStringToOID typo trips the test. + wantSerialNumberOID := asn1.ObjectIdentifier{2, 5, 4, 5} + wantServerAuthOID := asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 3, 1} + wantClientAuthOID := asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 3, 2} + have := make(map[string]bool, len(got)) + for _, o := range got { + have[o.String()] = true + } + for _, want := range []asn1.ObjectIdentifier{wantServerAuthOID, wantClientAuthOID, wantSerialNumberOID} { + if !have[want.String()] { + t.Errorf("missing OID %v in csrattrs response", want) + } + } +} + +func TestESTService_GetCSRAttrs_EmptyProfile_Returns204Body(t *testing.T) { + svc, repo := newCSRAttrsTestService(t) + svc.SetProfileID("prof-empty") + repo.AddProfile(&domain.CertificateProfile{ + ID: "prof-empty", + Name: "empty", + Enabled: true, + }) + got, err := svc.GetCSRAttrs(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != nil { + t.Errorf("empty profile should return nil body for 204; got %x", got) + } +} + +func TestESTService_GetCSRAttrs_GarbageProfile_DropsUnknownAndKeepsValid(t *testing.T) { + svc, repo := newCSRAttrsTestService(t) + svc.SetProfileID("prof-garbage") + repo.AddProfile(&domain.CertificateProfile{ + ID: "prof-garbage", + Name: "garbage", + AllowedEKUs: []string{"serverAuth", "thisIsNotAnEKU"}, + RequiredCSRAttributes: []string{"serialNumber", "blarg-not-an-attribute"}, + Enabled: true, + }) + der, err := svc.GetCSRAttrs(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + var got []asn1.ObjectIdentifier + if _, err := asn1.Unmarshal(der, &got); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if len(got) != 2 { + t.Errorf("expected 2 OIDs (the valid subset); got %d: %v", len(got), got) + } +} + +func TestESTService_GetCSRAttrs_ProfileLookupError_DegradesToNoHints(t *testing.T) { + svc, repo := newCSRAttrsTestService(t) + svc.SetProfileID("prof-missing") + repo.GetErr = errors.New("repo unreachable") + got, err := svc.GetCSRAttrs(context.Background()) + if err != nil { + t.Fatalf("profile lookup error must NOT propagate; got: %v", err) + } + if got != nil { + t.Errorf("profile-lookup-error path must degrade to nil body; got %x", got) + } +} diff --git a/migrations/000022_certificate_profiles_csrattrs.down.sql b/migrations/000022_certificate_profiles_csrattrs.down.sql new file mode 100644 index 0000000..cd65cfb --- /dev/null +++ b/migrations/000022_certificate_profiles_csrattrs.down.sql @@ -0,0 +1,3 @@ +-- EST RFC 7030 hardening master bundle Phase 6.1 rollback. +ALTER TABLE certificate_profiles DROP COLUMN IF EXISTS required_csr_attributes; +ALTER TABLE certificate_profiles DROP COLUMN IF EXISTS must_staple; diff --git a/migrations/000022_certificate_profiles_csrattrs.up.sql b/migrations/000022_certificate_profiles_csrattrs.up.sql new file mode 100644 index 0000000..acfae1c --- /dev/null +++ b/migrations/000022_certificate_profiles_csrattrs.up.sql @@ -0,0 +1,33 @@ +-- EST RFC 7030 hardening master bundle Phase 6.1. +-- +-- Add `required_csr_attributes` JSONB column to certificate_profiles so the +-- EST `csrattrs` endpoint (RFC 7030 §4.5) can return a profile-derived OID +-- list to enrolling clients. Clients use the response as a hint for which +-- attributes / EKUs to include in their PKCS#10 CSR — example: the +-- IoT-bootstrap profile might require `serialNumber` (OID 2.5.4.5) so the +-- device serial appears in the issued cert's Subject DN. +-- +-- Defaults to `[]` for back-compat (existing profiles see no behavior change; +-- their EST csrattrs response stays the legacy 204-No-Content). +-- +-- Also lands `must_staple` as a real column. The 5.6 follow-up wired +-- CertificateProfile.MustStaple all the way through the issuer/service +-- layer but the postgres repo never grew the column — every existing +-- deploy implicitly has must_staple=false because the field couldn't be +-- persisted. The column is added with default false so existing profiles +-- behave identically; operators flipping must_staple via the API now +-- actually round-trip to disk. +-- +-- Both columns ship in the same migration to keep the schema-history +-- contiguous; rolling back drops both. + +ALTER TABLE certificate_profiles + ADD COLUMN IF NOT EXISTS required_csr_attributes JSONB NOT NULL DEFAULT '[]'; + +ALTER TABLE certificate_profiles + ADD COLUMN IF NOT EXISTS must_staple BOOLEAN NOT NULL DEFAULT false; + +-- Index isn't necessary — required_csr_attributes is read on every EST +-- csrattrs request but only at the per-profile granularity (always a +-- direct PK lookup); must_staple is a per-issuance bool with no query +-- pattern that benefits from indexing.