From 692cd7adae793bce93d5c9a43d3c8386fc9ca471 Mon Sep 17 00:00:00 2001 From: jrandom <jrandom> Date: Tue, 17 Aug 2004 20:37:47 +0000 Subject: [PATCH] * reduced the period used to detect / avoid peers who send invalid data (60m instead of 120m) * expose the reason for a dbStore rejection more cleanly --- .../HandleDatabaseStoreMessageJob.java | 15 +++-- .../KademliaNetworkDatabaseFacade.java | 57 +++++++++++-------- .../kademlia/SearchUpdateReplyFoundJob.java | 4 ++ .../net/i2p/router/peermanager/DBHistory.java | 2 +- .../router/peermanager/ProfileOrganizer.java | 2 +- 5 files changed, 47 insertions(+), 33 deletions(-) diff --git a/router/java/src/net/i2p/router/networkdb/HandleDatabaseStoreMessageJob.java b/router/java/src/net/i2p/router/networkdb/HandleDatabaseStoreMessageJob.java index d7fa4479fd..37e8c32872 100644 --- a/router/java/src/net/i2p/router/networkdb/HandleDatabaseStoreMessageJob.java +++ b/router/java/src/net/i2p/router/networkdb/HandleDatabaseStoreMessageJob.java @@ -48,16 +48,14 @@ public class HandleDatabaseStoreMessageJob extends JobImpl { if (_log.shouldLog(Log.DEBUG)) _log.debug("Handling database store message"); - boolean invalid = false; + String invalidMessage = null; boolean wasNew = false; if (_message.getValueType() == DatabaseStoreMessage.KEY_TYPE_LEASESET) { try { Object match = getContext().netDb().store(_message.getKey(), _message.getLeaseSet()); wasNew = (null == match); } catch (IllegalArgumentException iae) { - if (_log.shouldLog(Log.WARN)) - _log.warn("Not storing a leaseSet", iae); - invalid = true; + invalidMessage = iae.getMessage(); } } else if (_message.getValueType() == DatabaseStoreMessage.KEY_TYPE_ROUTERINFO) { if (_log.shouldLog(Log.INFO)) @@ -68,9 +66,7 @@ public class HandleDatabaseStoreMessageJob extends JobImpl { wasNew = (null == match); getContext().profileManager().heardAbout(_message.getKey()); } catch (IllegalArgumentException iae) { - if (_log.shouldLog(Log.WARN)) - _log.warn("Not storing a routerInfo", iae); - invalid = true; + invalidMessage = iae.getMessage(); } } else { if (_log.shouldLog(Log.ERROR)) @@ -84,9 +80,12 @@ public class HandleDatabaseStoreMessageJob extends JobImpl { if (_from != null) _fromHash = _from.getHash(); if (_fromHash != null) { - if (!invalid) { + if (invalidMessage == null) { getContext().profileManager().dbStoreReceived(_fromHash, wasNew); getContext().statManager().addRateData("netDb.storeHandled", 1, 0); + } else { + if (_log.shouldLog(Log.WARN)) + _log.warn("Peer " + _fromHash.toBase64() + " sent bad data: " + invalidMessage); } } } 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 023e2683b7..d6e742b8f8 100644 --- a/router/java/src/net/i2p/router/networkdb/kademlia/KademliaNetworkDatabaseFacade.java +++ b/router/java/src/net/i2p/router/networkdb/kademlia/KademliaNetworkDatabaseFacade.java @@ -437,31 +437,36 @@ public class KademliaNetworkDatabaseFacade extends NetworkDatabaseFacade { * Determine whether this leaseSet will be accepted as valid and current * given what we know now. * + * @return reason why the entry is not valid, or null if it is valid */ - boolean validate(Hash key, LeaseSet leaseSet) { + String validate(Hash key, LeaseSet leaseSet) { if (!key.equals(leaseSet.getDestination().calculateHash())) { - if (_log.shouldLog(Log.ERROR)) - _log.error("Invalid store attempt! key does not match leaseSet.destination! key = " - + key + ", leaseSet = " + leaseSet); - return false; + if (_log.shouldLog(Log.WARN)) + _log.warn("Invalid store attempt! key does not match leaseSet.destination! key = " + + key + ", leaseSet = " + leaseSet); + return "Key does not match leaseSet.destination - " + key.toBase64(); } else if (!leaseSet.verifySignature()) { - if (_log.shouldLog(Log.ERROR)) - _log.error("Invalid leaseSet signature! leaseSet = " + leaseSet); - return false; + if (_log.shouldLog(Log.WARN)) + _log.warn("Invalid leaseSet signature! leaseSet = " + leaseSet); + return "Invalid leaseSet signature on " + leaseSet.getDestination().calculateHash().toBase64(); } else if (leaseSet.getEarliestLeaseDate() <= _context.clock().now() - Router.CLOCK_FUDGE_FACTOR) { + long age = _context.clock().now() - leaseSet.getEarliestLeaseDate(); if (_log.shouldLog(Log.WARN)) _log.warn("Old leaseSet! not storing it: " + leaseSet.getDestination().calculateHash().toBase64() + " expires on " + new Date(leaseSet.getEarliestLeaseDate()), new Exception("Rejecting store")); - return false; + return "Expired leaseSet for " + leaseSet.getDestination().calculateHash().toBase64() + + " expired " + DataHelper.formatDuration(age) + " ago"; } else if (leaseSet.getEarliestLeaseDate() > _context.clock().now() + Router.CLOCK_FUDGE_FACTOR + MAX_LEASE_FUTURE) { + long age = leaseSet.getEarliestLeaseDate() - _context.clock().now(); if (_log.shouldLog(Log.WARN)) _log.warn("LeaseSet to expire too far in the future: " + leaseSet.getDestination().calculateHash().toBase64() + " expires on " + new Date(leaseSet.getEarliestLeaseDate()), new Exception("Rejecting store")); - return false; + return "Expired leaseSet for " + leaseSet.getDestination().calculateHash().toBase64() + + " expiring in " + DataHelper.formatDuration(age); } - return true; + return null; } /** @@ -472,8 +477,9 @@ public class KademliaNetworkDatabaseFacade extends NetworkDatabaseFacade { public LeaseSet store(Hash key, LeaseSet leaseSet) throws IllegalArgumentException { if (!_initialized) return null; - boolean valid = validate(key, leaseSet); - if (!valid) throw new IllegalArgumentException("LeaseSet is not valid"); + String err = validate(key, leaseSet); + if (err != null) + throw new IllegalArgumentException("Invalid store attempt - " + err); LeaseSet rv = null; if (_ds.isKnown(key)) @@ -509,21 +515,24 @@ public class KademliaNetworkDatabaseFacade extends NetworkDatabaseFacade { * given what we know now. * */ - boolean validate(Hash key, RouterInfo routerInfo) { + String validate(Hash key, RouterInfo routerInfo) throws IllegalArgumentException { long now = _context.clock().now(); if (!key.equals(routerInfo.getIdentity().getHash())) { - _log.error("Invalid store attempt! key does not match routerInfo.identity! key = " + key + ", router = " + routerInfo); - return false; + if (_log.shouldLog(Log.WARN)) + _log.warn("Invalid store attempt! key does not match routerInfo.identity! key = " + key + ", router = " + routerInfo); + return "Key does not match routerInfo.identity - " + key.toBase64(); } else if (!routerInfo.isValid()) { - _log.error("Invalid routerInfo signature! forged router structure! router = " + routerInfo); - return false; + if (_log.shouldLog(Log.WARN)) + _log.warn("Invalid routerInfo signature! forged router structure! router = " + routerInfo); + return "Invalid routerInfo signature on " + key.toBase64(); } else if (!routerInfo.isCurrent(Router.CLOCK_FUDGE_FACTOR + ExpireRoutersJob.EXPIRE_DELAY)) { + long age = _context.clock().now() - routerInfo.getPublished(); int existing = _kb.size(); if (existing >= MIN_REMAINING_ROUTERS) { if (_log.shouldLog(Log.INFO)) _log.info("Not storing expired router for " + key.toBase64(), new Exception("Rejecting store")); - return false; + return "Peer " + key.toBase64() + " expired " + DataHelper.formatDuration(age) + "ms ago"; } else { if (_log.shouldLog(Log.WARN)) _log.warn("Even though the peer is old, we have only " + existing @@ -531,12 +540,13 @@ public class KademliaNetworkDatabaseFacade extends NetworkDatabaseFacade { + new Date(routerInfo.getPublished())); } } else if (routerInfo.getPublished() > now + Router.CLOCK_FUDGE_FACTOR) { + long age = routerInfo.getPublished() - _context.clock().now(); if (_log.shouldLog(Log.WARN)) _log.warn("Peer " + key.toBase64() + " published their routerInfo in the future?! [" + new Date(routerInfo.getPublished()) + "]", new Exception("Rejecting store")); - return false; + return "Peer " + key.toBase64() + " published " + DataHelper.formatDuration(age) + " in the future?!"; } - return true; + return null; } /** @@ -547,8 +557,9 @@ public class KademliaNetworkDatabaseFacade extends NetworkDatabaseFacade { public RouterInfo store(Hash key, RouterInfo routerInfo) throws IllegalArgumentException { if (!_initialized) return null; - boolean valid = validate(key, routerInfo); - if (!valid) throw new IllegalArgumentException("LeaseSet is not valid"); + String err = validate(key, routerInfo); + if (err != null) + throw new IllegalArgumentException("Invalid store attempt - " + err); RouterInfo rv = null; if (_ds.isKnown(key)) diff --git a/router/java/src/net/i2p/router/networkdb/kademlia/SearchUpdateReplyFoundJob.java b/router/java/src/net/i2p/router/networkdb/kademlia/SearchUpdateReplyFoundJob.java index 77922f1340..1618b4cade 100644 --- a/router/java/src/net/i2p/router/networkdb/kademlia/SearchUpdateReplyFoundJob.java +++ b/router/java/src/net/i2p/router/networkdb/kademlia/SearchUpdateReplyFoundJob.java @@ -50,6 +50,8 @@ class SearchUpdateReplyFoundJob extends JobImpl implements ReplyJob { _facade.store(msg.getKey(), msg.getLeaseSet()); getContext().profileManager().dbLookupSuccessful(_peer, timeToReply); } catch (IllegalArgumentException iae) { + if (_log.shouldLog(Log.ERROR)) + _log.warn("Peer " + _peer + " sent us an invalid leaseSet: " + iae.getMessage()); getContext().profileManager().dbLookupReply(_peer, 0, 0, 1, 0, timeToReply); } } else if (msg.getValueType() == DatabaseStoreMessage.KEY_TYPE_ROUTERINFO) { @@ -61,6 +63,8 @@ class SearchUpdateReplyFoundJob extends JobImpl implements ReplyJob { _facade.store(msg.getKey(), msg.getRouterInfo()); getContext().profileManager().dbLookupSuccessful(_peer, timeToReply); } catch (IllegalArgumentException iae) { + if (_log.shouldLog(Log.ERROR)) + _log.warn("Peer " + _peer + " sent us an invalid routerInfo: " + iae.getMessage()); getContext().profileManager().dbLookupReply(_peer, 0, 0, 1, 0, timeToReply); } } else { diff --git a/router/java/src/net/i2p/router/peermanager/DBHistory.java b/router/java/src/net/i2p/router/peermanager/DBHistory.java index 8c381c7fb7..0b3307885c 100644 --- a/router/java/src/net/i2p/router/peermanager/DBHistory.java +++ b/router/java/src/net/i2p/router/peermanager/DBHistory.java @@ -220,7 +220,7 @@ public class DBHistory { if (_failedLookupRate == null) _failedLookupRate = new RateStat("dbHistory.failedLookupRate", "How often does this peer to respond to a lookup?", "dbHistory", new long[] { 60*1000l, 60*60*1000l, 24*60*60*1000l }); if (_invalidReplyRate == null) - _invalidReplyRate = new RateStat("dbHistory.invalidReplyRate", "How often does this peer give us a bad (nonexistant, forged, etc) peer?", "dbHistory", new long[] { 60*1000l, 60*60*1000l, 24*60*60*1000l }); + _invalidReplyRate = new RateStat("dbHistory.invalidReplyRate", "How often does this peer give us a bad (nonexistant, forged, etc) peer?", "dbHistory", new long[] { 30*60*1000l, 60*60*1000l, 24*60*60*1000l }); } private final static long getLong(Properties props, String key) { diff --git a/router/java/src/net/i2p/router/peermanager/ProfileOrganizer.java b/router/java/src/net/i2p/router/peermanager/ProfileOrganizer.java index c6f1c919e5..803c0a4428 100644 --- a/router/java/src/net/i2p/router/peermanager/ProfileOrganizer.java +++ b/router/java/src/net/i2p/router/peermanager/ProfileOrganizer.java @@ -217,7 +217,7 @@ public class ProfileOrganizer { PeerProfile profile = getProfile(peer); if (profile != null) { RateStat invalidReplyRateStat = profile.getDBHistory().getInvalidReplyRate(); - Rate invalidReplyRate = invalidReplyRateStat.getRate(60*60*1000l); + Rate invalidReplyRate = invalidReplyRateStat.getRate(30*60*1000l); if ( (invalidReplyRate.getCurrentTotalValue() > MAX_BAD_REPLIES_PER_HOUR) || (invalidReplyRate.getLastTotalValue() > MAX_BAD_REPLIES_PER_HOUR) ) { return true; -- GitLab