From fde7e7152f0e44e2d71167d972409093e3ac5f6d Mon Sep 17 00:00:00 2001 From: cyli Date: Wed, 3 May 2017 10:52:19 -0700 Subject: [PATCH] Fix bug where we could not cross-sign an RSA certificate with an ECDSA certificate and vice versa. Signed-off-by: cyli --- ca/certificates.go | 7 ++- ca/certificates_test.go | 125 +++++++++++++++++++++++++++------------- ca/external_test.go | 101 +++++++++++++++++--------------- 3 files changed, 144 insertions(+), 89 deletions(-) diff --git a/ca/certificates.go b/ca/certificates.go index 8f03640e98..76131cc08a 100644 --- a/ca/certificates.go +++ b/ca/certificates.go @@ -386,16 +386,17 @@ func (rca *RootCA) CrossSignCACertificate(otherCAPEM []byte) ([]byte, error) { } // create a new cert with exactly the same parameters, including the public key and exact NotBefore and NotAfter - newCert, err := helpers.ParseCertificatePEM(otherCAPEM) + template, err := helpers.ParseCertificatePEM(otherCAPEM) if err != nil { return nil, errors.New("could not parse new CA certificate") } - if !newCert.IsCA { + if !template.IsCA { return nil, errors.New("certificate not a CA") } - derBytes, err := x509.CreateCertificate(cryptorand.Reader, newCert, signer.parsedCert, newCert.PublicKey, signer.cryptoSigner) + template.SignatureAlgorithm = signer.parsedCert.SignatureAlgorithm // make sure we can sign with the signer key + derBytes, err := x509.CreateCertificate(cryptorand.Reader, template, signer.parsedCert, template.PublicKey, signer.cryptoSigner) if err != nil { return nil, errors.Wrap(err, "could not cross-sign new CA certificate using old CA material") } diff --git a/ca/certificates_test.go b/ca/certificates_test.go index 6abf26299a..d5dc2e8973 100644 --- a/ca/certificates_test.go +++ b/ca/certificates_test.go @@ -22,6 +22,7 @@ import ( cfcsr "github.com/cloudflare/cfssl/csr" "github.com/cloudflare/cfssl/helpers" + "github.com/cloudflare/cfssl/initca" "github.com/docker/go-events" "github.com/docker/swarmkit/api" "github.com/docker/swarmkit/ca" @@ -1494,67 +1495,111 @@ func TestValidateCertificateChain(t *testing.T) { } } -// Tests cross-signing using a certificate +// Tests cross-signing an RSA cert with an ECDSA cert and vice versa, and an ECDSA +// cert with another ECDSA cert and a RSA cert with another RSA cert func TestRootCACrossSignCACertificate(t *testing.T) { t.Parallel() + if cautils.External { + return + } - cert1, key1, err := cautils.CreateRootCertAndKey("rootCN") - require.NoError(t, err) + oldCAs := []struct { + cert, key []byte + }{ + { + cert: cautils.ECDSA256SHA256Cert, + key: cautils.ECDSA256Key, + }, + { + cert: cautils.RSA2048SHA256Cert, + key: cautils.RSA2048Key, + }, + } - rootCA1, err := ca.NewRootCA(cert1, cert1, key1, ca.DefaultNodeCertExpiration, nil) + cert1, key1, err := cautils.CreateRootCertAndKey("rootCNECDSA") require.NoError(t, err) - cert2, key2, err := cautils.CreateRootCertAndKey("rootCN2") - require.NoError(t, err) + rsaReq := cfcsr.CertificateRequest{ + CN: "rootCNRSA", + KeyRequest: &cfcsr.BasicKeyRequest{ + A: "rsa", + S: 2048, + }, + CA: &cfcsr.CAConfig{Expiry: ca.RootCAExpiration}, + } - rootCA2, err := ca.NewRootCA(cert2, cert2, key2, ca.DefaultNodeCertExpiration, nil) + // Generate the CA and get the certificate and private key + cert2, _, key2, err := initca.New(&rsaReq) require.NoError(t, err) + newCAs := []struct { + cert, key []byte + }{ + { + cert: cert1, + key: key1, + }, + { + cert: cert2, + key: key2, + }, + } + tempdir, err := ioutil.TempDir("", "cross-sign-cert") require.NoError(t, err) defer os.RemoveAll(tempdir) paths := ca.NewConfigPaths(tempdir) krw := ca.NewKeyReadWriter(paths.Node, nil, nil) - _, _, err = rootCA2.IssueAndSaveNewCertificates(krw, "cn", "ou", "org") - require.NoError(t, err) - certBytes, _, err := krw.Read() - require.NoError(t, err) - leafCert, err := helpers.ParseCertificatePEM(certBytes) - require.NoError(t, err) + for _, oldRoot := range oldCAs { + for _, newRoot := range newCAs { + rootCA1, err := ca.NewRootCA(oldRoot.cert, oldRoot.cert, oldRoot.key, ca.DefaultNodeCertExpiration, nil) + require.NoError(t, err) - // cross-signing a non-CA fails - _, err = rootCA1.CrossSignCACertificate(certBytes) - require.Error(t, err) + rootCA2, err := ca.NewRootCA(newRoot.cert, newRoot.cert, newRoot.key, ca.DefaultNodeCertExpiration, nil) + require.NoError(t, err) - // cross-signing some non-cert PEM bytes fail - _, err = rootCA1.CrossSignCACertificate(key1) - require.Error(t, err) + _, _, err = rootCA2.IssueAndSaveNewCertificates(krw, "cn", "ou", "org") + require.NoError(t, err) + certBytes, keyBytes, err := krw.Read() + require.NoError(t, err) + leafCert, err := helpers.ParseCertificatePEM(certBytes) + require.NoError(t, err) - intermediate, err := rootCA1.CrossSignCACertificate(cert2) - require.NoError(t, err) - parsedIntermediate, err := helpers.ParseCertificatePEM(intermediate) - require.NoError(t, err) - parsedRoot2, err := helpers.ParseCertificatePEM(cert2) - require.NoError(t, err) - require.Equal(t, parsedRoot2.RawSubject, parsedIntermediate.RawSubject) - require.Equal(t, parsedRoot2.RawSubjectPublicKeyInfo, parsedIntermediate.RawSubjectPublicKeyInfo) - require.True(t, parsedIntermediate.IsCA) + // cross-signing a non-CA fails + _, err = rootCA1.CrossSignCACertificate(certBytes) + require.Error(t, err) - intermediatePool := x509.NewCertPool() - intermediatePool.AddCert(parsedIntermediate) + // cross-signing some non-cert PEM bytes fail + _, err = rootCA1.CrossSignCACertificate(keyBytes) + require.Error(t, err) - // we can validate a chain from the leaf to the first root through the intermediate, - // or from the leaf cert to the second root with or without the intermediate - _, err = leafCert.Verify(x509.VerifyOptions{Roots: rootCA1.Pool}) - require.Error(t, err) - _, err = leafCert.Verify(x509.VerifyOptions{Roots: rootCA1.Pool, Intermediates: intermediatePool}) - require.NoError(t, err) + intermediate, err := rootCA1.CrossSignCACertificate(newRoot.cert) + require.NoError(t, err) + parsedIntermediate, err := helpers.ParseCertificatePEM(intermediate) + require.NoError(t, err) + parsedRoot2, err := helpers.ParseCertificatePEM(newRoot.cert) + require.NoError(t, err) + require.Equal(t, parsedRoot2.RawSubject, parsedIntermediate.RawSubject) + require.Equal(t, parsedRoot2.RawSubjectPublicKeyInfo, parsedIntermediate.RawSubjectPublicKeyInfo) + require.True(t, parsedIntermediate.IsCA) - _, err = leafCert.Verify(x509.VerifyOptions{Roots: rootCA2.Pool}) - require.NoError(t, err) - _, err = leafCert.Verify(x509.VerifyOptions{Roots: rootCA2.Pool, Intermediates: intermediatePool}) - require.NoError(t, err) + intermediatePool := x509.NewCertPool() + intermediatePool.AddCert(parsedIntermediate) + + // we can validate a chain from the leaf to the first root through the intermediate, + // or from the leaf cert to the second root with or without the intermediate + _, err = leafCert.Verify(x509.VerifyOptions{Roots: rootCA1.Pool}) + require.Error(t, err) + _, err = leafCert.Verify(x509.VerifyOptions{Roots: rootCA1.Pool, Intermediates: intermediatePool}) + require.NoError(t, err) + + _, err = leafCert.Verify(x509.VerifyOptions{Roots: rootCA2.Pool}) + require.NoError(t, err) + _, err = leafCert.Verify(x509.VerifyOptions{Roots: rootCA2.Pool, Intermediates: intermediatePool}) + require.NoError(t, err) + } + } } func concat(byteSlices ...[]byte) []byte { diff --git a/ca/external_test.go b/ca/external_test.go index 4c415eb864..7c7034864d 100644 --- a/ca/external_test.go +++ b/ca/external_test.go @@ -33,52 +33,61 @@ func TestExternalCACrossSign(t *testing.T) { externalCA := secConfig.ExternalCA() externalCA.UpdateURLs(tc.ExternalSigningServer.URL) - cert2, key2, err := testutils.CreateRootCertAndKey("rootCN2") - require.NoError(t, err) - - rootCA2, err := ca.NewRootCA(cert2, cert2, key2, ca.DefaultNodeCertExpiration, nil) - require.NoError(t, err) - - krw := ca.NewKeyReadWriter(paths.Node, nil, nil) - - _, _, err = rootCA2.IssueAndSaveNewCertificates(krw, "cn", "ou", "org") - require.NoError(t, err) - certBytes, _, err := krw.Read() - require.NoError(t, err) - leafCert, err := helpers.ParseCertificatePEM(certBytes) - require.NoError(t, err) - - // we have not enabled CA signing on the external server - _, err = externalCA.CrossSignRootCA(context.Background(), rootCA2) - require.Error(t, err) - - require.NoError(t, tc.ExternalSigningServer.EnableCASigning()) - - intermediate, err := externalCA.CrossSignRootCA(context.Background(), rootCA2) - require.NoError(t, err) - - parsedIntermediate, err := helpers.ParseCertificatePEM(intermediate) - require.NoError(t, err) - parsedRoot2, err := helpers.ParseCertificatePEM(cert2) - require.NoError(t, err) - require.Equal(t, parsedRoot2.RawSubject, parsedIntermediate.RawSubject) - require.Equal(t, parsedRoot2.RawSubjectPublicKeyInfo, parsedIntermediate.RawSubjectPublicKeyInfo) - require.True(t, parsedIntermediate.IsCA) - - intermediatePool := x509.NewCertPool() - intermediatePool.AddCert(parsedIntermediate) - - // we can validate a chain from the leaf to the first root through the intermediate, - // or from the leaf cert to the second root with or without the intermediate - _, err = leafCert.Verify(x509.VerifyOptions{Roots: tc.RootCA.Pool}) - require.Error(t, err) - _, err = leafCert.Verify(x509.VerifyOptions{Roots: tc.RootCA.Pool, Intermediates: intermediatePool}) - require.NoError(t, err) - - _, err = leafCert.Verify(x509.VerifyOptions{Roots: rootCA2.Pool}) - require.NoError(t, err) - _, err = leafCert.Verify(x509.VerifyOptions{Roots: rootCA2.Pool, Intermediates: intermediatePool}) - require.NoError(t, err) + for _, testcase := range []struct{ cert, key []byte }{ + { + cert: testutils.ECDSA256SHA256Cert, + key: testutils.ECDSA256Key, + }, + { + cert: testutils.RSA2048SHA256Cert, + key: testutils.RSA2048Key, + }, + } { + rootCA2, err := ca.NewRootCA(testcase.cert, testcase.cert, testcase.key, ca.DefaultNodeCertExpiration, nil) + require.NoError(t, err) + + krw := ca.NewKeyReadWriter(paths.Node, nil, nil) + + _, _, err = rootCA2.IssueAndSaveNewCertificates(krw, "cn", "ou", "org") + require.NoError(t, err) + certBytes, _, err := krw.Read() + require.NoError(t, err) + leafCert, err := helpers.ParseCertificatePEM(certBytes) + require.NoError(t, err) + + // we have not enabled CA signing on the external server + tc.ExternalSigningServer.DisableCASigning() + _, err = externalCA.CrossSignRootCA(context.Background(), rootCA2) + require.Error(t, err) + + require.NoError(t, tc.ExternalSigningServer.EnableCASigning()) + + intermediate, err := externalCA.CrossSignRootCA(context.Background(), rootCA2) + require.NoError(t, err) + + parsedIntermediate, err := helpers.ParseCertificatePEM(intermediate) + require.NoError(t, err) + parsedRoot2, err := helpers.ParseCertificatePEM(testcase.cert) + require.NoError(t, err) + require.Equal(t, parsedRoot2.RawSubject, parsedIntermediate.RawSubject) + require.Equal(t, parsedRoot2.RawSubjectPublicKeyInfo, parsedIntermediate.RawSubjectPublicKeyInfo) + require.True(t, parsedIntermediate.IsCA) + + intermediatePool := x509.NewCertPool() + intermediatePool.AddCert(parsedIntermediate) + + // we can validate a chain from the leaf to the first root through the intermediate, + // or from the leaf cert to the second root with or without the intermediate + _, err = leafCert.Verify(x509.VerifyOptions{Roots: tc.RootCA.Pool}) + require.Error(t, err) + _, err = leafCert.Verify(x509.VerifyOptions{Roots: tc.RootCA.Pool, Intermediates: intermediatePool}) + require.NoError(t, err) + + _, err = leafCert.Verify(x509.VerifyOptions{Roots: rootCA2.Pool}) + require.NoError(t, err) + _, err = leafCert.Verify(x509.VerifyOptions{Roots: rootCA2.Pool, Intermediates: intermediatePool}) + require.NoError(t, err) + } } func TestExternalCASignRequestTimesOut(t *testing.T) {