LinusU / secure-remote-password

A modern SRP implementation for Node.js and Web Browsers
101 stars 22 forks source link

Values seem to be incorrect compared with other libraries #24

Closed zbarbuto closed 4 years ago

zbarbuto commented 4 years ago

I've tested this library on it's own as per the docs and can see that it will correctly verify the client as expected. However, I've tried passing values between this and other SRP libraries and the values seem not to verify (while they would when passing between the other libraires alone).

This includes:

I've found that while sirp and jsrp will happily communicate and verify against each other, secure-remote-password will not verify client values from jsrp configured in 2048 mode and sirp on the server will not verify client values from secure-remote-password. I've yet to get secure-remote-password to successfully work with another library.

After much digging, the only thing I can find that might be causing the issue is that the k values being used are different. I compared all the hex values form both calculations of a B value (secure-remote-password's const B = k.multiply(v).add(g.modPow(b, N)).mod(N) and jsrp's this.k().multiply(v).add(this.params.g.modPow(b, this.params.N)).mod(this.params.N) and all hex values were identical expect the k value.

It's strange because the calculations seem to be the same:

secure-remote: exports.k = sha256(exports.N, exports.g)

vs

jsrp: createHash(this.params.hash).update(transform.pad.toN(this.params.N, this.params)).update(transform.pad.toN(this.params.g, this.params)).digest();

Both use the same values for N and g. The only difference seems to be that jsrp will pad the g value to the same length as the N value in the hash.

k value from jsrp:

5b9e8ef059c6b32ea59fc1d322d37f04aa30bae5aa9003b8321e21ddb04e300

k value from srp-js (a fork of node-srp by mozilla):

5b9e8ef059c6b32ea59fc1d322d37f04aa30bae5aa9003b8321e21ddb04e300

k value from secure-remote-password:

4cba3fb2923e01fb263ddbbb185a01c131c638f2561942e437727e02ca3c266d

LinusU commented 4 years ago

Hey @zbarbuto 👋

Thanks for opening this issue! There is indeed some incompatibilities, and they have previously been discussed in #12, and a potential solution is open as PR #13.

There is however still some uncertainty on wether we should also pad M and/or H(A, M, K). If you have any input on what should be the best way forward it would be invaluable if you could chime in at the thread in #12!

Thanks!

zbarbuto commented 4 years ago

Hey, sorry I tried to search for the issue but wasn't able to find. Thanks for the links - I suppose this could be marked as duplicate of #12

LinusU commented 4 years ago

Would you mind posting your findings in that thread as well, or would you be okay with me copying your post over to a comment there? I think that it's great information to have!

LinusU commented 4 years ago

Thanks! ❤️