crypto/x509: reject empty name constraints extension

Change-Id: Idcda0fc1607157cb5bbf0521fbdc0c77f043ca3a
Reviewed-on: https://go-review.googlesource.com/62691
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>

diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go
index 4324e89168b93b285eef42de2f8987d5a1987962..7d8150c1757909bcabcb8c613d3049ebb68bfc51 100644 (file)
--- a/src/crypto/x509/x509.go
+++ b/src/crypto/x509/x509.go
@@ -1210,6 +1210,14 @@ func parseCertificate(in *certificate) (*Certificate, error) {
                                        return nil, errors.New("x509: trailing data after X.509 NameConstraints")
                                }
 
+                               if len(constraints.Permitted) == 0 && len(constraints.Excluded) == 0 {
+                                       // https://tools.ietf.org/html/rfc5280#section-4.2.1.10:
+                                       //   “either the permittedSubtrees field
+                                       //   or the excludedSubtrees MUST be
+                                       //   present”
+                                       return nil, errors.New("x509: empty name constraints extension")
+                               }
+
                                getDNSNames := func(subtrees []generalSubtree, isCritical bool) (dnsNames []string, err error) {
                                        for _, subtree := range subtrees {
                                                if len(subtree.Name) == 0 {

diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go
index c644692f4e521d3a63d8737a801b156eadd6bf4b..a824bf6a0322b86c63b4e1701818bb39e3a59191 100644 (file)
--- a/src/crypto/x509/x509_test.go
+++ b/src/crypto/x509/x509_test.go
@@ -1512,3 +1512,36 @@ func TestSystemCertPool(t *testing.T) {
                t.Fatal(err)
        }
 }
+
+const emptyNameConstraintsPEM = `
+-----BEGIN CERTIFICATE-----
+MIIC1jCCAb6gAwIBAgICEjQwDQYJKoZIhvcNAQELBQAwKDEmMCQGA1UEAxMdRW1w
+dHkgbmFtZSBjb25zdHJhaW50cyBpc3N1ZXIwHhcNMTMwMjAxMDAwMDAwWhcNMjAw
+NTMwMTA0ODM4WjAhMR8wHQYDVQQDExZFbXB0eSBuYW1lIGNvbnN0cmFpbnRzMIIB
+IjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwriElUIt3LCqmJObs+yDoWPD
+F5IqgWk6moIobYjPfextZiYU6I3EfvAwoNxPDkN2WowcocUZMJbEeEq5ebBksFnx
+f12gBxlIViIYwZAzu7aFvhDMyPKQI3C8CG0ZSC9ABZ1E3umdA3CEueNOmP/TChNq
+Cl23+BG1Qb/PJkpAO+GfpWSVhTcV53Mf/cKvFHcjGNrxzdSoq9fyW7a6gfcGEQY0
+LVkmwFWUfJ0wT8kaeLr0E0tozkIfo01KNWNzv6NcYP80QOBRDlApWu9ODmEVJHPD
+blx4jzTQ3JLa+4DvBNOjVUOp+mgRmjiW0rLdrxwOxIqIOwNjweMCp/hgxX/hTQID
+AQABoxEwDzANBgNVHR4EBjAEoAChADANBgkqhkiG9w0BAQsFAAOCAQEAWG+/zUMH
+QhP8uNCtgSHyim/vh7wminwAvWgMKxlkLBFns6nZeQqsOV1lABY7U0Zuoqa1Z5nb
+6L+iJa4ElREJOi/erLc9uLwBdDCAR0hUTKD7a6i4ooS39DTle87cUnj0MW1CUa6H
+v5SsvpYW+1XleYJk/axQOOTcy4Es53dvnZsjXH0EA/QHnn7UV+JmlE3rtVxcYp6M
+LYPmRhTioROA/drghicRkiu9hxdPyxkYS16M5g3Zj30jdm+k/6C6PeNtN9YmOOga
+nCOSyFYfGhqOANYzpmuV+oIedAsPpIbfIzN8njYUs1zio+1IoI4o8ddM9sCbtPU8
+o+WoY6IsCKXV/g==
+-----END CERTIFICATE-----`
+
+func TestEmptyNameConstraints(t *testing.T) {
+       block, _ := pem.Decode([]byte(emptyNameConstraintsPEM))
+       _, err := ParseCertificate(block.Bytes)
+       if err == nil {
+               t.Fatal("unexpected success")
+       }
+
+       const expected = "empty name constraints"
+       if str := err.Error(); !strings.Contains(str, expected) {
+               t.Errorf("expected %q in error but got %q", expected, str)
+       }
+}