Enhance error handling and docs

boring-additions/src/aead/mod.rs

@@ -15,45 +15,53 @@ impl Algorithm {
     /// AES-128 in Galois Counter Mode.
     ///
     /// Note: AES-GCM should only be used with 12-byte (96-bit) nonces. Although it is specified to take a variable-length nonce, nonces with other lengths are effectively randomized, which means one must consider collisions. Unless implementing an existing protocol which has already specified incorrect parameters, only use 12-byte nonces.
+    #[must_use]
     pub fn aes_128_gcm() -> Self {
         Self(unsafe { boring_sys::EVP_aead_aes_128_gcm() })
     }
 
     /// AES-256 in Galois Counter Mode.
     ///
     /// Note: AES-GCM should only be used with 12-byte (96-bit) nonces. Although it is specified to take a variable-length nonce, nonces with other lengths are effectively randomized, which means one must consider collisions. Unless implementing an existing protocol which has already specified incorrect parameters, only use 12-byte nonces.
+    #[must_use]
     pub fn aes_256_gcm() -> Self {
         Self(unsafe { boring_sys::EVP_aead_aes_256_gcm() })
     }
 
-    /// ChaCha20 and Poly1305 as described in RFC 8439.
+    /// `ChaCha20` with `Poly1305` as described in RFC 8439.
+    #[must_use]
     pub fn chacha20_poly1305() -> Self {
         Self(unsafe { boring_sys::EVP_aead_chacha20_poly1305() })
     }
 
     /// ChaCha20-Poly1305 with an extended nonce that makes random generation of nonces safe.
     #[allow(unused)]
+    #[must_use]
     pub fn xchacha20_poly1305() -> Self {
         Self(unsafe { boring_sys::EVP_aead_xchacha20_poly1305() })
     }
 
     /// Returns the length, in bytes, of the keys used by aead
+    #[must_use]
     pub fn key_length(&self) -> usize {
         unsafe { boring_sys::EVP_AEAD_key_length(self.0) }
     }
 
     /// Returns the maximum number of additional bytes added by the act of sealing data with aead.
+    #[must_use]
     pub fn max_overhead(&self) -> usize {
         unsafe { boring_sys::EVP_AEAD_max_overhead(self.0) }
     }
 
     /// Returns the maximum tag length when using aead.
     #[allow(unused)]
+    #[must_use]
     pub fn max_tag_len(&self) -> usize {
         unsafe { boring_sys::EVP_AEAD_max_tag_len(self.0) }
     }
 
     /// Returns the length, in bytes, of the per-message nonce for aead.
+    #[must_use]
     pub fn nonce_len(&self) -> usize {
         unsafe { boring_sys::EVP_AEAD_nonce_length(self.0) }
     }
@@ -66,7 +74,14 @@ pub struct Crypter {
 }
 
 impl Crypter {
-    pub fn new(aead_alg: Algorithm, key: &[u8]) -> Result<Self, ErrorStack> {
+    /// Constructs a new AEAD crypter with the given algorithm and key
+    ///
+    /// # 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<Self, ErrorStack> {
         assert_eq!(aead_alg.key_length(), key.len());
         boring_sys::init();
 
@@ -86,13 +101,25 @@ impl Crypter {
         Ok(this)
     }
 
+    /// Returns the maximum required overhead in bytes
+    /// that will be added to a ciphertext, e.g.,
+    /// to hold an authentication tag.
+    #[must_use]
     pub fn max_overhead(&self) -> usize {
         self.max_overhead
     }
 
-    /// Encrypts and authenticates buffer and authenticates associated_data.
-    /// It writes the ciphertext to buffer and the authentication tag to tag.
-    /// On success, it returns the actual length of the tag
+    /// Encrypts and authenticates `buffer` and authenticates `associated_data`.
+    /// It writes the ciphertext to `buffer` and the authentication tag to `tag`.
+    /// `tag` needs to have sufficient space, see [`Self::max_overhead()`](fn@Self::max_overhead())
+    /// On success, it returns the actual length of the `tag`
+    ///
+    /// # 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],
@@ -124,6 +151,14 @@ impl Crypter {
         Ok(tag_len)
     }
 
+    /// Decrypts and authenticates `buffer` and authenticates `associated_data`.
+    /// It writes the cleartext to `buffer` and validates using `tag`.
+    ///
+    /// # 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],
@@ -157,25 +192,19 @@ mod tests {
 
     #[test]
     fn in_out() {
-        let key = Crypter::new(super::Algorithm::aes_128_gcm(), &[0u8; 16]).unwrap();
+        let key = Crypter::new(&super::Algorithm::aes_128_gcm(), &[0u8; 16]).unwrap();
         let nonce = [0u8; 12];
-        let associated_data = b"this is signed";
+        let associated_data = b"this is authenticated";
         let mut buffer = Vec::with_capacity(26);
-        buffer.push(b'A');
-        buffer.push(b'B');
-        buffer.push(b'C');
-        buffer.push(b'D');
-        buffer.push(b'E');
+        buffer.extend_from_slice(b"ABCDE");
 
         let mut tag = [0u8; 16];
         key.seal_in_place(&nonce, associated_data, buffer.as_mut_slice(), &mut tag)
             .unwrap();
 
-        println!("Encrypted: {:02X?}, Tag: {:02X?}", buffer, tag);
-
         key.open_in_place(&nonce, associated_data, buffer.as_mut_slice(), &tag[..])
             .unwrap();
 
-        println!("Plaintext: {}", String::from_utf8(buffer).unwrap());
+        assert_eq!(b"ABCDE", buffer.as_slice());
     }
 }

boring-additions/src/helper.rs

@@ -2,11 +2,11 @@ use std::os::raw::c_int;
 
 use boring::error::ErrorStack;
 
-/// Check the value returned from a BoringSSL ffi call
+/// Check the value returned from a `BoringSSL` ffi call
 /// that returns a pointer.
 ///
-/// If the pointer is null, this method returns the BoringSSL
-/// ErrorStack as Err, the pointer otherwise.
+/// If the pointer is null, this method returns the
+/// [`boring::error::ErrorStack`] as Err, the pointer otherwise.
 pub(crate) fn cvt_p<T>(r: *mut T) -> Result<*mut T, ErrorStack> {
     if r.is_null() {
         Err(ErrorStack::get())
@@ -15,10 +15,10 @@ pub(crate) fn cvt_p<T>(r: *mut T) -> Result<*mut T, ErrorStack> {
     }
 }
 
-/// Check the value returned from a BoringSSL ffi call that
+/// Check the value returned from a `BoringSSL` ffi call that
 /// returns a integer.
 ///
-/// Returns the BoringSSL Errorstack when the result is <= 0.
+/// Returns the [`boring::error::ErrorStack`] when the result is <= 0.
 /// And forwards the return code otherwise
 pub(crate) fn cvt(r: c_int) -> Result<i32, ErrorStack> {
     if r <= 0 {

boring-additions/src/hmac/types.rs

@@ -38,11 +38,14 @@ unsafe impl ForeignType for HmacCtx {
 impl Clone for HmacCtx {
     fn clone(&self) -> Self {
         unsafe {
-            let ctx = HmacCtx::from_ptr(cvt_p(boring_sys::HMAC_CTX_new()).unwrap());
-
-            cvt(boring_sys::HMAC_CTX_copy(ctx.as_ptr(), self.0.as_ptr())).unwrap();
-            ctx
+            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")
     }
 }
 

boring-rustls-provider/src/aead.rs

@@ -6,7 +6,7 @@ use boring_additions::aead::Algorithm;
 use rustls::crypto::cipher::{self, make_tls12_aad, make_tls13_aad, Iv};
 use rustls::{ConnectionTrafficSecrets, ContentType, ProtocolVersion};
 
-use crate::helper::error_stack_to_aead_error;
+use crate::helper::log_and_map;
 
 pub(crate) mod aes;
 pub(crate) mod chacha20;
@@ -47,6 +47,7 @@ impl<T: BoringAead> AeadCore for BoringAeadCrypter<T> {
 }
 
 impl<T: BoringAead> BoringAeadCrypter<T> {
+    /// Creates a new aead crypter
     pub fn new(iv: Iv, key: &[u8], tls_version: ProtocolVersion) -> Result<Self, ErrorStack> {
         assert!(match tls_version {
             #[cfg(feature = "tls12")]
@@ -63,7 +64,7 @@ impl<T: BoringAead> BoringAeadCrypter<T> {
         );
 
         let crypter = BoringAeadCrypter {
-            crypter: boring_additions::aead::Crypter::new(cipher, key)?,
+            crypter: boring_additions::aead::Crypter::new(&cipher, key)?,
             iv,
             tls_version,
             phantom: PhantomData,
@@ -82,7 +83,7 @@ impl<T: BoringAead> aead::AeadInPlace for BoringAeadCrypter<T> {
         let mut tag = Tag::<Self>::default();
         self.crypter
             .seal_in_place(nonce, associated_data, buffer, &mut tag)
-            .map_err(|e| error_stack_to_aead_error("seal_in_place", e))?;
+            .map_err(|e| log_and_map("seal_in_place", e, aead::Error))?;
 
         Ok(tag)
     }
@@ -96,7 +97,7 @@ impl<T: BoringAead> aead::AeadInPlace for BoringAeadCrypter<T> {
     ) -> aead::Result<()> {
         self.crypter
             .open_in_place(nonce, associated_data, buffer, tag)
-            .map_err(|e| error_stack_to_aead_error("open_in_place", e))?;
+            .map_err(|e| log_and_map("open_in_place", e, aead::Error))?;
         Ok(())
     }
 }

boring-rustls-provider/src/aead/aes.rs

@@ -3,11 +3,10 @@ use aead::consts::{U12, U16};
 use boring_additions::aead::Algorithm;
 use rustls::{crypto::cipher, ConnectionTrafficSecrets};
 
+/// Aes128 AEAD cipher
 pub struct Aes128 {}
 
 impl BoringAead for Aes128 {}
-unsafe impl Send for Aes128 {}
-unsafe impl Sync for Aes128 {}
 
 impl BoringCipher for Aes128 {
     fn new_cipher() -> Algorithm {
@@ -37,11 +36,10 @@ impl aead::AeadCore for Aes128 {
     type CiphertextOverhead = U16;
 }
 
+/// Aes256 AEAD cipher
 pub struct Aes256 {}
 
 impl BoringAead for Aes256 {}
-unsafe impl Send for Aes256 {}
-unsafe impl Sync for Aes256 {}
 
 impl BoringCipher for Aes256 {
     fn new_cipher() -> Algorithm {

boring-rustls-provider/src/aead/chacha20.rs

@@ -6,11 +6,10 @@ use aead::{
 use boring_additions::aead::Algorithm;
 use rustls::{crypto::cipher, ConnectionTrafficSecrets};
 
+/// `ChaCha20` with `Poly1305` cipher
 pub struct ChaCha20Poly1305 {}
 
 impl BoringAead for ChaCha20Poly1305 {}
-unsafe impl Send for ChaCha20Poly1305 {}
-unsafe impl Sync for ChaCha20Poly1305 {}
 
 impl BoringCipher for ChaCha20Poly1305 {
     fn new_cipher() -> Algorithm {

boring-rustls-provider/src/hash.rs

@@ -1,3 +1,4 @@
+use boring::hash::{Hasher, MessageDigest};
 use rustls::crypto::hash;
 
 pub const SHA256: &dyn hash::Hash = &Hash(boring::nid::Nid::SHA256);
@@ -7,8 +8,8 @@ pub struct Hash(pub boring::nid::Nid);
 
 impl hash::Hash for Hash {
     fn start(&self) -> Box<dyn hash::Context> {
-        let digest = boring::hash::MessageDigest::from_nid(self.0).unwrap();
-        let hasher = boring::hash::Hasher::new(digest).unwrap();
+        let digest = MessageDigest::from_nid(self.0).expect("failed getting hash digest");
+        let hasher = Hasher::new(digest).expect("failed getting hasher");
         Box::new(HasherContext(hasher))
     }
 
@@ -21,36 +22,38 @@ impl hash::Hash for Hash {
     fn algorithm(&self) -> hash::HashAlgorithm {
         match self.0 {
             boring::nid::Nid::SHA256 => hash::HashAlgorithm::SHA256,
+            boring::nid::Nid::SHA384 => hash::HashAlgorithm::SHA384,
+            boring::nid::Nid::SHA512 => hash::HashAlgorithm::SHA512,
             _ => unimplemented!(),
         }
     }
 
     fn output_len(&self) -> usize {
-        boring::hash::MessageDigest::from_nid(self.0)
-            .unwrap()
+        MessageDigest::from_nid(self.0)
+            .expect("failed getting digest")
             .size()
     }
 }
 
-struct HasherContext(boring::hash::Hasher);
+struct HasherContext(Hasher);
 
 impl hash::Context for HasherContext {
     fn fork_finish(&self) -> hash::Output {
         let mut cloned = self.0.clone();
 
-        hash::Output::new(&cloned.finish().unwrap()[..])
+        hash::Output::new(&cloned.finish().expect("failed finishing hash")[..])
     }
 
     fn fork(&self) -> Box<dyn hash::Context> {
         Box::new(HasherContext(self.0.clone()))
     }
 
     fn finish(mut self: Box<Self>) -> hash::Output {
-        hash::Output::new(&self.0.finish().unwrap()[..])
+        hash::Output::new(&self.0.finish().expect("failed finishing hash")[..])
     }
 
     fn update(&mut self, data: &[u8]) {
-        self.0.update(data).unwrap();
+        self.0.update(data).expect("failed adding data to hash");
     }
 }
 

boring-rustls-provider/src/helper.rs

@@ -4,11 +4,11 @@ use boring::error::ErrorStack;
 #[cfg(feature = "log")]
 use log::trace;
 
-/// Check the value returned from a BoringSSL ffi call
+/// Check the value returned from a `BoringSSL` ffi call
 /// that returns a pointer.
 ///
-/// If the pointer is null, this method returns the BoringSSL
-/// ErrorStack as Err, the pointer otherwise.
+/// If the pointer is null, this method returns the
+/// [`boring::error::ErrorStack`] as Err, the pointer otherwise.
 pub(crate) fn cvt_p<T>(r: *mut T) -> Result<*mut T, ErrorStack> {
     if r.is_null() {
         Err(ErrorStack::get())
@@ -17,10 +17,10 @@ pub(crate) fn cvt_p<T>(r: *mut T) -> Result<*mut T, ErrorStack> {
     }
 }
 
-/// Check the value returned from a BoringSSL ffi call that
+/// Check the value returned from a `BoringSSL` ffi call that
 /// returns a integer.
 ///
-/// Returns the BoringSSL Errorstack when the result is <= 0.
+/// Returns the [`boring::error::ErrorStack`] when the result is <= 0.
 /// And forwards the return code otherwise
 pub(crate) fn cvt(r: c_int) -> Result<i32, ErrorStack> {
     if r <= 0 {
@@ -30,17 +30,13 @@ pub(crate) fn cvt(r: c_int) -> Result<i32, ErrorStack> {
     }
 }
 
-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 {
+pub(crate) fn log_and_map<E: core::fmt::Display, T>(func: &'static str, e: E, mapped: T) -> T {
     trace!("failed {}, error: {}", func, e);
     mapped
 }
 
 #[cfg(not(feature = "log"))]
-pub(crate) fn map_error_stack<T>(func: &'static str, e: ErrorStack, mapped: T) -> T {
+pub(crate) fn log_and_map<E: core::fmt::Display, T>(func: &'static str, e: E, mapped: T) -> T {
     mapped
 }