Skip to content
This repository was archived by the owner on Oct 28, 2021. It is now read-only.

Commit 0886a34

Browse files
gumb0gumb0
committed
Check whether node's endpoint changed when handling discovery packets
1 parent 6f5d0f1 commit 0886a34

File tree

3 files changed

+148
-41
lines changed

3 files changed

+148
-41
lines changed

‎libp2p/NodeTable.cpp‎

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,8 @@ bool NodeTable::addNode(Node const& _node)
8383
DEV_GUARDED(x_nodes)
8484
{
8585
auto const it = m_allNodes.find(_node.id);
86-
needToPing = (it == m_allNodes.end() || !it->second->hasValidEndpointProof());
86+
needToPing = (it == m_allNodes.end() || it->second->endpoint != _node.endpoint ||
87+
!it->second->hasValidEndpointProof());
8788
}
8889

8990
if (needToPing)
@@ -115,7 +116,7 @@ bool NodeTable::addKnownNode(
115116
if (entry->hasValidEndpointProof())
116117
{
117118
LOG(m_logger) << "Known " << _node;
118-
noteActiveNode(move(entry), _node.endpoint);
119+
noteActiveNode(move(entry));
119120
}
120121
else
121122
{
@@ -356,7 +357,7 @@ void NodeTable::evict(NodeEntry const& _leastSeen, shared_ptr<NodeEntry> _replac
356357
m_nodeEventHandler->appendEvent(_leastSeen.id, NodeEntryScheduledForEviction);
357358
}
358359

359-
void NodeTable::noteActiveNode(shared_ptr<NodeEntry> _nodeEntry, bi::udp::endpoint const& _endpoint)
360+
void NodeTable::noteActiveNode(shared_ptr<NodeEntry> _nodeEntry)
360361
{
361362
assert(_nodeEntry);
362363

@@ -365,7 +366,7 @@ void NodeTable::noteActiveNode(shared_ptr<NodeEntry> _nodeEntry, bi::udp::endpoi
365366
LOG(m_logger) << "Skipping making self active.";
366367
return;
367368
}
368-
if (!isAllowedEndpoint(NodeIPEndpoint(_endpoint.address(), _endpoint.port(), _endpoint.port())))
369+
if (!isAllowedEndpoint(_nodeEntry->endpoint))
369370
{
370371
LOG(m_logger) << "Skipping making node with unallowed endpoint active. Node " << *_nodeEntry;
371372
return;
@@ -375,10 +376,6 @@ void NodeTable::noteActiveNode(shared_ptr<NodeEntry> _nodeEntry, bi::udp::endpoi
375376
return;
376377

377378
LOG(m_logger) << "Active node " << *_nodeEntry;
378-
// TODO: don't activate in case endpoint has changed
379-
_nodeEntry->endpoint.setAddress(_endpoint.address());
380-
_nodeEntry->endpoint.setUdpPort(_endpoint.port());
381-
382379

383380
shared_ptr<NodeEntry> nodeToEvict;
384381
{
@@ -513,6 +510,10 @@ void NodeTable::onPacketReceived(
513510
sourceNodeEntry = it->second;
514511
sourceNodeEntry->lastPongReceivedTime =
515512
RLPXDatagramFace::secondsSinceEpoch();
513+
514+
if (sourceNodeEntry->endpoint != _from)
515+
sourceNodeEntry->endpoint = NodeIPEndpoint{
516+
_from.address(), _from.port(), nodeValidation.tcpPort};
516517
}
517518
}
518519

@@ -538,6 +539,11 @@ void NodeTable::onPacketReceived(
538539
<< ") not found in node table. Ignoring Neighbours packet.";
539540
return;
540541
}
542+
if (sourceNodeEntry->endpoint != _from)
543+
{
544+
LOG(m_logger) << "Neighbours packet from unexpected endpoint.";
545+
return;
546+
}
541547

542548
auto const& in = dynamic_cast<Neighbours const&>(*packet);
543549

@@ -571,6 +577,11 @@ void NodeTable::onPacketReceived(
571577
<< ") not found in node table. Ignoring FindNode request.";
572578
return;
573579
}
580+
if (sourceNodeEntry->endpoint != _from)
581+
{
582+
LOG(m_logger) << "FindNode packet from unexpected endpoint.";
583+
return;
584+
}
574585
if (!sourceNodeEntry->lastPongReceivedTime)
575586
{
576587
LOG(m_logger) << "Unexpected FindNode packet! Endpoint proof hasn't been performed yet.";
@@ -627,7 +638,7 @@ void NodeTable::onPacketReceived(
627638
}
628639

629640
if (sourceNodeEntry)
630-
noteActiveNode(move(sourceNodeEntry), _from);
641+
noteActiveNode(move(sourceNodeEntry));
631642
}
632643
catch (exception const& _e)
633644
{
@@ -711,7 +722,7 @@ void NodeTable::doHandleTimeouts()
711722

712723
// activate replacement nodes and put them into buckets
713724
for (auto const& n : nodesToActivate)
714-
noteActiveNode(n, n->endpoint);
725+
noteActiveNode(n);
715726

716727
doHandleTimeouts();
717728
});

‎libp2p/NodeTable.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ class NodeTable : UDPSocketEvents
261261

262262
/// Called whenever activity is received from a node in order to maintain node table. Only
263263
/// called for nodes for which we've completed an endpoint proof.
264-
void noteActiveNode(std::shared_ptr<NodeEntry> _nodeEntry, bi::udp::endpoint const& _endpoint);
264+
void noteActiveNode(std::shared_ptr<NodeEntry> _nodeEntry);
265265

266266
/// Used to drop node when timeout occurs or when evict() result is to keep previous node.
267267
void dropNode(std::shared_ptr<NodeEntry> _n);

‎test/unittests/libp2p/net.cpp‎

Lines changed: 126 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ struct TestNodeTable: public NodeTable
7979
auto entry = make_shared<NodeEntry>(m_hostNodeID, n.first,
8080
NodeIPEndpoint(ourIp, n.second, n.second),
8181
RLPXDatagramFace::secondsSinceEpoch(), RLPXDatagramFace::secondsSinceEpoch());
82-
noteActiveNode(move(entry), bi::udp::endpoint(ourIp, n.second));
82+
noteActiveNode(move(entry));
8383
}
8484
else
8585
break;
@@ -100,7 +100,7 @@ struct TestNodeTable: public NodeTable
100100
NodeIPEndpoint(ourIp, testNode->second, testNode->second),
101101
RLPXDatagramFace::secondsSinceEpoch(), RLPXDatagramFace::secondsSinceEpoch()));
102102
auto distance = node->distance;
103-
noteActiveNode(move(node), bi::udp::endpoint(ourIp, testNode->second));
103+
noteActiveNode(move(node));
104104

105105
{
106106
Guard stateGuard(x_state);
@@ -138,7 +138,7 @@ struct TestNodeTable: public NodeTable
138138
auto entry = make_shared<NodeEntry>(m_hostNodeID, testNode->first,
139139
NodeIPEndpoint(ourIp, testNode->second, testNode->second),
140140
RLPXDatagramFace::secondsSinceEpoch(), RLPXDatagramFace::secondsSinceEpoch());
141-
noteActiveNode(move(entry), bi::udp::endpoint(ourIp, testNode->second));
141+
noteActiveNode(move(entry));
142142

143143
++testNode;
144144
}
@@ -500,7 +500,7 @@ BOOST_AUTO_TEST_CASE(noteActiveNodeUpdatesKnownNode)
500500
auto& nodeTable = nodeTableHost.nodeTable;
501501
auto knownNode = nodeTable->bucketFirstNode(bucketIndex);
502502

503-
nodeTable->noteActiveNode(knownNode, knownNode->endpoint);
503+
nodeTable->noteActiveNode(knownNode);
504504

505505
// check that node was moved to the back of the bucket
506506
BOOST_CHECK_NE(nodeTable->bucketFirstNode(bucketIndex), knownNode);
@@ -550,32 +550,6 @@ BOOST_AUTO_TEST_CASE(noteActiveNodeEvictsTheNodeWhenBucketIsFull)
550550
BOOST_CHECK_EQUAL(evicted->replacementNodeEntry->id, newNodeId);
551551
}
552552

553-
BOOST_AUTO_TEST_CASE(noteActiveNodeReplacesNodeInFullBucketWhenEndpointChanged)
554-
{
555-
TestNodeTableHost nodeTableHost(512);
556-
int const bucketIndex = nodeTableHost.populateUntilBucketSize(16);
557-
BOOST_REQUIRE(bucketIndex >= 0);
558-
559-
auto& nodeTable = nodeTableHost.nodeTable;
560-
auto leastRecentlySeenNode = nodeTable->bucketFirstNode(bucketIndex);
561-
562-
// addNode will replace the node in the m_allNodes map, because it's the same id with enother
563-
// endpoint
564-
auto const port = randomPortNumber();
565-
NodeIPEndpoint newEndpoint{bi::address::from_string(c_localhostIp), port, port };
566-
nodeTable->noteActiveNode(leastRecentlySeenNode, newEndpoint);
567-
568-
// the bucket is still max size
569-
BOOST_CHECK_EQUAL(nodeTable->bucketSize(bucketIndex), 16);
570-
// least recently seen node removed
571-
BOOST_CHECK_NE(nodeTable->bucketFirstNode(bucketIndex)->id, leastRecentlySeenNode->id);
572-
// but added as most recently seen with new endpoint
573-
auto mostRecentNodeEntry = nodeTable->bucketLastNode(bucketIndex);
574-
BOOST_CHECK_EQUAL(mostRecentNodeEntry->id, leastRecentlySeenNode->id);
575-
BOOST_CHECK_EQUAL(mostRecentNodeEntry->endpoint.address(), newEndpoint.address());
576-
BOOST_CHECK_EQUAL(mostRecentNodeEntry->endpoint.udpPort(), newEndpoint.udpPort());
577-
}
578-
579553
BOOST_AUTO_TEST_CASE(unexpectedPong)
580554
{
581555
// NodeTable receiving PONG
@@ -1173,6 +1147,128 @@ BOOST_AUTO_TEST_CASE(addNodePingsNodeOnlyOnce)
11731147
BOOST_REQUIRE_EQUAL(sentPing->pingHash, sentPing2->pingHash);
11741148
}
11751149

1150+
class PacketsWithChangedEndpointFixture : public TestOutputHelperFixture
1151+
{
1152+
public:
1153+
PacketsWithChangedEndpointFixture()
1154+
{
1155+
nodeTableHost.start();
1156+
nodeSocketHost1.start();
1157+
nodePort1 = nodeSocketHost1.port;
1158+
nodeSocketHost2.start();
1159+
nodePort2 = nodeSocketHost2.port;
1160+
1161+
nodeEndpoint1 =
1162+
NodeIPEndpoint{bi::address::from_string(c_localhostIp), nodePort1, nodePort1};
1163+
nodeEndpoint2 =
1164+
NodeIPEndpoint{bi::address::from_string(c_localhostIp), nodePort2, nodePort2};
1165+
1166+
// add a node to node table, initiating PING
1167+
nodeTable->addNode(Node{nodePubKey, nodeEndpoint1});
1168+
1169+
// handle received PING
1170+
auto pingDataReceived = nodeSocketHost1.packetsReceived.pop();
1171+
auto pingDatagram =
1172+
DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(pingDataReceived));
1173+
auto ping = dynamic_cast<PingNode const&>(*pingDatagram);
1174+
1175+
// send PONG
1176+
Pong pong(nodeTable->m_hostNodeEndpoint);
1177+
pong.echo = ping.echo;
1178+
pong.sign(nodeKeyPair.secret());
1179+
nodeSocketHost1.socket->send(pong);
1180+
1181+
// wait for PONG to be received and handled
1182+
nodeTable->packetsReceived.pop(chrono::seconds(5));
1183+
1184+
nodeEntry = nodeTable->nodeEntry(nodePubKey);
1185+
}
1186+
1187+
TestNodeTableHost nodeTableHost{0};
1188+
shared_ptr<TestNodeTable>& nodeTable = nodeTableHost.nodeTable;
1189+
1190+
// socket representing initial peer node
1191+
TestUDPSocketHost nodeSocketHost1;
1192+
uint16_t nodePort1 = 0;
1193+
1194+
// socket representing peer with changed endpoint
1195+
TestUDPSocketHost nodeSocketHost2;
1196+
uint16_t nodePort2 = 0;
1197+
1198+
NodeIPEndpoint nodeEndpoint1;
1199+
NodeIPEndpoint nodeEndpoint2;
1200+
KeyPair nodeKeyPair = KeyPair::create();
1201+
NodeID nodePubKey = nodeKeyPair.pub();
1202+
1203+
shared_ptr<NodeEntry> nodeEntry;
1204+
};
1205+
1206+
BOOST_FIXTURE_TEST_SUITE(packetsWithChangedEndpointSuite, PacketsWithChangedEndpointFixture)
1207+
1208+
BOOST_AUTO_TEST_CASE(addNode)
1209+
{
1210+
// this should initiate Ping to endpoint 2
1211+
nodeTable->addNode(Node{nodePubKey, nodeEndpoint2});
1212+
1213+
// handle received PING
1214+
auto pingDataReceived = nodeSocketHost2.packetsReceived.pop();
1215+
auto pingDatagram =
1216+
DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(pingDataReceived));
1217+
auto ping = dynamic_cast<PingNode const&>(*pingDatagram);
1218+
1219+
// send PONG
1220+
Pong pong(nodeTable->m_hostNodeEndpoint);
1221+
pong.echo = ping.echo;
1222+
pong.sign(nodeKeyPair.secret());
1223+
nodeSocketHost2.socket->send(pong);
1224+
1225+
// wait for PONG to be received and handled
1226+
nodeTable->packetsReceived.pop(chrono::seconds(5));
1227+
1228+
BOOST_REQUIRE_EQUAL(nodeEntry->endpoint, nodeEndpoint2);
1229+
}
1230+
1231+
BOOST_AUTO_TEST_CASE(findNode)
1232+
{
1233+
// Create and send the FindNode packet through endpoint 2
1234+
FindNode findNode(nodeTable->m_hostNodeEndpoint, KeyPair::create().pub() /* target */);
1235+
findNode.sign(nodeKeyPair.secret());
1236+
nodeSocketHost2.socket->send(findNode);
1237+
1238+
// Wait for FindNode to be received
1239+
nodeTable->packetsReceived.pop(chrono::seconds(5));
1240+
1241+
// Verify that no neighbours response is received
1242+
BOOST_CHECK_THROW(nodeSocketHost2.packetsReceived.pop(chrono::seconds(5)), WaitTimeout);
1243+
}
1244+
1245+
BOOST_AUTO_TEST_CASE(neighbours)
1246+
{
1247+
// Wait for FindNode arriving to endpoint 1
1248+
auto findNodeDataReceived = nodeSocketHost1.packetsReceived.pop(chrono::seconds(10));
1249+
auto findNodeDatagram =
1250+
DiscoveryDatagram::interpretUDP(bi::udp::endpoint{}, dev::ref(findNodeDataReceived));
1251+
auto findNode = dynamic_cast<FindNode const&>(*findNodeDatagram);
1252+
1253+
// send Neighbours through endpoint 2
1254+
// TODO fill nearest with one node
1255+
NodeIPEndpoint neighbourEndpoint{boost::asio::ip::address::from_string("200.200.200.200"),
1256+
c_defaultListenPort, c_defaultListenPort};
1257+
vector<shared_ptr<NodeEntry>> nearest{make_shared<NodeEntry>(nodeTable->m_hostNodeID,
1258+
KeyPair::create().pub(), neighbourEndpoint, RLPXDatagramFace::secondsSinceEpoch(), 0)};
1259+
Neighbours neighbours(nodeTable->m_hostNodeEndpoint, nearest);
1260+
neighbours.sign(nodeKeyPair.secret());
1261+
nodeSocketHost2.socket->send(neighbours);
1262+
1263+
// Wait for Neighbours to be received
1264+
nodeTable->packetsReceived.pop(chrono::seconds(5));
1265+
1266+
// no Ping is sent to neighbour
1267+
auto sentPing = nodeTable->nodeValidation(neighbourEndpoint);
1268+
BOOST_REQUIRE(!sentPing.is_initialized());
1269+
}
1270+
1271+
BOOST_AUTO_TEST_SUITE_END()
11761272
BOOST_AUTO_TEST_SUITE_END()
11771273

11781274
BOOST_FIXTURE_TEST_SUITE(netTypes, TestOutputHelperFixture)

0 commit comments

Comments
 (0)