From 0e54b85eaa1008bf3f8a3127d28784faee80e031 Mon Sep 17 00:00:00 2001
From: Jack O'Connor <jack.oconnor@zoom.us>
Date: Tue, 22 Dec 2020 11:05:24 -0500
Subject: [PATCH] derive Copy and Clone for UnboundKey

OpeningKey and SealingKey intentionally avoid implementing Copy and
Clone, because they're attached to a nonce sequence that should be
unique. UnboundKey isn't attached to a nonce sequence, though, and in
the case of AES it also takes some nontrivial key setup work to
construct from raw bytes. Making it Copy and Clone allows lets callers
avoid repeating that work in cases where a key is reused without a fixed
nonce sequence.
---
 src/aead.rs         |  2 ++
 src/aead/aes.rs     |  2 ++
 src/aead/aes_gcm.rs |  1 +
 src/aead/chacha.rs  |  1 +
 src/aead/gcm.rs     |  5 +++--
 tests/aead_tests.rs | 44 ++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/src/aead.rs b/src/aead.rs
index 4d6bdb7904..cb3fb11dd3 100644
--- a/src/aead.rs
+++ b/src/aead.rs
@@ -384,6 +384,7 @@ impl Aad<[u8; 0]> {
 }
 
 /// An AEAD key without a designated role or nonce sequence.
+#[derive(Copy, Clone)]
 pub struct UnboundKey {
     inner: KeyInner,
     algorithm: &'static Algorithm,
@@ -399,6 +400,7 @@ impl core::fmt::Debug for UnboundKey {
 }
 
 #[allow(clippy::large_enum_variant, variant_size_differences)]
+#[derive(Copy, Clone)]
 enum KeyInner {
     AesGcm(aes_gcm::Key),
     ChaCha20Poly1305(chacha20_poly1305::Key),
diff --git a/src/aead/aes.rs b/src/aead/aes.rs
index 5028e242c2..186cda6189 100644
--- a/src/aead/aes.rs
+++ b/src/aead/aes.rs
@@ -15,6 +15,7 @@
 use super::{counter, iv::Iv, quic::Sample, Block, Direction, BLOCK_LEN};
 use crate::{bits::BitLength, c, cpu, endian::*, error, polyfill};
 
+#[derive(Copy, Clone)]
 pub(crate) struct Key {
     inner: AES_KEY,
     cpu_features: cpu::Features,
@@ -322,6 +323,7 @@ impl Key {
 
 // Keep this in sync with AES_KEY in aes.h.
 #[repr(C)]
+#[derive(Copy, Clone)]
 pub(super) struct AES_KEY {
     pub rd_key: [u32; 4 * (MAX_ROUNDS + 1)],
     pub rounds: c::uint,
diff --git a/src/aead/aes_gcm.rs b/src/aead/aes_gcm.rs
index b225e76821..ec4439b73d 100644
--- a/src/aead/aes_gcm.rs
+++ b/src/aead/aes_gcm.rs
@@ -38,6 +38,7 @@ pub static AES_256_GCM: aead::Algorithm = aead::Algorithm {
     max_input_len: AES_GCM_MAX_INPUT_LEN,
 };
 
+#[derive(Copy, Clone)]
 pub struct Key {
     gcm_key: gcm::Key, // First because it has a large alignment requirement.
     aes_key: aes::Key,
diff --git a/src/aead/chacha.rs b/src/aead/chacha.rs
index 5e015083b1..cbf4ff4e35 100644
--- a/src/aead/chacha.rs
+++ b/src/aead/chacha.rs
@@ -17,6 +17,7 @@ use super::{counter, iv::Iv, quic::Sample, BLOCK_LEN};
 use crate::{c, endian::*};
 
 #[repr(transparent)]
+#[derive(Copy, Clone)]
 pub struct Key([LittleEndian<u32>; KEY_LEN / 4]);
 
 impl From<[u8; KEY_LEN]> for Key {
diff --git a/src/aead/gcm.rs b/src/aead/gcm.rs
index 6696f776dd..7c956739dd 100644
--- a/src/aead/gcm.rs
+++ b/src/aead/gcm.rs
@@ -18,6 +18,7 @@ use crate::cpu;
 #[cfg(not(target_arch = "aarch64"))]
 mod gcm_nohw;
 
+#[derive(Copy, Clone)]
 pub struct Key(HTable);
 
 impl Key {
@@ -86,7 +87,7 @@ impl Context {
             inner: ContextInner {
                 Xi: Xi(Block::zero()),
                 _unused: Block::zero(),
-                Htable: key.0.clone(),
+                Htable: key.0,
             },
             cpu_features,
         };
@@ -234,7 +235,7 @@ impl Context {
 }
 
 // The alignment is required by non-Rust code that uses `GCM128_CONTEXT`.
-#[derive(Clone)]
+#[derive(Copy, Clone)]
 #[repr(C, align(16))]
 struct HTable {
     Htable: [u128; HTABLE_LEN],
diff --git a/tests/aead_tests.rs b/tests/aead_tests.rs
index 75e0e9e92d..8dc11ac678 100644
--- a/tests/aead_tests.rs
+++ b/tests/aead_tests.rs
@@ -427,6 +427,50 @@ fn test_aead_key_debug() {
     );
 }
 
+#[test]
+#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
+fn test_aead_unboundkey_copy_clone() {
+    let key_bytes = [0; 32];
+    let nonce = [0; aead::NONCE_LEN];
+
+    let key1 = aead::UnboundKey::new(&aead::AES_256_GCM, &key_bytes).unwrap();
+    // Copy and clone key1.
+    let key2 = key1;
+    #[allow(clippy::clone_on_copy)]
+    let key3 = key1.clone();
+
+    // UnboundKey doesn't support AsRef or PartialEq, so instead just check that all three keys
+    // produce the same encrypted output.
+    let mut buf1 = [0; 32];
+    let tag1 = aead::LessSafeKey::new(key1)
+        .seal_in_place_separate_tag(
+            aead::Nonce::try_assume_unique_for_key(&nonce).unwrap(),
+            aead::Aad::empty(),
+            &mut buf1,
+        )
+        .unwrap();
+    let mut buf2 = [0; 32];
+    let tag2 = aead::LessSafeKey::new(key2)
+        .seal_in_place_separate_tag(
+            aead::Nonce::try_assume_unique_for_key(&nonce).unwrap(),
+            aead::Aad::empty(),
+            &mut buf2,
+        )
+        .unwrap();
+    let mut buf3 = [0; 32];
+    let tag3 = aead::LessSafeKey::new(key3)
+        .seal_in_place_separate_tag(
+            aead::Nonce::try_assume_unique_for_key(&nonce).unwrap(),
+            aead::Aad::empty(),
+            &mut buf3,
+        )
+        .unwrap();
+    assert_eq!(tag1.as_ref(), tag2.as_ref());
+    assert_eq!(tag1.as_ref(), tag3.as_ref());
+    assert_eq!(buf1, buf2);
+    assert_eq!(buf1, buf3);
+}
+
 fn make_key<K: aead::BoundKey<OneNonceSequence>>(
     algorithm: &'static aead::Algorithm,
     key: &[u8],