Adam Langley has uploaded this change for review.
crypto/x509: enforce name constaints in a “voice of god” style and support IP constraints
This change makes crypto/x509 enforce name constraints for all names in
a leaf certificate, not just the name being validation. Thus, after this
change, if a certificate validates then all the names in it can be
trusted – one doesn't have a validate again for each interesting name.
Making extended key usage work in this fashion still remains to be done.
Change-Id: I72ed5ff2f7284082d5bf3e1e86faf76cef62f9b5
---
A src/crypto/x509/name_constraints_test.go
M src/crypto/x509/verify.go
M src/crypto/x509/x509.go
M src/crypto/x509/x509_test.go
4 files changed, 1,185 insertions(+), 38 deletions(-)
diff --git a/src/crypto/x509/name_constraints_test.go b/src/crypto/x509/name_constraints_test.go
new file mode 100644
index 0000000..aea0329
--- /dev/null
+++ b/src/crypto/x509/name_constraints_test.go
@@ -0,0 +1,954 @@
+package x509
+
+import (
+ "bytes"
+ "crypto/ecdsa"
+ "crypto/elliptic"
+ "crypto/rand"
+ "crypto/x509/pkix"
+ "encoding/pem"
+ "fmt"
+ "math/big"
+ "net"
+ "strings"
+ "sync"
+ "testing"
+ "time"
+)
+
+type nameConstraintsTest struct {
+ roots []constraintsSpec
+ intermediates [][]constraintsSpec
+ leaf []string
+ expectedError string
+}
+
+type constraintsSpec struct {
+ ok []string
+ bad []string
+}
+
+var nameConstraintsTests = []nameConstraintsTest{
+ // #0: dummy test for the certificate generation process itself.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{},
+ },
+ leaf: []string{"dns:example.com"},
+ },
+
+ // #1: dummy test for the certificate generation process itself: single
+ // level of intermediate.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{},
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:example.com"},
+ },
+
+ // #2: dummy test for the certificate generation process itself: two
+ // levels of intermediates.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{},
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:example.com"},
+ },
+
+ // #3: matching DNS constraint in root
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:example.com"},
+ },
+
+ // #4: matching DNS constraint in intermediate.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{},
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:example.com"},
+ },
+ },
+ },
+ leaf: []string{"dns:example.com"},
+ },
+
+ // #5: .example.com only matches subdomains.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:.example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:example.com"},
+ expectedError: "\"example.com\" is not permitted",
+ },
+
+ // #6: .example.com matches subdomains.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:.example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:foo.example.com"},
+ },
+
+ // #7: .example.com matches multiple levels of subdomains
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:.example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:foo.bar.example.com"},
+ },
+
+ // #8: specifying a permitted list of names, implicitly excludes other
+ // name types
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:.example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"ip:10.1.1.1"},
+ expectedError: "unknown or unconstrained name: 10.1.1.1",
+ },
+
+ // #9: specifying a permitted list of names, implicitly excludes other
+ // name types
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"ip:10.0.0.0/8"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:example.com"},
+ expectedError: "unknown or unconstrained name: example.com",
+ },
+
+ // #10: intermediates can try to permit other names, which isn't
+ // forbidden if the leaf doesn't mention them.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:example.com", "dns:foo.com"},
+ },
+ },
+ },
+ leaf: []string{"dns:example.com"},
+ },
+
+ // #11: intermediates cannot add permitted names that the root doesn't
+ // grant them.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:example.com", "dns:foo.com"},
+ },
+ },
+ },
+ leaf: []string{"dns:foo.com"},
+ expectedError: "\"foo.com\" is not permitted",
+ },
+
+ // #12: intermediates can further limit their scope if they wish.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:.example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:.bar.example.com"},
+ },
+ },
+ },
+ leaf: []string{"dns:foo.bar.example.com"},
+ },
+
+ // #13: intermediates can further limit their scope and that limitation
+ // is effective
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:.example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:.bar.example.com"},
+ },
+ },
+ },
+ leaf: []string{"dns:foo.notbar.example.com"},
+ expectedError: "\"foo.notbar.example.com\" is not permitted",
+ },
+
+ // #14: intermediates can further limit their scope and that limitation
+ // is effective
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:.example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:.bar.example.com"},
+ },
+ },
+ },
+ leaf: []string{"dns:foo.notbar.example.com"},
+ expectedError: "\"foo.notbar.example.com\" is not permitted",
+ },
+
+ // #15: roots can exclude subtrees and that doesn't affect other names.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ bad: []string{"dns:.example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:foo.com"},
+ },
+
+ // #16: roots exclusions are effective.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ bad: []string{"dns:.example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:foo.example.com"},
+ expectedError: "\"foo.example.com\" is excluded",
+ },
+
+ // #17: intermediates can also exclude names and that doesn't affect
+ // other names.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{},
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{
+ bad: []string{"dns:.example.com"},
+ },
+ },
+ },
+ leaf: []string{"dns:foo.com"},
+ },
+
+ // #18: intermediate exclusions are effectives.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{},
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{
+ bad: []string{"dns:.example.com"},
+ },
+ },
+ },
+ leaf: []string{"dns:foo.example.com"},
+ expectedError: "\"foo.example.com\" is excluded",
+ },
+
+ // #19: having an exclusion doesn't prohibit other types of names.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ bad: []string{"dns:.example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:foo.com", "ip:10.1.1.1"},
+ },
+
+ // #20: IP-based exclusions are permitted and don't affect unrelated IP
+ // addresses.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ bad: []string{"ip:10.0.0.0/8"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"ip:192.168.1.1"},
+ },
+
+ // #21: IP-based exclusions are effective
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ bad: []string{"ip:10.0.0.0/8"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"ip:10.0.0.1"},
+ expectedError: "10.0.0.1 is part of excluded range",
+ },
+
+ // #22: intermediates can further constrain IP ranges.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ bad: []string{"ip:10.0.0.0/8"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{
+ bad: []string{"ip:11.0.0.0/8"},
+ },
+ },
+ },
+ leaf: []string{"ip:11.0.0.1"},
+ expectedError: "11.0.0.1 is part of excluded range",
+ },
+
+ // #23: when multiple intermediates are present, chain building can
+ // avoid intermediates with incompatible constraints.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{},
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:.foo.com"},
+ },
+ constraintsSpec{
+ ok: []string{"dns:.example.com"},
+ },
+ },
+ },
+ leaf: []string{"dns:foo.example.com"},
+ },
+
+ // #24: (same as the previous test, but in the other order in ensure
+ // that we don't pass it by luck.)
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{},
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:.example.com"},
+ },
+ constraintsSpec{
+ ok: []string{"dns:.foo.com"},
+ },
+ },
+ },
+ leaf: []string{"dns:foo.example.com"},
+ },
+
+ // #25: when multiple roots are valid, chain building can avoid roots
+ // with incompatible constraints.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{},
+ constraintsSpec{
+ ok: []string{"dns:foo.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:example.com"},
+ },
+
+ // #26: (same as the previous test, but in the other order in ensure
+ // that we don't pass it by luck.)
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:foo.com"},
+ },
+ constraintsSpec{},
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:example.com"},
+ },
+
+ // #27: chain building can find a valid path even with multiple levels
+ // of alternative intermediates and alternative roots.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:foo.com"},
+ },
+ constraintsSpec{
+ ok: []string{"dns:example.com"},
+ },
+ constraintsSpec{},
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ constraintsSpec{
+ ok: []string{"dns:foo.com"},
+ },
+ },
+ []constraintsSpec{
+ constraintsSpec{},
+ constraintsSpec{
+ ok: []string{"dns:foo.com"},
+ },
+ },
+ },
+ leaf: []string{"dns:bar.com"},
+ },
+
+ // #28: chain building doesn't get stuck when there is no valid path.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:foo.com"},
+ },
+ constraintsSpec{
+ ok: []string{"dns:example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ constraintsSpec{
+ ok: []string{"dns:foo.com"},
+ },
+ },
+ []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:bar.com"},
+ },
+ constraintsSpec{
+ ok: []string{"dns:foo.com"},
+ },
+ },
+ },
+ leaf: []string{"dns:bar.com"},
+ expectedError: "\"bar.com\" is not permitted",
+ },
+
+ // #29: unknown name types don't cause a problem without constraints.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{},
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"unknown:"},
+ },
+
+ // #30: unknown name types are not accepted in a name constrained path.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:foo.com", "dns:.foo.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"unknown:"},
+ expectedError: "unknown name of type 9",
+ },
+
+ // #31: unknown name types are accepted if an unconstrained path
+ // exists.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:foo.com", "dns:.foo.com"},
+ },
+ constraintsSpec{},
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"unknown:"},
+ },
+
+ // #32: without SANs, a certificate is rejected in a constrained chain.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:foo.com", "dns:.foo.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{},
+ expectedError: "leaf doesn't have a SAN extension",
+ },
+
+ // #33: IPv6 addresses work in constraints: roots can permit them as
+ // expected.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"ip:2000:abcd::/32"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"ip:2000:abcd:1234::"},
+ },
+
+ // #34: IPv6 addresses work in constraints: root restrictions are
+ // effective.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"ip:2000:abcd::/32"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"ip:2000:1234:abcd::"},
+ expectedError: "2000:1234:abcd:: is not permitted",
+ },
+
+ // #35: An IPv6 permitted subtree implicitly excludes all DNS names.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"ip:2000:abcd::/32"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"ip:2000:1234:abcd::", "dns:foo.com"},
+ expectedError: "unconstrained name: foo.com",
+ },
+
+ // #36: IPv6 exclusions don't affect unrelated addresses.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ bad: []string{"ip:2000:abcd::/32"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"ip:2000:1234::"},
+ },
+
+ // #37: IPv6 exclusions are effective.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ bad: []string{"ip:2000:abcd::/32"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"ip:2000:abcd::"},
+ expectedError: "2000:abcd:: is part of excluded range",
+ },
+
+ // #38: IPv6 constraints do not permit IPv4 addresses.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"ip:2000:abcd::/32"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"ip:10.0.0.1"},
+ expectedError: "10.0.0.1 is not permitted",
+ },
+
+ // #39: IPv4 constraints do not permit IPv6 addresses.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"ip:10.0.0.0/8"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"ip:2000:abcd::"},
+ expectedError: "2000:abcd:: is not permitted",
+ },
+
+ // #40: DNS constraints don't block IP address if there's an excluded IP
+ // range, as that indicates that the issuer thought about IP addresses
+ // at least.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:.example.com"},
+ bad: []string{"ip:10.0.0.1/8"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"ip:192.168.1.1"},
+ },
+}
+
+func makeConstraintsCACert(constraints constraintsSpec, name string, key *ecdsa.PrivateKey, parent *Certificate, parentKey *ecdsa.PrivateKey) (*Certificate, error) {
+ var serialBytes [16]byte
+ rand.Read(serialBytes[:])
+
+ template := &Certificate{
+ SerialNumber: new(big.Int).SetBytes(serialBytes[:]),
+ Subject: pkix.Name{
+ CommonName: name,
+ },
+ NotBefore: time.Unix(1000, 0),
+ NotAfter: time.Unix(2000, 0),
+ KeyUsage: KeyUsageCertSign,
+ BasicConstraintsValid: true,
+ IsCA: true,
+ }
+
+ if err := addConstraintsToTemplate(constraints, template); err != nil {
+ return nil, err
+ }
+
+ if parent == nil {
+ parent = template
+ }
+ derBytes, err := CreateCertificate(rand.Reader, template, parent, &key.PublicKey, parentKey)
+ if err != nil {
+ return nil, err
+ }
+
+ caCert, err := ParseCertificate(derBytes)
+ if err != nil {
+ return nil, err
+ }
+
+ return caCert, nil
+}
+
+func addConstraintsToTemplate(constraints constraintsSpec, template *Certificate) error {
+ parse := func(constraints []string) (dnsNames []string, ips []*net.IPNet, err error) {
+ for _, constraint := range constraints {
+ switch {
+ case strings.HasPrefix(constraint, "dns:"):
+ dnsNames = append(dnsNames, constraint[4:])
+
+ case strings.HasPrefix(constraint, "ip:"):
+ _, ipNet, err := net.ParseCIDR(constraint[3:])
+ if err != nil {
+ return nil, nil, err
+ }
+ ips = append(ips, ipNet)
+
+ default:
+ return nil, nil, fmt.Errorf("unknown constraint %q", constraint)
+ }
+ }
+
+ return dnsNames, ips, err
+ }
+
+ var err error
+ template.PermittedDNSDomains, template.PermittedIPRanges, err = parse(constraints.ok)
+ if err != nil {
+ return err
+ }
+
+ template.ExcludedDNSDomains, template.ExcludedIPRanges, err = parse(constraints.bad)
+ if err != nil {
+ return err
+ }
+
+ return nil
+}
+
+func TestNameConstraintCases(t *testing.T) {
+ privateKeys := sync.Pool{
+ New: func() interface{} {
+ priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+ if err != nil {
+ panic(err)
+ }
+ return priv
+ },
+ }
+
+ var keys []*ecdsa.PrivateKey
+
+ for i, test := range nameConstraintsTests {
+ rootPool := NewCertPool()
+ rootKey := privateKeys.Get().(*ecdsa.PrivateKey)
+ keys = append(keys, rootKey)
+ rootName := fmt.Sprintf("Root %d", i)
+ var parent *Certificate
+ var parentKey *ecdsa.PrivateKey
+
+ for _, root := range test.roots {
+ rootCert, err := makeConstraintsCACert(root, rootName, rootKey, nil, rootKey)
+ if err != nil {
+ t.Fatalf("#%d: failed to create root: %s", i, err)
+ }
+
+ parent, parentKey = rootCert, rootKey
+ rootPool.AddCert(rootCert)
+ }
+
+ intermediatePool := NewCertPool()
+
+ for level, intermediates := range test.intermediates {
+ levelKey := privateKeys.Get().(*ecdsa.PrivateKey)
+ keys = append(keys, levelKey)
+ levelName := fmt.Sprintf("Intermediate level %d", level)
+ var last *Certificate
+
+ for _, intermediate := range intermediates {
+ caCert, err := makeConstraintsCACert(intermediate, levelName, levelKey, parent, parentKey)
+ if err != nil {
+ t.Fatalf("#%d: failed to create %q: %s", i, levelName, err)
+ }
+
+ last = caCert
+ intermediatePool.AddCert(caCert)
+ }
+
+ parent = last
+ parentKey = levelKey
+ }
+
+ leafKey := privateKeys.Get().(*ecdsa.PrivateKey)
+ keys = append(keys, leafKey)
+ var serialBytes [16]byte
+ rand.Read(serialBytes[:])
+
+ template := &Certificate{
+ SerialNumber: new(big.Int).SetBytes(serialBytes[:]),
+ Subject: pkix.Name{
+ CommonName: "Leaf",
+ },
+ NotBefore: time.Unix(1000, 0),
+ NotAfter: time.Unix(2000, 0),
+ KeyUsage: KeyUsageDigitalSignature,
+ BasicConstraintsValid: true,
+ IsCA: false,
+ }
+
+ for _, name := range test.leaf {
+ switch {
+ case strings.HasPrefix(name, "dns:"):
+ template.DNSNames = append(template.DNSNames, name[4:])
+
+ case strings.HasPrefix(name, "ip:"):
+ ip := net.ParseIP(name[3:])
+ if ip == nil {
+ t.Fatalf("#%d: cannot parse IP %q", i, name[3:])
+ }
+ template.IPAddresses = append(template.IPAddresses, ip)
+
+ case strings.HasPrefix(name, "unknown:"):
+ // This is a special case for testing unknown
+ // name types. A custom SAN extension is
+ // injected into the certificate.
+ if len(test.leaf) != 1 {
+ panic("when using unknown name types, it must be the sole name")
+ }
+
+ template.ExtraExtensions = append(template.ExtraExtensions, pkix.Extension{
+ Id: []int{2, 5, 29, 17},
+ Value: []byte{
+ 0x30, // SEQUENCE
+ 3, // three bytes
+ 9, // undefined GeneralName type 9
+ 1,
+ 1,
+ },
+ })
+
+ default:
+ t.Fatalf("#%d: unknown name type %q", i, name)
+ }
+ }
+
+ derBytes, err := CreateCertificate(rand.Reader, template, parent, &leafKey.PublicKey, parentKey)
+ if err != nil {
+ t.Fatalf("#%d: cannot create leaf: %s", i, err)
+ }
+
+ privateKeys.Put(leafKey)
+
+ leafCert, err := ParseCertificate(derBytes)
+ if err != nil {
+ t.Fatalf("#%d: cannot create leaf: %s", i, err)
+ }
+
+ verifyOpts := VerifyOptions{
+ Roots: rootPool,
+ Intermediates: intermediatePool,
+ CurrentTime: time.Unix(1500, 0),
+ }
+ _, err = leafCert.Verify(verifyOpts)
+
+ logInfo := false
+ if len(test.expectedError) == 0 {
+ if err != nil {
+ logInfo = true
+ t.Errorf("#%d: unexpected failure: %s", i, err)
+ }
+ } else {
+ if err == nil {
+ logInfo = true
+ t.Errorf("#%d: unexpected success", i)
+ } else if !strings.Contains(err.Error(), test.expectedError) {
+ logInfo = true
+ t.Errorf("#%d: expected error containing %q, but got: %s", i, test.expectedError, err)
+ }
+ }
+
+ if logInfo {
+ certAsPEM := func(cert *Certificate) string {
+ var buf bytes.Buffer
+ pem.Encode(&buf, &pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw})
+ return string(buf.Bytes())
+ }
+ t.Errorf("#%d: root:\n%s", i, certAsPEM(rootPool.certs[0]))
+ t.Errorf("#%d: leaf:\n%s", i, certAsPEM(leafCert))
+ t.Errorf("#%d: %v\n", i, rootPool.certs[0].ExcludedIPRanges)
+ }
+
+ for _, key := range keys {
+ privateKeys.Put(key)
+ }
+ keys = keys[:0]
+ }
+}
diff --git a/src/crypto/x509/verify.go b/src/crypto/x509/verify.go
index 2b4f39d..f4b1186 100644
--- a/src/crypto/x509/verify.go
+++ b/src/crypto/x509/verify.go
@@ -25,8 +25,8 @@
// given in the VerifyOptions.
Expired
// CANotAuthorizedForThisName results when an intermediate or root
- // certificate has a name constraint which doesn't include the name
- // being checked.
+ // certificate has a name constraint which doesn't permit a DNS or
+ // other name (including IP address) in the leaf certificate.
CANotAuthorizedForThisName
// TooManyIntermediates results when a path length constraint is
// violated.
@@ -37,6 +37,14 @@
// NameMismatch results when the subject name of a parent certificate
// does not match the issuer name in the child.
NameMismatch
+ // NameConstraintsWithoutSANs results when a leaf certificate doesn't
+ // contain a Subject Alternative Name extension, but a CA certificate
+ // contains name constraints.
+ NameConstraintsWithoutSANs
+ // UnconstrainedName results when a CA certificate contains permitted
+ // name constraints, but leaf certificate contains a name of an
+ // unsupported or unconstrained type.
+ UnconstrainedName
)
// CertificateInvalidError results when an odd error occurs. Users of this
@@ -44,6 +52,7 @@
type CertificateInvalidError struct {
Cert *Certificate
Reason InvalidReason
+ Detail string
}
func (e CertificateInvalidError) Error() string {
@@ -53,13 +62,17 @@
case Expired:
return "x509: certificate has expired or is not yet valid"
case CANotAuthorizedForThisName:
- return "x509: a root or intermediate certificate is not authorized to sign in this domain"
+ return "x509: a root or intermediate certificate is not authorized to sign for this name: " + e.Detail
case TooManyIntermediates:
return "x509: too many intermediates for path length constraint"
case IncompatibleUsage:
return "x509: certificate specifies an incompatible key usage"
case NameMismatch:
return "x509: issuer name does not match subject from issuing certificate"
+ case NameConstraintsWithoutSANs:
+ return "x509: issuer has name constraints but leaf doesn't have a SAN extension"
+ case UnconstrainedName:
+ return "x509: issuer has name constraints but leaf contains unknown or unconstrained name: " + e.Detail
}
return "x509: unknown error"
}
@@ -189,12 +202,13 @@
return isSubdomain != constraintHasLeadingDot
}
-// isValid performs validity checks on the c.
+// isValid performs validity checks on c given that it is a candidate to append
+// to the chain in currentChain.
func (c *Certificate) isValid(certType int, currentChain []*Certificate, opts *VerifyOptions) error {
if len(currentChain) > 0 {
child := currentChain[len(currentChain)-1]
if !bytes.Equal(child.RawIssuer, c.RawSubject) {
- return CertificateInvalidError{c, NameMismatch}
+ return CertificateInvalidError{c, NameMismatch, ""}
}
}
@@ -203,26 +217,97 @@
now = time.Now()
}
if now.Before(c.NotBefore) || now.After(c.NotAfter) {
- return CertificateInvalidError{c, Expired}
+ return CertificateInvalidError{c, Expired, ""}
}
- if len(c.PermittedDNSDomains) > 0 {
- ok := false
- for _, constraint := range c.PermittedDNSDomains {
- ok = matchNameConstraint(opts.DNSName, constraint)
- if ok {
- break
- }
+ if (certType == intermediateCertificate || certType == rootCertificate) && c.hasNameConstraints() {
+ if len(currentChain) == 0 {
+ return errors.New("x509: internal error: empty chain when appending CA cert")
}
+ leaf := currentChain[0]
+ sanExtension, ok := leaf.getSANExtension()
if !ok {
- return CertificateInvalidError{c, CANotAuthorizedForThisName}
+ // This is the deprecated, legacy case of depending on
+ // the CN as a hostname. Chains modern enough to be
+ // using name constraints should not be depending on
+ // CNs.
+ return CertificateInvalidError{c, NameConstraintsWithoutSANs, ""}
}
- }
- for _, constraint := range c.ExcludedDNSDomains {
- if matchNameConstraint(opts.DNSName, constraint) {
- return CertificateInvalidError{c, CANotAuthorizedForThisName}
+ somePermitted := len(c.PermittedDNSDomains) != 0 || len(c.PermittedIPRanges) != 0
+
+ err := forEachSAN(sanExtension, func(tag int, data []byte) error {
+ switch tag {
+ case 2:
+ name := string(data)
+
+ if somePermitted && len(c.ExcludedDNSDomains) == 0 && len(c.PermittedDNSDomains) == 0 {
+ return CertificateInvalidError{c, UnconstrainedName, name}
+ }
+
+ for _, constraint := range c.ExcludedDNSDomains {
+ if matchNameConstraint(name, constraint) {
+ return CertificateInvalidError{c, CANotAuthorizedForThisName, fmt.Sprintf("DNS name %q is excluded by constraint %q", name, constraint)}
+ }
+ }
+
+ ok := true
+ for _, constraint := range c.PermittedDNSDomains {
+ ok = matchNameConstraint(name, constraint)
+ if ok {
+ break
+ }
+ }
+
+ if !ok {
+ return CertificateInvalidError{c, CANotAuthorizedForThisName, fmt.Sprintf("DNS name %q is not permitted by any constraint", name)}
+ }
+
+ case 7:
+ ip := net.IP(data)
+ if l := len(ip); l != net.IPv4len && l != net.IPv6len {
+ // This certificate should not have
+ // parsed successfully.
+ panic("bad IP length")
+ }
+
+ if somePermitted && len(c.ExcludedIPRanges) == 0 && len(c.PermittedIPRanges) == 0 {
+ return CertificateInvalidError{c, UnconstrainedName, ip.String()}
+ }
+
+ for _, constraint := range c.ExcludedIPRanges {
+ if constraint.Contains(ip) {
+ return CertificateInvalidError{c, CANotAuthorizedForThisName, fmt.Sprintf("IP address %s is part of excluded range %s", ip, constraint)}
+ }
+ }
+
+ ok := true
+ for _, constraint := range c.PermittedIPRanges {
+ ok = constraint.Contains(ip)
+ if ok {
+ break
+ }
+ }
+
+ if !ok {
+ return CertificateInvalidError{c, CANotAuthorizedForThisName, fmt.Sprintf("IP address %s is not permitted by any constraint", ip)}
+ }
+
+ default:
+ // If a parent has name constraints then all
+ // the SANs in the leaf must be of a supported
+ // type and must match the constraints.
+ if somePermitted {
+ return CertificateInvalidError{c, UnconstrainedName, fmt.Sprintf("unknown name of type %d", tag)}
+ }
+ }
+
+ return nil
+ })
+
+ if err != nil {
+ return err
}
}
@@ -244,13 +329,13 @@
// encryption key could only be used for Diffie-Hellman key agreement.
if certType == intermediateCertificate && (!c.BasicConstraintsValid || !c.IsCA) {
- return CertificateInvalidError{c, NotAuthorizedToSign}
+ return CertificateInvalidError{c, NotAuthorizedToSign, ""}
}
if c.BasicConstraintsValid && c.MaxPathLen >= 0 {
numIntermediates := len(currentChain) - 1
if numIntermediates > c.MaxPathLen {
- return CertificateInvalidError{c, TooManyIntermediates}
+ return CertificateInvalidError{c, TooManyIntermediates, ""}
}
}
@@ -337,7 +422,7 @@
}
if len(chains) == 0 {
- err = CertificateInvalidError{c, IncompatibleUsage}
+ err = CertificateInvalidError{c, IncompatibleUsage, ""}
}
return
diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go
index 2a8ee59..42c984c 100644
--- a/src/crypto/x509/x509.go
+++ b/src/crypto/x509/x509.go
@@ -703,6 +703,8 @@
PermittedDNSDomainsCritical bool // if true then the name constraints are marked critical.
PermittedDNSDomains []string
ExcludedDNSDomains []string
+ PermittedIPRanges []*net.IPNet
+ ExcludedIPRanges []*net.IPNet
// CRL Distribution Points
CRLDistributionPoints []string
@@ -821,6 +823,26 @@
return checkSignature(algo, signed, signature, c.PublicKey)
}
+func (c *Certificate) hasNameConstraints() bool {
+ for _, e := range c.Extensions {
+ if len(e.Id) == 4 && e.Id[0] == 2 && e.Id[1] == 5 && e.Id[2] == 29 && e.Id[3] == 30 {
+ return true
+ }
+ }
+
+ return false
+}
+
+func (c *Certificate) getSANExtension() ([]byte, bool) {
+ for _, e := range c.Extensions {
+ if len(e.Id) == 4 && e.Id[0] == 2 && e.Id[1] == 5 && e.Id[2] == 29 && e.Id[3] == 17 {
+ return e.Value, true
+ }
+ }
+
+ return nil, false
+}
+
func signaturePublicKeyAlgoMismatchError(expectedPubKeyAlgo PublicKeyAlgorithm, pubKey interface{}) error {
return fmt.Errorf("x509: signature algorithm specifies an %s public key, but have public key of type %T", expectedPubKeyAlgo.String(), pubKey)
}
@@ -931,7 +953,8 @@
}
type generalSubtree struct {
- Name string `asn1:"tag:2,optional,ia5"`
+ Name string `asn1:"tag:2,optional,ia5"`
+ IPAndMask []byte `asn1:"tag:7,optional"`
}
// RFC 5280, 4.2.2.1
@@ -1108,6 +1131,31 @@
return
}
+// isValidIPMask returns true iff mask consists of zero or more 1 bits, followed by zero bits.
+func isValidIPMask(mask []byte) bool {
+ seenZero := false
+
+ for _, b := range mask {
+ if seenZero {
+ if b != 0 {
+ return false
+ }
+
+ continue
+ }
+
+ switch b {
+ case 0x00, 0x80, 0xc0, 0xe0, 0xf0, 0xf8, 0xfc, 0xfe:
+ seenZero = true
+ case 0xff:
+ default:
+ return false
+ }
+ }
+
+ return true
+}
+
func parseCertificate(in *certificate) (*Certificate, error) {
out := new(Certificate)
out.Raw = in.Raw
@@ -1228,24 +1276,50 @@
return nil, errors.New("x509: empty name constraints extension")
}
- getDNSNames := func(subtrees []generalSubtree, isCritical bool) (dnsNames []string, err error) {
+ getValues := func(subtrees []generalSubtree) (dnsNames []string, ips []*net.IPNet, err error) {
for _, subtree := range subtrees {
- if len(subtree.Name) == 0 {
- if isCritical {
- return nil, UnhandledCriticalExtension{}
+ switch {
+ case len(subtree.Name) != 0:
+ dnsNames = append(dnsNames, subtree.Name)
+
+ case len(subtree.IPAndMask) != 0:
+ l := len(subtree.IPAndMask)
+ var ip, mask []byte
+
+ switch l {
+ case 8:
+ ip = subtree.IPAndMask[:4]
+ mask = subtree.IPAndMask[4:]
+
+ case 32:
+ ip = subtree.IPAndMask[:16]
+ mask = subtree.IPAndMask[16:]
+
+ default:
+ return nil, nil, fmt.Errorf("x509: IP constraint contained value of length %d", l)
}
- continue
+
+ if !isValidIPMask(mask) {
+ return nil, nil, fmt.Errorf("x509: IP constraint contained invalid mask %x", mask)
+ }
+
+ ips = append(ips, &net.IPNet{IP: net.IP(ip), Mask: net.IPMask(mask)})
+
+ default:
+ // The critical flag can be ignored here because, when verifying
+ // a chain, /any/ unknown name types in the leaf certificate
+ // will cause a failure if an up-chain certificate has name
+ // constraints.
}
- dnsNames = append(dnsNames, subtree.Name)
}
- return dnsNames, nil
+ return dnsNames, ips, nil
}
- if out.PermittedDNSDomains, err = getDNSNames(constraints.Permitted, e.Critical); err != nil {
+ if out.PermittedDNSDomains, out.PermittedIPRanges, err = getValues(constraints.Permitted); err != nil {
return out, err
}
- if out.ExcludedDNSDomains, err = getDNSNames(constraints.Excluded, e.Critical); err != nil {
+ if out.ExcludedDNSDomains, out.ExcludedIPRanges, err = getValues(constraints.Excluded); err != nil {
return out, err
}
out.PermittedDNSDomainsCritical = e.Critical
@@ -1632,20 +1706,36 @@
n++
}
- if (len(template.PermittedDNSDomains) > 0 || len(template.ExcludedDNSDomains) > 0) &&
+ if (len(template.PermittedDNSDomains) > 0 || len(template.ExcludedDNSDomains) > 0 ||
+ len(template.PermittedIPRanges) > 0 || len(template.ExcludedIPRanges) > 0) &&
!oidInExtensions(oidExtensionNameConstraints, template.ExtraExtensions) {
ret[n].Id = oidExtensionNameConstraints
ret[n].Critical = template.PermittedDNSDomainsCritical
var out nameConstraints
- out.Permitted = make([]generalSubtree, len(template.PermittedDNSDomains))
- for i, permitted := range template.PermittedDNSDomains {
- out.Permitted[i] = generalSubtree{Name: permitted}
+ ipAndMask := func(ipNet *net.IPNet) []byte {
+ maskedIP := ipNet.IP.Mask(ipNet.Mask)
+ ipAndMask := make([]byte, 0, len(maskedIP)+len(ipNet.Mask))
+ ipAndMask = append(ipAndMask, maskedIP...)
+ ipAndMask = append(ipAndMask, ipNet.Mask...)
+ return ipAndMask
}
- out.Excluded = make([]generalSubtree, len(template.ExcludedDNSDomains))
- for i, excluded := range template.ExcludedDNSDomains {
- out.Excluded[i] = generalSubtree{Name: excluded}
+
+ out.Permitted = make([]generalSubtree, 0, len(template.PermittedDNSDomains)+len(template.PermittedIPRanges))
+ for _, permitted := range template.PermittedDNSDomains {
+ out.Permitted = append(out.Permitted, generalSubtree{Name: permitted})
+ }
+ for _, permitted := range template.PermittedIPRanges {
+ out.Permitted = append(out.Permitted, generalSubtree{IPAndMask: ipAndMask(permitted)})
+ }
+
+ out.Excluded = make([]generalSubtree, 0, len(template.ExcludedDNSDomains)+len(template.ExcludedIPRanges))
+ for _, excluded := range template.ExcludedDNSDomains {
+ out.Excluded = append(out.Excluded, generalSubtree{Name: excluded})
+ }
+ for _, excluded := range template.ExcludedIPRanges {
+ out.Excluded = append(out.Excluded, generalSubtree{IPAndMask: ipAndMask(excluded)})
}
ret[n].Value, err = asn1.Marshal(out)
diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go
index a824bf6..6b665ff 100644
--- a/src/crypto/x509/x509_test.go
+++ b/src/crypto/x509/x509_test.go
@@ -352,6 +352,14 @@
"9048084225c53e8acb7feb6f04d16dc574a2f7a27c7b603c77cd0ece48027f012fb69b37e02a2a" +
"36dcd585d6ace53f546f961e05af"
+func parseCIDR(s string) *net.IPNet {
+ _, net, err := net.ParseCIDR(s)
+ if err != nil {
+ panic(err)
+ }
+ return net
+}
+
func TestCreateSelfSignedCertificate(t *testing.T) {
random := rand.Reader
@@ -427,6 +435,8 @@
PolicyIdentifiers: []asn1.ObjectIdentifier{[]int{1, 2, 3}},
PermittedDNSDomains: []string{".example.com", "example.com"},
ExcludedDNSDomains: []string{"bar.example.com"},
+ PermittedIPRanges: []*net.IPNet{parseCIDR("192.168.1.1/16"), parseCIDR("1.2.3.4/8")},
+ ExcludedIPRanges: []*net.IPNet{parseCIDR("2001:db8::/48")},
CRLDistributionPoints: []string{"http://crl1.example.com/ca1.crl", "http://crl2.example.com/ca1.crl"},
@@ -468,6 +478,14 @@
t.Errorf("%s: failed to parse name constraint exclusions: %#v", test.name, cert.ExcludedDNSDomains)
}
+ if len(cert.PermittedIPRanges) != 2 || cert.PermittedIPRanges[0].String() != "192.168.0.0/16" || cert.PermittedIPRanges[1].String() != "1.0.0.0/8" {
+ t.Errorf("%s: failed to parse IP constraints: %#v", test.name, cert.PermittedIPRanges)
+ }
+
+ if len(cert.ExcludedIPRanges) != 1 || cert.ExcludedIPRanges[0].String() != "2001:db8::/48" {
+ t.Errorf("%s: failed to parse IP constraint exclusions: %#v", test.name, cert.ExcludedIPRanges)
+ }
+
if cert.Subject.CommonName != commonName {
t.Errorf("%s: subject wasn't correctly copied from the template. Got %s, want %s", test.name, cert.Subject.CommonName, commonName)
}
To view, visit change 62693. To unsubscribe, or for help writing mail filters, visit settings.
Adam Langley uploaded patch set #2 to this change.
crypto/x509: enforce name constaints in a “voice of god” style and support IP constraints
This change makes crypto/x509 enforce name constraints for all names in
a leaf certificate, not just the name being validation. Thus, after this
change, if a certificate validates then all the names in it can be
trusted – one doesn't have a validate again for each interesting name.
Making extended key usage work in this fashion still remains to be done.
Change-Id: I72ed5ff2f7284082d5bf3e1e86faf76cef62f9b5
---
A src/crypto/x509/name_constraints_test.go
M src/crypto/x509/verify.go
M src/crypto/x509/x509.go
M src/crypto/x509/x509_test.go
4 files changed, 1,270 insertions(+), 38 deletions(-)
To view, visit change 62693. To unsubscribe, or for help writing mail filters, visit settings.
First, I didn't review this carefully. I think that this is an intermediate step towards full name constraint support and so most of my comments are me learning what the intended short-term effect is, which is probably different than "fully implement name constraints 100% correctly."
6 comments:
in
a leaf certificate
What about names in intermediate certificates being constrained by the name constraints above it? Is that supposed to be included here or is that work deferred? Probably worth clarifying that here.
File src/crypto/x509/verify.go:
Patch Set #2, Line 29: in the leaf certificate
Will there be a different error code for intermediate certificates that violate name constraints?
contains permitted
// name constraints, but leaf certificate contains a name of an
// unsupported or unconstrained type.
At least: s/but leaf/but the leaf/
Also, I suggest "contains a permitted name constraint for an unsupported type of name, and the leaf certificate contains a name of that type." In other words, don't reject a leaf certificate just because it contains a name of an unsupported type; only fail if there's actually a name constraint of that same type. For example, if the leaf contains an otherName (e.g. SRVName) name and there are no otherName constraints then the certificate chain should be accepted.
In particular, I'm pretty sure there are publicly-trusted certificate chains where the end-entity certificate contains an rfc822Name subjectAltName entry and the chain contains dNSName name constraints. See https://bugzilla.mozilla.org/show_bug.cgi?id=1111399.
In a full implementation, both permitted and excluded constraints would be considered, not just "permitted" as noted here, and also "the leaf certificate contains" should eventually become "any certificate lower in the chain."
Patch Set #2, Line 227: leaf := currentChain[0]
It would be more correct to loop over the entire chain, including all the intermediates below `c`. Otherwise, at least it would be good to have a comment here about why the names in the intermediates aren't being checked.
Patch Set #2, Line 235: return CertificateInvalidError{c, NameConstraintsWithoutSANs, ""}
In order for this to work, VerifyHostname would need to know whether or not there is any chain available without name constraints. But, it doesn't know anything about the chain because it's only given the leaf certificate. Accordingly, either VerifyHostname should be chained to never look at the subject CN or this logic needs to be changed to enforce name constraints for the value of the subject CN whenever the subject CN could possibly match a call to VerifyHostname (i.e. it looks like a valid dNSName).
Patch Set #2, Line 238: somePermitted := c.hasSomePermittedNameConstraint()
It isn't clear why the presence of permitted name constraints matters differently than the presence of excluded name constraints. I think maybe it is just a temporary limitation of the implementation in which case I think there should be a comment pointing on the limitation here.
To view, visit change 62693. To unsubscribe, or for help writing mail filters, visit settings.
Also see https://boringssl.googlesource.com/boringssl/+/b86be3617d4d79db9b418bed78bbf71eadd61f5c, which is something to take into consideration here.
Patch Set 2:
Also see https://boringssl.googlesource.com/boringssl/+/b86be3617d4d79db9b418bed78bbf71eadd61f5c, which is something to take into consideration here.
Yes, thanks.
4 comments:
in
a leaf certificate
What about names in intermediate certificates being constrained by the name constraints above it? Is […]
Are you suggesting the intermediates might have SANs, or that the constraints in intermediates should be limited too? I.e. trying to permit foo.com in an intermediate when the root only permits bar.com should be invalid, even if the leaf is only for bar.com? I've no objection to that.
What if an intermediate tries to exclude bar.com? (Which is meaningless if the root only permits foo.com, but also out of the root's scope.) Would you prohibit that too?
File src/crypto/x509/verify.go:
Patch Set #2, Line 29: in the leaf certificate
Will there be a different error code for intermediate certificates that violate name constraints?
There can be.
contains permitted
// name constraints, but leaf certificate contains a name of an
// unsupported or unconstrained type.
At least: s/but leaf/but the leaf/
Also, I suggest "contains a permitted name constraint for an unsupported type of name, and the leaf certificate contains a name of that type." In other words, don't reject a leaf certificate just because it contains a name of an unsupported type; only fail if there's actually a name constraint of that same type. For example, if the leaf contains an otherName (e.g. SRVName) name and there are no otherName constraints then the certificate chain should be accepted.
If we accept unknown name types then, if a new name type is added, then all "constrained" CA certificates have full authority over the type until verifiers are updated.
With this change I'm saying that a "permitted" constraint anywhere in the chain means that all names need to be recognised. (At the moment that's just names in the leaf but, as you note, perhaps it should include constraints in intermediates too.)
Is that reasonable?
In particular, I'm pretty sure there are publicly-trusted certificate chains where the end-entity certificate contains an rfc822Name subjectAltName entry and the chain contains dNSName name constraints. See https://bugzilla.mozilla.org/show_bug.cgi?id=1111399.
So they would be blocked until rfc822Name is supported, but I do plan on doing that before the next release.
Patch Set #2, Line 235: return CertificateInvalidError{c, NameConstraintsWithoutSANs, ""}
In order for this to work, VerifyHostname would need to know whether or not there is any chain avail […]
I do hope that VerifyHostname can be made to ignore the CN if SANs are present. That would align with Chromium, at least.
To view, visit change 62693. To unsubscribe, or for help writing mail filters, visit settings.
3 comments:
in
a leaf certificate
Are you suggesting the intermediates might have SANs
Yes, they can. It would be unusual for them to have a SAN with a dNSName, but there are intermediates that have directoryName and, IIRC, email address, SANs. Besides, directoryName name constraints apply to the subject field, too, regardless of whether there is a SAN extension.
No, that's allowed. Name constraints constrain only names; they don't constrain name constraints.
File src/crypto/x509/verify.go:
contains permitted
// name constraints, but leaf certificate contains a name of an
// unsupported or unconstrained type.
If we accept unknown name types then, if a new name type is added, then all "constrained" CA certificates have full authority over the type until verifiers are updated.
RFC 5280 semantics are such that if a name type isn't explicitly constrained, then all names of that type are allowed.
I think it makes some sense to have a stricter rule that says "if a CA certificate contains constraints for some name types, then assume it wants to exclude all names of all types not mentioned in its constraints." I tried to implement a similar rule years ago in the Firefox web browser, and found that there were many end-entity certificates issued by CAs that were had dNSName name constraints but not rfc822Name constraints; i.e. they were taking advantage of the RFC 5280 "all names of types without constraints are allowed." So, I think there are compatibility issues with this approach.
However, I don't think it makes sense to have such a rule, but only applied to unknown types. In particular, I don't think that it makes sense to have such a rule for only new types of names, but not old ones like rfc822Name.
Consider also that every certificate contains a Subject name, equivalent to a subjectAltName of type directoryName. Using the "if a CA certificate contains constraints for some name types, then assume it wants to exclude all names of all types not mentioned in its constraints" logic, either we'd need a special exception for subject names (and probably directoryNames in general), or every CA that has any constraints would need to have directoryName constraints.
Patch Set #2, Line 235: return CertificateInvalidError{c, NameConstraintsWithoutSANs, ""}
I do hope that VerifyHostname can be made to ignore the CN if SANs are present. That would align with Chromium, at least.
I think it already does that. And, IIRC, Chromium requires a subjectAltName and doesn't do the fallback to subject CN any more (unless a flag is set?).
However, if the rule is "ignore the CN if SANs are present" then the name constraint code needs to enforce dNSName constraints on the subject CN if there are no SANs present, to match.
To view, visit change 62693. To unsubscribe, or for help writing mail filters, visit settings.
2 comments:
Patch Set #2, Line 7: “voice of god”
maybe this is a term of art, but a definition or a reference in the commit message might be nice
File src/crypto/x509/name_constraints_test.go:
missing copyright header
To view, visit change 62693. To unsubscribe, or for help writing mail filters, visit settings.
Patch Set 2:
(2 comments)
2 comments:
Patch Set #2, Line 7: “voice of god”
maybe this is a term of art, but a definition or a reference in the commit message might be nice
I think it would be better to just avoid this term. I've only seen it here and in email relating to this PR.
File src/crypto/x509/verify.go:
Patch Set #2, Line 235: return CertificateInvalidError{c, NameConstraintsWithoutSANs, ""}
However, if the rule is "ignore the CN if SANs are present" then the name constraint code needs to enforce dNSName constraints on the subject CN if there are no SANs present, to match.
I think I understand your logic now. In order to safely use VerifyHostname, one has to verify the certificate too. If the certificate chain had name constraints and verification succeeds, then there must be a subjectAltName extension, and VerifyHostname will use the subjectAltName. If there is no subjectAltName extension and verification succeeds, then there were no name constraints in effect.
I think that makes sense, but it is far from obvious, so at least I think the comment should be more explicit about this. Also, I suggest that VerifyHostname should have a comment referring to this comment.
To view, visit change 62693. To unsubscribe, or for help writing mail filters, visit settings.
(Resolving other comments is pending data from the CT logs.)
3 comments:
Patch Set #2, Line 7: “voice of god”
I think it would be better to just avoid this term. […]
Done
in
a leaf certificate
Yes, they can. It would be unusual for them to have a SAN with a dNSName, but there are intermediates that have directoryName and, IIRC, email address, SANs. Besides, directoryName name constraints apply to the subject field, too, regardless of whether there is a SAN extension.
Understood. Agreed that SANs in intermediates should also be limited. Will update the code to do that.
File src/crypto/x509/verify.go:
contains permitted
// name constraints, but leaf certificate contains a name of an
// unsupported or unconstrained type.
RFC 5280 semantics are such that if a name type isn't explicitly constrained, then all names of that type are allowed.
My position is that RFC 5280 is flawed in several respects, and that this is one of them. It would effectively mean giving up on ever adding new name types.
I see two paths from there:
a) accept that the battle is lost and that new name types are impossible in the future. Try to support all the name types currently in use and do not constrain unknown name types.
b) try to implement better semantics that allow new name types in the future.
I'm going for (b). Even if we accept that the battle is lost, (b) has robustness benefits in that future groups may try to add new name types anyway with a disjoint PKI, which people might mistakenly merge into a pool of WebPKI roots.
A good argument for (a) would be that it'll break things and you've helpfully noted that you think that some certificates are already so broken by claiming an rfc822Name without any permitted constraints for it. That's the question for the CT logs and I'm running that query now. I'll update this thread with the results.
> However, I don't think it makes sense to have such a rule, but only applied to unknown types. In particular, I don't think that it makes sense to have such a rule for only new types of names, but not old ones like rfc822Name.
I agree here, although perhaps with an exception for directory names, as noted below.
> Consider also that every certificate contains a Subject name, equivalent to a subjectAltName of type directoryName. Using the "if a CA certificate contains constraints for some name types, then assume it wants to exclude all names of all types not mentioned in its constraints" logic, either we'd need a special exception for subject names (and probably directoryNames in general), or every CA that has any constraints would need to have directoryName constraints.
I do think that making an exception for directoryNames is plausible since certificates cannot avoid having one. If this CL went down the path of rejecting names from constrained intermediates that didn't permit that name type, then either it would have to support directoryName constraints, or else make a special exception for them.
Are you aware of any use for directoryName constraints? That would weight towards supporting them.
To view, visit change 62693. To unsubscribe, or for help writing mail filters, visit settings.
1 comment:
contains permitted
// name constraints, but leaf certificate contains a name of an
// unsupported or unconstrained type.
A good argument for (a) would be that it'll break things and you've helpfully noted that you think that some certificates are already so broken by claiming an rfc822Name without any permitted constraints for it. That's the question for the CT logs and I'm running that query now. I'll update this thread with the results.
I've run a query over the "Pilot" CT log for leaf certificates with an rfc822Name SAN and where any intermediate or root contains a name constraints extension. There are only five results.
Four of the results are within the Federal Bridge PKI, which I'm inclined to dismiss as a well-known case of insanity that should not have been liked to the public Web PKI in any case.
The last case involves an rfc822Name and an otherName which, based on the OID, is a "Microsoft Universal Principal Name". The intermediate contains permitted constraints for both, but since this code wouldn't recognise the otherName it would be blocked. It seems reasonable that we wouldn't verify a name that we don't understand.
I think I'm going to continue on this path and eventually run a A/B test on the Pilot log to see what the expected impact is.
The question of what to do about directoryName constraints is still open. For the moment, I'm going to say that directoryName is ignored as a first step.
To view, visit change 62693. To unsubscribe, or for help writing mail filters, visit settings.
Adam Langley uploaded patch set #3 to this change.
crypto/x509: enforce all name constraints and support IP and (TODO) email constraints
This change makes crypto/x509 enforce name constraints for all names in
leaf and intermediate certificates, not just the name being validated.
Thus, after this change, if a certificate validates then all the names
in it can be trusted – one doesn't have a validate again for each
interesting name.
Making extended key usage work in this fashion still remains to be done.
Change-Id: I72ed5ff2f7284082d5bf3e1e86faf76cef62f9b5
---
A src/crypto/x509/name_constraints_test.go
M src/crypto/x509/verify.go
M src/crypto/x509/x509.go
M src/crypto/x509/x509_test.go
M src/encoding/asn1/asn1.go
5 files changed, 1,429 insertions(+), 43 deletions(-)
To view, visit change 62693. To unsubscribe, or for help writing mail filters, visit settings.
Adam Langley uploaded patch set #4 to this change.
crypto/x509: enforce all name constaints and support IP and email constraints
This change makes crypto/x509 enforce name constraints for all names in
a leaf certificate, not just the name being validated. Thus, after this
change, if a certificate validates then all the names in it can be
trusted – one doesn't have a validate again for each interesting name.
Making extended key usage work in this fashion still remains to be done.
Change-Id: I72ed5ff2f7284082d5bf3e1e86faf76cef62f9b5
---
A src/crypto/x509/name_constraints_test.go
M src/crypto/x509/verify.go
M src/crypto/x509/verify_test.go
M src/crypto/x509/x509.go
M src/crypto/x509/x509_test.go
5 files changed, 1,879 insertions(+), 64 deletions(-)
To view, visit change 62693. To unsubscribe, or for help writing mail filters, visit settings.
Adam Langley uploaded patch set #5 to this change.
crypto/x509: enforce all name constraints and support IP, email and URI constraints
This change makes crypto/x509 enforce name constraints for all names in
a leaf certificate, not just the name being validated. Thus, after this
change, if a certificate validates then all the names in it can be
trusted – one doesn't have a validate again for each interesting name.
Making extended key usage work in this fashion still remains to be done.
Change-Id: I72ed5ff2f7284082d5bf3e1e86faf76cef62f9b5
---
A src/crypto/x509/name_constraints_test.go
M src/crypto/x509/verify.go
M src/crypto/x509/verify_test.go
M src/crypto/x509/x509.go
M src/crypto/x509/x509_test.go
5 files changed, 2,213 insertions(+), 117 deletions(-)
To view, visit change 62693. To unsubscribe, or for help writing mail filters, visit settings.
Adam Langley uploaded patch set #6 to this change.
crypto/x509: enforce all name constraints and support IP, email and URI constraints
This change makes crypto/x509 enforce name constraints for all names in
a leaf certificate, not just the name being validated. Thus, after this
change, if a certificate validates then all the names in it can be
trusted – one doesn't have a validate again for each interesting name.
Making extended key usage work in this fashion still remains to be done.
Change-Id: I72ed5ff2f7284082d5bf3e1e86faf76cef62f9b5
---
A src/crypto/x509/name_constraints_test.go
M src/crypto/x509/verify.go
M src/crypto/x509/verify_test.go
M src/crypto/x509/x509.go
M src/crypto/x509/x509_test.go
M src/go/build/deps_test.go
6 files changed, 2,302 insertions(+), 120 deletions(-)
To view, visit change 62693. To unsubscribe, or for help writing mail filters, visit settings.
Adam Langley uploaded patch set #7 to this change.
crypto/x509: enforce all name constraints and support IP, email and URI constraints
This change makes crypto/x509 enforce name constraints for all names in
a leaf certificate, not just the name being validated. Thus, after this
change, if a certificate validates then all the names in it can be
trusted – one doesn't have a validate again for each interesting name.
Making extended key usage work in this fashion still remains to be done.
Change-Id: I72ed5ff2f7284082d5bf3e1e86faf76cef62f9b5
---
A src/crypto/x509/name_constraints_test.go
M src/crypto/x509/verify.go
M src/crypto/x509/verify_test.go
M src/crypto/x509/x509.go
M src/crypto/x509/x509_test.go
M src/go/build/deps_test.go
6 files changed, 2,329 insertions(+), 125 deletions(-)
To view, visit change 62693. To unsubscribe, or for help writing mail filters, visit settings.
Adam Langley uploaded patch set #8 to this change.
crypto/x509: enforce all name constraints and support IP, email and URI constraints
This change makes crypto/x509 enforce name constraints for all names in
a leaf certificate, not just the name being validated. Thus, after this
change, if a certificate validates then all the names in it can be
trusted – one doesn't have a validate again for each interesting name.
Making extended key usage work in this fashion still remains to be done.
Updates #15196
Change-Id: I72ed5ff2f7284082d5bf3e1e86faf76cef62f9b5
---
A src/crypto/x509/name_constraints_test.go
M src/crypto/x509/verify.go
M src/crypto/x509/verify_test.go
M src/crypto/x509/x509.go
M src/crypto/x509/x509_test.go
M src/go/build/deps_test.go
6 files changed, 2,329 insertions(+), 125 deletions(-)
To view, visit change 62693. To unsubscribe, or for help writing mail filters, visit settings.
rsc: I've had sleevi (our X.509 expert) review the test cases in name_constraints_test.go for sanity. This is 75% of the way to making SPIFFE happy.
Patch set 8:Run-TryBot +1
TryBots beginning. Status page: https://farmer.golang.org/try?commit=7c8cb584
Build is still in progress...
This change failed on windows-386-2008:
See https://storage.googleapis.com/go-build-log/7c8cb584/windows-386-2008_4044ca90.log
Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.
2 of 21 TryBots failed:
Failed on windows-386-2008: https://storage.googleapis.com/go-build-log/7c8cb584/windows-386-2008_4044ca90.log
Failed on windows-amd64-2016: https://storage.googleapis.com/go-build-log/7c8cb584/windows-amd64-2016_94554549.log
Consult https://build.golang.org/ to see whether they are new failures.
Patch set 8:TryBot-Result -1
Adam Langley uploaded patch set #9 to this change.
crypto/x509: enforce all name constraints and support IP, email and URI constraints
This change makes crypto/x509 enforce name constraints for all names in
a leaf certificate, not just the name being validated. Thus, after this
change, if a certificate validates then all the names in it can be
trusted – one doesn't have a validate again for each interesting name.
Making extended key usage work in this fashion still remains to be done.
Updates #15196
Change-Id: I72ed5ff2f7284082d5bf3e1e86faf76cef62f9b5
---
A src/crypto/x509/name_constraints_test.go
M src/crypto/x509/verify.go
M src/crypto/x509/verify_test.go
M src/crypto/x509/x509.go
M src/crypto/x509/x509_test.go
M src/go/build/deps_test.go
6 files changed, 2,328 insertions(+), 124 deletions(-)
To view, visit change 62693. To unsubscribe, or for help writing mail filters, visit settings.
Thanks for this. Looks like I dropped it on the floor for a month, so apologies for that. Reviewing now.
Patch set 9:Code-Review +2
BTW, on #21789 someone is asking for SRVNames to be added to DNSNames, EmailAddresses, and IPAddresses. This CL is adding URIs but not SRVNames. Should SRVNames be added too, if we're checking all constraints?
This looks like great progress. However, from reading (only) the test cases, I think there are some major and minor issues.
10 comments:
File src/crypto/x509/name_constraints_test.go:
Patch Set #9, Line 769: leaf: []string{"email:\"\\f\\o\\o\"@example.com"},
I don't think non-canonical names should be accepted at all, so I would expect this leaf to be rejected. We probably have to accept differences in case, but I don't know of a reason to accept other kinds of non-canonical encoding. X.509 certificates are DER which means generally there's only supposed to be one way of encoding everything and everything should be in its canonical form.
Patch Set #9, Line 854: ok: []string{"email:f...@EXAMPLE.com"},
If not now, then at least in the future we should consider trying to reject non-canonical use of case, like this.
Patch Set #9, Line 884: ok: []string{"dns:example.com."},
I recommend rejecting this name, as mozilla::pkix does: https://github.com/briansmith/mozillapkix/blob/687015fa069d692afaf1868abdcc756b057e748d/lib/pkixnames.cpp#L1996.
In particular, mozilla::pkix allows the reference ID to have the trailing dot, but it doesn't allow presented IDs or name constraints to have the trailing dot. I believe there has never been a compatibility issue with this.
Patch Set #9, Line 900: ok: []string{"email:example.com."},
Ditto, just reject this constraint because it ends in a trailing dot.
Patch Set #9, Line 916: ok: []string{"uri:example.com"},
Is this even a valid URI constraint? A URI constraint is encoded as a GeneralName::uniformResourceIdentifier and thus I expect that the constraint itself is a syntactically-valid URI, including in particular the scheme. Yet, this constraint isn't a valid URI because it is missing the scheme.
Further, despite RFC 5280 saying nothing about matching the schemes, URI name constraints would work terribly poorly if they weren't keyed on the scheme. That is, we should be able to have different `spiffe://example.com` and `https://example.com` and `http://example.com` constraints. In particular, I should be able to permit "https://example.com" and exclude "http://example.com."
Patch Set #9, Line 928: "uri:https://example.com./wibble#bar",
I would also reject both of these hostnames that end in a trailing dot.
Patch Set #9, Line 980: // #56: URIs with IPv6 addresses with ports are also rejected
There should probably also be tests where name constraints like `uri:1.2.3.4` is rejected, since the name constraint must be a FQDN (not an IP address).
Patch Set #9, Line 1028: // #59: URI constraints can allow subdomains
I think there should also be tests for constraint "uri:example.com" where the leaf URIs don't have the scheme://authority/rest form, e.g. "urn:whatever:example.com" or "urn:example.com". I would expect that this would not match the constraint.
Patch Set #9, Line 1044: // address.
I'm not convinced this is a good idea. I'm not sure it's a bad idea either; I like it in principle, but I'm not sure the benefit is worth deviating from RFC 5280.
I wonder if there are constraints that try to disallow all IPv6 addresses (including, perhaps accidentally, the IPv4-mapped range) but which intend to allow some/all IPv4 addresses. That is, I'm not sure this is a compatible extension to RFC 5280.
Patch Set #9, Line 1061: // that address.
Ditto. (Vice versa.)
To view, visit change 62693. To unsubscribe, or for help writing mail filters, visit settings.
Two more comments:
1. In mozilla::pkix we tried to ensure that there was a way to encode the following constraints for each name type: "permit all names of this type" and "exclude all names of this type". For example, for dNSName, we interpreted a completely empty dNSName inside permittedSubtrees to mean "allow all" and we interpreted a completely empty dNSName inside excludedSubtrees to mean "exclude all". IIRC we did the same for rfc822Name. I think it would be good to do the same for uniformResourceIdentifier.
2. I realized after I wrote my previous comment that in the case of rfc822Name (and to a lesser extent, dNSName), it is already the case that a constraint isn't necessarily a valid name of that form, e.g. "rfc822Name:.example.com". Thus, it is reasonable to accept constraints like "url:example.com" and "url:.example.com". However, I think it is still very important to support constraints that constrain the scheme.
Thank you very much for that, that's why I held off on landing this change.
Because of a vacation, it might be a week or two before I can address your points in detail, but that is not an indication that they aren't valued.
On thing that did catch my eye:
I read RFC 5280 as saying that URI constraints were just domain names:
"For URIs, the constraint applies to the host part of the name. The constraint MUST be specified as a fully qualified domain name and MAY specify a host or a domain. Examples would be "host.example.com" and ".example.com"."
Thus is it even valid to put a URL with a scheme in a URI constraint? (Is there any existing practice here?)
(I agree that it would make more sense if URI constraints were URIs themselves, and thus could restrict the scheme.)
Adam Langley uploaded patch set #10 to this change.
crypto/x509: enforce all name constraints and support IP, email and URI constraints
This change makes crypto/x509 enforce name constraints for all names in
a leaf certificate, not just the name being validated. Thus, after this
change, if a certificate validates then all the names in it can be
trusted – one doesn't have a validate again for each interesting name.
Making extended key usage work in this fashion still remains to be done.
Updates #15196
Change-Id: I72ed5ff2f7284082d5bf3e1e86faf76cef62f9b5
---
A src/crypto/x509/name_constraints_test.go
M src/crypto/x509/verify.go
M src/crypto/x509/verify_test.go
M src/crypto/x509/x509.go
M src/crypto/x509/x509_test.go
M src/go/build/deps_test.go
6 files changed, 2,440 insertions(+), 124 deletions(-)
To view, visit change 62693. To unsubscribe, or for help writing mail filters, visit settings.
1. In mozilla::pkix we tried to ensure that there was a way to encode the following constraints for each name type: "permit all names of this type" and "exclude all names of this type". For example, for dNSName, we interpreted a completely empty dNSName inside permittedSubtrees to mean "allow all" and we interpreted a completely empty dNSName inside excludedSubtrees to mean "exclude all". IIRC we did the same for rfc822Name. I think it would be good to do the same for uniformResourceIdentifier.
Agreed. But I'll have to do it in a different change because encoding/asn1 considers empty strings in a CHOICE to be missing.
10 comments:
Patch Set #9, Line 769: leaf: []string{"email:\"\\f\\o\\o\"@example.com"},
I don't think non-canonical names should be accepted at all, so I would expect this leaf to be rejec […]
For the case of email local parts, I have no idea what the canonical form is. Nor, as far as I can tell, do the RFCs. (Note the comments in the code where different RFCs contradict each other.) Since the canonical form is so ambiguous, I don't think that we can enforce it.
As for differences in case (in the local part), I believe that counts as a semantically-relevant difference. ("The local-part of a mailbox MUST BE treated as case sensitive", RFC 2821.)
Patch Set #9, Line 854: ok: []string{"email:f...@EXAMPLE.com"},
If not now, then at least in the future we should consider trying to reject non-canonical use of cas […]
No disagreement with tightening things from me. But I'd want to do this in browsers first: Go generally follows. (I've not checked what browsers are doing in this case, but OpenSSL, at least, does case-insensitive matches on domains.)
Patch Set #9, Line 884: ok: []string{"uri:example.com"},
I recommend rejecting this name, as mozilla::pkix does: https://github. […]
Cool, thanks. Glad to eliminate that option.
Patch Set #9, Line 900: nameConstraintsTest{
Ditto, just reject this constraint because it ends in a trailing dot.
Done
Patch Set #9, Line 916: nameConstraintsTest{
Is this even a valid URI constraint? A URI constraint is encoded as a GeneralName::uniformResourceId […]
See my previous reply, but it really does seem that URI constraints are domain names and don't match the scheme, bizarrely.
Patch Set #9, Line 928: expectedError: "URI with IP",
I would also reject both of these hostnames that end in a trailing dot.
Done
Patch Set #9, Line 980: nameConstraintsTest{
There should probably also be tests where name constraints like `uri:1.2.3. […]
Done. (Since it's a parse error it can't be tested here, but I've added TestBadNamesInConstraints at the bottom of this file.)
Patch Set #9, Line 1028: roots: []constraintsSpec{
I think there should also be tests for constraint "uri:example. […]
Done
Patch Set #9, Line 1044: nameConstraintsTest{
I'm not convinced this is a good idea. […]
Your example of excluding all IPv6 addresses and allowing some IPv4 addresses seems compelling. Thus I've dropped this behaviour.
Ditto. (Vice versa. […]
Ack
To view, visit change 62693. To unsubscribe, or for help writing mail filters, visit settings.
Patch set 10:Run-TryBot +1
TryBots beginning. Status page: https://farmer.golang.org/try?commit=a6e90d51
Build is still in progress...
This change failed on windows-386-2008:
See https://storage.googleapis.com/go-build-log/a6e90d51/windows-386-2008_340c11e5.log
Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.
2 of 21 TryBots failed:
Failed on windows-386-2008: https://storage.googleapis.com/go-build-log/a6e90d51/windows-386-2008_340c11e5.log
Failed on windows-amd64-2016: https://storage.googleapis.com/go-build-log/a6e90d51/windows-amd64-2016_34362c39.log
Consult https://build.golang.org/ to see whether they are new failures.
Patch set 10:TryBot-Result -1
(Since we're now in code freeze, but this was done prior to the freeze, I plan on landing this patch series within a week or two. I'm just holding off in case anyone wants to give it another look since it's a significant amount of sensitive code.)
SGTM re landing it during the freeze. Earlier is better than later.
Patch set 11:Run-TryBot +1
TryBots beginning. Status page: https://farmer.golang.org/try?commit=2fead8cb
Build is still in progress...
This change failed on windows-386-2008:
See https://storage.googleapis.com/go-build-log/2fead8cb/windows-386-2008_71c7e653.log
Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.
2 of 21 TryBots failed:
Failed on windows-386-2008: https://storage.googleapis.com/go-build-log/2fead8cb/windows-386-2008_71c7e653.log
Failed on windows-amd64-2016: https://storage.googleapis.com/go-build-log/2fead8cb/windows-amd64-2016_2075e9db.log
Consult https://build.golang.org/ to see whether they are new failures.
Patch set 11:TryBot-Result -1
Adam Langley uploaded patch set #12 to this change.
crypto/x509: enforce all name constraints and support IP, email and URI constraints
This change makes crypto/x509 enforce name constraints for all names in
a leaf certificate, not just the name being validated. Thus, after this
change, if a certificate validates then all the names in it can be
trusted – one doesn't have a validate again for each interesting name.
Making extended key usage work in this fashion still remains to be done.
Updates #15196
Change-Id: I72ed5ff2f7284082d5bf3e1e86faf76cef62f9b5
---
A src/crypto/x509/name_constraints_test.go
M src/crypto/x509/root_windows.go
M src/crypto/x509/verify.go
M src/crypto/x509/verify_test.go
M src/crypto/x509/x509.go
M src/crypto/x509/x509_test.go
M src/go/build/deps_test.go
7 files changed, 2,442 insertions(+), 126 deletions(-)
To view, visit change 62693. To unsubscribe, or for help writing mail filters, visit settings.
Patch set 12:Run-TryBot +1
TryBots beginning. Status page: https://farmer.golang.org/try?commit=c3137f9d
TryBots are happy.
Patch set 12:TryBot-Result +1
Adam Langley merged this change.
crypto/x509: enforce all name constraints and support IP, email and URI constraints
This change makes crypto/x509 enforce name constraints for all names in
a leaf certificate, not just the name being validated. Thus, after this
change, if a certificate validates then all the names in it can be
trusted – one doesn't have a validate again for each interesting name.
Making extended key usage work in this fashion still remains to be done.
Updates #15196
Change-Id: I72ed5ff2f7284082d5bf3e1e86faf76cef62f9b5
Reviewed-on: https://go-review.googlesource.com/62693
Run-TryBot: Adam Langley <a...@golang.org>
TryBot-Result: Gobot Gobot <go...@golang.org>
Reviewed-by: Russ Cox <r...@golang.org>
---
A src/crypto/x509/name_constraints_test.go
M src/crypto/x509/root_windows.go
M src/crypto/x509/verify.go
M src/crypto/x509/verify_test.go
M src/crypto/x509/x509.go
M src/crypto/x509/x509_test.go
M src/go/build/deps_test.go
7 files changed, 2,442 insertions(+), 126 deletions(-)
diff --git a/src/crypto/x509/name_constraints_test.go b/src/crypto/x509/name_constraints_test.go
new file mode 100644
index 0000000..84e66be
--- /dev/null
+++ b/src/crypto/x509/name_constraints_test.go
@@ -0,0 +1,1569 @@
+// Copyright 2017 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package x509
+
+import (
+ "bytes"
+ "crypto/ecdsa"
+ "crypto/elliptic"
+ "crypto/rand"
+ "crypto/x509/pkix"
+ "encoding/pem"
+ "fmt"
+ "io/ioutil"
+ "math/big"
+ "net"
+ "net/url"
+ "os"
+ "os/exec"
+ "strconv"
+ "strings"
+ "sync"
+ "testing"
+ "time"
+)
+
+const (
+ // testNameConstraintsAgainstOpenSSL can be set to true to run tests
+ // against the system OpenSSL. This is disabled by default because Go
+ // cannot depend on having OpenSSL installed at testing time.
+ testNameConstraintsAgainstOpenSSL = false
+
+ // debugOpenSSLFailure can be set to true, when
+ // testNameConstraintsAgainstOpenSSL is also true, to cause
+ // intermediate files to be preserved for debugging.
+ debugOpenSSLFailure = false
+)
+
+type nameConstraintsTest struct {
+ roots []constraintsSpec
+ intermediates [][]constraintsSpec
+ leaf []string
+ expectedError string
+ noOpenSSL bool
+}
+
+type constraintsSpec struct {
+ ok []string
+ bad []string
+}
+
+var nameConstraintsTests = []nameConstraintsTest{
+ // #0: dummy test for the certificate generation process itself.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{},
+ },
+ leaf: []string{"dns:example.com"},
+ },
+
+ // #1: dummy test for the certificate generation process itself: single
+ // level of intermediate.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{},
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:example.com"},
+ },
+
+ // #2: dummy test for the certificate generation process itself: two
+ // levels of intermediates.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{},
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:example.com"},
+ },
+
+ // #3: matching DNS constraint in root
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:example.com"},
+ },
+
+ // #4: matching DNS constraint in intermediate.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{},
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:example.com"},
+ },
+ },
+ },
+ leaf: []string{"dns:example.com"},
+ },
+
+ // #5: .example.com only matches subdomains.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:.example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:example.com"},
+ expectedError: "\"example.com\" is not permitted",
+ },
+
+ // #6: .example.com matches subdomains.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{},
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:.example.com"},
+ },
+ },
+ },
+ leaf: []string{"dns:foo.example.com"},
+ },
+
+ // #7: .example.com matches multiple levels of subdomains
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:.example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:foo.bar.example.com"},
+ },
+
+ // #8: specifying a permitted list of names does not exclude other name
+ // types
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:.example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"ip:10.1.1.1"},
+ },
+
+ // #9: specifying a permitted list of names does not exclude other name
+ // types
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"ip:10.0.0.0/8"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:example.com"},
+ },
+
+ // #10: intermediates can try to permit other names, which isn't
+ // forbidden if the leaf doesn't mention them. I.e. name constraints
+ // apply to names, not constraints themselves.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:example.com", "dns:foo.com"},
+ },
+ },
+ },
+ leaf: []string{"dns:example.com"},
+ },
+
+ // #11: intermediates cannot add permitted names that the root doesn't
+ // grant them.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:example.com", "dns:foo.com"},
+ },
+ },
+ },
+ leaf: []string{"dns:foo.com"},
+ expectedError: "\"foo.com\" is not permitted",
+ },
+
+ // #12: intermediates can further limit their scope if they wish.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:.example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:.bar.example.com"},
+ },
+ },
+ },
+ leaf: []string{"dns:foo.bar.example.com"},
+ },
+
+ // #13: intermediates can further limit their scope and that limitation
+ // is effective
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:.example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:.bar.example.com"},
+ },
+ },
+ },
+ leaf: []string{"dns:foo.notbar.example.com"},
+ expectedError: "\"foo.notbar.example.com\" is not permitted",
+ },
+
+ // #14: roots can exclude subtrees and that doesn't affect other names.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ bad: []string{"dns:.example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:foo.com"},
+ },
+
+ // #15: roots exclusions are effective.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ bad: []string{"dns:.example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:foo.example.com"},
+ expectedError: "\"foo.example.com\" is excluded",
+ },
+
+ // #16: intermediates can also exclude names and that doesn't affect
+ // other names.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{},
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{
+ bad: []string{"dns:.example.com"},
+ },
+ },
+ },
+ leaf: []string{"dns:foo.com"},
+ },
+
+ // #17: intermediate exclusions are effective.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{},
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{
+ bad: []string{"dns:.example.com"},
+ },
+ },
+ },
+ leaf: []string{"dns:foo.example.com"},
+ expectedError: "\"foo.example.com\" is excluded",
+ },
+
+ // #18: having an exclusion doesn't prohibit other types of names.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ bad: []string{"dns:.example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:foo.com", "ip:10.1.1.1"},
+ },
+
+ // #19: IP-based exclusions are permitted and don't affect unrelated IP
+ // addresses.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ bad: []string{"ip:10.0.0.0/8"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"ip:192.168.1.1"},
+ },
+
+ // #20: IP-based exclusions are effective
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ bad: []string{"ip:10.0.0.0/8"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"ip:10.0.0.1"},
+ expectedError: "\"10.0.0.1\" is excluded",
+ },
+
+ // #21: intermediates can further constrain IP ranges.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ bad: []string{"ip:0.0.0.0/1"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{
+ bad: []string{"ip:11.0.0.0/8"},
+ },
+ },
+ },
+ leaf: []string{"ip:11.0.0.1"},
+ expectedError: "\"11.0.0.1\" is excluded",
+ },
+
+ // #22: when multiple intermediates are present, chain building can
+ // avoid intermediates with incompatible constraints.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{},
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:.foo.com"},
+ },
+ constraintsSpec{
+ ok: []string{"dns:.example.com"},
+ },
+ },
+ },
+ leaf: []string{"dns:foo.example.com"},
+ noOpenSSL: true, // OpenSSL's chain building is not informed by constraints.
+ },
+
+ // #23: (same as the previous test, but in the other order in ensure
+ // that we don't pass it by luck.)
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{},
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:.example.com"},
+ },
+ constraintsSpec{
+ ok: []string{"dns:.foo.com"},
+ },
+ },
+ },
+ leaf: []string{"dns:foo.example.com"},
+ noOpenSSL: true, // OpenSSL's chain building is not informed by constraints.
+ },
+
+ // #24: when multiple roots are valid, chain building can avoid roots
+ // with incompatible constraints.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{},
+ constraintsSpec{
+ ok: []string{"dns:foo.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:example.com"},
+ noOpenSSL: true, // OpenSSL's chain building is not informed by constraints.
+ },
+
+ // #25: (same as the previous test, but in the other order in ensure
+ // that we don't pass it by luck.)
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:foo.com"},
+ },
+ constraintsSpec{},
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:example.com"},
+ noOpenSSL: true, // OpenSSL's chain building is not informed by constraints.
+ },
+
+ // #26: chain building can find a valid path even with multiple levels
+ // of alternative intermediates and alternative roots.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:foo.com"},
+ },
+ constraintsSpec{
+ ok: []string{"dns:example.com"},
+ },
+ constraintsSpec{},
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ constraintsSpec{
+ ok: []string{"dns:foo.com"},
+ },
+ },
+ []constraintsSpec{
+ constraintsSpec{},
+ constraintsSpec{
+ ok: []string{"dns:foo.com"},
+ },
+ },
+ },
+ leaf: []string{"dns:bar.com"},
+ noOpenSSL: true, // OpenSSL's chain building is not informed by constraints.
+ },
+
+ // #27: chain building doesn't get stuck when there is no valid path.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:foo.com"},
+ },
+ constraintsSpec{
+ ok: []string{"dns:example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ constraintsSpec{
+ ok: []string{"dns:foo.com"},
+ },
+ },
+ []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:bar.com"},
+ },
+ constraintsSpec{
+ ok: []string{"dns:foo.com"},
+ },
+ },
+ },
+ leaf: []string{"dns:bar.com"},
+ expectedError: "\"bar.com\" is not permitted",
+ },
+
+ // #28: unknown name types don't cause a problem without constraints.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{},
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"unknown:"},
+ },
+
+ // #29: unknown name types are allowed even in constrained chains.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:foo.com", "dns:.foo.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"unknown:"},
+ },
+
+ // #30: without SANs, a certificate is rejected in a constrained chain.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:foo.com", "dns:.foo.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{},
+ expectedError: "leaf doesn't have a SAN extension",
+ noOpenSSL: true, // OpenSSL doesn't require SANs in this case.
+ },
+
+ // #31: IPv6 addresses work in constraints: roots can permit them as
+ // expected.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"ip:2000:abcd::/32"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"ip:2000:abcd:1234::"},
+ },
+
+ // #32: IPv6 addresses work in constraints: root restrictions are
+ // effective.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"ip:2000:abcd::/32"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"ip:2000:1234:abcd::"},
+ expectedError: "\"2000:1234:abcd::\" is not permitted",
+ },
+
+ // #33: An IPv6 permitted subtree doesn't affect DNS names.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"ip:2000:abcd::/32"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"ip:2000:abcd::", "dns:foo.com"},
+ },
+
+ // #34: IPv6 exclusions don't affect unrelated addresses.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ bad: []string{"ip:2000:abcd::/32"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"ip:2000:1234::"},
+ },
+
+ // #35: IPv6 exclusions are effective.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ bad: []string{"ip:2000:abcd::/32"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"ip:2000:abcd::"},
+ expectedError: "\"2000:abcd::\" is excluded",
+ },
+
+ // #36: IPv6 constraints do not permit IPv4 addresses.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"ip:2000:abcd::/32"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"ip:10.0.0.1"},
+ expectedError: "\"10.0.0.1\" is not permitted",
+ },
+
+ // #37: IPv4 constraints do not permit IPv6 addresses.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"ip:10.0.0.0/8"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"ip:2000:abcd::"},
+ expectedError: "\"2000:abcd::\" is not permitted",
+ },
+
+ // #38: an exclusion of an unknown type doesn't affect other names.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ bad: []string{"unknown:"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:example.com"},
+ },
+
+ // #39: a permitted subtree of an unknown type doesn't affect other
+ // name types.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"unknown:"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:example.com"},
+ },
+
+ // #40: exact email constraints work
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"email:f...@example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"email:f...@example.com"},
+ },
+
+ // #41: exact email constraints are effective
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"email:f...@example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"email:b...@example.com"},
+ expectedError: "\"b...@example.com\" is not permitted",
+ },
+
+ // #42: email canonicalisation works.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"email:f...@example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"email:\"\\f\\o\\o\"@example.com"},
+ noOpenSSL: true, // OpenSSL doesn't canonicalise email addresses before matching
+ },
+
+ // #43: limiting email addresses to a host works.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"email:example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"email:f...@example.com"},
+ },
+
+ // #44: a leading dot matches hosts one level deep
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"email:.example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"email:f...@sub.example.com"},
+ },
+
+ // #45: a leading dot does not match the host itself
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"email:.example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"email:f...@example.com"},
+ expectedError: "\"f...@example.com\" is not permitted",
+ },
+
+ // #46: a leading dot also matches two (or more) levels deep.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"email:.example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"email:f...@sub.sub.example.com"},
+ },
+
+ // #47: the local part of an email is case-sensitive
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"email:f...@example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"email:F...@example.com"},
+ expectedError: "\"F...@example.com\" is not permitted",
+ },
+
+ // #48: the domain part of an email is not case-sensitive
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"email:f...@EXAMPLE.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"email:f...@example.com"},
+ },
+
+ // #49: the domain part of a DNS constraint is also not case-sensitive.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"dns:EXAMPLE.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"dns:example.com"},
+ },
+
+ // #50: URI constraints only cover the host part of the URI
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"uri:example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{
+ "uri:http://example.com/bar",
+ "uri:http://example.com:8080/",
+ "uri:https://example.com/wibble#bar",
+ },
+ },
+
+ // #51: URIs with IPs are rejected
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"uri:example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"uri:http://1.2.3.4/"},
+ expectedError: "URI with IP",
+ },
+
+ // #52: URIs with IPs and ports are rejected
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"uri:example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"uri:http://1.2.3.4:43/"},
+ expectedError: "URI with IP",
+ },
+
+ // #53: URIs with IPv6 addresses are also rejected
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"uri:example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"uri:http://[2006:abcd::1]/"},
+ expectedError: "URI with IP",
+ },
+
+ // #54: URIs with IPv6 addresses with ports are also rejected
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"uri:example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"uri:http://[2006:abcd::1]:16/"},
+ expectedError: "URI with IP",
+ },
+
+ // #55: URI constraints are effective
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"uri:example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"uri:http://bar.com/"},
+ expectedError: "\"http://bar.com/\" is not permitted",
+ },
+
+ // #56: URI constraints are effective
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ bad: []string{"uri:foo.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"uri:http://foo.com/"},
+ expectedError: "\"http://foo.com/\" is excluded",
+ },
+
+ // #57: URI constraints can allow subdomains
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"uri:.foo.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"uri:http://www.foo.com/"},
+ },
+
+ // #58: excluding an IPv4-mapped-IPv6 address doesn't affect the IPv4
+ // version of that address.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ bad: []string{"ip:::ffff:1.2.3.4/128"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"ip:1.2.3.4"},
+ },
+
+ // #59: a URI constraint isn't matched by a URN.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"uri:example.com"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"uri:urn:example"},
+ expectedError: "URI with empty host",
+ },
+
+ // #60: excluding all IPv6 addresses doesn't exclude all IPv4 addresses
+ // too, even though IPv4 is mapped into the IPv6 range.
+ nameConstraintsTest{
+ roots: []constraintsSpec{
+ constraintsSpec{
+ ok: []string{"ip:1.2.3.0/24"},
+ bad: []string{"ip:::0/0"},
+ },
+ },
+ intermediates: [][]constraintsSpec{
+ []constraintsSpec{
+ constraintsSpec{},
+ },
+ },
+ leaf: []string{"ip:1.2.3.4"},
+ },
+
+ // TODO(agl): handle empty name constraints. Currently this doesn't
+ // work because empty values are treated as missing.
+}
+
+func makeConstraintsCACert(constraints constraintsSpec, name string, key *ecdsa.PrivateKey, parent *Certificate, parentKey *ecdsa.PrivateKey) (*Certificate, error) {
+ var serialBytes [16]byte
+ rand.Read(serialBytes[:])
+
+ template := &Certificate{
+ SerialNumber: new(big.Int).SetBytes(serialBytes[:]),
+ Subject: pkix.Name{
+ CommonName: name,
+ },
+ NotBefore: time.Unix(1000, 0),
+ NotAfter: time.Unix(2000, 0),
+ KeyUsage: KeyUsageCertSign,
+ BasicConstraintsValid: true,
+ IsCA: true,
+ }
+
+ if err := addConstraintsToTemplate(constraints, template); err != nil {
+ return nil, err
+ }
+
+ if parent == nil {
+ parent = template
+ }
+ derBytes, err := CreateCertificate(rand.Reader, template, parent, &key.PublicKey, parentKey)
+ if err != nil {
+ return nil, err
+ }
+
+ caCert, err := ParseCertificate(derBytes)
+ if err != nil {
+ return nil, err
+ }
+
+ return caCert, nil
+}
+
+func makeConstraintsLeafCert(sans []string, key *ecdsa.PrivateKey, parent *Certificate, parentKey *ecdsa.PrivateKey) (*Certificate, error) {
+ var serialBytes [16]byte
+ rand.Read(serialBytes[:])
+
+ template := &Certificate{
+ SerialNumber: new(big.Int).SetBytes(serialBytes[:]),
+ Subject: pkix.Name{
+ // Don't set a CommonName because OpenSSL (at least) will try to
+ // match it against name constraints.
+ OrganizationalUnit: []string{"Leaf"},
+ },
+ NotBefore: time.Unix(1000, 0),
+ NotAfter: time.Unix(2000, 0),
+ KeyUsage: KeyUsageDigitalSignature,
+ BasicConstraintsValid: true,
+ IsCA: false,
+ }
+
+ for _, name := range sans {
+ switch {
+ case strings.HasPrefix(name, "dns:"):
+ template.DNSNames = append(template.DNSNames, name[4:])
+
+ case strings.HasPrefix(name, "ip:"):
+ ip := net.ParseIP(name[3:])
+ if ip == nil {
+ return nil, fmt.Errorf("cannot parse IP %q", name[3:])
+ }
+ template.IPAddresses = append(template.IPAddresses, ip)
+
+ case strings.HasPrefix(name, "email:"):
+ template.EmailAddresses = append(template.EmailAddresses, name[6:])
+
+ case strings.HasPrefix(name, "uri:"):
+ uri, err := url.Parse(name[4:])
+ if err != nil {
+ return nil, fmt.Errorf("cannot parse URI %q: %s", name[4:], err)
+ }
+ template.URIs = append(template.URIs, uri)
+
+ case strings.HasPrefix(name, "unknown:"):
+ // This is a special case for testing unknown
+ // name types. A custom SAN extension is
+ // injected into the certificate.
+ if len(sans) != 1 {
+ panic("when using unknown name types, it must be the sole name")
+ }
+
+ template.ExtraExtensions = append(template.ExtraExtensions, pkix.Extension{
+ Id: []int{2, 5, 29, 17},
+ Value: []byte{
+ 0x30, // SEQUENCE
+ 3, // three bytes
+ 9, // undefined GeneralName type 9
+ 1,
+ 1,
+ },
+ })
+
+ default:
+ return nil, fmt.Errorf("unknown name type %q", name)
+ }
+ }
+
+ if parent == nil {
+ parent = template
+ }
+
+ derBytes, err := CreateCertificate(rand.Reader, template, parent, &key.PublicKey, parentKey)
+ if err != nil {
+ return nil, err
+ }
+
+ return ParseCertificate(derBytes)
+}
+
+func customConstraintsExtension(typeNum int, constraint []byte, isExcluded bool) pkix.Extension {
+ appendConstraint := func(contents []byte, tag uint8) []byte {
+ contents = append(contents, tag|32 /* constructed */ |0x80 /* context-specific */)
+ contents = append(contents, byte(4+len(constraint)) /* length */)
+ contents = append(contents, 0x30 /* SEQUENCE */)
+ contents = append(contents, byte(2+len(constraint)) /* length */)
+ contents = append(contents, byte(typeNum) /* GeneralName type */)
+ contents = append(contents, byte(len(constraint)))
+ return append(contents, constraint...)
+ }
+
+ var contents []byte
+ if !isExcluded {
+ contents = appendConstraint(contents, 0 /* tag 0 for permitted */)
+ } else {
+ contents = appendConstraint(contents, 1 /* tag 1 for excluded */)
+ }
+
+ var value []byte
+ value = append(value, 0x30 /* SEQUENCE */)
+ value = append(value, byte(len(contents)))
+ value = append(value, contents...)
+
+ return pkix.Extension{
+ Id: []int{2, 5, 29, 30},
+ Value: value,
+ }
+}
+
+func addConstraintsToTemplate(constraints constraintsSpec, template *Certificate) error {
+ parse := func(constraints []string) (dnsNames []string, ips []*net.IPNet, emailAddrs []string, uriDomains []string, err error) {
+ for _, constraint := range constraints {
+ switch {
+ case strings.HasPrefix(constraint, "dns:"):
+ dnsNames = append(dnsNames, constraint[4:])
+
+ case strings.HasPrefix(constraint, "ip:"):
+ _, ipNet, err := net.ParseCIDR(constraint[3:])
+ if err != nil {
+ return nil, nil, nil, nil, err
+ }
+ ips = append(ips, ipNet)
+
+ case strings.HasPrefix(constraint, "email:"):
+ emailAddrs = append(emailAddrs, constraint[6:])
+
+ case strings.HasPrefix(constraint, "uri:"):
+ uriDomains = append(uriDomains, constraint[4:])
+
+ default:
+ return nil, nil, nil, nil, fmt.Errorf("unknown constraint %q", constraint)
+ }
+ }
+
+ return dnsNames, ips, emailAddrs, uriDomains, err
+ }
+
+ handleSpecialConstraint := func(constraint string, isExcluded bool) bool {
+ switch {
+ case constraint == "unknown:":
+ template.ExtraExtensions = append(template.ExtraExtensions, customConstraintsExtension(9 /* undefined GeneralName type */, []byte{1}, isExcluded))
+
+ default:
+ return false
+ }
+
+ return true
+ }
+
+ if len(constraints.ok) == 1 && len(constraints.bad) == 0 {
+ if handleSpecialConstraint(constraints.ok[0], false) {
+ return nil
+ }
+ }
+
+ if len(constraints.bad) == 1 && len(constraints.ok) == 0 {
+ if handleSpecialConstraint(constraints.bad[0], true) {
+ return nil
+ }
+ }
+
+ var err error
+ template.PermittedDNSDomains, template.PermittedIPRanges, template.PermittedEmailAddresses, template.PermittedURIDomains, err = parse(constraints.ok)
+ if err != nil {
+ return err
+ }
+
+ template.ExcludedDNSDomains, template.ExcludedIPRanges, template.ExcludedEmailAddresses, template.ExcludedURIDomains, err = parse(constraints.bad)
+ if err != nil {
+ return err
+ }
+
+ return nil
+}
+
+func TestNameConstraintCases(t *testing.T) {
+ privateKeys := sync.Pool{
+ New: func() interface{} {
+ priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+ if err != nil {
+ panic(err)
+ }
+ return priv
+ },
+ }
+
+ for i, test := range nameConstraintsTests {
+ rootPool := NewCertPool()
+ rootKey := privateKeys.Get().(*ecdsa.PrivateKey)
+ rootName := "Root " + strconv.Itoa(i)
+
+ // keys keeps track of all the private keys used in a given
+ // test and puts them back in the privateKeys pool at the end.
+ keys := []*ecdsa.PrivateKey{rootKey}
+
+ // At each level (root, intermediate(s), leaf), parent points to
+ // an example parent certificate and parentKey the key for the
+ // parent level. Since all certificates at a given level have
+ // the same name and public key, any parent certificate is
+ // sufficient to get the correct issuer name and authority
+ // key ID.
+ var parent *Certificate
+ parentKey := rootKey
+
+ for _, root := range test.roots {
+ rootCert, err := makeConstraintsCACert(root, rootName, rootKey, nil, rootKey)
+ if err != nil {
+ t.Fatalf("#%d: failed to create root: %s", i, err)
+ }
+
+ parent = rootCert
+ rootPool.AddCert(rootCert)
+ }
+
+ intermediatePool := NewCertPool()
+
+ for level, intermediates := range test.intermediates {
+ levelKey := privateKeys.Get().(*ecdsa.PrivateKey)
+ keys = append(keys, levelKey)
+ levelName := "Intermediate level " + strconv.Itoa(level)
+ var last *Certificate
+
+ for _, intermediate := range intermediates {
+ caCert, err := makeConstraintsCACert(intermediate, levelName, levelKey, parent, parentKey)
+ if err != nil {
+ t.Fatalf("#%d: failed to create %q: %s", i, levelName, err)
+ }
+
+ last = caCert
+ intermediatePool.AddCert(caCert)
+ }
+
+ parent = last
+ parentKey = levelKey
+ }
+
+ leafKey := privateKeys.Get().(*ecdsa.PrivateKey)
+ keys = append(keys, leafKey)
+
+ leafCert, err := makeConstraintsLeafCert(test.leaf, leafKey, parent, parentKey)
+ if err != nil {
+ t.Fatalf("#%d: cannot create leaf: %s", i, err)
+ }
+
+ if !test.noOpenSSL && testNameConstraintsAgainstOpenSSL {
+ output, err := testChainAgainstOpenSSL(leafCert, intermediatePool, rootPool)
+ if err == nil && len(test.expectedError) > 0 {
+ t.Errorf("#%d: unexpectedly succeeded against OpenSSL", i)
+ if debugOpenSSLFailure {
+ return
+ }
+ }
+
+ if err != nil {
+ if _, ok := err.(*exec.ExitError); !ok {
+ t.Errorf("#%d: OpenSSL failed to run: %s", i, err)
+ } else if len(test.expectedError) == 0 {
+ t.Errorf("#%d: OpenSSL unexpectedly failed: %q", i, output)
+ if debugOpenSSLFailure {
+ return
+ }
+ }
+ }
+ }
+
+ verifyOpts := VerifyOptions{
+ Roots: rootPool,
+ Intermediates: intermediatePool,
+ CurrentTime: time.Unix(1500, 0),
+ }
+ _, err = leafCert.Verify(verifyOpts)
+
+ logInfo := true
+ if len(test.expectedError) == 0 {
+ if err != nil {
+ t.Errorf("#%d: unexpected failure: %s", i, err)
+ } else {
+ logInfo = false
+ }
+ } else {
+ if err == nil {
+ t.Errorf("#%d: unexpected success", i)
+ } else if !strings.Contains(err.Error(), test.expectedError) {
+ t.Errorf("#%d: expected error containing %q, but got: %s", i, test.expectedError, err)
+ } else {
+ logInfo = false
+ }
+ }
+
+ if logInfo {
+ certAsPEM := func(cert *Certificate) string {
+ var buf bytes.Buffer
+ pem.Encode(&buf, &pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw})
+ return string(buf.Bytes())
+ }
+ t.Errorf("#%d: root:\n%s", i, certAsPEM(rootPool.certs[0]))
+ t.Errorf("#%d: leaf:\n%s", i, certAsPEM(leafCert))
+ }
+
+ for _, key := range keys {
+ privateKeys.Put(key)
+ }
+ keys = keys[:0]
+ }
+}
+
+func writePEMsToTempFile(certs []*Certificate) *os.File {
+ file, err := ioutil.TempFile("", "name_constraints_test")
+ if err != nil {
+ panic("cannot create tempfile")
+ }
+
+ pemBlock := &pem.Block{Type: "CERTIFICATE"}
+ for _, cert := range certs {
+ pemBlock.Bytes = cert.Raw
+ pem.Encode(file, pemBlock)
+ }
+
+ return file
+}
+
+func testChainAgainstOpenSSL(leaf *Certificate, intermediates, roots *CertPool) (string, error) {
+ args := []string{"verify", "-no_check_time"}
+
+ rootsFile := writePEMsToTempFile(roots.certs)
+ if debugOpenSSLFailure {
+ println("roots file:", rootsFile.Name())
+ } else {
+ defer os.Remove(rootsFile.Name())
+ }
+ args = append(args, "-CAfile", rootsFile.Name())
+
+ if len(intermediates.certs) > 0 {
+ intermediatesFile := writePEMsToTempFile(intermediates.certs)
+ if debugOpenSSLFailure {
+ println("intermediates file:", intermediatesFile.Name())
+ } else {
+ defer os.Remove(intermediatesFile.Name())
+ }
+ args = append(args, "-untrusted", intermediatesFile.Name())
+ }
+
+ leafFile := writePEMsToTempFile([]*Certificate{leaf})
+ if debugOpenSSLFailure {
+ println("leaf file:", leafFile.Name())
+ } else {
+ defer os.Remove(leafFile.Name())
+ }
+ args = append(args, leafFile.Name())
+
+ var output bytes.Buffer
+ cmd := exec.Command("openssl", args...)
+ cmd.Stdout = &output
+ cmd.Stderr = &output
+
+ err := cmd.Run()
+ return string(output.Bytes()), err
+}
+
+var rfc2821Tests = []struct {
+ in string
+ localPart, domain string
+}{
+ {"f...@example.com", "foo", "example.com"},
+ {"@example.com", "", ""},
+ {"\"@example.com", "", ""},
+ {"\"\"@example.com", "", "example.com"},
+ {"\"a\"@example.com", "a", "example.com"},
+ {"\"\\a\"@example.com", "a", "example.com"},
+ {"a\"@example.com", "", ""},
+ {"foo..bar@example.com", "", ""},
+ {".foo...@example.com", "", ""},
+ {"foo.bar.@example.com", "", ""},
+ {"|{}?'@example.com", "|{}?'", "example.com"},
+
+ // Examples from RFC 3696
+ {"Abc\\@d...@example.com", "Abc@def", "example.com"},
+ {"Fred\\ Blo...@example.com", "Fred Bloggs", "example.com"},
+ {"Joe.\\\\Bl...@example.com", "Joe.\\Blow", "example.com"},
+ {"\"Abc@def\"@example.com", "Abc@def", "example.com"},
+ {"\"Fred Bloggs\"@example.com", "Fred Bloggs", "example.com"},
+ {"customer/department=ship...@example.com", "customer/department=shipping", "example.com"},
+ {"$A12...@example.com", "$A12345", "example.com"},
+ {"!def!xyz%a...@example.com", "!def!xyz%abc", "example.com"},
+ {"_some...@example.com", "_somename", "example.com"},
+}
+
+func TestRFC2821Parsing(t *testing.T) {
+ for i, test := range rfc2821Tests {
+ mailbox, ok := parseRFC2821Mailbox(test.in)
+ expectedFailure := len(test.localPart) == 0 && len(test.domain) == 0
+
+ if ok && expectedFailure {
+ t.Errorf("#%d: %q unexpectedly parsed as (%q, %q)", i, test.in, mailbox.local, mailbox.domain)
+ continue
+ }
+
+ if !ok && !expectedFailure {
+ t.Errorf("#%d: unexpected failure for %q", i, test.in)
+ continue
+ }
+
+ if !ok {
+ continue
+ }
+
+ if mailbox.local != test.localPart || mailbox.domain != test.domain {
+ t.Errorf("#%d: %q parsed as (%q, %q), but wanted (%q, %q)", i, test.in, mailbox.local, mailbox.domain, test.localPart, test.domain)
+ }
+ }
+}
+
+func TestBadNamesInConstraints(t *testing.T) {
+ // Bad names in constraints should not parse.
+ badNames := []string{
+ "dns:foo.com.",
+ "email:a...@foo.com.",
+ "email:foo.com.",
+ "uri:example.com.",
+ "uri:1.2.3.4",
+ "uri:ffff::1",
+ }
+
+ priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+ if err != nil {
+ panic(err)
+ }
+
+ for _, badName := range badNames {
+ _, err := makeConstraintsCACert(constraintsSpec{
+ ok: []string{badName},
+ }, "TestAbsoluteNamesInConstraints", priv, nil, priv)
+
+ if err == nil {
+ t.Errorf("bad name %q unexpectedly accepted in name constraint", badName)
+ continue
+ }
+
+ if err != nil {
+ if str := err.Error(); !strings.Contains(str, "failed to parse ") && !strings.Contains(str, "constraint") {
+ t.Errorf("bad name %q triggered unrecognised error: %s", badName, str)
+ }
+ }
+ }
+}
+
+func TestBadNamesInSANs(t *testing.T) {
+ // Bad names in SANs should not parse.
+ badNames := []string{
+ "dns:foo.com.",
+ "email:a...@foo.com.",
+ "email:foo.com.",
+ "uri:https://example.com./dsf",
+ }
+
+ priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+ if err != nil {
+ panic(err)
+ }
+
+ for _, badName := range badNames {
+ _, err := makeConstraintsLeafCert([]string{badName}, priv, nil, priv)
+
+ if err == nil {
+ t.Errorf("bad name %q unexpectedly accepted in SAN", badName)
+ continue
+ }
+
+ if err != nil {
+ if str := err.Error(); !strings.Contains(str, "cannot parse ") {
+ t.Errorf("bad name %q triggered unrecognised error: %s", badName, str)
+ }
+ }
+ }
+}
diff --git a/src/crypto/x509/root_windows.go b/src/crypto/x509/root_windows.go
index a936fec..92cc716 100644
--- a/src/crypto/x509/root_windows.go
+++ b/src/crypto/x509/root_windows.go
@@ -87,7 +87,7 @@
status := chainCtx.TrustStatus.ErrorStatus
switch status {
case syscall.CERT_TRUST_IS_NOT_TIME_VALID:
- return CertificateInvalidError{c, Expired}
+ return CertificateInvalidError{c, Expired, ""}
default:
return UnknownAuthorityError{c, nil, nil}
}
@@ -125,7 +125,7 @@
if status.Error != 0 {
switch status.Error {
case syscall.CERT_E_EXPIRED:
- return CertificateInvalidError{c, Expired}
+ return CertificateInvalidError{c, Expired, ""}
case syscall.CERT_E_CN_NO_MATCH:
return HostnameError{c, opts.DNSName}
case syscall.CERT_E_UNTRUSTEDROOT:
diff --git a/src/crypto/x509/verify.go b/src/crypto/x509/verify.go
index 1193a26..bbc4ad8 100644
--- a/src/crypto/x509/verify.go
+++ b/src/crypto/x509/verify.go
@@ -9,6 +9,8 @@
"errors"
"fmt"
"net"
+ "net/url"
+ "reflect"
"runtime"
"strings"
"time"
@@ -25,8 +27,8 @@
// given in the VerifyOptions.
Expired
// CANotAuthorizedForThisName results when an intermediate or root
- // certificate has a name constraint which doesn't include the name
- // being checked.
+ // certificate has a name constraint which doesn't permit a DNS or
+ // other name (including IP address) in the leaf certificate.
CANotAuthorizedForThisName
// TooManyIntermediates results when a path length constraint is
// violated.
@@ -37,6 +39,20 @@
// NameMismatch results when the subject name of a parent certificate
// does not match the issuer name in the child.
NameMismatch
+ // NameConstraintsWithoutSANs results when a leaf certificate doesn't
+ // contain a Subject Alternative Name extension, but a CA certificate
+ // contains name constraints.
+ NameConstraintsWithoutSANs
+ // UnconstrainedName results when a CA certificate contains permitted
+ // name constraints, but leaf certificate contains a name of an
+ // unsupported or unconstrained type.
+ UnconstrainedName
+ // TooManyConstraints results when the number of comparision operations
+ // needed to check a certificate exceeds the limit set by
+ // VerifyOptions.MaxConstraintComparisions. This limit exists to
+ // prevent pathological certificates can consuming excessive amounts of
+ // CPU time to verify.
+ TooManyConstraints
)
// CertificateInvalidError results when an odd error occurs. Users of this
@@ -44,6 +60,7 @@
type CertificateInvalidError struct {
Cert *Certificate
Reason InvalidReason
+ Detail string
}
func (e CertificateInvalidError) Error() string {
@@ -53,13 +70,17 @@
case Expired:
return "x509: certificate has expired or is not yet valid"
case CANotAuthorizedForThisName:
- return "x509: a root or intermediate certificate is not authorized to sign in this domain"
+ return "x509: a root or intermediate certificate is not authorized to sign for this name: " + e.Detail
case TooManyIntermediates:
return "x509: too many intermediates for path length constraint"
case IncompatibleUsage:
return "x509: certificate specifies an incompatible key usage"
case NameMismatch:
return "x509: issuer name does not match subject from issuing certificate"
+ case NameConstraintsWithoutSANs:
+ return "x509: issuer has name constraints but leaf doesn't have a SAN extension"
+ case UnconstrainedName:
+ return "x509: issuer has name constraints but leaf contains unknown or unconstrained name: " + e.Detail
}
return "x509: unknown error"
}
@@ -156,6 +177,12 @@
// constraint down the chain which mirrors Windows CryptoAPI behavior,
// but not the spec. To accept any key usage, include ExtKeyUsageAny.
KeyUsages []ExtKeyUsage
+ // MaxConstraintComparisions is the maximum number of comparisons to
+ // perform when checking a given certificate's name constraints. If
+ // zero, a sensible default is used. This limit prevents pathalogical
+ // certificates from consuming excessive amounts of CPU time when
+ // validating.
+ MaxConstraintComparisions int
}
const (
@@ -164,32 +191,354 @@
rootCertificate
)
-func matchNameConstraint(domain, constraint string) bool {
+// rfc2821Mailbox represents a “mailbox” (which is an email address to most
+// people) by breaking it into the “local” (i.e. before the '@') and “domain”
+// parts.
+type rfc2821Mailbox struct {
+ local, domain string
+}
+
+// parseRFC2821Mailbox parses an email address into local and domain parts,
+// based on the ABNF for a “Mailbox” from RFC 2821. According to
+// https://tools.ietf.org/html/rfc5280#section-4.2.1.6 that's correct for an
+// rfc822Name from a certificate: “The format of an rfc822Name is a "Mailbox"
+// as defined in https://tools.ietf.org/html/rfc2821#section-4.1.2”.
+func parseRFC2821Mailbox(in string) (mailbox rfc2821Mailbox, ok bool) {
+ if len(in) == 0 {
+ return mailbox, false
+ }
+
+ localPartBytes := make([]byte, 0, len(in)/2)
+
+ if in[0] == '"' {
+ // Quoted-string = DQUOTE *qcontent DQUOTE
+ // non-whitespace-control = %d1-8 / %d11 / %d12 / %d14-31 / %d127
+ // qcontent = qtext / quoted-pair
+ // qtext = non-whitespace-control /
+ // %d33 / %d35-91 / %d93-126
+ // quoted-pair = ("\" text) / obs-qp
+ // text = %d1-9 / %d11 / %d12 / %d14-127 / obs-text
+ //
+ // (Names beginning with “obs-” are the obsolete syntax from
+ // https://tools.ietf.org/html/rfc2822#section-4. Since it has
+ // been 16 years, we no longer accept that.)
+ in = in[1:]
+ QuotedString:
+ for {
+ if len(in) == 0 {
+ return mailbox, false
+ }
+ c := in[0]
+ in = in[1:]
+
+ switch {
+ case c == '"':
+ break QuotedString
+
+ case c == '\\':
+ // quoted-pair
+ if len(in) == 0 {
+ return mailbox, false
+ }
+ if in[0] == 11 ||
+ in[0] == 12 ||
+ (1 <= in[0] && in[0] <= 9) ||
+ (14 <= in[0] && in[0] <= 127) {
+ localPartBytes = append(localPartBytes, in[0])
+ in = in[1:]
+ } else {
+ return mailbox, false
+ }
+
+ case c == 11 ||
+ c == 12 ||
+ // Space (char 32) is not allowed based on the
+ // BNF, but RFC 3696 gives an example that
+ // assumes that it is. Several “verified”
+ // errata continue to argue about this point.
+ // We choose to accept it.
+ c == 32 ||
+ c == 33 ||
+ c == 127 ||
+ (1 <= c && c <= 8) ||
+ (14 <= c && c <= 31) ||
+ (35 <= c && c <= 91) ||
+ (93 <= c && c <= 126):
+ // qtext
+ localPartBytes = append(localPartBytes, c)
+
+ default:
+ return mailbox, false
+ }
+ }
+ } else {
+ // Atom ("." Atom)*
+ NextChar:
+ for len(in) > 0 {
+ // atext from https://tools.ietf.org/html/rfc2822#section-3.2.4
+ c := in[0]
+
+ switch {
+ case c == '\\':
+ // Examples given in RFC 3696 suggest that
+ // escaped characters can appear outside of a
+ // quoted string. Several “verified” errata
+ // continue to argue the point. We choose to
+ // accept it.
+ in = in[1:]
+ if len(in) == 0 {
+ return mailbox, false
+ }
+ fallthrough
+
+ case ('0' <= c && c <= '9') ||
+ ('a' <= c && c <= 'z') ||
+ ('A' <= c && c <= 'Z') ||
+ c == '!' || c == '#' || c == '$' || c == '%' ||
+ c == '&' || c == '\'' || c == '*' || c == '+' ||
+ c == '-' || c == '/' || c == '=' || c == '?' ||
+ c == '^' || c == '_' || c == '`' || c == '{' ||
+ c == '|' || c == '}' || c == '~' || c == '.':
+ localPartBytes = append(localPartBytes, in[0])
+ in = in[1:]
+
+ default:
+ break NextChar
+ }
+ }
+
+ if len(localPartBytes) == 0 {
+ return mailbox, false
+ }
+
+ // https://tools.ietf.org/html/rfc3696#section-3
+ // “period (".") may also appear, but may not be used to start
+ // or end the local part, nor may two or more consecutive
+ // periods appear.”
+ twoDots := []byte{'.', '.'}
+ if localPartBytes[0] == '.' ||
+ localPartBytes[len(localPartBytes)-1] == '.' ||
+ bytes.Contains(localPartBytes, twoDots) {
+ return mailbox, false
+ }
+ }
+
+ if len(in) == 0 || in[0] != '@' {
+ return mailbox, false
+ }
+ in = in[1:]
+
+ // The RFC species a format for domains, but that's known to be
+ // violated in practice so we accept that anything after an '@' is the
+ // domain part.
+ if _, ok := domainToReverseLabels(in); !ok {
+ return mailbox, false
+ }
+
+ mailbox.local = string(localPartBytes)
+ mailbox.domain = in
+ return mailbox, true
+}
+
+// domainToReverseLabels converts a textual domain name like foo.example.com to
+// the list of labels in reverse order, e.g. ["com", "example", "foo"].
+func domainToReverseLabels(domain string) (reverseLabels []string, ok bool) {
+ for len(domain) > 0 {
+ if i := strings.LastIndexByte(domain, '.'); i == -1 {
+ reverseLabels = append(reverseLabels, domain)
+ domain = ""
+ } else {
+ reverseLabels = append(reverseLabels, domain[i+1:len(domain)])
+ domain = domain[:i]
+ }
+ }
+
+ if len(reverseLabels) > 0 && len(reverseLabels[0]) == 0 {
+ // An empty label at the end indicates an absolute value.
+ return nil, false
+ }
+
+ for _, label := range reverseLabels {
+ if len(label) == 0 {
+ // Empty labels are otherwise invalid.
+ return nil, false
+ }
+
+ for _, c := range label {
+ if c < 33 || c > 126 {
+ // Invalid character.
+ return nil, false
+ }
+ }
+ }
+
+ return reverseLabels, true
+}
+
+func matchEmailConstraint(mailbox rfc2821Mailbox, constraint string) (bool, error) {
+ // If the constraint contains an @, then it specifies an exact mailbox
+ // name.
+ if strings.Contains(constraint, "@") {
+ constraintMailbox, ok := parseRFC2821Mailbox(constraint)
+ if !ok {
+ return false, fmt.Errorf("x509: internal error: cannot parse constraint %q", constraint)
+ }
+ return mailbox.local == constraintMailbox.local && strings.EqualFold(mailbox.domain, constraintMailbox.domain), nil
+ }
+
+ // Otherwise the constraint is like a DNS constraint of the domain part
+ // of the mailbox.
+ return matchDomainConstraint(mailbox.domain, constraint)
+}
+
+func matchURIConstraint(uri *url.URL, constraint string) (bool, error) {
+ // https://tools.ietf.org/html/rfc5280#section-4.2.1.10
+ // “a uniformResourceIdentifier that does not include an authority
+ // component with a host name specified as a fully qualified domain
+ // name (e.g., if the URI either does not include an authority
+ // component or includes an authority component in which the host name
+ // is specified as an IP address), then the application MUST reject the
+ // certificate.”
+
+ host := uri.Host
+ if len(host) == 0 {
+ return false, fmt.Errorf("URI with empty host (%q) cannot be matched against constraints", uri.String())
+ }
+
+ if strings.Contains(host, ":") && !strings.HasSuffix(host, "]") {
+ var err error
+ host, _, err = net.SplitHostPort(uri.Host)
+ if err != nil {
+ return false, err
+ }
+ }
+
+ if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") ||
+ net.ParseIP(host) != nil {
+ return false, fmt.Errorf("URI with IP (%q) cannot be matched against constraints", uri.String())
+ }
+
+ return matchDomainConstraint(host, constraint)
+}
+
+func matchIPConstraint(ip net.IP, constraint *net.IPNet) (bool, error) {
+ if len(ip) != len(constraint.IP) {
+ return false, nil
+ }
+
+ for i := range ip {
+ if mask := constraint.Mask[i]; ip[i]&mask != constraint.IP[i]&mask {
+ return false, nil
+ }
+ }
+
+ return true, nil
+}
+
+func matchDomainConstraint(domain, constraint string) (bool, error) {
// The meaning of zero length constraints is not specified, but this
// code follows NSS and accepts them as matching everything.
if len(constraint) == 0 {
- return true
+ return true, nil
}
- if len(domain) < len(constraint) {
- return false
+ domainLabels, ok := domainToReverseLabels(domain)
+ if !ok {
+ return false, fmt.Errorf("x509: internal error: cannot parse domain %q", domain)
}
- prefixLen := len(domain) - len(constraint)
- if !strings.EqualFold(domain[prefixLen:], constraint) {
- return false
+ // RFC 5280 says that a leading period in a domain name means that at
+ // least one label must be prepended, but only for URI and email
+ // constraints, not DNS constraints. The code also supports that
+ // behaviour for DNS constraints.
+
+ mustHaveSubdomains := false
+ if constraint[0] == '.' {
+ mustHaveSubdomains = true
+ constraint = constraint[1:]
}
- if prefixLen == 0 {
- return true
+ constraintLabels, ok := domainToReverseLabels(constraint)
+ if !ok {
+ return false, fmt.Errorf("x509: internal error: cannot parse domain %q", constraint)
}
- isSubdomain := domain[prefixLen-1] == '.'
- constraintHasLeadingDot := constraint[0] == '.'
- return isSubdomain != constraintHasLeadingDot
+ if len(domainLabels) < len(constraintLabels) ||
+ (mustHaveSubdomains && len(domainLabels) == len(constraintLabels)) {
+ return false, nil
+ }
+
+ for i, constraintLabel := range constraintLabels {
+ if !strings.EqualFold(constraintLabel, domainLabels[i]) {
+ return false, nil
+ }
+ }
+
+ return true, nil
}
-// isValid performs validity checks on the c.
+// checkNameConstraints checks that c permits a child certificate to claim the
+// given name, of type nameType. The argument parsedName contains the parsed
+// form of name, suitable for passing to the match function. The total number
+// of comparisons is tracked in the given count and should not exceed the given
+// limit.
+func (c *Certificate) checkNameConstraints(count *int,
+ maxConstraintComparisons int,
+ nameType string,
+ name string,
+ parsedName interface{},
+ match func(parsedName, constraint interface{}) (match bool, err error),
+ permitted, excluded interface{}) error {
+
+ excludedValue := reflect.ValueOf(excluded)
+
+ *count += excludedValue.Len()
+ if *count > maxConstraintComparisons {
+ return CertificateInvalidError{c, TooManyConstraints, ""}
+ }
+
+ for i := 0; i < excludedValue.Len(); i++ {
+ constraint := excludedValue.Index(i).Interface()
+ match, err := match(parsedName, constraint)
+ if err != nil {
+ return CertificateInvalidError{c, CANotAuthorizedForThisName, err.Error()}
+ }
+
+ if match {
+ return CertificateInvalidError{c, CANotAuthorizedForThisName, fmt.Sprintf("%s %q is excluded by constraint %q", nameType, name, constraint)}
+ }
+ }
+
+ permittedValue := reflect.ValueOf(permitted)
+
+ *count += permittedValue.Len()
+ if *count > maxConstraintComparisons {
+ return CertificateInvalidError{c, TooManyConstraints, ""}
+ }
+
+ ok := true
+ for i := 0; i < permittedValue.Len(); i++ {
+ constraint := permittedValue.Index(i).Interface()
+
+ var err error
+ if ok, err = match(parsedName, constraint); err != nil {
+ return CertificateInvalidError{c, CANotAuthorizedForThisName, err.Error()}
+ }
+
+ if ok {
+ break
+ }
+ }
+
+ if !ok {
+ return CertificateInvalidError{c, CANotAuthorizedForThisName, fmt.Sprintf("%s %q is not permitted by any constraint", nameType, name)}
+ }
+
+ return nil
+}
+
+// isValid performs validity checks on c given that it is a candidate to append
+// to the chain in currentChain.
func (c *Certificate) isValid(certType int, currentChain []*Certificate, opts *VerifyOptions) error {
if len(c.UnhandledCriticalExtensions) > 0 {
return UnhandledCriticalExtension{}
@@ -198,7 +547,7 @@
if len(currentChain) > 0 {
child := currentChain[len(currentChain)-1]
if !bytes.Equal(child.RawIssuer, c.RawSubject) {
- return CertificateInvalidError{c, NameMismatch}
+ return CertificateInvalidError{c, NameMismatch, ""}
}
}
@@ -207,26 +556,92 @@
now = time.Now()
}
if now.Before(c.NotBefore) || now.After(c.NotAfter) {
- return CertificateInvalidError{c, Expired}
+ return CertificateInvalidError{c, Expired, ""}
}
- if len(c.PermittedDNSDomains) > 0 {
- ok := false
- for _, constraint := range c.PermittedDNSDomains {
- ok = matchNameConstraint(opts.DNSName, constraint)
- if ok {
- break
- }
+ if (certType == intermediateCertificate || certType == rootCertificate) && c.hasNameConstraints() {
+ maxConstraintComparisons := opts.MaxConstraintComparisions
+ if maxConstraintComparisons == 0 {
+ maxConstraintComparisons = 250000
}
+ count := 0
+ if len(currentChain) == 0 {
+ return errors.New("x509: internal error: empty chain when appending CA cert")
+ }
+ leaf := currentChain[0]
+
+ sanExtension, ok := leaf.getSANExtension()
if !ok {
- return CertificateInvalidError{c, CANotAuthorizedForThisName}
+ // This is the deprecated, legacy case of depending on
+ // the CN as a hostname. Chains modern enough to be
+ // using name constraints should not be depending on
+ // CNs.
+ return CertificateInvalidError{c, NameConstraintsWithoutSANs, ""}
}
- }
- for _, constraint := range c.ExcludedDNSDomains {
- if matchNameConstraint(opts.DNSName, constraint) {
- return CertificateInvalidError{c, CANotAuthorizedForThisName}
+ err := forEachSAN(sanExtension, func(tag int, data []byte) error {
+ switch tag {
+ case nameTypeEmail:
+ name := string(data)
+ mailbox, ok := parseRFC2821Mailbox(name)
+ if !ok {
+ // This certificate should not have parsed.
+ return errors.New("x509: internal error: rfc822Name SAN failed to parse")
+ }
+
+ if err := c.checkNameConstraints(&count, maxConstraintComparisons, "email address", name, mailbox,
+ func(parsedName, constraint interface{}) (bool, error) {
+ return matchEmailConstraint(parsedName.(rfc2821Mailbox), constraint.(string))
+ }, c.PermittedEmailAddresses, c.ExcludedEmailAddresses); err != nil {
+ return err
+ }
+
+ case nameTypeDNS:
+ name := string(data)
+ if err := c.checkNameConstraints(&count, maxConstraintComparisons, "DNS name", name, name,
+ func(parsedName, constraint interface{}) (bool, error) {
+ return matchDomainConstraint(parsedName.(string), constraint.(string))
+ }, c.PermittedDNSDomains, c.ExcludedDNSDomains); err != nil {
+ return err
+ }
+
+ case nameTypeURI:
+ name := string(data)
+ uri, err := url.Parse(name)
+ if err != nil {
+ return fmt.Errorf("x509: internal error: URI SAN %q failed to parse", name)
+ }
+
+ if err := c.checkNameConstraints(&count, maxConstraintComparisons, "URI", name, uri,
+ func(parsedName, constraint interface{}) (bool, error) {
+ return matchURIConstraint(parsedName.(*url.URL), constraint.(string))
+ }, c.PermittedURIDomains, c.ExcludedURIDomains); err != nil {
+ return err
+ }
+
+ case nameTypeIP:
+ ip := net.IP(data)
+ if l := len(ip); l != net.IPv4len && l != net.IPv6len {
+ return fmt.Errorf("x509: internal error: IP SAN %x failed to parse", data)
+ }
+
+ if err := c.checkNameConstraints(&count, maxConstraintComparisons, "IP address", ip.String(), ip,
+ func(parsedName, constraint interface{}) (bool, error) {
+ return matchIPConstraint(parsedName.(net.IP), constraint.(*net.IPNet))
+ }, c.PermittedIPRanges, c.ExcludedIPRanges); err != nil {
+ return err
+ }
+
+ default:
+ // Unknown SAN types are ignored.
+ }
+
+ return nil
+ })
+
+ if err != nil {
+ return err
}
}
@@ -248,13 +663,13 @@
// encryption key could only be used for Diffie-Hellman key agreement.
if certType == intermediateCertificate && (!c.BasicConstraintsValid || !c.IsCA) {
- return CertificateInvalidError{c, NotAuthorizedToSign}
+ return CertificateInvalidError{c, NotAuthorizedToSign, ""}
}
if c.BasicConstraintsValid && c.MaxPathLen >= 0 {
numIntermediates := len(currentChain) - 1
if numIntermediates > c.MaxPathLen {
- return CertificateInvalidError{c, TooManyIntermediates}
+ return CertificateInvalidError{c, TooManyIntermediates, ""}
}
}
@@ -337,7 +752,7 @@
}
if len(chains) == 0 {
- err = CertificateInvalidError{c, IncompatibleUsage}
+ err = CertificateInvalidError{c, IncompatibleUsage, ""}
}
return
diff --git a/src/crypto/x509/verify_test.go b/src/crypto/x509/verify_test.go
index 41e295d..bd3df47 100644
--- a/src/crypto/x509/verify_test.go
+++ b/src/crypto/x509/verify_test.go
@@ -1551,22 +1551,37 @@
var nameConstraintTests = []struct {
constraint, domain string
+ expectError bool
shouldMatch bool
}{
- {"", "anything.com", true},
- {"example.com", "example.com", true},
- {"example.com", "ExAmPle.coM", true},
- {"example.com", "exampl1.com", false},
- {"example.com", "www.ExAmPle.coM", true},
- {"example.com", "notexample.com", false},
- {".example.com", "example.com", false},
- {".example.com", "www.example.com", true},
- {".example.com", "www..example.com", false},
+ {"", "anything.com", false, true},
+ {"example.com", "example.com", false, true},
+ {"example.com.", "example.com", true, false},
+ {"example.com", "example.com.", true, false},
+ {"example.com", "ExAmPle.coM", false, true},
+ {"example.com", "exampl1.com", false, false},
+ {"example.com", "www.ExAmPle.coM", false, true},
+ {"example.com", "sub.www.ExAmPle.coM", false, true},
+ {"example.com", "notexample.com", false, false},
+ {".example.com", "example.com", false, false},
+ {".example.com", "www.example.com", false, true},
+ {".example.com", "www..example.com", true, false},
}
func TestNameConstraints(t *testing.T) {
for i, test := range nameConstraintTests {
- result := matchNameConstraint(test.domain, test.constraint)
+ result, err := matchDomainConstraint(test.domain, test.constraint)
+
+ if err != nil && !test.expectError {
+ t.Errorf("unexpected error for test #%d: domain=%s, constraint=%s, err=%s", i, test.domain, test.constraint, err)
+ continue
+ }
+
+ if err == nil && test.expectError {
+ t.Errorf("unexpected success for test #%d: domain=%s, constraint=%s", i, test.domain, test.constraint)
+ continue
+ }
+
if result != test.shouldMatch {
t.Errorf("unexpected result for test #%d: domain=%s, constraint=%s, result=%t", i, test.domain, test.constraint, result)
}
diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go
index 2a8ee59..915cd2e 100644
--- a/src/crypto/x509/x509.go
+++ b/src/crypto/x509/x509.go
@@ -27,7 +27,9 @@
"io"
"math/big"
"net"
+ "net/url"
"strconv"
+ "strings"
"time"
)
@@ -698,11 +700,18 @@
DNSNames []string
EmailAddresses []string
IPAddresses []net.IP
+ URIs []*url.URL
// Name constraints
PermittedDNSDomainsCritical bool // if true then the name constraints are marked critical.
PermittedDNSDomains []string
ExcludedDNSDomains []string
+ PermittedIPRanges []*net.IPNet
+ ExcludedIPRanges []*net.IPNet
+ PermittedEmailAddresses []string
+ ExcludedEmailAddresses []string
+ PermittedURIDomains []string
+ ExcludedURIDomains []string
// CRL Distribution Points
CRLDistributionPoints []string
@@ -821,6 +830,26 @@
return checkSignature(algo, signed, signature, c.PublicKey)
}
+func (c *Certificate) hasNameConstraints() bool {
+ for _, e := range c.Extensions {
+ if len(e.Id) == 4 && e.Id[0] == 2 && e.Id[1] == 5 && e.Id[2] == 29 && e.Id[3] == 30 {
+ return true
+ }
+ }
+
+ return false
+}
+
+func (c *Certificate) getSANExtension() ([]byte, bool) {
+ for _, e := range c.Extensions {
+ if len(e.Id) == 4 && e.Id[0] == 2 && e.Id[1] == 5 && e.Id[2] == 29 && e.Id[3] == 17 {
+ return e.Value, true
+ }
+ }
+
+ return nil, false
+}
+
func signaturePublicKeyAlgoMismatchError(expectedPubKeyAlgo PublicKeyAlgorithm, pubKey interface{}) error {
return fmt.Errorf("x509: signature algorithm specifies an %s public key, but have public key of type %T", expectedPubKeyAlgo.String(), pubKey)
}
@@ -930,8 +959,18 @@
Excluded []generalSubtree `asn1:"optional,tag:1"`
}
+const (
+ nameTypeEmail = 1
+ nameTypeDNS = 2
+ nameTypeURI = 6
+ nameTypeIP = 7
+)
+
type generalSubtree struct {
- Name string `asn1:"tag:2,optional,ia5"`
+ Email string `asn1:"tag:1,optional,ia5"`
+ Name string `asn1:"tag:2,optional,ia5"`
+ URIDomain string `asn1:"tag:6,optional,ia5"`
+ IPAndMask []byte `asn1:"tag:7,optional"`
}
// RFC 5280, 4.2.2.1
@@ -1086,14 +1125,33 @@
return nil
}
-func parseSANExtension(value []byte) (dnsNames, emailAddresses []string, ipAddresses []net.IP, err error) {
+func parseSANExtension(value []byte) (dnsNames, emailAddresses []string, ipAddresses []net.IP, uris []*url.URL, err error) {
err = forEachSAN(value, func(tag int, data []byte) error {
switch tag {
- case 1:
- emailAddresses = append(emailAddresses, string(data))
- case 2:
- dnsNames = append(dnsNames, string(data))
- case 7:
+ case nameTypeEmail:
+ mailbox := string(data)
+ if _, ok := parseRFC2821Mailbox(mailbox); !ok {
+ return fmt.Errorf("x509: cannot parse rfc822Name %q", mailbox)
+ }
+ emailAddresses = append(emailAddresses, mailbox)
+ case nameTypeDNS:
+ domain := string(data)
+ if _, ok := domainToReverseLabels(domain); !ok {
+ return fmt.Errorf("x509: cannot parse dnsName %q", string(data))
+ }
+ dnsNames = append(dnsNames, domain)
+ case nameTypeURI:
+ uri, err := url.Parse(string(data))
+ if err != nil {
+ return fmt.Errorf("x509: cannot parse URI %q: %s", string(data), err)
+ }
+ if len(uri.Host) > 0 {
+ if _, ok := domainToReverseLabels(uri.Host); !ok {
+ return fmt.Errorf("x509: cannot parse URI %q: invalid domain", string(data))
+ }
+ }
+ uris = append(uris, uri)
+ case nameTypeIP:
switch len(data) {
case net.IPv4len, net.IPv6len:
ipAddresses = append(ipAddresses, data)
@@ -1108,6 +1166,160 @@
return
}
+// isValidIPMask returns true iff mask consists of zero or more 1 bits, followed by zero bits.
+func isValidIPMask(mask []byte) bool {
+ seenZero := false
+
+ for _, b := range mask {
+ if seenZero {
+ if b != 0 {
+ return false
+ }
+
+ continue
+ }
+
+ switch b {
+ case 0x00, 0x80, 0xc0, 0xe0, 0xf0, 0xf8, 0xfc, 0xfe:
+ seenZero = true
+ case 0xff:
+ default:
+ return false
+ }
+ }
+
+ return true
+}
+
+func parseNameConstraintsExtension(out *Certificate, e pkix.Extension) (unhandled bool, err error) {
+ // RFC 5280, 4.2.1.10
+
+ // NameConstraints ::= SEQUENCE {
+ // permittedSubtrees [0] GeneralSubtrees OPTIONAL,
+ // excludedSubtrees [1] GeneralSubtrees OPTIONAL }
+ //
+ // GeneralSubtrees ::= SEQUENCE SIZE (1..MAX) OF GeneralSubtree
+ //
+ // GeneralSubtree ::= SEQUENCE {
+ // base GeneralName,
+ // minimum [0] BaseDistance DEFAULT 0,
+ // maximum [1] BaseDistance OPTIONAL }
+ //
+ // BaseDistance ::= INTEGER (0..MAX)
+
+ var constraints nameConstraints
+ if rest, err := asn1.Unmarshal(e.Value, &constraints); err != nil {
+ return false, err
+ } else if len(rest) != 0 {
+ return false, 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 false, errors.New("x509: empty name constraints extension")
+ }
+
+ getValues := func(subtrees []generalSubtree) (dnsNames []string, ips []*net.IPNet, emails, uriDomains []string, err error) {
+ for _, subtree := range subtrees {
+ switch {
+ case len(subtree.Name) != 0:
+ domain := subtree.Name
+ if len(domain) > 0 && domain[0] == '.' {
+ // constraints can have a leading
+ // period to exclude the domain
+ // itself, but that's not valid in a
+ // normal domain name.
+ domain = domain[1:]
+ }
+ if _, ok := domainToReverseLabels(domain); !ok {
+ return nil, nil, nil, nil, fmt.Errorf("x509: failed to parse dnsName constraint %q", subtree.Name)
+ }
+ dnsNames = append(dnsNames, subtree.Name)
+
+ case len(subtree.IPAndMask) != 0:
+ l := len(subtree.IPAndMask)
+ var ip, mask []byte
+
+ switch l {
+ case 8:
+ ip = subtree.IPAndMask[:4]
+ mask = subtree.IPAndMask[4:]
+
+ case 32:
+ ip = subtree.IPAndMask[:16]
+ mask = subtree.IPAndMask[16:]
+
+ default:
+ return nil, nil, nil, nil, fmt.Errorf("x509: IP constraint contained value of length %d", l)
+ }
+
+ if !isValidIPMask(mask) {
+ return nil, nil, nil, nil, fmt.Errorf("x509: IP constraint contained invalid mask %x", mask)
+ }
+
+ ips = append(ips, &net.IPNet{IP: net.IP(ip), Mask: net.IPMask(mask)})
+
+ case len(subtree.Email) != 0:
+ constraint := subtree.Email
+ // If the constraint contains an @ then
+ // it specifies an exact mailbox name.
+ if strings.Contains(constraint, "@") {
+ if _, ok := parseRFC2821Mailbox(constraint); !ok {
+ return nil, nil, nil, nil, fmt.Errorf("x509: failed to parse rfc822Name constraint %q", constraint)
+ }
+ } else {
+ // Otherwise it's a domain name.
+ domain := constraint
+ if len(domain) > 0 && domain[0] == '.' {
+ domain = domain[1:]
+ }
+ if _, ok := domainToReverseLabels(domain); !ok {
+ return nil, nil, nil, nil, fmt.Errorf("x509: failed to parse rfc822Name constraint %q", constraint)
+ }
+ }
+ emails = append(emails, constraint)
+
+ case len(subtree.URIDomain) != 0:
+ domain := subtree.URIDomain
+
+ if net.ParseIP(domain) != nil {
+ return nil, nil, nil, nil, fmt.Errorf("x509: failed to parse URI constraint %q: cannot be IP address", subtree.URIDomain)
+ }
+
+ if len(domain) > 0 && domain[0] == '.' {
+ // constraints can have a leading
+ // period to exclude the domain
+ // itself, but that's not valid in a
+ // normal domain name.
+ domain = domain[1:]
+ }
+ if _, ok := domainToReverseLabels(domain); !ok {
+ return nil, nil, nil, nil, fmt.Errorf("x509: failed to parse URI constraint %q", subtree.URIDomain)
+ }
+ uriDomains = append(uriDomains, subtree.URIDomain)
+
+ default:
+ unhandled = true
+ }
+ }
+
+ return dnsNames, ips, emails, uriDomains, nil
+ }
+
+ if out.PermittedDNSDomains, out.PermittedIPRanges, out.PermittedEmailAddresses, out.PermittedURIDomains, err = getValues(constraints.Permitted); err != nil {
+ return false, err
+ }
+ if out.ExcludedDNSDomains, out.ExcludedIPRanges, out.ExcludedEmailAddresses, out.ExcludedURIDomains, err = getValues(constraints.Excluded); err != nil {
+ return false, err
+ }
+ out.PermittedDNSDomainsCritical = e.Critical
+
+ return unhandled, nil
+}
+
func parseCertificate(in *certificate) (*Certificate, error) {
out := new(Certificate)
out.Raw = in.Raw
@@ -1187,69 +1399,22 @@
out.MaxPathLenZero = out.MaxPathLen == 0
// TODO: map out.MaxPathLen to 0 if it has the -1 default value? (Issue 19285)
case 17:
- out.DNSNames, out.EmailAddresses, out.IPAddresses, err = parseSANExtension(e.Value)
+ out.DNSNames, out.EmailAddresses, out.IPAddresses, out.URIs, err = parseSANExtension(e.Value)
if err != nil {
return nil, err
}
- if len(out.DNSNames) == 0 && len(out.EmailAddresses) == 0 && len(out.IPAddresses) == 0 {
+ if len(out.DNSNames) == 0 && len(out.EmailAddresses) == 0 && len(out.IPAddresses) == 0 && len(out.URIs) == 0 {
// If we didn't parse anything then we do the critical check, below.
unhandled = true
}
case 30:
- // RFC 5280, 4.2.1.10
-
- // NameConstraints ::= SEQUENCE {
- // permittedSubtrees [0] GeneralSubtrees OPTIONAL,
- // excludedSubtrees [1] GeneralSubtrees OPTIONAL }
- //
- // GeneralSubtrees ::= SEQUENCE SIZE (1..MAX) OF GeneralSubtree
- //
- // GeneralSubtree ::= SEQUENCE {
- // base GeneralName,
- // minimum [0] BaseDistance DEFAULT 0,
- // maximum [1] BaseDistance OPTIONAL }
- //
- // BaseDistance ::= INTEGER (0..MAX)
-
- var constraints nameConstraints
- if rest, err := asn1.Unmarshal(e.Value, &constraints); err != nil {
+ unhandled, err = parseNameConstraintsExtension(out, e)
+ if err != nil {
return nil, err
- } else if len(rest) != 0 {
- 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 {
- if isCritical {
- return nil, UnhandledCriticalExtension{}
- }
- continue
- }
- dnsNames = append(dnsNames, subtree.Name)
- }
-
- return dnsNames, nil
- }
-
- if out.PermittedDNSDomains, err = getDNSNames(constraints.Permitted, e.Critical); err != nil {
- return out, err
- }
- if out.ExcludedDNSDomains, err = getDNSNames(constraints.Excluded, e.Critical); err != nil {
- return out, err
- }
- out.PermittedDNSDomainsCritical = e.Critical
-
case 31:
// RFC 5280, 4.2.1.13
@@ -1483,13 +1648,13 @@
// marshalSANs marshals a list of addresses into a the contents of an X.509
// SubjectAlternativeName extension.
-func marshalSANs(dnsNames, emailAddresses []string, ipAddresses []net.IP) (derBytes []byte, err error) {
+func marshalSANs(dnsNames, emailAddresses []string, ipAddresses []net.IP, uris []*url.URL) (derBytes []byte, err error) {
var rawValues []asn1.RawValue
for _, name := range dnsNames {
- rawValues = append(rawValues, asn1.RawValue{Tag: 2, Class: 2, Bytes: []byte(name)})
+ rawValues = append(rawValues, asn1.RawValue{Tag: nameTypeDNS, Class: 2, Bytes: []byte(name)})
}
for _, email := range emailAddresses {
- rawValues = append(rawValues, asn1.RawValue{Tag: 1, Class: 2, Bytes: []byte(email)})
+ rawValues = append(rawValues, asn1.RawValue{Tag: nameTypeEmail, Class: 2, Bytes: []byte(email)})
}
for _, rawIP := range ipAddresses {
// If possible, we always want to encode IPv4 addresses in 4 bytes.
@@ -1497,7 +1662,10 @@
if ip == nil {
ip = rawIP
}
- rawValues = append(rawValues, asn1.RawValue{Tag: 7, Class: 2, Bytes: ip})
+ rawValues = append(rawValues, asn1.RawValue{Tag: nameTypeIP, Class: 2, Bytes: ip})
+ }
+ for _, uri := range uris {
+ rawValues = append(rawValues, asn1.RawValue{Tag: nameTypeURI, Class: 2, Bytes: []byte(uri.String())})
}
return asn1.Marshal(rawValues)
}
@@ -1608,10 +1776,10 @@
n++
}
- if (len(template.DNSNames) > 0 || len(template.EmailAddresses) > 0 || len(template.IPAddresses) > 0) &&
+ if (len(template.DNSNames) > 0 || len(template.EmailAddresses) > 0 || len(template.IPAddresses) > 0 || len(template.URIs) > 0) &&
!oidInExtensions(oidExtensionSubjectAltName, template.ExtraExtensions) {
ret[n].Id = oidExtensionSubjectAltName
- ret[n].Value, err = marshalSANs(template.DNSNames, template.EmailAddresses, template.IPAddresses)
+ ret[n].Value, err = marshalSANs(template.DNSNames, template.EmailAddresses, template.IPAddresses, template.URIs)
if err != nil {
return
}
@@ -1632,20 +1800,50 @@
n++
}
- if (len(template.PermittedDNSDomains) > 0 || len(template.ExcludedDNSDomains) > 0) &&
+ if (len(template.PermittedDNSDomains) > 0 || len(template.ExcludedDNSDomains) > 0 ||
+ len(template.PermittedIPRanges) > 0 || len(template.ExcludedIPRanges) > 0 ||
+ len(template.PermittedEmailAddresses) > 0 || len(template.ExcludedEmailAddresses) > 0 ||
+ len(template.PermittedURIDomains) > 0 || len(template.ExcludedURIDomains) > 0) &&
!oidInExtensions(oidExtensionNameConstraints, template.ExtraExtensions) {
ret[n].Id = oidExtensionNameConstraints
ret[n].Critical = template.PermittedDNSDomainsCritical
var out nameConstraints
- out.Permitted = make([]generalSubtree, len(template.PermittedDNSDomains))
- for i, permitted := range template.PermittedDNSDomains {
- out.Permitted[i] = generalSubtree{Name: permitted}
+ ipAndMask := func(ipNet *net.IPNet) []byte {
+ maskedIP := ipNet.IP.Mask(ipNet.Mask)
+ ipAndMask := make([]byte, 0, len(maskedIP)+len(ipNet.Mask))
+ ipAndMask = append(ipAndMask, maskedIP...)
+ ipAndMask = append(ipAndMask, ipNet.Mask...)
+ return ipAndMask
}
- out.Excluded = make([]generalSubtree, len(template.ExcludedDNSDomains))
- for i, excluded := range template.ExcludedDNSDomains {
- out.Excluded[i] = generalSubtree{Name: excluded}
+
+ out.Permitted = make([]generalSubtree, 0, len(template.PermittedDNSDomains)+len(template.PermittedIPRanges))
+ for _, permitted := range template.PermittedDNSDomains {
+ out.Permitted = append(out.Permitted, generalSubtree{Name: permitted})
+ }
+ for _, permitted := range template.PermittedIPRanges {
+ out.Permitted = append(out.Permitted, generalSubtree{IPAndMask: ipAndMask(permitted)})
+ }
+ for _, permitted := range template.PermittedEmailAddresses {
+ out.Permitted = append(out.Permitted, generalSubtree{Email: permitted})
+ }
+ for _, permitted := range template.PermittedURIDomains {
+ out.Permitted = append(out.Permitted, generalSubtree{URIDomain: permitted})
+ }
+
+ out.Excluded = make([]generalSubtree, 0, len(template.ExcludedDNSDomains)+len(template.ExcludedIPRanges))
+ for _, excluded := range template.ExcludedDNSDomains {
+ out.Excluded = append(out.Excluded, generalSubtree{Name: excluded})
+ }
+ for _, excluded := range template.ExcludedIPRanges {
+ out.Excluded = append(out.Excluded, generalSubtree{IPAndMask: ipAndMask(excluded)})
+ }
+ for _, excluded := range template.ExcludedEmailAddresses {
+ out.Excluded = append(out.Excluded, generalSubtree{Email: excluded})
+ }
+ for _, excluded := range template.ExcludedURIDomains {
+ out.Excluded = append(out.Excluded, generalSubtree{URIDomain: excluded})
}
ret[n].Value, err = asn1.Marshal(out)
@@ -1997,6 +2195,7 @@
DNSNames []string
EmailAddresses []string
IPAddresses []net.IP
+ URIs []*url.URL
}
// These structures reflect the ASN.1 structure of X.509 certificate
@@ -2088,7 +2287,7 @@
// CreateCertificateRequest creates a new certificate request based on a
// template. The following members of template are used: Attributes, DNSNames,
-// EmailAddresses, ExtraExtensions, IPAddresses, SignatureAlgorithm, and
+// EmailAddresses, ExtraExtensions, IPAddresses, URIs, SignatureAlgorithm, and
// Subject. The private key is the private key of the signer.
//
// The returned slice is the certificate request in DER encoding.
@@ -2117,9 +2316,9 @@
var extensions []pkix.Extension
- if (len(template.DNSNames) > 0 || len(template.EmailAddresses) > 0 || len(template.IPAddresses) > 0) &&
+ if (len(template.DNSNames) > 0 || len(template.EmailAddresses) > 0 || len(template.IPAddresses) > 0 || len(template.URIs) > 0) &&
!oidInExtensions(oidExtensionSubjectAltName, template.ExtraExtensions) {
- sanBytes, err := marshalSANs(template.DNSNames, template.EmailAddresses, template.IPAddresses)
+ sanBytes, err := marshalSANs(template.DNSNames, template.EmailAddresses, template.IPAddresses, template.URIs)
if err != nil {
return nil, err
}
@@ -2294,7 +2493,7 @@
for _, extension := range out.Extensions {
if extension.Id.Equal(oidExtensionSubjectAltName) {
- out.DNSNames, out.EmailAddresses, out.IPAddresses, err = parseSANExtension(extension.Value)
+ out.DNSNames, out.EmailAddresses, out.IPAddresses, out.URIs, err = parseSANExtension(extension.Value)
if err != nil {
return nil, err
}
diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go
index 100c8be..f28f2fb 100644
--- a/src/crypto/x509/x509_test.go
+++ b/src/crypto/x509/x509_test.go
@@ -22,6 +22,7 @@
"internal/testenv"
"math/big"
"net"
+ "net/url"
"os/exec"
"reflect"
"runtime"
@@ -352,6 +353,22 @@
"9048084225c53e8acb7feb6f04d16dc574a2f7a27c7b603c77cd0ece48027f012fb69b37e02a2a" +
"36dcd585d6ace53f546f961e05af"
+func parseCIDR(s string) *net.IPNet {
+ _, net, err := net.ParseCIDR(s)
+ if err != nil {
+ panic(err)
+ }
+ return net
+}
+
+func parseURI(s string) *url.URL {
+ uri, err := url.Parse(s)
+ if err != nil {
+ panic(err)
+ }
+ return uri
+}
+
func TestCreateSelfSignedCertificate(t *testing.T) {
random := rand.Reader
@@ -423,10 +440,17 @@
DNSNames: []string{"test.example.com"},
EmailAddresses: []string{"gop...@golang.org"},
IPAddresses: []net.IP{net.IPv4(127, 0, 0, 1).To4(), net.ParseIP("2001:4860:0:2001::68")},
+ URIs: []*url.URL{parseURI("https://foo.com/wibble#foo")},
- PolicyIdentifiers: []asn1.ObjectIdentifier{[]int{1, 2, 3}},
- PermittedDNSDomains: []string{".example.com", "example.com"},
- ExcludedDNSDomains: []string{"bar.example.com"},
+ PolicyIdentifiers: []asn1.ObjectIdentifier{[]int{1, 2, 3}},
+ PermittedDNSDomains: []string{".example.com", "example.com"},
+ ExcludedDNSDomains: []string{"bar.example.com"},
+ PermittedIPRanges: []*net.IPNet{parseCIDR("192.168.1.1/16"), parseCIDR("1.2.3.4/8")},
+ ExcludedIPRanges: []*net.IPNet{parseCIDR("2001:db8::/48")},
+ PermittedEmailAddresses: []string{"f...@example.com"},
+ ExcludedEmailAddresses: []string{".example.com", "example.com"},
+ PermittedURIDomains: []string{".bar.com", "bar.com"},
+ ExcludedURIDomains: []string{".bar2.com", "bar2.com"},
CRLDistributionPoints: []string{"http://crl1.example.com/ca1.crl", "http://crl2.example.com/ca1.crl"},
@@ -468,6 +492,30 @@
t.Errorf("%s: failed to parse name constraint exclusions: %#v", test.name, cert.ExcludedDNSDomains)
}
+ if len(cert.PermittedIPRanges) != 2 || cert.PermittedIPRanges[0].String() != "192.168.0.0/16" || cert.PermittedIPRanges[1].String() != "1.0.0.0/8" {
+ t.Errorf("%s: failed to parse IP constraints: %#v", test.name, cert.PermittedIPRanges)
+ }
+
+ if len(cert.ExcludedIPRanges) != 1 || cert.ExcludedIPRanges[0].String() != "2001:db8::/48" {
+ t.Errorf("%s: failed to parse IP constraint exclusions: %#v", test.name, cert.ExcludedIPRanges)
+ }
+
+ if len(cert.PermittedEmailAddresses) != 1 || cert.PermittedEmailAddresses[0] != "f...@example.com" {
+ t.Errorf("%s: failed to parse permitted email addreses: %#v", test.name, cert.PermittedEmailAddresses)
+ }
+
+ if len(cert.ExcludedEmailAddresses) != 2 || cert.ExcludedEmailAddresses[0] != ".example.com" || cert.ExcludedEmailAddresses[1] != "example.com" {
+ t.Errorf("%s: failed to parse excluded email addreses: %#v", test.name, cert.ExcludedEmailAddresses)
+ }
+
+ if len(cert.PermittedURIDomains) != 2 || cert.PermittedURIDomains[0] != ".bar.com" || cert.PermittedURIDomains[1] != "bar.com" {
+ t.Errorf("%s: failed to parse permitted URIs: %#v", test.name, cert.PermittedURIDomains)
+ }
+
+ if len(cert.ExcludedURIDomains) != 2 || cert.ExcludedURIDomains[0] != ".bar2.com" || cert.ExcludedURIDomains[1] != "bar2.com" {
+ t.Errorf("%s: failed to parse excluded URIs: %#v", test.name, cert.ExcludedURIDomains)
+ }
+
if cert.Subject.CommonName != commonName {
t.Errorf("%s: subject wasn't correctly copied from the template. Got %s, want %s", test.name, cert.Subject.CommonName, commonName)
}
@@ -519,6 +567,10 @@
t.Errorf("%s: SAN emails differ from template. Got %v, want %v", test.name, cert.EmailAddresses, template.EmailAddresses)
}
+ if len(cert.URIs) != 1 || cert.URIs[0].String() != "https://foo.com/wibble#foo" {
+ t.Errorf("%s: URIs differ from template. Got %v, want %v", test.name, cert.URIs, template.URIs)
+ }
+
if !reflect.DeepEqual(cert.IPAddresses, template.IPAddresses) {
t.Errorf("%s: SAN IPs differ from template. Got %v, want %v", test.name, cert.IPAddresses, template.IPAddresses)
}
@@ -1012,7 +1064,7 @@
}
func TestCertificateRequestOverrides(t *testing.T) {
- sanContents, err := marshalSANs([]string{"foo.example.com"}, nil, nil)
+ sanContents, err := marshalSANs([]string{"foo.example.com"}, nil, nil, nil)
if err != nil {
t.Fatal(err)
}
@@ -1069,7 +1121,7 @@
t.Errorf("bad attributes: %#v\n", csr.Attributes)
}
- sanContents2, err := marshalSANs([]string{"foo2.example.com"}, nil, nil)
+ sanContents2, err := marshalSANs([]string{"foo2.example.com"}, nil, nil, nil)
if err != nil {
t.Fatal(err)
}
@@ -1567,3 +1619,69 @@
}
}
}
+
+const criticalNameConstraintWithUnknownTypePEM = `
+-----BEGIN CERTIFICATE-----
+MIIC/TCCAeWgAwIBAgICEjQwDQYJKoZIhvcNAQELBQAwKDEmMCQGA1UEAxMdRW1w
+dHkgbmFtZSBjb25zdHJhaW50cyBpc3N1ZXIwHhcNMTMwMjAxMDAwMDAwWhcNMjAw
+NTMwMTA0ODM4WjAhMR8wHQYDVQQDExZFbXB0eSBuYW1lIGNvbnN0cmFpbnRzMIIB
+IjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwriElUIt3LCqmJObs+yDoWPD
+F5IqgWk6moIobYjPfextZiYU6I3EfvAwoNxPDkN2WowcocUZMJbEeEq5ebBksFnx
+f12gBxlIViIYwZAzu7aFvhDMyPKQI3C8CG0ZSC9ABZ1E3umdA3CEueNOmP/TChNq
+Cl23+BG1Qb/PJkpAO+GfpWSVhTcV53Mf/cKvFHcjGNrxzdSoq9fyW7a6gfcGEQY0
+LVkmwFWUfJ0wT8kaeLr0E0tozkIfo01KNWNzv6NcYP80QOBRDlApWu9ODmEVJHPD
+blx4jzTQ3JLa+4DvBNOjVUOp+mgRmjiW0rLdrxwOxIqIOwNjweMCp/hgxX/hTQID
+AQABozgwNjA0BgNVHR4BAf8EKjAooCQwIokgIACrzQAAAAAAAAAAAAAAAP////8A
+AAAAAAAAAAAAAAChADANBgkqhkiG9w0BAQsFAAOCAQEAWG+/zUMHQhP8uNCtgSHy
+im/vh7wminwAvWgMKxlkLBFns6nZeQqsOV1lABY7U0Zuoqa1Z5nb6L+iJa4ElREJ
+Oi/erLc9uLwBdDCAR0hUTKD7a6i4ooS39DTle87cUnj0MW1CUa6Hv5SsvpYW+1Xl
+eYJk/axQOOTcy4Es53dvnZsjXH0EA/QHnn7UV+JmlE3rtVxcYp6MLYPmRhTioROA
+/drghicRkiu9hxdPyxkYS16M5g3Zj30jdm+k/6C6PeNtN9YmOOganCOSyFYfGhqO
+ANYzpmuV+oIedAsPpIbfIzN8njYUs1zio+1IoI4o8ddM9sCbtPU8o+WoY6IsCKXV
+/g==
+-----END CERTIFICATE-----`
+
+func TestCriticalNameConstraintWithUnknownType(t *testing.T) {
+ block, _ := pem.Decode([]byte(criticalNameConstraintWithUnknownTypePEM))
+ cert, err := ParseCertificate(block.Bytes)
+ if err != nil {
+ t.Fatalf("unexpected parsing failure: %s", err)
+ }
+
+ if l := len(cert.UnhandledCriticalExtensions); l != 1 {
+ t.Fatalf("expected one unhandled critical extension, but found %d", l)
+ }
+}
+
+const badIPMaskPEM = `
+-----BEGIN CERTIFICATE-----
+MIICzzCCAbegAwIBAgICEjQwDQYJKoZIhvcNAQELBQAwHTEbMBkGA1UEAxMSQmFk
+IElQIG1hc2sgaXNzdWVyMB4XDTEzMDIwMTAwMDAwMFoXDTIwMDUzMDEwNDgzOFow
+FjEUMBIGA1UEAxMLQmFkIElQIG1hc2swggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAw
+ggEKAoIBAQDCuISVQi3csKqYk5uz7IOhY8MXkiqBaTqagihtiM997G1mJhTojcR+
+8DCg3E8OQ3ZajByhxRkwlsR4Srl5sGSwWfF/XaAHGUhWIhjBkDO7toW+EMzI8pAj
+cLwIbRlIL0AFnUTe6Z0DcIS5406Y/9MKE2oKXbf4EbVBv88mSkA74Z+lZJWFNxXn
+cx/9wq8UdyMY2vHN1Kir1/JbtrqB9wYRBjQtWSbAVZR8nTBPyRp4uvQTS2jOQh+j
+TUo1Y3O/o1xg/zRA4FEOUCla704OYRUkc8NuXHiPNNDcktr7gO8E06NVQ6n6aBGa
+OJbSst2vHA7Eiog7A2PB4wKn+GDFf+FNAgMBAAGjIDAeMBwGA1UdHgEB/wQSMBCg
+DDAKhwgBAgME//8BAKEAMA0GCSqGSIb3DQEBCwUAA4IBAQBYb7/NQwdCE/y40K2B
+IfKKb++HvCaKfAC9aAwrGWQsEWezqdl5Cqw5XWUAFjtTRm6iprVnmdvov6IlrgSV
+EQk6L96stz24vAF0MIBHSFRMoPtrqLiihLf0NOV7ztxSePQxbUJRroe/lKy+lhb7
+VeV5gmT9rFA45NzLgSznd2+dmyNcfQQD9AeeftRX4maUTeu1XFxinowtg+ZGFOKh
+E4D92uCGJxGSK72HF0/LGRhLXozmDdmPfSN2b6T/oLo942031iY46BqcI5LIVh8a
+Go4A1jOma5X6gh50Cw+kht8jM3yeNhSzXOKj7Uigjijx10z2wJu09Tyj5ahjoiwI
+pdX+
+-----END CERTIFICATE-----`
+
+func TestBadIPMask(t *testing.T) {
+ block, _ := pem.Decode([]byte(badIPMaskPEM))
+ _, err := ParseCertificate(block.Bytes)
+ if err == nil {
+ t.Fatalf("unexpected success")
+ }
+
+ const expected = "contained invalid mask"
+ if !strings.Contains(err.Error(), expected) {
+ t.Fatalf("expected %q in error but got: %s", expected, err)
+ }
+}
diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go
index a82e779..16ac51e 100644
--- a/src/go/build/deps_test.go
+++ b/src/go/build/deps_test.go
@@ -377,7 +377,7 @@
},
"crypto/x509": {
"L4", "CRYPTO-MATH", "OS", "CGO",
- "crypto/x509/pkix", "encoding/pem", "encoding/hex", "net", "os/user", "syscall",
+ "crypto/x509/pkix", "encoding/pem", "encoding/hex", "net", "os/user", "syscall", "net/url",
},
"crypto/x509/pkix": {"L4", "CRYPTO-MATH", "encoding/hex"},
To view, visit change 62693. To unsubscribe, or for help writing mail filters, visit settings.