mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-14 23:08:51 +00:00
crypto/signer: introduce Signer interface; refactor local issuer to use it
This is a load-bearing internal refactor with no user-visible behavior
change. The new internal/crypto/signer package abstracts CA private-key
signing behind a Signer interface (embeds stdlib crypto.Signer + adds
Algorithm()). The local issuer now consumes this interface; the
historical c.caKey crypto.Signer field is renamed c.caSigner signer.Signer.
What landed:
* internal/crypto/signer/ — new stdlib-only package
- Signer interface: crypto.Signer + Algorithm()
- Algorithm enum: RSA-2048, RSA-3072, RSA-4096, ECDSA-P256, ECDSA-P384
- Driver interface: Load / Generate / Name
- FileDriver: production driver, wraps file-on-disk PEM, hooks for
DirHardener + Marshaler so the local package can inject Bundle 9
keystore.ensureKeyDirSecure + keymem.marshalPrivateKeyAndZeroize
- MemoryDriver: in-memory test driver; safe for concurrent use
- parse.go: ParsePrivateKey moved here from local.go (PKCS#1, SEC 1, PKCS#8)
- 91.6% coverage (gate ≥85)
* internal/connector/issuer/local/local.go — refactor
- Rename c.caKey crypto.Signer → c.caSigner signer.Signer
- Rewire 4 signing call sites: leaf cert (line ~613), CRL (~849),
OCSP response (~887), CA bootstrap (~482) — all access the
interface; the bootstrap also switches to interface-level
Public() + Signer
- Wrap freshly-generated and freshly-loaded keys; reject Ed25519
and other unsupported algorithms at load time (was silently
accepted before, would have failed at first sign)
- Delete the duplicated parsePrivateKey helper (single source of
truth now lives in the signer package)
- Update the L-014 threat-model comment block (lines 1-29) with a
forward-reference paragraph: file-on-disk caveats apply only to
FileDriver-backed signers; alternative drivers close that leg
- Coverage 86.7 → 86.5 (above CI floor of 86); the 0.2pp drop is
mechanical from deleting parsePrivateKey, partially recovered by
a new test pinning the Wrap error path
* internal/crypto/signer/equivalence_test.go — Phase 3 safety net
- RSA byte-strict equality for leaf certs / CRLs / OCSP responses
(PKCS#1 v1.5 is deterministic)
- ECDSA TBS-strict equality (signature differs because of random k)
- Both signatures independently validate against the CA
- Negative sentinel proves the equivalence checker isn't trivially-
passing
* docs/architecture.md — new 'CA Signing Abstraction' section under
Security Model, with ASCII diagram of FileDriver / MemoryDriver /
future PKCS11Driver / future CloudKMSDriver
* Test file mechanical edits (only):
- bundle9_coverage_test.go: parsePrivateKey → signer.ParsePrivateKey
(function moved, not behavior changed)
- local_test.go: append one targeted test
(TestSubCA_LoadCAFromDisk_RejectsUnsupportedKeyAlgorithm) that
pins the new Wrap error path I introduced — recovers coverage
cost of the deletion above
What did NOT change (verified empty diffs):
* api/openapi.yaml
* migrations/
* internal/connector/issuer/interface.go
* go.mod / go.sum (no new dependencies; stdlib only)
This refactor is the prerequisite for three downstream items:
- PKCS#11/HSM driver (V3-Pro)
- CRL/OCSP responder (V2)
- SSH CA lifecycle (V2)
Each of those adds a new signing call site. Doing the abstraction now
costs once; deferring would cost three times.
This commit is contained in:
@@ -3,6 +3,7 @@ package local_test
|
||||
import (
|
||||
"context"
|
||||
"crypto/ecdsa"
|
||||
"crypto/ed25519"
|
||||
"crypto/elliptic"
|
||||
"crypto/rand"
|
||||
"crypto/rsa"
|
||||
@@ -1170,3 +1171,90 @@ func TestSignOCSPResponse_SubCA(t *testing.T) {
|
||||
|
||||
t.Log("SubCA OCSP response generated successfully")
|
||||
}
|
||||
|
||||
// TestSubCA_LoadCAFromDisk_RejectsUnsupportedKeyAlgorithm pins the new
|
||||
// signer.Wrap error path introduced when local.go was refactored to
|
||||
// route every CA-signing call through the Signer interface. The
|
||||
// historical parsePrivateKey accepted any PKCS#8 key that satisfied
|
||||
// crypto.Signer (including Ed25519). The new flow keeps that
|
||||
// parse-time acceptance but adds a Wrap step that enforces the
|
||||
// certctl-supported algorithm enum (RSA-2048/3072/4096, ECDSA-P256/P384).
|
||||
//
|
||||
// This test confirms an Ed25519 sub-CA key fails LOUDLY at load time
|
||||
// with a clear "wrap CA private key as signer" error — instead of
|
||||
// either crashing later at sign time or silently producing a cert
|
||||
// chain certctl cannot revalidate. Pins both:
|
||||
// - the new error path coverage (recovers the 0.5pp drop introduced
|
||||
// by the parsePrivateKey deletion)
|
||||
// - the contract that loaded sub-CA keys MUST be in the supported
|
||||
// algorithm enum
|
||||
func TestSubCA_LoadCAFromDisk_RejectsUnsupportedKeyAlgorithm(t *testing.T) {
|
||||
logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug}))
|
||||
ctx := context.Background()
|
||||
tmpDir := t.TempDir()
|
||||
|
||||
// Build a valid CA cert signed by RSA so cert-validation passes...
|
||||
rsaKey, err := rsa.GenerateKey(rand.Reader, 2048)
|
||||
if err != nil {
|
||||
t.Fatalf("rsa keygen: %v", err)
|
||||
}
|
||||
caTemplate := &x509.Certificate{
|
||||
SerialNumber: big.NewInt(42),
|
||||
Subject: pkix.Name{CommonName: "Mismatched-Key Test CA"},
|
||||
NotBefore: time.Now().Add(-time.Hour),
|
||||
NotAfter: time.Now().Add(24 * time.Hour),
|
||||
KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign,
|
||||
IsCA: true,
|
||||
BasicConstraintsValid: true,
|
||||
}
|
||||
certDER, err := x509.CreateCertificate(rand.Reader, caTemplate, caTemplate, &rsaKey.PublicKey, rsaKey)
|
||||
if err != nil {
|
||||
t.Fatalf("create cert: %v", err)
|
||||
}
|
||||
certPath := filepath.Join(tmpDir, "ca.crt")
|
||||
if err := os.WriteFile(certPath, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER}), 0o600); err != nil {
|
||||
t.Fatalf("write cert: %v", err)
|
||||
}
|
||||
|
||||
// ...but write an UNRELATED Ed25519 key to disk. The Connector's
|
||||
// loadCAFromDisk does not enforce key-cert key match — it only
|
||||
// validates the cert and parses the key. The newly-introduced
|
||||
// signer.Wrap step is what rejects Ed25519.
|
||||
_, edPriv, err := ed25519.GenerateKey(rand.Reader)
|
||||
if err != nil {
|
||||
t.Fatalf("ed25519 keygen: %v", err)
|
||||
}
|
||||
edDER, err := x509.MarshalPKCS8PrivateKey(edPriv)
|
||||
if err != nil {
|
||||
t.Fatalf("marshal ed25519 PKCS#8: %v", err)
|
||||
}
|
||||
keyPath := filepath.Join(tmpDir, "ca.key")
|
||||
if err := os.WriteFile(keyPath, pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: edDER}), 0o600); err != nil {
|
||||
t.Fatalf("write key: %v", err)
|
||||
}
|
||||
|
||||
conn := local.New(&local.Config{
|
||||
CACommonName: "Mismatched-Key Test CA",
|
||||
ValidityDays: 90,
|
||||
CACertPath: certPath,
|
||||
CAKeyPath: keyPath,
|
||||
}, logger)
|
||||
|
||||
// IssueCertificate triggers ensureCA → loadCAFromDisk → ParsePrivateKey
|
||||
// (succeeds for Ed25519 PKCS#8) → signer.Wrap (rejects Ed25519).
|
||||
dummyKey, _ := rsa.GenerateKey(rand.Reader, 2048)
|
||||
csrTpl := &x509.CertificateRequest{Subject: pkix.Name{CommonName: "leaf.example.com"}}
|
||||
csrDER, _ := x509.CreateCertificateRequest(rand.Reader, csrTpl, dummyKey)
|
||||
csrPEM := string(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csrDER}))
|
||||
|
||||
_, err = conn.IssueCertificate(ctx, issuer.IssuanceRequest{
|
||||
CommonName: "leaf.example.com",
|
||||
CSRPEM: csrPEM,
|
||||
})
|
||||
if err == nil {
|
||||
t.Fatal("expected IssueCertificate to fail for Ed25519 sub-CA key, got nil")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "wrap CA private key as signer") {
|
||||
t.Fatalf("expected error to mention 'wrap CA private key as signer', got: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user