Add inverse Elligator2 mapping to Curve25519. #357
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
lealanko
force-pushed
the
feature/elligator-decode-montgomery
branch
from
7e95845 to
43dc857
Compare
lealanko
force-pushed
the
feature/elligator-decode-montgomery
branch
from
43dc857 to
ab0cc6c
Compare
The result of the sqrt(2u(u+A)) test can be used directly to compute the final field element. This optimization was adapted from Loup Vaillant's Monocypher library.
SnowyCoder
added a commit
to SnowyCoder/curve25519-dalek
that referenced
this pull request
Jun 21, 2023
Code stolen from @lealanko dalek-cryptography#357 This commit only modifies the API to be more "standard" conforming (as described in https://www.elligator.org/), having a tweak and "dirty" key generation. Also various tests were added to ensure that it conforms to already-developed implementations. The original pull-request differed from standard implementations when checking for element positivity since FieldElement::is_positive performs an entirely different check than what the standard describes. Since this commit includes standard-conformity, the original maps downloaded from https://www.elligator.org/ are included in the vendor folder.
koba-e964
reviewed
Apr 14, 2024
| @@ -193,6 +193,81 @@ pub(crate) fn elligator_encode(r_0: &FieldElement) -> MontgomeryPoint { | |||
| MontgomeryPoint(u.to_bytes()) | |||
| } | |||
|
|
|||
| /// Perform the inverse Elligator2 mapping from a Montgomery point to | |||
| /// field element. This algorithm is based on [Elligator: | |||
There was a problem hiding this comment.
nit: the indefinite article
Suggested change
| /// field element. This algorithm is based on [Elligator: | |
| /// a field element. This algorithm is based on [Elligator: |
| // Both r and -r are valid results. Pick the nonnegative one. | ||
| let result = FieldElement::conditional_select(&r, &-&r, r.is_negative()); | ||
|
|
||
| return Some(result); |
There was a problem hiding this comment.
You may omit return here in the last line of a function, as in other functions in this file.
Suggested change
| return Some(result); | |
| Some(result) |
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.
This PR adds
montgomery_elligator_decode, an inverse tomontgomery_elligator_encode. This is my first contribution, so I'm very interested in advice on how to best accommodate Dalek's conventions.Some points of note:
encodefunction is clearly named from the perspective of hash-to-curve operations, where pre-existing uniform data is "encoded" into a curve point. However, the inverse mapping can be used to map an existing curve point invertibly into uniform-looking data. Calling this operation "decode" is confusing. I would suggest more neutral naming likemap_to_fieldandmap_from_field.OsRngsince that means that test runs are unpredictable..montgomery_elligator_encodedoes compute the sign of the v-coordinate (aseps_is_sq) but it does not return it. This is an unfortunate asymmetry with the inverse function (whose output depends on the sign), and it may be critical for applications that require a full curve point, not just a u-coordinate.I can try to address some of these issues already within this PR if that's a reasonable thing to do.