From aa95a1eb5a3423d96873946d47c663bdc8f3565e Mon Sep 17 00:00:00 2001 From: Filippo Valsorda Date: Mon, 4 Feb 2019 18:08:43 -0500 Subject: [PATCH] [release-branch.go1.11] crypto/x509: consider parents by Subject if AKID has no match If a certificate somehow has an AKID, it should still chain successfully to a parent without a SKID, even if the latter is invalid according to RFC 5280, because only the Subject is authoritative. This reverts to the behavior before #29233 was fixed in 770130659. Roots with the right subject will still be shadowed by roots with the right SKID and the wrong subject, but that's been the case for a long time, and is left for a more complete fix in Go 1.13. Updates #30079 Fixes #30081 Change-Id: If8ab0179aca86cb74caa926d1ef93fb5e416b4bb Reviewed-on: https://go-review.googlesource.com/c/161097 Reviewed-by: Adam Langley (cherry picked from commit 95e5b07cf5fdf3352f04f5557df38f22c55ce8e8) Reviewed-on: https://go-review.googlesource.com/c/163739 Run-TryBot: Filippo Valsorda TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/crypto/x509/cert_pool.go | 9 ++- src/crypto/x509/verify_test.go | 116 +++++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 2 deletions(-) diff --git a/src/crypto/x509/cert_pool.go b/src/crypto/x509/cert_pool.go index 61107f86b8e10..1229fb43d61fb 100644 --- a/src/crypto/x509/cert_pool.go +++ b/src/crypto/x509/cert_pool.go @@ -71,10 +71,15 @@ func (s *CertPool) findPotentialParents(cert *Certificate) []int { if s == nil { return nil } + + var candidates []int if len(cert.AuthorityKeyId) > 0 { - return s.bySubjectKeyId[string(cert.AuthorityKeyId)] + candidates = s.bySubjectKeyId[string(cert.AuthorityKeyId)] + } + if len(candidates) == 0 { + candidates = s.byName[string(cert.RawIssuer)] } - return s.byName[string(cert.RawIssuer)] + return candidates } func (s *CertPool) contains(cert *Certificate) bool { diff --git a/src/crypto/x509/verify_test.go b/src/crypto/x509/verify_test.go index 85f4703d4c3fd..86fe76a57d7f8 100644 --- a/src/crypto/x509/verify_test.go +++ b/src/crypto/x509/verify_test.go @@ -386,6 +386,19 @@ var verifyTests = []verifyTest{ errorCallback: expectHostnameError("not valid for any names"), }, + { + // A certificate with an AKID should still chain to a parent without SKID. + // See Issue 30079. + leaf: leafWithAKID, + roots: []string{rootWithoutSKID}, + currentTime: 1550000000, + dnsName: "example", + systemSkip: true, + + expectedChains: [][]string{ + {"Acme LLC", "Acme Co"}, + }, + }, } func expectHostnameError(msg string) func(*testing.T, int, error) bool { @@ -1679,6 +1692,109 @@ h7olHCpY9yMRiz0= -----END CERTIFICATE----- ` +const ( + rootWithoutSKID = ` +Certificate: + Data: + Version: 3 (0x2) + Serial Number: + 78:29:2a:dc:2f:12:39:7f:c9:33:93:ea:61:39:7d:70 + Signature Algorithm: ecdsa-with-SHA256 + Issuer: O = Acme Co + Validity + Not Before: Feb 4 22:56:34 2019 GMT + Not After : Feb 1 22:56:34 2029 GMT + Subject: O = Acme Co + Subject Public Key Info: + Public Key Algorithm: id-ecPublicKey + Public-Key: (256 bit) + pub: + 04:84:a6:8c:69:53:af:87:4b:39:64:fe:04:24:e6: + d8:fc:d6:46:39:35:0e:92:dc:48:08:7e:02:5f:1e: + 07:53:5c:d9:e0:56:c5:82:07:f6:a3:e2:ad:f6:ad: + be:a0:4e:03:87:39:67:0c:9c:46:91:68:6b:0e:8e: + f8:49:97:9d:5b + ASN1 OID: prime256v1 + NIST CURVE: P-256 + X509v3 extensions: + X509v3 Key Usage: critical + Digital Signature, Key Encipherment, Certificate Sign + X509v3 Extended Key Usage: + TLS Web Server Authentication + X509v3 Basic Constraints: critical + CA:TRUE + X509v3 Subject Alternative Name: + DNS:example + Signature Algorithm: ecdsa-with-SHA256 + 30:46:02:21:00:c6:81:61:61:42:8d:37:e7:d0:c3:72:43:44: + 17:bd:84:ff:88:81:68:9a:99:08:ab:3c:3a:c0:1e:ea:8c:ba: + c0:02:21:00:de:c9:fa:e5:5e:c6:e2:db:23:64:43:a9:37:42: + 72:92:7f:6e:89:38:ea:9e:2a:a7:fd:2f:ea:9a:ff:20:21:e7 +-----BEGIN CERTIFICATE----- +MIIBbzCCARSgAwIBAgIQeCkq3C8SOX/JM5PqYTl9cDAKBggqhkjOPQQDAjASMRAw +DgYDVQQKEwdBY21lIENvMB4XDTE5MDIwNDIyNTYzNFoXDTI5MDIwMTIyNTYzNFow +EjEQMA4GA1UEChMHQWNtZSBDbzBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABISm +jGlTr4dLOWT+BCTm2PzWRjk1DpLcSAh+Al8eB1Nc2eBWxYIH9qPirfatvqBOA4c5 +ZwycRpFoaw6O+EmXnVujTDBKMA4GA1UdDwEB/wQEAwICpDATBgNVHSUEDDAKBggr +BgEFBQcDATAPBgNVHRMBAf8EBTADAQH/MBIGA1UdEQQLMAmCB2V4YW1wbGUwCgYI +KoZIzj0EAwIDSQAwRgIhAMaBYWFCjTfn0MNyQ0QXvYT/iIFompkIqzw6wB7qjLrA +AiEA3sn65V7G4tsjZEOpN0Jykn9uiTjqniqn/S/qmv8gIec= +-----END CERTIFICATE----- +` + leafWithAKID = ` + Certificate: + Data: + Version: 3 (0x2) + Serial Number: + f0:8a:62:f0:03:84:a2:cf:69:63:ad:71:3b:b6:5d:8c + Signature Algorithm: ecdsa-with-SHA256 + Issuer: O = Acme Co + Validity + Not Before: Feb 4 23:06:52 2019 GMT + Not After : Feb 1 23:06:52 2029 GMT + Subject: O = Acme LLC + Subject Public Key Info: + Public Key Algorithm: id-ecPublicKey + Public-Key: (256 bit) + pub: + 04:5a:4e:4d:fb:ff:17:f7:b6:13:e8:29:45:34:81: + 39:ff:8c:9c:d9:8c:0a:9f:dd:b5:97:4c:2b:20:91: + 1c:4f:6b:be:53:27:66:ec:4a:ad:08:93:6d:66:36: + 0c:02:70:5d:01:ca:7f:c3:29:e9:4f:00:ba:b4:14: + ec:c5:c3:34:b3 + ASN1 OID: prime256v1 + NIST CURVE: P-256 + X509v3 extensions: + X509v3 Key Usage: critical + Digital Signature, Key Encipherment + X509v3 Extended Key Usage: + TLS Web Server Authentication + X509v3 Basic Constraints: critical + CA:FALSE + X509v3 Authority Key Identifier: + keyid:C2:2B:5F:91:78:34:26:09:42:8D:6F:51:B2:C5:AF:4C:0B:DE:6A:42 + + X509v3 Subject Alternative Name: + DNS:example + Signature Algorithm: ecdsa-with-SHA256 + 30:44:02:20:64:e0:ba:56:89:63:ce:22:5e:4f:22:15:fd:3c: + 35:64:9a:3a:6b:7b:9a:32:a0:7f:f7:69:8c:06:f0:00:58:b8: + 02:20:09:e4:9f:6d:8b:9e:38:e1:b6:01:d5:ee:32:a4:94:65: + 93:2a:78:94:bb:26:57:4b:c7:dd:6c:3d:40:2b:63:90 +-----BEGIN CERTIFICATE----- +MIIBjTCCATSgAwIBAgIRAPCKYvADhKLPaWOtcTu2XYwwCgYIKoZIzj0EAwIwEjEQ +MA4GA1UEChMHQWNtZSBDbzAeFw0xOTAyMDQyMzA2NTJaFw0yOTAyMDEyMzA2NTJa +MBMxETAPBgNVBAoTCEFjbWUgTExDMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE +Wk5N+/8X97YT6ClFNIE5/4yc2YwKn921l0wrIJEcT2u+Uydm7EqtCJNtZjYMAnBd +Acp/wynpTwC6tBTsxcM0s6NqMGgwDgYDVR0PAQH/BAQDAgWgMBMGA1UdJQQMMAoG +CCsGAQUFBwMBMAwGA1UdEwEB/wQCMAAwHwYDVR0jBBgwFoAUwitfkXg0JglCjW9R +ssWvTAveakIwEgYDVR0RBAswCYIHZXhhbXBsZTAKBggqhkjOPQQDAgNHADBEAiBk +4LpWiWPOIl5PIhX9PDVkmjpre5oyoH/3aYwG8ABYuAIgCeSfbYueOOG2AdXuMqSU +ZZMqeJS7JldLx91sPUArY5A= +-----END CERTIFICATE----- +` +) + var unknownAuthorityErrorTests = []struct { cert string expected string