From c65b4689d0253a8360ee18f0da2a97bb360cc86b Mon Sep 17 00:00:00 2001
From: zzz <zzz@i2pmail.org>
Date: Fri, 18 Jun 2021 09:11:42 -0400
Subject: [PATCH] SSU: Fix handling of bad peer test responses

Always abort test with an unknown result,
to prevent false firewalled indication.
Log tweaks
---
 .../router/transport/udp/PeerTestManager.java | 36 +++++++++++--------
 .../router/transport/udp/PeerTestState.java   |  3 +-
 2 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/router/java/src/net/i2p/router/transport/udp/PeerTestManager.java b/router/java/src/net/i2p/router/transport/udp/PeerTestManager.java
index ea677b89f8..5d9c807a45 100644
--- a/router/java/src/net/i2p/router/transport/udp/PeerTestManager.java
+++ b/router/java/src/net/i2p/router/transport/udp/PeerTestManager.java
@@ -327,29 +327,35 @@ class PeerTestManager {
             boolean expectV6 = test.isIPv6();
             if ((!expectV6 && ipSize != 4) ||
                 (expectV6 && ipSize != 16)) {
-                // There appears to be a bug where Bob is sending us a zero-length IP.
+                // There appears to be an i2pd bug where Bob is sending us a zero-length IP.
                 // We could proceed without setting the IP, but then when Charlie
                 // sends us his message, we will think we are behind a symmetric NAT
                 // because the Bob and Charlie IPs won't match.
-                // So for now we just return and pretend we didn't hear from Bob at all.
-                // Which is essentially what catching the uhe below did,
-                // but without the error message to the log.
-                // To do: fix the bug.
+                // Stop the test.
+                // Sometimes, the first response has an IP but a later one does not,
+                // check every time.
                 if (_log.shouldLog(Log.WARN))
-                    _log.warn("Bad IP length " + ipSize +
-                               " from bob's reply: " + from + ", " + testInfo);
+                    _log.warn("Bad IP length " + ipSize + " from bob's reply: " + from);
+                // reset all state
+                // so testComplete() will return UNKNOWN
+                test.setAlicePortFromCharlie(0);
+                test.setReceiveCharlieTime(0);
+                test.setReceiveBobTime(0);
+                testComplete();
                 return;
             }
             byte ip[] = new byte[ipSize];
             testInfo.readIP(ip, 0);
             try {
-                InetAddress addr = InetAddress.getByAddress(ip);
-                test.setAliceIP(addr);
+                if (test.getReceiveBobTime() <= 0) {
+                    InetAddress addr = InetAddress.getByAddress(ip);
+                    test.setAliceIP(addr);
+                    int testPort = testInfo.readPort();
+                    if (testPort == 0)
+                        throw new UnknownHostException("port 0");
+                    test.setAlicePort(testPort);
+                } // else ignore IP/port
                 test.setReceiveBobTime(_context.clock().now());
-                int testPort = testInfo.readPort();
-                if (testPort == 0)
-                    throw new UnknownHostException("port 0");
-                test.setAlicePort(testPort);
 
                 if (_log.shouldLog(Log.DEBUG))
                     _log.debug("Receive test reply from Bob: " + test);
@@ -358,7 +364,7 @@ class PeerTestManager {
             } catch (UnknownHostException uhe) {
                 if (_log.shouldLog(Log.WARN))
                     _log.warn("Unable to get our IP (length " + ipSize +
-                               ") from bob's reply: " + from + ", " + testInfo, uhe);
+                               ") from bob's reply: " + from, uhe);
                 _context.statManager().addRateData("udp.testBadIP", 1);
             }
         } else {
@@ -431,7 +437,7 @@ class PeerTestManager {
                     sendTestToCharlie();
                 } catch (UnknownHostException uhe) {
                     if (_log.shouldLog(Log.WARN))
-                        _log.warn("Charlie's IP is b0rked: " + from + ": " + testInfo);
+                        _log.warn("Charlie's IP is b0rked: " + from);
                     _context.statManager().addRateData("udp.testBadIP", 1);
                 }
             }
diff --git a/router/java/src/net/i2p/router/transport/udp/PeerTestState.java b/router/java/src/net/i2p/router/transport/udp/PeerTestState.java
index 10a3158d1b..d9e883656e 100644
--- a/router/java/src/net/i2p/router/transport/udp/PeerTestState.java
+++ b/router/java/src/net/i2p/router/transport/udp/PeerTestState.java
@@ -131,7 +131,8 @@ class PeerTestState {
             buf.append("; Bob: ").append(_bobIP).append(':').append(_bobPort);
         if (_charlieIP != null)
             buf.append(" Charlie: ").append(_charlieIP).append(':').append(_charliePort);
-        buf.append("; last send after ").append(_lastSendTime - _beginTime);
+        if (_lastSendTime > 0)
+            buf.append("; last send after ").append(_lastSendTime - _beginTime);
         if (_receiveAliceTime > 0)
             buf.append("; rcvd from Alice after ").append(_receiveAliceTime - _beginTime);
         if (_receiveBobTime > 0)
-- 
GitLab