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

Add support for different SRP revisions (3, 6) #15

Closed JustArchi closed 2 years ago

JustArchi commented 2 years ago

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.

codecov-commenter commented 2 years ago

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%) :arrow_down:
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 commented 2 years ago

Thanks a lot for the PR! 👍

JustArchi commented 2 years ago

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! :sunglasses:

yallie commented 2 years ago

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

Done! :)

JustArchi commented 2 years ago

Thank you a lot @yallie, all the best! :partying_face:

yallie commented 2 years ago

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 commented 2 years ago

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 :slightly_frowning_face:. 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 commented 2 years ago

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