Merge pull request #202 from status-im/discv5-bunch

Add IP checks on ENRs + allow for node creation without IP in ENR
This commit is contained in:
Kim De Mey 2020-03-31 11:09:02 +02:00 committed by GitHub
commit fe6df94a19
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 81 additions and 38 deletions

View File

@ -101,19 +101,23 @@ macro initRecord*(seqNum: uint64, pk: PrivateKey, pairs: untyped{nkTableConstr})
proc init*(T: type Record, seqNum: uint64,
pk: PrivateKey,
address: Option[enode.Address]): T =
if address.isSome():
ip: Option[IpAddress],
tcpPort, udpPort: Port): T =
if ip.isSome():
let
a = address.get()
isV6 = a.ip.family == IPv6
ipField = if isV6: ("ip6", a.ip.address_v6.toField)
else: ("ip", a.ip.address_v4.toField)
tcpField = ((if isV6: "tcp6" else: "tcp"), a.tcpPort.uint16.toField)
udpField = ((if isV6: "udp6" else: "udp"), a.udpPort.uint16.toField)
ipExt = ip.get()
isV6 = ipExt.family == IPv6
ipField = if isV6: ("ip6", ipExt.address_v6.toField)
else: ("ip", ipExt.address_v4.toField)
tcpField = ((if isV6: "tcp6" else: "tcp"), tcpPort.uint16.toField)
udpField = ((if isV6: "udp6" else: "udp"), udpPort.uint16.toField)
makeEnrAux(seqNum, pk, [ipField, tcpField, udpField])
else:
makeEnrAux(seqNum, pk, [])
let
tcpField = ("tcp", tcpPort.uint16.toField)
udpField = ("udp", udpPort.uint16.toField)
makeEnrAux(seqNum, pk, [tcpField, udpField])
proc getField(r: Record, name: string, field: var Field): bool =
# It might be more correct to do binary search,

View File

@ -38,6 +38,9 @@ proc newNode*(r: Record): Node =
address_v4: ipBytes),
udpPort: Port udpPort)
except KeyError:
# 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
var pk: PublicKey

View File

@ -6,6 +6,8 @@ import
import nimcrypto except toHex
export options
logScope:
topics = "discv5"
@ -216,8 +218,11 @@ proc receive*(d: Protocol, a: Address, msg: Bytes) {.gcsafe,
if node.isNil:
node = d.routingTable.getNode(sender)
else:
debug "Adding new node to routing table", node = $node, localNode = $d.localNode
discard d.routingTable.addNode(node)
# Not filling table with nodes without correct IP in the ENR
if a.ip == node.address.ip:
debug "Adding new node to routing table", node = $node,
localNode = $d.localNode
discard d.routingTable.addNode(node)
case packet.kind
of ping:
@ -239,8 +244,10 @@ proc receive*(d: Protocol, a: Address, msg: Bytes) {.gcsafe,
# 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)
if a.ip == node.address.ip:
debug "Adding new node to routing table", node = $node,
localNode = $d.localNode
discard d.routingTable.addNode(node)
proc processClient(transp: DatagramTransport,
raddr: TransportAddress): Future[void] {.async, gcsafe.} =
@ -259,6 +266,23 @@ proc processClient(transp: DatagramTransport,
debug "Receive failed", exception = e.name, msg = e.msg,
stacktrace = e.getStackTrace()
proc validIp(sender, address: IpAddress): bool =
let
s = initTAddress(sender, Port(0))
a = initTAddress(address, Port(0))
if a.isAnyLocal():
return false
if a.isMulticast():
return false
if a.isLoopback() and not s.isLoopback():
return false
if a.isSiteLocal() and not s.isSiteLocal():
return false
# TODO: Also check for special reserved ip addresses:
# https://www.iana.org/assignments/iana-ipv4-special-registry/iana-ipv4-special-registry.xhtml
# https://www.iana.org/assignments/iana-ipv6-special-registry/iana-ipv6-special-registry.xhtml
return true
# 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
@ -323,7 +347,11 @@ proc sendFindNode(d: Protocol, toNode: Node, distance: uint32): RequestId =
proc findNode*(d: Protocol, toNode: Node, distance: uint32): Future[seq[Node]] {.async.} =
let reqId = sendFindNode(d, toNode, distance)
result = await d.waitNodes(toNode, reqId)
let nodes = await d.waitNodes(toNode, reqId)
for n in nodes:
if validIp(toNode.address.ip, n.address.ip):
result.add(n)
proc lookupDistances(target, dest: NodeId): seq[uint32] =
let td = logDist(target, dest)
@ -416,6 +444,8 @@ proc revalidateNode*(d: Protocol, n: Node)
# would be to simply not remove the nodes immediatly but only after x
# amount of failures.
discard d.codec.db.deleteKeys(n.id, n.address)
else:
debug "Revalidation of bootstrap node failed", enr = toURI(n.record)
proc revalidateLoop(d: Protocol) {.async.} =
try:
@ -447,12 +477,13 @@ proc lookupLoop(d: Protocol) {.async.} =
trace "lookupLoop canceled"
proc newProtocol*(privKey: PrivateKey, db: Database,
ip: IpAddress, tcpPort, udpPort: Port,
externalIp: Option[IpAddress], tcpPort, udpPort: Port,
bootstrapRecords: openarray[Record] = []): Protocol =
let
a = Address(ip: ip, tcpPort: tcpPort, udpPort: udpPort)
a = Address(ip: externalIp.get(IPv4_any()),
tcpPort: tcpPort, udpPort: udpPort)
enode = initENode(privKey.getPublicKey(), a)
enrRec = enr.Record.init(12, privKey, some(a))
enrRec = enr.Record.init(1, privKey, externalIp, tcpPort, udpPort)
node = newNode(enode, enrRec)
result = Protocol(

View File

@ -1,16 +1,16 @@
import
random, unittest, chronos, sequtils, chronicles, tables, stint, options,
unittest, chronos, sequtils, chronicles, tables, stint,
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,
./p2p_test_helper
proc initDiscoveryNode*(privKey: PrivateKey, address: Address,
bootstrapRecords: seq[Record]):
bootstrapRecords: openarray[Record] = []):
discv5_protocol.Protocol =
var db = DiscoveryDB.init(newMemoryDB())
result = newProtocol(privKey, db,
parseIpAddress("127.0.0.1"),
some(parseIpAddress("127.0.0.1")),
address.tcpPort, address.udpPort,
bootstrapRecords)
@ -32,15 +32,17 @@ proc randomPacket(tag: PacketTag): seq[byte] =
result.add(rlp.encode(authTag))
result.add(msg)
proc generateNode(privKey = newPrivateKey()): Node =
let enr = enr.Record.init(1, privKey, none(Address))
proc generateNode(privKey = newPrivateKey(), port: int): Node =
let port = Port(port)
let enr = enr.Record.init(1, privKey, some(parseIpAddress("127.0.0.1")),
port, port)
result = newNode(enr)
suite "Discovery v5 Tests":
asyncTest "Random nodes":
let
bootNodeKey = initPrivateKey("a2b50376a79b1a8c8a3296485572bdfbf54708bb46d3c25d73d2723aaaf6a617")
bootNode = initDiscoveryNode(bootNodeKey, localAddress(20301), @[])
bootNode = initDiscoveryNode(bootNodeKey, localAddress(20301))
let nodeKeys = [
initPrivateKey("a2b50376a79b1a8c8a3296485572bdfbf54708bb46d3c25d73d2723aaaf6a618"),
@ -72,7 +74,7 @@ suite "Discovery v5 Tests":
const
nodeCount = 17
let bootNode = initDiscoveryNode(newPrivateKey(), localAddress(20301), @[])
let bootNode = initDiscoveryNode(newPrivateKey(), localAddress(20301))
bootNode.start()
var nodes = newSeqOfCap[discv5_protocol.Protocol](nodeCount)
@ -94,11 +96,11 @@ suite "Discovery v5 Tests":
asyncTest "FindNode with test table":
let mainNode = initDiscoveryNode(newPrivateKey(), localAddress(20301), @[])
let mainNode = initDiscoveryNode(newPrivateKey(), localAddress(20301))
# Generate 1000 random nodes and add to our main node's routing table
for i in 0..<1000:
mainNode.addNode(generateNode())
mainNode.addNode(generateNode(port = 20302 + i))
let
neighbours = mainNode.neighbours(mainNode.localNode.id)
@ -121,13 +123,13 @@ suite "Discovery v5 Tests":
asyncTest "GetNode":
# TODO: This could be tested in just a routing table only context
let
node = initDiscoveryNode(newPrivateKey(), localAddress(20302), @[])
targetNode = generateNode()
node = initDiscoveryNode(newPrivateKey(), localAddress(20302))
targetNode = generateNode(port = 20303)
node.addNode(targetNode)
for i in 0..<1000:
node.addNode(generateNode())
node.addNode(generateNode(port = 20303 + i))
check node.getNode(targetNode.id) == targetNode
@ -135,7 +137,7 @@ suite "Discovery v5 Tests":
asyncTest "Node deletion":
let
bootnode = initDiscoveryNode(newPrivateKey(), localAddress(20301), @[])
bootnode = initDiscoveryNode(newPrivateKey(), localAddress(20301))
node1 = initDiscoveryNode(newPrivateKey(), localAddress(20302),
@[bootnode.localNode.record])
node2 = initDiscoveryNode(newPrivateKey(), localAddress(20303),
@ -158,7 +160,7 @@ suite "Discovery v5 Tests":
asyncTest "Handshake cleanup":
let node = initDiscoveryNode(newPrivateKey(), localAddress(20302), @[])
let node = initDiscoveryNode(newPrivateKey(), localAddress(20302))
var tag: PacketTag
let a = localAddress(20303)
@ -177,7 +179,7 @@ suite "Discovery v5 Tests":
await node.closeWait()
asyncTest "Handshake different address":
let node = initDiscoveryNode(newPrivateKey(), localAddress(20302), @[])
let node = initDiscoveryNode(newPrivateKey(), localAddress(20302))
var tag: PacketTag
for i in 0 ..< 5:
@ -189,7 +191,7 @@ suite "Discovery v5 Tests":
await node.closeWait()
asyncTest "Handshake duplicates":
let node = initDiscoveryNode(newPrivateKey(), localAddress(20302), @[])
let node = initDiscoveryNode(newPrivateKey(), localAddress(20302))
var tag: PacketTag
let a = localAddress(20303)

View File

@ -34,8 +34,7 @@ suite "ENR":
let
keys = newKeyPair()
ip = parseIpAddress("10.20.30.40")
enodeAddress = Address(ip: ip, tcpPort: Port 9000, udpPort: Port 9000)
enr = Record.init(100, keys.seckey, some(enodeAddress))
enr = Record.init(100, keys.seckey, some(ip), Port(9000), Port(9000))
typedEnr = get enr.toTypedRecord()
check:
@ -54,7 +53,7 @@ suite "ENR":
test "ENR without address":
let
keys = newKeyPair()
enr = Record.init(100, keys.seckey, none(Address))
enr = Record.init(100, keys.seckey, none(IpAddress), Port(9000), Port(9000))
typedEnr = get enr.toTypedRecord()
check:
@ -62,8 +61,12 @@ suite "ENR":
typedEnr.secp256k1.get() == keys.pubkey.getRawCompressed()
typedEnr.ip.isNone()
typedEnr.tcp.isNone()
typedEnr.udp.isNone()
typedEnr.tcp.isSome()
typedEnr.tcp.get() == 9000
typedEnr.udp.isSome()
typedEnr.udp.get() == 9000
typedEnr.ip6.isNone()
typedEnr.tcp6.isNone()
typedEnr.udp6.isNone()