mirror of
https://git.yoctoproject.org/poky
synced 2026-01-29 21:08:42 +01:00
084488d13f
Upstream-Status: Backport from ca6a5545ba
(From OE-Core rev: 43b3d2b2ef77c97b323b86bd6ee54996c38e46ed)
Signed-off-by: Vijay Anusuri <vanusuri@mvista.com>
Signed-off-by: Steve Sakoman <steve@sakoman.com>
350 lines
13 KiB
Diff
350 lines
13 KiB
Diff
From f334417e71f8b078ad64035bddb6df7f8910da6c Mon Sep 17 00:00:00 2001
|
|
From: Neal Patel <nealpatel@google.com>
|
|
Date: Mon, 15 Sep 2025 16:31:22 -0400
|
|
Subject: [PATCH] crypto/x509: improve domain name verification
|
|
|
|
Don't use domainToReverseLabels to check if domain names are valid,
|
|
since it is not particularly performant, and can contribute to DoS
|
|
vectors. Instead just iterate over the name and enforce the properties
|
|
we care about.
|
|
|
|
This also enforces that DNS names, both in SANs and name constraints,
|
|
are valid. We previously allowed invalid SANs, because some
|
|
intermediates had these weird names (see #23995), but there are
|
|
currently no trusted intermediates that have this property, and since we
|
|
target the web PKI, supporting this particular case is not a high
|
|
priority.
|
|
|
|
Thank you to Jakub Ciolek for reporting this issue.
|
|
|
|
Fixes CVE-2025-58187
|
|
For #75681
|
|
Fixes #75714
|
|
|
|
Change-Id: I6ebce847dcbe5fc63ef2f9a74f53f11c4c56d3d1
|
|
Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/2820
|
|
Reviewed-by: Damien Neil <dneil@google.com>
|
|
Reviewed-by: Roland Shoemaker <bracewell@google.com>
|
|
Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/2982
|
|
Reviewed-by: Nicholas Husin <husin@google.com>
|
|
Reviewed-on: https://go-review.googlesource.com/c/go/+/709839
|
|
Auto-Submit: Michael Pratt <mpratt@google.com>
|
|
Reviewed-by: Carlos Amedee <carlos@golang.org>
|
|
TryBot-Bypass: Michael Pratt <mpratt@google.com>
|
|
|
|
CVE: CVE-2025-58187
|
|
|
|
Upstream-Status: Backport [https://github.com/golang/go/commit/f334417e71f8b078ad64035bddb6df7f8910da6c]
|
|
|
|
Signed-off-by: Archana Polampalli <archana.polampalli@windriver.com>
|
|
---
|
|
src/crypto/x509/name_constraints_test.go | 66 ++------------------
|
|
src/crypto/x509/parser.go | 77 ++++++++++++++----------
|
|
src/crypto/x509/parser_test.go | 43 +++++++++++++
|
|
src/crypto/x509/verify.go | 1 +
|
|
4 files changed, 95 insertions(+), 92 deletions(-)
|
|
|
|
diff --git a/src/crypto/x509/name_constraints_test.go b/src/crypto/x509/name_constraints_test.go
|
|
index c59a7dc..d4f7d41 100644
|
|
--- a/src/crypto/x509/name_constraints_test.go
|
|
+++ b/src/crypto/x509/name_constraints_test.go
|
|
@@ -1452,63 +1452,7 @@ var nameConstraintsTests = []nameConstraintsTest{
|
|
requestedEKUs: []ExtKeyUsage{ExtKeyUsageServerAuth},
|
|
},
|
|
|
|
- // 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{
|
|
@@ -1523,7 +1467,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),
|
|
@@ -1542,7 +1486,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{
|
|
{
|
|
@@ -1559,7 +1503,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{
|
|
@@ -1578,7 +1522,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{
|
|
{
|
|
diff --git a/src/crypto/x509/parser.go b/src/crypto/x509/parser.go
|
|
index 635e74b..0788210 100644
|
|
--- a/src/crypto/x509/parser.go
|
|
+++ b/src/crypto/x509/parser.go
|
|
@@ -391,10 +391,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))
|
|
@@ -404,14 +408,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) {
|
|
@@ -551,15 +550,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)
|
|
@@ -600,12 +591,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)
|
|
}
|
|
}
|
|
@@ -621,15 +607,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)
|
|
@@ -1011,3 +989,40 @@ func ParseCertificates(der []byte) ([]*Certificate, error) {
|
|
}
|
|
return certs, 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 d7cf7ea..95ed116 100644
|
|
--- a/src/crypto/x509/parser_test.go
|
|
+++ b/src/crypto/x509/parser_test.go
|
|
@@ -5,6 +5,7 @@ package x509
|
|
|
|
import (
|
|
"encoding/asn1"
|
|
+ "strings"
|
|
"testing"
|
|
|
|
cryptobyte_asn1 "golang.org/x/crypto/cryptobyte/asn1"
|
|
@@ -100,3 +101,45 @@ func TestParseASN1String(t *testing.T) {
|
|
})
|
|
}
|
|
}
|
|
+
|
|
+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 3e95808..fb2f4b2 100644
|
|
--- a/src/crypto/x509/verify.go
|
|
+++ b/src/crypto/x509/verify.go
|
|
@@ -357,6 +357,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)
|
|
--
|
|
2.40.0
|
|
|