search

cocagne / pysrp

Python implementation of the Secure Remote Password protocol (SRP)
MIT License
113 stars 42 forks source link

Remove HNxorHG padding per reference implementation #38

Open LinusU opened 6 years ago

LinusU commented 6 years ago

per discussion in LinusU/secure-remote-password#12

This change together with LinusU/secure-remote-password#13 would make these two libraries compatible with each other 🙌

masihyeganeh commented 6 years ago

@LinusU It is padded in Apple's implementation of SRP in CoreCrypto

LinusU commented 6 years ago

Are you sure about that? This only touches the padding when calculating M = H(H(N) xor H(g), H(I), s, A, B, K), the padding for k and u are left intact.

It seems like SRP.swift doesn't pad M, and it claims to be compatible with Apple's implementation: https://github.com/LinusU/secure-remote-password/issues/12#issuecomment-391348885

masihyeganeh commented 6 years ago

In Apple's implementation, these two g and N variables are both bytes, not number and they are the same size. So it is padded by default.

Lejo1 commented 4 years ago

So is this implementation after this change fully RFC 5054 compatible? Does it pass the test-vectors?

cocagne commented 4 years ago

There's an rfc 5054 compat branch that's compatible with the RFC, that's probably what you're looking for.

On Wed, Jul 15, 2020, 1:28 PM Lejo notifications@github.com wrote:

So is this implementation after this change fully RFC 5054 compatible? Does it pass the test-vectors?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/cocagne/pysrp/pull/38#issuecomment-658899949, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANMW7CCG6HA6ZXMOZMF3WDR3XRNBANCNFSM4FBNEJRQ .

Lejo1 commented 4 years ago

I tried both branches and the srp.enable_rfc5054() method but it ditn't worked... I'm trying to get it compatible with https://github.com/est31/csrp-gmp which were able to "do" the test vectors. So it seems to be a problem here but I'm honestly not sure!

That's also the reason why I'm asking for the test vectors.

About this PR: It generates for me bytes_M with length 20 instead of 16 is this wanted or a side effect?

cocagne commented 4 years ago

pysrp with srp.enable_rfc5054 should be correct. It's been successfully used with several other SRP 6-a implementations. From what I can tell, csrp-gmp is a port of my openssl-based csrp library but the port was done before RFC5054 support was added. As I recall, there were numerous changes that had to be made to support RFC5054, particularly in the area of padding. There's a very good chance that csrp-gmp is lacking the full set of changes needed to properly support the RFC, especially since it looks like the gmp port was made before the RFC5054 work was done to the original csrp library. The simplest solution would probably be to use the csrp library instead of csrp-gmp. If you cannot handle the openssl dependency, it'd be pretty easy to port the known-rfc5054-good csrp library over to gmp. In fact, my original implementation was one with the gmp library but I switched to openssl because it's a more widely supported library. As I recall, the conversion was very straightforward.

On Wed, Jul 15, 2020 at 3:16 PM Lejo notifications@github.com wrote:

I tried both branches and the srp.enable_rfc5054() method but it ditn't worked... I'm trying to get it compatible with https://github.com/est31/csrp-gmp which were able to "do" the test vectors. So it seems to be a problem here but I'm honestly not sure!

That's also the reason why I'm asking for the test vectors.

About this PR: It generates for me bytes_M with length 20 instead of 16 is this wanted or a side effect?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/cocagne/pysrp/pull/38#issuecomment-658986231, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANMW7DSWGQ54SEPLY4P6I3R3YFDRANCNFSM4FBNEJRQ .