--- /dev/null
+From ca6a5545ba18844a97c88a90a385eb6335bb7526 Mon Sep 17 00:00:00 2001
+From: Roland Shoemaker <roland@golang.org>
+Date: Thu, 9 Oct 2025 13:35:24 -0700
+Subject: [PATCH] [release-branch.go1.24] crypto/x509: rework fix for
+ CVE-2025-58187
+
+In CL 709854 we enabled strict validation for a number of properties of
+domain names (and their constraints). This caused significant breakage,
+since we didn't previously disallow the creation of certificates which
+contained these malformed domains.
+
+Rollback a number of the properties we enforced, making domainNameValid
+only enforce the same properties that domainToReverseLabels does. Since
+this also undoes some of the DoS protections our initial fix enabled,
+this change also adds caching of constraints in isValid (which perhaps
+is the fix we should've initially chosen).
+
+Updates #75835
+Updates #75828
+Fixes #75860
+
+Change-Id: Ie6ca6b4f30e9b8a143692b64757f7bbf4671ed0e
+Reviewed-on: https://go-review.googlesource.com/c/go/+/710735
+LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
+Reviewed-by: Damien Neil <dneil@google.com>
+(cherry picked from commit 1cd71689f2ed8f07031a0cc58fc3586ca501839f)
+Reviewed-on: https://go-review.googlesource.com/c/go/+/710879
+Reviewed-by: Michael Pratt <mpratt@google.com>
+Auto-Submit: Michael Pratt <mpratt@google.com>
+
+Upstream-Status: Backport [https://github.com/golang/go/commit/ca6a5545ba18844a97c88a90a385eb6335bb7526]
+CVE: CVE-2025-58187
+Signed-off-by: Vijay Anusuri <vanusuri@mvista.com>
+---
+ src/crypto/x509/name_constraints_test.go | 66 +++++++++++++++++--
+ src/crypto/x509/parser.go | 57 +++++++++++-----
+ src/crypto/x509/parser_test.go | 84 +++++++++++++++++++++---
+ src/crypto/x509/verify.go | 53 ++++++++++-----
+ src/crypto/x509/verify_test.go | 2 +-
+ 5 files changed, 213 insertions(+), 49 deletions(-)
+
+diff --git a/src/crypto/x509/name_constraints_test.go b/src/crypto/x509/name_constraints_test.go
+index 9aaa6d7..78263fc 100644
+--- a/src/crypto/x509/name_constraints_test.go
++++ b/src/crypto/x509/name_constraints_test.go
+@@ -1456,7 +1456,63 @@ var nameConstraintsTests = []nameConstraintsTest{
+ expectedError: "incompatible key usage",
+ },
+
+- // #77: if several EKUs are requested, satisfying any of them is sufficient.
++ // 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.
+ {
+ roots: make([]constraintsSpec, 1),
+ intermediates: [][]constraintsSpec{
+@@ -1471,7 +1527,7 @@ var nameConstraintsTests = []nameConstraintsTest{
+ requestedEKUs: []ExtKeyUsage{ExtKeyUsageClientAuth, ExtKeyUsageEmailProtection},
+ },
+
+- // #78: EKUs that are not asserted in VerifyOpts are not required to be
++ // #81: EKUs that are not asserted in VerifyOpts are not required to be
+ // nested.
+ {
+ roots: make([]constraintsSpec, 1),
+@@ -1490,7 +1546,7 @@ var nameConstraintsTests = []nameConstraintsTest{
+ },
+ },
+
+- // #79: a certificate without SANs and CN is accepted in a constrained chain.
++ // #82: a certificate without SANs and CN is accepted in a constrained chain.
+ {
+ roots: []constraintsSpec{
+ {
+@@ -1507,7 +1563,7 @@ var nameConstraintsTests = []nameConstraintsTest{
+ },
+ },
+
+- // #80: a certificate without SANs and with a CN that does not parse as a
++ // #83: a certificate without SANs and with a CN that does not parse as a
+ // hostname is accepted in a constrained chain.
+ {
+ roots: []constraintsSpec{
+@@ -1526,7 +1582,7 @@ var nameConstraintsTests = []nameConstraintsTest{
+ },
+ },
+
+- // #81: a certificate with SANs and CN is accepted in a constrained chain.
++ // #84: 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 9a3bcd6..f8beff7 100644
+--- a/src/crypto/x509/parser.go
++++ b/src/crypto/x509/parser.go
+@@ -378,14 +378,10 @@ 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 || (err == nil && !domainNameValid(name, false)) {
++ if err := isIA5String(name); err != nil {
+ return errors.New("x509: SAN dNSName is malformed")
+ }
+ dnsNames = append(dnsNames, string(name))
+@@ -395,9 +391,12 @@ func parseSANExtension(der cryptobyte.String) (dnsNames, emailAddresses []string
+ return errors.New("x509: SAN uniformResourceIdentifier is malformed")
+ }
+ uri, err := url.Parse(uriStr)
+- if err != nil || (err == nil && uri.Host != "" && !domainNameValid(uri.Host, false)) {
++ if err != nil {
+ return fmt.Errorf("x509: cannot parse URI %q: %s", uriStr, err)
+ }
++ if len(uri.Host) > 0 && !domainNameValid(uri.Host, false) {
++ return fmt.Errorf("x509: cannot parse URI %q: invalid domain", uriStr)
++ }
+ uris = append(uris, uri)
+ case nameTypeIP:
+ switch len(data) {
+@@ -1176,36 +1175,58 @@ 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.
++// domainNameValid is an alloc-less version of the checks that
++// domainToReverseLabels does.
+ func domainNameValid(s string, constraint bool) bool {
+- if len(s) == 0 && constraint {
++ // TODO(#75835): This function omits a number of checks which we
++ // really should be doing to enforce that domain names are valid names per
++ // RFC 1034. We previously enabled these checks, but this broke a
++ // significant number of certificates we previously considered valid, and we
++ // happily create via CreateCertificate (et al). We should enable these
++ // checks, but will need to gate them behind a GODEBUG.
++ //
++ // I have left the checks we previously enabled, noted with "TODO(#75835)" so
++ // that we can easily re-enable them once we unbreak everyone.
++
++ // TODO(#75835): this should only be true for constraints.
++ if len(s) == 0 {
+ return true
+ }
+- if len(s) == 0 || (!constraint && s[0] == '.') || s[len(s)-1] == '.' || len(s) > 253 {
++
++ // Do not allow trailing period (FQDN format is not allowed in SANs or
++ // constraints).
++ if s[len(s)-1] == '.' {
+ return false
+ }
++
++ // TODO(#75835): domains must have at least one label, cannot have
++ // a leading empty label, and cannot be longer than 253 characters.
++ // if len(s) == 0 || (!constraint && s[0] == '.') || 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] < 33 || s[i] > 126) {
++ // Invalid character.
++ return false
++ }
+ if i == len(s) || s[i] == '.' {
+ labelLen := i
+ if lastDot >= 0 {
+ labelLen -= lastDot + 1
+ }
+- if labelLen == 0 || labelLen > 63 {
++ if labelLen == 0 {
+ return false
+ }
++ // TODO(#75835): labels cannot be longer than 63 characters.
++ // if labelLen > 63 {
++ // return false
++ // }
+ lastDot = i
+ }
+ }
+diff --git a/src/crypto/x509/parser_test.go b/src/crypto/x509/parser_test.go
+index a6cdfb8..35f872a 100644
+--- a/src/crypto/x509/parser_test.go
++++ b/src/crypto/x509/parser_test.go
+@@ -5,6 +5,9 @@
+ package x509
+
+ import (
++ "crypto/ecdsa"
++ "crypto/elliptic"
++ "crypto/rand"
+ "encoding/asn1"
+ "strings"
+ "testing"
+@@ -110,7 +113,31 @@ func TestDomainNameValid(t *testing.T) {
+ constraint bool
+ valid bool
+ }{
+- {"empty name, name", "", false, false},
++ // TODO(#75835): these tests are for stricter name validation, which we
++ // had to disable. Once we reenable these strict checks, behind a
++ // GODEBUG, we should add them back in.
++ // {"empty name, name", "", false, false},
++ // {"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},
++ // {"64 char label, name", "a." + strings.Repeat("a", 64), false, false},
++ // {"64 char label, constraint", "a." + strings.Repeat("a", 64), true, false},
++
++ // TODO(#75835): these are the inverse of the tests above, they should be removed
++ // once the strict checking is enabled.
++ {"254 char label, name", strings.Repeat("a.a", 84) + "aaa", false, true},
++ {"254 char label, constraint", strings.Repeat("a.a", 84) + "aaa", true, true},
++ {"253 char label, name", strings.Repeat("a.a", 84) + "aa", false, true},
++ {"253 char label, constraint", strings.Repeat("a.a", 84) + "aa", true, true},
++ {"64 char single label, name", strings.Repeat("a", 64), false, true},
++ {"64 char single label, constraint", strings.Repeat("a", 64), true, true},
++ {"64 char label, name", "a." + strings.Repeat("a", 64), false, true},
++ {"64 char label, constraint", "a." + strings.Repeat("a", 64), true, true},
++
++ // Check we properly enforce properties of domain names.
+ {"empty name, constraint", "", true, true},
+ {"empty label, name", "a..a", false, false},
+ {"empty label, constraint", "a..a", true, false},
+@@ -124,23 +151,60 @@ func TestDomainNameValid(t *testing.T) {
+ {"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) {
++ valid := domainNameValid(tc.dnsName, tc.constraint)
++ if tc.valid != valid {
+ t.Errorf("domainNameValid(%q, %t) = %v; want %v", tc.dnsName, tc.constraint, !tc.valid, tc.valid)
+ }
++ // Also check that we enforce the same properties as domainToReverseLabels
++ trimmedName := tc.dnsName
++ if tc.constraint && len(trimmedName) > 1 && trimmedName[0] == '.' {
++ trimmedName = trimmedName[1:]
++ }
++ _, revValid := domainToReverseLabels(trimmedName)
++ if valid != revValid {
++ t.Errorf("domainNameValid(%q, %t) = %t != domainToReverseLabels(%q) = %t", tc.dnsName, tc.constraint, valid, trimmedName, revValid)
++ }
+ })
+ }
+ }
++
++func TestRoundtripWeirdSANs(t *testing.T) {
++ // TODO(#75835): check that certificates we create with CreateCertificate that have malformed SAN values
++ // can be parsed by ParseCertificate. We should eventually restrict this, but for now we have to maintain
++ // this property as people have been relying on it.
++ k, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
++ if err != nil {
++ t.Fatal(err)
++ }
++ badNames := []string{
++ "baredomain",
++ "baredomain.",
++ strings.Repeat("a", 255),
++ strings.Repeat("a", 65) + ".com",
++ }
++ tmpl := &Certificate{
++ EmailAddresses: badNames,
++ DNSNames: badNames,
++ }
++ b, err := CreateCertificate(rand.Reader, tmpl, tmpl, &k.PublicKey, k)
++ if err != nil {
++ t.Fatal(err)
++ }
++ _, err = ParseCertificate(b)
++ if err != nil {
++ t.Fatalf("Couldn't roundtrip certificate: %v", err)
++ }
++}
++
++func FuzzDomainNameValid(f *testing.F) {
++ f.Fuzz(func(t *testing.T, data string) {
++ domainNameValid(data, false)
++ domainNameValid(data, true)
++ })
++}
+diff --git a/src/crypto/x509/verify.go b/src/crypto/x509/verify.go
+index 14cd23f..e670786 100644
+--- a/src/crypto/x509/verify.go
++++ b/src/crypto/x509/verify.go
+@@ -393,7 +393,7 @@ func domainToReverseLabels(domain string) (reverseLabels []string, ok bool) {
+ return reverseLabels, true
+ }
+
+-func matchEmailConstraint(mailbox rfc2821Mailbox, constraint string) (bool, error) {
++func matchEmailConstraint(mailbox rfc2821Mailbox, constraint string, reversedDomainsCache map[string][]string, reversedConstraintsCache map[string][]string) (bool, error) {
+ // If the constraint contains an @, then it specifies an exact mailbox
+ // name.
+ if strings.Contains(constraint, "@") {
+@@ -406,10 +406,10 @@ func matchEmailConstraint(mailbox rfc2821Mailbox, constraint string) (bool, erro
+
+ // Otherwise the constraint is like a DNS constraint of the domain part
+ // of the mailbox.
+- return matchDomainConstraint(mailbox.domain, constraint)
++ return matchDomainConstraint(mailbox.domain, constraint, reversedDomainsCache, reversedConstraintsCache)
+ }
+
+-func matchURIConstraint(uri *url.URL, constraint string) (bool, error) {
++func matchURIConstraint(uri *url.URL, constraint string, reversedDomainsCache map[string][]string, reversedConstraintsCache map[string][]string) (bool, error) {
+ // From RFC 5280, Section 4.2.1.10:
+ // “a uniformResourceIdentifier that does not include an authority
+ // component with a host name specified as a fully qualified domain
+@@ -438,7 +438,7 @@ func matchURIConstraint(uri *url.URL, constraint string) (bool, error) {
+ return false, fmt.Errorf("URI with IP (%q) cannot be matched against constraints", uri.String())
+ }
+
+- return matchDomainConstraint(host, constraint)
++ return matchDomainConstraint(host, constraint, reversedDomainsCache, reversedConstraintsCache)
+ }
+
+ func matchIPConstraint(ip net.IP, constraint *net.IPNet) (bool, error) {
+@@ -455,16 +455,21 @@ func matchIPConstraint(ip net.IP, constraint *net.IPNet) (bool, error) {
+ return true, nil
+ }
+
+-func matchDomainConstraint(domain, constraint string) (bool, error) {
++func matchDomainConstraint(domain, constraint string, reversedDomainsCache map[string][]string, reversedConstraintsCache map[string][]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, nil
+ }
+
+- domainLabels, ok := domainToReverseLabels(domain)
+- if !ok {
+- return false, fmt.Errorf("x509: internal error: cannot parse domain %q", domain)
++ domainLabels, found := reversedDomainsCache[domain]
++ if !found {
++ var ok bool
++ domainLabels, ok = domainToReverseLabels(domain)
++ if !ok {
++ return false, fmt.Errorf("x509: internal error: cannot parse domain %q", domain)
++ }
++ reversedDomainsCache[domain] = domainLabels
+ }
+
+ // RFC 5280 says that a leading period in a domain name means that at
+@@ -478,9 +483,14 @@ func matchDomainConstraint(domain, constraint string) (bool, error) {
+ constraint = constraint[1:]
+ }
+
+- constraintLabels, ok := domainToReverseLabels(constraint)
+- if !ok {
+- return false, fmt.Errorf("x509: internal error: cannot parse domain %q", constraint)
++ constraintLabels, found := reversedConstraintsCache[constraint]
++ if !found {
++ var ok bool
++ constraintLabels, ok = domainToReverseLabels(constraint)
++ if !ok {
++ return false, fmt.Errorf("x509: internal error: cannot parse domain %q", constraint)
++ }
++ reversedConstraintsCache[constraint] = constraintLabels
+ }
+
+ if len(domainLabels) < len(constraintLabels) ||
+@@ -601,6 +611,19 @@ func (c *Certificate) isValid(certType int, currentChain []*Certificate, opts *V
+ }
+ }
+
++ // Each time we do constraint checking, we need to check the constraints in
++ // the current certificate against all of the names that preceded it. We
++ // reverse these names using domainToReverseLabels, which is a relatively
++ // expensive operation. Since we check each name against each constraint,
++ // this requires us to do N*C calls to domainToReverseLabels (where N is the
++ // total number of names that preceed the certificate, and C is the total
++ // number of constraints in the certificate). By caching the results of
++ // calling domainToReverseLabels, we can reduce that to N+C calls at the
++ // cost of keeping all of the parsed names and constraints in memory until
++ // we return from isValid.
++ reversedDomainsCache := map[string][]string{}
++ reversedConstraintsCache := map[string][]string{}
++
+ if (certType == intermediateCertificate || certType == rootCertificate) &&
+ c.hasNameConstraints() {
+ toCheck := []*Certificate{}
+@@ -621,20 +644,20 @@ func (c *Certificate) isValid(certType int, currentChain []*Certificate, opts *V
+
+ if err := c.checkNameConstraints(&comparisonCount, maxConstraintComparisons, "email address", name, mailbox,
+ func(parsedName, constraint any) (bool, error) {
+- return matchEmailConstraint(parsedName.(rfc2821Mailbox), constraint.(string))
++ return matchEmailConstraint(parsedName.(rfc2821Mailbox), constraint.(string), reversedDomainsCache, reversedConstraintsCache)
+ }, c.PermittedEmailAddresses, c.ExcludedEmailAddresses); err != nil {
+ return err
+ }
+
+ case nameTypeDNS:
+ name := string(data)
+- if _, ok := domainToReverseLabels(name); !ok {
++ if !domainNameValid(name, false) {
+ return fmt.Errorf("x509: cannot parse dnsName %q", name)
+ }
+
+ if err := c.checkNameConstraints(&comparisonCount, maxConstraintComparisons, "DNS name", name, name,
+ func(parsedName, constraint any) (bool, error) {
+- return matchDomainConstraint(parsedName.(string), constraint.(string))
++ return matchDomainConstraint(parsedName.(string), constraint.(string), reversedDomainsCache, reversedConstraintsCache)
+ }, c.PermittedDNSDomains, c.ExcludedDNSDomains); err != nil {
+ return err
+ }
+@@ -648,7 +671,7 @@ func (c *Certificate) isValid(certType int, currentChain []*Certificate, opts *V
+
+ if err := c.checkNameConstraints(&comparisonCount, maxConstraintComparisons, "URI", name, uri,
+ func(parsedName, constraint any) (bool, error) {
+- return matchURIConstraint(parsedName.(*url.URL), constraint.(string))
++ return matchURIConstraint(parsedName.(*url.URL), constraint.(string), reversedDomainsCache, reversedConstraintsCache)
+ }, c.PermittedURIDomains, c.ExcludedURIDomains); err != nil {
+ return err
+ }
+diff --git a/src/crypto/x509/verify_test.go b/src/crypto/x509/verify_test.go
+index 4a7d8da..ba5c392 100644
+--- a/src/crypto/x509/verify_test.go
++++ b/src/crypto/x509/verify_test.go
+@@ -1549,7 +1549,7 @@ var nameConstraintTests = []struct {
+
+ func TestNameConstraints(t *testing.T) {
+ for i, test := range nameConstraintTests {
+- result, err := matchDomainConstraint(test.domain, test.constraint)
++ result, err := matchDomainConstraint(test.domain, test.constraint, map[string][]string{}, map[string][]string{})
+
+ if err != nil && !test.expectError {
+ t.Errorf("unexpected error for test #%d: domain=%s, constraint=%s, err=%s", i, test.domain, test.constraint, err)
+--
+2.43.0
+
projects / thirdparty / openembedded / openembedded-core.git / commitdiff
