crypto/x509: prevent HostnameError.Error() from consuming excessive resource

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.

Fixes #76445
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-review.googlesource.com/c/go/+/725920
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Bypass: Dmitri Shuralyov <dmitshur@golang.org>
This commit is contained in:
Nicholas S. Husin 2025-11-24 14:56:23 -05:00 committed by Roland Shoemaker
parent 8ae5d408ed
commit c1acdcb345
2 changed files with 61 additions and 7 deletions

View file

@ -108,31 +108,38 @@ type HostnameError struct {
func (h HostnameError) Error() string { func (h HostnameError) Error() string {
c := h.Certificate c := h.Certificate
maxNamesIncluded := 100
if !c.hasSANExtension() && matchHostnames(c.Subject.CommonName, h.Host) { if !c.hasSANExtension() && matchHostnames(c.Subject.CommonName, h.Host) {
return "x509: certificate relies on legacy Common Name field, use SANs instead" 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 { if ip := net.ParseIP(h.Host); ip != nil {
// Trying to validate an IP // Trying to validate an IP
if len(c.IPAddresses) == 0 { if len(c.IPAddresses) == 0 {
return "x509: cannot validate certificate for " + h.Host + " because it doesn't contain any IP SANs" 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 { for _, san := range c.IPAddresses {
if len(valid) > 0 { if valid.Len() > 0 {
valid += ", " valid.WriteString(", ")
} }
valid += san.String() valid.WriteString(san.String())
} }
} else { } 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 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 // UnknownAuthorityError results when the certificate issuer is unknown

View file

@ -10,13 +10,16 @@ import (
"crypto/ecdsa" "crypto/ecdsa"
"crypto/elliptic" "crypto/elliptic"
"crypto/rand" "crypto/rand"
"crypto/rsa"
"crypto/x509/pkix" "crypto/x509/pkix"
"encoding/asn1" "encoding/asn1"
"encoding/pem" "encoding/pem"
"errors" "errors"
"fmt" "fmt"
"internal/testenv" "internal/testenv"
"log"
"math/big" "math/big"
"net"
"os" "os"
"os/exec" "os/exec"
"runtime" "runtime"
@ -89,6 +92,26 @@ var verifyTests = []verifyTest{
errorCallback: expectHostnameError("certificate is valid for"), 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", name: "IPMissing",
leaf: googleLeaf, 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 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----- const gtsIntermediate = `-----BEGIN CERTIFICATE-----
MIIFljCCA36gAwIBAgINAgO8U1lrNMcY9QFQZjANBgkqhkiG9w0BAQsFADBHMQsw MIIFljCCA36gAwIBAgINAgO8U1lrNMcY9QFQZjANBgkqhkiG9w0BAQsFADBHMQsw
CQYDVQQGEwJVUzEiMCAGA1UEChMZR29vZ2xlIFRydXN0IFNlcnZpY2VzIExMQzEU CQYDVQQGEwJVUzEiMCAGA1UEChMZR29vZ2xlIFRydXN0IFNlcnZpY2VzIExMQzEU