Merge pull request #41 from brycx/fix-err-prop-osrng

Fix error propagation on OsRng in generate()
orion-rs · Nov 24, 2018 · da5ff81 · da5ff81
2 parents 54592ec + 5dda88d
commit da5ff81
Show file tree

Hide file tree

Showing 12 changed files with 72 additions and 94 deletions.
diff --git a/src/aead.rs b/src/aead.rs
@@ -68,7 +68,7 @@
 
 use errors::UnknownCryptoError;
 use hazardous::aead;
-use hazardous::constants::{CHACHA_KEYSIZE, POLY1305_BLOCKSIZE, XCHACHA_NONCESIZE};
+use hazardous::constants::{POLY1305_BLOCKSIZE, XCHACHA_NONCESIZE};
 use hazardous::stream::chacha20;
 use hazardous::stream::xchacha20::Nonce;
 pub use hltypes::SecretKey;
@@ -79,17 +79,14 @@ pub fn seal(secret_key: &SecretKey, plaintext: &[u8]) -> Result<Vec<u8>, Unknown
     if plaintext.is_empty() {
         return Err(UnknownCryptoError);
     }
-    if secret_key.get_length() != CHACHA_KEYSIZE {
-        return Err(UnknownCryptoError);
-    }
 
-    let nonce = Nonce::generate();
+    let nonce = Nonce::generate()?;
 
     let mut dst_out = vec![0u8; plaintext.len() + (XCHACHA_NONCESIZE + POLY1305_BLOCKSIZE)];
     dst_out[..XCHACHA_NONCESIZE].copy_from_slice(&nonce.as_bytes());
 
     aead::xchacha20poly1305::seal(
-        &chacha20::SecretKey::from_slice(&secret_key.unprotected_as_bytes()).unwrap(),
+        &chacha20::SecretKey::from_slice(&secret_key.unprotected_as_bytes())?,
         &nonce,
         plaintext,
         None,
@@ -109,16 +106,13 @@ pub fn open(
     if ciphertext_with_tag_and_nonce.len() < (XCHACHA_NONCESIZE + POLY1305_BLOCKSIZE + 1) {
         return Err(UnknownCryptoError);
     }
-    if secret_key.get_length() != CHACHA_KEYSIZE {
-        return Err(UnknownCryptoError);
-    }
 
     let mut dst_out =
         vec![0u8; ciphertext_with_tag_and_nonce.len() - (XCHACHA_NONCESIZE + POLY1305_BLOCKSIZE)];
 
     aead::xchacha20poly1305::open(
-        &chacha20::SecretKey::from_slice(&secret_key.unprotected_as_bytes()).unwrap(),
-        &Nonce::from_slice(&ciphertext_with_tag_and_nonce[..XCHACHA_NONCESIZE]).unwrap(),
+        &chacha20::SecretKey::from_slice(&secret_key.unprotected_as_bytes())?,
+        &Nonce::from_slice(&ciphertext_with_tag_and_nonce[..XCHACHA_NONCESIZE])?,
         &ciphertext_with_tag_and_nonce[XCHACHA_NONCESIZE..],
         None,
         &mut dst_out,
@@ -202,7 +196,8 @@ fn test_diff_secret_key_err() {
 #[test]
 fn test_secret_length_err() {
     let key = SecretKey::generate(31).unwrap();
-    let plaintext = "Secret message".as_bytes().to_vec();
+    let plaintext = "Secret message Secret message Secret message Secret message ".as_bytes().to_vec();
 
     assert!(seal(&key, &plaintext).is_err());
+    assert!(open(&key, &plaintext).is_err());
 }
diff --git a/src/hazardous/aead/chacha20poly1305.rs b/src/hazardous/aead/chacha20poly1305.rs
@@ -62,7 +62,7 @@
 //! ```
 //! use orion::hazardous::aead;
 //!
-//! let secret_key = aead::chacha20poly1305::SecretKey::generate();
+//! let secret_key = aead::chacha20poly1305::SecretKey::generate().unwrap();
 //!
 //! let nonce = aead::chacha20poly1305::Nonce::from_slice(&[ 0x07, 0x00, 0x00, 0x00, 0x40, 0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x47 ]).unwrap();
 //! let ad = [ 0x50, 0x51, 0x52, 0x53, 0xc0, 0xc1, 0xc2, 0xc3, 0xc4, 0xc5, 0xc6, 0xc7 ];
@@ -214,7 +214,7 @@ pub fn open(
         &optional_ad,
         ciphertext_with_tag,
         ciphertext_len,
-    )?;
+    ).unwrap();
 
     util::secure_cmp(
         &poly1305_state.finalize().unwrap().unprotected_as_bytes(),
@@ -242,12 +242,11 @@ fn length_padding_tests() {
 }
 
 #[test]
-#[should_panic]
 fn test_auth_process_with_above_length_index() {
     let poly1305_key = poly1305_key_gen(&[0u8; 32], &[0u8; 12]).unwrap();
     let mut poly1305_state = poly1305::init(&poly1305_key);
 
-    process_authentication(&mut poly1305_state, &[0u8; 0], &[0u8; 64], 65).unwrap();
+    assert!(process_authentication(&mut poly1305_state, &[0u8; 0], &[0u8; 64], 65).is_err());
 }
 
 #[test]
@@ -268,7 +267,6 @@ fn test_nonce_sizes() {
 }
 
 #[test]
-#[should_panic]
 fn test_modified_tag_error() {
     let mut dst_out_ct = [0u8; 80]; // 64 + Poly1305TagLen
     let mut dst_out_pt = [0u8; 64];
@@ -282,13 +280,13 @@ fn test_modified_tag_error() {
     ).unwrap();
     // Modify the tags first byte
     dst_out_ct[65] ^= 1;
-    open(
+    assert!(open(
         &SecretKey::from_slice(&[0u8; 32]).unwrap(),
         &Nonce::from_slice(&[0u8; 12]).unwrap(),
         &dst_out_ct,
         None,
         &mut dst_out_pt,
-    ).unwrap();
+    ).is_err());
 }
 
 #[test]

diff --git a/src/hazardous/aead/xchacha20poly1305.rs b/src/hazardous/aead/xchacha20poly1305.rs
@@ -56,8 +56,8 @@
 //! ```
 //! use orion::hazardous::aead;
 //!
-//! let secret_key = aead::xchacha20poly1305::SecretKey::generate();
-//! let nonce = aead::xchacha20poly1305::Nonce::generate();
+//! let secret_key = aead::xchacha20poly1305::SecretKey::generate().unwrap();
+//! let nonce = aead::xchacha20poly1305::Nonce::generate().unwrap();
 //!
 //! let ad = [ 0x50, 0x51, 0x52, 0x53, 0xc0, 0xc1, 0xc2, 0xc3, 0xc4, 0xc5, 0xc6, 0xc7 ];
 //! let plaintext = b"\
@@ -136,7 +136,6 @@ pub fn open(
 }
 
 #[test]
-#[should_panic]
 fn test_modified_tag_error() {
     let mut dst_out_ct = [0u8; 80]; // 64 + Poly1305TagLen
     let mut dst_out_pt = [0u8; 64];
@@ -150,11 +149,11 @@ fn test_modified_tag_error() {
     ).unwrap();
     // Modify the tags first byte
     dst_out_ct[65] ^= 1;
-    open(
+    assert!(open(
         &SecretKey::from_slice(&[0u8; 32]).unwrap(),
         &Nonce::from_slice(&[0u8; 24]).unwrap(),
         &dst_out_ct,
         None,
         &mut dst_out_pt,
-    ).unwrap();
+    ).is_err());
 }
diff --git a/src/hazardous/kdf/hkdf.rs b/src/hazardous/kdf/hkdf.rs
@@ -135,7 +135,7 @@ pub fn verify(
     info: Option<&[u8]>,
     dst_out: &mut [u8],
 ) -> Result<bool, ValidationCryptoError> {
-    expand(&extract(salt, ikm), info, dst_out).unwrap();
+    expand(&extract(salt, ikm), info, dst_out)?;
 
     if util::secure_cmp(&dst_out, expected).is_err() {
         Err(ValidationCryptoError)

diff --git a/src/hazardous/mac/hmac.rs b/src/hazardous/mac/hmac.rs
@@ -45,7 +45,7 @@
 //! ```
 //! use orion::hazardous::mac::hmac;
 //!
-//! let key = hmac::SecretKey::generate();
+//! let key = hmac::SecretKey::generate().unwrap();
 //! let msg = "Some message.";
 //!
 //! let mut tag = hmac::init(&key);
@@ -56,7 +56,7 @@
 //! ```
 //! use orion::hazardous::mac::hmac;
 //!
-//! let key = hmac::SecretKey::generate();
+//! let key = hmac::SecretKey::generate().unwrap();
 //! let msg = "Some message.";
 //!
 //! let mut tag = hmac::init(&key);
@@ -210,10 +210,10 @@ pub fn verify(
     secret_key: &SecretKey,
     data: &[u8],
 ) -> Result<bool, ValidationCryptoError> {
-    let mut tag = init(secret_key);
-    tag.update(data).unwrap();
+    let mut hmac_state = init(secret_key);
+    hmac_state.update(data).unwrap();
 
-    if expected == &tag.finalize().unwrap() {
+    if expected == &hmac_state.finalize().unwrap() {
         Ok(true)
     } else {
         Err(ValidationCryptoError)
@@ -273,15 +273,14 @@ fn veriy_false_wrong_secret_key() {
 }
 
 #[test]
-#[should_panic]
 fn double_finalize_err() {
     let secret_key = SecretKey::from_slice("Jefe".as_bytes());
     let data = "what do ya want for nothing?".as_bytes();
 
     let mut tag = init(&secret_key);
     tag.update(data).unwrap();
     let _ = tag.finalize().unwrap();
-    let _ = tag.finalize().unwrap();
+    assert!(tag.finalize().is_err());
 }
 
 #[test]
@@ -310,15 +309,14 @@ fn double_finalize_with_reset_no_update_ok() {
 }
 
 #[test]
-#[should_panic]
 fn update_after_finalize_err() {
     let secret_key = SecretKey::from_slice("Jefe".as_bytes());
     let data = "what do ya want for nothing?".as_bytes();
 
     let mut tag = init(&secret_key);
     tag.update(data).unwrap();
     let _ = tag.finalize().unwrap();
-    tag.update(data).unwrap();
+    assert!(tag.update(data).is_err());
 }
 
 #[test]

diff --git a/src/hazardous/mac/poly1305.rs b/src/hazardous/mac/poly1305.rs
@@ -47,7 +47,7 @@
 //! ```
 //! use orion::hazardous::mac::poly1305;
 //!
-//! let one_time_key = poly1305::OneTimeKey::generate();
+//! let one_time_key = poly1305::OneTimeKey::generate().unwrap();
 //! let msg = "Some message.";
 //!
 //! let mut poly1305_state = poly1305::init(&one_time_key);
@@ -402,9 +402,7 @@ pub fn verify(
     one_time_key: &OneTimeKey,
     data: &[u8],
 ) -> Result<bool, ValidationCryptoError> {
-    let tag = poly1305(one_time_key, data)?;
-
-    if &tag == expected {
+    if &poly1305(one_time_key, data)? == expected {
         Ok(true)
     } else {
         Err(ValidationCryptoError)
@@ -434,37 +432,33 @@ fn test_poly1305_verify_ok() {
 }
 
 #[test]
-#[should_panic]
 fn test_poly1305_verify_err() {
     let mut tag = poly1305(&OneTimeKey::from_slice(&[0u8; 32]).unwrap(), &[0u8; 16]).unwrap();
     tag.value[0] ^= 1;
-    verify(
+    assert!(verify(
         &tag,
         &OneTimeKey::from_slice(&[0u8; 32]).unwrap(),
         &[0u8; 16],
-    ).unwrap();
+    ).is_err());
 }
 
 #[test]
-#[should_panic]
-fn test_poly1305_oneshot_bad_key_err_less() {
-    let _ = poly1305(&OneTimeKey::from_slice(&[0u8; 31]).unwrap(), &[0u8; 16]).unwrap();
+fn test_bad_key_err_less() {
+    assert!(OneTimeKey::from_slice(&[0u8; 31]).is_err());
 }
 
 #[test]
-#[should_panic]
 fn test_poly1305_oneshot_bad_key_err_greater() {
-    let _ = poly1305(&OneTimeKey::from_slice(&[0u8; 33]).unwrap(), &[0u8; 16]).unwrap();
+    assert!(OneTimeKey::from_slice(&[0u8; 33]).is_err());
 }
 
 #[test]
-#[should_panic]
 fn double_finalize_err() {
     let mut poly1305_state = init(&OneTimeKey::from_slice(&[0u8; 32]).unwrap());
 
     poly1305_state.update(&[0u8; 16]).unwrap();
     let _ = poly1305_state.finalize().unwrap();
-    let _ = poly1305_state.finalize().unwrap();
+    assert!(poly1305_state.finalize().is_err());
 }
 
 #[test]
@@ -489,13 +483,12 @@ fn double_finalize_with_reset_no_update_ok() {
 }
 
 #[test]
-#[should_panic]
 fn update_after_finalize_err() {
     let mut poly1305_state = init(&OneTimeKey::from_slice(&[0u8; 32]).unwrap());
 
     poly1305_state.update(&[0u8; 16]).unwrap();
     let _ = poly1305_state.finalize().unwrap();
-    poly1305_state.update(&[0u8; 16]).unwrap();
+    assert!(poly1305_state.update(&[0u8; 16]).is_err());
 }
 
 #[test]

diff --git a/src/hazardous/stream/chacha20.rs b/src/hazardous/stream/chacha20.rs
@@ -69,7 +69,7 @@
 //! ```
 //! use orion::hazardous::stream::chacha20;
 //!
-//! let secret_key = chacha20::SecretKey::generate();
+//! let secret_key = chacha20::SecretKey::generate().unwrap();
 //!
 //! let nonce = chacha20::Nonce::from_slice(&[
 //!     0x07, 0x00, 0x00, 0x00, 0x40, 0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x47,
@@ -546,59 +546,55 @@ fn test_diff_ct_pt_len() {
 }
 
 #[test]
-#[should_panic]
-fn test_err_on_diff_ct_pt_len_xchacha_long() {
+fn test_err_on_diff_ct_pt_len_chacha_long() {
     let mut dst = [0u8; 64];
 
-    encrypt(
+    assert!(encrypt(
         &SecretKey::from_slice(&[0u8; 32]).unwrap(),
         &Nonce::from_slice(&[0u8; 12]).unwrap(),
         0,
         &[0u8; 128],
         &mut dst,
-    ).unwrap();
+    ).is_err());
 }
 
 #[test]
-#[should_panic]
-fn test_err_on_diff_ct_pt_len_xchacha_short() {
+fn test_err_on_diff_ct_pt_len_chacha_short() {
     let mut dst = [0u8; 64];
 
-    encrypt(
+    assert!(encrypt(
         &SecretKey::from_slice(&[0u8; 32]).unwrap(),
         &Nonce::from_slice(&[0u8; 12]).unwrap(),
         0,
         &[0u8; 0],
         &mut dst,
-    ).unwrap();
+    ).is_err());
 }
 
 #[test]
-#[should_panic]
 fn test_err_on_empty_pt() {
     let mut dst = [0u8; 64];
 
-    encrypt(
+    assert!(encrypt(
         &SecretKey::from_slice(&[0u8; 32]).unwrap(),
         &Nonce::from_slice(&[0u8; 12]).unwrap(),
         0,
         &[0u8; 0],
         &mut dst,
-    ).unwrap();
+    ).is_err());
 }
 
 #[test]
-#[should_panic]
 fn test_err_on_initial_counter_overflow() {
     let mut dst = [0u8; 65];
 
-    encrypt(
+    assert!(encrypt(
         &SecretKey::from_slice(&[0u8; 32]).unwrap(),
         &Nonce::from_slice(&[0u8; 12]).unwrap(),
         4294967295,
         &[0u8; 65],
         &mut dst,
-    ).unwrap();
+    ).is_err());
 }
 
 #[test]