diff --git a/src/crypto/x509/verify.go b/src/crypto/x509/verify.go index 7226d0a8d5..21b870c171 100644 --- a/src/crypto/x509/verify.go +++ b/src/crypto/x509/verify.go @@ -215,6 +215,10 @@ func (c *Certificate) Verify(opts VerifyOptions) (chains [][]*Certificate, err e return c.systemVerify(&opts) } + if len(c.UnhandledCriticalExtensions) > 0 { + return nil, UnhandledCriticalExtension{} + } + if opts.Roots == nil { opts.Roots = systemRootsPool() if opts.Roots == nil { diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go index 71b0804d0a..987e28ab6c 100644 --- a/src/crypto/x509/x509.go +++ b/src/crypto/x509/x509.go @@ -488,6 +488,16 @@ type Certificate struct { // field is not populated when parsing certificates, see Extensions. ExtraExtensions []pkix.Extension + // UnhandledCriticalExtensions contains a list of extension IDs that + // were not (fully) processed when parsing. Verify will fail if this + // slice is non-empty, unless verification is delegated to an OS + // library which understands all the critical extensions. + // + // Users can access these extensions using Extensions and can remove + // elements from this slice if they believe that they have been + // handled. + UnhandledCriticalExtensions []asn1.ObjectIdentifier + ExtKeyUsage []ExtKeyUsage // Sequence of extended key usages. UnknownExtKeyUsage []asn1.ObjectIdentifier // Encountered extended key usages unknown to this package. @@ -897,7 +907,7 @@ func parseCertificate(in *certificate) (*Certificate, error) { for _, e := range in.TBSCertificate.Extensions { out.Extensions = append(out.Extensions, e) - failIfCritical := false + unhandled := false if len(e.Id) == 4 && e.Id[0] == 2 && e.Id[1] == 5 && e.Id[2] == 29 { switch e.Id[3] { @@ -936,7 +946,7 @@ func parseCertificate(in *certificate) (*Certificate, error) { if len(out.DNSNames) == 0 && len(out.EmailAddresses) == 0 && len(out.IPAddresses) == 0 { // If we didn't parse anything then we do the critical check, below. - failIfCritical = true + unhandled = true } case 30: @@ -1054,8 +1064,8 @@ func parseCertificate(in *certificate) (*Certificate, error) { } default: - // Unknown extensions cause an error if marked as critical. - failIfCritical = true + // Unknown extensions are recorded if critical. + unhandled = true } } else if e.Id.Equal(oidExtensionAuthorityInfoAccess) { // RFC 5280 4.2.2.1: Authority Information Access @@ -1076,12 +1086,12 @@ func parseCertificate(in *certificate) (*Certificate, error) { } } } else { - // Unknown extensions cause an error if marked as critical. - failIfCritical = true + // Unknown extensions are recorded if critical. + unhandled = true } - if e.Critical && failIfCritical { - return out, UnhandledCriticalExtension{} + if e.Critical && unhandled { + out.UnhandledCriticalExtensions = append(out.UnhandledCriticalExtensions, e.Id) } } diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go index 95efaf33b5..86a8b16cba 100644 --- a/src/crypto/x509/x509_test.go +++ b/src/crypto/x509/x509_test.go @@ -509,7 +509,13 @@ func TestUnknownCriticalExtension(t *testing.T) { CommonName: "foo", }, NotBefore: time.Unix(1000, 0), - NotAfter: time.Unix(100000, 0), + NotAfter: time.Now().AddDate(1, 0, 0), + + BasicConstraintsValid: true, + IsCA: true, + + KeyUsage: KeyUsageCertSign, + ExtKeyUsage: []ExtKeyUsage{ExtKeyUsageServerAuth}, ExtraExtensions: []pkix.Extension{ { @@ -525,13 +531,29 @@ func TestUnknownCriticalExtension(t *testing.T) { t.Fatalf("failed to create certificate: %s", err) } - _, err = ParseCertificate(derBytes) + cert, err := ParseCertificate(derBytes) + if err != nil { + t.Fatalf("Certificate with unknown critical extension was not parsed: %s", err) + } + + roots := NewCertPool() + roots.AddCert(cert) + + // Setting Roots ensures that Verify won't delegate to the OS + // library and thus the correct error should always be + // returned. + _, err = cert.Verify(VerifyOptions{Roots: roots}) if err == nil { - t.Fatalf("Certificate with critical extension was parsed without error.") + t.Fatal("Certificate with unknown critical extension was verified without error") } if _, ok := err.(UnhandledCriticalExtension); !ok { t.Fatalf("Error was %#v, but wanted one of type UnhandledCriticalExtension", err) } + + cert.UnhandledCriticalExtensions = nil + if _, err = cert.Verify(VerifyOptions{Roots: roots}); err != nil { + t.Errorf("Certificate failed to verify after unhandled critical extensions were cleared: %s", err) + } } }