diff --git a/Cargo.lock b/Cargo.lock index 36410f2f69ca5..c25c8119faffd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3159,10 +3159,23 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e7c1832837b905bbfb5101e07cc24c8deddf52f93225eee6ead5f4d63d53ddcb" dependencies = [ "const-oid 0.9.6", + "der_derive", + "flagset", "pem-rfc7468", "zeroize", ] +[[package]] +name = "der_derive" +version = "0.7.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8034092389675178f570469e6c3b0465d3d30b4505c294a6550db47f3c17ad18" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.119", +] + [[package]] name = "deranged" version = "0.5.8" @@ -3884,6 +3897,12 @@ version = "0.5.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d674e81391d1e1ab681a28d99df07927c6d4aa5b027d7da16ba32d1d21ecd99" +[[package]] +name = "flagset" +version = "0.4.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b7ac824320a75a52197e8f2d787f6a38b6718bb6897a35142d749af3c0e8f4fe" + [[package]] name = "flatbuffers" version = "25.2.10" @@ -6602,12 +6621,15 @@ dependencies = [ "prost-build", "reqwest 0.13.5", "rustls", + "rustls-platform-verifier", "serde", "serde_json", "thiserror 2.0.21", "tokio", + "tokio-openssl", "tracing", "url", + "x509-cert", "zeroize", ] @@ -15125,6 +15147,17 @@ dependencies = [ "tap", ] +[[package]] +name = "x509-cert" +version = "0.2.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1301e935010a701ae5f8655edc0ad17c44bad3ac5ce8c39185f75453b720ae94" +dependencies = [ + "const-oid 0.9.6", + "der", + "spki", +] + [[package]] name = "xattr" version = "1.4.0" diff --git a/Cargo.toml b/Cargo.toml index 89cf72f28d166..ebfee8420558c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -478,6 +478,7 @@ rlimit = "0.11.0" rocksdb = { version = "0.25.0", default-features = false, features = ["lz4", "snappy", "zstd"] } ropey = "1.6.1" rustls = "0.23.38" +rustls-platform-verifier = "0.7.0" rpassword = "7.5.1" rusqlite = { version = "0.40.2", features = ["bundled"] } ryu = "1.0.23" @@ -566,6 +567,7 @@ uuid = "1.19.0" version-compare = "0.2.1" walkdir = "2.5.0" which = "8" +x509-cert = { version = "0.2.5", default-features = false } yansi = "1.0.1" zeroize = { version = "1.8.2", features = ["derive", "serde"] } zip = { version = "8.6.0", default-features = false, features = ["deflate-flate2"] } diff --git a/src/ccsr/Cargo.toml b/src/ccsr/Cargo.toml index b54ea464d184a..cc377f2f4a272 100644 --- a/src/ccsr/Cargo.toml +++ b/src/ccsr/Cargo.toml @@ -16,15 +16,18 @@ proptest.workspace = true zeroize.workspace = true proptest-derive.workspace = true rustls.workspace = true +rustls-platform-verifier.workspace = true serde.workspace = true thiserror.workspace = true url = { workspace = true, features = ["serde"] } +x509-cert.workspace = true [dev-dependencies] hyper.workspace = true hyper-util.workspace = true mz-ore = { path = "../ore", features = ["async", "test"] } openssl.workspace = true +tokio-openssl.workspace = true serde_json.workspace = true tokio.workspace = true tracing.workspace = true diff --git a/src/ccsr/src/config.rs b/src/ccsr/src/config.rs index d9d1a1c5ce239..23c4255da45ba 100644 --- a/src/ccsr/src/config.rs +++ b/src/ccsr/src/config.rs @@ -63,6 +63,10 @@ impl ClientConfig { /// Adds a trusted root TLS certificate. /// /// Certificates in the system's certificate store are trusted by default. + /// A server certificate identical to `cert` is trusted even if it is not + /// a valid end-entity certificate, for example a self-signed CA. Its name + /// and validity period are still checked, and its name may match any + /// subject common name if it has no DNS or IP subjectAltName. pub fn add_root_certificate(mut self, cert: Certificate) -> ClientConfig { self.root_certs.push(cert); self @@ -106,11 +110,13 @@ impl ClientConfig { pub fn build(self) -> Result { let mut builder = reqwest::ClientBuilder::new(); - for root_cert in self.root_certs { - builder = builder.add_root_certificate(root_cert.into()); - } - - if let Some(ident) = self.identity { + // NOTE: A preconfigured TLS backend makes reqwest ignore all of its TLS + // builder settings (roots, identity, ALPN, SNI, TLS versions, CRLs), so + // any such setting must go into `rustls_config` instead. + if !self.root_certs.is_empty() { + let tls = crate::tls::rustls_config(&self.root_certs, self.identity.as_ref())?; + builder = builder.tls_backend_preconfigured(tls); + } else if let Some(ident) = self.identity { builder = builder.identity(ident.into()); } diff --git a/src/ccsr/src/tls.rs b/src/ccsr/src/tls.rs index cbcc9cf55329a..fc04ceaca7b80 100644 --- a/src/ccsr/src/tls.rs +++ b/src/ccsr/src/tls.rs @@ -12,9 +12,18 @@ use std::fmt; use std::sync::Arc; +use rustls::client::danger::{HandshakeSignatureValid, ServerCertVerified, ServerCertVerifier}; +use rustls::client::verify_server_name; use rustls::pki_types::pem::PemObject; -use rustls::pki_types::{CertificateDer, PrivateKeyDer}; +use rustls::pki_types::{CertificateDer, PrivateKeyDer, ServerName, UnixTime}; +use rustls::server::ParsedCertificate; +use rustls::{CertificateError, DigitallySignedStruct, SignatureScheme}; use serde::{Deserialize, Serialize}; +use x509_cert::der::asn1::{Ia5StringRef, PrintableStringRef, Utf8StringRef}; +use x509_cert::der::oid::db::rfc4519; +use x509_cert::der::{Decode, Tag, Tagged}; +use x509_cert::ext::pkix::SubjectAltName; +use x509_cert::ext::pkix::name::GeneralName; use zeroize::{Zeroize, Zeroizing}; /// An error constructing a [`Certificate`] or [`Identity`]. @@ -30,6 +39,8 @@ pub enum TlsError { Certificate(rustls::CertificateError), #[error("invalid TLS identity: {0}")] Identity(rustls::Error), + #[error("invalid TLS configuration: {0}")] + Config(rustls::Error), #[error(transparent)] Reqwest(#[from] reqwest::Error), } @@ -75,16 +86,7 @@ impl Identity { pem.push(b'\n'); pem.extend_from_slice(cert); - // Mirror `reqwest::Identity::from_pem`, which uses the last private - // key in the buffer. - let mut keys = PrivateKeyDer::pem_slice_iter(&pem).collect::, _>>()?; - let key = keys.pop().ok_or(TlsError::NoPrivateKey)?; - keys.iter_mut().for_each(Zeroize::zeroize); - let certs = CertificateDer::pem_slice_iter(&pem).collect::, _>>()?; - if certs.is_empty() { - return Err(TlsError::NoCertificate); - } - + let (certs, key) = parse_identity_pem(&pem)?; // reqwest only checks that the key matches the certificate when the // client is built, so check here to report the error up front. let provider = rustls::crypto::aws_lc_rs::default_provider(); @@ -97,6 +99,192 @@ impl Identity { } } +/// Splits an identity PEM buffer into its certificate chain and private key. +fn parse_identity_pem( + pem: &[u8], +) -> Result<(Vec>, PrivateKeyDer<'static>), TlsError> { + // Mirror `reqwest::Identity::from_pem`, which uses the last private key in + // the buffer. + let mut keys = PrivateKeyDer::pem_slice_iter(pem).collect::, _>>()?; + let key = keys.pop().ok_or(TlsError::NoPrivateKey)?; + keys.iter_mut().for_each(Zeroize::zeroize); + let certs = CertificateDer::pem_slice_iter(pem).collect::, _>>()?; + if certs.is_empty() { + return Err(TlsError::NoCertificate); + } + Ok((certs, key)) +} + +/// Builds the rustls configuration for a client that trusts `roots` in +/// addition to the platform's trust store, and that presents `identity`, if +/// any, for client authentication. +/// +/// Server certificates are verified by `rustls-platform-verifier`, as reqwest +/// does by default, with the [`ExactRootMatch`] fallback. +pub(crate) fn rustls_config( + roots: &[Certificate], + identity: Option<&Identity>, +) -> Result { + let provider = Arc::new(rustls::crypto::aws_lc_rs::default_provider()); + let roots: Vec<_> = roots + .iter() + .map(|cert| CertificateDer::from(cert.der.clone())) + .collect(); + let inner = rustls_platform_verifier::Verifier::new_with_extra_roots( + roots.clone(), + Arc::clone(&provider), + ) + .map_err(TlsError::Config)?; + let builder = rustls::ClientConfig::builder_with_provider(provider) + .with_safe_default_protocol_versions() + .map_err(TlsError::Config)? + .dangerous() + .with_custom_certificate_verifier(Arc::new(ExactRootMatch { inner, roots })); + let mut config = match identity { + Some(identity) => { + let (certs, key) = parse_identity_pem(&identity.pem)?; + builder + .with_client_auth_cert(certs, key) + .map_err(TlsError::Identity)? + } + None => builder.with_no_client_auth(), + }; + // reqwest only sets ALPN on TLS configurations it builds itself. This + // mirrors its choice while the workspace enables reqwest's `http2` feature. + config.alpn_protocols = vec![b"h2".to_vec(), b"http/1.1".to_vec()]; + Ok(config) +} + +/// A server certificate verifier that accepts a server certificate that is +/// byte-for-byte identical to one of `roots`, and otherwise defers to `inner`. +/// +/// An exact match must still be within its validity period and valid for the +/// server name, but skips the chain, basic constraints and key usage checks. +/// This keeps a self-signed `CA:TRUE` certificate working when it is supplied +/// as its own certificate authority, which webpki rejects as +/// `CaUsedAsEndEntity`. The name check falls back to the subject common names +/// when an exact match has no DNS or IP subjectAltName, see +/// [`common_name_matches`]. Every other certificate is checked against +/// subjectAltNames only. An exact match is decided without consulting +/// `inner`. Handshake signatures are always verified by `inner`. +#[derive(Debug)] +struct ExactRootMatch { + inner: rustls_platform_verifier::Verifier, + roots: Vec>, +} + +/// Returns whether `server_name` is a DNS name equal, ignoring ASCII case, to +/// any subject common name of `cert`, and `cert` has no DNS or IP +/// subjectAltName. +/// +/// This is stricter than OpenSSL's fallback, which also applies when only IP +/// subjectAltNames are present, matches wildcard common names, and decodes +/// string types other than UTF8String, PrintableString and IA5String. +fn common_name_matches(cert: &x509_cert::Certificate, server_name: &ServerName<'_>) -> bool { + let ServerName::DnsName(name) = server_name else { + return false; + }; + let tbs = &cert.tbs_certificate; + // An undecodable subjectAltName extension counts as present. + let has_san = tbs.filter::().any(|san| match san { + Ok((_, SubjectAltName(names))) => names + .iter() + .any(|name| matches!(name, GeneralName::DnsName(_) | GeneralName::IpAddress(_))), + Err(_) => true, + }); + if has_san { + return false; + } + tbs.subject + .0 + .iter() + .flat_map(|rdn| rdn.0.iter()) + .filter(|atv| atv.oid == rfc4519::CN) + .any(|cn| { + let cn = match cn.value.tag() { + Tag::Utf8String => cn + .value + .decode_as::>() + .map(|s| s.as_str().to_owned()), + Tag::PrintableString => cn + .value + .decode_as::>() + .map(|s| s.as_str().to_owned()), + Tag::Ia5String => cn + .value + .decode_as::>() + .map(|s| s.as_str().to_owned()), + _ => return false, + }; + cn.is_ok_and(|cn| cn.eq_ignore_ascii_case(name.as_ref())) + }) +} + +impl ServerCertVerifier for ExactRootMatch { + fn verify_server_cert( + &self, + end_entity: &CertificateDer<'_>, + intermediates: &[CertificateDer<'_>], + server_name: &ServerName<'_>, + ocsp_response: &[u8], + now: UnixTime, + ) -> Result { + // Checked before `inner`, which would reject a `CA:TRUE` certificate and + // logs every rejection at error level. + if self + .roots + .iter() + .any(|root| root.as_ref() == end_entity.as_ref()) + { + let cert = x509_cert::Certificate::from_der(end_entity) + .map_err(|_| rustls::Error::InvalidCertificate(CertificateError::BadEncoding))?; + let validity = &cert.tbs_certificate.validity; + let now = now.as_secs(); + if now < validity.not_before.to_unix_duration().as_secs() { + return Err(rustls::Error::InvalidCertificate( + CertificateError::NotValidYet, + )); + } + if now > validity.not_after.to_unix_duration().as_secs() { + return Err(rustls::Error::InvalidCertificate(CertificateError::Expired)); + } + // Checks subjectAltNames only, not basic constraints, so a + // `CA:TRUE` certificate passes. Errors match `inner`'s on Linux. + let parsed = ParsedCertificate::try_from(end_entity)?; + match verify_server_name(&parsed, server_name) { + Ok(()) => {} + Err(_) if common_name_matches(&cert, server_name) => {} + Err(e) => return Err(e), + } + return Ok(ServerCertVerified::assertion()); + } + self.inner + .verify_server_cert(end_entity, intermediates, server_name, ocsp_response, now) + } + + fn verify_tls12_signature( + &self, + message: &[u8], + cert: &CertificateDer<'_>, + dss: &DigitallySignedStruct, + ) -> Result { + self.inner.verify_tls12_signature(message, cert, dss) + } + + fn verify_tls13_signature( + &self, + message: &[u8], + cert: &CertificateDer<'_>, + dss: &DigitallySignedStruct, + ) -> Result { + self.inner.verify_tls13_signature(message, cert, dss) + } + + fn supported_verify_schemes(&self) -> Vec { + self.inner.supported_verify_schemes() + } +} + impl From for reqwest::Identity { fn from(id: Identity) -> Self { reqwest::Identity::from_pem(&id.pem).expect("known to be a valid identity") diff --git a/src/ccsr/tests/client.rs b/src/ccsr/tests/client.rs index 96ea1786f0145..7ad7677795bee 100644 --- a/src/ccsr/tests/client.rs +++ b/src/ccsr/tests/client.rs @@ -455,36 +455,343 @@ fn test_invalid_tls_config_returns_error() { assert!(err.is_err(), "garbage DER cert should be rejected"); } +/// A test certificate and its private key. +struct TestCert { + key: openssl::pkey::PKey, + cert: openssl::x509::X509, +} + +impl TestCert { + /// Generates a certificate with common name `cn`, a DNS subjectAltName + /// `san` if given, and `CA:TRUE` if `ca`. It is signed by `issuer`, or + /// self-signed if `issuer` is `None`. It is valid from now until a day + /// from now. + fn new(cn: &str, san: Option<&str>, ca: bool, issuer: Option<&TestCert>) -> TestCert { + Self::with_validity(cn, san, ca, issuer, 0, 1) + } + + /// Like [`TestCert::new`], but valid from `not_before` to `not_after` + /// days from now. + fn with_validity( + cn: &str, + san: Option<&str>, + ca: bool, + issuer: Option<&TestCert>, + not_before: i64, + not_after: i64, + ) -> TestCert { + Self::generate(&[cn], san, None, ca, issuer, not_before, not_after) + } + + /// Generates a self-signed `CA:TRUE` certificate with the common names + /// `cns`, in order, and the subjectAltNames `dns_san` and `ip_san`, if + /// given. + fn pinned(cns: &[&str], dns_san: Option<&str>, ip_san: Option<&str>) -> TestCert { + Self::generate(cns, dns_san, ip_san, true, None, 0, 1) + } + + fn generate( + cns: &[&str], + dns_san: Option<&str>, + ip_san: Option<&str>, + ca: bool, + issuer: Option<&TestCert>, + not_before: i64, + not_after: i64, + ) -> TestCert { + use openssl::asn1::Asn1Time; + use openssl::bn::{BigNum, MsbOption}; + use openssl::ec::{EcGroup, EcKey}; + use openssl::hash::MessageDigest; + use openssl::nid::Nid; + use openssl::pkey::PKey; + use openssl::x509::extension::{ + BasicConstraints, ExtendedKeyUsage, SubjectAlternativeName, + }; + use openssl::x509::{X509, X509NameBuilder}; + + let group = EcGroup::from_curve_name(Nid::X9_62_PRIME256V1).unwrap(); + let key = PKey::from_ec_key(EcKey::generate(&group).unwrap()).unwrap(); + let mut name = X509NameBuilder::new().unwrap(); + for cn in cns { + name.append_entry_by_text("CN", cn).unwrap(); + } + let name = name.build(); + let mut serial = BigNum::new().unwrap(); + serial.rand(64, MsbOption::MAYBE_ZERO, false).unwrap(); + + let mut cert = X509::builder().unwrap(); + cert.set_version(2).unwrap(); + cert.set_serial_number(&serial.to_asn1_integer().unwrap()) + .unwrap(); + cert.set_subject_name(&name).unwrap(); + cert.set_issuer_name(issuer.map_or(&name, |i| i.cert.subject_name())) + .unwrap(); + cert.set_pubkey(&key).unwrap(); + let days_from_now = |days: i64| { + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs(); + Asn1Time::from_unix(i64::try_from(now).unwrap() + days * 86_400).unwrap() + }; + cert.set_not_before(&days_from_now(not_before)).unwrap(); + cert.set_not_after(&days_from_now(not_after)).unwrap(); + let mut constraints = BasicConstraints::new(); + constraints.critical(); + if ca { + constraints.ca(); + } + cert.append_extension(constraints.build().unwrap()).unwrap(); + if !ca { + let eku = ExtendedKeyUsage::new() + .server_auth() + .client_auth() + .build() + .unwrap(); + cert.append_extension(eku).unwrap(); + } + if dns_san.is_some() || ip_san.is_some() { + let mut san = SubjectAlternativeName::new(); + if let Some(dns) = dns_san { + san.dns(dns); + } + if let Some(ip) = ip_san { + san.ip(ip); + } + let ctx = cert.x509v3_context(issuer.map(|i| &*i.cert), None); + cert.append_extension(san.build(&ctx).unwrap()).unwrap(); + } + let signer = issuer.map_or(&key, |i| &i.key); + cert.sign(signer, MessageDigest::sha256()).unwrap(); + TestCert { + key, + cert: cert.build(), + } + } + + fn cert_pem(&self) -> Vec { + self.cert.to_pem().unwrap() + } +} + /// Returns a PEM-encoded PKCS #8 key and a self-signed certificate for it. fn self_signed_pem() -> (Vec, Vec) { - use openssl::asn1::Asn1Time; - use openssl::ec::{EcGroup, EcKey}; - use openssl::hash::MessageDigest; - use openssl::nid::Nid; - use openssl::pkey::PKey; - use openssl::x509::{X509, X509NameBuilder}; - - let group = EcGroup::from_curve_name(Nid::X9_62_PRIME256V1).unwrap(); - let key = PKey::from_ec_key(EcKey::generate(&group).unwrap()).unwrap(); - let mut name = X509NameBuilder::new().unwrap(); - name.append_entry_by_text("CN", "ccsr-test").unwrap(); - let name = name.build(); - let mut cert = X509::builder().unwrap(); - cert.set_version(2).unwrap(); - cert.set_subject_name(&name).unwrap(); - cert.set_issuer_name(&name).unwrap(); - cert.set_pubkey(&key).unwrap(); - cert.set_not_before(&Asn1Time::days_from_now(0).unwrap()) - .unwrap(); - cert.set_not_after(&Asn1Time::days_from_now(1).unwrap()) - .unwrap(); - cert.sign(&key, MessageDigest::sha256()).unwrap(); + let cert = TestCert::new("ccsr-test", None, false, None); ( - key.private_key_to_pem_pkcs8().unwrap(), - cert.build().to_pem().unwrap(), + cert.key.private_key_to_pem_pkcs8().unwrap(), + cert.cert_pem(), ) } +const TLS_TEST_HOST: &str = "sr.test"; + +/// Starts an HTTPS server presenting `server` and returns a client for it that +/// trusts `root`. +async fn start_tls_server(server: &TestCert, root: &TestCert) -> Client { + let addr = start_tls_listener(server, None).await; + tls_client_config(addr, root).build().unwrap() +} + +/// Returns a client configuration for a server started by +/// [`start_tls_listener`] that trusts `root`. +fn tls_client_config(addr: std::net::SocketAddr, root: &TestCert) -> mz_ccsr::ClientConfig { + let url = format!("https://{TLS_TEST_HOST}:{}", addr.port()) + .parse() + .unwrap(); + let root = mz_ccsr::tls::Certificate::from_pem(&root.cert_pem()).unwrap(); + mz_ccsr::ClientConfig::new(url) + .resolve_to_addrs(TLS_TEST_HOST, &[addr]) + .add_root_certificate(root) +} + +/// Starts an HTTPS server presenting `server`. If `client_ca` is given, the +/// server requires a client certificate signed by it. +async fn start_tls_listener( + server: &TestCert, + client_ca: Option<&TestCert>, +) -> std::net::SocketAddr { + use openssl::ssl::{Ssl, SslAcceptor, SslMethod, SslVerifyMode}; + + let mut acceptor = SslAcceptor::mozilla_intermediate_v5(SslMethod::tls()).unwrap(); + acceptor.set_private_key(&server.key).unwrap(); + acceptor.set_certificate(&server.cert).unwrap(); + if let Some(client_ca) = client_ca { + acceptor + .cert_store_mut() + .add_cert(client_ca.cert.clone()) + .unwrap(); + acceptor.set_verify(SslVerifyMode::PEER | SslVerifyMode::FAIL_IF_NO_PEER_CERT); + } + let acceptor = acceptor.build(); + + let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).await.unwrap(); + let addr = listener.local_addr().unwrap(); + mz_ore::task::spawn(|| "start_tls_server", async move { + loop { + let (conn, _) = listener.accept().await.unwrap(); + let ssl = Ssl::new(acceptor.context()).unwrap(); + let mut stream = tokio_openssl::SslStream::new(ssl, conn).unwrap(); + mz_ore::task::spawn(|| "start_tls_server:conn", async move { + // Some tests expect the handshake to fail. + if std::pin::Pin::new(&mut stream).accept().await.is_err() { + return; + } + let service = service::service_fn(|_req| async { + Response::builder() + .status(StatusCode::OK) + .body("[]".to_string()) + }); + let _ = hyper::server::conn::http1::Builder::new() + .serve_connection(TokioIo::new(stream), service) + .await; + }); + } + }); + addr +} + +#[mz_ore::test(tokio::test)] +#[cfg_attr(miri, ignore)] // unsupported operation: can't call foreign function `OPENSSL_init_ssl` on OS `linux` +async fn test_tls_server_verification() { + use mz_ore::error::ErrorExt; + + let ca = TestCert::new("ccsr test ca", None, true, None); + + let leaf = TestCert::new(TLS_TEST_HOST, Some(TLS_TEST_HOST), false, Some(&ca)); + let client = start_tls_server(&leaf, &ca).await; + client + .list_subjects() + .await + .expect("SAN leaf signed by the root should be accepted"); + + // webpki rejects a `CA:TRUE` server certificate as `CaUsedAsEndEntity`. + // It is accepted because it is identical to the configured root. + let self_signed_ca = TestCert::new(TLS_TEST_HOST, Some(TLS_TEST_HOST), true, None); + let client = start_tls_server(&self_signed_ca, &self_signed_ca).await; + client + .list_subjects() + .await + .expect("server certificate identical to the root should be accepted"); + + // A pinned certificate without a subjectAltName, as `openssl req -x509 + // -subj /CN=` produces, matches on its common name. + let cn_only_pinned = TestCert::new(TLS_TEST_HOST, None, true, None); + let client = start_tls_server(&cn_only_pinned, &cn_only_pinned).await; + client + .list_subjects() + .await + .expect("pinned certificate whose common name matches should be accepted"); + + let cn_only_wrong_name = TestCert::new("other.test", None, true, None); + let client = start_tls_server(&cn_only_wrong_name, &cn_only_wrong_name).await; + let err = client.list_subjects().await.unwrap_err(); + let err = err.display_with_causes().to_string(); + assert!( + err.contains("not valid for name"), + "unexpected error: {err}" + ); + + // Any common name may match. + for cns in [[TLS_TEST_HOST, "other.test"], ["other.test", TLS_TEST_HOST]] { + let pinned = TestCert::pinned(&cns, None, None); + let client = start_tls_server(&pinned, &pinned).await; + client + .list_subjects() + .await + .expect("pinned certificate with a matching common name should be accepted"); + } + + // Stricter than OpenSSL: an IP subjectAltName also disables the common + // name fallback. + let ip_san = TestCert::pinned(&[TLS_TEST_HOST], None, Some("127.0.0.1")); + let client = start_tls_server(&ip_san, &ip_san).await; + let err = client.list_subjects().await.unwrap_err(); + let err = err.display_with_causes().to_string(); + assert!( + err.contains("not valid for name"), + "unexpected error: {err}" + ); + + // A subjectAltName takes precedence over a matching common name. + let wrong_name = TestCert::new(TLS_TEST_HOST, Some("other.test"), true, None); + let client = start_tls_server(&wrong_name, &wrong_name).await; + let err = client.list_subjects().await.unwrap_err(); + let err = err.display_with_causes().to_string(); + assert!( + err.contains("not valid for name"), + "unexpected error: {err}" + ); + + for (not_before, not_after, expected) in [(-2, -1, "Expired"), (1, 2, "NotValidYet")] { + let pinned = TestCert::with_validity( + TLS_TEST_HOST, + Some(TLS_TEST_HOST), + true, + None, + not_before, + not_after, + ); + let client = start_tls_server(&pinned, &pinned).await; + let err = client.list_subjects().await.unwrap_err(); + let err = err.display_with_causes().to_string(); + assert!(err.contains(expected), "unexpected error: {err}"); + } + + let other = TestCert::new(TLS_TEST_HOST, Some(TLS_TEST_HOST), true, None); + let client = start_tls_server(&self_signed_ca, &other).await; + let err = client.list_subjects().await.unwrap_err(); + let err = err.display_with_causes().to_string(); + assert!( + err.contains("invalid peer certificate"), + "unexpected error: {err}" + ); + + // webpki does not fall back to the common name when there is no + // subjectAltName. + let cn_only = TestCert::new(TLS_TEST_HOST, None, false, Some(&ca)); + let client = start_tls_server(&cn_only, &ca).await; + let err = client.list_subjects().await.unwrap_err(); + let err = err.display_with_causes().to_string(); + // NOTE: On macOS the platform verifier delegates to Security.framework, + // which words this error differently. + let expected = if cfg!(target_os = "linux") { + "not valid for name" + } else { + "invalid peer certificate" + }; + assert!(err.contains(expected), "unexpected error: {err}"); +} + +#[mz_ore::test(tokio::test)] +#[cfg_attr(miri, ignore)] // unsupported operation: can't call foreign function `OPENSSL_init_ssl` on OS `linux` +async fn test_tls_client_identity_with_roots() { + let ca = TestCert::new("ccsr test ca", None, true, None); + let server = TestCert::new(TLS_TEST_HOST, Some(TLS_TEST_HOST), false, Some(&ca)); + let client_cert = TestCert::new("ccsr test client", None, false, Some(&ca)); + let addr = start_tls_listener(&server, Some(&ca)).await; + + let identity = Identity::from_pem( + &client_cert.key.private_key_to_pem_pkcs8().unwrap(), + &client_cert.cert_pem(), + ) + .unwrap(); + let client = tls_client_config(addr, &ca) + .identity(identity) + .build() + .unwrap(); + client + .list_subjects() + .await + .expect("client certificate should be presented"); + + let client = tls_client_config(addr, &ca).build().unwrap(); + client + .list_subjects() + .await + .expect_err("server requires a client certificate"); +} + #[mz_ore::test] #[cfg_attr(miri, ignore)] // unsupported operation: can't call foreign function `OPENSSL_init_ssl` on OS `linux` fn test_pem_identity() {