Skip to content

Conversation

@JustArchi
Copy link
Member

This PR adds support for different SRP revisions - namely 3 and 6, on top of existing 6a.

The server that I'm working with supports only SRP in revision 6. Using 6a implementation of srp.net for it obviously generates invalid session, as the algorithm is different. However, it's quite simple to add missing support for revision 6, which affects only how K is generated.

I tested SRP algorithm with revision 6 against the real server and it seems to be working properly. I've also added one unit test.

I didn't test revision 3, but found it along 6 and 6a that should differ only in value of K. I've added support for it along the way.

Thank you in advance for considering this enhancement.

@JustArchi JustArchi changed the title Add support for different SRP revisions Add support for different SRP revisions (3, 6) Mar 26, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #15 (aa0a115) into master (e7c2294) will decrease coverage by 0.08%.
The diff coverage is 76.92%.

@@            Coverage Diff             @@
##           master      #15      +/-   ##
==========================================
- Coverage   89.58%   89.50%   -0.09%     
==========================================
  Files          38       38              
  Lines        4255     4279      +24     
  Branches      145      148       +3     
==========================================
+ Hits         3812     3830      +18     
- Misses        390      394       +4     
- Partials       53       55       +2     
Impacted Files Coverage Δ
src/srp/SrpParameters.cs 85.45% <53.84%> (-10.00%) ⬇️
src/srp.tests/SrpClientTests.cs 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e7c2294...aa0a115. Read the comment docs.

@yallie yallie merged commit 2b3462f into secure-remote-password:master Mar 28, 2022
@yallie
Copy link
Member

yallie commented Mar 28, 2022

Thanks a lot for the PR! 👍

@JustArchi JustArchi deleted the srp-revisions branch March 28, 2022 09:51
@JustArchi
Copy link
Member Author

Thanks a lot for merging!

If it's not too big of a hassle for you, I'd really appreciate a new nuget release with this feature in, so I could drop usage of my fork in the main project, of course once you're satisfied and ready with it. Thanks in advance! 😎

@yallie
Copy link
Member

yallie commented Mar 28, 2022

https://www.nuget.org/packages/srp/1.0.7

Done! :)

@JustArchi
Copy link
Member Author

Thank you a lot @yallie, all the best! 🥳

@yallie
Copy link
Member

yallie commented Mar 29, 2022

Thanks for the PR! 🥇

Out of curiosity, what's the name of the server that supports plain SRP-6 protocol?
Is it open-source? Or does it use an open-source library that can be listed as compatible implementation?

@JustArchi
Copy link
Member Author

JustArchi commented Mar 29, 2022

It's a proprietary third-party broker (stock market) service, there is a chance that they're using some open-source solution behind it, but I don't have the details about their exact implementation and I don't want to ask as I'm not directly connected with that company (just a guy who is writing integration with it). If you'd like to I can shoot you an e-mail with the URL privately if you want to pursue it further, but chance is it won't even reach the people who will be able to answer your question 🙁. Sorry, I don't know, I just needed to implement SRP protocol to pass their login procedure, and found out they're using revision 6 instead of 6a.

@yallie
Copy link
Member

yallie commented Mar 29, 2022

Ah, nevermind, I was just asking.
Never seen SRP-3 or SRP-6 implementations before, got curious :)

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.

3 participants