Repository: novifinancial/opaque-ke
Pull request #266 (2022-01-25 12:01:52)

Backport `zeroize` fix
As discussed in #264.

daxpedda: CI currently breaks of criterion, there is no way to really fix this. Considering it only applies to the MSRV, I consider this fine, MSRV still builds, criterion is a `dev-dependency`.
kevinlewi: Hmm, not a huge fan of the failing CI. Perhaps we should just update the MSRV and release a 0.7.0?
daxpedda: I don't think this is necessary, I can fix the CI by simply removing the benchmark and therefore the reliance on criterion.

Alternatively I can make another crate (making this a workspace) and put non-MSRV tests there, but I don't consider this worth the effort.
kevinlewi: Ah I see! Yes, please go ahead and remove the benchmark (for the sake of fixing CI), plus the criterion dependency. Will be happy to merge and push a new release afterwards!
daxpedda: After removing criterion I encountered another issue with MSRV: `rustyline`. I didn't want to remove the examples, so continuing to follow down that path, `nix` is using `bitflags` v1, which breaks MSRV. So the latest version of `rustyline` using a version of `nix` that uses an earlier version of `bitflags` then v1, was `rustyline` v1, so I had to downgrade to that.

Turns out `proptest` is also using bitflags` v1, the earliest version of `proptest` not using `bitflags` v1 is v0.3, but that breaks everything ... looking into it.
daxpedda: A lot of downgrading, @kevinlewi should be good to go now.

Patch:
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index b2576a9..06b79a6 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -135,26 +135,6 @@ jobs:
       - name: Run expect (which then runs cargo run)
         run: expect -f scripts/digital_locker.exp
 
-  benches:
-    name: cargo bench compilation
-    runs-on: ubuntu-latest
-    steps:
-      - name: Checkout sources
-        uses: actions/checkout@v2
-
-      - name: Install stable toolchain
-        uses: actions-rs/toolchain@v1
-        with:
-          profile: minimal
-          toolchain: stable
-          override: true
-
-      - name: Run cargo bench --no-run
-        uses: actions-rs/cargo@v1
-        with:
-          command: bench
-          args: --features "bench" --no-run
-
   clippy:
     name: cargo clippy
     runs-on: ubuntu-latest
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 39d0f12..955053a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,9 @@
 # Changelog
 
+## 0.6.1 (January 25, 2022)
+
+* Fix `zeroize` implementing `Drop` on `enum`s now
+
 ## 0.6.0 (June 30, 2021)
 
 * Synced implementation with draft-irtf-cfrg-opaque-05, which changes
diff --git a/Cargo.toml b/Cargo.toml
index 68bcdef..7fbcdf0 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "opaque-ke"
-version = "0.6.0"
+version = "0.6.1"
 repository = "https://github.com/novifinancial/opaque-ke"
 keywords = ["cryptography", "crypto", "opaque", "passwords", "authentication"]
 description = "An implementation of the OPAQUE password-authenticated key exchange protocol"
@@ -31,22 +31,16 @@ rand = "0.8"
 serde = { version = "1", features = ["derive"], optional = true }
 subtle = { version = "2.3.0", default-features = false }
 thiserror = "1.0.22"
-zeroize = { version = "1.1.1", features = ["zeroize_derive"] }
+zeroize = { version = "~1.1", features = ["zeroize_derive"] }
 
 [dev-dependencies]
 anyhow = "1.0.35"
 base64 = "0.13.0"
 bincode = "1"
 chacha20poly1305 = "0.7.1"
-criterion = "0.3.3"
 hex = "0.4.2"
 lazy_static = "1.4.0"
 serde_json = "1.0.60"
 sha2 = "0.9.2"
-proptest = "0.10.1"
-rustyline = "6.3.0"
-
-[[bench]]
-name = "oprf"
-harness = false
-required-features = ["bench"]
+proptest = "0.3"
+rustyline = "1"
diff --git a/README.md b/README.md
index 41443d8..358ab39 100644
--- a/README.md
+++ b/README.md
@@ -22,7 +22,7 @@ Installation
 Add the following line to the dependencies of your `Cargo.toml`:
 
 ```
-opaque-ke = "0.6.0"
+opaque-ke = "0.6.1"
 ```
 
 Resources
diff --git a/benches/oprf.rs b/benches/oprf.rs
deleted file mode 100644
index 9e9fbc6..0000000
--- a/benches/oprf.rs
+++ /dev/null
@@ -1,68 +0,0 @@
-// Copyright (c) Facebook, Inc. and its affiliates.
-//
-// This source code is licensed under the MIT license found in the
-// LICENSE file in the root directory of this source tree.
-
-#[macro_use]
-extern crate criterion;
-
-use criterion::Criterion;
-use curve25519_dalek::ristretto::RistrettoPoint;
-use generic_array::arr;
-use opaque_ke::{
-    group::Group,
-    oprf::{blind_shim, evaluate_shim, finalize_shim},
-};
-use rand::{prelude::ThreadRng, thread_rng};
-use sha2::Sha512;
-
-fn oprf1(c: &mut Criterion) {
-    let mut csprng: ThreadRng = thread_rng();
-    let input = b"hunter2";
-
-    c.bench_function("blind with Ristretto", move |b| {
-        b.iter(|| {
-            blind_shim::<_, RistrettoPoint, Sha512>(&input[..], &mut csprng).unwrap();
-        })
-    });
-}
-
-fn oprf2(c: &mut Criterion) {
-    let mut csprng: ThreadRng = thread_rng();
-    let input = b"hunter2";
-
-    let (_, alpha) = blind_shim::<_, RistrettoPoint, Sha512>(&input[..], &mut csprng).unwrap();
-    let salt_bytes = arr![
-        u8; 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23,
-        24, 25, 26, 27, 28, 29, 30, 31, 32,
-    ];
-    let salt = RistrettoPoint::from_scalar_slice(&salt_bytes).unwrap();
-
-    c.bench_function("evaluate with Ristretto", move |b| {
-        b.iter(|| {
-            let _beta = evaluate_shim::<RistrettoPoint>(alpha, &salt);
-        })
-    });
-}
-
-fn oprf3(c: &mut Criterion) {
-    let mut csprng: ThreadRng = thread_rng();
-    let input = b"hunter2";
-
-    let (token, alpha) = blind_shim::<_, RistrettoPoint, Sha512>(&input[..], &mut csprng).unwrap();
-    let salt_bytes = arr![
-        u8; 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23,
-        24, 25, 26, 27, 28, 29, 30, 31, 32,
-    ];
-    let salt = RistrettoPoint::from_scalar_slice(&salt_bytes).unwrap();
-    let beta = evaluate_shim::<RistrettoPoint>(alpha, &salt);
-
-    c.bench_function("finalize with Ristretto", move |b| {
-        b.iter(|| {
-            let _res = finalize_shim::<RistrettoPoint, Sha512>(&token, beta).unwrap();
-        })
-    });
-}
-
-criterion_group!(oprf_benches, oprf1, oprf2, oprf3);
-criterion_main!(oprf_benches);
diff --git a/src/envelope.rs b/src/envelope.rs
index e1f71cb..1170ca8 100644
--- a/src/envelope.rs
+++ b/src/envelope.rs
@@ -61,7 +61,7 @@ fn recover_keys_internal<CS: CipherSuite>(
     Ok(client_static_keypair)
 }
 
-#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, Zeroize)]
+#[derive(Clone, Debug, Eq, Hash, PartialEq, Zeroize)]
 #[zeroize(drop)]
 pub(crate) enum InnerEnvelopeMode {
     Zero = 0,
@@ -98,7 +98,7 @@ pub(crate) struct Envelope<CS: CipherSuite> {
 impl<CS: CipherSuite> Clone for Envelope<CS> {
     fn clone(&self) -> Self {
         Self {
-            mode: self.mode,
+            mode: self.mode.clone(),
             nonce: self.nonce.clone(),
             hmac: self.hmac.clone(),
         }
diff --git a/src/key_exchange/tripledh.rs b/src/key_exchange/tripledh.rs
index fa7debf..bef2b2b 100644
--- a/src/key_exchange/tripledh.rs
+++ b/src/key_exchange/tripledh.rs
@@ -170,7 +170,7 @@ impl<D: Hash, G: Group> KeyExchange<D, G> for TripleDH {
             ));
         }
 
-        transcript_hasher.update(ke2_message.mac.to_vec());
+        transcript_hasher.update(ke2_message.mac);
 
         let mut client_mac =
             Hmac::<D>::new_from_slice(&km3).map_err(|_| InternalPakeError::HmacError)?;
diff --git a/src/keypair.rs b/src/keypair.rs
index 798487d..c0363fb 100644
--- a/src/keypair.rs
+++ b/src/keypair.rs
@@ -144,10 +144,10 @@ impl<G: Group + Debug> KeyPair<G> {
     fn uniform_keypair_strategy() -> BoxedStrategy<Self> {
         // The no_shrink is because keypairs should be fixed -- shrinking would cause a different
         // keypair to be generated, which appears to not be very useful.
-        any::<[u8; 32]>()
-            .prop_filter_map("valid random keypair", |seed| {
+        prop::array::uniform32(0_u8..)
+            .prop_map(|seed| {
                 let mut rng = StdRng::from_seed(seed);
-                Some(Self::generate_random(&mut rng))
+                Self::generate_random(&mut rng)
             })
             .no_shrink()
             .boxed()
@@ -281,21 +281,21 @@ mod tests {
 
     proptest! {
         #[test]
-        fn test_ristretto_check(kp in KeyPair::<RistrettoPoint>::uniform_keypair_strategy()) {
+        fn test_ristretto_check(ref kp in KeyPair::<RistrettoPoint>::uniform_keypair_strategy()) {
             let pk = kp.public();
             prop_assert!(KeyPair::<RistrettoPoint>::check_public_key(pk.clone()).is_ok());
         }
 
         #[test]
-        fn test_ristretto_pub_from_priv(kp in KeyPair::<RistrettoPoint>::uniform_keypair_strategy()) {
+        fn test_ristretto_pub_from_priv(ref kp in KeyPair::<RistrettoPoint>::uniform_keypair_strategy()) {
             let pk = kp.public();
             let sk = kp.private();
             prop_assert_eq!(&KeyPair::<RistrettoPoint>::public_from_private(sk), pk);
         }
 
         #[test]
-        fn test_ristretto_dh(kp1 in KeyPair::<RistrettoPoint>::uniform_keypair_strategy(),
-                          kp2 in KeyPair::<RistrettoPoint>::uniform_keypair_strategy()) {
+        fn test_ristretto_dh(ref kp1 in KeyPair::<RistrettoPoint>::uniform_keypair_strategy(),
+        ref kp2 in KeyPair::<RistrettoPoint>::uniform_keypair_strategy()) {
 
             let dh1 = KeyPair::<RistrettoPoint>::diffie_hellman(kp1.public().clone(), kp2.private().clone())?;
             let dh2 = KeyPair::<RistrettoPoint>::diffie_hellman(kp2.public().clone(), kp1.private().clone())?;
@@ -304,7 +304,7 @@ mod tests {
         }
 
         #[test]
-        fn test_private_key_slice(kp in KeyPair::<RistrettoPoint>::uniform_keypair_strategy()) {
+        fn test_private_key_slice(ref kp in KeyPair::<RistrettoPoint>::uniform_keypair_strategy()) {
             let sk_bytes = kp.private().to_vec();
 
             let kp2 = KeyPair::<RistrettoPoint>::from_private_key_slice(&sk_bytes)?;
diff --git a/src/lib.rs b/src/lib.rs
index 8857e2c..ae8f274 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -747,6 +747,10 @@ compile_error!(
      please enable one of: u64_backend, u32_backend"
 );
 
+#[cfg(test)]
+#[macro_use]
+extern crate proptest;
+
 // Error types
 pub mod errors;
 


Test patch:
diff --git a/src/serialization/tests.rs b/src/serialization/tests.rs
index aea155d..acd606d 100644
--- a/src/serialization/tests.rs
+++ b/src/serialization/tests.rs
@@ -346,57 +346,57 @@ fn ke3_message_roundtrip() {
 proptest! {
 
 #[test]
-fn test_i2osp_os2ip(bytes in vec(any::<u8>(), 0..std::mem::size_of::<usize>())) {
-    assert_eq!(i2osp(os2ip(&bytes)?, bytes.len()), bytes);
+fn test_i2osp_os2ip(ref bytes in vec(prop::num::u8::ANY, 0..std::mem::size_of::<usize>())) {
+    assert_eq!(&i2osp(os2ip(&bytes)?, bytes.len()), bytes);
 }
 
 #[test]
-fn test_nocrash_registration_request(bytes in vec(any::<u8>(), 0..200)) {
+fn test_nocrash_registration_request(ref bytes in vec(prop::num::u8::ANY, 0..200)) {
     RegistrationRequest::<Default>::deserialize(&bytes[..]).map_or(true, |_| true);
 }
 
 #[test]
-fn test_nocrash_registration_response(bytes in vec(any::<u8>(), 0..200)) {
+fn test_nocrash_registration_response(ref bytes in vec(prop::num::u8::ANY, 0..200)) {
     RegistrationResponse::<Default>::deserialize(&bytes[..]).map_or(true, |_| true);
 }
 
 #[test]
-fn test_nocrash_registration_upload(bytes in vec(any::<u8>(), 0..200)) {
+fn test_nocrash_registration_upload(ref bytes in vec(prop::num::u8::ANY, 0..200)) {
     RegistrationUpload::<Default>::deserialize(&bytes[..]).map_or(true, |_| true);
 }
 
 #[test]
-fn test_nocrash_credential_request(bytes in vec(any::<u8>(), 0..500)) {
+fn test_nocrash_credential_request(ref bytes in vec(prop::num::u8::ANY, 0..500)) {
     CredentialRequest::<Default>::deserialize(&bytes[..]).map_or(true, |_| true);
 }
 
 #[test]
-fn test_nocrash_credential_response(bytes in vec(any::<u8>(), 0..500)) {
+fn test_nocrash_credential_response(ref bytes in vec(prop::num::u8::ANY, 0..500)) {
     CredentialResponse::<Default>::deserialize(&bytes[..]).map_or(true, |_| true);
 }
 
 #[test]
-fn test_nocrash_credential_finalization(bytes in vec(any::<u8>(), 0..500)) {
+fn test_nocrash_credential_finalization(ref bytes in vec(prop::num::u8::ANY, 0..500)) {
     CredentialFinalization::<Default>::deserialize(&bytes[..]).map_or(true, |_| true);
 }
 
 #[test]
-fn test_nocrash_client_registration(bytes in vec(any::<u8>(), 0..700)) {
+fn test_nocrash_client_registration(ref bytes in vec(prop::num::u8::ANY, 0..700)) {
     ClientRegistration::<Default>::deserialize(&bytes[..]).map_or(true, |_| true);
 }
 
 #[test]
-fn test_nocrash_server_registration(bytes in vec(any::<u8>(), 0..700)) {
+fn test_nocrash_server_registration(ref bytes in vec(prop::num::u8::ANY, 0..700)) {
     ServerRegistration::<Default>::deserialize(&bytes[..]).map_or(true, |_| true);
 }
 
 #[test]
-fn test_nocrash_client_login(bytes in vec(any::<u8>(), 0..700)) {
+fn test_nocrash_client_login(ref bytes in vec(prop::num::u8::ANY, 0..700)) {
     ClientLogin::<Default>::deserialize(&bytes[..]).map_or(true, |_| true);
 }
 
 #[test]
-fn test_nocrash_server_login(bytes in vec(any::<u8>(), 0..700)) {
+fn test_nocrash_server_login(ref bytes in vec(prop::num::u8::ANY, 0..700)) {
     ServerLogin::<Default>::deserialize(&bytes[..]).map_or(true, |_| true);
 }