Repository: mina-sshd Updated Branches: refs/heads/master 3c9efa8a5 -> 11f85a5ac
[SSHD-813] Execute KEX related session code under synchronization lock Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/11f85a5a Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/11f85a5a Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/11f85a5a Branch: refs/heads/master Commit: 11f85a5ac6f4bba93cadf780f02c80a5e66b2cd0 Parents: 3c9efa8 Author: Lyor Goldstein <lyor.goldst...@gmail.com> Authored: Sat Apr 7 17:21:18 2018 +0300 Committer: Lyor Goldstein <lyor.goldst...@gmail.com> Committed: Tue Apr 10 19:51:19 2018 +0300 ---------------------------------------------------------------------- .../client/session/AbstractClientSession.java | 11 ++- .../org/apache/sshd/common/NamedFactory.java | 12 +-- .../common/session/helpers/AbstractSession.java | 82 ++++++++++++++++---- .../server/session/AbstractServerSession.java | 4 +- 4 files changed, 84 insertions(+), 25 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/11f85a5a/sshd-core/src/main/java/org/apache/sshd/client/session/AbstractClientSession.java ---------------------------------------------------------------------- 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 1d84d28..6a1a15d 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 @@ -471,13 +471,13 @@ public abstract class AbstractClientSession extends AbstractSession implements C @Override protected void setKexSeed(byte... seed) { - i_c = ValidateUtils.checkNotNullAndNotEmpty(seed, "No KEX seed"); + setClientKexData(seed); } @Override protected void receiveKexInit(Map<KexProposalOption, String> proposal, byte[] seed) throws IOException { mergeProposals(serverProposal, proposal); - i_s = seed; + setServerKexData(seed); } @Override @@ -550,8 +550,11 @@ public abstract class AbstractClientSession extends AbstractSession implements C proposal.put(KexProposalOption.C2SENC, BuiltinCiphers.Constants.NONE); proposal.put(KexProposalOption.S2CENC, BuiltinCiphers.Constants.NONE); - byte[] seed = sendKexInit(proposal); - setKexSeed(seed); + byte[] seed; + synchronized (kexState) { + seed = sendKexInit(proposal); + setKexSeed(seed); + } } return Objects.requireNonNull(kexFutureHolder.get(), "No current KEX future"); http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/11f85a5a/sshd-core/src/main/java/org/apache/sshd/common/NamedFactory.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/common/NamedFactory.java b/sshd-core/src/main/java/org/apache/sshd/common/NamedFactory.java index 9e4f612..5386447 100644 --- a/sshd-core/src/main/java/org/apache/sshd/common/NamedFactory.java +++ b/sshd-core/src/main/java/org/apache/sshd/common/NamedFactory.java @@ -40,7 +40,7 @@ public interface NamedFactory<T> extends Factory<T>, NamedResource { * @param <T> type of object to create * @return a newly created object or {@code null} if the factory is not in the list */ - static <T> T create(Collection<? extends NamedFactory<T>> factories, String name) { + static <T> T create(Collection<? extends NamedFactory<? extends T>> factories, String name) { NamedFactory<? extends T> f = NamedResource.findByName(name, String.CASE_INSENSITIVE_ORDER, factories); if (f != null) { return f.create(); @@ -52,15 +52,15 @@ public interface NamedFactory<T> extends Factory<T>, NamedResource { static <S extends OptionalFeature, T, E extends NamedFactory<T>> List<NamedFactory<T>> setUpTransformedFactories( boolean ignoreUnsupported, Collection<? extends S> preferred, Function<? super S, ? extends E> xform) { return preferred.stream() - .filter(f -> ignoreUnsupported || f.isSupported()) - .map(xform) - .collect(Collectors.toList()); + .filter(f -> ignoreUnsupported || f.isSupported()) + .map(xform) + .collect(Collectors.toList()); } static <T, E extends NamedFactory<T> & OptionalFeature> List<NamedFactory<T>> setUpBuiltinFactories( boolean ignoreUnsupported, Collection<? extends E> preferred) { return preferred.stream() - .filter(f -> ignoreUnsupported || f.isSupported()) - .collect(Collectors.toList()); + .filter(f -> ignoreUnsupported || f.isSupported()) + .collect(Collectors.toList()); } } http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/11f85a5a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java ---------------------------------------------------------------------- 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 5ca2192..b947e3a 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 @@ -160,8 +160,6 @@ public abstract class AbstractSession extends AbstractKexFactoryManager implemen protected final Map<KexProposalOption, String> serverProposal = new EnumMap<>(KexProposalOption.class); protected final Map<KexProposalOption, String> clientProposal = new EnumMap<>(KexProposalOption.class); protected final Map<KexProposalOption, String> negotiationResult = new EnumMap<>(KexProposalOption.class); - protected byte[] i_c; // the payload of the client's SSH_MSG_KEXINIT - protected byte[] i_s; // the payload of the factoryManager's SSH_MSG_KEXINIT protected KeyExchange kex; protected Boolean firstKexPacketFollows; protected final AtomicReference<KexState> kexState = new AtomicReference<>(KexState.UNKNOWN); @@ -247,6 +245,9 @@ public abstract class AbstractSession extends AbstractKexFactoryManager implemen private ChannelStreamPacketWriterResolver channelStreamPacketWriterResolver; private UnknownChannelReferenceHandler unknownChannelReferenceHandler; + private byte[] clientKexData; // the payload of the client's SSH_MSG_KEXINIT + private byte[] serverKexData; // the payload of the factoryManager's SSH_MSG_KEXINIT + /** * Create a new session. * @@ -780,6 +781,7 @@ public abstract class AbstractSession extends AbstractKexFactoryManager implemen log.debug("handleServiceRequest({}) SSH_MSG_SERVICE_REQUEST '{}'", this, serviceName); } validateKexState(SshConstants.SSH_MSG_SERVICE_REQUEST, KexState.DONE); + try { startService(serviceName); } catch (Throwable e) { @@ -821,6 +823,7 @@ public abstract class AbstractSession extends AbstractKexFactoryManager implemen log.debug("handleKexInit({}) SSH_MSG_KEXINIT", this); } receiveKexInit(buffer); + if (kexState.compareAndSet(KexState.DONE, KexState.RUN)) { sendKexInit(); } else if (!kexState.compareAndSet(KexState.INIT, KexState.RUN)) { @@ -829,10 +832,21 @@ public abstract class AbstractSession extends AbstractKexFactoryManager implemen Map<KexProposalOption, String> result = negotiate(); String kexAlgorithm = result.get(KexProposalOption.ALGORITHMS); - kex = ValidateUtils.checkNotNull(NamedFactory.create(getKeyExchangeFactories(), kexAlgorithm), + Collection<? extends NamedFactory<KeyExchange>> kexFactories = getKeyExchangeFactories(); + kex = ValidateUtils.checkNotNull(NamedFactory.create(kexFactories, kexAlgorithm), "Unknown negotiated KEX algorithm: %s", kexAlgorithm); - kex.init(this, serverVersion.getBytes(StandardCharsets.UTF_8), clientVersion.getBytes(StandardCharsets.UTF_8), i_s, i_c); + + byte[] v_s = serverVersion.getBytes(StandardCharsets.UTF_8); + byte[] v_c = clientVersion.getBytes(StandardCharsets.UTF_8); + byte[] i_s; + byte[] i_c; + synchronized (kexState) { + i_s = getServerKexData(); + i_c = getClientKexData(); + } + kex.init(this, v_s, v_c, i_s, i_c); + signalSessionEvent(SessionListener.Event.KexCompleted); } @@ -855,6 +869,7 @@ public abstract class AbstractSession extends AbstractKexFactoryManager implemen } signalSessionEvent(SessionListener.Event.KeyEstablished); + Collection<? extends Map.Entry<? extends SshFutureListener<IoWriteFuture>, IoWriteFuture>> pendingWrites; synchronized (pendingPackets) { pendingWrites = sendPendingPackets(pendingPackets); @@ -1085,7 +1100,8 @@ public abstract class AbstractSession extends AbstractKexFactoryManager implemen public IoWriteFuture writePacket(Buffer buffer) throws IOException { // While exchanging key, queue high level packets if (!KexState.DONE.equals(kexState.get())) { - byte cmd = buffer.array()[buffer.rpos()]; + byte[] bufData = buffer.array(); + byte cmd = bufData[buffer.rpos()]; if (cmd > SshConstants.SSH_MSG_KEX_LAST) { String cmdName = SshConstants.getCommandMessageName(cmd & 0xFF); synchronized (pendingPackets) { @@ -1179,7 +1195,8 @@ public abstract class AbstractSession extends AbstractKexFactoryManager implemen } synchronized (random) { - ignorePacketsCount.set(calculateNextIgnorePacketCount(random, ignorePacketsFrequency, ignorePacketsVariance)); + count = calculateNextIgnorePacketCount(random, ignorePacketsFrequency, ignorePacketsVariance); + ignorePacketsCount.set(count); return ignorePacketDataLength + random.random(ignorePacketDataLength); } } @@ -2583,17 +2600,46 @@ public abstract class AbstractSession extends AbstractKexFactoryManager implemen } Map<KexProposalOption, String> proposal = createProposal(resolvedAlgorithms); - byte[] seed = sendKexInit(proposal); + byte[] seed; + synchronized (kexState) { + seed = sendKexInit(proposal); + setKexSeed(seed); + } + if (log.isTraceEnabled()) { log.trace("sendKexInit({}) proposal={} seed: {}", this, proposal, BufferUtils.toHex(':', seed)); } - setKexSeed(seed); return seed; } + protected byte[] getClientKexData() { + synchronized (kexState) { + return (clientKexData == null) ? null : clientKexData.clone(); + } + } + + protected void setClientKexData(byte[] data) { + ValidateUtils.checkNotNullAndNotEmpty(data, "No client KEX seed"); + synchronized (kexState) { + clientKexData = data.clone(); + } + } + + protected byte[] getServerKexData() { + synchronized (kexState) { + return (serverKexData == null) ? null : serverKexData.clone(); + } + } + + protected void setServerKexData(byte[] data) { + ValidateUtils.checkNotNullAndNotEmpty(data, "No server KEX seed"); + synchronized (kexState) { + serverKexData = data.clone(); + } + } + /** - * @param seed The result of the KEXINIT handshake - required for correct - * session key establishment + * @param seed The result of the KEXINIT handshake - required for correct session key establishment */ protected abstract void setKexSeed(byte... seed); @@ -2622,10 +2668,20 @@ public abstract class AbstractSession extends AbstractKexFactoryManager implemen */ protected abstract void checkKeys() throws IOException; - protected void receiveKexInit(Buffer buffer) throws IOException { + protected byte[] receiveKexInit(Buffer buffer) throws IOException { Map<KexProposalOption, String> proposal = new EnumMap<>(KexProposalOption.class); - byte[] seed = receiveKexInit(buffer, proposal); - receiveKexInit(proposal, seed); + + byte[] seed; + synchronized (kexState) { + seed = receiveKexInit(buffer, proposal); + receiveKexInit(proposal, seed); + } + + if (log.isTraceEnabled()) { + log.trace("receiveKexInit({}) proposal={} seed: {}", this, proposal, BufferUtils.toHex(':', seed)); + } + + return seed; } protected abstract void receiveKexInit(Map<KexProposalOption, String> proposal, byte[] seed) throws IOException; http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/11f85a5a/sshd-core/src/main/java/org/apache/sshd/server/session/AbstractServerSession.java ---------------------------------------------------------------------- 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 d73d38e..07937df 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 @@ -236,7 +236,7 @@ public abstract class AbstractServerSession extends AbstractSession implements S @Override protected void setKexSeed(byte... seed) { - i_s = ValidateUtils.checkNotNullAndNotEmpty(seed, "No KEX seed"); + setServerKexData(seed); } @Override @@ -379,7 +379,7 @@ public abstract class AbstractServerSession extends AbstractSession implements S @Override protected void receiveKexInit(Map<KexProposalOption, String> proposal, byte[] seed) throws IOException { mergeProposals(clientProposal, proposal); - i_c = seed; + setClientKexData(seed); } @Override