Follow the original changes

actix-http/src/cookie/secure/key.rs

@@ -1,4 +1,4 @@
-use ring::hkdf::{HKDF_SHA256, Algorithm, Prk, KeyType};
+use ring::hkdf::{Algorithm, KeyType, Prk, HKDF_SHA256};
 use ring::rand::{SecureRandom, SystemRandom};
 
 use super::private::KEY_LEN as PRIVATE_KEY_LEN;
@@ -55,13 +55,16 @@ impl Key {
     /// ```
     pub fn from_master(master_key: &[u8]) -> Key {
         if master_key.len() < 32 {
-            panic!("bad master key length: expected >= 32 bytes, found {}", master_key.len());
+            panic!(
+                "bad master key length: expected >= 32 bytes, found {}",
+                master_key.len()
+            );
         }
 
         // An empty `Key` structure; will be filled in with HKDF derived keys.
         let mut output_key = Key {
             signing_key: [0; SIGNED_KEY_LEN],
-            encryption_key: [0; PRIVATE_KEY_LEN]
+            encryption_key: [0; PRIVATE_KEY_LEN],
         };
 
         // Expand the master key into two HKDF generated keys.
@@ -71,8 +74,12 @@ impl Key {
         okm.fill(&mut both_keys).expect("fill keys");
 
         // Copy the key parts into their respective fields.
-        output_key.signing_key.copy_from_slice(&both_keys[..SIGNED_KEY_LEN]);
+        output_key
-        output_key.encryption_key.copy_from_slice(&both_keys[SIGNED_KEY_LEN..]);
+            .signing_key
+            .copy_from_slice(&both_keys[..SIGNED_KEY_LEN]);
+        output_key
+            .encryption_key
+            .copy_from_slice(&both_keys[SIGNED_KEY_LEN..]);
         output_key
     }
 

actix-http/src/cookie/secure/private.rs

@@ -1,10 +1,5 @@
-use std::str;
-
-use log::warn;
 use ring::aead::{Aad, Algorithm, Nonce, AES_256_GCM};
-use ring::aead::{OpeningKey, SealingKey};
+use ring::aead::{LessSafeKey, UnboundKey};
-use ring::OpeningKey::open_in_place;
-use ring::SealingKey::seal_in_place;
 use ring::rand::{SecureRandom, SystemRandom};
 
 use super::Key;
@@ -55,23 +50,19 @@ impl<'a> PrivateJar<'a> {
         }
 
         let ad = Aad::from(name.as_bytes());
-        let key = OpeningKey::new(ALGO, &self.key).expect("opening key");
+        let key = LessSafeKey::new(
-        let (nonce, sealed) = data.split_at_mut(NONCE_LEN);
+            UnboundKey::new(&ALGO, &self.key).expect("matching key length"),
+        );
+        let (nonce, mut sealed) = data.split_at_mut(NONCE_LEN);
         let nonce =
             Nonce::try_assume_unique_for_key(nonce).expect("invalid length of `nonce`");
-        let unsealed = open_in_place(&key, nonce, ad, 0, sealed)
+        let unsealed = key
+            .open_in_place(nonce, ad, &mut sealed)
             .map_err(|_| "invalid key/nonce/value: bad seal")?;
 
-        if let Ok(unsealed_utf8) = str::from_utf8(unsealed) {
+        ::std::str::from_utf8(unsealed)
-            Ok(unsealed_utf8.to_string())
+            .map(|s| s.to_string())
-        } else {
+            .map_err(|_| "bad unsealed utf8")
-            warn!(
-                "Private cookie does not have utf8 content!
-It is likely the secret key used to encrypt them has been leaked.
-Please change it as soon as possible."
-            );
-            Err("bad unsealed utf8")
-        }
     }
 
     /// Returns a reference to the `Cookie` inside this jar with the name `name`
@@ -156,16 +147,41 @@ Please change it as soon as possible."
         self.parent.add_original(cookie);
     }
 
-    /// Encrypts the cookie's value with
+    /// Encrypts the cookie's value with authenticated encryption assuring
-    /// authenticated encryption assuring confidentiality, integrity, and authenticity.
+    /// confidentiality, integrity, and authenticity.
     fn encrypt_cookie(&self, cookie: &mut Cookie) {
-        let name = cookie.name().as_bytes();
+        // Create the `LessSafeKey` structure.
-        let value = cookie.value().as_bytes();
+        let unbound = UnboundKey::new(&ALGO, &self.key).expect("matching key length");
-        let data = encrypt_name_value(name, value, &self.key);
+        let key = LessSafeKey::new(unbound);
 
-        // Base64 encode the nonce and encrypted value.
+        // Create a vec to hold the [nonce | cookie value | tag].
-        let sealed_value = base64::encode(&data);
+        let cookie_val = cookie.value().as_bytes();
-        cookie.set_value(sealed_value);
+        let mut data = vec![0; NONCE_LEN + cookie_val.len() + ALGO.tag_len()];
+
+        // Split data into three: nonce, input/output, tag. Copy input.
+        let (nonce, in_out) = data.split_at_mut(NONCE_LEN);
+        let (in_out, tag) = in_out.split_at_mut(cookie_val.len());
+        in_out.copy_from_slice(cookie_val);
+
+        // Randomly generate the nonce into the nonce piece.
+        SystemRandom::new()
+            .fill(nonce)
+            .expect("couldn't random fill nonce");
+        let nonce =
+            Nonce::try_assume_unique_for_key(nonce).expect("invalid `nonce` length");
+
+        // Perform the actual sealing operation, using the cookie's name as
+        // associated data to prevent value swapping.
+        let ad = Aad::from(cookie.name().as_bytes());
+        let ad_tag = key
+            .seal_in_place_separate_tag(nonce, ad, in_out)
+            .expect("in-place seal");
+
+        // Copy the tag into the tag piece.
+        tag.copy_from_slice(ad_tag.as_ref());
+
+        // Base64 encode [none | encrypted value | tag].
+        cookie.set_value(base64::encode(&data));
     }
 
     /// Removes `cookie` from the parent jar.
@@ -196,38 +212,9 @@ Please change it as soon as possible."
     }
 }
 
-fn encrypt_name_value(name: &[u8], value: &[u8], key: &[u8]) -> Vec<u8> {
-    // Create the `SealingKey` structure.
-    let key = SealingKey::new(ALGO, key).expect("sealing key creation");
-
-    // Create a vec to hold the [nonce | cookie value | overhead].
-    let overhead = ALGO.tag_len();
-    let mut data = vec![0; NONCE_LEN + value.len() + overhead];
-
-    // Randomly generate the nonce, then copy the cookie value as input.
-    let (nonce, in_out) = data.split_at_mut(NONCE_LEN);
-    SystemRandom::new()
-        .fill(nonce)
-        .expect("couldn't random fill nonce");
-    in_out[..value.len()].copy_from_slice(value);
-    let nonce =
-        Nonce::try_assume_unique_for_key(nonce).expect("invalid length of `nonce`");
-
-    // Use cookie's name as associated data to prevent value swapping.
-    let ad = Aad::from(name);
-
-    // Perform the actual sealing operation and get the output length.
-    let output_len =
-        seal_in_place(&key, nonce, ad, in_out, overhead).expect("in-place seal");
-
-    // Remove the overhead and return the sealed content.
-    data.truncate(NONCE_LEN + output_len);
-    data
-}
-
 #[cfg(test)]
 mod test {
-    use super::{encrypt_name_value, Cookie, CookieJar, Key};
+    use super::{Cookie, CookieJar, Key};
 
     #[test]
     fn simple() {
@@ -244,28 +231,48 @@ mod test {
     }
 
     #[test]
-    fn non_utf8() {
+    fn roundtrip_tests() {
-        let key = Key::generate();
+        // Secret is 'Super secret!' passed through sha256
+        let secret = b"\x1b\x1c\x6f\x1b\x31\x99\x82\x77\x0e\x05\xb6\x05\x54\x0b\xd9\xea\
+            \x54\x9f\x9a\x56\xf4\x0f\x97\xdc\x6e\xf2\x89\x86\x91\xe0\xa5\x79";
+        let key = Key::from_master(secret);
+
         let mut jar = CookieJar::new();
+        jar.add(Cookie::new(
+            "signed_with_ring014",
+            "3tdHXEQ2kf6fxC7dWzBGmpSLMtJenXLKrZ9cHkSsl1w=Tamper-proof",
+        ));
+        jar.add(Cookie::new(
+            "encrypted_with_ring014",
+            "lObeZJorGVyeSWUA8khTO/8UCzFVBY9g0MGU6/J3NN1R5x11dn2JIA==",
+        ));
+        jar.add(Cookie::new(
+            "signed_with_ring016",
+            "3tdHXEQ2kf6fxC7dWzBGmpSLMtJenXLKrZ9cHkSsl1w=Tamper-proof",
+        ));
+        jar.add(Cookie::new(
+            "encrypted_with_ring016",
+            "SU1ujceILyMBg3fReqRmA9HUtAIoSPZceOM/CUpObROHEujXIjonkA==",
+        ));
 
-        let name = "malicious";
+        let signed = jar.signed(&key);
-        let mut assert_non_utf8 = |value: &[u8]| {
+        assert_eq!(
-            let sealed = encrypt_name_value(name.as_bytes(), value, &key.encryption());
+            signed.get("signed_with_ring014").unwrap().value(),
-            let encoded = base64::encode(&sealed);
+            "Tamper-proof"
-            assert_eq!(
+        );
-                jar.private(&key).unseal(name, &encoded),
+        assert_eq!(
-                Err("bad unsealed utf8")
+            signed.get("signed_with_ring016").unwrap().value(),
-            );
+            "Tamper-proof"
-            jar.add(Cookie::new(name, encoded));
+        );
-            assert_eq!(jar.private(&key).get(name), None);
-        };
 
-        assert_non_utf8(&[0x72, 0xfb, 0xdf, 0x74]); // rûst in ISO/IEC 8859-1
+        let private = jar.private(&key);
-
+        assert_eq!(
-        let mut malicious =
+            private.get("encrypted_with_ring014").unwrap().value(),
-            String::from(r#"{"id":"abc123??%X","admin":true}"#).into_bytes();
+            "Tamper-proof"
-        malicious[8] |= 0b1100_0000;
+        );
-        malicious[9] |= 0b1100_0000;
+        assert_eq!(
-        assert_non_utf8(&malicious);
+            private.get("encrypted_with_ring016").unwrap().value(),
+            "Tamper-proof"
+        );
     }
 }

actix-http/src/cookie/secure/signed.rs

@@ -1,12 +1,11 @@
-use ring::digest::{Algorithm, SHA256};
+use ring::hmac::{self, sign, verify, Algorithm, HMAC_SHA256};
-use ring::hmac::{sign, verify, Key};
 
 use super::Key;
 use crate::cookie::{Cookie, CookieJar};
 
 // Keep these in sync, and keep the key len synced with the `signed` docs as
 // well as the `KEYS_INFO` const in secure::Key.
-static HMAC_DIGEST: &Algorithm = &SHA256;
+static HMAC_DIGEST: Algorithm = HMAC_SHA256;
 const BASE64_DIGEST_LEN: usize = 44;
 pub const KEY_LEN: usize = 32;
 
@@ -21,7 +20,7 @@ pub const KEY_LEN: usize = 32;
 /// This type is only available when the `secure` feature is enabled.
 pub struct SignedJar<'a> {
     parent: &'a mut CookieJar,
-    key: Key,
+    key: hmac::Key,
 }
 
 impl<'a> SignedJar<'a> {
@@ -31,8 +30,8 @@ impl<'a> SignedJar<'a> {
     #[doc(hidden)]
     pub fn new(parent: &'a mut CookieJar, key: &Key) -> SignedJar<'a> {
         SignedJar {
-            parent,
+            parent: parent,
-            key: Key::new(HMAC_DIGEST, key.signing()),
+            key: hmac::Key::new(HMAC_DIGEST, key.signing()),
         }
     }