From 89915030bc5887b3e0fd7ce08c990eba80083d10 Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Wed, 7 Jan 2026 17:45:52 +0100 Subject: [PATCH] tokenrequest: Use custom HostnameError formatting This is to mitigate CVE-2025-61729 Assisted-by: Claude Code --- pkg/crypto/errors.go | 46 +++++++ pkg/crypto/errors_test.go | 156 ++++++++++++++++++++++++ pkg/oauth/tokenrequest/request_token.go | 9 +- 3 files changed, 209 insertions(+), 2 deletions(-) create mode 100644 pkg/crypto/errors.go create mode 100644 pkg/crypto/errors_test.go diff --git a/pkg/crypto/errors.go b/pkg/crypto/errors.go new file mode 100644 index 0000000000..07f79c9a45 --- /dev/null +++ b/pkg/crypto/errors.go @@ -0,0 +1,46 @@ +package crypto + +import ( + "crypto/x509" + "fmt" + "net" + "strings" +) + +// FormatHostnameError formats hostname errors without calling HostnameError.Error() +// to mitigate CVE-2025-61729 (quadratic runtime from repeated string concatenation with unlimited SANs). +func FormatHostnameError(h x509.HostnameError) string { + c := h.Certificate + if c == nil { + return "x509: cannot validate certificate for " + h.Host + } + + const maxNamesIncluded = 100 + + // Check if host is an IP address + if ip := net.ParseIP(h.Host); ip != nil { + 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) + } + var valid strings.Builder + for i, san := range c.IPAddresses { + if i > 0 { + valid.WriteString(", ") + } + valid.WriteString(san.String()) + } + return "x509: certificate is valid for " + valid.String() + ", not " + h.Host + } + + // DNS name validation + if len(c.DNSNames) == 0 { + return "x509: certificate is not valid for any names, but wanted to match " + h.Host + } + if len(c.DNSNames) >= maxNamesIncluded { + return fmt.Sprintf("x509: certificate is valid for %d names, but none matched %s", len(c.DNSNames), h.Host) + } + return "x509: certificate is valid for " + strings.Join(c.DNSNames, ", ") + ", not " + h.Host +} diff --git a/pkg/crypto/errors_test.go b/pkg/crypto/errors_test.go new file mode 100644 index 0000000000..a301cf1a72 --- /dev/null +++ b/pkg/crypto/errors_test.go @@ -0,0 +1,156 @@ +package crypto + +import ( + "crypto/x509" + "fmt" + "net" + "testing" +) + +func TestFormatHostnameError(t *testing.T) { + tests := []struct { + name string + err x509.HostnameError + expected string + }{ + { + name: "nil certificate", + err: x509.HostnameError{ + Host: "example.com", + Certificate: nil, + }, + expected: "x509: cannot validate certificate for example.com", + }, + { + name: "DNS name mismatch with single SAN", + err: x509.HostnameError{ + Host: "example.com", + Certificate: &x509.Certificate{ + DNSNames: []string{"other.com"}, + }, + }, + expected: "x509: certificate is valid for other.com, not example.com", + }, + { + name: "DNS name mismatch with multiple SANs", + err: x509.HostnameError{ + Host: "example.com", + Certificate: &x509.Certificate{ + DNSNames: []string{"foo.com", "bar.com", "baz.com"}, + }, + }, + expected: "x509: certificate is valid for foo.com, bar.com, baz.com, not example.com", + }, + { + name: "DNS name with no SANs", + err: x509.HostnameError{ + Host: "example.com", + Certificate: &x509.Certificate{ + DNSNames: []string{}, + }, + }, + expected: "x509: certificate is not valid for any names, but wanted to match example.com", + }, + { + name: "IP address mismatch with single IP SAN", + err: x509.HostnameError{ + Host: "192.168.1.1", + Certificate: &x509.Certificate{ + IPAddresses: []net.IP{net.ParseIP("10.0.0.1")}, + }, + }, + expected: "x509: certificate is valid for 10.0.0.1, not 192.168.1.1", + }, + { + name: "IP address mismatch with multiple IP SANs", + err: x509.HostnameError{ + Host: "192.168.1.1", + Certificate: &x509.Certificate{ + IPAddresses: []net.IP{ + net.ParseIP("10.0.0.1"), + net.ParseIP("10.0.0.2"), + }, + }, + }, + expected: "x509: certificate is valid for 10.0.0.1, 10.0.0.2, not 192.168.1.1", + }, + { + name: "IP address with no IP SANs", + err: x509.HostnameError{ + Host: "192.168.1.1", + Certificate: &x509.Certificate{ + IPAddresses: []net.IP{}, + }, + }, + expected: "x509: cannot validate certificate for 192.168.1.1 because it doesn't contain any IP SANs", + }, + { + name: "IPv6 address mismatch", + err: x509.HostnameError{ + Host: "::1", + Certificate: &x509.Certificate{ + IPAddresses: []net.IP{net.ParseIP("fe80::1")}, + }, + }, + expected: "x509: certificate is valid for fe80::1, not ::1", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := FormatHostnameError(tt.err) + if result != tt.expected { + t.Errorf("FormatHostnameError() = %q, want %q", result, tt.expected) + } + }) + } +} + +func TestFormatHostnameError_LargeNumberOfSANs(t *testing.T) { + // Test that we handle certificates with >= 100 SANs by showing count instead of listing + manyDNSNames := make([]string, 100) + for i := range manyDNSNames { + manyDNSNames[i] = fmt.Sprintf("host%d.example.com", i) + } + + manyIPs := make([]net.IP, 100) + for i := range manyIPs { + manyIPs[i] = net.ParseIP(fmt.Sprintf("10.0.0.%d", i%256)) + } + + tests := []struct { + name string + err x509.HostnameError + expected string + }{ + { + name: "many DNS SANs shows count", + err: x509.HostnameError{ + Host: "notfound.example.com", + Certificate: &x509.Certificate{ + DNSNames: manyDNSNames, + }, + }, + expected: "x509: certificate is valid for 100 names, but none matched notfound.example.com", + }, + { + name: "many IP SANs shows count", + err: x509.HostnameError{ + Host: "192.168.1.1", + Certificate: &x509.Certificate{ + IPAddresses: manyIPs, + }, + }, + expected: "x509: certificate is valid for 100 IP SANs, but none matched 192.168.1.1", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := FormatHostnameError(tt.err) + if result != tt.expected { + t.Errorf("FormatHostnameError() = %q, want %q", result, tt.expected) + } + }) + } +} diff --git a/pkg/oauth/tokenrequest/request_token.go b/pkg/oauth/tokenrequest/request_token.go index db1f7f4434..0d37714d54 100644 --- a/pkg/oauth/tokenrequest/request_token.go +++ b/pkg/oauth/tokenrequest/request_token.go @@ -21,6 +21,7 @@ import ( restclient "k8s.io/client-go/rest" "k8s.io/klog/v2" + "github.com/openshift/library-go/pkg/crypto" "github.com/openshift/library-go/pkg/oauth/oauthdiscovery" "github.com/openshift/library-go/pkg/oauth/tokenrequest/challengehandlers" ) @@ -551,7 +552,7 @@ func transportWithSystemRoots(issuer string, clientConfig *restclient.Config) (h resp.Body.Close() _, err = verifyServerCertChain(issuerURL.Hostname(), resp.TLS.PeerCertificates) - switch err.(type) { + switch err := err.(type) { case nil: // copy the config so we can freely mutate it configWithSystemRoots := restclient.CopyConfig(clientConfig) @@ -571,7 +572,11 @@ func transportWithSystemRoots(issuer string, clientConfig *restclient.Config) (h return nil, err } return systemRootsRT, nil - case x509.UnknownAuthorityError, x509.HostnameError, x509.CertificateInvalidError, x509.SystemRootsError, + case x509.HostnameError: + // fallback to the CA in the kubeconfig since the system roots did not work + klog.V(4).Infof("falling back to kubeconfig CA due to possible x509 error: %s", crypto.FormatHostnameError(err)) + return restclient.TransportFor(clientConfig) + case x509.UnknownAuthorityError, x509.CertificateInvalidError, x509.SystemRootsError, tls.RecordHeaderError, *net.OpError: // fallback to the CA in the kubeconfig since the system roots did not work // we are very broad on the errors here to avoid failing when we should fallback