This is an automated email from the ASF dual-hosted git repository.

twolf pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git

commit 9e4e040edebab7eb3616dabc382b742714b9f580
Author: Thomas Wolf <[email protected]>
AuthorDate: Tue Aug 6 20:31:26 2024 +0200

    Clean-up: timeout-free KEX
    
    Do not wait synchronously until the kexInitializedFuture is fulfilled.
    Instead simply submit the negotiation as a listener on that future.
    It'll run once our own KEX proposal has been composed.
    
    Waiting synchronously has the potential to block if the MINA transport
    is used.
    
    Deprecate CoreModuleProperties.KEX_PROPOSAL_SETUP_TIMEOUT; it's unused
    now.
---
 .../common/session/helpers/AbstractSession.java    | 35 ++++++++++++++--------
 .../org/apache/sshd/core/CoreModuleProperties.java | 11 ++++---
 2 files changed, 28 insertions(+), 18 deletions(-)

diff --git 
a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
 
b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
index 31589a4bf..023ea777b 100644
--- 
a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
+++ 
b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
@@ -890,25 +890,36 @@ public abstract class AbstractSession extends 
SessionHelper {
                 sendKexInit();
                 break;
             case BOTH:
-                DefaultKeyExchangeFuture initFuture;
-                synchronized (kexState) {
-                    initFuture = kexInitializedFuture;
-                    if (initFuture == null) {
-                        initFuture = new DefaultKeyExchangeFuture(toString(), 
null);
-                        kexInitializedFuture = initFuture;
-                    }
-                }
-                // requestNewKeyExchange() is running in some other thread: 
wait until it has set up our own proposal.
-                // The timeout is a last resort only to avoid blocking 
indefinitely in case something goes
-                // catastrophically wrong somewhere; it should never be hit. 
If it is, an exception will be thrown.
+                // We are in the process of sending our own KEX_INIT. Do the 
negotiation once that's done.
                 //
                 // See https://issues.apache.org/jira/browse/SSHD-1197
-                
initFuture.await(CoreModuleProperties.KEX_PROPOSAL_SETUP_TIMEOUT.getRequired(this));
                 break;
             default:
                 throw new IllegalStateException("Received SSH_MSG_KEXINIT 
while key exchange is running");
         }
+        // Note: we should not wait here; it might block (in particular with 
the MINA transport back-end).
+        DefaultKeyExchangeFuture initFuture;
+        synchronized (kexState) {
+            initFuture = kexInitializedFuture;
+            if (initFuture == null) {
+                initFuture = new DefaultKeyExchangeFuture(toString(), null);
+                kexInitializedFuture = initFuture;
+            }
+        }
+        initFuture.addListener(f -> {
+            if (f.isDone()) {
+                try {
+                    performKexNegotiation();
+                } catch (Exception e) {
+                    exceptionCaught(e);
+                }
+            } else {
+                exceptionCaught(f.getException());
+            }
+        });
+    }
 
+    protected void performKexNegotiation() throws Exception {
         Map<KexProposalOption, String> result = negotiate();
         String kexAlgorithm = result.get(KexProposalOption.ALGORITHMS);
         Collection<? extends KeyExchangeFactory> kexFactories = 
getKeyExchangeFactories();
diff --git 
a/sshd-core/src/main/java/org/apache/sshd/core/CoreModuleProperties.java 
b/sshd-core/src/main/java/org/apache/sshd/core/CoreModuleProperties.java
index d8c8d35c1..d130648d5 100644
--- a/sshd-core/src/main/java/org/apache/sshd/core/CoreModuleProperties.java
+++ b/sshd-core/src/main/java/org/apache/sshd/core/CoreModuleProperties.java
@@ -138,14 +138,13 @@ public final class CoreModuleProperties {
             = Property.bool("allow-dhg1-kex-fallback", false);
 
     /**
-     * If the peer initiates a key exchange, we send our own KEX_INIT message 
with the proposal. This is a last-resort
-     * timeout for waiting until we have prepared our own KEX proposal. This 
timeout should actually never be hit unless
-     * there is a serious deadlock somewhere and the session is never closed. 
It should be set to a reasonably high
-     * value; it must be at least 5 seconds and the default is 42 seconds. If 
the timeout is ever hit, the key exchange
-     * initiated by the peer will fail.
+     * Unused.
+     *
+     * @deprecated since 2.14.0
      */
+    @Deprecated
     public static final Property<Duration> KEX_PROPOSAL_SETUP_TIMEOUT
-            = Property.durationSec("kex-proposal-setup-timeout", 
Duration.ofSeconds(42), Duration.ofSeconds(5));
+            = Property.duration("kex-proposal-setup-timeout", Duration.ZERO);
 
     /**
      * Key used to set the heartbeat interval in milliseconds (0 to disable = 
default)

Reply via email to