search

RustCrypto / PAKEs

Password-Authenticated Key Agreement protocols
104 stars 34 forks source link

srp: alternate implementation, based on @brndnmtthws changes #81

Open Leandros opened 2 years ago

Leandros commented 2 years ago

This is the continuation of #27 #28 and #29 from @brndnmtthws.

All changes have been rebased on master.

tarcieri commented 2 years ago

Thanks! I'll try to review this soon.

jbis9051 commented 2 years ago

  1. Could this be integrated into #79 ?

There's nothing in the spec that says you need to use unsigned integers, or that you need to mod n every operation, so I just removed that, and then everything was okay. - https://github.com/RustCrypto/PAKEs/pull/29

As for the mod n, I'm pretty sure this is modular reduction, which is a mathematical shortcut (from what I understand). It's not needed however it speeds up operations, which is why removing it didn't cause issues, just likely slowed down the computations. Other SRP libraries seem to do the same thing, see https://github.com/1Password/srp/issues/6.

  1. If we want to change the proof to be RFC2945 complaint, we may want to create multiple proofs for maximum compatibility. For example, TSSRP6a and nimbus-srp both use the incorrect (current) proof mechanism.
Leandros commented 2 years ago

@jbis9051:

  1. Sure, this can be integrated into #79. I can contribute a benchmark suite for the client implementation as well.

  2. I've got no big opinion on whether mod N makes sense or not. I've got to say this worked and that's what I was most concerned about.

  3. I'd definitely want to keep the proof to be RFC2945 compliant, for compliancy with other implementations. Further, I could contribute a client implementation written in Typescript which is fully compliant with this branch of SRP.

jbis9051 commented 2 years ago

@Leandros

  1. I think the H(A, B, K) scheme comes from the SRPv6 whitepaper http://srp.stanford.edu/srp6.ps
image

Therefore, I think it's reasonable for this library to support both implementations. In #79 this would be as simple as adding two methods for computing m1 in https://github.com/jbis9051/PAKEs/blob/master/srp/src/utils.rs.

Leandros commented 2 years ago

Yes, makes sense. I'll see if I can integrate the proof into #79.

tarcieri commented 2 years ago

FYI, #79 has been merged, so this needs to be rebased