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

Add Elligator Square module #982

Closed
wants to merge 4 commits into from
Closed

Conversation

sipa
Copy link
Contributor

@sipa sipa commented Sep 28, 2021

Based on #979.

This adds a module with an implementation of the Elligator Squared algorithm for encoding/decoding public keys in uniformly random byte arrays.

@sipa sipa force-pushed the 202109_ellsq branch 2 times, most recently from 168f68b to af4047c Compare October 2, 2021 04:08
@sipa sipa force-pushed the 202109_ellsq branch 3 times, most recently from 87af2c4 to f1ad505 Compare November 9, 2021 22:23
Copy link
Contributor

@robot-dreams robot-dreams left a comment

Choose a reason for hiding this comment

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

tACK 7b1e626

I'll give a full ack once I carefully review:

  • Optimizations in section 3 of the writeup
  • Low-level implementation
  • Argument that the encoding is statistically indistinguishable from 64 random bytes, even with the minor changes (e.g. point at infinity, omitting some duplicate checks)

Basic question, what would go wrong if we use naive approaches such as:

(1) Given a random curve point (x1, y1), increment the x coordinate until you get another curve point (x2, y2). Use a random field element in [x1, x2) as the encoding. Decode by decrementing until you get an x corresponding to a curve point.

(2) Just generate and use a random field element as the encoding; both parties decrement x until they get a valid curve point.

I'm guessing the reason we don't do something like that is (1) will produce field elements that don't look random, and (2) will generate keys in a biased way?

src/modules/ellsq/tests_impl.h Show resolved Hide resolved
src/modules/ellsq/tests_impl.h Show resolved Hide resolved
@sipa
Copy link
Contributor Author

sipa commented Nov 12, 2021

@robot-dreams:

(1) Indeed, this will result in biased encodings, because the distance between field elements which are valid X coordinates varies, and attacker-known, so they can detect a bias: encodings for which the previous and next valid X coordinate are close together will occur more frequently.

(2) Biased private key isn't really a problem as long as (a) the bias is small and (b) the bias isn't recognizable from the encodings/public keys, which is the case here. There is another problem however: the encoder wouldn't know the corresponding private key.

@robot-dreams
Copy link
Contributor

Oops, good point about (2), I am silly. 🤣

@sipa
Copy link
Contributor Author

sipa commented Nov 12, 2021

But about (1), if there were a known upper bound on the distance between valid X coordinates (AFAIK there isn't one, but more than ~128 is probably negligibly rare), you could actually use f(x) = (first point with largest X coordinate not larger than x) as mapping function in the scheme. I suspect that's significantly slower than what we have now though, as you'd need avg 128 iterations, and counting the distance between valid X coordinates around each...

@robot-dreams
Copy link
Contributor

robot-dreams commented Nov 13, 2021

Code Review ACK 7b1e626, modulo verifying the proof that the encoding is actually indistinguishable from 64 random bytes

I opened the different implementations of f, r side by side and checked that:

  • Section 2.6 is equivalent to Section 3
  • Section 3 is equivalent to the broken-down Sage implementation
  • The C and Sage implementations are equivalent

Basic question:

  • The use of SHA256 for generating branch values seems reasonable, but just wondering, is there a justification that this use of SHA256 gives you a secure PRG (taking rnd32 as the key)? (Is this a well-known construction?)

Copy link
Contributor

@robot-dreams robot-dreams left a comment

Choose a reason for hiding this comment

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

ACK 7b1e626, I don't think the rest of my comments are blocking.

  • I'm happy with the argument that the sampling algorithm produces a uniformly random preimage of a curve point
  • Although I wasn't yet able to calculate the exact statistical distance between the distribution implemented here and the distribution described in the paper (handling infinity being the only difference), I suspect that it's O(1/p), i.e. negligible unless I made a big mistake in my estimation.
  • I've convinced myself (in part with the help of the section starting with "Back to Delphia" in this article) that the use of SHA256 to generate random branches / field elements is secure under a random oracle model 😅 sipa's favorite! Here, an attacker would have no choice but to guess rnd32.

The only thing I haven't verified is that the function f is indeed "well-distributed", but I don't want to block this PR until I've learned the background material needed to check that—I'm happy to accept that without verifying the proof for now.

Finally, would it make sense to either:

  1. Elaborate on the statement not terribly non-uniform in https://github.com/sipa/writeups/tree/main/elligator-square-for-bn#25-dealing-with-infinity
  2. Make the encoder NOT target the f(u) = -f(v) case, so that it's really easy to show that the statistical distance between the implemented distribution and the one in the paper is neglible (the decoder can still handle that case, so that every 64-byte string decodes to a valid public key)

src/modules/ellsq/main_impl.h Show resolved Hide resolved
return 1;
}
/* Only returned in case the provided pubkey is invalid. */
return 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this happen? The coverage report shows that this line doesn't get hit, and secp256k1_pubkey_load never seems to return 0.

If not, would it make sense to either:

  • Add an explicit check that the pubkey is valid
  • Replace the if (secp256k1_pubkey_load(ctx, &p, pubkey)) { with a VERIFY_CHECK and de-indent the body

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, good point. I will address this.

Perhaps we should also make secp256k1_load_pubkey return void (not in this PR, though).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

secp256k1_pubkey_load can actually return 0, through its ARG_CHECK macro.

Copy link
Contributor

@robot-dreams robot-dreams Nov 16, 2021

Choose a reason for hiding this comment

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

Oh nice, I missed that.

Do we want more strict checks here than ARG_CHECK(!secp256k1_fe_is_zero(&ge->x))? For example the following passes, but I wouldn't expect 1000 randomly generate 64-byte strings to all produce valid pubkeys:

        int N = 1000;
        int j = 0, success = 0;
        secp256k1_ge g;
        secp256k1_pubkey pubkey;
        secp256k1_testrand256(pubkey.data);
        secp256k1_testrand256(pubkey.data + 32);
        for (j = 0; j < N; j++) {
            if (secp256k1_pubkey_load(ctx, &g, &pubkey)) success++;
        }
        CHECK(success == N);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think that's needed. That check is a last-resort sanity check (several functions will set pubkeys on output to all-zeros when they return failure). Triggering that situation implies you're already using the API incorrectly; a secp256k1_pubkey object must contain a valid public key, in normal usage.

src/modules/ellsq/main_impl.h Show resolved Hide resolved
@sipa
Copy link
Contributor Author

sipa commented Nov 15, 2021

Elaborate on the statement not terribly non-uniform in https://github.com/sipa/writeups/tree/main/elligator-square-for-bn#25-dealing-with-infinity

I should drop that, as it's confusing. Given the immensely low probability of hitting infinity in the first place, how it is handled is totally irrelevant.

This comment aims to indicate that it's better than e.g. mapping infinity to the generator or so (which it is, as it'd mean the generator has ~2n ellsq preimages, while other points only have ~n), but I don't think that's a useful justification given it's only changing an negligibly frequent event anyway. The real justification is: it's probably the easiest way to implement this edge case.

Make the encoder NOT target the f(u) = -f(v) case, so that it's really easy to show that the statistical distance between the implemented distribution and the one in the paper is neglible (the decoder can still handle that case, so that every 64-byte string decodes to a valid public key)

I don't think it should be hard to show the difference is negligible, given the fact that even triggering (much less observing) a case in which the outcome is different is negligible (it requires picking a uniformly random u for which f(u)=-P).

@robot-dreams
Copy link
Contributor

OK, I'm fully on board with this now! If you're curious, see https://gist.github.com/robot-dreams/86302bfca28bf9dc83ced365cd428f64 for way too much detail that nobody needed or asked for.

@sipa sipa force-pushed the 202109_ellsq branch 3 times, most recently from 60e718b to 4f018e2 Compare November 16, 2021 21:21
@robot-dreams
Copy link
Contributor

ACK 07a4ef5

stratospher added a commit to stratospher/bitcoin that referenced this pull request Jan 7, 2022
stratospher added a commit to stratospher/bitcoin that referenced this pull request Jan 7, 2022
- source: src/modules/ellsq/tests_impl.h from bitcoin-core/secp256k1#982
- 3 tests are added:
    1. Generate random field elements and use f to map it to a valid group element on the curve.
       Then use r to map back the group element to the 4 possible pre-images, out of which only 1
       is the field element we started with.
    2. Generate random group elements on the curve and use r to map it to the 4 possible pre-images.
       Then map the field elements back to the group element and check if it's the same group element
       we started with, also making sure that the pre-images are distinct.
    3. Verify the test cases which consists of group element and the 4 field elements.Map the group element
       to the 4 possible pre-images using r and check whether it's consistent with the 4 field elements
       given in the test case. Map the field element back to the group element using f and check whether
       it matches the test case.
stratospher added a commit to stratospher/bitcoin that referenced this pull request Jan 7, 2022
- source: src/modules/ellsq/tests_impl.h from bitcoin-core/secp256k1#982
- 2 tests are added:
    1. Generate a random curve point and encode it to ell64 using encode_bytes(). Then decoding it back to the curve point and check if it is the same curve point we started with.
    2. Decode the test vector bytes to a group element. Serialise it into the compressed pubkey format. Check if this pubkey matches the test vector pubkey.
stratospher added a commit to stratospher/bitcoin that referenced this pull request Jan 7, 2022
- source: src/modules/ellsq/tests_impl.h from bitcoin-core/secp256k1#982
- 2 tests are added:
    1. Generate a random curve point and encode it to ell64 using encode_bytes(). Then decoding it back to the curve point and check if it is the same curve point we started with.
    2. Decode the test vector bytes to a group element. Serialise it into the compressed pubkey format. Check if this pubkey matches the test vector pubkey.
stratospher added a commit to stratospher/bitcoin that referenced this pull request Jan 7, 2022
- source: src/modules/ellsq/tests_impl.h from bitcoin-core/secp256k1#982
- 2 tests are added:
    1. Generate a random curve point and encode it to ell64 using encode_bytes(). Then decoding it back to the curve point and check if it is the same curve point we started with.
    2. Decode the test vector bytes to a group element. Serialise it into the compressed pubkey format. Check if this pubkey matches the test vector pubkey.
stratospher added a commit to stratospher/bitcoin that referenced this pull request Jan 7, 2022
- source: src/modules/ellsq/tests_impl.h from bitcoin-core/secp256k1#982
- 3 tests are added:
    1. Generate random field elements and use f to map it to a valid group element on the curve.
       Then use r to map back the group element to the 4 possible pre-images, out of which only 1
       is the field element we started with.
    2. Generate random group elements on the curve and use r to map it to the 4 possible pre-images.
       Then map the field elements back to the group element and check if it's the same group element
       we started with, also making sure that the pre-images are distinct.
    3. Verify the test cases which consists of group element and the 4 field elements.Map the group element
       to the 4 possible pre-images using r and check whether it's consistent with the 4 field elements
       given in the test case. Map the field element back to the group element using f and check whether
       it matches the test case.
stratospher added a commit to stratospher/bitcoin that referenced this pull request Jan 7, 2022
- source: src/modules/ellsq/tests_impl.h from bitcoin-core/secp256k1#982
- 2 tests are added:
    1. Generate a random curve point and encode it to ell64 using encode_bytes(). Then decoding it back to the curve point and check if it is the same curve point we started with.
    2. Decode the test vector bytes to a group element. Serialise it into the compressed pubkey format. Check if this pubkey matches the test vector pubkey.
stratospher added a commit to stratospher/bitcoin that referenced this pull request Jan 8, 2022
Source: src/modules/ellsq/tests_impl.h
from bitcoin-core/secp256k1#982
stratospher added a commit to stratospher/bitcoin that referenced this pull request Jan 8, 2022
- source: src/modules/ellsq/tests_impl.h from bitcoin-core/secp256k1#982
- 3 tests are added:
    1. Generate random field elements and use f to map it to a valid group element on the curve.
       Then use r to map back the group element to the 4 possible pre-images, out of which only 1
       is the field element we started with.
    2. Generate random group elements on the curve and use r to map it to the 4 possible pre-images.
       Then map the field elements back to the group element and check if it's the same group element
       we started with, also making sure that the pre-images are distinct.
    3. Verify the test cases which consists of group element and the 4 field elements.Map the group element
       to the 4 possible pre-images using r and check whether it's consistent with the 4 field elements
       given in the test case. Map the field element back to the group element using f and check whether
       it matches the test case.
stratospher added a commit to stratospher/bitcoin that referenced this pull request Jan 8, 2022
- source: src/modules/ellsq/tests_impl.h from bitcoin-core/secp256k1#982
- 2 tests are added:
    1. Generate a random curve point and encode it to ell64 using encode_bytes(). Then decoding it back to the curve point and check if it is the same curve point we started with.
    2. Decode the test vector bytes to a group element. Serialise it into the compressed pubkey format. Check if this pubkey matches the test vector pubkey.
stratospher added a commit to stratospher/bitcoin that referenced this pull request Jan 8, 2022
Source: src/modules/ellsq/tests_impl.h
from bitcoin-core/secp256k1#982
stratospher added a commit to stratospher/bitcoin that referenced this pull request Jan 8, 2022
- source: src/modules/ellsq/tests_impl.h from bitcoin-core/secp256k1#982
- 3 tests are added:
    1. Generate random field elements and use f to map it to a valid group element on the curve.
       Then use r to map back the group element to the 4 possible pre-images, out of which only 1
       is the field element we started with.
    2. Generate random group elements on the curve and use r to map it to the 4 possible pre-images.
       Then map the field elements back to the group element and check if it's the same group element
       we started with, also making sure that the pre-images are distinct.
    3. Verify the test cases which consists of group element and the 4 field elements.Map the group element
       to the 4 possible pre-images using r and check whether it's consistent with the 4 field elements
       given in the test case. Map the field element back to the group element using f and check whether
       it matches the test case.
stratospher added a commit to stratospher/bitcoin that referenced this pull request Jan 8, 2022
- source: src/modules/ellsq/tests_impl.h from bitcoin-core/secp256k1#982
- 2 tests are added:
    1. Generate a random curve point and encode it to ell64 using encode_bytes(). Then decoding it back to the curve point and check if it is the same curve point we started with.
    2. Decode the test vector bytes to a group element. Serialise it into the compressed pubkey format. Check if this pubkey matches the test vector pubkey.
stratospher added a commit to stratospher/bitcoin that referenced this pull request Jan 8, 2022
Source: src/modules/ellsq/tests_impl.h
from bitcoin-core/secp256k1#982
stratospher added a commit to stratospher/bitcoin that referenced this pull request Jan 8, 2022
- source: src/modules/ellsq/tests_impl.h from bitcoin-core/secp256k1#982
- 3 tests are added:
    1. Generate random field elements and use f to map it to a valid group element on the curve.
       Then use r to map back the group element to the 4 possible pre-images, out of which only 1
       is the field element we started with.
    2. Generate random group elements on the curve and use r to map it to the 4 possible pre-images.
       Then map the field elements back to the group element and check if it's the same group element
       we started with, also making sure that the pre-images are distinct.
    3. Verify the test cases which consists of group element and the 4 field elements.Map the group element
       to the 4 possible pre-images using r and check whether it's consistent with the 4 field elements
       given in the test case. Map the field element back to the group element using f and check whether
       it matches the test case.
stratospher added a commit to stratospher/bitcoin that referenced this pull request Jan 8, 2022
- source: src/modules/ellsq/tests_impl.h from bitcoin-core/secp256k1#982
- 2 tests are added:
    1. Generate a random curve point and encode it to ell64 using encode_bytes(). Then decoding it back to the curve point and check if it is the same curve point we started with.
    2. Decode the test vector bytes to a group element. Serialise it into the compressed pubkey format. Check if this pubkey matches the test vector pubkey.
stratospher added a commit to stratospher/bitcoin that referenced this pull request Jan 8, 2022
Source: src/modules/ellsq/tests_impl.h
from bitcoin-core/secp256k1#982
stratospher added a commit to stratospher/bitcoin that referenced this pull request Jan 8, 2022
- source: src/modules/ellsq/tests_impl.h from bitcoin-core/secp256k1#982
- 3 tests are added:
    1. Generate random field elements and use f to map it to a valid group element on the curve.
       Then use r to map back the group element to the 4 possible pre-images, out of which only 1
       is the field element we started with.
    2. Generate random group elements on the curve and use r to map it to the 4 possible pre-images.
       Then map the field elements back to the group element and check if it's the same group element
       we started with, also making sure that the pre-images are distinct.
    3. Verify the test cases which consists of group element and the 4 field elements.Map the group element
       to the 4 possible pre-images using r and check whether it's consistent with the 4 field elements
       given in the test case. Map the field element back to the group element using f and check whether
       it matches the test case.
stratospher added a commit to stratospher/bitcoin that referenced this pull request Jan 8, 2022
- source: src/modules/ellsq/tests_impl.h from bitcoin-core/secp256k1#982
- 2 tests are added:
    1. Generate a random curve point and encode it to ell64 using encode_bytes(). Then decoding it back to the curve point and check if it is the same curve point we started with.
    2. Decode the test vector bytes to a group element. Serialise it into the compressed pubkey format. Check if this pubkey matches the test vector pubkey.
@sipa
Copy link
Contributor Author

sipa commented Jan 25, 2022

Rebased.

@robot-dreams
Copy link
Contributor

ACK f997dad (aside from most recent build / CI changes) based on:

@sipa
Copy link
Contributor Author

sipa commented Jan 31, 2022

Mental note: simplify this after #1033.

stratospher added a commit to stratospher/bitcoin that referenced this pull request Feb 3, 2022
Source: src/modules/ellsq/tests_impl.h from bitcoin-core/secp256k1#982
stratospher added a commit to stratospher/bitcoin that referenced this pull request Feb 3, 2022
- source: src/modules/ellsq/tests_impl.h from bitcoin-core/secp256k1#982
- 3 tests are added:
    1. Generate random field elements and use f to map it to a valid group element on the curve.
       Then use r to map back the group element to the 4 possible pre-images, out of which only 1
       is the field element we started with.
    2. Generate random group elements on the curve and use r to map it to the 4 possible pre-images.
       Then map the field elements back to the group element and check if it's the same group element
       we started with, also making sure that the pre-images are distinct.
    3. Verify the test cases which consists of group element and the 4 field elements.Map the group element
       to the 4 possible pre-images using r and check whether it's consistent with the 4 field elements
       given in the test case. Map the field element back to the group element using f and check whether
       it matches the test case.
stratospher added a commit to stratospher/bitcoin that referenced this pull request Feb 3, 2022
- source: src/modules/ellsq/tests_impl.h from bitcoin-core/secp256k1#982
- 2 tests are added:
    1. Generate a pubkey from a random curve point and encode it using ellsq_encode(). Then decode it back to a pubkey and check if it is the same pubkey we started with.
    2. Decode the test vector bytes to a compressed pubkey using ellsq_decode() and check if it matches test vector pubkey.
sipa and others added 4 commits June 28, 2022 16:17
This introduces variants of the divsteps-based GCD algorithm used for
modular inverses to compute Jacobi symbols. Changes compared to
the normal vartime divsteps:
* Only positive matrices are used, guaranteeing that f and g remain
  positive.
* An additional jac variable is updated to track sign changes during
  matrix computation.
* There is (so far) no proof that this algorithm terminates within
  reasonable amount of time for every input, but experimentally it
  appears to almost always need less than 900 iterations. To account
  for that, only a bounded number of iterations is performed (1500),
  after which failure is returned. The field logic then falls back to
  using square roots to determining the result.
* The algorithm converges to f=g=gcd(f0,g0) rather than g=0. To keep
  this test simple, the end condition is f=1, which won't be reached
  if started with g=0. That case is dealt with specially.
This adds a module with an implementation of the Elligator Squared
algorithm for encoding/decoding public keys in uniformly random
byte arrays.
@sipa
Copy link
Contributor Author

sipa commented Jul 7, 2022

Closing, as Elligator Square seems strictly inferior in performance and complexity to ElligatorSwift.

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.

3 participants