From 09cf973712a616ec1c310e015c8e99cc0768ce0d Mon Sep 17 00:00:00 2001
From: zzz <zzz@mail.i2p>
Date: Thu, 4 Sep 2014 01:08:23 +0000
Subject: [PATCH] BuildHandler: Enforce request record timestamp
 BuildRequestor: Randomize timestamp to prevent hop ID at top of hour

---
 .../net/i2p/data/i2np/BuildRequestRecord.java |  6 ++--
 .../i2p/router/tunnel/pool/BuildHandler.java  | 33 +++++++++++++++----
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/router/java/src/net/i2p/data/i2np/BuildRequestRecord.java b/router/java/src/net/i2p/data/i2np/BuildRequestRecord.java
index 10533cfac8..805c8cc4d7 100644
--- a/router/java/src/net/i2p/data/i2np/BuildRequestRecord.java
+++ b/router/java/src/net/i2p/data/i2np/BuildRequestRecord.java
@@ -146,10 +146,10 @@ public class BuildRequestRecord {
         return (_data.getData()[_data.getOffset() + OFF_FLAG] & FLAG_OUTBOUND_ENDPOINT) != 0;
     }
     /**
-     * Time that the request was sent, truncated to the nearest hour
+     * Time that the request was sent (ms), truncated to the nearest hour
      */
     public long readRequestTime() {
-        return DataHelper.fromLong(_data.getData(), _data.getOffset() + OFF_REQ_TIME, 4) * 60l * 60l * 1000l;
+        return DataHelper.fromLong(_data.getData(), _data.getOffset() + OFF_REQ_TIME, 4) * (60 * 60 * 1000L);
     }
     /**
      * What message ID should we send the request to the next hop with.  If this is the outbound tunnel endpoint,
@@ -250,6 +250,8 @@ public class BuildRequestRecord {
         else if (isOutEndpoint)
             buf[OFF_FLAG] |= FLAG_OUTBOUND_ENDPOINT;
         long truncatedHour = ctx.clock().now();
+        // prevent hop identification at top of the hour
+        truncatedHour -= ctx.random().nextInt(90*1000);
         truncatedHour /= (60l*60l*1000l);
         DataHelper.toLong(buf, OFF_REQ_TIME, 4, truncatedHour);
         DataHelper.toLong(buf, OFF_SEND_MSG_ID, 4, nextMsgId);
diff --git a/router/java/src/net/i2p/router/tunnel/pool/BuildHandler.java b/router/java/src/net/i2p/router/tunnel/pool/BuildHandler.java
index 3522e5c4ed..0f914fbcc0 100644
--- a/router/java/src/net/i2p/router/tunnel/pool/BuildHandler.java
+++ b/router/java/src/net/i2p/router/tunnel/pool/BuildHandler.java
@@ -85,6 +85,11 @@ class BuildHandler implements Runnable {
      */
     private static final int NEXT_HOP_SEND_TIMEOUT = 25*1000;
 
+    private static final long MAX_REQUEST_FUTURE = 5*60*1000;
+    /** must be > 1 hour due to rouding down */
+    private static final long MAX_REQUEST_AGE = 65*60*1000;
+
+
     public BuildHandler(RouterContext ctx, TunnelPoolManager manager, BuildExecutor exec) {
         _context = ctx;
         _log = ctx.logManager().getLog(getClass());
@@ -101,8 +106,10 @@ class BuildHandler implements Runnable {
         _context.statManager().createRateStat("tunnel.reject.50", "How often we reject a tunnel because of a critical issue (shutdown, etc)", "Tunnels", new long[] { 60*1000, 10*60*1000 });
 
         _context.statManager().createRequiredRateStat("tunnel.decryptRequestTime", "Time to decrypt a build request (ms)", "Tunnels", new long[] { 60*1000, 10*60*1000 });
-        _context.statManager().createRequiredRateStat("tunnel.rejectTimeout", "Reject tunnel count (unknown next hop)", "Tunnels", new long[] { 60*1000, 10*60*1000 });
-        _context.statManager().createRequiredRateStat("tunnel.rejectTimeout2", "Reject tunnel count (can't contact next hop)", "Tunnels", new long[] { 60*1000, 10*60*1000 });
+        _context.statManager().createRateStat("tunnel.rejectTooOld", "Reject tunnel count (too old)", "Tunnels", new long[] { 3*60*60*1000 });
+        _context.statManager().createRateStat("tunnel.rejectFuture", "Reject tunnel count (time in future)", "Tunnels", new long[] { 3*60*60*1000 });
+        _context.statManager().createRateStat("tunnel.rejectTimeout", "Reject tunnel count (unknown next hop)", "Tunnels", new long[] { 60*60*1000 });
+        _context.statManager().createRateStat("tunnel.rejectTimeout2", "Reject tunnel count (can't contact next hop)", "Tunnels", new long[] { 60*60*1000 });
         _context.statManager().createRequiredRateStat("tunnel.rejectDupID", "Part. tunnel dup ID", "Tunnels", new long[] { 24*60*60*1000 });
         _context.statManager().createRequiredRateStat("tunnel.ownDupID", "Our tunnel dup. ID", "Tunnels", new long[] { 24*60*60*1000 });
         _context.statManager().createRequiredRateStat("tunnel.rejectHostile", "Reject malicious tunnel", "Tunnels", new long[] { 24*60*60*1000 });
@@ -587,11 +594,24 @@ class BuildHandler implements Runnable {
             }
         }
 
-        // time is in hours, and only for log below - what's the point?
-        // tunnel-alt-creation.html specifies that this is enforced +/- 1 hour but it is not.
+        // time is in hours, rounded down.
+        // tunnel-alt-creation.html specifies that this is enforced +/- 1 hour but it was not.
+        // As of 0.9.16, allow + 5 minutes to - 65 minutes.
         long time = req.readRequestTime();
         long now = (_context.clock().now() / (60l*60l*1000l)) * (60*60*1000);
-        int ourSlot = -1;
+        long timeDiff = now - time;
+        if (timeDiff > MAX_REQUEST_AGE) {
+            _context.statManager().addRateData("tunnel.rejectTooOld", 1);
+            if (_log.shouldLog(Log.WARN))
+                _log.warn("Dropping build request too old... replay attack? " + DataHelper.formatDuration(timeDiff));
+            return;
+        }
+        if (timeDiff < 0 - MAX_REQUEST_FUTURE) {
+            _context.statManager().addRateData("tunnel.rejectFuture", 1);
+            if (_log.shouldLog(Log.WARN))
+                _log.warn("Dropping build request too far in future " + DataHelper.formatDuration(0 - timeDiff));
+            return;
+        }
 
         int response;
         if (_context.router().isHidden()) {
@@ -764,6 +784,7 @@ class BuildHandler implements Runnable {
 
         byte reply[] = BuildResponseRecord.create(_context, response, req.readReplyKey(), req.readReplyIV(), state.msg.getUniqueId());
         int records = state.msg.getRecordCount();
+        int ourSlot = -1;
         for (int j = 0; j < records; j++) {
             if (state.msg.getRecord(j) == null) {
                 ourSlot = j;
@@ -780,7 +801,7 @@ class BuildHandler implements Runnable {
                       + " accepted? " + response + " receiving on " + ourId 
                       + " sending to " + nextId
                       + " on " + nextPeer
-                      + " inGW? " + isInGW + " outEnd? " + isOutEnd + " time difference " + (now-time)
+                      + " inGW? " + isInGW + " outEnd? " + isOutEnd
                       + " recvDelay " + recvDelay + " replyMessage " + req.readReplyMessageId()
                       + " replyKey " + req.readReplyKey() + " replyIV " + Base64.encode(req.readReplyIV()));
 
-- 
GitLab