Uh oh!
There was an error while loading. Please reload this page.
OSSWU update - #1157
Merged
Merged
Conversation
mikelodder7
commented
Nov 20, 2022
Contributor
Thanks for reviewing. I’ll take a look after the week of thanksgiving. I’m swamped until then |
daxpedda
commented
Dec 15, 2022
ContributorAuthor
Rebased after #1166. |
daxpedda
commented
Mar 17, 2023
ContributorAuthor
Rebased after #1197. |
ContributorAuthor
@tarcieri this suddenly started working now on |
daxpedda
marked this pull request as ready for review
March 17, 2023 11:20
tarcieri
commented
Mar 17, 2023
Member
Thanks! |
Merged
This file contains hidden or 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
Sign up for freeto 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 found some code I had lying around that I never got to upstream because it has some error in it.
When we were working on hash2curve, I was always confused by the algorithm @mikelodder7 was using for OSSWU, because it wasn't the one used in the specification, later when I finally started digging I found that he was using the "original one" from the paper.
Considering we are not implementing the paper but the specification, I would have liked to update it to the specification, which should basically be the same but definitely yield the exact same results: https://www.ietf.org/archive/id/draft-irtf-cfrg-hash-to-curve-16.html#straightline-sswu.
This would also have the advantage of removing the limitation to q = 3 mod 4 curves in the future, as the spec defines a subroutine that works for all: https://www.ietf.org/archive/id/draft-irtf-cfrg-hash-to-curve-16.html#name-sqrt_ratio-for-any-field. The optimized one can be overwritten for when implementing the trait.
For some reason though I never got it to work. It fails the tests in the
p256crate and I also tried it against the P-384 implementation I did: RustCrypto/elliptic-curves#600.I would appreciate somebody taking a look so this work doesn't go to waste (or somebody telling me why this isn't a good idea).
Cc @mikelodder7.