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
commit 53091ca9e4c26a1f6960c0df144e1c4663924d54 Author: Lyor Goldstein <[email protected]> AuthorDate: Tue Feb 12 14:59:10 2019 +0200 [SSHD-894] Fixed handling of delayed compression negotiated during KEX --- .../common/session/helpers/AbstractSession.java | 30 ++++++++++++---------- .../sshd/common/session/helpers/SessionHelper.java | 4 +-- .../sshd/server/session/AbstractServerSession.java | 30 ++++++++++++++++++++++ .../apache/sshd/server/session/ServerSession.java | 25 ++++++++++++++++++ .../sshd/server/session/ServerUserAuthService.java | 9 +------ 5 files changed, 75 insertions(+), 23 deletions(-) diff --git a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java index b949d6a..8e1c947 100644 --- a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java +++ b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java @@ -728,7 +728,8 @@ public abstract class AbstractSession extends SessionHelper { } } - protected IoWriteFuture doWritePacket(Buffer buffer) throws IOException { + // NOTE: must acquire encodeLock when calling this method + protected Buffer resolveOutputPacket(Buffer buffer) throws IOException { Buffer ignoreBuf = null; int ignoreDataLen = resolveIgnoreBufferDataLength(); if (ignoreDataLen > 0) { @@ -742,7 +743,7 @@ public abstract class AbstractSession extends SessionHelper { ignoreBuf.wpos(wpos + ignoreDataLen); if (log.isDebugEnabled()) { - log.debug("doWritePacket({}) append SSH_MSG_IGNORE message", this); + log.debug("resolveOutputPacket({}) append SSH_MSG_IGNORE message", this); } } @@ -751,22 +752,25 @@ public abstract class AbstractSession extends SessionHelper { int cmd = data[curPos] & 0xFF; // usually the 1st byte is the command buffer = validateTargetBuffer(cmd, buffer); + if (ignoreBuf != null) { + ignoreBuf = encode(ignoreBuf); + + IoSession networkSession = getIoSession(); + networkSession.writePacket(ignoreBuf); + } + + return encode(buffer); + } + + protected IoWriteFuture doWritePacket(Buffer buffer) throws IOException { // Synchronize all write requests as needed by the encoding algorithm // and also queue the write request in this synchronized block to ensure // packets are sent in the correct order - IoWriteFuture future; - IoSession networkSession = getIoSession(); synchronized (encodeLock) { - if (ignoreBuf != null) { - ignoreBuf = encode(ignoreBuf); - networkSession.writePacket(ignoreBuf); - } - - buffer = encode(buffer); - future = networkSession.writePacket(buffer); + Buffer packet = resolveOutputPacket(buffer); + IoSession networkSession = getIoSession(); + return networkSession.writePacket(packet); } - - return future; } protected int resolveIgnoreBufferDataLength() { diff --git a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java index 41e85ac..2846157 100644 --- a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java +++ b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java @@ -107,11 +107,11 @@ public abstract class SessionHelper extends AbstractKexFactoryManager implements /** * The name of the authenticated user */ - private String username; + private volatile String username; /** * Boolean indicating if this session has been authenticated or not */ - private boolean authed; + private volatile boolean authed; /** * Create a new session. diff --git a/sshd-core/src/main/java/org/apache/sshd/server/session/AbstractServerSession.java b/sshd-core/src/main/java/org/apache/sshd/server/session/AbstractServerSession.java index 523f16a..2aafa06 100644 --- a/sshd-core/src/main/java/org/apache/sshd/server/session/AbstractServerSession.java +++ b/sshd-core/src/main/java/org/apache/sshd/server/session/AbstractServerSession.java @@ -256,6 +256,36 @@ public abstract class AbstractServerSession extends AbstractSession implements S } @Override + public IoWriteFuture signalAuthenticationSuccess( + String username, String authService, Buffer buffer) + throws Exception { + KexState curState = kexState.get(); + if (!KexState.DONE.equals(curState)) { + throw new SshException(SshConstants.SSH2_DISCONNECT_PROTOCOL_ERROR, + "Authentication success signalled though KEX state=" + curState); + } + + Buffer response = createBuffer(SshConstants.SSH_MSG_USERAUTH_SUCCESS, Byte.SIZE); + IoWriteFuture future; + IoSession networkSession = getIoSession(); + synchronized (encodeLock) { + Buffer packet = resolveOutputPacket(response); + + setUsername(username); + // must be AFTER the USERAUTH-SUCCESS packet created in case delayed compression is used + setAuthenticated(); + startService(authService, buffer); + + // Now we can inform the peer that authentication is successful + future = networkSession.writePacket(packet); + } + + resetIdleTimeout(); + log.info("Session {}@{} authenticated", username, networkSession.getRemoteAddress()); + return future; + } + + @Override protected void handleServiceAccept(String serviceName, Buffer buffer) throws Exception { super.handleServiceAccept(serviceName, buffer); diff --git a/sshd-core/src/main/java/org/apache/sshd/server/session/ServerSession.java b/sshd-core/src/main/java/org/apache/sshd/server/session/ServerSession.java index 99bec17..f06261c 100644 --- a/sshd-core/src/main/java/org/apache/sshd/server/session/ServerSession.java +++ b/sshd-core/src/main/java/org/apache/sshd/server/session/ServerSession.java @@ -22,7 +22,9 @@ package org.apache.sshd.server.session; import java.net.SocketAddress; import java.security.KeyPair; +import org.apache.sshd.common.io.IoWriteFuture; import org.apache.sshd.common.session.Session; +import org.apache.sshd.common.util.buffer.Buffer; import org.apache.sshd.server.ServerAuthenticationManager; import org.apache.sshd.server.ServerFactoryManager; @@ -61,4 +63,27 @@ public interface ServerSession * @return The current number of live <code>SshSession</code> objects associated with the user */ int getActiveSessionCountForUser(String userName); + + /** + * <UL> + * <P><LI> + * Marks the session as authenticated. + * </LI></P> + * + * <P><LI> + * Starts the specified service. + * </LI></P> + * + * <P><LI> + * Sends the {@code SSH_MSG_USERAUTH_SUCCESS} message. + * </LI></P> + * </UL> + * @param username The authenticated username + * @param authService The service to start + * @param buffer Any extra data received to use to start the service + * @return An {@link IoWriteFuture} that can be used to wait for the + * {@code SSH_MSG_USERAUTH_SUCCESS} message send result + * @throws Exception if cannot handle the request + */ + IoWriteFuture signalAuthenticationSuccess(String username, String authService, Buffer buffer) throws Exception; } diff --git a/sshd-core/src/main/java/org/apache/sshd/server/session/ServerUserAuthService.java b/sshd-core/src/main/java/org/apache/sshd/server/session/ServerUserAuthService.java index 653d8b1..cf8cb4d 100644 --- a/sshd-core/src/main/java/org/apache/sshd/server/session/ServerUserAuthService.java +++ b/sshd-core/src/main/java/org/apache/sshd/server/session/ServerUserAuthService.java @@ -359,14 +359,7 @@ public class ServerUserAuthService extends AbstractCloseable implements Service, sendWelcomeBanner(session); } - session.setUsername(username); - session.setAuthenticated(); - session.startService(authService, buffer); - - Buffer response = session.createBuffer(SshConstants.SSH_MSG_USERAUTH_SUCCESS, Byte.SIZE); - session.writePacket(response); - session.resetIdleTimeout(); - log.info("Session {}@{} authenticated", username, session.getIoSession().getRemoteAddress()); + session.signalAuthenticationSuccess(username, authService, buffer); } else { String remaining = authMethods.stream() .filter(GenericUtils::isNotEmpty)
