* I2CP:

- Add session limit, add new status code for refused
   - Ramdomize session ID, prevent dups
   - Make session IDs immutable

core/java/src/net/i2p/data/i2cp/SessionId.java

@@ -43,14 +43,25 @@ public class SessionId extends DataStructureImpl {
         return _sessionId;
     }
 
-    /** @param id 0-65535 */
+    /**
+     *  @param id 0-65535
+     *  @throws IllegalArgumentException
+     *  @throws IllegalStateException if already set
+     */
     public void setSessionId(int id) {
         if (id < 0 || id > 65535)
             throw new IllegalArgumentException();
+        if (_sessionId >= 0)
+            throw new IllegalStateException();
         _sessionId = id;
     }
 
+    /**
+     *  @throws IllegalStateException if already set
+     */
     public void readBytes(InputStream in) throws DataFormatException, IOException {
+        if (_sessionId >= 0)
+            throw new IllegalStateException();
         _sessionId = (int) DataHelper.readLong(in, 2);
     }
 
@@ -62,12 +73,12 @@ public class SessionId extends DataStructureImpl {
     @Override
     public boolean equals(Object obj) {
         if ((obj == null) || !(obj instanceof SessionId)) return false;
-        return _sessionId == ((SessionId) obj).getSessionId();
+        return _sessionId == ((SessionId) obj)._sessionId;
     }
 
     @Override
     public int hashCode() {
-        return _sessionId;
+        return 777 * _sessionId;
     }
 
     @Override

core/java/src/net/i2p/data/i2cp/SessionStatusMessage.java

@@ -31,6 +31,8 @@ public class SessionStatusMessage extends I2CPMessageImpl {
     public final static int STATUS_CREATED = 1;
     public final static int STATUS_UPDATED = 2;
     public final static int STATUS_INVALID = 3;
+    /** @since 0.9.12 */
+    public final static int STATUS_REFUSED = 4;
 
     public SessionStatusMessage() {
         setStatus(STATUS_INVALID);

history.txt

@@ -1,3 +1,13 @@
+2014-02-14 zzz
+ * I2CP:
+   - Add session limit, add new status code for refused
+   - Ramdomize session ID, prevent dups
+   - Make SessionId immutable
+
+2014-02-13 zzz
+ * Router: Convert to getopt (ticket #1173)
+ * Tunnels: Change expl. OB default to 3+0
+
 2014-02-11 zzz
  * HTTP client proxy: Don't flush after headers for a POST,
    so the POST data is included in the SYN packet,

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 = 3;
+    public final static long BUILD = 4;
 
     /** for example "-test" */
     public final static String EXTRA = "";

router/java/src/net/i2p/router/client/ClientConnectionRunner.java

@@ -218,9 +218,21 @@ class ClientConnectionRunner {
      */
     public Hash getDestHash() { return _destHashCache; }
     
-    /** current client's sessionId */
+    /**
+     * @return current client's sessionId or null if not yet set
+     */
     SessionId getSessionId() { return _sessionId; }
-    void setSessionId(SessionId id) { if (id != null) _sessionId = id; }
+
+    /**
+     *  To be called only by ClientManager.
+     *
+     *  @throws IllegalStateException if already set
+     */
+    void setSessionId(SessionId id) {
+        if (_sessionId != null)
+            throw new IllegalStateException();
+        _sessionId = id;
+    }
 
     /** data for the current leaseRequest, or null if there is no active leaseSet request */
     LeaseRequestState getLeaseRequest() { return _leaseRequest; }
@@ -263,7 +275,14 @@ class ClientConnectionRunner {
         _messages.remove(id); 
     }
     
-    void sessionEstablished(SessionConfig config) {
+    /**
+     *  Caller must send a SessionStatusMessage to the client with the returned code.
+     *  Caller must call disconnectClient() on failure.
+     *  Side effect: Sets the session ID.
+     *
+     *  @return SessionStatusMessage return code, 1 for success, != 1 for failure
+     */
+    public int sessionEstablished(SessionConfig config) {
         _destHashCache = config.getDestination().calculateHash();
         if (_log.shouldLog(Log.DEBUG))
             _log.debug("SessionEstablished called for destination " + _destHashCache.toBase64());
@@ -293,7 +312,7 @@ class ClientConnectionRunner {
         } else {
             _log.error("SessionEstablished called for twice for destination " + _destHashCache.toBase64().substring(0,4));
         }
-        _manager.destinationEstablished(this);
+        return _manager.destinationEstablished(this);
     }
     
     /** 

router/java/src/net/i2p/router/client/ClientManager.java

@@ -27,6 +27,8 @@ import net.i2p.data.i2cp.I2CPMessage;
 import net.i2p.data.i2cp.MessageId;
 import net.i2p.data.i2cp.MessageStatusMessage;
 import net.i2p.data.i2cp.SessionConfig;
+import net.i2p.data.i2cp.SessionId;
+import net.i2p.data.i2cp.SessionStatusMessage;
 import net.i2p.internal.I2CPMessageQueue;
 import net.i2p.router.ClientManagerFacade;
 import net.i2p.router.ClientMessage;
@@ -52,6 +54,7 @@ class ClientManager {
     private final Map<Hash, ClientConnectionRunner>  _runnersByHash;
     // ClientConnectionRunner for clients w/out a Dest yet
     private final Set<ClientConnectionRunner> _pendingRunners;
+    private final Set<SessionId> _runnerSessionIds;
     protected final RouterContext _ctx;
     protected final int _port;
     protected volatile boolean _isStarted;
@@ -65,6 +68,14 @@ class ClientManager {
 
     private static final long REQUEST_LEASESET_TIMEOUT = 60*1000;
 
+    /** 2 bytes, save 65535 for unknown */
+    private static final int MAX_SESSION_ID = 65534;
+    private static final String PROP_MAX_SESSIONS = "i2cp.maxSessions";
+    private static final int DEFAULT_MAX_SESSIONS = 100;
+    /** 65535 */
+    public static final SessionId UNKNOWN_SESSION_ID = new SessionId(MAX_SESSION_ID + 1);
+
+
     /**
      *  Does not start the listeners.
      *  Caller must call start()
@@ -79,6 +90,7 @@ class ClientManager {
         _runners = new ConcurrentHashMap<Destination, ClientConnectionRunner>();
         _runnersByHash = new ConcurrentHashMap<Hash, ClientConnectionRunner>();
         _pendingRunners = new HashSet<ClientConnectionRunner>();
+        _runnerSessionIds = new HashSet<SessionId>();
         _port = port;
         // following are for RequestLeaseSetJob
         _ctx.statManager().createRateStat("client.requestLeaseSetSuccess", "How frequently the router requests successfully a new leaseSet?", "ClientMessages", new long[] { 60*60*1000 });
@@ -182,6 +194,9 @@ class ClientManager {
             // after connection establishment
             Destination dest = runner.getConfig().getDestination();
             synchronized (_runners) {
+                SessionId id = runner.getSessionId();
+                if (id != null)
+                    _runnerSessionIds.remove(id);
                 _runners.remove(dest);
                 _runnersByHash.remove(dest.calculateHash());
             }
@@ -189,9 +204,13 @@ class ClientManager {
     }
     
     /**
-     * Add to the clients list. Check for a dup destination.
+     *  Add to the clients list. Check for a dup destination.
+     *  Side effect: Sets the session ID of the runner.
+     *  Caller must call runner.disconnectClient() on failure.
+     *
+     *  @return SessionStatusMessage return code, 1 for success, != 1 for failure
      */
-    public void destinationEstablished(ClientConnectionRunner runner) {
+    public int destinationEstablished(ClientConnectionRunner runner) {
         Destination dest = runner.getConfig().getDestination();
         if (_log.shouldLog(Log.DEBUG))
             _log.debug("DestinationEstablished called for destination " + dest.calculateHash().toBase64());
@@ -199,20 +218,51 @@ class ClientManager {
         synchronized (_pendingRunners) {
             _pendingRunners.remove(runner);
         }
-        boolean fail = false;
+        int rv;
         synchronized (_runners) {
-            fail = _runnersByHash.containsKey(dest.calculateHash());
+            boolean fail = _runnersByHash.containsKey(dest.calculateHash());
-            if (!fail) {
+            if (fail) {
-                _runners.put(dest, runner);
+                rv = SessionStatusMessage.STATUS_INVALID;
-                _runnersByHash.put(dest.calculateHash(), runner);
+            } else {
+                SessionId id = locked_getNextSessionId();
+                if (id != null) {
+                    runner.setSessionId(id);
+                    _runners.put(dest, runner);
+                    _runnersByHash.put(dest.calculateHash(), runner);
+                    rv = SessionStatusMessage.STATUS_CREATED;
+                } else {
+                    rv = SessionStatusMessage.STATUS_REFUSED;
+                }
             }
         }
-        if (fail) {
+        if (rv == SessionStatusMessage.STATUS_INVALID) {
             _log.log(Log.CRIT, "Client attempted to register duplicate destination " + dest.calculateHash().toBase64());
-            runner.disconnectClient("Duplicate destination");
+        } else if (rv == SessionStatusMessage.STATUS_REFUSED) {
+            _log.error("Max sessions exceeded " + dest.calculateHash().toBase64());
         }
+        return rv;
     }
     
+    /**
+     *  Generate a new random, unused sessionId. Caller must synch on _runners.
+     *  @return null on failure
+     *  @since 0.9.12
+     */
+    private SessionId locked_getNextSessionId() { 
+        int max = Math.max(1, Math.min(2048, _ctx.getProperty(PROP_MAX_SESSIONS, DEFAULT_MAX_SESSIONS)));
+        if (_runnerSessionIds.size() >= max) {
+            _log.logAlways(Log.WARN, "Session refused, max is " + max + ", increase " + PROP_MAX_SESSIONS);
+            return null;
+        }
+        for (int i = 0; i < 100; i++) {
+            SessionId id = new SessionId(_ctx.random().nextInt(MAX_SESSION_ID + 1));
+            if (_runnerSessionIds.add(id))
+                return id; 
+        }
+        _log.logAlways(Log.WARN, "Session refused, can't find id slot");
+        return null;
+    }
+
     /**
      * Distribute message to a local or remote destination.
      * @param flags ignored for local

router/java/src/net/i2p/router/client/ClientMessageEventListener.java

@@ -197,6 +197,7 @@ class ClientMessageEventListener implements I2CPMessageReader.I2CPMessageEventLi
         } else {
             if (_log.shouldLog(Log.ERROR))
                 _log.error("Signature verification *FAILED* on a create session message.  Hijack attempt?");
+            // For now, we do NOT send a SessionStatusMessage - see javadoc above
             _runner.disconnectClient("Invalid signature on CreateSessionMessage");
             return;
         }
@@ -206,10 +207,11 @@ class ClientMessageEventListener implements I2CPMessageReader.I2CPMessageEventLi
         if (!checkAuth(inProps))
             return;
 
-        SessionId sessionId = new SessionId();
+        SessionId id = _runner.getSessionId();
-        sessionId.setSessionId(getNextSessionId()); 
+        if (id != null) {
-        _runner.setSessionId(sessionId);
+            _runner.disconnectClient("Already have session " + id);
-        sendStatusMessage(SessionStatusMessage.STATUS_CREATED);
+            return;
+        }
 
         // Copy over the whole config structure so we don't later corrupt it on
         // the client side if we change settings or later get a
@@ -219,10 +221,25 @@ class ClientMessageEventListener implements I2CPMessageReader.I2CPMessageEventLi
         Properties props = new Properties();
         props.putAll(in.getOptions());
         cfg.setOptions(props);
-        _runner.sessionEstablished(cfg);
+        int status = _runner.sessionEstablished(cfg);
+        if (status != SessionStatusMessage.STATUS_CREATED) {
+            // For now, we do NOT send a SessionStatusMessage - see javadoc above
+            if (_log.shouldLog(Log.ERROR))
+                _log.error("Session establish failed: code = " + status);
+            String msg;
+            if (status == SessionStatusMessage.STATUS_INVALID)
+                msg = "duplicate destination";
+            else if (status == SessionStatusMessage.STATUS_REFUSED)
+                msg = "session limit exceeded";
+            else
+                msg = "unknown error";
+            _runner.disconnectClient(msg);
+            return;
+        }
+        sendStatusMessage(status);
 
-        if (_log.shouldLog(Log.DEBUG))
+        if (_log.shouldLog(Log.INFO))
-            _log.debug("after sessionEstablished for " + _runner.getDestHash());
+            _log.info("Session " + _runner.getSessionId() + " established for " + _runner.getDestHash());
         startCreateSessionJob();
     }
     
@@ -410,7 +427,10 @@ class ClientMessageEventListener implements I2CPMessageReader.I2CPMessageEventLi
     
     private void sendStatusMessage(int status) {
         SessionStatusMessage msg = new SessionStatusMessage();
-        msg.setSessionId(_runner.getSessionId());
+        SessionId id = _runner.getSessionId();
+        if (id == null)
+            id = ClientManager.UNKNOWN_SESSION_ID;
+        msg.setSessionId(id);
         msg.setStatus(status);
         try {
             _runner.doSend(msg);
@@ -435,23 +455,8 @@ class ClientMessageEventListener implements I2CPMessageReader.I2CPMessageEventLi
             _runner.doSend(msg);
         } catch (I2CPMessageException ime) {
             if (_log.shouldLog(Log.WARN))
-                _log.warn("Error writing out the session status message", ime);
+                _log.warn("Error writing bw limits msg", ime);
         }
     }
 
-    // this *should* be mod 65536, but UnsignedInteger is still b0rked.  FIXME
-    private final static int MAX_SESSION_ID = 32767;
-
-    private static volatile int _id = RandomSource.getInstance().nextInt(MAX_SESSION_ID); // sessionId counter
-    private final static Object _sessionIdLock = new Object();
-    
-    /** generate a new sessionId */
-    private final static int getNextSessionId() { 
-        synchronized (_sessionIdLock) {
-            int id = (++_id)%MAX_SESSION_ID;
-            if (_id >= MAX_SESSION_ID)
-                _id = 0; 
-            return id; 
-        }
-    }
 }