Enhance error handling and docs

boring-additions/src/aead/mod.rs

@@ -15,6 +15,7 @@ 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() })
     }
@@ -22,38 +23,45 @@ impl Algorithm {
     /// 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")
     }
 }