diff --git a/boring-additions/src/aead/mod.rs b/boring-additions/src/aead/mod.rs index 353d440..2803393 100644 --- a/boring-additions/src/aead/mod.rs +++ b/boring-additions/src/aead/mod.rs @@ -78,11 +78,7 @@ impl Crypter { /// /// # Errors /// Returns the `BoringSSL` error in case of an internal error - /// - /// # Panics - /// * If the key length mismatches the `aead_alg` required key length pub fn new(aead_alg: &Algorithm, key: &[u8]) -> Result { - assert_eq!(aead_alg.key_length(), key.len()); boring_sys::init(); let this = unsafe { @@ -116,10 +112,6 @@ impl Crypter { /// /// # Errors /// In case of an error, returns the `BoringSSL` error - /// - /// # Panics - /// * If the `nonce` is not the expected lenght - /// * If the `tag` has not enough space pub fn seal_in_place( &self, nonce: &[u8], @@ -127,8 +119,9 @@ impl Crypter { buffer: &mut [u8], tag: &mut [u8], ) -> Result { - assert!(tag.len() >= self.max_overhead); - assert_eq!(nonce.len(), self.nonce_len); + if tag.len() < self.max_overhead || nonce.len() != self.nonce_len { + return Err(ErrorStack::get()); + } let mut tag_len = tag.len(); unsafe { @@ -156,9 +149,6 @@ impl Crypter { /// /// # Errors /// In case of an error, returns the `BoringSSL` error - /// - /// # Panics - /// * if the nonce has the wrong lenght pub fn open_in_place( &self, nonce: &[u8], @@ -166,7 +156,9 @@ impl Crypter { buffer: &mut [u8], tag: &[u8], ) -> Result<(), ErrorStack> { - assert_eq!(nonce.len(), self.nonce_len); + if nonce.len() != self.nonce_len { + return Err(ErrorStack::get()); + } unsafe { cvt(boring_sys::EVP_AEAD_CTX_open_gather( @@ -188,11 +180,11 @@ impl Crypter { #[cfg(test)] mod tests { - use super::Crypter; + use super::{Algorithm, Crypter}; #[test] fn in_out() { - let key = Crypter::new(&super::Algorithm::aes_128_gcm(), &[0u8; 16]).unwrap(); + let key = Crypter::new(&Algorithm::aes_128_gcm(), &[0u8; 16]).unwrap(); let nonce = [0u8; 12]; let associated_data = b"this is authenticated"; let mut buffer = Vec::with_capacity(26); @@ -207,4 +199,35 @@ mod tests { assert_eq!(b"ABCDE", buffer.as_slice()); } + + #[test] + fn new_rejects_invalid_key_length() { + let result = Crypter::new(&Algorithm::aes_128_gcm(), &[0u8; 15]); + + assert!(result.is_err()); + } + + #[test] + fn seal_rejects_invalid_nonce_and_tag_lengths() { + let key = Crypter::new(&Algorithm::aes_128_gcm(), &[0u8; 16]).unwrap(); + let mut payload = [0u8; 8]; + + let mut short_tag = [0u8; 8]; + let short_tag_result = key.seal_in_place(&[0u8; 12], b"", &mut payload, &mut short_tag); + assert!(short_tag_result.is_err()); + + let mut tag = [0u8; 16]; + let wrong_nonce_result = key.seal_in_place(&[0u8; 11], b"", &mut payload, &mut tag); + assert!(wrong_nonce_result.is_err()); + } + + #[test] + fn open_rejects_invalid_nonce_length() { + let key = Crypter::new(&Algorithm::aes_128_gcm(), &[0u8; 16]).unwrap(); + let mut payload = [0u8; 8]; + let tag = [0u8; 16]; + + let result = key.open_in_place(&[0u8; 11], b"", &mut payload, &tag); + assert!(result.is_err()); + } } diff --git a/boring-additions/src/hmac/types.rs b/boring-additions/src/hmac/types.rs index 7601eb9..ad3661c 100644 --- a/boring-additions/src/hmac/types.rs +++ b/boring-additions/src/hmac/types.rs @@ -5,8 +5,6 @@ use std::{ use foreign_types::{ForeignType, ForeignTypeRef, Opaque}; -use crate::helper::{cvt, cvt_p}; - pub struct HmacCtxRef(Opaque); unsafe impl ForeignTypeRef for HmacCtxRef { @@ -35,20 +33,6 @@ unsafe impl ForeignType for HmacCtx { } } -impl Clone for HmacCtx { - fn clone(&self) -> Self { - unsafe { - cvt_p(boring_sys::HMAC_CTX_new()) - .map(|ctx| HmacCtx::from_ptr(ctx)) - .and_then(|ctx| { - cvt(boring_sys::HMAC_CTX_copy(ctx.as_ptr(), self.0.as_ptr()))?; - Ok(ctx) - }) - } - .expect("failed cloning hmac ctx") - } -} - impl Drop for HmacCtx { fn drop(&mut self) { unsafe { diff --git a/boring-rustls-provider/src/aead.rs b/boring-rustls-provider/src/aead.rs index cade8a3..6bcd3ee 100644 --- a/boring-rustls-provider/src/aead.rs +++ b/boring-rustls-provider/src/aead.rs @@ -35,6 +35,9 @@ pub(crate) trait BoringCipher { /// confidentiality limit const CONFIDENTIALITY_LIMIT: u64; + /// Whether this algorithm is FIPS-approved. + const FIPS_APPROVED: bool; + /// Constructs a new instance of this cipher as an AEAD algorithm fn new_cipher() -> Algorithm; @@ -49,7 +52,7 @@ pub(crate) trait QuicCipher: Send + Sync { /// the expected length of a sample const SAMPLE_LEN: usize; - fn header_protection_mask(hp_key: &[u8], sample: &[u8]) -> [u8; 5]; + fn header_protection_mask(hp_key: &[u8], sample: &[u8]) -> Result<[u8; 5], rustls::Error>; } pub(crate) trait BoringAead: BoringCipher + AeadCore + Send + Sync {} @@ -74,19 +77,21 @@ impl AeadCore for BoringAeadCrypter { impl BoringAeadCrypter { /// Creates a new aead crypter pub fn new(iv: Iv, key: &[u8], tls_version: ProtocolVersion) -> Result { - assert!(match tls_version { + let tls_version_supported = match tls_version { #[cfg(feature = "tls12")] ProtocolVersion::TLSv1_2 => true, ProtocolVersion::TLSv1_3 => true, _ => false, - }); + }; + if !tls_version_supported { + return Err(ErrorStack::get()); + } let cipher = ::new_cipher(); - assert_eq!( - cipher.nonce_len(), - rustls::crypto::cipher::Nonce::new(&iv, 0).0.len() - ); + if cipher.nonce_len() != rustls::crypto::cipher::Nonce::new(&iv, 0).0.len() { + return Err(ErrorStack::get()); + } let crypter = BoringAeadCrypter { crypter: boring_additions::aead::Crypter::new(&cipher, key)?, @@ -181,18 +186,21 @@ where ) }) } - _ => unimplemented!(), + _ => Err(rustls::Error::EncryptError), } } fn encrypted_payload_len(&self, payload_len: usize) -> usize { - match self.tls_version { - ProtocolVersion::TLSv1_2 => { - payload_len + self.crypter.max_overhead() + ::EXPLICIT_NONCE_LEN - } - ProtocolVersion::TLSv1_3 => payload_len + 1 + self.crypter.max_overhead(), - _ => unimplemented!(), - } + let per_record_overhead = match self.tls_version { + ProtocolVersion::TLSv1_2 => self + .crypter + .max_overhead() + .saturating_add(::EXPLICIT_NONCE_LEN), + ProtocolVersion::TLSv1_3 => 1usize.saturating_add(self.crypter.max_overhead()), + _ => return payload_len, + }; + + payload_len.saturating_add(per_record_overhead) } } @@ -209,10 +217,17 @@ where #[cfg(feature = "tls12")] ProtocolVersion::TLSv1_2 => { let explicit_nonce_len = ::EXPLICIT_NONCE_LEN; + let tag_len = self.crypter.max_overhead(); + let min_payload_len = explicit_nonce_len + .checked_add(tag_len) + .ok_or(rustls::Error::DecryptError)?; + + if m.payload.len() < min_payload_len { + return Err(rustls::Error::DecryptError); + } // payload is: [nonce] | [ciphertext] | [auth tag] - let actual_payload_length = - m.payload.len() - self.crypter.max_overhead() - explicit_nonce_len; + let actual_payload_length = m.payload.len() - tag_len - explicit_nonce_len; let aad = make_tls12_aad(seq, m.typ, m.version, actual_payload_length); @@ -224,7 +239,9 @@ where let nonce = { let fixed_iv_len = ::FIXED_IV_LEN; - assert_eq!(explicit_nonce_len + fixed_iv_len, 12); + if explicit_nonce_len + fixed_iv_len != 12 { + return Err(rustls::Error::DecryptError); + } // grab the IV by constructing a nonce, this is just an xor let iv = cipher::Nonce::new(&self.iv, seq).0; @@ -235,8 +252,7 @@ where }; // split off the authentication tag - let (payload, tag) = - payload.split_at_mut(payload.len() - self.crypter.max_overhead()); + let (payload, tag) = payload.split_at_mut(payload.len() - tag_len); self.crypter .open_in_place(&nonce, &aad, payload, tag) @@ -262,7 +278,7 @@ where .map_err(|_| rustls::Error::DecryptError) .and_then(|_| m.into_tls13_unpadded_message()) } - _ => unimplemented!(), + _ => Err(rustls::Error::DecryptError), } } } @@ -295,6 +311,10 @@ where let associated_data = header; let nonce = cipher::Nonce::new(&self.iv, packet_number); + if payload.len() < self.crypter.max_overhead() { + return Err(rustls::Error::DecryptError); + } + let (buffer, tag) = payload.split_at_mut(payload.len() - self.crypter.max_overhead()); self.crypter @@ -317,6 +337,68 @@ where } } +struct InvalidMessageEncrypter; + +impl cipher::MessageEncrypter for InvalidMessageEncrypter { + fn encrypt( + &mut self, + _msg: cipher::OutboundPlainMessage<'_>, + _seq: u64, + ) -> Result { + Err(rustls::Error::EncryptError) + } + + fn encrypted_payload_len(&self, payload_len: usize) -> usize { + payload_len + } +} + +struct InvalidMessageDecrypter; + +impl cipher::MessageDecrypter for InvalidMessageDecrypter { + fn decrypt<'a>( + &mut self, + _msg: cipher::InboundOpaqueMessage<'a>, + _seq: u64, + ) -> Result, rustls::Error> { + Err(rustls::Error::DecryptError) + } +} + +struct InvalidPacketKey(PhantomData); + +impl rustls::quic::PacketKey for InvalidPacketKey { + fn encrypt_in_place( + &self, + _packet_number: u64, + _header: &[u8], + _payload: &mut [u8], + ) -> Result { + Err(rustls::Error::EncryptError) + } + + fn decrypt_in_place<'a>( + &self, + _packet_number: u64, + _header: &[u8], + _payload: &'a mut [u8], + ) -> Result<&'a [u8], rustls::Error> { + Err(rustls::Error::DecryptError) + } + + fn tag_len(&self) -> usize { + ::TAG_LEN + } + + fn confidentiality_limit(&self) -> u64 { + 0 + } + + fn integrity_limit(&self) -> u64 { + 0 + } +} + pub(crate) struct Aead(PhantomData); impl Aead { @@ -325,17 +407,31 @@ impl Aead { impl cipher::Tls13AeadAlgorithm for Aead { fn encrypter(&self, key: cipher::AeadKey, iv: cipher::Iv) -> Box { - Box::new( - BoringAeadCrypter::::new(iv, key.as_ref(), ProtocolVersion::TLSv1_3) - .expect("failed to create AEAD crypter"), - ) + match BoringAeadCrypter::::new(iv, key.as_ref(), ProtocolVersion::TLSv1_3) { + Ok(crypter) => Box::new(crypter), + Err(err) => { + log_and_map( + "Tls13AeadAlgorithm::encrypter BoringAeadCrypter::new", + err, + (), + ); + Box::new(InvalidMessageEncrypter) + } + } } fn decrypter(&self, key: cipher::AeadKey, iv: cipher::Iv) -> Box { - Box::new( - BoringAeadCrypter::::new(iv, key.as_ref(), ProtocolVersion::TLSv1_3) - .expect("failed to create AEAD crypter"), - ) + match BoringAeadCrypter::::new(iv, key.as_ref(), ProtocolVersion::TLSv1_3) { + Ok(crypter) => Box::new(crypter), + Err(err) => { + log_and_map( + "Tls13AeadAlgorithm::decrypter BoringAeadCrypter::new", + err, + (), + ); + Box::new(InvalidMessageDecrypter) + } + } } fn key_len(&self) -> usize { @@ -349,6 +445,10 @@ impl cipher::Tls13AeadAlgorithm for Aead { ) -> Result { Ok(::extract_keys(key, iv)) } + + fn fips(&self) -> bool { + cfg!(feature = "fips") && ::FIPS_APPROVED + } } #[cfg(feature = "tls12")] @@ -362,24 +462,42 @@ impl cipher::Tls12AeadAlgorithm for Aead { let mut full_iv = Vec::with_capacity(iv.len() + extra.len()); full_iv.extend_from_slice(iv); full_iv.extend_from_slice(extra); - Box::new( - BoringAeadCrypter::::new(Iv::copy(&full_iv), key.as_ref(), ProtocolVersion::TLSv1_2) - .expect("failed to create AEAD crypter"), - ) + match BoringAeadCrypter::::new( + Iv::copy(&full_iv), + key.as_ref(), + ProtocolVersion::TLSv1_2, + ) { + Ok(crypter) => Box::new(crypter), + Err(err) => { + log_and_map( + "Tls12AeadAlgorithm::encrypter BoringAeadCrypter::new", + err, + (), + ); + Box::new(InvalidMessageEncrypter) + } + } } fn decrypter(&self, key: cipher::AeadKey, iv: &[u8]) -> Box { let mut pseudo_iv = Vec::with_capacity(iv.len() + ::EXPLICIT_NONCE_LEN); pseudo_iv.extend_from_slice(iv); pseudo_iv.extend_from_slice(&vec![0u8; ::EXPLICIT_NONCE_LEN]); - Box::new( - BoringAeadCrypter::::new( - Iv::copy(&pseudo_iv), - key.as_ref(), - ProtocolVersion::TLSv1_2, - ) - .expect("failed to create AEAD crypter"), - ) + match BoringAeadCrypter::::new( + Iv::copy(&pseudo_iv), + key.as_ref(), + ProtocolVersion::TLSv1_2, + ) { + Ok(crypter) => Box::new(crypter), + Err(err) => { + log_and_map( + "Tls12AeadAlgorithm::decrypter BoringAeadCrypter::new", + err, + (), + ); + Box::new(InvalidMessageDecrypter) + } + } } fn key_block_shape(&self) -> cipher::KeyBlockShape { @@ -399,7 +517,13 @@ impl cipher::Tls12AeadAlgorithm for Aead { let nonce = { let fixed_iv_len = ::FIXED_IV_LEN; let explicit_nonce_len = ::EXPLICIT_NONCE_LEN; - assert_eq!(explicit_nonce_len + fixed_iv_len, 12); + if explicit_nonce_len + fixed_iv_len != 12 { + return Err(cipher::UnsupportedOperationError); + } + + if iv.len() != fixed_iv_len || explicit.len() != explicit_nonce_len { + return Err(cipher::UnsupportedOperationError); + } // grab the IV by constructing a nonce, this is just an xor @@ -410,6 +534,10 @@ impl cipher::Tls12AeadAlgorithm for Aead { }; Ok(::extract_keys(key, Iv::copy(&nonce))) } + + fn fips(&self) -> bool { + cfg!(feature = "fips") && ::FIPS_APPROVED + } } struct QuicHeaderProtector { @@ -425,8 +553,8 @@ impl QuicHeaderProtector { first: &mut u8, packet_number: &mut [u8], remove: bool, - ) { - let mask = T::header_protection_mask(self.key.as_ref(), sample); + ) -> Result<(), rustls::Error> { + let mask = T::header_protection_mask(self.key.as_ref(), sample)?; const LONG_HEADER_FORMAT: u8 = 0x80; let bits_to_mask = if (*first & LONG_HEADER_FORMAT) == LONG_HEADER_FORMAT { @@ -453,6 +581,8 @@ impl QuicHeaderProtector { for (pn_byte, m) in packet_number.iter_mut().zip(&mask[1..]).take(pn_length) { *pn_byte ^= m; } + + Ok(()) } } @@ -468,7 +598,7 @@ impl rustls::quic::HeaderProtectionKey for QuicHeaderProtector return Err(rustls::Error::General("packet number too long".into())); } - self.rfc9001_header_protection(sample, first, packet_number, false); + self.rfc9001_header_protection(sample, first, packet_number, false)?; Ok(()) } @@ -483,7 +613,7 @@ impl rustls::quic::HeaderProtectionKey for QuicHeaderProtector return Err(rustls::Error::General("packet number too long".into())); } - self.rfc9001_header_protection(sample, first, packet_number, true); + self.rfc9001_header_protection(sample, first, packet_number, true)?; Ok(()) } @@ -498,10 +628,13 @@ where T: QuicCipher + BoringAead + 'static, { fn packet_key(&self, key: cipher::AeadKey, iv: Iv) -> Box { - Box::new( - BoringAeadCrypter::::new(iv, key.as_ref(), ProtocolVersion::TLSv1_3) - .expect("failed to create AEAD crypter"), - ) + match BoringAeadCrypter::::new(iv, key.as_ref(), ProtocolVersion::TLSv1_3) { + Ok(crypter) => Box::new(crypter), + Err(err) => { + log_and_map("QuicAlgorithm::packet_key BoringAeadCrypter::new", err, ()); + Box::new(InvalidPacketKey::(PhantomData)) + } + } } fn header_protection_key( @@ -517,6 +650,10 @@ where fn aead_key_len(&self) -> usize { ::KEY_SIZE } + + fn fips(&self) -> bool { + cfg!(feature = "fips") && ::FIPS_APPROVED + } } struct DecryptBufferAdapter<'a, 'p>(&'a mut BorrowedPayload<'p>); @@ -535,7 +672,7 @@ impl AsMut<[u8]> for DecryptBufferAdapter<'_, '_> { impl Buffer for DecryptBufferAdapter<'_, '_> { fn extend_from_slice(&mut self, _: &[u8]) -> aead::Result<()> { - unreachable!("not used by `AeadInPlace::decrypt_in_place`") + Err(aead::Error) } fn truncate(&mut self, len: usize) { @@ -571,13 +708,20 @@ impl Buffer for EncryptBufferAdapter<'_> { #[cfg(test)] mod tests { use hex_literal::hex; - use rustls::crypto::cipher::AeadKey; - use rustls::crypto::cipher::Iv; + use rustls::crypto::cipher::Tls13AeadAlgorithm; + use rustls::crypto::cipher::{AeadKey, InboundOpaqueMessage, Iv}; + #[cfg(feature = "tls12")] + use rustls::crypto::cipher::{MessageDecrypter, Tls12AeadAlgorithm}; + use rustls::quic::Algorithm as QuicAlgorithm; + use rustls::quic::HeaderProtectionKey; + use rustls::ContentType; + use rustls::ProtocolVersion; use crate::aead::BoringAeadCrypter; use rustls::quic::PacketKey; - use super::{chacha20::ChaCha20Poly1305, QuicHeaderProtector}; + use super::aes::Aes128; + use super::{chacha20::ChaCha20Poly1305, BoringCipher, QuicHeaderProtector}; #[test] fn quic_header_protection_short() { @@ -594,14 +738,97 @@ mod tests { phantom: std::marker::PhantomData::, }; - protector.rfc9001_header_protection(&sample, &mut first[0], packet_number, false); + protector + .rfc9001_header_protection(&sample, &mut first[0], packet_number, false) + .expect("valid sample should protect QUIC header"); assert_eq!(&header[..], &protected_header[..]); let (first, packet_number) = header.split_at_mut(1); - protector.rfc9001_header_protection(&sample, &mut first[0], packet_number, true); + protector + .rfc9001_header_protection(&sample, &mut first[0], packet_number, true) + .expect("valid sample should unprotect QUIC header"); assert_eq!(&header[..], &unprotected_header[..]); } + #[test] + fn quic_header_protection_rejects_invalid_sample_without_mutation() { + let hp_key = hex!("25a282b9e82f06f21f488917a4fc8f1b73573685608597d0efcb076b0ab7a7a4"); + let sample = hex!("5e5cd55c41f69080575d7999c25a5bfb"); + let mut header = hex!("4200bff4"); + let original = header; + let (first, packet_number) = header.split_at_mut(1); + + let protector = QuicHeaderProtector { + key: AeadKey::from(hp_key), + phantom: std::marker::PhantomData::, + }; + + let err = protector + .encrypt_in_place(&sample[..sample.len() - 1], &mut first[0], packet_number) + .expect_err("short sample must be rejected"); + + assert!(matches!(err, rustls::Error::General(_))); + assert_eq!(header, original); + } + + #[test] + fn tls13_decrypter_constructor_failure_returns_erroring_decrypter() { + let mut decrypter = Tls13AeadAlgorithm::decrypter( + &super::Aead::::DEFAULT, + AeadKey::from([0u8; 32]), + Iv::from([0u8; 12]), + ); + let mut payload = vec![0u8; 8]; + let msg = InboundOpaqueMessage::new( + ContentType::ApplicationData, + ProtocolVersion::TLSv1_3, + &mut payload, + ); + + let err = decrypter + .decrypt(msg, 0) + .expect_err("invalid constructor inputs should produce decrypt errors"); + + assert!(matches!(err, rustls::Error::DecryptError)); + } + + #[test] + fn quic_packet_key_constructor_failure_returns_erroring_key() { + let packet_key = super::Aead::::DEFAULT + .packet_key(AeadKey::from([0u8; 32]), Iv::from([0u8; 12])); + let mut payload = [0u8; 0]; + + let enc_err = packet_key.encrypt_in_place(0, &[], &mut payload); + assert!(matches!(enc_err, Err(rustls::Error::EncryptError))); + + let dec_err = packet_key + .decrypt_in_place(0, &[], &mut payload) + .expect_err("invalid constructor inputs should produce decrypt errors"); + assert!(matches!(dec_err, rustls::Error::DecryptError)); + } + + #[cfg(feature = "tls12")] + #[test] + fn tls12_decrypter_constructor_failure_returns_erroring_decrypter() { + let mut decrypter = Tls12AeadAlgorithm::decrypter( + &super::Aead::::DEFAULT, + AeadKey::from([0u8; 32]), + &[0u8; 4], + ); + let mut payload = vec![0u8; 8]; + let msg = InboundOpaqueMessage::new( + ContentType::ApplicationData, + ProtocolVersion::TLSv1_2, + &mut payload, + ); + + let err = decrypter + .decrypt(msg, 0) + .expect_err("invalid constructor inputs should produce decrypt errors"); + + assert!(matches!(err, rustls::Error::DecryptError)); + } + #[test] fn quic_chacha20_crypt() { // test vector from https://www.rfc-editor.org/rfc/rfc9001.html#name-chacha20-poly1305-short-hea @@ -616,7 +843,7 @@ mod tests { let protector = BoringAeadCrypter::::new( Iv::from(iv), &key, - rustls::ProtocolVersion::TLSv1_3, + ProtocolVersion::TLSv1_3, ) .unwrap(); @@ -635,4 +862,47 @@ mod tests { assert_eq!(cleartext, expected_cleartext); } + + #[test] + fn quic_decrypt_rejects_truncated_payload() { + let key = [0u8; ::KEY_SIZE]; + let iv = [0u8; 12]; + let crypter = BoringAeadCrypter::::new( + Iv::from(iv), + &key, + ProtocolVersion::TLSv1_3, + ) + .unwrap(); + + let mut payload = [0u8; ::TAG_LEN - 1]; + let err = crypter + .decrypt_in_place(0, &[], &mut payload) + .expect_err("truncated QUIC payload must fail decryption"); + + assert!(matches!(err, rustls::Error::DecryptError)); + } + + #[cfg(feature = "tls12")] + #[test] + fn tls12_decrypt_rejects_truncated_payload() { + let key = [0u8; ::KEY_SIZE]; + let iv = [0u8; 12]; + let mut decrypter = + BoringAeadCrypter::::new(Iv::from(iv), &key, ProtocolVersion::TLSv1_2).unwrap(); + + let min_payload_len = + ::EXPLICIT_NONCE_LEN + ::TAG_LEN; + let mut payload = vec![0u8; min_payload_len - 1]; + let msg = InboundOpaqueMessage::new( + ContentType::ApplicationData, + ProtocolVersion::TLSv1_2, + &mut payload, + ); + + let err = decrypter + .decrypt(msg, 0) + .expect_err("truncated TLS1.2 payload must fail decryption"); + + assert!(matches!(err, rustls::Error::DecryptError)); + } } diff --git a/boring-rustls-provider/src/aead/aes.rs b/boring-rustls-provider/src/aead/aes.rs index 03e0f3c..4fd591c 100644 --- a/boring-rustls-provider/src/aead/aes.rs +++ b/boring-rustls-provider/src/aead/aes.rs @@ -20,6 +20,7 @@ impl BoringCipher for Aes128 { const INTEGRITY_LIMIT: u64 = 1 << 52; const CONFIDENTIALITY_LIMIT: u64 = 1 << 23; + const FIPS_APPROVED: bool = true; fn new_cipher() -> Algorithm { Algorithm::aes_128_gcm() @@ -40,7 +41,7 @@ impl QuicCipher for Aes128 { const KEY_SIZE: usize = ::KEY_SIZE; const SAMPLE_LEN: usize = 16; - fn header_protection_mask(hp_key: &[u8], sample: &[u8]) -> [u8; 5] { + fn header_protection_mask(hp_key: &[u8], sample: &[u8]) -> Result<[u8; 5], rustls::Error> { quic_header_protection_mask::< { ::KEY_SIZE }, { ::SAMPLE_LEN }, @@ -65,6 +66,7 @@ impl BoringCipher for Aes256 { const INTEGRITY_LIMIT: u64 = 1 << 52; const CONFIDENTIALITY_LIMIT: u64 = 1 << 23; + const FIPS_APPROVED: bool = true; fn new_cipher() -> Algorithm { Algorithm::aes_256_gcm() @@ -85,7 +87,7 @@ impl QuicCipher for Aes256 { const KEY_SIZE: usize = ::KEY_SIZE; const SAMPLE_LEN: usize = 16; - fn header_protection_mask(hp_key: &[u8], sample: &[u8]) -> [u8; 5] { + fn header_protection_mask(hp_key: &[u8], sample: &[u8]) -> Result<[u8; 5], rustls::Error> { quic_header_protection_mask::< { ::KEY_SIZE }, { ::SAMPLE_LEN }, @@ -97,19 +99,32 @@ fn quic_header_protection_mask( cipher: boring::symm::Cipher, hp_key: &[u8], sample: &[u8], -) -> [u8; 5] { - assert!(hp_key.len() == KEY_SIZE); - assert!(sample.len() >= SAMPLE_LEN); +) -> Result<[u8; 5], rustls::Error> { + if hp_key.len() != KEY_SIZE { + return Err(rustls::Error::General( + "header protection key of invalid length".into(), + )); + } + if sample.len() != SAMPLE_LEN { + return Err(rustls::Error::General("sample of invalid length".into())); + } let mut output = [0u8; SAMPLE_LEN]; let mut crypter = boring::symm::Crypter::new(cipher, boring::symm::Mode::Encrypt, hp_key, None) - .expect("failed getting crypter"); + .map_err(|_| rustls::Error::General("failed generating header protection mask".into()))?; - let len = crypter.update(sample, &mut output).unwrap(); - let _ = len + crypter.finalize(&mut output[len..]).unwrap(); + let len = crypter + .update(sample, &mut output) + .map_err(|_| rustls::Error::General("failed generating header protection mask".into()))?; + let _total = len + + crypter.finalize(&mut output[len..]).map_err(|_| { + rustls::Error::General("failed generating header protection mask".into()) + })?; - output[..5].try_into().unwrap() + let mut mask = [0u8; 5]; + mask.copy_from_slice(&output[..5]); + Ok(mask) } #[cfg(test)] diff --git a/boring-rustls-provider/src/aead/chacha20.rs b/boring-rustls-provider/src/aead/chacha20.rs index c0af24c..142dd59 100644 --- a/boring-rustls-provider/src/aead/chacha20.rs +++ b/boring-rustls-provider/src/aead/chacha20.rs @@ -23,6 +23,7 @@ impl BoringCipher for ChaCha20Poly1305 { const INTEGRITY_LIMIT: u64 = 1 << 36; const CONFIDENTIALITY_LIMIT: u64 = u64::MAX; + const FIPS_APPROVED: bool = false; fn new_cipher() -> Algorithm { Algorithm::chacha20_poly1305() @@ -37,13 +38,21 @@ impl QuicCipher for ChaCha20Poly1305 { const KEY_SIZE: usize = 32; const SAMPLE_LEN: usize = 16; - fn header_protection_mask(hp_key: &[u8], sample: &[u8]) -> [u8; 5] { - assert!(hp_key.len() == ::KEY_SIZE); - assert!(sample.len() >= ::SAMPLE_LEN); + fn header_protection_mask(hp_key: &[u8], sample: &[u8]) -> Result<[u8; 5], rustls::Error> { + if hp_key.len() != ::KEY_SIZE { + return Err(rustls::Error::General( + "header protection key of invalid length".into(), + )); + } + if sample.len() != ::SAMPLE_LEN { + return Err(rustls::Error::General("sample of invalid length".into())); + } let mut mask = [0u8; 5]; // RFC9001 5.4.4: The first 4 bytes of the sampled ciphertext are the block counter. A ChaCha20 implementation could take a 32-bit integer in place of a byte sequence, in which case, the byte sequence is interpreted as a little-endian value. - let counter = u32::from_le_bytes(sample[0..4].try_into().unwrap()); + let mut counter_bytes = [0u8; 4]; + counter_bytes.copy_from_slice(&sample[0..4]); + let counter = u32::from_le_bytes(counter_bytes); // RFC9001 5.4.4: The remaining 12 bytes are used as the nonce. let nonce = &sample[4..16]; unsafe { @@ -56,7 +65,7 @@ impl QuicCipher for ChaCha20Poly1305 { counter, ); }; - mask + Ok(mask) } } @@ -95,7 +104,8 @@ mod tests { let sample = hex!("5e5cd55c41f69080575d7999c25a5bfb"); let hp_key = hex!("25a282b9e82f06f21f488917a4fc8f1b73573685608597d0efcb076b0ab7a7a4"); let expected_mask = hex!("aefefe7d03"); - let mask = ChaCha20Poly1305::header_protection_mask(&hp_key, &sample); + let mask = ChaCha20Poly1305::header_protection_mask(&hp_key, &sample) + .expect("valid QUIC sample/key should produce a mask"); assert_eq!(mask, expected_mask); } } diff --git a/boring-rustls-provider/src/hash.rs b/boring-rustls-provider/src/hash.rs index 44bad44..95ddc41 100644 --- a/boring-rustls-provider/src/hash.rs +++ b/boring-rustls-provider/src/hash.rs @@ -24,10 +24,14 @@ impl hash::Hash for Hash { boring::nid::Nid::SHA256 => hash::HashAlgorithm::SHA256, boring::nid::Nid::SHA384 => hash::HashAlgorithm::SHA384, boring::nid::Nid::SHA512 => hash::HashAlgorithm::SHA512, - _ => unimplemented!(), + _ => unreachable!("hash::Hash is only instantiated with SHA-2 digests"), } } + fn fips(&self) -> bool { + cfg!(feature = "fips") + } + fn output_len(&self) -> usize { MessageDigest::from_nid(self.0) .expect("failed getting digest") diff --git a/boring-rustls-provider/src/hkdf.rs b/boring-rustls-provider/src/hkdf.rs index cccaaf2..d4b070d 100644 --- a/boring-rustls-provider/src/hkdf.rs +++ b/boring-rustls-provider/src/hkdf.rs @@ -138,6 +138,10 @@ impl RustlsHkdf for Hkdf { } rustls::crypto::hmac::Tag::new(&hash[..hash_len as usize]) } + + fn fips(&self) -> bool { + cfg!(feature = "fips") + } } struct HkdfExpander { @@ -163,9 +167,17 @@ impl tls13::HkdfExpander for HkdfExpander { info: &[&[u8]], output: &mut [u8], ) -> Result<(), tls13::OutputLengthError> { + let max_output_len = self + .hash_len() + .checked_mul(255) + .ok_or(tls13::OutputLengthError)?; + if output.len() > max_output_len { + return Err(tls13::OutputLengthError); + } + let info_concat = info.concat(); unsafe { - boring_sys::HKDF_expand( + cvt(boring_sys::HKDF_expand( output.as_mut_ptr(), output.len(), self.digest.as_ptr(), @@ -173,8 +185,9 @@ impl tls13::HkdfExpander for HkdfExpander { self.prk_len, info_concat.as_ptr(), info_concat.len(), - ); - }; + )) + .map_err(|_| tls13::OutputLengthError)?; + } Ok(()) } @@ -201,3 +214,35 @@ impl tls13::HkdfExpander for HkdfExpander { self.digest.size() } } + +#[cfg(test)] +mod tests { + use boring::hash::MessageDigest; + use rustls::crypto::tls13::Hkdf as _; + + use super::{Hkdf, Sha256}; + + #[test] + fn expand_slice_rejects_output_larger_than_rfc_limit() { + let hkdf = Hkdf::::DEFAULT; + let expander = hkdf.extract_from_secret(None, b"ikm"); + let hash_len = MessageDigest::sha256().size(); + let mut output = vec![0u8; hash_len * 255 + 1]; + + assert!(expander.expand_slice(&[b"info"], &mut output).is_err()); + } + + #[test] + fn expand_slice_accepts_output_at_rfc_limit() { + let hkdf = Hkdf::::DEFAULT; + let expander = hkdf.extract_from_secret(None, b"ikm"); + let hash_len = MessageDigest::sha256().size(); + let mut output = vec![0u8; hash_len * 255]; + + expander + .expand_slice(&[b"info"], &mut output) + .expect("HKDF expand at RFC limit should succeed"); + + assert!(output.iter().any(|byte| *byte != 0)); + } +} diff --git a/boring-rustls-provider/src/hmac.rs b/boring-rustls-provider/src/hmac.rs index e0d5e49..fdcd508 100644 --- a/boring-rustls-provider/src/hmac.rs +++ b/boring-rustls-provider/src/hmac.rs @@ -20,16 +20,9 @@ struct BoringHmac(pub boring::nid::Nid); impl crypto::hmac::Hmac for BoringHmac { fn with_key(&self, key: &[u8]) -> Box { - let ctx = unsafe { - HmacCtx::from_ptr( - cvt_p(boring_sys::HMAC_CTX_new()).expect("failed getting hmac context"), - ) - }; - let md = MessageDigest::from_nid(self.0).expect("failed getting digest"); Box::new(BoringHmacKey { - ctx, md, key: Zeroizing::new(key.to_vec()), }) @@ -40,33 +33,35 @@ impl crypto::hmac::Hmac for BoringHmac { .expect("failed getting digest") .size() } + + fn fips(&self) -> bool { + cfg!(feature = "fips") + } } struct BoringHmacKey { - ctx: HmacCtx, md: MessageDigest, key: Zeroizing>, } impl BoringHmacKey { - fn init(&self) { + fn init(ctx: &HmacCtx, key: &[u8], md: MessageDigest) { unsafe { - // initialize a new hmac cvt(boring_sys::HMAC_Init_ex( - self.ctx.as_ptr(), - self.key.as_ptr() as *const c_void, - self.key.len(), - self.md.as_ptr(), + ctx.as_ptr(), + key.as_ptr() as *const c_void, + key.len(), + md.as_ptr(), ptr::null_mut(), )) } .expect("failed initializing hmac"); } - fn update(&self, bytes: &[u8]) { + fn update(ctx: &HmacCtx, bytes: &[u8]) { unsafe { cvt(boring_sys::HMAC_Update( - self.ctx.as_ptr(), + ctx.as_ptr(), bytes.as_ptr(), bytes.len(), )) @@ -74,11 +69,11 @@ impl BoringHmacKey { .expect("failed updating hmac"); } - fn finish(&self, out: &mut [u8]) -> usize { + fn finish(ctx: &HmacCtx, out: &mut [u8]) -> usize { let mut out_len = 0; unsafe { cvt(boring_sys::HMAC_Final( - self.ctx.as_ptr(), + ctx.as_ptr(), out.as_mut_ptr(), &mut out_len, )) @@ -86,21 +81,28 @@ impl BoringHmacKey { .expect("failed hmac final"); out_len as usize } + + fn new_ctx() -> HmacCtx { + unsafe { + HmacCtx::from_ptr(cvt_p(boring_sys::HMAC_CTX_new()).expect("failed creating HMAC_CTX")) + } + } } impl crypto::hmac::Key for BoringHmacKey { fn sign_concat(&self, first: &[u8], middle: &[&[u8]], last: &[u8]) -> crypto::hmac::Tag { - self.init(); + let ctx = Self::new_ctx(); + Self::init(&ctx, self.key.as_slice(), self.md); - self.update(first); + Self::update(&ctx, first); for m in middle { - self.update(m); + Self::update(&ctx, m); } - self.update(last); + Self::update(&ctx, last); let mut out = Zeroizing::new([0u8; boring_sys::EVP_MAX_MD_SIZE as usize]); - let out_len = self.finish(&mut out[..]); + let out_len = Self::finish(&ctx, &mut out[..]); crypto::hmac::Tag::new(&out[..out_len]) } diff --git a/boring-rustls-provider/src/kx/ex.rs b/boring-rustls-provider/src/kx/ex.rs index 5e80694..297844b 100644 --- a/boring-rustls-provider/src/kx/ex.rs +++ b/boring-rustls-provider/src/kx/ex.rs @@ -5,6 +5,7 @@ use boring::{ ec::{EcGroup, EcKey}, error::ErrorStack, nid::Nid, + pkey::Id, pkey::{PKey, PKeyRef, Private}, }; #[cfg(not(feature = "fips"))] @@ -12,7 +13,6 @@ use boring_additions::evp::EvpPkeyCtx; #[cfg(not(feature = "fips"))] use foreign_types::ForeignType; use rustls::crypto; -use spki::der::Decode; use crate::helper::log_and_map; #[cfg(not(feature = "fips"))] @@ -97,18 +97,24 @@ impl KeyExchange { /// Decodes a SPKI public key to it's raw public key component fn raw_public_key(pkey: &PKeyRef) -> Result, ErrorStack> { - let spki = pkey.public_key_to_der()?; + if pkey.id() == Id::EC { + let ec_key = pkey.ec_key()?; + let mut bn_ctx = boring::bn::BigNumContext::new()?; - // parse the key - let pkey = spki::SubjectPublicKeyInfoRef::from_der(spki.as_ref()) - .expect("failed parsing spki bytes"); + return ec_key.public_key().to_bytes( + ec_key.group(), + boring::ec::PointConversionForm::UNCOMPRESSED, + &mut bn_ctx, + ); + } - // return the raw public key as a new vec - Ok(Vec::from( - pkey.subject_public_key - .as_bytes() - .expect("failed getting raw spki bytes"), - )) + let mut output = vec![0u8; pkey.raw_public_key_len()?]; + let used_len = { + let used = pkey.raw_public_key(&mut output)?; + used.len() + }; + output.truncate(used_len); + Ok(output) } /// Derives a shared secret using the peer's raw public key diff --git a/boring-rustls-provider/src/kx/mod.rs b/boring-rustls-provider/src/kx/mod.rs index e38c295..44b524c 100644 --- a/boring-rustls-provider/src/kx/mod.rs +++ b/boring-rustls-provider/src/kx/mod.rs @@ -32,6 +32,10 @@ impl crypto::SupportedKxGroup for X25519 { fn name(&self) -> rustls::NamedGroup { rustls::NamedGroup::X25519 } + + fn fips(&self) -> bool { + false + } } /// A secp256r1-based key exchange @@ -48,6 +52,10 @@ impl crypto::SupportedKxGroup for Secp256r1 { fn name(&self) -> rustls::NamedGroup { rustls::NamedGroup::secp256r1 } + + fn fips(&self) -> bool { + cfg!(feature = "fips") + } } /// A secp384r1-based key exchange @@ -64,4 +72,8 @@ impl crypto::SupportedKxGroup for Secp384r1 { fn name(&self) -> rustls::NamedGroup { rustls::NamedGroup::secp384r1 } + + fn fips(&self) -> bool { + cfg!(feature = "fips") + } } diff --git a/boring-rustls-provider/src/lib.rs b/boring-rustls-provider/src/lib.rs index 6908297..e02da06 100644 --- a/boring-rustls-provider/src/lib.rs +++ b/boring-rustls-provider/src/lib.rs @@ -1,6 +1,8 @@ use std::sync::Arc; use helper::log_and_map; +#[cfg(all(feature = "fips", feature = "log"))] +use log::warn; use rustls::{ crypto::{CryptoProvider, GetRandomFailed, SupportedKxGroup}, SupportedCipherSuite, @@ -33,6 +35,25 @@ pub fn provider() -> CryptoProvider { } pub fn provider_with_ciphers(ciphers: Vec) -> CryptoProvider { + #[cfg(feature = "fips")] + let ciphers = { + let original_len = ciphers.len(); + let filtered = ciphers + .into_iter() + .filter(|suite| ALL_FIPS_CIPHER_SUITES.contains(suite)) + .collect::>(); + + if filtered.len() != original_len { + #[cfg(feature = "log")] + warn!( + "filtered {} non-FIPS cipher suite(s) from provider_with_ciphers", + original_len - filtered.len() + ); + } + + filtered + }; + CryptoProvider { cipher_suites: ciphers, #[cfg(feature = "fips")] @@ -55,6 +76,10 @@ impl rustls::crypto::SecureRandom for Provider { fn fill(&self, bytes: &mut [u8]) -> Result<(), rustls::crypto::GetRandomFailed> { boring::rand::rand_bytes(bytes).map_err(|e| log_and_map("rand_bytes", e, GetRandomFailed)) } + + fn fips(&self) -> bool { + cfg!(feature = "fips") + } } impl rustls::crypto::KeyProvider for Provider { @@ -64,6 +89,10 @@ impl rustls::crypto::KeyProvider for Provider { ) -> Result, rustls::Error> { sign::BoringPrivateKey::try_from(key_der).map(|x| Arc::new(x) as _) } + + fn fips(&self) -> bool { + cfg!(feature = "fips") + } } #[cfg(feature = "fips")] diff --git a/boring-rustls-provider/src/prf.rs b/boring-rustls-provider/src/prf.rs index ca174f5..ca5fd35 100644 --- a/boring-rustls-provider/src/prf.rs +++ b/boring-rustls-provider/src/prf.rs @@ -29,6 +29,10 @@ impl crypto::tls12::Prf for PrfTls1WithDigest { let digest = boring::hash::MessageDigest::from_nid(self.0).expect("failed getting digest"); prf(digest, output, secret, label, seed).expect("failed calculating prf") } + + fn fips(&self) -> bool { + cfg!(feature = "fips") + } } fn prf( diff --git a/boring-rustls-provider/src/sign.rs b/boring-rustls-provider/src/sign.rs index 41a2ead..819b49a 100644 --- a/boring-rustls-provider/src/sign.rs +++ b/boring-rustls-provider/src/sign.rs @@ -2,6 +2,7 @@ use std::sync::Arc; use boring::{ hash::MessageDigest, + nid::Nid, pkey::{Id, PKeyRef, Private}, rsa::Padding, sign::{RsaPssSaltlen, Signer}, @@ -20,16 +21,28 @@ const ALL_RSA_SCHEMES: &[SignatureScheme] = &[ SignatureScheme::RSA_PKCS1_SHA256, ]; -const ALL_EC_SCHEMES: &[SignatureScheme] = &[ - SignatureScheme::ECDSA_NISTP256_SHA256, - SignatureScheme::ECDSA_NISTP384_SHA384, - SignatureScheme::ECDSA_NISTP521_SHA512, -]; +const EC_P256_SCHEMES: &[SignatureScheme] = &[SignatureScheme::ECDSA_NISTP256_SHA256]; +const EC_P384_SCHEMES: &[SignatureScheme] = &[SignatureScheme::ECDSA_NISTP384_SHA384]; +const EC_P521_SCHEMES: &[SignatureScheme] = &[SignatureScheme::ECDSA_NISTP521_SHA512]; -/// An abstraction over a boringssl private key -/// used for signing +#[derive(Debug, Clone, Copy)] +enum EcCurve { + P256, + P384, + P521, +} + +#[derive(Debug, Clone, Copy)] +enum KeyKind { + Rsa, + Ec(EcCurve), + Ed25519, + Ed448, +} + +/// An abstraction over a boringssl private key used for signing. #[derive(Debug)] -pub struct BoringPrivateKey(Arc>, rustls::SignatureAlgorithm); +pub struct BoringPrivateKey(Arc>, KeyKind); impl TryFrom> for BoringPrivateKey { type Error = rustls::Error; @@ -37,25 +50,83 @@ impl TryFrom> for BoringPrivateKey { fn try_from(value: PrivateKeyDer<'static>) -> Result { let pkey = match value { PrivateKeyDer::Pkcs8(der) => { - boring::pkey::PKey::private_key_from_pkcs8(der.secret_pkcs8_der()) - .map_err(|e| log_and_map("private_key_from_pkcs8", e, ())) + boring::pkey::PKey::private_key_from_pkcs8(der.secret_pkcs8_der()).map_err(|e| { + log_and_map( + "private_key_from_pkcs8", + e, + rustls::Error::General("failed loading private key".into()), + ) + }) } PrivateKeyDer::Pkcs1(der) => { - boring::pkey::PKey::private_key_from_der(der.secret_pkcs1_der()) - .map_err(|e| log_and_map("private_key_from_der", e, ())) + boring::pkey::PKey::private_key_from_der(der.secret_pkcs1_der()).map_err(|e| { + log_and_map( + "private_key_from_der_pkcs1", + e, + rustls::Error::General("failed loading private key".into()), + ) + }) } - _ => Err(()), - } - .map_err(|_| rustls::Error::General("failed loading private key".into()))?; + PrivateKeyDer::Sec1(der) => { + boring::pkey::PKey::private_key_from_der(der.secret_sec1_der()).map_err(|e| { + log_and_map( + "private_key_from_der_sec1", + e, + rustls::Error::General("failed loading private key".into()), + ) + }) + } + _ => { + return Err(rustls::Error::General( + "unsupported private key encoding".into(), + )); + } + }?; - let sig = match pkey.id() { - Id::RSA => rustls::SignatureAlgorithm::RSA, - Id::EC => rustls::SignatureAlgorithm::ECDSA, - Id::ED25519 => rustls::SignatureAlgorithm::ED25519, - Id::ED448 => rustls::SignatureAlgorithm::ED448, + let kind = match pkey.id() { + Id::RSA => KeyKind::Rsa, + Id::EC => { + let ec_key = pkey.ec_key().map_err(|e| { + log_and_map( + "ec_key", + e, + rustls::Error::General("failed loading EC private key".into()), + ) + })?; + + let curve_nid = ec_key.group().curve_name().ok_or_else(|| { + rustls::Error::General("unsupported EC key without named curve".into()) + })?; + + let curve = match curve_nid { + Nid::X9_62_PRIME256V1 => EcCurve::P256, + Nid::SECP384R1 => EcCurve::P384, + Nid::SECP521R1 => EcCurve::P521, + _ => { + return Err(rustls::Error::General( + "unsupported EC private key curve".into(), + )); + } + }; + + KeyKind::Ec(curve) + } + Id::ED25519 => KeyKind::Ed25519, + Id::ED448 => KeyKind::Ed448, _ => return Err(rustls::Error::General("unsupported key format".into())), }; - Ok(Self(Arc::new(pkey), sig)) + + #[cfg(feature = "fips")] + match kind { + KeyKind::Rsa | KeyKind::Ec(EcCurve::P256 | EcCurve::P384) => {} + KeyKind::Ec(EcCurve::P521) | KeyKind::Ed25519 | KeyKind::Ed448 => { + return Err(rustls::Error::General( + "key type is not allowed in FIPS mode".into(), + )); + } + } + + Ok(Self(Arc::new(pkey), kind)) } } @@ -63,75 +134,105 @@ fn rsa_signer_from_params( key: &PKeyRef, digest: MessageDigest, padding: Padding, -) -> Signer<'_> { - let mut signer = Signer::new(digest, key).expect("failed getting signer"); - signer - .set_rsa_padding(padding) - .expect("failed setting padding"); +) -> Result, rustls::Error> { + let mut signer = Signer::new(digest, key).map_err(|e| { + log_and_map( + "Signer::new", + e, + rustls::Error::General("failed preparing signer".into()), + ) + })?; + + signer.set_rsa_padding(padding).map_err(|e| { + log_and_map( + "set_rsa_padding", + e, + rustls::Error::General("failed preparing signer".into()), + ) + })?; + if padding == Padding::PKCS1_PSS { signer .set_rsa_pss_saltlen(RsaPssSaltlen::DIGEST_LENGTH) - .expect("failed setting rsa_pss salt lengths"); - signer - .set_rsa_mgf1_md(digest) - .expect("failed setting mgf1 digest"); + .map_err(|e| { + log_and_map( + "set_rsa_pss_saltlen", + e, + rustls::Error::General("failed preparing signer".into()), + ) + })?; + + signer.set_rsa_mgf1_md(digest).map_err(|e| { + log_and_map( + "set_rsa_mgf1_md", + e, + rustls::Error::General("failed preparing signer".into()), + ) + })?; } - signer + Ok(signer) } -fn ec_signer_from_params(key: &PKeyRef, digest: MessageDigest) -> Signer<'_> { - let signer = Signer::new(digest, key).expect("failed getting signer"); - signer +fn ec_signer_from_params( + key: &PKeyRef, + digest: MessageDigest, +) -> Result, rustls::Error> { + Signer::new(digest, key).map_err(|e| { + log_and_map( + "Signer::new", + e, + rustls::Error::General("failed preparing signer".into()), + ) + }) } -impl BoringPrivateKey {} - impl SigningKey for BoringPrivateKey { fn choose_scheme( &self, offered: &[rustls::SignatureScheme], ) -> Option> { - match self.1 { - rustls::SignatureAlgorithm::RSA => ALL_RSA_SCHEMES + let scheme = match self.1 { + KeyKind::Rsa => ALL_RSA_SCHEMES .iter() - .find(|scheme| offered.contains(scheme)) - .map(|&scheme| Box::new(BoringSigner(self.0.clone(), scheme)) as _), - rustls::SignatureAlgorithm::ECDSA => ALL_EC_SCHEMES + .find(|scheme| offered.contains(scheme)), + KeyKind::Ec(EcCurve::P256) => EC_P256_SCHEMES .iter() - .find(|scheme| offered.contains(scheme)) - .map(|&scheme| Box::new(BoringSigner(self.0.clone(), scheme)) as _), - rustls::SignatureAlgorithm::ED25519 - if offered.contains(&rustls::SignatureScheme::ED25519) => - { - Some(Box::new(BoringSigner( - self.0.clone(), - rustls::SignatureScheme::ED25519, - ))) + .find(|scheme| offered.contains(scheme)), + KeyKind::Ec(EcCurve::P384) => EC_P384_SCHEMES + .iter() + .find(|scheme| offered.contains(scheme)), + KeyKind::Ec(EcCurve::P521) => EC_P521_SCHEMES + .iter() + .find(|scheme| offered.contains(scheme)), + KeyKind::Ed25519 if offered.contains(&rustls::SignatureScheme::ED25519) => { + Some(&rustls::SignatureScheme::ED25519) } - rustls::SignatureAlgorithm::ED448 - if offered.contains(&rustls::SignatureScheme::ED448) => - { - Some(Box::new(BoringSigner( - self.0.clone(), - rustls::SignatureScheme::ED448, - ))) + KeyKind::Ed448 if offered.contains(&rustls::SignatureScheme::ED448) => { + Some(&rustls::SignatureScheme::ED448) } _ => None, - } + }?; + + Some(Box::new(BoringSigner(self.0.clone(), *scheme))) } fn algorithm(&self) -> rustls::SignatureAlgorithm { - self.1 + match self.1 { + KeyKind::Rsa => rustls::SignatureAlgorithm::RSA, + KeyKind::Ec(_) => rustls::SignatureAlgorithm::ECDSA, + KeyKind::Ed25519 => rustls::SignatureAlgorithm::ED25519, + KeyKind::Ed448 => rustls::SignatureAlgorithm::ED448, + } } } -/// A boringssl-based Signer +/// A boringssl-based Signer. #[derive(Debug)] pub struct BoringSigner(Arc>, rustls::SignatureScheme); impl BoringSigner { - fn get_signer(&self) -> Signer<'_> { + fn get_signer(&self) -> Result, rustls::Error> { match self.1 { SignatureScheme::RSA_PKCS1_SHA256 => { rsa_signer_from_params(self.0.as_ref(), MessageDigest::sha256(), Padding::PKCS1) @@ -142,7 +243,6 @@ impl BoringSigner { SignatureScheme::RSA_PKCS1_SHA512 => { rsa_signer_from_params(self.0.as_ref(), MessageDigest::sha512(), Padding::PKCS1) } - SignatureScheme::RSA_PSS_SHA256 => { rsa_signer_from_params(self.0.as_ref(), MessageDigest::sha256(), Padding::PKCS1_PSS) } @@ -152,7 +252,6 @@ impl BoringSigner { SignatureScheme::RSA_PSS_SHA512 => { rsa_signer_from_params(self.0.as_ref(), MessageDigest::sha512(), Padding::PKCS1_PSS) } - SignatureScheme::ECDSA_NISTP256_SHA256 => { ec_signer_from_params(self.0.as_ref(), MessageDigest::sha256()) } @@ -162,19 +261,25 @@ impl BoringSigner { SignatureScheme::ECDSA_NISTP521_SHA512 => { ec_signer_from_params(self.0.as_ref(), MessageDigest::sha512()) } - SignatureScheme::ED25519 | SignatureScheme::ED448 => { - Signer::new_without_digest(self.0.as_ref()).expect("failed getting signer") + Signer::new_without_digest(self.0.as_ref()).map_err(|e| { + log_and_map( + "Signer::new_without_digest", + e, + rustls::Error::General("failed preparing signer".into()), + ) + }) } - - _ => unimplemented!(), + _ => Err(rustls::Error::General( + "unsupported signature scheme for private key".into(), + )), } } } impl rustls::sign::Signer for BoringSigner { fn sign(&self, message: &[u8]) -> Result, rustls::Error> { - let mut signer = self.get_signer(); + let mut signer = self.get_signer()?; let max_sig_len = signer .len() .map_err(|e| log_and_map("len", e, rustls::Error::General("failed signing".into())))?; @@ -195,3 +300,75 @@ impl rustls::sign::Signer for BoringSigner { self.1 } } + +#[cfg(test)] +mod tests { + use boring::{ + ec::{EcGroup, EcKey}, + nid::Nid, + pkey::{PKey, Private}, + rsa::Rsa, + }; + use rustls::sign::SigningKey; + use rustls::{SignatureAlgorithm, SignatureScheme}; + use rustls_pki_types::{PrivateKeyDer, PrivatePkcs8KeyDer, PrivateSec1KeyDer}; + + use super::BoringPrivateKey; + + fn p256_private_key() -> PKey { + let group = EcGroup::from_curve_name(Nid::X9_62_PRIME256V1).unwrap(); + let ec_key = EcKey::generate(&group).unwrap(); + PKey::from_ec_key(ec_key).unwrap() + } + + #[test] + fn loads_sec1_ec_private_key() { + let pkey = p256_private_key(); + let sec1_der = pkey.private_key_to_der().unwrap(); + let key_der = PrivateKeyDer::Sec1(PrivateSec1KeyDer::from(sec1_der)); + + let key = BoringPrivateKey::try_from(key_der).expect("SEC1 private key should load"); + + assert_eq!(key.algorithm(), SignatureAlgorithm::ECDSA); + } + + #[test] + fn p256_key_chooses_only_p256_scheme() { + let pkey = p256_private_key(); + let pkcs8_der = pkey.private_key_to_der_pkcs8().unwrap(); + let key_der = PrivateKeyDer::Pkcs8(PrivatePkcs8KeyDer::from(pkcs8_der)); + + let key = BoringPrivateKey::try_from(key_der).expect("P-256 private key should load"); + + let offered = [ + SignatureScheme::ECDSA_NISTP384_SHA384, + SignatureScheme::ECDSA_NISTP256_SHA256, + ]; + let signer = key + .choose_scheme(&offered) + .expect("P-256 key should select P-256 scheme"); + assert_eq!(signer.scheme(), SignatureScheme::ECDSA_NISTP256_SHA256); + + assert!(key + .choose_scheme(&[SignatureScheme::ECDSA_NISTP384_SHA384]) + .is_none()); + } + + #[test] + fn rsa_key_prefers_pss_when_available() { + let rsa = Rsa::generate(2048).unwrap(); + let pkey = PKey::from_rsa(rsa).unwrap(); + let pkcs8_der = pkey.private_key_to_der_pkcs8().unwrap(); + let key_der = PrivateKeyDer::Pkcs8(PrivatePkcs8KeyDer::from(pkcs8_der)); + + let key = BoringPrivateKey::try_from(key_der).expect("RSA private key should load"); + let signer = key + .choose_scheme(&[ + SignatureScheme::RSA_PKCS1_SHA256, + SignatureScheme::RSA_PSS_SHA256, + ]) + .expect("RSA key should select an offered scheme"); + + assert_eq!(signer.scheme(), SignatureScheme::RSA_PSS_SHA256); + } +} diff --git a/boring-rustls-provider/src/verify/ec.rs b/boring-rustls-provider/src/verify/ec.rs index f8984e3..34c8a3d 100644 --- a/boring-rustls-provider/src/verify/ec.rs +++ b/boring-rustls-provider/src/verify/ec.rs @@ -40,7 +40,7 @@ impl SignatureVerificationAlgorithm for BoringEcVerifier { ec_verifier_from_params(public_key.as_ref(), MessageDigest::sha512()) } - _ => unimplemented!(), + _ => return Err(InvalidSignature), } .map_err(|e| helper::log_and_map("ec_verifier_from_params", e, InvalidSignature))?; @@ -61,7 +61,7 @@ impl SignatureVerificationAlgorithm for BoringEcVerifier { 0x04, 0x00, 0x23, ]) } - _ => unimplemented!(), + _ => unreachable!("BoringEcVerifier only supports configured ECDSA schemes"), } } @@ -75,9 +75,13 @@ impl SignatureVerificationAlgorithm for BoringEcVerifier { 0x06, 0x08, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x04, 0x03, 0x04, ]) } - _ => unimplemented!(), + _ => unreachable!("BoringEcVerifier only supports configured ECDSA schemes"), } } + + fn fips(&self) -> bool { + cfg!(feature = "fips") && !matches!(self.0, SignatureScheme::ECDSA_NISTP521_SHA512) + } } fn ec_verifier_from_params( @@ -92,7 +96,7 @@ fn group_for_scheme(scheme: SignatureScheme) -> Result boring::nid::Nid::X9_62_PRIME256V1, SignatureScheme::ECDSA_NISTP384_SHA384 => boring::nid::Nid::SECP384R1, SignatureScheme::ECDSA_NISTP521_SHA512 => boring::nid::Nid::SECP521R1, - _ => unimplemented!(), + _ => unreachable!("BoringEcVerifier only supports configured ECDSA schemes"), }; boring::ec::EcGroup::from_curve_name(nid) } diff --git a/boring-rustls-provider/src/verify/ed.rs b/boring-rustls-provider/src/verify/ed.rs index e7cb81e..3eeeb26 100644 --- a/boring-rustls-provider/src/verify/ed.rs +++ b/boring-rustls-provider/src/verify/ed.rs @@ -46,9 +46,13 @@ impl SignatureVerificationAlgorithm for BoringEdVerifier { // rfc8410#section-3: 1.3.101.113: -> DER: 06 03 2B 65 71 rustls_pki_types::AlgorithmIdentifier::from_slice(&[0x06, 0x03, 0x2B, 0x65, 0x71]) } - _ => unimplemented!(), + _ => unreachable!("BoringEdVerifier only supports configured EdDSA schemes"), } } + + fn fips(&self) -> bool { + false + } } fn ed_verifier_from_params( @@ -64,7 +68,7 @@ fn ed_public_key_for_scheme( let nid = boring::nid::Nid::from_raw(match scheme { SignatureScheme::ED25519 => boring_sys::EVP_PKEY_ED25519, SignatureScheme::ED448 => boring_sys::EVP_PKEY_ED448, - _ => unimplemented!(), + _ => return Err(ErrorStack::get()), }); public_key(spki_spk, nid) } diff --git a/boring-rustls-provider/src/verify/rsa.rs b/boring-rustls-provider/src/verify/rsa.rs index 5691369..e41d494 100644 --- a/boring-rustls-provider/src/verify/rsa.rs +++ b/boring-rustls-provider/src/verify/rsa.rs @@ -64,8 +64,8 @@ impl SignatureVerificationAlgorithm for BoringRsaVerifier { Padding::PKCS1_PSS, ), - _ => unimplemented!(), - }; + _ => return Err(InvalidSignature), + }?; verifier.verify_oneshot(signature, message).map_or_else( |_| Err(InvalidSignature), |res| if res { Ok(()) } else { Err(InvalidSignature) }, @@ -86,30 +86,35 @@ impl SignatureVerificationAlgorithm for BoringRsaVerifier { SignatureScheme::RSA_PSS_SHA384 => alg_id::RSA_PSS_SHA384, SignatureScheme::RSA_PSS_SHA512 => alg_id::RSA_PSS_SHA512, - _ => unimplemented!(), + _ => unreachable!("BoringRsaVerifier only supports configured RSA schemes"), } } + + fn fips(&self) -> bool { + cfg!(feature = "fips") + } } fn rsa_verifier_from_params( key: &boring::pkey::PKeyRef, digest: MessageDigest, padding: Padding, -) -> boring::sign::Verifier<'_> { - let mut verifier = boring::sign::Verifier::new(digest, key).expect("failed getting verifier"); +) -> Result, InvalidSignature> { + let mut verifier = boring::sign::Verifier::new(digest, key) + .map_err(|e| log_and_map("Verifier::new", e, InvalidSignature))?; verifier .set_rsa_padding(padding) - .expect("failed setting padding"); + .map_err(|e| log_and_map("set_rsa_padding", e, InvalidSignature))?; if padding == Padding::PKCS1_PSS { verifier .set_rsa_pss_saltlen(RsaPssSaltlen::DIGEST_LENGTH) - .expect("failed setting rsa_pss salt lengths"); + .map_err(|e| log_and_map("set_rsa_pss_saltlen", e, InvalidSignature))?; verifier .set_rsa_mgf1_md(digest) - .expect("failed setting mgf1 digest"); + .map_err(|e| log_and_map("set_rsa_mgf1_md", e, InvalidSignature))?; } - verifier + Ok(verifier) } pub(crate) fn decode_spki_spk( diff --git a/boring-rustls-provider/tests/e2e.rs b/boring-rustls-provider/tests/e2e.rs index fa5bd5a..c53616e 100644 --- a/boring-rustls-provider/tests/e2e.rs +++ b/boring-rustls-provider/tests/e2e.rs @@ -5,8 +5,6 @@ use tokio::{ net::TcpStream, }; -#[cfg(feature = "tls12")] -use boring_rustls_provider::tls12; use boring_rustls_provider::tls13; #[cfg(feature = "tls12")] use rustls::version::TLS12; @@ -15,6 +13,63 @@ use rustls_pki_types::{CertificateDer, PrivateKeyDer, PrivatePkcs8KeyDer}; use tokio::net::TcpListener; use tokio_rustls::{TlsAcceptor, TlsConnector}; +fn tls13_provider_suites() -> Vec { + boring_rustls_provider::provider() + .cipher_suites + .into_iter() + .filter(|suite| matches!(suite, SupportedCipherSuite::Tls13(_))) + .collect() +} + +#[cfg(feature = "tls12")] +fn tls12_provider_suites_for_ecdsa() -> Vec { + boring_rustls_provider::provider() + .cipher_suites + .into_iter() + .filter(|suite| { + let SupportedCipherSuite::Tls12(suite) = suite else { + return false; + }; + + suite.sign.iter().any(|scheme| { + matches!( + scheme, + rustls::SignatureScheme::ECDSA_NISTP256_SHA256 + | rustls::SignatureScheme::ECDSA_NISTP384_SHA384 + | rustls::SignatureScheme::ECDSA_NISTP521_SHA512 + | rustls::SignatureScheme::ED25519 + | rustls::SignatureScheme::ED448 + ) + }) + }) + .collect() +} + +#[cfg(feature = "tls12")] +fn tls12_provider_suites_for_rsa() -> Vec { + boring_rustls_provider::provider() + .cipher_suites + .into_iter() + .filter(|suite| { + let SupportedCipherSuite::Tls12(suite) = suite else { + return false; + }; + + suite.sign.iter().any(|scheme| { + matches!( + scheme, + rustls::SignatureScheme::RSA_PKCS1_SHA256 + | rustls::SignatureScheme::RSA_PKCS1_SHA384 + | rustls::SignatureScheme::RSA_PKCS1_SHA512 + | rustls::SignatureScheme::RSA_PSS_SHA256 + | rustls::SignatureScheme::RSA_PSS_SHA384 + | rustls::SignatureScheme::RSA_PSS_SHA512 + ) + }) + }) + .collect() +} + #[tokio::test] async fn test_tls13_crypto() { let pki = TestPki::new(&rcgen::PKCS_ECDSA_P256_SHA256); @@ -22,17 +77,7 @@ async fn test_tls13_crypto() { let root_store = pki.client_root_store(); let server_config = pki.server_config(); - #[cfg(feature = "fips")] - let ciphers = vec![ - SupportedCipherSuite::Tls13(&tls13::AES_128_GCM_SHA256), - SupportedCipherSuite::Tls13(&tls13::AES_256_GCM_SHA384), - ]; - #[cfg(not(feature = "fips"))] - let ciphers = vec![ - SupportedCipherSuite::Tls13(&tls13::AES_128_GCM_SHA256), - SupportedCipherSuite::Tls13(&tls13::AES_256_GCM_SHA384), - SupportedCipherSuite::Tls13(&tls13::CHACHA20_POLY1305_SHA256), - ]; + let ciphers = tls13_provider_suites(); for cipher in ciphers { let config = ClientConfig::builder_with_provider(Arc::new( @@ -47,6 +92,43 @@ async fn test_tls13_crypto() { } } +#[test] +fn provider_kx_groups_reject_invalid_peer_keys_without_panicking() { + for group in boring_rustls_provider::provider().kx_groups { + let kx = group.start().expect("provider KX group should initialize"); + + let outcome = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| kx.complete(&[]))); + assert!(outcome.is_ok(), "KX group {:?} panicked", group.name()); + assert!( + outcome.expect("already checked for panic").is_err(), + "KX group {:?} accepted an invalid key share", + group.name() + ); + } +} + +#[test] +fn provider_verifiers_reject_malformed_inputs_without_panicking() { + let provider = boring_rustls_provider::provider(); + + for (index, verifier) in provider + .signature_verification_algorithms + .all + .iter() + .enumerate() + { + let outcome = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + verifier.verify_signature(&[], b"message", &[]) + })); + + assert!(outcome.is_ok(), "verifier #{index} panicked"); + assert!( + outcome.expect("already checked for panic").is_err(), + "verifier #{index} accepted malformed inputs" + ); + } +} + /// Self-to-self TLS 1.3 handshake using only the X25519MLKEM768 PQ hybrid group. #[cfg(feature = "mlkem")] #[tokio::test] @@ -227,6 +309,25 @@ fn fips_provider_excludes_chacha20_cipher_suites() { } } +#[test] +#[cfg(feature = "fips")] +fn fips_provider_with_ciphers_filters_non_fips_input() { + use rustls::CipherSuite; + + let provider = boring_rustls_provider::provider_with_ciphers(vec![ + SupportedCipherSuite::Tls13(&tls13::CHACHA20_POLY1305_SHA256), + SupportedCipherSuite::Tls13(&tls13::AES_128_GCM_SHA256), + ]); + + let suites = provider + .cipher_suites + .iter() + .map(|suite| suite.suite()) + .collect::>(); + + assert_eq!(suites, vec![CipherSuite::TLS13_AES_128_GCM_SHA256]); +} + #[test] #[cfg(feature = "fips")] fn fips_provider_restricts_kx_groups() { @@ -310,6 +411,40 @@ fn non_fips_provider_keeps_non_fips_algorithms() { .any(|group| group.name() == NamedGroup::X25519)); } +#[test] +#[cfg(not(feature = "fips"))] +fn non_fips_provider_components_report_non_fips() { + let provider = boring_rustls_provider::provider(); + + assert!(!provider.secure_random.fips()); + assert!(!provider.key_provider.fips()); +} + +#[test] +#[cfg(not(feature = "fips"))] +fn non_fips_provider_with_ciphers_keeps_requested_suites() { + use rustls::CipherSuite; + + let provider = boring_rustls_provider::provider_with_ciphers(vec![ + SupportedCipherSuite::Tls13(&tls13::CHACHA20_POLY1305_SHA256), + SupportedCipherSuite::Tls13(&tls13::AES_128_GCM_SHA256), + ]); + + let suites = provider + .cipher_suites + .iter() + .map(|suite| suite.suite()) + .collect::>(); + + assert_eq!( + suites, + vec![ + CipherSuite::TLS13_CHACHA20_POLY1305_SHA256, + CipherSuite::TLS13_AES_128_GCM_SHA256, + ] + ); +} + #[test] #[cfg(all(not(feature = "fips"), feature = "mlkem"))] fn non_fips_provider_includes_pq_group() { @@ -347,17 +482,7 @@ async fn test_tls12_ec_crypto() { let root_store = pki.client_root_store(); let server_config = pki.server_config(); - #[cfg(feature = "fips")] - let ciphers = vec![ - SupportedCipherSuite::Tls12(&tls12::ECDHE_ECDSA_AES128_GCM_SHA256), - SupportedCipherSuite::Tls12(&tls12::ECDHE_ECDSA_AES256_GCM_SHA384), - ]; - #[cfg(not(feature = "fips"))] - let ciphers = vec![ - SupportedCipherSuite::Tls12(&tls12::ECDHE_ECDSA_AES128_GCM_SHA256), - SupportedCipherSuite::Tls12(&tls12::ECDHE_ECDSA_AES256_GCM_SHA384), - SupportedCipherSuite::Tls12(&tls12::ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256), - ]; + let ciphers = tls12_provider_suites_for_ecdsa(); for cipher in ciphers { let config = ClientConfig::builder_with_provider(Arc::new( @@ -380,17 +505,7 @@ async fn test_tls12_rsa_crypto() { let root_store = pki.client_root_store(); let server_config = pki.server_config(); - #[cfg(feature = "fips")] - let ciphers = vec![ - SupportedCipherSuite::Tls12(&tls12::ECDHE_RSA_AES128_GCM_SHA256), - SupportedCipherSuite::Tls12(&tls12::ECDHE_RSA_AES256_GCM_SHA384), - ]; - #[cfg(not(feature = "fips"))] - let ciphers = vec![ - SupportedCipherSuite::Tls12(&tls12::ECDHE_RSA_AES128_GCM_SHA256), - SupportedCipherSuite::Tls12(&tls12::ECDHE_RSA_AES256_GCM_SHA384), - SupportedCipherSuite::Tls12(&tls12::ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256), - ]; + let ciphers = tls12_provider_suites_for_rsa(); for cipher in ciphers { let config = ClientConfig::builder_with_provider(Arc::new( diff --git a/boring-rustls-provider/tests/panic_surface.rs b/boring-rustls-provider/tests/panic_surface.rs new file mode 100644 index 0000000..3a960f4 --- /dev/null +++ b/boring-rustls-provider/tests/panic_surface.rs @@ -0,0 +1,244 @@ +use std::fs; +use std::path::{Path, PathBuf}; + +const BANNED_TOKENS: &[&str] = &[ + "unwrap(", + "expect(", + "assert!(", + "assert_eq!(", + "assert_ne!(", + "panic!(", + "unreachable!(", + "unimplemented!(", +]; + +struct AllowlistedPanic { + path_suffix: &'static str, + line_fragment: &'static str, + reason: &'static str, +} + +const ALLOWLIST: &[AllowlistedPanic] = &[ + AllowlistedPanic { + path_suffix: "boring-rustls-provider/src/hash.rs", + line_fragment: "failed getting hash digest", + reason: "rustls hash trait is infallible; provider currently cannot surface this as Result", + }, + AllowlistedPanic { + path_suffix: "boring-rustls-provider/src/hash.rs", + line_fragment: "failed getting hasher", + reason: "rustls hash trait is infallible; provider currently cannot surface this as Result", + }, + AllowlistedPanic { + path_suffix: "boring-rustls-provider/src/hash.rs", + line_fragment: "hash::Hash is only instantiated with SHA-2 digests", + reason: "constructor invariant over static hash-provider constants", + }, + AllowlistedPanic { + path_suffix: "boring-rustls-provider/src/hash.rs", + line_fragment: "failed getting digest", + reason: "rustls hash trait is infallible; provider currently cannot surface this as Result", + }, + AllowlistedPanic { + path_suffix: "boring-rustls-provider/src/hash.rs", + line_fragment: "failed finishing hash", + reason: "rustls hash trait is infallible; provider currently cannot surface this as Result", + }, + AllowlistedPanic { + path_suffix: "boring-rustls-provider/src/hash.rs", + line_fragment: "failed adding data to hash", + reason: "rustls hash trait is infallible; provider currently cannot surface this as Result", + }, + AllowlistedPanic { + path_suffix: "boring-rustls-provider/src/hkdf.rs", + line_fragment: "HKDF_extract failed", + reason: "rustls hkdf trait is infallible at this call site", + }, + AllowlistedPanic { + path_suffix: "boring-rustls-provider/src/hkdf.rs", + line_fragment: "HMAC failed", + reason: "rustls hkdf trait is infallible at this call site", + }, + AllowlistedPanic { + path_suffix: "boring-rustls-provider/src/hkdf.rs", + line_fragment: "failed hkdf expand", + reason: "expand_block API is infallible in rustls", + }, + AllowlistedPanic { + path_suffix: "boring-rustls-provider/src/hmac.rs", + line_fragment: "failed getting digest", + reason: "rustls hmac trait is infallible at this call site", + }, + AllowlistedPanic { + path_suffix: "boring-rustls-provider/src/hmac.rs", + line_fragment: "failed initializing hmac", + reason: "rustls hmac trait is infallible at this call site", + }, + AllowlistedPanic { + path_suffix: "boring-rustls-provider/src/hmac.rs", + line_fragment: "failed updating hmac", + reason: "rustls hmac trait is infallible at this call site", + }, + AllowlistedPanic { + path_suffix: "boring-rustls-provider/src/hmac.rs", + line_fragment: "failed hmac final", + reason: "rustls hmac trait is infallible at this call site", + }, + AllowlistedPanic { + path_suffix: "boring-rustls-provider/src/hmac.rs", + line_fragment: "failed creating HMAC_CTX", + reason: "rustls hmac trait is infallible at this call site", + }, + AllowlistedPanic { + path_suffix: "boring-rustls-provider/src/prf.rs", + line_fragment: "failed getting digest", + reason: "rustls tls12::Prf::for_secret is infallible", + }, + AllowlistedPanic { + path_suffix: "boring-rustls-provider/src/prf.rs", + line_fragment: "failed calculating prf", + reason: "rustls tls12::Prf::for_secret is infallible", + }, + AllowlistedPanic { + path_suffix: "boring-rustls-provider/src/verify/rsa.rs", + line_fragment: "BoringRsaVerifier only supports configured RSA schemes", + reason: "static verifier configuration invariant", + }, + AllowlistedPanic { + path_suffix: "boring-rustls-provider/src/verify/ec.rs", + line_fragment: "BoringEcVerifier only supports configured ECDSA schemes", + reason: "static verifier configuration invariant", + }, + AllowlistedPanic { + path_suffix: "boring-rustls-provider/src/verify/ed.rs", + line_fragment: "BoringEdVerifier only supports configured EdDSA schemes", + reason: "static verifier configuration invariant", + }, + AllowlistedPanic { + path_suffix: "boring-rustls-provider/src/kx/ex.rs", + line_fragment: "unsupported key type", + reason: "static KX type invariant", + }, +]; + +#[test] +fn no_unreviewed_runtime_panic_constructs() { + let manifest_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + let repo_root = manifest_dir + .parent() + .expect("crate must be within repository root"); + + let mut violations = Vec::new(); + for root in [ + repo_root.join("boring-rustls-provider/src"), + repo_root.join("boring-additions/src"), + ] { + collect_rs_files(&root) + .expect("must be able to enumerate source files") + .into_iter() + .for_each(|path| { + let rel = path + .strip_prefix(repo_root) + .expect("path should be under repo root") + .to_string_lossy() + .to_string(); + + let content = fs::read_to_string(&path) + .unwrap_or_else(|e| panic!("failed to read {}: {e}", rel)); + + for (line_no, line) in runtime_lines_only(&content) { + let trimmed = line.trim(); + if trimmed.starts_with("//") { + continue; + } + + for token in BANNED_TOKENS { + if !line.contains(token) { + continue; + } + + let allowed = ALLOWLIST.iter().find(|allow| { + rel.ends_with(allow.path_suffix) && line.contains(allow.line_fragment) + }); + if allowed.is_none() { + violations.push(format!("{rel}:{line_no}: {trimmed}")); + } + } + } + }); + } + + if !violations.is_empty() { + violations.sort(); + panic!( + "found unreviewed panic constructs in runtime code:\n{}\n\nIf intentional, add a targeted allowlist entry with rationale.", + violations.join("\n") + ); + } +} + +#[test] +fn allowlist_entries_have_matching_runtime_lines() { + let manifest_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + let repo_root = manifest_dir + .parent() + .expect("crate must be within repository root"); + + let mut missing = Vec::new(); + for entry in ALLOWLIST { + let path = repo_root.join(entry.path_suffix); + let content = fs::read_to_string(&path) + .unwrap_or_else(|e| panic!("failed to read {}: {e}", entry.path_suffix)); + let found = runtime_lines_only(&content) + .into_iter() + .any(|(_, line)| line.contains(entry.line_fragment)); + if !found { + missing.push(format!( + "{} :: '{}' ({})", + entry.path_suffix, entry.line_fragment, entry.reason + )); + } + } + + if !missing.is_empty() { + missing.sort(); + panic!( + "panic allowlist entries no longer match runtime code:\n{}", + missing.join("\n") + ); + } +} + +fn runtime_lines_only(content: &str) -> Vec<(usize, &str)> { + let mut lines = Vec::new(); + for (index, line) in content.lines().enumerate() { + if line.trim_start().starts_with("#[cfg(test)]") { + break; + } + lines.push((index + 1, line)); + } + lines +} + +fn collect_rs_files(root: &Path) -> Result, std::io::Error> { + let mut files = Vec::new(); + collect_rs_files_rec(root, &mut files)?; + Ok(files) +} + +fn collect_rs_files_rec(root: &Path, acc: &mut Vec) -> Result<(), std::io::Error> { + for entry in fs::read_dir(root)? { + let entry = entry?; + let path = entry.path(); + if path.is_dir() { + collect_rs_files_rec(&path, acc)?; + continue; + } + + if path.extension().and_then(|ext| ext.to_str()) == Some("rs") { + acc.push(path); + } + } + + Ok(()) +} diff --git a/examples/src/bin/client.rs b/examples/src/bin/client.rs index f1780c1..bbbb32b 100644 --- a/examples/src/bin/client.rs +++ b/examples/src/bin/client.rs @@ -2,7 +2,7 @@ use std::io::{stdout, Read, Write}; use std::net::TcpStream; use std::sync::Arc; -fn main() { +fn main() -> Result<(), Box> { env_logger::init(); let mut root_store = rustls::RootCertStore::empty(); @@ -11,13 +11,15 @@ fn main() { let config = rustls::ClientConfig::builder_with_provider(boring_rustls_provider::provider().into()) .with_safe_default_protocol_versions() - .unwrap() + .map_err(|_| std::io::Error::other("failed selecting protocol versions"))? .with_root_certificates(root_store) .with_no_client_auth(); - let server_name = "www.rust-lang.org".try_into().unwrap(); - let mut conn = rustls::ClientConnection::new(Arc::new(config), server_name).unwrap(); - let mut sock = TcpStream::connect("www.rust-lang.org:443").unwrap(); + let server_name = "www.rust-lang.org" + .try_into() + .map_err(|_| std::io::Error::other("invalid server name"))?; + let mut conn = rustls::ClientConnection::new(Arc::new(config), server_name)?; + let mut sock = TcpStream::connect("www.rust-lang.org:443")?; let mut tls = rustls::Stream::new(&mut conn, &mut sock); tls.write_all( concat!( @@ -28,16 +30,19 @@ fn main() { "\r\n" ) .as_bytes(), - ) - .unwrap(); - let ciphersuite = tls.conn.negotiated_cipher_suite().unwrap(); + )?; + let ciphersuite = tls + .conn + .negotiated_cipher_suite() + .ok_or_else(|| std::io::Error::other("no negotiated ciphersuite"))?; writeln!( &mut std::io::stderr(), "Current ciphersuite: {:?}", ciphersuite.suite() - ) - .unwrap(); + )?; let mut plaintext = Vec::new(); - tls.read_to_end(&mut plaintext).unwrap(); - stdout().write_all(&plaintext).unwrap(); + tls.read_to_end(&mut plaintext)?; + stdout().write_all(&plaintext)?; + + Ok(()) }