This PR is a complete rewrite of the SRP library. It includes many improvements over the old library at the expense of backwards compatibility.

Improvements include:

• Improved file and code organization
• Access to individual SRP computations
• Consistent sever and client API
• Simpler API
• Improved documentation with tests in documentation
• New tests for compatibility with the RFC
• Bumps dependencies
• Timing safe verification comparisons
• Modernized error handling
• And much more...

This does a "git subtree add", moving the entire history of https://github.com/warner/spake2.rs into the spake2/ subdirectory of this repo. I think that's the appropriate thing to do.. it makes the history look a little funky (there are now two unparented commits, not just the usual one original commit), but it keeps all the revision IDs identical.

If this seems reasonable and we land it, I'll add tags next (with names like spake2-0.1.0), and then update the travis config, then the READMEs.

It would be awesome if the function get_password_verifier wouldn't require a client but just had a signature like fn get_password_verifier<D: Digest>(private_key: &[u8], params, &[u8]) -> Vec<u8> or something.

This now only needs rand_core, not the full rand hierarchy, because the latest rand_core includes a basic OsRng function.

The latest curve25519-dalek uses zeroize, which uses the alloc feature, which raises the spake2 MSRV to rustc-1.36 .

It upgrades srp to use rand_core as well. The srp MSRV remains at rustc-1.32 .

I update the travis config to exercise 1.36, because it seemed too hard to make it test srp/spake2 with different versions.

fixes #21

@newpavlov you might want to defer this until you made the SRP API changes you mentioned in #21

This is a backward-incompatible change.

For details, refer to:

• https://tools.ietf.org/html/rfc2945
• https://tools.ietf.org/html/rfc5054
• http://srp.stanford.edu/design.html
• https://github.com/RustCrypto/PAKEs/issues/20

This resolves issue #20.

RFC5054 refers to RFC2945 for the computation of the 'M' value. However, this implementation does not seem to be following the standard.

'M' should be computed as: M = H(H(N) XOR H(g) | H(U) | s | A | B | K)

But instead, it is computed as: M1 = H(A, B, K) https://github.com/RustCrypto/PAKEs/blob/master/srp/src/client.rs#L170 https://github.com/RustCrypto/PAKEs/blob/master/srp/src/server.rs#L132

I am not an expert of SRP, but am I missing something? The samples on the wikipedia page match the RFC, but I can't figure out why this implementation differs in that regard.

Thanks for clarifying this portion of the code!

1.31.1 is our intended MSRV (Minimum Supported Rust Version), but we probably don't actually work there because of an insufficiently-constrained dependency that requires 1.32.

1.32 is probably our actual MSRV, and will be the one we aim for going forward.

refs #21

There are methods for much more efficient and cryptographically appropriate modular exponentiation than what is currently implemented.

I'm not familiar enough with the Rust ecosystem to specifically recommend something at this point. (Indeed, I read this code in the hopes of finding the recommended practice.)

debug_struct is built in and makes your Debug impl more consistent with others.

I noodled for a while over trying to make it also tell you which Group it's generic over, I don't think there's a way to do it unmodified, but you can either add a Default bound to it, or just add a name() fn to the Group trait.

Hello, I'm writing a client implementation of SRP, but the server I'm connecting to requires that the nousernameinx flag be set. How would I go about doing this, or what would this library have to change to support this? Thanks!

Bumps rand_core from 0.5.1 to 0.6.3.

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

Hi,

I noticed that the spake2 crate uses HKDF instead of a memory-hard hash function when converting the password to a scalar: https://github.com/RustCrypto/PAKEs/blob/04ca077f2706fbd4bb5ed903a22bdcd2f20b2b0a/spake2/src/lib.rs#L473

According to the draft specification, as well as this analysis, implementers should use a function like scrypt to slow down brute-force attacks. My guess is that HKDF was used for interoperability with Magic Wormhole's Python implementation, where the one-time nature of passwords means brute force isn't a viable attack. However, this may be a problem for other use cases where a password can be attempted more than once without manual intervention from the other side.

Workarounds include request rate limiting or for callers to send the password through something like scrypt first, using the derived key as input to SPAKE2. However, many users of the library won't know to do this, since the lack of a memory-hard function is not necessarily clear from the documentation.

I don't know what the Rust convention is, but when I see cargo outdated telling me that there are newer versions of dependencies that we might use, I'm tempted to upgrade. SPAKE2 is currently out-of-date on HKDF and the rand crate.

I've got a PR for spake2's use of HKDF that I'll submit in a minute, but we can't update to rand-0.7 until curve25519-dalek does the same, because the random-element selection API cites a rand_core::CryptoRng trait that must be the same on both sides of the interface.

I haven't looked closely at SRP, but it's behind on both rand (which should be easy) and generic-array (about which I have no idea).

In srp/src/server.rs for example, we see

if user_proof == d.result().as_slice() {

where the types are byte slices, &[u8]. I suspect that the same kind of thing appears throughout the code (although I haven't checked).

That will result in a non-constant time comparison, and expose this to timing attacks.

I am new to Rust, so take my suggestion with a large grain of salt. It seems that if we create a trait for secrets and then implement comparison tests for that trait with constant time checks, we could use Rust's type system to enforce that we always have constant time comparisons.

I'd like to start working on optional no_std support, is there anything major preventing this, and if not, is it the sort of thing you would like to see merged?