OCPBUGS-45290: Block Upgrades for CA-Signed Certs Using SHA1

Previously, upgrades from 4.15 to 4.16 were only blocked for default
leaf certs using SHA1. However, in 4.16, any SHA1 cert that is CA-signed
(not self-signed) is unsupported. As a result, we were incorrectly
allowing upgrades for SHA1 intermediate certificates, while blocking
upgrades for self-signed SHA1 leaf certificates.

This fix blocks upgrades to 4.16 for IngressControllers with default
certificates containing CA-signed SHA1 certs. Root CA certs using SHA1
are still supported.

pkg/operator/controller/ingress/status.go

@@ -1,6 +1,7 @@
 package ingress
 
 import (
+	"bytes"
 	"context"
 	"crypto/x509"
 	"encoding/pem"
@@ -728,19 +729,63 @@ func checkDefaultCertificate(secret *corev1.Secret, domain string) error {
 		if cert.Subject.CommonName == domain && !foundSAN {
 			return fmt.Errorf("certificate in secret %s/%s has legacy Common Name (CN) but has no Subject Alternative Name (SAN) for domain: %s", secret.Namespace, secret.Name, domain)
 		}
-		// Prevent the upgrade only if the leaf certificate (the first certificate) uses a SHA1 signature algorithm.
-		// SHA1 can still be used by other certificates in the chain, such as the root and intermediate certificates.
-		if !cert.IsCA {
+		// Block the upgrade only if CA-signed certificates (not self-signed) uses a SHA1 signature algorithm.
+		// SHA1 can still be used by the root CA certificate.
+		if !isSelfSignedCert(cert) {
 			switch cert.SignatureAlgorithm {
-			case x509.SHA1WithRSA, x509.ECDSAWithSHA1:
-				return fmt.Errorf("certificate in secret %s/%s has weak SHA1 signature algorithm: %s (see https://docs.openshift.com/container-platform/4.16/release_notes/ocp-4-16-release-notes.html#ocp-4-16-sha-haproxy-support-removed_release-notes for more details)", secret.Namespace, secret.Name, cert.SignatureAlgorithm)
+			case x509.SHA1WithRSA, x509.ECDSAWithSHA1, x509.DSAWithSHA1:
+				return fmt.Errorf("a CA-signed certificate in secret %s/%s has weak SHA1 signature algorithm: %s (see https://docs.openshift.com/container-platform/4.16/release_notes/ocp-4-16-release-notes.html#ocp-4-16-sha-haproxy-support-removed_release-notes for more details)", secret.Namespace, secret.Name, cert.SignatureAlgorithm)
 			}
 		}
 	}
 
 	return nil
 }
 
+// isSelfSignedCert determines if a certificate is self-signed by verifying that the issuer matches the subject,
+// the authority key identifier matches the subject key identifier, and the public key algorithm matches the
+// signature algorithm. This logic mirrors the approach that OpenSSL uses to set the EXFLAG_SS flag, which
+// indicates a certificate is self-signed.
+// Ref: https://github.com/openssl/openssl/blob/b85e6f534906f0bf9114386d227e481d2336a0ff/crypto/x509/v3_purp.c#L557
+func isSelfSignedCert(cert *x509.Certificate) bool {
+	issuerIsEqualToSubject := bytes.Equal(cert.RawIssuer, cert.RawSubject)
+	authorityKeyIsEqualToSubjectKey := bytes.Equal(cert.AuthorityKeyId, cert.SubjectKeyId)
+	algorithmIsConsistent := signatureAlgorithmToPublicKeyAlgorithm(cert.SignatureAlgorithm) == cert.PublicKeyAlgorithm
+	return issuerIsEqualToSubject &&
+		(cert.AuthorityKeyId == nil || authorityKeyIsEqualToSubjectKey) &&
+		algorithmIsConsistent
+}
+
+// signatureAlgorithmToPublicKeyAlgorithm maps a SignatureAlgorithm to its corresponding PublicKeyAlgorithm.
+// Unfortunately, the x509 library does not expose a public mapping function for this.
+// Returns UnknownPublicKeyAlgorithm if the mapping is not recognized.
+func signatureAlgorithmToPublicKeyAlgorithm(sigAlgo x509.SignatureAlgorithm) x509.PublicKeyAlgorithm {
+	switch sigAlgo {
+	case x509.MD2WithRSA,
+		x509.MD5WithRSA,
+		x509.SHA1WithRSA,
+		x509.SHA256WithRSA,
+		x509.SHA384WithRSA,
+		x509.SHA512WithRSA,
+		x509.SHA256WithRSAPSS,
+		x509.SHA384WithRSAPSS,
+		x509.SHA512WithRSAPSS:
+		return x509.RSA
+	case x509.DSAWithSHA1,
+		x509.DSAWithSHA256:
+		return x509.DSA
+	case x509.ECDSAWithSHA1,
+		x509.ECDSAWithSHA256,
+		x509.ECDSAWithSHA384,
+		x509.ECDSAWithSHA512:
+		return x509.ECDSA
+	case x509.PureEd25519:
+		return x509.Ed25519
+	default:
+		return x509.UnknownPublicKeyAlgorithm
+	}
+}
+
 func formatConditions(conditions []*operatorv1.OperatorCondition) string {
 	var formatted string
 	if len(conditions) == 0 {

pkg/operator/controller/ingress/status_test.go

@@ -2246,6 +2246,11 @@ func Test_computeIngressUpgradeableCondition(t *testing.T) {
 		// If root cert is not provided, use the certTemplate to create a self-signed cert.
 		if rootCert == nil {
 			rootCert = certTemplate
+		} else {
+			// Go's x509.CreateCertificate assumes that if subject == issuer, the certificate is self-signed and does
+			// not add AuthorityKeyId. This fallback ensures AuthorityKeyId is included in cases where subject == issuer
+			// but the certificate is NOT self-signed. See https://github.com/golang/go/issues/62060.
+			certTemplate.AuthorityKeyId = rootCert.SubjectKeyId
 		}
 		var certBytes []byte
 		var privateKey crypto.PrivateKey
@@ -2297,14 +2302,21 @@ func Test_computeIngressUpgradeableCondition(t *testing.T) {
 	sha256ECDSACertWithSans, _ := makeCertificate(wildcardDomain, []string{wildcardDomain}, x509.ECDSAWithSHA256, nil, nil, false)
 	sha256ECDSACertWithNoSans, _ := makeCertificate(wildcardDomain, []string{}, x509.ECDSAWithSHA256, nil, nil, false)
 
-	sha1ECDSACert, _ := makeCertificate(wildcardDomain, []string{wildcardDomain}, x509.ECDSAWithSHA1, nil, nil, false)
-	sha1RSACert, _ := makeCertificate(wildcardDomain, []string{wildcardDomain}, x509.SHA1WithRSA, nil, nil, false)
+	sha1ECDSACertSelfSigned, _ := makeCertificate(wildcardDomain, []string{wildcardDomain}, x509.ECDSAWithSHA1, nil, nil, false)
+	sha1RSACertSelfSigned, _ := makeCertificate(wildcardDomain, []string{wildcardDomain}, x509.SHA1WithRSA, nil, nil, false)
 
-	sha1RSARootCA, sha1RSARootCAKey := makeCertificate(wildcardDomain, []string{wildcardDomain}, x509.SHA1WithRSA, nil, nil, true)
+	// CA's need to use different common name to make the subject != issuer.
+	sha1RSARootCA, sha1RSARootCAKey := makeCertificate("sha1RootCA.com", []string{wildcardDomain}, x509.SHA1WithRSA, nil, nil, true)
 	sha256RSACertSignedBySha1RSA, _ := makeCertificate(wildcardDomain, []string{wildcardDomain}, x509.SHA256WithRSA, sha1RSARootCA, sha1RSARootCAKey, false)
 
-	sha256RSARootCA, sha256RSARootCAKey := makeCertificate(wildcardDomain, []string{wildcardDomain}, x509.SHA256WithRSA, nil, nil, true)
-	sha1RSACertSignedBySha256RSA, _ := makeCertificate(wildcardDomain, []string{wildcardDomain}, x509.SHA1WithRSA, sha256RSARootCA, sha256RSARootCAKey, false)
+	sha256RSARootCA, sha256RSARootCAKey := makeCertificate("sha256RootCA.com", []string{wildcardDomain}, x509.SHA256WithRSA, nil, nil, true)
+	sha1RSACertSignedBySha256RSARootCA, _ := makeCertificate(wildcardDomain, []string{wildcardDomain}, x509.SHA1WithRSA, sha256RSARootCA, sha256RSARootCAKey, false)
+
+	sha1RSAIntCASignedBySha256RSARootCA, sha1RSAIntCASignedBySha256RSARootCAKey := makeCertificate("sha1IntCA.com", []string{wildcardDomain}, x509.SHA1WithRSA, sha256RSARootCA, sha256RSARootCAKey, true)
+	sha256RSACertSignedBySha1RSAIntCA, _ := makeCertificate(wildcardDomain, []string{wildcardDomain}, x509.SHA256WithRSA, sha1RSAIntCASignedBySha256RSARootCA, sha1RSAIntCASignedBySha256RSARootCAKey, false)
+
+	// Intentionally making the subject == issuer.
+	sha1RSACertSignedBySha256RSARootCASameSubjectIssuer, _ := makeCertificate("sha256RootCA.com", []string{wildcardDomain}, x509.SHA1WithRSA, sha256RSARootCA, sha256RSARootCAKey, false)
 
 	testCases := []struct {
 		description string
@@ -2352,13 +2364,13 @@ func Test_computeIngressUpgradeableCondition(t *testing.T) {
 		},
 		{
 			description: "if the self-signed default certificate uses ECDSA SHA1",
-			secret:      makeCertificateSecret(sha1ECDSACert),
-			expect:      false,
+			secret:      makeCertificateSecret(sha1ECDSACertSelfSigned),
+			expect:      true,
 		},
 		{
 			description: "if the self-signed default certificate uses RSA SHA1",
-			secret:      makeCertificateSecret(sha1RSACert),
-			expect:      false,
+			secret:      makeCertificateSecret(sha1RSACertSelfSigned),
+			expect:      true,
 		},
 		{
 			description: "if the default leaf certificate uses RSA SHA256 included with its root certificate that uses RSA SHA1",
@@ -2367,12 +2379,22 @@ func Test_computeIngressUpgradeableCondition(t *testing.T) {
 		},
 		{
 			description: "if the default leaf certificate uses RSA SHA1 included with its root certificate that uses RSA SHA256",
-			secret:      makeCertificateSecret(sha1RSACertSignedBySha256RSA, sha256RSARootCA),
+			secret:      makeCertificateSecret(sha1RSACertSignedBySha256RSARootCA, sha256RSARootCA),
 			expect:      false,
 		},
 		{
 			description: "if the default leaf certificate uses RSA SHA1 included with its root certificate that uses RSA SHA256, but the order is reversed",
-			secret:      makeCertificateSecret(sha256RSARootCA, sha1RSACertSignedBySha256RSA),
+			secret:      makeCertificateSecret(sha256RSARootCA, sha1RSACertSignedBySha256RSARootCA),
+			expect:      false,
+		},
+		{
+			description: "if the default root and leaf certificate uses RSA SHA256, but the intermediate certificate uses RSA SHA1",
+			secret:      makeCertificateSecret(sha256RSARootCA, sha1RSAIntCASignedBySha256RSARootCA, sha256RSACertSignedBySha1RSAIntCA),
+			expect:      false,
+		},
+		{
+			description: "if the default leaf certificate uses RSA SHA1, but the subject is the same as issuer's subject",
+			secret:      makeCertificateSecret(sha1RSACertSignedBySha256RSARootCASameSubjectIssuer),
 			expect:      false,
 		},
 	}