From: Adam Langley Date: Tue, 12 Apr 2016 18:14:25 +0000 (-0700) Subject: crypto/x509: don't add an AuthorityKeyId to self-signed certificates. X-Git-Tag: go1.7beta1~685 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=b623b71509b2d24df915d5bc68602e1c6edf38ca;p=gostls13.git 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 Run-TryBot: Adam Langley TryBot-Result: Gobot Gobot --- diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go index d35c29434c..c93a7663f1 100644 --- 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 cd70a27da3..a48d0d918a 100644 --- 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