Add BIP324 encrypted p2p transport de-/serializer (only used in tests) (
p2p) Jun 17, 2020
The PR branch HEAD was 7aa91cd1 at the time of this review club meeting.
Note: in this PR Review club we will review the first three commits only
Expose MAC length in chacha_poly_ahead_h,
Add BIP324 short-IDs to
Add BIP324 v2 transport serializer and deserializer).
Today’s PR is part of the implementation of
BIP324: Version 2 Peer-to-Peer
proposed by Jonas Schnelli to improve privacy on the p2p network.
Currently, P2P messages are transported in plaintext, which
makes them vulnerable to eavesdropping by infrastructure entities such as
ISPs. Such entities can tamper with, drop, or delay messages between anonymous
peers. A malicious infrastructure entity could use this to influence the
network topology, determine the origin of transactions, or perform attacks
against off-chain protocols.
BIP 324 is the second proposal for an encryption standard for the Bitcoin P2P
network. The previous proposal (
also authored by Jonas Schnelli and has been withdrawn.
In BIP 324, messages are encrypted by the stream cipher
ChaCha20 with a Poly1305 Message
A BIP 324 session starts with an
Elliptic Curve Diffie Hellman key
to establish a shared session key between peers (this key exchange is not yet
implemented). From this shared sesssion secret, 2 keys,
K_1 is used to encrypt the 3 bytes packet length.
K_2 is used to
encrypt and authenticate the rest of the packet. Using symmetric keys
K_2, the receiver first decrypts the length number and from this offset
authenticates the packet. If authentication succeeds, the receiver decrypts the
message payload and hands the content to the processing layer.
Other PRs that are part of BIP 324 implementation:
Jonas Schnelli’s talk
at Breaking Bitcoin 2019 has additional information on BIP 324. A further step to prevent eavesdropping on the P2P network would be to add
peer authentication, which is outside the scope of BIP 324. How to
authenticate peers in an anonymous network is an
area of active
Did you review the PR?
Concept ACK, approach ACK, tested ACK, or
(Don’t forget to put your PR review on GitHub.)
What’s your process to review a PR implementing a new BIP? Did you read the
BIP or the code first? How can you ensure that BIP is correct? How can you ensure
that code implements the BIP correctly?
BIP 324 introduces a new message structure, notably with short command ID.
What do you think about those new short command ID?
Why was the Chacha20/Poly1305 construction chosen? Have you read Bitcoin
Core implementations of these primitives?
Beyond code review, how can testing be improved? What failure cases should
Could the serialization/deserialization code be better documented or
simplified? Consider the choices of data structures and algorithms.
1 13:00 <jnewbery> #startmeeting
12 13:01 <troygiorshev> hi
13 13:01 <jnewbery> ariard is our host. Over to you, ariard!
14 13:01 <ariard> thanks jnewbery!
15 13:02 <ariard> so to get started, usual question who has reviewed the PR ?
19 13:02 <ariard> that's great! have you left any comment on it ?
21 13:02 <willcl_ark> no :(
27 13:04 <ariard> okay what was your process to review a PR implementing a BIP ?
28 13:04 <ariard> did you read the BIP or code first ? or dig into context around the BIP, like reading old mail threads ?
29 13:05 <willcl_ark> The context to me seems pretty rational from the BIP
30 13:05 <raj_149> 1. read the bip. 2. read on missing gaps that i dont understand yet 3. read the code. 4. tally the code with the BIP.
31 13:05 <willcl_ark> (i read the BIP first)
32 13:05 <ariard> raj_149: quick reply, it's hard to be sure you have enough pointer without a second implementation not hurting wrinkles
33 13:06 <ariard> willcl_ark: what seems you rational here ? what this BIP is trying to achieve?
34 13:06 <emzy> I read the BIP first.
35 13:06 <ariard> raj_149: what were the missings gaps?
36 13:07 <willcl_ark> ariard: we want to prevent kinds of MITM attacks which are easy on unencrypted traffic.
37 13:07 <raj_149> ariard: depends on individual's previous knowledge. for me i didn't knew chacha20, poly1305 and AEAD details. so had to read them up first.
38 13:09 <raj_149> willcl_ark: it seems this doesn't necessarily solves all forms MITM, i guess the draft also mentions this somewhere. if i am not mistaken.
39 13:09 <ariard> willcl_ark: in Bitcoin context, why it makes sense to encrypt traffic ? Most of data is already public like blocks, headers
40 13:10 <ariard> raj_149: yes generally you have to, it's pretty lengthy reviewing this kind of PR when it involves cryptographic primitives
41 13:10 <ariard> have you been through the process to assert this pick up of cryptographic primitives compare to alternative ?
42 13:10 <jnewbery> (anyone can feel free to answer any of these questions)
43 13:11 <pinheadmz> encrypted traffic obscures the fact that you even have abitcoin node running
44 13:11 <raj_149> ariard: for start it stops traffic analysis and IP to publickey linkage. In general all sorts of privacy leaks that can occur by monitoring network traffic of a node.
45 13:11 <pinheadmz> and withou, people can snoop on the network connectivity graph, TX propogation and maybe resolve the origin of a ne TX
46 13:12 <ariard> pinheadmz: obscures to whom? not if you're running on port 8333
47 13:12 <pinheadmz> who says I am? :-)
48 13:12 <pinheadmz> I dunno, oppressive regimes
49 13:12 <willcl_ark> if you can tamper with messages, you can get legitimate (truthful) peers banned
50 13:12 <ariard> no I mean your ISP can still learn you're running bitcoin if you're using port 8333
51 13:13 <pinheadmz> ariard I understand but you dont have to use port 8333 and if my ISP sees a bunch of encrypted blobs coming into port 9000, that is at least plausible denyability
52 13:13 <jnewbery> and even if you're not using port 8333, bitcoin p2p traffic has an unmistakable pattern
53 13:13 <ariard> pinheadmz: with tx propogation for sure it obfuscates tx origin for on-path attacs, now spy peers can still observe origin by connecting to you
54 13:14 <pinheadmz> certainly for SPV or Neutrino, there is privacy leaked with plaintext mesages
55 13:14 <pinheadmz> but that privacy is leaked to the full node you connect to anyway
56 13:14 <ariard> raj_149: which publickey you're pointing to ? what kind of privacy leaks you can think of beyond tx origin ?
57 13:15 <pinheadmz> Im not sure if theres an attack vector around this, but you mightbe abe to tell where a node is in the sync process by reading their traffic
58 13:15 <ariard> jnewbery: that's a huge concern, even tx propagation may still leak being interactive and we don't do padding
59 13:16 <emzy> it's not only good to secure the p2p trafic from your ISP eyes, it's also good agains state level surveillance.
60 13:16 <pinheadmz> ariard do you know - i asked this on the BIP draft - why not use Noise Protocol for the key exchange the way Lightning does?
61 13:16 <ariard> pinheadmz: if you assume attacker knows about 1) block announcement 2) listen for transactions flooding 3) can map encrypted blob size to transactions received?
62 13:17 <raj_149> ariard: i am not exactly sure on the kind of possible leaks that can happen over clear text data. I read somewhere by monitoring network traffic and some kind of triangulation observer can link origin transactions with IP addresses, without connecting to me as a node. Would like to know about other possible leaks that i dont know about. clearly there should be some over encrypted data.
63 13:17 <jnewbery> pinheadmz: you surely can tell if a node is in IBD, even if you used a stream cipher to encrypt the messages
65 13:18 <ariard> pinheadmz: I'm not sure but I would say historical reasons, Noise Protocol wasn't fully spec out when BIP 151 was first submitted
66 13:18 <emzy> also in flight modification of the data is easy posible without MAC
67 13:19 <sipa> pinheadmz, ariard: there is some interaction with development of a private authentication protocol we've been working on
68 13:19 <sipa> noise's authentication relies on revealing identities, which is something we want to avoid
69 13:19 <willcl_ark> if you wanted to eclipse someone, tampering with their current peers' messages (and having them disconnected) seems like a good start
70 13:19 <willcl_ark> maybe this PR doesn't quite solve that though
72 13:20 <ariard> emzy: but what about a MitM attacker intercepting communications from both side? there is no authentication with BIP324
73 13:21 <sipa> the full functionality obviously needs encryption + authentication
74 13:21 <willcl_ark> ONe thing I noticed it this BIP (and lightning) vs "standard" ChaCha20 Poly1305 is that we encrypt the packet length, why is this?
75 13:21 <sipa> but authentication is very easy to add in a modular way once we have encryption
76 13:21 <jnewbery> willcl_ark: if you can tamper with traffic you can get a peer disconnected, but not banned. The terminology is a bit confusing currently, but the disconnected peer is allowed to reconnect
77 13:21 <willcl_ark> jnewbery: ok, thanks for the clarification
78 13:22 <emzy> ariard: Right, but it's only posible at first connection attempt. A running session is save from MitM attack. So it is better then not.
79 13:22 <jnewbery> of course, if you're able to tamper with messages, you could just continue to do that to get them disconnected again, or just block all traffic
80 13:22 <willcl_ark> even a "ban" is always temporary (some length of time) also I think, right?
81 13:22 <raj_149> ariard: thanks.
82 13:22 <ariard> sipa: but we could have reuse a Noise pattern with ephemeral key pairs? It doesn't mandate how you exchange key pairs?
83 13:22 <jnewbery> willcl_ark: 24 hours by default
85 13:22 <sipa> ariard: right, that could work, using noise purely with ephemeral keys
86 13:23 <ariard> willcl_ark: good question, to make dumb traffic analysis harder?, see real_and_random and my posts on the BIP
87 13:23 <pinheadmz> would it make sense to have static keys for "precious" nodes ?
88 13:23 <sipa> though i expect even in that case we'd want our own "vendored" version, with secp256k1 keys etc
89 13:23 <pinheadmz> for example, connecting my personal SPV to my remote Full Node
90 13:23 <sipa> pinheadmz: yes, that's called authentication!
91 13:23 <sipa> pinheadmz: and of course that's the eventual goal
92 13:23 <willcl_ark> ariard: Oh i didn't see the comments at the end of the BIP!
93 13:23 <pinheadmz> i thought you meant authentication like message integirty
94 13:24 <sipa> pinheadmz: ah, no
95 13:24 <sipa> pinheadmz: history is that we original had BIP150/BIP151 for auth and enc
96 13:24 <sipa> BIP151 was replaced with BIP324
97 13:24 <pinheadmz> eevntual goal, so nodes would have <pubkey>@IP:port kind of hostnmaes like lightning ?
98 13:24 <ariard> jnewbery: I was verifying just before is this really what we do in AcceptConnection
99 13:24 <ariard> pinheadmz: please not permanent pubkeys like in LN
100 13:25 <sipa> pinheadmz: perhaps that's getting off-topic, but the idea is that there wouldn't be any *observable* identities (for obvious reasons)
101 13:25 <sipa> but if you *know* the identity of a peer you're connecting to, you can private determine whether it's them
102 13:25 <pinheadmz> excellent
103 13:25 <sipa> (without them even knowing that authentication succeeds)
105 13:26 <ariard> emzy: right, but in practice if you assume ISP-capabilities like attackers they would be in place for intercepting at any moment of the session
106 13:27 <willcl_ark> ariard: so am I right after reading the comments in thinking we only encrypt the packet length, but still don't MAC it (not re-read the code changes)?
107 13:27 <sipa> willcl_ark: that's right
108 13:27 <ariard> sipa: right does Noise make it mandatory to use Curve25519?
109 13:27 <sipa> ariard: i honestly don't know
110 13:27 <emzy> ariard: If you have a long running node, the time window is very narrow.
111 13:28 <ariard> willcl_ark: yes we don't MAC it, BOLT8 does it, I spent few hours yesterday trying to find even theoritical attacks on modifying a length field MAC
112 13:29 <sipa> ariard: secp256k1 isn't supported by the noise framework, but it'd be possible to of course use a noise protocol that we instantiate ourselves
113 13:29 <willcl_ark> slightly off topic, but I appreciated the short command ID improvement here
114 13:29 <sipa> though at that point i think the advantages are rather small
115 13:29 <ariard> and found nothing, so likely too minor
116 13:29 <sipa> at a high conceptual level, bip324 is effectively that
117 13:29 <willcl_ark> ariard: sipa, thanks
118 13:30 <raj_149> sipa: can you through some pointer on what do you mean by noise framework?
119 13:30 <raj_149> *throw
121 13:30 <raj_149> thanks..
122 13:30 <ariard> used by whatsapp or lightning among others
123 13:32 <raj_149> feels like kinda same thing BIP324 is doing?
124 13:32 <ariard> sipa: yes I slightly agree, does anyone have done at least of properties comparison between a Noise pattern with ephemeral keys vs BIP324 ?
125 13:32 <sipa> raj_149: bip324 is encryption only, no auth
126 13:33 <sipa> (it's authenticated encryption, but that's detecting tampering, not identity of the peer)
127 13:33 <ariard> emzy: I'm not sure the time window matters here, internet infrastructure is quite static
128 13:33 <ariard> okay let's move to next question
129 13:33 <raj_149> sipa: got it. ariard: no but seems interesting exercise.
130 13:34 <ariard> BIP324 introduces a new message structure, notably with short command ID, what do you think about it ?
131 13:34 <raj_149> loved it. :D
132 13:34 <ariard> especially the new short command ID? is it worth the complexity vs bandwidth saving argued ?
133 13:34 <ariard> raj_149: haha please explain
134 13:34 <raj_149> thougb currently it seems hardcode. Not p2p agrrement. is that correct?
135 13:35 <ariard> raj_149: yes no p2p agreement for now
136 13:35 <ariard> which makes sense bip324 is already big enough, it can be done in future specifications
137 13:35 <willcl_ark> I saw some discussion over Cap'n'proto (for the GUI), was there a concious choice not to also leverage that here?
138 13:35 <troygiorshev> At this level it seems just fine to have the compact IDs
139 13:35 <willcl_ark> (but very happy to see bandwidth reductions!)
140 13:35 <raj_149> ariard: well if we only have a finite set of command lists, it makes sense to map them into numbers instead of transmiting them as chars.
141 13:36 <sipa> willcl_ark: i don't see how those are related at all
142 13:36 <sipa> the P2P protocol is not an RPC protocol (it's message based, not function call based), and capnproto doesn't do encryption
143 13:36 <willcl_ark> well the short IDs are basically a protocol spec?
144 13:36 <willcl_ark> hmmm ok, seemed like defining a protocol for messages to me
145 13:37 <sipa> you could probably route capnproto over the encrypted stream bip324 defines
146 13:37 <sipa> but they're fundamentally a very different layer
147 13:37 <willcl_ark> as in, analagous to how Protobufs encode messages according to a .proto file
148 13:37 <willcl_ark> sipa: OK
149 13:37 <sipa> willcl_ark: it's like asking why not use HTTP instead of SSL
150 13:38 <ariard> raj_149: can you envision what nodes would negotiate a different set of mappings?
152 13:39 <raj_149> ariard: honestly i couldn't come up with any reason. maybe for some they just need a subset of those so dont wanna assign all the numbers? not sure if its worth the effort though.
153 13:39 <willcl_ark> sipa: it just seemed to me that e.g. Protobuf kind of does what that massive `if else` statment does, but with a well-defined proto file
154 13:40 <willcl_ark> but will research it more :)
155 13:40 <sipa> willcl_ark: bitcoin already has a p2p protocol, we're not proposing to replace it with something else entirely
156 13:40 <ariard> raj_149: well we may have experimentation with new P2P messages, like LN custom messages, I can see people trying to communicate through P2P to their wallets
157 13:40 <sipa> (or at least, no reason to do that simultaneously with introducing encryption)
158 13:41 <ariard> especially if we add authentication
159 13:41 <willcl_ark> sipa: yes I gues you can't just do it for a single part (command)
160 13:41 <willcl_ark> anyway I'm very much in favour of some bandwidth reductions on this front!
161 13:41 <ariard> next question: why ChaCha20/Poly1305 were chosen ? have you read implementation
162 13:41 <lightlike> it could lead to confusion when there are several bips that introduce new p2p messages (that need new numbers). e.g. BIP 157 messages that are merged are not assigned a number yet.
163 13:42 <sipa> lightlike: a new bip can introduce short numbers for newly introduced messages
164 13:42 <raj_149> ariard: yes they can use negotiations to add extra *special* messages between them. But these messages still needs to be defined in the protocol right? So they can as well be assigned with short_id there?
165 13:42 <sipa> so that there doesn't need to be coordination
166 13:42 <troygiorshev> lightlike: agreed. Might not be too hard to say "just give this the next available number" though
167 13:42 <willcl_ark> ariard: faster than AES on most CPUs
168 13:44 <willcl_ark> I think it's better for chips without AES-NI, right?
169 13:44 <ariard> raj_149: if they are negotiated and protocol is custom, your bandwidth savings priorities may not be equivalent to the default ones
170 13:45 <ariard> willcl_ark: right for software implementations
171 13:45 <raj_149> ariard: I am not seeing how bandwidth saving is different in a negotiated short_id than a hardcoded one.
172 13:46 <raj_149> ariard: on poly1305, why are we using goto in the code? isn't there any better way to do it? for example the donna implementation doesn't seem use it.
174 13:48 <ariard> raj_149: let's say I have my own range of custom messages which make 80% of my traffic, there is no room left in the default short command ID table, by negotiating I can overrules with my own maps and thus save traffic?
175 13:48 <willcl_ark> ariard: is there another reason that better (faster) performance on average across different CPUs?
176 13:48 <willcl_ark> than*
177 13:49 <sipa> raj_149: i assume it's based on the openssh code
179 13:49 <sipa> (i haven't checked)
180 13:49 <raj_149> ariard: why there wont be any room left in default table if the protocol itself defines the short_ids while defining the command?
181 13:49 <ariard> willcl_ark: yes it's design have been vetted as more conservative than AES and less prone to cryptographic breakage
183 13:50 <sipa> ariard: chacha more conservative than aes?
184 13:50 <ariard> see cryptanalysis table at the end of Salsa20 paper, though I've not compare to ones on AES
185 13:50 <sipa> i wouldn't say that
186 13:50 <sipa> it's just faster and more modern, and well-studied
187 13:50 <sipa> (aes being a block cipher has inherent complexity, which is unnecessary for stream encryption)
188 13:51 <ariard> sipa: it's claimed by Bernstein in the ChaCha paper, "Salsa20/20 is amore conservative design than AES, and the community seems to have rapidlygained confidence in the security of the cipher."
189 13:51 <ariard> which should be taken with carefulness
190 13:51 <sipa> ariard: such claims by the author should be taken with a grain of salt, i think :)
191 13:51 <sipa> but i guess i see what he means
192 13:52 <ariard> I completly agree and why I said I've not read cryptanalysis AES to compare
193 13:52 <sipa> but i wouldn't claim that chacha is more well-analyzed than AES
194 13:52 <willcl_ark> perhaps, "good enough" (and faster)?
195 13:53 <troygiorshev> modern and well-analyzed (if not-quite-as-well-analyzed) seems notable
196 13:53 <willcl_ark> was the alternative AES + CBC? becuase it would seem prefereable over that
197 13:53 <sipa> AES-GCM would be the typical alternative
198 13:54 <willcl_ark> ah ok
199 13:54 <sipa> or AES-CTR-GCM more precisely
200 13:54 <sipa> AES-CTR is a stream cipher like ChaCha20; GCM is a MAC like Poly1305
201 13:54 <ariard> raj_149: which protocol are you talking about ? If I experiment with my custom one, it won't be supported by the default implementation
202 13:55 <willcl_ark> sipe: thanks. I have much cryptography to learn!
203 13:55 <ariard> next question: how testing can be improved ? what failure cases should be tested?
204 13:55 <jnewbery> 5 minutes left!
205 13:55 <raj_149> ariard: ah i see that now.
206 13:57 <troygiorshev> it's great that fuzzing is already in
207 13:57 <troygiorshev> a parallel implementation in python could be nice
208 13:57 <raj_149> ariard: we definitely need functional tests once its fully getched. So far it only implements en/decryption, which seems to be adequately tested in unit test.
209 13:58 <raj_149> *fetched
210 13:58 <ariard> troygiorshev: have you looked on fuzz scope ? what kind of cases could be missed by fuzzer?
212 13:58 <willcl_ark> I couldn't get the fuzz tests to work on MacOS 10.15 :(
213 13:59 <troygiorshev> ariard: not deeply enough, no. definitely worth doing!
214 13:59 <willcl_ark> I couldn't see any tests for the re-keying process, but maybe missed them!
215 13:59 <ariard> last question: could the serialization/deserialization code be better documented or simplified ? have you look on data struct and algorithm choice ?
216 14:00 <raj_149> i have taken up an issue which will force me to look deeper into fuzzing. Planning to fuzz this once i get there.
217 14:00 <ariard> willcl_ark: I think the re-keying is still WIP, or at least we need to think about some of its implications
218 14:00 <willcl_ark> ariard: ah ok
219 14:01 <jnewbery> that's time!
220 14:01 <willcl_ark> thanks ariard very interesting discussion
222 14:01 <raj_149> thanks ariard, nice review session.
223 14:01 <thomasb0`> thanks ariard
224 14:01 <troygiorshev> thanks ariard!
225 14:01 <jnewbery> See you all there!
226 14:01 <ariard> great, I invite anyone who hasn't review yet the PR to do it with questions in mind :)
227 14:01 <emzy> thanks ariard!
229 14:01 <populate> thanks ariard
230 14:02 <SirRichard> Thanks ariard great discussion
231 14:02 <andrewtoth> thanks ariard! very interesting discussion
232 14:02 <furunodo> thanks!
233 14:03 <thomasb0`> sipa: what I was thinking about was to pick a random and compute either Euclide, theStack's pow, or the exponential ladder. What do you think?
235 14:03 <thomasb0`> to use several algorithms
236 14:03 <sipa> why wouldn't you always use the fastest
237 14:04 <thomasb0`> to track down bugs
238 14:04 <sipa> the tests already do both
239 14:04 <thomasb0`> then, no need for change
240 14:07 <furunodo> what's a good place to start for getting into the python test code, beyond bitcoin/test/functional/README.md? say if I wanted to start (or work on) a parallel test implemtation in python?
241 14:08 <furunodo> (as suggested by troygiorshev)
242 14:08 <thomasb0`> sipa: btw I was talking about python, only...
243 14:10 <thomasb0`> furunodo: it looks like you need to be confident to make relevant modifications by yourself
244 14:10 <sipa> furunodo: that would be the place to start
245 14:11 <sipa> all the functional python tests are in test/functional (with lots of utilities in test/functional/test_framework)