Merge pull request #246 from status-im/check-authresponse

Add checks on auth-response

eth/p2p/discoveryv5/encoding.nim

@@ -89,7 +89,7 @@ proc encryptGCM*(key, nonce, pt, authData: openarray[byte]): seq[byte] =
   ectx.getTag(result.toOpenArray(pt.len, result.high))
   ectx.clear()
 
-proc encodeAuthHeader(c: Codec,
+proc encodeAuthHeader*(c: Codec,
                       toId: NodeID,
                       nonce: array[gcmNonceSize, byte],
                       challenge: Whoareyou):
@@ -225,16 +225,16 @@ proc decodeMessage(body: openarray[byte]):
   else:
     err(PacketError)
 
-proc decodeAuthResp(c: Codec, fromId: NodeId, head: AuthHeader,
-    challenge: Whoareyou, secrets: var HandshakeSecrets, newNode: var Node):
-    DecodeResult[void] {.raises:[Defect].} =
+proc decodeAuthResp*(c: Codec, fromId: NodeId, head: AuthHeader,
+    challenge: Whoareyou, newNode: var Node):
+    DecodeResult[HandshakeSecrets] {.raises:[Defect].} =
   if head.scheme != authSchemeName:
     warn "Unknown auth scheme"
     return err(HandshakeError)
 
   let ephKey = ? PublicKey.fromRaw(head.ephemeralKey).mapErrTo(HandshakeError)
 
-  secrets = ? deriveKeys(fromId, c.localNode.id, c.privKey, ephKey,
+  let secrets = ? deriveKeys(fromId, c.localNode.id, c.privKey, ephKey,
     challenge.idNonce).mapErrTo(HandshakeError)
 
   var zeroNonce: array[gcmNonceSize, byte]
@@ -244,18 +244,26 @@ proc decodeAuthResp(c: Codec, fromId: NodeId, head: AuthHeader,
 
   var authResp: AuthResponse
   try:
+    # Signature check of record happens in decode.
     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
+  # 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
 
   # Node returned might not have an address or not a valid address
   newNode = ? newNode(authResp.record).mapErrTo(HandshakeError)
-  ok()
+  if newNode.id != fromId:
+    return err(HandshakeError)
+
+  # Verify the id-nonce-sig
+  let sig = ? SignatureNR.fromRaw(authResp.signature).mapErrTo(HandshakeError)
+  let h = idNonceHash(head.idNonce, head.ephemeralKey)
+  if verify(sig, h, newNode.pubkey):
+    ok(secrets)
+  else:
+    err(HandshakeError)
 
 proc decodePacket*(c: var Codec,
                       fromId: NodeID,
@@ -287,10 +295,11 @@ proc decodePacket*(c: var Codec,
       trace "Decoding failed (different nonce)"
       return err(HandshakeError)
 
-    var sec: HandshakeSecrets
-    if c.decodeAuthResp(fromId, auth, challenge, sec, newNode).isErr:
-      trace "Decoding failed (bad auth)"
+    let secrets = c.decodeAuthResp(fromId, auth, challenge, newNode)
+    if secrets.isErr:
+      trace "Decoding failed (invalid auth response)"
       return err(HandshakeError)
+    var sec = secrets[]
 
     c.handshakes.del(key)
 

tests/p2p/test_discv5_encoding.nim

@@ -1,6 +1,6 @@
 import
-  unittest, options, sequtils, stew/byteutils, stint,
-  eth/[rlp, keys] , eth/p2p/discoveryv5/[types, encoding, enr]
+  unittest, options, sequtils, net, stew/byteutils, stint,
+  eth/[rlp, keys] , eth/p2p/discoveryv5/[types, encoding, enr, node]
 
 # According to test vectors:
 # https://github.com/ethereum/devp2p/blob/master/discv5/discv5-wire-test-vectors.md
@@ -229,3 +229,28 @@ suite "Discovery v5 Additional":
     # invalid tag/data itself
     invalidCipher = repeat(byte(4), gcmTagSize + 1)
     check decryptGCM(encryptionKey, nonce, invalidCipher, ad).isNone()
+
+  test "AuthHeader encode/decode":
+    let
+      privKey = PrivateKey.random()[]
+      enrRec = enr.Record.init(1, privKey, none(IpAddress), Port(9000),
+        Port(9000)).expect("Properly intialized private key")
+      node = newNode(enrRec).expect("Properly initialized record")
+      nonce = hexToByteArray[authTagSize]("0x27b5af763c446acd2749fe8e")
+      pubKey = PrivateKey.random()[].toPublicKey()[]
+      nodeId = pubKey.toNodeId()
+      idNonce = hexToByteArray[idNonceSize](
+        "0xa77e3aa0c144ae7c0a3af73692b7d6e5b7a2fdc0eda16e8d5e6cb0d08e88dd04")
+      whoareyou = Whoareyou(idNonce: idNonce, recordSeq: 0, pubKey: pubKey)
+      c = Codec(localNode: node, privKey: privKey)
+
+    let (auth, _) = c.encodeAuthHeader(nodeId, nonce, whoareyou)[]
+    var rlp = rlpFromBytes(auth)
+    let authHeader = rlp.read(AuthHeader)
+    var newNode: Node
+    let secrets = c.decodeAuthResp(privKey.toPublicKey()[].toNodeId(),
+      authHeader, whoareyou, newNode)
+
+    # TODO: Test cases with invalid nodeId and invalid signature, the latter
+    # is in the current code structure rather difficult and would need some
+    # helper proc.