LinusU / secure-remote-password

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

💥 Pad values following RFC5054 #13

Open LinusU opened 6 years ago

LinusU commented 6 years ago

Fixes #12

Migration guide:

While verifiers generated with an earlier version of this library will continue to work, the client and the server must either both be using the padding, or both not.

The recommended upgrade path is to have both versions running on your server, which gives you time to upgrade all the clients. This can be accomplished by depending on this library two times:

{
  // ...
  "dependencies": {
    // ...
    "secure-remote-password": "^0.4.0",
    "secure-remote-password-pre-rfc5054": "LinusU/secure-remote-password#pre-rfc5054"
  }
}

You will then have to implement another version of your endpoint, which uses the newer version.

const loginCtrl1 = require('./controller/login-v1')
const loginCtrl2 = require('./controller/login-v2')

app.post('/v1/login-sessions', loginCtrl1.initiate)
app.post('/v1/login-sessions/:sessionId/finalize', loginCtrl1.finalize)

app.post('/v2/login-sessions', loginCtrl2.initiate)
app.post('/v2/login-sessions/:sessionId/finalize', loginCtrl2.finalize)

The login-v1 controller should import the library as such:

require('secure-remote-password-pre-rfc5054/server')

The login-v2 controller should import the library as usual:

require('secure-remote-password/server')
LinusU commented 6 years ago

ping @yallie, would love your 👀 on this

yallie commented 6 years ago

Looks great! 👍

Don't you want to add the RFC5054 test vectors as a unit test?

LinusU commented 6 years ago

Their test vectors are for the 1024-bit group while this library is currently hardcoded to the 2048-bit group.

We should probably/maybe add a way to select the group, but I also think it's really nice that a sensible one is picked for you and you don't really have to think about it 🤔

yallie commented 6 years ago

but I also think it's really nice that a sensible one is picked for you and you don't really have to think about it

Totally agree with that! :)

dobesv commented 5 years ago

Wow, I did not know you could depend on two different versions of a package at once.

LinusU commented 4 years ago

Sorry for not moving forward with this in a very long time. I would really like to figure out if we should pad HNxorHG or not 🤔

https://github.com/cocagne/pysrp/pull/38

If anyone has any information to chime in with I would be very happy to hear it!

dobesv commented 4 years ago

Is there an authoritative source you can ask? The SRP authors, or the authors of the RFC ?

LinusU commented 4 years ago

I tried emailing the four original authors of RFC 5054. Two of the email addresses bounced directly, but I'll keep this thread updated if I get a response from the other two.

BuckarooBanzay commented 2 years ago

just fyi: the master branch of this package didn't work for me so i tried it with this PR, works well as a client to https://github.com/est31/csrp-gmp :+1:

Also: why don't you leave the decision to use padding or not to the user of the library, as an option/config for example?

TheBinaryGuy commented 1 year ago

Hey @LinusU, any updates on this? Is it ok to pin the package version to this PR?

dobesv commented 1 year ago

Instead of migrating via different package versions, could it be a flag passed in?

Maybe at first the flag defaults to the current padding method, and recommend to people they explictly specify this as true or false. Then do a major revision update where the padding defaults to this RFC 5054 format going forward so that this is more compliant with other implementations by default?