From ea8579f3dff36152af5652585f908544c34d8d43 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Tue, 30 Aug 2022 16:49:56 -0700 Subject: [PATCH] Fix bad signature algorithm on EC+RSA PKI When the root certificate has an EC key and he intermediate has an RSA key, the signature algorithm of the leafs should be the default one, SHA256WithRSA, instead of the one that the intermediate has. Fixes #1033 --- .golangci.yml | 1 - cas/softcas/softcas.go | 17 +++++- cas/softcas/softcas_test.go | 117 ++++++++++++++++++++++++++++++++++++ 3 files changed, 133 insertions(+), 2 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 67aac2df..af723230 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -50,7 +50,6 @@ linters-settings: linters: disable-all: true enable: - - deadcode - gocritic - gofmt - gosimple diff --git a/cas/softcas/softcas.go b/cas/softcas/softcas.go index 36c1dd69..dc6343f6 100644 --- a/cas/softcas/softcas.go +++ b/cas/softcas/softcas.go @@ -248,8 +248,23 @@ func createCertificate(template, parent *x509.Certificate, pub crypto.PublicKey, if sa, ok := signer.(apiv1.SignatureAlgorithmGetter); ok { template.SignatureAlgorithm = sa.SignatureAlgorithm() } else if _, ok := parent.PublicKey.(*rsa.PublicKey); ok { - template.SignatureAlgorithm = parent.SignatureAlgorithm + // For RSA issuers, only overwrite the default algorithm is the + // intermediate is signed with an RSA signature scheme. + if isRSA(parent.SignatureAlgorithm) { + template.SignatureAlgorithm = parent.SignatureAlgorithm + } } } return x509util.CreateCertificate(template, parent, pub, signer) } + +func isRSA(sa x509.SignatureAlgorithm) bool { + switch sa { + case x509.SHA256WithRSA, x509.SHA384WithRSA, x509.SHA512WithRSA: + return true + case x509.SHA256WithRSAPSS, x509.SHA384WithRSAPSS, x509.SHA512WithRSAPSS: + return true + default: + return false + } +} diff --git a/cas/softcas/softcas_test.go b/cas/softcas/softcas_test.go index 3caa0561..f8d940b3 100644 --- a/cas/softcas/softcas_test.go +++ b/cas/softcas/softcas_test.go @@ -4,6 +4,8 @@ import ( "bytes" "context" "crypto" + "crypto/ecdsa" + "crypto/elliptic" "crypto/rand" "crypto/rsa" "crypto/x509" @@ -412,6 +414,92 @@ func TestSoftCAS_CreateCertificate_pss(t *testing.T) { } } +func TestSoftCAS_CreateCertificate_ec_rsa(t *testing.T) { + rootSigner, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + + intSigner, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) + } + + now := time.Now() + + // Root template + template := &x509.Certificate{ + Subject: pkix.Name{CommonName: "Test Root CA"}, + KeyUsage: x509.KeyUsageCRLSign | x509.KeyUsageCertSign, + PublicKey: rootSigner.Public(), + BasicConstraintsValid: true, + IsCA: true, + MaxPathLen: 0, + SerialNumber: big.NewInt(1234), + NotBefore: now, + NotAfter: now.Add(24 * time.Hour), + } + + root, err := x509util.CreateCertificate(template, template, rootSigner.Public(), rootSigner) + if err != nil { + t.Fatal(err) + } + + // Intermediate template + template = &x509.Certificate{ + Subject: pkix.Name{CommonName: "Test Intermediate CA"}, + KeyUsage: x509.KeyUsageCRLSign | x509.KeyUsageCertSign, + PublicKey: intSigner.Public(), + BasicConstraintsValid: true, + IsCA: true, + MaxPathLen: 0, + SerialNumber: big.NewInt(1234), + NotBefore: now, + NotAfter: now.Add(24 * time.Hour), + } + + iss, err := x509util.CreateCertificate(template, root, intSigner.Public(), rootSigner) + if err != nil { + t.Fatal(err) + } + + if iss.SignatureAlgorithm != x509.ECDSAWithSHA256 { + t.Errorf("Certificate.SignatureAlgorithm = %v, want %v", iss.SignatureAlgorithm, x509.ECDSAWithSHA256) + } + + c := &SoftCAS{ + CertificateChain: []*x509.Certificate{iss}, + Signer: intSigner, + } + cert, err := c.CreateCertificate(&apiv1.CreateCertificateRequest{ + Template: &x509.Certificate{ + Subject: pkix.Name{CommonName: "test.smallstep.com"}, + DNSNames: []string{"test.smallstep.com"}, + KeyUsage: x509.KeyUsageDigitalSignature, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}, + PublicKey: testSigner.Public(), + SerialNumber: big.NewInt(1234), + }, + Lifetime: time.Hour, Backdate: time.Minute, + }) + if err != nil { + t.Fatalf("SoftCAS.CreateCertificate() error = %v", err) + } + if cert.Certificate.SignatureAlgorithm != x509.SHA256WithRSA { + t.Errorf("Certificate.SignatureAlgorithm = %v, want %v", iss.SignatureAlgorithm, x509.SHA256WithRSAPSS) + } + + pool := x509.NewCertPool() + pool.AddCert(iss) + if _, err = cert.Certificate.Verify(x509.VerifyOptions{ + CurrentTime: time.Now(), + Roots: pool, + KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}, + }); err != nil { + t.Errorf("Certificate.Verify() error = %v", err) + } +} + func TestSoftCAS_RenewCertificate(t *testing.T) { mockNow(t) @@ -772,3 +860,32 @@ func TestSoftCAS_defaultKeyManager(t *testing.T) { }) } } + +func Test_isRSA(t *testing.T) { + type args struct { + sa x509.SignatureAlgorithm + } + tests := []struct { + name string + args args + want bool + }{ + {"SHA256WithRSA", args{x509.SHA256WithRSA}, true}, + {"SHA384WithRSA", args{x509.SHA384WithRSA}, true}, + {"SHA512WithRSA", args{x509.SHA512WithRSA}, true}, + {"SHA256WithRSAPSS", args{x509.SHA256WithRSAPSS}, true}, + {"SHA384WithRSAPSS", args{x509.SHA384WithRSAPSS}, true}, + {"SHA512WithRSAPSS", args{x509.SHA512WithRSAPSS}, true}, + {"ECDSAWithSHA256", args{x509.ECDSAWithSHA256}, false}, + {"ECDSAWithSHA384", args{x509.ECDSAWithSHA384}, false}, + {"ECDSAWithSHA512", args{x509.ECDSAWithSHA512}, false}, + {"PureEd25519", args{x509.PureEd25519}, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := isRSA(tt.args.sa); got != tt.want { + t.Errorf("isRSA() = %v, want %v", got, tt.want) + } + }) + } +}