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 3f036c581a6ee88847b8348407891b4f4dcf4de5 Author: Thomas Wolf <[email protected]> AuthorDate: Fri Jun 26 00:45:25 2026 +0200 Restrict remote execution of git commands Running arbitrary git commands via JGit remotely is a potentially problematic idea unless one has an OS-level chrooted environment. The implementation with GitPgmCommand(Factory) also leads to uses where each command is executed in a new channel, which restricts the usefulness of the feature somewhat. One use case for which it might make sense is in a git server to provide some authenticated powerusers some maintenance commands. A git server normally stores bare git repositories that it makes accessible for fetching and pushing via git-receive-pack and git-upload-pack. For bare repositories, only a subset of commands might make sense as maintenance commands. Even powerusers should not be able to create arbitrary files on such a git server. So restrict the available commands to archive, blame, branch, describe, diff, gc, log, reflog, show, and status. These commands can be run via the GitPgmCommand on bare repositories stored on the server, and return their output on the channel's output stream. They can be executed from a client via session.executeRemoteCommand("git --git-dir <repo> <cmd>"), where <repo> is the server's advertised path for the bare git repository. The archive command ignores the --output or -o option and always returns the archive via the output stream. If for any reason the old behavior is wanted and arbitrary commands like "init" or "add" should be allowed on the server, set the new global property GitModuleProperties.RESTRICT_COMMANDS to false on the server session. Update the old README.txt file. --- CHANGES.md | 28 +++++- sshd-git/README.txt | 21 +++- .../org/apache/sshd/git/GitModuleProperties.java | 9 ++ .../apache/sshd/git/pgm/EmbeddedCommandRunner.java | 77 ++++++++++++--- .../org/apache/sshd/git/pgm/GitPgmCommand.java | 5 +- .../org/apache/sshd/git/pgm/SubcommandHandler.java | 61 ++++++++++++ .../org/apache/sshd/git/pgm/GitPgmCommandTest.java | 108 +++++++++++++-------- 7 files changed, 249 insertions(+), 60 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 6c19fa96d..8ca9becbc 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -36,15 +36,41 @@ * [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 -* Improve handling of repository paths in `sshd-git` on Windows. +* Improve handling of repository paths in `sshd-git` on Windows * Validate file names in SCP * Escape newlines in filenames in the SCP protocol +* Restrict JGit commands accessible via `GitPgmCommandFactory` in `sshd-git` ## New Features ## Potential Compatibility Issues +### Restrict JGit commands accessible via `GitPgmCommandFactory` in `sshd-git` + +Bundle `sshd-git` contains a `GitPgmCommandFactory` that can be configured for an Apache MINA SSHD server. +It enables remote execution of git commands via JGit. + +However, running arbitrary git commands remotely is an exotic use case, and some commands (like +`git archive --format zip --output somefile.zip` or also `git clone` or `git checkout`) could even create +files on the server. This should be only allowed in an OS-level chrooted environment, which Apache MINA +SSHD does not and cannot provide. + +We have therefore limited the available commands to a small set of whitelisted git commands that might +be useful as maintenance commands for authorized power users on a git server implemented based on `sshd-git`. +The command are: `archive`, `blame`, `branch`, `describe`, `diff`, `gc`, `log`, `reflog`, `show`, and `status`. +All these commands can be executed from an client via `git --git-dir <repo> <command> <args>`, where `<repo>` +is the server-side name of the git repository (its local path on the server relative to the configured +root), `<command>` is the command name (e.g., `archive`) and `<args>` are the command arguments. Results are +sent back via the command output streams (output or error streams) and are thus sent back to the client. +For the `archive`command, the `--output` (or `-o`) argument is ignored; the archive is always returned via +the command's output stream and this is also sent to the client. + +Applications that might have relied on the ability to run other JGit command remotely may no longer work. +Either implement your own dedicated command factory to enable accesss to certain commands, or set the +property `GitModuleProperties.RESTRICT_COMMANDS` to `false` on the server-side `SshServer` (for all sessions) +or on particular `ServerSession`s (for particular individual sessions) to re-enable access to _all_ commands +supported by JGit. ## Major Code Re-factoring diff --git a/sshd-git/README.txt b/sshd-git/README.txt index 4977b9828..6c74ce848 100644 --- a/sshd-git/README.txt +++ b/sshd-git/README.txt @@ -1,4 +1,17 @@ -This module contains an SSHD CommandFactory to support git through SSH. -The git commands are provided by a modified version of jgit-pgm -available at: - https://github.com/gnodet/jgit/tree/413522 +This module provides two command factories for git over SSH. +It uses JGit to implement the git functionality. + +The GitPackCommand implements basic git push and fetch support +via git-receive-pack and git-upload-pack for an SSH git server. + +The GitPgmCommand provides remote executon of git commands, so +clients can execute git commands on a server. This is by default +limited to non-invasive commands that might make sense on a git +server. The property GitModuleProperties.RESTRICT_COMMANDS can be +set to false on a server session to allow execution of arbitrary +git commands (if supported by JGit), but this is not recommended. + +Finally, there is support for client-side git-over-SSH though the +GitSshdSession. However, note that JGit includes in its bundle +org.eclipse.jgit.ssh.apache its own git-over-SSH implementation +based on Apache MINA SSHD. \ No newline at end of file diff --git a/sshd-git/src/main/java/org/apache/sshd/git/GitModuleProperties.java b/sshd-git/src/main/java/org/apache/sshd/git/GitModuleProperties.java index 7ede9b42a..9f01568b2 100644 --- a/sshd-git/src/main/java/org/apache/sshd/git/GitModuleProperties.java +++ b/sshd-git/src/main/java/org/apache/sshd/git/GitModuleProperties.java @@ -57,6 +57,15 @@ public final class GitModuleProperties { public static final Property<String> GIT_PROTOCOL_VERSION = Property.string("git-ssh-protocol-version"); + /** + * Property defining whether only a subset of "safe" JGit command shall be allowed by the + * {@link org.apache.sshd.git.pgm.GitPgmCommand}. By default {@code true}. Setting this to {@code false} will allow + * any command supported by JGit, but this would be unsafe. The intent of these commands is to provide authorized + * users with some maintenance commands on (bare) git repositories stored in a git server. + */ + public static final Property<Boolean> RESTRICT_COMMANDS + = Property.bool("git-commands-restricted", true); + private GitModuleProperties() { // private } diff --git a/sshd-git/src/main/java/org/apache/sshd/git/pgm/EmbeddedCommandRunner.java b/sshd-git/src/main/java/org/apache/sshd/git/pgm/EmbeddedCommandRunner.java index a6296ae90..0338a341e 100644 --- a/sshd-git/src/main/java/org/apache/sshd/git/pgm/EmbeddedCommandRunner.java +++ b/sshd-git/src/main/java/org/apache/sshd/git/pgm/EmbeddedCommandRunner.java @@ -29,9 +29,13 @@ import java.lang.reflect.Method; import java.nio.file.Path; import java.text.MessageFormat; import java.util.ArrayList; +import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Objects; +import java.util.Set; +import org.apache.sshd.common.util.io.output.NoCloseOutputStream; import org.apache.sshd.git.AbstractGitCommand; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.lib.Repository; @@ -41,7 +45,6 @@ import org.eclipse.jgit.pgm.Die; import org.eclipse.jgit.pgm.TextBuiltin; import org.eclipse.jgit.pgm.internal.CLIText; import org.eclipse.jgit.pgm.opt.CmdLineParser; -import org.eclipse.jgit.pgm.opt.SubcommandHandler; import org.eclipse.jgit.util.io.ThrowingPrintWriter; import org.kohsuke.args4j.Argument; import org.kohsuke.args4j.CmdLineException; @@ -54,6 +57,24 @@ import org.kohsuke.args4j.OptionHandlerFilter; * @author <a href="mailto:[email protected]";>Apache MINA SSHD Project</a> */ public class EmbeddedCommandRunner { + + private static final Set<String> WHITELIST = new HashSet<>(); + + static { + WHITELIST.add("archive"); + WHITELIST.add("blame"); + WHITELIST.add("branch"); + WHITELIST.add("describe"); + WHITELIST.add("diff"); + WHITELIST.add("gc"); + WHITELIST.add("log"); + WHITELIST.add("reflog"); + WHITELIST.add("show"); + WHITELIST.add("status"); + } + + private final boolean useWhitelist; + @Option(name = "--help", usage = "usage_displayThisHelpText", aliases = { "-h" }) private boolean help; @@ -64,15 +85,20 @@ public class EmbeddedCommandRunner { private String gitdir; @Argument(index = 0, metaVar = "metaVar_command", required = true, handler = SubcommandHandler.class) - private TextBuiltin subcommand; + private CommandRef subcommand; @Argument(index = 1, metaVar = "metaVar_arg") private List<String> arguments = new ArrayList<>(); private Path rootDir; - public EmbeddedCommandRunner(Path rootDir) { + public EmbeddedCommandRunner(Path rootDir, boolean useWhitelist) { this.rootDir = Objects.requireNonNull(rootDir, "No root directory specified"); + this.useWhitelist = useWhitelist; + } + + public EmbeddedCommandRunner(Path rootDir) { + this(rootDir, true); } /** @@ -110,18 +136,22 @@ public class EmbeddedCommandRunner { final CommandRef[] common = CommandCatalog.common(); int width = 0; for (final CommandRef c : common) { - width = Math.max(width, c.getName().length()); + if (!useWhitelist || WHITELIST.contains(c.getName())) { + width = Math.max(width, c.getName().length()); + } } width += 2; for (final CommandRef c : common) { - writer.print(' '); - writer.print(c.getName()); - for (int i = c.getName().length(); i < width; i++) { + if (!useWhitelist || WHITELIST.contains(c.getName())) { writer.print(' '); + writer.print(c.getName()); + for (int i = c.getName().length(); i < width; i++) { + writer.print(' '); + } + writer.print(CLIText.get().resourceBundle().getString(c.getUsage())); + writer.println(); } - writer.print(CLIText.get().resourceBundle().getString(c.getUsage())); - writer.println(); } writer.println(); } @@ -131,9 +161,18 @@ public class EmbeddedCommandRunner { gitdir = Objects.toString(AbstractGitCommand.resolveGitRepo(rootDir, gitdir)); - TextBuiltin cmd = subcommand; + String subcommandName = subcommand.getName(); + if (useWhitelist && !WHITELIST.contains(subcommandName)) { + throw new Die("JGit subcommand '" + subcommandName + "' not allowed by whitelist"); + } + if ("archive".equals(subcommandName) && useWhitelist) { + // Make archive return the archive via stdout + arguments = removeOutput(arguments); + } + TextBuiltin cmd = subcommand.create(); + set(cmd, "ins", in); - set(cmd, "outs", out); + set(cmd, "outs", new NoCloseOutputStream(out)); // Archive closes the output stream! set(cmd, "errs", err); Boolean success = (Boolean) call(cmd, "requiresRepository"); @@ -154,6 +193,22 @@ public class EmbeddedCommandRunner { } } + private List<String> removeOutput(List<String> args) { + List<String> result = new ArrayList<>(args.size()); + Iterator<String> a = args.iterator(); + while (a.hasNext()) { + String opt = a.next(); + if ("--output".equals(opt) || "-o".equals(opt)) { + if (a.hasNext()) { + a.next(); + } + } else { + result.add(opt); + } + } + return result; + } + private Object get(Object obj, String name) throws IllegalAccessException, NoSuchFieldException { Class<?> clazz = obj.getClass(); while (clazz != null) { diff --git a/sshd-git/src/main/java/org/apache/sshd/git/pgm/GitPgmCommand.java b/sshd-git/src/main/java/org/apache/sshd/git/pgm/GitPgmCommand.java index 7d13da89a..7c0b5b502 100644 --- a/sshd-git/src/main/java/org/apache/sshd/git/pgm/GitPgmCommand.java +++ b/sshd-git/src/main/java/org/apache/sshd/git/pgm/GitPgmCommand.java @@ -28,6 +28,7 @@ import org.apache.sshd.common.util.ValidateUtils; import org.apache.sshd.common.util.threads.CloseableExecutorService; import org.apache.sshd.git.AbstractGitCommand; import org.apache.sshd.git.GitLocationResolver; +import org.apache.sshd.git.GitModuleProperties; /** * TODO Add javadoc @@ -35,6 +36,7 @@ import org.apache.sshd.git.GitLocationResolver; * @author <a href="mailto:[email protected]";>Apache MINA SSHD Project</a> */ public class GitPgmCommand extends AbstractGitCommand { + /** * @param rootDirResolver Resolver for GIT root directory * @param command Command to execute @@ -69,7 +71,8 @@ public class GitPgmCommand extends AbstractGitCommand { Path rootDir = resolver.resolveRootDirectory(command, args, getServerSession(), getFileSystem()); ValidateUtils.checkState(rootDir != null, "No root directory provided for %s command", command); - new EmbeddedCommandRunner(rootDir).execute(args, getInputStream(), getOutputStream(), err); + new EmbeddedCommandRunner(rootDir, GitModuleProperties.RESTRICT_COMMANDS.getRequired(getServerSession())) + .execute(args, getInputStream(), getOutputStream(), err); onExit(0); } catch (Throwable t) { try { diff --git a/sshd-git/src/main/java/org/apache/sshd/git/pgm/SubcommandHandler.java b/sshd-git/src/main/java/org/apache/sshd/git/pgm/SubcommandHandler.java new file mode 100644 index 000000000..e616ef08e --- /dev/null +++ b/sshd-git/src/main/java/org/apache/sshd/git/pgm/SubcommandHandler.java @@ -0,0 +1,61 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.sshd.git.pgm; + +import org.eclipse.jgit.pgm.CommandCatalog; +import org.eclipse.jgit.pgm.CommandRef; +import org.eclipse.jgit.pgm.internal.CLIText; +import org.kohsuke.args4j.CmdLineException; +import org.kohsuke.args4j.CmdLineParser; +import org.kohsuke.args4j.OptionDef; +import org.kohsuke.args4j.spi.OptionHandler; +import org.kohsuke.args4j.spi.Parameters; +import org.kohsuke.args4j.spi.Setter; + +public class SubcommandHandler extends OptionHandler<CommandRef> { + private final org.eclipse.jgit.pgm.opt.CmdLineParser clp; + + public SubcommandHandler(final CmdLineParser parser, final OptionDef option, final Setter<? super CommandRef> setter) { + super(parser, option, setter); + clp = (org.eclipse.jgit.pgm.opt.CmdLineParser) parser; + } + + /** {@inheritDoc} */ + @Override + public int parseArguments(Parameters params) throws CmdLineException { + final String name = params.getParameter(0); + final CommandRef cr = CommandCatalog.get(name); + if (cr == null) { + throw new CmdLineException(clp, CLIText.format(CLIText.get().notAJgitCommand), name); + } + // Force option parsing to stop. Everything after us should + // be arguments known only to this command and must not be + // recognized by the current parser. + // + owner.stopOptionParsing(); + setter.addValue(cr); + return 1; + } + + /** {@inheritDoc} */ + @Override + public String getDefaultMetaVariable() { + return CLIText.get().metaVar_command; + } +} diff --git a/sshd-git/src/test/java/org/apache/sshd/git/pgm/GitPgmCommandTest.java b/sshd-git/src/test/java/org/apache/sshd/git/pgm/GitPgmCommandTest.java index d3b47e108..1a5fec73b 100644 --- a/sshd-git/src/test/java/org/apache/sshd/git/pgm/GitPgmCommandTest.java +++ b/sshd-git/src/test/java/org/apache/sshd/git/pgm/GitPgmCommandTest.java @@ -18,22 +18,22 @@ */ package org.apache.sshd.git.pgm; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; -import java.util.Collection; +import java.rmi.RemoteException; import java.util.Collections; -import java.util.EnumSet; -import java.util.concurrent.TimeUnit; +import java.util.zip.ZipEntry; +import java.util.zip.ZipInputStream; import org.apache.sshd.client.SshClient; -import org.apache.sshd.client.channel.ChannelExec; -import org.apache.sshd.client.channel.ClientChannelEvent; import org.apache.sshd.client.session.ClientSession; import org.apache.sshd.common.util.net.SshdSocketAddress; import org.apache.sshd.git.GitLocationResolver; import org.apache.sshd.git.GitTestSupport; import org.apache.sshd.server.SshServer; -import org.apache.sshd.sftp.server.SftpSubsystemFactory; import org.apache.sshd.util.test.CommonTestSupportUtils; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.util.SystemReader; @@ -42,10 +42,12 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestMethodOrder; /** + * Tests for running git commands. */ @TestMethodOrder(MethodName.class) -public class GitPgmCommandTest extends GitTestSupport { - public GitPgmCommandTest() { +class GitPgmCommandTest extends GitTestSupport { + + GitPgmCommandTest() { super(); } @@ -54,35 +56,13 @@ public class GitPgmCommandTest extends GitTestSupport { Path serverDir = getTempTargetRelativeFile(getClass().getSimpleName()); SystemReader defaultSystemReader = mockGitConfig(serverDir.getParent()); try (SshServer sshd = setupTestServer()) { - sshd.setSubsystemFactories(Collections.singletonList(new SftpSubsystemFactory())); sshd.setCommandFactory(new GitPgmCommandFactory(GitLocationResolver.constantPath(serverDir))); sshd.start(); int port = sshd.getPort(); try { CommonTestSupportUtils.deleteRecursive(serverDir); - - try (SshClient client = setupTestClient()) { - client.start(); - - try (ClientSession session = client.connect(getCurrentTestName(), SshdSocketAddress.LOCALHOST_IPV4, port) - .verify(CONNECT_TIMEOUT).getSession()) { - session.addPasswordIdentity(getCurrentTestName()); - session.auth().verify(AUTH_TIMEOUT); - - Path repo = serverDir.resolve(getCurrentTestName()); - Git.init().setDirectory(repo.toFile()).call(); - Git git = Git.open(repo.toFile()); - git.commit().setMessage("First Commit").setCommitter(getCurrentTestName(), "[email protected]").call(); - - Path readmeFile = Files.createFile(repo.resolve("readme.txt")); - String commandPrefix = "git --git-dir " + repo.getFileName(); - execute(session, commandPrefix + " add " + readmeFile.getFileName()); - execute(session, commandPrefix + " commit -m \"readme\""); - } finally { - client.stop(); - } - } + runGitCommands(getCurrentTestName(), port, serverDir); } finally { sshd.stop(); } @@ -91,19 +71,61 @@ public class GitPgmCommandTest extends GitTestSupport { } } - private void execute(ClientSession session, String command) throws Exception { - try (ChannelExec channel = session.createExecChannel(command)) { - channel.setOut(System.out); - channel.setErr(System.err); - channel.open().verify(OPEN_TIMEOUT); - - Collection<ClientChannelEvent> result - = channel.waitFor(EnumSet.of(ClientChannelEvent.CLOSED), TimeUnit.MINUTES.toMillis(1L)); - assertTrue(result.contains(ClientChannelEvent.CLOSED), "Command '" + command + "'not completed on time: " + result); + private void runGitCommands(String testName, int port, Path serverDir) throws Exception { + try (SshClient client = setupTestClient()) { + client.start(); + try (ClientSession session = client.connect(testName, SshdSocketAddress.LOCALHOST_IPV4, port) + .verify(CONNECT_TIMEOUT).getSession()) { + session.addPasswordIdentity(testName); + session.auth().verify(AUTH_TIMEOUT); - Integer status = channel.getExitStatus(); - if (status != null) { - assertEquals(0, status.intValue(), "Failed (" + status + ") " + command); + Path repo = serverDir.resolve(testName); + Path readmeFile; + try (Git git = Git.init().setDirectory(repo.toFile()).call()) { + readmeFile = Files.write(repo.resolve("readme.txt"), Collections.singletonList("README")); + git.add().addFilepattern("readme.txt").call(); + git.commit() // + .setMessage("First Commit") // + .setCommitter(testName, "[email protected]") // + .setAuthor(testName, "[email protected]") // + .call(); + } + String commandPrefix = "git --git-dir " + repo.getFileName(); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + ByteArrayOutputStream err = new ByteArrayOutputStream(); + RemoteException ex = assertThrows(RemoteException.class, + () -> session.executeRemoteCommand(commandPrefix + " add " + readmeFile.getFileName(), out, err, + StandardCharsets.UTF_8, AUTH_TIMEOUT)); + assertTrue(ex.getMessage().contains("Remote command failed")); + assertTrue(err.toString().contains("not allowed")); + out.reset(); + err.reset(); + session.executeRemoteCommand(commandPrefix + " log ", out, err, StandardCharsets.UTF_8, AUTH_TIMEOUT); + String log = out.toString(); + assertTrue(log.contains("First Commit")); + assertTrue(log.contains(testName)); + assertTrue(log.contains("[email protected]")); + out.reset(); + err.reset(); + session.executeRemoteCommand(commandPrefix + " archive -o archive.zip --format zip HEAD", out, err, + StandardCharsets.UTF_8, AUTH_TIMEOUT); + assertFalse(Files.exists(repo.resolve("archive.zip"))); + assertFalse(Files.exists(serverDir.resolve("archive.zip"))); + ByteArrayInputStream in = new ByteArrayInputStream(out.toByteArray()); + try (ZipInputStream zIn = new ZipInputStream(in)) { + ZipEntry entry = zIn.getNextEntry(); + assertEquals(readmeFile.getFileName().toString(), entry.getName()); + ByteArrayOutputStream content = new ByteArrayOutputStream(); + byte[] chunk = new byte[1024]; + int n = 0; + while ((n = zIn.read(chunk)) >= 0) { + content.write(chunk, 0, n); + } + zIn.closeEntry(); + assertEquals(Files.size(readmeFile), content.size()); + } + } finally { + client.stop(); } } }
