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

Commit f246aef

Browse files
committed
Check whether node's endpoint changed when handling discovery packets
1 parent fe2eaab commit f246aef

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
@@ -79,7 +79,8 @@ bool NodeTable::addNode(Node const& _node)
7979
DEV_GUARDED(x_nodes)
8080
{
8181
auto const it = m_allNodes.find(_node.id);
82-
needToPing = (it == m_allNodes.end() || !it->second->hasValidEndpointProof());
82+
needToPing = (it == m_allNodes.end() || it->second->endpoint != _node.endpoint ||
83+
!it->second->hasValidEndpointProof());
8384
}
8485

8586
if (needToPing)
@@ -111,7 +112,7 @@ bool NodeTable::addKnownNode(
111112
if (entry->hasValidEndpointProof())
112113
{
113114
LOG(m_logger) << "Known " << _node;
114-
noteActiveNode(move(entry), _node.endpoint);
115+
noteActiveNode(move(entry));
115116
}
116117
else
117118
{
@@ -346,7 +347,7 @@ void NodeTable::evict(NodeEntry const& _leastSeen, shared_ptr<NodeEntry> _replac
346347
m_nodeEventHandler->appendEvent(_leastSeen.id, NodeEntryScheduledForEviction);
347348
}
348349

349-
void NodeTable::noteActiveNode(shared_ptr<NodeEntry> _nodeEntry, bi::udp::endpoint const& _endpoint)
350+
void NodeTable::noteActiveNode(shared_ptr<NodeEntry> _nodeEntry)
350351
{
351352
assert(_nodeEntry);
352353

@@ -355,7 +356,7 @@ void NodeTable::noteActiveNode(shared_ptr<NodeEntry> _nodeEntry, bi::udp::endpoi
355356
LOG(m_logger) << "Skipping making self active.";
356357
return;
357358
}
358-
if (!isAllowedEndpoint(NodeIPEndpoint(_endpoint.address(), _endpoint.port(), _endpoint.port())))
359+
if (!isAllowedEndpoint(_nodeEntry->endpoint))
359360
{
360361
LOG(m_logger) << "Skipping making node with unallowed endpoint active. Node " << *_nodeEntry;
361362
return;
@@ -365,10 +366,6 @@ void NodeTable::noteActiveNode(shared_ptr<NodeEntry> _nodeEntry, bi::udp::endpoi
365366
return;
366367

367368
LOG(m_logger) << "Active node " << *_nodeEntry;
368-
// TODO: don't activate in case endpoint has changed
369-
_nodeEntry->endpoint.setAddress(_endpoint.address());
370-
_nodeEntry->endpoint.setUdpPort(_endpoint.port());
371-
372369

373370
shared_ptr<NodeEntry> nodeToEvict;
374371
{
@@ -502,6 +499,10 @@ void NodeTable::onPacketReceived(
502499
sourceNodeEntry = it->second;
503500
sourceNodeEntry->lastPongReceivedTime =
504501
RLPXDatagramFace::secondsSinceEpoch();
502+
503+
if (sourceNodeEntry->endpoint != _from)
504+
sourceNodeEntry->endpoint = NodeIPEndpoint{
505+
_from.address(), _from.port(), nodeValidation.tcpPort};
505506
}
506507
}
507508

@@ -527,6 +528,11 @@ void NodeTable::onPacketReceived(
527528
<< ") not found in node table. Ignoring Neighbours packet.";
528529
return;
529530
}
531+
if (sourceNodeEntry->endpoint != _from)
532+
{
533+
LOG(m_logger) << "Neighbours packet from unexpected endpoint.";
534+
return;
535+
}
530536

531537
auto const& in = dynamic_cast<Neighbours const&>(*packet);
532538

@@ -560,6 +566,11 @@ void NodeTable::onPacketReceived(
560566
<< ") not found in node table. Ignoring FindNode request.";
561567
return;
562568
}
569+
if (sourceNodeEntry->endpoint != _from)
570+
{
571+
LOG(m_logger) << "FindNode packet from unexpected endpoint.";
572+
return;
573+
}
563574
if (!sourceNodeEntry->lastPongReceivedTime)
564575
{
565576
LOG(m_logger) << "Unexpected FindNode packet! Endpoint proof hasn't been performed yet.";
@@ -616,7 +627,7 @@ void NodeTable::onPacketReceived(
616627
}
617628

618629
if (sourceNodeEntry)
619-
noteActiveNode(move(sourceNodeEntry), _from);
630+
noteActiveNode(move(sourceNodeEntry));
620631
}
621632
catch (std::exception const& _e)
622633
{
@@ -680,7 +691,7 @@ void NodeTable::doHandleTimeouts()
680691

681692
// activate replacement nodes and put them into buckets
682693
for (auto const& n : nodesToActivate)
683-
noteActiveNode(n, n->endpoint);
694+
noteActiveNode(n);
684695

685696
doHandleTimeouts();
686697
});

libp2p/NodeTable.h

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

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

248248
/// Used to drop node when timeout occurs or when evict() result is to keep previous node.
249249
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
@@ -80,7 +80,7 @@ struct TestNodeTable: public NodeTable
8080
auto entry = make_shared<NodeEntry>(m_hostNodeID, n.first,
8181
NodeIPEndpoint(ourIp, n.second, n.second),
8282
RLPXDatagramFace::secondsSinceEpoch(), RLPXDatagramFace::secondsSinceEpoch());
83-
noteActiveNode(move(entry), bi::udp::endpoint(ourIp, n.second));
83+
noteActiveNode(move(entry));
8484
}
8585
else
8686
break;
@@ -101,7 +101,7 @@ struct TestNodeTable: public NodeTable
101101
NodeIPEndpoint(ourIp, testNode->second, testNode->second),
102102
RLPXDatagramFace::secondsSinceEpoch(), RLPXDatagramFace::secondsSinceEpoch()));
103103
auto distance = node->distance;
104-
noteActiveNode(move(node), bi::udp::endpoint(ourIp, testNode->second));
104+
noteActiveNode(move(node));
105105

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

144144
++testNode;
145145
}
@@ -502,7 +502,7 @@ BOOST_AUTO_TEST_CASE(noteActiveNodeUpdatesKnownNode)
502502
auto& nodeTable = nodeTableHost.nodeTable;
503503
auto knownNode = nodeTable->bucketFirstNode(bucketIndex);
504504

505-
nodeTable->noteActiveNode(knownNode, knownNode->endpoint);
505+
nodeTable->noteActiveNode(knownNode);
506506

507507
// check that node was moved to the back of the bucket
508508
BOOST_CHECK_NE(nodeTable->bucketFirstNode(bucketIndex), knownNode);
@@ -552,32 +552,6 @@ BOOST_AUTO_TEST_CASE(noteActiveNodeEvictsTheNodeWhenBucketIsFull)
552552
BOOST_CHECK_EQUAL(evicted->replacementNodeEntry->id, newNodeId);
553553
}
554554

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

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

11611257
BOOST_FIXTURE_TEST_SUITE(netTypes, TestOutputHelperFixture)

0 commit comments

Comments
 (0)