Repository: facebook/opaque-ke (language: rust)

Issue:
SlowHash configurability
At the moment, it isn't possible to pass parameters to `SlowHash::hash()`, the only way I could figure out is to make a new type implementing `SlowHash` that just executes a different function.

Would it be in scope to pass down a instantiated `CipherSuite::SlowHash` to `SlowHash::hash()` through either an additional parameter in `ClientRegistration/ClientLogin::finish()` or adding something to `ClientRegistrationFinishParameters/ClientLoginFinishParameters`?


Discussion / hints:
I see... just to clarify: are these parameters which must be set upon calling `ClientRegistration/ClientLogin::finish()`, and cannot be set during the setup / configuration phase? Feel free to open a PR with what you have in mind.
> I see... just to clarify: are these parameters which must be set upon calling `ClientRegistration/ClientLogin::finish()`, and cannot be set during the setup / configuration phase?

No, they could be setup at any time, I just don't know where best to place it.
> Feel free to open a PR with what you have in mind.

Will do.

@daxpedda, some more thoughts on this after seeing the PR: ~~can't we already support custom configurations of SlowHash without having to make any changes to master?~~ perhaps this can be supported by just making a small change to the function signature for the `hash` function of the `SlowHash` trait, by adding a `&self` parameter?

For instance:

```
struct Scrypt {
  n: u32,
  r: u32,
  p: u32,
}

impl Scrypt {
  fn new(n: u32, r: u32, p: u32) -> Self {
    Self { n, r, p }
}

impl<D: Hash> SlowHash<D> for Scrypt {
  fn hash(
    &self,
    input: GenericArray<u8, <D as Digest>::OutputSize>,
  ) -> Result<Vec<u8>, InternalPakeError> {
    // use n,r,p as set by new()
    let params =
      scrypt::ScryptParams::new(self.n, self.r, self.p)
        .map_err(|_| InternalPakeError::SlowHashError)?;
    let mut output = vec![0u8; <D as Digest>::OutputSize::to_usize()];
    scrypt::scrypt(&input, &[], &params, &mut output)
      .map_err(|_| InternalPakeError::SlowHashError)?;
    Ok(output)
  }
}
```
Actually nevermind, please ignore the above, I realized why it would not work as I had initially expected. I am still looking for a way to keep all of the parameters in the ciphersuite configuration rather than passing them into the finish() functions, though...

For instance, can we just have callers define multiple SlowHash options, and create multiple CipherSuite objects for each option?

```
struct ScryptWeak;

impl<D: Hash> SlowHash<D> for ScryptWeak {
  fn hash(
    input: GenericArray<u8, <D as Digest>::OutputSize>,
  ) -> Result<Vec<u8>, InternalPakeError> {
    // use n,r,p with some configuration
  }
}

struct ScryptStrong;

impl<D: Hash> SlowHash<D> for ScryptStrong {
  fn hash(
    input: GenericArray<u8, <D as Digest>::OutputSize>,
  ) -> Result<Vec<u8>, InternalPakeError> {
    // use n,r,p with some configuration
  }
}

struct WeakOption;
impl CipherSuite for WeakOption {
    // ...
    type SlowHash = ScryptWeak;
}

struct StrongOption;
impl CipherSuite for StrongOption {
    // ...
    type SlowHash = ScryptStrong;
}
```
This is basically how it has to be done right now. The issue is actually not really `SlowHash`, the issue is the whole `CipherSuite`, currently all settings are compile-time only as they are defined with generics, no passing down of parameters can actually happen.

Imagine I want to make multiple `Hash` options, like SHA-256, SHA3 and BLAKE3. I also want to provide multiple Argon2 variants and configurations. Additionally I would like to add support for P-256 and P-384. You can see combining all of these will add a huge amount of `CipherSuite` implementations.

One way to solve this with the current setup, is to turn `CipherSuite` `type`s into instantiatable configurations that can be passed down or setup once at the start.

```rust
struct Default;
impl CipherSuite for Default {
    type Group = Groups;
    type KeyExchange = TripleDH;
    type Hash = Hashs;
    type SlowHash = SlowHashs;
}

enum Groups {
  P256,
  P384,
  RistrettoPoint,
}

enum Hashs {
  Sha256,
  Sha384,
  Sha512,
  Sha3,
  Blake3,
}

enum SlowHashs {
  Argon2id { more_configuration, ... },
  Argon2d { more_configuration, ... },
  Scrypt { more_configuration, ... },
  Pbkdf2 { more_configuration, ... },
}
```

This approach would require passing these instantiated objects into the relevant functions. I don't know how else to solve this problem.

Specifically `SlowHash`, may require additional configuration that can't be solved at all right now, like passing a salt, additional data or similar.

Alternatively `CipherSuite` could be changed into this:
```rust
struct CipherSuite<G, KE, H, SH>
where
  G: GroupWithMapToCurve<..>,
  KE: KeyExchange<..>,
  H: Hash,
  SH: SlowHash<..>,
{
    group: G,
    key_exchange: KE,
    hash: H,
    slow_hash: SH,
}
```
and passed down only once as a configuration object.
Yep, the trait-based approach can't be a dynamic (runtime) configuration approach.

But in reaching for dynamic configuration, I want to figure out if this means to solve:
- external customization ("You can see combining all of these will add a huge amount of CipherSuite implementations"):  here the problem is the orphan rules require a library user to have their favorite combination defined upstream.
- or real, dynamic configuration (i.e. passing something you could not possibly decide at compilation time).

If we're trying to solve the first problem, there are possible ways to do it with:
- macros (that way all combinations are upstream, they're just not "written" by the maintainer),
- conditional compilation. See [e.g. curve25519-dalek](https://github.com/dalek-cryptography/curve25519-dalek/blob/main/src/field.rs) for how config flags can be used to succintly define what implementations to use at compile time: this requires selecting one of each alternative, rather than requiring one implementation to match the CipherSuite.

If we're trying to solve the second problem, there is no way around it. Things that are legitimately the matter of runtime should move out of `CipherSuite` and be a parametric struct, as you suggest. That they're here in the first place probably means this component has outgrown its original scope as a convenience trait. 

For our needs, we definitely don't need real dynamic configuration (yet :laughing:), so macros was the approach I was taking already at the moment. But to be clear, I don't see any further **requirements** for opaque_ke, I am only proposing pure API improvements (subjective).

So currently I'm trying to solve the shortcomings by exposing this API to our library consumers:
```rust
struct Config {
  group: RistrettoPoint,
  key_exchange: TripleDH,
  hash: Sha512,
  slow_hash: Argon2,
}
```
On top of that I don't want to let our consumers be constrained by generics, this is useful in the case of transitioning between different configurations for example without having to add additional storage layers because of different data structures, so I wrap objects in an `enum`:
```rust
enum ClientRegistration {
  RistrettoTripleDhSha512Argon2id(opaque_ke::ClientRegistration<RistrettoTripleDhSha512Argon2id>),
  RistrettoTripleDhSha512Argon2d(opaque_ke::ClientRegistration<RistrettoTripleDhSha512Argon2d>),
  RistrettoTripleDhSha512Scrypt(opaque_ke::ClientRegistration<RistrettoTripleDhSha512Scrypt>),
  RistrettoTripleDhSha512Pbkdf2(opaque_ke::ClientRegistration<RistrettoTripleDhSha512Pbkdf2>),
  RistrettoTripleDhBlake3Argon2id(opaque_ke::ClientRegistration<RistrettoTripleDhBlake3Argon2id>),
  ...
}
```
I don't see how to get rid of generics unless some run-time configuration is introduced (building configuration `struct`s could still use `const fn`).

As for things that require true run-time configuration, personally I only looked at passing a salt for Argon2, I believe this would be impossible to do without adding some parameter. On the other hand I am no cryptographer, until now I was actually assuming that adding a salt in this case would have no real benefit.
For reference, this is what we did now:
https://github.com/khonsulabs/custodian/blob/main/password/src/cipher_suite/mod.rs

Fix patch:
diff --git a/Cargo.toml b/Cargo.toml
index 24e7d0c4..41178c4e 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -22,7 +22,7 @@ std = ["curve25519-dalek/std", "getrandom", "rand/std", "rand/std_rng", "num-big
 serialize = ["serde", "base64", "generic-array/serde", "curve25519-dalek/serde"]
 
 [dependencies]
-argon2 = { version = "0.2", default-features = false, optional = true }
+argon2 = { version = "0.3", default-features = false, features = ["alloc"], optional = true }
 base64 = { version = "0.13", default-features = false, features = ["alloc"], optional = true }
 constant_time_eq = "0.1"
 curve25519-dalek = { version = "3", default-features = false }
diff --git a/src/group/x25519.rs b/src/group/x25519.rs
index a48c7405..228c223a 100644
--- a/src/group/x25519.rs
+++ b/src/group/x25519.rs
@@ -127,7 +127,7 @@ fn test() -> Result<(), ProtocolError> {
     } = client.finish(
         &mut OsRng,
         message,
-        ClientRegistrationFinishParameters::Default,
+        ClientRegistrationFinishParameters::default(),
     )?;
     let server_registration = ServerRegistration::finish(message);
 
@@ -152,7 +152,7 @@ fn test() -> Result<(), ProtocolError> {
         session_key: client_session_key,
         export_key: login_export_key,
         ..
-    } = client.finish(message, ClientLoginFinishParameters::Default)?;
+    } = client.finish(message, ClientLoginFinishParameters::default())?;
     let server_session_key = server.finish(message)?.session_key;
 
     assert_eq!(register_export_key, login_export_key);
@@ -172,7 +172,7 @@ fn test() -> Result<(), ProtocolError> {
     )?;
 
     assert!(matches!(
-        client.finish(message, ClientLoginFinishParameters::Default),
+        client.finish(message, ClientLoginFinishParameters::default()),
         Err(ProtocolError::InvalidLoginError)
     ));
 
diff --git a/src/keypair.rs b/src/keypair.rs
index 4d6f2a23..caf4b723 100644
--- a/src/keypair.rs
+++ b/src/keypair.rs
@@ -507,7 +507,7 @@ mod tests {
             .finish(
                 &mut OsRng,
                 message,
-                ClientRegistrationFinishParameters::Default,
+                ClientRegistrationFinishParameters::default(),
             )
             .unwrap();
         let file = ServerRegistration::finish(message);
@@ -530,7 +530,7 @@ mod tests {
         )
         .unwrap();
         let ClientLoginFinishResult { message, .. } = client
-            .finish(message, ClientLoginFinishParameters::Default)
+            .finish(message, ClientLoginFinishParameters::default())
             .unwrap();
         server.finish(message).unwrap();
     }
diff --git a/src/lib.rs b/src/lib.rs
index d6e5d6ae..65f4ceb5 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -578,7 +578,7 @@
 //!
 //! But, for applications that wish to cryptographically bind these identities to
 //! the registered password file as well as the session key output by the login phase, these custom identifiers can be specified through
-//! [ClientRegistrationFinishParameters::WithIdentifiers] in [Client Registration Finish](#client-registration-finish):
+//! [ClientRegistrationFinishParameters] in [Client Registration Finish](#client-registration-finish):
 //! ```
 //! # use opaque_ke::{
 //! #   errors::ProtocolError,
@@ -606,11 +606,12 @@
 //! let client_registration_finish_result = client_registration_start_result.state.finish(
 //!     &mut client_rng,
 //!     server_registration_start_result.message,
-//!     ClientRegistrationFinishParameters::WithIdentifiers(
-//!         Identifiers::ClientAndServerIdentifiers(
+//!     ClientRegistrationFinishParameters::new(
+//!         Some(Identifiers::ClientAndServerIdentifiers(
 //!             b"Alice_the_Cryptographer".to_vec(),
 //!             b"Facebook".to_vec(),
-//!         ),
+//!         )),
+//!         None,
 //!     ),
 //! )?;
 //! # Ok::<(), ProtocolError>(())
@@ -641,7 +642,7 @@
 //! # let mut server_rng = OsRng;
 //! # let server_setup = ServerSetup::<Default>::new(&mut server_rng);
 //! # let server_registration_start_result = ServerRegistration::<Default>::start(&server_setup, client_registration_start_result.message, b"alice@example.com")?;
-//! # let client_registration_finish_result = client_registration_start_result.state.finish(&mut client_rng, server_registration_start_result.message, ClientRegistrationFinishParameters::WithIdentifiers(Identifiers::ClientAndServerIdentifiers(b"Alice_the_Cryptographer".to_vec(), b"Facebook".to_vec())))?;
+//! # let client_registration_finish_result = client_registration_start_result.state.finish(&mut client_rng, server_registration_start_result.message, ClientRegistrationFinishParameters::new(Some(Identifiers::ClientAndServerIdentifiers(b"Alice_the_Cryptographer".to_vec(), b"Facebook".to_vec())), None))?;
 //! # let password_file_bytes = ServerRegistration::<Default>::finish(client_registration_finish_result.message).serialize();
 //! # let client_login_start_result = ClientLogin::<Default>::start(
 //! #   &mut client_rng,
@@ -666,7 +667,7 @@
 //! # Ok::<(), ProtocolError>(())
 //! ```
 //!
-//! as well as [ClientLoginFinishParameters::WithIdentifiers] in [Client Login Finish](#client-login-finish):
+//! as well as [ClientLoginFinishParameters] in [Client Login Finish](#client-login-finish):
 //! ```
 //! # use opaque_ke::{
 //! #   errors::ProtocolError,
@@ -691,7 +692,7 @@
 //! # let mut server_rng = OsRng;
 //! # let server_setup = ServerSetup::<Default>::new(&mut server_rng);
 //! # let server_registration_start_result = ServerRegistration::<Default>::start(&server_setup, client_registration_start_result.message, b"alice@example.com")?;
-//! # let client_registration_finish_result = client_registration_start_result.state.finish(&mut client_rng, server_registration_start_result.message, ClientRegistrationFinishParameters::WithIdentifiers(Identifiers::ClientAndServerIdentifiers(b"Alice_the_Cryptographer".to_vec(), b"Facebook".to_vec())))?;
+//! # let client_registration_finish_result = client_registration_start_result.state.finish(&mut client_rng, server_registration_start_result.message, ClientRegistrationFinishParameters::new(Some(Identifiers::ClientAndServerIdentifiers(b"Alice_the_Cryptographer".to_vec(), b"Facebook".to_vec())), None))?;
 //! # let password_file_bytes = ServerRegistration::<Default>::finish(client_registration_finish_result.message).serialize();
 //! # let client_login_start_result = ClientLogin::<Default>::start(
 //! #     &mut client_rng,
@@ -705,11 +706,13 @@
 //! #     ServerLogin::start(&mut server_rng, &server_setup, Some(password_file), client_login_start_result.message, b"alice@example.com", ServerLoginStartParameters::WithIdentifiers(Identifiers::ClientAndServerIdentifiers(b"Alice_the_Cryptographer".to_vec(), b"Facebook".to_vec())))?;
 //! let client_login_finish_result = client_login_start_result.state.finish(
 //!     server_login_start_result.message,
-//!     ClientLoginFinishParameters::WithIdentifiers(
-//!         Identifiers::ClientAndServerIdentifiers(
+//!     ClientLoginFinishParameters::new(
+//!         None,
+//!         Some(Identifiers::ClientAndServerIdentifiers(
 //!             b"Alice_the_Cryptographer".to_vec(),
 //!             b"Facebook".to_vec(),
-//!         ),
+//!         )),
+//!         None,
 //!     ),
 //! )?;
 //!
@@ -727,7 +730,7 @@
 //! so as to bind the integrity of application-specific data or configuration parameters to the security of the key exchange.
 //! During the login phase, the client and server can specify this context using:
 //! - The second login message, where the server can populate [ServerLoginStartParameters::WithContext], and
-//! - The third login message, where the client can populate [ClientLoginFinishParameters::WithContext].
+//! - The third login message, where the client can populate [ClientLoginFinishParameters].
 //!
 //! For both of these messages, the `WithContextAndIdentifiers` variant can be used to specify these fields in addition to
 //! [custom identifiers](#custom-identifiers), with the ordering of the fields as
@@ -780,7 +783,8 @@
 //!     fn public_key(
 //!         &self
 //!     ) -> Result<PublicKey<RistrettoPoint>, InternalError<Self::Error>> {
-//!         YourRemoteKey::public_key(self).map(PublicKey::from_arr).map_err(InternalError::Custom)
+//!         YourRemoteKey::public_key(self).map(PublicKey::from_arr)
+//!             .map_err(InternalError::Custom)
 //!     }
 //!
 //!     fn serialize(&self) -> Vec<u8> {
diff --git a/src/opaque.rs b/src/opaque.rs
index b784bbec..1591f6c0 100644
--- a/src/opaque.rs
+++ b/src/opaque.rs
@@ -222,16 +222,29 @@ pub(crate) fn bytestrings_from_identifiers(
 
 /// Optional parameters for client registration finish
 #[derive(Clone)]
-pub enum ClientRegistrationFinishParameters {
+pub struct ClientRegistrationFinishParameters<'h, CS: CipherSuite> {
     /// Specifying the identifiers idU and idS
-    WithIdentifiers(Identifiers),
-    /// No identifiers or private key specified
-    Default,
+    pub identifiers: Option<Identifiers>,
+    /// Specifying a configuration for the slow hash
+    pub slow_hash: Option<&'h CS::SlowHash>,
 }
 
-impl Default for ClientRegistrationFinishParameters {
+impl<'h, CS: CipherSuite> Default for ClientRegistrationFinishParameters<'h, CS> {
     fn default() -> Self {
-        Self::Default
+        Self {
+            identifiers: None,
+            slow_hash: None,
+        }
+    }
+}
+
+impl<'h, CS: CipherSuite> ClientRegistrationFinishParameters<'h, CS> {
+    /// Create a new [`ClientRegistrationFinishParameters`]
+    pub fn new(identifiers: Option<Identifiers>, slow_hash: Option<&'h CS::SlowHash>) -> Self {
+        Self {
+            identifiers,
+            slow_hash,
+        }
     }
 }
 
@@ -312,22 +325,15 @@ impl<CS: CipherSuite> ClientRegistration<CS> {
         self,
         rng: &mut R,
         r2: RegistrationResponse<CS>,
-        params: ClientRegistrationFinishParameters,
+        params: ClientRegistrationFinishParameters<CS>,
     ) -> Result<ClientRegistrationFinishResult<CS>, ProtocolError> {
-        let optional_ids = match params {
-            ClientRegistrationFinishParameters::WithIdentifiers(ids) => Some(ids),
-            ClientRegistrationFinishParameters::Default => None,
-        };
-
         // Check for reflected value from server and halt if detected
         if self.alpha.ct_equal(&r2.beta) {
             return Err(ProtocolError::ReflectedValueError);
         }
 
-        let password_derived_key = get_password_derived_key::<CS::OprfGroup, CS::SlowHash, CS::Hash>(
-            &self.token,
-            r2.beta,
-        )?;
+        let password_derived_key =
+            get_password_derived_key::<CS>(&self.token, r2.beta, params.slow_hash)?;
 
         #[cfg_attr(not(test), allow(unused_variables))]
         let (randomized_pwd, h) = Hkdf::<CS::Hash>::extract(None, &password_derived_key);
@@ -335,8 +341,12 @@ impl<CS: CipherSuite> ClientRegistration<CS> {
         h.expand(STR_MASKING_KEY, &mut masking_key)
             .map_err(|_| InternalError::HkdfError)?;
 
-        let result =
-            Envelope::<CS>::seal(rng, &password_derived_key, &r2.server_s_pk, optional_ids)?;
+        let result = Envelope::<CS>::seal(
+            rng,
+            &password_derived_key,
+            &r2.server_s_pk,
+            params.identifiers,
+        )?;
 
         Ok(ClientRegistrationFinishResult {
             message: RegistrationUpload {
@@ -553,21 +563,37 @@ impl<CS: CipherSuite> Clone for ClientLoginStartResult<CS> {
 
 /// Optional parameters for client login finish
 #[derive(Clone)]
-pub enum ClientLoginFinishParameters {
+pub struct ClientLoginFinishParameters<'h, CS: CipherSuite> {
     /// Specifying a context field that the server must agree on
-    WithContext(Vec<u8>),
+    pub context: Option<Vec<u8>>,
     /// Specifying a user identifier and server identifier that will be matched against the server
-    WithIdentifiers(Identifiers),
-    /// Specifying a context field that the server must agree on,
-    /// along with a user identifier and server identifier and context that will be matched against the server
-    WithContextAndIdentifiers(Vec<u8>, Identifiers),
-    /// No custom identifiers and no context
-    Default,
+    pub identifiers: Option<Identifiers>,
+    /// Specifying a configuration for the slow hash
+    pub slow_hash: Option<&'h CS::SlowHash>,
 }
 
-impl Default for ClientLoginFinishParameters {
+impl<'h, CS: CipherSuite> Default for ClientLoginFinishParameters<'h, CS> {
     fn default() -> Self {
-        Self::Default
+        Self {
+            context: None,
+            identifiers: None,
+            slow_hash: None,
+        }
+    }
+}
+
+impl<'h, CS: CipherSuite> ClientLoginFinishParameters<'h, CS> {
+    /// Create a new [`ClientLoginFinishParameters`]
+    pub fn new(
+        context: Option<Vec<u8>>,
+        identifiers: Option<Identifiers>,
+        slow_hash: Option<&'h CS::SlowHash>,
+    ) -> Self {
+        Self {
+            context,
+            identifiers,
+            slow_hash,
+        }
     }
 }
 
@@ -638,18 +664,8 @@ impl<CS: CipherSuite> ClientLogin<CS> {
     pub fn finish(
         self,
         credential_response: CredentialResponse<CS>,
-        params: ClientLoginFinishParameters,
+        params: ClientLoginFinishParameters<CS>,
     ) -> Result<ClientLoginFinishResult<CS>, ProtocolError> {
-        let (context, optional_ids) = match params {
-            ClientLoginFinishParameters::Default => (vec![], None),
-            ClientLoginFinishParameters::WithContext(context) => (context, None),
-            ClientLoginFinishParameters::WithIdentifiers(ids) => (vec![], Some(ids)),
-            // add context
-            ClientLoginFinishParameters::WithContextAndIdentifiers(context, ids) => {
-                (context, Some(ids))
-            }
-        };
-
         // Check if beta value from server is equal to alpha value from client
         let credential_request =
             CredentialRequest::<CS>::deserialize(&self.serialized_credential_request[..])?;
@@ -657,9 +673,10 @@ impl<CS: CipherSuite> ClientLogin<CS> {
             return Err(ProtocolError::ReflectedValueError);
         }
 
-        let password_derived_key = get_password_derived_key::<CS::OprfGroup, CS::SlowHash, CS::Hash>(
+        let password_derived_key = get_password_derived_key::<CS>(
             &self.token,
             credential_response.beta,
+            params.slow_hash,
         )?;
 
         let h = Hkdf::<CS::Hash>::new(None, &password_derived_key);
@@ -679,7 +696,11 @@ impl<CS: CipherSuite> ClientLogin<CS> {
         let server_s_pk_bytes = server_s_pk.to_arr().to_vec();
 
         let opened_envelope = &envelope
-            .open(&password_derived_key, &server_s_pk_bytes, &optional_ids)
+            .open(
+                &password_derived_key,
+                &server_s_pk_bytes,
+                &params.identifiers,
+            )
             .map_err(|e| match e {
                 ProtocolError::LibraryError(InternalError::SealOpenHmacError) => {
                     ProtocolError::InvalidLoginError
@@ -702,7 +723,7 @@ impl<CS: CipherSuite> ClientLogin<CS> {
             opened_envelope.client_static_keypair.private().clone(),
             opened_envelope.id_u.clone(),
             opened_envelope.id_s.clone(),
-            context,
+            params.context.unwrap_or_default(),
         )?;
 
         Ok(ClientLoginFinishResult {
@@ -1007,12 +1028,19 @@ impl<CS: CipherSuite> Drop for ServerLogin<CS> {
 
 // Helper functions
 
-fn get_password_derived_key<G: Group, SH: SlowHash<D>, D: Hash>(
-    token: &oprf::Token<G>,
-    beta: G,
+fn get_password_derived_key<CS: CipherSuite>(
+    token: &oprf::Token<CS::OprfGroup>,
+    beta: CS::OprfGroup,
+    slow_hash: Option<&CS::SlowHash>,
 ) -> Result<Vec<u8>, ProtocolError> {
-    let oprf_output = oprf::finalize::<G, D>(&token.data, &token.blind, beta)?;
-    SH::hash(oprf_output).map_err(ProtocolError::from)
+    let oprf_output = oprf::finalize::<CS::OprfGroup, CS::Hash>(&token.data, &token.blind, beta)?;
+
+    if let Some(slow_hash) = slow_hash {
+        slow_hash.hash(oprf_output)
+    } else {
+        CS::SlowHash::default().hash(oprf_output)
+    }
+    .map_err(ProtocolError::from)
 }
 
 fn oprf_key_from_seed<G: Group, D: Hash>(
diff --git a/src/slow_hash.rs b/src/slow_hash.rs
index 19ce0e94..281dce72 100644
--- a/src/slow_hash.rs
+++ b/src/slow_hash.rs
@@ -13,33 +13,35 @@ use generic_array::typenum::Unsigned;
 use generic_array::GenericArray;
 
 /// Used for the slow hashing function in OPAQUE
-pub trait SlowHash<D: Hash> {
+pub trait SlowHash<D: Hash>: Default {
     /// Computes the slow hashing function
-    fn hash(input: GenericArray<u8, <D as Digest>::OutputSize>) -> Result<Vec<u8>, InternalError>;
+    fn hash(
+        &self,
+        input: GenericArray<u8, <D as Digest>::OutputSize>,
+    ) -> Result<Vec<u8>, InternalError>;
 }
 
 /// A no-op hash which simply returns its input
+#[derive(Default)]
 pub struct NoOpHash;
 
 impl<D: Hash> SlowHash<D> for NoOpHash {
-    fn hash(input: GenericArray<u8, <D as Digest>::OutputSize>) -> Result<Vec<u8>, InternalError> {
+    fn hash(
+        &self,
+        input: GenericArray<u8, <D as Digest>::OutputSize>,
+    ) -> Result<Vec<u8>, InternalError> {
         Ok(input.to_vec())
     }
 }
 
 #[cfg(feature = "slow-hash")]
 impl<D: Hash> SlowHash<D> for argon2::Argon2<'_> {
-    fn hash(input: GenericArray<u8, <D as Digest>::OutputSize>) -> Result<Vec<u8>, InternalError> {
-        let params = argon2::Argon2::default();
+    fn hash(
+        &self,
+        input: GenericArray<u8, <D as Digest>::OutputSize>,
+    ) -> Result<Vec<u8>, InternalError> {
         let mut output = alloc::vec![0u8; <D as Digest>::OutputSize::USIZE];
-        params
-            .hash_password_into(
-                argon2::Algorithm::Argon2id,
-                &input,
-                &[0; argon2::MIN_SALT_LENGTH],
-                &[],
-                &mut output,
-            )
+        self.hash_password_into(&input, &[0; argon2::MIN_SALT_LEN], &mut output)
             .map_err(|_| InternalError::SlowHashError)?;
         Ok(output)
     }


Test patch:
diff --git a/src/tests/full_test.rs b/src/tests/full_test.rs
index 28c58cc3..a42c8bcb 100644
--- a/src/tests/full_test.rs
+++ b/src/tests/full_test.rs
@@ -347,8 +347,12 @@ fn generate_parameters<CS: CipherSuite>() -> TestVectorParameters {
         .finish(
             &mut finish_registration_rng,
             server_registration_start_result.message,
-            ClientRegistrationFinishParameters::WithIdentifiers(
-                Identifiers::ClientAndServerIdentifiers(id_u.to_vec(), id_s.to_vec()),
+            ClientRegistrationFinishParameters::new(
+                Some(Identifiers::ClientAndServerIdentifiers(
+                    id_u.to_vec(),
+                    id_s.to_vec(),
+                )),
+                None,
             ),
         )
         .unwrap();
@@ -402,9 +406,13 @@ fn generate_parameters<CS: CipherSuite>() -> TestVectorParameters {
         .state
         .finish(
             server_login_start_result.message,
-            ClientLoginFinishParameters::WithContextAndIdentifiers(
-                context.to_vec(),
-                Identifiers::ClientAndServerIdentifiers(id_u.to_vec(), id_s.to_vec()),
+            ClientLoginFinishParameters::new(
+                Some(context.to_vec()),
+                Some(Identifiers::ClientAndServerIdentifiers(
+                    id_u.to_vec(),
+                    id_s.to_vec(),
+                )),
+                None,
             ),
         )
         .unwrap();
@@ -544,8 +552,12 @@ fn test_registration_upload() -> Result<(), ProtocolError> {
     .finish(
         &mut finish_registration_rng,
         RegistrationResponse::deserialize(&parameters.registration_response[..])?,
-        ClientRegistrationFinishParameters::WithIdentifiers(
-            Identifiers::ClientAndServerIdentifiers(parameters.id_u, parameters.id_s),
+        ClientRegistrationFinishParameters::new(
+            Some(Identifiers::ClientAndServerIdentifiers(
+                parameters.id_u,
+                parameters.id_s,
+            )),
+            None,
         ),
     )?;
 
@@ -662,9 +674,13 @@ fn test_credential_finalization() -> Result<(), ProtocolError> {
         CredentialResponse::<RistrettoSha5123dhNoSlowHash>::deserialize(
             &parameters.credential_response[..],
         )?,
-        ClientLoginFinishParameters::WithContextAndIdentifiers(
-            parameters.context,
-            Identifiers::ClientAndServerIdentifiers(parameters.id_u, parameters.id_s),
+        ClientLoginFinishParameters::new(
+            Some(parameters.context),
+            Some(Identifiers::ClientAndServerIdentifiers(
+                parameters.id_u,
+                parameters.id_s,
+            )),
+            None,
         ),
     )?;
 
diff --git a/src/tests/opaque_test_vectors.rs b/src/tests/opaque_test_vectors.rs
index 106eb2ba..1971813c 100644
--- a/src/tests/opaque_test_vectors.rs
+++ b/src/tests/opaque_test_vectors.rs
@@ -881,8 +881,8 @@ fn test_registration_upload<CS: CipherSuite>(tvs: &[&str]) -> Result<(), Protoco
             &mut finish_registration_rng,
             RegistrationResponse::deserialize(&parameters.registration_response[..]).unwrap(),
             match parse_identifiers(parameters.client_identity, parameters.server_identity) {
-                None => ClientRegistrationFinishParameters::Default,
-                Some(ids) => ClientRegistrationFinishParameters::WithIdentifiers(ids),
+                None => ClientRegistrationFinishParameters::default(),
+                Some(ids) => ClientRegistrationFinishParameters::new(Some(ids), None),
             },
         )?;
 
@@ -998,9 +998,9 @@ fn test_ke3<CS: CipherSuite>(tvs: &[&str]) -> Result<(), ProtocolError> {
         let client_login_finish_result = client_login_start_result.state.finish(
             CredentialResponse::<CS>::deserialize(&parameters.KE2[..])?,
             match parse_identifiers(parameters.client_identity, parameters.server_identity) {
-                None => ClientLoginFinishParameters::WithContext(parameters.context),
+                None => ClientLoginFinishParameters::new(Some(parameters.context), None, None),
                 Some(ids) => {
-                    ClientLoginFinishParameters::WithContextAndIdentifiers(parameters.context, ids)
+                    ClientLoginFinishParameters::new(Some(parameters.context), Some(ids), None)
                 }
             },
         )?;