Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

srp: why M1 is not calculated according to the spec? #152

Open
masihyeganeh opened this issue Sep 14, 2023 · 2 comments · May be fixed by #153
Open

srp: why M1 is not calculated according to the spec? #152

masihyeganeh opened this issue Sep 14, 2023 · 2 comments · May be fixed by #153

Comments

@masihyeganeh
Copy link
Contributor

Hello.
I was using your library in my code for a couple of years. I just updated it, and saw that M1 calculation has changed.
I see this comment where it is calculated:

// M1 = H(A, B, K) this doesn't follow the spec but apparently no one does for M1
// M1 should equal = H(H(N) XOR H(g) | H(U) | s | A | B | K) according to the spec

It makes sense that you decided to go with what most of the users prefer, but it is breaking functionality of my code.
I'm suggesting to add those back beside current ones, so there will be a proof() function and maybe a proof_std() that uses standard implementation of M1 calculation, and also there will be a verify_server_std() beside verify_server() that calculates M2 using this new M1.

Or just add a process_reply_std() that returns a SrpClientVerifier with the other M1 and M2.

I can also send a PR if you need me to.
Thanks in advance

@tarcieri tarcieri changed the title Why M1 is not calculated according to the spec? srp: why M1 is not calculated according to the spec? Sep 14, 2023
@tarcieri
Copy link
Member

For context this is regarding the changes in #79.

A PR sounds good. I think there will be some bikeshedding about naming, but otherwise we can hash out the details there.

@masihyeganeh masihyeganeh linked a pull request Sep 15, 2023 that will close this issue
@masihyeganeh
Copy link
Contributor Author

It's up. Please let me know if you like this implementation or I need to try another approach.
Also, the naming is terrible, I know. I'm not good at naming things 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants