Limit request id to maximum 8 bytes

eth/p2p/discoveryv5/encodingv1.nim

@@ -351,9 +351,8 @@ proc decodeMessage*(body: openarray[byte]): DecodeResult[Message] =
   var rlp = rlpFromBytes(body.toOpenArray(1, body.high))
   if rlp.enterList:
     try:
-      # TODO: 8 bytes limitation on RequestId decode.
       message.reqId = rlp.read(RequestId)
-    except RlpError:
+    except RlpError, ValueError:
       return err(PacketError)
 
     proc decode[T](rlp: var Rlp, v: var T)
@@ -556,9 +555,9 @@ proc decodePacket*(c: var Codec, fromAddr: Address, input: openArray[byte]):
       input.toOpenArray(ivSize + header.len, input.high))
 
 proc init*(T: type RequestId, rng: var BrHmacDrbgContext): T =
-  var id = newSeq[byte](8) # RequestId must be <= 8 bytes
-  brHmacDrbgGenerate(rng, id)
-  id
+  var reqId = RequestId(id: newSeq[byte](8)) # RequestId must be <= 8 bytes
+  brHmacDrbgGenerate(rng, reqId.id)
+  reqId
 
 proc numFields(T: typedesc): int =
   for k, v in fieldPairs(default(T)): inc result

eth/p2p/discoveryv5/typesv1.nim

@@ -1,7 +1,7 @@
 import
   std/hashes,
   stint,
-  enr, node
+  eth/rlp, enr, node
 
 {.push raises: [Defect].}
 
@@ -32,7 +32,8 @@ type
     regconfirmation = 0x09
     topicquery = 0x0A
 
-  RequestId* = seq[byte]
+  RequestId* = object
+    id*: seq[byte]
 
   PingMessage* = object
     enrSeq*: uint64
@@ -99,6 +100,23 @@ template messageKind*(T: typedesc[SomeMessage]): MessageKind =
   elif T is TalkReqMessage: talkreq
   elif T is TalkRespMessage: talkresp
 
+proc read*(rlp: var Rlp, T: type RequestId): T
+    {.raises: [ValueError, RlpError, Defect].} =
+  mixin read
+  var reqId: RequestId
+  reqId.id = rlp.toBytes()
+  if reqId.id.len > 8:
+    raise newException(ValueError, "RequestId is > 8 bytes")
+  rlp.skipElem()
+
+  reqId
+
+proc append*(writer: var RlpWriter, value: RequestId) =
+  writer.append(value.id)
+
+proc hash*(reqId: RequestId): Hash =
+  hash(reqId.id)
+
 proc toBytes*(id: NodeId): array[32, byte] {.inline.} =
   id.toByteArrayBE()
 

tests/p2p/test_discv51_encoding.nim

@@ -11,7 +11,7 @@ suite "Discovery v5.1 Protocol Message Encodings":
     let
       enrSeq = 1'u64
       p = PingMessage(enrSeq: enrSeq)
-      reqId: RequestId = @[1.byte]
+      reqId = RequestId(id: @[1.byte])
 
     let encoded = encodeMessage(p, reqId)
     check encoded.toHex == "01c20101"
@@ -31,7 +31,7 @@ suite "Discovery v5.1 Protocol Message Encodings":
       ip = @[127.byte, 0, 0, 1]
       port = 5000'u16
       p = PongMessage(enrSeq: enrSeq, ip: ip, port: port)
-      reqId: RequestId = @[1.byte]
+      reqId = RequestId(id: @[1.byte])
 
     let encoded = encodeMessage(p, reqId)
     check encoded.toHex == "02ca0101847f000001821388"
@@ -51,7 +51,7 @@ suite "Discovery v5.1 Protocol Message Encodings":
     let
       distances = @[0x0100'u32]
       fn = FindNodeMessage(distances: distances)
-      reqId: RequestId = @[1.byte]
+      reqId = RequestId(id: @[1.byte])
 
     let encoded = encodeMessage(fn, reqId)
     check encoded.toHex == "03c501c3820100"
@@ -69,7 +69,7 @@ suite "Discovery v5.1 Protocol Message Encodings":
     let
       total = 0x1'u32
       n = NodesMessage(total: total)
-      reqId: RequestId = @[1.byte]
+      reqId = RequestId(id: @[1.byte])
 
     let encoded = encodeMessage(n, reqId)
     check encoded.toHex == "04c30101c0"
@@ -91,7 +91,7 @@ suite "Discovery v5.1 Protocol Message Encodings":
     let
       total = 0x1'u32
       n = NodesMessage(total: total, enrs: @[e1, e2])
-      reqId: RequestId = @[1.byte]
+      reqId = RequestId(id: @[1.byte])
 
     let encoded = encodeMessage(n, reqId)
     check encoded.toHex == "04f8f20101f8eef875b8401ce2991c64993d7c84c29a00bdc871917551c7d330fca2dd0d69c706596dc655448f030b98a77d4001fd46ae0112ce26d613c5a6a02a81a6223cd0c4edaa53280182696482763489736563703235366b31a103ca634cae0d49acb401d8a4c6b6fe8c55b70d115bf400769cc1400f3258cd3138f875b840d7f1c39e376297f81d7297758c64cb37dcc5c3beea9f57f7ce9695d7d5a67553417d719539d6ae4b445946de4d99e680eb8063f29485b555d45b7df16a1850130182696482763489736563703235366b31a1030e2cb74241c0c4fc8e8166f1a79a05d5b0dd95813a74b094529f317d5c39d235"
@@ -108,6 +108,18 @@ suite "Discovery v5.1 Protocol Message Encodings":
       message.nodes.enrs[0] == e1
       message.nodes.enrs[1] == e2
 
+  test "Ping with too large RequestId":
+    let
+      enrSeq = 1'u64
+      p = PingMessage(enrSeq: enrSeq)
+      # 1 byte too large
+      reqId = RequestId(id: @[0.byte, 1, 2, 3, 4, 5, 6, 7, 8])
+    let encoded = encodeMessage(p, reqId)
+    check encoded.toHex == "01cb8900010203040506070801"
+
+    let decoded = decodeMessage(encoded)
+    check decoded.isErr()
+
 # According to test vectors:
 # https://github.com/fjl/devp2p/blob/discv5-v1-update/discv5/discv5-wire-test-vectors.md#cryptographic-primitives
 suite "Discovery v5.1 Cryptographic Primitives Test Vectors":
@@ -236,7 +248,7 @@ suite "Discovery v5.1 Packet Encodings Test Vectors":
     check:
       decoded.isOK()
       decoded.get().messageOpt.isSome()
-      decoded.get().messageOpt.get().reqId == hexToSeqByte(pingReqId)
+      decoded.get().messageOpt.get().reqId.id == hexToSeqByte(pingReqId)
       decoded.get().messageOpt.get().kind == ping
       decoded.get().messageOpt.get().ping.enrSeq == pingEnrSeq
 
@@ -300,7 +312,7 @@ suite "Discovery v5.1 Packet Encodings Test Vectors":
 
     check:
       decoded.isOk()
-      decoded.get().message.reqId == hexToSeqByte(pingReqId)
+      decoded.get().message.reqId.id == hexToSeqByte(pingReqId)
       decoded.get().message.kind == ping
       decoded.get().message.ping.enrSeq == pingEnrSeq
       decoded.get().node.isNone()
@@ -347,7 +359,7 @@ suite "Discovery v5.1 Packet Encodings Test Vectors":
 
     check:
       decoded.isOk()
-      decoded.get().message.reqId == hexToSeqByte(pingReqId)
+      decoded.get().message.reqId.id == hexToSeqByte(pingReqId)
       decoded.get().message.kind == ping
       decoded.get().message.ping.enrSeq == pingEnrSeq
       decoded.get().node.isSome()