From 651c78a00c8d0a0aeece640787730e7c26a53c5e Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Wed, 21 May 2025 13:53:33 +0200 Subject: [PATCH 01/19] feat!(stackable-certs): Support adding SAN entries --- crates/stackable-certs/src/ca/consts.rs | 6 +- crates/stackable-certs/src/ca/mod.rs | 118 ++++++++++++++++++------ crates/stackable-webhook/src/tls.rs | 9 +- 3 files changed, 102 insertions(+), 31 deletions(-) diff --git a/crates/stackable-certs/src/ca/consts.rs b/crates/stackable-certs/src/ca/consts.rs index 600bb5f7c..125a63a05 100644 --- a/crates/stackable-certs/src/ca/consts.rs +++ b/crates/stackable-certs/src/ca/consts.rs @@ -1,5 +1,7 @@ +use stackable_operator::time::Duration; + /// The default CA validity time span of one hour (3600 seconds). -pub const DEFAULT_CA_VALIDITY_SECONDS: u64 = 3600; +pub const DEFAULT_CA_VALIDITY: Duration = Duration::from_hours_unchecked(1); /// The root CA subject name containing only the common name. -pub const ROOT_CA_SUBJECT: &str = "CN=Stackable Data Platform Internal CA"; +pub const SDP_ROOT_CA_SUBJECT: &str = "CN=Stackable Data Platform Internal CA"; diff --git a/crates/stackable-certs/src/ca/mod.rs b/crates/stackable-certs/src/ca/mod.rs index 7c793d4f8..6334272a0 100644 --- a/crates/stackable-certs/src/ca/mod.rs +++ b/crates/stackable-certs/src/ca/mod.rs @@ -1,6 +1,6 @@ //! Contains types and functions to generate and sign certificate authorities //! (CAs). -use std::str::FromStr; +use std::{fmt::Debug, str::FromStr}; use const_oid::db::rfc5280::{ID_KP_CLIENT_AUTH, ID_KP_SERVER_AUTH}; use k8s_openapi::api::core::v1::Secret; @@ -9,9 +9,10 @@ use snafu::{OptionExt, ResultExt, Snafu}; use stackable_operator::{client::Client, commons::secret::SecretReference, time::Duration}; use tracing::{debug, instrument}; use x509_cert::{ + Certificate, builder::{Builder, CertificateBuilder, Profile}, - der::{DecodePem, pem::LineEnding, referenced::OwnedToRef}, - ext::pkix::{AuthorityKeyIdentifier, ExtendedKeyUsage}, + der::{DecodePem, asn1::Ia5String, pem::LineEnding, referenced::OwnedToRef}, + ext::pkix::{AuthorityKeyIdentifier, ExtendedKeyUsage, SubjectAltName, name::GeneralName}, name::Name, serial_number::SerialNumber, spki::{EncodePublicKey, SubjectPublicKeyInfoOwned}, @@ -66,14 +67,20 @@ pub enum Error { #[snafu(display("failed to parse AuthorityKeyIdentifier"))] ParseAuthorityKeyIdentifier { source: x509_cert::der::Error }, + + #[snafu(display("The subject alternative DNS name \"{dns_name}\" is not a Ia5String"))] + SaDnsNameNotAIa5String { + dns_name: String, + source: x509_cert::der::Error, + }, } /// Custom implementation of [`std::cmp::PartialEq`] because some inner types /// don't implement it. /// -/// Note that this implementation is restritced to testing because there is a +/// Note that this implementation is restricted to testing because there is a /// variant that is impossible to compare, and will cause a panic if it is -/// attemped. +/// attempted. #[cfg(test)] impl PartialEq for Error { fn eq(&self, other: &Self) -> bool { @@ -170,7 +177,7 @@ where /// These parameters include: /// /// - a randomly generated serial number - /// - a default validity of one hour (see [`DEFAULT_CA_VALIDITY_SECONDS`]) + /// - a default validity of one hour (see [`DEFAULT_CA_VALIDITY`]) /// /// The CA contains the public half of the provided `signing_key` and is /// signed by the private half of said key. @@ -181,9 +188,8 @@ where #[instrument(name = "create_certificate_authority", skip(signing_key_pair))] pub fn new(signing_key_pair: S) -> Result { let serial_number = rand::random::(); - let validity = Duration::from_secs(DEFAULT_CA_VALIDITY_SECONDS); - Self::new_with(signing_key_pair, serial_number, validity) + Self::new_with(signing_key_pair, serial_number, DEFAULT_CA_VALIDITY) } /// Creates a new CA certificate. @@ -200,8 +206,8 @@ where // We don't allow customization of the CA subject by callers. Every CA // created by us should contain the same subject consisting a common set // of distinguished names (DNs). - let subject = Name::from_str(ROOT_CA_SUBJECT).context(ParseSubjectSnafu { - subject: ROOT_CA_SUBJECT, + let subject = Name::from_str(SDP_ROOT_CA_SUBJECT).context(ParseSubjectSnafu { + subject: SDP_ROOT_CA_SUBJECT, })?; let spki_pem = signing_key_pair @@ -267,15 +273,16 @@ where /// authentication, because they include [`ID_KP_CLIENT_AUTH`] and /// [`ID_KP_SERVER_AUTH`] in the extended key usage extension. /// - /// It is also possible to directly greate RSA or ECDSA-based leaf + /// It is also possible to directly create RSA or ECDSA-based leaf /// certificates using [`CertificateAuthority::generate_rsa_leaf_certificate`] /// and [`CertificateAuthority::generate_ecdsa_leaf_certificate`]. #[instrument(skip(self, key_pair))] - pub fn generate_leaf_certificate( + pub fn generate_leaf_certificate<'a, T>( &mut self, key_pair: T, name: &str, scope: &str, + subject_alterative_dns_names: impl IntoIterator + Debug, validity: Duration, ) -> Result> where @@ -301,10 +308,6 @@ where let spki = SubjectPublicKeyInfoOwned::from_pem(spki_pem.as_bytes()) .context(DecodeSpkiFromPemSnafu)?; - // The leaf certificate can be used for WWW client and server - // authentication. This is a base requirement for TLS certs. - let eku = ExtendedKeyUsage(vec![ID_KP_CLIENT_AUTH, ID_KP_SERVER_AUTH]); - let signer = self.certificate_pair.key_pair.signing_key(); let mut builder = CertificateBuilder::new( Profile::Leaf { @@ -325,9 +328,27 @@ where ) .context(CreateCertificateBuilderSnafu)?; - // Again, add the extension created above. + // The leaf certificate can be used for WWW client and server + // authentication. This is a base requirement for TLS certs. builder - .add_extension(&eku) + .add_extension(&ExtendedKeyUsage(vec![ + ID_KP_CLIENT_AUTH, + ID_KP_SERVER_AUTH, + ])) + .context(AddCertificateExtensionSnafu)?; + + let sans = subject_alterative_dns_names + .into_iter() + .map(|dns_name| { + Ok(GeneralName::DnsName(Ia5String::new(dns_name).context( + SaDnsNameNotAIa5StringSnafu { + dns_name: dns_name.to_string(), + }, + )?)) + }) + .collect::, Error>>()?; + builder + .add_extension(&SubjectAltName(sans)) .context(AddCertificateExtensionSnafu)?; debug!("create and sign leaf certificate"); @@ -344,14 +365,15 @@ where /// See [`CertificateAuthority::generate_leaf_certificate`] for more /// information. #[instrument(skip(self))] - pub fn generate_rsa_leaf_certificate( + pub fn generate_rsa_leaf_certificate<'a>( &mut self, name: &str, scope: &str, + subject_alterative_dns_names: impl IntoIterator + Debug, validity: Duration, ) -> Result> { let key = rsa::SigningKey::new().context(GenerateRsaSigningKeySnafu)?; - self.generate_leaf_certificate(key, name, scope, validity) + self.generate_leaf_certificate(key, name, scope, subject_alterative_dns_names, validity) } /// Generates an ECDSAasync -based leaf certificate which is signed by this CA. @@ -359,14 +381,15 @@ where /// See [`CertificateAuthority::generate_leaf_certificate`] for more /// information. #[instrument(skip(self))] - pub fn generate_ecdsa_leaf_certificate( + pub fn generate_ecdsa_leaf_certificate<'a>( &mut self, name: &str, scope: &str, + subject_alterative_dns_names: impl IntoIterator + Debug, validity: Duration, ) -> Result> { let key = ecdsa::SigningKey::new().context(GenerateEcdsaSigningKeySnafu)?; - self.generate_leaf_certificate(key, name, scope, validity) + self.generate_leaf_certificate(key, name, scope, subject_alterative_dns_names, validity) } /// Create a [`CertificateAuthority`] from a Kubernetes [`Secret`]. @@ -443,6 +466,11 @@ where Self::from_secret(secret, key_certificate, key_private_key) } + + /// Returns the ca certificate. + pub fn ca_cert(&self) -> &Certificate { + &self.certificate_pair.certificate + } } impl CertificateAuthority { @@ -468,19 +496,57 @@ fn format_leaf_certificate_subject(name: &str, scope: &str) -> Result { #[cfg(test)] mod tests { + use const_oid::ObjectIdentifier; + use super::*; + const TEST_CERT_LIFETIME: Duration = Duration::from_hours_unchecked(1); + const TEST_SAN: &str = "airflow-0.airflow.default.svc.cluster.local"; + #[tokio::test] async fn rsa_key_generation() { let mut ca = CertificateAuthority::new_rsa().unwrap(); - ca.generate_rsa_leaf_certificate("Airflow", "pod", Duration::from_secs(3600)) - .unwrap(); + let cert = ca + .generate_rsa_leaf_certificate("Airflow", "pod", [TEST_SAN], TEST_CERT_LIFETIME) + .expect("RSA certificate generation failed"); + + assert_cert_attributes(cert.certificate()); } #[tokio::test] async fn ecdsa_key_generation() { let mut ca = CertificateAuthority::new_ecdsa().unwrap(); - ca.generate_ecdsa_leaf_certificate("Airflow", "pod", Duration::from_secs(3600)) - .unwrap(); + let cert = ca + .generate_ecdsa_leaf_certificate("Airflow", "pod", [TEST_SAN], TEST_CERT_LIFETIME) + .expect("ecdsa certificate generation failed"); + + assert_cert_attributes(cert.certificate()); + } + + fn assert_cert_attributes(cert: &Certificate) { + let cert = &cert.tbs_certificate; + // Test subject + assert_eq!( + cert.subject, + Name::from_str("CN=Airflow Certificate for pod").unwrap() + ); + + // Test SAN extension is present + let extensions = cert.extensions.as_ref().expect("cert had no extension"); + assert!( + extensions + .iter() + .any(|ext| ext.extn_id == ObjectIdentifier::new_unwrap("2.5.29.17")) + ); + + // Test lifetime + let not_before = cert.validity.not_before.to_system_time(); + let not_after = cert.validity.not_after.to_system_time(); + assert_eq!( + not_after + .duration_since(not_before) + .expect("Failed to calculate duration between notBefore and notAfter"), + *TEST_CERT_LIFETIME + ); } } diff --git a/crates/stackable-webhook/src/tls.rs b/crates/stackable-webhook/src/tls.rs index 92d9eb345..2aad52ee4 100644 --- a/crates/stackable-webhook/src/tls.rs +++ b/crates/stackable-webhook/src/tls.rs @@ -8,8 +8,11 @@ use hyper::{body::Incoming, service::service_fn}; use hyper_util::rt::{TokioExecutor, TokioIo}; use opentelemetry::trace::{FutureExt, SpanKind}; use snafu::{ResultExt, Snafu}; -use stackable_certs::{CertificatePairError, ca::CertificateAuthority, keys::rsa}; -use stackable_operator::time::Duration; +use stackable_certs::{ + CertificatePairError, + ca::{CertificateAuthority, DEFAULT_CA_VALIDITY}, + keys::rsa, +}; use tokio::net::TcpListener; use tokio_rustls::{ TlsAcceptor, @@ -106,7 +109,7 @@ impl TlsServer { CertificateAuthority::new_rsa().context(CreateCertificateAuthoritySnafu)?; let leaf_certificate = certificate_authority - .generate_rsa_leaf_certificate("Leaf", "webhook", Duration::from_secs(3600)) + .generate_rsa_leaf_certificate("Leaf", "webhook", [], DEFAULT_CA_VALIDITY) .context(GenerateLeafCertificateSnafu)?; let certificate_der = leaf_certificate From 6b79029a9f2c5af5ca74711556d832148ac95e4e Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Wed, 21 May 2025 13:54:35 +0200 Subject: [PATCH 02/19] changelog --- crates/stackable-certs/CHANGELOG.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/crates/stackable-certs/CHANGELOG.md b/crates/stackable-certs/CHANGELOG.md index 0f69fa198..109d1a6ec 100644 --- a/crates/stackable-certs/CHANGELOG.md +++ b/crates/stackable-certs/CHANGELOG.md @@ -4,6 +4,16 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Added + +- BREAKING: `CertificateAuthority::generate_leaf_certificate` (and `generate_rsa_leaf_certificate` and `generate_ecdsa_leaf_certificate`) + now take an additional parameter `subject_alterative_dns_names`. The passed SANs are added to the generated certificate, + this is needed for basically all modern TLS certificate validations when used with HTTPS. + Pass an empty list (`[]`) to keep the existing behavior ([#1044]). +- BREAKING: The constant `DEFAULT_CA_VALIDITY_SECONDS` has been renamed to `DEFAULT_CA_VALIDITY` and now is of type `stackable_operator::time::Duration`. + Also, the constant `ROOT_CA_SUBJECT` has been renamed to `SDP_ROOT_CA_SUBJECT` ([#1044]). +- Added the function `CertificateAuthority::ca_cert` to easily get the CA `Certificate` ([#1044]). + ## [0.3.1] - 2024-07-10 ### Changed @@ -11,6 +21,7 @@ All notable changes to this project will be documented in this file. - Bump rust-toolchain to 1.79.0 ([#822]). [#822]: https://github.com/stackabletech/operator-rs/pull/822 +[#1044]: https://github.com/stackabletech/operator-rs/pull/1044 ## [0.3.0] - 2024-05-08 From 88d850d14abe56df13af230c2b74eaa5cb5cfd1e Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 23 May 2025 09:50:50 +0200 Subject: [PATCH 03/19] Rewrite CA and cert generation to use builder pattern --- Cargo.lock | 26 + Cargo.toml | 1 + crates/stackable-certs/CHANGELOG.md | 11 +- crates/stackable-certs/Cargo.toml | 1 + crates/stackable-certs/src/ca/ca_builder.rs | 241 +++++++++ crates/stackable-certs/src/ca/consts.rs | 8 +- crates/stackable-certs/src/ca/k8s.rs | 127 +++++ crates/stackable-certs/src/ca/mod.rs | 543 +------------------- crates/stackable-certs/src/cert_builder.rs | 319 ++++++++++++ crates/stackable-certs/src/keys/ecdsa.rs | 22 +- crates/stackable-certs/src/keys/mod.rs | 19 +- crates/stackable-certs/src/keys/rsa.rs | 22 +- crates/stackable-certs/src/lib.rs | 15 +- crates/stackable-webhook/src/tls.rs | 45 +- 14 files changed, 821 insertions(+), 579 deletions(-) create mode 100644 crates/stackable-certs/src/ca/ca_builder.rs create mode 100644 crates/stackable-certs/src/ca/k8s.rs create mode 100644 crates/stackable-certs/src/cert_builder.rs diff --git a/Cargo.lock b/Cargo.lock index 7d4052e05..805c5ca50 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -292,6 +292,31 @@ dependencies = [ "generic-array", ] +[[package]] +name = "bon" +version = "3.6.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ced38439e7a86a4761f7f7d5ded5ff009135939ecb464a24452eaa4c1696af7d" +dependencies = [ + "bon-macros", + "rustversion", +] + +[[package]] +name = "bon-macros" +version = "3.6.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0ce61d2d3844c6b8d31b2353d9f66cf5e632b3e9549583fe3cac2f4f6136725e" +dependencies = [ + "darling", + "ident_case", + "prettyplease", + "proc-macro2", + "quote", + "rustversion", + "syn 2.0.101", +] + [[package]] name = "bstr" version = "1.12.0" @@ -2934,6 +2959,7 @@ checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3" name = "stackable-certs" version = "0.3.1" dependencies = [ + "bon", "const-oid", "ecdsa", "k8s-openapi", diff --git a/Cargo.toml b/Cargo.toml index 29c73bc9e..65ec0808a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,6 +12,7 @@ repository = "https://github.com/stackabletech/operator-rs" product-config = { git = "https://github.com/stackabletech/product-config.git", tag = "0.7.0" } axum = { version = "0.8.1", features = ["http2"] } +bon = "3.6.3" chrono = { version = "0.4.38", default-features = false } clap = { version = "4.5.17", features = ["derive", "cargo", "env"] } const_format = "0.2.33" diff --git a/crates/stackable-certs/CHANGELOG.md b/crates/stackable-certs/CHANGELOG.md index 109d1a6ec..625892d0a 100644 --- a/crates/stackable-certs/CHANGELOG.md +++ b/crates/stackable-certs/CHANGELOG.md @@ -6,13 +6,14 @@ All notable changes to this project will be documented in this file. ### Added -- BREAKING: `CertificateAuthority::generate_leaf_certificate` (and `generate_rsa_leaf_certificate` and `generate_ecdsa_leaf_certificate`) - now take an additional parameter `subject_alterative_dns_names`. The passed SANs are added to the generated certificate, - this is needed for basically all modern TLS certificate validations when used with HTTPS. - Pass an empty list (`[]`) to keep the existing behavior ([#1044]). +- Support adding SAN entries to generated certificates, this is needed for basically all modern TLS + certificate validations when used with HTTPS ([#1044]). - BREAKING: The constant `DEFAULT_CA_VALIDITY_SECONDS` has been renamed to `DEFAULT_CA_VALIDITY` and now is of type `stackable_operator::time::Duration`. Also, the constant `ROOT_CA_SUBJECT` has been renamed to `SDP_ROOT_CA_SUBJECT` ([#1044]). -- Added the function `CertificateAuthority::ca_cert` to easily get the CA `Certificate` ([#1044]). + +### Changed + +- GIGA BREAKING: Rewrite entire CA and cert generation to use a builder pattern ([#1044]). ## [0.3.1] - 2024-07-10 diff --git a/crates/stackable-certs/Cargo.toml b/crates/stackable-certs/Cargo.toml index be109d1ae..7dbf0682f 100644 --- a/crates/stackable-certs/Cargo.toml +++ b/crates/stackable-certs/Cargo.toml @@ -13,6 +13,7 @@ rustls = ["dep:tokio-rustls"] [dependencies] stackable-operator = { path = "../stackable-operator" } +bon.workspace = true const-oid.workspace = true ecdsa.workspace = true k8s-openapi.workspace = true diff --git a/crates/stackable-certs/src/ca/ca_builder.rs b/crates/stackable-certs/src/ca/ca_builder.rs new file mode 100644 index 000000000..04cc4eea5 --- /dev/null +++ b/crates/stackable-certs/src/ca/ca_builder.rs @@ -0,0 +1,241 @@ +use bon::Builder; +use rsa::pkcs8::EncodePublicKey; +use snafu::{ResultExt, Snafu}; +use stackable_operator::time::Duration; +use tracing::debug; +use x509_cert::{ + builder::{Builder, CertificateBuilder, Profile}, + der::{DecodePem, referenced::OwnedToRef}, + ext::pkix::AuthorityKeyIdentifier, + name::Name, + serial_number::SerialNumber, + spki::SubjectPublicKeyInfoOwned, + time::Validity, +}; + +use super::CertificateAuthority; +use crate::{ + CertificatePair, + ca::{DEFAULT_CA_VALIDITY, PEM_LINE_ENDING, SDP_ROOT_CA_SUBJECT}, + keys::CertificateKeypair, +}; + +/// Defines all error variants which can occur when creating a CA +#[derive(Debug, Snafu)] +pub enum CreateCertificateAuthorityError +where + E: std::error::Error + 'static, +{ + #[snafu(display("failed to parse validity"))] + ParseValidity { source: x509_cert::der::Error }, + + #[snafu(display("failed to parse \"{subject}\" as subject"))] + ParseSubject { + source: x509_cert::der::Error, + subject: String, + }, + + #[snafu(display("failed to create signing key pair"))] + CreateSigningKeyPair { source: E }, + + #[snafu(display("failed to serialize public key as PEM"))] + SerializePublicKey { source: x509_cert::spki::Error }, + + #[snafu(display("failed to decode SPKI from PEM"))] + DecodeSpkiFromPem { source: x509_cert::der::Error }, + + #[snafu(display("failed to parse AuthorityKeyIdentifier"))] + ParseAuthorityKeyIdentifier { source: x509_cert::der::Error }, + + #[snafu(display("failed to create certificate builder"))] + CreateCertificateBuilder { source: x509_cert::builder::Error }, + + #[snafu(display("failed to add certificate extension"))] + AddCertificateExtension { source: x509_cert::builder::Error }, + + #[snafu(display("failed to build certificate"))] + BuildCertificate { source: x509_cert::builder::Error }, +} + +/// This builder builds certificate authorities of type [`CertificateAuthority`]. +/// +/// Example code to construct a CA: +/// +/// ```no_run +/// use stackable_certs::{ +/// keys::ecdsa, +/// ca::{CertificateAuthority, CertificateAuthorityBuilder}, +/// }; +/// +/// let ca: CertificateAuthority = CertificateAuthorityBuilder::builder() +/// .build() +/// .build_ca() +/// .expect("failed to build CA"); +/// ``` +#[derive(Builder)] +pub struct CertificateAuthorityBuilder<'a, SKP> +where + SKP: CertificateKeypair, + ::VerifyingKey: EncodePublicKey, +{ + /// Required subject of the certificate authority, usually starts with `CN=`. + #[builder(default = SDP_ROOT_CA_SUBJECT)] + subject: &'a str, + + /// Validity/lifetime of the certificate. + /// + /// If not specified the default of [`DEFAULT_CA_VALIDITY`] will be used. + #[builder(default = DEFAULT_CA_VALIDITY)] + validity: Duration, + + /// Serial number of the generated certificate. + /// + /// If not specified a random serial will be generated. + serial_number: Option, + + /// Cryptographic keypair used to sign leaf certificates. + /// + /// If not specified a random keypair will be generated. + signing_key_pair: Option, +} + +impl CertificateAuthorityBuilder<'_, SKP> +where + SKP: CertificateKeypair, + ::VerifyingKey: EncodePublicKey, +{ + pub fn build_ca( + self, + ) -> Result, CreateCertificateAuthorityError> { + let serial_number = + SerialNumber::from(self.serial_number.unwrap_or_else(|| rand::random::())); + let validity = Validity::from_now(*self.validity).context(ParseValiditySnafu)?; + let subject: Name = self.subject.parse().context(ParseSubjectSnafu { + subject: self.subject, + })?; + let signing_key_pair = match self.signing_key_pair { + Some(signing_key_pair) => signing_key_pair, + None => SKP::new().context(CreateSigningKeyPairSnafu)?, + }; + + let spki_pem = signing_key_pair + .verifying_key() + .to_public_key_pem(PEM_LINE_ENDING) + .context(SerializePublicKeySnafu)?; + + let spki = SubjectPublicKeyInfoOwned::from_pem(spki_pem.as_bytes()) + .context(DecodeSpkiFromPemSnafu)?; + + // There are multiple default extensions included in the profile. For + // the root profile, these are: + // + // - BasicConstraints marked as critical and CA = true + // - SubjectKeyIdentifier with the 160-bit SHA-1 hash of the subject + // public key. + // - KeyUsage with KeyCertSign and CRLSign bits set. Ideally we also + // want to include the DigitalSignature bit, which for example is + // required for CA certs which want to sign an OCSP response. + // Currently, the root profile doesn't include that bit. + // + // The root profile doesn't add the AuthorityKeyIdentifier extension. + // We manually add it below by using the 160-bit SHA-1 hash of the + // subject public key. This conforms to one of the outlined methods for + // generating key identifiers outlined in RFC 5280, section 4.2.1.2. + // + // Prepare extensions so we can avoid clones. + let aki = AuthorityKeyIdentifier::try_from(spki.owned_to_ref()) + .context(ParseAuthorityKeyIdentifierSnafu)?; + + let signer = signing_key_pair.signing_key(); + let mut builder = CertificateBuilder::new( + Profile::Root, + serial_number, + validity, + subject, + spki, + signer, + ) + .context(CreateCertificateBuilderSnafu)?; + + // Add extension constructed above + builder + .add_extension(&aki) + .context(AddCertificateExtensionSnafu)?; + + debug!("create and sign CA certificate"); + let certificate = builder.build().context(BuildCertificateSnafu)?; + + Ok(CertificateAuthority { + certificate_pair: CertificatePair { + certificate, + key_pair: signing_key_pair, + }, + }) + } +} + +#[cfg(test)] +mod tests { + use x509_cert::certificate::TbsCertificateInner; + + use super::*; + use crate::keys::{ecdsa, rsa}; + + #[test] + fn minimal_ca() { + let ca: CertificateAuthority = CertificateAuthorityBuilder::builder() + .build() + .build_ca() + .expect("failed to build CA"); + + assert_ca_cert_attributes( + &ca.ca_cert().tbs_certificate, + SDP_ROOT_CA_SUBJECT, + DEFAULT_CA_VALIDITY, + None, + ) + } + + #[test] + fn customized_ca() { + let ca = CertificateAuthorityBuilder::builder() + .subject("CN=Test") + .serial_number(42) + .signing_key_pair(rsa::SigningKey::new().unwrap()) + .validity(Duration::from_days_unchecked(13)) + .build() + .build_ca() + .expect("failed to build CA"); + + assert_ca_cert_attributes( + &ca.ca_cert().tbs_certificate, + "CN=Test", + Duration::from_days_unchecked(13), + Some(42), + ) + } + + fn assert_ca_cert_attributes( + ca_cert: &TbsCertificateInner, + subject: &str, + validity: Duration, + serial_number: Option, + ) { + assert_eq!(ca_cert.subject, subject.parse().unwrap()); + + let not_before = ca_cert.validity.not_before.to_system_time(); + let not_after = ca_cert.validity.not_after.to_system_time(); + assert_eq!( + not_after + .duration_since(not_before) + .expect("Failed to calculate duration between notBefore and notAfter"), + *validity + ); + + if let Some(serial_number) = serial_number { + assert_eq!(ca_cert.serial_number, SerialNumber::from(serial_number)) + } else { + assert_ne!(ca_cert.serial_number, SerialNumber::from(0_u64)) + } + } +} diff --git a/crates/stackable-certs/src/ca/consts.rs b/crates/stackable-certs/src/ca/consts.rs index 125a63a05..a53c21622 100644 --- a/crates/stackable-certs/src/ca/consts.rs +++ b/crates/stackable-certs/src/ca/consts.rs @@ -1,7 +1,13 @@ +use rsa::pkcs8::LineEnding; use stackable_operator::time::Duration; -/// The default CA validity time span of one hour (3600 seconds). +/// The default CA validity time span of one hour. pub const DEFAULT_CA_VALIDITY: Duration = Duration::from_hours_unchecked(1); +/// The default certificate validity time span of one hour. +pub const DEFAULT_CERTIFICATE_VALIDITY: Duration = Duration::from_hours_unchecked(1); + /// The root CA subject name containing only the common name. pub const SDP_ROOT_CA_SUBJECT: &str = "CN=Stackable Data Platform Internal CA"; + +pub const PEM_LINE_ENDING: LineEnding = LineEnding::LF; diff --git a/crates/stackable-certs/src/ca/k8s.rs b/crates/stackable-certs/src/ca/k8s.rs new file mode 100644 index 000000000..7f160915c --- /dev/null +++ b/crates/stackable-certs/src/ca/k8s.rs @@ -0,0 +1,127 @@ +use k8s_openapi::api::core::v1::Secret; +use kube::runtime::reflector::ObjectRef; +use rsa::pkcs8::EncodePublicKey; +use snafu::{OptionExt, ResultExt, Snafu, ensure}; +use stackable_operator::{client::Client, commons::secret::SecretReference}; +use tracing::{debug, instrument}; + +use super::CertificateAuthority; +use crate::{CertificatePair, keys::CertificateKeypair}; + +pub const TLS_SECRET_TYPE: &str = "kubernetes.io/tls"; + +/// Defines all error variants which can occur when loading a CA from a +/// Kubernetes [`Secret`]. +#[derive(Debug, Snafu)] +pub enum SecretError +where + E: std::error::Error + 'static, +{ + #[snafu(display("failed to retrieve secret \"{secret_ref}\""))] + GetSecret { + source: kube::Error, + secret_ref: SecretReference, + }, + + #[snafu(display("invalid secret type, expected {TLS_SECRET_TYPE}"))] + InvalidSecretType, + + #[snafu(display("the secret {secret:?} does not contain any data"))] + NoSecretData { secret: ObjectRef }, + + #[snafu(display("the secret {secret:?} does not contain TLS certificate data"))] + NoCertificateData { secret: ObjectRef }, + + #[snafu(display("the secret {secret:?} does not contain TLS private key data"))] + NoPrivateKeyData { secret: ObjectRef }, + + #[snafu(display("failed to read PEM-encoded certificate chain from secret {secret:?}"))] + ReadChain { + source: x509_cert::der::Error, + secret: ObjectRef, + }, + + #[snafu(display("failed to parse UTF-8 encoded byte string"))] + DecodeUtf8String { source: std::str::Utf8Error }, + + #[snafu(display("failed to deserialize private key from PEM"))] + DeserializeKeyFromPem { source: E }, +} + +/// Create a [`CertificateAuthority`] from a Kubernetes [`Secret`]. +/// +/// Both the `certificate_key` and `private_key_key` parameters describe +/// the _key_ used to lookup the certificate and private key value in the +/// Kubernetes [`Secret`]. Common keys are `ca.crt` and `ca.key`. +#[instrument(skip(secret))] +pub fn ca_from_k8s_secret( + secret: Secret, + certificate_key: &str, + private_key_key: &str, +) -> Result, SecretError> +where + SK: CertificateKeypair, + ::VerifyingKey: EncodePublicKey, +{ + ensure!( + secret.type_.as_ref().is_none_or(|s| s != TLS_SECRET_TYPE), + InvalidSecretTypeSnafu + ); + let data = secret.data.as_ref().with_context(|| NoSecretDataSnafu { + secret: ObjectRef::from_obj(&secret), + })?; + + debug!("retrieving certificate data from secret via key {certificate_key:?}"); + let certificate_data = data + .get(certificate_key) + .with_context(|| NoCertificateDataSnafu { + secret: ObjectRef::from_obj(&secret), + })?; + + let certificate = x509_cert::Certificate::load_pem_chain(&certificate_data.0) + .with_context(|_| ReadChainSnafu { + secret: ObjectRef::from_obj(&secret), + })? + .remove(0); + + debug!("retrieving private key data from secret via key {private_key_key:?}"); + let private_key_data = data + .get(private_key_key) + .with_context(|| NoPrivateKeyDataSnafu { + secret: ObjectRef::from_obj(&secret), + })?; + + let private_key_data = + std::str::from_utf8(&private_key_data.0).context(DecodeUtf8StringSnafu)?; + + let signing_key_pair = + SK::from_pkcs8_pem(private_key_data).context(DeserializeKeyFromPemSnafu)?; + + Ok(CertificateAuthority::new(CertificatePair::new( + certificate, + signing_key_pair, + ))) +} + +/// Create a [`CertificateAuthority`] from a Kubernetes [`SecretReference`]. +#[instrument(skip(secret_ref, client))] +pub async fn ca_from_k8s_secret_ref( + secret_ref: &SecretReference, + key_certificate: &str, + key_private_key: &str, + client: &Client, +) -> Result, SecretError> +where + SK: CertificateKeypair, + ::VerifyingKey: EncodePublicKey, +{ + let secret_api = client.get_api::(&secret_ref.namespace); + let secret = secret_api + .get(&secret_ref.name) + .await + .with_context(|_| GetSecretSnafu { + secret_ref: secret_ref.to_owned(), + })?; + + ca_from_k8s_secret(secret, key_certificate, key_private_key) +} diff --git a/crates/stackable-certs/src/ca/mod.rs b/crates/stackable-certs/src/ca/mod.rs index 6334272a0..86acd1ec1 100644 --- a/crates/stackable-certs/src/ca/mod.rs +++ b/crates/stackable-certs/src/ca/mod.rs @@ -1,169 +1,27 @@ //! Contains types and functions to generate and sign certificate authorities -//! (CAs). -use std::{fmt::Debug, str::FromStr}; +//! (CAs) and certificates. +use std::fmt::Debug; -use const_oid::db::rfc5280::{ID_KP_CLIENT_AUTH, ID_KP_SERVER_AUTH}; -use k8s_openapi::api::core::v1::Secret; -use kube::runtime::reflector::ObjectRef; -use snafu::{OptionExt, ResultExt, Snafu}; -use stackable_operator::{client::Client, commons::secret::SecretReference, time::Duration}; -use tracing::{debug, instrument}; -use x509_cert::{ - Certificate, - builder::{Builder, CertificateBuilder, Profile}, - der::{DecodePem, asn1::Ia5String, pem::LineEnding, referenced::OwnedToRef}, - ext::pkix::{AuthorityKeyIdentifier, ExtendedKeyUsage, SubjectAltName, name::GeneralName}, - name::Name, - serial_number::SerialNumber, - spki::{EncodePublicKey, SubjectPublicKeyInfoOwned}, - time::Validity, -}; +use x509_cert::{Certificate, name::RdnSequence, spki::EncodePublicKey}; -use crate::{ - CertificatePair, - keys::{CertificateKeypair, ecdsa, rsa}, -}; +use crate::{CertificatePair, keys::CertificateKeypair}; +mod ca_builder; mod consts; +mod k8s; +pub use ca_builder::*; pub use consts::*; - -pub const TLS_SECRET_TYPE: &str = "kubernetes.io/tls"; - -pub type Result = std::result::Result; - -/// Defines all error variants which can occur when creating a CA and/or leaf -/// certificates. -#[derive(Debug, Snafu)] -pub enum Error { - #[snafu(display("failed to generate RSA signing key"))] - GenerateRsaSigningKey { source: rsa::Error }, - - #[snafu(display("failed to generate ECDSA signign key"))] - GenerateEcdsaSigningKey { source: ecdsa::Error }, - - #[snafu(display("failed to parse {subject:?} as subject"))] - ParseSubject { - source: x509_cert::der::Error, - subject: String, - }, - - #[snafu(display("failed to parse validity"))] - ParseValidity { source: x509_cert::der::Error }, - - #[snafu(display("failed to serialize public key as PEM"))] - SerializePublicKey { source: x509_cert::spki::Error }, - - #[snafu(display("failed to decode SPKI from PEM"))] - DecodeSpkiFromPem { source: x509_cert::der::Error }, - - #[snafu(display("failed to create certificate builder"))] - CreateCertificateBuilder { source: x509_cert::builder::Error }, - - #[snafu(display("failed to add certificate extension"))] - AddCertificateExtension { source: x509_cert::builder::Error }, - - #[snafu(display("failed to build certificate"))] - BuildCertificate { source: x509_cert::builder::Error }, - - #[snafu(display("failed to parse AuthorityKeyIdentifier"))] - ParseAuthorityKeyIdentifier { source: x509_cert::der::Error }, - - #[snafu(display("The subject alternative DNS name \"{dns_name}\" is not a Ia5String"))] - SaDnsNameNotAIa5String { - dns_name: String, - source: x509_cert::der::Error, - }, -} - -/// Custom implementation of [`std::cmp::PartialEq`] because some inner types -/// don't implement it. -/// -/// Note that this implementation is restricted to testing because there is a -/// variant that is impossible to compare, and will cause a panic if it is -/// attempted. -#[cfg(test)] -impl PartialEq for Error { - fn eq(&self, other: &Self) -> bool { - match (self, other) { - ( - Self::CreateCertificateBuilder { source: lhs_source }, - Self::CreateCertificateBuilder { source: rhs_source }, - ) - | ( - Self::AddCertificateExtension { source: lhs_source }, - Self::AddCertificateExtension { source: rhs_source }, - ) - | ( - Self::BuildCertificate { source: lhs_source }, - Self::BuildCertificate { source: rhs_source }, - ) => match (lhs_source, rhs_source) { - (x509_cert::builder::Error::Asn1(lhs), x509_cert::builder::Error::Asn1(rhs)) => { - lhs == rhs - } - ( - x509_cert::builder::Error::PublicKey(lhs), - x509_cert::builder::Error::PublicKey(rhs), - ) => lhs == rhs, - ( - x509_cert::builder::Error::Signature(_), - x509_cert::builder::Error::Signature(_), - ) => panic!( - "it is impossible to compare the opaque Error contained witin signature::error::Error" - ), - _ => false, - }, - (lhs, rhs) => lhs == rhs, - } - } -} - -/// Defines all error variants which can occur when loading a CA from a -/// Kubernetes [`Secret`]. -#[derive(Debug, Snafu)] -pub enum SecretError -where - E: std::error::Error + 'static, -{ - #[snafu(display("failed to retrieve secret \"{secret_ref}\""))] - GetSecret { - source: kube::Error, - secret_ref: SecretReference, - }, - - #[snafu(display("invalid secret type, expected {TLS_SECRET_TYPE}"))] - InvalidSecretType, - - #[snafu(display("the secret {secret:?} does not contain any data"))] - NoSecretData { secret: ObjectRef }, - - #[snafu(display("the secret {secret:?} does not contain TLS certificate data"))] - NoCertificateData { secret: ObjectRef }, - - #[snafu(display("the secret {secret:?} does not contain TLS private key data"))] - NoPrivateKeyData { secret: ObjectRef }, - - #[snafu(display("failed to read PEM-encoded certificate chain from secret {secret:?}"))] - ReadChain { - source: x509_cert::der::Error, - secret: ObjectRef, - }, - - #[snafu(display("failed to parse UTF-8 encoded byte string"))] - DecodeUtf8String { source: std::str::Utf8Error }, - - #[snafu(display("failed to deserialize private key from PEM"))] - DeserializeKeyFromPem { source: E }, -} +pub use k8s::*; /// A certificate authority (CA) which is used to generate and sign -/// intermidiate or leaf certificates. +/// intermediate or leaf certificates. #[derive(Debug)] -pub struct CertificateAuthority +pub struct CertificateAuthority where - S: CertificateKeypair, - ::VerifyingKey: EncodePublicKey, + SK: CertificateKeypair, + ::VerifyingKey: EncodePublicKey, { - certificate_pair: CertificatePair, + certificate_pair: CertificatePair, } impl CertificateAuthority @@ -171,382 +29,19 @@ where S: CertificateKeypair, ::VerifyingKey: EncodePublicKey, { - /// Creates a new CA certificate with many parameters set to their default - /// values. - /// - /// These parameters include: - /// - /// - a randomly generated serial number - /// - a default validity of one hour (see [`DEFAULT_CA_VALIDITY`]) - /// - /// The CA contains the public half of the provided `signing_key` and is - /// signed by the private half of said key. - /// - /// If the default values for the serial number and validity don't satisfy - /// the requirements of the caller, use [`CertificateAuthority::new_with`] - /// instead. - #[instrument(name = "create_certificate_authority", skip(signing_key_pair))] - pub fn new(signing_key_pair: S) -> Result { - let serial_number = rand::random::(); - - Self::new_with(signing_key_pair, serial_number, DEFAULT_CA_VALIDITY) + pub fn new(certificate_pair: CertificatePair) -> Self { + Self { certificate_pair } } - /// Creates a new CA certificate. - /// - /// Instead of providing sensible defaults for the serial number and - /// validity, this function offers complete control over these parameters. - /// If this level of control is not needed, use [`CertificateAuthority::new`] - /// instead. - #[instrument(name = "create_certificate_authority_with", skip(signing_key_pair))] - pub fn new_with(signing_key_pair: S, serial_number: u64, validity: Duration) -> Result { - let serial_number = SerialNumber::from(serial_number); - let validity = Validity::from_now(*validity).context(ParseValiditySnafu)?; - - // We don't allow customization of the CA subject by callers. Every CA - // created by us should contain the same subject consisting a common set - // of distinguished names (DNs). - let subject = Name::from_str(SDP_ROOT_CA_SUBJECT).context(ParseSubjectSnafu { - subject: SDP_ROOT_CA_SUBJECT, - })?; - - let spki_pem = signing_key_pair - .verifying_key() - .to_public_key_pem(LineEnding::LF) - .context(SerializePublicKeySnafu)?; - - let spki = SubjectPublicKeyInfoOwned::from_pem(spki_pem.as_bytes()) - .context(DecodeSpkiFromPemSnafu)?; - - // There are multiple default extensions included in the profile. For - // the root profile, these are: - // - // - BasicConstraints marked as critical and CA = true - // - SubjectKeyIdentifier with the 160-bit SHA-1 hash of the subject - // public key. - // - KeyUsage with KeyCertSign and CRLSign bits set. Ideally we also - // want to include the DigitalSignature bit, which for example is - // required for CA certs which want to sign an OCSP response. - // Currently, the root profile doesn't include that bit. - // - // The root profile doesn't add the AuthorityKeyIdentifier extension. - // We manually add it below by using the 160-bit SHA-1 hash of the - // subject pulic key. This conforms to one of the outlined methods for - // generating key identifiers outlined in RFC 5280, section 4.2.1.2. - // - // Prepare extensions so we can avoid clones. - let aki = AuthorityKeyIdentifier::try_from(spki.owned_to_ref()) - .context(ParseAuthorityKeyIdentifierSnafu)?; - - let signer = signing_key_pair.signing_key(); - let mut builder = CertificateBuilder::new( - Profile::Root, - serial_number, - validity, - subject, - spki, - signer, - ) - .context(CreateCertificateBuilderSnafu)?; - - // Add extension constructed above - builder - .add_extension(&aki) - .context(AddCertificateExtensionSnafu)?; - - debug!("create and sign CA certificate"); - let certificate = builder.build().context(BuildCertificateSnafu)?; - - Ok(Self { - certificate_pair: CertificatePair { - key_pair: signing_key_pair, - certificate, - }, - }) + pub fn signing_key(&self) -> &S::SigningKey { + self.certificate_pair.key_pair().signing_key() } - /// Generates a leaf certificate which is signed by this CA. - /// - /// The certificate requires a `name` and a `scope`. Both these values - /// are part of the certificate subject. The format is: `{name} Certificate - /// for {scope}`. These leaf certificates can be used for client/server - /// authentication, because they include [`ID_KP_CLIENT_AUTH`] and - /// [`ID_KP_SERVER_AUTH`] in the extended key usage extension. - /// - /// It is also possible to directly create RSA or ECDSA-based leaf - /// certificates using [`CertificateAuthority::generate_rsa_leaf_certificate`] - /// and [`CertificateAuthority::generate_ecdsa_leaf_certificate`]. - #[instrument(skip(self, key_pair))] - pub fn generate_leaf_certificate<'a, T>( - &mut self, - key_pair: T, - name: &str, - scope: &str, - subject_alterative_dns_names: impl IntoIterator + Debug, - validity: Duration, - ) -> Result> - where - T: CertificateKeypair, - ::VerifyingKey: EncodePublicKey, - { - // We generate a random serial number, but ensure the same CA didn't - // issue another certificate with the same serial number. We try to - // generate a unique serial number at max five times before giving up - // and returning an error. - let serial_number = SerialNumber::from(rand::random::()); - - // NOTE (@Techassi): Should we validate that the validity is shorter - // than the validity of the issuing CA? - let validity = Validity::from_now(*validity).context(ParseValiditySnafu)?; - let subject = format_leaf_certificate_subject(name, scope)?; - - let spki_pem = key_pair - .verifying_key() - .to_public_key_pem(LineEnding::LF) - .context(SerializePublicKeySnafu)?; - - let spki = SubjectPublicKeyInfoOwned::from_pem(spki_pem.as_bytes()) - .context(DecodeSpkiFromPemSnafu)?; - - let signer = self.certificate_pair.key_pair.signing_key(); - let mut builder = CertificateBuilder::new( - Profile::Leaf { - issuer: self - .certificate_pair - .certificate - .tbs_certificate - .issuer - .clone(), - enable_key_agreement: false, - enable_key_encipherment: true, - }, - serial_number, - validity, - subject, - spki, - signer, - ) - .context(CreateCertificateBuilderSnafu)?; - - // The leaf certificate can be used for WWW client and server - // authentication. This is a base requirement for TLS certs. - builder - .add_extension(&ExtendedKeyUsage(vec![ - ID_KP_CLIENT_AUTH, - ID_KP_SERVER_AUTH, - ])) - .context(AddCertificateExtensionSnafu)?; - - let sans = subject_alterative_dns_names - .into_iter() - .map(|dns_name| { - Ok(GeneralName::DnsName(Ia5String::new(dns_name).context( - SaDnsNameNotAIa5StringSnafu { - dns_name: dns_name.to_string(), - }, - )?)) - }) - .collect::, Error>>()?; - builder - .add_extension(&SubjectAltName(sans)) - .context(AddCertificateExtensionSnafu)?; - - debug!("create and sign leaf certificate"); - let certificate = builder.build().context(BuildCertificateSnafu)?; - - Ok(CertificatePair { - certificate, - key_pair, - }) - } - - /// Generates an RSA-based leaf certificate which is signed by this CA. - /// - /// See [`CertificateAuthority::generate_leaf_certificate`] for more - /// information. - #[instrument(skip(self))] - pub fn generate_rsa_leaf_certificate<'a>( - &mut self, - name: &str, - scope: &str, - subject_alterative_dns_names: impl IntoIterator + Debug, - validity: Duration, - ) -> Result> { - let key = rsa::SigningKey::new().context(GenerateRsaSigningKeySnafu)?; - self.generate_leaf_certificate(key, name, scope, subject_alterative_dns_names, validity) - } - - /// Generates an ECDSAasync -based leaf certificate which is signed by this CA. - /// - /// See [`CertificateAuthority::generate_leaf_certificate`] for more - /// information. - #[instrument(skip(self))] - pub fn generate_ecdsa_leaf_certificate<'a>( - &mut self, - name: &str, - scope: &str, - subject_alterative_dns_names: impl IntoIterator + Debug, - validity: Duration, - ) -> Result> { - let key = ecdsa::SigningKey::new().context(GenerateEcdsaSigningKeySnafu)?; - self.generate_leaf_certificate(key, name, scope, subject_alterative_dns_names, validity) - } - - /// Create a [`CertificateAuthority`] from a Kubernetes [`Secret`]. - /// - /// Both the `key_certificate` and `key_private_key` parameters describe - /// the _key_ used to lookup the certificate and private key value in the - /// Kubernetes [`Secret`]. Common keys are `ca.crt` and `ca.key`. - #[instrument(name = "create_certificate_authority_from_k8s_secret", skip(secret))] - pub fn from_secret( - secret: Secret, - key_certificate: &str, - key_private_key: &str, - ) -> Result> { - if secret.type_.as_ref().is_none_or(|s| s != TLS_SECRET_TYPE) { - return InvalidSecretTypeSnafu.fail(); - } - - let data = secret.data.as_ref().with_context(|| NoSecretDataSnafu { - secret: ObjectRef::from_obj(&secret), - })?; - - debug!("retrieving certificate data from secret via key {key_certificate:?}"); - let certificate_data = - data.get(key_certificate) - .with_context(|| NoCertificateDataSnafu { - secret: ObjectRef::from_obj(&secret), - })?; - - let certificate = x509_cert::Certificate::load_pem_chain(&certificate_data.0) - .with_context(|_| ReadChainSnafu { - secret: ObjectRef::from_obj(&secret), - })? - .remove(0); - - debug!("retrieving private key data from secret via key {key_certificate:?}"); - let private_key_data = - data.get(key_private_key) - .with_context(|| NoPrivateKeyDataSnafu { - secret: ObjectRef::from_obj(&secret), - })?; - - let private_key_data = - std::str::from_utf8(&private_key_data.0).context(DecodeUtf8StringSnafu)?; - - let signing_key_pair = - S::from_pkcs8_pem(private_key_data).context(DeserializeKeyFromPemSnafu)?; - - Ok(Self { - certificate_pair: CertificatePair { - key_pair: signing_key_pair, - certificate, - }, - }) - } - - /// Create a [`CertificateAuthority`] from a Kubernetes [`SecretReference`]. - #[instrument( - name = "create_certificate_authority_from_k8s_secret_ref", - skip(secret_ref, client) - )] - pub async fn from_secret_ref( - secret_ref: &SecretReference, - key_certificate: &str, - key_private_key: &str, - client: &Client, - ) -> Result> { - let secret_api = client.get_api::(&secret_ref.namespace); - let secret = secret_api - .get(&secret_ref.name) - .await - .with_context(|_| GetSecretSnafu { - secret_ref: secret_ref.to_owned(), - })?; - - Self::from_secret(secret, key_certificate, key_private_key) - } - - /// Returns the ca certificate. pub fn ca_cert(&self) -> &Certificate { &self.certificate_pair.certificate } -} - -impl CertificateAuthority { - /// High-level function to create a new CA using a RSA key pair. - #[instrument(name = "create_certificate_authority_with_rsa")] - pub fn new_rsa() -> Result { - Self::new(rsa::SigningKey::new().context(GenerateRsaSigningKeySnafu)?) - } -} - -impl CertificateAuthority { - /// High-level function to create a new CA using a ECDSA key pair. - #[instrument(name = "create_certificate_authority_with_ecdsa")] - pub fn new_ecdsa() -> Result { - Self::new(ecdsa::SigningKey::new().context(GenerateEcdsaSigningKeySnafu)?) - } -} - -fn format_leaf_certificate_subject(name: &str, scope: &str) -> Result { - let subject = format!("CN={name} Certificate for {scope}"); - Name::from_str(&subject).context(ParseSubjectSnafu { subject }) -} - -#[cfg(test)] -mod tests { - use const_oid::ObjectIdentifier; - - use super::*; - - const TEST_CERT_LIFETIME: Duration = Duration::from_hours_unchecked(1); - const TEST_SAN: &str = "airflow-0.airflow.default.svc.cluster.local"; - - #[tokio::test] - async fn rsa_key_generation() { - let mut ca = CertificateAuthority::new_rsa().unwrap(); - let cert = ca - .generate_rsa_leaf_certificate("Airflow", "pod", [TEST_SAN], TEST_CERT_LIFETIME) - .expect("RSA certificate generation failed"); - - assert_cert_attributes(cert.certificate()); - } - - #[tokio::test] - async fn ecdsa_key_generation() { - let mut ca = CertificateAuthority::new_ecdsa().unwrap(); - let cert = ca - .generate_ecdsa_leaf_certificate("Airflow", "pod", [TEST_SAN], TEST_CERT_LIFETIME) - .expect("ecdsa certificate generation failed"); - - assert_cert_attributes(cert.certificate()); - } - - fn assert_cert_attributes(cert: &Certificate) { - let cert = &cert.tbs_certificate; - // Test subject - assert_eq!( - cert.subject, - Name::from_str("CN=Airflow Certificate for pod").unwrap() - ); - - // Test SAN extension is present - let extensions = cert.extensions.as_ref().expect("cert had no extension"); - assert!( - extensions - .iter() - .any(|ext| ext.extn_id == ObjectIdentifier::new_unwrap("2.5.29.17")) - ); - // Test lifetime - let not_before = cert.validity.not_before.to_system_time(); - let not_after = cert.validity.not_after.to_system_time(); - assert_eq!( - not_after - .duration_since(not_before) - .expect("Failed to calculate duration between notBefore and notAfter"), - *TEST_CERT_LIFETIME - ); + pub fn issuer_name(&self) -> &RdnSequence { + &self.ca_cert().tbs_certificate.issuer } } diff --git a/crates/stackable-certs/src/cert_builder.rs b/crates/stackable-certs/src/cert_builder.rs new file mode 100644 index 000000000..9c1f9997a --- /dev/null +++ b/crates/stackable-certs/src/cert_builder.rs @@ -0,0 +1,319 @@ +use std::fmt::Debug; + +use bon::Builder; +use const_oid::db::rfc5280::{ID_KP_CLIENT_AUTH, ID_KP_SERVER_AUTH}; +use rsa::pkcs8::EncodePublicKey; +use snafu::{ResultExt, Snafu}; +use stackable_operator::time::Duration; +use tracing::debug; +use x509_cert::{ + builder::{Builder, Profile}, + der::{DecodePem, asn1::Ia5String}, + ext::pkix::{ExtendedKeyUsage, SubjectAltName, name::GeneralName}, + name::Name, + serial_number::SerialNumber, + spki::SubjectPublicKeyInfoOwned, + time::Validity, +}; + +use crate::{ + CertificatePair, + ca::{CertificateAuthority, DEFAULT_CERTIFICATE_VALIDITY, PEM_LINE_ENDING}, + keys::CertificateKeypair, +}; + +/// Defines all error variants which can occur when creating a CertificateRequest +#[derive(Debug, Snafu)] +pub enum CreateCertificateError +where + E: std::error::Error + 'static, +{ + #[snafu(display("failed to parse validity"))] + ParseValidity { source: x509_cert::der::Error }, + + #[snafu(display("failed to parse \"{subject}\" as subject"))] + ParseSubject { + source: x509_cert::der::Error, + subject: String, + }, + + // #[snafu(display("failed to create key pair"))] + // CreateKeyPair { source: Box }, + #[snafu(display("failed to create key pair"))] + CreateKeyPair { source: E }, + + #[snafu(display("failed to serialize public key as PEM"))] + SerializePublicKey { source: x509_cert::spki::Error }, + + #[snafu(display("failed to decode SPKI from PEM"))] + DecodeSpkiFromPem { source: x509_cert::der::Error }, + + #[snafu(display("failed to create certificate builder"))] + CreateCertificateBuilder { source: x509_cert::builder::Error }, + + #[snafu(display("failed to add certificate extension"))] + AddCertificateExtension { source: x509_cert::builder::Error }, + + #[snafu(display("The subject alternative DNS name \"{dns_name}\" is not a Ia5String"))] + SaDnsNameNotAIa5String { + dns_name: String, + source: x509_cert::der::Error, + }, + + #[snafu(display("failed to build certificate"))] + BuildCertificate { source: x509_cert::builder::Error }, +} + +/// This builder builds certificates of type [`CertificatePair`]. +/// +/// Example code to construct a certificate: +/// +/// ```no_run +/// use stackable_certs::{ +/// keys::ecdsa, +/// ca::{CertificateAuthority, CertificateAuthorityBuilder}, +/// CertificateBuilder, +/// }; +/// +/// let ca: CertificateAuthority = CertificateAuthorityBuilder::builder() +/// .build() +/// .build_ca() +/// .expect("failed to build CA"); +/// +/// let certificate = CertificateBuilder::builder() +/// .subject("CN=trino-coordinator-default-0") +/// .signed_by(&ca) +/// .build() +/// .build_certificate() +/// .expect("failed to build certificate"); +/// ``` +#[derive(Builder)] +pub struct CertificateBuilder<'a, KP> +where + KP: CertificateKeypair, + ::VerifyingKey: EncodePublicKey, +{ + /// Required subject of the certificate, usually starts with `CN=`. + subject: &'a str, + + /// Optional list of subject alternative names (SAN) DNS entries, + /// that are added to the certificate. + #[builder(default)] + subject_alterative_dns_names: &'a [&'a str], + + /// Validity/lifetime of the certificate. + /// + /// If not specified the default of [`DEFAULT_CERTIFICATE_VALIDITY`] will be used. + #[builder(default = DEFAULT_CERTIFICATE_VALIDITY)] + validity: Duration, + + /// Serial number of the generated certificate. + /// + /// If not specified a random serial will be generated. + serial_number: Option, + + /// Cryptographic keypair used to for the certificates. + /// + /// If not specified a random keypair will be generated. + key_pair: Option, + + /// Mandatorily sign the certificate using the provided [`CertificateAuthority`]. + signed_by: &'a CertificateAuthority, +} + +impl CertificateBuilder<'_, KP> +where + KP: CertificateKeypair, + ::VerifyingKey: EncodePublicKey, +{ + pub fn build_certificate( + self, + ) -> Result, CreateCertificateError> { + let serial_number = + SerialNumber::from(self.serial_number.unwrap_or_else(|| rand::random::())); + // NOTE (@Techassi): Should we validate that the validity is shorter + // than the validity of the issuing CA? + let validity = Validity::from_now(*self.validity).context(ParseValiditySnafu)?; + let subject: Name = self.subject.parse().context(ParseSubjectSnafu { + subject: self.subject, + })?; + let key_pair = match self.key_pair { + Some(key_pair) => key_pair, + None => KP::new().context(CreateKeyPairSnafu)?, + }; + + let spki_pem = key_pair + .verifying_key() + .to_public_key_pem(PEM_LINE_ENDING) + .context(SerializePublicKeySnafu)?; + + let spki = SubjectPublicKeyInfoOwned::from_pem(spki_pem.as_bytes()) + .context(DecodeSpkiFromPemSnafu)?; + + let signing_key = self.signed_by.signing_key(); + let mut builder = x509_cert::builder::CertificateBuilder::new( + Profile::Leaf { + issuer: self.signed_by.issuer_name().clone(), + enable_key_agreement: false, + enable_key_encipherment: true, + }, + serial_number, + validity, + subject, + spki, + signing_key, + ) + .context(CreateCertificateBuilderSnafu)?; + + // The leaf certificate can be used for WWW client and server + // authentication. This is a base requirement for TLS certs. + builder + .add_extension(&ExtendedKeyUsage(vec![ + ID_KP_CLIENT_AUTH, + ID_KP_SERVER_AUTH, + ])) + .context(AddCertificateExtensionSnafu)?; + + let sans = self + .subject_alterative_dns_names + .iter() + .map(|dns_name| { + Ok(GeneralName::DnsName(Ia5String::new(dns_name).context( + SaDnsNameNotAIa5StringSnafu { + dns_name: dns_name.to_string(), + }, + )?)) + }) + .collect::, CreateCertificateError>>()?; + builder + .add_extension(&SubjectAltName(sans)) + .context(AddCertificateExtensionSnafu)?; + + debug!("create and sign leaf certificate"); + let certificate = builder.build().context(BuildCertificateSnafu)?; + + Ok(CertificatePair { + certificate, + key_pair, + }) + } +} + +#[cfg(test)] +mod tests { + use x509_cert::{ + certificate::TbsCertificateInner, der::Decode, ext::pkix::ID_CE_SUBJECT_ALT_NAME, + }; + + use super::*; + use crate::{ + ca::CertificateAuthorityBuilder, + keys::{ecdsa, rsa}, + }; + + #[test] + fn minimal_certificate() { + let ca = get_ecdsa_ca(); + let certificate = CertificateBuilder::builder() + .subject("CN=trino-coordinator-default-0") + .signed_by(&ca) + .build() + .build_certificate() + .expect("failed to build certificate"); + + assert_certificate_attributes( + &certificate.certificate.tbs_certificate, + "CN=trino-coordinator-default-0", + &[], + DEFAULT_CERTIFICATE_VALIDITY, + None, + ); + } + + #[test] + fn customized_certificate() { + let ca = get_rsa_ca(); + let sans = [ + "trino-coordinator-default-0.trino-coordinator-default.default.svc.cluster-local", + "trino-coordinator-default.default.svc.cluster-local", + ]; + let certificate = CertificateBuilder::builder() + .subject("CN=trino-coordinator-default-0") + .subject_alterative_dns_names(&sans) + .serial_number(08121997) + .validity(Duration::from_days_unchecked(42)) + .key_pair(rsa::SigningKey::new().unwrap()) + .signed_by(&ca) + .build() + .build_certificate() + .expect("failed to build certificate"); + + assert_certificate_attributes( + &certificate.certificate.tbs_certificate, + "CN=trino-coordinator-default-0", + &sans, + Duration::from_days_unchecked(42), + Some(08121997), + ); + } + + fn assert_certificate_attributes( + certificate: &TbsCertificateInner, + subject: &str, + sans: &[&str], + validity: Duration, + serial_number: Option, + ) { + assert_eq!(certificate.subject, subject.parse().unwrap()); + + let extensions = certificate + .extensions + .as_ref() + .expect("cert had no extension"); + let san_extension = extensions + .iter() + .find(|ext| ext.extn_id == ID_CE_SUBJECT_ALT_NAME) + .expect("cert had no SAN extension"); + + let actual_sans = SubjectAltName::from_der(san_extension.extn_value.as_bytes()) + .expect("failed to parse SAN"); + let actual_sans = actual_sans + .0 + .iter() + .filter_map(|san| match san { + GeneralName::DnsName(dns_name) => Some(dns_name.as_str()), + _ => None, + }) + .collect::>(); + assert_eq!(actual_sans, sans); + + let not_before = certificate.validity.not_before.to_system_time(); + let not_after = certificate.validity.not_after.to_system_time(); + assert_eq!( + not_after + .duration_since(not_before) + .expect("Failed to calculate duration between notBefore and notAfter"), + *validity + ); + + if let Some(serial_number) = serial_number { + assert_eq!(certificate.serial_number, SerialNumber::from(serial_number)) + } else { + assert_ne!(certificate.serial_number, SerialNumber::from(0_u64)) + } + } + + fn get_ecdsa_ca() -> CertificateAuthority { + CertificateAuthorityBuilder::builder() + .build() + .build_ca() + .expect("failed to build CA") + } + + fn get_rsa_ca() -> CertificateAuthority { + CertificateAuthorityBuilder::builder() + .build() + .build_ca() + .expect("failed to build CA") + } +} diff --git a/crates/stackable-certs/src/keys/ecdsa.rs b/crates/stackable-certs/src/keys/ecdsa.rs index 38de9d225..8d7adea51 100644 --- a/crates/stackable-certs/src/keys/ecdsa.rs +++ b/crates/stackable-certs/src/keys/ecdsa.rs @@ -22,28 +22,26 @@ pub enum Error { #[derive(Debug)] pub struct SigningKey(p256::ecdsa::SigningKey); -impl SigningKey { +impl CertificateKeypair for SigningKey { + type Error = Error; + type Signature = ecdsa::der::Signature; + type SigningKey = p256::ecdsa::SigningKey; + type VerifyingKey = p256::ecdsa::VerifyingKey; + #[instrument(name = "create_ecdsa_signing_key")] - pub fn new() -> Result { + fn new() -> Result { let mut csprng = OsRng; Self::new_with_rng(&mut csprng) } #[instrument(name = "create_ecdsa_signing_key_custom_rng", skip_all)] - pub fn new_with_rng(csprng: &mut R) -> Result + fn new_with_rng(rng: &mut Rng) -> Result where - R: CryptoRngCore + Sized, + Rng: CryptoRngCore + Sized, { - let signing_key = p256::ecdsa::SigningKey::random(csprng); + let signing_key = p256::ecdsa::SigningKey::random(rng); Ok(Self(signing_key)) } -} - -impl CertificateKeypair for SigningKey { - type Error = Error; - type Signature = ecdsa::der::Signature; - type SigningKey = p256::ecdsa::SigningKey; - type VerifyingKey = p256::ecdsa::VerifyingKey; fn signing_key(&self) -> &Self::SigningKey { &self.0 diff --git a/crates/stackable-certs/src/keys/mod.rs b/crates/stackable-certs/src/keys/mod.rs index 862d33a49..0608cf556 100644 --- a/crates/stackable-certs/src/keys/mod.rs +++ b/crates/stackable-certs/src/keys/mod.rs @@ -9,8 +9,8 @@ //! [`ecdsa`], which provides primitives and traits, and [`p256`] which //! implements the NIST P-256 elliptic curve and supports ECDSA. //! -//! ```ignore -//! use stackable_certs::keys::ecdsa::SigningKey; +//! ```no_run +//! use stackable_certs::keys::{ecdsa::SigningKey, CertificateKeypair}; //! let key = SigningKey::new().unwrap(); //! ``` //! @@ -18,9 +18,9 @@ //! //! In order to work with RSA keys, this crate requires the [`rsa`] dependency. //! -//! ```ignore -//! use stackable_certs::keys::rsa::SigningKey; -//! let key = SigningKey::new().unwrap(); +//! ```no_run +//! use stackable_certs::keys::{rsa::SigningKey, CertificateKeypair}; +//! let key = SigningKey::new().unwrap();; //! ``` //! //! It should be noted, that the crate is currently vulnerable to the recently @@ -32,6 +32,7 @@ use std::fmt::Debug; use p256::pkcs8::EncodePrivateKey; +use rand_core::CryptoRngCore; use signature::{Keypair, Signer}; use x509_cert::spki::{EncodePublicKey, SignatureAlgorithmIdentifier, SignatureBitStringEncoding}; @@ -54,6 +55,14 @@ where type Error: std::error::Error + 'static; + /// Generates a new key with the default random-number generator [`rand_core::OsRng`]. + fn new() -> Result; + + /// Generates a new key with a custom random-number generator. + fn new_with_rng(rng: &mut Rng) -> Result + where + Rng: CryptoRngCore + Sized; + /// Returns the signing (private) key half of the keypair. fn signing_key(&self) -> &Self::SigningKey; diff --git a/crates/stackable-certs/src/keys/rsa.rs b/crates/stackable-certs/src/keys/rsa.rs index 2bcd018ee..836a8b36e 100644 --- a/crates/stackable-certs/src/keys/rsa.rs +++ b/crates/stackable-certs/src/keys/rsa.rs @@ -29,7 +29,12 @@ pub enum Error { #[derive(Debug)] pub struct SigningKey(rsa::pkcs1v15::SigningKey); -impl SigningKey { +impl CertificateKeypair for SigningKey { + type Error = Error; + type Signature = rsa::pkcs1v15::Signature; + type SigningKey = rsa::pkcs1v15::SigningKey; + type VerifyingKey = rsa::pkcs1v15::VerifyingKey; + /// Generates a new RSA key with the default random-number generator /// [`OsRng`]. /// @@ -37,7 +42,7 @@ impl SigningKey { /// larger key sizes. The generation of an RSA key with a key size of /// `4096` (which is used) can take up to multiple seconds. #[instrument(name = "create_rsa_signing_key")] - pub fn new() -> Result { + fn new() -> Result { let mut csprng = OsRng; Self::new_with_rng(&mut csprng) } @@ -48,22 +53,15 @@ impl SigningKey { /// larger key sizes. The generation of an RSA key with a key size of /// `4096` (which is used) can take up to multiple seconds. #[instrument(name = "create_rsa_signing_key_custom_rng", skip_all)] - pub fn new_with_rng(csprng: &mut R) -> Result + fn new_with_rng(rng: &mut Rng) -> Result where - R: CryptoRngCore + ?Sized, + Rng: CryptoRngCore + ?Sized, { - let private_key = RsaPrivateKey::new(csprng, KEY_SIZE).context(CreateKeySnafu)?; + let private_key = RsaPrivateKey::new(rng, KEY_SIZE).context(CreateKeySnafu)?; let signing_key = rsa::pkcs1v15::SigningKey::::new(private_key); Ok(Self(signing_key)) } -} - -impl CertificateKeypair for SigningKey { - type Error = Error; - type Signature = rsa::pkcs1v15::Signature; - type SigningKey = rsa::pkcs1v15::SigningKey; - type VerifyingKey = rsa::pkcs1v15::VerifyingKey; fn signing_key(&self) -> &Self::SigningKey { &self.0 diff --git a/crates/stackable-certs/src/lib.rs b/crates/stackable-certs/src/lib.rs index 5b9c87327..e74ccfae8 100644 --- a/crates/stackable-certs/src/lib.rs +++ b/crates/stackable-certs/src/lib.rs @@ -34,8 +34,11 @@ use { use crate::keys::CertificateKeypair; pub mod ca; +mod cert_builder; pub mod keys; +pub use cert_builder::*; + /// Error variants which can be encountered when creating a new /// [`CertificatePair`]. #[derive(Debug, Snafu)] @@ -43,7 +46,7 @@ pub enum CertificatePairError where E: std::error::Error + 'static, { - #[snafu(display("failed to seralize certificate as {key_encoding}"))] + #[snafu(display("failed to serialize certificate as {key_encoding}"))] SerializeCertificate { source: x509_cert::der::Error, key_encoding: KeyEncoding, @@ -74,7 +77,8 @@ where ReadFile { source: std::io::Error }, } -/// Custom implementation of [`std::cmp::PartialEq`] because [`std::io::Error`] doesn't implement it, but [`std::io::ErrorKind`] does. +/// Custom implementation of [`std::cmp::PartialEq`] because [`std::io::Error`] doesn't implement it, +/// but [`std::io::ErrorKind`] does. impl PartialEq for CertificatePairError { fn eq(&self, other: &Self) -> bool { match (self, other) { @@ -110,6 +114,13 @@ where S: CertificateKeypair, ::VerifyingKey: EncodePublicKey, { + pub fn new(certificate: Certificate, key_pair: S) -> Self { + Self { + certificate, + key_pair, + } + } + /// Returns a reference to the [`Certificate`]. pub fn certificate(&self) -> &Certificate { &self.certificate diff --git a/crates/stackable-webhook/src/tls.rs b/crates/stackable-webhook/src/tls.rs index 2aad52ee4..ad5bb1257 100644 --- a/crates/stackable-webhook/src/tls.rs +++ b/crates/stackable-webhook/src/tls.rs @@ -9,9 +9,9 @@ use hyper_util::rt::{TokioExecutor, TokioIo}; use opentelemetry::trace::{FutureExt, SpanKind}; use snafu::{ResultExt, Snafu}; use stackable_certs::{ - CertificatePairError, - ca::{CertificateAuthority, DEFAULT_CA_VALIDITY}, - keys::rsa, + CertificateBuilder, CertificatePairError, CreateCertificateError, + ca::{CertificateAuthority, CertificateAuthorityBuilder, CreateCertificateAuthorityError}, + keys::ecdsa, }; use tokio::net::TcpListener; use tokio_rustls::{ @@ -39,20 +39,24 @@ pub enum Error { socket_addr: SocketAddr, }, - #[snafu(display("failed to create CA to generate and sign webhook leaf certificate"))] - CreateCertificateAuthority { source: stackable_certs::ca::Error }, + #[snafu(display("failed to create certificate authority"))] + CreateCertificateAuthority { + source: CreateCertificateAuthorityError, + }, - #[snafu(display("failed to generate webhook leaf certificate"))] - GenerateLeafCertificate { source: stackable_certs::ca::Error }, + #[snafu(display("failed to create certificate"))] + CreateCertificate { + source: CreateCertificateError, + }, #[snafu(display("failed to encode leaf certificate as DER"))] EncodeCertificateDer { - source: CertificatePairError, + source: CertificatePairError, }, #[snafu(display("failed to encode private key as DER"))] EncodePrivateKeyDer { - source: CertificatePairError, + source: CertificatePairError, }, #[snafu(display("failed to set safe TLS protocol versions"))] @@ -105,18 +109,23 @@ impl TlsServer { // blocked. // See https://docs.rs/tokio/latest/tokio/task/fn.spawn_blocking.html let task = tokio::task::spawn_blocking(move || { - let mut certificate_authority = - CertificateAuthority::new_rsa().context(CreateCertificateAuthoritySnafu)?; - - let leaf_certificate = certificate_authority - .generate_rsa_leaf_certificate("Leaf", "webhook", [], DEFAULT_CA_VALIDITY) - .context(GenerateLeafCertificateSnafu)?; - - let certificate_der = leaf_certificate + let ca: CertificateAuthority = + CertificateAuthorityBuilder::builder() + .build() + .build_ca() + .context(CreateCertificateAuthoritySnafu)?; + let certificate = CertificateBuilder::builder() + .subject("CN=webhook") + .signed_by(&ca) + .build() + .build_certificate() + .context(CreateCertificateSnafu)?; + + let certificate_der = certificate .certificate_der() .context(EncodeCertificateDerSnafu)?; - let private_key_der = leaf_certificate + let private_key_der = certificate .private_key_der() .context(EncodePrivateKeyDerSnafu)?; From 3fd41d720b0f6326e809405450541263796a1d4e Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 23 May 2025 09:51:32 +0200 Subject: [PATCH 04/19] changelog --- crates/stackable-certs/CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/stackable-certs/CHANGELOG.md b/crates/stackable-certs/CHANGELOG.md index 625892d0a..797da96a7 100644 --- a/crates/stackable-certs/CHANGELOG.md +++ b/crates/stackable-certs/CHANGELOG.md @@ -8,12 +8,12 @@ All notable changes to this project will be documented in this file. - Support adding SAN entries to generated certificates, this is needed for basically all modern TLS certificate validations when used with HTTPS ([#1044]). -- BREAKING: The constant `DEFAULT_CA_VALIDITY_SECONDS` has been renamed to `DEFAULT_CA_VALIDITY` and now is of type `stackable_operator::time::Duration`. - Also, the constant `ROOT_CA_SUBJECT` has been renamed to `SDP_ROOT_CA_SUBJECT` ([#1044]). ### Changed - GIGA BREAKING: Rewrite entire CA and cert generation to use a builder pattern ([#1044]). +- BREAKING: The constant `DEFAULT_CA_VALIDITY_SECONDS` has been renamed to `DEFAULT_CA_VALIDITY` and now is of type `stackable_operator::time::Duration`. + Also, the constant `ROOT_CA_SUBJECT` has been renamed to `SDP_ROOT_CA_SUBJECT` ([#1044]). ## [0.3.1] - 2024-07-10 From d3e7643b6033b32f1e0375caf10e7519ac634aab Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 23 May 2025 10:05:28 +0200 Subject: [PATCH 05/19] misc cleanup --- crates/stackable-certs/src/ca/ca_builder.rs | 3 +-- crates/stackable-certs/src/ca/consts.rs | 1 + crates/stackable-certs/src/ca/k8s.rs | 15 +++++++-------- crates/stackable-certs/src/cert_builder.rs | 8 +++----- crates/stackable-certs/src/keys/mod.rs | 2 +- crates/stackable-webhook/CHANGELOG.md | 6 ++++++ crates/stackable-webhook/src/tls.rs | 1 + 7 files changed, 20 insertions(+), 16 deletions(-) diff --git a/crates/stackable-certs/src/ca/ca_builder.rs b/crates/stackable-certs/src/ca/ca_builder.rs index 04cc4eea5..413df7c1a 100644 --- a/crates/stackable-certs/src/ca/ca_builder.rs +++ b/crates/stackable-certs/src/ca/ca_builder.rs @@ -157,12 +157,11 @@ where ) .context(CreateCertificateBuilderSnafu)?; - // Add extension constructed above builder .add_extension(&aki) .context(AddCertificateExtensionSnafu)?; - debug!("create and sign CA certificate"); + debug!("creating and signing CA certificate"); let certificate = builder.build().context(BuildCertificateSnafu)?; Ok(CertificateAuthority { diff --git a/crates/stackable-certs/src/ca/consts.rs b/crates/stackable-certs/src/ca/consts.rs index a53c21622..966ef5513 100644 --- a/crates/stackable-certs/src/ca/consts.rs +++ b/crates/stackable-certs/src/ca/consts.rs @@ -10,4 +10,5 @@ pub const DEFAULT_CERTIFICATE_VALIDITY: Duration = Duration::from_hours_unchecke /// The root CA subject name containing only the common name. pub const SDP_ROOT_CA_SUBJECT: &str = "CN=Stackable Data Platform Internal CA"; +/// As we are mostly on Unix systems, we are using `\ņ`. pub const PEM_LINE_ENDING: LineEnding = LineEnding::LF; diff --git a/crates/stackable-certs/src/ca/k8s.rs b/crates/stackable-certs/src/ca/k8s.rs index 7f160915c..bc923a3d2 100644 --- a/crates/stackable-certs/src/ca/k8s.rs +++ b/crates/stackable-certs/src/ca/k8s.rs @@ -10,8 +10,7 @@ use crate::{CertificatePair, keys::CertificateKeypair}; pub const TLS_SECRET_TYPE: &str = "kubernetes.io/tls"; -/// Defines all error variants which can occur when loading a CA from a -/// Kubernetes [`Secret`]. +/// Defines all error variants which can occur when loading a CA from a Kubernetes [`Secret`]. #[derive(Debug, Snafu)] pub enum SecretError where @@ -50,7 +49,7 @@ where /// Create a [`CertificateAuthority`] from a Kubernetes [`Secret`]. /// -/// Both the `certificate_key` and `private_key_key` parameters describe +/// Both the `certificate_key` and `private_key_key` parameters describe /// the _key_ used to lookup the certificate and private key value in the /// Kubernetes [`Secret`]. Common keys are `ca.crt` and `ca.key`. #[instrument(skip(secret))] @@ -71,7 +70,7 @@ where secret: ObjectRef::from_obj(&secret), })?; - debug!("retrieving certificate data from secret via key {certificate_key:?}"); + debug!("retrieving certificate data from secret via key \"{certificate_key}\""); let certificate_data = data .get(certificate_key) .with_context(|| NoCertificateDataSnafu { @@ -84,7 +83,7 @@ where })? .remove(0); - debug!("retrieving private key data from secret via key {private_key_key:?}"); + debug!("retrieving private key data from secret via key \"{private_key_key}\""); let private_key_data = data .get(private_key_key) .with_context(|| NoPrivateKeyDataSnafu { @@ -107,8 +106,8 @@ where #[instrument(skip(secret_ref, client))] pub async fn ca_from_k8s_secret_ref( secret_ref: &SecretReference, - key_certificate: &str, - key_private_key: &str, + certificate_key: &str, + private_key_key: &str, client: &Client, ) -> Result, SecretError> where @@ -123,5 +122,5 @@ where secret_ref: secret_ref.to_owned(), })?; - ca_from_k8s_secret(secret, key_certificate, key_private_key) + ca_from_k8s_secret(secret, certificate_key, private_key_key) } diff --git a/crates/stackable-certs/src/cert_builder.rs b/crates/stackable-certs/src/cert_builder.rs index 9c1f9997a..287a7a7fc 100644 --- a/crates/stackable-certs/src/cert_builder.rs +++ b/crates/stackable-certs/src/cert_builder.rs @@ -22,7 +22,7 @@ use crate::{ keys::CertificateKeypair, }; -/// Defines all error variants which can occur when creating a CertificateRequest +/// Defines all error variants which can occur when creating a certificate #[derive(Debug, Snafu)] pub enum CreateCertificateError where @@ -37,8 +37,6 @@ where subject: String, }, - // #[snafu(display("failed to create key pair"))] - // CreateKeyPair { source: Box }, #[snafu(display("failed to create key pair"))] CreateKeyPair { source: E }, @@ -275,9 +273,9 @@ mod tests { .find(|ext| ext.extn_id == ID_CE_SUBJECT_ALT_NAME) .expect("cert had no SAN extension"); - let actual_sans = SubjectAltName::from_der(san_extension.extn_value.as_bytes()) + let san_extension = SubjectAltName::from_der(san_extension.extn_value.as_bytes()) .expect("failed to parse SAN"); - let actual_sans = actual_sans + let actual_sans = san_extension .0 .iter() .filter_map(|san| match san { diff --git a/crates/stackable-certs/src/keys/mod.rs b/crates/stackable-certs/src/keys/mod.rs index 0608cf556..fe3fe5fd9 100644 --- a/crates/stackable-certs/src/keys/mod.rs +++ b/crates/stackable-certs/src/keys/mod.rs @@ -20,7 +20,7 @@ //! //! ```no_run //! use stackable_certs::keys::{rsa::SigningKey, CertificateKeypair}; -//! let key = SigningKey::new().unwrap();; +//! let key = SigningKey::new().unwrap(); //! ``` //! //! It should be noted, that the crate is currently vulnerable to the recently diff --git a/crates/stackable-webhook/CHANGELOG.md b/crates/stackable-webhook/CHANGELOG.md index 638391632..9669c791a 100644 --- a/crates/stackable-webhook/CHANGELOG.md +++ b/crates/stackable-webhook/CHANGELOG.md @@ -4,11 +4,17 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Changed + +- Use `ecdsa` keys instead of `rsa`. This is because generating 4096 bit `rsa` keys takes forever, + thus making webhook development nearly impossible ([#1044]). + ### Fixed - Don't pull in the `aws-lc-rs` crate, as this currently fails to build in `make run-dev` ([#1043]). [#1043]: https://github.com/stackabletech/operator-rs/pull/1043 +[#1044]: https://github.com/stackabletech/operator-rs/pull/1044 ## [0.3.1] - 2024-07-10 diff --git a/crates/stackable-webhook/src/tls.rs b/crates/stackable-webhook/src/tls.rs index ad5bb1257..092db63b8 100644 --- a/crates/stackable-webhook/src/tls.rs +++ b/crates/stackable-webhook/src/tls.rs @@ -114,6 +114,7 @@ impl TlsServer { .build() .build_ca() .context(CreateCertificateAuthoritySnafu)?; + let certificate = CertificateBuilder::builder() .subject("CN=webhook") .signed_by(&ca) From 70dc74132a9250ed0bf7d4bc6bafb6e01091259c Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 23 May 2025 10:13:10 +0200 Subject: [PATCH 06/19] warn when cert lifes longer than CA --- crates/stackable-certs/src/cert_builder.rs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/crates/stackable-certs/src/cert_builder.rs b/crates/stackable-certs/src/cert_builder.rs index 287a7a7fc..e6b50eb1b 100644 --- a/crates/stackable-certs/src/cert_builder.rs +++ b/crates/stackable-certs/src/cert_builder.rs @@ -5,7 +5,7 @@ use const_oid::db::rfc5280::{ID_KP_CLIENT_AUTH, ID_KP_SERVER_AUTH}; use rsa::pkcs8::EncodePublicKey; use snafu::{ResultExt, Snafu}; use stackable_operator::time::Duration; -use tracing::debug; +use tracing::{debug, warn}; use x509_cert::{ builder::{Builder, Profile}, der::{DecodePem, asn1::Ia5String}, @@ -129,8 +129,7 @@ where ) -> Result, CreateCertificateError> { let serial_number = SerialNumber::from(self.serial_number.unwrap_or_else(|| rand::random::())); - // NOTE (@Techassi): Should we validate that the validity is shorter - // than the validity of the issuing CA? + let validity = Validity::from_now(*self.validity).context(ParseValiditySnafu)?; let subject: Name = self.subject.parse().context(ParseSubjectSnafu { subject: self.subject, @@ -140,6 +139,20 @@ where None => KP::new().context(CreateKeyPairSnafu)?, }; + let ca_validity = self.signed_by.ca_cert().tbs_certificate.validity; + let ca_not_after = ca_validity.not_after.to_system_time(); + let cert_not_after = validity.not_after.to_system_time(); + if ca_not_after < cert_not_after { + warn!( + ca.validity = ?ca_validity, + cert.validity = ?validity, + ca.not_after = ?ca_not_after, + cert.not_after = ?cert_not_after, + subject = ?subject, + "The lifetime of certificate authority is shorted than the lifetime of the generated certificate", + ); + } + let spki_pem = key_pair .verifying_key() .to_public_key_pem(PEM_LINE_ENDING) From b8c667be6f5c471cab1c43b81ad549689b86d34f Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 23 May 2025 13:30:02 +0200 Subject: [PATCH 07/19] Offer convenience functions --- crates/stackable-certs/src/ca/ca_builder.rs | 15 ++++++++++++++- crates/stackable-certs/src/cert_builder.rs | 16 ++++++++++++++-- crates/stackable-webhook/src/tls.rs | 2 -- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/crates/stackable-certs/src/ca/ca_builder.rs b/crates/stackable-certs/src/ca/ca_builder.rs index 413df7c1a..91370c76c 100644 --- a/crates/stackable-certs/src/ca/ca_builder.rs +++ b/crates/stackable-certs/src/ca/ca_builder.rs @@ -68,7 +68,6 @@ where /// }; /// /// let ca: CertificateAuthority = CertificateAuthorityBuilder::builder() -/// .build() /// .build_ca() /// .expect("failed to build CA"); /// ``` @@ -99,6 +98,20 @@ where signing_key_pair: Option, } +impl CertificateAuthorityBuilderBuilder<'_, SKP, S> +where + SKP: CertificateKeypair, + ::VerifyingKey: EncodePublicKey, + S: certificate_authority_builder_builder::IsComplete, +{ + /// Convenience function to avoid calling `builder().build().build_ca()` + pub fn build_ca( + self: Self, + ) -> Result, CreateCertificateAuthorityError> { + self.build().build_ca() + } +} + impl CertificateAuthorityBuilder<'_, SKP> where SKP: CertificateKeypair, diff --git a/crates/stackable-certs/src/cert_builder.rs b/crates/stackable-certs/src/cert_builder.rs index e6b50eb1b..16fbb21a3 100644 --- a/crates/stackable-certs/src/cert_builder.rs +++ b/crates/stackable-certs/src/cert_builder.rs @@ -74,14 +74,12 @@ where /// }; /// /// let ca: CertificateAuthority = CertificateAuthorityBuilder::builder() -/// .build() /// .build_ca() /// .expect("failed to build CA"); /// /// let certificate = CertificateBuilder::builder() /// .subject("CN=trino-coordinator-default-0") /// .signed_by(&ca) -/// .build() /// .build_certificate() /// .expect("failed to build certificate"); /// ``` @@ -119,6 +117,20 @@ where signed_by: &'a CertificateAuthority, } +impl CertificateBuilderBuilder<'_, KP, S> +where + KP: CertificateKeypair, + ::VerifyingKey: EncodePublicKey, + S: certificate_builder_builder::IsComplete, +{ + /// Convenience function to avoid calling `builder().build().build_certificate()` + pub fn build_certificate( + self, + ) -> Result, CreateCertificateError> { + self.build().build_certificate() + } +} + impl CertificateBuilder<'_, KP> where KP: CertificateKeypair, diff --git a/crates/stackable-webhook/src/tls.rs b/crates/stackable-webhook/src/tls.rs index 092db63b8..3282231a7 100644 --- a/crates/stackable-webhook/src/tls.rs +++ b/crates/stackable-webhook/src/tls.rs @@ -111,14 +111,12 @@ impl TlsServer { let task = tokio::task::spawn_blocking(move || { let ca: CertificateAuthority = CertificateAuthorityBuilder::builder() - .build() .build_ca() .context(CreateCertificateAuthoritySnafu)?; let certificate = CertificateBuilder::builder() .subject("CN=webhook") .signed_by(&ca) - .build() .build_certificate() .context(CreateCertificateSnafu)?; From 07fcc893b76995a3ef3931424a6df2ab5bebeb30 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 23 May 2025 14:39:19 +0200 Subject: [PATCH 08/19] Add builder() convenience function --- crates/stackable-certs/src/ca/ca_builder.rs | 5 ++--- crates/stackable-certs/src/ca/mod.rs | 14 +++++++++----- crates/stackable-certs/src/cert_builder.rs | 8 ++++---- crates/stackable-certs/src/lib.rs | 4 ++++ crates/stackable-webhook/src/tls.rs | 13 ++++++------- 5 files changed, 25 insertions(+), 19 deletions(-) diff --git a/crates/stackable-certs/src/ca/ca_builder.rs b/crates/stackable-certs/src/ca/ca_builder.rs index 91370c76c..53253f534 100644 --- a/crates/stackable-certs/src/ca/ca_builder.rs +++ b/crates/stackable-certs/src/ca/ca_builder.rs @@ -63,11 +63,10 @@ where /// /// ```no_run /// use stackable_certs::{ -/// keys::ecdsa, -/// ca::{CertificateAuthority, CertificateAuthorityBuilder}, +/// keys::ecdsa, ca::CertificateAuthority, /// }; /// -/// let ca: CertificateAuthority = CertificateAuthorityBuilder::builder() +/// let ca = CertificateAuthority::::builder() /// .build_ca() /// .expect("failed to build CA"); /// ``` diff --git a/crates/stackable-certs/src/ca/mod.rs b/crates/stackable-certs/src/ca/mod.rs index 86acd1ec1..5545e84ba 100644 --- a/crates/stackable-certs/src/ca/mod.rs +++ b/crates/stackable-certs/src/ca/mod.rs @@ -24,16 +24,20 @@ where certificate_pair: CertificatePair, } -impl CertificateAuthority +impl CertificateAuthority where - S: CertificateKeypair, - ::VerifyingKey: EncodePublicKey, + SK: CertificateKeypair, + ::VerifyingKey: EncodePublicKey, { - pub fn new(certificate_pair: CertificatePair) -> Self { + pub fn new(certificate_pair: CertificatePair) -> Self { Self { certificate_pair } } - pub fn signing_key(&self) -> &S::SigningKey { + pub fn builder() -> CertificateAuthorityBuilderBuilder<'static, SK> { + CertificateAuthorityBuilder::builder() + } + + pub fn signing_key(&self) -> &SK::SigningKey { self.certificate_pair.key_pair().signing_key() } diff --git a/crates/stackable-certs/src/cert_builder.rs b/crates/stackable-certs/src/cert_builder.rs index 16fbb21a3..0b8df8a2a 100644 --- a/crates/stackable-certs/src/cert_builder.rs +++ b/crates/stackable-certs/src/cert_builder.rs @@ -69,15 +69,15 @@ where /// ```no_run /// use stackable_certs::{ /// keys::ecdsa, -/// ca::{CertificateAuthority, CertificateAuthorityBuilder}, -/// CertificateBuilder, +/// ca::CertificateAuthority, +/// CertificatePair, /// }; /// -/// let ca: CertificateAuthority = CertificateAuthorityBuilder::builder() +/// let ca = CertificateAuthority::::builder() /// .build_ca() /// .expect("failed to build CA"); /// -/// let certificate = CertificateBuilder::builder() +/// let certificate = CertificatePair::builder() /// .subject("CN=trino-coordinator-default-0") /// .signed_by(&ca) /// .build_certificate() diff --git a/crates/stackable-certs/src/lib.rs b/crates/stackable-certs/src/lib.rs index e74ccfae8..941b8a2f9 100644 --- a/crates/stackable-certs/src/lib.rs +++ b/crates/stackable-certs/src/lib.rs @@ -121,6 +121,10 @@ where } } + pub fn builder() -> CertificateBuilderBuilder<'static, S> { + CertificateBuilder::builder() + } + /// Returns a reference to the [`Certificate`]. pub fn certificate(&self) -> &Certificate { &self.certificate diff --git a/crates/stackable-webhook/src/tls.rs b/crates/stackable-webhook/src/tls.rs index 3282231a7..c8f5cf8d0 100644 --- a/crates/stackable-webhook/src/tls.rs +++ b/crates/stackable-webhook/src/tls.rs @@ -9,8 +9,8 @@ use hyper_util::rt::{TokioExecutor, TokioIo}; use opentelemetry::trace::{FutureExt, SpanKind}; use snafu::{ResultExt, Snafu}; use stackable_certs::{ - CertificateBuilder, CertificatePairError, CreateCertificateError, - ca::{CertificateAuthority, CertificateAuthorityBuilder, CreateCertificateAuthorityError}, + CertificatePair, CertificatePairError, CreateCertificateError, + ca::{CertificateAuthority, CreateCertificateAuthorityError}, keys::ecdsa, }; use tokio::net::TcpListener; @@ -109,12 +109,11 @@ impl TlsServer { // blocked. // See https://docs.rs/tokio/latest/tokio/task/fn.spawn_blocking.html let task = tokio::task::spawn_blocking(move || { - let ca: CertificateAuthority = - CertificateAuthorityBuilder::builder() - .build_ca() - .context(CreateCertificateAuthoritySnafu)?; + let ca: CertificateAuthority = CertificateAuthority::builder() + .build_ca() + .context(CreateCertificateAuthoritySnafu)?; - let certificate = CertificateBuilder::builder() + let certificate = CertificatePair::builder() .subject("CN=webhook") .signed_by(&ca) .build_certificate() From 4c84aea95c7126d4a017eccc9267fecc0fe8ec87 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 23 May 2025 14:46:42 +0200 Subject: [PATCH 09/19] Rename the convenience function ro build() --- crates/stackable-certs/src/ca/ca_builder.rs | 13 ++++++------- crates/stackable-certs/src/cert_builder.rs | 21 +++++++-------------- crates/stackable-webhook/src/tls.rs | 4 ++-- 3 files changed, 15 insertions(+), 23 deletions(-) diff --git a/crates/stackable-certs/src/ca/ca_builder.rs b/crates/stackable-certs/src/ca/ca_builder.rs index 53253f534..08ef0cbc4 100644 --- a/crates/stackable-certs/src/ca/ca_builder.rs +++ b/crates/stackable-certs/src/ca/ca_builder.rs @@ -67,10 +67,11 @@ where /// }; /// /// let ca = CertificateAuthority::::builder() -/// .build_ca() +/// .build() /// .expect("failed to build CA"); /// ``` #[derive(Builder)] +#[builder(finish_fn = finish_builder)] pub struct CertificateAuthorityBuilder<'a, SKP> where SKP: CertificateKeypair, @@ -103,11 +104,11 @@ where ::VerifyingKey: EncodePublicKey, S: certificate_authority_builder_builder::IsComplete, { - /// Convenience function to avoid calling `builder().build().build_ca()` - pub fn build_ca( + /// Convenience function to avoid calling `builder().finish_builder().build()` + pub fn build( self: Self, ) -> Result, CreateCertificateAuthorityError> { - self.build().build_ca() + self.finish_builder().build() } } @@ -116,7 +117,7 @@ where SKP: CertificateKeypair, ::VerifyingKey: EncodePublicKey, { - pub fn build_ca( + pub fn build( self, ) -> Result, CreateCertificateAuthorityError> { let serial_number = @@ -196,7 +197,6 @@ mod tests { fn minimal_ca() { let ca: CertificateAuthority = CertificateAuthorityBuilder::builder() .build() - .build_ca() .expect("failed to build CA"); assert_ca_cert_attributes( @@ -215,7 +215,6 @@ mod tests { .signing_key_pair(rsa::SigningKey::new().unwrap()) .validity(Duration::from_days_unchecked(13)) .build() - .build_ca() .expect("failed to build CA"); assert_ca_cert_attributes( diff --git a/crates/stackable-certs/src/cert_builder.rs b/crates/stackable-certs/src/cert_builder.rs index 0b8df8a2a..c85cbe80e 100644 --- a/crates/stackable-certs/src/cert_builder.rs +++ b/crates/stackable-certs/src/cert_builder.rs @@ -74,16 +74,17 @@ where /// }; /// /// let ca = CertificateAuthority::::builder() -/// .build_ca() +/// .build() /// .expect("failed to build CA"); /// /// let certificate = CertificatePair::builder() /// .subject("CN=trino-coordinator-default-0") /// .signed_by(&ca) -/// .build_certificate() +/// .build() /// .expect("failed to build certificate"); /// ``` #[derive(Builder)] +#[builder(finish_fn = finish_builder)] pub struct CertificateBuilder<'a, KP> where KP: CertificateKeypair, @@ -123,11 +124,9 @@ where ::VerifyingKey: EncodePublicKey, S: certificate_builder_builder::IsComplete, { - /// Convenience function to avoid calling `builder().build().build_certificate()` - pub fn build_certificate( - self, - ) -> Result, CreateCertificateError> { - self.build().build_certificate() + /// Convenience function to avoid calling `builder().finish_builder().build()` + pub fn build(self) -> Result, CreateCertificateError> { + self.finish_builder().build() } } @@ -136,9 +135,7 @@ where KP: CertificateKeypair, ::VerifyingKey: EncodePublicKey, { - pub fn build_certificate( - self, - ) -> Result, CreateCertificateError> { + pub fn build(self) -> Result, CreateCertificateError> { let serial_number = SerialNumber::from(self.serial_number.unwrap_or_else(|| rand::random::())); @@ -241,7 +238,6 @@ mod tests { .subject("CN=trino-coordinator-default-0") .signed_by(&ca) .build() - .build_certificate() .expect("failed to build certificate"); assert_certificate_attributes( @@ -268,7 +264,6 @@ mod tests { .key_pair(rsa::SigningKey::new().unwrap()) .signed_by(&ca) .build() - .build_certificate() .expect("failed to build certificate"); assert_certificate_attributes( @@ -329,14 +324,12 @@ mod tests { fn get_ecdsa_ca() -> CertificateAuthority { CertificateAuthorityBuilder::builder() .build() - .build_ca() .expect("failed to build CA") } fn get_rsa_ca() -> CertificateAuthority { CertificateAuthorityBuilder::builder() .build() - .build_ca() .expect("failed to build CA") } } diff --git a/crates/stackable-webhook/src/tls.rs b/crates/stackable-webhook/src/tls.rs index c8f5cf8d0..4348b8ce3 100644 --- a/crates/stackable-webhook/src/tls.rs +++ b/crates/stackable-webhook/src/tls.rs @@ -110,13 +110,13 @@ impl TlsServer { // See https://docs.rs/tokio/latest/tokio/task/fn.spawn_blocking.html let task = tokio::task::spawn_blocking(move || { let ca: CertificateAuthority = CertificateAuthority::builder() - .build_ca() + .build() .context(CreateCertificateAuthoritySnafu)?; let certificate = CertificatePair::builder() .subject("CN=webhook") .signed_by(&ca) - .build_certificate() + .build() .context(CreateCertificateSnafu)?; let certificate_der = certificate From ac18bd9301d85f676e0c5104436e8d0a1a1eda8b Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Mon, 26 May 2025 16:12:45 +0200 Subject: [PATCH 10/19] Simply tests --- crates/stackable-certs/src/cert_builder.rs | 23 +++++++++------------- crates/stackable-webhook/src/tls.rs | 2 +- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/crates/stackable-certs/src/cert_builder.rs b/crates/stackable-certs/src/cert_builder.rs index c85cbe80e..96069f8b2 100644 --- a/crates/stackable-certs/src/cert_builder.rs +++ b/crates/stackable-certs/src/cert_builder.rs @@ -233,7 +233,10 @@ mod tests { #[test] fn minimal_certificate() { - let ca = get_ecdsa_ca(); + let ca = CertificateAuthorityBuilder::::builder() + .build() + .expect("failed to build CA"); + let certificate = CertificateBuilder::builder() .subject("CN=trino-coordinator-default-0") .signed_by(&ca) @@ -251,11 +254,15 @@ mod tests { #[test] fn customized_certificate() { - let ca = get_rsa_ca(); + let ca = CertificateAuthorityBuilder::builder() + .build() + .expect("failed to build CA"); + let sans = [ "trino-coordinator-default-0.trino-coordinator-default.default.svc.cluster-local", "trino-coordinator-default.default.svc.cluster-local", ]; + let certificate = CertificateBuilder::builder() .subject("CN=trino-coordinator-default-0") .subject_alterative_dns_names(&sans) @@ -320,16 +327,4 @@ mod tests { assert_ne!(certificate.serial_number, SerialNumber::from(0_u64)) } } - - fn get_ecdsa_ca() -> CertificateAuthority { - CertificateAuthorityBuilder::builder() - .build() - .expect("failed to build CA") - } - - fn get_rsa_ca() -> CertificateAuthority { - CertificateAuthorityBuilder::builder() - .build() - .expect("failed to build CA") - } } diff --git a/crates/stackable-webhook/src/tls.rs b/crates/stackable-webhook/src/tls.rs index 4348b8ce3..37131aad8 100644 --- a/crates/stackable-webhook/src/tls.rs +++ b/crates/stackable-webhook/src/tls.rs @@ -109,7 +109,7 @@ impl TlsServer { // blocked. // See https://docs.rs/tokio/latest/tokio/task/fn.spawn_blocking.html let task = tokio::task::spawn_blocking(move || { - let ca: CertificateAuthority = CertificateAuthority::builder() + let ca = CertificateAuthority::::builder() .build() .context(CreateCertificateAuthoritySnafu)?; From 8f86a935a164309ac5b7974045bf9cb623b46ca8 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Mon, 26 May 2025 16:49:13 +0200 Subject: [PATCH 11/19] Support SAN IPs --- crates/stackable-certs/src/cert_builder.rs | 83 +++++++++++++++++----- 1 file changed, 65 insertions(+), 18 deletions(-) diff --git a/crates/stackable-certs/src/cert_builder.rs b/crates/stackable-certs/src/cert_builder.rs index 96069f8b2..d9ba04963 100644 --- a/crates/stackable-certs/src/cert_builder.rs +++ b/crates/stackable-certs/src/cert_builder.rs @@ -1,4 +1,4 @@ -use std::fmt::Debug; +use std::{fmt::Debug, net::IpAddr}; use bon::Builder; use const_oid::db::rfc5280::{ID_KP_CLIENT_AUTH, ID_KP_SERVER_AUTH}; @@ -52,9 +52,11 @@ where #[snafu(display("failed to add certificate extension"))] AddCertificateExtension { source: x509_cert::builder::Error }, - #[snafu(display("The subject alternative DNS name \"{dns_name}\" is not a Ia5String"))] - SaDnsNameNotAIa5String { - dns_name: String, + #[snafu(display( + "failed to parse subject alternative DNS name \"{subject_alternative_dns_name}\" as a Ia5 string" + ))] + ParseSubjectAlternativeDnsName { + subject_alternative_dns_name: String, source: x509_cert::der::Error, }, @@ -93,11 +95,16 @@ where /// Required subject of the certificate, usually starts with `CN=`. subject: &'a str, - /// Optional list of subject alternative names (SAN) DNS entries, + /// Optional list of subject alternative name DNS entries /// that are added to the certificate. #[builder(default)] subject_alterative_dns_names: &'a [&'a str], + /// Optional list of subject alternative name IP address entries + /// that are added to the certificate. + #[builder(default)] + subject_alterative_ip_addresses: &'a [IpAddr], + /// Validity/lifetime of the certificate. /// /// If not specified the default of [`DEFAULT_CERTIFICATE_VALIDITY`] will be used. @@ -194,17 +201,23 @@ where ])) .context(AddCertificateExtensionSnafu)?; - let sans = self - .subject_alterative_dns_names + let san_dns = self.subject_alterative_dns_names.iter().map(|dns_name| { + Ok(GeneralName::DnsName( + Ia5String::new(dns_name).with_context(|_| ParseSubjectAlternativeDnsNameSnafu { + subject_alternative_dns_name: dns_name.to_string(), + })?, + )) + }); + let san_ips = self + .subject_alterative_ip_addresses .iter() - .map(|dns_name| { - Ok(GeneralName::DnsName(Ia5String::new(dns_name).context( - SaDnsNameNotAIa5StringSnafu { - dns_name: dns_name.to_string(), - }, - )?)) - }) + .copied() + .map(GeneralName::from) + .map(Result::Ok); + let sans = san_dns + .chain(san_ips) .collect::, CreateCertificateError>>()?; + builder .add_extension(&SubjectAltName(sans)) .context(AddCertificateExtensionSnafu)?; @@ -221,6 +234,8 @@ where #[cfg(test)] mod tests { + use std::net::{Ipv4Addr, Ipv6Addr}; + use x509_cert::{ certificate::TbsCertificateInner, der::Decode, ext::pkix::ID_CE_SUBJECT_ALT_NAME, }; @@ -247,6 +262,7 @@ mod tests { &certificate.certificate.tbs_certificate, "CN=trino-coordinator-default-0", &[], + &[], DEFAULT_CERTIFICATE_VALIDITY, None, ); @@ -262,10 +278,12 @@ mod tests { "trino-coordinator-default-0.trino-coordinator-default.default.svc.cluster-local", "trino-coordinator-default.default.svc.cluster-local", ]; + let san_ips = ["10.0.0.1".parse().unwrap(), "fe80::42".parse().unwrap()]; let certificate = CertificateBuilder::builder() .subject("CN=trino-coordinator-default-0") .subject_alterative_dns_names(&sans) + .subject_alterative_ip_addresses(&san_ips) .serial_number(08121997) .validity(Duration::from_days_unchecked(42)) .key_pair(rsa::SigningKey::new().unwrap()) @@ -277,6 +295,7 @@ mod tests { &certificate.certificate.tbs_certificate, "CN=trino-coordinator-default-0", &sans, + &san_ips, Duration::from_days_unchecked(42), Some(08121997), ); @@ -286,6 +305,7 @@ mod tests { certificate: &TbsCertificateInner, subject: &str, sans: &[&str], + san_ips: &[IpAddr], validity: Duration, serial_number: Option, ) { @@ -300,10 +320,10 @@ mod tests { .find(|ext| ext.extn_id == ID_CE_SUBJECT_ALT_NAME) .expect("cert had no SAN extension"); - let san_extension = SubjectAltName::from_der(san_extension.extn_value.as_bytes()) - .expect("failed to parse SAN"); - let actual_sans = san_extension - .0 + let san_entries = SubjectAltName::from_der(san_extension.extn_value.as_bytes()) + .expect("failed to parse SAN") + .0; + let actual_sans = san_entries .iter() .filter_map(|san| match san { GeneralName::DnsName(dns_name) => Some(dns_name.as_str()), @@ -311,6 +331,14 @@ mod tests { }) .collect::>(); assert_eq!(actual_sans, sans); + let actual_san_ips = san_entries + .iter() + .filter_map(|san| match san { + GeneralName::IpAddress(ip) => Some(bytes_to_ip_addr(ip.as_bytes())), + _ => None, + }) + .collect::>(); + assert_eq!(actual_san_ips, san_ips); let not_before = certificate.validity.not_before.to_system_time(); let not_after = certificate.validity.not_after.to_system_time(); @@ -327,4 +355,23 @@ mod tests { assert_ne!(certificate.serial_number, SerialNumber::from(0_u64)) } } + + fn bytes_to_ip_addr(bytes: &[u8]) -> IpAddr { + match bytes.len() { + 4 => { + let mut array = [0u8; 4]; + array.copy_from_slice(bytes); + IpAddr::V4(Ipv4Addr::from(array)) + } + 16 => { + let mut array = [0u8; 16]; + array.copy_from_slice(bytes); + IpAddr::V6(Ipv6Addr::from(array)) + } + _ => panic!( + "Invalid IP byte length: expected 4 or 16, got {}", + bytes.len() + ), + } + } } From 399571724a21ed7c214de68600e9449fa26306e3 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Tue, 27 May 2025 08:40:38 +0200 Subject: [PATCH 12/19] Rename inner builder() to start_builder() --- crates/stackable-certs/src/ca/ca_builder.rs | 6 +++--- crates/stackable-certs/src/ca/mod.rs | 2 +- crates/stackable-certs/src/cert_builder.rs | 15 ++++++--------- crates/stackable-certs/src/lib.rs | 2 +- 4 files changed, 11 insertions(+), 14 deletions(-) diff --git a/crates/stackable-certs/src/ca/ca_builder.rs b/crates/stackable-certs/src/ca/ca_builder.rs index 08ef0cbc4..fa30d14eb 100644 --- a/crates/stackable-certs/src/ca/ca_builder.rs +++ b/crates/stackable-certs/src/ca/ca_builder.rs @@ -71,7 +71,7 @@ where /// .expect("failed to build CA"); /// ``` #[derive(Builder)] -#[builder(finish_fn = finish_builder)] +#[builder(start_fn = start_builder, finish_fn = finish_builder)] pub struct CertificateAuthorityBuilder<'a, SKP> where SKP: CertificateKeypair, @@ -195,7 +195,7 @@ mod tests { #[test] fn minimal_ca() { - let ca: CertificateAuthority = CertificateAuthorityBuilder::builder() + let ca: CertificateAuthority = CertificateAuthority::builder() .build() .expect("failed to build CA"); @@ -209,7 +209,7 @@ mod tests { #[test] fn customized_ca() { - let ca = CertificateAuthorityBuilder::builder() + let ca = CertificateAuthority::builder() .subject("CN=Test") .serial_number(42) .signing_key_pair(rsa::SigningKey::new().unwrap()) diff --git a/crates/stackable-certs/src/ca/mod.rs b/crates/stackable-certs/src/ca/mod.rs index 5545e84ba..cbed74b7c 100644 --- a/crates/stackable-certs/src/ca/mod.rs +++ b/crates/stackable-certs/src/ca/mod.rs @@ -34,7 +34,7 @@ where } pub fn builder() -> CertificateAuthorityBuilderBuilder<'static, SK> { - CertificateAuthorityBuilder::builder() + CertificateAuthorityBuilder::start_builder() } pub fn signing_key(&self) -> &SK::SigningKey { diff --git a/crates/stackable-certs/src/cert_builder.rs b/crates/stackable-certs/src/cert_builder.rs index d9ba04963..b7ae707c4 100644 --- a/crates/stackable-certs/src/cert_builder.rs +++ b/crates/stackable-certs/src/cert_builder.rs @@ -86,7 +86,7 @@ where /// .expect("failed to build certificate"); /// ``` #[derive(Builder)] -#[builder(finish_fn = finish_builder)] +#[builder(start_fn = start_builder, finish_fn = finish_builder)] pub struct CertificateBuilder<'a, KP> where KP: CertificateKeypair, @@ -241,18 +241,15 @@ mod tests { }; use super::*; - use crate::{ - ca::CertificateAuthorityBuilder, - keys::{ecdsa, rsa}, - }; + use crate::keys::{ecdsa, rsa}; #[test] fn minimal_certificate() { - let ca = CertificateAuthorityBuilder::::builder() + let ca = CertificateAuthority::::builder() .build() .expect("failed to build CA"); - let certificate = CertificateBuilder::builder() + let certificate = CertificatePair::builder() .subject("CN=trino-coordinator-default-0") .signed_by(&ca) .build() @@ -270,7 +267,7 @@ mod tests { #[test] fn customized_certificate() { - let ca = CertificateAuthorityBuilder::builder() + let ca = CertificateAuthority::builder() .build() .expect("failed to build CA"); @@ -280,7 +277,7 @@ mod tests { ]; let san_ips = ["10.0.0.1".parse().unwrap(), "fe80::42".parse().unwrap()]; - let certificate = CertificateBuilder::builder() + let certificate = CertificatePair::builder() .subject("CN=trino-coordinator-default-0") .subject_alterative_dns_names(&sans) .subject_alterative_ip_addresses(&san_ips) diff --git a/crates/stackable-certs/src/lib.rs b/crates/stackable-certs/src/lib.rs index 941b8a2f9..ae03c0f65 100644 --- a/crates/stackable-certs/src/lib.rs +++ b/crates/stackable-certs/src/lib.rs @@ -122,7 +122,7 @@ where } pub fn builder() -> CertificateBuilderBuilder<'static, S> { - CertificateBuilder::builder() + CertificateBuilder::start_builder() } /// Returns a reference to the [`Certificate`]. From e9e8bacf23b6f7d3fec9affd88be8607484519db Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Tue, 27 May 2025 08:51:43 +0200 Subject: [PATCH 13/19] Dont let users choose serial --- crates/stackable-certs/src/ca/ca_builder.rs | 24 ++------------------- crates/stackable-certs/src/cert_builder.rs | 19 +--------------- 2 files changed, 3 insertions(+), 40 deletions(-) diff --git a/crates/stackable-certs/src/ca/ca_builder.rs b/crates/stackable-certs/src/ca/ca_builder.rs index fa30d14eb..4888f8346 100644 --- a/crates/stackable-certs/src/ca/ca_builder.rs +++ b/crates/stackable-certs/src/ca/ca_builder.rs @@ -87,11 +87,6 @@ where #[builder(default = DEFAULT_CA_VALIDITY)] validity: Duration, - /// Serial number of the generated certificate. - /// - /// If not specified a random serial will be generated. - serial_number: Option, - /// Cryptographic keypair used to sign leaf certificates. /// /// If not specified a random keypair will be generated. @@ -120,8 +115,6 @@ where pub fn build( self, ) -> Result, CreateCertificateAuthorityError> { - let serial_number = - SerialNumber::from(self.serial_number.unwrap_or_else(|| rand::random::())); let validity = Validity::from_now(*self.validity).context(ParseValiditySnafu)?; let subject: Name = self.subject.parse().context(ParseSubjectSnafu { subject: self.subject, @@ -130,6 +123,7 @@ where Some(signing_key_pair) => signing_key_pair, None => SKP::new().context(CreateSigningKeyPairSnafu)?, }; + let serial_number = SerialNumber::from(rand::random::()); let spki_pem = signing_key_pair .verifying_key() @@ -203,7 +197,6 @@ mod tests { &ca.ca_cert().tbs_certificate, SDP_ROOT_CA_SUBJECT, DEFAULT_CA_VALIDITY, - None, ) } @@ -211,7 +204,6 @@ mod tests { fn customized_ca() { let ca = CertificateAuthority::builder() .subject("CN=Test") - .serial_number(42) .signing_key_pair(rsa::SigningKey::new().unwrap()) .validity(Duration::from_days_unchecked(13)) .build() @@ -221,16 +213,10 @@ mod tests { &ca.ca_cert().tbs_certificate, "CN=Test", Duration::from_days_unchecked(13), - Some(42), ) } - fn assert_ca_cert_attributes( - ca_cert: &TbsCertificateInner, - subject: &str, - validity: Duration, - serial_number: Option, - ) { + fn assert_ca_cert_attributes(ca_cert: &TbsCertificateInner, subject: &str, validity: Duration) { assert_eq!(ca_cert.subject, subject.parse().unwrap()); let not_before = ca_cert.validity.not_before.to_system_time(); @@ -241,11 +227,5 @@ mod tests { .expect("Failed to calculate duration between notBefore and notAfter"), *validity ); - - if let Some(serial_number) = serial_number { - assert_eq!(ca_cert.serial_number, SerialNumber::from(serial_number)) - } else { - assert_ne!(ca_cert.serial_number, SerialNumber::from(0_u64)) - } } } diff --git a/crates/stackable-certs/src/cert_builder.rs b/crates/stackable-certs/src/cert_builder.rs index b7ae707c4..6599d8e77 100644 --- a/crates/stackable-certs/src/cert_builder.rs +++ b/crates/stackable-certs/src/cert_builder.rs @@ -111,11 +111,6 @@ where #[builder(default = DEFAULT_CERTIFICATE_VALIDITY)] validity: Duration, - /// Serial number of the generated certificate. - /// - /// If not specified a random serial will be generated. - serial_number: Option, - /// Cryptographic keypair used to for the certificates. /// /// If not specified a random keypair will be generated. @@ -143,9 +138,6 @@ where ::VerifyingKey: EncodePublicKey, { pub fn build(self) -> Result, CreateCertificateError> { - let serial_number = - SerialNumber::from(self.serial_number.unwrap_or_else(|| rand::random::())); - let validity = Validity::from_now(*self.validity).context(ParseValiditySnafu)?; let subject: Name = self.subject.parse().context(ParseSubjectSnafu { subject: self.subject, @@ -154,6 +146,7 @@ where Some(key_pair) => key_pair, None => KP::new().context(CreateKeyPairSnafu)?, }; + let serial_number = SerialNumber::from(rand::random::()); let ca_validity = self.signed_by.ca_cert().tbs_certificate.validity; let ca_not_after = ca_validity.not_after.to_system_time(); @@ -261,7 +254,6 @@ mod tests { &[], &[], DEFAULT_CERTIFICATE_VALIDITY, - None, ); } @@ -281,7 +273,6 @@ mod tests { .subject("CN=trino-coordinator-default-0") .subject_alterative_dns_names(&sans) .subject_alterative_ip_addresses(&san_ips) - .serial_number(08121997) .validity(Duration::from_days_unchecked(42)) .key_pair(rsa::SigningKey::new().unwrap()) .signed_by(&ca) @@ -294,7 +285,6 @@ mod tests { &sans, &san_ips, Duration::from_days_unchecked(42), - Some(08121997), ); } @@ -304,7 +294,6 @@ mod tests { sans: &[&str], san_ips: &[IpAddr], validity: Duration, - serial_number: Option, ) { assert_eq!(certificate.subject, subject.parse().unwrap()); @@ -345,12 +334,6 @@ mod tests { .expect("Failed to calculate duration between notBefore and notAfter"), *validity ); - - if let Some(serial_number) = serial_number { - assert_eq!(certificate.serial_number, SerialNumber::from(serial_number)) - } else { - assert_ne!(certificate.serial_number, SerialNumber::from(0_u64)) - } } fn bytes_to_ip_addr(bytes: &[u8]) -> IpAddr { From e946556435efe813d5ae3d054e4d62cee5c7bc84 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Tue, 27 May 2025 10:15:31 +0200 Subject: [PATCH 14/19] Add some instrumentation --- crates/stackable-certs/src/ca/ca_builder.rs | 18 +++++++-- crates/stackable-certs/src/cert_builder.rs | 44 ++++++++++++++------- crates/stackable-certs/src/keys/ecdsa.rs | 9 +++++ crates/stackable-certs/src/keys/mod.rs | 6 +++ crates/stackable-certs/src/keys/rsa.rs | 8 ++++ 5 files changed, 68 insertions(+), 17 deletions(-) diff --git a/crates/stackable-certs/src/ca/ca_builder.rs b/crates/stackable-certs/src/ca/ca_builder.rs index 4888f8346..493a7919c 100644 --- a/crates/stackable-certs/src/ca/ca_builder.rs +++ b/crates/stackable-certs/src/ca/ca_builder.rs @@ -2,7 +2,7 @@ use bon::Builder; use rsa::pkcs8::EncodePublicKey; use snafu::{ResultExt, Snafu}; use stackable_operator::time::Duration; -use tracing::debug; +use tracing::{debug, instrument}; use x509_cert::{ builder::{Builder, CertificateBuilder, Profile}, der::{DecodePem, referenced::OwnedToRef}, @@ -112,6 +112,11 @@ where SKP: CertificateKeypair, ::VerifyingKey: EncodePublicKey, { + #[instrument( + name = "build_certificate_authority", + skip(self), + fields(subject = self.subject), + )] pub fn build( self, ) -> Result, CreateCertificateAuthorityError> { @@ -153,6 +158,15 @@ where let aki = AuthorityKeyIdentifier::try_from(spki.owned_to_ref()) .context(ParseAuthorityKeyIdentifierSnafu)?; + debug!( + ca.subject = %subject, + ca.not_after = %validity.not_after, + ca.not_before = %validity.not_before, + ca.serial = ?serial_number, + ca.public_key.algorithm = SKP::algorithm_name(), + ca.public_key.size = SKP::key_size(), + "creating certificate authority" + ); let signer = signing_key_pair.signing_key(); let mut builder = CertificateBuilder::new( Profile::Root, @@ -167,8 +181,6 @@ where builder .add_extension(&aki) .context(AddCertificateExtensionSnafu)?; - - debug!("creating and signing CA certificate"); let certificate = builder.build().context(BuildCertificateSnafu)?; Ok(CertificateAuthority { diff --git a/crates/stackable-certs/src/cert_builder.rs b/crates/stackable-certs/src/cert_builder.rs index 6599d8e77..602363be5 100644 --- a/crates/stackable-certs/src/cert_builder.rs +++ b/crates/stackable-certs/src/cert_builder.rs @@ -5,7 +5,7 @@ use const_oid::db::rfc5280::{ID_KP_CLIENT_AUTH, ID_KP_SERVER_AUTH}; use rsa::pkcs8::EncodePublicKey; use snafu::{ResultExt, Snafu}; use stackable_operator::time::Duration; -use tracing::{debug, warn}; +use tracing::{debug, instrument, warn}; use x509_cert::{ builder::{Builder, Profile}, der::{DecodePem, asn1::Ia5String}, @@ -98,12 +98,12 @@ where /// Optional list of subject alternative name DNS entries /// that are added to the certificate. #[builder(default)] - subject_alterative_dns_names: &'a [&'a str], + subject_alternative_dns_names: &'a [&'a str], /// Optional list of subject alternative name IP address entries /// that are added to the certificate. #[builder(default)] - subject_alterative_ip_addresses: &'a [IpAddr], + subject_alternative_ip_addresses: &'a [IpAddr], /// Validity/lifetime of the certificate. /// @@ -132,19 +132,24 @@ where } } -impl CertificateBuilder<'_, KP> +impl CertificateBuilder<'_, SKP> where - KP: CertificateKeypair, - ::VerifyingKey: EncodePublicKey, + SKP: CertificateKeypair, + ::VerifyingKey: EncodePublicKey, { - pub fn build(self) -> Result, CreateCertificateError> { + #[instrument( + name = "build_certificate", + skip(self), + fields(subject = self.subject), + )] + pub fn build(self) -> Result, CreateCertificateError> { let validity = Validity::from_now(*self.validity).context(ParseValiditySnafu)?; let subject: Name = self.subject.parse().context(ParseSubjectSnafu { subject: self.subject, })?; let key_pair = match self.key_pair { Some(key_pair) => key_pair, - None => KP::new().context(CreateKeyPairSnafu)?, + None => SKP::new().context(CreateKeyPairSnafu)?, }; let serial_number = SerialNumber::from(rand::random::()); @@ -170,6 +175,18 @@ where let spki = SubjectPublicKeyInfoOwned::from_pem(spki_pem.as_bytes()) .context(DecodeSpkiFromPemSnafu)?; + debug!( + certificate.subject = %subject, + certificate.not_after = %validity.not_after, + certificate.not_before = %validity.not_before, + certificate.serial = %serial_number, + certificate.san.dns_names = ?self.subject_alternative_dns_names, + certificate.san.ip_addresses = ?self.subject_alternative_ip_addresses, + certificate.signed_by.issuer = %self.signed_by.issuer_name(), + certificate.public_key.algorithm = SKP::algorithm_name(), + certificate.public_key.size = SKP::key_size(), + "creating and signing certificate" + ); let signing_key = self.signed_by.signing_key(); let mut builder = x509_cert::builder::CertificateBuilder::new( Profile::Leaf { @@ -194,7 +211,7 @@ where ])) .context(AddCertificateExtensionSnafu)?; - let san_dns = self.subject_alterative_dns_names.iter().map(|dns_name| { + let san_dns = self.subject_alternative_dns_names.iter().map(|dns_name| { Ok(GeneralName::DnsName( Ia5String::new(dns_name).with_context(|_| ParseSubjectAlternativeDnsNameSnafu { subject_alternative_dns_name: dns_name.to_string(), @@ -202,20 +219,19 @@ where )) }); let san_ips = self - .subject_alterative_ip_addresses + .subject_alternative_ip_addresses .iter() .copied() .map(GeneralName::from) .map(Result::Ok); let sans = san_dns .chain(san_ips) - .collect::, CreateCertificateError>>()?; + .collect::, CreateCertificateError>>()?; builder .add_extension(&SubjectAltName(sans)) .context(AddCertificateExtensionSnafu)?; - debug!("create and sign leaf certificate"); let certificate = builder.build().context(BuildCertificateSnafu)?; Ok(CertificatePair { @@ -271,8 +287,8 @@ mod tests { let certificate = CertificatePair::builder() .subject("CN=trino-coordinator-default-0") - .subject_alterative_dns_names(&sans) - .subject_alterative_ip_addresses(&san_ips) + .subject_alternative_dns_names(&sans) + .subject_alternative_ip_addresses(&san_ips) .validity(Duration::from_days_unchecked(42)) .key_pair(rsa::SigningKey::new().unwrap()) .signed_by(&ca) diff --git a/crates/stackable-certs/src/keys/ecdsa.rs b/crates/stackable-certs/src/keys/ecdsa.rs index 8d7adea51..b9311dd7a 100644 --- a/crates/stackable-certs/src/keys/ecdsa.rs +++ b/crates/stackable-certs/src/keys/ecdsa.rs @@ -58,4 +58,13 @@ impl CertificateKeypair for SigningKey { Ok(Self(signing_key)) } + + fn algorithm_name() -> &'static str { + "ecdsa" + } + + fn key_size() -> usize { + // Different than by RSA, we can not pass the key size during construction + 256 + } } diff --git a/crates/stackable-certs/src/keys/mod.rs b/crates/stackable-certs/src/keys/mod.rs index fe3fe5fd9..804c5ca31 100644 --- a/crates/stackable-certs/src/keys/mod.rs +++ b/crates/stackable-certs/src/keys/mod.rs @@ -71,4 +71,10 @@ where /// Creates a signing key pair from the PEM-encoded private key. fn from_pkcs8_pem(input: &str) -> Result; + + /// The name of the algorithm such as `rsa` or `ecdsa`. + fn algorithm_name() -> &'static str; + + /// The key length in bits + fn key_size() -> usize; } diff --git a/crates/stackable-certs/src/keys/rsa.rs b/crates/stackable-certs/src/keys/rsa.rs index 836a8b36e..babeb72b7 100644 --- a/crates/stackable-certs/src/keys/rsa.rs +++ b/crates/stackable-certs/src/keys/rsa.rs @@ -79,4 +79,12 @@ impl CertificateKeypair for SigningKey { Ok(Self(signing_key)) } + + fn algorithm_name() -> &'static str { + "rsa" + } + + fn key_size() -> usize { + KEY_SIZE + } } From 13227797f3275f4f348b5cb86050019d2988e9de Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Tue, 27 May 2025 13:18:56 +0200 Subject: [PATCH 15/19] Cary over some docs --- crates/stackable-certs/src/ca/ca_builder.rs | 16 +++++++++++++++- crates/stackable-certs/src/cert_builder.rs | 21 +++++++++++++++++++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/crates/stackable-certs/src/ca/ca_builder.rs b/crates/stackable-certs/src/ca/ca_builder.rs index 493a7919c..832ee546c 100644 --- a/crates/stackable-certs/src/ca/ca_builder.rs +++ b/crates/stackable-certs/src/ca/ca_builder.rs @@ -59,6 +59,17 @@ where /// This builder builds certificate authorities of type [`CertificateAuthority`]. /// +/// It has many default values, notably; +/// +/// - A default validity of [`DEFAULT_CA_VALIDITY`] +/// - A default subject of [`SDP_ROOT_CA_SUBJECT`] +/// - A randomly generated serial number +/// - In case no `signing_key_pair` was provided, a fresh keypair will be created. The algorithm +/// (`rsa`/`ecdsa`) is chosen by the generic [`CertificateKeypair`] type of this struct. +/// +/// The CA contains the public half of the provided `signing_key_pair` and is signed by the private +/// half of said key. +/// /// Example code to construct a CA: /// /// ```no_run @@ -101,7 +112,7 @@ where { /// Convenience function to avoid calling `builder().finish_builder().build()` pub fn build( - self: Self, + self, ) -> Result, CreateCertificateAuthorityError> { self.finish_builder().build() } @@ -128,6 +139,9 @@ where Some(signing_key_pair) => signing_key_pair, None => SKP::new().context(CreateSigningKeyPairSnafu)?, }; + + // By choosing a random serial number we can make the reasonable assumption that we generate + // a unique serial for each CA. let serial_number = SerialNumber::from(rand::random::()); let spki_pem = signing_key_pair diff --git a/crates/stackable-certs/src/cert_builder.rs b/crates/stackable-certs/src/cert_builder.rs index 602363be5..06141eca2 100644 --- a/crates/stackable-certs/src/cert_builder.rs +++ b/crates/stackable-certs/src/cert_builder.rs @@ -66,7 +66,21 @@ where /// This builder builds certificates of type [`CertificatePair`]. /// -/// Example code to construct a certificate: +/// Currently you are required to specify a [`CertificateAuthority`], which is used to create a leaf +/// certificate, which is signed by this CA. +/// +/// These leaf certificates can be used for client/server authentication, because they include +/// [`ID_KP_CLIENT_AUTH`] and [`ID_KP_SERVER_AUTH`] in the extended key usage extension. +/// +/// This builder has many default values, notably; +/// +/// - A default validity of [`DEFAULT_CERTIFICATE_VALIDITY`] +/// - A randomly generated serial number +/// - In case no `key_pair` was provided, a fresh keypair will be created. The algorithm +/// (`rsa`/`ecdsa`) is chosen by the generic [`CertificateKeypair`] type of this struct, +/// which is normally inferred from the [`CertificateAuthority`]. +/// +/// Example code to construct a CA and a signed certificate: /// /// ```no_run /// use stackable_certs::{ @@ -92,7 +106,7 @@ where KP: CertificateKeypair, ::VerifyingKey: EncodePublicKey, { - /// Required subject of the certificate, usually starts with `CN=`. + /// Required subject of the certificate, usually starts with `CN=`, e.g. `CN=mypod`. subject: &'a str, /// Optional list of subject alternative name DNS entries @@ -151,6 +165,9 @@ where Some(key_pair) => key_pair, None => SKP::new().context(CreateKeyPairSnafu)?, }; + + // By choosing a random serial number we can make the reasonable assumption that we generate + // a unique serial for each certificate. let serial_number = SerialNumber::from(rand::random::()); let ca_validity = self.signed_by.ca_cert().tbs_certificate.validity; From fd19a03359f7ecfb35e88d5e9d4580c27abfde5b Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Tue, 27 May 2025 13:39:45 +0200 Subject: [PATCH 16/19] Add builder_with_rsa and builder_with_ecdsa helpers --- crates/stackable-certs/src/ca/ca_builder.rs | 17 ++++++++++++-- crates/stackable-certs/src/ca/mod.rs | 26 ++++++++++++++++++--- crates/stackable-certs/src/cert_builder.rs | 6 ++--- crates/stackable-certs/src/lib.rs | 3 +++ crates/stackable-webhook/src/tls.rs | 2 +- 5 files changed, 45 insertions(+), 9 deletions(-) diff --git a/crates/stackable-certs/src/ca/ca_builder.rs b/crates/stackable-certs/src/ca/ca_builder.rs index 832ee546c..5bdb3412a 100644 --- a/crates/stackable-certs/src/ca/ca_builder.rs +++ b/crates/stackable-certs/src/ca/ca_builder.rs @@ -81,6 +81,19 @@ where /// .build() /// .expect("failed to build CA"); /// ``` +/// +/// Instead of using generics to determine the algorithm to use you can also use [`CertificateAuthority::builder_with_rsa`] +/// or [`CertificateAuthority::builder_with_ecdsa`] instead: +/// +/// ```no_run +/// use stackable_certs::{ +/// keys::ecdsa, ca::CertificateAuthority, +/// }; +/// +/// let ca = CertificateAuthority::builder_with_ecdsa() +/// .build() +/// .expect("failed to build CA"); +/// ``` #[derive(Builder)] #[builder(start_fn = start_builder, finish_fn = finish_builder)] pub struct CertificateAuthorityBuilder<'a, SKP> @@ -211,11 +224,11 @@ mod tests { use x509_cert::certificate::TbsCertificateInner; use super::*; - use crate::keys::{ecdsa, rsa}; + use crate::keys::rsa; #[test] fn minimal_ca() { - let ca: CertificateAuthority = CertificateAuthority::builder() + let ca = CertificateAuthority::builder_with_ecdsa() .build() .expect("failed to build CA"); diff --git a/crates/stackable-certs/src/ca/mod.rs b/crates/stackable-certs/src/ca/mod.rs index cbed74b7c..18987475b 100644 --- a/crates/stackable-certs/src/ca/mod.rs +++ b/crates/stackable-certs/src/ca/mod.rs @@ -4,7 +4,10 @@ use std::fmt::Debug; use x509_cert::{Certificate, name::RdnSequence, spki::EncodePublicKey}; -use crate::{CertificatePair, keys::CertificateKeypair}; +use crate::{ + CertificatePair, + keys::{CertificateKeypair, ecdsa, rsa}, +}; mod ca_builder; mod consts; @@ -13,8 +16,10 @@ pub use ca_builder::*; pub use consts::*; pub use k8s::*; -/// A certificate authority (CA) which is used to generate and sign -/// intermediate or leaf certificates. +/// A certificate authority (CA) which is used to generate and sign intermediate or leaf +/// certificates. +/// +/// Use [`CertificateAuthorityBuilder`] to create new certificates. #[derive(Debug)] pub struct CertificateAuthority where @@ -33,6 +38,7 @@ where Self { certificate_pair } } + /// Use this function in combination with [`CertificateAuthorityBuilder`] to create new CAs. pub fn builder() -> CertificateAuthorityBuilderBuilder<'static, SK> { CertificateAuthorityBuilder::start_builder() } @@ -49,3 +55,17 @@ where &self.ca_cert().tbs_certificate.issuer } } + +impl CertificateAuthority { + /// Same as [`Self::builder`], but enforces the RSA algorithm for key creation. + pub fn builder_with_rsa() -> CertificateAuthorityBuilderBuilder<'static, rsa::SigningKey> { + Self::builder() + } +} + +impl CertificateAuthority { + /// Same as [`Self::builder`], but enforces the ecdsa algorithm for key creation. + pub fn builder_with_ecdsa() -> CertificateAuthorityBuilderBuilder<'static, ecdsa::SigningKey> { + Self::builder() + } +} diff --git a/crates/stackable-certs/src/cert_builder.rs b/crates/stackable-certs/src/cert_builder.rs index 06141eca2..c65c8806a 100644 --- a/crates/stackable-certs/src/cert_builder.rs +++ b/crates/stackable-certs/src/cert_builder.rs @@ -267,11 +267,11 @@ mod tests { }; use super::*; - use crate::keys::{ecdsa, rsa}; + use crate::keys::rsa; #[test] fn minimal_certificate() { - let ca = CertificateAuthority::::builder() + let ca = CertificateAuthority::builder_with_ecdsa() .build() .expect("failed to build CA"); @@ -292,7 +292,7 @@ mod tests { #[test] fn customized_certificate() { - let ca = CertificateAuthority::builder() + let ca = CertificateAuthority::builder_with_rsa() .build() .expect("failed to build CA"); diff --git a/crates/stackable-certs/src/lib.rs b/crates/stackable-certs/src/lib.rs index ae03c0f65..debad4940 100644 --- a/crates/stackable-certs/src/lib.rs +++ b/crates/stackable-certs/src/lib.rs @@ -99,6 +99,8 @@ impl PartialEq for CertificatePairError where @@ -121,6 +123,7 @@ where } } + /// Use this function in combination with [`CertificateBuilder`] to create new CAs. pub fn builder() -> CertificateBuilderBuilder<'static, S> { CertificateBuilder::start_builder() } diff --git a/crates/stackable-webhook/src/tls.rs b/crates/stackable-webhook/src/tls.rs index 37131aad8..e90fbbab8 100644 --- a/crates/stackable-webhook/src/tls.rs +++ b/crates/stackable-webhook/src/tls.rs @@ -109,7 +109,7 @@ impl TlsServer { // blocked. // See https://docs.rs/tokio/latest/tokio/task/fn.spawn_blocking.html let task = tokio::task::spawn_blocking(move || { - let ca = CertificateAuthority::::builder() + let ca = CertificateAuthority::builder_with_ecdsa() .build() .context(CreateCertificateAuthoritySnafu)?; From 3d3e0c1516f653f01fdf7678986f00d8b2894a27 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Tue, 27 May 2025 14:01:13 +0200 Subject: [PATCH 17/19] Make cert outlives ca a hard error --- crates/stackable-certs/src/ca/ca_builder.rs | 2 +- crates/stackable-certs/src/ca/consts.rs | 2 +- crates/stackable-certs/src/cert_builder.rs | 63 +++++++++++++++------ 3 files changed, 49 insertions(+), 18 deletions(-) diff --git a/crates/stackable-certs/src/ca/ca_builder.rs b/crates/stackable-certs/src/ca/ca_builder.rs index 5bdb3412a..babcb96fe 100644 --- a/crates/stackable-certs/src/ca/ca_builder.rs +++ b/crates/stackable-certs/src/ca/ca_builder.rs @@ -65,7 +65,7 @@ where /// - A default subject of [`SDP_ROOT_CA_SUBJECT`] /// - A randomly generated serial number /// - In case no `signing_key_pair` was provided, a fresh keypair will be created. The algorithm -/// (`rsa`/`ecdsa`) is chosen by the generic [`CertificateKeypair`] type of this struct. +/// (`rsa`/`ecdsa`) is chosen by the generic [`CertificateKeypair`] type of this struct. /// /// The CA contains the public half of the provided `signing_key_pair` and is signed by the private /// half of said key. diff --git a/crates/stackable-certs/src/ca/consts.rs b/crates/stackable-certs/src/ca/consts.rs index 966ef5513..329e7301c 100644 --- a/crates/stackable-certs/src/ca/consts.rs +++ b/crates/stackable-certs/src/ca/consts.rs @@ -2,7 +2,7 @@ use rsa::pkcs8::LineEnding; use stackable_operator::time::Duration; /// The default CA validity time span of one hour. -pub const DEFAULT_CA_VALIDITY: Duration = Duration::from_hours_unchecked(1); +pub const DEFAULT_CA_VALIDITY: Duration = Duration::from_days_unchecked(1); /// The default certificate validity time span of one hour. pub const DEFAULT_CERTIFICATE_VALIDITY: Duration = Duration::from_hours_unchecked(1); diff --git a/crates/stackable-certs/src/cert_builder.rs b/crates/stackable-certs/src/cert_builder.rs index c65c8806a..513f712a7 100644 --- a/crates/stackable-certs/src/cert_builder.rs +++ b/crates/stackable-certs/src/cert_builder.rs @@ -1,9 +1,9 @@ -use std::{fmt::Debug, net::IpAddr}; +use std::{fmt::Debug, net::IpAddr, time::SystemTime}; use bon::Builder; use const_oid::db::rfc5280::{ID_KP_CLIENT_AUTH, ID_KP_SERVER_AUTH}; use rsa::pkcs8::EncodePublicKey; -use snafu::{ResultExt, Snafu}; +use snafu::{ResultExt, Snafu, ensure}; use stackable_operator::time::Duration; use tracing::{debug, instrument, warn}; use x509_cert::{ @@ -62,6 +62,19 @@ where #[snafu(display("failed to build certificate"))] BuildCertificate { source: x509_cert::builder::Error }, + + #[snafu(display( + "the generated certificate would outlive the CA, subject {subject:?}, \ + CA notAfter {ca_not_after:?}, CA notBefore {ca_not_before:?}, \ + cert notAfter {cert_not_after:?}, cert notBefore {cert_not_before:?}" + ))] + CertOutlivesCa { + subject: String, + ca_not_after: SystemTime, + ca_not_before: SystemTime, + cert_not_after: SystemTime, + cert_not_before: SystemTime, + }, } /// This builder builds certificates of type [`CertificatePair`]. @@ -77,8 +90,8 @@ where /// - A default validity of [`DEFAULT_CERTIFICATE_VALIDITY`] /// - A randomly generated serial number /// - In case no `key_pair` was provided, a fresh keypair will be created. The algorithm -/// (`rsa`/`ecdsa`) is chosen by the generic [`CertificateKeypair`] type of this struct, -/// which is normally inferred from the [`CertificateAuthority`]. +/// (`rsa`/`ecdsa`) is chosen by the generic [`CertificateKeypair`] type of this struct, +/// which is normally inferred from the [`CertificateAuthority`]. /// /// Example code to construct a CA and a signed certificate: /// @@ -158,6 +171,7 @@ where )] pub fn build(self) -> Result, CreateCertificateError> { let validity = Validity::from_now(*self.validity).context(ParseValiditySnafu)?; + let subject_for_error = &self.subject; let subject: Name = self.subject.parse().context(ParseSubjectSnafu { subject: self.subject, })?; @@ -172,17 +186,17 @@ where let ca_validity = self.signed_by.ca_cert().tbs_certificate.validity; let ca_not_after = ca_validity.not_after.to_system_time(); + let ca_not_before = ca_validity.not_before.to_system_time(); let cert_not_after = validity.not_after.to_system_time(); - if ca_not_after < cert_not_after { - warn!( - ca.validity = ?ca_validity, - cert.validity = ?validity, - ca.not_after = ?ca_not_after, - cert.not_after = ?cert_not_after, - subject = ?subject, - "The lifetime of certificate authority is shorted than the lifetime of the generated certificate", - ); - } + let cert_not_before = validity.not_before.to_system_time(); + + ensure!(ca_not_after > cert_not_after, CertOutlivesCaSnafu { + subject: subject_for_error.to_string(), + ca_not_after, + ca_not_before, + cert_not_after, + cert_not_before, + }); let spki_pem = key_pair .verifying_key() @@ -306,7 +320,7 @@ mod tests { .subject("CN=trino-coordinator-default-0") .subject_alternative_dns_names(&sans) .subject_alternative_ip_addresses(&san_ips) - .validity(Duration::from_days_unchecked(42)) + .validity(Duration::from_hours_unchecked(12)) .key_pair(rsa::SigningKey::new().unwrap()) .signed_by(&ca) .build() @@ -317,10 +331,27 @@ mod tests { "CN=trino-coordinator-default-0", &sans, &san_ips, - Duration::from_days_unchecked(42), + Duration::from_hours_unchecked(12), ); } + #[test] + fn cert_outlives_ca() { + let ca = CertificateAuthority::builder_with_ecdsa() + .validity(Duration::from_days_unchecked(365)) + .build() + .expect("failed to build CA"); + + let err = CertificatePair::builder() + .subject("CN=Test") + .signed_by(&ca) + .validity(Duration::from_days_unchecked(366)) + .build() + .err() + .expect("Certificate creation must error"); + assert!(matches!(err, CreateCertificateError::CertOutlivesCa { .. })); + } + fn assert_certificate_attributes( certificate: &TbsCertificateInner, subject: &str, From 5cf7c6454df94aa0e4f9eca9b668991fa5f49d2d Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Tue, 27 May 2025 14:13:07 +0200 Subject: [PATCH 18/19] clippy --- crates/stackable-certs/src/cert_builder.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/stackable-certs/src/cert_builder.rs b/crates/stackable-certs/src/cert_builder.rs index 513f712a7..90d161e6d 100644 --- a/crates/stackable-certs/src/cert_builder.rs +++ b/crates/stackable-certs/src/cert_builder.rs @@ -347,8 +347,7 @@ mod tests { .signed_by(&ca) .validity(Duration::from_days_unchecked(366)) .build() - .err() - .expect("Certificate creation must error"); + .expect_err("Certificate creation must error"); assert!(matches!(err, CreateCertificateError::CertOutlivesCa { .. })); } From c263ae74c7565977a55d5b15e2ed8b52f16bb113 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Tue, 27 May 2025 14:16:59 +0200 Subject: [PATCH 19/19] new fmt --- crates/stackable-certs/src/cert_builder.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/crates/stackable-certs/src/cert_builder.rs b/crates/stackable-certs/src/cert_builder.rs index 90d161e6d..94ffa155a 100644 --- a/crates/stackable-certs/src/cert_builder.rs +++ b/crates/stackable-certs/src/cert_builder.rs @@ -190,13 +190,16 @@ where let cert_not_after = validity.not_after.to_system_time(); let cert_not_before = validity.not_before.to_system_time(); - ensure!(ca_not_after > cert_not_after, CertOutlivesCaSnafu { - subject: subject_for_error.to_string(), - ca_not_after, - ca_not_before, - cert_not_after, - cert_not_before, - }); + ensure!( + ca_not_after > cert_not_after, + CertOutlivesCaSnafu { + subject: subject_for_error.to_string(), + ca_not_after, + ca_not_before, + cert_not_after, + cert_not_before, + } + ); let spki_pem = key_pair .verifying_key()