[release-4.15] OCPBUGS-28928: Prevent upgrades for SHA1 default cert and SHA1 route certs

go.mod

@@ -143,6 +143,6 @@ require (
 // github.com/operator-framework/operator-sdk.
 replace (
 	bitbucket.org/ww/goautoneg => github.com/munnerz/goautoneg v0.0.0-20120707110453-a547fc61f48d
-	github.com/openshift/api => github.com/openshift/api v0.0.0-20240131192415-e18b9cc8aa8b
+	github.com/openshift/api => github.com/openshift/api v0.0.0-20240522145529-93d6bda14341
 	k8s.io/client-go => k8s.io/client-go v0.28.0
 )

go.sum

@@ -918,8 +918,8 @@ github.com/opencontainers/runtime-spec v0.1.2-0.20190507144316-5b71a03e2700/go.m
 github.com/opencontainers/runtime-spec v0.1.2-0.20190618234442-a950415649c7/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0=
 github.com/opencontainers/runtime-spec v1.0.0/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0=
 github.com/opencontainers/runtime-tools v0.0.0-20181011054405-1d69bd0f9c39/go.mod h1:r3f7wjNzSs2extwzU3Y+6pKfobzPh+kKFJ3ofN+3nfs=
-github.com/openshift/api v0.0.0-20240131192415-e18b9cc8aa8b h1:+oWnXf9QvOlVG4Y5NJ18iiWMbbwsp5CND+jjM3M/qRA=
-github.com/openshift/api v0.0.0-20240131192415-e18b9cc8aa8b/go.mod h1:qNtV0315F+f8ld52TLtPvrfivZpdimOzTi3kn9IVbtU=
+github.com/openshift/api v0.0.0-20240522145529-93d6bda14341 h1:JQpzgk+p24rkgNbNsrNR0yLm63WTKapuT60INU5BqT8=
+github.com/openshift/api v0.0.0-20240522145529-93d6bda14341/go.mod h1:qNtV0315F+f8ld52TLtPvrfivZpdimOzTi3kn9IVbtU=
 github.com/openshift/build-machinery-go v0.0.0-20200211121458-5e3d6e570160/go.mod h1:1CkcsT3aVebzRBzVTSbiKSkJMsC/CASqxesfqEMfJEc=
 github.com/openshift/client-go v0.0.0-20200116152001-92a2713fa240/go.mod h1:4riOwdj99Hd/q+iAcJZfNCsQQQMwURnZV6RL4WHYS5w=
 github.com/openshift/client-go v0.0.0-20230120202327-72f107311084 h1:66uaqNwA+qYyQDwsMWUfjjau8ezmg1dzCqub13KZOcE=

pkg/operator/controller/ingress/status.go

@@ -689,15 +689,18 @@ func computeIngressEvaluationConditionsDetectedCondition(ic *operatorv1.IngressC
 }
 
 // checkDefaultCertificate returns an error value indicating whether the default
-// certificate is safe for upgrades.  In particular, if the current default
-// certificate specifies a Subject Alternative Name (SAN) for the ingress
-// domain, then it is safe to upgrade, and the return value is nil.  Otherwise,
-// if the certificate has a legacy Common Name (CN) and no SAN, then the return
+// certificate is safe for upgrades. There are two scenarios we handle:
+// 1.If the current default certificate specifies a legacy Common Name (CN) and no
+// Subject Alternative Name (SAN) for the ingress domain, then the return
 // value is an error indicating that the certificate must be replaced by one
-// with a SAN before upgrading is allowed.  This check is necessary because
-// OpenShift 4.10 and newer are built using Go 1.17, which rejects certificates
-// without SANs.  Note that this function only checks the validity of the
-// certificate insofar as it affects upgrades.
+// with a SAN. This check is necessary because OpenShift 4.10 and newer are
+// built using Go 1.17, which rejects certificates without SANs.
+// 2. If the current default certificate uses a SHA1-based signature algorithm, then
+// the return value is an error indicating SHA1 is too weak. This check is necessary
+// because OpenShift 4.16+ uses RHEL 9 and OpenSSL 3.0, which disallow SHA1.
+// Note that this function only checks the validity of the certificate insofar as it
+// affects upgrades. If the default certificate is safe for upgrades, then the return
+// value is nil.
 func checkDefaultCertificate(secret *corev1.Secret, domain string) error {
 	var certData []byte
 	if v, ok := secret.Data["tls.crt"]; !ok {
@@ -725,6 +728,10 @@ 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)
 		}
+		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)
+		}
 	}
 
 	return nil

pkg/operator/controller/ingress/status_test.go

@@ -1,6 +1,8 @@
 package ingress
 
 import (
+	"crypto/ecdsa"
+	"crypto/elliptic"
 	"crypto/rand"
 	"crypto/rsa"
 	"crypto/x509"
@@ -2213,20 +2215,36 @@ func Test_checkZoneInConfig(t *testing.T) {
 }
 
 func Test_computeIngressUpgradeableCondition(t *testing.T) {
-	makeDefaultCertificateSecret := func(cn string, sans []string) *corev1.Secret {
-		key, err := rsa.GenerateKey(rand.Reader, 2048)
-		if err != nil {
-			t.Fatalf("failed to generate key: %v", err)
-		}
-
+	makeDefaultCertificateSecret := func(cn string, sans []string, signatureAlgorithm x509.SignatureAlgorithm) *corev1.Secret {
 		certTemplate := &x509.Certificate{
-			SerialNumber: big.NewInt(1),
-			Subject:      pkix.Name{CommonName: cn},
-			DNSNames:     sans,
+			SerialNumber:       big.NewInt(1),
+			Subject:            pkix.Name{CommonName: cn},
+			DNSNames:           sans,
+			SignatureAlgorithm: signatureAlgorithm,
 		}
-		cert, err := x509.CreateCertificate(rand.Reader, certTemplate, certTemplate, &key.PublicKey, key)
-		if err != nil {
-			t.Fatalf("failed to generate certificate: %v", err)
+
+		var cert []byte
+		switch signatureAlgorithm {
+		case x509.SHA1WithRSA, x509.SHA256WithRSA, x509.SHA384WithRSA, x509.SHA512WithRSA:
+			key, err := rsa.GenerateKey(rand.Reader, 2048)
+			if err != nil {
+				t.Fatalf("failed to generate RSA key: %v", err)
+			}
+			cert, err = x509.CreateCertificate(rand.Reader, certTemplate, certTemplate, &key.PublicKey, key)
+			if err != nil {
+				t.Fatalf("failed to generate RSA certificate: %v", err)
+			}
+		case x509.ECDSAWithSHA1, x509.ECDSAWithSHA256, x509.ECDSAWithSHA384, x509.ECDSAWithSHA512:
+			key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+			if err != nil {
+				t.Fatalf("failed to generate ECDSA key: %v", err)
+			}
+			cert, err = x509.CreateCertificate(rand.Reader, certTemplate, certTemplate, &key.PublicKey, key)
+			if err != nil {
+				t.Fatalf("failed to generate ECDSA certificate: %v", err)
+			}
+		default:
+			t.Fatal("unsupported signature algorithm")
 		}
 
 		certData := pem.EncodeToMemory(&pem.Block{
@@ -2278,12 +2296,22 @@ func Test_computeIngressUpgradeableCondition(t *testing.T) {
 		},
 		{
 			description: "if the default certificate has a SAN",
-			secret:      makeDefaultCertificateSecret(wildcardDomain, []string{wildcardDomain}),
+			secret:      makeDefaultCertificateSecret(wildcardDomain, []string{wildcardDomain}, x509.ECDSAWithSHA256),
 			expect:      true,
 		},
 		{
 			description: "if the default certificate has no SAN",
-			secret:      makeDefaultCertificateSecret(wildcardDomain, []string{}),
+			secret:      makeDefaultCertificateSecret(wildcardDomain, []string{}, x509.ECDSAWithSHA256),
+			expect:      false,
+		},
+		{
+			description: "if the default certificate uses ECDSA SHA1",
+			secret:      makeDefaultCertificateSecret(wildcardDomain, []string{wildcardDomain}, x509.ECDSAWithSHA1),
+			expect:      false,
+		},
+		{
+			description: "if the default certificate uses RSA SHA1",
+			secret:      makeDefaultCertificateSecret(wildcardDomain, []string{wildcardDomain}, x509.SHA1WithRSA),
 			expect:      false,
 		},
 	}
@@ -2333,7 +2361,7 @@ func Test_computeIngressUpgradeableCondition(t *testing.T) {
 			}
 			secret := tc.secret
 			if secret == nil {
-				secret = makeDefaultCertificateSecret("", []string{wildcardDomain})
+				secret = makeDefaultCertificateSecret("", []string{wildcardDomain}, x509.SHA256WithRSA)
 			}
 
 			expectedStatus := operatorv1.ConditionFalse

pkg/operator/controller/names.go

@@ -292,3 +292,12 @@ func GatewayDNSRecordName(gateway *gatewayapiv1beta1.Gateway, host string) types
 		Name:      fmt.Sprintf("%s-%s-wildcard", gateway.Name, util.Hash(host)),
 	}
 }
+
+// AdminGatesConfigMapName returns the namespaced name for the
+// configmap that contains admin gates.
+func AdminGatesConfigMapName() types.NamespacedName {
+	return types.NamespacedName{
+		Name:      "admin-gates",
+		Namespace: GlobalMachineSpecifiedConfigNamespace,
+	}
+}

pkg/operator/controller/route-metrics/controller.go

@@ -39,20 +39,11 @@ var (
 // New creates the route metrics controller. This is the controller
 // that handles all the logic for gathering and exporting
 // metrics related to route resources.
-func New(mgr manager.Manager, namespace string) (controller.Controller, error) {
-	// Create a new cache to watch on Route objects from every namespace.
-	newCache, err := cache.New(mgr.GetConfig(), cache.Options{
-		Scheme: mgr.GetScheme(),
-	})
-	if err != nil {
-		return nil, err
-	}
-	// Add the cache to the manager so that the cache is started along with the other runnables.
-	mgr.Add(newCache)
+func New(mgr manager.Manager, config Config) (controller.Controller, error) {
 	operatorCache := mgr.GetCache()
 	reconciler := &reconciler{
-		cache:            newCache,
-		namespace:        namespace,
+		cache:            config.Cache,
+		namespace:        config.Namespace,
 		routeToIngresses: make(map[types.NamespacedName]sets.String),
 	}
 	c, err := controller.New(controllerName, mgr, controller.Options{
@@ -74,7 +65,7 @@ func New(mgr manager.Manager, namespace string) (controller.Controller, error) {
 		return nil, err
 	}
 	// add watch for changes in Route
-	if err := c.Watch(source.Kind(newCache, &routev1.Route{}),
+	if err := c.Watch(source.Kind(reconciler.cache, &routev1.Route{}),
 		handler.EnqueueRequestsFromMapFunc(reconciler.routeToIngressController)); err != nil {
 		return nil, err
 	}
@@ -144,6 +135,16 @@ func (r *reconciler) routeToIngressController(context context.Context, obj clien
 	return requests
 }
 
+// Config holds all the configuration that must be provided when creating the
+// controller.
+type Config struct {
+	// Namespace specifies the namespace where IngressControllers reside.
+	Namespace string
+	// Cache should be a namespace global cache since routes
+	// are not restricted to any particular namespace.
+	Cache cache.Cache
+}
+
 // reconciler handles the actual ingresscontroller reconciliation logic in response to events.
 type reconciler struct {
 	cache     cache.Cache