UPnP:

- Pass device IP back in forward port callback
 - Only declare success if forwarded IP is public

NTCP: Bad bind config not fatal

GeoIP:
 - Use cached IP in RouterAddresses
 - Use both NTCP and SSU addresses
 - Skip IPv6 for now

Blocklist:
 - Add IPv6 in-memory single list
 - Limit in-memory single list size
 - Fix dup check in getAddresses()

router/java/src/net/i2p/router/Blocklist.java

@@ -11,10 +11,12 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.io.InputStreamReader;
 import java.io.Writer;
+import java.math.BigInteger;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -31,6 +33,7 @@ import net.i2p.data.RouterInfo;
 import net.i2p.router.networkdb.kademlia.FloodfillNetworkDatabaseFacade;
 import net.i2p.util.Addresses;
 import net.i2p.util.ConcurrentHashSet;
+import net.i2p.util.LHMCache;
 import net.i2p.util.Log;
 import net.i2p.util.Translate;
 
@@ -62,6 +65,8 @@ import net.i2p.util.Translate;
  * banlist it forever, then go back to the file to get the original
  * entry so we can add the reason to the banlist text.
  *
+ * On-disk blocklist supports IPv4 only.
+ * In-memory supports both IPv4 and IPv6.
  */
 public class Blocklist {
     private final Log _log;
@@ -72,8 +77,20 @@ public class Blocklist {
     private Entry _wrapSave;
     private final Set<Hash> _inProcess = new HashSet(4);
     private Map<Hash, String> _peerBlocklist = new HashMap(4);
+
+    /**
+     *  Limits of transient (in-memory) blocklists.
+     *  Note that it's impossible to prevent clogging up
+     *  the tables by a determined attacker, esp. on IPv6
+     */
+    private static final int MAX_IPV4_SINGLES = 256;
+    private static final int MAX_IPV6_SINGLES = 512;
+
     private final Set<Integer> _singleIPBlocklist = new ConcurrentHashSet(4);
-    
+    private final Map<BigInteger, Object> _singleIPv6Blocklist = new LHMCache(MAX_IPV6_SINGLES);
+
+    private static final Object DUMMY = Integer.valueOf(0);    
+
     public Blocklist(RouterContext context) {
         _context = context;
         _log = context.logManager().getLog(Blocklist.class);
@@ -437,6 +454,8 @@ public class Blocklist {
      * Maintain a simple in-memory single-IP blocklist
      * This is used for new additions, NOT for the main list
      * of IP ranges read in from the file.
+     *
+     * @param ip IPv4 or IPv6
      */
     public void add(String ip) {
         byte[] pib = Addresses.getIP(ip);
@@ -448,16 +467,24 @@ public class Blocklist {
      * Maintain a simple in-memory single-IP blocklist
      * This is used for new additions, NOT for the main list
      * of IP ranges read in from the file.
+     *
+     * @param ip IPv4 or IPv6
      */
     public void add(byte ip[]) {
-        if (ip.length != 4)
+        boolean rv;
-            return;
+        if (ip.length == 4)
-        if (add(toInt(ip)))
+            rv = add(toInt(ip));
-            if (_log.shouldLog(Log.WARN))
+        else if (ip.length == 16)
-                _log.warn("Adding IP to blocklist: " + Addresses.toString(ip));
+            rv = add(new BigInteger(1, ip));
+        else
+            rv = false;
+        if (rv && _log.shouldLog(Log.WARN))
+            _log.warn("Adding IP to blocklist: " + Addresses.toString(ip));
     }
 
     private boolean add(int ip) {
+        if (_singleIPBlocklist.size() >= MAX_IPV4_SINGLES)
+            return false;
         return _singleIPBlocklist.add(Integer.valueOf(ip));
     }
 
@@ -466,20 +493,41 @@ public class Blocklist {
     }
 
     /**
-     * this tries to not return duplicates
+     * @param ip IPv6 non-negative
-     * but I suppose it could.
+     * @since IPv6
+     */
+    private boolean add(BigInteger ip) {
+        synchronized(_singleIPv6Blocklist) {
+            return _singleIPv6Blocklist.put(ip, DUMMY) == null;
+        }
+    }
+
+    /**
+     * @param ip IPv6 non-negative
+     * @since IPv6
+     */
+    private boolean isOnSingleList(BigInteger ip) {
+        synchronized(_singleIPv6Blocklist) {
+            return _singleIPv6Blocklist.get(ip) != null;
+        }
+    }
+
+    /**
+     * Will not contain duplicates.
      */
     private List<byte[]> getAddresses(Hash peer) {
-        List<byte[]> rv = new ArrayList(1);
         RouterInfo pinfo = _context.netDb().lookupRouterInfoLocally(peer);
-        if (pinfo == null) return rv;
+        if (pinfo == null)
-        byte[] oldpib = null;
+            return Collections.EMPTY_LIST;
+        List<byte[]> rv = new ArrayList(4);
         // for each peer address
         for (RouterAddress pa : pinfo.getAddresses()) {
             byte[] pib = pa.getIP();
             if (pib == null) continue;
-            if (DataHelper.eq(oldpib, pib)) continue;
+            // O(n**2)
-            oldpib = pib;
+            for (int i = 0; i < rv.size(); i++) {
+                if (DataHelper.eq(rv.get(i), pib)) continue;
+            }
             rv.add(pib);
          }
          return rv;
@@ -491,8 +539,9 @@ public class Blocklist {
      */
     public boolean isBlocklisted(Hash peer) {
         List<byte[]> ips = getAddresses(peer);
-        for (Iterator<byte[]> iter = ips.iterator(); iter.hasNext(); ) {
+        if (ips.isEmpty())
-            byte ip[] = iter.next();
+            return false;
+        for (byte[] ip : ips) {
             if (isBlocklisted(ip)) {
                 if (! _context.banlist().isBanlisted(peer))
                     // nice knowing you...
@@ -505,6 +554,8 @@ public class Blocklist {
 
     /**
      * calling this externally won't banlist the peer, this is just an IP check
+     *
+     * @param ip IPv4 or IPv6
      */
     public boolean isBlocklisted(String ip) {
         byte[] pib = Addresses.getIP(ip);
@@ -514,11 +565,15 @@ public class Blocklist {
 
     /**
      * calling this externally won't banlist the peer, this is just an IP check
+     *
+     * @param ip IPv4 or IPv6
      */
     public boolean isBlocklisted(byte ip[]) {
-        if (ip.length != 4)
+        if (ip.length == 4)
-            return false;
+            return isBlocklisted(toInt(ip));
-        return isBlocklisted(toInt(ip));
+        if (ip.length == 16)
+            return isOnSingleList(new BigInteger(1, ip));
+        return false;
     }
 
     /**
@@ -760,7 +815,7 @@ public class Blocklist {
         //out.write("<h2>Banned IPs</h2>");
         Set<Integer> singles = new TreeSet();
         singles.addAll(_singleIPBlocklist);
-        if (!singles.isEmpty()) {
+        if (!(singles.isEmpty() && _singleIPv6Blocklist.isEmpty())) {
             out.write("<table><tr><th align=\"center\" colspan=\"2\"><b>");
             out.write(_("IPs Banned Until Restart"));
             out.write("</b></td></tr>");
@@ -782,6 +837,19 @@ public class Blocklist {
                  out.write(toStr(ip));
                  out.write("</td><td width=\"50%\">&nbsp;</td></tr>\n");
             }
+            // then IPv6
+            if (!_singleIPv6Blocklist.isEmpty()) {
+                List<BigInteger> s6;
+                synchronized(_singleIPv6Blocklist) {
+                    s6 = new ArrayList(_singleIPv6Blocklist.keySet());
+                }
+                Collections.sort(s6);
+                for (BigInteger bi : s6) {
+                     out.write("<tr><td align=\"center\" width=\"50%\">");
+                     out.write(Addresses.toString(toIPBytes(bi)));
+                     out.write("</td><td width=\"50%\">&nbsp;</td></tr>\n");
+                }
+            }
             out.write("</table>");
         }
         if (_blocklistSize > 0) {
@@ -832,6 +900,23 @@ public class Blocklist {
         out.flush();
     }
 
+    /**
+     *  Convert a (non-negative) two's complement IP to exactly 16 bytes
+     *  @since IPv6
+     */
+    private static byte[] toIPBytes(BigInteger bi) {
+        byte[] ba = bi.toByteArray();
+        int len = ba.length;
+        if (len == 16)
+            return ba;
+        byte[] rv = new byte[16];
+        if (len < 16)
+            System.arraycopy(ba, 0, rv, 16 - len, len);
+        else
+            System.arraycopy(ba, len - 16, rv, 0, 16);
+        return rv;
+    }
+
     /**
      *  Mark a string for extraction by xgettext and translation.
      *  Use this only in static initializers.

router/java/src/net/i2p/router/transport/CommSystemFacadeImpl.java

@@ -240,10 +240,10 @@ public class CommSystemFacadeImpl extends CommSystemFacade {
                 RouterInfo ri = _context.netDb().lookupRouterInfoLocally(iter.next());
                 if (ri == null)
                     continue;
-                String host = getIPString(ri);
+                byte[] ip = getIP(ri);
-                if (host == null)
+                if (ip == null)
                     continue;
-                _geoIP.add(host);
+                _geoIP.add(ip);
             }
             _context.simpleScheduler().addPeriodicEvent(new Lookup(), 5000, LOOKUP_TIME);
         }
@@ -287,24 +287,27 @@ public class CommSystemFacadeImpl extends CommSystemFacade {
     @Override
     public String getCountry(Hash peer) {
         byte[] ip = TransportImpl.getIP(peer);
-        if (ip != null)
+        // Assume IPv6 doesn't have geoIP for now
+        if (ip != null && ip.length == 4)
             return _geoIP.get(ip);
         RouterInfo ri = _context.netDb().lookupRouterInfoLocally(peer);
         if (ri == null)
             return null;
-        String s = getIPString(ri);
+        ip = getIP(ri);
-        if (s != null)
+        if (ip != null)
-            return _geoIP.get(s);
+            return _geoIP.get(ip);
         return null;
     }
 
-    private String getIPString(RouterInfo ri) {
+    private static byte[] getIP(RouterInfo ri) {
-        // use SSU only, it is likely to be an IP not a hostname,
+        // Return first IPv4 we find, any transport
-        // we don't want to generate a lot of DNS queries at startup
+        // Assume IPv6 doesn't have geoIP for now
-        RouterAddress ra = ri.getTargetAddress("SSU");
+        for (RouterAddress ra : ri.getAddresses()) {
-        if (ra == null)
+            byte[] rv = ra.getIP();
-            return null;
+            if (rv != null && rv.length == 4)
-        return ra.getOption("host");
+                return rv;
+        }
+        return null;
     }
 
     /** full name for a country code, or the code if we don't know the name */

router/java/src/net/i2p/router/transport/Transport.java

@@ -97,11 +97,12 @@ public interface Transport {
     /**
      *  Notify a transport of the results of trying to forward a port.
      *
+     *  @param ip may be null
      *  @param port the internal port
      *  @param externalPort the external port, which for now should always be the same as
      *                      the internal port if the forwarding was successful.
      */
-    public void forwardPortStatus(int port, int externalPort, boolean success, String reason);
+    public void forwardPortStatus(byte[] ip, int port, int externalPort, boolean success, String reason);
 
     /**
      * What INTERNAL port would the transport like to have forwarded by UPnP.

router/java/src/net/i2p/router/transport/TransportImpl.java

@@ -589,11 +589,12 @@ public abstract class TransportImpl implements Transport {
      *
      *  This implementation does nothing. Transports should override if they want notification.
      *
+     *  @param ip may be null
      *  @param port the internal port
      *  @param externalPort the external port, which for now should always be the same as
      *                      the internal port if the forwarding was successful.
      */
-    public void forwardPortStatus(int port, int externalPort, boolean success, String reason) {}
+    public void forwardPortStatus(byte[] ip, int port, int externalPort, boolean success, String reason) {}
 
     /**
      * What INTERNAL port would the transport like to have forwarded by UPnP.
@@ -691,6 +692,11 @@ public abstract class TransportImpl implements Transport {
             _log.info(this.getStyle() + " setting wasUnreachable to " + yes + " for " + peer);
     }
 
+    /**
+     * IP of the peer from the last connection (in or out, any transport).
+     *
+     * @param IPv4 or IPv6, non-null
+     */
     public void setIP(Hash peer, byte[] ip) {
         byte[] old;
         synchronized (_IPMap) {
@@ -700,6 +706,11 @@ public abstract class TransportImpl implements Transport {
             _context.commSystem().queueLookup(ip);
     }
 
+    /**
+     * IP of the peer from the last connection (in or out, any transport).
+     *
+     * @return IPv4 or IPv6 or null
+     */
     public static byte[] getIP(Hash peer) {
         synchronized (_IPMap) {
             return _IPMap.get(peer);

router/java/src/net/i2p/router/transport/TransportManager.java

@@ -147,10 +147,10 @@ public class TransportManager implements TransportEventListener {
      * callback from UPnP
      *
      */
-    public void forwardPortStatus(String style, int port, int externalPort, boolean success, String reason) {
+    public void forwardPortStatus(String style, byte[] ip, int port, int externalPort, boolean success, String reason) {
         Transport t = getTransport(style);
         if (t != null)
-            t.forwardPortStatus(port, externalPort, success, reason);
+            t.forwardPortStatus(ip, port, externalPort, success, reason);
     }
 
     public synchronized void startListening() {
@@ -351,6 +351,8 @@ public class TransportManager implements TransportEventListener {
      *
      * For blocking purposes, etc. it's worth checking both
      * the netDb addresses and this address.
+     *
+     * @return IPv4 or IPv6 or null
      */
     public byte[] getIP(Hash dest) {
         return TransportImpl.getIP(dest);

router/java/src/net/i2p/router/transport/UPnPManager.java

@@ -153,6 +153,7 @@ class UPnPManager {
             if (_log.shouldLog(Log.DEBUG))
                  _log.debug("UPnP Callback:");
 
+            byte[] ipaddr = null;
             DetectedIP[] ips = _upnp.getAddress();
             if (ips != null) {
                 for (DetectedIP ip : ips) {
@@ -164,6 +165,7 @@ class UPnPManager {
                             _detectedAddress = ip.publicAddress;
                             _manager.externalAddressReceived(SOURCE_UPNP, _detectedAddress.getAddress(), 0);
                         }
+                        ipaddr = ip.publicAddress.getAddress();
                         break;
                     }
                 }
@@ -186,7 +188,7 @@ class UPnPManager {
                 else
                     continue;
                 boolean success = fps.status >= ForwardPortStatus.MAYBE_SUCCESS;
-                _manager.forwardPortStatus(style, fp.portNumber, fps.externalPort, success, fps.reasonString);
+                _manager.forwardPortStatus(style, ipaddr, fp.portNumber, fps.externalPort, success, fps.reasonString);
             }
         }
     }

router/java/src/net/i2p/router/transport/ntcp/NTCPTransport.java

@@ -579,17 +579,18 @@ public class NTCPTransport extends TransportImpl {
                 try {
                     bindToAddr = InetAddress.getByName(bindTo);
                 } catch (UnknownHostException uhe) {
-                    _log.log(Log.CRIT, "Invalid NTCP bind interface specified [" + bindTo + "]", uhe);
+                    _log.error("Invalid NTCP bind interface specified [" + bindTo + "]", uhe);
                     // this can be implemented later, just updates some stats
                     // see udp/UDPTransport.java
                     //setReachabilityStatus(CommSystemFacade.STATUS_HOSED);
-                    return null;
+                    //return null;
+                    // fall thru
                 }
             }
 
             try {
                 InetSocketAddress addr;
-                if(bindToAddr==null) {
+                if (bindToAddr == null) {
                     addr = new InetSocketAddress(port);
                 } else {
                     addr = new InetSocketAddress(bindToAddr, port);
@@ -974,10 +975,10 @@ public class NTCPTransport extends TransportImpl {
      *  NTCP address when it transitions to OK.
      */
     @Override
-    public void forwardPortStatus(int port, int externalPort, boolean success, String reason) {
+    public void forwardPortStatus(byte[] ip, int port, int externalPort, boolean success, String reason) {
         if (_log.shouldLog(Log.WARN)) {
             if (success)
-                _log.warn("UPnP has opened the NTCP port: " + port);
+                _log.warn("UPnP has opened the NTCP port: " + port + " via " + Addresses.toString(ip, externalPort));
             else
                 _log.warn("UPnP has failed to open the NTCP port: " + port + " reason: " + reason);
         }

router/java/src/net/i2p/router/transport/udp/UDPTransport.java

@@ -671,14 +671,14 @@ public class UDPTransport extends TransportImpl implements TimedWeightedPriority
      *  Don't do anything if UPnP claims failure.
      */
     @Override
-    public void forwardPortStatus(int port, int externalPort, boolean success, String reason) {
+    public void forwardPortStatus(byte[] ip, int port, int externalPort, boolean success, String reason) {
         if (_log.shouldLog(Log.WARN)) {
             if (success)
-                _log.warn("UPnP has opened the SSU port: " + port + " via external port: " + externalPort);
+                _log.warn("UPnP has opened the SSU port: " + port + " via " + Addresses.toString(ip, externalPort));
             else
                 _log.warn("UPnP has failed to open the SSU port: " + port + " reason: " + reason);
         }
-        if (success && getExternalIP() != null)
+        if (success && ip != null && getExternalIP() != null)
             setReachabilityStatus(CommSystemFacade.STATUS_OK);
     }