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.
This commit is contained in:
shankar0123
2026-05-02 00:08:24 +00:00
parent 62d03d9134
commit 989ad403e3
5 changed files with 382 additions and 27 deletions
+61 -7
View File
@@ -20,6 +20,7 @@ package ejbca
import (
"bytes"
"context"
"crypto/tls"
"crypto/x509"
"encoding/base64"
"encoding/json"
@@ -77,13 +78,66 @@ type Connector struct {
}
// New creates a new EJBCA connector with the given configuration and logger.
func New(config *Config, logger *slog.Logger) *Connector {
return &Connector{
config: config,
logger: logger,
httpClient: &http.Client{
Timeout: 30 * time.Second,
},
//
// When config.AuthMode is "mtls" (or empty — mtls is the default), New
// loads config.ClientCertPath + config.ClientKeyPath via tls.LoadX509KeyPair
// and configures *http.Transport.TLSClientConfig so the client presents the
// cert on every request. When AuthMode is "oauth2", New returns a client
// with no transport customization (the OAuth2 Bearer header path is wired
// in setAuthHeaders). Any other AuthMode value returns (nil, error).
//
// Returns an error if mTLS cert/key load fails (missing file, malformed
// PEM, mismatched cert/key) so misconfigured operators get an immediate
// failure at issuer construction rather than a cryptic 401 at first
// issuance.
//
// Callers wanting to inject a pre-built *http.Client (tests, fake EJBCA
// servers) should use NewWithHTTPClient.
func New(config *Config, logger *slog.Logger) (*Connector, error) {
authMode := "mtls"
if config != nil && config.AuthMode != "" {
authMode = config.AuthMode
}
switch authMode {
case "mtls":
// Build a fresh *http.Transport (do NOT clone http.DefaultTransport
// — mutation would leak across the package boundary). Set
// MinVersion: TLS 1.2 as a compatibility floor for on-prem EJBCA
// installs that may predate TLS 1.3.
if config == nil || config.ClientCertPath == "" || config.ClientKeyPath == "" {
return nil, fmt.Errorf("EJBCA mTLS requires client_cert_path and client_key_path")
}
cert, err := tls.LoadX509KeyPair(config.ClientCertPath, config.ClientKeyPath)
if err != nil {
return nil, fmt.Errorf("EJBCA mTLS cert load: %w", err)
}
transport := &http.Transport{
TLSClientConfig: &tls.Config{
Certificates: []tls.Certificate{cert},
MinVersion: tls.VersionTLS12,
},
}
return &Connector{
config: config,
logger: logger,
httpClient: &http.Client{
Timeout: 30 * time.Second,
Transport: transport,
},
}, nil
case "oauth2":
// OAuth2 path uses default transport; setAuthHeaders adds the
// Bearer header on every request.
return &Connector{
config: config,
logger: logger,
httpClient: &http.Client{
Timeout: 30 * time.Second,
},
}, nil
default:
return nil, fmt.Errorf("EJBCA invalid auth_mode %q (must be \"mtls\" or \"oauth2\")", authMode)
}
}