You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by tw...@apache.org on 2026/07/01 19:51:22 UTC
(mina-sshd) 03/14: GH-899: Fix ProcessShellFactory
This is an automated email from the ASF dual-hosted git repository.
tomaswolf pushed a commit to branch rc-2.19.0
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
commit 5b9d431f38546a150ab91e8f7ed8de11779dbf8b
Author: Thomas Wolf <tw...@apache.org>
AuthorDate: Sat Jun 6 14:32:34 2026 +0200
GH-899: Fix ProcessShellFactory
Also explicitly run a shell on Linux. Add caveats to the javadoc and to
the documentation page.
---
CHANGES.md | 1 +
docs/server-setup.md | 6 +++-
.../java/org/apache/sshd/common/util/OsUtils.java | 2 +-
.../shell/InteractiveProcessShellFactory.java | 10 +++++-
.../sshd/server/shell/ProcessShellFactory.java | 39 ++++++++++++----------
.../apache/sshd/server/shell/ProcessShellTest.java | 28 +++++++++++++---
6 files changed, 61 insertions(+), 25 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index 6ecffda8b..effeff8ba 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -32,6 +32,7 @@
## Bug Fixes
+* [GH-899](https://github.com/apache/mina-sshd/issues/899) Fix `ProcessShellFactory` on Linux
* [GH-902](https://github.com/apache/mina-sshd/pull/902) Fix client-side handling of sk-* public key signatures (also in the agent interfaces)
* Limit size of decompressed SSH packets
* Improve checking SSH user certificates in public-key authentication
diff --git a/docs/server-setup.md b/docs/server-setup.md
index 408196045..019a5e586 100644
--- a/docs/server-setup.md
+++ b/docs/server-setup.md
@@ -46,12 +46,16 @@ so it's mostly useful to launch the OS native shell. E.g.,
```java
sshd.setShellFactory(new ProcessShellFactory(new String[] { "/bin/sh", "-i", "-l" }));
-
```
There is an out-of-the-box `InteractiveProcessShellFactory` that detects the O/S and spawns the relevant shell. Note
that the `ShellFactory` is not required. If none is configured, any request for an interactive shell will be denied to clients.
+**Caveat:** Apache MINA SSHD does _not_ provide privilege separation, and SSH users are by default _not_ tied to OS users.
+The OS shell created by `ProcessShellFactory` or `InteractiveProcessShellFactory` will run as the same OS user the process using
+Apache MINA SSHD runs. The shell will have the same access rights as the Apache MINA SSHD server process itself. It is in
+general _not_ recommended to use these shell factories as-is in a production system.
+
Furthermore, one can select a specific factory based on the current session by using an `AggregateShellFactory` that
wraps a group of `ShellFactorySelector` - each one tailored for a specific set of criteria. The simplest use-case is
one the detects the client and provides a specially tailored shell for it - e.g.,
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/util/OsUtils.java b/sshd-common/src/main/java/org/apache/sshd/common/util/OsUtils.java
index c8905096e..556e69fa4 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/util/OsUtils.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/util/OsUtils.java
@@ -77,7 +77,7 @@ public final class OsUtils {
public static final List<String> LINUX_COMMAND
= Collections.unmodifiableList(Arrays.asList(LINUX_SHELL_COMMAND_NAME, "-i", "-l"));
public static final List<String> WINDOWS_COMMAND
- = Collections.unmodifiableList(Collections.singletonList(WINDOWS_SHELL_COMMAND_NAME));
+ = Collections.singletonList(WINDOWS_SHELL_COMMAND_NAME);
/**
* System properties consulted in order to detect {@link #isAndroid() Android O/S}.
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/shell/InteractiveProcessShellFactory.java b/sshd-core/src/main/java/org/apache/sshd/server/shell/InteractiveProcessShellFactory.java
index 2e2fca136..150dbbefc 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/shell/InteractiveProcessShellFactory.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/shell/InteractiveProcessShellFactory.java
@@ -25,7 +25,15 @@ import org.apache.sshd.common.util.OsUtils;
import org.apache.sshd.server.channel.ChannelSession;
/**
- * A simplistic interactive shell factory
+ * A simplistic interactive shell factory.
+ * <p>
+ * <b>Caveat:</b> Apache MINA SSHD does <em>not</em> provide privilege separation, and SSH users are by default
+ * <em>not</em> tied to OS users. The OS shell will run as the same OS user the process using Apache MINA SSHD runs. The
+ * shell will have the same access rights as the Apache MINA SSHD server process itself.
+ * </p>
+ * <p>
+ * It is in general <em>not</em> recommended to use this class as is in a production server.
+ * </p>
*
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/shell/ProcessShellFactory.java b/sshd-core/src/main/java/org/apache/sshd/server/shell/ProcessShellFactory.java
index 55f27b047..1d4e3ebeb 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/shell/ProcessShellFactory.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/shell/ProcessShellFactory.java
@@ -31,7 +31,15 @@ import org.apache.sshd.server.channel.ChannelSession;
import org.apache.sshd.server.command.Command;
/**
- * A {@link Factory} of {@link Command} that will create a new process and bridge the streams.
+ * A {@link Factory} of {@link Command} that will create a new process executing the given command in an OS shell.
+ * <p>
+ * <b>Caveat:</b> Apache MINA SSHD does <em>not</em> provide privilege separation, and SSH users are by default
+ * <em>not</em> tied to OS users. The OS shell will run as the same OS user the process using Apache MINA SSHD runs. The
+ * shell will have the same access rights as the Apache MINA SSHD server process itself.
+ * </p>
+ * <p>
+ * It is in general <em>not</em> recommended to use this class as is in a production server.
+ * </p>
*
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
@@ -86,25 +94,20 @@ public class ProcessShellFactory extends AbstractLoggingBean implements ShellFac
return new ProcessShell(resolveEffectiveCommand(channel, getCommand(), getElements()));
}
+ /**
+ * Determines the shell command to run. On Windows {@code cmd.exe /C} is used; on other systems {@code /bin/sh -c}.
+ *
+ * @param channel {@link ChannelSession} the shell will be executed in
+ * @param rawCommand the shell command passed
+ * @param parsedElements legacy; unused
+ * @return a list containing the full command to run to execute the given {@code rawCommand}
+ */
protected List<String> resolveEffectiveCommand(
ChannelSession channel, String rawCommand, List<String> parsedElements) {
- if (!OsUtils.isWin32()) {
- return ValidateUtils.checkNotNullAndNotEmpty(parsedElements, "No parsed command elements");
- }
-
- // Turns out that running a command with no arguments works just fine in Windows
- if (GenericUtils.size(parsedElements) <= 1) {
- return ValidateUtils.checkNotNullAndNotEmpty(parsedElements, "No parsed command elements");
+ ValidateUtils.checkNotNullAndNotEmpty(rawCommand, "No command");
+ if (OsUtils.isWin32()) {
+ return Arrays.asList(OsUtils.WINDOWS_SHELL_COMMAND_NAME, "/C", rawCommand);
}
-
- // For windows create a "cmd.exe /C "..."" string
- String cmdName = parsedElements.get(0);
- // If already using shell prefix then assume callers knows what they're doing
- if (OsUtils.WINDOWS_SHELL_COMMAND_NAME.equalsIgnoreCase(cmdName)) {
- return ValidateUtils.checkNotNullAndNotEmpty(parsedElements, "No parsed command elements");
- }
-
- return Arrays.asList(OsUtils.WINDOWS_SHELL_COMMAND_NAME, "/C",
- ValidateUtils.checkNotNullAndNotEmpty(rawCommand, "No command"));
+ return Arrays.asList(OsUtils.LINUX_SHELL_COMMAND_NAME, "-c", rawCommand);
}
}
diff --git a/sshd-core/src/test/java/org/apache/sshd/server/shell/ProcessShellTest.java b/sshd-core/src/test/java/org/apache/sshd/server/shell/ProcessShellTest.java
index 75b9b8b38..4c0bf7ec4 100644
--- a/sshd-core/src/test/java/org/apache/sshd/server/shell/ProcessShellTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/server/shell/ProcessShellTest.java
@@ -21,6 +21,7 @@ package org.apache.sshd.server.shell;
import java.io.ByteArrayOutputStream;
import java.io.OutputStream;
import java.nio.charset.StandardCharsets;
+import java.time.Duration;
import org.apache.sshd.client.SshClient;
import org.apache.sshd.client.channel.ChannelShell;
@@ -30,6 +31,7 @@ import org.apache.sshd.server.SshServer;
import org.apache.sshd.util.test.BaseTestSupport;
import org.apache.sshd.util.test.CoreTestSupportUtils;
import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -76,9 +78,7 @@ public class ProcessShellTest extends BaseTestSupport {
*/
@Test
void testNoRedundantEchoOnStderr() throws Exception {
- if (OsUtils.isWin32()) {
- return;
- }
+ Assumptions.assumeFalse(OsUtils.isWin32());
try (SshServer localSshd = CoreTestSupportUtils.setupTestServer(getClass())) {
localSshd.setShellFactory(new ProcessShellFactory("cat", "cat"));
@@ -100,7 +100,7 @@ public class ProcessShellTest extends BaseTestSupport {
pipe.write("hello\n".getBytes(StandardCharsets.UTF_8));
pipe.flush();
- Thread.sleep(1000);
+ channel.waitFor(ClientSession.REMOTE_COMMAND_WAIT_EVENTS, Duration.ofSeconds(2));
String stderr = err.toString(StandardCharsets.UTF_8.name());
assertFalse(stderr.contains("hello"), "Redundant echo detected on stderr: " + stderr);
@@ -113,4 +113,24 @@ public class ProcessShellTest extends BaseTestSupport {
}
}
}
+
+ @Test
+ void testShellCommand() throws Exception {
+ Assumptions.assumeFalse(OsUtils.isWin32());
+
+ try (SshServer localSshd = CoreTestSupportUtils.setupTestServer(getClass())) {
+ localSshd.setCommandFactory(new ProcessShellCommandFactory());
+ localSshd.start();
+
+ try (ClientSession session = client.connect(getCurrentTestName(), "localhost", localSshd.getPort()).verify(10_000)
+ .getSession()) {
+ session.addPasswordIdentity(getCurrentTestName());
+ session.auth().verify(10_000);
+
+ assertEquals("hello\nbye\n", session.executeRemoteCommand("echo 'hello'; echo 'bye'", Duration.ofSeconds(2)));
+ } finally {
+ localSshd.stop(true);
+ }
+ }
+ }
}