diff --git a/eth/p2p/discoveryv5/discovery_db.nim b/eth/p2p/discoveryv5/discovery_db.nim index 64b8293..ed01cbf 100644 --- a/eth/p2p/discoveryv5/discovery_db.nim +++ b/eth/p2p/discoveryv5/discovery_db.nim @@ -50,3 +50,10 @@ method loadKeys*(db: DiscoveryDB, id: NodeId, address: Address, r, w: var AesKey except CatchableError: return false +method deleteKeys*(db: DiscoveryDB, id: NodeId, address: Address): + bool {.raises: [Defect].} = + try: + db.backend.del(makeKey(id, address)) + return true + except CatchableError: + return false diff --git a/eth/p2p/discoveryv5/encoding.nim b/eth/p2p/discoveryv5/encoding.nim index 23e9f3d..51a4427 100644 --- a/eth/p2p/discoveryv5/encoding.nim +++ b/eth/p2p/discoveryv5/encoding.nim @@ -1,5 +1,5 @@ import - std/tables, nimcrypto, stint, chronicles, + std/[tables, options], nimcrypto, stint, chronicles, types, node, enr, hkdf, ../enode, eth/[rlp, keys] const @@ -7,7 +7,7 @@ const keyAgreementPrefix = "discovery v5 key agreement" authSchemeName* = "gcm" gcmNonceSize* = 12 - gcmTagSize = 16 + gcmTagSize* = 16 tagSize* = 32 ## size of the tag where each message (except whoareyou) starts ## with @@ -43,7 +43,8 @@ type DecodeStatus* = enum Success, HandshakeError, - PacketError + PacketError, + DecryptError proc randomBytes*(v: var openarray[byte]) = if nimcrypto.randomBytes(v) != v.len: @@ -159,17 +160,25 @@ proc encodeEncrypted*(c: Codec, headBuf.add(encryptGCM(writeKey, nonce, body, tag)) return (headBuf, nonce) -proc decryptGCM(key: AesKey, nonce, ct, authData: openarray[byte]): seq[byte] = +proc decryptGCM*(key: AesKey, nonce, ct, authData: openarray[byte]): + Option[seq[byte]] = + if ct.len <= gcmTagSize: + debug "cipher is missing tag", len = ct.len + return + var dctx: GCM[aes128] dctx.init(key, nonce, authData) - result = newSeq[byte](ct.len - gcmTagSize) + var res = newSeq[byte](ct.len - gcmTagSize) var tag: array[gcmTagSize, byte] - dctx.decrypt(ct.toOpenArray(0, ct.high - gcmTagSize), result) + dctx.decrypt(ct.toOpenArray(0, ct.high - gcmTagSize), res) dctx.getTag(tag) - if tag != ct.toOpenArray(ct.len - gcmTagSize, ct.high): - result = @[] dctx.clear() + if tag != ct.toOpenArray(ct.len - gcmTagSize, ct.high): + return + + return some(res) + type DecodePacketResult = enum decodingSuccessful @@ -222,7 +231,15 @@ proc decodeAuthResp(c: Codec, fromId: NodeId, head: AuthHeader, var zeroNonce: array[gcmNonceSize, byte] let respData = decryptGCM(secrets.authRespKey, zeroNonce, head.response, []) - let authResp = rlp.decode(respData, AuthResponse) + if respData.isNone(): + return false + + let authResp = rlp.decode(respData.get(), AuthResponse) + # TODO: + # 1. Should allow for not having an ENR included, solved for now by sending + # whoareyou with always recordSeq of 0 + # 2. Should verify ENR and check for correct id in case an ENR is included + # 3. Should verify id nonce signature newNode = newNode(authResp.record) return true @@ -275,7 +292,7 @@ proc decodeEncrypted*(c: var Codec, var writeKey: AesKey if not c.db.loadKeys(fromId, fromAddr, readKey, writeKey): trace "Decoding failed (no keys)" - return PacketError + return DecryptError # doAssert(false, "TODO: HANDLE ME!") let headSize = tagSize + r.position @@ -283,8 +300,14 @@ proc decodeEncrypted*(c: var Codec, let body = decryptGCM(readKey, auth.auth, bodyEnc.toOpenArray, input[0 .. tagSize - 1].toOpenArray) - if body.len > 1: - let status = decodePacketBody(body[0], body.toOpenArray(1, body.high), packet) + if body.isNone(): + discard c.db.deleteKeys(fromId, fromAddr) + return DecryptError + + let packetData = body.get() + if packetData.len > 1: + let status = decodePacketBody(packetData[0], + packetData.toOpenArray(1, packetData.high), packet) if status == decodingSuccessful: return Success else: diff --git a/eth/p2p/discoveryv5/protocol.nim b/eth/p2p/discoveryv5/protocol.nim index e9b9b9c..a334342 100644 --- a/eth/p2p/discoveryv5/protocol.nim +++ b/eth/p2p/discoveryv5/protocol.nim @@ -93,7 +93,7 @@ proc decodeWhoAreYou(d: Protocol, msg: Bytes): Whoareyou = proc sendWhoareyou(d: Protocol, address: Address, toNode: NodeId, authTag: AuthTag) = trace "sending who are you", to = $toNode, toAddress = $address - let challenge = Whoareyou(authTag: authTag, recordSeq: 1) + let challenge = Whoareyou(authTag: authTag, recordSeq: 0) encoding.randomBytes(challenge.idNonce) # If there is already a handshake going on for this nodeid then we drop this # new one. Handshake will get cleaned up after `handshakeTimeout`. @@ -211,11 +211,17 @@ proc receive*(d: Protocol, a: Address, msg: Bytes) {.gcsafe, waiter.complete(packet.some) else: debug "TODO: handle packet: ", packet = packet.kind, origin = $node - elif decoded == DecodeStatus.PacketError: - debug "Could not decode packet, respond with whoareyou", + elif decoded == DecodeStatus.DecryptError: + debug "Could not decrypt packet, respond with whoareyou", localNode = $d.localNode, address = a + # only sendingWhoareyou in case it is a decryption failure d.sendWhoareyou(a, sender, authTag) - # No Whoareyou in case it is a Handshake Failure + elif decoded == DecodeStatus.PacketError: + # Still adding the node in case there is a packet error (could be + # unsupported packet) + if not node.isNil: + debug "Adding new node to routing table", node = $node, localNode = $d.localNode + discard d.routingTable.addNode(node) proc waitPacket(d: Protocol, fromNode: Node, reqId: RequestId): Future[Option[Packet]] = result = newFuture[Option[Packet]]("waitPacket") @@ -356,6 +362,7 @@ proc revalidateNode(p: Protocol, n: Node) discard p.routingTable.setJustSeen(n) + trace "Revalidated node", node = $n else: if false: # TODO: if not bootnode: p.routingTable.removeNode(n) @@ -386,7 +393,8 @@ proc lookupLoop(d: Protocol) {.async.} = trace "lookupLoop canceled" proc open*(d: Protocol) = - debug "Starting discovery node", node = $d.localNode + debug "Starting discovery node", node = $d.localNode, + uri = toURI(d.localNode.record) # TODO allow binding to specific IP / IPv6 / etc let ta = initTAddress(IPv4_any(), d.localNode.node.address.udpPort) d.transp = newDatagramTransport(processClient, udata = d, local = ta) diff --git a/eth/p2p/discoveryv5/types.nim b/eth/p2p/discoveryv5/types.nim index a60b01e..1033e90 100644 --- a/eth/p2p/discoveryv5/types.nim +++ b/eth/p2p/discoveryv5/types.nim @@ -87,6 +87,9 @@ method storeKeys*(db: Database, id: NodeId, address: Address, r, w: AesKey): method loadKeys*(db: Database, id: NodeId, address: Address, r, w: var AesKey): bool {.base, raises: [Defect].} = discard +method deleteKeys*(db: Database, id: NodeId, address: Address): + bool {.raises: [Defect].} = discard + proc toBytes*(id: NodeId): array[32, byte] {.inline.} = id.toByteArrayBE() diff --git a/tests/p2p/test_discv5_encoding.nim b/tests/p2p/test_discv5_encoding.nim index 5a7bfaa..3b665b7 100644 --- a/tests/p2p/test_discv5_encoding.nim +++ b/tests/p2p/test_discv5_encoding.nim @@ -1,5 +1,5 @@ import - unittest, stew/byteutils, stint, + unittest, options, sequtils, stew/byteutils, stint, eth/[rlp, keys] , eth/p2p/discoveryv5/[types, encoding, enr] # According to test vectors: @@ -141,16 +141,16 @@ suite "Discovery v5 Cryptographic Primitives": eph.data == hexToSeqByte(sharedSecret) test "Key Derivation": - const - # input - secretKey = "0x02a77e3aa0c144ae7c0a3af73692b7d6e5b7a2fdc0eda16e8d5e6cb0d08e88dd04" - nodeIdA = "0xa448f24c6d18e575453db13171562b71999873db5b286df957af199ec94617f7" - nodeIdB = "0x885bba8dfeddd49855459df852ad5b63d13a3fae593f3f9fa7e317fd43651409" - idNonce = "0x0101010101010101010101010101010101010101010101010101010101010101" - # expected output - initiatorKey = "0x238d8b50e4363cf603a48c6cc3542967" - recipientKey = "0xbebc0183484f7e7ca2ac32e3d72c8891" - authRespKey = "0xe987ad9e414d5b4f9bfe4ff1e52f2fae" + # const + # # input + # secretKey = "0x02a77e3aa0c144ae7c0a3af73692b7d6e5b7a2fdc0eda16e8d5e6cb0d08e88dd04" + # nodeIdA = "0xa448f24c6d18e575453db13171562b71999873db5b286df957af199ec94617f7" + # nodeIdB = "0x885bba8dfeddd49855459df852ad5b63d13a3fae593f3f9fa7e317fd43651409" + # idNonce = "0x0101010101010101010101010101010101010101010101010101010101010101" + # # expected output + # initiatorKey = "0x238d8b50e4363cf603a48c6cc3542967" + # recipientKey = "0xbebc0183484f7e7ca2ac32e3d72c8891" + # authRespKey = "0xe987ad9e414d5b4f9bfe4ff1e52f2fae" # Code doesn't allow to start from shared `secretKey`, but only from the # public and private key. Would require pulling `ecdhAgree` out of @@ -194,3 +194,38 @@ suite "Discovery v5 Cryptographic Primitives": # The encryption of the auth-resp-pt uses one of these keys, as does the # encryption of the message itself. So the whole test depends on this. skip() + +suite "Discovery v5 Additional": + test "Encryption/Decryption": + let + encryptionKey = hexToByteArray[aesKeySize]("0x9f2d77db7004bf8a1a85107ac686990b") + nonce = hexToByteArray[authTagSize]("0x27b5af763c446acd2749fe8e") + ad = hexToByteArray[tagSize]("0x93a7400fa0d6a694ebc24d5cf570f65d04215b6ac00757875e3f3a5f42107903") + pt = hexToSeqByte("0xa1") + + let ct = encryptGCM(encryptionKey, nonce, pt, ad) + let decrypted = decryptGCM(encryptionKey, nonce, ct, ad) + + check decrypted.get() == pt + + test "Decryption": + let + encryptionKey = hexToByteArray[aesKeySize]("0x9f2d77db7004bf8a1a85107ac686990b") + nonce = hexToByteArray[authTagSize]("0x27b5af763c446acd2749fe8e") + ad = hexToByteArray[tagSize]("0x93a7400fa0d6a694ebc24d5cf570f65d04215b6ac00757875e3f3a5f42107903") + pt = hexToSeqByte("0x01c20101") + ct = hexToSeqByte("0xa5d12a2d94b8ccb3ba55558229867dc13bfa3648") + + # valid case + check decryptGCM(encryptionKey, nonce, ct, ad).get() == pt + + # invalid tag/data sizes + var invalidCipher: seq[byte] = @[] + check decryptGCM(encryptionKey, nonce, invalidCipher, ad).isNone() + + invalidCipher = repeat(byte(4), gcmTagSize) + check decryptGCM(encryptionKey, nonce, invalidCipher, ad).isNone() + + # invalid tag/data itself + invalidCipher = repeat(byte(4), gcmTagSize + 1) + check decryptGCM(encryptionKey, nonce, invalidCipher, ad).isNone()