From fba037233956cc230fb987edc49b5c13ee99fddd Mon Sep 17 00:00:00 2001 From: zzz Date: Wed, 27 May 2015 21:00:46 +0000 Subject: [PATCH] Banlist: Ban all-zero hash NetDb: Drop all-zero lookups and stores, add stats SSU: - Fix debug logging of dumped packets - Drop sessions with bad clock skew, banlist peer, add stats - Drop sessions with corrupt DSM, banlist peer, add stats Log tweaks --- .../i2p/data/i2np/DatabaseStoreMessage.java | 7 ++ router/java/src/net/i2p/router/Banlist.java | 2 + .../HandleDatabaseLookupMessageJob.java | 8 +++ .../KademliaNetworkDatabaseFacade.java | 4 ++ .../router/transport/udp/MessageReceiver.java | 36 ++++++++++- .../router/transport/udp/PacketHandler.java | 64 +++++++++++++++++-- 6 files changed, 113 insertions(+), 8 deletions(-) diff --git a/router/java/src/net/i2p/data/i2np/DatabaseStoreMessage.java b/router/java/src/net/i2p/data/i2np/DatabaseStoreMessage.java index cff476b61..e9efdd641 100644 --- a/router/java/src/net/i2p/data/i2np/DatabaseStoreMessage.java +++ b/router/java/src/net/i2p/data/i2np/DatabaseStoreMessage.java @@ -103,6 +103,13 @@ public class DatabaseStoreMessage extends FastI2NPMessageImpl { int curIndex = offset; _key = Hash.create(data, curIndex); + // i2pd bug? Generally followed by corrupt gzipped content. + // Fast-fail here to save resources. + if (_key.equals(Hash.FAKE_HASH)) { + // createRateStat in KNDF + _context.statManager().addRateData("netDb.DSMAllZeros", 1); + throw new I2NPMessageException("DSM all zeros"); + } curIndex += Hash.HASH_LENGTH; // as of 0.9.18, ignore other 7 bits of the type byte, in preparation for future options diff --git a/router/java/src/net/i2p/router/Banlist.java b/router/java/src/net/i2p/router/Banlist.java index 180e9366f..41fb9612d 100644 --- a/router/java/src/net/i2p/router/Banlist.java +++ b/router/java/src/net/i2p/router/Banlist.java @@ -60,6 +60,8 @@ public class Banlist { _log = context.logManager().getLog(Banlist.class); _entries = new ConcurrentHashMap(16); _context.jobQueue().addJob(new Cleanup(_context)); + // i2pd bug? + banlistRouterForever(Hash.FAKE_HASH, "Invalid Hash"); } private class Cleanup extends JobImpl { diff --git a/router/java/src/net/i2p/router/networkdb/HandleDatabaseLookupMessageJob.java b/router/java/src/net/i2p/router/networkdb/HandleDatabaseLookupMessageJob.java index 5a4c3a00b..12d6c4e61 100644 --- a/router/java/src/net/i2p/router/networkdb/HandleDatabaseLookupMessageJob.java +++ b/router/java/src/net/i2p/router/networkdb/HandleDatabaseLookupMessageJob.java @@ -83,6 +83,14 @@ public class HandleDatabaseLookupMessageJob extends JobImpl { return; } + // i2pd bug? + if (_message.getSearchKey().equals(Hash.FAKE_HASH)) { + if (_log.shouldWarn()) + _log.warn("Zero lookup", new Exception()); + getContext().statManager().addRateData("netDb.DLMAllZeros", 1); + return; + } + DatabaseLookupMessage.Type lookupType = _message.getSearchType(); // only lookup once, then cast to correct type DatabaseEntry dbe = getContext().netDb().lookupLocally(_message.getSearchKey()); 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 fb89f60a5..11ffa9d27 100644 --- a/router/java/src/net/i2p/router/networkdb/kademlia/KademliaNetworkDatabaseFacade.java +++ b/router/java/src/net/i2p/router/networkdb/kademlia/KademliaNetworkDatabaseFacade.java @@ -171,6 +171,10 @@ public class KademliaNetworkDatabaseFacade extends NetworkDatabaseFacade { context.statManager().createRateStat("netDb.replyTimeout", "How long after a netDb send does the timeout expire (when the peer doesn't reply in time)?", "NetworkDatabase", new long[] { 60*60*1000l }); // following is for RepublishLeaseSetJob context.statManager().createRateStat("netDb.republishLeaseSetCount", "How often we republish a leaseSet?", "NetworkDatabase", new long[] { 60*60*1000l }); + // following is for DatabaseStoreMessage + context.statManager().createRateStat("netDb.DSMAllZeros", "Store with zero key", "NetworkDatabase", new long[] { 60*60*1000l }); + // following is for HandleDatabaseLookupMessageJob + context.statManager().createRateStat("netDb.DLMAllZeros", "Lookup with zero key", "NetworkDatabase", new long[] { 60*60*1000l }); } @Override diff --git a/router/java/src/net/i2p/router/transport/udp/MessageReceiver.java b/router/java/src/net/i2p/router/transport/udp/MessageReceiver.java index 01bca49ab..1b6bb03c0 100644 --- a/router/java/src/net/i2p/router/transport/udp/MessageReceiver.java +++ b/router/java/src/net/i2p/router/transport/udp/MessageReceiver.java @@ -4,6 +4,7 @@ import java.util.concurrent.BlockingQueue; import net.i2p.data.Base64; import net.i2p.data.ByteArray; +import net.i2p.data.i2np.DatabaseStoreMessage; import net.i2p.data.i2np.I2NPMessage; import net.i2p.data.i2np.I2NPMessageException; import net.i2p.data.i2np.I2NPMessageHandler; @@ -218,9 +219,27 @@ class MessageReceiver { return m; } catch (I2NPMessageException ime) { if (_log.shouldLog(Log.WARN)) { - _log.warn("Message invalid: " + state, ime); - _log.warn("DUMP:\n" + HexDump.dump(buf.getData(), 0, state.getCompleteSize())); - _log.warn("RAW:\n" + Base64.encode(buf.getData(), 0, state.getCompleteSize())); + ByteArray ba; + if (state.getFragmentCount() > 1) + ba = buf; + else + ba = state.getFragments()[0]; + byte[] data = ba.getData(); + _log.warn("Message invalid: " + state + + " PeerState: " + _transport.getPeerState(state.getFrom()) + + "\nDUMP:\n" + HexDump.dump(data, 0, state.getCompleteSize()) + + "\nRAW:\n" + Base64.encode(data, 0, state.getCompleteSize()), + ime); + } + if (state.getFragments()[0].getData()[0] == DatabaseStoreMessage.MESSAGE_TYPE) { + PeerState ps = _transport.getPeerState(state.getFrom()); + if (ps != null && ps.getRemotePort() == 65520) { + // distinct port of buggy router + _transport.sendDestroy(ps); + _transport.dropPeer(ps, true, "Corrupt DSM"); + _context.banlist().banlistRouterForever(state.getFrom(), + _x("Sent corrupt DSM")); + } } _context.messageHistory().droppedInboundMessage(state.getMessageId(), state.getFrom(), "error: " + ime.toString() + ": " + state.toString()); return null; @@ -234,4 +253,15 @@ class MessageReceiver { state.releaseResources(); } } + + /** + * Mark a string for extraction by xgettext and translation. + * Use this only in static initializers. + * It does not translate! + * @return s + * @since 0.9.20 + */ + private static final String _x(String s) { + return s; + } } diff --git a/router/java/src/net/i2p/router/transport/udp/PacketHandler.java b/router/java/src/net/i2p/router/transport/udp/PacketHandler.java index 29a7b69d8..736171b80 100644 --- a/router/java/src/net/i2p/router/transport/udp/PacketHandler.java +++ b/router/java/src/net/i2p/router/transport/udp/PacketHandler.java @@ -88,6 +88,7 @@ class PacketHandler { _context.statManager().createRateStat("udp.droppedInvalidEstablish.new", "How old the packet we dropped due to invalidity (even though we do not have any active establishment with the peer) was", "udp", UDPTransport.RATES); _context.statManager().createRateStat("udp.droppedInvalidInboundEstablish", "How old the packet we dropped due to invalidity (inbound establishment, bad key) was", "udp", UDPTransport.RATES); _context.statManager().createRateStat("udp.droppedInvalidSkew", "How skewed the packet we dropped due to invalidity (valid except bad skew) was", "udp", UDPTransport.RATES); + _context.statManager().createRateStat("udp.destroyedInvalidSkew", "Destroyed session due to bad skew", "udp", UDPTransport.RATES); //_context.statManager().createRateStat("udp.packetDequeueTime", "How long it takes the UDPReader to pull a packet off the inbound packet queue (when its slow)", "udp", UDPTransport.RATES); //_context.statManager().createRateStat("udp.packetVerifyTime", "How long it takes the PacketHandler to verify a data packet after dequeueing (period is dequeue time)", "udp", UDPTransport.RATES); //_context.statManager().createRateStat("udp.packetVerifyTimeSlow", "How long it takes the PacketHandler to verify a data packet after dequeueing when its slow (period is dequeue time)", "udp", UDPTransport.RATES); @@ -631,19 +632,61 @@ class PacketHandler { // this doesn't seem to work for big skews, we never get anything back, // so we have to wait for NTCP to do it _context.clock().setOffset(0 - skew, true); - if (skew != 0) + if (skew != 0) { _log.logAlways(Log.WARN, "NTP failure, UDP adjusting clock by " + DataHelper.formatDuration(Math.abs(skew))); + skew = 0; + } } if (skew > GRACE_PERIOD) { - if (_log.shouldLog(Log.WARN)) - _log.warn("Packet too far in the past: " + new Date(sendOn) + ": " + packet); _context.statManager().addRateData("udp.droppedInvalidSkew", skew); + if (state != null && skew > 4 * GRACE_PERIOD && state.getPacketsReceived() <= 0) { + _transport.sendDestroy(state); + _transport.dropPeer(state, true, "Clock skew"); + if (state.getRemotePort() == 65520) { + // distinct port of buggy router + _context.banlist().banlistRouterForever(state.getRemotePeer(), + _x("Excessive clock skew: {0}"), + DataHelper.formatDuration(skew)); + } else { + _context.banlist().banlistRouter(DataHelper.formatDuration(skew), + state.getRemotePeer(), + _x("Excessive clock skew: {0}")); + } + _context.statManager().addRateData("udp.destroyedInvalidSkew", skew); + if (_log.shouldWarn()) + _log.warn("Dropped conn, packet too far in the past: " + new Date(sendOn) + ": " + packet + + " PeerState: " + state); + } else { + if (_log.shouldWarn()) + _log.warn("Packet too far in the past: " + new Date(sendOn) + ": " + packet + + " PeerState: " + state); + } return; } else if (skew < 0 - GRACE_PERIOD) { - if (_log.shouldLog(Log.WARN)) - _log.warn("Packet too far in the future: " + new Date(sendOn) + ": " + packet); _context.statManager().addRateData("udp.droppedInvalidSkew", 0-skew); + if (state != null && skew < 0 - (4 * GRACE_PERIOD) && state.getPacketsReceived() <= 0) { + _transport.sendDestroy(state); + _transport.dropPeer(state, true, "Clock skew"); + if (state.getRemotePort() == 65520) { + // distinct port of buggy router + _context.banlist().banlistRouterForever(state.getRemotePeer(), + _x("Excessive clock skew: {0}"), + DataHelper.formatDuration(0 - skew)); + } else { + _context.banlist().banlistRouter(DataHelper.formatDuration(0 - skew), + state.getRemotePeer(), + _x("Excessive clock skew: {0}")); + } + _context.statManager().addRateData("udp.destroyedInvalidSkew", 0-skew); + if (_log.shouldWarn()) + _log.warn("Dropped conn, packet too far in the future: " + new Date(sendOn) + ": " + packet + + " PeerState: " + state); + } else { + if (_log.shouldWarn()) + _log.warn("Packet too far in the future: " + new Date(sendOn) + ": " + packet + + " PeerState: " + state); + } return; } @@ -795,4 +838,15 @@ class PacketHandler { } } } + + /** + * Mark a string for extraction by xgettext and translation. + * Use this only in static initializers. + * It does not translate! + * @return s + * @since 0.9.20 + */ + private static final String _x(String s) { + return s; + } }