Elligator2 Forward and Reverse Mappings

[jmwample]

Hello,

This is an implementation of the Elligator2 forward and reverse mappings --- points to representative values, as well as representative values to points. The specific goal is to make Elligator mapping for x25519 handshakes possible in pure rust.

This implementation is gated behind a feature flag "elligator2" in the curve25519_dalek crate for point transforms. I have tested this against the test vectors from the Kleshni/Elligator-2 C implementation as well as (a fork of) the agl/ed25519/extra25519 golang implementation. These can be seen in the test cases for this PR.

I also added a feature flag "elligator2" to the x25519_dalek crate that exposes a PublicRepresentative type that mirrors the PublicKey type in order to exchange only Elligator Representatives when performing a diffie-hellman handshake.

This implementation of the Elligator2 transforms (to the best of my ability):

• runs in constant time
• is equivalent to agl/ed25519/extra25519
• is equivalent to Kleshni/Elligator-2
• supports existing backends (I have tested with u64 and u32, I am not sure how to test others.)

I have seen a couple issues and PRs dealing with elligator2 mappings and this library, so I apologize if this is muddying the water . This is a feature that I really needed so I have implemented the functionality based on several existing implementations in other languages.

I am also not an expert in crypto implementations and would greatly appreciate any feedback. I hope this is helpful and potentially solid enough to include in the library. 👍

Related to:

• Add inverse Elligator2 mapping to Curve25519. #357
• Hash to curve as defined in the standard #377
• Elligator2 inverse mapping #533

[rozbb]

Thank you! It will take a bit for me to take a look at this (trying to make some paper deadlines).

Re the IETF standard, hasn't that officially been standardized now? RFC 9380. Would be good to have test vectors from there.

[jmwample]

I will get those added. I haven't spent much time looking at that RFC yet, so I can try to work in the interface there as well (and get the rest of the tests passing).

[jmwample]

I have added the RFC9380 test vectors for the elligator2 implementation, and an interface that should work if someone wanted to use this to implement the h2c interface for curve25519 and/or edwards25519. Not sure if the full h2c implementation belongs in this crate or a different crate with a more general / uniform interface.

The CI checks should also be passing now.

[randombit]

Any chance of this landing soon?

[jmwample]

I am working on adding some final tests to ensure that the bits of the ellgator2 representatives appear as uniform random.

TLDR: The sqrt_ratio_i function seems to be canonical so this library shouldn't suffer from the described computational distinguisher. However, deriving an elligator2 representative only gives 254 bits of random to begin with, so the high order bits need handled in some way to prevent a trivial distinguisher.

The specific issue that this is testing for can be described as:

An instantiation of Elligator is parameterized by what might be called
a “canonical” square root function, one with the property that
`√a^2 = √(−a)^2` for all field elements `a`. That is, we designate just
over half the field elements as “non-negative,” and the image of the
square root function consists of exactly those elements. A convenient
definition of “non-negative” for Curve25519, suggested by its authors,
is the lower half of the field, the elements `{0, 1, …, (q − 1)+/+2}`.
When there are two options for a square root, take the smaller of the two.

Any Elligator implementation that does not do this canonicalization of the final square root, and instead maps a given input systematically to either its negative or non-negative root is vulnerable to the following computational distinguisher.

[An adversary could] observe a representative, interpret it as a field
element, square it, then take the square root using the same
non-canonical square root algorithm. With representatives produced by
an affected version of [the elligator2 implementation], the output of
the square-then-root operation would always match the input. With
random strings, the output would match only half the time.

The solution from the agl/ed25519 is to randomize the high two bits when getting the representative, and clear the those same high order bits when performing a map_to_curve operation. One challenge is determining how this aligns with the test vectors from other implementations (kleshni and signal specifically, the rfc9380 tests seem to be handled properly).

For a more in-depth explanation see:

• Incorrect (non canonical) representative output for ScalarBaseMult() agl/ed25519#27
• https://www.bamsoftware.com/papers/fep-flaws/

This should not impact the general interface of the PR, and I am hoping to have the changes finished within the week.

[jmwample]

The latest commit fixes several issues.

1. The Edwards RFC9380 testcases were not actually testing the things they were meant to be testing. This forced some changes in the way structure of the map_to_point functions as mapping to Montgomery, then to Edwards was missing a sign bit.

   • map_to_point for Edwards RFC9380 test cases now testing properly and passing

2. The high order two bits of the representative are always 0 by default because correctly computed elligator2 representatives always finish with a sqrt() that takes the least-square-root value. That is, a value less than 2^254-10 (254 bits).

   • In order for the representative to be (optionally) indistinguishable from random we use a tweak byte to provide the extra randomness, added in when representative is created, and cleared when converting back to a point.
   • Both Kleshni & signal contain test cases that include non-least-square-root values which is not technically inline with the spec. In order to handle this (if interop is absolutely necessary) a map_to_point_unbounded() function is added that does not clear the high order bits before mapping to the curve.
   • A statistical test showing the effect that the tweak has on the apparent distribution of the bits over many representatives can be used to look at entropy based distinguishers (this does not necessarily help with computation based distinguishers).

I have no other changes planned for this PR without review / input.

[jmwample]

I have added another refactor to the elligator2 implementation motivated by feedback based on issues encountered with encoded key distinguishability in obfs4. The changes required to fix the distinguisher resulted in two versions of the elligator2 algorithm which are not interchangeable.

More information on the issue can be found here, the solution added to the Randomized variant is described in Method 1: add a random low order point. The RFC9380 variant is the default and follows the RFC including test vectors.

A test variant exists for legacy implementations that do not use least-square-root value of the representative (i.e. kleshni & signal), but it is not exposed by default.

---

For now I have published my fork as its own crate (see curve25519-elligator2), but my intention is to hopefully get this merged here and eventually yank the forked crate.

[rozbb]

Hi, thank you for this! I played around with this and had some notes:

1. It seems like I can't get the Elligator2 tests to fail, even when they definitely should. For example, in src/field.rs, I replaced the return value of FieldElement::ct_gt with the constant Choice::from(0u8). Running cargo test --features "alloc,elligator2" did not produce any errors. Is a KAT missing?

2. The reason I was playing with the above is because I was wondering about the correctness of the gt function in the fiat backend in the diff. My understanding is that this does a libgmp-style bigint subtraction without the subtraction. But the difference between a bigint and a fiat_25519_tight_field_element is that the latter has nontrivial equivalences. I'm worried gt might consider an unreduced fiat_25519_tight_field_element of value, say 2²⁵⁵ - 1 (equivalent to 18) as greater than 30, for example. I was trying to test this but ran into point (1) above.

I'll be reviewing the rest of the PR, but I think these should be addressed before merging. Thanks again!

[jmwample]

1. It seems like I can't get the Elligator2 tests to fail, even when they definitely should. Is a KAT missing?

2. I was wondering about the correctness of the gt function in the fiat backend in the diff. [fiat_25519_tight_field_element] has nontrivial equivalences. I'm worried gt might consider an unreduced fiat_25519_tight_field_element of value, say 2²⁵⁵ - 1 (equivalent to 18) as greater than 30, for example.

You are definitely correct about the implementation of the gt function. I was relying on the idea that other implementations were using a reliable compare, but it turns out they fall victim to the exact issue that you describe. Really what they are trying to check is whether the value is negative, but they do so with by subtracting $(p-1)/2$ and
checking for overflow. However a direct negative check doesn't work for the Legacy implementations because the signal and kleshni implementations use the subtract and check for overflow which has a different result from a proper greater than comparison (or is_negative()) on the canonical representation of the FieldElement. I am trying to decide what the best way to handle the separate cases is. I have a proper ct_gt() implementation, but I am not sure it is even useful as the RFC9380 and Randomized variants can use is_negative() and Legacy can't use a proper greater-than.

I replaced the return value of FieldElement::ct_gt with the constant Choice::from(0u8). Running cargo test --features "alloc,elligator2" did not produce any errors.

For the RFC9380 and Randomized variants the effect is less obvious as it only takes effect when $\sqrt{\frac{-(a+A)}{2a}} \ne \sqrt{\frac{-a}{2*(a+A)}}$ (code ref) which for some reason is not the case for the test vectors from RFC8380 or my randomly selected test vectors. However a simple test where they are different is the identity value (which I have added in my work branch looking into this).

---

As an aside, is the implementation of is_negative() flawed in a similar way? In the elligator2 implementations
the check for negative values is to determine if the FieldElement is less than $(p-1)/2 = 2^{254}-10$ while the
FieldElement::is_negative() function only checks if the low bit is set meaning that values between $2^{254}-9$ and $2^{254}-1$ would be considered positive?

[carloskiki]

Is there anything blocking this PR?

I would love to have it included in a future release as I find myself needing this feature.

[rozbb]

I'm gonna spend some time this week going thru this. I really appreciate this PR @jmwample. It's been tough co-maintaining this while having a full time job. I don't know a lot about Elligator2, and especially the distinguishing attacks mentioned here, so I will have to read a lot here, and probably ask a bunch of questions. Thank u in advance!

[carloskiki]

Thanks @rozbb!

[jmwample]

Great to hear! I rebased onto the latest changes in main. I am happy to help with any clarifications and work in any interface changes.

Okay, I am working on getting myself back up to speed as well. I think the current sticking point regards the following

• Do we care about compatibility with the Signal (and kleshni-C) implementations of elligator2?
  • If yes we have to use a custom compare that matches mpn_sub_n (which is distinct from an actual greater-than)
  • If no, we can just remove the Legacy mode, Signal test cases, nonspec_map_to_curve, and the greater than implementation all together

Things to consider here are:

• The pre-existing nonspec_map-to-curve digest feature was based on the Signal implementation so despite being deprecated, there may be people that want this functionality.
• The Legacy mode is computationally distinguishable from uniform random.
• The Legacy mode is not fully compatible with RFC9380

[rozbb]

Ok this was a lot but I left a first round of comments. The high level comment is that I'm not sure where any of this math comes from. It seems like a mix of the RFC, the paper, Signal's impl, and agl's impl. If that's actually the case, then this will need a lot of references and explanations.

One thing I realized is the inverse maps are not standardized. So the RFC cannot be followed, and in fact it looks like it is not followed (eg using a range check versus x mod 2 for the is-nonneg function). These choices should probably be documented somewhere public. We cannot interoperate with other Elligator implementations if we don't have a reference.

Sorry for the wall of text. This was a bunch of reading and I learned a lot

[rozbb]

Should probably be called sk_to_pubkey_representative or something. And the input should be called sk

[rozbb]

Also, why is this the API and not the actual point -> repr mapping?

[rozbb]

This function looks like it's supposed to be map_to_curve_elligator2 or map_to_curve_elligator2_curve25519, but it doesn't appear to follow RFC 9380 F.3 or G.2.1. Where is it from? Can you comment it clearer so I can follow along?

[rozbb]

Where are you copying this function from? I don't see this notation in the paper or the website. And the RFC doesn't specify inverses at all

[rozbb]

Ditto where did this function come from? According to the site, you only need to check u ≠ −A, −Zu(u + A) is a square, and v = 0 => u = 0

[rozbb]

Why is this function called this? Shouldn't it be pubkey_v_is_nonneg? Confused me a lot while reading.

Also it'd be nice to refactor to remove this function entirely. IMO it's doing too many ops here (mask, compute pubkey, extract v coord, return is-nonneg)

[rozbb]

This isn't clamping, right? This is just clearing the top 2 bits which are random

[rozbb]

Ditto

[rozbb]

I don't understand the distinction. The two things you linked above do not seem to point out any flaw in the RFC, rather implementation errors in some libraries. Is there something inherent in the RFC that requires modification? The impl above already appears to do randomization.

It appears that the actual issue is that the above RFC 9380 mapping always clears the cofactor, which is a leaky thing to do. If that's the issue, why not change the API to do the real mappings point -> field rather than scalar -> point -> field?

[rozbb]

Comment needed

[rozbb]

Should mention this is map_to_curve_elligator2_edwards25519 from RFC 9380 section G.2.2