From fae5ceb5a74dd05039568fcfbfd0dc3b9fb31b46 Mon Sep 17 00:00:00 2001 From: Johannes Date: Sun, 29 Nov 2020 12:25:40 +0100 Subject: [PATCH] Fix not using builder nonce in token encoding (#19) * Add test to detect builder nonce is not used for encoding * Make private encode_with_nonce to allow passing the nonce * Correctly set newly generated nonce on each encode call * Update fuzzer --- fuzz/fuzz_targets/fuzz_branca.rs | 2 +- src/lib.rs | 118 ++++++++++++++++++++++--------- 2 files changed, 85 insertions(+), 35 deletions(-) diff --git a/fuzz/fuzz_targets/fuzz_branca.rs b/fuzz/fuzz_targets/fuzz_branca.rs index f3740cc..bdb18e1 100644 --- a/fuzz/fuzz_targets/fuzz_branca.rs +++ b/fuzz/fuzz_targets/fuzz_branca.rs @@ -18,7 +18,7 @@ fuzz_target!(|data: &[u8]| { csprng.try_fill_bytes(&mut key).unwrap(); - let ctx = Branca::new(&key).unwrap(); + let mut ctx = Branca::new(&key).unwrap(); if !ctx.decode(&String::from_utf8_lossy(data).to_string(), 0).is_err() { panic!("Decoded random string successfully"); diff --git a/src/lib.rs b/src/lib.rs index 55689a7..c33a477 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -40,7 +40,7 @@ use branca::Branca; fn main() { let key = b"supersecretkeyyoushouldnotcommit".to_vec(); - let token = Branca::new(&key).unwrap(); + let mut token = Branca::new(&key).unwrap(); let ciphertext = token.encode(b"Hello World!").unwrap(); let payload = token.decode(ciphertext.as_str(), 0).unwrap(); @@ -176,8 +176,7 @@ impl core::fmt::Debug for Branca { } impl Branca { - /// Create a new Branca struct with a specified key. The length of the key must be exactly 32 bytes. - /// This function panics if unable to securely generate random bytes. + /// Create a new Branca struct with a specified key. The length of the key must be exactly 32 bytes. /// /// `key` - The key to be used for encrypting and decrypting the input. /// @@ -186,8 +185,8 @@ impl Branca { /// use branca::Branca; /// /// fn main() { - /// let key = b"supersecretkeyyoushouldnotcommit".to_vec(); - /// let token = Branca::new(&key); + /// let key = b"supersecretkeyyoushouldnotcommit"; + /// let token = Branca::new(key); /// } ///``` pub fn new(key: &[u8]) -> Result { @@ -196,10 +195,6 @@ impl Branca { return Err(BrancaError::BadKeyLength); } - // Generate Nonce (24 bytes in length) - let mut nonce = vec![0; XCHACHA_NONCESIZE]; - secure_rand_bytes(&mut nonce).unwrap(); - // Generate a timestamp instead of a zero supplied one. let timestamp = SystemTime::now() .duration_since(SystemTime::UNIX_EPOCH) @@ -208,7 +203,7 @@ impl Branca { Ok(Branca { key: key.to_vec(), - nonce, + nonce: Vec::new(), ttl: 0, timestamp, }) @@ -219,6 +214,8 @@ impl Branca { self.key } /// The Branca nonce used with the key for encrypting the message. + /// A new nonce is generated each time `encode()` is called. If `encode()` + /// hasn't been called yet, this returns an empty vector. pub fn nonce(self) -> Vec { self.nonce } @@ -254,6 +251,7 @@ impl Branca { self } /// Encodes the message with the created Branca struct. + /// This function panics if unable to securely generate random bytes. /// /// The contents of the message can be of any arbitrary sequence of bytes, ie. text, JSON, Protobuf, JWT or a MessagePack, etc. /// @@ -265,14 +263,14 @@ impl Branca { /// use branca::Branca; /// /// fn main() { - /// let key = b"supersecretkeyyoushouldnotcommit".to_vec(); - /// let token = Branca::new(&key).unwrap(); + /// let key = b"supersecretkeyyoushouldnotcommit"; + /// let mut token = Branca::new(key).unwrap(); /// /// let ciphertext = token.encode(b"Hello World!").unwrap(); /// // Branca token is now in 'ciphertext' as a String. /// } /// ``` - pub fn encode(&self, message: &[u8]) -> Result { + pub fn encode(&mut self, message: &[u8]) -> Result { let mut timestamp = self.timestamp; if timestamp == 0 { // Generate a timestamp instead of a zero supplied one. @@ -282,7 +280,12 @@ impl Branca { timestamp = ts.as_secs() as u32; } - encode(message, &self.key, timestamp) + // Generate Nonce (24 bytes in length) + let mut nonce = [0; XCHACHA_NONCESIZE]; + secure_rand_bytes(&mut nonce).unwrap(); + self.nonce = nonce.to_vec(); + + encode_with_nonce(message, &self.key, &Nonce::from(nonce), timestamp) } /// Decodes a Branca token with the provided key in the struct. /// @@ -299,7 +302,7 @@ impl Branca { /// use branca::Branca; /// /// fn main() { - /// let token = Branca::new(&b"supersecretkeyyoushouldnotcommit".to_vec()).unwrap(); + /// let mut token = Branca::new(&b"supersecretkeyyoushouldnotcommit".to_vec()).unwrap(); /// let crypted = token.encode(b"Hello World!").unwrap(); /// // Branca token is now in 'crypted' as a String. /// @@ -334,22 +337,32 @@ impl Branca { /// * The key must be 32 bytes in length, otherwise it returns a `BrancaError::BadKeyLength` Result. /// /// * The generated nonce is 24 bytes in length, otherwise it returns a `BrancaError::BadNonceLength` Result. -/// +/// /// * This function panics if unable to securely generate random bytes. pub fn encode(data: &[u8], key: &[u8], timestamp: u32) -> Result { + // Use CSPRNG to generate a unique nonce. + let n = Nonce::generate(); + + encode_with_nonce(data, key, &n, timestamp) +} + +fn encode_with_nonce( + data: &[u8], + key: &[u8], + nonce: &Nonce, + timestamp: u32, +) -> Result { let sk: SecretKey = match SecretKey::from_slice(key) { Ok(key) => key, Err(UnknownCryptoError) => return Err(BrancaError::BadKeyLength), }; - // Use CSPRNG to generate a unique nonce. - let n = Nonce::generate(); // Version || Timestamp || Nonce let mut header = [0u8; 29]; header[0] = VERSION; BigEndian::write_u32(&mut header[1..5], timestamp); - header[5..29].copy_from_slice(n.as_ref()); + header[5..29].copy_from_slice(nonce.as_ref()); let mut buf_crypt = vec![0u8; data.len() + 16 + 29]; // 16 bytes for the Poly1305 Tag. // The header is prepended to the ciphertext and tag. @@ -357,7 +370,7 @@ pub fn encode(data: &[u8], key: &[u8], timestamp: u32) -> Result