Separate file crypto wrappers, starting with AES-CBC (#298)

* refactor key wrapping with tests

* remove backwards compatiblity tests

* adds AES-CBC tests for IV and RNG

src/ctap/crypto_wrapper.rs

@@ -0,0 +1,147 @@
+// Copyright 2021 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use crate::ctap::status_code::Ctap2StatusCode;
+use alloc::vec;
+use alloc::vec::Vec;
+use crypto::cbc::{cbc_decrypt, cbc_encrypt};
+use crypto::rng256::Rng256;
+
+/// Wraps the AES256-CBC encryption to match what we need in CTAP.
+pub fn aes256_cbc_encrypt(
+    rng: &mut dyn Rng256,
+    aes_enc_key: &crypto::aes256::EncryptionKey,
+    plaintext: &[u8],
+    embeds_iv: bool,
+) -> Result<Vec<u8>, Ctap2StatusCode> {
+    if plaintext.len() % 16 != 0 {
+        return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER);
+    }
+    let iv = if embeds_iv {
+        let random_bytes = rng.gen_uniform_u8x32();
+        *array_ref!(random_bytes, 0, 16)
+    } else {
+        [0u8; 16]
+    };
+    let mut blocks = Vec::with_capacity(plaintext.len() / 16);
+    // TODO(https://github.com/rust-lang/rust/issues/74985) Use array_chunks when stable.
+    for block in plaintext.chunks_exact(16) {
+        blocks.push(*array_ref!(block, 0, 16));
+    }
+    cbc_encrypt(aes_enc_key, iv, &mut blocks);
+    let mut ciphertext = if embeds_iv { iv.to_vec() } else { vec![] };
+    ciphertext.extend(blocks.iter().flatten());
+    Ok(ciphertext)
+}
+
+/// Wraps the AES256-CBC decryption to match what we need in CTAP.
+pub fn aes256_cbc_decrypt(
+    aes_enc_key: &crypto::aes256::EncryptionKey,
+    ciphertext: &[u8],
+    embeds_iv: bool,
+) -> Result<Vec<u8>, Ctap2StatusCode> {
+    if ciphertext.len() % 16 != 0 {
+        return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER);
+    }
+    let mut block_len = ciphertext.len() / 16;
+    // TODO(https://github.com/rust-lang/rust/issues/74985) Use array_chunks when stable.
+    let mut block_iter = ciphertext.chunks_exact(16);
+    let iv = if embeds_iv {
+        block_len -= 1;
+        let iv_block = block_iter
+            .next()
+            .ok_or(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER)?;
+        *array_ref!(iv_block, 0, 16)
+    } else {
+        [0u8; 16]
+    };
+    let mut blocks = Vec::with_capacity(block_len);
+    for block in block_iter {
+        blocks.push(*array_ref!(block, 0, 16));
+    }
+    let aes_dec_key = crypto::aes256::DecryptionKey::new(aes_enc_key);
+    cbc_decrypt(&aes_dec_key, iv, &mut blocks);
+    Ok(blocks.iter().flatten().cloned().collect::<Vec<u8>>())
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use crypto::rng256::ThreadRng256;
+
+    #[test]
+    fn test_encrypt_decrypt_with_iv() {
+        let mut rng = ThreadRng256 {};
+        let aes_enc_key = crypto::aes256::EncryptionKey::new(&[0xC2; 32]);
+        let plaintext = vec![0xAA; 64];
+        let ciphertext = aes256_cbc_encrypt(&mut rng, &aes_enc_key, &plaintext, true).unwrap();
+        let decrypted = aes256_cbc_decrypt(&aes_enc_key, &ciphertext, true).unwrap();
+        assert_eq!(decrypted, plaintext);
+    }
+
+    #[test]
+    fn test_encrypt_decrypt_without_iv() {
+        let mut rng = ThreadRng256 {};
+        let aes_enc_key = crypto::aes256::EncryptionKey::new(&[0xC2; 32]);
+        let plaintext = vec![0xAA; 64];
+        let ciphertext = aes256_cbc_encrypt(&mut rng, &aes_enc_key, &plaintext, false).unwrap();
+        let decrypted = aes256_cbc_decrypt(&aes_enc_key, &ciphertext, false).unwrap();
+        assert_eq!(decrypted, plaintext);
+    }
+
+    #[test]
+    fn test_correct_iv_usage() {
+        let mut rng = ThreadRng256 {};
+        let aes_enc_key = crypto::aes256::EncryptionKey::new(&[0xC2; 32]);
+        let plaintext = vec![0xAA; 64];
+        let mut ciphertext_no_iv =
+            aes256_cbc_encrypt(&mut rng, &aes_enc_key, &plaintext, false).unwrap();
+        let mut ciphertext_with_iv = vec![0u8; 16];
+        ciphertext_with_iv.append(&mut ciphertext_no_iv);
+        let decrypted = aes256_cbc_decrypt(&aes_enc_key, &ciphertext_with_iv, true).unwrap();
+        assert_eq!(decrypted, plaintext);
+    }
+
+    #[test]
+    fn test_iv_manipulation_property() {
+        let mut rng = ThreadRng256 {};
+        let aes_enc_key = crypto::aes256::EncryptionKey::new(&[0xC2; 32]);
+        let plaintext = vec![0xAA; 64];
+        let mut ciphertext = aes256_cbc_encrypt(&mut rng, &aes_enc_key, &plaintext, true).unwrap();
+        let mut expected_plaintext = plaintext;
+        for i in 0..16 {
+            ciphertext[i] ^= 0xBB;
+            expected_plaintext[i] ^= 0xBB;
+        }
+        let decrypted = aes256_cbc_decrypt(&aes_enc_key, &ciphertext, true).unwrap();
+        assert_eq!(decrypted, expected_plaintext);
+    }
+
+    #[test]
+    fn test_chaining() {
+        let mut rng = ThreadRng256 {};
+        let aes_enc_key = crypto::aes256::EncryptionKey::new(&[0xC2; 32]);
+        let plaintext = vec![0xAA; 64];
+        let ciphertext1 = aes256_cbc_encrypt(&mut rng, &aes_enc_key, &plaintext, true).unwrap();
+        let ciphertext2 = aes256_cbc_encrypt(&mut rng, &aes_enc_key, &plaintext, true).unwrap();
+        assert_eq!(ciphertext1.len(), 80);
+        assert_eq!(ciphertext2.len(), 80);
+        // The ciphertext should mutate in all blocks with a different IV.
+        let block_iter1 = ciphertext1.chunks_exact(16);
+        let block_iter2 = ciphertext2.chunks_exact(16);
+        for (block1, block2) in block_iter1.zip(block_iter2) {
+            assert_ne!(block1, block2);
+        }
+    }
+}

src/ctap/mod.rs

@@ -17,6 +17,7 @@ mod client_pin;
 pub mod command;
 mod config_command;
 mod credential_management;
+mod crypto_wrapper;
 #[cfg(feature = "with_ctap1")]
 mod ctap1;
 mod customization;
@@ -38,6 +39,7 @@ use self::command::{
 };
 use self::config_command::process_config;
 use self::credential_management::process_credential_management;
+use self::crypto_wrapper::{aes256_cbc_decrypt, aes256_cbc_encrypt};
 use self::customization::{
     DEFAULT_CRED_PROTECT, ENTERPRISE_ATTESTATION_MODE, ENTERPRISE_RP_ID_LIST,
     MAX_CREDENTIAL_COUNT_IN_LIST, MAX_CRED_BLOB_LENGTH, MAX_LARGE_BLOB_ARRAY_SIZE,
@@ -71,7 +73,6 @@ use cbor::cbor_map_options;
 use core::convert::TryFrom;
 #[cfg(feature = "debug_ctap")]
 use core::fmt::Write;
-use crypto::cbc::{cbc_decrypt, cbc_encrypt};
 use crypto::hmac::{hmac_256, verify_hmac_256};
 use crypto::rng256::Rng256;
 use crypto::sha256::Sha256;
@@ -338,23 +339,11 @@ where
     ) -> Result<Vec<u8>, Ctap2StatusCode> {
         let master_keys = self.persistent_store.master_keys()?;
         let aes_enc_key = crypto::aes256::EncryptionKey::new(&master_keys.encryption);
-        let mut sk_bytes = [0; 32];
+        let mut plaintext = [0; 64];
-        private_key.to_bytes(&mut sk_bytes);
+        private_key.to_bytes(array_mut_ref!(plaintext, 0, 32));
-        let mut iv = [0; 16];
+        plaintext[32..64].copy_from_slice(application);
-        iv.copy_from_slice(&self.rng.gen_uniform_u8x32()[..16]);
 
-        let mut blocks = [[0u8; 16]; 4];
+        let mut encrypted_id = aes256_cbc_encrypt(self.rng, &aes_enc_key, &plaintext, true)?;
-        blocks[0].copy_from_slice(&sk_bytes[..16]);
-        blocks[1].copy_from_slice(&sk_bytes[16..]);
-        blocks[2].copy_from_slice(&application[..16]);
-        blocks[3].copy_from_slice(&application[16..]);
-        cbc_encrypt(&aes_enc_key, iv, &mut blocks);
-
-        let mut encrypted_id = Vec::with_capacity(0x70);
-        encrypted_id.extend(&iv);
-        for b in &blocks {
-            encrypted_id.extend(b);
-        }
         let id_hmac = hmac_256::<Sha256>(&master_keys.hmac, &encrypted_id[..]);
         encrypted_id.extend(&id_hmac);
         Ok(encrypted_id)
@@ -381,26 +370,12 @@ where
             return Ok(None);
         }
         let aes_enc_key = crypto::aes256::EncryptionKey::new(&master_keys.encryption);
-        let aes_dec_key = crypto::aes256::DecryptionKey::new(&aes_enc_key);
-        let mut iv = [0; 16];
-        iv.copy_from_slice(&credential_id[..16]);
-        let mut blocks = [[0u8; 16]; 4];
-        for i in 0..4 {
-            blocks[i].copy_from_slice(&credential_id[16 * (i + 1)..16 * (i + 2)]);
-        }
 
-        cbc_decrypt(&aes_dec_key, iv, &mut blocks);
+        let decrypted_id = aes256_cbc_decrypt(&aes_enc_key, &credential_id[..payload_size], true)?;
-        let mut decrypted_sk = [0; 32];
+        if rp_id_hash != &decrypted_id[32..64] {
-        let mut decrypted_rp_id_hash = [0; 32];
-        decrypted_sk[..16].clone_from_slice(&blocks[0]);
-        decrypted_sk[16..].clone_from_slice(&blocks[1]);
-        decrypted_rp_id_hash[..16].clone_from_slice(&blocks[2]);
-        decrypted_rp_id_hash[16..].clone_from_slice(&blocks[3]);
-        if rp_id_hash != decrypted_rp_id_hash {
             return Ok(None);
         }
-
+        let sk_option = crypto::ecdsa::SecKey::from_bytes(array_ref!(decrypted_id, 0, 32));
-        let sk_option = crypto::ecdsa::SecKey::from_bytes(&decrypted_sk);
         Ok(sk_option.map(|sk| PublicKeyCredentialSource {
             key_type: PublicKeyCredentialType::PublicKey,
             credential_id,

src/ctap/pin_protocol.rs

@@ -13,13 +13,12 @@
 // limitations under the License.
 
 use crate::ctap::client_pin::PIN_TOKEN_LENGTH;
+use crate::ctap::crypto_wrapper::{aes256_cbc_decrypt, aes256_cbc_encrypt};
 use crate::ctap::data_formats::{CoseKey, PinUvAuthProtocol};
 use crate::ctap::status_code::Ctap2StatusCode;
 use alloc::boxed::Box;
-use alloc::vec;
 use alloc::vec::Vec;
 use core::convert::TryInto;
-use crypto::cbc::{cbc_decrypt, cbc_encrypt};
 use crypto::hkdf::hkdf_empty_salt_256;
 #[cfg(test)]
 use crypto::hmac::hmac_256;
@@ -135,61 +134,6 @@ pub trait SharedSecret {
     fn authenticate(&self, message: &[u8]) -> Vec<u8>;
 }
 
-fn aes256_cbc_encrypt(
-    rng: &mut dyn Rng256,
-    aes_enc_key: &crypto::aes256::EncryptionKey,
-    plaintext: &[u8],
-    has_iv: bool,
-) -> Result<Vec<u8>, Ctap2StatusCode> {
-    if plaintext.len() % 16 != 0 {
-        return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER);
-    }
-    let iv = if has_iv {
-        let random_bytes = rng.gen_uniform_u8x32();
-        *array_ref!(random_bytes, 0, 16)
-    } else {
-        [0u8; 16]
-    };
-    let mut blocks = Vec::with_capacity(plaintext.len() / 16);
-    // TODO(https://github.com/rust-lang/rust/issues/74985) Use array_chunks when stable.
-    for block in plaintext.chunks_exact(16) {
-        blocks.push(*array_ref!(block, 0, 16));
-    }
-    cbc_encrypt(aes_enc_key, iv, &mut blocks);
-    let mut ciphertext = if has_iv { iv.to_vec() } else { vec![] };
-    ciphertext.extend(blocks.iter().flatten());
-    Ok(ciphertext)
-}
-
-fn aes256_cbc_decrypt(
-    aes_enc_key: &crypto::aes256::EncryptionKey,
-    ciphertext: &[u8],
-    has_iv: bool,
-) -> Result<Vec<u8>, Ctap2StatusCode> {
-    if ciphertext.len() % 16 != 0 {
-        return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER);
-    }
-    let mut block_len = ciphertext.len() / 16;
-    // TODO(https://github.com/rust-lang/rust/issues/74985) Use array_chunks when stable.
-    let mut block_iter = ciphertext.chunks_exact(16);
-    let iv = if has_iv {
-        block_len -= 1;
-        let iv_block = block_iter
-            .next()
-            .ok_or(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER)?;
-        *array_ref!(iv_block, 0, 16)
-    } else {
-        [0u8; 16]
-    };
-    let mut blocks = Vec::with_capacity(block_len);
-    for block in block_iter {
-        blocks.push(*array_ref!(block, 0, 16));
-    }
-    let aes_dec_key = crypto::aes256::DecryptionKey::new(aes_enc_key);
-    cbc_decrypt(&aes_dec_key, iv, &mut blocks);
-    Ok(blocks.iter().flatten().cloned().collect::<Vec<u8>>())
-}
-
 fn verify_v1(key: &[u8], message: &[u8], signature: &[u8]) -> Result<(), Ctap2StatusCode> {
     if signature.len() != 16 {
         return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER);