From ea8579f3dff36152af5652585f908544c34d8d43 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Tue, 30 Aug 2022 16:49:56 -0700 Subject: [PATCH 1/3] 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) + } + }) + } +} From 30c54a555d4897e4195f8af96ea50a1ed7843dfd Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Tue, 30 Aug 2022 16:57:31 -0700 Subject: [PATCH 2/3] Add entry in changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2d6ee81..d82f5149 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. --- ## [Unreleased] +### Fixed +- Fix signature algorithm on EC (root) + RSA (intermediate) PKIs. + +## [0.22.0] - 2022-08-26 ### Added - Added automatic configuration of Linked RAs. - Send provisioner configuration on Linked RAs. From a7fcfe0e4ebb5119f8c82fe0010862a143e635eb Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Tue, 30 Aug 2022 17:11:44 -0700 Subject: [PATCH 3/3] Verify with roots and intermediates --- cas/softcas/softcas_test.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/cas/softcas/softcas_test.go b/cas/softcas/softcas_test.go index f8d940b3..8867b9b4 100644 --- a/cas/softcas/softcas_test.go +++ b/cas/softcas/softcas_test.go @@ -489,12 +489,15 @@ func TestSoftCAS_CreateCertificate_ec_rsa(t *testing.T) { t.Errorf("Certificate.SignatureAlgorithm = %v, want %v", iss.SignatureAlgorithm, x509.SHA256WithRSAPSS) } - pool := x509.NewCertPool() - pool.AddCert(iss) + roots := x509.NewCertPool() + roots.AddCert(root) + intermediates := x509.NewCertPool() + intermediates.AddCert(iss) if _, err = cert.Certificate.Verify(x509.VerifyOptions{ - CurrentTime: time.Now(), - Roots: pool, - KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}, + CurrentTime: time.Now(), + Roots: roots, + Intermediates: intermediates, + KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}, }); err != nil { t.Errorf("Certificate.Verify() error = %v", err) }