3 Commits

Author	SHA1	Message	Date
shankar0123	336a745f41	secret: migrate EJBCA / GlobalSign / Sectigo credentials to *secret.Ref (Phase 2) ... Phase 2 of the #6 acquisition-readiness fix from the 2026-05-01 issuer coverage audit. Phase 1 (commit 520cda3) shipped the secret.Ref opaque credential type with PBKDF2-derived key, ChaCha20-Poly1305 envelope, String/MarshalJSON redaction to "[redacted]", and the Use callback that zero-fills the per-call buffer after the consumer returns. This commit applies the type to the three connectors flagged by the audit and adds the JSON-roundtrip glue that the production factory path needs. Shared (internal/secret/): - Add UnmarshalJSON on *Ref so json.Unmarshal of a stored config blob (issuerfactory.NewFromConfig) parses the bytes-as-string into NewRefFromString without callers having to know the field type changed. Null and missing keys leave the receiver nil; non-string payloads (numbers, bools) are rejected with a typed error. Pinned by TestRef_UnmarshalJSON: string_value, null, missing_key, number_rejected, roundtrip_marshal_then_unmarshal (the round-trip goes through "[redacted]" intentionally — JSON-marshal-then- unmarshal of a Config with secrets is NOT a supported test pattern; callers that construct a rawConfig must use a JSON literal with the real values). Per-connector migration: - EJBCA (ejbca.go): Config.Token: string → *secret.Ref. ValidateConfig empty-check uses Token.IsEmpty() (nil-safe). setAuthHeaders rewritten to call Token.Use; the Bearer header string is built inside the callback and the buffer is zeroed on return. mTLS path is unaffected. - GlobalSign (globalsign.go): Config.APIKey + Config.APISecret: string → *secret.Ref. Both ValidateConfig empty-checks use IsEmpty(). Extracted setAuthHeaders helper consolidates the four duplicated triple-Set sites (ValidateConfig probe, IssueCertificate, RevokeCertificate, pollCertificateOnce) so any future header-shape change applies once. ValidateConfig now pulls from the local cfg (post-Unmarshal) so the helper takes a *Config rather than the receiver — needed because ValidateConfig writes the validated cfg onto c.config only AFTER the probe succeeds. - Sectigo (sectigo.go): Config.Login + Config.Password: string → *secret.Ref. CustomerURI stays plain string (org identifier, not a credential). setAuthHeaders rewritten to call Login.Use + Password.Use; ValidateConfig's inline header writes use the same pattern (the ValidateConfig probe writes to a local cfg, not c.config, so it can't share setAuthHeaders without rewiring — the inline form is fine, kept consistent in shape). Test migration: - ejbca_test.go, ejbca_failure_test.go, ejbca_stubs_test.go: bulk Token: "X" → Token: secret.NewRefFromString("X") via sed; secret import added. - globalsign_test.go, globalsign_failure_test.go: same pattern for APIKey + APISecret. - sectigo_test.go, sectigo_failure_test.go: same pattern for Login + Password. Two tests (TestGlobalSign_ServerTLSConfig/PinnedCA_TrustsExpectedServer and TestSectigoConnector/ValidateConfig_Success) used to construct rawConfig via json.Marshal(config) → ValidateConfig(rawConfig). After the migration, json.Marshal redacts *secret.Ref to "[redacted]" by design, so the roundtripped rawConfig wrote "[redacted]" as the actual header value and the mock server's auth-header check 403'd. Both tests now build rawConfig as a JSON literal (the production- shape input — the factory path always feeds rawConfig from the DB or env, never from json.Marshal of an in-memory Config). The new tests have a comment explaining the trap so the next person who adds a similar test sees the pattern. Out of scope (intentional): - The `internal/config/config.SectigoConfig` / `GlobalSignConfig` / `EJBCAConfig` env-var-loader structs are still plain strings — those types are the env-load shape, not the steady-state runtime shape. The seed path in service/issuer.go json-marshals them into a map[string]interface{} which the factory then UnmarshalJSON's into the connector Config; the new UnmarshalJSON on *Ref handles the conversion at the boundary. - DigiCert.APIKey + Vault.Token are still plain strings; Phase 3 will pick them up. The audit explicitly named EJBCA / GlobalSign / Sectigo as the Phase 2 scope (RESULTS.md L633). Verified locally: - gofmt -l . clean - go vet ./... clean - staticcheck across all four packages clean - go test -short -count=1 across secret, ejbca, globalsign, sectigo, issuerfactory, service, api/handler: green Audit reference: cowork/issuer-coverage-audit-2026-05-01/RESULTS.md Top-10 fix #6 — Phase 2.	2026-05-02 12:53:58 +00:00
shankar0123	989ad403e3	ejbca: wire mTLS client cert in New() ... Closes the #2 acquisition-readiness blocker from the 2026-05-01 issuer coverage audit. New() at ejbca.go:L79-L88 previously constructed an http.Client with only Timeout set — no Transport, no TLSClientConfig. When AuthMode=mtls (the default), the client never presented the configured ClientCert/ClientKey. The OAuth2 path worked; mTLS always failed authentication. Tests passed because they injected a pre-built *http.Client via NewWithHTTPClient, a path the production factory never took. This commit: - Rewrites New() to load ClientCertPath + ClientKeyPath via tls.LoadX509KeyPair when AuthMode=mtls, configure *http.Transport.TLSClientConfig with MinVersion: TLS 1.2 (compatibility floor for on-prem EJBCA installs that may predate TLS 1.3), and return (*Connector, error). Constructs a fresh *http.Transport — does NOT clone http.DefaultTransport, which would leak mutation across the package boundary. - OAuth2 mode unchanged: returns a client with no transport customization (the Bearer header path is wired in setAuthHeaders). - Invalid auth_mode values return (nil, error) immediately rather than falling through to the mtls default and erroring at cert load. - Updates the factory call site at issuerfactory/factory.go for the new signature; the factory's outer (issuer.Connector, error) shape was already in place. - Adds TestNew_MTLSWiresClientCert: calls production New() (NOT NewWithHTTPClient) with real cert/key files generated via stdlib crypto/x509, asserts httpClient.Transport.TLSClientConfig.Certificates is non-empty. Includes an httptest TLS server with ClientAuth: tls.RequireAndVerifyClientCert that proves the cert is actually presented on the wire — not just stashed in a struct field. - Adds TestNew_MTLSCertLoadFailure: missing-cert path returns an error wrapping fs.ErrNotExist (verified via errors.Is). - Adds TestNew_OAuth2NoTransportTuning: OAuth2 path leaves Transport nil, ensuring no accidental mTLS bleedthrough. - Adds TestNew_InvalidAuthMode: explicit guard that auth_mode values other than "mtls"/"oauth2" return (nil, error) at New() time. - Adds export_test.go with HTTPClientForTest helper so the external ejbca_test package can inspect the connector's internal *http.Client for the wiring assertions. Compile-only during `go test`; production builds don't expose it. - Adds mustNewForValidateConfig test helper (OAuth2 placeholder connector) for the existing ValidateConfig-only tests; pre-fix they used New(nil, ...) which is no longer valid because nil config falls into the mTLS default branch that requires non-nil cert paths. - Updates ejbca_stubs_test.go (internal package) for the new (*Connector, error) signature; switches the dummy connector to OAuth2 mode so Config{} doesn't error at New(). Out of scope (separate follow-ups, per the prompt's explicit fence): - OAuth2 token refresh missing - Config.Token plaintext at runtime (needs SecretRef abstraction) - RevokeCertificate composite OrderID parsing (the issuerDN := "" line at ejbca.go:L313) Verified locally: - gofmt clean - go vet ./... clean - staticcheck ./... clean - golangci-lint run --timeout 5m ./... → 0 issues - go test -short -count=1 ./internal/connector/issuer/ejbca/ green - go test -short -count=1 ./internal/connector/issuerfactory/ green - go test -short -count=1 ./internal/service/ green - go build ./... success Audit reference: cowork/issuer-coverage-audit-2026-05-01/RESULTS.md Top-10 fix #2.	2026-05-02 00:08:24 +00:00
cowork	933edfb1d0	Bundle N (Coverage Audit Closure) [partial]: issuer-connector stubs coverage ... Closes M-001 partially; M-002, M-003, and CI threshold raise #2 deferred. Stubs coverage shipped across 8 issuer connectors via per-connector <conn>_stubs_test.go (~50 LoC each) pinning the not-supported issuer.Connector interface methods (GenerateCRL, SignOCSPResponse, GetCACertPEM, GetRenewalInfo). Most CAs delegate CRL/OCSP/CA-cert distribution to managed services, so these are documented stubs that return errors. Pinning them ensures the stubs aren't silently replaced with no-ops in a future refactor. Coverage delta: digicert: 79.3% -> 81.0% (+1.7pp) ejbca: 75.8% -> 76.5% (+0.7pp) entrust: 70.8% -> 70.8% (stubs already covered) sectigo: 78.0% -> 79.4% (+1.4pp) vault: 81.0% -> 84.1% (+3.1pp) openssl: 76.9% -> 78.0% (+1.1pp) googlecas: 81.0% -> 83.4% (+2.4pp) globalsign: 75.9% -> 78.2% (+2.3pp) (awsacmpca not included; its 0%-coverage hotspots are stubClient methods structurally different from the others' interface stubs. Already at 83.5%.) Why the gates aren't yet met: the stub functions are tiny (1-2 lines each, mostly 'return nil, fmt.Errorf("not supported")'). Lifting each connector to >=85% requires per-connector failure-mode test files mirroring Bundle J's ACME pattern (httptest.Server + canned 401/403/ 429+Retry-After/5xx/malformed responses against the actual API methods). That's ~200-300 LoC x 9 connectors = ~2000-2700 LoC of bespoke per-CA mock work; exceeds this session's budget. Tracked as follow-on Bundle N.A-extended / N.B-extended. Deferred sub-batches: N.C (M-002 + M-003): internal/service (70.5%) + internal/api/handler (79.4%) round-out NOT YET STARTED. Tracked as Bundle N.C-extended. N.CI (CI threshold raise #2): prescribed raises require underlying coverage at proposed floors first. Premature raise would fail CI immediately. Tracked as Bundle N.CI-extended. Verification: go vet ./internal/connector/issuer/{8-pkgs}/... clean gofmt -l clean go test -short -count=1 PASS for all 8 Audit deliverables: gap-backlog.md: M-001 partial-strikethrough with per-connector table + Bundle N closure-log entry covering all 4 sub-batch statuses closure-plan.md: Bundle N [~] with per-sub-batch status breakdown CHANGELOG.md: [unreleased] Bundle N entry	2026-04-27 17:45:18 +00:00