LinusU / secure-remote-password

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

Only require `b` to be persisted instead of both `b` & `B` #7

Closed alexgoldstone closed 6 years ago

alexgoldstone commented 6 years ago

Hi Linus.

I'm seeking assistance with regard to implementation details when using the library.

Assuming a stateless server implementation (other than specifically persisted data), the following link makes it clear that when we send B ( serverEphemeral.public ) to the client we should also persist it on the server so that it can be referenced by the server during the later stage:

https://bitbucket.org/simon_massey/thinbus-srp-js

This seems to be inline with other SRP related documentation and is also inline with your README which doesn't document persistence but does document what is sent to the client.

However, in the case of your library it looks like we need to pass the entire serverEphemeral (consisting of both public and secret parts) to the deriveSession function as documented in stage 4 of the README.

  1. Is this correct?

  2. Why does the implementation differ in that we need to persist two values rather than just the one?

LinusU commented 6 years ago

Hmm 🤔

I haven't digged to deep into the other libraries yet, but as you can see from the design document here http://srp.stanford.edu/design.html both b and B is required to derive the session key on the host side.

Now, it would be easy to compute B from b so we could potentially make it so that you only need to save that one.

Another potential approach is to split deriveSession into two steps on the server, deriveSession and verifySession. That way you could compute and persist the key K and B instead of b and B, although not sure that this would be any better; and the downside is that it's easier to misuse the api and send K prematurely or forget to verify the session...

The second figure in the linked documentation seem to imply that you only need to persist B, but I'm not really sure how that would be possible. At the very least it seems like you would need to persist b, but it could be enough to only persist that one...

for the record, my current server implementation has a SQL table login_session with user_id, secret_ephemeral and public_ephemeral, a.k.a. I'm persisting both b and B

alexgoldstone commented 6 years ago

Thanks Linus... Good to know I did indeed understand correctly.

I checked with a colleague who has used: https://github.com/alax/jsrp

He confirmed that he persists only a single value B so there is definitely a difference in implementation.

I have no problem storing two parameters if required but it would be good to understand what the difference in implementation is between this and other libraries. Obviously you have a much better understanding of the SRP algorithm than I do but I will do my best to compare.

LinusU commented 6 years ago

Looking at that code (src/server.coffee#L7-L24) it really looks like it would need the server to persist b (the private part) and not B (the public part).

It does however calculate B (serverEphemeral.public) from b (serverEphemeral.private) so that you only need to persist that one value. We could absolutely do that as well if we feel that it increases usability, which it probably does since you only need to remember one value instead of two...

alexgoldstone commented 6 years ago

I have been able to confirm that my colleague using the https://github.com/alax/jsrp library does indeed send B (public) to the client but persists b (private) in the database.

So this is inline with your assertion and looks like the first link above is the outlier. I have raised a ticket there to query the difference.

I do agree that it would be better to persist only the single private value of b and calculate the public value B as required. That seems to be more inline with other implementations and is slightly simpler to use as we would only need to pass the one value rather than re-build the object from two persisted values as we have to now.

alexgoldstone commented 6 years ago

I forgot to say in my earlier message... thank you for the analysis and feedback... clearly you have much better insight regarding SRP implementation and I have found it very useful.

LinusU commented 6 years ago

Yeah, I think it sounds like a good idea to make it only take b (private) instead of both, I'll fix it 👌

simbo1905 commented 6 years ago

I have fixed the diagram at the link in the OP question it was a typo. From the original spec at http://srp.stanford.edu/design.html we can see that B is the public ephemeral key sent to the client and b is the private ephemeral key retained at the server.

alexgoldstone commented 6 years ago

Sorry - Did not realise all those title changes would notify everyone !

LinusU commented 6 years ago

Sorry for the delay, fix up in #9

alexgoldstone commented 6 years ago

Sorry for the slow response. I have now tested by following the migration guide. I confirmed that this both works and the implementation remains compatible with credentials generated for previous versions (as you would expect).