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

Added SRP message verification using RFC2945 #507

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

jimm98y
Copy link

@jimm98y jimm98y commented Dec 24, 2023

Added methods for calculating M1 using the formula described in RFC 2945. I tried to avoid any breaking changes, which is why the PR just adds new methods while keeping the original implementation unchanged. Fixes #506.

@cipherboy cipherboy self-requested a review February 7, 2024 19:27
testWithRandomParams(512);
}

private void rfc2945MessageVerify()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you happen to know if there's any known answer tests floating around for this version of SRPs? Below we have tests for RFC 5054 which include known-answer tests, so presumably you could tell whether or not your given (third-party) SRP implementation corresponds to RFC 5054 by feeding it these vectors... it'd be good to have that for the RFC 2945 implementation as well so we can ensure interoperability!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Searching for RFC 2945 I found this: https://github.com/secure-remote-password/test-vectors/tree/8381017c812a273f30ba866af673e3726b314ab3 Their library seems to implement both RFC 5054 and RFC 2945 - knowing this before, it might have simplified the task quite a bit :)
I can just confirm that my implementation seems to work fine with Apple's after more than 2 months of testing. I might be able to capture some test vectors as well.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\o hello @jimm98y -- I'm not too particular about the source of the vectors, just that there's something we can potentially point to and say, we expect to be compatible with these values; check and see if your SRP protocol matches and if not, we'll need to potentially implement a third. :-)

That said, if those vectors pass with this code, happy to take them!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cipherboy Now they do! It turned out I've made a mistake porting my original code to BC: I did not realize that HashPaddedTriplet does exactly what the name suggests - it pads to the desired length and then hashes. For RFC2945 M2 the padding must not be performed for M1 and K components, so I had to add a new method CalculateM2RFC2945 that implements a customized hash function.

@cipherboy cipherboy self-assigned this Apr 11, 2024
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.

SRP6 calculating M1, M2 incorrectly
2 participants