OCPBUGS-70835: tokenrequest: Use custom HostnameError formatting

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
+}

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)
+			}
+		})
+	}
+}

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