From dfda8469897cf6250042f554b2c2f97718c904f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20R=C3=BCth?= Date: Fri, 24 Nov 2023 12:55:57 +0100 Subject: [PATCH] Rework KeyExchange types --- boring-rustls-provider/src/helper.rs | 12 ++- boring-rustls-provider/src/kx.rs | 54 +++++--------- boring-rustls-provider/src/kx/dh.rs | 42 ++++++++--- boring-rustls-provider/src/kx/ex.rs | 97 ++++++++++++++++--------- boring-rustls-provider/src/sign.rs | 47 ++++++------ boring-rustls-provider/src/verify/ec.rs | 19 +++-- boring-rustls-provider/src/verify/ed.rs | 11 +-- 7 files changed, 156 insertions(+), 126 deletions(-) diff --git a/boring-rustls-provider/src/helper.rs b/boring-rustls-provider/src/helper.rs index 0fbf942..1f15c02 100644 --- a/boring-rustls-provider/src/helper.rs +++ b/boring-rustls-provider/src/helper.rs @@ -30,13 +30,17 @@ pub(crate) fn cvt(r: c_int) -> Result { } } -#[cfg(feature = "log")] pub(crate) fn error_stack_to_aead_error(func: &'static str, e: ErrorStack) -> aead::Error { + map_error_stack(func, e, aead::Error) +} + +#[cfg(feature = "log")] +pub(crate) fn map_error_stack(func: &'static str, e: ErrorStack, mapped: T) -> T { trace!("failed {}, error: {}", func, e); - aead::Error + mapped } #[cfg(not(feature = "log"))] -pub(crate) fn error_stack_to_aead_error(_: &'static str, _: ErrorStack) -> aead::Error { - aead::Error +pub(crate) fn map_error_stack(func: &'static str, e: ErrorStack, mapped: T) -> T { + mapped } diff --git a/boring-rustls-provider/src/kx.rs b/boring-rustls-provider/src/kx.rs index 3595b36..16ab2be 100644 --- a/boring-rustls-provider/src/kx.rs +++ b/boring-rustls-provider/src/kx.rs @@ -1,5 +1,7 @@ use rustls::crypto::{self, ActiveKeyExchange}; +use crate::helper::map_error_stack; + mod dh; mod ex; @@ -14,9 +16,9 @@ pub struct X25519; impl crypto::SupportedKxGroup for X25519 { fn start(&self) -> Result, rustls::Error> { - Ok(Box::new( - ex::ExKeyExchange::with_x25519().map_err(|_| crypto::GetRandomFailed)?, - )) + Ok(Box::new(ex::ExKeyExchange::with_x25519().map_err(|e| { + map_error_stack("X25519.start", e, crypto::GetRandomFailed) + })?)) } fn name(&self) -> rustls::NamedGroup { @@ -29,9 +31,9 @@ pub struct X448; impl crypto::SupportedKxGroup for X448 { fn start(&self) -> Result, rustls::Error> { - Ok(Box::new( - ex::ExKeyExchange::with_x448().map_err(|_| crypto::GetRandomFailed)?, - )) + Ok(Box::new(ex::ExKeyExchange::with_x448().map_err(|e| { + map_error_stack("X448.start", e, crypto::GetRandomFailed) + })?)) } fn name(&self) -> rustls::NamedGroup { @@ -44,9 +46,9 @@ pub struct Secp256r1; impl crypto::SupportedKxGroup for Secp256r1 { fn start(&self) -> Result, rustls::Error> { - Ok(Box::new( - ex::ExKeyExchange::with_secp256r1().map_err(|_| crypto::GetRandomFailed)?, - )) + Ok(Box::new(ex::ExKeyExchange::with_secp256r1().map_err( + |e| map_error_stack("Secp256r1.start", e, crypto::GetRandomFailed), + )?)) } fn name(&self) -> rustls::NamedGroup { @@ -59,9 +61,9 @@ pub struct Secp384r1; impl crypto::SupportedKxGroup for Secp384r1 { fn start(&self) -> Result, rustls::Error> { - Ok(Box::new( - ex::ExKeyExchange::with_secp384r1().map_err(|_| crypto::GetRandomFailed)?, - )) + Ok(Box::new(ex::ExKeyExchange::with_secp384r1().map_err( + |e| map_error_stack("Secp384r1.start", e, crypto::GetRandomFailed), + )?)) } fn name(&self) -> rustls::NamedGroup { @@ -74,9 +76,9 @@ pub struct Secp521r1; impl crypto::SupportedKxGroup for Secp521r1 { fn start(&self) -> Result, rustls::Error> { - Ok(Box::new( - ex::ExKeyExchange::with_secp521r1().map_err(|_| crypto::GetRandomFailed)?, - )) + Ok(Box::new(ex::ExKeyExchange::with_secp521r1().map_err( + |e| map_error_stack("Secp521r1.start", e, crypto::GetRandomFailed), + )?)) } fn name(&self) -> rustls::NamedGroup { @@ -89,28 +91,12 @@ pub struct FfDHe2048; impl crypto::SupportedKxGroup for FfDHe2048 { fn start(&self) -> Result, rustls::Error> { - Ok(Box::new( - dh::BoringDhKey::generate_ffdhe_2048().map_err(|_| crypto::GetRandomFailed)?, - )) + Ok(Box::new(dh::DhKeyExchange::generate_ffdhe_2048().map_err( + |e| map_error_stack("FfDHe2048.start", e, crypto::GetRandomFailed), + )?)) } fn name(&self) -> rustls::NamedGroup { rustls::NamedGroup::FFDHE2048 } } - -#[cfg(test)] -mod tests { - use rustls::crypto::ActiveKeyExchange; - - #[test] - fn test_derive_dh() { - let alice = super::dh::BoringDhKey::generate_ffdhe_2048().unwrap(); - let bob = super::dh::BoringDhKey::generate_ffdhe_2048().unwrap(); - - let shared_secret1 = alice.diffie_hellman(&bob.pub_key()).unwrap(); - let shared_secret2 = bob.diffie_hellman(&alice.pub_key()).unwrap(); - - assert_eq!(shared_secret1, shared_secret2) - } -} diff --git a/boring-rustls-provider/src/kx/dh.rs b/boring-rustls-provider/src/kx/dh.rs index bf19e69..6b62303 100644 --- a/boring-rustls-provider/src/kx/dh.rs +++ b/boring-rustls-provider/src/kx/dh.rs @@ -2,17 +2,18 @@ use boring::{dh::Dh, error::ErrorStack, pkey::Private}; use foreign_types::ForeignType; use rustls::crypto; -use crate::helper::{cvt, cvt_p}; +use crate::helper::{cvt, cvt_p, map_error_stack}; use super::DhKeyType; -pub struct BoringDhKey { +pub struct DhKeyExchange { dh: Dh, pub_bytes: Vec, key_type: DhKeyType, } -impl BoringDhKey { +impl DhKeyExchange { + // Generate a new KeyExchange with a random FFDHE_2048 private key pub fn generate_ffdhe_2048() -> Result { let mut me = Self { dh: unsafe { Dh::from_ptr(cvt_p(boring_sys::DH_get_rfc7919_2048())?) }, @@ -46,7 +47,8 @@ impl BoringDhKey { Ok(me) } - pub fn diffie_hellman(&self, raw_public_key: &[u8]) -> Result, ErrorStack> { + /// Generate a shared secret with the other's raw public key + fn diffie_hellman(&self, raw_public_key: &[u8]) -> Result, ErrorStack> { let peer = boring::bn::BigNum::from_slice(raw_public_key).unwrap(); let secret_len = unsafe { cvt(boring_sys::DH_size(self.dh.as_ptr()))? } as usize; let mut secret = vec![0u8; secret_len]; @@ -60,14 +62,9 @@ impl BoringDhKey { secret.truncate(secret_len); Ok(secret) } - - #[allow(unused)] - fn pub_key(&self) -> &[u8] { - self.pub_bytes.as_ref() - } } -impl crypto::ActiveKeyExchange for BoringDhKey { +impl crypto::ActiveKeyExchange for DhKeyExchange { fn complete( self: Box, peer_pub_key: &[u8], @@ -80,7 +77,13 @@ impl crypto::ActiveKeyExchange for BoringDhKey { Ok(crypto::SharedSecret::from( self.diffie_hellman(peer_pub_key) - .map_err(|x| rustls::Error::General(x.to_string()))? + .map_err(|e| { + map_error_stack( + "dh.diffie_hellman", + e, + rustls::PeerMisbehaved::InvalidKeyShare, + ) + })? .as_ref(), )) } @@ -96,3 +99,20 @@ impl crypto::ActiveKeyExchange for BoringDhKey { } } } + +#[cfg(test)] +mod tests { + use crate::kx::dh::DhKeyExchange; + use rustls::crypto::ActiveKeyExchange; + + #[test] + fn test_derive_dh() { + let alice = DhKeyExchange::generate_ffdhe_2048().unwrap(); + let bob = DhKeyExchange::generate_ffdhe_2048().unwrap(); + + let shared_secret1 = alice.diffie_hellman(&bob.pub_key()).unwrap(); + let shared_secret2 = bob.diffie_hellman(&alice.pub_key()).unwrap(); + + assert_eq!(shared_secret1, shared_secret2) + } +} diff --git a/boring-rustls-provider/src/kx/ex.rs b/boring-rustls-provider/src/kx/ex.rs index efe8231..f234d00 100644 --- a/boring-rustls-provider/src/kx/ex.rs +++ b/boring-rustls-provider/src/kx/ex.rs @@ -14,7 +14,7 @@ use foreign_types::ForeignType; use rustls::crypto; use spki::der::Decode; -use crate::helper::{cvt, cvt_p}; +use crate::helper::{cvt, cvt_p, map_error_stack}; use super::DhKeyType; @@ -25,26 +25,39 @@ pub struct ExKeyExchange { } impl ExKeyExchange { + /// Creates a new KeyExchange using a random + /// private key for the X25519 Edwards curve pub fn with_x25519() -> Result { Self::ed_from_curve(Nid::from_raw(boring_sys::NID_X25519)) } + /// Creates a new KeyExchange using a random + /// private key for the X448 Edwards curve pub fn with_x448() -> Result { Self::ed_from_curve(Nid::from_raw(boring_sys::NID_X448)) } + /// Creates a new KeyExchange using a random + /// private key for sepc256r1 curve + /// Also known as X9_62_PRIME256V1 pub fn with_secp256r1() -> Result { Self::ec_from_curve(Nid::X9_62_PRIME256V1) } + /// Creates a new KeyExchange using a random + /// private key for sepc384r1 curve pub fn with_secp384r1() -> Result { Self::ec_from_curve(Nid::SECP384R1) } + /// Creates a new KeyExchange using a random + /// private key for sep521r1 curve pub fn with_secp521r1() -> Result { Self::ec_from_curve(Nid::SECP521R1) } + /// Allows getting a new KeyExchange using Eliptic Curves + /// on the specified curve fn ec_from_curve(nid: Nid) -> Result { let ec_group = EcGroup::from_curve_name(nid)?; let ec_key = EcKey::generate(&ec_group)?; @@ -58,6 +71,8 @@ impl ExKeyExchange { }) } + /// Allows getting a new KeyExchange using Edwards Curves + /// on the specified curve fn ed_from_curve(nid: Nid) -> Result { let pkey_ctx = unsafe { EvpPkeyCtx::from_ptr(cvt_p(boring_sys::EVP_PKEY_CTX_new_id( @@ -85,6 +100,7 @@ impl ExKeyExchange { }) } + /// Decodes a SPKI public key to it's raw public key component fn raw_public_key(pkey: &PKeyRef) -> Vec { let spki = pkey.public_key_to_der().unwrap(); @@ -94,6 +110,29 @@ impl ExKeyExchange { // return the raw public key as a new vec Vec::from(key.subject_public_key.as_bytes().unwrap()) } + + /// Derives a shared secret using the peer's raw public key + fn diffie_hellman(&self, peer_pub_key: &[u8]) -> Result, ErrorStack> { + let peerkey = match &self.key_type { + DhKeyType::EC((group, _)) => { + let mut bn_ctx = boring::bn::BigNumContext::new()?; + + let point = crate::verify::ec::ec_point(group, &mut bn_ctx, peer_pub_key)?; + + crate::verify::ec::ec_public_key(group, point.as_ref())? + } + DhKeyType::ED(nid) => { + crate::verify::ed::ed_public_key(peer_pub_key, Nid::from_raw(*nid))? + } + _ => unimplemented!(), + }; + + let mut deriver = boring::derive::Deriver::new(&self.own_key)?; + + deriver.set_peer(&peerkey)?; + + deriver.derive_to_vec() + } } impl crypto::ActiveKeyExchange for ExKeyExchange { @@ -101,33 +140,15 @@ impl crypto::ActiveKeyExchange for ExKeyExchange { self: Box, peer_pub_key: &[u8], ) -> Result { - let peerkey = match &self.key_type { - DhKeyType::EC((group, _)) => { - let mut bn_ctx = boring::bn::BigNumContext::new() - .map_err(|x| rustls::Error::General(x.to_string()))?; - - let point = crate::verify::ec::ec_point(group, &mut bn_ctx, peer_pub_key) - .map_err(|_| rustls::Error::from(rustls::PeerMisbehaved::InvalidKeyShare))?; - - crate::verify::ec::ec_public_key(group, point.as_ref()) - .map_err(|_| rustls::Error::from(rustls::PeerMisbehaved::InvalidKeyShare))? - } - DhKeyType::ED(nid) => { - crate::verify::ed::ed_public_key(peer_pub_key, Nid::from_raw(*nid)) - .map_err(|_| rustls::Error::from(rustls::PeerMisbehaved::InvalidKeyShare))? - } - _ => unimplemented!(), - }; - - let mut deriver = boring::derive::Deriver::new(&self.own_key).unwrap(); - - deriver - .set_peer(&peerkey) - .map_err(|_| rustls::Error::from(rustls::PeerMisbehaved::InvalidKeyShare))?; - - Ok(crypto::SharedSecret::from( - deriver.derive_to_vec().unwrap().as_slice(), - )) + self.diffie_hellman(peer_pub_key) + .map(|x| crypto::SharedSecret::from(x.as_slice())) + .map_err(|e| { + map_error_stack( + "ex.diffie_hellman", + e, + rustls::Error::PeerMisbehaved(rustls::PeerMisbehaved::InvalidKeyShare), + ) + }) } fn pub_key(&self) -> &[u8] { @@ -153,19 +174,23 @@ mod tests { #[test] fn test_derive_ec() { - let kx = Box::new(ExKeyExchange::with_secp256r1().unwrap()); - let kx1 = ExKeyExchange::with_secp256r1().unwrap(); + let alice = Box::new(ExKeyExchange::with_secp256r1().unwrap()); + let bob = ExKeyExchange::with_secp256r1().unwrap(); - kx.group(); - kx.complete(kx1.pub_key()).unwrap(); + assert_eq!( + alice.diffie_hellman(bob.pub_key()).unwrap(), + bob.diffie_hellman(alice.pub_key()).unwrap() + ); } #[test] fn test_derive_ed() { - let kx = Box::new(ExKeyExchange::with_x25519().unwrap()); - let kx1 = ExKeyExchange::with_x25519().unwrap(); + let alice = Box::new(ExKeyExchange::with_x25519().unwrap()); + let bob = ExKeyExchange::with_x25519().unwrap(); - kx.group(); - kx.complete(kx1.pub_key()).unwrap(); + assert_eq!( + alice.diffie_hellman(bob.pub_key()).unwrap(), + bob.diffie_hellman(alice.pub_key()).unwrap() + ); } } diff --git a/boring-rustls-provider/src/sign.rs b/boring-rustls-provider/src/sign.rs index 2f57ebc..f625187 100644 --- a/boring-rustls-provider/src/sign.rs +++ b/boring-rustls-provider/src/sign.rs @@ -1,10 +1,12 @@ use std::sync::Arc; -use boring::{hash::MessageDigest, pkey::Id, rsa::Padding, sign::RsaPssSaltlen}; -use rustls::{ - sign::{Signer, SigningKey}, - SignatureScheme, +use boring::{ + hash::MessageDigest, + pkey::{Id, PKeyRef, Private}, + rsa::Padding, + sign::{RsaPssSaltlen, Signer}, }; +use rustls::{sign::SigningKey, SignatureScheme}; use rustls_pki_types::PrivateKeyDer; const ALL_RSA_SCHEMES: &[SignatureScheme] = &[ @@ -23,10 +25,7 @@ const ALL_EC_SCHEMES: &[SignatureScheme] = &[ ]; #[derive(Debug)] -pub struct BoringPrivateKey( - Arc>, - rustls::SignatureAlgorithm, -); +pub struct BoringPrivateKey(Arc>, rustls::SignatureAlgorithm); impl TryFrom> for BoringPrivateKey { type Error = rustls::Error; @@ -55,11 +54,11 @@ impl TryFrom> for BoringPrivateKey { } fn rsa_signer_from_params( - key: &boring::pkey::PKeyRef, + key: &PKeyRef, digest: MessageDigest, padding: Padding, -) -> boring::sign::Signer { - let mut signer = boring::sign::Signer::new(digest.clone(), key).expect("failed getting signer"); +) -> Signer { + let mut signer = Signer::new(digest.clone(), key).expect("failed getting signer"); signer .set_rsa_padding(padding) .expect("failed setting padding"); @@ -75,11 +74,8 @@ fn rsa_signer_from_params( signer } -fn ec_signer_from_params( - key: &boring::pkey::PKeyRef, - digest: MessageDigest, -) -> boring::sign::Signer { - let signer = boring::sign::Signer::new(digest, key).expect("failed getting signer"); +fn ec_signer_from_params(key: &PKeyRef, digest: MessageDigest) -> Signer { + let signer = Signer::new(digest, key).expect("failed getting signer"); signer } @@ -125,13 +121,10 @@ impl SigningKey for BoringPrivateKey { } #[derive(Debug)] -pub struct BoringSigner( - Arc>, - rustls::SignatureScheme, -); +pub struct BoringSigner(Arc>, rustls::SignatureScheme); impl BoringSigner { - fn get_signer(&self) -> boring::sign::Signer { + fn get_signer(&self) -> Signer { match self.1 { SignatureScheme::RSA_PKCS1_SHA256 => { rsa_signer_from_params(self.0.as_ref(), MessageDigest::sha256(), Padding::PKCS1) @@ -163,17 +156,19 @@ impl BoringSigner { ec_signer_from_params(self.0.as_ref(), MessageDigest::sha512()) } - SignatureScheme::ED25519 => boring::sign::Signer::new_without_digest(self.0.as_ref()) - .expect("failed getting signer"), - SignatureScheme::ED448 => boring::sign::Signer::new_without_digest(self.0.as_ref()) - .expect("failed getting signer"), + SignatureScheme::ED25519 => { + Signer::new_without_digest(self.0.as_ref()).expect("failed getting signer") + } + SignatureScheme::ED448 => { + Signer::new_without_digest(self.0.as_ref()).expect("failed getting signer") + } _ => unimplemented!(), } } } -impl Signer for BoringSigner { +impl rustls::sign::Signer for BoringSigner { fn sign(&self, message: &[u8]) -> Result, rustls::Error> { let signer = self.get_signer(); let mut msg_with_sig = diff --git a/boring-rustls-provider/src/verify/ec.rs b/boring-rustls-provider/src/verify/ec.rs index d42df1f..d254bba 100644 --- a/boring-rustls-provider/src/verify/ec.rs +++ b/boring-rustls-provider/src/verify/ec.rs @@ -1,4 +1,4 @@ -use boring::hash::MessageDigest; +use boring::{error::ErrorStack, hash::MessageDigest}; use rustls::SignatureScheme; use rustls_pki_types::{InvalidSignature, SignatureVerificationAlgorithm}; @@ -18,8 +18,10 @@ impl SignatureVerificationAlgorithm for BoringEcVerifier { signature: &[u8], ) -> Result<(), rustls_pki_types::InvalidSignature> { let (group, mut bn_ctx) = setup_ec_key(self.0); - let ec_point = ec_point(group.as_ref(), bn_ctx.as_mut(), public_key)?; - let public_key = ec_public_key(group.as_ref(), ec_point.as_ref())?; + let ec_point = + ec_point(group.as_ref(), bn_ctx.as_mut(), public_key).map_err(|_| InvalidSignature)?; + let public_key = + ec_public_key(group.as_ref(), ec_point.as_ref()).map_err(|_| InvalidSignature)?; let mut verifier = match self.0 { SignatureScheme::ECDSA_NISTP256_SHA256 => { ec_verifier_from_params(public_key.as_ref(), MessageDigest::sha256()) @@ -100,16 +102,13 @@ pub(crate) fn ec_point( group: &boring::ec::EcGroupRef, bignum_ctx: &mut boring::bn::BigNumContextRef, spki_spk: &[u8], -) -> Result { - boring::ec::EcPoint::from_bytes(group, spki_spk, bignum_ctx).map_err(|_| InvalidSignature) +) -> Result { + boring::ec::EcPoint::from_bytes(group, spki_spk, bignum_ctx) } pub(crate) fn ec_public_key( group: &boring::ec::EcGroupRef, ec_point: &boring::ec::EcPointRef, -) -> Result, InvalidSignature> { - boring::pkey::PKey::from_ec_key( - boring::ec::EcKey::from_public_key(group, ec_point).map_err(|_| InvalidSignature)?, - ) - .map_err(|_| InvalidSignature) +) -> Result, ErrorStack> { + boring::pkey::PKey::from_ec_key(boring::ec::EcKey::from_public_key(group, ec_point)?) } diff --git a/boring-rustls-provider/src/verify/ed.rs b/boring-rustls-provider/src/verify/ed.rs index a96bcb8..4917e2e 100644 --- a/boring-rustls-provider/src/verify/ed.rs +++ b/boring-rustls-provider/src/verify/ed.rs @@ -1,5 +1,6 @@ use std::ptr; +use boring::error::ErrorStack; use foreign_types::ForeignType; use rustls::SignatureScheme; use rustls_pki_types::{InvalidSignature, SignatureVerificationAlgorithm}; @@ -20,7 +21,8 @@ impl SignatureVerificationAlgorithm for BoringEdVerifier { message: &[u8], signature: &[u8], ) -> Result<(), rustls_pki_types::InvalidSignature> { - let public_key = ed_public_key_for_scheme(public_key, self.0)?; + let public_key = + ed_public_key_for_scheme(public_key, self.0).map_err(|_| InvalidSignature)?; let mut verifier = ed_verifier_from_params(public_key.as_ref()); verifier.verify_oneshot(signature, message).map_or_else( @@ -58,7 +60,7 @@ fn ed_verifier_from_params( fn ed_public_key_for_scheme( spki_spk: &[u8], scheme: SignatureScheme, -) -> Result, InvalidSignature> { +) -> Result, ErrorStack> { let nid = boring::nid::Nid::from_raw(match scheme { SignatureScheme::ED25519 => boring_sys::EVP_PKEY_ED25519, SignatureScheme::ED448 => boring_sys::EVP_PKEY_ED448, @@ -70,15 +72,14 @@ fn ed_public_key_for_scheme( pub fn ed_public_key( spki_spk: &[u8], nid: boring::nid::Nid, -) -> Result, InvalidSignature> { +) -> Result, ErrorStack> { Ok(unsafe { let pkey = cvt_p(boring_sys::EVP_PKEY_new_raw_public_key( nid.as_raw(), ptr::null_mut(), spki_spk.as_ptr(), spki_spk.len(), - )) - .map_err(|_| InvalidSignature)?; + ))?; boring::pkey::PKey::from_ptr(pkey) })