Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OCPBUGS-52178: Filter out OCP certs from the external ones #177

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 70 additions & 10 deletions src/cluster_crypto/scanning/external_certs.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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<_>>>()
}
Expand Down Expand Up @@ -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
}
}