x-only ECDH support #994

pull rustyrussell wants to merge 3 commits into bitcoin-core:master from rustyrussell:guilt/xonly-ecdh changing 3 files +120 −18
  1. rustyrussell commented at 1:03 am on October 20, 2021: contributor

    Technically this isn’t required: you can always implement an x-only hashfn yourself, and convert to a compressed pubkey to call secp256k1_ecdh(). But I’m not smart enough to figure that out myself (thanks @jonasnick !) so here we are.

    API question: should the xonly version take a scalar or a keypair?

    Impl question: should this implementation live in ecdh, extrakeys, or even a new ecdh_extrakeys module?

    Thanks!

  2. secp256k1_ecdh: add secp256k1_ecdh_hash_function_xonly_sha256
    In a future world where xonly pubkeys rule, it seems strange to hash
    the y-coordinate.  It also fails if you need to invert one of the
    keys for the ECDH.
    
    Suggested-by: Jonas Nick
    Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    839c8b6baf
  3. ecdh: add (optional) xonly variant.
    This is in the ecdh module, since that allows convenient inline access.
    The alternative would be to expose ecdh_core() and put this inside
    extrakeys, which would avoid open-coding secp256k1_xonly_pubkey_load().
    
    Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    4f7ebc44b6
  4. extrakeys: test for x-only ecdh.
    Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    a3a4cff8b8
  5. real-or-random commented at 9:02 am on October 20, 2021: contributor

    Thanks! Unfortunately this may be more involved than you’d expect.

    In a future world where xonly pubkeys rule, it seems strange to hash the y-coordinate.

    Hm, I don’t think it’s strange. So far, we use 33 bytes pubkeys but hash 64 bytes. Is that strange? I guess not.

    So my real question is if there are other advantages to hashing only x. Here’s one you may not have been aware of: The point multiplication for x-only can be made faster (#262).

    It also fails if you need to invert one of the keys for the ECDH.

    That’s exactly why people didn’t like it back then. When “in doubt”, crypto should rather fail than succeed. So the failure is a feature, and removing it adds a potential footgun. This is exactly why we didn’t like #262 back then, see #262 (comment) for the relevant comments.

    Now I don’t think the footgun potential is very high, and if you read the comments there, people weren’t strongly opposed to the idea. You could argue that BIP32 derivation and then converting the key to x-only is similar and BIP 340 accepted this fact (very explicitly even: https://github.com/sipa/bips/blob/bip-taproot/bip-0340.mediawiki#public-key-conversion). Moreover, X25519 (which is widely used, e.g., in Noise) uses x-only ECDH key and I never heard of any subtle issue, even though RFC7748 has a similar warning:

    Designers using these curves should be aware that for each public key, there are several publicly computable public keys that are equivalent to it, i.e., they produce the same shared secrets. Thus using a public key as an identifier and knowledge of a shared secret as proof of ownership (without including the public keys in the key derivation) might lead to subtle vulnerabilities.

    (https://datatracker.ietf.org/doc/html/rfc7748#section-7)

    Of course, all of this depends on the inputs to the ECDH function. If it’s an x-only -> x-only function, then formally speaking there’s no issue because you simply can’t negate the input pubkeys. On the other hand, that argument is mood if people effectively derive their x-only keys from full keys.

    Taking a further step back:

    In a future world where xonly pubkeys rule,

    Is it really true that x-only pubkeys will rule? The way I see x-only pubkeys is that this is mostly an optimization when putting keys on chain. So you can use your normal BIP32/whatever key derivation step and only as the very last step when you’re going to use the key on the chain, you drop a byte. But I admit that this is more a rule of thumb than a strict requirement, and I’m not sure if everyone will agree. I think this deserves a broader discussion. In general, I feel we have added a confusing element to BIP340 with x-only keys.

  6. rustyrussell commented at 11:09 pm on October 20, 2021: contributor

    The way I see x-only pubkeys is that this is mostly an optimization when putting keys on chain.

    I see things differently from you: compressed pubkeys are a vestigial tail, especially the 0x02/0x03 SEC1 encoding which is arbitrary.

    As a simple crypto API user, 32-byte pubkeys are a minor but clear API improvement. They align nicely, they index ~uniformly, and I expect with encouragement and tooling they will become the standard over time.

    So, I’ve started using x-only pubkeys everywhere in new greenfields projects. They’re used in https://github.com/rustyrussell/centurymetadata (where I hit this ECDH issue) and in BOLT12 offers so far. I’m considering them for my gossip update proposal, too.,

    I think future generations will thank us for encouraging and supporting their ubiquitous adoption.

  7. rustyrussell commented at 11:59 pm on October 20, 2021: contributor

    Hm, I don’t think it’s strange. So far, we use 33 bytes pubkeys but hash 64 bytes. Is that strange? I guess not.

    BTW, I think this is wrong: the default hashes 33 bytes, a-la a compressed key:

    0    unsigned char version = (y32[31] & 0x01) | 0x02;
    1    secp256k1_sha256 sha;
    2    (void)data;
    3
    4    secp256k1_sha256_initialize(&sha);
    5    secp256k1_sha256_write(&sha, &version, 1);
    6    secp256k1_sha256_write(&sha, x32, 32);
    7    secp256k1_sha256_finalize(&sha, output);
    
  8. real-or-random commented at 8:36 am on October 21, 2021: contributor

    Hm, I don’t think it’s strange. So far, we use 33 bytes pubkeys but hash 64 bytes. Is that strange? I guess not.

    BTW, I think this is wrong: the default hashes 33 bytes, a-la a compressed key:

    Oh you’re right! My point is if it were 64 bytes, it wouldn’t be strange either because the two representations are equivalent (whereas 32 bytes are not equivalent). Of course everyone needs to agree on the choice but apart from that you don’t even need to know as a user.

  9. real-or-random commented at 8:46 am on October 21, 2021: contributor

    As a simple crypto API user, 32-byte pubkeys are a minor but clear API improvement. They align nicely, they index ~uniformly, and I expect with encouragement and tooling they will become the standard over time.

    But then pragmatically speaking, what you really care about is that there’s an ECDH function which takes x-only pubkeys as input and it doesn’t really matter what is hashed. Or does it matter?

  10. rustyrussell commented at 2:33 am on October 22, 2021: contributor

    As a simple crypto API user, 32-byte pubkeys are a minor but clear API improvement. They align nicely, they index ~uniformly, and I expect with encouragement and tooling they will become the standard over time.

    But then pragmatically speaking, what you really care about is that there’s an ECDH function which takes x-only pubkeys as input and it doesn’t really matter what is hashed. Or does it matter?

    Yes, this is how I got into this mess. The current scheme breaks: given S1->PK1, S2->PK2, ECDH(S1,PK2) != ECHD(S2,PK1) unless PK1 and PK2 are both even (or maybe both odd, not sure). I had to invert the secret if its pubkey was 03…

    Further, interop matters, when setting standards (which is what libsecp256k1 is doing here, no doubt). We could also prepend a BIP-340 style tag, but prepending 02 or 03 is just a weird thing to lock in for future generations to ogle at?

    I know @gmaxwell disliked the malleation which x-only hashing enabled, but that problem doesn’t really exist in an x-only world. With that discounted, the fact that there’s a further optimization possible in future is compelling IMHO.

  11. real-or-random commented at 9:20 am on October 22, 2021: contributor

    The current scheme breaks: given S1->PK1, S2->PK2, ECDH(S1,PK2) != ECHD(S2,PK1) unless PK1 and PK2 are both even (or maybe both odd, not sure). I had to invert the secret if its pubkey was 03…

    Yes, ok, that’s indeed a problem… I hadn’t thought that through. Sorry that it took as that many posts to arrive at the core of the problem.

    Here’s some background: (as you probably know), the requirement to negate the secret key appears also with signatures. The problem is that you’d need to know the EC point/full pubkey in order to tell whether to negate or not. When we came up with x-only keys in BIP340, our thinking was:

    • We want to stay compatible with secret key generation, so we won’t touch that part. (And you can stil generate secret keys without computing the public key.)
    • For BIP340 sigs, we can do the negation at signing time, because the signing algorithm anyway needs to full pubkey because it’s put into the challenge hash. In secp256k1, this led to the keypair type in the API which avoids that you need to recompute the pubkey every time you create a signature, and then our signing function takes key pairs.

    Now this is different in case of ECDH. There’s no natural reason to for the ECDH function to take the own pubkey. So what could do? The only possibilities are:

    1. Stick with full keys
    2. Make the ECDH function take a key pair
    3. Hash only the x-coord (this PR)

    I understand you don’t like option 1. There’s a tendency to re-purpose signature pubkeys (which represent identities) as static keys for key exchange. You can say the ECDH module has been built with this use case in mind, otherwise we’d probably have different datatypes for ECDSA keys and ECDH keys. It’s useful in practice, and since BIP340 has x-only keys, people will want to repurpose those x-only keys for ECDH (this PR is an example). Careful crypto engineering tells us that repurposing keys is in general bad practice but in this case we even have a security proof that is very close to what’s done in practice [1].

    Option 2 is probably okay. It’s a performance penalty but if this is a static key, you need to compute the full pubkey part of the key pair only once.

    But if we accept that users want to re-purpose x-only signature keys for ECDH, then option 3 is just strictly better… In particular, option 2 does not protect against the malleability issue either (and we anyway don’t have evidence that it does any harm in practice).

    [1] https://crypto.stackexchange.com/a/3311

  12. Rogelio165 approved
  13. w0xlt cross-referenced this on Apr 2, 2022 from issue Convert `secp256k1_xonly_pubkey` to `secp256k1_pubkey` by w0xlt
  14. w0xlt commented at 8:51 pm on April 3, 2022: none

    I created an example of using x-only ECDH from this PR. Implements the “Basic Scheme” mentioned in the article “Silent Payment”. https://github.com/w0xlt/secp256k1/blob/a9677ad9f064efd6c1f91afb9fa2f5d2ab43cd03/examples/spbs.c

    More information at #1097 (comment)

  15. w0xlt cross-referenced this on Apr 17, 2022 from issue [Draft / POC] Silent Payments by w0xlt
  16. real-or-random cross-referenced this on Jul 12, 2022 from issue The principle of just-in-time xonly by LLFourn
  17. sipa commented at 6:48 pm on January 27, 2023: contributor
    I’ve opened an alternative: #1198, which uses more efficient x-only EC multiplication routines internally.
  18. real-or-random cross-referenced this on May 11, 2023 from issue x-only ECDH without sqrt by peterdettman
  19. real-or-random commented at 4:25 pm on May 11, 2023: contributor
    @rustyrussell We’re trying to gauge demand for this. Are you still interested in this feature? If yes, do you agree that this PR could be closed in favor of #1198?
  20. real-or-random added the label feature on May 11, 2023
  21. real-or-random added the label performance on May 11, 2023
  22. real-or-random removed the label performance on May 11, 2023

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-10-30 03:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me