Alternate implementation #29
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is a backward-incompatible change. For details, refer to: - https://tools.ietf.org/html/rfc2945 - https://tools.ietf.org/html/rfc5054 - http://srp.stanford.edu/design.html - RustCrypto#20 This resolves issue RustCrypto#20.
This implementation removes the use of unsigned integers (which is not anywhere in the spec) and a bunch of un-specified mod operations.
|
@brndnmtthws like I said on #27, sorry these PRs haven't been reviewed in a very long time. We're trying to triage some other basic project maintenance, but if you're still interested in discussing these PRs I can take a look after that. |
Leandros
added a commit
to Leandros/PAKEs
that referenced
this pull request
Jan 4, 2022
BREAKING CHANGE: Replaces all BigUInt with BigInt and removes the modulo N from each operation. Based on @brndnmtthws changes. Refs: RustCrypto#27 RustCrypto#28 RustCrypto#29
|
Continued as #81 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I was having a really tough time getting this library to work correctly with another library, and I noticed some differences in the implementation. Specifically, there was a lot of truncation going on (doing mod n everywhere), and the use of BigUint instead of BigInt caused some odd behaviour. This part is particularly confusing.
There's nothing in the spec that says you need to use unsigned integers, or that you need to mod n every operation, so I just removed that, and then everything was okay.
I'm sure there are good reasons for why things were done the way they were, however I'm going to open this PR and leave it here for the benefit of others.
It also includes my 2 other patches from #27 and #28.