[release-branch.go1.24] crypto/x509: prevent HostnameError.Error() from consuming...

Constructing HostnameError.Error() takes O(N^2) runtime due to using a
string concatenation in a loop. Additionally, there is no limit on how
many names are included in the error message. As a result, a malicious
attacker could craft a certificate with an infinite amount of names to
unfairly consume resource.

To remediate this, we will now use strings.Builder to construct the
error message, preventing O(N^2) runtime. When a certificate has 100 or
more names, we will also not print each name individually.

Thanks to Philippe Antoine (Catena cyber) for reporting this issue.

Updates #76445
Fixes #76460
Fixes CVE-2025-61729

Change-Id: I6343776ec3289577abc76dad71766c491c1a7c81
Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/3000
Reviewed-by: Neal Patel <nealpatel@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/3220
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/725820
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Bypass: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Mark Freeman <markfreeman@google.com>

diff --git a/src/crypto/x509/verify.go b/src/crypto/x509/verify.go
index 9175fa4dc147a2bdbb970c4349d6daad89bf8e8c..3de9f93b2c4b16279a50874c926ccc09ec81b886 100644 (file)
--- a/src/crypto/x509/verify.go
+++ b/src/crypto/x509/verify.go
@@ -110,31 +110,38 @@ type HostnameError struct {
 
 func (h HostnameError) Error() string {
        c := h.Certificate
+       maxNamesIncluded := 100
 
        if !c.hasSANExtension() && matchHostnames(c.Subject.CommonName, h.Host) {
                return "x509: certificate relies on legacy Common Name field, use SANs instead"
        }
 
-       var valid string
+       var valid strings.Builder
        if ip := net.ParseIP(h.Host); ip != nil {
                // Trying to validate an IP
                if len(c.IPAddresses) == 0 {
                        return "x509: cannot validate certificate for " + h.Host + " because it doesn't contain any IP SANs"
                }
+               if len(c.IPAddresses) >= maxNamesIncluded {
+                       return fmt.Sprintf("x509: certificate is valid for %d IP SANs, but none matched %s", len(c.IPAddresses), h.Host)
+               }
                for _, san := range c.IPAddresses {
-                       if len(valid) > 0 {
-                               valid += ", "
+                       if valid.Len() > 0 {
+                               valid.WriteString(", ")
                        }
-                       valid += san.String()
+                       valid.WriteString(san.String())
                }
        } else {
-               valid = strings.Join(c.DNSNames, ", ")
+               if len(c.DNSNames) >= maxNamesIncluded {
+                       return fmt.Sprintf("x509: certificate is valid for %d names, but none matched %s", len(c.DNSNames), h.Host)
+               }
+               valid.WriteString(strings.Join(c.DNSNames, ", "))
        }
 
-       if len(valid) == 0 {
+       if valid.Len() == 0 {
                return "x509: certificate is not valid for any names, but wanted to match " + h.Host
        }
-       return "x509: certificate is valid for " + valid + ", not " + h.Host
+       return "x509: certificate is valid for " + valid.String() + ", not " + h.Host
 }
 
 // UnknownAuthorityError results when the certificate issuer is unknown

diff --git a/src/crypto/x509/verify_test.go b/src/crypto/x509/verify_test.go
index 6a394e46e94f5a766c77aedf764a3416cd62fb7c..9a21218ee4b465987a74e64c14d0e28024be29cb 100644 (file)
--- a/src/crypto/x509/verify_test.go
+++ b/src/crypto/x509/verify_test.go
@@ -10,13 +10,16 @@ import (
        "crypto/ecdsa"
        "crypto/elliptic"
        "crypto/rand"
+       "crypto/rsa"
        "crypto/x509/pkix"
        "encoding/asn1"
        "encoding/pem"
        "errors"
        "fmt"
        "internal/testenv"
+       "log"
        "math/big"
+       "net"
        "os"
        "os/exec"
        "runtime"
@@ -89,6 +92,26 @@ var verifyTests = []verifyTest{
 
                errorCallback: expectHostnameError("certificate is valid for"),
        },
+       {
+               name:        "TooManyDNS",
+               leaf:        generatePEMCertWithRepeatSAN(1677615892, 200, "fake.dns"),
+               roots:       []string{generatePEMCertWithRepeatSAN(1677615892, 200, "fake.dns")},
+               currentTime: 1677615892,
+               dnsName:     "www.example.com",
+               systemSkip:  true, // does not chain to a system root
+
+               errorCallback: expectHostnameError("certificate is valid for 200 names, but none matched"),
+       },
+       {
+               name:        "TooManyIPs",
+               leaf:        generatePEMCertWithRepeatSAN(1677615892, 150, "4.3.2.1"),
+               roots:       []string{generatePEMCertWithRepeatSAN(1677615892, 150, "4.3.2.1")},
+               currentTime: 1677615892,
+               dnsName:     "1.2.3.4",
+               systemSkip:  true, // does not chain to a system root
+
+               errorCallback: expectHostnameError("certificate is valid for 150 IP SANs, but none matched"),
+       },
        {
                name:          "IPMissing",
                leaf:          googleLeaf,
@@ -552,6 +575,30 @@ func nameToKey(name *pkix.Name) string {
        return strings.Join(name.Country, ",") + "/" + strings.Join(name.Organization, ",") + "/" + strings.Join(name.OrganizationalUnit, ",") + "/" + name.CommonName
 }
 
+func generatePEMCertWithRepeatSAN(currentTime int64, count int, san string) string {
+       cert := Certificate{
+               NotBefore: time.Unix(currentTime, 0),
+               NotAfter:  time.Unix(currentTime, 0),
+       }
+       if ip := net.ParseIP(san); ip != nil {
+               cert.IPAddresses = slices.Repeat([]net.IP{ip}, count)
+       } else {
+               cert.DNSNames = slices.Repeat([]string{san}, count)
+       }
+       privKey, err := rsa.GenerateKey(rand.Reader, 4096)
+       if err != nil {
+               log.Fatal(err)
+       }
+       certBytes, err := CreateCertificate(rand.Reader, &cert, &cert, &privKey.PublicKey, privKey)
+       if err != nil {
+               log.Fatal(err)
+       }
+       return string(pem.EncodeToMemory(&pem.Block{
+               Type:  "CERTIFICATE",
+               Bytes: certBytes,
+       }))
+}
+
 const gtsIntermediate = `-----BEGIN CERTIFICATE-----
 MIIFljCCA36gAwIBAgINAgO8U1lrNMcY9QFQZjANBgkqhkiG9w0BAQsFADBHMQsw
 CQYDVQQGEwJVUzEiMCAGA1UEChMZR29vZ2xlIFRydXN0IFNlcnZpY2VzIExMQzEU