ejbca: port mTLS keypair to mtlscache (close Bundle M for the last issuer)

Closes Top-10 fix #1 of the 2026-05-03 issuer-coverage audit (see
cowork/issuer-coverage-audit-2026-05-03/RESULTS.md). Pre-fix,
ejbca.go::New called tls.LoadX509KeyPair once at construction and
configured the keypair into *http.Transport.TLSClientConfig with
no mtime watch. mTLS rotation required a server restart — quarterly
rotation per any reasonable security policy = quarterly deploy
outage.

Bundle M from the prior 2026-05-01 audit shipped the mtlscache
helper at internal/connector/issuer/mtlscache/cache.go and wired
it into Entrust + GlobalSign. EJBCA was missed in Bundle M's
scope. This commit ports the same helper onto EJBCA's
auth_mode=mtls path. The OAuth2 path is unchanged.

Implementation:
  - New imports internal/connector/issuer/mtlscache.
  - Connector struct gains an mtls *mtlscache.Cache field
    (mirroring Entrust + GlobalSign).
  - New()'s case 'mtls': replaces tls.LoadX509KeyPair + manual
    *http.Transport with mtlscache.New(certPath, keyPath,
    Options{HTTPTimeout: 30s}). Cache build happens at construction
    so misconfigured operators fail fast (matches pre-fix
    behaviour).
  - New helper getHTTPClient() returns the cached client; on the
    mTLS path it calls RefreshIfStale before returning so the
    next request uses the new keypair if disk has rotated. On
    OAuth2 / test paths (c.mtls == nil), returns c.httpClient
    as-is.
  - All 3 c.httpClient.Do call sites (IssueCertificate enroll,
    RevokeCertificate revoke, GetOrderStatus cert lookup) replaced
    with c.getHTTPClient() + client.Do.
  - crypto/tls import removed (no longer used at this layer).

Tests:
  - TestEJBCA_MTLSKeypairRotation_PicksUpNewCertWithoutRestart
    (new, ejbca_mtls_rotation_test.go): generates two CAs (caA,
    caB), signs leafA + leafB, spins up an httptest TLS server
    that trusts both CAs and records the issuer DN of every
    presented client cert, writes leafA, makes request 1, writes
    leafB + advances mtime by 2s, makes request 2. Asserts the
    server saw caA's DN on req 1 and caB's DN on req 2 — the
    cache picked up the rotation without ejbca.New re-running.
  - export_test.go: GetHTTPClientForTest helper exposes the
    private getHTTPClient so the rotation test drives the
    production code path.
  - All existing EJBCA tests still pass (TestNew_MTLSWiresClientCert,
    TestNew_MTLSCertLoadFailure, TestNew_OAuth2NoTransportTuning,
    TestNew_InvalidAuthMode).

Verified locally:
  - gofmt clean across the repo.
  - go vet ./... clean across the repo.
  - go test -race -count=1 -short ./internal/connector/issuer/ejbca/...
    ./internal/connector/issuer/mtlscache/... green.

Audit reference: cowork/issuer-coverage-audit-2026-05-03/
RESULTS.md Top-10 fix #1.

internal/connector/issuer/ejbca/ejbca.go

@@ -20,7 +20,6 @@ package ejbca
 import (
 	"bytes"
 	"context"
-	"crypto/tls"
 	"crypto/x509"
 	"encoding/base64"
 	"encoding/json"
@@ -33,6 +32,7 @@ import (
 	"time"
 
 	"github.com/shankar0123/certctl/internal/connector/issuer"
+	"github.com/shankar0123/certctl/internal/connector/issuer/mtlscache"
 	"github.com/shankar0123/certctl/internal/secret"
 )
 
@@ -84,16 +84,32 @@ type Connector struct {
 	config     *Config
 	logger     *slog.Logger
 	httpClient *http.Client
+
+	// mtls caches the parsed client keypair + a precomputed
+	// *http.Transport so steady-state API calls don't re-parse
+	// the keypair on every request, AND picks up rotated certs
+	// on the next call without a process restart. nil on the
+	// OAuth2 path (auth_mode=oauth2) and on the test path
+	// (NewWithHTTPClient). Closes Top-10 fix #1 of the
+	// 2026-05-03 issuer-coverage audit.
+	mtls *mtlscache.Cache
 }
 
 // New creates a new EJBCA connector with the given configuration and logger.
 //
 // 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).
+// builds an mtlscache.Cache from config.ClientCertPath + config.ClientKeyPath.
+// The cache parses the keypair once and configures
+// *http.Transport.TLSClientConfig so the client presents the cert on every
+// request. Subsequent calls go through getHTTPClient, which calls
+// RefreshIfStale on the cache — operators rotating the cert+key on disk
+// (e.g. quarterly per security policy) get the new keypair on the next
+// API call without a server restart. Closes Top-10 fix #1 of the
+// 2026-05-03 issuer-coverage audit.
+//
+// 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
@@ -110,30 +126,24 @@ func New(config *Config, logger *slog.Logger) (*Connector, error) {
 
 	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)
+		// Build the cache up-front so misconfigured operators fail fast
+		// at construction rather than discover a broken cert path on
+		// the first issuance call. mtlscache enforces TLS 1.2 floor
+		// (compat with on-prem EJBCA installs that predate TLS 1.3).
+		cache, err := mtlscache.New(config.ClientCertPath, config.ClientKeyPath, mtlscache.Options{
+			HTTPTimeout: 30 * time.Second,
+		})
 		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 nil, fmt.Errorf("EJBCA mTLS cache build: %w", err)
 		}
 		return &Connector{
-			config: config,
-			logger: logger,
-			httpClient: &http.Client{
-				Timeout:   30 * time.Second,
-				Transport: transport,
-			},
+			config:     config,
+			logger:     logger,
+			httpClient: cache.Client(),
+			mtls:       cache,
 		}, nil
 	case "oauth2":
 		// OAuth2 path uses default transport; setAuthHeaders adds the
@@ -159,6 +169,25 @@ func NewWithHTTPClient(config *Config, logger *slog.Logger, client *http.Client)
 	}
 }
 
+// getHTTPClient returns the HTTP client to use for an EJBCA API call.
+// On the mTLS path (auth_mode=mtls), it calls RefreshIfStale on the
+// mtlscache so a rotated keypair on disk is picked up before the next
+// request — operators rotating their EJBCA client cert quarterly no
+// longer need a server restart. On the OAuth2 path (auth_mode=oauth2)
+// or the test path (NewWithHTTPClient), it returns c.httpClient as-is
+// because there's no keypair to refresh. Closes Top-10 fix #1 of the
+// 2026-05-03 issuer-coverage audit. Mirrors the Entrust/GlobalSign
+// pattern from Bundle M of the 2026-05-01 audit.
+func (c *Connector) getHTTPClient() (*http.Client, error) {
+	if c.mtls == nil {
+		return c.httpClient, nil
+	}
+	if err := c.mtls.RefreshIfStale(); err != nil {
+		return nil, fmt.Errorf("EJBCA mTLS cache refresh: %w", err)
+	}
+	return c.mtls.Client(), nil
+}
+
 // enrollResponse represents the EJBCA /certificate/pkcs10enroll response.
 type enrollResponse struct {
 	Certificate string   `json:"certificate"`
@@ -250,7 +279,11 @@ func (c *Connector) IssueCertificate(ctx context.Context, request issuer.Issuanc
 	c.setAuthHeaders(req)
 	req.Header.Set("Content-Type", "application/json")
 
-	resp, err := c.httpClient.Do(req)
+	client, err := c.getHTTPClient()
+	if err != nil {
+		return nil, err
+	}
+	resp, err := client.Do(req)
 	if err != nil {
 		return nil, fmt.Errorf("EJBCA enroll request failed: %w", err)
 	}
@@ -392,7 +425,11 @@ func (c *Connector) RevokeCertificate(ctx context.Context, request issuer.Revoca
 	c.setAuthHeaders(req)
 	req.Header.Set("Content-Type", "application/json")
 
-	resp, err := c.httpClient.Do(req)
+	client, err := c.getHTTPClient()
+	if err != nil {
+		return err
+	}
+	resp, err := client.Do(req)
 	if err != nil {
 		return fmt.Errorf("EJBCA revoke request failed: %w", err)
 	}
@@ -438,7 +475,11 @@ func (c *Connector) GetOrderStatus(ctx context.Context, orderID string) (*issuer
 
 	c.setAuthHeaders(req)
 
-	resp, err := c.httpClient.Do(req)
+	client, err := c.getHTTPClient()
+	if err != nil {
+		return nil, err
+	}
+	resp, err := client.Do(req)
 	if err != nil {
 		return nil, fmt.Errorf("EJBCA cert get request failed: %w", err)
 	}

internal/connector/issuer/ejbca/ejbca_mtls_rotation_test.go

@@ -0,0 +1,260 @@
+package ejbca_test
+
+// Top-10 fix #1 of the 2026-05-03 issuer-coverage audit. Pre-fix,
+// ejbca.New called tls.LoadX509KeyPair once at construction; rotating
+// the client cert+key on disk required a server restart to take
+// effect. Post-fix, ejbca.New constructs an mtlscache.Cache and the
+// hot-path getHTTPClient calls RefreshIfStale before every API
+// request — operators rotating quarterly per security policy no
+// longer pay the deploy outage.
+//
+// This test pins the rotation behaviour end-to-end: write certA,
+// make one mTLS request, write certB at the same paths, advance
+// mtime, make a second request, assert the leaf cert presented on
+// the wire flipped from certA to certB.
+
+import (
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/rand"
+	"crypto/tls"
+	"crypto/x509"
+	"crypto/x509/pkix"
+	"encoding/pem"
+	"io"
+	"log/slog"
+	"math/big"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/shankar0123/certctl/internal/connector/issuer/ejbca"
+)
+
+// TestEJBCA_MTLSKeypairRotation_PicksUpNewCertWithoutRestart verifies
+// the mtlscache wiring catches a hot-rotated keypair. Sequence:
+//
+//  1. Generate caA + caB roots (different CAs so the server can tell
+//     which client cert was presented by inspecting the issuer DN).
+//  2. Sign clientA against caA, clientB against caB.
+//  3. Spin up an httptest TLS server that requires a client cert
+//     signed by caA OR caB (ClientCAs pool with both roots) and
+//     records which CA actually signed the presented client cert.
+//  4. Write clientA's cert+key to {certPath, keyPath}.
+//  5. Construct ejbca.Connector via production New (mTLS mode).
+//  6. Make request #1 → server records "presented cert from caA".
+//  7. Overwrite {certPath, keyPath} with clientB's cert+key. Advance
+//     mtime via os.Chtimes (ext4 mtime granularity is 1s; advancing
+//     by 2s defeats the no-op cheap path).
+//  8. Make request #2 → server records "presented cert from caB".
+//  9. Assert the two recorded issuers differ — the cache picked up
+//     the rotation without ejbca.New re-running.
+func TestEJBCA_MTLSKeypairRotation_PicksUpNewCertWithoutRestart(t *testing.T) {
+	logger := slog.New(slog.NewTextHandler(io.Discard, nil))
+	dir := t.TempDir()
+	certPath := filepath.Join(dir, "client.crt")
+	keyPath := filepath.Join(dir, "client.key")
+
+	caA, caACert, caAKey := mustCA(t, "EJBCA-RotationTest-CA-A")
+	caB, caBCert, caBKey := mustCA(t, "EJBCA-RotationTest-CA-B")
+
+	// Sign one leaf cert per CA; both have CN="ejbca-rotation-client"
+	// so the only distinguishing feature on the wire is the issuer DN.
+	leafA, leafAKey := mustLeafSignedBy(t, caACert, caAKey)
+	leafB, leafBKey := mustLeafSignedBy(t, caBCert, caBKey)
+
+	// httptest TLS server with a ClientCAs pool that trusts BOTH
+	// roots. The handler captures the issuer DN of the presented
+	// cert into a thread-safe slice the test inspects after the
+	// requests complete.
+	pool := x509.NewCertPool()
+	pool.AddCert(caACert)
+	pool.AddCert(caBCert)
+	var (
+		mu              sync.Mutex
+		seenIssuerDNs   []string
+		seenCommonNames []string
+	)
+	srv := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.TLS == nil || len(r.TLS.PeerCertificates) == 0 {
+			http.Error(w, "no client cert", http.StatusUnauthorized)
+			return
+		}
+		leaf := r.TLS.PeerCertificates[0]
+		mu.Lock()
+		seenIssuerDNs = append(seenIssuerDNs, leaf.Issuer.String())
+		seenCommonNames = append(seenCommonNames, leaf.Subject.CommonName)
+		mu.Unlock()
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write([]byte("ok"))
+	}))
+	srv.TLS = &tls.Config{
+		ClientAuth: tls.RequireAndVerifyClientCert,
+		ClientCAs:  pool,
+		MinVersion: tls.VersionTLS12,
+	}
+	srv.StartTLS()
+	defer srv.Close()
+
+	// Step 4 — write clientA initially.
+	mustWriteKeypair(t, certPath, keyPath, leafA, leafAKey)
+
+	// Step 5 — construct via production New().
+	cfg := &ejbca.Config{
+		APIUrl:         srv.URL,
+		AuthMode:       "mtls",
+		ClientCertPath: certPath,
+		ClientKeyPath:  keyPath,
+		CAName:         "Management CA",
+	}
+	conn, err := ejbca.New(cfg, logger)
+	if err != nil {
+		t.Fatalf("ejbca.New: %v", err)
+	}
+
+	// To talk to httptest's self-signed server cert, mutate the cached
+	// transport's RootCAs to trust the test server. The Certificates
+	// field (the client cert) stays intact — that's the field we're
+	// proving rotates without a New() re-run.
+	httpClient := ejbca.HTTPClientForTest(conn)
+	tr := httpClient.Transport.(*http.Transport)
+	srvPool := x509.NewCertPool()
+	srvPool.AddCert(srv.Certificate())
+	tr.TLSClientConfig.RootCAs = srvPool
+
+	// Step 6 — first request via the production code path.
+	clientA, err := ejbca.GetHTTPClientForTest(conn)
+	if err != nil {
+		t.Fatalf("getHTTPClient (req 1): %v", err)
+	}
+	// Re-apply the server-cert trust pool to the freshly-returned
+	// client's transport (cache rebuild would have wiped any prior
+	// mutation, but on the cheap path this is a no-op since the same
+	// transport pointer is returned).
+	if trA, ok := clientA.Transport.(*http.Transport); ok && trA.TLSClientConfig.RootCAs == nil {
+		trA.TLSClientConfig.RootCAs = srvPool
+	}
+	if _, err := clientA.Get(srv.URL); err != nil {
+		t.Fatalf("request 1: %v", err)
+	}
+
+	// Step 7 — overwrite the keypair with leafB, advance mtime.
+	mustWriteKeypair(t, certPath, keyPath, leafB, leafBKey)
+	future := time.Now().Add(2 * time.Second)
+	if err := os.Chtimes(certPath, future, future); err != nil {
+		t.Fatalf("chtimes cert: %v", err)
+	}
+	if err := os.Chtimes(keyPath, future, future); err != nil {
+		t.Fatalf("chtimes key: %v", err)
+	}
+
+	// Step 8 — second request. RefreshIfStale should rebuild the
+	// transport with leafB's keypair. The rebuild creates a new
+	// transport whose RootCAs is unset, so re-apply.
+	clientB, err := ejbca.GetHTTPClientForTest(conn)
+	if err != nil {
+		t.Fatalf("getHTTPClient (req 2): %v", err)
+	}
+	if trB, ok := clientB.Transport.(*http.Transport); ok && trB.TLSClientConfig.RootCAs == nil {
+		trB.TLSClientConfig.RootCAs = srvPool
+	}
+	if _, err := clientB.Get(srv.URL); err != nil {
+		t.Fatalf("request 2: %v", err)
+	}
+
+	// Step 9 — assert.
+	mu.Lock()
+	defer mu.Unlock()
+	if len(seenIssuerDNs) != 2 {
+		t.Fatalf("expected exactly 2 server-side observations, got %d: %v", len(seenIssuerDNs), seenIssuerDNs)
+	}
+	if seenIssuerDNs[0] == seenIssuerDNs[1] {
+		t.Fatalf("issuer DN unchanged across rotation: req1=%q req2=%q — cache did not pick up the new keypair",
+			seenIssuerDNs[0], seenIssuerDNs[1])
+	}
+	if want := caA.String(); seenIssuerDNs[0] != want {
+		t.Errorf("req 1 issuer = %q, want %q (caA)", seenIssuerDNs[0], want)
+	}
+	if want := caB.String(); seenIssuerDNs[1] != want {
+		t.Errorf("req 2 issuer = %q, want %q (caB)", seenIssuerDNs[1], want)
+	}
+}
+
+// --- test helpers ------------------------------------------------------
+
+// mustCA generates a self-signed CA cert + key. Returns the parsed CA
+// pkix.Name, the parsed cert, and the private key.
+func mustCA(t *testing.T, commonName string) (pkix.Name, *x509.Certificate, *ecdsa.PrivateKey) {
+	t.Helper()
+	key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Fatalf("CA key gen: %v", err)
+	}
+	template := x509.Certificate{
+		SerialNumber:          big.NewInt(1),
+		Subject:               pkix.Name{CommonName: commonName},
+		NotBefore:             time.Now().Add(-time.Hour),
+		NotAfter:              time.Now().Add(time.Hour),
+		KeyUsage:              x509.KeyUsageCertSign | x509.KeyUsageDigitalSignature,
+		ExtKeyUsage:           []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth},
+		IsCA:                  true,
+		BasicConstraintsValid: true,
+	}
+	der, err := x509.CreateCertificate(rand.Reader, &template, &template, &key.PublicKey, key)
+	if err != nil {
+		t.Fatalf("CA cert: %v", err)
+	}
+	cert, err := x509.ParseCertificate(der)
+	if err != nil {
+		t.Fatalf("parse CA: %v", err)
+	}
+	return template.Subject, cert, key
+}
+
+// mustLeafSignedBy generates a leaf cert+key, signed by the supplied
+// CA. Both rotation halves share the same Subject CN so the
+// distinguishing field on the wire is exclusively the Issuer DN.
+func mustLeafSignedBy(t *testing.T, caCert *x509.Certificate, caKey *ecdsa.PrivateKey) (*x509.Certificate, *ecdsa.PrivateKey) {
+	t.Helper()
+	leafKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Fatalf("leaf key gen: %v", err)
+	}
+	leafTemplate := x509.Certificate{
+		SerialNumber: big.NewInt(2),
+		Subject:      pkix.Name{CommonName: "ejbca-rotation-client"},
+		NotBefore:    time.Now().Add(-time.Hour),
+		NotAfter:     time.Now().Add(time.Hour),
+		KeyUsage:     x509.KeyUsageDigitalSignature,
+		ExtKeyUsage:  []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth},
+	}
+	der, err := x509.CreateCertificate(rand.Reader, &leafTemplate, caCert, &leafKey.PublicKey, caKey)
+	if err != nil {
+		t.Fatalf("leaf cert: %v", err)
+	}
+	leaf, err := x509.ParseCertificate(der)
+	if err != nil {
+		t.Fatalf("parse leaf: %v", err)
+	}
+	return leaf, leafKey
+}
+
+func mustWriteKeypair(t *testing.T, certPath, keyPath string, leaf *x509.Certificate, leafKey *ecdsa.PrivateKey) {
+	t.Helper()
+	certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: leaf.Raw})
+	if err := os.WriteFile(certPath, certPEM, 0o600); err != nil {
+		t.Fatalf("write cert: %v", err)
+	}
+	keyDER, err := x509.MarshalECPrivateKey(leafKey)
+	if err != nil {
+		t.Fatalf("marshal key: %v", err)
+	}
+	keyPEM := pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: keyDER})
+	if err := os.WriteFile(keyPath, keyPEM, 0o600); err != nil {
+		t.Fatalf("write key: %v", err)
+	}
+}

internal/connector/issuer/ejbca/export_test.go

@@ -12,3 +12,12 @@ import "net/http"
 func HTTPClientForTest(c *Connector) *http.Client {
 	return c.httpClient
 }
+
+// GetHTTPClientForTest exposes the per-call hot-path getHTTPClient
+// helper so the rotation test can drive the production code path
+// (which calls RefreshIfStale on the mtls cache before returning
+// the client). Production callers reach this same path implicitly
+// via IssueCertificate / RevokeCertificate / GetOrderStatus.
+func GetHTTPClientForTest(c *Connector) (*http.Client, error) {
+	return c.getHTTPClient()
+}