BIP324: Handshake prerequisites #23561

dhruv · 2021-11-20T22:50:38Z

Depends on #25361 for some constants, and on bitcoin-core/secp256k1#1129 for ellswift integrated XDH but can be reviewed independently. Only the last 5 commits belong to this PR.

This PR adds xonly ECDH and HKDF key derivation code for BIP324. Unit, bench and fuzz tests are included.

The dependency tree for BIP324 PRs is here.

DrahtBot · 2021-11-21T10:27:18Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	naumenkogs, PastaPastaPasta, theStack

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#27385 (net: extract Network and BIP155Network logic to node/network by jonatack)
#27294 (refactor: Move chain names to the util library by TheCharlatan)
#27254 (refactor: Extract util/fs from util/system by TheCharlatan)
#26684 (bench: add readblock benchmark by andrewtoth)
#25572 (refactor: Introduce EvictionManager and use it for the inbound eviction logic by dergoegge)
#25390 (sync: introduce a thread-safe generic container and use it to remove a bunch of "GlobalMutex"es by vasild)
#25268 (refactor: Introduce EvictionManager by dergoegge)
#23432 (BIP324: CKey encode/decode to elligator-swift by dhruv)
#23233 (BIP324: Add encrypted p2p transport {de}serializer by dhruv)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

naumenkogs · 2021-11-22T07:14:31Z

Concept ACK.
Moving forward with BIP324 is a good idea, and this code seems to not have any negative/unexpected side-effects.

src/key.cpp

src/test/fuzz/key.cpp

sipa · 2021-11-23T21:02:47Z

src/util/bip324.h

+
+    // Takes ownership of the ECDH secret and wipes the ephemeral secret once
+    // the derivation is complete.
+    explicit BIP324Keys(ECDHSecret&& ecdh_secret, const Span<uint8_t>& initiator_hdata, const Span<uint8_t>& responder_hdata);


Nit: pass Spans by value.

src/util/bip324.cpp

src/util/bip324.h

dhruv · 2021-11-25T20:25:45Z

Addressed review comments from @sipa. Ready for further review.

src/net.cpp

This makes it harder for others to tamper with our adjusted time.

dhruv · 2021-12-09T03:02:47Z

Addressed comments; Rebased with master to incorporate #23413. I am still investigating CI failures which seem like a timeout on Cirrus CI but re-running hasn't helped (other recent open PRs seem to be seeing this error as well). Meanwhile, ready for further review.

dhruv · 2022-01-22T02:27:00Z

Rebased against master. Ready for further review.

PastaPastaPasta

concept ACK / light utACK. I did a high level review, and everything makes sense to me.

I do have 1 tiny nit

src/bench/ecdh.cpp

dhruv · 2022-03-11T01:30:16Z

Rebased. Ready for further review.

dhruv · 2023-03-08T15:28:29Z

Rebased. Brought in changes from bitcoin-core/secp256k1#1129.

8034c67a48 Add doc/ellswift.md with ElligatorSwift explanation e90aa4e62e Add ellswift testing to CI 131faedd8a Add ElligatorSwift ctime tests 198a04c058 Add tests for ElligatorSwift 9984bfe476 Add ElligatorSwift benchmarks f053da3ab7 Add ellswift module implementing ElligatorSwift 76c64be237 Add functions to test if X coordinate is valid aff948fca2 Add benchmark for key generation 5ed9314d6d Add exhaustive tests for ecmult_const_xonly b69fe88d5e Add x-only ecmult_const version for x=n/d 427bc3c Merge bitcoin-core/secp256k1#1236: Update comment for secp256k1_modinv32_inv256 647f0a5 Update comment for secp256k1_modinv32_inv256 5658209 Merge bitcoin-core/secp256k1#1228: release cleanup: bump version after 0.3.0 28e63f7 release cleanup: bump version after 0.3.0 git-subtree-dir: src/secp256k1 git-subtree-split: 8034c67a48dc1334bc74ee4ba239111a23d9789e

…andshake

dhruv · 2023-03-21T04:03:30Z

Rebased.

DrahtBot · 2023-04-03T14:09:00Z

🐙 This pull request conflicts with the target branch and needs rebase.

fanquake · 2023-04-18T08:50:23Z

Closing this for now, as it's partly included in #27479.

3168b08 Bench test for EllSwift ECDH (Pieter Wuille) 42d759f Bench tests for CKey->EllSwift (dhruv) 2e5a8a4 Fuzz test for Ellswift ECDH (dhruv) c3ac9f5 Fuzz test for CKey->EllSwift->CPubKey creation/decoding (dhruv) aae432a Unit test for ellswift creation/decoding roundtrip (dhruv) eff72a0 Add ElligatorSwift key creation and ECDH logic (Pieter Wuille) 42239f8 Enable ellswift module in libsecp256k1 (dhruv) 901336e Squashed 'src/secp256k1/' changes from 4258c54..705ce7e (Pieter Wuille) Pull request description: This replaces #23432 and part of #23561. This PR introduces all of the ElligatorSwift-related changes (libsecp256k1 updates, generation, decoding, ECDH, tests, fuzzing, benchmarks) needed for BIP324. ElligatorSwift is a special 64-byte encoding format for public keys introduced in libsecp256k1 in bitcoin-core/secp256k1#1129. It has the property that *every* 64-byte array is a valid encoding for some public key, and every key has approximately $2^{256}$ encodings. Furthermore, it is possible to efficiently generate a uniformly random encoding for a given public key or private key. This is used for the key exchange phase in BIP324, to achieve a byte stream that is entirely pseudorandom, even before the shared encryption key is established. ACKs for top commit: instagibbs: reACK 3168b08 achow101: ACK 3168b08 theStack: re-ACK 3168b08 Tree-SHA512: 308ac3d33e9a2deecb65826cbf0390480a38de201918429c35c796f3421cdf94c5501d027a043ae8f012cfaa0584656da1de6393bfba3532ab4c20f9533f06a6

DrahtBot added Build system Utils/log/libs labels Nov 20, 2021

DrahtBot mentioned this pull request Nov 21, 2021

BIP324: CKey encode/decode to elligator-swift #23432

Closed

dhruv changed the title ~~BIP324 handshake prerequisites~~ BIP324: Handshake prerequisites Nov 23, 2021

sipa reviewed Nov 23, 2021

View reviewed changes

dhruv force-pushed the bip324-handshake branch from 8109854 to 00142e1 Compare November 25, 2021 20:24

This was referenced Nov 26, 2021

BIP324: Add encrypted p2p transport {de}serializer #23233

Closed

net: fix GetListenPort() to derive the proper port #20196

Merged

Update libsecp256k1 subtree to current master #23383

Merged

naumenkogs reviewed Dec 6, 2021

View reviewed changes

src/net.cpp Outdated Show resolved Hide resolved

naumenkogs reviewed Dec 6, 2021

View reviewed changes

src/net.cpp Show resolved Hide resolved

alirezaayande referenced this pull request Dec 7, 2021

p2p: Don't use timestamps from inbound peers

0c85dc3

This makes it harder for others to tamper with our adjusted time.

dhruv force-pushed the bip324-handshake branch from 00142e1 to e3143a9 Compare December 8, 2021 22:19

DrahtBot added the Needs rebase label Dec 18, 2021

dhruv force-pushed the bip324-handshake branch from e3143a9 to 773cbc9 Compare January 21, 2022 15:34

DrahtBot removed the Needs rebase label Jan 21, 2022

PastaPastaPasta reviewed Feb 7, 2022

View reviewed changes

src/bench/ecdh.cpp Outdated Show resolved Hide resolved

DrahtBot mentioned this pull request Feb 15, 2022

build: remove some no-longer-needed var unexporting from configure #24341

Closed

DrahtBot mentioned this pull request Mar 2, 2022

refactor: Split ArgsManager out of util/system #24455

Closed

DrahtBot added the Needs rebase label Mar 3, 2022

dhruv force-pushed the bip324-handshake branch from 773cbc9 to ea94adb Compare March 10, 2022 22:27

DrahtBot removed the Needs rebase label Mar 10, 2022

dhruv mentioned this pull request Mar 12, 2022

BIP324: Enable v2 P2P encrypted transport #24545

Closed

maflcko mentioned this pull request Mar 16, 2022

doc: Add template for empty release notes #24583

Merged

dhruv force-pushed the bip324-handshake branch from 9bcb8d4 to 151fb5e Compare February 21, 2023 03:35

DrahtBot removed the Needs rebase label Feb 21, 2023

dhruv force-pushed the bip324-handshake branch from 151fb5e to bbe5b3f Compare March 8, 2023 15:23

DrahtBot mentioned this pull request Mar 9, 2023

Update src/secp256k1 subtree to upstream release v0.3.0 #27230

Merged

DrahtBot added the Needs rebase label Mar 12, 2023

dhruv added 10 commits March 20, 2023 13:34

BIP324 Cipher Suite

187761f

Allow for RFC8439 AD in cipher suite interface

8405856

Merge branch 'bip324-cipher-suite' into bip324-handshake

ea3e911

Merge commit '984b3d0a1faabc1657e0dd33432fdadfd78335a9' into bip324-h…

007a92e

…andshake

Enable ECDH computation on secp256k1 keys

3d1b276

Bench test for ECDH

4b7be85

Fuzz test for ECDH

6ffa68f

HKDF key derivation from ECDH secret for BIP324

dc2527f

Fuzz test for BIP324 key derivation

178ef75

dhruv force-pushed the bip324-handshake branch from bbe5b3f to 178ef75 Compare March 21, 2023 04:03

DrahtBot removed the Needs rebase label Mar 21, 2023

This was referenced Mar 21, 2023

refactor: Extract util/fs from util/system #27254

Merged

refactor: Move chain names to the util library #27294

Closed

DrahtBot mentioned this pull request Apr 1, 2023

net, refactor: extract Network and BIP155Network logic to node/network #27385

Closed

DrahtBot added the Needs rebase label Apr 3, 2023

sipa mentioned this pull request Apr 17, 2023

BIP324: ElligatorSwift integrations #27479

Merged

fanquake closed this Apr 18, 2023

sipa mentioned this pull request May 12, 2023

BIP324 tracking issue #27634

Closed

43 tasks

bitcoin locked and limited conversation to collaborators Apr 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BIP324: Handshake prerequisites #23561

BIP324: Handshake prerequisites #23561

dhruv commented Nov 20, 2021 •

edited

Loading

DrahtBot commented Nov 21, 2021 •

edited

Loading

naumenkogs commented Nov 22, 2021

sipa Nov 23, 2021

dhruv Nov 25, 2021

dhruv commented Nov 25, 2021

dhruv commented Dec 9, 2021 •

edited

Loading

dhruv commented Jan 22, 2022

PastaPastaPasta left a comment

dhruv commented Mar 11, 2022

dhruv commented Mar 8, 2023

dhruv commented Mar 21, 2023

DrahtBot commented Apr 3, 2023

fanquake commented Apr 18, 2023

BIP324: Handshake prerequisites #23561

BIP324: Handshake prerequisites #23561

Conversation

dhruv commented Nov 20, 2021 • edited Loading

DrahtBot commented Nov 21, 2021 • edited Loading

Reviews

Conflicts

naumenkogs commented Nov 22, 2021

sipa Nov 23, 2021

Choose a reason for hiding this comment

dhruv Nov 25, 2021

Choose a reason for hiding this comment

dhruv commented Nov 25, 2021

dhruv commented Dec 9, 2021 • edited Loading

dhruv commented Jan 22, 2022

PastaPastaPasta left a comment

Choose a reason for hiding this comment

dhruv commented Mar 11, 2022

dhruv commented Mar 8, 2023

dhruv commented Mar 21, 2023

DrahtBot commented Apr 3, 2023

fanquake commented Apr 18, 2023

dhruv commented Nov 20, 2021 •

edited

Loading

DrahtBot commented Nov 21, 2021 •

edited

Loading

dhruv commented Dec 9, 2021 •

edited

Loading