diff --git a/src/tls/inspect.rs b/src/tls/inspect.rs index f57da48c..9ed19a74 100644 --- a/src/tls/inspect.rs +++ b/src/tls/inspect.rs @@ -478,24 +478,19 @@ fn build_client_config( .with_protocol_versions(&versions) .map_err(|_| FetchError::Message("invalid TLS versions".to_string()))? }; - let builder = if cli.insecure { - builder - .dangerous() - .with_custom_certificate_verifier(Arc::new(NoCertificateVerification { - ocsp_capture, - supported_schemes: provider - .signature_verification_algorithms - .supported_schemes(), - })) + let verifier: Arc = if cli.insecure { + Arc::new(super::InsecureServerVerifier::new( + provider.signature_verification_algorithms, + )) } else { - let verifier = super::rustls_platform_verifier(&cli.ca_cert, provider)?; - builder - .dangerous() - .with_custom_certificate_verifier(Arc::new(CapturingServerVerifier { - inner: Arc::new(verifier), - ocsp_capture, - })) + Arc::new(super::rustls_platform_verifier(&cli.ca_cert, provider)?) }; + let builder = builder + .dangerous() + .with_custom_certificate_verifier(Arc::new(CapturingServerVerifier { + inner: verifier, + ocsp_capture, + })); if let Some((certs, key)) = super::rustls_client_auth(cli.cert.as_deref(), cli.key.as_deref())? { @@ -611,48 +606,6 @@ impl ServerCertVerifier for CapturingServerVerifier { } } -#[derive(Debug)] -struct NoCertificateVerification { - ocsp_capture: OcspCapture, - supported_schemes: Vec, -} - -impl ServerCertVerifier for NoCertificateVerification { - fn verify_server_cert( - &self, - _end_entity: &CertificateDer<'_>, - _intermediates: &[CertificateDer<'_>], - _server_name: &ServerName<'_>, - ocsp_response: &[u8], - _now: UnixTime, - ) -> Result { - self.ocsp_capture.set(ocsp_response); - Ok(ServerCertVerified::assertion()) - } - - fn verify_tls12_signature( - &self, - _message: &[u8], - _cert: &CertificateDer<'_>, - _dss: &DigitallySignedStruct, - ) -> Result { - Ok(HandshakeSignatureValid::assertion()) - } - - fn verify_tls13_signature( - &self, - _message: &[u8], - _cert: &CertificateDer<'_>, - _dss: &DigitallySignedStruct, - ) -> Result { - Ok(HandshakeSignatureValid::assertion()) - } - - fn supported_verify_schemes(&self) -> Vec { - self.supported_schemes.clone() - } -} - impl fmt::Debug for Inspection { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("Inspection") diff --git a/src/tls/mod.rs b/src/tls/mod.rs index cc4944db..9e991dee 100644 --- a/src/tls/mod.rs +++ b/src/tls/mod.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use rustls::client::EchMode; use rustls::client::danger::{HandshakeSignatureValid, ServerCertVerified, ServerCertVerifier}; +use rustls::crypto::WebPkiSupportedAlgorithms; use rustls::pki_types::{CertificateDer, PrivateKeyDer, ServerName, UnixTime}; use rustls::{DigitallySignedStruct, SignatureScheme, SupportedProtocolVersion}; @@ -165,11 +166,9 @@ pub fn rustls_platform_client_config_with_options( let builder = if insecure { builder .dangerous() - .with_custom_certificate_verifier(Arc::new(NoCertificateVerification { - supported_schemes: provider - .signature_verification_algorithms - .supported_schemes(), - })) + .with_custom_certificate_verifier(Arc::new(InsecureServerVerifier::new( + provider.signature_verification_algorithms, + ))) } else { let verifier = rustls_platform_verifier(ca_cert_paths, provider)?; builder @@ -424,12 +423,21 @@ fn first_private_key(data: &[u8]) -> Result>, Fetc Ok(None) } +/// Accepts any server certificate chain while preserving TLS handshake integrity. #[derive(Debug)] -struct NoCertificateVerification { - supported_schemes: Vec, +pub(crate) struct InsecureServerVerifier { + supported_algorithms: WebPkiSupportedAlgorithms, } -impl ServerCertVerifier for NoCertificateVerification { +impl InsecureServerVerifier { + pub(crate) fn new(supported_algorithms: WebPkiSupportedAlgorithms) -> Self { + Self { + supported_algorithms, + } + } +} + +impl ServerCertVerifier for InsecureServerVerifier { fn verify_server_cert( &self, _end_entity: &CertificateDer<'_>, @@ -443,24 +451,24 @@ impl ServerCertVerifier for NoCertificateVerification { fn verify_tls12_signature( &self, - _message: &[u8], - _cert: &CertificateDer<'_>, - _dss: &DigitallySignedStruct, + message: &[u8], + cert: &CertificateDer<'_>, + dss: &DigitallySignedStruct, ) -> Result { - Ok(HandshakeSignatureValid::assertion()) + rustls::crypto::verify_tls12_signature(message, cert, dss, &self.supported_algorithms) } fn verify_tls13_signature( &self, - _message: &[u8], - _cert: &CertificateDer<'_>, - _dss: &DigitallySignedStruct, + message: &[u8], + cert: &CertificateDer<'_>, + dss: &DigitallySignedStruct, ) -> Result { - Ok(HandshakeSignatureValid::assertion()) + rustls::crypto::verify_tls13_signature(message, cert, dss, &self.supported_algorithms) } fn supported_verify_schemes(&self) -> Vec { - self.supported_schemes.clone() + self.supported_algorithms.supported_schemes() } } @@ -538,9 +546,7 @@ mod tests { let supported_schemes = provider .signature_verification_algorithms .supported_schemes(); - let verifier = NoCertificateVerification { - supported_schemes: supported_schemes.clone(), - }; + let verifier = InsecureServerVerifier::new(provider.signature_verification_algorithms); assert!(supported_schemes.contains(&SignatureScheme::ECDSA_NISTP521_SHA512)); assert_eq!(verifier.supported_verify_schemes(), supported_schemes); diff --git a/tests/network.rs b/tests/network.rs index 74d5c55f..9b464f4b 100644 --- a/tests/network.rs +++ b/tests/network.rs @@ -29,7 +29,8 @@ use support::proxy::{ start_stalling_proxy, }; use support::tls::{ - start_h2_tls_server, start_h2_tls_server_with_accept_delay, start_mtls_server, start_tls_server, + start_h2_tls_server, start_h2_tls_server_with_accept_delay, + start_invalid_certificate_verify_server, start_mtls_server, start_tls_server, }; use tempfile::TempDir; use url::Url; @@ -2023,6 +2024,34 @@ fn tls_certificate_validation_inspection_and_bounds_cases() { assert!(res.stderr.contains("timing") || res.stderr.contains("TLS")); } +#[test] +fn insecure_rejects_invalid_certificate_verify_signatures() { + let tls = start_invalid_certificate_verify_server(); + + for version in ["1.2", "1.3"] { + let res = run_fetch(&[ + "--insecure", + "--min-tls", + version, + "--max-tls", + version, + &tls.url, + ]); + assert_exit(&res, 1); + + let res = run_fetch(&[ + "--inspect-tls", + "--insecure", + "--min-tls", + version, + "--max-tls", + version, + &tls.url, + ]); + assert_exit(&res, 1); + } +} + #[test] fn mtls_client_certificate_go_cases() { let mtls = start_mtls_server(); diff --git a/tests/support/tls.rs b/tests/support/tls.rs index 93cab6e8..939b1c33 100644 --- a/tests/support/tls.rs +++ b/tests/support/tls.rs @@ -7,6 +7,9 @@ use std::sync::{Arc, mpsc}; use std::thread; use std::time::Duration; +use rustls::pki_types::SubjectPublicKeyInfoDer; +use rustls::sign::{CertifiedKey, Signer, SigningKey, SingleCertAndKey}; +use rustls::{SignatureAlgorithm, SignatureScheme}; use tempfile::TempDir; use super::http::{TestRequest, TestResponse, read_request, write_response}; @@ -114,6 +117,107 @@ pub(crate) fn start_tls_server( } } +#[derive(Debug)] +struct InvalidSignatureKey { + inner: Arc, +} + +impl SigningKey for InvalidSignatureKey { + fn choose_scheme(&self, offered: &[SignatureScheme]) -> Option> { + self.inner + .choose_scheme(offered) + .map(|inner| Box::new(InvalidSigner { inner }) as Box) + } + + fn public_key(&self) -> Option> { + self.inner.public_key() + } + + fn algorithm(&self) -> SignatureAlgorithm { + self.inner.algorithm() + } +} + +#[derive(Debug)] +struct InvalidSigner { + inner: Box, +} + +impl Signer for InvalidSigner { + fn sign(&self, message: &[u8]) -> Result, rustls::Error> { + let mut signature = self.inner.sign(message)?; + signature[0] ^= 1; + Ok(signature) + } + + fn scheme(&self) -> SignatureScheme { + self.inner.scheme() + } +} + +pub(crate) fn start_invalid_certificate_verify_server() -> TlsTestServer { + let provider = rustls::crypto::aws_lc_rs::default_provider(); + let certified = + rcgen::generate_simple_self_signed(vec!["127.0.0.1".to_string(), "localhost".to_string()]) + .unwrap(); + let dir = TempDir::new().unwrap().keep(); + let ca_cert_path = dir.join("ca.pem"); + fs::write(&ca_cert_path, certified.cert.pem()).unwrap(); + let key_der = rustls::pki_types::PrivateKeyDer::Pkcs8( + rustls::pki_types::PrivatePkcs8KeyDer::from(certified.signing_key.serialize_der()), + ); + let signing_key = provider.key_provider.load_private_key(key_der).unwrap(); + let certified_key = CertifiedKey::new( + vec![certified.cert.der().clone()], + Arc::new(InvalidSignatureKey { inner: signing_key }), + ); + let config = rustls::ServerConfig::builder_with_provider(Arc::new(provider)) + .with_safe_default_protocol_versions() + .unwrap() + .with_no_client_auth() + .with_cert_resolver(Arc::new(SingleCertAndKey::from(certified_key))); + let config = Arc::new(config); + let listener = TcpListener::bind("127.0.0.1:0").expect("bind invalid signature TLS server"); + listener.set_nonblocking(true).unwrap(); + let port = listener.local_addr().unwrap().port(); + let url = format!("https://localhost:{port}"); + let (tx, rx) = mpsc::channel(); + let join = thread::spawn(move || { + loop { + if rx.try_recv().is_ok() { + break; + } + match listener.accept() { + Ok((stream, _)) => { + let config = Arc::clone(&config); + thread::spawn(move || { + let Ok(conn) = rustls::ServerConnection::new(config) else { + return; + }; + let mut tls = rustls::StreamOwned::new(conn, stream); + let mut reader = BufReader::new(&mut tls); + let Some(_) = read_request(&mut reader) else { + return; + }; + let tls = reader.into_inner(); + write_response(tls, TestResponse::ok("invalid signature accepted")); + }); + } + Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => { + thread::sleep(Duration::from_millis(5)); + } + Err(_) => break, + } + } + }); + TlsTestServer { + url, + ca_cert_path, + shutdown: Some(tx), + join: Some(join), + } +} + pub(crate) fn start_h2_tls_server( handler: impl Fn(TestRequest) -> TestResponse + Send + Sync + 'static, ) -> TlsTestServer {