From a7a59a2b1b032e9b986f02797e595f18d4f0649d Mon Sep 17 00:00:00 2001
From: zzz <zzz@i2pmail.org>
Date: Tue, 5 Oct 2021 08:40:30 -0400
Subject: [PATCH] NetDB: Reduce ban time for routers without netId
Don't ban routers with bad netId before RI validation,
unless that router sent the RI
---
router/java/src/net/i2p/router/Banlist.java | 3 +-
.../KademliaNetworkDatabaseFacade.java | 11 ++-
.../transport/ntcp/InboundEstablishState.java | 14 ++--
.../router/transport/ntcp/NTCPTransport.java | 9 +-
.../transport/udp/EstablishmentManager.java | 9 +-
.../router/transport/udp/UDPTransport.java | 84 +++++++------------
6 files changed, 63 insertions(+), 67 deletions(-)
diff --git a/router/java/src/net/i2p/router/Banlist.java b/router/java/src/net/i2p/router/Banlist.java
index 4df19899a9..0d2f9603e5 100644
--- a/router/java/src/net/i2p/router/Banlist.java
+++ b/router/java/src/net/i2p/router/Banlist.java
@@ -53,7 +53,8 @@ public class Banlist {
public final static long BANLIST_DURATION_MAX = 30*60*1000;
public final static long BANLIST_DURATION_PARTIAL = 10*60*1000;
public final static long BANLIST_DURATION_FOREVER = 181l*24*60*60*1000; // will get rounded down to 180d on console
- public final static long BANLIST_CLEANER_START_DELAY = BANLIST_DURATION_PARTIAL;
+ public final static long BANLIST_DURATION_NO_NETWORK = 6*60*60*1000;
+ private final static long BANLIST_CLEANER_START_DELAY = BANLIST_DURATION_PARTIAL;
public Banlist(RouterContext context) {
_context = context;
diff --git a/router/java/src/net/i2p/router/networkdb/kademlia/KademliaNetworkDatabaseFacade.java b/router/java/src/net/i2p/router/networkdb/kademlia/KademliaNetworkDatabaseFacade.java
index d878802559..10612b8b91 100644
--- a/router/java/src/net/i2p/router/networkdb/kademlia/KademliaNetworkDatabaseFacade.java
+++ b/router/java/src/net/i2p/router/networkdb/kademlia/KademliaNetworkDatabaseFacade.java
@@ -42,6 +42,7 @@ import net.i2p.data.router.RouterInfo;
import net.i2p.kademlia.KBucketSet;
import net.i2p.kademlia.RejectTrimmer;
import net.i2p.kademlia.SelectionCollector;
+import net.i2p.router.Banlist;
import net.i2p.router.Job;
import net.i2p.router.NetworkDatabaseFacade;
import net.i2p.router.Router;
@@ -1110,8 +1111,14 @@ public abstract class KademliaNetworkDatabaseFacade extends NetworkDatabaseFacad
_log.warn("Invalid routerInfo signature! forged router structure! router = " + routerInfo);
return "Invalid routerInfo signature";
}
- if (routerInfo.getNetworkId() != _networkID){
- _context.banlist().banlistRouterForever(key, "Not in our network: " + routerInfo.getNetworkId());
+ int id = routerInfo.getNetworkId();
+ if (id != _networkID){
+ if (id == -1) {
+ // old i2pd bug, possibly at startup, don't ban forever
+ _context.banlist().banlistRouter(key, "No network specified", null, null, _context.clock().now() + Banlist.BANLIST_DURATION_NO_NETWORK);
+ } else {
+ _context.banlist().banlistRouterForever(key, "Not in our network: " + id);
+ }
if (_log.shouldLog(Log.WARN))
_log.warn("Not in our network: " + routerInfo, new Exception());
return "Not in our network";
diff --git a/router/java/src/net/i2p/router/transport/ntcp/InboundEstablishState.java b/router/java/src/net/i2p/router/transport/ntcp/InboundEstablishState.java
index 798db55cae..ba6dccc97e 100644
--- a/router/java/src/net/i2p/router/transport/ntcp/InboundEstablishState.java
+++ b/router/java/src/net/i2p/router/transport/ntcp/InboundEstablishState.java
@@ -230,11 +230,9 @@ class InboundEstablishState extends EstablishBase implements NTCP2Payload.Payloa
/**
* Validate network ID, NTCP 2 only.
- * Call after receiving Alice's RouterInfo,
- * but before storing it in the netdb.
+ * Call if storing in netdb fails.
*
- * Side effects: When returning false, sets _msg3p2FailReason,
- * banlists permanently and blocklists
+ * Side effects: When returning false, sets _msg3p2FailReason
*
* @return success
* @since 0.9.38
@@ -253,7 +251,6 @@ class InboundEstablishState extends EstablishBase implements NTCP2Payload.Payloa
byte[] ip = addr.getAddress();
_context.blocklist().add(ip);
}
- _context.banlist().banlistRouterForever(aliceHash, "Not in our network: " + aliceID);
_transport.markUnreachable(aliceHash);
_msg3p2FailReason = NTCPConnection.REASON_BANNED;
}
@@ -652,9 +649,6 @@ class InboundEstablishState extends EstablishBase implements NTCP2Payload.Payloa
boolean ok = verifyInbound(h);
if (!ok)
throw new DataFormatException("NTCP2 verifyInbound() fail");
- ok = verifyInboundNetworkID(ri);
- if (!ok)
- throw new DataFormatException("NTCP2 network ID mismatch");
try {
RouterInfo old = _context.netDb().store(h, ri);
if (flood && !ri.equals(old)) {
@@ -668,6 +662,10 @@ class InboundEstablishState extends EstablishBase implements NTCP2Payload.Payloa
}
}
} catch (IllegalArgumentException iae) {
+ // sets _msg3p2FailReason
+ ok = verifyInboundNetworkID(ri);
+ if (!ok)
+ throw new DataFormatException("NTCP2 network ID mismatch");
// hash collision?
// expired RI?
_msg3p2FailReason = NTCPConnection.REASON_MSG3;
diff --git a/router/java/src/net/i2p/router/transport/ntcp/NTCPTransport.java b/router/java/src/net/i2p/router/transport/ntcp/NTCPTransport.java
index 23ee8469be..fc70ab0f50 100644
--- a/router/java/src/net/i2p/router/transport/ntcp/NTCPTransport.java
+++ b/router/java/src/net/i2p/router/transport/ntcp/NTCPTransport.java
@@ -39,6 +39,7 @@ import net.i2p.data.router.RouterIdentity;
import net.i2p.data.router.RouterInfo;
import net.i2p.data.i2np.DatabaseStoreMessage;
import net.i2p.data.i2np.I2NPMessage;
+import net.i2p.router.Banlist;
import net.i2p.router.CommSystemFacade.Status;
import net.i2p.router.OutNetMessage;
import net.i2p.router.RouterContext;
@@ -511,8 +512,12 @@ public class NTCPTransport extends TransportImpl {
if (established) { // should we check the queue size? nah, if its valid, use it
return _fastBid;
}
- if (toAddress.getNetworkId() != _networkID) {
- _context.banlist().banlistRouterForever(peer, "Not in our network: " + toAddress.getNetworkId());
+ int nid = toAddress.getNetworkId();
+ if (nid != _networkID) {
+ if (nid == -1)
+ _context.banlist().banlistRouter(peer, "No network specified", null, null, _context.clock().now() + Banlist.BANLIST_DURATION_NO_NETWORK);
+ else
+ _context.banlist().banlistRouterForever(peer, "Not in our network: " + nid);
if (_log.shouldWarn())
_log.warn("Not in our network: " + toAddress, new Exception());
markUnreachable(peer);
diff --git a/router/java/src/net/i2p/router/transport/udp/EstablishmentManager.java b/router/java/src/net/i2p/router/transport/udp/EstablishmentManager.java
index e649bfbc43..d7c782af06 100644
--- a/router/java/src/net/i2p/router/transport/udp/EstablishmentManager.java
+++ b/router/java/src/net/i2p/router/transport/udp/EstablishmentManager.java
@@ -17,6 +17,7 @@ import net.i2p.data.SessionKey;
import net.i2p.data.i2np.DatabaseStoreMessage;
import net.i2p.data.i2np.DeliveryStatusMessage;
import net.i2p.data.i2np.I2NPMessage;
+import net.i2p.router.Banlist;
import net.i2p.router.OutNetMessage;
import net.i2p.router.Router;
import net.i2p.router.RouterContext;
@@ -249,8 +250,12 @@ class EstablishmentManager {
}
RouterIdentity toIdentity = toRouterInfo.getIdentity();
Hash toHash = toIdentity.calculateHash();
- if (toRouterInfo.getNetworkId() != _networkID) {
- _context.banlist().banlistRouterForever(toHash, "Not in our network: " + toRouterInfo.getNetworkId());
+ int id = toRouterInfo.getNetworkId();
+ if (id != _networkID) {
+ if (id == -1)
+ _context.banlist().banlistRouter(toHash, "No network specified", null, null, _context.clock().now() + Banlist.BANLIST_DURATION_NO_NETWORK);
+ else
+ _context.banlist().banlistRouterForever(toHash, "Not in our network: " + id);
if (_log.shouldWarn())
_log.warn("Not in our network: " + toRouterInfo, new Exception());
_transport.markUnreachable(toHash);
diff --git a/router/java/src/net/i2p/router/transport/udp/UDPTransport.java b/router/java/src/net/i2p/router/transport/udp/UDPTransport.java
index 98432b5f89..b96a172a0c 100644
--- a/router/java/src/net/i2p/router/transport/udp/UDPTransport.java
+++ b/router/java/src/net/i2p/router/transport/udp/UDPTransport.java
@@ -33,6 +33,7 @@ import net.i2p.data.router.RouterInfo;
import net.i2p.data.SessionKey;
import net.i2p.data.i2np.DatabaseStoreMessage;
import net.i2p.data.i2np.I2NPMessage;
+import net.i2p.router.Banlist;
import net.i2p.router.CommSystemFacade;
import net.i2p.router.CommSystemFacade.Status;
import net.i2p.router.OutNetMessage;
@@ -1628,60 +1629,35 @@ public class UDPTransport extends TransportImpl implements TimedWeightedPriority
DatabaseEntry entry = dsm.getEntry();
if (entry == null)
return;
- if (entry.getType() == DatabaseEntry.KEY_TYPE_ROUTERINFO &&
- ((RouterInfo) entry).getNetworkId() != _networkID) {
- // this is pre-0.6.1.10, so it isn't going to happen any more
-
- /*
- if (remoteIdentHash != null) {
- _context.banlist().banlistRouter(remoteIdentHash, "Sent us a peer from the wrong network");
- dropPeer(remoteIdentHash);
- if (_log.shouldLog(Log.ERROR))
- _log.error("Dropping the peer " + remoteIdentHash
- + " because they are in the wrong net");
- } else if (remoteIdent != null) {
- _context.banlist().banlistRouter(remoteIdent.calculateHash(), "Sent us a peer from the wrong network");
- dropPeer(remoteIdent.calculateHash());
- if (_log.shouldLog(Log.ERROR))
- _log.error("Dropping the peer " + remoteIdent.calculateHash()
- + " because they are in the wrong net");
- }
- */
- Hash peerHash = entry.getHash();
- PeerState peer = getPeerState(peerHash);
- if (peer != null) {
- RemoteHostId remote = peer.getRemoteHostId();
- _dropList.add(remote);
- _context.statManager().addRateData("udp.dropPeerDroplist", 1);
- _context.simpleTimer2().addEvent(new RemoveDropList(remote), DROPLIST_PERIOD);
- }
- markUnreachable(peerHash);
- _context.banlist().banlistRouterForever(peerHash, "Not in our network: " + ((RouterInfo) entry).getNetworkId());
- //_context.banlist().banlistRouter(peerHash, "Part of the wrong network", STYLE);
- if (peer != null)
- sendDestroy(peer);
- dropPeer(peerHash, false, "Not in our network");
- if (_log.shouldLog(Log.WARN))
- _log.warn("Not in our network: " + entry, new Exception());
- return;
- } else {
- if (entry.getType() == DatabaseEntry.KEY_TYPE_ROUTERINFO) {
- if (_log.shouldLog(Log.DEBUG))
- _log.debug("Received an RI from the same net");
- } else {
- if (_log.shouldLog(Log.DEBUG))
- _log.debug("Received a leaseSet: " + dsm);
+ if (entry.getType() == DatabaseEntry.KEY_TYPE_ROUTERINFO) {
+ RouterInfo ri = (RouterInfo) entry;
+ int id = ri.getNetworkId();
+ if (id != _networkID) {
+ Hash peerHash = entry.getHash();
+ if (peerHash.equals(remoteIdentHash)) {
+ PeerState peer = getPeerState(peerHash);
+ if (peer != null) {
+ RemoteHostId remote = peer.getRemoteHostId();
+ _dropList.add(remote);
+ _context.statManager().addRateData("udp.dropPeerDroplist", 1);
+ _context.simpleTimer2().addEvent(new RemoveDropList(remote), DROPLIST_PERIOD);
+ }
+ markUnreachable(peerHash);
+ if (id == -1)
+ _context.banlist().banlistRouter(peerHash, "No network specified", null, null, _context.clock().now() + Banlist.BANLIST_DURATION_NO_NETWORK);
+ else
+ _context.banlist().banlistRouterForever(peerHash, "Not in our network: " + id);
+ if (peer != null)
+ sendDestroy(peer);
+ dropPeer(peerHash, false, "Not in our network");
+ if (_log.shouldLog(Log.WARN))
+ _log.warn("Not in our network: " + entry, new Exception());
+ return;
+ } // else will be invalidated and handled by netdb
}
}
- } else {
- //if (_log.shouldLog(Log.DEBUG))
- // _log.debug("Received another message: " + inMsg.getClass().getName());
}
- //PeerState peer = getPeerState(remoteIdentHash);
super.messageReceived(inMsg, remoteIdent, remoteIdentHash, msToReceive, bytesReceived);
- // Called in IMF, not needed here too
- //if (peer != null)
- // peer.expireInboundMessages();
}
private class RemoveDropList implements SimpleTimer.TimedEvent {
@@ -2036,8 +2012,12 @@ public class UDPTransport extends TransportImpl implements TimedWeightedPriority
else
return _cachedBid[FAST_BID];
} else {
- if (toAddress.getNetworkId() != _networkID) {
- _context.banlist().banlistRouterForever(to, "Not in our network: " + toAddress.getNetworkId());
+ int nid = toAddress.getNetworkId();
+ if (nid != _networkID) {
+ if (nid == -1)
+ _context.banlist().banlistRouter(to, "No network specified", null, null, _context.clock().now() + Banlist.BANLIST_DURATION_NO_NETWORK);
+ else
+ _context.banlist().banlistRouterForever(to, "Not in our network: " + nid);
markUnreachable(to);
return null;
}
--
GitLab