secure-remote-password / srp.net

SRP-6a protocol implementation for .NET Standard 1.6+ and .NET Framework 3.5+
https://www.nuget.org/packages/srp
MIT License
64 stars 10 forks source link

Compatibility instructions with 'secure-remote-password' npm package #7

Closed nin-o closed 4 years ago

nin-o commented 4 years ago

Thanks for the hard work on this library.

I noticed this line from the README file:

Based on and is compatible with secure-remote-password npm package by Linus Unnebäck.

We could update the README to explain how to do this (or maybe just having this issue is enough for people to find).

The answer is to do the following:

new SrpServer(new SrpParameters { PaddedLength = 0 })

yallie commented 4 years ago

Hi @nin-o,

thanks for getting in touch!

The library should be 100% compatible with no tweaks once this PR is merged:

https://github.com/LinusU/secure-remote-password/pull/13

Unfortunately, @LinusU still hasn't merged it yet for some reason. I ended up using git repository URL from this pull request instead of the official npm package in my Javascript projects, so that project.json looks like this:

{
  "name": "whatever",
  "version": "1.2.3",
  "dependencies": {
    "secure-remote-password": "https://github.com/LinusU/secure-remote-password.git#73e5f732b6ca0cdbdc19da1a0c5f48cdbad2cbf0"
  }
}

Please let me know if it works for you.

nin-o commented 4 years ago

Thanks for the quick reply!

Yes we could use the URL to the pull request while we wait for it to be merged.

Do you recommend against setting the padding to 0?

yallie commented 4 years ago

As far as I remember, padding should be used according to the RFC document. Most of the other compatible SRP libraries like srptools add this padding, too. See the related discussion for details.

danielbrauer commented 2 years ago

As someone who just spent several hours debugging a client incompatibility, I definitely interpreted “is compatible with” to mean “with default settings”.

I understand that the node version needs updating, but I think it would be worth mentioning the required parameter now. Even once the Node version is gets an update, it would be useful to point out that you need to specify zero padding to work with previous versions.

yallie commented 2 years ago

Hi @danielbrauer, terribly sorry for the inconvenience. If you would suggest a correction to the readme, I'd be happy to merge a pull request.

I don't recommend disabling the padding to mimic the wrong behavior of the current version of the npm package as suggested by the topic starter. The preferable workaround is mentioned above.

danielbrauer commented 2 years ago

Hi @yallie, I appreciate the response! SRP just proved difficult to debug the other day because of the random components, and that I couldn't convince VS Mac to show me what was happening in the compiled library. I'm still in far better shape than I would be if you hadn't made this solution. 😄

I'll gladly submit a PR. I understand that updating the npm library is preferable, but do you think it's worth mentioning disabling the padding in the case where the developer does not control the server? With the appropriate caveats, of course.

yallie commented 2 years ago

do you think it's worth mentioning disabling the padding in the case where the developer does not control the server? With the appropriate caveats, of course.

Yes, you're right, it never occurred to me that sometimes it can be the only option.