From 5dec5c5a9b9a8e81b6e1064782ed823bc06bf972 Mon Sep 17 00:00:00 2001 From: kdeme Date: Fri, 24 Apr 2020 15:40:30 +0200 Subject: [PATCH 1/5] discv5 encoding: First steps to move to result based error handling --- eth/p2p/discoveryv5/encoding.nim | 230 +++++++++++++++-------------- eth/p2p/discoveryv5/hkdf.nim | 5 +- eth/p2p/discoveryv5/protocol.nim | 33 +++-- tests/p2p/test_discoveryv5.nim | 8 +- tests/p2p/test_discv5_encoding.nim | 7 +- 5 files changed, 150 insertions(+), 133 deletions(-) diff --git a/eth/p2p/discoveryv5/encoding.nim b/eth/p2p/discoveryv5/encoding.nim index fa43b7b..27aa9d1 100644 --- a/eth/p2p/discoveryv5/encoding.nim +++ b/eth/p2p/discoveryv5/encoding.nim @@ -1,9 +1,11 @@ import - std/[tables, options], nimcrypto, stint, chronicles, + std/[tables, options], nimcrypto, stint, chronicles, stew/results, types, node, enr, hkdf, ../enode, eth/[rlp, keys] export keys +{.push raises: [Defect].} + const idNoncePrefix = "discovery-id-nonce" keyAgreementPrefix = "discovery v5 key agreement" @@ -14,7 +16,6 @@ const ## with type - PacketTag* = array[tagSize, byte] AuthResponse = object @@ -40,23 +41,20 @@ type ephemeralKey*: array[64, byte] response*: seq[byte] - RandomSourceDepleted* = object of CatchableError - - DecodeStatus* = enum - Success, + DecodeError* = enum HandshakeError, PacketError, - DecryptError + DecryptError, + UnsupportedPacketType -proc randomBytes2*(v: var openarray[byte]) = - # TODO if this is called randomBytes it breaks calling the real randomBytes - # in other modules... sigh, nim modules and global namespaces... - # ideally, a new random library will take the place of both these proc's - # in as setting without exceptions for such low-level constructs.. - if randomBytes(v) != v.len: - raise newException(RandomSourceDepleted, "Could not randomize bytes") + DecodeResult*[T] = Result[T, DecodeError] + EncodeResult*[T] = Result[T, cstring] -proc idNonceHash(nonce, ephkey: openarray[byte]): MDigest[256] = +proc mapErrTo[T, E](r: Result[T, E], v: static DecodeError): + DecodeResult[T] {.raises:[].} = + r.mapErr(proc (e: E): DecodeError = v) + +proc idNonceHash(nonce, ephkey: openarray[byte]): MDigest[256] {.raises:[].} = var ctx: sha256 ctx.init() ctx.update(idNoncePrefix) @@ -64,31 +62,27 @@ proc idNonceHash(nonce, ephkey: openarray[byte]): MDigest[256] = ctx.update(ephkey) ctx.finish() -proc signIDNonce*(c: Codec, idNonce, ephKey: openarray[byte]): SignatureNR = - let sig = signNR(c.privKey, idNonceHash(idNonce, ephKey)) - if sig.isErr: - raise newException(CatchableError, "Could not sign idNonce") - sig[] +proc signIDNonce*(privKey: PrivateKey, idNonce, ephKey: openarray[byte]): + Result[SignatureNR, cstring] = + signNR(privKey, idNonceHash(idNonce, ephKey)) proc deriveKeys(n1, n2: NodeID, priv: PrivateKey, pub: PublicKey, - idNonce: openarray[byte], result: var HandshakeSecrets) = - let eph = ecdhRawFull(priv, pub) - if eph.isErr: - raise newException(CatchableError, "ecdhRawFull failed") + idNonce: openarray[byte]): Result[HandshakeSecrets, cstring] = + let eph = ? ecdhRawFull(priv, pub) - # TODO: Unneeded allocation here var info = newSeqOfCap[byte](idNoncePrefix.len + 32 * 2) for i, c in keyAgreementPrefix: info.add(byte(c)) info.add(n1.toByteArrayBE()) info.add(n2.toByteArrayBE()) - # echo "EPH: ", eph.data.toHex, " idNonce: ", challenge.idNonce.toHex, "info: ", info.toHex + var secrets: HandshakeSecrets + static: assert(sizeof(secrets) == aesKeySize * 3) + var res = cast[ptr UncheckedArray[byte]](addr secrets) + hkdf(sha256, eph.data, idNonce, info, toOpenArray(res, 0, sizeof(secrets) - 1)) + ok(secrets) - static: assert(sizeof(result) == aesKeySize * 3) - var res = cast[ptr UncheckedArray[byte]](addr result) - hkdf(sha256, eph[].data, idNonce, info, toOpenArray(res, 0, sizeof(result) - 1)) - -proc encryptGCM*(key, nonce, pt, authData: openarray[byte]): seq[byte] = +proc encryptGCM*(key, nonce, pt, authData: openarray[byte]): + seq[byte] {.raises:[].} = var ectx: GCM[aes128] ectx.init(key, nonce, authData) result = newSeq[byte](pt.len + gcmTagSize) @@ -96,21 +90,26 @@ proc encryptGCM*(key, nonce, pt, authData: openarray[byte]): seq[byte] = ectx.getTag(result.toOpenArray(pt.len, result.high)) ectx.clear() -proc makeAuthHeader(c: Codec, toId: NodeID, nonce: array[gcmNonceSize, byte], +proc makeAuthHeader(c: Codec, + toId: NodeID, + nonce: array[gcmNonceSize, byte], handshakeSecrets: var HandshakeSecrets, - challenge: Whoareyou): seq[byte] = + challenge: Whoareyou): + EncodeResult[seq[byte]] = var resp = AuthResponse(version: 5) let ln = c.localNode + # TODO: What goes over the wire now in case of no updated ENR? if challenge.recordSeq < ln.record.seqNum: resp.record = ln.record - let ephKeys = KeyPair.random().tryGet() + let ephKeys = ? KeyPair.random() + let signature = ? signIDNonce(c.privKey, challenge.idNonce, + ephKeys.pubkey.toRaw) + resp.signature = signature.toRaw - resp.signature = c.signIDNonce(challenge.idNonce, ephKeys.pubkey.toRaw).toRaw - - deriveKeys(ln.id, toId, ephKeys.seckey, challenge.pubKey, challenge.idNonce, - handshakeSecrets) + handshakeSecrets = ? deriveKeys(ln.id, toId, ephKeys.seckey, challenge.pubKey, + challenge.idNonce) let respRlp = rlp.encode(resp) @@ -118,17 +117,19 @@ proc makeAuthHeader(c: Codec, toId: NodeID, nonce: array[gcmNonceSize, byte], let respEnc = encryptGCM(handshakeSecrets.authRespKey, zeroNonce, respRLP, []) let header = AuthHeader(auth: nonce, idNonce: challenge.idNonce, - scheme: authSchemeName, ephemeralKey: ephKeys.pubkey.toRaw, response: respEnc) - rlp.encode(header) + scheme: authSchemeName, ephemeralKey: ephKeys.pubkey.toRaw, + response: respEnc) + ok(rlp.encode(header)) -proc `xor`[N: static[int], T](a, b: array[N, T]): array[N, T] = +proc `xor`[N: static[int], T](a, b: array[N, T]): array[N, T] {.raises:[].} = for i in 0 .. a.high: result[i] = a[i] xor b[i] -proc packetTag(destNode, srcNode: NodeID): PacketTag = - let destId = destNode.toByteArrayBE() - let srcId = srcNode.toByteArrayBE() - let destidHash = sha256.digest(destId) +proc packetTag(destNode, srcNode: NodeID): PacketTag {.raises:[].} = + let + destId = destNode.toByteArrayBE() + srcId = srcNode.toByteArrayBE() + destidHash = sha256.digest(destId) result = srcId xor destidHash.data proc encodeEncrypted*(c: Codec, @@ -136,9 +137,11 @@ proc encodeEncrypted*(c: Codec, toAddr: Address, packetData: seq[byte], challenge: Whoareyou): - (seq[byte], array[gcmNonceSize, byte]) = + EncodeResult[(seq[byte], array[gcmNonceSize, byte])] = var nonce: array[gcmNonceSize, byte] - randomBytes2(nonce) + if randomBytes(nonce) != nonce.len: + return err("Could not randomize bytes") + var headEnc: seq[byte] var writeKey: AesKey @@ -152,7 +155,7 @@ proc encodeEncrypted*(c: Codec, discard c.db.loadKeys(toId, toAddr, readKey, writeKey) else: var sec: HandshakeSecrets - headEnc = c.makeAuthHeader(toId, nonce, sec, challenge) + headEnc = ? c.makeAuthHeader(toId, nonce, sec, challenge) writeKey = sec.writeKey # TODO: is it safe to ignore the error here? @@ -166,10 +169,10 @@ proc encodeEncrypted*(c: Codec, headBuf.add(headEnc) headBuf.add(encryptGCM(writeKey, nonce, body, tag)) - return (headBuf, nonce) + ok((headBuf, nonce)) proc decryptGCM*(key: AesKey, nonce, ct, authData: openarray[byte]): - Option[seq[byte]] = + Option[seq[byte]] {.raises:[].} = if ct.len <= gcmTagSize: debug "cipher is missing tag", len = ct.len return @@ -187,62 +190,66 @@ proc decryptGCM*(key: AesKey, nonce, ct, authData: openarray[byte]): return some(res) -type - DecodePacketResult = enum - decodingSuccessful - invalidPacketPayload - invalidPacketType - unsupportedPacketType - proc decodePacketBody(typ: byte, body: openarray[byte], - res: var Packet): DecodePacketResult = + res: var Packet): + DecodeResult[void] {.raises:[Defect].} = if typ < PacketKind.low.byte or typ > PacketKind.high.byte: - return invalidPacketType + return err(PacketError) let kind = cast[PacketKind](typ) res = Packet(kind: kind) var rlp = rlpFromBytes(body) if rlp.enterList: - res.reqId = rlp.read(RequestId) + try: + res.reqId = rlp.read(RequestId) + except RlpError: + return err(PacketError) - proc decode[T](rlp: var Rlp, v: var T) {.inline, nimcall.} = + proc decode[T](rlp: var Rlp, v: var T) + {.inline, nimcall, raises:[RlpError, ValueError, Defect].} = for k, v in v.fieldPairs: v = rlp.read(typeof(v)) - case kind - of unused: return invalidPacketPayload - of ping: rlp.decode(res.ping) - of pong: rlp.decode(res.pong) - of findNode: rlp.decode(res.findNode) - of nodes: rlp.decode(res.nodes) - of regtopic, ticket, regconfirmation, topicquery: - # TODO Implement these packet types - return unsupportedPacketType + try: + case kind + of unused: return err(PacketError) + of ping: rlp.decode(res.ping) + of pong: rlp.decode(res.pong) + of findNode: rlp.decode(res.findNode) + of nodes: rlp.decode(res.nodes) + of regtopic, ticket, regconfirmation, topicquery: + # TODO: Implement support for topic advertisement + return err(UnsupportedPacketType) + except RlpError, ValueError: + return err(PacketError) - return decodingSuccessful + ok() else: - return invalidPacketPayload + err(PacketError) proc decodeAuthResp(c: Codec, fromId: NodeId, head: AuthHeader, - challenge: Whoareyou, secrets: var HandshakeSecrets, newNode: var Node): bool = + challenge: Whoareyou, secrets: var HandshakeSecrets, newNode: var Node): + DecodeResult[void] {.raises:[Defect].} = if head.scheme != authSchemeName: warn "Unknown auth scheme" - return false + return err(HandshakeError) - var ephKey = PublicKey.fromRaw(head.ephemeralKey) - if ephKey.isErr: - return false + let ephKey = ? PublicKey.fromRaw(head.ephemeralKey).mapErrTo(HandshakeError) - deriveKeys(fromId, c.localNode.id, c.privKey, ephKey[], challenge.idNonce, - secrets) + secrets = ? deriveKeys(fromId, c.localNode.id, c.privKey, ephKey, + challenge.idNonce).mapErrTo(HandshakeError) var zeroNonce: array[gcmNonceSize, byte] let respData = decryptGCM(secrets.authRespKey, zeroNonce, head.response, []) if respData.isNone(): - return false + return err(HandshakeError) - let authResp = rlp.decode(respData.get(), AuthResponse) + var authResp: AuthResponse + try: + authResp = rlp.decode(respData.get(), AuthResponse) + except RlpError, ValueError: + return err(HandshakeError) # TODO: # 1. Should allow for not having an ENR included, solved for now by sending # whoareyou with always recordSeq of 0 @@ -252,8 +259,11 @@ proc decodeAuthResp(c: Codec, fromId: NodeId, head: AuthHeader, # More TODO: # This will also not work if ENR does not contain an IP address or if the # IP address is out of date and doesn't match current UDP end point - newNode = newNode(authResp.record) - return true + try: + newNode = newNode(authResp.record) + ok() + except KeyError, ValueError: + err(HandshakeError) proc decodeEncrypted*(c: var Codec, fromId: NodeID, @@ -261,7 +271,7 @@ proc decodeEncrypted*(c: var Codec, input: openArray[byte], authTag: var AuthTag, newNode: var Node, - packet: var Packet): DecodeStatus = + packet: var Packet): DecodeResult[void] = var r = rlpFromBytes(input.toOpenArray(tagSize, input.high)) var auth: AuthHeader @@ -270,23 +280,27 @@ proc decodeEncrypted*(c: var Codec, if r.isList: # Handshake - rlp list indicates auth-header - auth = r.read(AuthHeader) + try: + auth = r.read(AuthHeader) + except RlpError: + return err(PacketError) authTag = auth.auth let key = HandShakeKey(nodeId: fromId, address: $fromAddr) let challenge = c.handshakes.getOrDefault(key) if challenge.isNil: trace "Decoding failed (no challenge)" - return HandshakeError + return err(HandshakeError) if auth.idNonce != challenge.idNonce: trace "Decoding failed (different nonce)" - return HandshakeError + return err(HandshakeError) var sec: HandshakeSecrets - if not c.decodeAuthResp(fromId, auth, challenge, sec, newNode): + if c.decodeAuthResp(fromId, auth, challenge, sec, newNode).isErr: trace "Decoding failed (bad auth)" - return HandshakeError + return err(HandshakeError) + c.handshakes.del(key) # For an incoming handshake, we are not sure the address in the ENR is there @@ -299,16 +313,17 @@ proc decodeEncrypted*(c: var Codec, # TODO: is it safe to ignore the error here? discard c.db.storeKeys(fromId, fromAddr, sec.readKey, sec.writeKey) readKey = sec.readKey - else: # Message packet or random packet - rlp bytes (size 12) indicates auth-tag - authTag = r.read(AuthTag) + try: + authTag = r.read(AuthTag) + except RlpError: + return err(PacketError) auth.auth = authTag var writeKey: AesKey if not c.db.loadKeys(fromId, fromAddr, readKey, writeKey): trace "Decoding failed (no keys)" - return DecryptError - # doAssert(false, "TODO: HANDLE ME!") + return err(DecryptError) let headSize = tagSize + r.position @@ -318,31 +333,29 @@ proc decodeEncrypted*(c: var Codec, input.toOpenArray(0, tagSize - 1)) if body.isNone(): discard c.db.deleteKeys(fromId, fromAddr) - return DecryptError + return err(DecryptError) let packetData = body.get() if packetData.len > 1: - let status = decodePacketBody(packetData[0], + decodePacketBody(packetData[0], packetData.toOpenArray(1, packetData.high), packet) - if status == decodingSuccessful: - return Success - else: - debug "Failed to decode discovery packet", reason = status - return PacketError else: - return PacketError + err(PacketError) -proc newRequestId*(): RequestId = - if randomBytes(addr result, sizeof(result)) != sizeof(result): - raise newException(RandomSourceDepleted, "Could not randomize bytes") +proc newRequestId*(): Result[RequestId, cstring] {.raises:[].} = + var id: RequestId + if randomBytes(addr id, sizeof(id)) != sizeof(id): + err("Could not randomize bytes") + else: + ok(id) -proc numFields(T: typedesc): int = +proc numFields(T: typedesc): int {.raises:[].} = for k, v in fieldPairs(default(T)): inc result -proc encodePacket*[T: SomePacket](p: T, reqId: RequestId): seq[byte] = +proc encodePacket*[T: SomePacket](p: T, reqId: RequestId): + seq[byte] {.raises:[].} = result = newSeqOfCap[byte](64) result.add(packetKind(T).ord) - # result.add(rlp.encode(p)) const sz = numFields(T) var writer = initRlpList(sz + 1) @@ -350,6 +363,3 @@ proc encodePacket*[T: SomePacket](p: T, reqId: RequestId): seq[byte] = for k, v in fieldPairs(p): writer.append(v) result.add(writer.finish()) - -proc encodePacket*[T: SomePacket](p: T): seq[byte] = - encodePacket(p, newRequestId()) diff --git a/eth/p2p/discoveryv5/hkdf.nim b/eth/p2p/discoveryv5/hkdf.nim index 325b77c..b659c3d 100644 --- a/eth/p2p/discoveryv5/hkdf.nim +++ b/eth/p2p/discoveryv5/hkdf.nim @@ -1,9 +1,10 @@ import nimcrypto -proc hkdf*(HashType: typedesc, secret, salt, info: openarray[byte], output: var openarray[byte]) = +proc hkdf*(HashType: typedesc, ikm, salt, info: openarray[byte], + output: var openarray[byte]) = var ctx: HMAC[HashType] ctx.init(salt) - ctx.update(secret) + ctx.update(ikm) let prk = ctx.finish().data const hashLen = HashType.bits div 8 diff --git a/eth/p2p/discoveryv5/protocol.nim b/eth/p2p/discoveryv5/protocol.nim index 8c5d4db..84f0a0f 100644 --- a/eth/p2p/discoveryv5/protocol.nim +++ b/eth/p2p/discoveryv5/protocol.nim @@ -120,6 +120,8 @@ type node: Node packet: seq[byte] + RandomSourceDepleted* = object of CatchableError + proc addNode*(d: Protocol, node: Node) = discard d.routingTable.addNode(node) @@ -182,7 +184,9 @@ proc decodeWhoAreYou(d: Protocol, msg: openArray[byte]): 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: 0) - encoding.randomBytes2(challenge.idNonce) + + if randomBytes(challenge.idNonce) != challenge.idNonce.len: + raise newException(RandomSourceDepleted, "Could not randomize bytes") # If there is already a handshake going on for this nodeid then we drop this # new one. Handshake will get cleaned up after `handshakeTimeout`. # If instead overwriting the handshake would be allowed, the handshake timeout @@ -208,7 +212,7 @@ proc sendNodes(d: Protocol, toId: NodeId, toAddr: Address, reqId: RequestId, proc sendNodes(d: Protocol, toId: NodeId, toAddr: Address, packet: NodesPacket, reqId: RequestId) {.nimcall.} = let (data, _) = d.codec.encodeEncrypted(toId, toAddr, - encodePacket(packet, reqId), challenge = nil) + encodePacket(packet, reqId), challenge = nil).tryGet() d.send(toAddr, data) var packet: NodesPacket @@ -234,7 +238,7 @@ proc handlePing(d: Protocol, fromId: NodeId, fromAddr: Address, pong.port = a.udpPort.uint16 let (data, _) = d.codec.encodeEncrypted(fromId, fromAddr, - encodePacket(pong, reqId), challenge = nil) + encodePacket(pong, reqId), challenge = nil).tryGet() d.send(fromAddr, data) proc handleFindNode(d: Protocol, fromId: NodeId, fromAddr: Address, @@ -270,9 +274,9 @@ proc receive*(d: Protocol, a: Address, msg: openArray[byte]) {.gcsafe, whoareyou.pubKey = toNode.node.pubkey # TODO: Yeah, rather ugly this. try: let (data, _) = d.codec.encodeEncrypted(toNode.id, toNode.address, - pr.packet, challenge = whoareyou) + pr.packet, challenge = whoareyou).tryGet() d.send(toNode, data) - except RandomSourceDepleted as err: + except RandomSourceDepleted: debug "Failed to respond to a who-you-are msg " & "due to randomness source depletion." @@ -286,7 +290,7 @@ proc receive*(d: Protocol, a: Address, msg: openArray[byte]) {.gcsafe, var node: Node var packet: Packet let decoded = d.codec.decodeEncrypted(sender, a, msg, authTag, node, packet) - if decoded == DecodeStatus.Success: + if decoded.isOk: if not node.isNil: # Not filling table with nodes without correct IP in the ENR if a.ip == node.address.ip: @@ -305,19 +309,20 @@ proc receive*(d: Protocol, a: Address, msg: openArray[byte]) {.gcsafe, waiter.complete(packet.some) else: debug "TODO: handle packet: ", packet = packet.kind, origin = a - elif decoded == DecodeStatus.DecryptError: + elif decoded.error == DecodeError.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) - elif decoded == DecodeStatus.PacketError: - # Still adding the node in case there is a packet error (could be - # unsupported packet) + elif decoded.error == DecodeError.UnsupportedPacketType: + # Still adding the node in case failure is because of unsupported packet. if not node.isNil: if a.ip == node.address.ip: debug "Adding new node to routing table", node = $node, localNode = $d.localNode discard d.routingTable.addNode(node) + # elif decoded.error == DecodeError.PacketError: + # Not adding this node as from our perspective it is sending rubbish. proc processClient(transp: DatagramTransport, raddr: TransportAddress): Future[void] {.async, gcsafe.} = @@ -389,11 +394,11 @@ proc waitNodes(d: Protocol, fromNode: Node, reqId: RequestId): Future[seq[Node]] proc sendPing(d: Protocol, toNode: Node): RequestId = let - reqId = newRequestId() + reqId = newRequestId().tryGet() ping = PingPacket(enrSeq: d.localNode.record.seqNum) packet = encodePacket(ping, reqId) (data, nonce) = d.codec.encodeEncrypted(toNode.id, toNode.address, packet, - challenge = nil) + challenge = nil).tryGet() d.registerRequest(toNode, packet, nonce) d.send(toNode, data) return reqId @@ -406,10 +411,10 @@ proc ping*(d: Protocol, toNode: Node): Future[Option[PongPacket]] {.async.} = return some(resp.get().pong) proc sendFindNode(d: Protocol, toNode: Node, distance: uint32): RequestId = - let reqId = newRequestId() + let reqId = newRequestId().tryGet() let packet = encodePacket(FindNodePacket(distance: distance), reqId) let (data, nonce) = d.codec.encodeEncrypted(toNode.id, toNode.address, packet, - challenge = nil) + challenge = nil).tryGet() d.registerRequest(toNode, packet, nonce) d.send(toNode, data) diff --git a/tests/p2p/test_discoveryv5.nim b/tests/p2p/test_discoveryv5.nim index 186069b..055a0d6 100644 --- a/tests/p2p/test_discoveryv5.nim +++ b/tests/p2p/test_discoveryv5.nim @@ -1,5 +1,5 @@ import - unittest, chronos, sequtils, chronicles, tables, stint, + unittest, chronos, sequtils, chronicles, tables, stint, nimcrypto, eth/[keys, rlp], eth/p2p/enode, eth/trie/db, eth/p2p/discoveryv5/[discovery_db, enr, node, types, routing_table, encoding], eth/p2p/discoveryv5/protocol as discv5_protocol, @@ -26,8 +26,8 @@ proc randomPacket(tag: PacketTag): seq[byte] = authTag: AuthTag msg: array[44, byte] - randomBytes2(authTag) - randomBytes2(msg) + require randomBytes(authTag) == authTag.len + require randomBytes(msg) == msg.len result.add(tag) result.add(rlp.encode(authTag)) result.add(msg) @@ -98,7 +98,7 @@ suite "Discovery v5 Tests": let a = localAddress(20303) for i in 0 ..< 5: - randomBytes2(tag) + require randomBytes(tag) == tag.len node.receive(a, randomPacket(tag)) # Checking different nodeIds but same address diff --git a/tests/p2p/test_discv5_encoding.nim b/tests/p2p/test_discv5_encoding.nim index 8cb69f6..a50724d 100644 --- a/tests/p2p/test_discv5_encoding.nim +++ b/tests/p2p/test_discv5_encoding.nim @@ -166,10 +166,11 @@ suite "Discovery v5 Cryptographic Primitives": idNonceSig = "0xc5036e702a79902ad8aa147dabfe3958b523fd6fa36cc78e2889b912d682d8d35fdea142e141f690736d86f50b39746ba2d2fc510b46f82ee08f08fd55d133a4" let - c = Codec(privKey: PrivateKey.fromHex(localSecretKey)[]) - signature = signIDNonce(c, hexToByteArray[idNonceSize](idNonce), + privKey = PrivateKey.fromHex(localSecretKey)[] + signature = signIDNonce(privKey, hexToByteArray[idNonceSize](idNonce), hexToByteArray[64](ephemeralKey)) - check signature.toRaw() == hexToByteArray[64](idNonceSig) + require signature.isOK() + check signature[].toRaw() == hexToByteArray[64](idNonceSig) test "Encryption/Decryption": const From 09a127ef1b4ac9b31bf36f246dadff4a5425d9f7 Mon Sep 17 00:00:00 2001 From: kdeme Date: Fri, 24 Apr 2020 16:52:41 +0200 Subject: [PATCH 2/5] Pass packet as decoding result --- eth/p2p/discoveryv5/encoding.nim | 39 ++++++++++++++------------------ eth/p2p/discoveryv5/protocol.nim | 4 ++-- 2 files changed, 19 insertions(+), 24 deletions(-) diff --git a/eth/p2p/discoveryv5/encoding.nim b/eth/p2p/discoveryv5/encoding.nim index 27aa9d1..039cf32 100644 --- a/eth/p2p/discoveryv5/encoding.nim +++ b/eth/p2p/discoveryv5/encoding.nim @@ -190,19 +190,20 @@ proc decryptGCM*(key: AesKey, nonce, ct, authData: openarray[byte]): return some(res) -proc decodePacketBody(typ: byte, - body: openarray[byte], - res: var Packet): - DecodeResult[void] {.raises:[Defect].} = - if typ < PacketKind.low.byte or typ > PacketKind.high.byte: +proc decodePacketBody(body: openarray[byte]): + DecodeResult[Packet] {.raises:[Defect].} = + if body.len < 1: return err(PacketError) - let kind = cast[PacketKind](typ) - res = Packet(kind: kind) - var rlp = rlpFromBytes(body) + if body[0] < PacketKind.low.byte or body[0] > PacketKind.high.byte: + return err(PacketError) + + let kind = cast[PacketKind](body[0]) + var packet = Packet(kind: kind) + var rlp = rlpFromBytes(body.toOpenArray(1, body.high)) if rlp.enterList: try: - res.reqId = rlp.read(RequestId) + packet.reqId = rlp.read(RequestId) except RlpError: return err(PacketError) @@ -214,17 +215,17 @@ proc decodePacketBody(typ: byte, try: case kind of unused: return err(PacketError) - of ping: rlp.decode(res.ping) - of pong: rlp.decode(res.pong) - of findNode: rlp.decode(res.findNode) - of nodes: rlp.decode(res.nodes) + of ping: rlp.decode(packet.ping) + of pong: rlp.decode(packet.pong) + of findNode: rlp.decode(packet.findNode) + of nodes: rlp.decode(packet.nodes) of regtopic, ticket, regconfirmation, topicquery: # TODO: Implement support for topic advertisement return err(UnsupportedPacketType) except RlpError, ValueError: return err(PacketError) - ok() + ok(packet) else: err(PacketError) @@ -270,8 +271,7 @@ proc decodeEncrypted*(c: var Codec, fromAddr: Address, input: openArray[byte], authTag: var AuthTag, - newNode: var Node, - packet: var Packet): DecodeResult[void] = + newNode: var Node): DecodeResult[Packet] = var r = rlpFromBytes(input.toOpenArray(tagSize, input.high)) var auth: AuthHeader @@ -335,12 +335,7 @@ proc decodeEncrypted*(c: var Codec, discard c.db.deleteKeys(fromId, fromAddr) return err(DecryptError) - let packetData = body.get() - if packetData.len > 1: - decodePacketBody(packetData[0], - packetData.toOpenArray(1, packetData.high), packet) - else: - err(PacketError) + decodePacketBody(body.get()) proc newRequestId*(): Result[RequestId, cstring] {.raises:[].} = var id: RequestId diff --git a/eth/p2p/discoveryv5/protocol.nim b/eth/p2p/discoveryv5/protocol.nim index 84f0a0f..1a896eb 100644 --- a/eth/p2p/discoveryv5/protocol.nim +++ b/eth/p2p/discoveryv5/protocol.nim @@ -288,9 +288,9 @@ proc receive*(d: Protocol, a: Address, msg: openArray[byte]) {.gcsafe, var authTag: AuthTag var node: Node - var packet: Packet - let decoded = d.codec.decodeEncrypted(sender, a, msg, authTag, node, packet) + let decoded = d.codec.decodeEncrypted(sender, a, msg, authTag, node) if decoded.isOk: + let packet = decoded[] if not node.isNil: # Not filling table with nodes without correct IP in the ENR if a.ip == node.address.ip: From 3a6d4336bcb378fc3106a7266719318b830a17ee Mon Sep 17 00:00:00 2001 From: kdeme Date: Mon, 27 Apr 2020 14:13:00 +0200 Subject: [PATCH 3/5] Bunch of renames to use same nomenclature as spec --- eth/p2p/discoveryv5/encoding.nim | 70 ++++++++-------- eth/p2p/discoveryv5/protocol.nim | 127 +++++++++++++++-------------- eth/p2p/discoveryv5/types.nim | 36 ++++---- tests/p2p/test_discv5_encoding.nim | 20 ++--- 4 files changed, 126 insertions(+), 127 deletions(-) diff --git a/eth/p2p/discoveryv5/encoding.nim b/eth/p2p/discoveryv5/encoding.nim index 039cf32..d79cc9a 100644 --- a/eth/p2p/discoveryv5/encoding.nim +++ b/eth/p2p/discoveryv5/encoding.nim @@ -45,7 +45,7 @@ type HandshakeError, PacketError, DecryptError, - UnsupportedPacketType + UnsupportedMessage DecodeResult*[T] = Result[T, DecodeError] EncodeResult*[T] = Result[T, cstring] @@ -90,12 +90,12 @@ proc encryptGCM*(key, nonce, pt, authData: openarray[byte]): ectx.getTag(result.toOpenArray(pt.len, result.high)) ectx.clear() -proc makeAuthHeader(c: Codec, - toId: NodeID, - nonce: array[gcmNonceSize, byte], - handshakeSecrets: var HandshakeSecrets, - challenge: Whoareyou): - EncodeResult[seq[byte]] = +proc encodeAuthHeader(c: Codec, + toId: NodeID, + nonce: array[gcmNonceSize, byte], + handshakeSecrets: var HandshakeSecrets, + challenge: Whoareyou): + EncodeResult[seq[byte]] = var resp = AuthResponse(version: 5) let ln = c.localNode @@ -132,10 +132,10 @@ proc packetTag(destNode, srcNode: NodeID): PacketTag {.raises:[].} = destidHash = sha256.digest(destId) result = srcId xor destidHash.data -proc encodeEncrypted*(c: Codec, +proc encodePacket*(c: Codec, toId: NodeID, toAddr: Address, - packetData: seq[byte], + message: seq[byte], challenge: Whoareyou): EncodeResult[(seq[byte], array[gcmNonceSize, byte])] = var nonce: array[gcmNonceSize, byte] @@ -155,21 +155,19 @@ proc encodeEncrypted*(c: Codec, discard c.db.loadKeys(toId, toAddr, readKey, writeKey) else: var sec: HandshakeSecrets - headEnc = ? c.makeAuthHeader(toId, nonce, sec, challenge) + headEnc = ? c.encodeAuthHeader(toId, nonce, sec, challenge) writeKey = sec.writeKey # TODO: is it safe to ignore the error here? discard c.db.storeKeys(toId, toAddr, sec.readKey, sec.writeKey) - var body = packetData let tag = packetTag(toId, c.localNode.id) - var headBuf = newSeqOfCap[byte](tag.len + headEnc.len) - headBuf.add(tag) - headBuf.add(headEnc) - - headBuf.add(encryptGCM(writeKey, nonce, body, tag)) - ok((headBuf, nonce)) + var packet = newSeqOfCap[byte](tag.len + headEnc.len) + packet.add(tag) + packet.add(headEnc) + packet.add(encryptGCM(writeKey, nonce, message, tag)) + ok((packet, nonce)) proc decryptGCM*(key: AesKey, nonce, ct, authData: openarray[byte]): Option[seq[byte]] {.raises:[].} = @@ -190,20 +188,20 @@ proc decryptGCM*(key: AesKey, nonce, ct, authData: openarray[byte]): return some(res) -proc decodePacketBody(body: openarray[byte]): - DecodeResult[Packet] {.raises:[Defect].} = +proc decodeMessage(body: openarray[byte]): + DecodeResult[Message] {.raises:[Defect].} = if body.len < 1: return err(PacketError) - if body[0] < PacketKind.low.byte or body[0] > PacketKind.high.byte: + if body[0] < MessageKind.low.byte or body[0] > MessageKind.high.byte: return err(PacketError) - let kind = cast[PacketKind](body[0]) - var packet = Packet(kind: kind) + let kind = cast[MessageKind](body[0]) + var message = Message(kind: kind) var rlp = rlpFromBytes(body.toOpenArray(1, body.high)) if rlp.enterList: try: - packet.reqId = rlp.read(RequestId) + message.reqId = rlp.read(RequestId) except RlpError: return err(PacketError) @@ -215,17 +213,17 @@ proc decodePacketBody(body: openarray[byte]): try: case kind of unused: return err(PacketError) - of ping: rlp.decode(packet.ping) - of pong: rlp.decode(packet.pong) - of findNode: rlp.decode(packet.findNode) - of nodes: rlp.decode(packet.nodes) + of ping: rlp.decode(message.ping) + of pong: rlp.decode(message.pong) + of findNode: rlp.decode(message.findNode) + of nodes: rlp.decode(message.nodes) of regtopic, ticket, regconfirmation, topicquery: # TODO: Implement support for topic advertisement - return err(UnsupportedPacketType) + return err(UnsupportedMessage) except RlpError, ValueError: return err(PacketError) - ok(packet) + ok(message) else: err(PacketError) @@ -266,12 +264,12 @@ proc decodeAuthResp(c: Codec, fromId: NodeId, head: AuthHeader, except KeyError, ValueError: err(HandshakeError) -proc decodeEncrypted*(c: var Codec, +proc decodePacket*(c: var Codec, fromId: NodeID, fromAddr: Address, input: openArray[byte], authTag: var AuthTag, - newNode: var Node): DecodeResult[Packet] = + newNode: var Node): DecodeResult[Message] = var r = rlpFromBytes(input.toOpenArray(tagSize, input.high)) var auth: AuthHeader @@ -327,15 +325,15 @@ proc decodeEncrypted*(c: var Codec, let headSize = tagSize + r.position - let body = decryptGCM( + let message = decryptGCM( readKey, auth.auth, input.toOpenArray(headSize, input.high), input.toOpenArray(0, tagSize - 1)) - if body.isNone(): + if message.isNone(): discard c.db.deleteKeys(fromId, fromAddr) return err(DecryptError) - decodePacketBody(body.get()) + decodeMessage(message.get()) proc newRequestId*(): Result[RequestId, cstring] {.raises:[].} = var id: RequestId @@ -347,10 +345,10 @@ proc newRequestId*(): Result[RequestId, cstring] {.raises:[].} = proc numFields(T: typedesc): int {.raises:[].} = for k, v in fieldPairs(default(T)): inc result -proc encodePacket*[T: SomePacket](p: T, reqId: RequestId): +proc encodeMessage*[T: SomeMessage](p: T, reqId: RequestId): seq[byte] {.raises:[].} = result = newSeqOfCap[byte](64) - result.add(packetKind(T).ord) + result.add(messageKind(T).ord) const sz = numFields(T) var writer = initRlpList(sz + 1) diff --git a/eth/p2p/discoveryv5/protocol.nim b/eth/p2p/discoveryv5/protocol.nim index 1a896eb..a1c7720 100644 --- a/eth/p2p/discoveryv5/protocol.nim +++ b/eth/p2p/discoveryv5/protocol.nim @@ -89,7 +89,7 @@ const alpha = 3 ## Kademlia concurrency factor lookupRequestLimit = 3 findNodeResultLimit = 15 # applies in FINDNODE handler - maxNodesPerPacket = 3 + maxNodesPerMessage = 3 lookupInterval = 60.seconds ## Interval of launching a random lookup to ## populate the routing table. go-ethereum seems to do 3 runs every 30 ## minutes. Trinity starts one every minute. @@ -111,14 +111,14 @@ type db: Database routingTable: RoutingTable codec*: Codec - awaitedPackets: Table[(NodeId, RequestId), Future[Option[Packet]]] + awaitedMessages: Table[(NodeId, RequestId), Future[Option[Message]]] lookupLoop: Future[void] revalidateLoop: Future[void] bootstrapRecords*: seq[Record] PendingRequest = object node: Node - packet: seq[byte] + message: seq[byte] RandomSourceDepleted* = object of CatchableError @@ -173,13 +173,13 @@ proc whoareyouMagic(toNode: NodeId): array[magicSize, byte] = for i, c in prefix: data[sizeof(toNode) + i] = byte(c) sha256.digest(data).data -proc isWhoAreYou(d: Protocol, msg: openArray[byte]): bool = - if msg.len > d.whoareyouMagic.len: - result = d.whoareyouMagic == msg.toOpenArray(0, magicSize - 1) +proc isWhoAreYou(d: Protocol, packet: openArray[byte]): bool = + if packet.len > d.whoareyouMagic.len: + result = d.whoareyouMagic == packet.toOpenArray(0, magicSize - 1) -proc decodeWhoAreYou(d: Protocol, msg: openArray[byte]): Whoareyou = +proc decodeWhoAreYou(d: Protocol, packet: openArray[byte]): Whoareyou = result = Whoareyou() - result[] = rlp.decode(msg.toOpenArray(magicSize, msg.high), WhoareyouObj) + result[] = rlp.decode(packet.toOpenArray(magicSize, packet.high), WhoareyouObj) proc sendWhoareyou(d: Protocol, address: Address, toNode: NodeId, authTag: AuthTag) = trace "sending who are you", to = $toNode, toAddress = $address @@ -210,39 +210,39 @@ proc sendWhoareyou(d: Protocol, address: Address, toNode: NodeId, authTag: AuthT proc sendNodes(d: Protocol, toId: NodeId, toAddr: Address, reqId: RequestId, nodes: openarray[Node]) = proc sendNodes(d: Protocol, toId: NodeId, toAddr: Address, - packet: NodesPacket, reqId: RequestId) {.nimcall.} = - let (data, _) = d.codec.encodeEncrypted(toId, toAddr, - encodePacket(packet, reqId), challenge = nil).tryGet() + message: NodesMessage, reqId: RequestId) {.nimcall.} = + let (data, _) = d.codec.encodePacket(toId, toAddr, + encodeMessage(message, reqId), challenge = nil).tryGet() d.send(toAddr, data) - var packet: NodesPacket - packet.total = ceil(nodes.len / maxNodesPerPacket).uint32 + var message: NodesMessage + message.total = ceil(nodes.len / maxNodesPerMessage).uint32 for i in 0 ..< nodes.len: - packet.enrs.add(nodes[i].record) - if packet.enrs.len == 3: - d.sendNodes(toId, toAddr, packet, reqId) - packet.enrs.setLen(0) + message.enrs.add(nodes[i].record) + if message.enrs.len == 3: # TODO: Uh, what is this? + d.sendNodes(toId, toAddr, message, reqId) + message.enrs.setLen(0) - if packet.enrs.len != 0: - d.sendNodes(toId, toAddr, packet, reqId) + if message.enrs.len != 0: + d.sendNodes(toId, toAddr, message, reqId) proc handlePing(d: Protocol, fromId: NodeId, fromAddr: Address, - ping: PingPacket, reqId: RequestId) = + ping: PingMessage, reqId: RequestId) = let a = fromAddr - var pong: PongPacket + var pong: PongMessage pong.enrSeq = ping.enrSeq pong.ip = case a.ip.family of IpAddressFamily.IPv4: @(a.ip.address_v4) of IpAddressFamily.IPv6: @(a.ip.address_v6) pong.port = a.udpPort.uint16 - let (data, _) = d.codec.encodeEncrypted(fromId, fromAddr, - encodePacket(pong, reqId), challenge = nil).tryGet() + let (data, _) = d.codec.encodePacket(fromId, fromAddr, + encodeMessage(pong, reqId), challenge = nil).tryGet() d.send(fromAddr, data) proc handleFindNode(d: Protocol, fromId: NodeId, fromAddr: Address, - fn: FindNodePacket, reqId: RequestId) = + fn: FindNodeMessage, reqId: RequestId) = if fn.distance == 0: d.sendNodes(fromId, fromAddr, reqId, [d.localNode]) else: @@ -250,7 +250,7 @@ proc handleFindNode(d: Protocol, fromId: NodeId, fromAddr: Address, d.sendNodes(fromId, fromAddr, reqId, d.routingTable.neighboursAtDistance(distance)) -proc receive*(d: Protocol, a: Address, msg: openArray[byte]) {.gcsafe, +proc receive*(d: Protocol, a: Address, packet: openArray[byte]) {.gcsafe, raises: [ Defect, # TODO This is now coming from Chronos's callSoon @@ -260,37 +260,37 @@ proc receive*(d: Protocol, a: Address, msg: openArray[byte]) {.gcsafe, IOError, TransportAddressError, ].} = - if msg.len < tagSize: # or magicSize, can be either - return # Invalid msg + if packet.len < tagSize: # or magicSize, can be either + return # Invalid packet - # debug "Packet received: ", length = msg.len + # debug "Packet received: ", length = packet.len - if d.isWhoAreYou(msg): + if d.isWhoAreYou(packet): trace "Received whoareyou", localNode = $d.localNode, address = a - let whoareyou = d.decodeWhoAreYou(msg) + let whoareyou = d.decodeWhoAreYou(packet) var pr: PendingRequest if d.pendingRequests.take(whoareyou.authTag, pr): let toNode = pr.node whoareyou.pubKey = toNode.node.pubkey # TODO: Yeah, rather ugly this. try: - let (data, _) = d.codec.encodeEncrypted(toNode.id, toNode.address, - pr.packet, challenge = whoareyou).tryGet() + let (data, _) = d.codec.encodePacket(toNode.id, toNode.address, + pr.message, challenge = whoareyou).tryGet() d.send(toNode, data) except RandomSourceDepleted: - debug "Failed to respond to a who-you-are msg " & + debug "Failed to respond to a who-you-are packet " & "due to randomness source depletion." else: var tag: array[tagSize, byte] - tag[0 .. ^1] = msg.toOpenArray(0, tagSize - 1) + tag[0 .. ^1] = packet.toOpenArray(0, tagSize - 1) let senderData = tag xor d.idHash let sender = readUintBE[256](senderData) var authTag: AuthTag var node: Node - let decoded = d.codec.decodeEncrypted(sender, a, msg, authTag, node) + let decoded = d.codec.decodePacket(sender, a, packet, authTag, node) if decoded.isOk: - let packet = decoded[] + let message = decoded[] if not node.isNil: # Not filling table with nodes without correct IP in the ENR if a.ip == node.address.ip: @@ -298,24 +298,25 @@ proc receive*(d: Protocol, a: Address, msg: openArray[byte]) {.gcsafe, localNode = $d.localNode discard d.routingTable.addNode(node) - case packet.kind + case message.kind of ping: - d.handlePing(sender, a, packet.ping, packet.reqId) + d.handlePing(sender, a, message.ping, message.reqId) of findNode: - d.handleFindNode(sender, a, packet.findNode, packet.reqId) + d.handleFindNode(sender, a, message.findNode, message.reqId) else: - var waiter: Future[Option[Packet]] - if d.awaitedPackets.take((sender, packet.reqId), waiter): - waiter.complete(packet.some) + var waiter: Future[Option[Message]] + if d.awaitedMessages.take((sender, message.reqId), waiter): + waiter.complete(some(message)) else: - debug "TODO: handle packet: ", packet = packet.kind, origin = a + trace "Timed out or unrequested message", message = message.kind, + origin = a elif decoded.error == DecodeError.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) - elif decoded.error == DecodeError.UnsupportedPacketType: - # Still adding the node in case failure is because of unsupported packet. + elif decoded.error == DecodeError.UnsupportedMessage: + # Still adding the node in case failure is because of unsupported message. if not node.isNil: if a.ip == node.address.ip: debug "Adding new node to routing table", node = $node, @@ -361,32 +362,32 @@ proc validIp(sender, address: IpAddress): bool = # TODO: This could be improved to do the clean-up immediatily in case a non # whoareyou response does arrive, but we would need to store the AuthTag # somewhere -proc registerRequest(d: Protocol, n: Node, packet: seq[byte], nonce: AuthTag) = - let request = PendingRequest(node: n, packet: packet) +proc registerRequest(d: Protocol, n: Node, message: seq[byte], nonce: AuthTag) = + let request = PendingRequest(node: n, message: message) if not d.pendingRequests.hasKeyOrPut(nonce, request): sleepAsync(responseTimeout).addCallback() do(data: pointer): d.pendingRequests.del(nonce) -proc waitPacket(d: Protocol, fromNode: Node, reqId: RequestId): Future[Option[Packet]] = - result = newFuture[Option[Packet]]("waitPacket") +proc waitMessage(d: Protocol, fromNode: Node, reqId: RequestId): Future[Option[Message]] = + result = newFuture[Option[Message]]("waitMessage") let res = result let key = (fromNode.id, reqId) sleepAsync(responseTimeout).addCallback() do(data: pointer): - d.awaitedPackets.del(key) + d.awaitedMessages.del(key) if not res.finished: - res.complete(none(Packet)) - d.awaitedPackets[key] = result + res.complete(none(Message)) + d.awaitedMessages[key] = result proc addNodesFromENRs(result: var seq[Node], enrs: openarray[Record]) = for r in enrs: result.add(newNode(r)) proc waitNodes(d: Protocol, fromNode: Node, reqId: RequestId): Future[seq[Node]] {.async.} = - var op = await d.waitPacket(fromNode, reqId) + var op = await d.waitMessage(fromNode, reqId) if op.isSome and op.get.kind == nodes: result.addNodesFromENRs(op.get.nodes.enrs) let total = op.get.nodes.total for i in 1 ..< total: - op = await d.waitPacket(fromNode, reqId) + op = await d.waitMessage(fromNode, reqId) if op.isSome and op.get.kind == nodes: result.addNodesFromENRs(op.get.nodes.enrs) else: @@ -395,27 +396,27 @@ proc waitNodes(d: Protocol, fromNode: Node, reqId: RequestId): Future[seq[Node]] proc sendPing(d: Protocol, toNode: Node): RequestId = let reqId = newRequestId().tryGet() - ping = PingPacket(enrSeq: d.localNode.record.seqNum) - packet = encodePacket(ping, reqId) - (data, nonce) = d.codec.encodeEncrypted(toNode.id, toNode.address, packet, + ping = PingMessage(enrSeq: d.localNode.record.seqNum) + message = encodeMessage(ping, reqId) + (data, nonce) = d.codec.encodePacket(toNode.id, toNode.address, message, challenge = nil).tryGet() - d.registerRequest(toNode, packet, nonce) + d.registerRequest(toNode, message, nonce) d.send(toNode, data) return reqId -proc ping*(d: Protocol, toNode: Node): Future[Option[PongPacket]] {.async.} = +proc ping*(d: Protocol, toNode: Node): Future[Option[PongMessage]] {.async.} = let reqId = d.sendPing(toNode) - let resp = await d.waitPacket(toNode, reqId) + let resp = await d.waitMessage(toNode, reqId) if resp.isSome() and resp.get().kind == pong: return some(resp.get().pong) proc sendFindNode(d: Protocol, toNode: Node, distance: uint32): RequestId = let reqId = newRequestId().tryGet() - let packet = encodePacket(FindNodePacket(distance: distance), reqId) - let (data, nonce) = d.codec.encodeEncrypted(toNode.id, toNode.address, packet, + let message = encodeMessage(FindNodeMessage(distance: distance), reqId) + let (data, nonce) = d.codec.encodePacket(toNode.id, toNode.address, message, challenge = nil).tryGet() - d.registerRequest(toNode, packet, nonce) + d.registerRequest(toNode, message, nonce) d.send(toNode, data) return reqId diff --git a/eth/p2p/discoveryv5/types.nim b/eth/p2p/discoveryv5/types.nim index 1687cc9..426608c 100644 --- a/eth/p2p/discoveryv5/types.nim +++ b/eth/p2p/discoveryv5/types.nim @@ -27,9 +27,9 @@ type Database* = ref object of RootRef - PacketKind* = enum + MessageKind* = enum # TODO This is needed only to make Nim 1.0.4 happy - # Without it, the `PacketKind` type cannot be used as + # Without it, the `MessageKind` type cannot be used as # a discriminator in case objects. unused = 0x00 @@ -44,43 +44,43 @@ type RequestId* = uint64 - PingPacket* = object + PingMessage* = object enrSeq*: uint64 - PongPacket* = object + PongMessage* = object enrSeq*: uint64 ip*: seq[byte] port*: uint16 - FindNodePacket* = object + FindNodeMessage* = object distance*: uint32 - NodesPacket* = object + NodesMessage* = object total*: uint32 enrs*: seq[Record] - SomePacket* = PingPacket or PongPacket or FindNodePacket or NodesPacket + SomeMessage* = PingMessage or PongMessage or FindNodeMessage or NodesMessage - Packet* = object + Message* = object reqId*: RequestId - case kind*: PacketKind + case kind*: MessageKind of ping: - ping*: PingPacket + ping*: PingMessage of pong: - pong*: PongPacket + pong*: PongMessage of findnode: - findNode*: FindNodePacket + findNode*: FindNodeMessage of nodes: - nodes*: NodesPacket + nodes*: NodesMessage else: # TODO: Define the rest discard -template packetKind*(T: typedesc[SomePacket]): PacketKind = - when T is PingPacket: ping - elif T is PongPacket: pong - elif T is FindNodePacket: findNode - elif T is NodesPacket: nodes +template messageKind*(T: typedesc[SomeMessage]): MessageKind = + when T is PingMessage: ping + elif T is PongMessage: pong + elif T is FindNodeMessage: findNode + elif T is NodesMessage: nodes method storeKeys*(db: Database, id: NodeId, address: Address, r, w: AesKey): bool {.base, raises: [Defect].} = discard diff --git a/tests/p2p/test_discv5_encoding.nim b/tests/p2p/test_discv5_encoding.nim index a50724d..f5ed97a 100644 --- a/tests/p2p/test_discv5_encoding.nim +++ b/tests/p2p/test_discv5_encoding.nim @@ -86,33 +86,33 @@ suite "Discovery v5 Packet Encodings": suite "Discovery v5 Protocol Message Encodings": test "Ping Request": - var p: PingPacket + var p: PingMessage p.enrSeq = 1 var reqId: RequestId = 1 - check encodePacket(p, reqId).toHex == "01c20101" + check encodeMessage(p, reqId).toHex == "01c20101" test "Pong Response": - var p: PongPacket + var p: PongMessage p.enrSeq = 1 p.port = 5000 p.ip = @[127.byte, 0, 0, 1] var reqId: RequestId = 1 - check encodePacket(p, reqId).toHex == "02ca0101847f000001821388" + check encodeMessage(p, reqId).toHex == "02ca0101847f000001821388" test "FindNode Request": - var p: FindNodePacket + var p: FindNodeMessage p.distance = 0x0100 var reqId: RequestId = 1 - check encodePacket(p, reqId).toHex == "03c401820100" + check encodeMessage(p, reqId).toHex == "03c401820100" test "Nodes Response (empty)": - var p: NodesPacket + var p: NodesMessage p.total = 0x1 var reqId: RequestId = 1 - check encodePacket(p, reqId).toHex == "04c30101c0" + check encodeMessage(p, reqId).toHex == "04c30101c0" test "Nodes Response (multiple)": - var p: NodesPacket + var p: NodesMessage p.total = 0x1 var e1, e2: Record check e1.fromURI("enr:-HW4QBzimRxkmT18hMKaAL3IcZF1UcfTMPyi3Q1pxwZZbcZVRI8DC5infUAB_UauARLOJtYTxaagKoGmIjzQxO2qUygBgmlkgnY0iXNlY3AyNTZrMaEDymNMrg1JrLQB2KTGtv6MVbcNEVv0AHacwUAPMljNMTg") @@ -120,7 +120,7 @@ suite "Discovery v5 Protocol Message Encodings": p.enrs = @[e1, e2] var reqId: RequestId = 1 - check encodePacket(p, reqId).toHex == "04f8f20101f8eef875b8401ce2991c64993d7c84c29a00bdc871917551c7d330fca2dd0d69c706596dc655448f030b98a77d4001fd46ae0112ce26d613c5a6a02a81a6223cd0c4edaa53280182696482763489736563703235366b31a103ca634cae0d49acb401d8a4c6b6fe8c55b70d115bf400769cc1400f3258cd3138f875b840d7f1c39e376297f81d7297758c64cb37dcc5c3beea9f57f7ce9695d7d5a67553417d719539d6ae4b445946de4d99e680eb8063f29485b555d45b7df16a1850130182696482763489736563703235366b31a1030e2cb74241c0c4fc8e8166f1a79a05d5b0dd95813a74b094529f317d5c39d235" + check encodeMessage(p, reqId).toHex == "04f8f20101f8eef875b8401ce2991c64993d7c84c29a00bdc871917551c7d330fca2dd0d69c706596dc655448f030b98a77d4001fd46ae0112ce26d613c5a6a02a81a6223cd0c4edaa53280182696482763489736563703235366b31a103ca634cae0d49acb401d8a4c6b6fe8c55b70d115bf400769cc1400f3258cd3138f875b840d7f1c39e376297f81d7297758c64cb37dcc5c3beea9f57f7ce9695d7d5a67553417d719539d6ae4b445946de4d99e680eb8063f29485b555d45b7df16a1850130182696482763489736563703235366b31a1030e2cb74241c0c4fc8e8166f1a79a05d5b0dd95813a74b094529f317d5c39d235" suite "Discovery v5 Cryptographic Primitives": test "ECDH": From 74df90e16d1a71134bee25e505d2a2cd12ff3588 Mon Sep 17 00:00:00 2001 From: kdeme Date: Thu, 30 Apr 2020 00:11:03 +0200 Subject: [PATCH 4/5] discv5: further prepping for results error handling --- eth/p2p/discoveryv5/encoding.nim | 2 +- eth/p2p/discoveryv5/enr.nim | 84 ++++++++++++++++----------- eth/p2p/discoveryv5/node.nim | 39 ++++++------- eth/p2p/discoveryv5/protocol.nim | 3 +- eth/p2p/discoveryv5/routing_table.nim | 41 +++++++------ tests/p2p/test_discoveryv5.nim | 6 +- tests/p2p/test_enr.nim | 6 +- 7 files changed, 97 insertions(+), 84 deletions(-) diff --git a/eth/p2p/discoveryv5/encoding.nim b/eth/p2p/discoveryv5/encoding.nim index d79cc9a..128245f 100644 --- a/eth/p2p/discoveryv5/encoding.nim +++ b/eth/p2p/discoveryv5/encoding.nim @@ -170,7 +170,7 @@ proc encodePacket*(c: Codec, ok((packet, nonce)) proc decryptGCM*(key: AesKey, nonce, ct, authData: openarray[byte]): - Option[seq[byte]] {.raises:[].} = + Option[seq[byte]] = if ct.len <= gcmTagSize: debug "cipher is missing tag", len = ct.len return diff --git a/eth/p2p/discoveryv5/enr.nim b/eth/p2p/discoveryv5/enr.nim index 7c43937..21351f8 100644 --- a/eth/p2p/discoveryv5/enr.nim +++ b/eth/p2p/discoveryv5/enr.nim @@ -8,6 +8,8 @@ import export options +{.push raises: [Defect].} + const maxEnrSize = 300 minRlpListLen = 4 # for signature, seqId, "id" key, id @@ -47,6 +49,8 @@ type of kBytes: bytes: seq[byte] + EnrResult*[T] = Result[T, cstring] + template toField[T](v: T): Field = when T is string: Field(kind: kString, str: v) @@ -59,20 +63,24 @@ template toField[T](v: T): Field = else: {.error: "Unsupported field type".} -proc makeEnrAux(seqNum: uint64, pk: PrivateKey, pairs: openarray[(string, Field)]): Record = - result.pairs = @pairs - result.seqNum = seqNum +proc makeEnrAux(seqNum: uint64, pk: PrivateKey, + pairs: openarray[(string, Field)]): EnrResult[Record] = + var record: Record + record.pairs = @pairs + record.seqNum = seqNum - let pubkey = pk.toPublicKey().tryGet() + let pubkey = ? pk.toPublicKey() - result.pairs.add(("id", Field(kind: kString, str: "v4"))) - result.pairs.add(("secp256k1", Field(kind: kBytes, bytes: @(pubkey.toRawCompressed())))) + record.pairs.add(("id", Field(kind: kString, str: "v4"))) + record.pairs.add(("secp256k1", + Field(kind: kBytes, bytes: @(pubkey.toRawCompressed())))) # Sort by key - result.pairs.sort() do(a, b: (string, Field)) -> int: + record.pairs.sort() do(a, b: (string, Field)) -> int: cmp(a[0], b[0]) - proc append(w: var RlpWriter, seqNum: uint64, pairs: openarray[(string, Field)]): seq[byte] = + proc append(w: var RlpWriter, seqNum: uint64, + pairs: openarray[(string, Field)]): seq[byte] {.raises: [].} = w.append(seqNum) for (k, v) in pairs: w.append(k) @@ -83,19 +91,20 @@ proc makeEnrAux(seqNum: uint64, pk: PrivateKey, pairs: openarray[(string, Field) w.finish() let toSign = block: - var w = initRlpList(result.pairs.len * 2 + 1) - w.append(seqNum, result.pairs) + var w = initRlpList(record.pairs.len * 2 + 1) + w.append(seqNum, record.pairs) - let sig = signNR(pk, toSign) - if sig.isErr: - raise newException(CatchableError, "Could not sign ENR (internal error)") + let sig = ? signNR(pk, toSign) - result.raw = block: - var w = initRlpList(result.pairs.len * 2 + 2) - w.append(sig[].toRaw()) - w.append(seqNum, result.pairs) + record.raw = block: + var w = initRlpList(record.pairs.len * 2 + 2) + w.append(sig.toRaw()) + w.append(seqNum, record.pairs) -macro initRecord*(seqNum: uint64, pk: PrivateKey, pairs: untyped{nkTableConstr}): untyped = + ok(record) + +macro initRecord*(seqNum: uint64, pk: PrivateKey, + pairs: untyped{nkTableConstr}): untyped = for c in pairs: c.expectKind(nnkExprColonExpr) c[1] = newCall(bindSym"toField", c[1]) @@ -110,7 +119,8 @@ proc init*(T: type Record, seqNum: uint64, pk: PrivateKey, ip: Option[IpAddress], tcpPort, udpPort: Port, - extraFields: openarray[FieldPair] = []): T = + extraFields: openarray[FieldPair] = []): + EnrResult[T] = var fields = newSeq[FieldPair]() if ip.isSome(): @@ -129,7 +139,7 @@ proc init*(T: type Record, seqNum: uint64, fields.add extraFields makeEnrAux(seqNum, pk, fields) -proc getField(r: Record, name: string, field: var Field): bool = +proc getField(r: Record, name: string, field: var Field): bool {.raises: [].} = # It might be more correct to do binary search, # as the fields are sorted, but it's unlikely to # make any difference in reality. @@ -138,11 +148,11 @@ proc getField(r: Record, name: string, field: var Field): bool = field = v return true -proc requireKind(f: Field, kind: FieldKind) = +proc requireKind(f: Field, kind: FieldKind) {.raises: [ValueError].} = if f.kind != kind: raise newException(ValueError, "Wrong field kind") -proc get*(r: Record, key: string, T: type): T = +proc get*(r: Record, key: string, T: type): T {.raises: [ValueError].} = var f: Field if r.getField(key, f): when T is SomeInteger: @@ -180,7 +190,7 @@ proc get*(r: Record, T: type PublicKey): Option[T] {.raises: [Defect].} = if pk.isOk: return some pk[] -proc tryGet*(r: Record, key: string, T: type): Option[T] = +proc tryGet*(r: Record, key: string, T: type): Option[T] {.raises: [].} = try: return some get(r, key, T) except CatchableError: @@ -205,7 +215,8 @@ proc toTypedRecord*(r: Record): Option[TypedRecord] = return some(tr) -proc verifySignatureV4(r: Record, sigData: openarray[byte], content: seq[byte]): bool = +proc verifySignatureV4(r: Record, sigData: openarray[byte], content: seq[byte]): + bool = let publicKey = r.get(PublicKey) if publicKey.isSome: let sig = SignatureNR.fromRaw(sigData) @@ -213,7 +224,7 @@ proc verifySignatureV4(r: Record, sigData: openarray[byte], content: seq[byte]): var h = keccak256.digest(content) return verify(sig[], h, publicKey.get) -proc verifySignature(r: Record): bool = +proc verifySignature(r: Record): bool {.raises: [RlpError, Defect].} = var rlp = rlpFromBytes(r.raw) let sz = rlp.listLen if not rlp.enterList: @@ -236,7 +247,7 @@ proc verifySignature(r: Record): bool = # Unknown Identity Scheme discard -proc fromBytesAux(r: var Record): bool = +proc fromBytesAux(r: var Record): bool {.raises: [RlpError, Defect].} = if r.raw.len > maxEnrSize: return false @@ -275,7 +286,7 @@ proc fromBytesAux(r: var Record): bool = verifySignature(r) proc fromBytes*(r: var Record, s: openarray[byte]): bool = - # Loads ENR from rlp-encoded bytes, and validated the signature. + ## Loads ENR from rlp-encoded bytes, and validated the signature. r.raw = @s try: result = fromBytesAux(r) @@ -283,7 +294,8 @@ proc fromBytes*(r: var Record, s: openarray[byte]): bool = discard proc fromBase64*(r: var Record, s: string): bool = - # Loads ENR from base64-encoded rlp-encoded bytes, and validated the signature. + ## Loads ENR from base64-encoded rlp-encoded bytes, and validated the + ## signature. try: r.raw = Base64Url.decode(s) result = fromBytesAux(r) @@ -291,7 +303,8 @@ proc fromBase64*(r: var Record, s: string): bool = discard proc fromURI*(r: var Record, s: string): bool = - # Loads ENR from its text encoding: base64-encoded rlp-encoded bytes, prefixed with "enr:". + ## Loads ENR from its text encoding: base64-encoded rlp-encoded bytes, + ## prefixed with "enr:". const prefix = "enr:" if s.startsWith(prefix): result = r.fromBase64(s[prefix.len .. ^1]) @@ -299,12 +312,12 @@ proc fromURI*(r: var Record, s: string): bool = template fromURI*(r: var Record, url: EnrUri): bool = fromURI(r, string(url)) -proc toBase64*(r: Record): string = +proc toBase64*(r: Record): string {.raises: [].} = result = Base64Url.encode(r.raw) -proc toURI*(r: Record): string = "enr:" & r.toBase64 +proc toURI*(r: Record): string {.raises: [].} = "enr:" & r.toBase64 -proc `$`(f: Field): string = +proc `$`(f: Field): string {.raises: [].} = case f.kind of kNum: $f.num @@ -313,7 +326,7 @@ proc `$`(f: Field): string = of kString: "\"" & f.str & "\"" -proc `$`*(r: Record): string = +proc `$`*(r: Record): string {.raises: [].} = result = "(" var first = true for (k, v) in r.pairs: @@ -328,10 +341,11 @@ proc `$`*(r: Record): string = proc `==`*(a, b: Record): bool = a.raw == b.raw -proc read*(rlp: var Rlp, T: typedesc[Record]): T {.inline.} = +proc read*(rlp: var Rlp, T: typedesc[Record]): + T {.inline, raises:[RlpError, ValueError, Defect].} = if not result.fromBytes(rlp.rawData): raise newException(ValueError, "Could not deserialize") rlp.skipElem() -proc append*(rlpWriter: var RlpWriter, value: Record) = +proc append*(rlpWriter: var RlpWriter, value: Record) {.raises: [].} = rlpWriter.appendRawBytes(value.raw) diff --git a/eth/p2p/discoveryv5/node.nim b/eth/p2p/discoveryv5/node.nim index 76c2c7e..d32dd68 100644 --- a/eth/p2p/discoveryv5/node.nim +++ b/eth/p2p/discoveryv5/node.nim @@ -2,6 +2,8 @@ import std/[net, hashes], nimcrypto, stint, chronicles, types, enr, eth/keys, ../enode +{.push raises: [Defect].} + type Node* = ref object node*: ENode @@ -11,21 +13,13 @@ type proc toNodeId*(pk: PublicKey): NodeId = readUintBE[256](keccak256.digest(pk.toRaw()).data) -proc newNode*(enode: ENode): Node = - Node(node: enode, - id: enode.pubkey.toNodeId()) - +# TODO: Lets not allow to create a node where enode info is not in sync with the +# record proc newNode*(enode: ENode, r: Record): Node = Node(node: enode, id: enode.pubkey.toNodeId(), record: r) -proc newNode*(uriString: string): Node = - newNode ENode.fromString(uriString).tryGet() - -proc newNode*(pk: PublicKey, address: Address): Node = - newNode ENode(pubkey: pk, address: address) - proc newNode*(r: Record): Node = # TODO: Handle IPv6 var a: Address @@ -37,28 +31,33 @@ proc newNode*(r: Record): Node = a = Address(ip: IpAddress(family: IpAddressFamily.IPv4, address_v4: ipBytes), udpPort: Port udpPort) - except KeyError: + except KeyError, ValueError: # TODO: This will result in a 0.0.0.0 address. Might introduce more bugs. # Maybe we shouldn't allow the creation of Node from Record without IP. # Will need some refactor though. discard - let pk = PublicKey.fromRaw(r.get("secp256k1", seq[byte])) - if pk.isErr: - warn "Could not recover public key", err = pk.error + let pk = r.get(PublicKey) + if pk.isNone(): + warn "Could not recover public key from ENR" return - result = newNode(ENode(pubkey: pk[], address: a)) - result.record = r + let enode = ENode(pubkey: pk.get(), address: a) + result = Node(node: enode, + id: enode.pubkey.toNodeId(), + record: r) proc hash*(n: Node): hashes.Hash = hash(n.node.pubkey.toRaw) -proc `==`*(a, b: Node): bool = (a.isNil and b.isNil) or (not a.isNil and not b.isNil and a.node.pubkey == b.node.pubkey) +proc `==`*(a, b: Node): bool {.raises: [].} = + (a.isNil and b.isNil) or + (not a.isNil and not b.isNil and a.node.pubkey == b.node.pubkey) -proc address*(n: Node): Address {.inline.} = n.node.address +proc address*(n: Node): Address {.inline, raises: [].} = n.node.address -proc updateEndpoint*(n: Node, a: Address) {.inline.} = n.node.address = a +proc updateEndpoint*(n: Node, a: Address) {.inline, raises: [].} = + n.node.address = a -proc `$`*(n: Node): string = +proc `$`*(n: Node): string {.raises: [].} = if n == nil: "Node[local]" else: diff --git a/eth/p2p/discoveryv5/protocol.nim b/eth/p2p/discoveryv5/protocol.nim index a1c7720..2c69cd3 100644 --- a/eth/p2p/discoveryv5/protocol.nim +++ b/eth/p2p/discoveryv5/protocol.nim @@ -585,7 +585,8 @@ proc newProtocol*(privKey: PrivateKey, db: Database, a = Address(ip: externalIp.get(IPv4_any()), tcpPort: tcpPort, udpPort: udpPort) enode = ENode(pubkey: privKey.toPublicKey().tryGet(), address: a) - enrRec = enr.Record.init(1, privKey, externalIp, tcpPort, udpPort, localEnrFields) + enrRec = enr.Record.init(1, privKey, externalIp, tcpPort, udpPort, + localEnrFields).expect("Properly intialized private key") node = newNode(enode, enrRec) result = Protocol( diff --git a/eth/p2p/discoveryv5/routing_table.nim b/eth/p2p/discoveryv5/routing_table.nim index b1b633b..14f3092 100644 --- a/eth/p2p/discoveryv5/routing_table.nim +++ b/eth/p2p/discoveryv5/routing_table.nim @@ -3,6 +3,8 @@ import stint, chronicles, types, node +{.push raises: [Defect].} + type RoutingTable* = object thisNode: Node @@ -19,11 +21,11 @@ const BITS_PER_HOP = 8 ID_SIZE = 256 -proc distanceTo(n: Node, id: NodeId): UInt256 = +proc distanceTo(n: Node, id: NodeId): UInt256 {.raises: [].} = ## Calculate the distance to a NodeId. n.id xor id -proc logDist*(a, b: NodeId): uint32 = +proc logDist*(a, b: NodeId): uint32 {.raises: [].} = ## Calculate the logarithmic distance between two `NodeId`s. ## ## According the specification, this is the log base 2 of the distance. But it @@ -42,7 +44,7 @@ proc logDist*(a, b: NodeId): uint32 = break return uint32(a.len * 8 - lz) -proc newKBucket(istart, iend: NodeId): KBucket = +proc newKBucket(istart, iend: NodeId): KBucket {.raises: [].} = result.new() result.istart = istart result.iend = iend @@ -53,13 +55,13 @@ proc midpoint(k: KBucket): NodeId = k.istart + (k.iend - k.istart) div 2.u256 proc distanceTo(k: KBucket, id: NodeId): UInt256 = k.midpoint xor id -proc nodesByDistanceTo(k: KBucket, id: NodeId): seq[Node] = +proc nodesByDistanceTo(k: KBucket, id: NodeId): seq[Node] {.raises: [].} = sortedByIt(k.nodes, it.distanceTo(id)) -proc len(k: KBucket): int {.inline.} = k.nodes.len -proc head(k: KBucket): Node {.inline.} = k.nodes[0] +proc len(k: KBucket): int {.inline, raises: [].} = k.nodes.len +proc head(k: KBucket): Node {.inline, raises: [].} = k.nodes[0] -proc add(k: KBucket, n: Node): Node = +proc add(k: KBucket, n: Node): Node {.raises: [].} = ## Try to add the given node to this bucket. ## If the node is already present, it is moved to the tail of the list, and we return nil. @@ -82,7 +84,7 @@ proc add(k: KBucket, n: Node): Node = return k.head return nil -proc removeNode(k: KBucket, n: Node) = +proc removeNode(k: KBucket, n: Node) {.raises: [].} = let i = k.nodes.find(n) if i != -1: k.nodes.delete(i) @@ -98,12 +100,10 @@ proc split(k: KBucket): tuple[lower, upper: KBucket] = let bucket = if node.id <= splitid: result.lower else: result.upper bucket.replacementCache.add(node) -proc inRange(k: KBucket, n: Node): bool {.inline.} = +proc inRange(k: KBucket, n: Node): bool {.inline, raises: [].} = k.istart <= n.id and n.id <= k.iend -proc isFull(k: KBucket): bool = k.len == BUCKET_SIZE - -proc contains(k: KBucket, n: Node): bool = n in k.nodes +proc contains(k: KBucket, n: Node): bool {.raises: [].} = n in k.nodes proc binaryGetBucketForNode(buckets: openarray[KBucket], id: NodeId): KBucket {.inline.} = @@ -116,8 +116,10 @@ proc binaryGetBucketForNode(buckets: openarray[KBucket], if bucket.istart <= id and id <= bucket.iend: result = bucket + # TODO: Is this really an error that should occur? Feels a lot like a work- + # around to another problem. Set to Defect for now. if result.isNil: - raise newException(ValueError, "No bucket found for node with id " & $id) + raise (ref Defect)(msg: "No bucket found for node with id " & $id) proc computeSharedPrefixBits(nodes: openarray[Node]): int = ## Count the number of prefix bits shared by all nodes. @@ -138,7 +140,7 @@ proc computeSharedPrefixBits(nodes: openarray[Node]): int = doAssert(false, "Unable to calculate number of shared prefix bits") -proc init*(r: var RoutingTable, thisNode: Node) {.inline.} = +proc init*(r: var RoutingTable, thisNode: Node) {.inline, raises: [].} = r.thisNode = thisNode r.buckets = @[newKBucket(0.u256, high(Uint256))] randomize() # for later `randomNodes` selection @@ -186,9 +188,6 @@ proc contains*(r: RoutingTable, n: Node): bool = n in r.bucketForNode(n.id) proc bucketsByDistanceTo(r: RoutingTable, id: NodeId): seq[KBucket] = sortedByIt(r.buckets, it.distanceTo(id)) -proc notFullBuckets(r: RoutingTable): seq[KBucket] = - r.buckets.filterIt(not it.isFull) - proc neighbours*(r: RoutingTable, id: NodeId, k: int = BUCKET_SIZE): seq[Node] = ## Return up to k neighbours of the given node. result = newSeqOfCap[Node](k * 2) @@ -205,7 +204,7 @@ proc neighbours*(r: RoutingTable, id: NodeId, k: int = BUCKET_SIZE): seq[Node] = if result.len > k: result.setLen(k) -proc idAtDistance*(id: NodeId, dist: uint32): NodeId = +proc idAtDistance*(id: NodeId, dist: uint32): NodeId {.raises: [].} = ## Calculate the "lowest" `NodeId` for given logarithmic distance. ## A logarithmic distance obviously covers a whole range of distances and thus ## potential `NodeId`s. @@ -220,10 +219,10 @@ proc neighboursAtDistance*(r: RoutingTable, distance: uint32, # that are exactly the requested distance. keepIf(result, proc(n: Node): bool = logDist(n.id, r.thisNode.id) == distance) -proc len*(r: RoutingTable): int = +proc len*(r: RoutingTable): int {.raises: [].} = for b in r.buckets: result += b.len -proc moveRight[T](arr: var openarray[T], a, b: int) {.inline.} = +proc moveRight[T](arr: var openarray[T], a, b: int) {.inline, raises: [].} = ## In `arr` move elements in range [a, b] right by 1. var t: T shallowCopy(t, arr[b + 1]) @@ -241,7 +240,7 @@ proc setJustSeen*(r: RoutingTable, n: Node) = b.nodes[0] = n b.lastUpdated = epochTime() -proc nodeToRevalidate*(r: RoutingTable): Node {.raises:[].} = +proc nodeToRevalidate*(r: RoutingTable): Node {.raises: [].} = var buckets = r.buckets shuffle(buckets) # TODO: Should we prioritize less-recently-updated buckets instead? diff --git a/tests/p2p/test_discoveryv5.nim b/tests/p2p/test_discoveryv5.nim index 055a0d6..de00ed8 100644 --- a/tests/p2p/test_discoveryv5.nim +++ b/tests/p2p/test_discoveryv5.nim @@ -35,7 +35,7 @@ proc randomPacket(tag: PacketTag): seq[byte] = proc generateNode(privKey = PrivateKey.random()[], port: int = 20302): Node = let port = Port(port) let enr = enr.Record.init(1, privKey, some(parseIpAddress("127.0.0.1")), - port, port) + port, port).expect("Properly intialized private key") result = newNode(enr) proc nodeAtDistance(n: Node, d: uint32): Node = @@ -344,7 +344,7 @@ suite "Discovery v5 Tests": # TODO: need to add some logic to update ENRs properly targetSeqNum.inc() let r = enr.Record.init(targetSeqNum, targetKey, - some(targetAddress.ip), targetAddress.tcpPort, targetAddress.udpPort) + some(targetAddress.ip), targetAddress.tcpPort, targetAddress.udpPort)[] targetNode.localNode.record = r targetNode.open() let n = await mainNode.resolve(targetId) @@ -358,7 +358,7 @@ suite "Discovery v5 Tests": block: targetSeqNum.inc() let r = enr.Record.init(3, targetKey, some(targetAddress.ip), - targetAddress.tcpPort, targetAddress.udpPort) + targetAddress.tcpPort, targetAddress.udpPort)[] targetNode.localNode.record = r let pong = await targetNode.ping(lookupNode.localNode) require pong.isSome() diff --git a/tests/p2p/test_enr.nim b/tests/p2p/test_enr.nim index 9edb39b..7edac0d 100644 --- a/tests/p2p/test_enr.nim +++ b/tests/p2p/test_enr.nim @@ -7,7 +7,7 @@ suite "ENR": test "Serialization": var pk = PrivateKey.fromHex( "5d2908f3f09ea1ff2e327c3f623159639b00af406e9009de5fd4b910fc34049d")[] - var r = initRecord(123, pk, {"udp": 1234'u, "ip": [byte 5, 6, 7, 8]}) + var r = initRecord(123, pk, {"udp": 1234'u, "ip": [byte 5, 6, 7, 8]})[] doAssert($r == """(id: "v4", ip: 0x05060708, secp256k1: 0x02E51EFA66628CE09F689BC2B82F165A75A9DDECBB6A804BE15AC3FDF41F3B34E7, udp: 1234)""") let uri = r.toURI() var r2: Record @@ -35,7 +35,7 @@ suite "ENR": let keys = KeyPair.random()[] ip = parseIpAddress("10.20.30.40") - enr = Record.init(100, keys.seckey, some(ip), Port(9000), Port(9000), @[]) + enr = Record.init(100, keys.seckey, some(ip), Port(9000), Port(9000), @[])[] typedEnr = get enr.toTypedRecord() check: @@ -54,7 +54,7 @@ suite "ENR": test "ENR without address": let keys = KeyPair.random()[] - enr = Record.init(100, keys.seckey, none(IpAddress), Port(9000), Port(9000)) + enr = Record.init(100, keys.seckey, none(IpAddress), Port(9000), Port(9000))[] typedEnr = get enr.toTypedRecord() check: From 887cbba563b155a99dd4027dfa451f78168572e1 Mon Sep 17 00:00:00 2001 From: kdeme Date: Fri, 1 May 2020 22:34:26 +0200 Subject: [PATCH 5/5] discv5: Address review comments --- eth/p2p/discoveryv5/encoding.nim | 45 +++++++++++++-------------- eth/p2p/discoveryv5/enr.nim | 20 ++++++------ eth/p2p/discoveryv5/node.nim | 8 ++--- eth/p2p/discoveryv5/routing_table.nim | 30 +++++++++--------- 4 files changed, 50 insertions(+), 53 deletions(-) diff --git a/eth/p2p/discoveryv5/encoding.nim b/eth/p2p/discoveryv5/encoding.nim index 128245f..d749e8e 100644 --- a/eth/p2p/discoveryv5/encoding.nim +++ b/eth/p2p/discoveryv5/encoding.nim @@ -42,19 +42,19 @@ type response*: seq[byte] DecodeError* = enum - HandshakeError, - PacketError, - DecryptError, - UnsupportedMessage + HandshakeError = "discv5: handshake failed" + PacketError = "discv5: invalid packet", + DecryptError = "discv5: decryption failed", + UnsupportedMessage = "discv5: unsupported message" DecodeResult*[T] = Result[T, DecodeError] EncodeResult*[T] = Result[T, cstring] proc mapErrTo[T, E](r: Result[T, E], v: static DecodeError): - DecodeResult[T] {.raises:[].} = + DecodeResult[T] = r.mapErr(proc (e: E): DecodeError = v) -proc idNonceHash(nonce, ephkey: openarray[byte]): MDigest[256] {.raises:[].} = +proc idNonceHash(nonce, ephkey: openarray[byte]): MDigest[256] = var ctx: sha256 ctx.init() ctx.update(idNoncePrefix) @@ -81,8 +81,7 @@ proc deriveKeys(n1, n2: NodeID, priv: PrivateKey, pub: PublicKey, hkdf(sha256, eph.data, idNonce, info, toOpenArray(res, 0, sizeof(secrets) - 1)) ok(secrets) -proc encryptGCM*(key, nonce, pt, authData: openarray[byte]): - seq[byte] {.raises:[].} = +proc encryptGCM*(key, nonce, pt, authData: openarray[byte]): seq[byte] = var ectx: GCM[aes128] ectx.init(key, nonce, authData) result = newSeq[byte](pt.len + gcmTagSize) @@ -93,9 +92,8 @@ proc encryptGCM*(key, nonce, pt, authData: openarray[byte]): proc encodeAuthHeader(c: Codec, toId: NodeID, nonce: array[gcmNonceSize, byte], - handshakeSecrets: var HandshakeSecrets, challenge: Whoareyou): - EncodeResult[seq[byte]] = + EncodeResult[(seq[byte], HandshakeSecrets)] = var resp = AuthResponse(version: 5) let ln = c.localNode @@ -108,24 +106,24 @@ proc encodeAuthHeader(c: Codec, ephKeys.pubkey.toRaw) resp.signature = signature.toRaw - handshakeSecrets = ? deriveKeys(ln.id, toId, ephKeys.seckey, challenge.pubKey, + let secrets = ? deriveKeys(ln.id, toId, ephKeys.seckey, challenge.pubKey, challenge.idNonce) let respRlp = rlp.encode(resp) var zeroNonce: array[gcmNonceSize, byte] - let respEnc = encryptGCM(handshakeSecrets.authRespKey, zeroNonce, respRLP, []) + let respEnc = encryptGCM(secrets.authRespKey, zeroNonce, respRLP, []) let header = AuthHeader(auth: nonce, idNonce: challenge.idNonce, scheme: authSchemeName, ephemeralKey: ephKeys.pubkey.toRaw, response: respEnc) - ok(rlp.encode(header)) + ok((rlp.encode(header), secrets)) -proc `xor`[N: static[int], T](a, b: array[N, T]): array[N, T] {.raises:[].} = +proc `xor`[N: static[int], T](a, b: array[N, T]): array[N, T] = for i in 0 .. a.high: result[i] = a[i] xor b[i] -proc packetTag(destNode, srcNode: NodeID): PacketTag {.raises:[].} = +proc packetTag(destNode, srcNode: NodeID): PacketTag = let destId = destNode.toByteArrayBE() srcId = srcNode.toByteArrayBE() @@ -135,7 +133,7 @@ proc packetTag(destNode, srcNode: NodeID): PacketTag {.raises:[].} = proc encodePacket*(c: Codec, toId: NodeID, toAddr: Address, - message: seq[byte], + message: openarray[byte], challenge: Whoareyou): EncodeResult[(seq[byte], array[gcmNonceSize, byte])] = var nonce: array[gcmNonceSize, byte] @@ -154,12 +152,12 @@ proc encodePacket*(c: Codec, # yet. That's fine, we will be responded with whoareyou. discard c.db.loadKeys(toId, toAddr, readKey, writeKey) else: - var sec: HandshakeSecrets - headEnc = ? c.encodeAuthHeader(toId, nonce, sec, challenge) + var secrets: HandshakeSecrets + (headEnc, secrets) = ? c.encodeAuthHeader(toId, nonce, challenge) - writeKey = sec.writeKey + writeKey = secrets.writeKey # TODO: is it safe to ignore the error here? - discard c.db.storeKeys(toId, toAddr, sec.readKey, sec.writeKey) + discard c.db.storeKeys(toId, toAddr, secrets.readKey, secrets.writeKey) let tag = packetTag(toId, c.localNode.id) @@ -335,18 +333,17 @@ proc decodePacket*(c: var Codec, decodeMessage(message.get()) -proc newRequestId*(): Result[RequestId, cstring] {.raises:[].} = +proc newRequestId*(): Result[RequestId, cstring] = var id: RequestId if randomBytes(addr id, sizeof(id)) != sizeof(id): err("Could not randomize bytes") else: ok(id) -proc numFields(T: typedesc): int {.raises:[].} = +proc numFields(T: typedesc): int = for k, v in fieldPairs(default(T)): inc result -proc encodeMessage*[T: SomeMessage](p: T, reqId: RequestId): - seq[byte] {.raises:[].} = +proc encodeMessage*[T: SomeMessage](p: T, reqId: RequestId): seq[byte] = result = newSeqOfCap[byte](64) result.add(messageKind(T).ord) diff --git a/eth/p2p/discoveryv5/enr.nim b/eth/p2p/discoveryv5/enr.nim index 21351f8..9712775 100644 --- a/eth/p2p/discoveryv5/enr.nim +++ b/eth/p2p/discoveryv5/enr.nim @@ -80,7 +80,7 @@ proc makeEnrAux(seqNum: uint64, pk: PrivateKey, cmp(a[0], b[0]) proc append(w: var RlpWriter, seqNum: uint64, - pairs: openarray[(string, Field)]): seq[byte] {.raises: [].} = + pairs: openarray[(string, Field)]): seq[byte] = w.append(seqNum) for (k, v) in pairs: w.append(k) @@ -139,7 +139,7 @@ proc init*(T: type Record, seqNum: uint64, fields.add extraFields makeEnrAux(seqNum, pk, fields) -proc getField(r: Record, name: string, field: var Field): bool {.raises: [].} = +proc getField(r: Record, name: string, field: var Field): bool = # It might be more correct to do binary search, # as the fields are sorted, but it's unlikely to # make any difference in reality. @@ -152,7 +152,7 @@ proc requireKind(f: Field, kind: FieldKind) {.raises: [ValueError].} = if f.kind != kind: raise newException(ValueError, "Wrong field kind") -proc get*(r: Record, key: string, T: type): T {.raises: [ValueError].} = +proc get*(r: Record, key: string, T: type): T {.raises: [ValueError, Defect].} = var f: Field if r.getField(key, f): when T is SomeInteger: @@ -183,14 +183,14 @@ proc get*(r: Record, key: string, T: type): T {.raises: [ValueError].} = else: raise newException(KeyError, "Key not found in ENR: " & key) -proc get*(r: Record, T: type PublicKey): Option[T] {.raises: [Defect].} = +proc get*(r: Record, T: type PublicKey): Option[T] = var pubkeyField: Field if r.getField("secp256k1", pubkeyField) and pubkeyField.kind == kBytes: let pk = PublicKey.fromRaw(pubkeyField.bytes) if pk.isOk: return some pk[] -proc tryGet*(r: Record, key: string, T: type): Option[T] {.raises: [].} = +proc tryGet*(r: Record, key: string, T: type): Option[T] = try: return some get(r, key, T) except CatchableError: @@ -312,12 +312,12 @@ proc fromURI*(r: var Record, s: string): bool = template fromURI*(r: var Record, url: EnrUri): bool = fromURI(r, string(url)) -proc toBase64*(r: Record): string {.raises: [].} = +proc toBase64*(r: Record): string = result = Base64Url.encode(r.raw) -proc toURI*(r: Record): string {.raises: [].} = "enr:" & r.toBase64 +proc toURI*(r: Record): string = "enr:" & r.toBase64 -proc `$`(f: Field): string {.raises: [].} = +proc `$`(f: Field): string = case f.kind of kNum: $f.num @@ -326,7 +326,7 @@ proc `$`(f: Field): string {.raises: [].} = of kString: "\"" & f.str & "\"" -proc `$`*(r: Record): string {.raises: [].} = +proc `$`*(r: Record): string = result = "(" var first = true for (k, v) in r.pairs: @@ -347,5 +347,5 @@ proc read*(rlp: var Rlp, T: typedesc[Record]): raise newException(ValueError, "Could not deserialize") rlp.skipElem() -proc append*(rlpWriter: var RlpWriter, value: Record) {.raises: [].} = +proc append*(rlpWriter: var RlpWriter, value: Record) = rlpWriter.appendRawBytes(value.raw) diff --git a/eth/p2p/discoveryv5/node.nim b/eth/p2p/discoveryv5/node.nim index d32dd68..8aca85b 100644 --- a/eth/p2p/discoveryv5/node.nim +++ b/eth/p2p/discoveryv5/node.nim @@ -48,16 +48,16 @@ proc newNode*(r: Record): Node = record: r) proc hash*(n: Node): hashes.Hash = hash(n.node.pubkey.toRaw) -proc `==`*(a, b: Node): bool {.raises: [].} = +proc `==`*(a, b: Node): bool = (a.isNil and b.isNil) or (not a.isNil and not b.isNil and a.node.pubkey == b.node.pubkey) -proc address*(n: Node): Address {.inline, raises: [].} = n.node.address +proc address*(n: Node): Address {.inline.} = n.node.address -proc updateEndpoint*(n: Node, a: Address) {.inline, raises: [].} = +proc updateEndpoint*(n: Node, a: Address) {.inline.} = n.node.address = a -proc `$`*(n: Node): string {.raises: [].} = +proc `$`*(n: Node): string = if n == nil: "Node[local]" else: diff --git a/eth/p2p/discoveryv5/routing_table.nim b/eth/p2p/discoveryv5/routing_table.nim index 14f3092..dc19a56 100644 --- a/eth/p2p/discoveryv5/routing_table.nim +++ b/eth/p2p/discoveryv5/routing_table.nim @@ -21,11 +21,11 @@ const BITS_PER_HOP = 8 ID_SIZE = 256 -proc distanceTo(n: Node, id: NodeId): UInt256 {.raises: [].} = +proc distanceTo(n: Node, id: NodeId): UInt256 = ## Calculate the distance to a NodeId. n.id xor id -proc logDist*(a, b: NodeId): uint32 {.raises: [].} = +proc logDist*(a, b: NodeId): uint32 = ## Calculate the logarithmic distance between two `NodeId`s. ## ## According the specification, this is the log base 2 of the distance. But it @@ -44,7 +44,7 @@ proc logDist*(a, b: NodeId): uint32 {.raises: [].} = break return uint32(a.len * 8 - lz) -proc newKBucket(istart, iend: NodeId): KBucket {.raises: [].} = +proc newKBucket(istart, iend: NodeId): KBucket = result.new() result.istart = istart result.iend = iend @@ -55,13 +55,13 @@ proc midpoint(k: KBucket): NodeId = k.istart + (k.iend - k.istart) div 2.u256 proc distanceTo(k: KBucket, id: NodeId): UInt256 = k.midpoint xor id -proc nodesByDistanceTo(k: KBucket, id: NodeId): seq[Node] {.raises: [].} = +proc nodesByDistanceTo(k: KBucket, id: NodeId): seq[Node] = sortedByIt(k.nodes, it.distanceTo(id)) -proc len(k: KBucket): int {.inline, raises: [].} = k.nodes.len -proc head(k: KBucket): Node {.inline, raises: [].} = k.nodes[0] +proc len(k: KBucket): int {.inline.} = k.nodes.len +proc head(k: KBucket): Node {.inline.} = k.nodes[0] -proc add(k: KBucket, n: Node): Node {.raises: [].} = +proc add(k: KBucket, n: Node): Node = ## Try to add the given node to this bucket. ## If the node is already present, it is moved to the tail of the list, and we return nil. @@ -84,7 +84,7 @@ proc add(k: KBucket, n: Node): Node {.raises: [].} = return k.head return nil -proc removeNode(k: KBucket, n: Node) {.raises: [].} = +proc removeNode(k: KBucket, n: Node) = let i = k.nodes.find(n) if i != -1: k.nodes.delete(i) @@ -100,10 +100,10 @@ proc split(k: KBucket): tuple[lower, upper: KBucket] = let bucket = if node.id <= splitid: result.lower else: result.upper bucket.replacementCache.add(node) -proc inRange(k: KBucket, n: Node): bool {.inline, raises: [].} = +proc inRange(k: KBucket, n: Node): bool {.inline.} = k.istart <= n.id and n.id <= k.iend -proc contains(k: KBucket, n: Node): bool {.raises: [].} = n in k.nodes +proc contains(k: KBucket, n: Node): bool = n in k.nodes proc binaryGetBucketForNode(buckets: openarray[KBucket], id: NodeId): KBucket {.inline.} = @@ -140,7 +140,7 @@ proc computeSharedPrefixBits(nodes: openarray[Node]): int = doAssert(false, "Unable to calculate number of shared prefix bits") -proc init*(r: var RoutingTable, thisNode: Node) {.inline, raises: [].} = +proc init*(r: var RoutingTable, thisNode: Node) {.inline.} = r.thisNode = thisNode r.buckets = @[newKBucket(0.u256, high(Uint256))] randomize() # for later `randomNodes` selection @@ -204,7 +204,7 @@ proc neighbours*(r: RoutingTable, id: NodeId, k: int = BUCKET_SIZE): seq[Node] = if result.len > k: result.setLen(k) -proc idAtDistance*(id: NodeId, dist: uint32): NodeId {.raises: [].} = +proc idAtDistance*(id: NodeId, dist: uint32): NodeId = ## Calculate the "lowest" `NodeId` for given logarithmic distance. ## A logarithmic distance obviously covers a whole range of distances and thus ## potential `NodeId`s. @@ -219,10 +219,10 @@ proc neighboursAtDistance*(r: RoutingTable, distance: uint32, # that are exactly the requested distance. keepIf(result, proc(n: Node): bool = logDist(n.id, r.thisNode.id) == distance) -proc len*(r: RoutingTable): int {.raises: [].} = +proc len*(r: RoutingTable): int = for b in r.buckets: result += b.len -proc moveRight[T](arr: var openarray[T], a, b: int) {.inline, raises: [].} = +proc moveRight[T](arr: var openarray[T], a, b: int) {.inline.} = ## In `arr` move elements in range [a, b] right by 1. var t: T shallowCopy(t, arr[b + 1]) @@ -240,7 +240,7 @@ proc setJustSeen*(r: RoutingTable, n: Node) = b.nodes[0] = n b.lastUpdated = epochTime() -proc nodeToRevalidate*(r: RoutingTable): Node {.raises: [].} = +proc nodeToRevalidate*(r: RoutingTable): Node = var buckets = r.buckets shuffle(buckets) # TODO: Should we prioritize less-recently-updated buckets instead?