aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
Diffstat (limited to 'src')
-rw-r--r--src/crypto/x509/name_constraints_test.go75
-rw-r--r--src/crypto/x509/parser.go77
-rw-r--r--src/crypto/x509/parser_test.go43
-rw-r--r--src/crypto/x509/verify.go1
4 files changed, 96 insertions, 100 deletions
diff --git a/src/crypto/x509/name_constraints_test.go b/src/crypto/x509/name_constraints_test.go
index a585184516..831fcbc8d2 100644
--- a/src/crypto/x509/name_constraints_test.go
+++ b/src/crypto/x509/name_constraints_test.go
@@ -1456,63 +1456,7 @@ var nameConstraintsTests = []nameConstraintsTest{
expectedError: "incompatible key usage",
},
- // An invalid DNS SAN should be detected only at validation time so
- // that we can process CA certificates in the wild that have invalid SANs.
- // See https://github.com/golang/go/issues/23995
-
- // #77: an invalid DNS or mail SAN will not be detected if name constraint
- // checking is not triggered.
- {
- roots: make([]constraintsSpec, 1),
- intermediates: [][]constraintsSpec{
- {
- {},
- },
- },
- leaf: leafSpec{
- sans: []string{"dns:this is invalid", "email:this @ is invalid"},
- },
- },
-
- // #78: an invalid DNS SAN will be detected if any name constraint checking
- // is triggered.
- {
- roots: []constraintsSpec{
- {
- bad: []string{"uri:"},
- },
- },
- intermediates: [][]constraintsSpec{
- {
- {},
- },
- },
- leaf: leafSpec{
- sans: []string{"dns:this is invalid"},
- },
- expectedError: "cannot parse dnsName",
- },
-
- // #79: an invalid email SAN will be detected if any name constraint
- // checking is triggered.
- {
- roots: []constraintsSpec{
- {
- bad: []string{"uri:"},
- },
- },
- intermediates: [][]constraintsSpec{
- {
- {},
- },
- },
- leaf: leafSpec{
- sans: []string{"email:this @ is invalid"},
- },
- expectedError: "cannot parse rfc822Name",
- },
-
- // #80: if several EKUs are requested, satisfying any of them is sufficient.
+ // #77: if several EKUs are requested, satisfying any of them is sufficient.
{
roots: make([]constraintsSpec, 1),
intermediates: [][]constraintsSpec{
@@ -1527,7 +1471,7 @@ var nameConstraintsTests = []nameConstraintsTest{
requestedEKUs: []ExtKeyUsage{ExtKeyUsageClientAuth, ExtKeyUsageEmailProtection},
},
- // #81: EKUs that are not asserted in VerifyOpts are not required to be
+ // #78: EKUs that are not asserted in VerifyOpts are not required to be
// nested.
{
roots: make([]constraintsSpec, 1),
@@ -1546,7 +1490,7 @@ var nameConstraintsTests = []nameConstraintsTest{
},
},
- // #82: a certificate without SANs and CN is accepted in a constrained chain.
+ // #79: a certificate without SANs and CN is accepted in a constrained chain.
{
roots: []constraintsSpec{
{
@@ -1563,7 +1507,7 @@ var nameConstraintsTests = []nameConstraintsTest{
},
},
- // #83: a certificate without SANs and with a CN that does not parse as a
+ // #80: a certificate without SANs and with a CN that does not parse as a
// hostname is accepted in a constrained chain.
{
roots: []constraintsSpec{
@@ -1582,7 +1526,7 @@ var nameConstraintsTests = []nameConstraintsTest{
},
},
- // #84: a certificate with SANs and CN is accepted in a constrained chain.
+ // #81: a certificate with SANs and CN is accepted in a constrained chain.
{
roots: []constraintsSpec{
{
@@ -1600,14 +1544,7 @@ var nameConstraintsTests = []nameConstraintsTest{
},
},
- // #85: .example.com is an invalid DNS name, it should not match the
- // constraint example.com.
- {
- roots: []constraintsSpec{{ok: []string{"dns:example.com"}}},
- leaf: leafSpec{sans: []string{"dns:.example.com"}},
- expectedError: "cannot parse dnsName \".example.com\"",
- },
- // #86: URIs with IPv6 addresses with zones and ports are rejected
+ // #82: URIs with IPv6 addresses with zones and ports are rejected
{
roots: []constraintsSpec{
{
diff --git a/src/crypto/x509/parser.go b/src/crypto/x509/parser.go
index 4abcc1b7b5..9d6bfd6e95 100644
--- a/src/crypto/x509/parser.go
+++ b/src/crypto/x509/parser.go
@@ -413,10 +413,14 @@ func parseSANExtension(der cryptobyte.String) (dnsNames, emailAddresses []string
if err := isIA5String(email); err != nil {
return errors.New("x509: SAN rfc822Name is malformed")
}
+ parsed, ok := parseRFC2821Mailbox(email)
+ if !ok || (ok && !domainNameValid(parsed.domain, false)) {
+ return errors.New("x509: SAN rfc822Name is malformed")
+ }
emailAddresses = append(emailAddresses, email)
case nameTypeDNS:
name := string(data)
- if err := isIA5String(name); err != nil {
+ if err := isIA5String(name); err != nil || (err == nil && !domainNameValid(name, false)) {
return errors.New("x509: SAN dNSName is malformed")
}
dnsNames = append(dnsNames, string(name))
@@ -426,14 +430,9 @@ func parseSANExtension(der cryptobyte.String) (dnsNames, emailAddresses []string
return errors.New("x509: SAN uniformResourceIdentifier is malformed")
}
uri, err := url.Parse(uriStr)
- if err != nil {
+ if err != nil || (err == nil && uri.Host != "" && !domainNameValid(uri.Host, false)) {
return fmt.Errorf("x509: cannot parse URI %q: %s", uriStr, err)
}
- if len(uri.Host) > 0 {
- if _, ok := domainToReverseLabels(uri.Host); !ok {
- return fmt.Errorf("x509: cannot parse URI %q: invalid domain", uriStr)
- }
- }
uris = append(uris, uri)
case nameTypeIP:
switch len(data) {
@@ -598,15 +597,7 @@ func parseNameConstraintsExtension(out *Certificate, e pkix.Extension) (unhandle
return nil, nil, nil, nil, errors.New("x509: invalid constraint value: " + err.Error())
}
- trimmedDomain := domain
- if len(trimmedDomain) > 0 && trimmedDomain[0] == '.' {
- // constraints can have a leading
- // period to exclude the domain
- // itself, but that's not valid in a
- // normal domain name.
- trimmedDomain = trimmedDomain[1:]
- }
- if _, ok := domainToReverseLabels(trimmedDomain); !ok {
+ if !domainNameValid(domain, true) {
return nil, nil, nil, nil, fmt.Errorf("x509: failed to parse dnsName constraint %q", domain)
}
dnsNames = append(dnsNames, domain)
@@ -647,12 +638,7 @@ func parseNameConstraintsExtension(out *Certificate, e pkix.Extension) (unhandle
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 {
+ if !domainNameValid(constraint, true) {
return nil, nil, nil, nil, fmt.Errorf("x509: failed to parse rfc822Name constraint %q", constraint)
}
}
@@ -668,15 +654,7 @@ func parseNameConstraintsExtension(out *Certificate, e pkix.Extension) (unhandle
return nil, nil, nil, nil, fmt.Errorf("x509: failed to parse URI constraint %q: cannot be IP address", domain)
}
- trimmedDomain := domain
- if len(trimmedDomain) > 0 && trimmedDomain[0] == '.' {
- // constraints can have a leading
- // period to exclude the domain itself,
- // but that's not valid in a normal
- // domain name.
- trimmedDomain = trimmedDomain[1:]
- }
- if _, ok := domainToReverseLabels(trimmedDomain); !ok {
+ if !domainNameValid(domain, true) {
return nil, nil, nil, nil, fmt.Errorf("x509: failed to parse URI constraint %q", domain)
}
uriDomains = append(uriDomains, domain)
@@ -1317,3 +1295,40 @@ func ParseRevocationList(der []byte) (*RevocationList, error) {
return rl, nil
}
+
+// domainNameValid does minimal domain name validity checking. In particular it
+// enforces the following properties:
+// - names cannot have the trailing period
+// - names can only have a leading period if constraint is true
+// - names must be <= 253 characters
+// - names cannot have empty labels
+// - names cannot labels that are longer than 63 characters
+//
+// Note that this does not enforce the LDH requirements for domain names.
+func domainNameValid(s string, constraint bool) bool {
+ if len(s) == 0 && constraint {
+ return true
+ }
+ if len(s) == 0 || (!constraint && s[0] == '.') || s[len(s)-1] == '.' || len(s) > 253 {
+ return false
+ }
+ lastDot := -1
+ if constraint && s[0] == '.' {
+ s = s[1:]
+ }
+
+ for i := 0; i <= len(s); i++ {
+ if i == len(s) || s[i] == '.' {
+ labelLen := i
+ if lastDot >= 0 {
+ labelLen -= lastDot + 1
+ }
+ if labelLen == 0 || labelLen > 63 {
+ return false
+ }
+ lastDot = i
+ }
+ }
+
+ return true
+}
diff --git a/src/crypto/x509/parser_test.go b/src/crypto/x509/parser_test.go
index 3b9d9aed82..1b553e362e 100644
--- a/src/crypto/x509/parser_test.go
+++ b/src/crypto/x509/parser_test.go
@@ -8,6 +8,7 @@ import (
"encoding/asn1"
"encoding/pem"
"os"
+ "strings"
"testing"
cryptobyte_asn1 "golang.org/x/crypto/cryptobyte/asn1"
@@ -251,3 +252,45 @@ d5l1tRhScKu2NBgm74nYmJxJYgvuTA38wGhRrGU=
}
}
}
+
+func TestDomainNameValid(t *testing.T) {
+ for _, tc := range []struct {
+ name string
+ dnsName string
+ constraint bool
+ valid bool
+ }{
+ {"empty name, name", "", false, false},
+ {"empty name, constraint", "", true, true},
+ {"empty label, name", "a..a", false, false},
+ {"empty label, constraint", "a..a", true, false},
+ {"period, name", ".", false, false},
+ {"period, constraint", ".", true, false}, // TODO(roland): not entirely clear if this is a valid constraint (require at least one label?)
+ {"valid, name", "a.b.c", false, true},
+ {"valid, constraint", "a.b.c", true, true},
+ {"leading period, name", ".a.b.c", false, false},
+ {"leading period, constraint", ".a.b.c", true, true},
+ {"trailing period, name", "a.", false, false},
+ {"trailing period, constraint", "a.", true, false},
+ {"bare label, name", "a", false, true},
+ {"bare label, constraint", "a", true, true},
+ {"254 char label, name", strings.Repeat("a.a", 84) + "aaa", false, false},
+ {"254 char label, constraint", strings.Repeat("a.a", 84) + "aaa", true, false},
+ {"253 char label, name", strings.Repeat("a.a", 84) + "aa", false, false},
+ {"253 char label, constraint", strings.Repeat("a.a", 84) + "aa", true, false},
+ {"64 char single label, name", strings.Repeat("a", 64), false, false},
+ {"64 char single label, constraint", strings.Repeat("a", 64), true, false},
+ {"63 char single label, name", strings.Repeat("a", 63), false, true},
+ {"63 char single label, constraint", strings.Repeat("a", 63), true, true},
+ {"64 char label, name", "a." + strings.Repeat("a", 64), false, false},
+ {"64 char label, constraint", "a." + strings.Repeat("a", 64), true, false},
+ {"63 char label, name", "a." + strings.Repeat("a", 63), false, true},
+ {"63 char label, constraint", "a." + strings.Repeat("a", 63), true, true},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ if tc.valid != domainNameValid(tc.dnsName, tc.constraint) {
+ t.Errorf("domainNameValid(%q, %t) = %v; want %v", tc.dnsName, tc.constraint, !tc.valid, tc.valid)
+ }
+ })
+ }
+}
diff --git a/src/crypto/x509/verify.go b/src/crypto/x509/verify.go
index 755c1db96c..058153fbe7 100644
--- a/src/crypto/x509/verify.go
+++ b/src/crypto/x509/verify.go
@@ -391,6 +391,7 @@ func parseRFC2821Mailbox(in string) (mailbox rfc2821Mailbox, ok bool) {
// 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) {
+ reverseLabels = make([]string, 0, strings.Count(domain, ".")+1)
for len(domain) > 0 {
if i := strings.LastIndexByte(domain, '.'); i == -1 {
reverseLabels = append(reverseLabels, domain)
generated by cgit v1.3-5-g9baa (git 2.53.0) at 2026-04-17 15:57:16 +0000