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 <[email protected]> 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:[email protected]";>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:[email protected]";>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); + } + } + } }
