From 651047194eb8757fdbca0b239b14434db258f118 Mon Sep 17 00:00:00 2001 From: Andrei Maiboroda Date: Mon, 15 Apr 2019 17:54:38 +0200 Subject: [PATCH 01/10] Support ENRRequest / ENRResponse discovery packets --- libp2p/Host.cpp | 8 +-- libp2p/NodeTable.cpp | 90 +++++++++++++++++++++++-- libp2p/NodeTable.h | 113 +++++++++++++++++++++++++++----- test/unittests/libp2p/eip-8.cpp | 10 +-- test/unittests/libp2p/net.cpp | 8 ++- 5 files changed, 193 insertions(+), 36 deletions(-) diff --git a/libp2p/Host.cpp b/libp2p/Host.cpp index b1484387a83..f4ec51abe9e 100644 --- a/libp2p/Host.cpp +++ b/libp2p/Host.cpp @@ -800,13 +800,9 @@ void Host::startedWorking() m_listenPort = m_netConfig.listenPort; determinePublic(); - auto nodeTable = make_shared( - m_ioService, - m_alias, + auto nodeTable = make_shared(m_ioService, m_alias, NodeIPEndpoint(bi::address::from_string(listenAddress()), listenPort(), listenPort()), - m_netConfig.discovery, - m_netConfig.allowLocalDiscovery - ); + m_enr, m_netConfig.discovery, m_netConfig.allowLocalDiscovery); // Don't set an event handler if we don't have capabilities, because no capabilities // means there's no host state to update in response to node table events diff --git a/libp2p/NodeTable.cpp b/libp2p/NodeTable.cpp index 633225467cb..299eaa6e8b2 100644 --- a/libp2p/NodeTable.cpp +++ b/libp2p/NodeTable.cpp @@ -32,9 +32,10 @@ inline bool operator==(weak_ptr const& _weak, shared_ptr c } NodeTable::NodeTable(ba::io_service& _io, KeyPair const& _alias, NodeIPEndpoint const& _endpoint, - bool _enabled, bool _allowLocalDiscovery) + ENR const& _enr, bool _enabled, bool _allowLocalDiscovery) : m_hostNodeID{_alias.pub()}, m_hostNodeEndpoint{_endpoint}, + m_hostENR{_enr}, m_secret{_alias.secret()}, m_socket{make_shared( _io, static_cast(*this), (bi::udp::endpoint)m_hostNodeEndpoint)}, @@ -220,7 +221,7 @@ void NodeTable::doDiscoveryRound( } FindNode p(nodeEntry->endpoint(), _node); - p.ts = nextRequestExpirationTime(); + p.timestamp = nextRequestExpirationTime(); p.sign(m_secret); m_sentFindNodes.emplace_back(nodeEntry->id(), chrono::steady_clock::now()); LOG(m_logger) << p.typeName() << " to " << nodeEntry->node << " (target: " << _node @@ -302,7 +303,8 @@ void NodeTable::ping(Node const& _node, shared_ptr _replacementNodeEn } PingNode p{m_hostNodeEndpoint, _node.endpoint}; - p.ts = nextRequestExpirationTime(); + p.timestamp = nextRequestExpirationTime(); + p.seq = m_hostENR.sequenceNumber(); auto const pingHash = p.sign(m_secret); LOG(m_logger) << p.typeName() << " to " << _node; m_socket->send(p); @@ -454,6 +456,14 @@ void NodeTable::onPacketReceived( case PingNode::type: sourceNodeEntry = handlePingNode(_from, *packet); break; + + case ENRRequest::type: + sourceNodeEntry = handleENRRequest(_from, *packet); + break; + + case ENRResponse::type: + sourceNodeEntry = handleENRResponse(_from, *packet); + break; } if (sourceNodeEntry) @@ -608,7 +618,7 @@ std::shared_ptr NodeTable::handleFindNode( for (unsigned offset = 0; offset < nearest.size(); offset += nlimit) { Neighbours out(_from, nearest, offset, nlimit); - out.ts = nextRequestExpirationTime(); + out.timestamp = nextRequestExpirationTime(); LOG(m_logger) << out.typeName() << " to " << in.sourceid << "@" << _from; out.sign(m_secret); if (out.data.size() > 1280) @@ -631,8 +641,9 @@ std::shared_ptr NodeTable::handlePingNode( // Send PONG response. Pong p(sourceEndpoint); LOG(m_logger) << p.typeName() << " to " << in.sourceid << "@" << _from; - p.ts = nextRequestExpirationTime(); + p.timestamp = nextRequestExpirationTime(); p.echo = in.echo; + p.seq = m_hostENR.sequenceNumber(); p.sign(m_secret); m_socket->send(p); @@ -647,6 +658,69 @@ std::shared_ptr NodeTable::handlePingNode( return sourceNodeEntry; } +std::shared_ptr NodeTable::handleENRRequest( + bi::udp::endpoint const& _from, DiscoveryDatagram const& _packet) +{ + std::shared_ptr sourceNodeEntry = nodeEntry(_packet.sourceid); + if (!sourceNodeEntry) + { + LOG(m_logger) << "Source node (" << _packet.sourceid << "@" << _from + << ") not found in node table. Ignoring ENRRequest request."; + return {}; + } + if (sourceNodeEntry->endpoint() != _from) + { + LOG(m_logger) << "ENRRequest packet from unexpected endpoint " << _from << " instead of " + << sourceNodeEntry->endpoint(); + return {}; + } + if (!sourceNodeEntry->lastPongReceivedTime) + { + LOG(m_logger) << "Unexpected ENRRequest packet! Endpoint proof hasn't been performed yet."; + return {}; + } + if (!sourceNodeEntry->hasValidEndpointProof()) + { + LOG(m_logger) << "Unexpected ENRRequest packet! Endpoint proof has expired."; + return {}; + } + + auto const& in = dynamic_cast(_packet); + + ENRResponse response(_from, m_hostENR); + LOG(m_logger) << response.typeName() << " to " << in.sourceid << "@" << _from; + response.timestamp = nextRequestExpirationTime(); + response.echo = in.echo; + response.sign(m_secret); + m_socket->send(response); + + return sourceNodeEntry; +} + +std::shared_ptr NodeTable::handleENRResponse( + bi::udp::endpoint const& _from, DiscoveryDatagram const& _packet) +{ + std::shared_ptr sourceNodeEntry = nodeEntry(_packet.sourceid); + if (!sourceNodeEntry) + { + LOG(m_logger) << "Source node (" << _packet.sourceid << "@" << _from + << ") not found in node table. Ignoring Neighbours packet."; + return {}; + } + if (sourceNodeEntry->endpoint() != _from) + { + LOG(m_logger) << "ENRResponse packet from unexpected endpoint " << _from << " instead of " + << sourceNodeEntry->endpoint(); + return {}; + } + + auto const& in = dynamic_cast(_packet); + LOG(m_logger) << "Received ENR: " << *in.enr; + + return sourceNodeEntry; +} + + void NodeTable::doDiscovery() { m_discoveryTimer->expires_from_now(c_bucketRefreshMs); @@ -766,6 +840,12 @@ unique_ptr DiscoveryDatagram::interpretUDP(bi::udp::endpoint case Neighbours::type: decoded.reset(new Neighbours(_from, sourceid, echo)); break; + case ENRRequest::type: + decoded.reset(new ENRRequest(_from, sourceid, echo)); + break; + case ENRResponse::type: + decoded.reset(new ENRResponse(_from, sourceid, echo)); + break; default: LOG(g_discoveryWarnLogger::get()) << "Invalid packet (unknown packet type) from " << _from.address().to_string() << ":" << _from.port(); diff --git a/libp2p/NodeTable.h b/libp2p/NodeTable.h index 8fbf8ebbda4..d9369afc36f 100644 --- a/libp2p/NodeTable.h +++ b/libp2p/NodeTable.h @@ -8,8 +8,9 @@ #include -#include #include "Common.h" +#include "ENR.h" +#include namespace dev { @@ -109,7 +110,7 @@ class NodeTable : UDPSocketEvents /// Constructor requiring host for I/O, credentials, and IP Address and port to listen on. NodeTable(ba::io_service& _io, KeyPair const& _alias, NodeIPEndpoint const& _endpoint, - bool _enabled = true, bool _allowLocalDiscovery = false); + ENR const& _enr, bool _enabled = true, bool _allowLocalDiscovery = false); /// Returns distance based on xor metric two node ids. Used by NodeEntry and NodeTable. static int distance(NodeID const& _a, NodeID const& _b) { u256 d = sha3(_a) ^ sha3(_b); unsigned ret; for (ret = 0; d >>= 1; ++ret) {}; return ret; } @@ -286,6 +287,10 @@ class NodeTable : UDPSocketEvents bi::udp::endpoint const& _from, DiscoveryDatagram const& _packet); std::shared_ptr handlePingNode( bi::udp::endpoint const& _from, DiscoveryDatagram const& _packet); + std::shared_ptr handleENRRequest( + bi::udp::endpoint const& _from, DiscoveryDatagram const& _packet); + std::shared_ptr handleENRResponse( + bi::udp::endpoint const& _from, DiscoveryDatagram const& _packet); /// Called by m_socket when socket is disconnected. void onSocketDisconnected(UDPSocketFace*) override {} @@ -316,6 +321,7 @@ class NodeTable : UDPSocketEvents NodeID const m_hostNodeID; NodeIPEndpoint m_hostNodeEndpoint; + ENR const m_hostENR; Secret m_secret; ///< This nodes secret key. mutable Mutex x_nodes; ///< LOCK x_state first if both locks are required. Mutable for thread-safe copy in nodes() const. @@ -394,7 +400,7 @@ struct DiscoveryDatagram: public RLPXDatagramFace /// Constructor used for sending. DiscoveryDatagram(bi::udp::endpoint const& _to) - : RLPXDatagramFace(_to), ts(futureFromEpoch(c_timeToLiveS)) + : RLPXDatagramFace(_to), timestamp(futureFromEpoch(c_timeToLiveS)) {} /// Constructor used for parsing inbound packets. @@ -404,10 +410,13 @@ struct DiscoveryDatagram: public RLPXDatagramFace NodeID sourceid; // sender public key (from signature) h256 echo; // hash of encoded packet, for reply tracking - // All discovery packets carry a timestamp, which must be greater + // Most discovery packets carry a timestamp, which must be greater // than the current local time. This prevents replay attacks. - uint32_t ts = 0; - bool isExpired() const { return secondsSinceEpoch() > ts; } + boost::optional timestamp; + bool isExpired() const + { + return timestamp.is_initialized() && secondsSinceEpoch() > *timestamp; + } /// Decodes UDP packets. static std::unique_ptr interpretUDP(bi::udp::endpoint const& _from, bytesConstRef _packet); @@ -415,7 +424,7 @@ struct DiscoveryDatagram: public RLPXDatagramFace /** * Ping packet: Sent to check if node is alive. - * PingNode is cached and regenerated after ts + t, where t is timeout. + * PingNode is cached and regenerated after timestamp + t, where t is timeout. * * Ping is used to implement evict. When a new node is seen for * a given bucket which is full, the least-responsive node is pinged. @@ -434,14 +443,17 @@ struct PingNode: DiscoveryDatagram unsigned version = 0; NodeIPEndpoint source; NodeIPEndpoint destination; + boost::optional seq; void streamRLP(RLPStream& _s) const override { - _s.appendList(4); + _s.appendList(seq.is_initialized() ? 5 : 4); _s << dev::p2p::c_protocolVersion; source.streamRLP(_s); destination.streamRLP(_s); - _s << ts; + _s << *timestamp; + if (seq.is_initialized()) + _s << *seq; } void interpretRLP(bytesConstRef _bytes) override { @@ -449,7 +461,9 @@ struct PingNode: DiscoveryDatagram version = r[0].toInt(); source.interpretRLP(r[1]); destination.interpretRLP(r[2]); - ts = r[3].toInt(); + timestamp = r[3].toInt(); + if (r.itemCount() > 4) + seq = r[4].toInt(); } std::string typeName() const override { return "Ping"; } @@ -467,20 +481,25 @@ struct Pong: DiscoveryDatagram uint8_t packetType() const override { return type; } NodeIPEndpoint destination; + boost::optional seq; void streamRLP(RLPStream& _s) const override { - _s.appendList(3); + _s.appendList(seq.is_initialized() ? 4 : 3); destination.streamRLP(_s); _s << echo; - _s << ts; + _s << *timestamp; + if (seq.is_initialized()) + _s << *seq; } void interpretRLP(bytesConstRef _bytes) override { RLP r(_bytes, RLP::AllowNonCanon|RLP::ThrowOnFail); destination.interpretRLP(r[0]); echo = (h256)r[1]; - ts = r[2].toInt(); + timestamp = r[2].toInt(); + if (r.itemCount() > 3) + seq = r[3].toInt(); } std::string typeName() const override { return "Pong"; } @@ -488,7 +507,7 @@ struct Pong: DiscoveryDatagram /** * FindNode Packet: Request k-nodes, closest to the target. - * FindNode is cached and regenerated after ts + t, where t is timeout. + * FindNode is cached and regenerated after timestamp + t, where t is timeout. * FindNode implicitly results in finding neighbours of a given node. * * RLP Encoded Items: 2 @@ -510,13 +529,14 @@ struct FindNode: DiscoveryDatagram void streamRLP(RLPStream& _s) const override { - _s.appendList(2); _s << target << ts; + _s.appendList(2); + _s << target << *timestamp; } void interpretRLP(bytesConstRef _bytes) override { RLP r(_bytes, RLP::AllowNonCanon|RLP::ThrowOnFail); target = r[0].toHash(); - ts = r[1].toInt(); + timestamp = r[1].toInt(); } std::string typeName() const override { return "FindNode"; } @@ -556,18 +576,75 @@ struct Neighbours: DiscoveryDatagram _s.appendList(neighbours.size()); for (auto const& n: neighbours) n.streamRLP(_s); - _s << ts; + _s << *timestamp; } void interpretRLP(bytesConstRef _bytes) override { RLP r(_bytes, RLP::AllowNonCanon|RLP::ThrowOnFail); for (auto const& n: r[0]) neighbours.emplace_back(n); - ts = r[1].toInt(); + timestamp = r[1].toInt(); } std::string typeName() const override { return "Neighbours"; } }; +struct ENRRequest : DiscoveryDatagram +{ + // Constructor for outgoing packets + ENRRequest(bi::udp::endpoint _to) : DiscoveryDatagram{_to} {} + // Constructor for incoming packets + ENRRequest(bi::udp::endpoint const& _from, NodeID const& _fromid, h256 const& _echo) + : DiscoveryDatagram{_from, _fromid, _echo} + {} + + static uint8_t const type = 5; + uint8_t packetType() const override { return type; } + + void streamRLP(RLPStream& _s) const override + { + _s.appendList(1); + _s << *timestamp; + } + void interpretRLP(bytesConstRef _bytes) override + { + RLP r(_bytes, RLP::AllowNonCanon | RLP::ThrowOnFail); + timestamp = r[0].toInt(); + } + + std::string typeName() const override { return "ENRRequest"; } +}; + +struct ENRResponse : DiscoveryDatagram +{ + // Constructor for outgoing packets + ENRResponse(bi::udp::endpoint const& _dest, ENR const& _enr) + : DiscoveryDatagram{_dest}, enr{_enr} + {} + // Constructor for incoming packets + ENRResponse(bi::udp::endpoint const& _from, NodeID const& _fromid, h256 const& _echo) + : DiscoveryDatagram{_from, _fromid, _echo} + {} + + static uint8_t const type = 6; + uint8_t packetType() const override { return type; } + + boost::optional enr; + + void streamRLP(RLPStream& _s) const override + { + _s.appendList(2); + _s << echo; + enr->streamRLP(_s); + } + void interpretRLP(bytesConstRef _bytes) override + { + RLP r(_bytes, RLP::ThrowOnFail); + echo = (h256)r[0]; + enr = parseV4ENR(r[1]); + } + + std::string typeName() const override { return "ENRResponse"; } +}; } } diff --git a/test/unittests/libp2p/eip-8.cpp b/test/unittests/libp2p/eip-8.cpp index 8970764f32c..5bb9f45ece8 100644 --- a/test/unittests/libp2p/eip-8.cpp +++ b/test/unittests/libp2p/eip-8.cpp @@ -28,7 +28,7 @@ TEST(eip8, test_discovery_packets) auto ping1 = dynamic_cast(*DiscoveryDatagram::interpretUDP(ep, bytesConstRef(&packet))); EXPECT_EQ(ping1.version, 4); - EXPECT_EQ(ping1.ts, 1136239445); + EXPECT_EQ(*ping1.timestamp, 1136239445); EXPECT_EQ(ping1.source, NodeIPEndpoint(bi::address::from_string("127.0.0.1"), 3322, 5544)); EXPECT_EQ(ping1.destination, NodeIPEndpoint(bi::address::from_string("::1"), 2222, 3333)); @@ -49,7 +49,7 @@ TEST(eip8, test_discovery_packets) EXPECT_EQ(ping2.destination, NodeIPEndpoint( bi::address::from_string("2001:db8:85a3:8d3:1319:8a2e:370:7348"), 2222, 33338)); - EXPECT_EQ(ping2.ts, 1136239445); + EXPECT_EQ(*ping2.timestamp, 1136239445); packet = fromHex( "09b2428d83348d27cdf7064ad9024f526cebc19e4958f0fdad87c15eb598dd61d08423e0bf66b206" @@ -63,7 +63,7 @@ TEST(eip8, test_discovery_packets) NodeIPEndpoint( bi::address::from_string("2001:db8:85a3:8d3:1319:8a2e:370:7348"), 2222, 33338)); EXPECT_EQ(pong.echo, h256("fbc914b16819237dcd8801d7e53f69e9719adecb3cc0e790c57e91ca4461c954")); - EXPECT_EQ(pong.ts, 1136239445); + EXPECT_EQ(*pong.timestamp, 1136239445); packet = fromHex( "c7c44041b9f7c7e41934417ebac9a8e1a4c6298f74553f2fcfdcae6ed6fe53163eb3d2b52e39fe91" @@ -77,7 +77,7 @@ TEST(eip8, test_discovery_packets) EXPECT_EQ( findnode.target, Public("ca634cae0d49acb401d8a4c6b6fe8c55b70d115bf400769cc1400f3258cd313875" "74077f301b421bc84df7266c44e9e6d569fc56be00812904767bf5ccd1fc7f")); - EXPECT_EQ(findnode.ts, 1136239445); + EXPECT_EQ(*findnode.timestamp, 1136239445); packet = fromHex( "c679fc8fe0b8b12f06577f2e802d34f6fa257e6137a995f6f4cbfc9ee50ed3710faf6e66f932c4c8" @@ -116,7 +116,7 @@ TEST(eip8, test_discovery_packets) EXPECT_EQ(neighbours.neighbours[3].node, Public("8dcab8618c3253b558d459da53bd8fa68935a719aff8b811197101a4b2b47dd2d47295286fc00cc081b" "b542d760717d1bdd6bec2c37cd72eca367d6dd3b9df73")); - EXPECT_EQ(neighbours.ts, 1136239445); + EXPECT_EQ(*neighbours.timestamp, 1136239445); } class TestHandshake : public RLPXHandshake diff --git a/test/unittests/libp2p/net.cpp b/test/unittests/libp2p/net.cpp index 0ed9c316c49..234ba82b37d 100644 --- a/test/unittests/libp2p/net.cpp +++ b/test/unittests/libp2p/net.cpp @@ -47,7 +47,8 @@ struct TestNodeTable: public NodeTable /// Constructor TestNodeTable( ba::io_service& _io, KeyPair _alias, bi::address const& _addr, uint16_t _port = 30311) - : NodeTable(_io, _alias, NodeIPEndpoint(_addr, _port, _port), true /* discovery enabled */, + : NodeTable(_io, _alias, NodeIPEndpoint(_addr, _port, _port), + createV4ENR(_alias.secret(), _addr, _port, _port), true /* discovery enabled */, true /* allow local discovery */) {} @@ -1421,7 +1422,10 @@ BOOST_AUTO_TEST_CASE(nodeTableReturnsUnspecifiedNode) { ba::io_service io; auto const port = randomPortNumber(); - NodeTable t(io, KeyPair::create(), NodeIPEndpoint(bi::address::from_string(c_localhostIp), port, port)); + auto const keyPair = KeyPair::create(); + auto const addr = bi::address::from_string(c_localhostIp); + NodeTable t(io, keyPair, NodeIPEndpoint(addr, port, port), + createV4ENR(keyPair.secret(), addr, port, port)); if (Node n = t.node(NodeID())) BOOST_REQUIRE(false); } From 628afb5f31f9b4e33922dac6041778b2a76db9dd Mon Sep 17 00:00:00 2001 From: Andrei Maiboroda Date: Tue, 16 Apr 2019 13:21:42 +0200 Subject: [PATCH 02/10] Fix failing EIP-8 tests The reason is the test vectors there contain RLP-list in place of additional field in Ping and Pong packets, and the parsing code for ENR extension expects an integer sequence number there. --- libp2p/NodeTable.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libp2p/NodeTable.h b/libp2p/NodeTable.h index d9369afc36f..5f528bdaa11 100644 --- a/libp2p/NodeTable.h +++ b/libp2p/NodeTable.h @@ -462,7 +462,7 @@ struct PingNode: DiscoveryDatagram source.interpretRLP(r[1]); destination.interpretRLP(r[2]); timestamp = r[3].toInt(); - if (r.itemCount() > 4) + if (r.itemCount() > 4 && r[4].isInt()) seq = r[4].toInt(); } @@ -498,7 +498,7 @@ struct Pong: DiscoveryDatagram destination.interpretRLP(r[0]); echo = (h256)r[1]; timestamp = r[2].toInt(); - if (r.itemCount() > 3) + if (r.itemCount() > 3 && r[3].isInt()) seq = r[3].toInt(); } @@ -639,7 +639,7 @@ struct ENRResponse : DiscoveryDatagram } void interpretRLP(bytesConstRef _bytes) override { - RLP r(_bytes, RLP::ThrowOnFail); + RLP r(_bytes, RLP::AllowNonCanon | RLP::ThrowOnFail); echo = (h256)r[0]; enr = parseV4ENR(r[1]); } From 62c00d194a3a374d8b94ebc4d09fcd4275dd975d Mon Sep 17 00:00:00 2001 From: Andrei Maiboroda Date: Tue, 16 Apr 2019 17:52:37 +0200 Subject: [PATCH 03/10] Unit test for handling ENRRequest --- test/unittests/libp2p/net.cpp | 53 +++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/test/unittests/libp2p/net.cpp b/test/unittests/libp2p/net.cpp index 234ba82b37d..311ecf7037a 100644 --- a/test/unittests/libp2p/net.cpp +++ b/test/unittests/libp2p/net.cpp @@ -1236,6 +1236,59 @@ BOOST_AUTO_TEST_CASE(addNodePingsNodeOnlyOnce) BOOST_REQUIRE_EQUAL(sentPing->pingHash, sentPing2->pingHash); } +BOOST_AUTO_TEST_CASE(validENRRequest) +{ + // NodeTable receiving ENRRequest + TestNodeTableHost nodeTableHost(0); + nodeTableHost.start(); + auto& nodeTable = nodeTableHost.nodeTable; + + // socket sending ENRRequest + TestUDPSocketHost nodeSocketHost; + nodeSocketHost.start(); + uint16_t nodePort = nodeSocketHost.port; + + // Exchange Ping/Pongs before sending ENRRequest + + // add a node to node table, initiating PING + auto nodeEndpoint = NodeIPEndpoint{bi::address::from_string(c_localhostIp), nodePort, nodePort}; + auto nodeKeyPair = KeyPair::create(); + auto nodePubKey = nodeKeyPair.pub(); + nodeTable->addNode(Node{nodePubKey, nodeEndpoint}); + + // handle received PING + auto pingDataReceived = nodeSocketHost.packetsReceived.pop(); + auto pingDatagram = + DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(pingDataReceived)); + BOOST_REQUIRE_EQUAL(pingDatagram->typeName(), "Ping"); + auto ping = dynamic_cast(*pingDatagram); + + // send PONG + Pong pong(nodeTable->m_hostNodeEndpoint); + pong.echo = ping.echo; + pong.sign(nodeKeyPair.secret()); + nodeSocketHost.socket->send(pong); + + // wait for PONG to be received and handled + auto pongDataReceived = nodeTable->packetsReceived.pop(chrono::seconds(5)); + auto pongDatagram = + DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(pongDataReceived)); + BOOST_REQUIRE_EQUAL(pongDatagram->typeName(), "Pong"); + + // send ENRRequest + ENRRequest enrRequest(nodeTable->m_hostNodeEndpoint); + enrRequest.sign(nodeKeyPair.secret()); + nodeSocketHost.socket->send(enrRequest); + + // wait for ENRRequest to be received and handled + nodeTable->packetsReceived.pop(chrono::seconds(5)); + + auto enrResponsePacket = nodeSocketHost.packetsReceived.pop(chrono::seconds(5)); + auto datagram = + DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(enrResponsePacket)); + BOOST_REQUIRE_EQUAL(datagram->typeName(), "ENRResponse"); +} + class PacketsWithChangedEndpointFixture : public TestOutputHelperFixture { public: From aecf6e4c596b1c1ad7f22886222be119842d1934 Mon Sep 17 00:00:00 2001 From: Andrei Maiboroda Date: Tue, 23 Apr 2019 13:33:49 +0200 Subject: [PATCH 04/10] Function to convert compressed public key to uncompressed --- aleth/main.cpp | 2 +- libdevcrypto/Common.cpp | 20 ++++++++++++++++++++ libdevcrypto/Common.h | 3 +++ test/unittests/libdevcrypto/crypto.cpp | 10 +++++++++- 4 files changed, 33 insertions(+), 2 deletions(-) diff --git a/aleth/main.cpp b/aleth/main.cpp index fc578d89814..08c30ff669c 100644 --- a/aleth/main.cpp +++ b/aleth/main.cpp @@ -110,7 +110,7 @@ int main(int argc, char** argv) setDefaultOrCLocale(); // Init secp256k1 context by calling one of the functions. - toPublic({}); + toPublic(Secret{}); // Init defaults Ethash::init(); diff --git a/libdevcrypto/Common.cpp b/libdevcrypto/Common.cpp index 962c0188d59..7f63efff8e6 100644 --- a/libdevcrypto/Common.cpp +++ b/libdevcrypto/Common.cpp @@ -89,6 +89,26 @@ Public dev::toPublic(Secret const& _secret) return Public{&serializedPubkey[1], Public::ConstructFromPointer}; } +Public dev::toPublic(PublicCompressed const& _publicCompressed) +{ + auto* ctx = getCtx(); + + secp256k1_pubkey rawPubkey; + if (!secp256k1_ec_pubkey_parse( + ctx, &rawPubkey, _publicCompressed.data(), PublicCompressed::size)) + return {}; + + std::array serializedPubkey; + size_t serializedPubkeySize = serializedPubkey.size(); + secp256k1_ec_pubkey_serialize( + ctx, serializedPubkey.data(), &serializedPubkeySize, &rawPubkey, SECP256K1_EC_UNCOMPRESSED); + assert(serializedPubkeySize == serializedPubkey.size()); + // Expect single byte header of value 0x04 -- uncompressed public key. + assert(serializedPubkey[0] == 0x04); + // Create the Public skipping the header. + return Public{&serializedPubkey[1], Public::ConstructFromPointer}; +} + PublicCompressed dev::toPublicCompressed(Secret const& _secret) { PublicCompressed serializedPubkey; diff --git a/libdevcrypto/Common.h b/libdevcrypto/Common.h index f7e0e63b344..6babb44632d 100644 --- a/libdevcrypto/Common.h +++ b/libdevcrypto/Common.h @@ -68,6 +68,9 @@ using Secrets = std::vector; /// Convert a secret key into the public key equivalent. Public toPublic(Secret const& _secret); +/// Convert a compressed public key into the uncompressed equivalent. +Public toPublic(PublicCompressed const& _publicCompressed); + /// Convert a secret key into the public key in compressed format. PublicCompressed toPublicCompressed(Secret const& _secret); diff --git a/test/unittests/libdevcrypto/crypto.cpp b/test/unittests/libdevcrypto/crypto.cpp index 8a7e2d990d4..d5cffecc451 100644 --- a/test/unittests/libdevcrypto/crypto.cpp +++ b/test/unittests/libdevcrypto/crypto.cpp @@ -93,7 +93,7 @@ BOOST_AUTO_TEST_CASE(emptySHA3Types) BOOST_AUTO_TEST_CASE(pubkeyOfZero) { - auto pub = toPublic({}); + auto pub = toPublic(Secret{}); BOOST_REQUIRE_EQUAL(pub, Public{}); } @@ -209,6 +209,14 @@ BOOST_AUTO_TEST_CASE(signAndVerify) BOOST_REQUIRE(verify(pubCompressed, sigWithoutV, msg)); } +BOOST_AUTO_TEST_CASE(compressedToUncompressed) +{ + auto const kp = KeyPair::create(); + auto const pubCompressed = toPublicCompressed(kp.secret()); + + BOOST_REQUIRE(toPublic(pubCompressed) == kp.pub()); +} + BOOST_AUTO_TEST_CASE(common_encrypt_decrypt) { string message("Now is the time for all good persons to come to the aid of humanity."); From 4bbe8543c8c28aac9877ad7fbe9e985724369525 Mon Sep 17 00:00:00 2001 From: Andrei Maiboroda Date: Tue, 23 Apr 2019 15:08:02 +0200 Subject: [PATCH 05/10] More checks in ENRResponse test --- test/unittests/libp2p/net.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/unittests/libp2p/net.cpp b/test/unittests/libp2p/net.cpp index 311ecf7037a..289dd802941 100644 --- a/test/unittests/libp2p/net.cpp +++ b/test/unittests/libp2p/net.cpp @@ -1287,6 +1287,18 @@ BOOST_AUTO_TEST_CASE(validENRRequest) auto datagram = DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(enrResponsePacket)); BOOST_REQUIRE_EQUAL(datagram->typeName(), "ENRResponse"); + + auto enrResponse = dynamic_cast(*datagram); + auto const& keyValuePairs = enrResponse.enr->keyValuePairs(); + BOOST_REQUIRE_EQUAL(RLP{keyValuePairs.at("id")}.toString(), "v4"); + PublicCompressed publicCompressed{RLP{keyValuePairs.at("secp256k1")}.toBytesConstRef()}; + BOOST_REQUIRE(toPublic(publicCompressed) == nodeTable->m_hostNodeID); + bytes const localhostBytes{127, 0, 0, 1}; + BOOST_REQUIRE(RLP{keyValuePairs.at("ip")}.toBytes() == localhostBytes); + BOOST_REQUIRE_EQUAL( + RLP{keyValuePairs.at("tcp")}.toInt(), nodeTable->m_hostNodeEndpoint.tcpPort()); + BOOST_REQUIRE_EQUAL( + RLP{keyValuePairs.at("udp")}.toInt(), nodeTable->m_hostNodeEndpoint.udpPort()); } class PacketsWithChangedEndpointFixture : public TestOutputHelperFixture From 92210a0d0194322696e4ce6c54fbe76327d7d985 Mon Sep 17 00:00:00 2001 From: Andrei Maiboroda Date: Wed, 24 Apr 2019 17:19:26 +0200 Subject: [PATCH 06/10] Add changelog item --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f61a4ca3979..af66fbb1801 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## [1.7.0] - Unreleased - Added: [#5537](https://github.com/ethereum/aleth/pull/5537) Creating Ethereum Node Record (ENR) at program start. +- Added: [#5571](https://github.com/ethereum/aleth/pull/5571) Support Discovery v4 ENR Extension messages. - Added: [#5557](https://github.com/ethereum/aleth/pull/5557) Improved debug logging of full sync. - Added: [#5564](https://github.com/ethereum/aleth/pull/5564) Improved help output of Aleth by adding list of channels. - Added: [#5575](https://github.com/ethereum/aleth/pull/5575) Log active peer count and peer list every 30 seconds. From dd22aad4d59c917026d39d83608d21f793d4a8f7 Mon Sep 17 00:00:00 2001 From: Andrei Maiboroda Date: Thu, 2 May 2019 16:13:13 +0200 Subject: [PATCH 07/10] Address issues from review --- libdevcrypto/Common.cpp | 2 +- libp2p/NodeTable.cpp | 4 ++-- libp2p/NodeTable.h | 27 ++++++++++++++------------- test/unittests/libp2p/net.cpp | 25 +++++++++++++------------ 4 files changed, 30 insertions(+), 28 deletions(-) diff --git a/libdevcrypto/Common.cpp b/libdevcrypto/Common.cpp index 7f63efff8e6..9f604753642 100644 --- a/libdevcrypto/Common.cpp +++ b/libdevcrypto/Common.cpp @@ -99,7 +99,7 @@ Public dev::toPublic(PublicCompressed const& _publicCompressed) return {}; std::array serializedPubkey; - size_t serializedPubkeySize = serializedPubkey.size(); + auto serializedPubkeySize = serializedPubkey.size(); secp256k1_ec_pubkey_serialize( ctx, serializedPubkey.data(), &serializedPubkeySize, &rawPubkey, SECP256K1_EC_UNCOMPRESSED); assert(serializedPubkeySize == serializedPubkey.size()); diff --git a/libp2p/NodeTable.cpp b/libp2p/NodeTable.cpp index 299eaa6e8b2..2aeb5f99ba2 100644 --- a/libp2p/NodeTable.cpp +++ b/libp2p/NodeTable.cpp @@ -687,7 +687,7 @@ std::shared_ptr NodeTable::handleENRRequest( auto const& in = dynamic_cast(_packet); - ENRResponse response(_from, m_hostENR); + ENRResponse response{_from, m_hostENR}; LOG(m_logger) << response.typeName() << " to " << in.sourceid << "@" << _from; response.timestamp = nextRequestExpirationTime(); response.echo = in.echo; @@ -704,7 +704,7 @@ std::shared_ptr NodeTable::handleENRResponse( if (!sourceNodeEntry) { LOG(m_logger) << "Source node (" << _packet.sourceid << "@" << _from - << ") not found in node table. Ignoring Neighbours packet."; + << ") not found in node table. Ignoring ENRResponse packet."; return {}; } if (sourceNodeEntry->endpoint() != _from) diff --git a/libp2p/NodeTable.h b/libp2p/NodeTable.h index 5f528bdaa11..84f581d27d9 100644 --- a/libp2p/NodeTable.h +++ b/libp2p/NodeTable.h @@ -108,7 +108,8 @@ class NodeTable : UDPSocketEvents // Period during which we consider last PONG results to be valid before sending new PONG static constexpr uint32_t c_bondingTimeSeconds{12 * 60 * 60}; - /// Constructor requiring host for I/O, credentials, and IP Address and port to listen on. + /// Constructor requiring host for I/O, credentials, and IP Address, port to listen on + /// and host ENR. NodeTable(ba::io_service& _io, KeyPair const& _alias, NodeIPEndpoint const& _endpoint, ENR const& _enr, bool _enabled = true, bool _allowLocalDiscovery = false); @@ -437,7 +438,7 @@ struct PingNode: DiscoveryDatagram PingNode(NodeIPEndpoint const& _src, NodeIPEndpoint const& _dest): DiscoveryDatagram(_dest), source(_src), destination(_dest) {} PingNode(bi::udp::endpoint const& _from, NodeID const& _fromid, h256 const& _echo): DiscoveryDatagram(_from, _fromid, _echo) {} - static const uint8_t type = 1; + static constexpr uint8_t type = 1; uint8_t packetType() const override { return type; } unsigned version = 0; @@ -477,7 +478,7 @@ struct Pong: DiscoveryDatagram Pong(NodeIPEndpoint const& _dest): DiscoveryDatagram((bi::udp::endpoint)_dest), destination(_dest) {} Pong(bi::udp::endpoint const& _from, NodeID const& _fromid, h256 const& _echo): DiscoveryDatagram(_from, _fromid, _echo) {} - static const uint8_t type = 2; + static constexpr uint8_t type = 2; uint8_t packetType() const override { return type; } NodeIPEndpoint destination; @@ -522,7 +523,7 @@ struct FindNode: DiscoveryDatagram FindNode(bi::udp::endpoint _to, h512 _target): DiscoveryDatagram(_to), target(_target) {} FindNode(bi::udp::endpoint const& _from, NodeID const& _fromid, h256 const& _echo): DiscoveryDatagram(_from, _fromid, _echo) {} - static const uint8_t type = 3; + static constexpr uint8_t type = 3; uint8_t packetType() const override { return type; } h512 target; @@ -565,7 +566,7 @@ struct Neighbours: DiscoveryDatagram void streamRLP(RLPStream& _s) const { _s.appendList(4); endpoint.streamRLP(_s, NodeIPEndpoint::StreamInline); _s << node; } }; - static const uint8_t type = 4; + static constexpr uint8_t type = 4; uint8_t packetType() const override { return type; } std::vector neighbours; @@ -592,13 +593,13 @@ struct Neighbours: DiscoveryDatagram struct ENRRequest : DiscoveryDatagram { // Constructor for outgoing packets - ENRRequest(bi::udp::endpoint _to) : DiscoveryDatagram{_to} {} + ENRRequest(bi::udp::endpoint const& _to) : DiscoveryDatagram{_to} {} // Constructor for incoming packets ENRRequest(bi::udp::endpoint const& _from, NodeID const& _fromid, h256 const& _echo) : DiscoveryDatagram{_from, _fromid, _echo} {} - static uint8_t const type = 5; + static constexpr uint8_t type = 5; uint8_t packetType() const override { return type; } void streamRLP(RLPStream& _s) const override @@ -619,17 +620,17 @@ struct ENRResponse : DiscoveryDatagram { // Constructor for outgoing packets ENRResponse(bi::udp::endpoint const& _dest, ENR const& _enr) - : DiscoveryDatagram{_dest}, enr{_enr} + : DiscoveryDatagram{_dest}, enr{new ENR{_enr}} {} // Constructor for incoming packets - ENRResponse(bi::udp::endpoint const& _from, NodeID const& _fromid, h256 const& _echo) - : DiscoveryDatagram{_from, _fromid, _echo} + ENRResponse(bi::udp::endpoint const& _from, NodeID const& _fromID, h256 const& _echo) + : DiscoveryDatagram{_from, _fromID, _echo} {} - static uint8_t const type = 6; + static constexpr uint8_t type = 6; uint8_t packetType() const override { return type; } - boost::optional enr; + std::unique_ptr enr; void streamRLP(RLPStream& _s) const override { @@ -641,7 +642,7 @@ struct ENRResponse : DiscoveryDatagram { RLP r(_bytes, RLP::AllowNonCanon | RLP::ThrowOnFail); echo = (h256)r[0]; - enr = parseV4ENR(r[1]); + enr.reset(new ENR{parseV4ENR(r[1])}); } std::string typeName() const override { return "ENRResponse"; } diff --git a/test/unittests/libp2p/net.cpp b/test/unittests/libp2p/net.cpp index 289dd802941..98b7ee5fded 100644 --- a/test/unittests/libp2p/net.cpp +++ b/test/unittests/libp2p/net.cpp @@ -1246,22 +1246,23 @@ BOOST_AUTO_TEST_CASE(validENRRequest) // socket sending ENRRequest TestUDPSocketHost nodeSocketHost; nodeSocketHost.start(); - uint16_t nodePort = nodeSocketHost.port; + uint16_t const nodePort = nodeSocketHost.port; // Exchange Ping/Pongs before sending ENRRequest // add a node to node table, initiating PING - auto nodeEndpoint = NodeIPEndpoint{bi::address::from_string(c_localhostIp), nodePort, nodePort}; - auto nodeKeyPair = KeyPair::create(); - auto nodePubKey = nodeKeyPair.pub(); + auto const nodeEndpoint = + NodeIPEndpoint{bi::address::from_string(c_localhostIp), nodePort, nodePort}; + auto const nodeKeyPair = KeyPair::create(); + auto const nodePubKey = nodeKeyPair.pub(); nodeTable->addNode(Node{nodePubKey, nodeEndpoint}); // handle received PING - auto pingDataReceived = nodeSocketHost.packetsReceived.pop(); - auto pingDatagram = + auto const pingDataReceived = nodeSocketHost.packetsReceived.pop(); + auto const pingDatagram = DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(pingDataReceived)); BOOST_REQUIRE_EQUAL(pingDatagram->typeName(), "Ping"); - auto ping = dynamic_cast(*pingDatagram); + auto const& ping = dynamic_cast(*pingDatagram); // send PONG Pong pong(nodeTable->m_hostNodeEndpoint); @@ -1270,8 +1271,8 @@ BOOST_AUTO_TEST_CASE(validENRRequest) nodeSocketHost.socket->send(pong); // wait for PONG to be received and handled - auto pongDataReceived = nodeTable->packetsReceived.pop(chrono::seconds(5)); - auto pongDatagram = + auto const pongDataReceived = nodeTable->packetsReceived.pop(chrono::seconds(5)); + auto const pongDatagram = DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(pongDataReceived)); BOOST_REQUIRE_EQUAL(pongDatagram->typeName(), "Pong"); @@ -1283,12 +1284,12 @@ BOOST_AUTO_TEST_CASE(validENRRequest) // wait for ENRRequest to be received and handled nodeTable->packetsReceived.pop(chrono::seconds(5)); - auto enrResponsePacket = nodeSocketHost.packetsReceived.pop(chrono::seconds(5)); - auto datagram = + auto const enrResponsePacket = nodeSocketHost.packetsReceived.pop(chrono::seconds(5)); + auto const datagram = DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(enrResponsePacket)); BOOST_REQUIRE_EQUAL(datagram->typeName(), "ENRResponse"); - auto enrResponse = dynamic_cast(*datagram); + auto const& enrResponse = dynamic_cast(*datagram); auto const& keyValuePairs = enrResponse.enr->keyValuePairs(); BOOST_REQUIRE_EQUAL(RLP{keyValuePairs.at("id")}.toString(), "v4"); PublicCompressed publicCompressed{RLP{keyValuePairs.at("secp256k1")}.toBytesConstRef()}; From c62cb8add1e6d9dc60191ed442c5575d7280c146 Mon Sep 17 00:00:00 2001 From: Andrei Maiboroda Date: Thu, 2 May 2019 19:19:12 +0200 Subject: [PATCH 08/10] Helper function waiting for the packet and checking its type --- libp2p/NodeTable.h | 4 +- test/unittests/libp2p/net.cpp | 126 ++++++++++------------------------ 2 files changed, 38 insertions(+), 92 deletions(-) diff --git a/libp2p/NodeTable.h b/libp2p/NodeTable.h index 84f581d27d9..00c48e820ed 100644 --- a/libp2p/NodeTable.h +++ b/libp2p/NodeTable.h @@ -595,8 +595,8 @@ struct ENRRequest : DiscoveryDatagram // Constructor for outgoing packets ENRRequest(bi::udp::endpoint const& _to) : DiscoveryDatagram{_to} {} // Constructor for incoming packets - ENRRequest(bi::udp::endpoint const& _from, NodeID const& _fromid, h256 const& _echo) - : DiscoveryDatagram{_from, _fromid, _echo} + ENRRequest(bi::udp::endpoint const& _from, NodeID const& _fromID, h256 const& _echo) + : DiscoveryDatagram{_from, _fromID, _echo} {} static constexpr uint8_t type = 5; diff --git a/test/unittests/libp2p/net.cpp b/test/unittests/libp2p/net.cpp index 98b7ee5fded..014d800f9e5 100644 --- a/test/unittests/libp2p/net.cpp +++ b/test/unittests/libp2p/net.cpp @@ -325,6 +325,17 @@ struct TestNodeTableEventHandler : NodeTableEventHandler concurrent_queue scheduledForEviction; }; +template +std::unique_ptr waitForPacketReceived(ReceiverType& _receiver, + std::string const& _receiverType, chrono::seconds const& _timeout = chrono::seconds{5}) +{ + auto const dataReceived = _receiver.packetsReceived.pop(_timeout); + auto datagram = DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(dataReceived)); + BOOST_REQUIRE_EQUAL(datagram->typeName(), _receiverType); + + return datagram; +} + BOOST_AUTO_TEST_CASE(isIPAddressType) { string wildcard = "0.0.0.0"; @@ -680,10 +691,7 @@ BOOST_AUTO_TEST_CASE(invalidPong) nodeTable->addNode(Node{nodePubKey, nodeEndpoint}); // validate received ping - auto const pingDataReceived = nodeSocketHost.packetsReceived.pop(); - auto const pingDatagram = - DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(pingDataReceived)); - BOOST_REQUIRE_EQUAL(pingDatagram->typeName(), "Ping"); + waitForPacketReceived(nodeSocketHost, "Ping"); // send invalid pong (pong without ping hash) Pong pong(nodeTable->m_hostNodeEndpoint); @@ -719,9 +727,7 @@ BOOST_AUTO_TEST_CASE(validPong) nodeTable->addNode(Node{nodePubKey, nodeEndpoint}); // handle received PING - auto pingDataReceived = nodeSocketHost.packetsReceived.pop(); - auto pingDatagram = - DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(pingDataReceived)); + auto pingDatagram = waitForPacketReceived(nodeSocketHost, "Ping"); auto ping = dynamic_cast(*pingDatagram); // send PONG @@ -759,10 +765,7 @@ BOOST_AUTO_TEST_CASE(pongWithChangedNodeID) nodeTable->addNode(Node{nodePubKey, nodeEndpoint}); // handle received PING - auto const pingDataReceived = nodeSocketHost.packetsReceived.pop(); - auto const pingDatagram = - DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(pingDataReceived)); - BOOST_REQUIRE_EQUAL(pingDatagram->typeName(), "Ping"); + auto pingDatagram = waitForPacketReceived(nodeSocketHost, "Ping"); auto const& ping = dynamic_cast(*pingDatagram); // send PONG with different NodeID @@ -812,9 +815,7 @@ BOOST_AUTO_TEST_CASE(pingTimeout) BOOST_CHECK(!sentPing.is_initialized()); // handle received PING - auto pingDataReceived = nodeSocketHost.packetsReceived.pop(); - auto pingDatagram = - DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(pingDataReceived)); + auto pingDatagram = waitForPacketReceived(nodeSocketHost, "Ping"); auto ping = dynamic_cast(*pingDatagram); // send PONG after timeout @@ -824,10 +825,7 @@ BOOST_AUTO_TEST_CASE(pingTimeout) nodeSocketHost.socket->send(pong); // wait for PONG to be received and handled - auto pongDataReceived = nodeTable->packetsReceived.pop(); - auto pongDatagram = - DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(pongDataReceived)); - BOOST_REQUIRE_EQUAL(pongDatagram->typeName(), "Pong"); + waitForPacketReceived(*nodeTable, "Pong"); BOOST_CHECK(!nodeTable->nodeExists(nodePubKey)); sentPing = nodeTable->nodeValidation(nodeEndpoint); @@ -886,9 +884,7 @@ BOOST_AUTO_TEST_CASE(neighboursSentAfterFindNode) nodeTable->packetsReceived.pop(chrono::seconds(5)); // Wait for the Neighbours packet to be received - auto packetReceived = nodeSocketHost.packetsReceived.pop(chrono::seconds(5)); - auto datagram = DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(packetReceived)); - BOOST_CHECK_EQUAL(datagram->typeName(), "Neighbours"); + waitForPacketReceived(nodeSocketHost, "Neighbours"); // TODO: Validate the contents of the neighbours packet } @@ -966,9 +962,7 @@ BOOST_AUTO_TEST_CASE(evictionWithOldNodeAnswering) BOOST_REQUIRE(evicted.is_initialized()); // handle received PING - auto pingDataReceived = nodeSocketHost.packetsReceived.pop(); - auto pingDatagram = - DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(pingDataReceived)); + auto pingDatagram = waitForPacketReceived(nodeSocketHost, "Ping"); auto ping = dynamic_cast(*pingDatagram); // send valid PONG @@ -1113,11 +1107,8 @@ BOOST_AUTO_TEST_CASE(findNodeIsSentAfterPong) // Add the TestUDPSocketHost to the node table, triggering a ping nodeTable->addNode(Node{nodeId, nodeEndpoint}); - auto packetReceived1 = nodeSocketHost.packetsReceived.pop(); - auto datagram1 = - DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(packetReceived1)); - BOOST_CHECK_EQUAL(datagram1->typeName(), "Ping"); - auto const& ping = dynamic_cast(*datagram1); + auto pingDatagram = waitForPacketReceived(nodeSocketHost, "Ping"); + auto const& ping = dynamic_cast(*pingDatagram); // Reply to the ping (which should trigger the node table to send a FindNode packet once the // next discovery pass starts) @@ -1126,10 +1117,7 @@ BOOST_AUTO_TEST_CASE(findNodeIsSentAfterPong) pong.sign(nodeKeyPair.secret()); nodeSocketHost.socket->send(pong); - auto packetReceived2 = nodeSocketHost.packetsReceived.pop(); - auto datagram2 = - DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(packetReceived2)); - BOOST_CHECK_EQUAL(datagram2->typeName(), "FindNode"); + waitForPacketReceived(nodeSocketHost, "FindNode", chrono::seconds(20)); } BOOST_AUTO_TEST_CASE(pingNotSentAfterPongForKnownNode) @@ -1155,10 +1143,7 @@ BOOST_AUTO_TEST_CASE(pingNotSentAfterPongForKnownNode) nodeTable1->addNode(node2); // Verify ping is received - auto packetReceived1 = nodeSocketHost2.packetsReceived.pop(chrono::seconds(5)); - auto datagram1 = - DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(packetReceived1)); - BOOST_REQUIRE_EQUAL(datagram1->typeName(), "Ping"); + auto datagram1 = waitForPacketReceived(nodeSocketHost2, "Ping"); // Send pong to trigger endpoint proof in node table Pong pong(nodeTable1->m_hostNodeEndpoint); @@ -1168,10 +1153,7 @@ BOOST_AUTO_TEST_CASE(pingNotSentAfterPongForKnownNode) nodeSocketHost2.socket->send(pong); // Receive FindNode packet - auto packetReceived3 = nodeSocketHost2.packetsReceived.pop(chrono::seconds(20)); - auto datagram3 = - DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(packetReceived3)); - BOOST_REQUIRE_EQUAL(datagram3->typeName(), "FindNode"); + waitForPacketReceived(nodeSocketHost2, "FindNode", chrono::seconds{20}); // Ping the node table and verify that a pong is received but no ping is received after // (since the endpoint proof has already been completed) @@ -1179,10 +1161,7 @@ BOOST_AUTO_TEST_CASE(pingNotSentAfterPongForKnownNode) ping2.sign(nodeKeyPair2.secret()); nodeSocketHost2.socket->send(ping2); - auto packetReceived4 = nodeSocketHost2.packetsReceived.pop(chrono::seconds(5)); - auto datagram4 = - DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(packetReceived4)); - BOOST_REQUIRE_EQUAL(datagram4->typeName(), "Pong"); + waitForPacketReceived(nodeSocketHost2, "Pong"); // Verify that another ping isn't sent BOOST_REQUIRE_THROW(nodeSocketHost2.packetsReceived.pop(chrono::seconds(3)), WaitTimeout); @@ -1197,15 +1176,9 @@ BOOST_AUTO_TEST_CASE(pingNotSentAfterPongForKnownNode) ping3.sign(nodeKeyPair2.secret()); nodeSocketHost2.socket->send(ping3); - auto packetReceived6 = nodeSocketHost2.packetsReceived.pop(chrono::seconds(5)); - auto datagram6 = - DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(packetReceived4)); - BOOST_CHECK_EQUAL(datagram4->typeName(), "Pong"); + waitForPacketReceived(nodeSocketHost2, "Pong"); - auto packetReceived7 = nodeSocketHost2.packetsReceived.pop(chrono::seconds(5)); - auto datagram7 = - DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(packetReceived7)); - BOOST_REQUIRE(datagram7->typeName() == "Ping"); + waitForPacketReceived(nodeSocketHost2, "Ping"); } BOOST_AUTO_TEST_CASE(addNodePingsNodeOnlyOnce) @@ -1258,10 +1231,7 @@ BOOST_AUTO_TEST_CASE(validENRRequest) nodeTable->addNode(Node{nodePubKey, nodeEndpoint}); // handle received PING - auto const pingDataReceived = nodeSocketHost.packetsReceived.pop(); - auto const pingDatagram = - DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(pingDataReceived)); - BOOST_REQUIRE_EQUAL(pingDatagram->typeName(), "Ping"); + auto pingDatagram = waitForPacketReceived(nodeSocketHost, "Ping"); auto const& ping = dynamic_cast(*pingDatagram); // send PONG @@ -1271,10 +1241,7 @@ BOOST_AUTO_TEST_CASE(validENRRequest) nodeSocketHost.socket->send(pong); // wait for PONG to be received and handled - auto const pongDataReceived = nodeTable->packetsReceived.pop(chrono::seconds(5)); - auto const pongDatagram = - DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(pongDataReceived)); - BOOST_REQUIRE_EQUAL(pongDatagram->typeName(), "Pong"); + waitForPacketReceived(*nodeTable, "Pong"); // send ENRRequest ENRRequest enrRequest(nodeTable->m_hostNodeEndpoint); @@ -1282,12 +1249,9 @@ BOOST_AUTO_TEST_CASE(validENRRequest) nodeSocketHost.socket->send(enrRequest); // wait for ENRRequest to be received and handled - nodeTable->packetsReceived.pop(chrono::seconds(5)); + waitForPacketReceived(*nodeTable, "ENRRequest"); - auto const enrResponsePacket = nodeSocketHost.packetsReceived.pop(chrono::seconds(5)); - auto const datagram = - DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(enrResponsePacket)); - BOOST_REQUIRE_EQUAL(datagram->typeName(), "ENRResponse"); + auto datagram = waitForPacketReceived(nodeSocketHost, "ENRResponse"); auto const& enrResponse = dynamic_cast(*datagram); auto const& keyValuePairs = enrResponse.enr->keyValuePairs(); @@ -1322,10 +1286,7 @@ class PacketsWithChangedEndpointFixture : public TestOutputHelperFixture nodeTable->addNode(Node{nodePubKey, nodeEndpoint1}); // handle received PING - auto pingDataReceived = nodeSocketHost1.packetsReceived.pop(chrono::seconds(5)); - auto pingDatagram = - DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(pingDataReceived)); - BOOST_REQUIRE_EQUAL(pingDatagram->typeName(), "Ping"); + auto pingDatagram = waitForPacketReceived(nodeSocketHost1, "Ping"); auto ping = dynamic_cast(*pingDatagram); // send PONG @@ -1367,10 +1328,7 @@ BOOST_AUTO_TEST_CASE(addNode) nodeTable->addNode(Node{nodePubKey, nodeEndpoint2}); // handle received PING - auto pingDataReceived = nodeSocketHost2.packetsReceived.pop(); - auto pingDatagram = - DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(pingDataReceived)); - BOOST_REQUIRE_EQUAL(pingDatagram->typeName(), "Ping"); + auto pingDatagram = waitForPacketReceived(nodeSocketHost2, "Ping"); auto ping = dynamic_cast(*pingDatagram); // send PONG @@ -1380,10 +1338,7 @@ BOOST_AUTO_TEST_CASE(addNode) nodeSocketHost2.socket->send(pong); // wait for PONG to be received and handled - auto pongDataReceived = nodeTable->packetsReceived.pop(chrono::seconds(5)); - auto pongDatagram = - DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(pongDataReceived)); - BOOST_REQUIRE_EQUAL(pongDatagram->typeName(), "Pong"); + waitForPacketReceived(*nodeTable, "Pong"); BOOST_REQUIRE_EQUAL(nodeEntry->endpoint(), nodeEndpoint2); } @@ -1396,10 +1351,7 @@ BOOST_AUTO_TEST_CASE(findNode) nodeSocketHost2.socket->send(findNode); // Wait for FindNode to be received - auto findNodeDataReceived = nodeTable->packetsReceived.pop(chrono::seconds(5)); - auto findNodeDatagram = - DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(findNodeDataReceived)); - BOOST_REQUIRE_EQUAL(findNodeDatagram->typeName(), "FindNode"); + waitForPacketReceived(*nodeTable, "FindNode"); // Verify that no neighbours response is received BOOST_CHECK_THROW(nodeSocketHost2.packetsReceived.pop(chrono::seconds(5)), WaitTimeout); @@ -1408,10 +1360,7 @@ BOOST_AUTO_TEST_CASE(findNode) BOOST_AUTO_TEST_CASE(neighbours) { // Wait for FindNode arriving to endpoint 1 - auto findNodeDataReceived = nodeSocketHost1.packetsReceived.pop(chrono::seconds(10)); - auto findNodeDatagram = - DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(findNodeDataReceived)); - BOOST_REQUIRE_EQUAL(findNodeDatagram->typeName(), "FindNode"); + auto findNodeDatagram = waitForPacketReceived(nodeSocketHost1, "FindNode", chrono::seconds{10}); auto findNode = dynamic_cast(*findNodeDatagram); // send Neighbours through endpoint 2 @@ -1425,10 +1374,7 @@ BOOST_AUTO_TEST_CASE(neighbours) nodeSocketHost2.socket->send(neighbours); // Wait for Neighbours to be received - auto neighboursDataReceived = nodeTable->packetsReceived.pop(chrono::seconds(5)); - auto neighboursDatagram = - DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(neighboursDataReceived)); - BOOST_REQUIRE_EQUAL(neighboursDatagram->typeName(), "Neighbours"); + waitForPacketReceived(*nodeTable, "Neighbours"); // no Ping is sent to neighbour auto sentPing = nodeTable->nodeValidation(neighbourEndpoint); From 28dc37eaaa7c38c6f13f12cd4d137264c2bdb398 Mon Sep 17 00:00:00 2001 From: Andrei Maiboroda Date: Mon, 6 May 2019 13:06:16 +0200 Subject: [PATCH 09/10] Rename DiscoveryDatagram::timestamp to expiration --- libp2p/NodeTable.cpp | 10 +++++----- libp2p/NodeTable.h | 27 ++++++++++++++------------- test/unittests/libp2p/eip-8.cpp | 10 +++++----- 3 files changed, 24 insertions(+), 23 deletions(-) diff --git a/libp2p/NodeTable.cpp b/libp2p/NodeTable.cpp index 2aeb5f99ba2..b22cdeabb01 100644 --- a/libp2p/NodeTable.cpp +++ b/libp2p/NodeTable.cpp @@ -221,7 +221,7 @@ void NodeTable::doDiscoveryRound( } FindNode p(nodeEntry->endpoint(), _node); - p.timestamp = nextRequestExpirationTime(); + p.expiration = nextRequestExpirationTime(); p.sign(m_secret); m_sentFindNodes.emplace_back(nodeEntry->id(), chrono::steady_clock::now()); LOG(m_logger) << p.typeName() << " to " << nodeEntry->node << " (target: " << _node @@ -303,7 +303,7 @@ void NodeTable::ping(Node const& _node, shared_ptr _replacementNodeEn } PingNode p{m_hostNodeEndpoint, _node.endpoint}; - p.timestamp = nextRequestExpirationTime(); + p.expiration = nextRequestExpirationTime(); p.seq = m_hostENR.sequenceNumber(); auto const pingHash = p.sign(m_secret); LOG(m_logger) << p.typeName() << " to " << _node; @@ -618,7 +618,7 @@ std::shared_ptr NodeTable::handleFindNode( for (unsigned offset = 0; offset < nearest.size(); offset += nlimit) { Neighbours out(_from, nearest, offset, nlimit); - out.timestamp = nextRequestExpirationTime(); + out.expiration = nextRequestExpirationTime(); LOG(m_logger) << out.typeName() << " to " << in.sourceid << "@" << _from; out.sign(m_secret); if (out.data.size() > 1280) @@ -641,7 +641,7 @@ std::shared_ptr NodeTable::handlePingNode( // Send PONG response. Pong p(sourceEndpoint); LOG(m_logger) << p.typeName() << " to " << in.sourceid << "@" << _from; - p.timestamp = nextRequestExpirationTime(); + p.expiration = nextRequestExpirationTime(); p.echo = in.echo; p.seq = m_hostENR.sequenceNumber(); p.sign(m_secret); @@ -689,7 +689,7 @@ std::shared_ptr NodeTable::handleENRRequest( ENRResponse response{_from, m_hostENR}; LOG(m_logger) << response.typeName() << " to " << in.sourceid << "@" << _from; - response.timestamp = nextRequestExpirationTime(); + response.expiration = nextRequestExpirationTime(); response.echo = in.echo; response.sign(m_secret); m_socket->send(response); diff --git a/libp2p/NodeTable.h b/libp2p/NodeTable.h index 00c48e820ed..a57f726c0c6 100644 --- a/libp2p/NodeTable.h +++ b/libp2p/NodeTable.h @@ -401,7 +401,7 @@ struct DiscoveryDatagram: public RLPXDatagramFace /// Constructor used for sending. DiscoveryDatagram(bi::udp::endpoint const& _to) - : RLPXDatagramFace(_to), timestamp(futureFromEpoch(c_timeToLiveS)) + : RLPXDatagramFace(_to), expiration(futureFromEpoch(c_timeToLiveS)) {} /// Constructor used for parsing inbound packets. @@ -413,10 +413,11 @@ struct DiscoveryDatagram: public RLPXDatagramFace // Most discovery packets carry a timestamp, which must be greater // than the current local time. This prevents replay attacks. - boost::optional timestamp; + // Optional because some packets (ENRResponse) don't have it + boost::optional expiration; bool isExpired() const { - return timestamp.is_initialized() && secondsSinceEpoch() > *timestamp; + return expiration.is_initialized() && secondsSinceEpoch() > *expiration; } /// Decodes UDP packets. @@ -452,7 +453,7 @@ struct PingNode: DiscoveryDatagram _s << dev::p2p::c_protocolVersion; source.streamRLP(_s); destination.streamRLP(_s); - _s << *timestamp; + _s << *expiration; if (seq.is_initialized()) _s << *seq; } @@ -462,7 +463,7 @@ struct PingNode: DiscoveryDatagram version = r[0].toInt(); source.interpretRLP(r[1]); destination.interpretRLP(r[2]); - timestamp = r[3].toInt(); + expiration = r[3].toInt(); if (r.itemCount() > 4 && r[4].isInt()) seq = r[4].toInt(); } @@ -489,7 +490,7 @@ struct Pong: DiscoveryDatagram _s.appendList(seq.is_initialized() ? 4 : 3); destination.streamRLP(_s); _s << echo; - _s << *timestamp; + _s << *expiration; if (seq.is_initialized()) _s << *seq; } @@ -498,7 +499,7 @@ struct Pong: DiscoveryDatagram RLP r(_bytes, RLP::AllowNonCanon|RLP::ThrowOnFail); destination.interpretRLP(r[0]); echo = (h256)r[1]; - timestamp = r[2].toInt(); + expiration = r[2].toInt(); if (r.itemCount() > 3 && r[3].isInt()) seq = r[3].toInt(); } @@ -531,13 +532,13 @@ struct FindNode: DiscoveryDatagram void streamRLP(RLPStream& _s) const override { _s.appendList(2); - _s << target << *timestamp; + _s << target << *expiration; } void interpretRLP(bytesConstRef _bytes) override { RLP r(_bytes, RLP::AllowNonCanon|RLP::ThrowOnFail); target = r[0].toHash(); - timestamp = r[1].toInt(); + expiration = r[1].toInt(); } std::string typeName() const override { return "FindNode"; } @@ -577,14 +578,14 @@ struct Neighbours: DiscoveryDatagram _s.appendList(neighbours.size()); for (auto const& n: neighbours) n.streamRLP(_s); - _s << *timestamp; + _s << *expiration; } void interpretRLP(bytesConstRef _bytes) override { RLP r(_bytes, RLP::AllowNonCanon|RLP::ThrowOnFail); for (auto const& n: r[0]) neighbours.emplace_back(n); - timestamp = r[1].toInt(); + expiration = r[1].toInt(); } std::string typeName() const override { return "Neighbours"; } @@ -605,12 +606,12 @@ struct ENRRequest : DiscoveryDatagram void streamRLP(RLPStream& _s) const override { _s.appendList(1); - _s << *timestamp; + _s << *expiration; } void interpretRLP(bytesConstRef _bytes) override { RLP r(_bytes, RLP::AllowNonCanon | RLP::ThrowOnFail); - timestamp = r[0].toInt(); + expiration = r[0].toInt(); } std::string typeName() const override { return "ENRRequest"; } diff --git a/test/unittests/libp2p/eip-8.cpp b/test/unittests/libp2p/eip-8.cpp index 5bb9f45ece8..ee518c55faa 100644 --- a/test/unittests/libp2p/eip-8.cpp +++ b/test/unittests/libp2p/eip-8.cpp @@ -28,7 +28,7 @@ TEST(eip8, test_discovery_packets) auto ping1 = dynamic_cast(*DiscoveryDatagram::interpretUDP(ep, bytesConstRef(&packet))); EXPECT_EQ(ping1.version, 4); - EXPECT_EQ(*ping1.timestamp, 1136239445); + EXPECT_EQ(*ping1.expiration, 1136239445); EXPECT_EQ(ping1.source, NodeIPEndpoint(bi::address::from_string("127.0.0.1"), 3322, 5544)); EXPECT_EQ(ping1.destination, NodeIPEndpoint(bi::address::from_string("::1"), 2222, 3333)); @@ -49,7 +49,7 @@ TEST(eip8, test_discovery_packets) EXPECT_EQ(ping2.destination, NodeIPEndpoint( bi::address::from_string("2001:db8:85a3:8d3:1319:8a2e:370:7348"), 2222, 33338)); - EXPECT_EQ(*ping2.timestamp, 1136239445); + EXPECT_EQ(*ping2.expiration, 1136239445); packet = fromHex( "09b2428d83348d27cdf7064ad9024f526cebc19e4958f0fdad87c15eb598dd61d08423e0bf66b206" @@ -63,7 +63,7 @@ TEST(eip8, test_discovery_packets) NodeIPEndpoint( bi::address::from_string("2001:db8:85a3:8d3:1319:8a2e:370:7348"), 2222, 33338)); EXPECT_EQ(pong.echo, h256("fbc914b16819237dcd8801d7e53f69e9719adecb3cc0e790c57e91ca4461c954")); - EXPECT_EQ(*pong.timestamp, 1136239445); + EXPECT_EQ(*pong.expiration, 1136239445); packet = fromHex( "c7c44041b9f7c7e41934417ebac9a8e1a4c6298f74553f2fcfdcae6ed6fe53163eb3d2b52e39fe91" @@ -77,7 +77,7 @@ TEST(eip8, test_discovery_packets) EXPECT_EQ( findnode.target, Public("ca634cae0d49acb401d8a4c6b6fe8c55b70d115bf400769cc1400f3258cd313875" "74077f301b421bc84df7266c44e9e6d569fc56be00812904767bf5ccd1fc7f")); - EXPECT_EQ(*findnode.timestamp, 1136239445); + EXPECT_EQ(*findnode.expiration, 1136239445); packet = fromHex( "c679fc8fe0b8b12f06577f2e802d34f6fa257e6137a995f6f4cbfc9ee50ed3710faf6e66f932c4c8" @@ -116,7 +116,7 @@ TEST(eip8, test_discovery_packets) EXPECT_EQ(neighbours.neighbours[3].node, Public("8dcab8618c3253b558d459da53bd8fa68935a719aff8b811197101a4b2b47dd2d47295286fc00cc081b" "b542d760717d1bdd6bec2c37cd72eca367d6dd3b9df73")); - EXPECT_EQ(*neighbours.timestamp, 1136239445); + EXPECT_EQ(*neighbours.expiration, 1136239445); } class TestHandshake : public RLPXHandshake From 6f151065d98775520f9eed53de2caa247d723b04 Mon Sep 17 00:00:00 2001 From: Andrei Maiboroda Date: Mon, 6 May 2019 14:32:06 +0200 Subject: [PATCH 10/10] Pass RLP by const reference --- libp2p/ENR.cpp | 4 ++-- libp2p/ENR.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libp2p/ENR.cpp b/libp2p/ENR.cpp index eddb2465261..7e9f90cce7f 100644 --- a/libp2p/ENR.cpp +++ b/libp2p/ENR.cpp @@ -29,7 +29,7 @@ bytes addressToBytes(Address const& _address) } } // namespace -ENR::ENR(RLP _rlp, VerifyFunction const& _verifyFunction) +ENR::ENR(RLP const& _rlp, VerifyFunction const& _verifyFunction) { if (_rlp.data().size() > c_ENRMaxSizeBytes) BOOST_THROW_EXCEPTION(ENRIsTooBig()); @@ -110,7 +110,7 @@ ENR createV4ENR(Secret const& _secret, boost::asio::ip::address const& _ip, uint return ENR{0 /* sequence number */, keyValuePairs, signFunction}; } -ENR parseV4ENR(RLP _rlp) +ENR parseV4ENR(RLP const& _rlp) { ENR::VerifyFunction verifyFunction = [](std::map const& _keyValuePairs, bytesConstRef _signature, bytesConstRef _data) { diff --git a/libp2p/ENR.h b/libp2p/ENR.h index 8174e89021b..c58965a3f96 100644 --- a/libp2p/ENR.h +++ b/libp2p/ENR.h @@ -29,7 +29,7 @@ class ENR std::function const&, bytesConstRef, bytesConstRef)>; // Parse from RLP with given signature verification function - ENR(RLP _rlp, VerifyFunction const& _verifyFunction); + ENR(RLP const& _rlp, VerifyFunction const& _verifyFunction); // Create with given sign function ENR(uint64_t _seq, std::map const& _keyValues, SignFunction const& _signFunction); @@ -54,7 +54,7 @@ class ENR ENR createV4ENR(Secret const& _secret, boost::asio::ip::address const& _ip, uint16_t _tcpPort, uint16_t _udpPort); -ENR parseV4ENR(RLP _rlp); +ENR parseV4ENR(RLP const& _rlp); std::ostream& operator<<(std::ostream& _out, ENR const& _enr);