From 674b2c9a81b40dfd2a6a4ab7c3dce47fc675c1f9 Mon Sep 17 00:00:00 2001
From: Michail Resvanis <mresvani@redhat.com>
Date: Tue, 4 Mar 2025 16:56:12 +0100
Subject: [PATCH] Filter out ocp certs from the external ones

When using RHACM to deploy disconnected IBU seed SNO clusters, the
trusted-ca-bundle, which should only contain external to OCP CA
certificates, can contain also OCP internal CA certificates. E.g.:

- internal-loadbalancer-serving
- service-network-serving-cert
- localhost-serving-cert
- external-loadbalancer-serving

This change filters out all certs found in there with their subject
containing openshift as the organisation unit (i.e. OU=openshift), as
they are not external certs and should not be ignored by recert when
regenerating and re-signing OCP certificates.

Signed-off-by: Michail Resvanis <mresvani@redhat.com>
---
 src/cluster_crypto/scanning/external_certs.rs | 80 ++++++++++++++++---
 1 file changed, 70 insertions(+), 10 deletions(-)

diff --git a/src/cluster_crypto/scanning/external_certs.rs b/src/cluster_crypto/scanning/external_certs.rs
index 3f4e4c2a..1bd96530 100644
--- a/src/cluster_crypto/scanning/external_certs.rs
+++ b/src/cluster_crypto/scanning/external_certs.rs
@@ -1,12 +1,28 @@
 use super::super::locations::K8sResourceLocation;
 use crate::k8s_etcd::get_etcd_json;
 use crate::k8s_etcd::InMemoryK8sEtcd;
-use anyhow::{bail, Context, Result};
+use anyhow::{Context, Result};
 use itertools::Itertools;
 use std::collections::HashSet;
 use std::sync::Arc;
 use x509_certificate::X509Certificate;
 
+/// Patterns used to identify OpenShift-specific certificates that should be
+/// filtered out from external certificates.
+const OPENSHIFT_CN_PATTERNS: &[&str] = &[
+    "ou=openshift",
+    "cn=ingress-operator@",
+    "cn=openshift-kube-apiserver-operator_localhost-recovery-serving-signer@",
+];
+
+/// Determines if a certificate is an OpenShift internal certificate by
+/// checking if its subject contains any of the OpenShift-specific patterns.
+/// The check is case-insensitive.
+fn is_openshift_certificate(subject: &str) -> bool {
+    let subject_lower = subject.to_lowercase();
+    OPENSHIFT_CN_PATTERNS.iter().any(|pattern| subject_lower.contains(*pattern))
+}
+
 pub(crate) async fn discover_external_certs(in_memory_etcd_client: Arc<InMemoryK8sEtcd>) -> Result<HashSet<String>> {
     let proxy_trusted_certs = vec![get_openshift_proxy_trusted_certs(&in_memory_etcd_client)
         .await
@@ -35,16 +51,22 @@ pub(crate) async fn discover_external_certs(in_memory_etcd_client: Arc<InMemoryK
     pem::parse_many(all_certs_bundled)
         .context("parsing")?
         .into_iter()
-        .map(|pem| match pem.tag() {
-            "CERTIFICATE" => Ok({
-                let crt = X509Certificate::from_der(pem.contents()).context("from der")?;
-                let cn = crt.subject_name().user_friendly_str().unwrap_or("undecodable".to_string());
-
-                log::trace!("Found external certificate: {}", cn);
+        .filter_map(|pem| match pem.tag() {
+            "CERTIFICATE" => match X509Certificate::from_der(pem.contents()) {
+                Ok(crt) => {
+                    let cn = crt.subject_name().user_friendly_str().unwrap_or("undecodable".to_string());
 
-                cn.to_string()
-            }),
-            _ => bail!("unexpected tag"),
+                    if is_openshift_certificate(&cn) {
+                        log::trace!("Ignoring OpenShift certificate found in external certificates: {}", cn);
+                        None
+                    } else {
+                        log::trace!("Found external certificate: {}", cn);
+                        Some(Ok(cn))
+                    }
+                }
+                Err(e) => Some(Err(anyhow::Error::new(e).context("from der"))),
+            },
+            _ => Some(Err(anyhow::anyhow!("unexpected tag"))),
         })
         .collect::<Result<HashSet<_>>>()
 }
@@ -134,3 +156,41 @@ pub(crate) async fn get_openshift_user_ca_bundle(in_memory_etcd_client: &Arc<InM
         )),
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_is_openshift_certificate() {
+        // These should match
+        assert!(is_openshift_certificate("OU=OpenShift,CN=service"));
+        assert!(is_openshift_certificate("CN=ingress-operator@something,O=cluster"));
+        assert!(is_openshift_certificate(
+            "CN=openshift-kube-apiserver-operator_localhost-recovery-serving-signer@example,DC=com"
+        ));
+        assert!(is_openshift_certificate("DC=com,OU=openshift,CN=service"));
+
+        // Case insensitivity tests
+        assert!(is_openshift_certificate("ou=openshift,CN=service"));
+        assert!(is_openshift_certificate("CN=INGRESS-OPERATOR@something,O=cluster"));
+
+        // Test with more complex DN strings
+        assert!(is_openshift_certificate("CN=service,OU=OpenShift,O=Example,L=City,ST=State,C=US"));
+        assert!(is_openshift_certificate(
+            "C=US,ST=State,L=City,O=Example,OU=Department,CN=ingress-operator@cluster"
+        ));
+
+        // These should not match
+        assert!(!is_openshift_certificate("CN=service,OU=kubernetes"));
+        assert!(!is_openshift_certificate("CN=operator,O=cluster"));
+        assert!(!is_openshift_certificate("CN=kube-apiserver,DC=example,DC=com"));
+        assert!(!is_openshift_certificate("DC=com,OU=other,CN=service"));
+
+        // Edge cases
+        assert!(!is_openshift_certificate(""));
+        assert!(!is_openshift_certificate("CN=almost-ingress-operator-but-not"));
+        assert!(!is_openshift_certificate("CN=openshift-kube-apiserver-operator-no-at-sign"));
+        assert!(!is_openshift_certificate("OUopenshift")); // No equals sign, should not match
+    }
+}