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=213b4c13307ab0430d78733c804e8ff4fc15d4be;hb=eaace15120044a3e602fd3093b2412be03fd2133;hp=fa59b344b86ad1ca29c26c3cf7a3d2f971207523;hpb=df1a4dbb37e0fb187c6d50d3bab1f9d88b888928;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 fa59b344b8..213b4c1330 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,23 +5,27 @@ * 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.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.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import org.eclipse.jdt.annotation.Nullable; import org.opendaylight.netconf.nettyutil.handler.ssh.authentication.AuthenticationHandler; +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.session.ClientSession; +import org.opendaylight.netconf.shaded.sshd.core.CoreModuleProperties; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -34,15 +38,16 @@ public class AsyncSshHandler extends ChannelOutboundHandlerAdapter { 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 NetconfSshClient c = new NetconfClientBuilder().build(); + // Disable default timeouts from mina sshd + c.getProperties().put(CoreModuleProperties.AUTH_TIMEOUT.getName(), "0"); + c.getProperties().put(CoreModuleProperties.IDLE_TIMEOUT.getName(), "0"); + c.getProperties().put(CoreModuleProperties.NIO2_READ_TIMEOUT.getName(), "0"); + c.getProperties().put(CoreModuleProperties.TCP_NODELAY.getName(), true); // TODO make configurable, or somehow reuse netty threadpool c.setNioWorkers(SSH_DEFAULT_NIO_WORKERS); @@ -50,21 +55,22 @@ public class AsyncSshHandler extends ChannelOutboundHandlerAdapter { DEFAULT_CLIENT = c; } + private final AtomicBoolean isDisconnected = new AtomicBoolean(); private final AuthenticationHandler authenticationHandler; - private final SshClient sshClient; - private Future negotiationFuture; + private final Future negotiationFuture; + private final NetconfSshClient sshClient; - private AsyncSshHandlerReader sshReadAsyncListener; private AsyncSshHandlerWriter sshWriteAsyncHandler; - private ClientChannel channel; + private NettyAwareChannelSubsystem channel; private ClientSession session; private ChannelPromise connectPromise; - private GenericFutureListener negotiationFutureListener; + private FutureListener negotiationFutureListener; - public AsyncSshHandler(final AuthenticationHandler authenticationHandler, final SshClient sshClient, + public AsyncSshHandler(final AuthenticationHandler authenticationHandler, final NetconfSshClient sshClient, final Future negotiationFuture) { - this(authenticationHandler, sshClient); + this.authenticationHandler = requireNonNull(authenticationHandler); + this.sshClient = requireNonNull(sshClient); this.negotiationFuture = negotiationFuture; } @@ -74,10 +80,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,14 +97,16 @@ 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) { + return new AsyncSshHandler(authenticationHandler, sshClient != null ? sshClient : DEFAULT_CLIENT, + negotiationFuture); } 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); + final ConnectFuture sshConnectionFuture = sshClient.connect(authenticationHandler.getUsername(), address) + .verify(ctx.channel().config().getConnectTimeoutMillis(), TimeUnit.MILLISECONDS); sshConnectionFuture.addListener(future -> { if (future.isConnected()) { handleSshSessionCreated(future, ctx); @@ -115,16 +121,16 @@ public class AsyncSshHandler extends ChannelOutboundHandlerAdapter { LOG.trace("SSH session created on channel: {}", ctx.channel()); session = future.getSession(); + verify(session instanceof NettyAwareClientSession, "Unexpected session %s", session); + final AuthFuture authenticateFuture = authenticationHandler.authenticate(session); - final ClientSession localSession = session; + final NettyAwareClientSession localSession = (NettyAwareClientSession) 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); + handleSshSetupFailure(ctx, new AuthenticationFailedException("Authentication failed", + future1.getException())); } }); } catch (final IOException e) { @@ -132,12 +138,13 @@ public class AsyncSshHandler extends ChannelOutboundHandlerAdapter { } } - private synchronized void handleSshAuthenticated(final ClientSession newSession, final ChannelHandlerContext ctx) { + private synchronized void handleSshAuthenticated(final NettyAwareClientSession newSession, + final ChannelHandlerContext ctx) { try { LOG.debug("SSH session authenticated on channel: {}, server version: {}", ctx.channel(), newSession.getServerVersion()); - channel = newSession.createSubsystemChannel(SUBSYSTEM); + channel = newSession.createSubsystemChannel(SUBSYSTEM, ctx); channel.setStreaming(ClientChannel.Streaming.Async); channel.open().addListener(future -> { if (future.isOpened()) { @@ -160,18 +167,9 @@ public class AsyncSshHandler extends ChannelOutboundHandlerAdapter { connectPromise.setSuccess(); } - // TODO we should also read from error stream and at least log from that - - ClientChannel localChannel = channel; - sshReadAsyncListener = new AsyncSshHandlerReader(() -> AsyncSshHandler.this.disconnect(ctx, ctx.newPromise()), - msg -> ctx.fireChannelRead(msg), localChannel.toString(), localChannel.getAsyncOut()); - - // 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(); - } + sshWriteAsyncHandler = new AsyncSshHandlerWriter(channel.getAsyncIn()); + ctx.fireChannelActive(); + channel.onClose(() -> disconnect(ctx, ctx.newPromise())); } private synchronized void handleSshSetupFailure(final ChannelHandlerContext ctx, final Throwable error) { @@ -193,8 +191,8 @@ public class AsyncSshHandler extends ChannelOutboundHandlerAdapter { @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; + LOG.debug("SSH session connecting on channel {}. promise: {}", ctx.channel(), promise); + connectPromise = requireNonNull(promise); if (negotiationFuture != null) { negotiationFutureListener = future -> { @@ -213,11 +211,17 @@ public class AsyncSshHandler extends ChannelOutboundHandlerAdapter { disconnect(ctx, promise); } - @SuppressWarnings("checkstyle:IllegalCatch") @Override - public synchronized void disconnect(final ChannelHandlerContext ctx, final ChannelPromise promise) { + public void disconnect(final ChannelHandlerContext ctx, final ChannelPromise promise) { + if (isDisconnected.compareAndSet(false, true)) { + safelyDisconnect(ctx, promise); + } + } + + @SuppressWarnings("checkstyle:IllegalCatch") + private synchronized void safelyDisconnect(final ChannelHandlerContext ctx, final ChannelPromise promise) { LOG.trace("Closing SSH session on channel: {} with connect promise in state: {}", - ctx.channel(),connectPromise); + 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 +233,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()) { @@ -268,9 +268,13 @@ public class AsyncSshHandler extends ChannelOutboundHandlerAdapter { LOG.warn("Unable to cleanup all resources for channel: {}. Ignoring.", ctx.channel(), e); } - channel = null; + if (channel != null) { + //TODO: see if calling just close() is sufficient + //channel.close(false); + channel.close(); + channel = null; + } promise.setSuccess(); LOG.debug("SSH session closed on channel: {}", ctx.channel()); } - }