Argon2 Example

[zer0x64]

As discussed here:
#281

This PR aims to update the simple_login example to add an example on how to use Argon2 as the client-side KDF, as recommended. The Argon2 struct requires a lifetime, which makes the empty struct pattern somewhat unintuitive without an example and requires a PhantomData to work.

I'm opening as a draft for now because:

1. I'd like to also make an example on how to specify the derivation params instead on relying on RustCrypto's default, but I'm not sure doing it in the simple_login example is the best idea since it's supposed to be simple.
2. @daxpedda also suggested adding a custom KDF example. I think copying the simple_login file but implementing Ksf on, let's say, scrypt would be a good idea for that? If you expect to eventually first-party support scrypt we could go with something more obscure like PBKDF2-BLAKE2/3 instead so the example stays relevant.

[daxpedda]

The Argon2 struct requires a lifetime, which makes the empty struct pattern somewhat unintuitive without an example and requires a PhantomData to work.

A secret isn't used in this case, so using a 'static lifetime avoids the workaround with a PhantomData.

1. I'd like to also make an example on how to specify the derivation params instead on relying on RustCrypto's default, but I'm not sure doing it in the simple_login example is the best idea since it's supposed to be simple.

We can add this to the example with a custom KSF.

2. @daxpedda also suggested adding a custom KDF example. I think copying the simple_login file but implementing Ksf on, let's say, scrypt would be a good idea for that? If you expect to eventually first-party support scrypt we could go with something more obscure like PBKDF2-BLAKE2/3 instead so the example stays relevant.

I think scrypt is appropriate considering it's also suggested by the draft. I'm happy with whatever tough. This can also be added in a separate PR if desired.

[zer0x64]

So I made the changes you asked for in the PR. A few notes:

1. I made sure that using 'static for Argon2 does not break even if you use custom parameters. While you probably still need the PhantomData workaround if using a secret key, I can't tell how much of a niche use case that would be (I'm thinking about a case where you'd want to authenticate using both your password and some device key or keyfile).

2. For the custom Ksf example, I copied the entire simple_login example and modified it. I also included custom derivation parameters. I'm not sure if that's too overkill, but because of custom parameters we basically need the full login process, so I'm not sure how we could have a shorter "runnable" example.

3. I think maybe we shouldn't use Default for the Ciphersuite because it conflicts with core::default::Default. I'd suggest renaming it DefaultCiphersuite or something like that while I'm at it.

[daxpedda]

1. I made sure that using 'static for Argon2 does not break even if you use custom parameters. While you probably still need the PhantomData workaround if using a secret key, I can't tell how much of a niche use case that would be (I'm thinking about a case where you'd want to authenticate using both your password and some device key or keyfile).

Great! I'm not entirely sure what the point would be of using a secret key in OPAQUE. If you have a device key or keyfile, you wouldn't need OPAQUE in the first place.

2. For the custom Ksf example, I copied the entire simple_login example and modified it. I also included custom derivation parameters. I'm not sure if that's too overkill, but because of custom parameters we basically need the full login process, so I'm not sure how we could have a shorter "runnable" example.

I don't mind, but any ideas are welcome.

3. I think maybe we shouldn't use Default for the Ciphersuite because it conflicts with core::default::Default. I'd suggest renaming it DefaultCiphersuite or something like that while I'm at it.

Yeah, sounds good to me.

[kevinlewi]

Thanks for putting this up!

I wonder, instead of adding a new runnable example that is mostly a copy of simple_login, could we move the custom KSF example into the docs of lib.rs? I was thinking that we could put it under the "Advanced Usage" section. And the code snippets there could just consist of what you have in the custom_ksf.rs file (the struct definition for CustomKsf and the impl opaque_ke::ksf::Ksf for CustomKsf).

[zer0x64]

Great! I'm not entirely sure what the point would be of using a secret key in OPAQUE. If you have a device key or keyfile, you wouldn't need OPAQUE in the first place.

It's a somewhat common practice in password managers to use both a password and a keyfile as a form of cryptographically strong 2FA. I know for instance KeePass(XC) allows is and 1Password enforces it.

Thanks for putting this up!

I wonder, instead of adding a new runnable example that is mostly a copy of simple_login, could we move the custom KSF example into the docs of lib.rs? I was thinking that we could put it under the "Advanced Usage" section. And the code snippets there could just consist of what you have in the custom_ksf.rs file (the struct definition for CustomKsf and the impl opaque_ke::ksf::Ksf for CustomKsf).

That's a good idea! I did a bit of research and just learned that you can put the no_run annotation on docs, which was my worry since we "should" use a slow hashing function for this kind of example, and passing really weak values wouldn't be a good idea IMO because examples are expected to be copy-pasted.

[daxpedda]

Great! I'm not entirely sure what the point would be of using a secret key in OPAQUE. If you have a device key or keyfile, you wouldn't need OPAQUE in the first place.

It's a somewhat common practice in password managers to use both a password and a keyfile as a form of cryptographically strong 2FA. I know for instance KeePass(XC) allows is and 1Password enforces it.

This is a bit off-topic now, but I really enjoy discussing these things 😅.
My response was poorly worded, what I wanted to say is that you probably shouldn't use the secret in the KDF of OPAQUE to validate a security key or keyfile. There are other well defined protocols for this which can be run parallel to OPAQUE.

That's a good idea! I did a bit of research and just learned that you can put the no_run annotation on docs, which was my worry since we "should" use a slow hashing function for this kind of example, and passing really weak values wouldn't be a good idea IMO because examples are expected to be copy-pasted.

Argon2 is hopefully not bad enough to actually significantly affect our CI times, hopefully no_run won't be necessary while still using recommended values.

[zer0x64]

So I transferred the examples to the doc. I use scrypt to show how to use a foreign KSF and argon2 to show how to override custom parameters. Since the scrypt example does not actually run Scrypt and doesn't require much boilerplate code, I let it live as a doctest. However, I've put the argon2 example as ignore for multiple reasons:

• It would need a lot of hidden boilerplate code to get it to compile.
• The argon2 dependency being optional, I'm not sure how to force it when running the doctest. Without the feature, it simply doesn't compile.
• This example requires a run of Argon2, which as stated because would slow down devellopment cycle because it would run at every cargo test command issued.

With that said, I'm happy with the current result, so I'll move the PR out of the draft state!

[daxpedda]

I've put the argon2 example as ignore for multiple reasons:

I would strongly discourage from ignoring any documentation test, ultimately it will turn stale and we will have somebody opening an issue saying the code doesn't work.

• It would need a lot of hidden boilerplate code to get it to compile.

All our documentation tests have a huge amount of boilerplate code.

• The argon2 dependency being optional, I'm not sure how to force it when running the doctest. Without the feature, it simply doesn't compile.

You can simply put a #[cfg(feature = "argon2")] on top.

• This example requires a run of Argon2, which as stated because would slow down devellopment cycle because it would run at every cargo test command issued.

The current test suit executes a ton of asymmetric cryptography, which is already damn slow, adding a one second Argon2 hashing run is completely insignificant.

With that said, I'm happy with the current result, so I'll move the PR out of the draft state!

Indeed, looks great!

[daxpedda]

Other then actually getting the test to run this is great!

[kevinlewi]

Requesting changes due to failing CI

[daxpedda]

If the CI is fixed, and except that tiny improvement, LGTM! 🎉

[zer0x64]

Both doctests now runs, and the CI passes!

[kevinlewi]

This is great, thanks very much for the contribution @zer0x64 !