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

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

Closed
nin-o opened this issue Dec 3, 2019 · 7 comments
Closed

Comments

@nin-o
Copy link

nin-o commented Dec 3, 2019

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
Copy link
Member

yallie commented Dec 3, 2019

Hi @nin-o,

thanks for getting in touch!

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

LinusU/secure-remote-password#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
Copy link
Author

nin-o commented Dec 5, 2019

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
Copy link
Member

yallie commented Dec 5, 2019

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.

@nin-o nin-o closed this as completed Dec 5, 2019
@danielbrauer
Copy link
Contributor

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
Copy link
Member

yallie commented Sep 11, 2021

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
Copy link
Contributor

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
Copy link
Member

yallie commented Sep 12, 2021

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.

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

No branches or pull requests

3 participants