From fa6e3d17eb261352330139c7a4d9cf79f9191d64 Mon Sep 17 00:00:00 2001 From: Luis Sanchez Date: Thu, 5 Mar 2026 11:42:28 -0500 Subject: [PATCH 1/2] crypto: add certificate generation utilities with tests Introduce utilities for creating signing, server, client, and peer certificates. Include support for configurable key algorithms, customizable subjects, lifetimes, and extensions. Add comprehensive unit tests to validate all functionality. --- pkg/crypto/cert_config.go | 225 +++++++++++++++ pkg/crypto/cert_config_test.go | 482 +++++++++++++++++++++++++++++++++ pkg/crypto/keygen.go | 118 ++++++++ pkg/crypto/options.go | 49 ++++ 4 files changed, 874 insertions(+) create mode 100644 pkg/crypto/cert_config.go create mode 100644 pkg/crypto/cert_config_test.go create mode 100644 pkg/crypto/keygen.go create mode 100644 pkg/crypto/options.go diff --git a/pkg/crypto/cert_config.go b/pkg/crypto/cert_config.go new file mode 100644 index 0000000000..76d71da2ab --- /dev/null +++ b/pkg/crypto/cert_config.go @@ -0,0 +1,225 @@ +package crypto + +import ( + "crypto" + "crypto/x509" + "crypto/x509/pkix" + "fmt" + "time" + + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apiserver/pkg/authentication/user" +) + +// KeyPairGenerator generates a cryptographic key pair. +type KeyPairGenerator interface { + GenerateKeyPair() (crypto.PublicKey, crypto.PrivateKey, error) +} + +// NewSigningCertificate creates a CA certificate. +// By default it creates a self-signed root CA. Use WithSigner to create an +// intermediate CA signed by a parent CA. +// The name parameter is used as the CommonName unless overridden with WithSubject. +// Optional: WithSigner, WithSubject, WithLifetime (defaults to DefaultCACertificateLifetimeDuration). +func NewSigningCertificate(name string, keyGen KeyPairGenerator, opts ...CertificateOption) (*TLSCertificateConfig, error) { + o := &CertificateOptions{ + lifetime: DefaultCACertificateLifetimeDuration, + } + for _, opt := range opts { + opt(o) + } + + subject := pkix.Name{CommonName: name} + if o.subject != nil { + subject = *o.subject + } + + publicKey, privateKey, err := keyGen.GenerateKeyPair() + if err != nil { + return nil, fmt.Errorf("failed to generate key pair: %w", err) + } + subjectKeyId, err := SubjectKeyIDFromPublicKey(publicKey) + if err != nil { + return nil, fmt.Errorf("failed to compute subject key ID: %w", err) + } + + if o.signer != nil { + // Intermediate CA signed by the provided signer. + authorityKeyId := o.signer.Config.Certs[0].SubjectKeyId + template := newSigningCertificateTemplateForDuration(subject, o.lifetime, time.Now, authorityKeyId, subjectKeyId) + template.SignatureAlgorithm = 0 + template.KeyUsage = KeyUsageForPublicKey(publicKey) | x509.KeyUsageCertSign + + cert, err := o.signer.SignCertificate(template, publicKey) + if err != nil { + return nil, fmt.Errorf("failed to sign certificate: %w", err) + } + + return &TLSCertificateConfig{ + Certs: append([]*x509.Certificate{cert}, o.signer.Config.Certs...), + Key: privateKey, + }, nil + } + + // Self-signed root CA. AuthorityKeyId and SubjectKeyId match. + template := newSigningCertificateTemplateForDuration(subject, o.lifetime, time.Now, subjectKeyId, subjectKeyId) + template.SignatureAlgorithm = 0 + template.KeyUsage = KeyUsageForPublicKey(publicKey) | x509.KeyUsageCertSign + + cert, err := signCertificate(template, publicKey, template, privateKey) + if err != nil { + return nil, fmt.Errorf("failed to sign certificate: %w", err) + } + + return &TLSCertificateConfig{ + Certs: []*x509.Certificate{cert}, + Key: privateKey, + }, nil +} + +// NewServerCertificate creates a server/serving certificate signed by this CA. +// Optional: WithLifetime (defaults to DefaultCertificateLifetimeDuration), WithExtensions. +func (ca *CA) NewServerCertificate(hostnames sets.Set[string], keyGen KeyPairGenerator, opts ...CertificateOption) (*TLSCertificateConfig, error) { + o := &CertificateOptions{ + lifetime: DefaultCertificateLifetimeDuration, + } + for _, opt := range opts { + opt(o) + } + + publicKey, privateKey, err := keyGen.GenerateKeyPair() + if err != nil { + return nil, fmt.Errorf("failed to generate key pair: %w", err) + } + subjectKeyId, err := SubjectKeyIDFromPublicKey(publicKey) + if err != nil { + return nil, fmt.Errorf("failed to compute subject key ID: %w", err) + } + + sortedHostnames := sets.List(hostnames) + authorityKeyId := ca.Config.Certs[0].SubjectKeyId + template := newServerCertificateTemplateForDuration( + pkix.Name{CommonName: sortedHostnames[0]}, + sortedHostnames, + o.lifetime, + time.Now, + authorityKeyId, + subjectKeyId, + ) + // Let x509.CreateCertificate auto-detect the signature algorithm from the CA's key. + template.SignatureAlgorithm = 0 + template.KeyUsage = KeyUsageForPublicKey(publicKey) + + for _, fn := range o.extensionFns { + if err := fn(template); err != nil { + return nil, fmt.Errorf("failed to apply certificate extension: %w", err) + } + } + + cert, err := ca.SignCertificate(template, publicKey) + if err != nil { + return nil, fmt.Errorf("failed to sign certificate: %w", err) + } + + return &TLSCertificateConfig{ + Certs: append([]*x509.Certificate{cert}, ca.Config.Certs...), + Key: privateKey, + }, nil +} + +// NewClientCertificate creates a client certificate signed by this CA. +// Optional: WithLifetime (defaults to DefaultCertificateLifetimeDuration). +func (ca *CA) NewClientCertificate(u user.Info, keyGen KeyPairGenerator, opts ...CertificateOption) (*TLSCertificateConfig, error) { + o := &CertificateOptions{ + lifetime: DefaultCertificateLifetimeDuration, + } + for _, opt := range opts { + opt(o) + } + + publicKey, privateKey, err := keyGen.GenerateKeyPair() + if err != nil { + return nil, fmt.Errorf("failed to generate key pair: %w", err) + } + subjectKeyId, err := SubjectKeyIDFromPublicKey(publicKey) + if err != nil { + return nil, fmt.Errorf("failed to compute subject key ID: %w", err) + } + + authorityKeyId := ca.Config.Certs[0].SubjectKeyId + template := NewClientCertificateTemplateForDuration(UserToSubject(u), o.lifetime, time.Now) + template.AuthorityKeyId = authorityKeyId + template.SubjectKeyId = subjectKeyId + // Let x509.CreateCertificate auto-detect the signature algorithm from the CA's key. + template.SignatureAlgorithm = 0 + template.KeyUsage = KeyUsageForPublicKey(publicKey) + + cert, err := ca.SignCertificate(template, publicKey) + if err != nil { + return nil, fmt.Errorf("failed to sign certificate: %w", err) + } + + return &TLSCertificateConfig{ + Certs: append([]*x509.Certificate{cert}, ca.Config.Certs...), + Key: privateKey, + }, nil +} + +// NewPeerCertificate creates a peer certificate (both server and client auth) +// signed by this CA. +// Optional: WithLifetime (defaults to DefaultCertificateLifetimeDuration), WithExtensions. +func (ca *CA) NewPeerCertificate(hostnames sets.Set[string], u user.Info, keyGen KeyPairGenerator, opts ...CertificateOption) (*TLSCertificateConfig, error) { + o := &CertificateOptions{ + lifetime: DefaultCertificateLifetimeDuration, + } + for _, opt := range opts { + opt(o) + } + + publicKey, privateKey, err := keyGen.GenerateKeyPair() + if err != nil { + return nil, fmt.Errorf("failed to generate key pair: %w", err) + } + subjectKeyId, err := SubjectKeyIDFromPublicKey(publicKey) + if err != nil { + return nil, fmt.Errorf("failed to compute subject key ID: %w", err) + } + + sortedHostnames := sets.List(hostnames) + authorityKeyId := ca.Config.Certs[0].SubjectKeyId + + // Start from a server certificate template for the hostnames. + template := newServerCertificateTemplateForDuration( + pkix.Name{CommonName: sortedHostnames[0]}, + sortedHostnames, + o.lifetime, + time.Now, + authorityKeyId, + subjectKeyId, + ) + // Let x509.CreateCertificate auto-detect the signature algorithm from the CA's key. + template.SignatureAlgorithm = 0 + template.KeyUsage = KeyUsageForPublicKey(publicKey) + + // Set subject from user info for client authentication. + template.Subject = UserToSubject(u) + + // Enable both server and client authentication. + template.ExtKeyUsage = []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth} + + for _, fn := range o.extensionFns { + if err := fn(template); err != nil { + return nil, fmt.Errorf("failed to apply certificate extension: %w", err) + } + } + + cert, err := ca.SignCertificate(template, publicKey) + if err != nil { + return nil, fmt.Errorf("failed to sign certificate: %w", err) + } + + return &TLSCertificateConfig{ + Certs: append([]*x509.Certificate{cert}, ca.Config.Certs...), + Key: privateKey, + }, nil +} diff --git a/pkg/crypto/cert_config_test.go b/pkg/crypto/cert_config_test.go new file mode 100644 index 0000000000..d5dee9abf8 --- /dev/null +++ b/pkg/crypto/cert_config_test.go @@ -0,0 +1,482 @@ +package crypto + +import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "fmt" + "slices" + "testing" + "time" + + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apiserver/pkg/authentication/user" +) + +var allKeyPairGenerators = []KeyPairGenerator{ + RSAKeyPairGenerator{Bits: 2048}, + RSAKeyPairGenerator{Bits: 4096}, + ECDSAKeyPairGenerator{Curve: P256}, + ECDSAKeyPairGenerator{Curve: P384}, + ECDSAKeyPairGenerator{Curve: P521}, +} + +func keyGenName(g KeyPairGenerator) string { + switch g := g.(type) { + case RSAKeyPairGenerator: + return fmt.Sprintf("RSA-%d", g.Bits) + case ECDSAKeyPairGenerator: + return "ECDSA-" + string(g.Curve) + } + return "unknown" +} + +func TestKeyUsageForPublicKey(t *testing.T) { + rsaGen := RSAKeyPairGenerator{Bits: 2048} + rsaPub, _, err := rsaGen.GenerateKeyPair() + if err != nil { + t.Fatal(err) + } + rsaUsage := KeyUsageForPublicKey(rsaPub) + if rsaUsage != x509.KeyUsageKeyEncipherment|x509.KeyUsageDigitalSignature { + t.Errorf("RSA KeyUsage = %v, want KeyEncipherment|DigitalSignature", rsaUsage) + } + + ecdsaGen := ECDSAKeyPairGenerator{Curve: P256} + ecdsaPub, _, err := ecdsaGen.GenerateKeyPair() + if err != nil { + t.Fatal(err) + } + ecdsaUsage := KeyUsageForPublicKey(ecdsaPub) + if ecdsaUsage != x509.KeyUsageDigitalSignature { + t.Errorf("ECDSA KeyUsage = %v, want DigitalSignature only", ecdsaUsage) + } + if ecdsaUsage&x509.KeyUsageKeyEncipherment != 0 { + t.Error("ECDSA KeyUsage should not include KeyEncipherment") + } +} + +func TestGenerateKeyPair(t *testing.T) { + testCases := []struct { + name string + gen KeyPairGenerator + }{ + { + name: "RSA-2048", + gen: RSAKeyPairGenerator{Bits: 2048}, + }, + { + name: "RSA-4096", + gen: RSAKeyPairGenerator{Bits: 4096}, + }, + { + name: "ECDSA-P256", + gen: ECDSAKeyPairGenerator{Curve: P256}, + }, + { + name: "ECDSA-P384", + gen: ECDSAKeyPairGenerator{Curve: P384}, + }, + { + name: "ECDSA-P521", + gen: ECDSAKeyPairGenerator{Curve: P521}, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + pub, priv, err := tc.gen.GenerateKeyPair() + if err != nil { + t.Fatalf("GenerateKeyPair() error = %v", err) + } + if pub == nil || priv == nil { + t.Fatal("GenerateKeyPair() returned nil key") + } + + switch g := tc.gen.(type) { + case RSAKeyPairGenerator: + rsaPub, ok := pub.(*rsa.PublicKey) + if !ok { + t.Fatalf("expected *rsa.PublicKey, got %T", pub) + } + if rsaPub.N.BitLen() != g.Bits { + t.Errorf("RSA key size = %d, want %d", rsaPub.N.BitLen(), g.Bits) + } + case ECDSAKeyPairGenerator: + ecPub, ok := pub.(*ecdsa.PublicKey) + if !ok { + t.Fatalf("expected *ecdsa.PublicKey, got %T", pub) + } + wantCurve := curveForName(g.Curve) + if ecPub.Curve != wantCurve { + t.Errorf("ECDSA curve = %v, want %v", ecPub.Curve.Params().Name, wantCurve.Params().Name) + } + } + }) + } +} + +func TestSubjectKeyIDFromPublicKey(t *testing.T) { + for _, g := range allKeyPairGenerators { + t.Run(keyGenName(g), func(t *testing.T) { + pub, _, err := g.GenerateKeyPair() + if err != nil { + t.Fatalf("GenerateKeyPair() error = %v", err) + } + skid, err := SubjectKeyIDFromPublicKey(pub) + if err != nil { + t.Fatalf("SubjectKeyIDFromPublicKey() error = %v", err) + } + // Truncated SHA-256 produces 20-byte (160-bit) subject key ID per RFC 7093 + if len(skid) != 20 { + t.Errorf("SubjectKeyID length = %d, want 20", len(skid)) + } + }) + } +} + +func TestNewSigningCertificate(t *testing.T) { + for _, g := range allKeyPairGenerators { + t.Run(keyGenName(g), func(t *testing.T) { + config, err := NewSigningCertificate("test-ca", g, + WithLifetime(24*time.Hour), + ) + if err != nil { + t.Fatalf("NewSigningCertificate() error = %v", err) + } + if len(config.Certs) != 1 { + t.Fatalf("expected 1 cert, got %d", len(config.Certs)) + } + + cert := config.Certs[0] + if cert.Subject.CommonName != "test-ca" { + t.Errorf("CN = %q, want %q", cert.Subject.CommonName, "test-ca") + } + if !cert.IsCA { + t.Error("expected IsCA to be true") + } + if len(cert.SubjectKeyId) == 0 { + t.Error("SubjectKeyId is empty") + } + if len(cert.AuthorityKeyId) == 0 { + t.Error("AuthorityKeyId is empty") + } + // Self-signed: SubjectKeyId == AuthorityKeyId + if string(cert.SubjectKeyId) != string(cert.AuthorityKeyId) { + t.Error("self-signed CA should have SubjectKeyId == AuthorityKeyId") + } + + assertKeyType(t, config.Key, g) + }) + } +} + +func TestNewSigningCertificate_WithSubject(t *testing.T) { + g := RSAKeyPairGenerator{Bits: 2048} + config, err := NewSigningCertificate("ignored-name", g, + WithSubject(x509pkixName("custom-cn", "custom-org")), + ) + if err != nil { + t.Fatalf("NewSigningCertificate() error = %v", err) + } + cert := config.Certs[0] + if cert.Subject.CommonName != "custom-cn" { + t.Errorf("CN = %q, want %q", cert.Subject.CommonName, "custom-cn") + } + if len(cert.Subject.Organization) != 1 || cert.Subject.Organization[0] != "custom-org" { + t.Errorf("Organization = %v, want [custom-org]", cert.Subject.Organization) + } +} + +func TestNewSigningCertificate_WithSigner(t *testing.T) { + testCases := []struct { + name string + rootKG KeyPairGenerator + intKG KeyPairGenerator + }{ + { + name: "RSA root, RSA intermediate", + rootKG: RSAKeyPairGenerator{Bits: 2048}, + intKG: RSAKeyPairGenerator{Bits: 4096}, + }, + { + name: "RSA root, ECDSA intermediate", + rootKG: RSAKeyPairGenerator{Bits: 4096}, + intKG: ECDSAKeyPairGenerator{Curve: P256}, + }, + { + name: "ECDSA root, RSA intermediate", + rootKG: ECDSAKeyPairGenerator{Curve: P384}, + intKG: RSAKeyPairGenerator{Bits: 2048}, + }, + { + name: "ECDSA root, ECDSA intermediate", + rootKG: ECDSAKeyPairGenerator{Curve: P256}, + intKG: ECDSAKeyPairGenerator{Curve: P384}, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + rootConfig, err := NewSigningCertificate("root-ca", tc.rootKG) + if err != nil { + t.Fatalf("root NewSigningCertificate() error = %v", err) + } + rootCA := &CA{ + Config: rootConfig, + SerialGenerator: &RandomSerialGenerator{}, + } + + intConfig, err := NewSigningCertificate("intermediate-ca", tc.intKG, + WithSigner(rootCA), + WithLifetime(24*time.Hour), + ) + if err != nil { + t.Fatalf("intermediate NewSigningCertificate() error = %v", err) + } + + intCert := intConfig.Certs[0] + + if !intCert.IsCA { + t.Error("intermediate cert should be CA") + } + if intCert.Subject.CommonName != "intermediate-ca" { + t.Errorf("CN = %q, want %q", intCert.Subject.CommonName, "intermediate-ca") + } + + // AuthorityKeyId should match the root's SubjectKeyId + if string(intCert.AuthorityKeyId) != string(rootConfig.Certs[0].SubjectKeyId) { + t.Error("intermediate AuthorityKeyId should match root SubjectKeyId") + } + // SubjectKeyId should differ from root + if string(intCert.SubjectKeyId) == string(rootConfig.Certs[0].SubjectKeyId) { + t.Error("intermediate SubjectKeyId should differ from root") + } + + // Cert chain should include both intermediate and root + if len(intConfig.Certs) != 2 { + t.Fatalf("expected 2 certs in chain, got %d", len(intConfig.Certs)) + } + if intConfig.Certs[1].Subject.CommonName != "root-ca" { + t.Errorf("chain[1] CN = %q, want %q", intConfig.Certs[1].Subject.CommonName, "root-ca") + } + + // Verify the key type matches the intermediate's config + assertKeyType(t, intConfig.Key, tc.intKG) + }) + } +} + +func TestNewServerCertificate(t *testing.T) { + for _, caKG := range allKeyPairGenerators { + for _, leafKG := range allKeyPairGenerators { + name := keyGenName(caKG) + "-CA_" + keyGenName(leafKG) + "-leaf" + t.Run(name, func(t *testing.T) { + caConfig, err := NewSigningCertificate("test-ca", caKG) + if err != nil { + t.Fatalf("NewSigningCertificate() error = %v", err) + } + ca := &CA{ + Config: caConfig, + SerialGenerator: &RandomSerialGenerator{}, + } + + hostnames := sets.New("localhost", "127.0.0.1", "example.com") + serverConfig, err := ca.NewServerCertificate(hostnames, leafKG, + WithLifetime(1*time.Hour), + ) + if err != nil { + t.Fatalf("NewServerCertificate() error = %v", err) + } + + cert := serverConfig.Certs[0] + + if cert.IsCA { + t.Error("server cert should not be CA") + } + assertHasEKU(t, cert, x509.ExtKeyUsageServerAuth) + assertKeyType(t, serverConfig.Key, leafKG) + + // Verify hostnames are in SANs + if !slices.Contains(cert.DNSNames, "localhost") { + t.Error("missing DNS SAN: localhost") + } + if !slices.Contains(cert.DNSNames, "example.com") { + t.Error("missing DNS SAN: example.com") + } + + // Verify chain: leaf signed by CA + if string(cert.AuthorityKeyId) != string(caConfig.Certs[0].SubjectKeyId) { + t.Error("leaf AuthorityKeyId should match CA SubjectKeyId") + } + }) + } + } +} + +func TestNewClientCertificate(t *testing.T) { + for _, caKG := range allKeyPairGenerators { + for _, leafKG := range allKeyPairGenerators { + name := keyGenName(caKG) + "-CA_" + keyGenName(leafKG) + "-leaf" + t.Run(name, func(t *testing.T) { + caConfig, err := NewSigningCertificate("test-ca", caKG) + if err != nil { + t.Fatalf("NewSigningCertificate() error = %v", err) + } + ca := &CA{ + Config: caConfig, + SerialGenerator: &RandomSerialGenerator{}, + } + + u := &user.DefaultInfo{ + Name: "system:test-user", + Groups: []string{"system:masters"}, + } + clientConfig, err := ca.NewClientCertificate(u, leafKG, + WithLifetime(1*time.Hour), + ) + if err != nil { + t.Fatalf("NewClientCertificate() error = %v", err) + } + + cert := clientConfig.Certs[0] + + if cert.IsCA { + t.Error("client cert should not be CA") + } + assertHasEKU(t, cert, x509.ExtKeyUsageClientAuth) + if cert.Subject.CommonName != "system:test-user" { + t.Errorf("CN = %q, want %q", cert.Subject.CommonName, "system:test-user") + } + }) + } + } +} + +func TestNewPeerCertificate(t *testing.T) { + for _, g := range allKeyPairGenerators { + t.Run(keyGenName(g), func(t *testing.T) { + caConfig, err := NewSigningCertificate("test-ca", g) + if err != nil { + t.Fatalf("NewSigningCertificate() error = %v", err) + } + ca := &CA{ + Config: caConfig, + SerialGenerator: &RandomSerialGenerator{}, + } + + hostnames := sets.New("peer.example.com") + u := &user.DefaultInfo{ + Name: "system:peer", + Groups: []string{"system:nodes"}, + } + peerConfig, err := ca.NewPeerCertificate(hostnames, u, g, + WithLifetime(1*time.Hour), + ) + if err != nil { + t.Fatalf("NewPeerCertificate() error = %v", err) + } + + cert := peerConfig.Certs[0] + + // Peer cert should have both ServerAuth and ClientAuth + assertHasEKU(t, cert, x509.ExtKeyUsageServerAuth) + assertHasEKU(t, cert, x509.ExtKeyUsageClientAuth) + + // Should have hostnames in SANs + if !slices.Contains(cert.DNSNames, "peer.example.com") { + t.Error("missing DNS SAN: peer.example.com") + } + + // Subject should come from user info + if cert.Subject.CommonName != "system:peer" { + t.Errorf("CN = %q, want %q", cert.Subject.CommonName, "system:peer") + } + + if cert.IsCA { + t.Error("peer cert should not be CA") + } + }) + } +} + +func TestNewServerCertificate_WithExtensions(t *testing.T) { + g := RSAKeyPairGenerator{Bits: 2048} + caConfig, err := NewSigningCertificate("test-ca", g) + if err != nil { + t.Fatalf("NewSigningCertificate() error = %v", err) + } + ca := &CA{ + Config: caConfig, + SerialGenerator: &RandomSerialGenerator{}, + } + + extensionCalled := false + serverConfig, err := ca.NewServerCertificate( + sets.New("localhost"), + g, + WithExtensions(func(cert *x509.Certificate) error { + extensionCalled = true + return nil + }), + ) + if err != nil { + t.Fatalf("NewServerCertificate() error = %v", err) + } + if !extensionCalled { + t.Error("extension function was not called") + } + if serverConfig == nil { + t.Error("server config is nil") + } +} + +// Helper functions + +func assertKeyType(t *testing.T, key any, g KeyPairGenerator) { + t.Helper() + switch g := g.(type) { + case RSAKeyPairGenerator: + rsaKey, ok := key.(*rsa.PrivateKey) + if !ok { + t.Errorf("expected *rsa.PrivateKey, got %T", key) + return + } + if rsaKey.N.BitLen() != g.Bits { + t.Errorf("RSA key size = %d, want %d", rsaKey.N.BitLen(), g.Bits) + } + case ECDSAKeyPairGenerator: + ecKey, ok := key.(*ecdsa.PrivateKey) + if !ok { + t.Errorf("expected *ecdsa.PrivateKey, got %T", key) + return + } + wantCurve := curveForName(g.Curve) + if ecKey.Curve != wantCurve { + t.Errorf("ECDSA curve = %v, want %v", ecKey.Curve.Params().Name, wantCurve.Params().Name) + } + } +} + +func assertHasEKU(t *testing.T, cert *x509.Certificate, eku x509.ExtKeyUsage) { + t.Helper() + if !slices.Contains(cert.ExtKeyUsage, eku) { + t.Errorf("certificate missing ExtKeyUsage %v", eku) + } +} + +func curveForName(c ECDSACurve) elliptic.Curve { + switch c { + case P256: + return elliptic.P256() + case P384: + return elliptic.P384() + case P521: + return elliptic.P521() + } + return nil +} + +func x509pkixName(cn, org string) pkix.Name { + return pkix.Name{CommonName: cn, Organization: []string{org}} +} diff --git a/pkg/crypto/keygen.go b/pkg/crypto/keygen.go new file mode 100644 index 0000000000..96b7559468 --- /dev/null +++ b/pkg/crypto/keygen.go @@ -0,0 +1,118 @@ +package crypto + +import ( + "crypto" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/rsa" + "crypto/sha256" + "crypto/x509" + "fmt" +) + +// KeyAlgorithm identifies the key generation algorithm. +type KeyAlgorithm string + +const ( + // RSAKeyAlgorithm specifies RSA key generation. + RSAKeyAlgorithm KeyAlgorithm = "RSA" + // ECDSAKeyAlgorithm specifies ECDSA key generation. + ECDSAKeyAlgorithm KeyAlgorithm = "ECDSA" +) + +// ECDSACurve identifies a named ECDSA curve. +type ECDSACurve string + +const ( + // P256 specifies the NIST P-256 curve (secp256r1), providing 128-bit security. + P256 ECDSACurve = "P256" + // P384 specifies the NIST P-384 curve (secp384r1), providing 192-bit security. + P384 ECDSACurve = "P384" + // P521 specifies the NIST P-521 curve (secp521r1), providing 256-bit security. + P521 ECDSACurve = "P521" +) + +// RSAKeyPairGenerator generates RSA key pairs. +type RSAKeyPairGenerator struct { + // Bits is the RSA key size in bits. Must be >= 2048. + Bits int +} + +func (g RSAKeyPairGenerator) GenerateKeyPair() (crypto.PublicKey, crypto.PrivateKey, error) { + bits := g.Bits + if bits == 0 { + bits = keyBits + } + privateKey, err := rsa.GenerateKey(rand.Reader, bits) + if err != nil { + return nil, nil, err + } + return &privateKey.PublicKey, privateKey, nil +} + +// ECDSAKeyPairGenerator generates ECDSA key pairs. +type ECDSAKeyPairGenerator struct { + // Curve is the named ECDSA curve. + Curve ECDSACurve +} + +func (g ECDSAKeyPairGenerator) GenerateKeyPair() (crypto.PublicKey, crypto.PrivateKey, error) { + curve, err := g.ellipticCurve() + if err != nil { + return nil, nil, err + } + privateKey, err := ecdsa.GenerateKey(curve, rand.Reader) + if err != nil { + return nil, nil, err + } + return &privateKey.PublicKey, privateKey, nil +} + +func (g ECDSAKeyPairGenerator) ellipticCurve() (elliptic.Curve, error) { + switch g.Curve { + case P256: + return elliptic.P256(), nil + case P384: + return elliptic.P384(), nil + case P521: + return elliptic.P521(), nil + default: + return nil, fmt.Errorf("unsupported ECDSA curve: %q", g.Curve) + } +} + +// KeyUsageForPublicKey returns the x509.KeyUsage flags appropriate for the +// given public key type. ECDSA keys use DigitalSignature only; RSA keys also +// include KeyEncipherment. +func KeyUsageForPublicKey(pub crypto.PublicKey) x509.KeyUsage { + switch pub.(type) { + case *ecdsa.PublicKey: + return x509.KeyUsageDigitalSignature + default: + return x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature + } +} + +// SubjectKeyIDFromPublicKey computes a truncated SHA-256 hash suitable for +// use as a certificate SubjectKeyId from any supported public key type. +// This uses the first 160 bits of the SHA-256 hash per RFC 7093, consistent +// with the Go standard library since Go 1.25 (go.dev/issue/71746) and +// Let's Encrypt. Prior Go versions used SHA-1 which is not FIPS-compatible. +func SubjectKeyIDFromPublicKey(pub crypto.PublicKey) ([]byte, error) { + var rawBytes []byte + switch pub := pub.(type) { + case *rsa.PublicKey: + rawBytes = pub.N.Bytes() + case *ecdsa.PublicKey: + ecdhKey, err := pub.ECDH() + if err != nil { + return nil, fmt.Errorf("failed to convert ECDSA public key: %w", err) + } + rawBytes = ecdhKey.Bytes() + default: + return nil, fmt.Errorf("unsupported public key type: %T", pub) + } + hash := sha256.Sum256(rawBytes) + return hash[:20], nil +} diff --git a/pkg/crypto/options.go b/pkg/crypto/options.go new file mode 100644 index 0000000000..983b115d22 --- /dev/null +++ b/pkg/crypto/options.go @@ -0,0 +1,49 @@ +package crypto + +import ( + "crypto/x509/pkix" + "time" +) + +// CertificateOptions holds optional configuration collected from functional options. +type CertificateOptions struct { + lifetime time.Duration + subject *pkix.Name + extensionFns []CertificateExtensionFunc + signer *CA +} + +// CertificateOption is a functional option for certificate creation. +type CertificateOption func(*CertificateOptions) + +// WithLifetime sets the certificate lifetime duration. +func WithLifetime(d time.Duration) CertificateOption { + return func(o *CertificateOptions) { + o.lifetime = d + } +} + +// WithSubject overrides the certificate subject. For signing certificates, +// this overrides the default subject derived from the name parameter. +func WithSubject(s pkix.Name) CertificateOption { + return func(o *CertificateOptions) { + o.subject = &s + } +} + +// WithSigner specifies a CA to sign the certificate. When used with +// NewSigningCertificate, this creates an intermediate CA signed by the +// given CA instead of a self-signed root CA. +func WithSigner(ca *CA) CertificateOption { + return func(o *CertificateOptions) { + o.signer = ca + } +} + +// WithExtensions adds certificate extension functions that are called +// to modify the certificate template before signing. +func WithExtensions(fns ...CertificateExtensionFunc) CertificateOption { + return func(o *CertificateOptions) { + o.extensionFns = append(o.extensionFns, fns...) + } +} From 1d2970c8beb4d7df87817cd4165fbb002325118d Mon Sep 17 00:00:00 2001 From: Luis Sanchez Date: Thu, 5 Mar 2026 11:42:47 -0500 Subject: [PATCH 2/2] pki: add PKI profile management with resolution and tests Introduce PKI profile utilities to manage certificate configurations, including resolution, key strength comparisons, and API conversions. Add comprehensive unit tests to validate all functionality. --- pkg/pki/profile.go | 114 +++++++++++++++++ pkg/pki/profile_test.go | 268 +++++++++++++++++++++++++++++++++++++++ pkg/pki/provider.go | 75 +++++++++++ pkg/pki/provider_test.go | 216 +++++++++++++++++++++++++++++++ pkg/pki/resolve.go | 77 +++++++++++ pkg/pki/resolve_test.go | 162 +++++++++++++++++++++++ pkg/pki/types.go | 23 ++++ 7 files changed, 935 insertions(+) create mode 100644 pkg/pki/profile.go create mode 100644 pkg/pki/profile_test.go create mode 100644 pkg/pki/provider.go create mode 100644 pkg/pki/provider_test.go create mode 100644 pkg/pki/resolve.go create mode 100644 pkg/pki/resolve_test.go create mode 100644 pkg/pki/types.go diff --git a/pkg/pki/profile.go b/pkg/pki/profile.go new file mode 100644 index 0000000000..6f534c94f0 --- /dev/null +++ b/pkg/pki/profile.go @@ -0,0 +1,114 @@ +package pki + +import ( + "fmt" + + configv1alpha1 "github.com/openshift/api/config/v1alpha1" + "github.com/openshift/library-go/pkg/crypto" +) + +// DefaultPKIProfile returns the default PKIProfile for OpenShift. +func DefaultPKIProfile() configv1alpha1.PKIProfile { + return configv1alpha1.PKIProfile{ + Defaults: configv1alpha1.DefaultCertificateConfig{ + Key: configv1alpha1.KeyConfig{ + Algorithm: configv1alpha1.KeyAlgorithmECDSA, + ECDSA: configv1alpha1.ECDSAKeyConfig{Curve: configv1alpha1.ECDSACurveP256}, + }, + }, + SignerCertificates: configv1alpha1.CertificateConfig{ + Key: configv1alpha1.KeyConfig{ + Algorithm: configv1alpha1.KeyAlgorithmECDSA, + ECDSA: configv1alpha1.ECDSAKeyConfig{Curve: configv1alpha1.ECDSACurveP384}, + }, + }, + } +} + +// KeyPairGeneratorFromAPI converts a configv1alpha1.KeyConfig to a +// crypto.KeyPairGenerator. +func KeyPairGeneratorFromAPI(apiKey configv1alpha1.KeyConfig) (crypto.KeyPairGenerator, error) { + switch apiKey.Algorithm { + case configv1alpha1.KeyAlgorithmRSA: + return crypto.RSAKeyPairGenerator{ + Bits: int(apiKey.RSA.KeySize), + }, nil + case configv1alpha1.KeyAlgorithmECDSA: + curve, err := ecdsaCurveFromAPI(apiKey.ECDSA.Curve) + if err != nil { + return nil, err + } + return crypto.ECDSAKeyPairGenerator{ + Curve: curve, + }, nil + default: + return nil, fmt.Errorf("unknown key algorithm: %q", apiKey.Algorithm) + } +} + +// ecdsaCurveFromAPI converts an API ECDSA curve name to the crypto package's ECDSACurve. +func ecdsaCurveFromAPI(c configv1alpha1.ECDSACurve) (crypto.ECDSACurve, error) { + switch c { + case configv1alpha1.ECDSACurveP256: + return crypto.P256, nil + case configv1alpha1.ECDSACurveP384: + return crypto.P384, nil + case configv1alpha1.ECDSACurveP521: + return crypto.P521, nil + default: + return "", fmt.Errorf("unknown ECDSA curve: %q", c) + } +} + +// securityBits returns the NIST security strength in bits for a given +// KeyPairGenerator. For RSA, values come from the rsaSecurityStrength table. +// For ECDSA, security strength is half the key size (fixed per curve). +func securityBits(g crypto.KeyPairGenerator) int { + switch g := g.(type) { + case crypto.RSAKeyPairGenerator: + return rsaSecurityStrength[g.Bits] + case crypto.ECDSAKeyPairGenerator: + switch g.Curve { + case crypto.P256: + return 128 + case crypto.P384: + return 192 + case crypto.P521: + return 256 + } + } + return 0 +} + +// rsaSecurityStrength maps RSA key sizes (2048-8192 in 1024-bit increments) +// to their security strengths from NIST SP 800-56B Rev 2 Table 2 or +// pre-calculated from the GNFS complexity estimate. +var rsaSecurityStrength = map[int]int{ + 2048: 112, + 3072: 128, + 4096: 152, + 5120: 168, + 6144: 176, + 7168: 192, + 8192: 200, +} + +// strongerKeyPairGenerator returns whichever of a or b provides higher NIST +// security strength. In case of a tie, ECDSA is preferred over RSA. +func strongerKeyPairGenerator(a, b crypto.KeyPairGenerator) crypto.KeyPairGenerator { + sa, sb := securityBits(a), securityBits(b) + if sb > sa { + return b + } + if sa > sb { + return a + } + // Equal strength: prefer ECDSA over RSA. + if _, ok := a.(crypto.ECDSAKeyPairGenerator); ok { + return a + } + if _, ok := b.(crypto.ECDSAKeyPairGenerator); ok { + return b + } + return a +} diff --git a/pkg/pki/profile_test.go b/pkg/pki/profile_test.go new file mode 100644 index 0000000000..445106bffc --- /dev/null +++ b/pkg/pki/profile_test.go @@ -0,0 +1,268 @@ +package pki + +import ( + "testing" + + configv1alpha1 "github.com/openshift/api/config/v1alpha1" + "github.com/openshift/library-go/pkg/crypto" +) + +func TestDefaultPKIProfile(t *testing.T) { + profile := DefaultPKIProfile() + + // Default should be ECDSA P-256 + if profile.Defaults.Key.Algorithm != configv1alpha1.KeyAlgorithmECDSA { + t.Errorf("defaults algorithm = %v, want ECDSA", profile.Defaults.Key.Algorithm) + } + if profile.Defaults.Key.ECDSA.Curve != configv1alpha1.ECDSACurveP256 { + t.Errorf("defaults ECDSA curve = %v, want P-256", profile.Defaults.Key.ECDSA.Curve) + } + + // Signers should be ECDSA P-384 + if profile.SignerCertificates.Key.Algorithm != configv1alpha1.KeyAlgorithmECDSA { + t.Errorf("signer algorithm = %v, want ECDSA", profile.SignerCertificates.Key.Algorithm) + } + if profile.SignerCertificates.Key.ECDSA.Curve != configv1alpha1.ECDSACurveP384 { + t.Errorf("signer ECDSA curve = %v, want P-384", profile.SignerCertificates.Key.ECDSA.Curve) + } + + // Serving and client should be unset (fall back to defaults) + if profile.ServingCertificates.Key.Algorithm != "" { + t.Errorf("serving algorithm = %v, want empty (unset)", profile.ServingCertificates.Key.Algorithm) + } + if profile.ClientCertificates.Key.Algorithm != "" { + t.Errorf("client algorithm = %v, want empty (unset)", profile.ClientCertificates.Key.Algorithm) + } +} + +func TestKeyPairGeneratorFromAPI(t *testing.T) { + testCases := []struct { + name string + input configv1alpha1.KeyConfig + want crypto.KeyPairGenerator + wantErr bool + }{ + { + name: "RSA-2048", + input: configv1alpha1.KeyConfig{ + Algorithm: configv1alpha1.KeyAlgorithmRSA, + RSA: configv1alpha1.RSAKeyConfig{KeySize: 2048}, + }, + want: crypto.RSAKeyPairGenerator{Bits: 2048}, + }, + { + name: "RSA-4096", + input: configv1alpha1.KeyConfig{ + Algorithm: configv1alpha1.KeyAlgorithmRSA, + RSA: configv1alpha1.RSAKeyConfig{KeySize: 4096}, + }, + want: crypto.RSAKeyPairGenerator{Bits: 4096}, + }, + { + name: "ECDSA-P256", + input: configv1alpha1.KeyConfig{ + Algorithm: configv1alpha1.KeyAlgorithmECDSA, + ECDSA: configv1alpha1.ECDSAKeyConfig{Curve: configv1alpha1.ECDSACurveP256}, + }, + want: crypto.ECDSAKeyPairGenerator{Curve: crypto.P256}, + }, + { + name: "ECDSA-P384", + input: configv1alpha1.KeyConfig{ + Algorithm: configv1alpha1.KeyAlgorithmECDSA, + ECDSA: configv1alpha1.ECDSAKeyConfig{Curve: configv1alpha1.ECDSACurveP384}, + }, + want: crypto.ECDSAKeyPairGenerator{Curve: crypto.P384}, + }, + { + name: "ECDSA-P521", + input: configv1alpha1.KeyConfig{ + Algorithm: configv1alpha1.KeyAlgorithmECDSA, + ECDSA: configv1alpha1.ECDSAKeyConfig{Curve: configv1alpha1.ECDSACurveP521}, + }, + want: crypto.ECDSAKeyPairGenerator{Curve: crypto.P521}, + }, + { + name: "unknown algorithm", + input: configv1alpha1.KeyConfig{ + Algorithm: "unknown", + }, + wantErr: true, + }, + { + name: "unknown ECDSA curve", + input: configv1alpha1.KeyConfig{ + Algorithm: configv1alpha1.KeyAlgorithmECDSA, + ECDSA: configv1alpha1.ECDSAKeyConfig{Curve: "unknown"}, + }, + wantErr: true, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got, err := KeyPairGeneratorFromAPI(tc.input) + if (err != nil) != tc.wantErr { + t.Errorf("KeyPairGeneratorFromAPI() error = %v, wantErr %v", err, tc.wantErr) + return + } + if !tc.wantErr && got != tc.want { + t.Errorf("KeyPairGeneratorFromAPI() = %+v, want %+v", got, tc.want) + } + }) + } +} + +func TestSecurityBits(t *testing.T) { + testCases := []struct { + name string + g crypto.KeyPairGenerator + want int + }{ + { + name: "RSA-2048", + g: crypto.RSAKeyPairGenerator{Bits: 2048}, + want: 112, + }, + { + name: "RSA-3072", + g: crypto.RSAKeyPairGenerator{Bits: 3072}, + want: 128, + }, + { + name: "RSA-4096", + g: crypto.RSAKeyPairGenerator{Bits: 4096}, + want: 152, + }, + { + name: "RSA-5120", + g: crypto.RSAKeyPairGenerator{Bits: 5120}, + want: 168, + }, + { + name: "RSA-6144", + g: crypto.RSAKeyPairGenerator{Bits: 6144}, + want: 176, + }, + { + name: "RSA-7168", + g: crypto.RSAKeyPairGenerator{Bits: 7168}, + want: 192, + }, + { + name: "RSA-8192", + g: crypto.RSAKeyPairGenerator{Bits: 8192}, + want: 200, + }, + { + name: "RSA-1024 unsupported", + g: crypto.RSAKeyPairGenerator{Bits: 1024}, + want: 0, + }, + { + name: "ECDSA-P256", + g: crypto.ECDSAKeyPairGenerator{Curve: crypto.P256}, + want: 128, + }, + { + name: "ECDSA-P384", + g: crypto.ECDSAKeyPairGenerator{Curve: crypto.P384}, + want: 192, + }, + { + name: "ECDSA-P521", + g: crypto.ECDSAKeyPairGenerator{Curve: crypto.P521}, + want: 256, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got := securityBits(tc.g) + if got != tc.want { + t.Errorf("securityBits(%s) = %d, want %d", tc.name, got, tc.want) + } + }) + } +} + +func TestStrongerKeyPairGenerator(t *testing.T) { + rsa2048 := crypto.RSAKeyPairGenerator{Bits: 2048} + rsa3072 := crypto.RSAKeyPairGenerator{Bits: 3072} + rsa4096 := crypto.RSAKeyPairGenerator{Bits: 4096} + ecP256 := crypto.ECDSAKeyPairGenerator{Curve: crypto.P256} + ecP384 := crypto.ECDSAKeyPairGenerator{Curve: crypto.P384} + + testCases := []struct { + name string + a, b crypto.KeyPairGenerator + want crypto.KeyPairGenerator + }{ + { + name: "RSA-2048 vs RSA-4096", + a: rsa2048, + b: rsa4096, + want: rsa4096, + }, + { + name: "RSA-4096 vs RSA-2048", + a: rsa4096, + b: rsa2048, + want: rsa4096, + }, + { + name: "RSA-2048 vs ECDSA-P256", // 112 vs 128 + a: rsa2048, + b: ecP256, + want: ecP256, + }, + { + name: "ECDSA-P256 vs RSA-2048", // 128 vs 112 + a: ecP256, + b: rsa2048, + want: ecP256, + }, + { + name: "RSA-4096 vs ECDSA-P256", // 152 vs 128 + a: rsa4096, + b: ecP256, + want: rsa4096, + }, + { + name: "ECDSA-P256 vs ECDSA-P384", // 128 vs 192 + a: ecP256, + b: ecP384, + want: ecP384, + }, + { + name: "RSA-3072 vs ECDSA-P256 (tie: prefer ECDSA)", // 128 vs 128 + a: rsa3072, + b: ecP256, + want: ecP256, + }, + { + name: "ECDSA-P256 vs RSA-3072 (tie: prefer ECDSA)", // 128 vs 128 + a: ecP256, + b: rsa3072, + want: ecP256, + }, + { + name: "equal RSA: returns first", + a: rsa2048, + b: rsa2048, + want: rsa2048, + }, + { + name: "equal ECDSA: returns first", + a: ecP256, + b: ecP256, + want: ecP256, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got := strongerKeyPairGenerator(tc.a, tc.b) + if got != tc.want { + t.Errorf("strongerKeyPairGenerator() = %+v, want %+v", got, tc.want) + } + }) + } +} diff --git a/pkg/pki/provider.go b/pkg/pki/provider.go new file mode 100644 index 0000000000..2007aa890e --- /dev/null +++ b/pkg/pki/provider.go @@ -0,0 +1,75 @@ +package pki + +import ( + "fmt" + + configv1alpha1 "github.com/openshift/api/config/v1alpha1" + configv1alpha1listers "github.com/openshift/client-go/config/listers/config/v1alpha1" +) + +// PKIProfileProvider provides the PKIProfile that determines certificate key +// configuration. A nil profile indicates Unmanaged mode where the caller +// should use its own defaults. +type PKIProfileProvider interface { + PKIProfile() (*configv1alpha1.PKIProfile, error) +} + +// StaticPKIProfileProvider is a PKIProfileProvider backed by a fixed PKIProfile. +type StaticPKIProfileProvider struct { + profile *configv1alpha1.PKIProfile +} + +// NewStaticPKIProfileProvider returns a PKIProfileProvider backed by the given +// profile. A nil profile signals Unmanaged mode. +func NewStaticPKIProfileProvider(profile *configv1alpha1.PKIProfile) *StaticPKIProfileProvider { + return &StaticPKIProfileProvider{profile: profile} +} + +// PKIProfile returns the static PKIProfile. +func (s *StaticPKIProfileProvider) PKIProfile() (*configv1alpha1.PKIProfile, error) { + return s.profile, nil +} + +// ListerPKIProfileProvider is a PKIProfileProvider that reads a named +// cluster-scoped PKI resource via a lister. +type ListerPKIProfileProvider struct { + lister configv1alpha1listers.PKILister + resourceName string +} + +// NewClusterPKIProfileProvider creates a PKIProfileProvider that resolves the +// PKIProfile from the OpenShift cluster configuration PKI resource. +func NewClusterPKIProfileProvider(lister configv1alpha1listers.PKILister) *ListerPKIProfileProvider { + return NewListerPKIProfileProvider(lister, "cluster") +} + +// NewListerPKIProfileProvider returns a PKIProfileProvider that reads the +// named cluster-scoped PKI resource via a lister. +func NewListerPKIProfileProvider(lister configv1alpha1listers.PKILister, resourceName string) *ListerPKIProfileProvider { + return &ListerPKIProfileProvider{ + lister: lister, + resourceName: resourceName, + } +} + +// PKIProfile reads the PKI resource and returns the profile based on its +// certificate management mode. Returns nil for Unmanaged mode. +func (l *ListerPKIProfileProvider) PKIProfile() (*configv1alpha1.PKIProfile, error) { + pki, err := l.lister.Get(l.resourceName) + if err != nil { + return nil, fmt.Errorf("failed to get PKI resource %q: %w", l.resourceName, err) + } + + switch pki.Spec.CertificateManagement.Mode { + case configv1alpha1.PKICertificateManagementModeUnmanaged: + return nil, nil + case configv1alpha1.PKICertificateManagementModeDefault: + profile := DefaultPKIProfile() + return &profile, nil + case configv1alpha1.PKICertificateManagementModeCustom: + profile := pki.Spec.CertificateManagement.Custom.PKIProfile + return &profile, nil + default: + return nil, fmt.Errorf("unknown PKI certificate management mode: %q", pki.Spec.CertificateManagement.Mode) + } +} diff --git a/pkg/pki/provider_test.go b/pkg/pki/provider_test.go new file mode 100644 index 0000000000..91f5de76e3 --- /dev/null +++ b/pkg/pki/provider_test.go @@ -0,0 +1,216 @@ +package pki + +import ( + "fmt" + "strings" + "testing" + + configv1alpha1 "github.com/openshift/api/config/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + + "github.com/openshift/library-go/pkg/crypto" +) + +// fakePKILister implements configv1alpha1listers.PKILister for testing. +type fakePKILister struct { + pki *configv1alpha1.PKI + err error +} + +func (f *fakePKILister) List(_ labels.Selector) ([]*configv1alpha1.PKI, error) { + panic("not implemented") +} + +func (f *fakePKILister) Get(_ string) (*configv1alpha1.PKI, error) { + return f.pki, f.err +} + +func TestListerPKIProfileProvider_Unmanaged(t *testing.T) { + lister := &fakePKILister{ + pki: &configv1alpha1.PKI{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: configv1alpha1.PKISpec{ + CertificateManagement: configv1alpha1.PKICertificateManagement{ + Mode: configv1alpha1.PKICertificateManagementModeUnmanaged, + }, + }, + }, + } + provider := NewClusterPKIProfileProvider(lister) + + profile, err := provider.PKIProfile() + if err != nil { + t.Fatalf("PKIProfile() error = %v", err) + } + if profile != nil { + t.Errorf("expected nil profile for Unmanaged mode, got %+v", profile) + } +} + +func TestListerPKIProfileProvider_Default(t *testing.T) { + lister := &fakePKILister{ + pki: &configv1alpha1.PKI{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: configv1alpha1.PKISpec{ + CertificateManagement: configv1alpha1.PKICertificateManagement{ + Mode: configv1alpha1.PKICertificateManagementModeDefault, + }, + }, + }, + } + provider := NewClusterPKIProfileProvider(lister) + + profile, err := provider.PKIProfile() + if err != nil { + t.Fatalf("PKIProfile() error = %v", err) + } + if profile == nil { + t.Fatal("expected non-nil profile for Default mode") + } + + // Should match DefaultPKIProfile() + want := DefaultPKIProfile() + if profile.Defaults.Key.Algorithm != want.Defaults.Key.Algorithm { + t.Errorf("defaults algorithm = %v, want %v", profile.Defaults.Key.Algorithm, want.Defaults.Key.Algorithm) + } +} + +func TestListerPKIProfileProvider_Custom(t *testing.T) { + customProfile := configv1alpha1.PKIProfile{ + Defaults: configv1alpha1.DefaultCertificateConfig{ + Key: configv1alpha1.KeyConfig{ + Algorithm: configv1alpha1.KeyAlgorithmRSA, + RSA: configv1alpha1.RSAKeyConfig{KeySize: 4096}, + }, + }, + } + lister := &fakePKILister{ + pki: &configv1alpha1.PKI{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: configv1alpha1.PKISpec{ + CertificateManagement: configv1alpha1.PKICertificateManagement{ + Mode: configv1alpha1.PKICertificateManagementModeCustom, + Custom: configv1alpha1.CustomPKIPolicy{ + PKIProfile: customProfile, + }, + }, + }, + }, + } + provider := NewClusterPKIProfileProvider(lister) + + profile, err := provider.PKIProfile() + if err != nil { + t.Fatalf("PKIProfile() error = %v", err) + } + if profile == nil { + t.Fatal("expected non-nil profile for Custom mode") + } + if profile.Defaults.Key.Algorithm != configv1alpha1.KeyAlgorithmRSA { + t.Errorf("defaults algorithm = %v, want RSA", profile.Defaults.Key.Algorithm) + } + if profile.Defaults.Key.RSA.KeySize != 4096 { + t.Errorf("defaults RSA key size = %d, want 4096", profile.Defaults.Key.RSA.KeySize) + } +} + +func TestListerPKIProfileProvider_UnknownMode(t *testing.T) { + lister := &fakePKILister{ + pki: &configv1alpha1.PKI{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: configv1alpha1.PKISpec{ + CertificateManagement: configv1alpha1.PKICertificateManagement{ + Mode: "SomeFutureMode", + }, + }, + }, + } + provider := NewClusterPKIProfileProvider(lister) + + _, err := provider.PKIProfile() + if err == nil { + t.Error("expected error for unknown mode") + } +} + +func TestListerPKIProfileProvider_ListerError(t *testing.T) { + lister := &fakePKILister{ + err: fmt.Errorf("connection refused"), + } + provider := NewClusterPKIProfileProvider(lister) + + _, err := provider.PKIProfile() + if err == nil { + t.Error("expected error when lister fails") + } +} + +// errorProvider is a PKIProfileProvider that always returns an error. +type errorProvider struct { + err error +} + +func (e *errorProvider) PKIProfile() (*configv1alpha1.PKIProfile, error) { + return nil, e.err +} + +func TestResolveCertificateConfig_ProviderError(t *testing.T) { + provider := &errorProvider{err: fmt.Errorf("connection refused")} + + _, err := ResolveCertificateConfig(provider, CertificateTypeSigner, "test") + if err == nil { + t.Fatal("expected error to propagate from provider") + } + + // Verify the original error is wrapped + if !strings.Contains(err.Error(), "connection refused") { + t.Errorf("error = %q, want it to contain %q", err.Error(), "connection refused") + } +} + +func TestResolveCertificateConfig_DefaultProfile_AllTypes(t *testing.T) { + profile := DefaultPKIProfile() + provider := NewStaticPKIProfileProvider(&profile) + + testCases := []struct { + name string + certType CertificateType + want crypto.KeyPairGenerator + }{ + { + name: "signer uses ECDSA P-384", + certType: CertificateTypeSigner, + want: crypto.ECDSAKeyPairGenerator{Curve: crypto.P384}, + }, + { + name: "serving falls back to ECDSA P-256", + certType: CertificateTypeServing, + want: crypto.ECDSAKeyPairGenerator{Curve: crypto.P256}, + }, + { + name: "client falls back to ECDSA P-256", + certType: CertificateTypeClient, + want: crypto.ECDSAKeyPairGenerator{Curve: crypto.P256}, + }, + { + name: "peer picks stronger of serving and client", + certType: CertificateTypePeer, + want: crypto.ECDSAKeyPairGenerator{Curve: crypto.P256}, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + cfg, err := ResolveCertificateConfig(provider, tc.certType, "test") + if err != nil { + t.Fatalf("ResolveCertificateConfig() error = %v", err) + } + if cfg == nil { + t.Fatal("expected non-nil config") + } + if cfg.Key != tc.want { + t.Errorf("Key = %+v, want %+v", cfg.Key, tc.want) + } + }) + } +} diff --git a/pkg/pki/resolve.go b/pkg/pki/resolve.go new file mode 100644 index 0000000000..1154fd48ea --- /dev/null +++ b/pkg/pki/resolve.go @@ -0,0 +1,77 @@ +package pki + +import ( + "fmt" + + configv1alpha1 "github.com/openshift/api/config/v1alpha1" + "github.com/openshift/library-go/pkg/crypto" +) + +// CertificateConfig holds the resolved configuration for a specific certificate. +// Currently contains key configuration; will grow as the PKI API expands to +// include additional certificate properties. +type CertificateConfig struct { + // Key is the resolved key pair generator. + Key crypto.KeyPairGenerator +} + +// ResolveCertificateConfig resolves the effective certificate configuration +// for a given certificate type and name from the PKI profile. +// +// Returns nil if the provider returns a nil profile (Unmanaged mode), +// indicating that the caller should use its own default behavior. +// +// The name parameter is reserved for future per-certificate overrides and +// can be used for metrics and logging. +func ResolveCertificateConfig(provider PKIProfileProvider, certType CertificateType, name string) (*CertificateConfig, error) { + profile, err := provider.PKIProfile() + if err != nil { + return nil, fmt.Errorf("resolving PKI profile for %s certificate %q: %w", certType, name, err) + } + if profile == nil { + return nil, nil + } + + switch certType { + case CertificateTypeSigner: + return resolveKeyConfig(profile.Defaults, profile.SignerCertificates) + case CertificateTypeServing: + return resolveKeyConfig(profile.Defaults, profile.ServingCertificates) + case CertificateTypeClient: + return resolveKeyConfig(profile.Defaults, profile.ClientCertificates) + case CertificateTypePeer: + return resolvePeerKeyConfig(profile) + default: + return nil, fmt.Errorf("unknown certificate type: %q", certType) + } +} + +// resolveKeyConfig returns the override KeyConfig if its Algorithm is set, +// otherwise falls back to the default. +func resolveKeyConfig(defaults configv1alpha1.DefaultCertificateConfig, override configv1alpha1.CertificateConfig) (*CertificateConfig, error) { + apiKey := defaults.Key + if override.Key.Algorithm != "" { + apiKey = override.Key + } + g, err := KeyPairGeneratorFromAPI(apiKey) + if err != nil { + return nil, err + } + return &CertificateConfig{Key: g}, nil +} + +// resolvePeerKeyConfig resolves both the serving and client configs and +// returns whichever has higher NIST security strength. +func resolvePeerKeyConfig(profile *configv1alpha1.PKIProfile) (*CertificateConfig, error) { + servingCfg, err := resolveKeyConfig(profile.Defaults, profile.ServingCertificates) + if err != nil { + return nil, fmt.Errorf("resolving serving config for peer: %w", err) + } + clientCfg, err := resolveKeyConfig(profile.Defaults, profile.ClientCertificates) + if err != nil { + return nil, fmt.Errorf("resolving client config for peer: %w", err) + } + return &CertificateConfig{ + Key: strongerKeyPairGenerator(servingCfg.Key, clientCfg.Key), + }, nil +} diff --git a/pkg/pki/resolve_test.go b/pkg/pki/resolve_test.go new file mode 100644 index 0000000000..42e94d969e --- /dev/null +++ b/pkg/pki/resolve_test.go @@ -0,0 +1,162 @@ +package pki + +import ( + "testing" + + configv1alpha1 "github.com/openshift/api/config/v1alpha1" + "github.com/openshift/library-go/pkg/crypto" +) + +func TestResolveCertificateConfig_Signer(t *testing.T) { + profile := DefaultPKIProfile() + provider := NewStaticPKIProfileProvider(&profile) + + cfg, err := ResolveCertificateConfig(provider, CertificateTypeSigner, "test-signer") + if err != nil { + t.Fatalf("ResolveCertificateConfig() error = %v", err) + } + if cfg == nil { + t.Fatal("expected non-nil config") + } + + // Default profile has ECDSA P-384 for signers + g, ok := cfg.Key.(crypto.ECDSAKeyPairGenerator) + if !ok { + t.Fatalf("expected ECDSAKeyPairGenerator, got %T", cfg.Key) + } + if g.Curve != crypto.P384 { + t.Errorf("Curve = %v, want P384", g.Curve) + } +} + +func TestResolveCertificateConfig_Serving(t *testing.T) { + profile := DefaultPKIProfile() + provider := NewStaticPKIProfileProvider(&profile) + + cfg, err := ResolveCertificateConfig(provider, CertificateTypeServing, "test-serving") + if err != nil { + t.Fatalf("ResolveCertificateConfig() error = %v", err) + } + + // Default profile has no serving override, falls back to ECDSA P-256 + g, ok := cfg.Key.(crypto.ECDSAKeyPairGenerator) + if !ok { + t.Fatalf("expected ECDSAKeyPairGenerator, got %T", cfg.Key) + } + if g.Curve != crypto.P256 { + t.Errorf("Curve = %v, want P256", g.Curve) + } +} + +func TestResolveCertificateConfig_Client(t *testing.T) { + profile := DefaultPKIProfile() + provider := NewStaticPKIProfileProvider(&profile) + + cfg, err := ResolveCertificateConfig(provider, CertificateTypeClient, "test-client") + if err != nil { + t.Fatalf("ResolveCertificateConfig() error = %v", err) + } + + // Default profile has no client override, falls back to ECDSA P-256 + g, ok := cfg.Key.(crypto.ECDSAKeyPairGenerator) + if !ok { + t.Fatalf("expected ECDSAKeyPairGenerator, got %T", cfg.Key) + } + if g.Curve != crypto.P256 { + t.Errorf("Curve = %v, want P256", g.Curve) + } +} + +func TestResolveCertificateConfig_Peer(t *testing.T) { + // Set serving to RSA-2048 (112 bits) and client to ECDSA-P256 (128 bits) + profile := configv1alpha1.PKIProfile{ + Defaults: configv1alpha1.DefaultCertificateConfig{ + Key: configv1alpha1.KeyConfig{ + Algorithm: configv1alpha1.KeyAlgorithmRSA, + RSA: configv1alpha1.RSAKeyConfig{KeySize: 2048}, + }, + }, + ServingCertificates: configv1alpha1.CertificateConfig{ + Key: configv1alpha1.KeyConfig{ + Algorithm: configv1alpha1.KeyAlgorithmRSA, + RSA: configv1alpha1.RSAKeyConfig{KeySize: 2048}, + }, + }, + ClientCertificates: configv1alpha1.CertificateConfig{ + Key: configv1alpha1.KeyConfig{ + Algorithm: configv1alpha1.KeyAlgorithmECDSA, + ECDSA: configv1alpha1.ECDSAKeyConfig{Curve: configv1alpha1.ECDSACurveP256}, + }, + }, + } + provider := NewStaticPKIProfileProvider(&profile) + + cfg, err := ResolveCertificateConfig(provider, CertificateTypePeer, "test-peer") + if err != nil { + t.Fatalf("ResolveCertificateConfig() error = %v", err) + } + + // Peer should pick the stronger: ECDSA-P256 (128 bits) > RSA-2048 (112 bits) + g, ok := cfg.Key.(crypto.ECDSAKeyPairGenerator) + if !ok { + t.Fatalf("expected ECDSAKeyPairGenerator, got %T", cfg.Key) + } + if g.Curve != crypto.P256 { + t.Errorf("Curve = %v, want P256", g.Curve) + } +} + +func TestResolveCertificateConfig_CustomOverride(t *testing.T) { + // Custom profile with ECDSA-P384 serving override + profile := configv1alpha1.PKIProfile{ + Defaults: configv1alpha1.DefaultCertificateConfig{ + Key: configv1alpha1.KeyConfig{ + Algorithm: configv1alpha1.KeyAlgorithmRSA, + RSA: configv1alpha1.RSAKeyConfig{KeySize: 2048}, + }, + }, + ServingCertificates: configv1alpha1.CertificateConfig{ + Key: configv1alpha1.KeyConfig{ + Algorithm: configv1alpha1.KeyAlgorithmECDSA, + ECDSA: configv1alpha1.ECDSAKeyConfig{Curve: configv1alpha1.ECDSACurveP384}, + }, + }, + } + provider := NewStaticPKIProfileProvider(&profile) + + cfg, err := ResolveCertificateConfig(provider, CertificateTypeServing, "test-serving") + if err != nil { + t.Fatalf("ResolveCertificateConfig() error = %v", err) + } + + // Serving should use the override, not defaults + g, ok := cfg.Key.(crypto.ECDSAKeyPairGenerator) + if !ok { + t.Fatalf("expected ECDSAKeyPairGenerator, got %T", cfg.Key) + } + if g.Curve != crypto.P384 { + t.Errorf("Curve = %v, want P384", g.Curve) + } +} + +func TestResolveCertificateConfig_Unmanaged(t *testing.T) { + provider := NewStaticPKIProfileProvider(nil) + + cfg, err := ResolveCertificateConfig(provider, CertificateTypeSigner, "test") + if err != nil { + t.Fatalf("ResolveCertificateConfig() error = %v", err) + } + if cfg != nil { + t.Errorf("expected nil config for Unmanaged mode, got %+v", cfg) + } +} + +func TestResolveCertificateConfig_UnknownType(t *testing.T) { + profile := DefaultPKIProfile() + provider := NewStaticPKIProfileProvider(&profile) + + _, err := ResolveCertificateConfig(provider, "unknown", "test") + if err == nil { + t.Error("expected error for unknown certificate type") + } +} diff --git a/pkg/pki/types.go b/pkg/pki/types.go new file mode 100644 index 0000000000..2cf8282255 --- /dev/null +++ b/pkg/pki/types.go @@ -0,0 +1,23 @@ +package pki + +// CertificateType identifies the category of a certificate for profile resolution. +type CertificateType string + +const ( + // CertificateTypeSigner identifies certificate authority (CA) certificates + // that sign other certificates. + CertificateTypeSigner CertificateType = "signer" + + // CertificateTypeServing identifies TLS server certificates used to serve + // HTTPS endpoints. + CertificateTypeServing CertificateType = "serving" + + // CertificateTypeClient identifies client authentication certificates used + // to authenticate to servers. + CertificateTypeClient CertificateType = "client" + + // CertificateTypePeer identifies certificates used for both server and client + // authentication. The resolved key configuration is the stronger of the + // serving and client configurations. + CertificateTypePeer CertificateType = "peer" +)