[apacheGH-445] Added StrictKexTest

lgoldstein · Dec 22, 2023 · 3e04fd7 · 3e04fd7
1 parent 1c11e3a
commit 3e04fd7
Show file tree

Hide file tree

Showing 9 changed files with 435 additions and 6 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -58,5 +58,12 @@ Provide (read-only) public access to internal session state values related to KE
 
 ## Potential compatibility issues
 
+### Added finite wait time for default implementation of `ClientSession#executeRemoteCommand`
+
+* `CoreModuleProperties#EXEC_CHANNEL_OPEN_TIMEOUT` - default = 30 seconds.
+* `CoreModuleProperties#EXEC_CHANNEL_CMD_TIMEOUT` - default = 30 seconds.
+
+This may cause failures for code that was running long execution commands using the default method implementations.
+
 ## Major Code Re-factoring
 
diff --git a/sshd-common/src/test/java/org/apache/sshd/util/test/JUnitTestSupport.java b/sshd-common/src/test/java/org/apache/sshd/util/test/JUnitTestSupport.java
@@ -703,7 +703,7 @@ public static void outputDebugMessage(String format, Object o) {
 
     public static void outputDebugMessage(String format, Object... args) {
         if (OUTPUT_DEBUG_MESSAGES) {
-            outputDebugMessage(String.format(format, args));
+            outputDebugMessage(GenericUtils.isEmpty(args) ? format : String.format(format, args));
         }
     }
 
@@ -713,6 +713,24 @@ public static void outputDebugMessage(Object message) {
         }
     }
 
+    public static void failWithWrittenErrorMessage(String format, Object... args) {
+        failWithWrittenErrorMessage(GenericUtils.isEmpty(args) ? format : String.format(format, args));
+    }
+
+    public static void failWithWrittenErrorMessage(Object message) {
+        writeErrorMessage(message);
+        fail(Objects.toString(message));
+    }
+
+    public static void writeErrorMessage(String format, Object... args) {
+        writeErrorMessage(GenericUtils.isEmpty(args) ? format : String.format(format, args));
+    }
+
+    public static void writeErrorMessage(Object message) {
+        System.err.append("===[ERROR]=== ").println(message);
+        System.err.flush();
+    }
+
     /* ---------------------------------------------------------------------------- */
 
     public static void replaceJULLoggers() {

diff --git a/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSession.java b/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSession.java
@@ -59,6 +59,7 @@
 import org.apache.sshd.common.util.io.output.NoCloseOutputStream;
 import org.apache.sshd.common.util.io.output.NullOutputStream;
 import org.apache.sshd.common.util.net.SshdSocketAddress;
+import org.apache.sshd.core.CoreModuleProperties;
 
 /**
  * <P>
@@ -304,10 +305,15 @@ default void executeRemoteCommand(
              ClientChannel channel = createExecChannel(command)) {
             channel.setOut(channelOut);
             channel.setErr(channelErr);
-            channel.open().await(); // TODO use verify and a configurable timeout
 
-            // TODO use a configurable timeout
-            Collection<ClientChannelEvent> waitMask = channel.waitFor(REMOTE_COMMAND_WAIT_EVENTS, 0L);
+            Duration openTimeout =
+                CoreModuleProperties.EXEC_CHANNEL_OPEN_TIMEOUT.getRequired(channel);
+            channel.open().verify(openTimeout);
+
+            Duration execTimeout =
+                CoreModuleProperties.EXEC_CHANNEL_CMD_TIMEOUT.getRequired(channel);
+            Collection<ClientChannelEvent> waitMask =
+                channel.waitFor(REMOTE_COMMAND_WAIT_EVENTS, execTimeout);
             if (waitMask.contains(ClientChannelEvent.TIMEOUT)) {
                 throw new SocketTimeoutException("Failed to retrieve command result in time: " + command);
             }

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
@@ -61,6 +61,20 @@ public final class CoreModuleProperties {
     public static final Property<Duration> CHANNEL_OPEN_TIMEOUT
             = Property.duration("ssh-agent-server-channel-open-timeout", Duration.ofSeconds(30));
 
+    /**
+     * Value that can be set on the {@link org.apache.sshd.common.FactoryManager} the session or the channel to configure the
+     * channel open timeout value (millis) for executing a remote command using default implementation.
+     */
+    public static final Property<Duration> EXEC_CHANNEL_OPEN_TIMEOUT
+           = Property.duration("ssh-exec-channel-open-timeout", Duration.ofSeconds(30));
+
+    /**
+     * Value that can be set on the {@link org.apache.sshd.common.FactoryManager} the session or the channel to configure the
+     * channel command execution timeout value (millis) for executing a remote command using default implementation.
+     */
+    public static final Property<Duration> EXEC_CHANNEL_CMD_TIMEOUT
+          = Property.duration("ssh-exec-channel-cmd-timeout", Duration.ofSeconds(30));
+
     /**
      * Value used to configure the type of proxy forwarding channel to be used. See also
      * https://tools.ietf.org/html/draft-ietf-secsh-agent-02

diff --git a/sshd-core/src/test/java/org/apache/sshd/client/ClientTest.java b/sshd-core/src/test/java/org/apache/sshd/client/ClientTest.java
@@ -1510,7 +1510,6 @@ public void testKeyboardInteractiveInSessionUserInteractiveFailure() throws Exce
         CoreModuleProperties.PASSWORD_PROMPTS.set(client, maxPrompts);
         AtomicInteger numberOfRequests = new AtomicInteger();
         UserAuthKeyboardInteractiveFactory auth = new UserAuthKeyboardInteractiveFactory() {
-
             @Override
             public UserAuthKeyboardInteractive createUserAuth(ClientSession session) throws IOException {
                 return new UserAuthKeyboardInteractive() {