ethereum / devp2p

Ethereum peer-to-peer networking specifications
979 stars 275 forks source link

discv5: protocol version v5.1 #157

Closed fjl closed 3 years ago

fjl commented 4 years ago

This change defines the first stable (non-draft) spec version of discv5.

Work in progress. Still needs:

Closes #152 Closes #141 Closes #135 Closes #131 Closes #117 Closes #83

pipermerriam commented 4 years ago

Looking at the wire protocol document it doesn't appear to specify the computation of the checksum field. It does appear to be defined in https://github.com/ethereum/devp2p/issues/152 as crc64("discv5" || dest-id)

pipermerriam commented 4 years ago

I'm realizing that the new documents don't mention the checksum and it is only present in the svg images that visualize the packet encoding.

fjl commented 4 years ago

Yeah, sorry, I need to update the images. In a discussion a couple weeks ago, we realized that the checksum is silly because the obfuscation layer hides away the plaintext protocol identifier, and the recipient node won't be able to decrypt the packet header if it is sent to the wrong node.

pipermerriam commented 4 years ago

@fjl are you referring to "Proposal 2" from https://github.com/ethereum/devp2p/issues/152 which uses aesctr_encrypt(masking-key, iv, plain-header) to obfuscate the header?

fjl commented 4 years ago

Yes

pipermerriam commented 4 years ago

I've generated the following test vectors that I'd be interested in validating since they represent my collective interpretation of the spec as documented in this PR, #152, as well as potentially some inference in areas where neither fully documents something yet.

# MessagePacket (flag=0)
initiator-key: 0x01010101010101010101010101010101
nonce: 0x020202020202020202020202
source-node-id: 0x0303030303030303030303030303030303030303030303030303030303030303
dest-node-id: 0x0404040404040404040404040404040404040404040404040404040404040404
message: PING: request-id=0x01  enr-sequence-number=0x00
auth-data: MessagePacket aesgcm-nonce= 0x050505050505050505050505

encoded-packet: 0x0aa2702c8135c1276a1c00c81e04f6cec58e496f3d2dd3283f34cbdeb97364e15e201821954a5ad0f20a16d9385f1d56eb847b9d8b936c09661ee331db73fd4166474b1a962a42522611e34e608ad5f8b619eeef6ad820edf74d35

# WhoAreYouPacket (flag=1)
initiator-key: 0x01010101010101010101010101010101
nonce: 0x020202020202020202020202
source-node-id: 0x0303030303030303030303030303030303030303030303030303030303030303
dest-node-id: 0x0404040404040404040404040404040404040404040404040404040404040404
message: PING: request-id=0x01  enr-sequence-number=0x00
auth-data: WhoAreYouPacket: request-nonce=0x050505050505050505050505  id-nonce=0x0606060606060606060606060606060606060606060606060606060606060606  enr-sequence-number=0x7

encoded-packet: 0xdb05f3d612ef0bf450a932aab441ab0b794619e0818f750601eeeb25017f1be55e187c749f21dab55d3031ecf7a0395adddab0c85508c14a92cd3ecefb2536aec95c618998c914a8bf7f9998bce735cc305ae648e825eb28554b6abd79193724a72a043a3dabc6cdecb49ba9e35251522611e35ecf69051276c32e0b149b8fee2c6729

# HandshakePacket (flag-2) (WITHOUT ENR)
initiator-key: 0x01010101010101010101010101010101
nonce: 0x020202020202020202020202
source-node-id: 0x0303030303030303030303030303030303030303030303030303030303030303
dest-node-id: 0x0404040404040404040404040404040404040404040404040404040404040404
message: PING: request-id=0x01  enr-sequence-number=0x00
auth-data: HandshakePacket: 
    auth-data-head=(version=0x01 signature_size=0x40 ephemeral_key_size=0x21)  
    id-signature=0x05050505050505050505050505050505050505050505050505050505050505050505050505050505050505050505050505050505050505050505050505050505  
    ephemeral-public-key=0x060606060606060606060606060606060606060606060606060606060606060606  
    record=0x

encoded-packet:  0xba4358d5b59616eae9bba87bdf70363fdef69b3ca7006d54713a1bc41c72c465414fc0e2672960eb14b6094c1a38a7d1bd2f2dd6021b4050963d07eb88b6e3c4b8b8c3342690f6d53fe1f09682fb139ad46f5fa2734c2e4eecd835ca726d95e80e5d73d90d022505bc8353749eda90128b32ff00e08687ec40d03fb818736041b7e49d295b4c55f351be13ed6dc5bb5959f5c80e7931c1565e2e925a2c9ea2522611e35e7c358bfc3987df51a2770f0c7b2f7f

# HandshakePacket (flag-2) (WITH ENR)
initiator-key: 0x01010101010101010101010101010101
nonce: 0x020202020202020202020202
source-node-id: 0x0303030303030303030303030303030303030303030303030303030303030303
dest-node-id: 0x0404040404040404040404040404040404040404040404040404040404040404
message: PING: request-id=0x01  enr-sequence-number=0x00
auth-data: HandshakePacket: 
    auth-data-head=(version=0x01 signature_size=0x40 ephemeral_key_size=0x21)  
    id-signature=0x05050505050505050505050505050505050505050505050505050505050505050505050505050505050505050505050505050505050505050505050505050505  
    ephemeral-public-key=0x060606060606060606060606060606060606060606060606060606060606060606  
    record=0xf88bb840242198bffec59a15dd134b44a125e39a3f0330aa481ca7359f17dd1889e7427a3f8974a0c4e64cd518619e12ffec0ece01f85e7807d3aa62d07443b847b7566635826964827634826970848d3ca05389736563703235366b31a103e3449d6a29899c3b29b2c4bc27b6407070d7f33b5524fd2e7f1d0d3dfa9bc68483746370829ceb8375647082851d

encoded-packet:  0x2e47005ae2c0c50eea1f32fd86029a128e26ad45a6e1b5d0dc58759528d237b0ba5bf7d24087e3add447709db01dd1cf53304a64f26e02d6a1dd895932bc2621e60eef1301be21851a04d3dd2b9398d290573cb2baceafe8e5e438953c787832e8295642ed61c676122e98c15e1fe7cb4319ffc72125c63c5bfb760ea945c456d1a919239078d632585ae192184f6e7ddef806d9d2e8530803c07a18597d4631d675ec7815939aea0194d88c608cf4fc253eacb43d8d5cb71a5c722cd835050b7cc606e4d0e9e8b48cb9fd72c48a39c43c268e2307644f13743ba1ce404b68b3588e5b8eff48ae34175e15d5fbcd1ecb0a572abbea4310986a8176b01da5be2a9b42f40740eebd965dd31ec4a0f6acb3e4f9ff935352165ea9fe24dd4357d628df45ce79884255f83e38489c522611e3d9cf00dc787a383be694bef0bb69b456

Note: The ENR record in the last example was randomly generated. If these get elevated to a more official status, that record should re-generated with appropriate test vector data.

pipermerriam commented 3 years ago

Consider this case. There is a handshake in progress between Initiator and Recipient. The Recipient has sent the WhoAreYouPacket and is waiting for the HandshakePacket to finalize the handshake.

The Recipient receives a MessagePacket.

At this stage there are two possibilities.

The Recipient must make a choice.

Since the Recipient doesn't know which case it is, I believe we should encourage implementations to assume case B and to always respond with a WhoAreYouPacket when encountering this situation.

This is a subtle enough case that it seems potentially worth documenting in some sort of implementers guide since an implementation that chooses the other route will result in both Initiator and Recipient having incomplete in-progress handshakes that will have to wait to be timed-out/expired before either is likely to initiate again.

fjl commented 3 years ago

I'm finally updating the spec with the header masking, and two new ideas came up:

fjl commented 3 years ago

Damn, it looks like I wiped out the definitions of id-nonce-input and id-signature when moving the handshake to theory.md. Will put them back!

fjl commented 3 years ago

Another thing we could still change: feed the dest-id into id-signature.

I'm suggesting this because I got mildly concerned that the removal of the encrypted auth-response might create an MITM risk. In the 5.0 handshake, we encrypted the id-signature with a separate session secret, but this was removed and the signature is now sent clear text (technically within the masked-header container, but that's no better than clear text).

Consider communication A -> X -> B, with X being the MITM node.

Preconditions:

Communication flow:

So, no MITM possible.

But can X replay the id-signature somehow to authenticate as A to a different node C? Technically not, because X can't influence the id-nonce sent by node C. But it feels too shaky with all those malleable parts. I think it might be safer to put B's ID into the signature to ensure this can't happen, if only to make the id-signature an even more specific statement.

zilm13 commented 3 years ago

@fjl Do you have Geth fork with all in it?

fjl commented 3 years ago

@zilm13 my current branch is here: https://github.com/fjl/go-ethereum/tree/p2p-discover-v5-new-format

fjl commented 3 years ago

@pipermerriam a bit more context about this idea:

I think it might be a good idea to distinguish requests from responses via the flag field.

When we discussed this on the phone, I wasn't able to explain why it is generally impossible to perform the handshake using a response message, and why this change is desirable. Let me try to explain it in text using a communication example:

A <- B   FINDNODE (message packet)

  -- B drops the session here --

A -> B   NODES (message packet)
A <- B   WHOAREYOU
A -> B   NODES (handshake packet)

This handshake will not work because B doesn't have a response to send to confirm the session. The outcome of the handshake is not verifiable because A doesn't know if B ever received NODES or if B is really who they are claiming to be. This is explained in the spec, in the last sentence of the handshake section:

Node A receives the response and authenticates/decrypts it with the new session keys. If decryption succeeds, node B's identity is verified, A considers the new session keys valid and uses them for all further communication.

Essentially, the session will be confirmed only when the recipient actually responds to the handshake packet. But B cannot respond if the handshake packet contains a response message like NODES.

If we want to make this work better, we have two options:

I prefer option (1) because it's simpler and will prevent useless handshakes when the session ends in between a request and response interaction. This is all very unlikely to happen anyway because B probably won't drop the session from its cache while it has a pending request against A.

Side note: it's actually not possible to replicate the bad handshake example with go-ethereum because it won't send NODES as a handshake packet following WHOAREYOU. This is because go-ethereum has distinct code paths for sending requests (via the call queue) and sending responses.

fjl commented 3 years ago

@pipermerriam I'm trying to decode the test vectors you made here, but running into trouble. The AES-GCM payload of the PING message packet doesn't decrypt with the provided key 0x01010101010101010101010101010101.

It would be nice if we could use the following secp256k1 private keys for test vectors:

node-a-key: eef77acb6c6a6eebc5b363a475ac583ec7eccdb42b6481424c60f59aa326547f
node-b-key: 66fb62bfbd66b9177a138c1e5cddbe4f7c30c343e94e68df8769459cb1cde628

The node IDs (with the 'v4' scheme) corresponding to these keys are:

node-a: aaaa8419e9f49d0083561b48287df592939a8d19947d8c0ef88f2a4856a69fbb
node-b: bbbb9d047f0488c0b5a93c1c3f2d8bafc7c8ff337024a55434a0d0555de64db9

Defining the private keys is important because it will allow us also make test vectors with valid signatures on the node record, for example.

pipermerriam commented 3 years ago

@fjl

Regarding https://github.com/ethereum/devp2p/pull/157#issuecomment-685517761

IIUC the problem with allowing response messages trigger handshake initiations is that according the the protocol the initiator will not consider the handshake to have been successful until they have received and decode a message using the derived session keys.

It seems we don't need to do anything to remedy this situation because either:

  1. Some other message will be sent by the recipient using the new keys, allowing the initiator to validate the keys are correct.
  2. No other messages will pass between the two nodes and the session will eventually be evicted.
  3. The initiator will eventually send a message to the initiator, and upon receiving the response to that second message, the initiator can consider the session keys valid.

It seems the spec-as is, but maybe with a bit more narrative explanation of this exact situation can deal with this situation just fine.

pipermerriam commented 3 years ago

@fjl I think that my implementation needs to be updated to latest spec, after which I'll re-generate new test vectors (with the keys you recommended).

fjl commented 3 years ago

Here's the list of things that still need a decision or additional spec text:

pipermerriam commented 3 years ago

@fjl my opinions

Nashatyrev commented 3 years ago

@pipermerriam are those test vectors up-to-date? https://github.com/ethereum/devp2p/pull/157#issuecomment-669289421

fjl commented 3 years ago

Here are some new test vectors matching the current spec text. These are VALID packets, i.e. they could actually be sent on the wire. Unfortunately, I cannot provide all decoded fields in the same way Piper could, because my encoder doesn't output them in the same way. The intention with these test vectors is that your client should load the node-b-key and then be able to decrypt and authenticate them as-is.

The secp256k1 private keys used here are:

node-a-key = 0xeef77acb6c6a6eebc5b363a475ac583ec7eccdb42b6481424c60f59aa326547f
node-b-key = 0x66fb62bfbd66b9177a138c1e5cddbe4f7c30c343e94e68df8769459cb1cde628

WHOAREYOU packet (flag 1):

# src-node-id = 0xaaaa8419e9f49d0083561b48287df592939a8d19947d8c0ef88f2a4856a69fbb
# dest-node-id = 0xbbbb9d047f0488c0b5a93c1c3f2d8bafc7c8ff337024a55434a0d0555de64db9
# whoareyou.request-nonce = 0x0102030405060708090a0b0c
# whoareyou.id-nonce = 0x0102030405060708090a0b0c0d0e0f1000000000000000000000000000000000
# whoareyou.enr-seq = 0

00000000000000000000000000000000088b3d4342776668980a4adf72a8fcaa
963f24b27a2f6bb44c7ed5ca10e87de130f94d2390b9853c3ecb9ad5e368892e
c562137bf19c6d0a9191a5651c4f415117bdfa0c7ab86af62b7a9784eceb2800
8d03ede83bd1369631f9f3d8da0b45

Ping message packet (flag 0):

# src-node-id = 0xaaaa8419e9f49d0083561b48287df592939a8d19947d8c0ef88f2a4856a69fbb
# dest-node-id = 0xbbbb9d047f0488c0b5a93c1c3f2d8bafc7c8ff337024a55434a0d0555de64db9
# nonce = 0xffffffffffffffffffffffff
# read-key = 0x00000000000000000000000000000000
# ping.req-id = 0x00000001
# ping.enr-seq = 2

00000000000000000000000000000000088b3d4342776668980a4adf72a8fcaa
963f24b27a2f6bb44c7ed5ca10e87de130f94d2390b9853c3fcba22b1e9472d4
3c9ae48d04689eb84102ed931f66d180cbb4219f369a24f4e6b24d7bdc2a04

Ping handshake packet (flag 2):

# src-node-id = 0xaaaa8419e9f49d0083561b48287df592939a8d19947d8c0ef88f2a4856a69fbb
# dest-node-id = 0xbbbb9d047f0488c0b5a93c1c3f2d8bafc7c8ff337024a55434a0d0555de64db9
# nonce = 0xffffffffffffffffffffffff
# read-key = 0x4917330b5aeb51650213f90d5f253c45
# ping.req-id = 0x00000001
# ping.enr-seq = 1
# 
# handshake inputs:
# 
# whoareyou.request-nonce = 0x0102030405060708090a0b0c
# whoareyou.id-nonce = 0x0102030405060708090a0b0c0d0e0f1000000000000000000000000000000000
# whoareyou.enr-seq = 1
# ephemeral-key = 0x0288ef00023598499cb6c940146d050d2b1fb914198c327f76aad590bead68b6
# ephemeral-pubkey = 0x039a003ba6517b473fa0cd74aefe99dadfdb34627f90fec6362df85803908f53a5

00000000000000000000000000000000088b3d4342776668980a4adf72a8fcaa
963f24b27a2f6bb44c7ed5ca10e87de130f94d2390b9853c3dcb21d51e9472d4
3c9ae48d04689ef4d3d2602a5e89ac340f9e81e722b1d7dac2578d520dd5bc6d
c1e38ad3ab33012be1a5d259267a0947bf242219834c5702d1c694c0ceb4a6a2
7b5d68bd2c2e32e6cb9696706adff216ab862a9186875f9494150c4ae06fa4d1
f0396c93f215fa4ef52417d9c40a31564e8d5f31a7f08c38045ff5e30d966183
8b1eabee9f1e561120bc7fccc3d4569a69fdf04f31230ae4be20404467d9ea9a
b3cd

Ping handshake message packet (flag 2, with ENR):

# src-node-id = 0xaaaa8419e9f49d0083561b48287df592939a8d19947d8c0ef88f2a4856a69fbb
# dest-node-id = 0xbbbb9d047f0488c0b5a93c1c3f2d8bafc7c8ff337024a55434a0d0555de64db9
# nonce = 0xffffffffffffffffffffffff
# read-key = 0x4917330b5aeb51650213f90d5f253c45
# ping.req-id = 0x00000001
# ping.enr-seq = 1
# 
# handshake inputs:
# 
# whoareyou.request-nonce = 0x0102030405060708090a0b0c
# whoareyou.id-nonce = 0x0102030405060708090a0b0c0d0e0f1000000000000000000000000000000000
# whoareyou.enr-seq = 0
# ephemeral-key = 0x0288ef00023598499cb6c940146d050d2b1fb914198c327f76aad590bead68b6
# ephemeral-pubkey = 0x039a003ba6517b473fa0cd74aefe99dadfdb34627f90fec6362df85803908f53a5

00000000000000000000000000000000088b3d4342776668980a4adf72a8fcaa
963f24b27a2f6bb44c7ed5ca10e87de130f94d2390b9853c3dcaa0d51e9472d4
3c9ae48d04689ef4d3d2602a5e89ac340f9e81e722b1d7dac2578d520dd5bc6d
c1e38ad3ab33012be1a5d259267a0947bf242219834c5702d1c694c0ceb4a6a2
7b5d68bd2c2e32e6cb9696706adff216ab862a9186875f9494150c4ae06fa4d1
f0396c93f215fa4ef52417d9c40a31564e8d5f31a7f08c38045ff5e30d966183
8b1eabee9f1e561120bcc4d9f2f9c839152b4ab970e029b2395b97e8c3aa8d3b
497ee98a15e865bcd34effa8b83eb6396bca60ad8f0bff1e047e278454bc2b3d
6404c12106a9d0b6107fc2383976fc05fbda2c954d402c28c8fb53a2b3a4b111
c286ba2ac4ff880168323c6e97b01dbcbeef4f234e5849f75ab007217c919820
aaa1c8a7926d3625917fccc3d4569a69fd8aca026be87afab8e8e645d1ee8889
92
Nashatyrev commented 3 years ago

May be it makes sense to limit the message req-id field size? I would suggest max 8 bytes

fjl commented 3 years ago

Yes that would make sense.

mkalinin commented 3 years ago
fjl commented 3 years ago

If we want we can use sha256("discovery-id-nonce" || initiator-key) for id_sign. This enshrines a session identity into the signature.

I don't want to do this because then we would need to run the key derivation before checking the signature. It increases the computational cost of verifying invalid handshakes.

Why would we need to redefine the formula? I don't see any risk in replacing id-nonce with iv and shrinking it down to 16 bytes. From KDF standpoint this change does look safe.

Yes, I've come to the same conclusion, will update it.

fjl commented 3 years ago

Looks like there is a bug in my test vectors: the ephemeral pubkey is 65 bytes, but should be 33 bytes (compressed key). I will update them once I have the other spec changes in as well.

mkalinin commented 3 years ago

We propose to include a couple of recommendations that nodes may implement to mitigate DoS and eclipse attacks. Counter-measures are described here https://github.com/ethereum/devp2p/issues/161

cc @jrhea

fjl commented 3 years ago

I've just pushed the last spec update. Thank you so much for all your amazing feedback on this pull request. I have tried to address all of the issues you guys have raised.

Here's the list of changes in the last couple commits:

We need to stop changing the spec now and focus on interop.

TODO: update images

fjl commented 3 years ago

Seeing all this confusion about the challenge fields and the concerns about packet header authentication, I have decided to use the simplest solution possible: just use the entire unmasked packet header as the challenge data for both id signature and KDF, and also use it as authenticated data in the AES/GCM step.

From a spec perspective, this change is very nice and I wonder why we didn't do it like that earlier.

From an implementation point of view, this change sucks, at least for my own implementation. Since the data dependencies have changed so much, I had to change the ordering of some of the encoding steps in order to be able to have the encoded headers and masking-iv available when encrypting the message. But it's implemented in go-ethereum now, and the test vectors are also updated.

BTW, I noticed that there was a problem with my HKDF usage, my own tests worked on Go 1.15 but not on earlier versions. This is now corrected and I hope you will be able to match the new test vectors.

kdeme commented 3 years ago

BTW, I noticed that there was a problem with my HKDF usage, my own tests worked on Go 1.15 but not on earlier versions. This is now corrected and I hope you will be able to match the new test vectors.

I can confirm that I can now decode your latest provided test vectors :+1:

fjl commented 3 years ago

Very nice! All of them?

If we get another implementation to agree, (and when I'm done updating the images) the PR can be merged!

pipermerriam commented 3 years ago

I'm now passing all of the test vectors in my implementation.

Nashatyrev commented 3 years ago

I'm passing them too with JVM implementation