From 49128b5a2c1ddf75422ece83b5e0fa5c07b24d58 Mon Sep 17 00:00:00 2001
From: zzz <zzz@mail.i2p>
Date: Sat, 28 Oct 2023 15:42:05 +0000
Subject: [PATCH] NetDB: Fix lifecycle issues for subsessions (Gitlab #460,
 #406)

---
 .../router/client/ClientConnectionRunner.java | 74 ++++++-------------
 .../net/i2p/router/client/ClientManager.java  | 17 +++--
 .../client/ClientMessageEventListener.java    |  8 +-
 .../net/i2p/router/client/LookupDestJob.java  | 17 +++--
 .../KademliaNetworkDatabaseFacade.java        | 27 +++----
 .../router/tunnel/pool/AliasedTunnelPool.java | 10 ++-
 6 files changed, 67 insertions(+), 86 deletions(-)

diff --git a/router/java/src/net/i2p/router/client/ClientConnectionRunner.java b/router/java/src/net/i2p/router/client/ClientConnectionRunner.java
index d3e551c537..b6cac2b9a3 100644
--- a/router/java/src/net/i2p/router/client/ClientConnectionRunner.java
+++ b/router/java/src/net/i2p/router/client/ClientConnectionRunner.java
@@ -49,9 +49,8 @@ import net.i2p.router.JobImpl;
 import net.i2p.router.RouterContext;
 import net.i2p.router.crypto.TransientSessionKeyManager;
 import net.i2p.router.crypto.ratchet.RatchetSKM;
-import net.i2p.router.networkdb.kademlia.FloodfillNetworkDatabaseFacade;
-import net.i2p.router.networkdb.kademlia.FloodfillNetworkDatabaseSegmentor;
 import net.i2p.router.crypto.ratchet.MuxedSKM;
+import net.i2p.router.networkdb.kademlia.FloodfillNetworkDatabaseFacade;
 import net.i2p.util.ConcurrentHashSet;
 import net.i2p.util.I2PThread;
 import net.i2p.util.Log;
@@ -160,8 +159,6 @@ class ClientConnectionRunner {
         _alreadyProcessed = new ArrayList<MessageId>();
         _acceptedPending = new ConcurrentHashSet<MessageId>();
         _messageId = new AtomicInteger(_context.random().nextInt());
-        // Set up the per-destination FloodfillNetworkDatabaseFacade to prevent clients from being able to
-        // update leaseSet entries in the floodfill netDb
     }
     
     private static final AtomicInteger __id = new AtomicInteger();
@@ -213,9 +210,6 @@ class ClientConnectionRunner {
         _acceptedPending.clear();
         if (_sessionKeyManager != null)
             _sessionKeyManager.shutdown();
-        if (_floodfillNetworkDatabaseFacade != null)
-            if (_floodfillNetworkDatabaseFacade.isClientDb())
-                _floodfillNetworkDatabaseFacade.shutdown();
         if (_encryptedLSHash != null)
             _manager.unregisterEncryptedDestination(this, _encryptedLSHash);
         _manager.unregisterConnection(this);
@@ -225,13 +219,8 @@ class ClientConnectionRunner {
             // removeSession() was called just before this, and
             // _sessions will be empty.
             for (SessionParams sp : _sessions.values()) {
-                LeaseSet ls = sp.currentLeaseSet;
-                if (ls != null && getFloodfillNetworkDatabaseFacade() != null)
-                    getFloodfillNetworkDatabaseFacade().unpublish(ls);
-                // unpublish encrypted LS also
-                ls = sp.currentEncryptedLeaseSet;
-                if (ls != null && getFloodfillNetworkDatabaseFacade() != null)
-                    getFloodfillNetworkDatabaseFacade().unpublish(ls);
+                // we don't need to unpublish(),
+                // as we shut down our subdb below.
                 if (!sp.isPrimary)
                     _context.tunnelManager().removeAlias(sp.dest);
             }
@@ -242,6 +231,8 @@ class ClientConnectionRunner {
                     sp.rerequestTimer.cancel();
             }
         }
+        if (_floodfillNetworkDatabaseFacade != null)
+            _floodfillNetworkDatabaseFacade.shutdown();
         synchronized (_alreadyProcessed) {
             _alreadyProcessed.clear();
         }
@@ -467,12 +458,12 @@ class ClientConnectionRunner {
                 // Tell client manger
                 _manager.unregisterSession(id, sp.dest);
                 LeaseSet ls = sp.currentLeaseSet;
-                if (ls != null && getFloodfillNetworkDatabaseFacade() != null)
-                    getFloodfillNetworkDatabaseFacade().unpublish(ls);
+                if (ls != null && _floodfillNetworkDatabaseFacade != null)
+                    _floodfillNetworkDatabaseFacade.unpublish(ls);
                 // unpublish encrypted LS also
                 ls = sp.currentEncryptedLeaseSet;
-                if (ls != null && getFloodfillNetworkDatabaseFacade() != null)
-                    getFloodfillNetworkDatabaseFacade().unpublish(ls);
+                if (ls != null && _floodfillNetworkDatabaseFacade != null)
+                    _floodfillNetworkDatabaseFacade.unpublish(ls);
                 isPrimary = sp.isPrimary;
                 if (isPrimary)
                     _context.tunnelManager().removeTunnels(sp.dest);
@@ -492,12 +483,12 @@ class ClientConnectionRunner {
                     _log.info("Destroying remaining client subsession " + sp.sessionId);
                 _manager.unregisterSession(sp.sessionId, sp.dest);
                 LeaseSet ls = sp.currentLeaseSet;
-                if (ls != null && getFloodfillNetworkDatabaseFacade() != null)
-                    getFloodfillNetworkDatabaseFacade().unpublish(ls);
+                if (ls != null && _floodfillNetworkDatabaseFacade != null)
+                    _floodfillNetworkDatabaseFacade.unpublish(ls);
                 // unpublish encrypted LS also
                 ls = sp.currentEncryptedLeaseSet;
-                if (ls != null && getFloodfillNetworkDatabaseFacade() != null)
-                    getFloodfillNetworkDatabaseFacade().unpublish(ls);
+                if (ls != null && _floodfillNetworkDatabaseFacade != null)
+                    _floodfillNetworkDatabaseFacade.unpublish(ls);
                 _context.tunnelManager().removeAlias(sp.dest);
                 synchronized(this) {
                     if (sp.rerequestTimer != null)
@@ -572,18 +563,6 @@ class ClientConnectionRunner {
     public int sessionEstablished(SessionConfig config) {
         Destination dest = config.getDestination();
         Hash destHash = dest.calculateHash();
-        if (destHash != null){
-            if (_log.shouldLog(Log.DEBUG)) {
-                _log.debug("Initializing subDb for client" + destHash);
-            }
-            _floodfillNetworkDatabaseFacade = new FloodfillNetworkDatabaseFacade(_context, destHash);
-            _floodfillNetworkDatabaseFacade.startup();
-        } else {
-            if (_log.shouldLog(Log.ERROR)) {
-                _log.error("Initializing subDb for unknown client" + dest, new Exception());
-            }
-            _floodfillNetworkDatabaseFacade = null;
-        }
         if (_log.shouldLog(Log.DEBUG))
             _log.debug("SessionEstablished called for destination " + destHash);
         if (_sessions.size() > MAX_SESSIONS)
@@ -610,6 +589,7 @@ class ClientConnectionRunner {
             _dontSendMSM = "none".equals(opts.getProperty(I2PClient.PROP_RELIABILITY, "").toLowerCase(Locale.US));
             _dontSendMSMOnReceive = Boolean.parseBoolean(opts.getProperty(I2PClient.PROP_FAST_RECEIVE));
         }
+
         // Set up the
         // per-destination session key manager to prevent rather easy correlation
         // based on the specified encryption types in the config
@@ -661,6 +641,12 @@ class ClientConnectionRunner {
                 }
             }
         }
+        if (isPrimary && _floodfillNetworkDatabaseFacade == null) {
+            if (_log.shouldDebug())
+                _log.debug("Initializing subDb for client" + destHash);
+            _floodfillNetworkDatabaseFacade = new FloodfillNetworkDatabaseFacade(_context, destHash);
+            _floodfillNetworkDatabaseFacade.startup();
+        }
         return _manager.destinationEstablished(this, dest);
     }
     
@@ -1172,27 +1158,15 @@ class ClientConnectionRunner {
 
     /**
      * Get the FloodfillNetworkDatabaseFacade for this runner. This is the client
-     * netDb if the router is configured to use subDbs, or the main netDb if the
-     * router is configured to use a monolithic netDb.
+     * netDb.
      * 
-     * If neither a client netDb or the main netDb is available, it will return null.
-     * This should be impossible.
-     * If you get the  `getFloodfillNetworkDatabaseFacade is null for runner` warning,
-     * the main netDb will be returned instead. If the main netDb is null, then null
-     * will be returned.
+     * If a session has not been created yet, it will return null.
      * 
-     * @return _floodfillNetworkDatabaseFacade
+     * @return the client netdb or null if no session was created yet
      * @since 0.9.60
      */
     public FloodfillNetworkDatabaseFacade getFloodfillNetworkDatabaseFacade() {
-        if (_log.shouldLog(Log.DEBUG))
-            _log.debug("getFloodfillNetworkDatabaseFacade is getting the subDb for dbid: " + this.getDestHash());
-        if (_floodfillNetworkDatabaseFacade == null) {
-            if (_log.shouldLog(Log.WARN))
-                _log.warn("getFloodfillNetworkDatabaseFacade is null for runner");
-            return _context.netDb();
-        }
-        return this._floodfillNetworkDatabaseFacade;
+        return _floodfillNetworkDatabaseFacade;
     }
     
     private class MessageDeliveryStatusUpdate extends JobImpl {
diff --git a/router/java/src/net/i2p/router/client/ClientManager.java b/router/java/src/net/i2p/router/client/ClientManager.java
index a0bd6d16cc..af586f9288 100644
--- a/router/java/src/net/i2p/router/client/ClientManager.java
+++ b/router/java/src/net/i2p/router/client/ClientManager.java
@@ -777,8 +777,9 @@ class ClientManager {
      * get the FloodfillNetworkDatabaseFacade associated with a particular client destination.
      * This is inside the runner, so it won't be there if the runner isn't ready.
      * 
-     * @param destHash destination hash associated with the client who's subDb we're looking for
-     * @return may be null if it does not exist and the main netDb is not initialized
+     * @param destHash destination hash associated with the client who's subDb we're looking for, may be null
+     * @return will be null if desthash is null or client does not exist or its netDb is not initialized
+     * @since 0.9.60
      */
     public FloodfillNetworkDatabaseFacade getClientFloodfillNetworkDatabaseFacade(Hash destHash) {
         if (destHash != null) {
@@ -801,15 +802,14 @@ class ClientManager {
      * get all of the FloodfillNetworkDatabaseFacades for all of the clients.
      * 
      * @return non-null
+     * @since 0.9.60
      */
     public Set<FloodfillNetworkDatabaseFacade> getClientFloodfillNetworkDatabaseFacades() {
         Set<FloodfillNetworkDatabaseFacade> rv = new HashSet<FloodfillNetworkDatabaseFacade>();
         for (ClientConnectionRunner runner : _runners.values()) {
-            if (runner != null){
-                FloodfillNetworkDatabaseFacade fndf = runner.getFloodfillNetworkDatabaseFacade();
-                if (fndf != null)
-                    rv.add(fndf);
-            }
+            FloodfillNetworkDatabaseFacade fndf = runner.getFloodfillNetworkDatabaseFacade();
+            if (fndf != null)
+                rv.add(fndf);
         }
         return rv;
     }
@@ -817,7 +817,8 @@ class ClientManager {
     /**
      * get all the primary hashes for all the clients and return them as a set
      * 
-     * @return
+     * @return non-null
+     * @since 0.9.60
      */
     public Set<Hash> getPrimaryHashes() {
         Set<Hash> rv = new HashSet<Hash>();
diff --git a/router/java/src/net/i2p/router/client/ClientMessageEventListener.java b/router/java/src/net/i2p/router/client/ClientMessageEventListener.java
index b65aac84fb..b80a8d62de 100644
--- a/router/java/src/net/i2p/router/client/ClientMessageEventListener.java
+++ b/router/java/src/net/i2p/router/client/ClientMessageEventListener.java
@@ -861,9 +861,9 @@ class ClientMessageEventListener implements I2CPMessageReader.I2CPMessageEventLi
                 _log.warn("Unsupported BlindingInfo type: " + message);
             return;
         }
-        BlindData obd = _runner.getFloodfillNetworkDatabaseFacade().getBlindData(spk);
+        BlindData obd = _context.netDb().getBlindData(spk);
         if (obd == null) {
-            _runner.getFloodfillNetworkDatabaseFacade().setBlindData(bd);
+            _context.netDb().setBlindData(bd);
             if (_log.shouldWarn())
                 _log.warn("New: " + bd);
         } else {
@@ -884,7 +884,7 @@ class ClientMessageEventListener implements I2CPMessageReader.I2CPMessageEventLi
                         return;
                     }
                 }
-                _runner.getFloodfillNetworkDatabaseFacade().setBlindData(bd);
+                _context.netDb().setBlindData(bd);
                 if (_log.shouldWarn())
                     _log.warn("Updated: " + bd);
             } else {
@@ -893,7 +893,7 @@ class ClientMessageEventListener implements I2CPMessageReader.I2CPMessageEventLi
                 if (nexp > oexp) {
                     obd.setExpiration(nexp);
                     // to force save at shutdown
-                    _runner.getFloodfillNetworkDatabaseFacade().setBlindData(obd);
+                    _context.netDb().setBlindData(obd);
                     if (_log.shouldWarn())
                         _log.warn("Updated expiration: " + obd);
                 } else {
diff --git a/router/java/src/net/i2p/router/client/LookupDestJob.java b/router/java/src/net/i2p/router/client/LookupDestJob.java
index 7bdfc50339..09516f299d 100644
--- a/router/java/src/net/i2p/router/client/LookupDestJob.java
+++ b/router/java/src/net/i2p/router/client/LookupDestJob.java
@@ -21,6 +21,7 @@ import net.i2p.data.i2cp.I2CPMessage;
 import net.i2p.data.i2cp.I2CPMessageException;
 import net.i2p.data.i2cp.SessionId;
 import net.i2p.router.JobImpl;
+import net.i2p.router.NetworkDatabaseFacade;
 import net.i2p.router.RouterContext;
 import net.i2p.util.Log;
 
@@ -91,7 +92,7 @@ class LookupDestJob extends JobImpl {
                         try {
                             bd = Blinding.decode(context, b);
                             SigningPublicKey spk = bd.getUnblindedPubKey();
-                            BlindData bd2 = _runner.getFloodfillNetworkDatabaseFacade().getBlindData(spk);
+                            BlindData bd2 = getContext().netDb().getBlindData(spk);
                             if (bd2 != null) {
                                 // BlindData from database may have privkey or secret
                                 // check if we need it but don't have it
@@ -110,7 +111,7 @@ class LookupDestJob extends JobImpl {
                                 long exp = now + ((bd.getAuthRequired() || bd.getSecretRequired()) ? 365*24*60*60*1000L
                                                                                                    :  90*24*68*60*1000L);
                                 bd.setExpiration(exp);
-                                _runner.getFloodfillNetworkDatabaseFacade().setBlindData(bd);
+                                getContext().netDb().setBlindData(bd);
                             }
                             h = bd.getBlindedHash();
                             if (_log.shouldDebug())
@@ -185,7 +186,10 @@ class LookupDestJob extends JobImpl {
             if (timeout > 1500)
                 timeout -= 500;
             // TODO tell router this is an encrypted lookup, skip 38 or earlier ffs?
-            _runner.getFloodfillNetworkDatabaseFacade().lookupDestination(_hash, done, timeout, _fromLocalDest);
+            NetworkDatabaseFacade db = _runner.getFloodfillNetworkDatabaseFacade();
+            if (db == null)
+                db = getContext().netDb();
+            db.lookupDestination(_hash, done, timeout, _fromLocalDest);
         } else {
             // blinding decode fail
             returnFail(HostReplyMessage.RESULT_DECRYPTION_FAILURE);
@@ -204,10 +208,13 @@ class LookupDestJob extends JobImpl {
         }
         public String getName() { return "LeaseSet Lookup Reply to Client"; }
         public void runJob() {
-            Destination dest = _runner.getFloodfillNetworkDatabaseFacade().lookupDestinationLocally(_hash);
+            NetworkDatabaseFacade db = _runner.getFloodfillNetworkDatabaseFacade();
+            if (db == null)
+                db = getContext().netDb();
+            Destination dest = db.lookupDestinationLocally(_hash);
             if (dest == null && _blindData != null) {
                 // TODO store and lookup original hash instead
-                LeaseSet ls = _runner.getFloodfillNetworkDatabaseFacade().lookupLeaseSetLocally(_hash);
+                LeaseSet ls = db.lookupLeaseSetLocally(_hash);
                 if (ls != null && ls.getType() == DatabaseEntry.KEY_TYPE_ENCRYPTED_LS2) {
                     // already decrypted
                     EncryptedLeaseSet encls = (EncryptedLeaseSet) ls;
diff --git a/router/java/src/net/i2p/router/networkdb/kademlia/KademliaNetworkDatabaseFacade.java b/router/java/src/net/i2p/router/networkdb/kademlia/KademliaNetworkDatabaseFacade.java
index 354b426600..d9e6cdc872 100644
--- a/router/java/src/net/i2p/router/networkdb/kademlia/KademliaNetworkDatabaseFacade.java
+++ b/router/java/src/net/i2p/router/networkdb/kademlia/KademliaNetworkDatabaseFacade.java
@@ -866,23 +866,6 @@ public abstract class KademliaNetworkDatabaseFacade extends NetworkDatabaseFacad
             _log.error("locally published leaseSet is not valid?", iae);
             throw iae;
         }
-        String dbid = "main netDb";
-        if (isClientDb()) {
-            dbid = "client netDb: " + _dbid;
-        }
-        if (_localKey != null) {
-            if (!_localKey.equals(localLeaseSet.getHash()))
-                if (_log.shouldLog(Log.ERROR))
-                    _log.error("[" + dbid + "]" + "Error, the local LS hash ("
-                            + _localKey + ") does not match the published hash ("
-                            + localLeaseSet.getHash() + ")! This shouldn't happen!",
-                            new Exception());
-        } else {
-            // This will only happen once when the local LS is first published
-            _localKey = localLeaseSet.getHash();
-            if (_log.shouldLog(Log.INFO))
-                _log.info("[" + dbid + "]" + "Local client LS key initialized to: " + _localKey);
-        }
         if (!_context.clientManager().shouldPublishLeaseSet(h))
             return;
         // If we're exiting, don't publish.
@@ -1681,4 +1664,14 @@ public abstract class KademliaNetworkDatabaseFacade extends NetworkDatabaseFacad
     public void renderStatusHTML(Writer out) throws IOException {
         out.write(_kb.toString().replace("\n", "<br>\n"));
     }
+
+    /**
+     * @since 0.9.60
+     */
+    @Override
+    public String toString() {
+        if (isMainDb())
+            return "Main NetDB";
+        return "Client NetDB " + _dbid.toBase64();
+    }
 }
diff --git a/router/java/src/net/i2p/router/tunnel/pool/AliasedTunnelPool.java b/router/java/src/net/i2p/router/tunnel/pool/AliasedTunnelPool.java
index 8e092d8d3e..ae264d733a 100644
--- a/router/java/src/net/i2p/router/tunnel/pool/AliasedTunnelPool.java
+++ b/router/java/src/net/i2p/router/tunnel/pool/AliasedTunnelPool.java
@@ -9,6 +9,7 @@ import net.i2p.data.TunnelId;
 import net.i2p.router.RouterContext;
 import net.i2p.router.TunnelInfo;
 import net.i2p.router.TunnelPoolSettings;
+import net.i2p.router.NetworkDatabaseFacade;
 import net.i2p.util.Log;
 
 /**
@@ -115,9 +116,14 @@ public class AliasedTunnelPool extends TunnelPool {
 
     @Override
     protected LeaseSet locked_buildNewLeaseSet() {
-        LeaseSet ls =  _context.clientNetDb(_aliasOf.getSettings().getDestination()).lookupLeaseSetLocally(_aliasOf.getSettings().getDestination());
-        if (ls == null)
+        Hash primary = _aliasOf.getSettings().getDestination();
+        NetworkDatabaseFacade db =  _context.clientNetDb(primary);
+        LeaseSet ls =  db.lookupLeaseSetLocally(primary);
+        if (ls == null) {
+            if (_log.shouldWarn())
+                _log.warn("No primary LS " + primary + " to copy for " + getSettings().getDestination() + " in db " + db);
             return null;
+        }
         // copy everything so it isn't corrupted
         LeaseSet rv = new LeaseSet();
         for (int i = 0; i < ls.getLeaseCount(); i++) {
-- 
GitLab