From afe9b27983926ebdbd0c60afc982837f85ebb59e Mon Sep 17 00:00:00 2001 From: Conrad Ludgate Date: Fri, 25 Apr 2025 10:09:14 +0100 Subject: [PATCH] fix(compute/tls): support for checking certificate chains (#11683) ## Problem It seems are production-ready cert-manager setup now includes a full certificate chain. This was not accounted for and the decoder would error. ## Summary of changes Change the way we decode certificates to support cert-chains, ignoring all but the first cert. This also changes a log line to not use multi-line errors. ~~I have tested this code manually against real certificates/keys, I didn't want to embed those in a test just yet, not until the cert expires in 24 hours.~~ --- Cargo.lock | 1 - compute_tools/Cargo.toml | 1 - compute_tools/src/tls.rs | 92 ++++++++++++++++++++++++++++++++++++++-- 3 files changed, 88 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4573629964..2cf260c88c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1323,7 +1323,6 @@ dependencies = [ "serde_json", "serde_with", "signal-hook", - "spki 0.7.3", "tar", "thiserror 1.0.69", "tokio", diff --git a/compute_tools/Cargo.toml b/compute_tools/Cargo.toml index d80ec41d34..8c1e7ad149 100644 --- a/compute_tools/Cargo.toml +++ b/compute_tools/Cargo.toml @@ -44,7 +44,6 @@ serde.workspace = true serde_with.workspace = true serde_json.workspace = true signal-hook.workspace = true -spki = { version = "0.7.3", features = ["std"] } tar.workspace = true tower.workspace = true tower-http.workspace = true diff --git a/compute_tools/src/tls.rs b/compute_tools/src/tls.rs index 8f465c7300..ab32a9459a 100644 --- a/compute_tools/src/tls.rs +++ b/compute_tools/src/tls.rs @@ -3,7 +3,6 @@ use std::{io::Write, os::unix::fs::OpenOptionsExt, path::Path, time::Duration}; use anyhow::{Context, Result, bail}; use compute_api::responses::TlsConfig; use ring::digest; -use spki::der::{Decode, PemReader}; use x509_cert::Certificate; #[derive(Clone, Copy)] @@ -52,7 +51,7 @@ pub fn update_key_path_blocking(pg_data: &Path, tls_config: &TlsConfig) { match try_update_key_path_blocking(pg_data, tls_config) { Ok(()) => break, Err(e) => { - tracing::error!("could not create key file {e:?}"); + tracing::error!(error = ?e, "could not create key file"); std::thread::sleep(Duration::from_secs(1)) } } @@ -92,8 +91,14 @@ fn try_update_key_path_blocking(pg_data: &Path, tls_config: &TlsConfig) -> Resul fn verify_key_cert(key: &str, cert: &str) -> Result<()> { use x509_cert::der::oid::db::rfc5912::ECDSA_WITH_SHA_256; - let cert = Certificate::decode(&mut PemReader::new(cert.as_bytes()).context("pem reader")?) - .context("decode cert")?; + let certs = Certificate::load_pem_chain(cert.as_bytes()) + .context("decoding PEM encoded certificates")?; + + // First certificate is our server-cert, + // all the rest of the certs are the CA cert chain. + let Some(cert) = certs.first() else { + bail!("no certificates found"); + }; match cert.signature_algorithm.oid { ECDSA_WITH_SHA_256 => { @@ -115,3 +120,82 @@ fn verify_key_cert(key: &str, cert: &str) -> Result<()> { Ok(()) } + +#[cfg(test)] +mod tests { + use super::verify_key_cert; + + /// Real certificate chain file, generated by cert-manager in dev. + /// The server auth certificate has expired since 2025-04-24T15:41:35Z. + const CERT: &str = " +-----BEGIN CERTIFICATE----- +MIICCDCCAa+gAwIBAgIQKhLomFcNULbZA/bPdGzaSzAKBggqhkjOPQQDAjBEMQsw +CQYDVQQGEwJVUzESMBAGA1UEChMJTmVvbiBJbmMuMSEwHwYDVQQDExhOZW9uIEs4 +cyBJbnRlcm1lZGlhdGUgQ0EwHhcNMjUwNDIzMTU0MTM1WhcNMjUwNDI0MTU0MTM1 +WjBBMT8wPQYDVQQDEzZjb21wdXRlLXdpc3B5LWdyYXNzLXcwY21laWp3LmRlZmF1 +bHQuc3ZjLmNsdXN0ZXIubG9jYWwwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAATF +QCcG2m/EVHAiZtSsYgVnHgoTjUL/Jtwfdrpvz2t0bVRZmBmSKhlo53uPV9Y5eKFG +AmR54p9/gT2eO3xU7vAgo4GFMIGCMA4GA1UdDwEB/wQEAwIFoDAMBgNVHRMBAf8E +AjAAMB8GA1UdIwQYMBaAFFR2JAhXkeiNQNEixTvAYIwxUu3QMEEGA1UdEQQ6MDiC +NmNvbXB1dGUtd2lzcHktZ3Jhc3MtdzBjbWVpancuZGVmYXVsdC5zdmMuY2x1c3Rl +ci5sb2NhbDAKBggqhkjOPQQDAgNHADBEAiBLG22wKG8XS9e9RxBT+kmUx/kIThcP +DIpp7jx0PrFcdQIgEMTdnXpx5Cv/Z0NIEDxtMHUD7G0vuRPfztki36JuakM= +-----END CERTIFICATE----- +-----BEGIN CERTIFICATE----- +MIICFzCCAb6gAwIBAgIUbbX98N2Ip6lWAONRk8dU9hSz+YIwCgYIKoZIzj0EAwIw +RDELMAkGA1UEBhMCVVMxEjAQBgNVBAoTCU5lb24gSW5jLjEhMB8GA1UEAxMYTmVv +biBBV1MgSW50ZXJtZWRpYXRlIENBMB4XDTI1MDQyMjE1MTAxMFoXDTI1MDcyMTE1 +MTAxMFowRDELMAkGA1UEBhMCVVMxEjAQBgNVBAoTCU5lb24gSW5jLjEhMB8GA1UE +AxMYTmVvbiBLOHMgSW50ZXJtZWRpYXRlIENBMFkwEwYHKoZIzj0CAQYIKoZIzj0D +AQcDQgAE5++m5owqNI4BPMTVNIUQH0qvU7pYhdpHGVGhdj/Lgars6ROvE6uSNQV4 +SAmJN5HBzj5/6kLQaTPWpXW7EHXjK6OBjTCBijAOBgNVHQ8BAf8EBAMCAQYwEgYD +VR0TAQH/BAgwBgEB/wIBADAdBgNVHQ4EFgQUVHYkCFeR6I1A0SLFO8BgjDFS7dAw +HwYDVR0jBBgwFoAUgHfNXfyKtHO0V9qoLOWCjkNiaI8wJAYDVR0eAQH/BBowGKAW +MBSCEi5zdmMuY2x1c3Rlci5sb2NhbDAKBggqhkjOPQQDAgNHADBEAiBObVFFdXaL +QpOXmN60dYUNnQRwjKreFduEkQgOdOlssgIgVAdJJQFgvlrvEOBhY8j5WyeKRwUN +k/ALs6KpgaFBCGY= +-----END CERTIFICATE----- +-----BEGIN CERTIFICATE----- +MIIB4jCCAYegAwIBAgIUFlxWFn/11yoGdmD+6gf+yQMToS0wCgYIKoZIzj0EAwIw +ODELMAkGA1UEBhMCVVMxEjAQBgNVBAoTCU5lb24gSW5jLjEVMBMGA1UEAxMMTmVv +biBSb290IENBMB4XDTI1MDQwMzA3MTUyMloXDTI2MDQwMzA3MTUyMlowRDELMAkG +A1UEBhMCVVMxEjAQBgNVBAoTCU5lb24gSW5jLjEhMB8GA1UEAxMYTmVvbiBBV1Mg +SW50ZXJtZWRpYXRlIENBMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEqonG/IQ6 +ZxtEtOUTkkoNopPieXDO5CBKUkNFTGeJEB7OxRlSpYJgsBpaYIaD6Vc4sVk3thIF +p+pLw52idQOIN6NjMGEwDgYDVR0PAQH/BAQDAgEGMA8GA1UdEwEB/wQFMAMBAf8w +HQYDVR0OBBYEFIB3zV38irRztFfaqCzlgo5DYmiPMB8GA1UdIwQYMBaAFKh7M4/G +FHvr/ORDQZt4bMLlJvHCMAoGCCqGSM49BAMCA0kAMEYCIQCbS4x7QPslONzBYbjC +UQaQ0QLDW4CJHvQ4u4gbWFG87wIhAJMsHQHjP9qTT27Q65zQCR7O8QeLAfha1jrH +Ag/LsxSr +-----END CERTIFICATE----- +"; + + /// The key corresponding to [`CERT`] + const KEY: &str = " +-----BEGIN EC PRIVATE KEY----- +MHcCAQEEIDnAnrqmIJjndCLWP1iIO5X3X63Aia48TGpGuMXwvm6IoAoGCCqGSM49 +AwEHoUQDQgAExUAnBtpvxFRwImbUrGIFZx4KE41C/ybcH3a6b89rdG1UWZgZkioZ +aOd7j1fWOXihRgJkeeKff4E9njt8VO7wIA== +-----END EC PRIVATE KEY----- +"; + + /// An incorrect key. + const INCORRECT_KEY: &str = " +-----BEGIN EC PRIVATE KEY----- +MHcCAQEEIL6WqqBDyvM0HWz7Ir5M5+jhFWB7IzOClGn26OPrzHCXoAoGCCqGSM49 +AwEHoUQDQgAE7XVvdOy5lfwtNKb+gJEUtnG+DrnnXLY5LsHDeGQKV9PTRcEMeCrG +YZzHyML4P6Sr4yi2ts+4B9i47uvAG8+XwQ== +-----END EC PRIVATE KEY----- +"; + + #[test] + fn certificate_verification() { + verify_key_cert(KEY, CERT).unwrap(); + } + + #[test] + #[should_panic(expected = "private key file does not match certificate")] + fn certificate_verification_fail() { + verify_key_cert(INCORRECT_KEY, CERT).unwrap(); + } +}