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 0b5544a8af5bf58c23e229f39acce6639a5feb7e Author: Lyor Goldstein <lgoldst...@apache.org> AuthorDate: Fri Jan 8 07:02:10 2021 +0200 [SSHD-1097] Added more SessionListener callbacks related to the initial version and key exchange --- CHANGES.md | 2 + .../sshd/client/session/AbstractClientSession.java | 4 + .../common/kex/extension/KexExtensionHandler.java | 6 +- .../sshd/common/session/SessionListener.java | 42 +++++++++ .../common/session/helpers/AbstractSession.java | 2 + .../sshd/common/session/helpers/SessionHelper.java | 103 ++++++++++++++++++--- .../sshd/server/session/AbstractServerSession.java | 8 +- .../sshd/server/session/ServerSessionImpl.java | 10 +- .../session/helpers/AbstractSessionTest.java | 20 ++-- 9 files changed, 169 insertions(+), 28 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 69e03b3..7d2b02c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -25,6 +25,8 @@ ## Behavioral changes and enhancements * [SSHD-1085](https://issues.apache.org/jira/browse/SSHD-1085) Added more notifications related to channel state change for detecting channel closing or closed earlier. +* [SSHD-1091](https://issues.apache.org/jira/browse/SSHD-1091) Renamed `sshd-contrib` top-level package in order to align naming convention. +* [SSHD-1097](https://issues.apache.org/jira/browse/SSHD-1097) Added more `SessionListener` callbacks related to the initial version and key exchange * [SSHD-1109](https://issues.apache.org/jira/browse/SSHD-1109) Replace log4j with logback as the slf4j logger implementation for tests * [SSHD-1114](https://issues.apache.org/jira/browse/SSHD-1114) Added callbacks for client-side password authentication progress * [SSHD-1114](https://issues.apache.org/jira/browse/SSHD-1114) Added callbacks for client-side public key authentication progress diff --git a/sshd-core/src/main/java/org/apache/sshd/client/session/AbstractClientSession.java b/sshd-core/src/main/java/org/apache/sshd/client/session/AbstractClientSession.java index 6b1faff..ce8e529 100644 --- a/sshd-core/src/main/java/org/apache/sshd/client/session/AbstractClientSession.java +++ b/sshd-core/src/main/java/org/apache/sshd/client/session/AbstractClientSession.java @@ -23,6 +23,7 @@ import java.io.IOException; import java.net.SocketAddress; import java.security.KeyPair; import java.security.PublicKey; +import java.util.Collections; import java.util.EnumMap; import java.util.List; import java.util.Map; @@ -353,6 +354,9 @@ public abstract class AbstractClientSession extends AbstractSession implements C protected IoWriteFuture sendClientIdentification() throws Exception { clientVersion = resolveIdentificationString(CoreModuleProperties.CLIENT_IDENTIFICATION.getName()); + // Note: we intentionally use an unmodifiable list in order to enforce the fact that client cannot send header lines + signalSendIdentification(clientVersion, Collections.emptyList()); + return sendIdentification(clientVersion); } diff --git a/sshd-core/src/main/java/org/apache/sshd/common/kex/extension/KexExtensionHandler.java b/sshd-core/src/main/java/org/apache/sshd/common/kex/extension/KexExtensionHandler.java index b5428c6..ae039f8 100644 --- a/sshd-core/src/main/java/org/apache/sshd/common/kex/extension/KexExtensionHandler.java +++ b/sshd-core/src/main/java/org/apache/sshd/common/kex/extension/KexExtensionHandler.java @@ -84,7 +84,7 @@ public interface KexExtensionHandler { * @param initiator {@code true} if the proposal is about to be sent, {@code false} if this is a proposal * received from the peer. * @param proposal The proposal contents - <B>Caveat emptor:</B> the proposal is <U>modifiable</U> i.e., the - * handler can modify before being sent or before being processed (if incoming) + * handler can modify it before being sent or before being processed (if incoming) * @throws IOException If failed to handle the request */ default void handleKexInitProposal( @@ -117,7 +117,7 @@ public interface KexExtensionHandler { /** * The phase at which {@code sendKexExtensions} is invoked - * + * * @author <a href="mailto:d...@mina.apache.org">Apache MINA SSHD Project</a> */ enum KexPhase { @@ -130,7 +130,7 @@ public interface KexExtensionHandler { /** * Invoked in order to allow the handler to send an {@code SSH_MSG_EXT_INFO} message. <B>Note:</B> this method is * called only if {@code isKexExtensionsAvailable} returns {@code true} for the session. - * + * * @param session The {@link Session} * @param phase The phase at which the handler is invoked * @throws IOException If failed to handle the invocation diff --git a/sshd-core/src/main/java/org/apache/sshd/common/session/SessionListener.java b/sshd-core/src/main/java/org/apache/sshd/common/session/SessionListener.java index 36f7275..7b89478 100644 --- a/sshd-core/src/main/java/org/apache/sshd/common/session/SessionListener.java +++ b/sshd-core/src/main/java/org/apache/sshd/common/session/SessionListener.java @@ -58,6 +58,38 @@ public interface SessionListener extends SshdEventListener { } /** + * About to send identification to peer + * + * @param session The {@link Session} instance + * @param version The resolved identification version + * @param extraLines Extra data preceding the identification to be sent. <B>Note:</B> the list is modifiable only if + * this is a server session. The user may modify it based on the peer. + * @see <A HREF="https://tools.ietf.org/html/rfc4253#section-4.2">RFC 4253 - section 4.2 - Protocol + * Version Exchange</A> + */ + default void sessionPeerIdentificationSend( + Session session, String version, List<String> extraLines) { + // ignored + } + + /** + * Successfully read a line as part of the initial peer identification + * + * @param session The {@link Session} instance + * @param line The data that was read so far - <B>Note:</B> might not be a full line if more packets are + * required for full identification data. Furthermore, it may be <U>repeated</U> data due to + * packets segmentation and re-assembly mechanism + * @param extraLines Previous lines that were before this one - <B>Note:</B> it may be <U>repeated</U> data due to + * packets segmentation and re-assembly mechanism + * @see <A HREF="https://tools.ietf.org/html/rfc4253#section-4.2">RFC 4253 - section 4.2 - Protocol + * Version Exchange</A> + */ + default void sessionPeerIdentificationLine( + Session session, String line, List<String> extraLines) { + // ignored + } + + /** * The peer's identification version was received * * @param session The {@link Session} instance @@ -72,6 +104,16 @@ public interface SessionListener extends SshdEventListener { } /** + * + * @param session The referenced {@link Session} + * @param proposal The proposals that will be sent to the peer - <B>Caveat emptor:</B> the proposal is + * <U>modifiable</U> i.e., the handler can modify it before being sent + */ + default void sessionNegotiationOptionsCreated(Session session, Map<KexProposalOption, String> proposal) { + // ignored + } + + /** * Signals the start of the negotiation options handling * * @param session The referenced {@link Session} 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 124895f..13bb2f9 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 @@ -2282,6 +2282,8 @@ public abstract class AbstractSession extends SessionHelper { } } + signalNegotiationOptionsCreated(proposal); + byte[] seed; synchronized (kexState) { seed = sendKexInit(proposal); 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 c62719a..7c0e74d 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 @@ -610,6 +610,57 @@ public abstract class SessionHelper extends AbstractKexFactoryManager implements listener.sessionCreated(this); } + protected void signalSendIdentification(String version, List<String> extraLines) throws Exception { + try { + invokeSessionSignaller(l -> { + signalSendIdentification(l, version, extraLines); + return null; + }); + } catch (Throwable err) { + Throwable e = GenericUtils.peelException(err); + if (e instanceof Exception) { + throw (Exception) e; + } else { + throw new RuntimeSshException(e); + } + } + } + + protected void signalSendIdentification(SessionListener listener, String version, List<String> extraLines) { + if (listener == null) { + return; + } + + listener.sessionPeerIdentificationSend(this, version, extraLines); + } + + protected void signalReadPeerIdentificationLine(String line, List<String> extraLines) throws Exception { + try { + invokeSessionSignaller(l -> { + signalReadPeerIdentificationLine(l, line, extraLines); + return null; + }); + } catch (Throwable err) { + Throwable e = GenericUtils.peelException(err); + debug("signalReadPeerIdentificationLine({}) Failed ({}) to announce peer={}: {}", + this, e.getClass().getSimpleName(), line, e.getMessage(), e); + if (e instanceof Exception) { + throw (Exception) e; + } else { + throw new RuntimeSshException(e); + } + } + } + + protected void signalReadPeerIdentificationLine( + SessionListener listener, String version, List<String> extraLines) { + if (listener == null) { + return; + } + + listener.sessionPeerIdentificationLine(this, version, extraLines); + } + protected void signalPeerIdentificationReceived(String version, List<String> extraLines) throws Exception { try { invokeSessionSignaller(l -> { @@ -626,10 +677,10 @@ public abstract class SessionHelper extends AbstractKexFactoryManager implements throw new RuntimeSshException(e); } } - } - protected void signalPeerIdentificationReceived(SessionListener listener, String version, List<String> extraLines) { + protected void signalPeerIdentificationReceived( + SessionListener listener, String version, List<String> extraLines) { if (listener == null) { return; } @@ -684,6 +735,7 @@ public abstract class SessionHelper extends AbstractKexFactoryManager implements if (l == null) { continue; } + try { invoker.invoke(l); } catch (Throwable t) { @@ -801,14 +853,13 @@ public abstract class SessionHelper extends AbstractKexFactoryManager implements * state and a {@code null} value will be returned. Else the identification string will be returned and the data * read will be consumed from the buffer. * - * @param buffer the buffer containing the identification string - * @param server {@code true} if it is called by the server session, {@code false} if by the client session - * @return A {@link List} of all received remote identification lines until the version line was read or - * {@code null} if more data is needed. The identification line is the <U>last</U> one in the - * list - * @throws IOException if malformed identification found + * @param buffer the buffer containing the identification string + * @param server {@code true} if it is called by the server session, {@code false} if by the client session + * @return A {@link List} of all received remote identification lines until the version line was read or + * {@code null} if more data is needed. The identification line is the <U>last</U> one in the list + * @throws Exception if malformed identification found */ - protected List<String> doReadIdentification(Buffer buffer, boolean server) throws IOException { + protected List<String> doReadIdentification(Buffer buffer, boolean server) throws Exception { int maxIdentSize = CoreModuleProperties.MAX_IDENTIFICATION_SIZE.getRequired(this); List<String> ident = null; int rpos = buffer.rpos(); @@ -869,6 +920,8 @@ public abstract class SessionHelper extends AbstractKexFactoryManager implements if (ident == null) { ident = new ArrayList<>(); } + + signalReadPeerIdentificationLine(str, ident); ident.add(str); // if this is a server then only one line is expected from the client @@ -943,6 +996,32 @@ public abstract class SessionHelper extends AbstractKexFactoryManager implements return proposal; } + protected void signalNegotiationOptionsCreated(Map<KexProposalOption, String> proposal) { + try { + invokeSessionSignaller(l -> { + signalNegotiationOptionsCreated(l, proposal); + return null; + }); + } catch (Throwable t) { + Throwable err = GenericUtils.peelException(t); + if (err instanceof RuntimeException) { + throw (RuntimeException) err; + } else if (err instanceof Error) { + throw (Error) err; + } else { + throw new RuntimeException(err); + } + } + } + + protected void signalNegotiationOptionsCreated(SessionListener listener, Map<KexProposalOption, String> proposal) { + if (listener == null) { + return; + } + + listener.sessionNegotiationOptionsCreated(this, proposal); + } + protected void signalNegotiationStart( Map<KexProposalOption, String> c2sOptions, Map<KexProposalOption, String> s2cOptions) { try { @@ -950,7 +1029,8 @@ public abstract class SessionHelper extends AbstractKexFactoryManager implements signalNegotiationStart(l, c2sOptions, s2cOptions); return null; }); - } catch (Throwable err) { + } catch (Throwable t) { + Throwable err = GenericUtils.peelException(t); if (err instanceof RuntimeException) { throw (RuntimeException) err; } else if (err instanceof Error) { @@ -978,7 +1058,8 @@ public abstract class SessionHelper extends AbstractKexFactoryManager implements signalNegotiationEnd(l, c2sOptions, s2cOptions, negotiatedGuess, reason); return null; }); - } catch (Throwable err) { + } catch (Throwable t) { + Throwable err = GenericUtils.peelException(t); if (err instanceof RuntimeException) { throw (RuntimeException) err; } else if (err instanceof Error) { 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 4c2c9b3..d663c10 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 @@ -219,16 +219,18 @@ public abstract class AbstractServerSession extends AbstractSession implements S * {@code null}/empty * @return An {@link IoWriteFuture} that can be used to be notified of identification data being written * successfully or failing - * @throws IOException If failed to send identification + * @throws Exception If failed to send identification * @see <A HREF="https://tools.ietf.org/html/rfc4253#section-4.2">RFC 4253 - section 4.2</A> */ - protected IoWriteFuture sendServerIdentification(String... headerLines) throws IOException { + protected IoWriteFuture sendServerIdentification(List<String> headerLines) throws Exception { serverVersion = resolveIdentificationString(CoreModuleProperties.SERVER_IDENTIFICATION.getName()); + signalSendIdentification(serverVersion, headerLines); String ident = serverVersion; - if (GenericUtils.length(headerLines) > 0) { + if (GenericUtils.size(headerLines) > 0) { ident = GenericUtils.join(headerLines, "\r\n") + "\r\n" + serverVersion; } + return sendIdentification(ident); } diff --git a/sshd-core/src/main/java/org/apache/sshd/server/session/ServerSessionImpl.java b/sshd-core/src/main/java/org/apache/sshd/server/session/ServerSessionImpl.java index 06d7c97..9fee875 100644 --- a/sshd-core/src/main/java/org/apache/sshd/server/session/ServerSessionImpl.java +++ b/sshd-core/src/main/java/org/apache/sshd/server/session/ServerSessionImpl.java @@ -18,6 +18,10 @@ */ package org.apache.sshd.server.session; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + import org.apache.sshd.common.io.IoSession; import org.apache.sshd.common.util.GenericUtils; import org.apache.sshd.core.CoreModuleProperties; @@ -35,6 +39,10 @@ public class ServerSessionImpl extends AbstractServerSession { String headerConfig = CoreModuleProperties.SERVER_EXTRA_IDENTIFICATION_LINES.getOrNull(this); String[] headers = GenericUtils.split(headerConfig, CoreModuleProperties.SERVER_EXTRA_IDENT_LINES_SEPARATOR); - sendServerIdentification(headers); + // We intentionally create a modifiable array so as to allow users to modify it via SessionListener + List<String> extraLines = GenericUtils.isEmpty(headers) + ? new ArrayList<>() + : new ArrayList<>(Arrays.asList(headers)); + sendServerIdentification(extraLines); } } diff --git a/sshd-core/src/test/java/org/apache/sshd/common/session/helpers/AbstractSessionTest.java b/sshd-core/src/test/java/org/apache/sshd/common/session/helpers/AbstractSessionTest.java index d8fd35f..a143cab 100644 --- a/sshd-core/src/test/java/org/apache/sshd/common/session/helpers/AbstractSessionTest.java +++ b/sshd-core/src/test/java/org/apache/sshd/common/session/helpers/AbstractSessionTest.java @@ -86,28 +86,28 @@ public class AbstractSessionTest extends BaseTestSupport { } @Test - public void testReadIdentSimple() throws IOException { + public void testReadIdentSimple() throws Exception { Buffer buf = new ByteArrayBuffer("SSH-2.0-software\r\n".getBytes(StandardCharsets.UTF_8)); String ident = readIdentification(session, buf); assertEquals("SSH-2.0-software", ident); } @Test - public void testReadIdentWithoutCR() throws IOException { + public void testReadIdentWithoutCR() throws Exception { Buffer buf = new ByteArrayBuffer("SSH-2.0-software\n".getBytes(StandardCharsets.UTF_8)); String ident = readIdentification(session, buf); assertEquals("SSH-2.0-software", ident); } @Test - public void testReadIdentWithHeaders() throws IOException { + public void testReadIdentWithHeaders() throws Exception { Buffer buf = new ByteArrayBuffer("a header line\r\nSSH-2.0-software\r\n".getBytes(StandardCharsets.UTF_8)); String ident = readIdentification(session, buf); assertEquals("SSH-2.0-software", ident); } @Test - public void testReadIdentWithSplitPackets() throws IOException { + public void testReadIdentWithSplitPackets() throws Exception { Buffer buf = new ByteArrayBuffer("header line\r\nSSH".getBytes(StandardCharsets.UTF_8)); String ident = readIdentification(session, buf); assertNull("Unexpected identification for header only", ident); @@ -118,14 +118,14 @@ public class AbstractSessionTest extends BaseTestSupport { } @Test(expected = StreamCorruptedException.class) - public void testReadIdentBadLineEnding() throws IOException { + public void testReadIdentBadLineEnding() throws Exception { Buffer buf = new ByteArrayBuffer("SSH-2.0-software\ra".getBytes(StandardCharsets.UTF_8)); String ident = readIdentification(session, buf); fail("Unexpected success: " + ident); } @Test(expected = StreamCorruptedException.class) - public void testReadIdentLongLine() throws IOException { + public void testReadIdentLongLine() throws Exception { StringBuilder sb = new StringBuilder(SessionContext.MAX_VERSION_LINE_LENGTH + Integer.SIZE); sb.append("SSH-2.0-software"); do { @@ -138,7 +138,7 @@ public class AbstractSessionTest extends BaseTestSupport { } @Test(expected = StreamCorruptedException.class) - public void testReadIdentWithNullChar() throws IOException { + public void testReadIdentWithNullChar() throws Exception { String id = "SSH-2.0" + '\0' + "-software\r\n"; Buffer buf = new ByteArrayBuffer(id.getBytes(StandardCharsets.UTF_8)); String ident = readIdentification(session, buf); @@ -146,7 +146,7 @@ public class AbstractSessionTest extends BaseTestSupport { } @Test(expected = StreamCorruptedException.class) - public void testReadIdentLongHeader() throws IOException { + public void testReadIdentLongHeader() throws Exception { int maxIdentSize = CoreModuleProperties.MAX_IDENTIFICATION_SIZE.getRequiredDefault(); StringBuilder sb = new StringBuilder(maxIdentSize + Integer.SIZE); do { @@ -300,7 +300,7 @@ public class AbstractSessionTest extends BaseTestSupport { } } - private static String readIdentification(MySession session, Buffer buf) throws IOException { + private static String readIdentification(MySession session, Buffer buf) throws Exception { List<String> lines = session.doReadIdentification(buf); return GenericUtils.isEmpty(lines) ? null : lines.get(lines.size() - 1); } @@ -439,7 +439,7 @@ public class AbstractSessionTest extends BaseTestSupport { return false; } - public List<String> doReadIdentification(Buffer buffer) throws IOException { + public List<String> doReadIdentification(Buffer buffer) throws Exception { return super.doReadIdentification(buffer, false); }