diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d07af1f..bfa484a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file. This projec If you are upgrading from v0.16.x, replace the binary (or run `docker pull`). If you are upgrading from v0.15.x and below, please read the [upgrading documentation](https://github.com/stalwartlabs/stalwart/blob/main/UPGRADING/v0_16.md) for more information on how to upgrade from previous versions. ## Added +- ACME: Allow specifying a preferred certificate chain. ## Changed diff --git a/crates/common/src/network/acme/directory.rs b/crates/common/src/network/acme/directory.rs index 9ca2b9b1..b485c131 100644 --- a/crates/common/src/network/acme/directory.rs +++ b/crates/common/src/network/acme/directory.rs @@ -9,7 +9,7 @@ use super::jose::{ key_authorization, key_authorization_sha256, key_authorization_sha256_base64, sign, }; -use crate::network::acme::http::{get_header, https, parse_retry_after}; +use crate::network::acme::http::{get_header, https, parse_alternate_links, parse_retry_after}; use crate::network::acme::{ AcmeError, AcmeResult, Auth, AuthStatus, Challenge, ChallengeType, Directory, Identifier, Order, SerializedCert, @@ -35,12 +35,14 @@ pub struct AcmeRequestBuilder { pub kid: String, pub challenge: ChallengeType, pub max_retries: u32, + pub preferred_chain: Option, } pub struct AcmeResponse { pub location: L, pub body: B, pub retry_after: Option, + pub alternates: Vec, } static ALG: &EcdsaSigningAlgorithm = &ECDSA_P256_SHA256_FIXED_SIGNING; @@ -65,6 +67,7 @@ impl AcmeRequestBuilder { kid: provider.account_uri, challenge: provider.challenge_type.into(), max_retries: provider.max_retries as u32, + preferred_chain: provider.preferred_chain, }) } @@ -85,6 +88,7 @@ impl AcmeRequestBuilder { Ok(AcmeResponse { location: get_header(&response, "Location").ok(), retry_after: parse_retry_after(&response), + alternates: parse_alternate_links(&response), body: response.text().await?, }) } @@ -103,6 +107,7 @@ impl AcmeRequestBuilder { )))?, body: serde_json::from_str(&response.body).map_err(AcmeError::Json)?, retry_after: response.retry_after, + alternates: response.alternates, }) } @@ -133,8 +138,11 @@ impl AcmeRequestBuilder { AcmeResponse::parse(self.request(&url, &payload).await?) } - pub async fn certificate(&self, url: impl AsRef) -> AcmeResult { - Ok(self.request(&url, "").await?.body) + pub async fn certificate( + &self, + url: impl AsRef, + ) -> AcmeResult, String>> { + self.request(&url, "").await } pub fn http_proof(&self, challenge: &Challenge) -> AcmeResult> { @@ -211,6 +219,7 @@ impl AcmeResponse { location: input.location, body, retry_after: input.retry_after, + alternates: input.alternates, }) } } diff --git a/crates/common/src/network/acme/http.rs b/crates/common/src/network/acme/http.rs index 518692f9..3fc01f48 100644 --- a/crates/common/src/network/acme/http.rs +++ b/crates/common/src/network/acme/http.rs @@ -76,6 +76,40 @@ pub(crate) fn get_header(response: &Response, header: &'static str) -> AcmeResul } } +pub(crate) fn parse_alternate_links(response: &Response) -> Vec { + alternate_links( + response + .headers() + .get_all("Link") + .iter() + .filter_map(|value| value.to_str().ok()), + ) +} + +fn alternate_links<'a>(values: impl Iterator) -> Vec { + let mut urls = Vec::new(); + for value in values { + for link in value.split(',') { + let mut url = None; + let mut is_alternate = false; + for (index, part) in link.split(';').enumerate() { + let part = part.trim(); + if index == 0 { + url = part + .strip_prefix('<') + .and_then(|part| part.strip_suffix('>')); + } else if let Some(rel) = part.strip_prefix("rel=") { + is_alternate = rel.trim_matches('"') == "alternate"; + } + } + if is_alternate && let Some(url) = url { + urls.push(url.to_string()); + } + } + } + urls +} + pub(crate) fn parse_retry_after(response: &Response) -> Option { let value = response.headers().get("Retry-After")?.to_str().ok()?; if let Ok(secs) = value.parse::() { @@ -90,3 +124,70 @@ pub(crate) fn parse_retry_after(response: &Response) -> Option { None } } + +#[cfg(test)] +mod tests { + use super::alternate_links; + + #[test] + fn parses_single_alternate_link() { + let links = + alternate_links([r#";rel="alternate""#].into_iter()); + assert_eq!(links, vec!["https://acme.example/cert/1/1".to_string()]); + } + + #[test] + fn parses_multiple_alternates_in_one_header() { + let links = alternate_links( + [r#";rel="alternate", ;rel="alternate""#] + .into_iter(), + ); + assert_eq!( + links, + vec![ + "https://acme.example/cert/1/1".to_string(), + "https://acme.example/cert/1/2".to_string(), + ] + ); + } + + #[test] + fn parses_alternates_across_multiple_headers() { + let links = alternate_links( + [ + r#";rel="alternate""#, + r#";rel="alternate""#, + ] + .into_iter(), + ); + assert_eq!( + links, + vec![ + "https://acme.example/cert/1/1".to_string(), + "https://acme.example/cert/1/2".to_string(), + ] + ); + } + + #[test] + fn ignores_non_alternate_relations() { + let links = alternate_links( + [r#";rel="index", ;rel="alternate""#] + .into_iter(), + ); + assert_eq!(links, vec!["https://acme.example/cert/1/1".to_string()]); + } + + #[test] + fn tolerates_unquoted_rel_and_extra_whitespace() { + let links = + alternate_links([r#" ; rel=alternate "#].into_iter()); + assert_eq!(links, vec!["https://acme.example/cert/1/1".to_string()]); + } + + #[test] + fn returns_empty_when_no_alternates() { + let links = alternate_links([r#";rel="index""#].into_iter()); + assert!(links.is_empty()); + } +} diff --git a/crates/common/src/network/acme/order.rs b/crates/common/src/network/acme/order.rs index c9a75a19..cc45a653 100644 --- a/crates/common/src/network/acme/order.rs +++ b/crates/common/src/network/acme/order.rs @@ -178,7 +178,7 @@ impl AcmeRequestBuilder { Hostname = domains.as_slice(), ); - let certificate = self.certificate(certificate).await?; + let certificate = self.select_certificate(&domains, certificate).await?; return Ok(PemCert { certificate, @@ -341,6 +341,60 @@ impl AcmeRequestBuilder { max_retries: self.max_retries, }) } + + async fn select_certificate(&self, domains: &[String], url: String) -> AcmeResult { + let response = self.certificate(url).await?; + let Some(preferred) = self.preferred_chain.as_deref() else { + return Ok(response.body); + }; + + if chain_matches(&response.body, preferred) { + return Ok(response.body); + } + + for alternate in &response.alternates { + match self.certificate(alternate).await { + Ok(alternate) if chain_matches(&alternate.body, preferred) => { + return Ok(alternate.body); + } + Ok(_) => {} + Err(err) => { + trc::event!( + Acme(AcmeEvent::ProcessCert), + Url = alternate.to_string(), + Hostname = domains, + Reason = err.to_string(), + ); + } + } + } + + trc::event!( + Acme(AcmeEvent::ProcessCert), + Hostname = domains, + Reason = format!( + "Preferred certificate chain '{preferred}' not offered by the CA; using the default chain", + ), + ); + + Ok(response.body) + } +} + +fn chain_matches(pem_chain: &str, preferred: &str) -> bool { + let Ok(blocks) = pem::parse_many(pem_chain) else { + return false; + }; + let Some(top) = blocks.last() else { + return false; + }; + let Ok((_, cert)) = parse_x509_certificate(top.contents()) else { + return false; + }; + cert.issuer() + .iter_common_name() + .filter_map(|cn| cn.as_str().ok()) + .any(|cn| cn == preferred) } impl ParsedCert { @@ -408,3 +462,56 @@ impl ParsedCert { }) } } + +#[cfg(test)] +mod tests { + use super::chain_matches; + use rcgen::{CertificateParams, DistinguishedName, DnType, KeyPair, PKCS_ECDSA_P256_SHA256}; + + fn self_signed_pem(common_name: &str) -> String { + let mut params = CertificateParams::new(vec!["host.example".to_string()]).unwrap(); + let mut dn = DistinguishedName::new(); + dn.push(DnType::CommonName, common_name); + params.distinguished_name = dn; + let key_pair = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).unwrap(); + params.self_signed(&key_pair).unwrap().pem() + } + + #[test] + fn matches_top_certificate_issuer() { + let chain = self_signed_pem("ISRG Root X1"); + assert!(chain_matches(&chain, "ISRG Root X1")); + } + + #[test] + fn match_is_case_sensitive() { + let chain = self_signed_pem("ISRG Root X1"); + assert!(!chain_matches(&chain, "isrg root x1")); + } + + #[test] + fn match_is_exact_not_substring() { + let chain = self_signed_pem("ISRG Root X10"); + assert!(!chain_matches(&chain, "ISRG Root X1")); + } + + #[test] + fn does_not_match_unrelated_issuer() { + let chain = self_signed_pem("ISRG Root X2"); + assert!(!chain_matches(&chain, "ISRG Root X1")); + } + + #[test] + fn uses_topmost_certificate_not_leaf() { + let leaf = self_signed_pem("Leaf Issuer"); + let top = self_signed_pem("ISRG Root X1"); + let chain = format!("{leaf}{top}"); + assert!(chain_matches(&chain, "ISRG Root X1")); + assert!(!chain_matches(&chain, "Leaf Issuer")); + } + + #[test] + fn rejects_unparseable_chain() { + assert!(!chain_matches("not a pem", "ISRG Root X1")); + } +} diff --git a/crates/http/src/request.rs b/crates/http/src/request.rs index db0978b7..44612782 100644 --- a/crates/http/src/request.rs +++ b/crates/http/src/request.rs @@ -35,7 +35,6 @@ use hyper::{ service::service_fn, }; use hyper_util::rt::TokioIo; -use percent_encoding::percent_decode_str; use jmap::{ api::{ ToJmapHttpResponse, event_source::EventSourceHandler, request::RequestHandler, @@ -45,6 +44,7 @@ use jmap::{ websocket::upgrade::WebSocketUpgrade, }; use jmap_proto::request::{Request, capability::Session}; +use percent_encoding::percent_decode_str; use registry::schema::enums::Permission; use std::{net::IpAddr, str::FromStr, sync::Arc}; use store::dispatch::lookup::KeyValue; diff --git a/crates/registry/src/schema/properties.rs b/crates/registry/src/schema/properties.rs index 19abd781..96927a98 100644 --- a/crates/registry/src/schema/properties.rs +++ b/crates/registry/src/schema/properties.rs @@ -893,6 +893,7 @@ pub enum Property { PoolTimeoutWait = 481, PoolWorkers = 657, Port = 299, + PreferredChain = 910, Prefix = 856, PreserveIntermediates = 306, Priority = 483, diff --git a/crates/registry/src/schema/properties_impl.rs b/crates/registry/src/schema/properties_impl.rs index fbde05b3..0cf0c6f1 100644 --- a/crates/registry/src/schema/properties_impl.rs +++ b/crates/registry/src/schema/properties_impl.rs @@ -1046,6 +1046,7 @@ impl EnumImpl for Property { b"poolTimeoutWait" => Property::PoolTimeoutWait, b"poolWorkers" => Property::PoolWorkers, b"port" => Property::Port, + b"preferredChain" => Property::PreferredChain, b"prefix" => Property::Prefix, b"preserveIntermediates" => Property::PreserveIntermediates, b"priority" => Property::Priority, @@ -1961,6 +1962,7 @@ impl EnumImpl for Property { Property::PoolTimeoutWait => "poolTimeoutWait", Property::PoolWorkers => "poolWorkers", Property::Port => "port", + Property::PreferredChain => "preferredChain", Property::Prefix => "prefix", Property::PreserveIntermediates => "preserveIntermediates", Property::Priority => "priority", @@ -2880,6 +2882,7 @@ impl EnumImpl for Property { 481 => Some(Property::PoolTimeoutWait), 657 => Some(Property::PoolWorkers), 299 => Some(Property::Port), + 910 => Some(Property::PreferredChain), 856 => Some(Property::Prefix), 306 => Some(Property::PreserveIntermediates), 483 => Some(Property::Priority), @@ -3155,7 +3158,7 @@ impl EnumImpl for Property { } } - const COUNT: usize = 910; + const COUNT: usize = 911; } impl serde::Serialize for Property { diff --git a/crates/registry/src/schema/structs.rs b/crates/registry/src/schema/structs.rs index 92949fe2..94720d0c 100644 --- a/crates/registry/src/schema/structs.rs +++ b/crates/registry/src/schema/structs.rs @@ -57,6 +57,8 @@ pub struct AcmeProvider { pub max_retries: i64, #[serde(rename = "memberTenantId")] pub member_tenant_id: Option, + #[serde(rename = "preferredChain")] + pub preferred_chain: Option, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] diff --git a/crates/registry/src/schema/structs_impl.rs b/crates/registry/src/schema/structs_impl.rs index 27db8905..c22ad6cc 100644 --- a/crates/registry/src/schema/structs_impl.rs +++ b/crates/registry/src/schema/structs_impl.rs @@ -289,7 +289,7 @@ impl RegistryJsonPropertyPatch for AccountSettings { impl ObjectImpl for AcmeProvider { const FLAGS: u64 = OBJ_FILTER_TENANT; - const VERSION: u8 = 0; + const VERSION: u8 = 1; const OBJECT: ObjectType = ObjectType::AcmeProvider; fn validate(&self, errors: &mut Vec) -> bool { @@ -320,6 +320,11 @@ impl ObjectImpl for AcmeProvider { errors.push(ValidationError::required(Property::MemberTenantId)); } } + if let Some(value) = &self.preferred_chain { + if value.is_empty() { + errors.push(ValidationError::required(Property::PreferredChain)); + } + } errors.len() == neb } @@ -345,6 +350,7 @@ impl Pickle for AcmeProvider { self.renew_before.pickle(out); self.max_retries.pickle(out); self.member_tenant_id.pickle(out); + self.preferred_chain.pickle(out); } fn unpickle(stream: &mut crate::pickle::PickledStream<'_>) -> Option { @@ -357,6 +363,9 @@ impl Pickle for AcmeProvider { this.renew_before = Pickle::unpickle(stream)?; this.max_retries = Pickle::unpickle(stream)?; this.member_tenant_id = Pickle::unpickle(stream)?; + if stream.version() >= 1 { + this.preferred_chain = Pickle::unpickle(stream)?; + } Some(this) } } @@ -372,13 +381,14 @@ impl Default for AcmeProvider { renew_before: AcmeRenewBefore::R23, max_retries: 10i64, member_tenant_id: Default::default(), + preferred_chain: Default::default(), } } } impl IntoValue for AcmeProvider { fn into_value(self) -> JmapValue<'static> { - let mut map = jmap_tools::Map::with_capacity(10); + let mut map = jmap_tools::Map::with_capacity(11); map.insert_unchecked(Property::ChallengeType, self.challenge_type.into_value()); map.insert_unchecked(Property::Contact, self.contact.into_value()); map.insert_unchecked(Property::Directory, self.directory.into_value()); @@ -387,6 +397,7 @@ impl IntoValue for AcmeProvider { map.insert_unchecked(Property::RenewBefore, self.renew_before.into_value()); map.insert_unchecked(Property::MaxRetries, self.max_retries.into_value()); map.insert_unchecked(Property::MemberTenantId, self.member_tenant_id.into_value()); + map.insert_unchecked(Property::PreferredChain, self.preferred_chain.into_value()); JmapValue::Object(map) } } @@ -421,6 +432,9 @@ impl RegistryJsonPropertyPatch for AcmeProvider { Some(Property::MemberTenantId) => self .member_tenant_id .patch(pointer.assert_can_set_tenant()?, value), + Some(Property::PreferredChain) => self + .preferred_chain + .patch(pointer.with_validators(&[StringValidator::Trim]), value), Some(Property::Type) => Ok(MaybeUnpatched::Unpatched { property: Property::Type, value, diff --git a/resources/schema/schema.json.gz b/resources/schema/schema.json.gz index 3a4b791e..ffa38c3d 100644 Binary files a/resources/schema/schema.json.gz and b/resources/schema/schema.json.gz differ diff --git a/resources/schema/schema.json.sha256 b/resources/schema/schema.json.sha256 index bc65d190..a33c822f 100644 --- a/resources/schema/schema.json.sha256 +++ b/resources/schema/schema.json.sha256 @@ -1 +1 @@ -nGrZtt6AUdU1bcCirQQzKiHbnluseipQ1zi4wqAu0AY \ No newline at end of file +-bCNRlw73NjcmJBDrWwU7hhuRX8FL4IZLWsQ_-iZh0o \ No newline at end of file diff --git a/tests/src/automation/acme.rs b/tests/src/automation/acme.rs index 562b68b3..f812bbcc 100644 --- a/tests/src/automation/acme.rs +++ b/tests/src/automation/acme.rs @@ -26,6 +26,7 @@ use registry::{ use serde_json::json; use store::{registry::write::RegistryWrite, write::now}; use x509_parser::parse_x509_certificate; +use x509_parser::pem::Pem; pub async fn test(test: &TestServer) { println!("Running ACME tests..."); @@ -415,6 +416,127 @@ pub async fn test(test: &TestServer) { vec!["*.persist.org".to_string(), "persist.org".to_string()] ); + // Test preferred chain selection against the alternate chains Pebble offers (RFC 8555 7.4.2) + let pebble_roots = pebble_root_common_names().await; + assert!( + pebble_roots.len() >= 2, + "Expected Pebble to offer multiple root chains, found: {:?}", + pebble_roots + ); + + // Renew without a preferred chain to discover the default root + let default_acme_id = account + .registry_create_object(AcmeProvider { + directory: "https://localhost:14000/dir".to_string(), + contact: Map::new(vec!["mailto:hello@chain.org".to_string()]), + challenge_type: AcmeChallengeType::TlsAlpn01, + ..Default::default() + }) + .await; + let default_domain_id = account + .registry_create_object(Domain { + name: "chain.org".to_string(), + certificate_management: CertificateManagement::Automatic( + CertificateManagementProperties { + acme_provider_id: default_acme_id, + subject_alternative_names: Default::default(), + }, + ), + dkim_management: DkimManagement::Manual, + dns_management: DnsManagement::Automatic(DnsManagementProperties { + dns_server_id: in_memory_dns_id, + ..Default::default() + }), + ..Default::default() + }) + .await; + test.wait_for_tasks_skip_not_due().await; + let default_chain = account + .registry_get_all::() + .await + .into_iter() + .next() + .unwrap() + .1 + .certificate + .value() + .await + .unwrap() + .into_owned(); + let default_root = top_issuer_common_name(&default_chain) + .expect("default chain should expose a top issuer common name"); + assert!( + pebble_roots.contains(&default_root), + "Default root {:?} not among Pebble roots {:?}", + default_root, + pebble_roots + ); + account.registry_destroy_all(ObjectType::Certificate).await; + account.registry_destroy_all(ObjectType::Task).await; + + // Renew with a preferred chain pointing at an alternate root and verify it is honored + let preferred_root = pebble_roots + .iter() + .find(|cn| **cn != default_root) + .cloned() + .expect("an alternate root distinct from the default"); + let preferred_acme_id = account + .registry_create_object(AcmeProvider { + directory: "https://localhost:14000/dir".to_string(), + contact: Map::new(vec!["mailto:hello@chainalt.org".to_string()]), + challenge_type: AcmeChallengeType::TlsAlpn01, + preferred_chain: Some(preferred_root.clone()), + ..Default::default() + }) + .await; + let preferred_domain_id = account + .registry_create_object(Domain { + name: "chainalt.org".to_string(), + certificate_management: CertificateManagement::Automatic( + CertificateManagementProperties { + acme_provider_id: preferred_acme_id, + subject_alternative_names: Default::default(), + }, + ), + dkim_management: DkimManagement::Manual, + dns_management: DnsManagement::Automatic(DnsManagementProperties { + dns_server_id: in_memory_dns_id, + ..Default::default() + }), + ..Default::default() + }) + .await; + test.wait_for_tasks_skip_not_due().await; + let preferred_chain = account + .registry_get_all::() + .await + .into_iter() + .next() + .unwrap() + .1 + .certificate + .value() + .await + .unwrap() + .into_owned(); + let selected_root = top_issuer_common_name(&preferred_chain) + .expect("preferred chain should expose a top issuer common name"); + assert_eq!( + selected_root, preferred_root, + "ACME did not select the preferred certificate chain" + ); + assert_ne!( + selected_root, default_root, + "Preferred chain matches the default; selection was not exercised" + ); + account + .registry_destroy(ObjectType::Domain, [default_domain_id, preferred_domain_id]) + .await + .assert_destroyed(&[default_domain_id, preferred_domain_id]); + account.registry_destroy_all(ObjectType::Certificate).await; + account.registry_destroy_all(ObjectType::Task).await; + account.registry_destroy_all(ObjectType::AcmeProvider).await; + // Cleanup account .registry_update_object( @@ -593,3 +715,49 @@ wRLU49cXsnLbCKTbfMxMa9HB1PuJivwuMf4IBWYsQQKBgQC5KNAWEHWrnxiNeCS0 9MBumBf1lgiJZSsloOKWQvLchg== -----END PRIVATE KEY----- "#; + +async fn pebble_root_common_names() -> Vec { + let client = reqwest::Client::builder() + .danger_accept_invalid_certs(true) + .build() + .expect("Failed to build HTTP client"); + let mut common_names = Vec::new(); + for index in 0.. { + let response = client + .get(format!("https://localhost:15000/roots/{index}")) + .send() + .await + .expect("Failed to query Pebble management API"); + if !response.status().is_success() { + break; + } + let pem = response.text().await.expect("Failed to read Pebble root"); + match subject_common_name(&pem) { + Some(common_name) => common_names.push(common_name), + None => break, + } + } + common_names +} + +fn subject_common_name(pem: &str) -> Option { + let block = Pem::iter_from_buffer(pem.as_bytes()).next()?.ok()?; + let cert = block.parse_x509().ok()?; + cert.subject() + .iter_common_name() + .filter_map(|cn| cn.as_str().ok()) + .next() + .map(str::to_string) +} + +fn top_issuer_common_name(chain: &str) -> Option { + let block = Pem::iter_from_buffer(chain.as_bytes()) + .filter_map(Result::ok) + .last()?; + let cert = block.parse_x509().ok()?; + cert.issuer() + .iter_common_name() + .filter_map(|cn| cn.as_str().ok()) + .next() + .map(str::to_string) +} diff --git a/tests/src/utils/containers.rs b/tests/src/utils/containers.rs index b517a122..85e42cd6 100644 --- a/tests/src/utils/containers.rs +++ b/tests/src/utils/containers.rs @@ -303,6 +303,7 @@ async fn ensure_pebble() { GenericImage::new("ghcr.io/letsencrypt/pebble", "latest") .with_env_var("PEBBLE_VA_NOSLEEP", "1") .with_env_var("PEBBLE_WFE_NONCEREJECT", "0") + .with_env_var("PEBBLE_ALTERNATE_ROOTS", "2") .with_cmd([ "-config", "/test/config/pebble-config.json",