From 38c839c389d5c1f69d2a9669d90e89292351057f Mon Sep 17 00:00:00 2001
From: zzz <zzz@i2pmail.org>
Date: Sat, 21 Jan 2023 11:47:31 -0500
Subject: [PATCH] SSU: NAT fixes part 3

Eliminate Symmetric NAT errors for "full cone" NATs with different external port
Separate isPortFixed() states for IPv4 and IPv6
Allow port changes when state is unknown
Require two tests to transition from unknown IPv4 state
Save external port config on change
Don't reset external port config at startup
Add port change event to event log
---
 .../router/transport/udp/UDPTransport.java    | 68 ++++++++++++++-----
 1 file changed, 51 insertions(+), 17 deletions(-)

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 33830e8aed..cba2007dbc 100644
--- a/router/java/src/net/i2p/router/transport/udp/UDPTransport.java
+++ b/router/java/src/net/i2p/router/transport/udp/UDPTransport.java
@@ -282,6 +282,20 @@ public class UDPTransport extends TransportImpl implements TimedWeightedPriority
 
     // various state bitmaps
 
+    private static final Set<Status> STATUS_IPV4_UNK =   EnumSet.of(Status.UNKNOWN,
+                                                                    Status.DISCONNECTED,
+                                                                    Status.HOSED,
+                                                                    Status.IPV4_UNKNOWN_IPV6_OK,
+                                                                    Status.IPV4_UNKNOWN_IPV6_FIREWALLED);
+
+    private static final Set<Status> STATUS_IPV6_UNK =   EnumSet.of(Status.UNKNOWN,
+                                                                    Status.DISCONNECTED,
+                                                                    Status.HOSED,
+                                                                    Status.IPV4_OK_IPV6_UNKNOWN,
+                                                                    Status.IPV4_FIREWALLED_IPV6_UNKNOWN,
+                                                                    Status.IPV4_SNAT_IPV6_UNKNOWN,
+                                                                    Status.IPV4_DISABLED_IPV6_UNKNOWN);
+
     private static final Set<Status> STATUS_IPV4_FW =    EnumSet.of(Status.DIFFERENT,
                                                                     Status.REJECT_UNSOLICITED,
                                                                     Status.IPV4_FIREWALLED_IPV6_OK,
@@ -323,10 +337,6 @@ public class UDPTransport extends TransportImpl implements TimedWeightedPriority
                                                                     Status.IPV4_DISABLED_IPV6_FIREWALLED,
                                                                     Status.DISCONNECTED);
 
-    private static final Set<Status> STATUS_NEED_INTRO = EnumSet.of(Status.REJECT_UNSOLICITED,
-                                                                    Status.IPV4_FIREWALLED_IPV6_OK,
-                                                                    Status.IPV4_FIREWALLED_IPV6_UNKNOWN);
-
     private static final Set<Status> STATUS_OK =         EnumSet.of(Status.OK,
                                                                     Status.IPV4_DISABLED_IPV6_OK);
 
@@ -541,8 +551,9 @@ public class UDPTransport extends TransportImpl implements TimedWeightedPriority
         if (port <= 0) {
             port = TransportUtil.selectRandomPort(_context, STYLE);
             Map<String, String> changes = new HashMap<String, String>(2);
-            changes.put(PROP_INTERNAL_PORT, Integer.toString(port));
-            changes.put(PROP_EXTERNAL_PORT, Integer.toString(port));
+            String sport = Integer.toString(port);
+            changes.put(PROP_INTERNAL_PORT, sport);
+            changes.put(PROP_EXTERNAL_PORT, sport);
             _context.router().saveConfig(changes, null);
             _log.logAlways(Log.INFO, "UDP selected random port " + port);
         }
@@ -758,12 +769,14 @@ public class UDPTransport extends TransportImpl implements TimedWeightedPriority
             return;
         }
         if (newPort > 0 &&
-            (newPort != port || newPort != oldIPort || newPort != oldEPort)) {
+            (newPort != port || newPort != oldIPort)) {
             // attempt to use it as our external port - this will be overridden by
             // externalAddressReceived(...)
             Map<String, String> changes = new HashMap<String, String>();
-            changes.put(PROP_INTERNAL_PORT, Integer.toString(newPort));
-            changes.put(PROP_EXTERNAL_PORT, Integer.toString(newPort));
+            String sport = Integer.toString(newPort);
+            changes.put(PROP_INTERNAL_PORT, sport);
+            if (oldEPort <= 0)
+                changes.put(PROP_EXTERNAL_PORT, sport);
             _context.router().saveConfig(changes, null);
         }
 
@@ -1426,7 +1439,7 @@ public class UDPTransport extends TransportImpl implements TimedWeightedPriority
                 if (!isIPv6) {
                     if (from.equals(_lastFromv4) || !eq(_lastOurIPv4, _lastOurPortv4, ourIP, ourPort)) {
                         if (_log.shouldLog(Log.INFO))
-                            _log.info("The router " + from + " told us we have a new IP - " 
+                            _log.info("The router " + from + " told us we have a new IP/port - " 
                                       + Addresses.toString(ourIP, ourPort) + ".  Wait until somebody else tells us the same thing.");
                     } else {
                         changeIt = true;
@@ -1438,7 +1451,7 @@ public class UDPTransport extends TransportImpl implements TimedWeightedPriority
                 } else {
                     if (from.equals(_lastFromv6) || !eq(_lastOurIPv6, _lastOurPortv6, ourIP, ourPort)) {
                         if (_log.shouldLog(Log.INFO))
-                            _log.info("The router " + from + " told us we have a new IP - " 
+                            _log.info("The router " + from + " told us we have a new IP/port - " 
                                       + Addresses.toString(ourIP, ourPort) + ".  Wait until somebody else tells us the same thing.");
                     } else {
                         changeIt = true;
@@ -1470,12 +1483,12 @@ public class UDPTransport extends TransportImpl implements TimedWeightedPriority
      * @return true if updated
      */
     private boolean changeAddress(byte ourIP[], int ourPort) {
-        // this defaults to true when we are firewalled and false otherwise.
-        boolean fixedPort = getIsPortFixed();
         boolean updated = false;
         boolean fireTest = false;
 
         boolean isIPv6 = ourIP.length == 16;
+        // this defaults to true when we are firewalled or unknown and false otherwise.
+        boolean fixedPort = getIsPortFixed(isIPv6);
 
         synchronized (_rebuildLock) {
             RouterAddress current = getCurrentExternalAddress(isIPv6);
@@ -1539,8 +1552,18 @@ public class UDPTransport extends TransportImpl implements TimedWeightedPriority
                     //if ( (_reachabilityStatus != CommSystemFacade.STATUS_OK) ||
                     //     (_externalListenHost == null) || (_externalListenPort <= 0) ||
                     //     (_context.clock().now() - _reachabilityStatusLastUpdated > 2*TEST_FREQUENCY) ) {
-                        // they told us something different and our tests are either old or failing
+
+                    // they told us something different and our tests are either old or failing
                     if (rebuild) {
+                            if (externalListenPort > 0 && ourPort > 0 &&
+                                externalListenPort != ourPort &&
+                                _context.getProperty(PROP_EXTERNAL_PORT, 0) != ourPort) {
+                                // save the external port setting only
+                                _context.router().saveConfig(PROP_EXTERNAL_PORT, Integer.toString(ourPort));
+                                _context.router().eventLog().addEvent(EventLog.CHANGE_PORT, "IPv" +
+                                                                                            (isIPv6 ? '6' : '4') +
+                                                                                            " port " + ourPort);
+                            }
                             if (_enableSSU2) {
                                 // flush SSU2 tokens
                                 if (ourPort != externalListenPort) {
@@ -1668,13 +1691,23 @@ public class UDPTransport extends TransportImpl implements TimedWeightedPriority
      *  our firewall is changing our port), unless overridden by the property.
      *  We must have an accurate external port when firewalled, or else
      *  our signature of the SessionCreated packet will be invalid.
+     *
+     *  As of 0.9.58, returns false if status is UNKNOWN
      */
-    private boolean getIsPortFixed() {
+    private boolean getIsPortFixed(boolean isIPv6) {
         String prop = _context.getProperty(PROP_FIXED_PORT);
         if (prop != null)
             return Boolean.parseBoolean(prop);
         Status status = getReachabilityStatus();
-        return !STATUS_NEED_INTRO.contains(status);
+        if (isIPv6) {
+            if (STATUS_IPV6_UNK.contains(status))
+                return false;
+            return !STATUS_IPV6_FW.contains(status);
+        } else {
+            if (STATUS_IPV4_UNK.contains(status))
+                return false;
+            return !STATUS_IPV4_FW.contains(status);
+        }
     }
 
     /** 
@@ -4002,7 +4035,8 @@ public class UDPTransport extends TransportImpl implements TimedWeightedPriority
                 // to prevent thrashing
                 if ((STATUS_OK.contains(old) && STATUS_FW.contains(status)) ||
                     (STATUS_OK.contains(status) && STATUS_FW.contains(old)) ||
-                    (STATUS_FW.contains(status) && STATUS_FW.contains(old))) {
+                    (STATUS_FW.contains(status) && STATUS_FW.contains(old)) ||
+                    (!isIPv6 && STATUS_IPV4_UNK.contains(old) && !STATUS_IPV4_UNK.contains(status))) {
                     if (status != _reachabilityStatusPending) {
                         if (_log.shouldLog(Log.WARN))
                             _log.warn("Old status: " + old + " status pending confirmation: " + status +
-- 
GitLab