You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by lg...@apache.org on 2020/08/21 08:23:42 UTC

[mina-sshd] branch master updated: [SSHD-1057] Added capability to select a ShellFactory based on the current session + use it for WinSCP

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 43ab52f  [SSHD-1057] Added capability to select a ShellFactory based on the current session + use it for WinSCP
43ab52f is described below

commit 43ab52f7047b4a58f34b8a606d2fd5ef75688fcf
Author: Lyor Goldstein <lg...@apache.org>
AuthorDate: Fri Aug 21 11:22:34 2020 +0300

    [SSHD-1057] Added capability to select a ShellFactory based on the current session + use it for WinSCP
---
 CHANGES.md                                         |  1 +
 docs/server-setup.md                               |  5 ++
 .../sshd/cli/server/SshServerCliSupport.java       | 35 +++++++--
 .../org/apache/sshd/cli/server/SshServerMain.java  |  9 +--
 .../sshd/server/shell/AggregateShellFactory.java   | 85 ++++++++++++++++++++++
 .../sshd/server/shell/InvertedShellWrapper.java    |  1 -
 .../sshd/server/shell/ShellFactorySelector.java    | 66 +++++++++++++++++
 .../apache/sshd/scp/server/ScpCommandFactory.java  | 57 +++++++++++++--
 8 files changed, 238 insertions(+), 21 deletions(-)

diff --git a/CHANGES.md b/CHANGES.md
index 1a8973e..095744a 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -43,4 +43,5 @@ or `-key-file` command line option.
 * [SSHD-1048](https://issues.apache.org/jira/browse/SSHD-1048) Wrap instead of rethrow IOException in Future.
 * [SSHD-1050](https://issues.apache.org/jira/browse/SSHD-1050) Fixed race condition in AuthFuture if exception caught before authentication started.
 * [SSHD-1056](https://issues.apache.org/jira/browse/SSHD-1005) Added support for SCP remote-to-remote directory transfer - including '-3' option of SCP command CLI.
+* [SSHD-1058](https://issues.apache.org/jira/browse/SSHD-1057) Added capability to select a ShellFactory based on the current session + use it for "WinSCP"
 * [SSHD-1058](https://issues.apache.org/jira/browse/SSHD-1058) Improve exception logging strategy.
diff --git a/docs/server-setup.md b/docs/server-setup.md
index 3d46fbe..16e374c 100644
--- a/docs/server-setup.md
+++ b/docs/server-setup.md
@@ -54,6 +54,11 @@ so it's mostly useful to launch the OS native shell. E.g.,
 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.
 
+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.,
+[the way we do for "WinSCP"](https://issues.apache.org/jira/browse/SSHD-1009) based on the peer client version string.
+
 * `CommandFactory` - The `CommandFactory` provides the ability to run a **single** direct command at a time instead
 of an interactive session (it also uses a **different** channel type than shells). It can be used **in addition** to the `ShellFactory`.
 
diff --git a/sshd-cli/src/main/java/org/apache/sshd/cli/server/SshServerCliSupport.java b/sshd-cli/src/main/java/org/apache/sshd/cli/server/SshServerCliSupport.java
index d731231..6b3340b 100644
--- a/sshd-cli/src/main/java/org/apache/sshd/cli/server/SshServerCliSupport.java
+++ b/sshd-cli/src/main/java/org/apache/sshd/cli/server/SshServerCliSupport.java
@@ -62,6 +62,7 @@ import org.apache.sshd.server.forward.ForwardingFilter;
 import org.apache.sshd.server.keyprovider.AbstractGeneratorHostKeyProvider;
 import org.apache.sshd.server.keyprovider.SimpleGeneratorHostKeyProvider;
 import org.apache.sshd.server.shell.InteractiveProcessShellFactory;
+import org.apache.sshd.server.shell.ProcessShellCommandFactory;
 import org.apache.sshd.server.shell.ShellFactory;
 import org.apache.sshd.server.subsystem.SubsystemFactory;
 import org.apache.sshd.sftp.common.SftpConstants;
@@ -274,20 +275,30 @@ public abstract class SshServerCliSupport extends CliSupport {
             return null;
         }
 
+        // Only SCP
         if (ScpCommandFactory.SCP_FACTORY_NAME.equalsIgnoreCase(factory)) {
-            ScpCommandFactory shell = new ScpCommandFactory();
-            if (isEnabledVerbosityLogging(level)) {
-                shell.addEventListener(new ScpCommandTransferEventListener(stdout, stderr));
-            }
+            return createScpCommandFactory(level, stdout, stderr, null);
+        }
+
+        // SCP + DEFAULT SHELL
+        if (("+" + ScpCommandFactory.SCP_FACTORY_NAME).equalsIgnoreCase(factory)) {
+            return createScpCommandFactory(level, stdout, stderr, DEFAULT_SHELL_FACTORY);
+        }
 
-            return shell;
+        boolean useScp = false;
+        // SCP + CUSTOM SHELL
+        if (factory.startsWith(ScpCommandFactory.SCP_FACTORY_NAME + "+")) {
+            factory = factory.substring(ScpCommandFactory.SCP_FACTORY_NAME.length() + 1);
+            ValidateUtils.checkNotNullAndNotEmpty(factory, "No extra custom shell factory class specified");
+            useScp = true;
         }
 
         ClassLoader cl = ThreadUtils.resolveDefaultClassLoader(ShellFactory.class);
         try {
             Class<?> clazz = cl.loadClass(factory);
             Object instance = clazz.newInstance();
-            return ShellFactory.class.cast(instance);
+            ShellFactory shellFactory = ShellFactory.class.cast(instance);
+            return useScp ? createScpCommandFactory(level, stdout, stderr, shellFactory) : shellFactory;
         } catch (Exception e) {
             stderr.append("ERROR: Failed (").append(e.getClass().getSimpleName()).append(')')
                     .append(" to instantiate shell factory=").append(factory)
@@ -296,4 +307,16 @@ public abstract class SshServerCliSupport extends CliSupport {
             throw e;
         }
     }
+
+    public static ScpCommandFactory createScpCommandFactory(
+            Level level, Appendable stdout, Appendable stderr, ShellFactory delegateShellFactory) {
+        ScpCommandFactory.Builder scp = new ScpCommandFactory.Builder()
+                .withDelegate(ProcessShellCommandFactory.INSTANCE)
+                .withDelegateShellFactory(delegateShellFactory);
+        if (isEnabledVerbosityLogging(level)) {
+            scp.addEventListener(new ScpCommandTransferEventListener(stdout, stderr));
+        }
+
+        return scp.build();
+    }
 }
diff --git a/sshd-cli/src/main/java/org/apache/sshd/cli/server/SshServerMain.java b/sshd-cli/src/main/java/org/apache/sshd/cli/server/SshServerMain.java
index 0e437b5..27e48b6 100644
--- a/sshd-cli/src/main/java/org/apache/sshd/cli/server/SshServerMain.java
+++ b/sshd-cli/src/main/java/org/apache/sshd/cli/server/SshServerMain.java
@@ -29,7 +29,6 @@ import java.util.TreeMap;
 import java.util.logging.Level;
 import java.util.stream.Collectors;
 
-import org.apache.sshd.cli.server.helper.ScpCommandTransferEventListener;
 import org.apache.sshd.common.NamedResource;
 import org.apache.sshd.common.PropertyResolver;
 import org.apache.sshd.common.PropertyResolverUtils;
@@ -45,7 +44,6 @@ import org.apache.sshd.server.command.CommandFactory;
 import org.apache.sshd.server.config.SshServerConfigFileReader;
 import org.apache.sshd.server.config.keys.ServerIdentity;
 import org.apache.sshd.server.keyprovider.AbstractGeneratorHostKeyProvider;
-import org.apache.sshd.server.shell.ProcessShellCommandFactory;
 import org.apache.sshd.server.shell.ShellFactory;
 import org.apache.sshd.server.subsystem.SubsystemFactory;
 
@@ -222,12 +220,7 @@ public class SshServerMain extends SshServerCliSupport {
         if (shellFactory instanceof ScpCommandFactory) {
             scpFactory = (ScpCommandFactory) shellFactory;
         } else {
-            ScpCommandFactory.Builder builder = new ScpCommandFactory.Builder()
-                    .withDelegate(ProcessShellCommandFactory.INSTANCE);
-            if (isEnabledVerbosityLogging(level)) {
-                builder = builder.addEventListener(new ScpCommandTransferEventListener(stdout, stderr));
-            }
-            scpFactory = builder.build();
+            scpFactory = createScpCommandFactory(level, stdout, stderr, null);
         }
         sshd.setCommandFactory(scpFactory);
         return scpFactory;
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/shell/AggregateShellFactory.java b/sshd-core/src/main/java/org/apache/sshd/server/shell/AggregateShellFactory.java
new file mode 100644
index 0000000..0b839c4
--- /dev/null
+++ b/sshd-core/src/main/java/org/apache/sshd/server/shell/AggregateShellFactory.java
@@ -0,0 +1,85 @@
+/*
+ * 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.server.shell;
+
+import java.io.IOException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Objects;
+
+import org.apache.sshd.common.util.logging.AbstractLoggingBean;
+import org.apache.sshd.server.channel.ChannelSession;
+import org.apache.sshd.server.command.Command;
+
+/**
+ * Provides different shell(s) based on some criteria of the provided {@link ChannelSession}
+ *
+ * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ */
+public class AggregateShellFactory extends AbstractLoggingBean implements ShellFactory, ShellFactorySelector {
+    protected final ShellFactory defaultFactory;
+    protected final Collection<? extends ShellFactorySelector> selectors;
+
+    /**
+     * @param selectors Selector {@link ShellFactorySelector}-s being consulted whether they wish to provide a
+     *                  {@link ShellFactory} for the provided {@link ChannelSession} argument. If a selector returns
+     *                  {@code null} then the next in line is consulted. If no match found then the default
+     *                  {@link InteractiveProcessShellFactory} is used
+     */
+    public AggregateShellFactory(
+                                 Collection<? extends ShellFactorySelector> selectors) {
+        this(selectors, InteractiveProcessShellFactory.INSTANCE);
+    }
+
+    /**
+     * @param selectors      Selector {@link ShellFactorySelector}-s being consulted whether they wish to provide a
+     *                       {@link ShellFactory} for the provided {@link ChannelSession} argument. If a selector
+     *                       returns {@code null} then the next in line is consulted.
+     * @param defaultFactory The (mandatory) default {@link ShellFactory} to use if no selector matched
+     */
+    public AggregateShellFactory(
+                                 Collection<? extends ShellFactorySelector> selectors, ShellFactory defaultFactory) {
+        this.selectors = (selectors == null) ? Collections.emptyList() : selectors;
+        this.defaultFactory = Objects.requireNonNull(defaultFactory, "No default factory provided");
+    }
+
+    @Override
+    public Command createShell(ChannelSession channel) throws IOException {
+        ShellFactory factory = selectShellFactory(channel);
+        if (factory == null) {
+            if (log.isDebugEnabled()) {
+                log.debug("createShell({}) using default factory={}", channel, defaultFactory);
+            }
+
+            factory = defaultFactory;
+        } else {
+            if (log.isDebugEnabled()) {
+                log.debug("createShell({}) using selected factory={}", channel, factory);
+            }
+        }
+
+        return factory.createShell(channel);
+    }
+
+    @Override
+    public ShellFactory selectShellFactory(ChannelSession channel) throws IOException {
+        return ShellFactorySelector.selectShellFactory(selectors, channel);
+    }
+}
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/shell/InvertedShellWrapper.java b/sshd-core/src/main/java/org/apache/sshd/server/shell/InvertedShellWrapper.java
index bfcafa7..201f8c0 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/shell/InvertedShellWrapper.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/shell/InvertedShellWrapper.java
@@ -142,7 +142,6 @@ public class InvertedShellWrapper extends AbstractLoggingBean implements Command
 
     @Override
     public synchronized void destroy(ChannelSession channel) throws Exception {
-        boolean debugEnabled = log.isDebugEnabled();
         Throwable err = null;
         try {
             shell.destroy(channel);
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/shell/ShellFactorySelector.java b/sshd-core/src/main/java/org/apache/sshd/server/shell/ShellFactorySelector.java
new file mode 100644
index 0000000..b92d658
--- /dev/null
+++ b/sshd-core/src/main/java/org/apache/sshd/server/shell/ShellFactorySelector.java
@@ -0,0 +1,66 @@
+/*
+ * 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.server.shell;
+
+import java.io.IOException;
+import java.util.Collection;
+
+import org.apache.sshd.common.util.GenericUtils;
+import org.apache.sshd.server.channel.ChannelSession;
+
+/**
+ * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ */
+@FunctionalInterface
+public interface ShellFactorySelector {
+    /**
+     *
+     * @param  channelSession The {@link ChannelSession}
+     * @return                The {@link ShellFactory} to use for the channel - {@code null} if none
+     * @throws IOException    If failed the selection
+     */
+    ShellFactory selectShellFactory(ChannelSession channelSession) throws IOException;
+
+    /**
+     * Consults each selector whether it wants to provide a factory for the {@link ChannelSession}
+     *
+     * @param  selectors   The {@link ShellFactorySelector}-s to consult - ignored if {@code null}/empty
+     * @param  channel     The {@link ChannelSession} instance
+     * @return             The selected {@link ShellFactory} - {@code null} if no selector matched (in which case the
+     *                     default factory is used)
+     * @throws IOException if any selector threw it
+     */
+    static ShellFactory selectShellFactory(
+            Collection<? extends ShellFactorySelector> selectors, ChannelSession channel)
+            throws IOException {
+        if (GenericUtils.isEmpty(selectors)) {
+            return null;
+        }
+
+        for (ShellFactorySelector sel : selectors) {
+            ShellFactory factory = sel.selectShellFactory(channel);
+            if (factory != null) {
+                return factory;
+            }
+        }
+
+        return null;
+    }
+}
diff --git a/sshd-scp/src/main/java/org/apache/sshd/scp/server/ScpCommandFactory.java b/sshd-scp/src/main/java/org/apache/sshd/scp/server/ScpCommandFactory.java
index 35c6735..6e9b348 100644
--- a/sshd-scp/src/main/java/org/apache/sshd/scp/server/ScpCommandFactory.java
+++ b/sshd-scp/src/main/java/org/apache/sshd/scp/server/ScpCommandFactory.java
@@ -23,6 +23,7 @@ import java.util.Collection;
 import java.util.concurrent.CopyOnWriteArraySet;
 import java.util.function.Supplier;
 
+import org.apache.sshd.common.session.SessionContext;
 import org.apache.sshd.common.util.EventListenerUtils;
 import org.apache.sshd.common.util.GenericUtils;
 import org.apache.sshd.common.util.ObjectBuilder;
@@ -36,7 +37,9 @@ import org.apache.sshd.server.channel.ChannelSession;
 import org.apache.sshd.server.command.AbstractDelegatingCommandFactory;
 import org.apache.sshd.server.command.Command;
 import org.apache.sshd.server.command.CommandFactory;
+import org.apache.sshd.server.shell.InteractiveProcessShellFactory;
 import org.apache.sshd.server.shell.ShellFactory;
+import org.apache.sshd.server.shell.ShellFactorySelector;
 
 /**
  * This <code>CommandFactory</code> can be used as a standalone command factory or can be used to augment another
@@ -48,7 +51,7 @@ import org.apache.sshd.server.shell.ShellFactory;
  */
 public class ScpCommandFactory
         extends AbstractDelegatingCommandFactory
-        implements ManagedExecutorServiceSupplier, ScpFileOpenerHolder, Cloneable, ShellFactory {
+        implements ManagedExecutorServiceSupplier, ScpFileOpenerHolder, Cloneable, ShellFactory, ShellFactorySelector {
 
     public static final String SCP_FACTORY_NAME = "scp";
 
@@ -98,6 +101,11 @@ public class ScpCommandFactory
             return this;
         }
 
+        public Builder withDelegateShellFactory(ShellFactory shellFactory) {
+            factory.setDelegateShellFactory(shellFactory);
+            return this;
+        }
+
         @Override
         public ScpCommandFactory build() {
             return factory.clone();
@@ -106,6 +114,7 @@ public class ScpCommandFactory
 
     private Supplier<? extends CloseableExecutorService> executorsProvider;
     private ScpFileOpener fileOpener;
+    private ShellFactory delegateShellFactory = InteractiveProcessShellFactory.INSTANCE;
     private int sendBufferSize = ScpHelper.DEFAULT_SEND_BUFFER_SIZE;
     private int receiveBufferSize = ScpHelper.DEFAULT_RECEIVE_BUFFER_SIZE;
     private Collection<ScpTransferEventListener> listeners = new CopyOnWriteArraySet<>();
@@ -219,13 +228,49 @@ public class ScpCommandFactory
                 getScpFileOpener(), listenerProxy);
     }
 
+    /**
+     * @return The delegate {@link ShellFactory} to use if {@link #selectShellFactory(ChannelSession)} decides not to
+     *         use itself as the {@link ShellFactory} - default={@link InteractiveProcessShellFactory}.
+     * @see    #setDelegateShellFactory(ShellFactory)
+     */
+    public ShellFactory getDelegateShellFactory() {
+        return delegateShellFactory;
+    }
+
+    /**
+     * @param delegateShellFactory The {@link ShellFactory} to use if {@link #selectShellFactory(ChannelSession)}
+     *                             decides not to use itself as the {@link ShellFactory}. If {@code null} then it will
+     *                             always decide to use itself regardless of the {@link ChannelSession}
+     * @see                        #selectShellFactory(ChannelSession)
+     */
+    public void setDelegateShellFactory(ShellFactory delegateShellFactory) {
+        this.delegateShellFactory = delegateShellFactory;
+    }
+
+    @Override
+    public ShellFactory selectShellFactory(ChannelSession channelSession) throws IOException {
+        SessionContext session = channelSession.getSessionContext();
+        String clientVersion = session.getClientVersion();
+        // SSHD-1009
+        if (clientVersion.contains("WinSCP")) {
+            return this;
+        }
+
+        return delegateShellFactory;
+    }
+
     @Override
     public Command createShell(ChannelSession channel) throws IOException {
-        return new ScpShell(
-                channel,
-                resolveExecutorService(),
-                getSendBufferSize(), getReceiveBufferSize(),
-                getScpFileOpener(), listenerProxy);
+        ShellFactory factory = selectShellFactory(channel);
+        if ((factory == this) || (factory == null)) {
+            return new ScpShell(
+                    channel,
+                    resolveExecutorService(),
+                    getSendBufferSize(), getReceiveBufferSize(),
+                    getScpFileOpener(), listenerProxy);
+        } else {
+            return factory.createShell(channel);
+        }
     }
 
     protected CloseableExecutorService resolveExecutorService(String command) {