diff --git a/apps/routerconsole/java/src/net/i2p/router/web/ConfigNetHelper.java b/apps/routerconsole/java/src/net/i2p/router/web/ConfigNetHelper.java index b7a2ad33a2..cbf8c859c7 100644 --- a/apps/routerconsole/java/src/net/i2p/router/web/ConfigNetHelper.java +++ b/apps/routerconsole/java/src/net/i2p/router/web/ConfigNetHelper.java @@ -50,7 +50,12 @@ public class ConfigNetHelper extends HelperBase { return ua.getHost(); } + /** + * To reduce confusion caused by NATs, this is the current internal SSU port, + * not the external port. + */ public String getUdpPort() { + /**** RouterAddress addr = _context.router().getRouterInfo().getTargetAddress("SSU"); if (addr == null) return _("unknown"); @@ -58,8 +63,17 @@ public class ConfigNetHelper extends HelperBase { if (ua.getPort() <= 0) return _("unknown"); return "" + ua.getPort(); + ****/ + // Since we can't get to UDPTransport.getRequestedPort() from here, just use + // configured port. If UDPTransport is changed such that the actual port + // could be different, fix this. + return getConfiguredUdpPort(); } + /** + * This should always be the actual internal SSU port, as UDPTransport udpates + * the config when it changes. + */ public String getConfiguredUdpPort() { return _context.getProperty(UDPTransport.PROP_INTERNAL_PORT, "unset"); } diff --git a/history.txt b/history.txt index e14d26a66f..a26c8fbbcb 100644 --- a/history.txt +++ b/history.txt @@ -1,3 +1,16 @@ +2013-05-06 zzz + * Transports: Clean up internal/external port confusion (ticket #873) + - Bind SSU to configured internal, not external, port at startup + - Use only internal ports for UPnP (getRequestedPort() fixups) + - Don't have NTCP follow frequent SSU port changes + - Don't use external SSU port for internal NTCP port + - Display internal SSU port on /confignet + +2013-05-01 zzz + * BuildRequestor: Slow down build loop if we have no exploratory tunnels + (ticket #926) + * IRC Server tunnel: Reject bad protocols immediately + 2013-04-30 str4d * Console: Updates to readme_ar and a CSS tweak from hamada (ticket #489) diff --git a/router/java/src/net/i2p/router/RouterVersion.java b/router/java/src/net/i2p/router/RouterVersion.java index f5c9264549..43dca92409 100644 --- a/router/java/src/net/i2p/router/RouterVersion.java +++ b/router/java/src/net/i2p/router/RouterVersion.java @@ -18,7 +18,7 @@ public class RouterVersion { /** deprecated */ public final static String ID = "Monotone"; public final static String VERSION = CoreVersion.VERSION; - public final static long BUILD = 20; + public final static long BUILD = 21; /** for example "-test" */ public final static String EXTRA = ""; diff --git a/router/java/src/net/i2p/router/transport/CommSystemFacadeImpl.java b/router/java/src/net/i2p/router/transport/CommSystemFacadeImpl.java index 82987ae8e6..6bed29d73c 100644 --- a/router/java/src/net/i2p/router/transport/CommSystemFacadeImpl.java +++ b/router/java/src/net/i2p/router/transport/CommSystemFacadeImpl.java @@ -272,8 +272,8 @@ public class CommSystemFacadeImpl extends CommSystemFacade { * This should really be moved to ntcp/NTCPTransport.java, why is it here? */ @Override - public synchronized void notifyReplaceAddress(RouterAddress UDPAddr) { - if (UDPAddr == null) + public synchronized void notifyReplaceAddress(RouterAddress udpAddr) { + if (udpAddr == null) return; NTCPTransport t = (NTCPTransport) _manager.getTransport(NTCPTransport.STYLE); if (t == null) @@ -296,19 +296,38 @@ public class CommSystemFacadeImpl extends CommSystemFacade { // Auto Port Setting // old behavior (<= 0.7.3): auto-port defaults to false, and true trumps explicit setting // new behavior (>= 0.7.4): auto-port defaults to true, but explicit setting trumps auto + // TODO rewrite this to operate on ints instead of strings String oport = newProps.getProperty(NTCPAddress.PROP_PORT); String nport = null; String cport = _context.getProperty(PROP_I2NP_NTCP_PORT); if (cport != null && cport.length() > 0) { nport = cport; } else if (_context.getBooleanPropertyDefaultTrue(PROP_I2NP_NTCP_AUTO_PORT)) { - nport = UDPAddr.getOption(UDPAddress.PROP_PORT); + // 0.9.6 change + // This wasn't quite right, as udpAddr is the EXTERNAL port and we really + // want NTCP to bind to the INTERNAL port the first time, + // because if they are different, the NAT is changing them, and + // it probably isn't mapping UDP and TCP the same. + Transport udp = _manager.getTransport(UDPTransport.STYLE); + if (udp != null) { + int udpIntPort = udp.getRequestedPort(); + if (udpIntPort > 0) + // should always be true + nport = Integer.toString(udpIntPort); + } + if (nport == null) + // fallback + nport = udpAddr.getOption(UDPAddress.PROP_PORT); } if (_log.shouldLog(Log.INFO)) _log.info("old: " + oport + " config: " + cport + " new: " + nport); if (nport == null || nport.length() <= 0) return; - if (oport == null || ! oport.equals(nport)) { + // 0.9.6 change + // Don't have NTCP "chase" SSU's external port, + // as it may change, possibly frequently. + //if (oport == null || ! oport.equals(nport)) { + if (oport == null) { newProps.setProperty(NTCPAddress.PROP_PORT, nport); changed = true; } @@ -335,7 +354,7 @@ public class CommSystemFacadeImpl extends CommSystemFacade { _log.info("old: " + ohost + " config: " + name + " auto: " + enabled + " status: " + status); if (enabled.equals("always") || (Boolean.parseBoolean(enabled) && status == STATUS_OK)) { - String nhost = UDPAddr.getOption(UDPAddress.PROP_HOST); + String nhost = udpAddr.getOption(UDPAddress.PROP_HOST); if (_log.shouldLog(Log.INFO)) _log.info("old: " + ohost + " config: " + name + " new: " + nhost); if (nhost == null || nhost.length() <= 0) diff --git a/router/java/src/net/i2p/router/transport/TransportManager.java b/router/java/src/net/i2p/router/transport/TransportManager.java index acd9bf0cd6..f6a7c71f80 100644 --- a/router/java/src/net/i2p/router/transport/TransportManager.java +++ b/router/java/src/net/i2p/router/transport/TransportManager.java @@ -345,21 +345,13 @@ public class TransportManager implements TransportEventListener { } /** - * Include the published port, or the requested port, for each transport - * which we will pass along to UPnP + * The actual or requested INTERNAL ports, for each transport, + * which we will pass along to UPnP to be forwarded. */ private Map getPorts() { Map rv = new HashMap(_transports.size()); for (Transport t : _transports.values()) { int port = t.getRequestedPort(); - if (t.getCurrentAddress() != null) { - String s = t.getCurrentAddress().getOption("port"); - if (s != null) { - try { - port = Integer.parseInt(s); - } catch (NumberFormatException nfe) {} - } - } // Use UDP port for NTCP too - see comment in NTCPTransport.getRequestedPort() for why this is here if (t.getStyle().equals(NTCPTransport.STYLE) && port <= 0 && _context.getBooleanProperty(CommSystemFacadeImpl.PROP_I2NP_NTCP_AUTO_PORT)) { diff --git a/router/java/src/net/i2p/router/transport/ntcp/NTCPTransport.java b/router/java/src/net/i2p/router/transport/ntcp/NTCPTransport.java index 66cd37e819..343ee5f0b0 100644 --- a/router/java/src/net/i2p/router/transport/ntcp/NTCPTransport.java +++ b/router/java/src/net/i2p/router/transport/ntcp/NTCPTransport.java @@ -638,6 +638,7 @@ public class NTCPTransport extends TransportImpl { //private boolean bindAllInterfaces() { return true; } + /** caller must synch on this */ private void configureLocalAddress() { RouterContext ctx = getContext(); if (ctx == null) { @@ -684,8 +685,17 @@ public class NTCPTransport extends TransportImpl { } } + /** + * @return current port, else NTCP configured port, else -1 (but not UDP port if auto) + */ @Override public int getRequestedPort() { + NTCPAddress addr = _myAddress; + if (addr != null) { + int port = addr.getPort(); + if (port > 0) + return port; + } // would be nice to do this here but we can't easily get to the UDP transport.getRequested_Port() // from here, so we do it in TransportManager. // if (Boolean.valueOf(_context.getProperty(CommSystemFacadeImpl.PROP_I2NP_NTCP_AUTO_PORT)).booleanValue()) 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 567ce43ece..97a88abc10 100644 --- a/router/java/src/net/i2p/router/transport/udp/UDPTransport.java +++ b/router/java/src/net/i2p/router/transport/udp/UDPTransport.java @@ -83,7 +83,10 @@ public class UDPTransport extends TransportImpl implements TimedWeightedPriority /** summary info to distribute */ private RouterAddress _externalAddress; - /** port number on which we can be reached, or -1 for error, or 0 for unset */ + /** + * Port number on which we can be reached, or -1 for error, or 0 for unset + * Do NOT use this for current internal port - use _endpoint.getListenPort() + */ private int _externalListenPort; /** IP address of externally reachable host, or null */ private InetAddress _externalListenHost; @@ -310,19 +313,18 @@ public class UDPTransport extends TransportImpl implements TimedWeightedPriority // Requested bind port // This may be -1 or may not be honored if busy, + // Priority: Configured internal, then already used, then configured external // we will check below after starting up the endpoint. int port; int oldIPort = _context.getProperty(PROP_INTERNAL_PORT, -1); + int oldBindPort = _endpoint != null ? _endpoint.getListenPort() : -1; int oldEPort = _context.getProperty(PROP_EXTERNAL_PORT, -1); - if (_externalListenPort <= 0) { - // no explicit external port, so lets try an internal one - if (oldIPort > 0) - port = oldIPort; - else - port = oldEPort; - } else { - port = _externalListenPort; - } + if (oldIPort > 0) + port = oldIPort; + else if (oldBindPort > 0) + port = oldBindPort; + else + port = oldEPort; if (bindToAddr != null && _log.shouldLog(Log.WARN)) _log.warn("Binding only to " + bindToAddr); if (_log.shouldLog(Log.INFO)) @@ -436,14 +438,23 @@ public class UDPTransport extends TransportImpl implements TimedWeightedPriority } /** - * _externalListenPort should always be set (by startup()) before this is called, - * so the returned value should be > 0 + * The current or configured internal port. + * UDPEndpoint should always be instantiated (and a random port picked if not configured) + * before this is called, so the returned value should be > 0 + * unless the endpoint failed to bind. */ @Override public int getRequestedPort() { - if (_externalListenPort > 0) - return _externalListenPort; - return _context.getProperty(PROP_INTERNAL_PORT, -1); + if (_endpoint != null) { + int rv = _endpoint.getListenPort(); + if (rv > 0) + return rv; + } + // fallbacks + int rv = _context.getProperty(PROP_INTERNAL_PORT, -1); + if (rv > 0) + return rv; + return _context.getProperty(PROP_EXTERNAL_PORT, -1); } /**