crypto/x509: don't add an AuthorityKeyId to self-signed certificates.

The AuthorityKeyId is optional for self-signed certificates, generally
useless, and takes up space. This change causes an AuthorityKeyId not to
be added to self-signed certificates, although it can still be set in
the template if the caller really wants to include it.

Fixes #15194.

Change-Id: If5d3c3d9ca9ae5fe67458291510ec7140829756e
Reviewed-on: https://go-review.googlesource.com/21895
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go
index d35c29434cb3e60887fa2ec579aa2b04c9441d7e..c93a7663f10395d177829fe8f716db87b99c2318 100644 (file)
--- a/src/crypto/x509/x509.go
+++ b/src/crypto/x509/x509.go
@@ -1587,21 +1587,21 @@ func CreateCertificate(rand io.Reader, template, parent *Certificate, pub, priv
                return nil, err
        }
 
-       if len(parent.SubjectKeyId) > 0 {
-               template.AuthorityKeyId = parent.SubjectKeyId
-       }
-
-       extensions, err := buildExtensions(template)
+       asn1Issuer, err := subjectBytes(parent)
        if err != nil {
                return
        }
 
-       asn1Issuer, err := subjectBytes(parent)
+       asn1Subject, err := subjectBytes(template)
        if err != nil {
                return
        }
 
-       asn1Subject, err := subjectBytes(template)
+       if !bytes.Equal(asn1Issuer, asn1Subject) && len(parent.SubjectKeyId) > 0 {
+               template.AuthorityKeyId = parent.SubjectKeyId
+       }
+
+       extensions, err := buildExtensions(template)
        if err != nil {
                return
        }

diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go
index cd70a27da3f2871dd9c978377116bc5be3ecf982..a48d0d918a8f0f935783173f239aa9cb58f135d0 100644 (file)
--- a/src/crypto/x509/x509_test.go
+++ b/src/crypto/x509/x509_test.go
@@ -96,6 +96,17 @@ tAboUGBxTDq3ZroNism3DaMIbKPyYrAqhKov1h5V
 -----END RSA PRIVATE KEY-----
 `
 
+var testPrivateKey *rsa.PrivateKey
+
+func init() {
+       block, _ := pem.Decode([]byte(pemPrivateKey))
+
+       var err error
+       if testPrivateKey, err = ParsePKCS1PrivateKey(block.Bytes); err != nil {
+               panic("Failed to parse private key: " + err.Error())
+       }
+}
+
 func bigFromString(s string) *big.Int {
        ret := new(big.Int)
        ret.SetString(s, 10)
@@ -314,12 +325,6 @@ var certBytes = "308203223082028ba00302010202106edf0d9499fd4533dd1297fc42a93be13
 func TestCreateSelfSignedCertificate(t *testing.T) {
        random := rand.Reader
 
-       block, _ := pem.Decode([]byte(pemPrivateKey))
-       rsaPriv, err := ParsePKCS1PrivateKey(block.Bytes)
-       if err != nil {
-               t.Fatalf("Failed to parse private key: %s", err)
-       }
-
        ecdsaPriv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
        if err != nil {
                t.Fatalf("Failed to generate ECDSA key: %s", err)
@@ -331,9 +336,9 @@ func TestCreateSelfSignedCertificate(t *testing.T) {
                checkSig  bool
                sigAlgo   SignatureAlgorithm
        }{
-               {"RSA/RSA", &rsaPriv.PublicKey, rsaPriv, true, SHA1WithRSA},
-               {"RSA/ECDSA", &rsaPriv.PublicKey, ecdsaPriv, false, ECDSAWithSHA384},
-               {"ECDSA/RSA", &ecdsaPriv.PublicKey, rsaPriv, false, SHA256WithRSA},
+               {"RSA/RSA", &testPrivateKey.PublicKey, testPrivateKey, true, SHA1WithRSA},
+               {"RSA/ECDSA", &testPrivateKey.PublicKey, ecdsaPriv, false, ECDSAWithSHA384},
+               {"ECDSA/RSA", &ecdsaPriv.PublicKey, testPrivateKey, false, SHA256WithRSA},
                {"ECDSA/ECDSA", &ecdsaPriv.PublicKey, ecdsaPriv, true, ECDSAWithSHA1},
        }
 
@@ -874,12 +879,6 @@ const pemCRLBase64 = "LS0tLS1CRUdJTiBYNTA5IENSTC0tLS0tDQpNSUlCOWpDQ0FWOENBUUV3RF
 func TestCreateCertificateRequest(t *testing.T) {
        random := rand.Reader
 
-       block, _ := pem.Decode([]byte(pemPrivateKey))
-       rsaPriv, err := ParsePKCS1PrivateKey(block.Bytes)
-       if err != nil {
-               t.Fatalf("Failed to parse private key: %s", err)
-       }
-
        ecdsa256Priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
        if err != nil {
                t.Fatalf("Failed to generate ECDSA key: %s", err)
@@ -900,7 +899,7 @@ func TestCreateCertificateRequest(t *testing.T) {
                priv    interface{}
                sigAlgo SignatureAlgorithm
        }{
-               {"RSA", rsaPriv, SHA1WithRSA},
+               {"RSA", testPrivateKey, SHA1WithRSA},
                {"ECDSA-256", ecdsa256Priv, ECDSAWithSHA1},
                {"ECDSA-384", ecdsa384Priv, ECDSAWithSHA1},
                {"ECDSA-521", ecdsa521Priv, ECDSAWithSHA1},
@@ -951,13 +950,7 @@ func TestCreateCertificateRequest(t *testing.T) {
 }
 
 func marshalAndParseCSR(t *testing.T, template *CertificateRequest) *CertificateRequest {
-       block, _ := pem.Decode([]byte(pemPrivateKey))
-       rsaPriv, err := ParsePKCS1PrivateKey(block.Bytes)
-       if err != nil {
-               t.Fatal(err)
-       }
-
-       derBytes, err := CreateCertificateRequest(rand.Reader, template, rsaPriv)
+       derBytes, err := CreateCertificateRequest(rand.Reader, template, testPrivateKey)
        if err != nil {
                t.Fatal(err)
        }
@@ -1113,13 +1106,25 @@ func TestCriticalFlagInCSRRequestedExtensions(t *testing.T) {
        }
 }
 
-func TestMaxPathLen(t *testing.T) {
-       block, _ := pem.Decode([]byte(pemPrivateKey))
-       rsaPriv, err := ParsePKCS1PrivateKey(block.Bytes)
+// serialiseAndParse generates a self-signed certificate from template and
+// returns a parsed version of it.
+func serialiseAndParse(t *testing.T, template *Certificate) *Certificate {
+       derBytes, err := CreateCertificate(rand.Reader, template, template, &testPrivateKey.PublicKey, testPrivateKey)
        if err != nil {
-               t.Fatalf("Failed to parse private key: %s", err)
+               t.Fatalf("failed to create certificate: %s", err)
+               return nil
        }
 
+       cert, err := ParseCertificate(derBytes)
+       if err != nil {
+               t.Fatalf("failed to parse certificate: %s", err)
+               return nil
+       }
+
+       return cert
+}
+
+func TestMaxPathLen(t *testing.T) {
        template := &Certificate{
                SerialNumber: big.NewInt(1),
                Subject: pkix.Name{
@@ -1132,23 +1137,7 @@ func TestMaxPathLen(t *testing.T) {
                IsCA: true,
        }
 
-       serialiseAndParse := func(template *Certificate) *Certificate {
-               derBytes, err := CreateCertificate(rand.Reader, template, template, &rsaPriv.PublicKey, rsaPriv)
-               if err != nil {
-                       t.Fatalf("failed to create certificate: %s", err)
-                       return nil
-               }
-
-               cert, err := ParseCertificate(derBytes)
-               if err != nil {
-                       t.Fatalf("failed to parse certificate: %s", err)
-                       return nil
-               }
-
-               return cert
-       }
-
-       cert1 := serialiseAndParse(template)
+       cert1 := serialiseAndParse(t, template)
        if m := cert1.MaxPathLen; m != -1 {
                t.Errorf("Omitting MaxPathLen didn't turn into -1, got %d", m)
        }
@@ -1157,7 +1146,7 @@ func TestMaxPathLen(t *testing.T) {
        }
 
        template.MaxPathLen = 1
-       cert2 := serialiseAndParse(template)
+       cert2 := serialiseAndParse(t, template)
        if m := cert2.MaxPathLen; m != 1 {
                t.Errorf("Setting MaxPathLen didn't work. Got %d but set 1", m)
        }
@@ -1167,7 +1156,7 @@ func TestMaxPathLen(t *testing.T) {
 
        template.MaxPathLen = 0
        template.MaxPathLenZero = true
-       cert3 := serialiseAndParse(template)
+       cert3 := serialiseAndParse(t, template)
        if m := cert3.MaxPathLen; m != 0 {
                t.Errorf("Setting MaxPathLenZero didn't work, got %d", m)
        }
@@ -1176,6 +1165,30 @@ func TestMaxPathLen(t *testing.T) {
        }
 }
 
+func TestNoAuthorityKeyIdInSelfSignedCert(t *testing.T) {
+       template := &Certificate{
+               SerialNumber: big.NewInt(1),
+               Subject: pkix.Name{
+                       CommonName: "Σ Acme Co",
+               },
+               NotBefore: time.Unix(1000, 0),
+               NotAfter:  time.Unix(100000, 0),
+
+               BasicConstraintsValid: true,
+               IsCA:         true,
+               SubjectKeyId: []byte{1, 2, 3, 4},
+       }
+
+       if cert := serialiseAndParse(t, template); len(cert.AuthorityKeyId) != 0 {
+               t.Fatalf("self-signed certificate contained default authority key id")
+       }
+
+       template.AuthorityKeyId = []byte{1,2,3,4}
+       if cert := serialiseAndParse(t, template); len(cert.AuthorityKeyId) == 0 {
+               t.Fatalf("self-signed certificate erased explicit authority key id")
+       }
+}
+
 func TestASN1BitLength(t *testing.T) {
        tests := []struct {
                bytes  []byte