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: compute K = H(S) correctly. #166

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

zadlg
Copy link

@zadlg zadlg commented Apr 8, 2024

srp: compute K = H(S) correctly.

According to the specification section The SRP protocol, K (which corresponds
to the session key) is computed as follows:

K = H(S)

where H is the digest algorithm, and S is the common exponential value.

In the current implementation, K is equal to S, which does not follow
the SRP protocol specification.

This commit fixes this issue by computing the right value for K.

According to [the specification] section _The SRP protocol_, `K` (which corresponds
to the session key) is computed as follows:

```
K = H(S)
```

where `H` is the digest algorithm, and `S` is the common exponential value.

In the [current implementation], `K` is equal to `S`, which does not follow
the SRP protocol specification.

This commit fixes this issue by computing the right value for `K`.

[the specification]: http://srp.stanford.edu/ndss.html#SECTION00032200000000000000
[current implementation]: https://github.com/RustCrypto/PAKEs/blob/8e46d6bfa24d44e6671616d730e978157b2b23e3/srp/src/client.rs#L211
@zadlg zadlg marked this pull request as ready for review April 8, 2024 17:00
@tarcieri
Copy link
Member

See also #153 and #163, as well as RFC5054

@zadlg
Copy link
Author

zadlg commented Jun 29, 2024

up

@tarcieri
Copy link
Member

@zadlg your opinion on the existing PRs and any overlap and existing discussion would be appreciated

@zadlg
Copy link
Author

zadlg commented Jun 29, 2024

Sorry, I didn't understand you were waiting for an answer from me.

I think the specification is quite clear, isn't it ?

@tarcieri
Copy link
Member

Let me be a little more explicit:

  • Do you think your PR is a duplicate?
  • What do you think about the approach in the other PR?
  • Which PRs should be merged?

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 this pull request may close these issues.

2 participants