X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=blobdiff_plain;f=netconf%2Fnetconf-netty-util%2Fsrc%2Fmain%2Fjava%2Forg%2Fopendaylight%2Fnetconf%2Fnettyutil%2Fhandler%2Fssh%2Fclient%2FAsyncSshHandler.java;h=f6956d7c5d4e3dc39940bb96d035839c5360254b;hb=1f754eb1b78fb0dda5dba1a1d17f4b15671d95e8;hp=149ccf16f9910dfbf38a866f5c0d89fbda50c7b6;hpb=1e174be6002940e17aee31d15f8914273d30f25c;p=netconf.git diff --git a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/ssh/client/AsyncSshHandler.java b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/ssh/client/AsyncSshHandler.java index 149ccf16f9..f6956d7c5d 100644 --- a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/ssh/client/AsyncSshHandler.java +++ b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/ssh/client/AsyncSshHandler.java @@ -5,44 +5,65 @@ * terms of the Eclipse Public License v1.0 which accompanies this distribution, * and is available at http://www.eclipse.org/legal/epl-v10.html */ - package org.opendaylight.netconf.nettyutil.handler.ssh.client; -import com.google.common.base.Preconditions; +import static com.google.common.base.Verify.verify; +import static java.util.Objects.requireNonNull; + import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelOutboundHandlerAdapter; import io.netty.channel.ChannelPromise; import io.netty.util.concurrent.Future; -import io.netty.util.concurrent.GenericFutureListener; +import io.netty.util.concurrent.FutureListener; import java.io.IOException; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.VarHandle; import java.net.SocketAddress; -import org.apache.sshd.client.SshClient; -import org.apache.sshd.client.channel.ClientChannel; -import org.apache.sshd.client.future.AuthFuture; -import org.apache.sshd.client.future.ConnectFuture; -import org.apache.sshd.client.session.ClientSession; +import java.time.Duration; +import java.util.concurrent.TimeUnit; +import org.checkerframework.checker.lock.qual.GuardedBy; +import org.checkerframework.checker.lock.qual.Holding; +import org.eclipse.jdt.annotation.Nullable; import org.opendaylight.netconf.nettyutil.handler.ssh.authentication.AuthenticationHandler; +import org.opendaylight.netconf.shaded.sshd.client.channel.ChannelSubsystem; +import org.opendaylight.netconf.shaded.sshd.client.channel.ClientChannel; +import org.opendaylight.netconf.shaded.sshd.client.future.AuthFuture; +import org.opendaylight.netconf.shaded.sshd.client.future.ConnectFuture; +import org.opendaylight.netconf.shaded.sshd.client.future.OpenFuture; +import org.opendaylight.netconf.shaded.sshd.client.session.ClientSession; +import org.opendaylight.netconf.shaded.sshd.core.CoreModuleProperties; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** * Netty SSH handler class. Acts as interface between Netty and SSH library. */ -public class AsyncSshHandler extends ChannelOutboundHandlerAdapter { +public final class AsyncSshHandler extends ChannelOutboundHandlerAdapter { private static final Logger LOG = LoggerFactory.getLogger(AsyncSshHandler.class); + private static final VarHandle DISCONNECTED; + + static { + try { + DISCONNECTED = MethodHandles.lookup().findVarHandle(AsyncSshHandler.class, "disconnected", boolean.class); + } catch (NoSuchFieldException | IllegalAccessException e) { + throw new ExceptionInInitializerError(e); + } + } public static final String SUBSYSTEM = "netconf"; public static final int SSH_DEFAULT_NIO_WORKERS = 8; - // Disable default timeouts from mina sshd - private static final long DEFAULT_TIMEOUT = -1L; - public static final SshClient DEFAULT_CLIENT; + public static final NetconfSshClient DEFAULT_CLIENT; static { - final SshClient c = SshClient.setUpDefaultClient(); - c.getProperties().put(SshClient.AUTH_TIMEOUT, Long.toString(DEFAULT_TIMEOUT)); - c.getProperties().put(SshClient.IDLE_TIMEOUT, Long.toString(DEFAULT_TIMEOUT)); + final var c = new NetconfClientBuilder().build(); + // Disable default timeouts from mina sshd + final var zero = Duration.ofMillis(0); + CoreModuleProperties.AUTH_TIMEOUT.set(c, zero); + CoreModuleProperties.IDLE_TIMEOUT.set(c, zero); + CoreModuleProperties.NIO2_READ_TIMEOUT.set(c, zero); + CoreModuleProperties.TCP_NODELAY.set(c, true); // TODO make configurable, or somehow reuse netty threadpool c.setNioWorkers(SSH_DEFAULT_NIO_WORKERS); @@ -51,21 +72,32 @@ public class AsyncSshHandler extends ChannelOutboundHandlerAdapter { } private final AuthenticationHandler authenticationHandler; - private final SshClient sshClient; - private Future negotiationFuture; + private final Future negotiationFuture; + private final NetconfSshClient sshClient; + private final String name; - private AsyncSshHandlerReader sshReadAsyncListener; - private AsyncSshHandlerWriter sshWriteAsyncHandler; + // Initialized by connect() + @GuardedBy("this") + private ChannelPromise connectPromise; - private ClientChannel channel; + private AsyncSshHandlerWriter sshWriteAsyncHandler; + private ChannelSubsystem channel; private ClientSession session; - private ChannelPromise connectPromise; - private GenericFutureListener negotiationFutureListener; + private FutureListener negotiationFutureListener; + + private volatile boolean disconnected; - public AsyncSshHandler(final AuthenticationHandler authenticationHandler, final SshClient sshClient, - final Future negotiationFuture) { - this(authenticationHandler, sshClient); + private AsyncSshHandler(final AuthenticationHandler authenticationHandler, final NetconfSshClient sshClient, + final @Nullable Future negotiationFuture, final @Nullable String name) { + this.authenticationHandler = requireNonNull(authenticationHandler); + this.sshClient = requireNonNull(sshClient); this.negotiationFuture = negotiationFuture; + this.name = name != null && !name.isBlank() ? name : "UNNAMED"; + } + + public AsyncSshHandler(final AuthenticationHandler authenticationHandler, final NetconfSshClient sshClient, + final @Nullable Future negotiationFuture) { + this(authenticationHandler, sshClient, negotiationFuture, null); } /** @@ -74,10 +106,8 @@ public class AsyncSshHandler extends ChannelOutboundHandlerAdapter { * @param authenticationHandler authentication handler * @param sshClient started SshClient */ - public AsyncSshHandler(final AuthenticationHandler authenticationHandler, - final SshClient sshClient) { - this.authenticationHandler = Preconditions.checkNotNull(authenticationHandler); - this.sshClient = Preconditions.checkNotNull(sshClient); + public AsyncSshHandler(final AuthenticationHandler authenticationHandler, final NetconfSshClient sshClient) { + this(authenticationHandler, sshClient, null); } public static AsyncSshHandler createForNetconfSubsystem(final AuthenticationHandler authenticationHandler) { @@ -93,131 +123,149 @@ public class AsyncSshHandler extends ChannelOutboundHandlerAdapter { * @return {@code AsyncSshHandler} */ public static AsyncSshHandler createForNetconfSubsystem(final AuthenticationHandler authenticationHandler, - final Future negotiationFuture) { - return new AsyncSshHandler(authenticationHandler, DEFAULT_CLIENT, negotiationFuture); + final Future negotiationFuture, final @Nullable NetconfSshClient sshClient, + final @Nullable String name) { + return new AsyncSshHandler(authenticationHandler, sshClient != null ? sshClient : DEFAULT_CLIENT, + negotiationFuture, name); } - private void startSsh(final ChannelHandlerContext ctx, final SocketAddress address) throws IOException { - LOG.debug("Starting SSH to {} on channel: {}", address, ctx.channel()); - - final ConnectFuture sshConnectionFuture = sshClient.connect(authenticationHandler.getUsername(), address); - sshConnectionFuture.addListener(future -> { - if (future.isConnected()) { - handleSshSessionCreated(future, ctx); - } else { - handleSshSetupFailure(ctx, future.getException()); - } - }); + @Override + public synchronized void write(final ChannelHandlerContext ctx, final Object msg, final ChannelPromise promise) { + sshWriteAsyncHandler.write(ctx, msg, promise); } - private synchronized void handleSshSessionCreated(final ConnectFuture future, final ChannelHandlerContext ctx) { - try { - LOG.trace("SSH session created on channel: {}", ctx.channel()); - - session = future.getSession(); - final AuthFuture authenticateFuture = authenticationHandler.authenticate(session); - final ClientSession localSession = session; - authenticateFuture.addListener(future1 -> { - if (future1.isSuccess()) { - handleSshAuthenticated(localSession, ctx); - } else { - // Exception does not have to be set in the future, add simple exception in such case - final Throwable exception = future1.getException() == null - ? new IllegalStateException("Authentication failed") : future1.getException(); - handleSshSetupFailure(ctx, exception); + @Override + public synchronized void connect(final ChannelHandlerContext ctx, final SocketAddress remoteAddress, + final SocketAddress localAddress, final ChannelPromise promise) throws IOException { + LOG.debug("{}: SSH session connecting on channel {}. promise: {}", name, ctx.channel(), promise); + connectPromise = requireNonNull(promise); + + if (negotiationFuture != null) { + negotiationFutureListener = future -> { + if (future.isSuccess()) { + promise.setSuccess(); } - }); - } catch (final IOException e) { - handleSshSetupFailure(ctx, e); + }; + //complete connection promise with netconf negotiation future + negotiationFuture.addListener(negotiationFutureListener); } + + LOG.debug("{}: Starting SSH to {} on channel: {}", name, remoteAddress, ctx.channel()); + sshClient.connect(authenticationHandler.getUsername(), remoteAddress) + // FIXME: this is a blocking call, we should handle this with a concurrently-scheduled timeout. We do not + // have a Timer ready, so perhaps we should be using the event loop? + .verify(ctx.channel().config().getConnectTimeoutMillis(), TimeUnit.MILLISECONDS) + .addListener(future -> onConnectComplete(future, ctx)); } - private synchronized void handleSshAuthenticated(final ClientSession newSession, final ChannelHandlerContext ctx) { - try { - LOG.debug("SSH session authenticated on channel: {}, server version: {}", ctx.channel(), - newSession.getServerVersion()); + private synchronized void onConnectComplete(final ConnectFuture connectFuture, final ChannelHandlerContext ctx) { + final var cause = connectFuture.getException(); + if (cause != null) { + onOpenFailure(ctx, cause); + return; + } - channel = newSession.createSubsystemChannel(SUBSYSTEM); - channel.setStreaming(ClientChannel.Streaming.Async); - channel.open().addListener(future -> { - if (future.isOpened()) { - handleSshChanelOpened(ctx); - } else { - handleSshSetupFailure(ctx, future.getException()); - } - }); + final var clientSession = connectFuture.getSession(); + LOG.trace("{}: SSH session {} created on channel: {}", name, clientSession, ctx.channel()); + verify(clientSession instanceof NettyAwareClientSession, "Unexpected session %s", clientSession); + final var localSession = (NettyAwareClientSession) clientSession; + session = localSession; + final AuthFuture authFuture; + try { + authFuture = authenticationHandler.authenticate(localSession); } catch (final IOException e) { - handleSshSetupFailure(ctx, e); + onOpenFailure(ctx, e); + return; } + + authFuture.addListener(future -> onAuthComplete(future, localSession, ctx)); } - private synchronized void handleSshChanelOpened(final ChannelHandlerContext ctx) { - LOG.trace("SSH subsystem channel opened successfully on channel: {}", ctx.channel()); + private synchronized void onAuthComplete(final AuthFuture authFuture, final NettyAwareClientSession clientSession, + final ChannelHandlerContext ctx) { + final var cause = authFuture.getException(); + if (cause != null) { + onOpenFailure(ctx, new AuthenticationFailedException("Authentication failed", cause)); + return; + } + if (disconnected) { + LOG.debug("{}: Skipping SSH subsystem allocation, channel: {}", name, ctx.channel()); + return; + } + + LOG.debug("{}: SSH session authenticated on channel: {}, server version: {}", name, ctx.channel(), + clientSession.getServerVersion()); - if (negotiationFuture == null) { - connectPromise.setSuccess(); + final OpenFuture openFuture; + try { + channel = clientSession.createSubsystemChannel(SUBSYSTEM, ctx); + channel.setStreaming(ClientChannel.Streaming.Async); + openFuture = channel.open(); + } catch (final IOException e) { + onOpenFailure(ctx, e); + return; } - // TODO we should also read from error stream and at least log from that + openFuture.addListener(future -> ctx.executor().execute(() -> onOpenComplete(future, ctx))); + } - ClientChannel localChannel = channel; - sshReadAsyncListener = new AsyncSshHandlerReader(() -> AsyncSshHandler.this.disconnect(ctx, ctx.newPromise()), - ctx::fireChannelRead, localChannel.toString(), localChannel.getAsyncOut()); + // This callback has to run on the channel's executor because it runs fireChannelActive(), which needs to be + // delivered synchronously. If we were to execute on some other thread we would end up delaying the event, + // potentially creating havoc in the pipeline. + private synchronized void onOpenComplete(final OpenFuture openFuture, final ChannelHandlerContext ctx) { + final var cause = openFuture.getException(); + if (cause != null) { + onOpenFailure(ctx, cause); + return; + } + if (disconnected) { + LOG.trace("{}: Skipping activation, channel: {}", name, ctx.channel()); + return; + } - // if readAsyncListener receives immediate close, - // it will close this handler and closing this handler sets channel variable to null - if (channel != null) { - sshWriteAsyncHandler = new AsyncSshHandlerWriter(channel.getAsyncIn()); - ctx.fireChannelActive(); + LOG.trace("{}: SSH subsystem channel opened successfully on channel: {}", name, ctx.channel()); + if (negotiationFuture == null) { + connectPromise.setSuccess(); } + + sshWriteAsyncHandler = new AsyncSshHandlerWriter(channel.getAsyncIn()); + ctx.fireChannelActive(); + channel.onClose(() -> disconnect(ctx, ctx.newPromise())); } - private synchronized void handleSshSetupFailure(final ChannelHandlerContext ctx, final Throwable error) { - LOG.warn("Unable to setup SSH connection on channel: {}", ctx.channel(), error); + @Holding("this") + private void onOpenFailure(final ChannelHandlerContext ctx, final Throwable cause) { + LOG.warn("{}: Unable to setup SSH connection on channel: {}", name, ctx.channel(), cause); // If the promise is not yet done, we have failed with initial connect and set connectPromise to failure if (!connectPromise.isDone()) { - connectPromise.setFailure(error); + connectPromise.setFailure(cause); } disconnect(ctx, ctx.newPromise()); } @Override - public synchronized void write(final ChannelHandlerContext ctx, final Object msg, final ChannelPromise promise) { - sshWriteAsyncHandler.write(ctx, msg, promise); + public void close(final ChannelHandlerContext ctx, final ChannelPromise promise) { + disconnect(ctx, promise); } @Override - public synchronized void connect(final ChannelHandlerContext ctx, final SocketAddress remoteAddress, - final SocketAddress localAddress, final ChannelPromise promise) throws Exception { - LOG.debug("SSH session connecting on channel {}. promise: {} ", ctx.channel(), connectPromise); - this.connectPromise = promise; - - if (negotiationFuture != null) { - negotiationFutureListener = future -> { - if (future.isSuccess()) { - promise.setSuccess(); - } - }; - //complete connection promise with netconf negotiation future - negotiationFuture.addListener(negotiationFutureListener); + public void disconnect(final ChannelHandlerContext ctx, final ChannelPromise promise) { + if (DISCONNECTED.compareAndSet(this, false, true)) { + ctx.executor().execute(() -> safelyDisconnect(ctx, promise)); } - startSsh(ctx, remoteAddress); - } - - @Override - public void close(final ChannelHandlerContext ctx, final ChannelPromise promise) { - disconnect(ctx, promise); } + // This method has the potential to interact with the channel pipeline, for example via fireChannelInactive(). These + // callbacks need to complete during execution of this method and therefore this method needs to be executing on + // the channel's executor. @SuppressWarnings("checkstyle:IllegalCatch") - @Override - public synchronized void disconnect(final ChannelHandlerContext ctx, final ChannelPromise promise) { - LOG.trace("Closing SSH session on channel: {} with connect promise in state: {}", - ctx.channel(),connectPromise); + private synchronized void safelyDisconnect(final ChannelHandlerContext ctx, final ChannelPromise promise) { + LOG.trace("{}: Closing SSH session on channel: {} with connect promise in state: {}", name, ctx.channel(), + connectPromise); // If we have already succeeded and the session was dropped after, // we need to fire inactive to notify reconnect logic @@ -229,10 +277,6 @@ public class AsyncSshHandler extends ChannelOutboundHandlerAdapter { sshWriteAsyncHandler.close(); } - if (sshReadAsyncListener != null) { - sshReadAsyncListener.close(); - } - //If connection promise is not already set, it means negotiation failed //we must set connection promise to failure if (!connectPromise.isDone()) { @@ -265,12 +309,14 @@ public class AsyncSshHandler extends ChannelOutboundHandlerAdapter { // Disconnect has to be closed after inactive channel event was fired, because it interferes with it super.disconnect(ctx, ctx.newPromise()); } catch (final Exception e) { - LOG.warn("Unable to cleanup all resources for channel: {}. Ignoring.", ctx.channel(), e); + LOG.warn("{}: Unable to cleanup all resources for channel: {}. Ignoring.", name, ctx.channel(), e); } - channel = null; + if (channel != null) { + channel.close(false); + channel = null; + } promise.setSuccess(); - LOG.debug("SSH session closed on channel: {}", ctx.channel()); + LOG.debug("{}: SSH session closed on channel: {}", name, ctx.channel()); } - }