Rework KeyExchange types

boring-rustls-provider/src/helper.rs

@@ -30,13 +30,17 @@ pub(crate) fn cvt(r: c_int) -> Result<i32, ErrorStack> {
     }
 }
 
-#[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<T>(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<T>(func: &'static str, e: ErrorStack, mapped: T) -> T {
+    mapped
 }

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<Box<(dyn ActiveKeyExchange + 'static)>, 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<Box<(dyn ActiveKeyExchange + 'static)>, 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<Box<(dyn ActiveKeyExchange + 'static)>, 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<Box<(dyn ActiveKeyExchange + 'static)>, 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<Box<(dyn ActiveKeyExchange + 'static)>, 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<Box<(dyn ActiveKeyExchange + 'static)>, 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)
-    }
-}

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<Private>,
     pub_bytes: Vec<u8>,
     key_type: DhKeyType,
 }
 
-impl BoringDhKey {
+impl DhKeyExchange {
+    // Generate a new KeyExchange with a random FFDHE_2048 private key
     pub fn generate_ffdhe_2048() -> Result<Self, ErrorStack> {
         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<Vec<u8>, ErrorStack> {
+    /// Generate a shared secret with the other's raw public key
+    fn diffie_hellman(&self, raw_public_key: &[u8]) -> Result<Vec<u8>, 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<Self>,
         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)
+    }
+}

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, ErrorStack> {
         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, ErrorStack> {
         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, ErrorStack> {
         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, ErrorStack> {
         Self::ec_from_curve(Nid::SECP384R1)
     }
 
+    /// Creates a new KeyExchange using a random
+    /// private key for sep521r1 curve
     pub fn with_secp521r1() -> Result<Self, ErrorStack> {
         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<Self, ErrorStack> {
         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<Self, ErrorStack> {
         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<Private>) -> Vec<u8> {
         let spki = pkey.public_key_to_der().unwrap();
 
@@ -94,40 +110,45 @@ impl ExKeyExchange {
         // return the raw public key as a new vec
         Vec::from(key.subject_public_key.as_bytes().unwrap())
     }
-}
 
-impl crypto::ActiveKeyExchange for ExKeyExchange {
-    fn complete(
-        self: Box<Self>,
-        peer_pub_key: &[u8],
-    ) -> Result<crypto::SharedSecret, rustls::Error> {
+    /// Derives a shared secret using the peer's raw public key
+    fn diffie_hellman(&self, peer_pub_key: &[u8]) -> Result<Vec<u8>, ErrorStack> {
         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 mut bn_ctx = boring::bn::BigNumContext::new()?;
 
-                let point = crate::verify::ec::ec_point(group, &mut bn_ctx, peer_pub_key)
-                    .map_err(|_| rustls::Error::from(rustls::PeerMisbehaved::InvalidKeyShare))?;
+                let point = crate::verify::ec::ec_point(group, &mut bn_ctx, peer_pub_key)?;
 
-                crate::verify::ec::ec_public_key(group, point.as_ref())
-                    .map_err(|_| rustls::Error::from(rustls::PeerMisbehaved::InvalidKeyShare))?
+                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))
-                    .map_err(|_| rustls::Error::from(rustls::PeerMisbehaved::InvalidKeyShare))?
+                crate::verify::ed::ed_public_key(peer_pub_key, Nid::from_raw(*nid))?
             }
             _ => unimplemented!(),
         };
 
-        let mut deriver = boring::derive::Deriver::new(&self.own_key).unwrap();
+        let mut deriver = boring::derive::Deriver::new(&self.own_key)?;
+
+        deriver.set_peer(&peerkey)?;
 
-        deriver
-            .set_peer(&peerkey)
-            .map_err(|_| rustls::Error::from(rustls::PeerMisbehaved::InvalidKeyShare))?;
+        deriver.derive_to_vec()
+    }
+}
 
-        Ok(crypto::SharedSecret::from(
-            deriver.derive_to_vec().unwrap().as_slice(),
-        ))
+impl crypto::ActiveKeyExchange for ExKeyExchange {
+    fn complete(
+        self: Box<Self>,
+        peer_pub_key: &[u8],
+    ) -> Result<crypto::SharedSecret, rustls::Error> {
+        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()
+        );
     }
 }