Require SubsystemFactory for SSHServer

Creating SSH servers without any subsystem is quite pointless. Require a
SubsystemFactory to be present and configure it directly in
TransportSshServer.

This improves safety and has the added benefit of reducing
ServerFactoryManagerConfigurator.

JIRA: NETCONF-1106
Change-Id: I70d37dbfa71bf13740d1c314b91777368fcf837c
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/protocol/netconf-server/src/main/java/org/opendaylight/netconf/server/BaseTransportChannelListener.java b/protocol/netconf-server/src/main/java/org/opendaylight/netconf/server/BaseTransportChannelListener.java
index c191cf82d29306bed42d6e4f93ca2ff73ad71afb..8a695b697944d9f479ed08013ab6b734952b51c8 100644 (file)
--- a/protocol/netconf-server/src/main/java/org/opendaylight/netconf/server/BaseTransportChannelListener.java
+++ b/protocol/netconf-server/src/main/java/org/opendaylight/netconf/server/BaseTransportChannelListener.java
@@ -12,7 +12,7 @@ import org.opendaylight.netconf.transport.api.TransportChannelListener;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-class BaseTransportChannelListener implements TransportChannelListener {
+public class BaseTransportChannelListener implements TransportChannelListener {
     private static final Logger LOG = LoggerFactory.getLogger(BaseTransportChannelListener.class);
 
     @Override

diff --git a/protocol/netconf-server/src/main/java/org/opendaylight/netconf/server/NetconfServerFactoryImpl.java b/protocol/netconf-server/src/main/java/org/opendaylight/netconf/server/NetconfServerFactoryImpl.java
index 0b18bfe42f96f4f06888f3e4f6b82cab8d805cc4..c2b5aae35a08b87251b9da576c59bc9cc3ff3a38 100644 (file)
--- a/protocol/netconf-server/src/main/java/org/opendaylight/netconf/server/NetconfServerFactoryImpl.java
+++ b/protocol/netconf-server/src/main/java/org/opendaylight/netconf/server/NetconfServerFactoryImpl.java
@@ -10,9 +10,7 @@ package org.opendaylight.netconf.server;
 import static java.util.Objects.requireNonNull;
 
 import com.google.common.util.concurrent.ListenableFuture;
-import java.util.List;
 import org.opendaylight.netconf.server.api.NetconfServerFactory;
-import org.opendaylight.netconf.shaded.sshd.server.SshServer;
 import org.opendaylight.netconf.transport.api.TransportChannelListener;
 import org.opendaylight.netconf.transport.api.UnsupportedConfigurationException;
 import org.opendaylight.netconf.transport.ssh.SSHServer;
@@ -44,13 +42,7 @@ public final class NetconfServerFactoryImpl implements NetconfServerFactory {
     public ListenableFuture<SSHServer> createSshServer(final TcpServerGrouping tcpParams,
             final SshServerGrouping sshParams, final ServerFactoryManagerConfigurator configurator)
                 throws UnsupportedConfigurationException {
-        return factory.listenServer(EMPTY_LISTENER, tcpParams, sshParams, factoryManager -> {
-            if (configurator != null) {
-                configurator.configureServerFactoryManager(factoryManager);
-            }
-            if (factoryManager instanceof SshServer server) {
-                server.setSubsystemFactories(List.of(new NetconfSubsystemFactory(channelInitializer)));
-            }
-        });
+        return factory.listenServer(EMPTY_LISTENER, new NetconfSubsystemFactory(channelInitializer), tcpParams,
+            sshParams, configurator);
     }
 }

diff --git a/transport/transport-ssh/src/main/java/org/opendaylight/netconf/transport/ssh/SSHServer.java b/transport/transport-ssh/src/main/java/org/opendaylight/netconf/transport/ssh/SSHServer.java
index 79dda056c9a110208f358b36f34db7c477752063..3dc4361ac5691d24e1deb579134838def298bc18 100644 (file)
--- a/transport/transport-ssh/src/main/java/org/opendaylight/netconf/transport/ssh/SSHServer.java
+++ b/transport/transport-ssh/src/main/java/org/opendaylight/netconf/transport/ssh/SSHServer.java
@@ -19,6 +19,7 @@ import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.netconf.shaded.sshd.common.io.IoHandler;
 import org.opendaylight.netconf.shaded.sshd.server.ServerFactoryManager;
 import org.opendaylight.netconf.shaded.sshd.server.session.SessionFactory;
+import org.opendaylight.netconf.shaded.sshd.server.subsystem.SubsystemFactory;
 import org.opendaylight.netconf.transport.api.TransportChannelListener;
 import org.opendaylight.netconf.transport.api.TransportStack;
 import org.opendaylight.netconf.transport.api.UnsupportedConfigurationException;
@@ -46,9 +47,9 @@ public final class SSHServer extends SSHTransportStack {
     }
 
     static SSHServer of(final EventLoopGroup group, final TransportChannelListener listener,
-            final SshServerGrouping serverParams, final ServerFactoryManagerConfigurator configurator)
-                throws UnsupportedConfigurationException {
-        return new SSHServer(listener, new TransportSshServer.Builder(group)
+            final SubsystemFactory subsystemFactory, final SshServerGrouping serverParams,
+            final ServerFactoryManagerConfigurator configurator) throws UnsupportedConfigurationException {
+        return new SSHServer(listener, new TransportSshServer.Builder(group, subsystemFactory)
             .serverParams(serverParams)
             .configurator(configurator)
             .buildChecked());

diff --git a/transport/transport-ssh/src/main/java/org/opendaylight/netconf/transport/ssh/SSHTransportStackFactory.java b/transport/transport-ssh/src/main/java/org/opendaylight/netconf/transport/ssh/SSHTransportStackFactory.java
index 3bbe710f74c8b98e6b4d95a766456d7458f189b4..d6a24cb3d1119816534dd2e7cec03bc9239820d0 100644 (file)
--- a/transport/transport-ssh/src/main/java/org/opendaylight/netconf/transport/ssh/SSHTransportStackFactory.java
+++ b/transport/transport-ssh/src/main/java/org/opendaylight/netconf/transport/ssh/SSHTransportStackFactory.java
@@ -15,6 +15,7 @@ import io.netty.bootstrap.Bootstrap;
 import io.netty.bootstrap.ServerBootstrap;
 import io.netty.channel.EventLoopGroup;
 import org.eclipse.jdt.annotation.NonNull;
+import org.opendaylight.netconf.shaded.sshd.server.subsystem.SubsystemFactory;
 import org.opendaylight.netconf.transport.api.TransportChannelListener;
 import org.opendaylight.netconf.transport.api.UnsupportedConfigurationException;
 import org.opendaylight.netconf.transport.tcp.NettyTransportSupport;
@@ -59,21 +60,23 @@ public final class SSHTransportStackFactory implements AutoCloseable {
     }
 
     public @NonNull ListenableFuture<SSHServer> connectServer(final TransportChannelListener listener,
-            final TcpClientGrouping connectParams, final SshServerGrouping serverParams)
-                throws UnsupportedConfigurationException {
-        return SSHServer.of(group, listener, requireNonNull(serverParams), null).connect(newBootstrap(), connectParams);
+            final SubsystemFactory subsystemFactory, final TcpClientGrouping connectParams,
+            final SshServerGrouping serverParams) throws UnsupportedConfigurationException {
+        return SSHServer.of(group, listener, subsystemFactory, requireNonNull(serverParams), null)
+            .connect(newBootstrap(), connectParams);
     }
 
     public @NonNull ListenableFuture<SSHServer> listenServer(final TransportChannelListener listener,
-            final TcpServerGrouping connectParams, final SshServerGrouping serverParams)
-                throws UnsupportedConfigurationException {
-        return listenServer(listener, connectParams, requireNonNull(serverParams), null);
+            final SubsystemFactory subsystemFactory, final TcpServerGrouping connectParams,
+            final SshServerGrouping serverParams) throws UnsupportedConfigurationException {
+        return listenServer(listener, subsystemFactory, connectParams, requireNonNull(serverParams), null);
     }
 
     /**
      * Builds and starts SSH Server.
      *
      * @param listener server channel listener, required
+     * @param subsystemFactory A {@link SubsystemFactory} for the hosted subsystem
      * @param listenParams TCP transport configuration, required
      * @param serverParams SSH overlay configuration, optional if configurator is defined, required otherwise
      * @param configurator server factory manager configurator, optional if serverParams is defined, required otherwise
@@ -83,11 +86,13 @@ public final class SSHTransportStackFactory implements AutoCloseable {
      * @throws IllegalArgumentException if both configurator and serverParams are null
      */
     public @NonNull ListenableFuture<SSHServer> listenServer(final TransportChannelListener listener,
-            final TcpServerGrouping listenParams, final SshServerGrouping serverParams,
-            final ServerFactoryManagerConfigurator configurator) throws UnsupportedConfigurationException {
+            final SubsystemFactory subsystemFactory, final TcpServerGrouping listenParams,
+            final SshServerGrouping serverParams, final ServerFactoryManagerConfigurator configurator)
+                    throws UnsupportedConfigurationException {
         checkArgument(serverParams != null || configurator != null,
             "Neither server parameters nor factory configurator is defined");
-        return SSHServer.of(group, listener, serverParams, configurator).listen(newServerBootstrap(), listenParams);
+        return SSHServer.of(group, listener, subsystemFactory, serverParams, configurator)
+            .listen(newServerBootstrap(), listenParams);
     }
 
     /**

diff --git a/transport/transport-ssh/src/main/java/org/opendaylight/netconf/transport/ssh/TransportSshServer.java b/transport/transport-ssh/src/main/java/org/opendaylight/netconf/transport/ssh/TransportSshServer.java
index bfe685114c394b9f137d896c689f18fc22dd0aec..20378fd6105527182127ee6ed7712408a1504486 100644 (file)
--- a/transport/transport-ssh/src/main/java/org/opendaylight/netconf/transport/ssh/TransportSshServer.java
+++ b/transport/transport-ssh/src/main/java/org/opendaylight/netconf/transport/ssh/TransportSshServer.java
@@ -22,6 +22,7 @@ import org.opendaylight.netconf.shaded.sshd.server.auth.UserAuthFactory;
 import org.opendaylight.netconf.shaded.sshd.server.auth.hostbased.UserAuthHostBasedFactory;
 import org.opendaylight.netconf.shaded.sshd.server.auth.password.UserAuthPasswordFactory;
 import org.opendaylight.netconf.shaded.sshd.server.auth.pubkey.UserAuthPublicKeyFactory;
+import org.opendaylight.netconf.shaded.sshd.server.subsystem.SubsystemFactory;
 import org.opendaylight.netconf.transport.api.UnsupportedConfigurationException;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.ssh.server.rev230417.SshServerGrouping;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.ssh.server.rev230417.ssh.server.grouping.ClientAuthentication;
@@ -67,14 +68,16 @@ final class TransportSshServer extends SshServer {
      */
     static final class Builder extends ServerBuilder {
         private final EventLoopGroup group;
+        private final SubsystemFactory subsystemFactory;
 
         private ServerFactoryManagerConfigurator configurator;
         private ClientAuthentication clientAuthentication;
         private ServerIdentity serverIdentity;
         private Keepalives keepAlives;
 
-        Builder(final EventLoopGroup group) {
+        Builder(final EventLoopGroup group, final SubsystemFactory subsystemFactory) {
             this.group = requireNonNull(group);
+            this.subsystemFactory = requireNonNull(subsystemFactory);
         }
 
         Builder serverParams(final SshServerGrouping serverParams) throws UnsupportedConfigurationException {
@@ -136,8 +139,11 @@ final class TransportSshServer extends SshServer {
             if (configurator != null) {
                 configurator.configureServerFactoryManager(ret);
             }
+
+            ret.setSubsystemFactories(List.of(subsystemFactory));
             ret.setScheduledExecutorService(group);
 
+
             try {
                 ret.checkConfig();
             } catch (IllegalArgumentException e) {

diff --git a/transport/transport-ssh/src/test/java/org/opendaylight/netconf/transport/ssh/SshClientServerTest.java b/transport/transport-ssh/src/test/java/org/opendaylight/netconf/transport/ssh/SshClientServerTest.java
index c447e5f0fd0aec19f77c896a138a396d5c7312cd..e68b7444c353374709615ccaa0dfa11e9d1b9433 100644 (file)
--- a/transport/transport-ssh/src/test/java/org/opendaylight/netconf/transport/ssh/SshClientServerTest.java
+++ b/transport/transport-ssh/src/test/java/org/opendaylight/netconf/transport/ssh/SshClientServerTest.java
@@ -55,6 +55,7 @@ import org.opendaylight.netconf.shaded.sshd.common.session.Session;
 import org.opendaylight.netconf.shaded.sshd.server.auth.password.UserAuthPasswordFactory;
 import org.opendaylight.netconf.shaded.sshd.server.keyprovider.SimpleGeneratorHostKeyProvider;
 import org.opendaylight.netconf.shaded.sshd.server.session.ServerSession;
+import org.opendaylight.netconf.shaded.sshd.server.subsystem.SubsystemFactory;
 import org.opendaylight.netconf.transport.api.TransportChannel;
 import org.opendaylight.netconf.transport.api.TransportChannelListener;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.Host;
@@ -93,6 +94,8 @@ public class SshClientServerTest {
     private SshServerGrouping sshServerConfig;
     @Mock
     private TransportChannelListener serverListener;
+    @Mock
+    private SubsystemFactory subsystemFactory;
 
     @Captor
     ArgumentCaptor<TransportChannel> clientTransportChannelCaptor;
@@ -217,7 +220,7 @@ public class SshClientServerTest {
 
     private void integrationTest() throws Exception {
         // start server
-        final var server = FACTORY.listenServer(serverListener, tcpServerConfig, sshServerConfig)
+        final var server = FACTORY.listenServer(serverListener, subsystemFactory, tcpServerConfig, sshServerConfig)
             .get(2, TimeUnit.SECONDS);
         try {
             // connect with client
@@ -254,7 +257,7 @@ public class SshClientServerTest {
         // Accept all keys
         when(sshClientConfig.getServerAuthentication()).thenReturn(null);
 
-        final var server = FACTORY.listenServer(serverListener, tcpServerConfig, null,
+        final var server = FACTORY.listenServer(serverListener, subsystemFactory, tcpServerConfig, null,
             factoryManager -> {
                 // authenticate user by credentials and generate host key
                 factoryManager.setUserAuthFactories(List.of(new UserAuthPasswordFactory()));