Skip to content

RFC: Use EllSwift for ECDH#66

Merged
Fi3 merged 2 commits into
stratum-mining:mainfrom
Sjors:2024/01/ellswift
Mar 13, 2024
Merged

RFC: Use EllSwift for ECDH#66
Fi3 merged 2 commits into
stratum-mining:mainfrom
Sjors:2024/01/ellswift

Conversation

@Sjors

@Sjors Sjors commented Jan 10, 2024

Copy link
Copy Markdown
Contributor

This updates the spec to use ElligatorSwift encoded public keys. It's option 3 in #65.

Todo:

Bitcoin Core's BIP324 peer-to-peer encryption uses ElligatorSwift to encode the x-coordinate of a public key in 64 pseudorandom bytes. We currently encode the x-coordinate in 32 bytes which can be distinguished from random by a passive observer. This impact the first step of the handshake which uses plain text.

BIP324 also defines a method of x-only ECDH, whereas our current spec is ambiguous on how to perform ECDH and in practice implementations grind or negate the private key in order to use only an even y-coordinate. This won't be needed anymore.

https://github.com/bitcoin/bips/blob/master/bip-0324.mediawiki

@Sjors

Sjors commented Jan 10, 2024

Copy link
Copy Markdown
Contributor Author

One thing I haven't checked is how this impacts generating and verifying the certificate. In this proposal we encode public keys with 64 byte EllSwift instead of 32-byte x-only. The ECDH algorithm in libsecp256k1 can use this encoding directly which makes it more efficient.

Signing the certificate involves only the private key, so that's not impacted.
But the certificate itself contains two public keys and the verifier needs access to those public keys. My current thinking is that we should use regular x-only encoding for this.

I'm assuming that we can just convert the EllSwift encoded server static key to x-only format for this purpose, but I haven't implemented this yet.

@Sjors

Sjors commented Jan 12, 2024

Copy link
Copy Markdown
Contributor Author

I implemented certificate signing and verification in bitcoin/bitcoin#28983 so next week I should be able to test how that combines with EllSwift.

@Sjors

Sjors commented Jan 15, 2024

Copy link
Copy Markdown
Contributor Author

I added additional clarification on the fact that that we use three encodings for x-only:

  1. 32 bytes (still used for the certificate)
  2. 64 bytes ellswift (for the handshake)
  3. Base58 for config files. Since these will be publicly communicated by pools and template providers, I figured they should be mentioned in the standard.

I also further clarified that with EllSwift we do not assume anything about the y-coordinate parity.

@Sjors

Sjors commented Jan 16, 2024

Copy link
Copy Markdown
Contributor Author

Rebased after #64 (footnote numbering).

Comment thread 04-Protocol-Security.md Outdated
@lorbax

lorbax commented Jan 31, 2024

Copy link
Copy Markdown

Just a small clarification. Here

The following functions will also be referenced:

  • generateKey(): generates and returns a fresh secp256k1 keypair

    • Where the object returned by generateKey has two attributes:
      • .public_key, which returns an abstract object representing the public key
      • .private_key, which represents the private key used to generate the public key
    • Where the object also has a single method:
      • .serializeEllSwift() that outputs a 64-byte EllSwift encoded serialization of the X-coordinate of EC point (the Y-coordinate is ignored)
  • a || b denotes the concatenation of two byte strings a and b

I understand that the method .public_key() outputs an internal representation of ElligatorSwift encoding, and we use .serislizeEllSwift() for obtaining the 64-bytes serialization. But the here

Initiator generates ephemeral keypair and sends the public key to the responder:

  1. initializes empty output buffer
  2. generates ephemeral keypair e, appends e.public_key to the buffer (64 bytes plaintext EllSwift encoded public key)
  3. calls MixHash(e.public_key)
  4. calls EncryptAndHash() with empty payload and appends the ciphertext to the buffer (note that k is empty at this point, so this effectively reduces down to MixHash() on empty data)
  5. submits the buffer for sending to the responder in the following format

should it be `e.public_key().serislizeEllSwift() ?

@Sjors

Sjors commented Feb 1, 2024

Copy link
Copy Markdown
Contributor Author

should it be e.public_key().serializeEllSwift()?

That probably makes sense. So maybe:

Where the public_key object also has a single method:

And then:

appends e.public_key.serializeEllSwift() to the buffer

@Sjors

Sjors commented Feb 7, 2024

Copy link
Copy Markdown
Contributor Author

Done.

I also clarified which sections of BIP324 we refer to for EllSwift encoding and for EllSwift x-only ECDH.

I don't think we should copy large portions of the BIP324 text here. The EllSwift stuff we use from it should be implemented in exactly the way described there. If we also described that here, implementers might think it's somehow different. We could even accidentally introduce an incompatibility if we decide to change the wording here.

It's fine to provide a high level summary though. And it should be clear which parts of BIP324 are used, and that nothing else is used.

Comment thread 04-Protocol-Security.md Outdated
Comment thread 04-Protocol-Security.md
Comment thread 04-Protocol-Security.md Outdated
Comment thread 04-Protocol-Security.md
@Sjors

Sjors commented Feb 9, 2024

Copy link
Copy Markdown
Contributor Author

I changed the protocol name to Noise_NX_Secp256k1+EllSwift_ChaChaPoly_SHA256.

@Sjors

Sjors commented Feb 9, 2024

Copy link
Copy Markdown
Contributor Author

The discussion above so far suggests more clarification is needed. Happy to copy-paste suggestions...

But at least I think we agree the implementations are now correct.

Comment thread 04-Protocol-Security.md Outdated
Co-authored-by: Jakub Trnka <jak3264@gmail.com>
@Sjors Sjors force-pushed the 2024/01/ellswift branch from 77a0ed6 to 5f0981d Compare March 8, 2024 18:15
@Sjors

Sjors commented Mar 8, 2024

Copy link
Copy Markdown
Contributor Author

@jakubtrnka I added your suggestion

Comment thread 04-Protocol-Security.md
we use the 64 byte ElligatorSwift x-only encoding as defined in BIP324<sup>[7](#reference-7)</sup> under "ElligatorSwift encoding of curve X coordinates". This encoding uses 64-bytes instead of 32-bytes in order to produce a
pseudo-random bytesteam. This is useful because the protocol handshake starts with
each side sending ther public key in plain text. Additionally the use of X-only
ElligatorSwift ECDH removes the need to grind or negate private keys.

@lorbax lorbax Mar 12, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is true apparently, but in detail seems not.
When ElligatorSwift is proposed in BIP340.
When it says "In what follows, we use the notation from BIP340", if we look at the lift_x() function then it seems not x-only to me.
Indeed the last step is.
Return the unique point P such that x(P) = x and y(P) = y if y mod 2 = 0 or y(P) = p-y otherwise.
I am not familiar with the implementation in C++ so I cannot tell where in the precise implementation appears the pub key negation of y-coordinate

Comment thread 04-Protocol-Security.md

- for each keypair `(sk, bytes(P.x))` there is another keypair `(n - sk, bytes(P.x))`, where `n` is group order of secp256k1 curve
- Each result of `ECDH(sk, Q)` is equal to `ECDH(n - sk, Q)` for some EC point `Q` where `n` is group order of secp256k1 curve
To perform X-only ECDH we use ellswift_ecdh_xonly(ellswift_theirs, d) as described in BIP324<sup>[7](#reference-7)</sup> under "Shared secret computation". The result is 32 bytes.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

as above, I have some doubt that ElligatorSwift ecdh is x-only

@lorbax

lorbax commented Mar 12, 2024

Copy link
Copy Markdown

As I remarked in a comment, I don't think that ECDH procedure in the ElligatorSwift module is really x-only. I mean, the encoding itself is x-only, but I have some reasons to think that the ECDH is not. This is because it uses the function lift_x() (that can be found here which grinds the pubkey, see also the doc, it appears to be a key grinding).
In my opinion, we could:

  1. eliminate all the references of "x-only" for ECDH procedure and just refer to ElligatorSwift module for encoding and ECDH in BIP324 as a black box. This is not ideal for a specification, but it is easy to do.
  2. Understand the implementation of ElligatorSwift in bitcoincore and make a resume of what is done there from a theoretical point of view. Then include this resume in the specs and mention ElligatorSwift module in bitcoincore as strongly suggested for any implementation.

@Sjors

Sjors commented Mar 12, 2024

Copy link
Copy Markdown
Contributor Author

Understand the implementation of ElligatorSwift in bitcoincore

It's in secp256k1 which is used by Bitcoin Core, but also by SRI and many other projects. I also haven't looked at exactly what it does under the hood.

I think the confusion here is what "x-only" means. The y-coordinate is not transmitted to the counterparty, and it has no effect on the result of the ECDH operation. If we don't mention that property at all, that seems confusing. Just using the term "EllSwift" doesn't communicate this either.

But if anyone has a specific suggestion on an updated text, I can use that.

@Sjors

Sjors commented Mar 12, 2024

Copy link
Copy Markdown
Contributor Author

The Python implementation that's used for Bitcoin Core tests may be useful as another reference: https://github.com/bitcoin/bitcoin/blob/master/test/functional/test_framework/crypto/ellswift.py

@Fi3 Fi3 merged commit e6d886d into stratum-mining:main Mar 13, 2024
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.

4 participants