You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by lg...@apache.org on 2015/08/30 14:25:01 UTC
mina-sshd git commit: [SSHD-558] Failures in the key re-exchange or
switching to none cipher are not signaled to the future instance
Repository: mina-sshd
Updated Branches:
refs/heads/master 1e5efb1a0 -> 80a73a28d
[SSHD-558] Failures in the key re-exchange or switching to none cipher are not signaled to the future instance
Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/80a73a28
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/80a73a28
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/80a73a28
Branch: refs/heads/master
Commit: 80a73a28d729036e33f98e203668b067a285b5fe
Parents: 1e5efb1
Author: Lyor Goldstein <lg...@vmware.com>
Authored: Sun Aug 30 15:24:50 2015 +0300
Committer: Lyor Goldstein <lg...@vmware.com>
Committed: Sun Aug 30 15:24:50 2015 +0300
----------------------------------------------------------------------
.../org/apache/sshd/client/kex/DHGClient.java | 2 +-
.../org/apache/sshd/client/kex/DHGEXClient.java | 2 +-
.../sshd/client/session/ClientSession.java | 7 +-
.../sshd/client/session/ClientSessionImpl.java | 26 ++--
.../sshd/common/channel/AbstractChannel.java | 2 +
.../org/apache/sshd/common/channel/Window.java | 2 +-
.../common/future/DefaultKeyExchangeFuture.java | 62 +++++++++
.../sshd/common/future/DefaultSshFuture.java | 5 +-
.../sshd/common/future/KeyExchangeFuture.java | 60 +++++++++
.../sshd/common/session/AbstractSession.java | 72 ++++++++--
.../org/apache/sshd/common/session/Session.java | 7 +-
.../org/apache/sshd/server/kex/DHGServer.java | 1 -
.../java/org/apache/sshd/KeyReExchangeTest.java | 130 ++++++++++++++++++-
.../java/org/apache/sshd/client/ClientTest.java | 25 +---
.../org/apache/sshd/client/kex/KexTest.java | 42 +++---
15 files changed, 356 insertions(+), 89 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/80a73a28/sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java b/sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java
index e7276b8..8e06415 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java
@@ -134,7 +134,7 @@ public class DHGClient extends AbstractDHClientKeyExchange {
verif.initVerifier(serverKey);
verif.update(h);
if (!verif.verify(sig)) {
- throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED, "KeyExchange signature verification failed");
+ throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED, "KeyExchange signature verification failed for key type=" + keyAlg);
}
return true;
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/80a73a28/sshd-core/src/main/java/org/apache/sshd/client/kex/DHGEXClient.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/kex/DHGEXClient.java b/sshd-core/src/main/java/org/apache/sshd/client/kex/DHGEXClient.java
index bd4003f..6f66f37 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/kex/DHGEXClient.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/kex/DHGEXClient.java
@@ -160,7 +160,7 @@ public class DHGEXClient extends AbstractDHClientKeyExchange {
verif.update(h);
if (!verif.verify(sig)) {
throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
- "KeyExchange signature verification failed");
+ "KeyExchange signature verification failed for key type=" + keyAlg);
}
return true;
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/80a73a28/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSession.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSession.java b/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSession.java
index b90eade..b67d826 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSession.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSession.java
@@ -35,7 +35,7 @@ import org.apache.sshd.client.scp.ScpClient;
import org.apache.sshd.client.subsystem.sftp.SftpClient;
import org.apache.sshd.client.subsystem.sftp.SftpVersionSelector;
import org.apache.sshd.common.SshdSocketAddress;
-import org.apache.sshd.common.future.SshFuture;
+import org.apache.sshd.common.future.KeyExchangeFuture;
import org.apache.sshd.common.scp.ScpTransferEventListener;
import org.apache.sshd.common.session.Session;
@@ -334,10 +334,9 @@ public interface ClientSession extends Session {
* If that's not the case, the returned future will be set with an exception.
* </P>
*
- * @return an {@link SshFuture} that can be used to wait for the exchange
+ * @return an {@link KeyExchangeFuture} that can be used to wait for the exchange
* to be finished
* @throws IOException if a key exchange is already running
*/
- @SuppressWarnings("rawtypes")
- SshFuture switchToNoneCipher() throws IOException;
+ KeyExchangeFuture switchToNoneCipher() throws IOException;
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/80a73a28/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java b/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java
index d44a855..44b767b 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java
@@ -57,8 +57,8 @@ import org.apache.sshd.common.cipher.BuiltinCiphers;
import org.apache.sshd.common.cipher.CipherNone;
import org.apache.sshd.common.config.keys.KeyUtils;
import org.apache.sshd.common.forward.TcpipForwarder;
-import org.apache.sshd.common.future.DefaultSshFuture;
-import org.apache.sshd.common.future.SshFuture;
+import org.apache.sshd.common.future.DefaultKeyExchangeFuture;
+import org.apache.sshd.common.future.KeyExchangeFuture;
import org.apache.sshd.common.io.IoSession;
import org.apache.sshd.common.kex.KexProposalOption;
import org.apache.sshd.common.kex.KexState;
@@ -271,14 +271,23 @@ public class ClientSessionImpl extends AbstractSession implements ClientSession
}
@Override
- @SuppressWarnings("rawtypes")
- public SshFuture switchToNoneCipher() throws IOException {
+ public KeyExchangeFuture switchToNoneCipher() throws IOException {
if (!(currentService instanceof AbstractConnectionService)
|| !((AbstractConnectionService) currentService).getChannels().isEmpty()) {
throw new IllegalStateException("The switch to the none cipher must be done immediately after authentication");
}
+
if (kexState.compareAndSet(KexState.DONE, KexState.INIT)) {
- reexchangeFuture = new DefaultSshFuture(null);
+ DefaultKeyExchangeFuture kexFuture = new DefaultKeyExchangeFuture(null);
+ DefaultKeyExchangeFuture prev = kexFutureHolder.getAndSet(kexFuture);
+ if (prev != null) {
+ synchronized (prev) {
+ Object value = prev.getValue();
+ if (value == null) {
+ prev.setValue(new SshException("Switch to none cipher while previous KEX is ongoing"));
+ }
+ }
+ }
String c2sEncServer;
String s2cEncServer;
@@ -300,9 +309,9 @@ public class ClientSessionImpl extends AbstractSession implements ClientSession
boolean s2cEncClientNone = BuiltinCiphers.Constants.isNoneCipherIncluded(s2cEncClient);
if ((!c2sEncServerNone) || (!s2cEncServerNone)) {
- reexchangeFuture.setValue(new SshException("Server does not support none cipher"));
+ kexFuture.setValue(new SshException("Server does not support none cipher"));
} else if ((!c2sEncClientNone) || (!s2cEncClientNone)) {
- reexchangeFuture.setValue(new SshException("Client does not support none cipher"));
+ kexFuture.setValue(new SshException("Client does not support none cipher"));
} else {
log.info("Switching to none cipher");
@@ -317,7 +326,8 @@ public class ClientSessionImpl extends AbstractSession implements ClientSession
byte[] seed = sendKexInit(proposal);
setKexSeed(seed);
}
- return reexchangeFuture;
+
+ return ValidateUtils.checkNotNull(kexFutureHolder.get(), "No current KEX future");
} else {
throw new SshException("In flight key exchange");
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/80a73a28/sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java b/sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java
index 62bd7c1..19ef0cb 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java
@@ -372,6 +372,8 @@ public abstract class AbstractChannel
}
}
}
+
+ super.preClose();
}
@Override
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/80a73a28/sshd-core/src/main/java/org/apache/sshd/common/channel/Window.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/channel/Window.java b/sshd-core/src/main/java/org/apache/sshd/common/channel/Window.java
index 15e7b8a..eed5505 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/channel/Window.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/channel/Window.java
@@ -167,7 +167,7 @@ public class Window extends AbstractLoggingBean implements java.nio.channels.Cha
check(maxSize);
} catch (RuntimeException e) {
throw new StreamCorruptedException("consumeAndCheck(" + this + ")"
- + " failed " + e.getClass().getSimpleName() + ")"
+ + " failed (" + e.getClass().getSimpleName() + ")"
+ " to consume " + len + " bytes"
+ ": " + e.getMessage());
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/80a73a28/sshd-core/src/main/java/org/apache/sshd/common/future/DefaultKeyExchangeFuture.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/future/DefaultKeyExchangeFuture.java b/sshd-core/src/main/java/org/apache/sshd/common/future/DefaultKeyExchangeFuture.java
new file mode 100644
index 0000000..5d89f99
--- /dev/null
+++ b/sshd-core/src/main/java/org/apache/sshd/common/future/DefaultKeyExchangeFuture.java
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.sshd.common.future;
+
+import java.io.IOException;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.sshd.common.SshException;
+
+/**
+ * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ */
+public class DefaultKeyExchangeFuture extends DefaultSshFuture<KeyExchangeFuture>implements KeyExchangeFuture {
+ public DefaultKeyExchangeFuture(Object lock) {
+ super(lock);
+ }
+
+ @Override // TODO for JDK-8 make this a default method
+ public void verify() throws IOException {
+ verify(Long.MAX_VALUE);
+ }
+
+ @Override // TODO for JDK-8 make this a default method
+ public void verify(long timeout, TimeUnit unit) throws IOException {
+ verify(unit.toMillis(timeout));
+ }
+
+ @Override // TODO for JDK-8 make this a default method
+ public void verify(long timeoutMillis) throws IOException {
+ Boolean result = verifyResult(Boolean.class, timeoutMillis);
+ if (!result.booleanValue()) {
+ throw new SshException("Key exchange failed");
+ }
+ }
+
+ @Override // TODO for JDK-8 make this a default method
+ public Throwable getException() {
+ Object v = getValue();
+ if (v instanceof Throwable) {
+ return (Throwable) v;
+ } else {
+ return null;
+ }
+ }
+}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/80a73a28/sshd-core/src/main/java/org/apache/sshd/common/future/DefaultSshFuture.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/future/DefaultSshFuture.java b/sshd-core/src/main/java/org/apache/sshd/common/future/DefaultSshFuture.java
index 92c915c..812e06e 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/future/DefaultSshFuture.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/future/DefaultSshFuture.java
@@ -238,9 +238,10 @@ public class DefaultSshFuture<T extends SshFuture> extends AbstractLoggingBean i
}
/**
- * @return The result of the asynchronous operation.
+ * @return The result of the asynchronous operation - or {@code null}
+ * if none set.
*/
- protected Object getValue() {
+ public Object getValue() {
synchronized (lock) {
return result == NULL ? null : result;
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/80a73a28/sshd-core/src/main/java/org/apache/sshd/common/future/KeyExchangeFuture.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/future/KeyExchangeFuture.java b/sshd-core/src/main/java/org/apache/sshd/common/future/KeyExchangeFuture.java
new file mode 100644
index 0000000..c4f4e1f
--- /dev/null
+++ b/sshd-core/src/main/java/org/apache/sshd/common/future/KeyExchangeFuture.java
@@ -0,0 +1,60 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.sshd.common.future;
+
+import java.io.IOException;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ */
+public interface KeyExchangeFuture extends SshFuture<KeyExchangeFuture> {
+ /**
+ * Wait and verify that the exchange has been successful
+ *
+ * @throws IOException if the action failed for any reason
+ */
+ void verify() throws IOException;
+
+ /**
+ * Wait and verify that the exchange has been successful
+ *
+ * @param timeout The number of time units to wait
+ * @param unit The wait {@link TimeUnit}
+ * @throws IOException If failed to verify successfully on time
+ */
+ void verify(long timeout, TimeUnit unit) throws IOException;
+
+ /**
+ * Wait and verify that the exchange has been successful
+ *
+ * @param timeoutMillis Wait timeout in milliseconds
+ * @throws IOException If failed to verify successfully on time
+ */
+ void verify(long timeoutMillis) throws IOException;
+
+ /**
+ * Returns the cause of the exchange failure.
+ *
+ * @return <code>null</code> if the exchange operation is not finished yet,
+ * or if the exchange attempt is successful.
+ */
+ Throwable getException();
+}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/80a73a28/sshd-core/src/main/java/org/apache/sshd/common/session/AbstractSession.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/session/AbstractSession.java b/sshd-core/src/main/java/org/apache/sshd/common/session/AbstractSession.java
index 8b1b59d..d0fce6f 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/session/AbstractSession.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/session/AbstractSession.java
@@ -47,8 +47,9 @@ import org.apache.sshd.common.channel.ChannelListener;
import org.apache.sshd.common.cipher.Cipher;
import org.apache.sshd.common.compression.Compression;
import org.apache.sshd.common.digest.Digest;
+import org.apache.sshd.common.future.DefaultKeyExchangeFuture;
import org.apache.sshd.common.future.DefaultSshFuture;
-import org.apache.sshd.common.future.SshFuture;
+import org.apache.sshd.common.future.KeyExchangeFuture;
import org.apache.sshd.common.future.SshFutureListener;
import org.apache.sshd.common.io.IoSession;
import org.apache.sshd.common.io.IoWriteFuture;
@@ -142,9 +143,8 @@ public abstract class AbstractSession extends CloseableUtils.AbstractInnerClosea
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 final AtomicReference<KexState> kexState = new AtomicReference<KexState>(KexState.UNKNOWN);
- @SuppressWarnings("rawtypes")
- protected DefaultSshFuture reexchangeFuture;
+ protected final AtomicReference<KexState> kexState = new AtomicReference<>(KexState.UNKNOWN);
+ protected final AtomicReference<DefaultKeyExchangeFuture> kexFutureHolder = new AtomicReference<>(null);
//
// SSH packets encoding / decoding support
@@ -345,11 +345,27 @@ public abstract class AbstractSession extends CloseableUtils.AbstractInnerClosea
* method returns.
*
* @param buffer the buffer containing the packet
- * @throws Exception if an exeption occurs while handling this packet.
+ * @throws Exception if an exception occurs while handling this packet.
+ * @see #doHandleMessage(Buffer)
*/
protected void handleMessage(Buffer buffer) throws Exception {
- synchronized (lock) {
- doHandleMessage(buffer);
+ try {
+ synchronized (lock) {
+ doHandleMessage(buffer);
+ }
+ } catch (Exception e) {
+ DefaultKeyExchangeFuture kexFuture = kexFutureHolder.get();
+ // if have any ongoing KEX notify it about the failure
+ if (kexFuture != null) {
+ synchronized (kexFuture) {
+ Object value = kexFuture.getValue();
+ if (value == null) {
+ kexFuture.setValue(e);
+ }
+ }
+ }
+
+ throw e;
}
}
@@ -466,9 +482,17 @@ public abstract class AbstractSession extends CloseableUtils.AbstractInnerClosea
log.debug("Received SSH_MSG_NEWKEYS");
validateKexState(cmd, KexState.KEYS);
receiveNewKeys();
- if (reexchangeFuture != null) {
- reexchangeFuture.setValue(Boolean.TRUE);
+
+ DefaultKeyExchangeFuture kexFuture = kexFutureHolder.get();
+ if (kexFuture != null) {
+ synchronized (kexFuture) {
+ Object value = kexFuture.getValue();
+ if (value == null) {
+ kexFuture.setValue(Boolean.TRUE);
+ }
+ }
}
+
sendSessionEvent(SessionListener.Event.KeyEstablished);
synchronized (pendingPackets) {
if (!pendingPackets.isEmpty()) {
@@ -538,6 +562,17 @@ public abstract class AbstractSession extends CloseableUtils.AbstractInnerClosea
@Override
protected void preClose() {
+ DefaultKeyExchangeFuture kexFuture = kexFutureHolder.get();
+ if (kexFuture != null) {
+ // if have any pending KEX then notify it about the closing session
+ synchronized (kexFuture) {
+ Object value = kexFuture.getValue();
+ if (value == null) {
+ kexFuture.setValue(new SshException("Session closing while KEX in progress"));
+ }
+ }
+ }
+
// Fire 'close' event
SessionListener listener = getSessionListenerProxy();
try {
@@ -550,6 +585,8 @@ public abstract class AbstractSession extends CloseableUtils.AbstractInnerClosea
this.sessionListeners.clear();
this.channelListeners.clear();
}
+
+ super.preClose();
}
protected Service[] getServices() {
@@ -1474,14 +1511,23 @@ public abstract class AbstractSession extends CloseableUtils.AbstractInnerClosea
}
@Override
- @SuppressWarnings("rawtypes")
- public SshFuture reExchangeKeys() throws IOException {
+ public KeyExchangeFuture reExchangeKeys() throws IOException {
if (kexState.compareAndSet(KexState.DONE, KexState.INIT)) {
log.info("Initiating key re-exchange");
sendKexInit();
- reexchangeFuture = new DefaultSshFuture(null);
+
+ DefaultKeyExchangeFuture kexFuture = kexFutureHolder.getAndSet(new DefaultKeyExchangeFuture(null));
+ if (kexFuture != null) {
+ synchronized (kexFuture) {
+ Object value = kexFuture.getValue();
+ if (value == null) {
+ kexFuture.setValue(new SshException("New KEX started while previous one still ongoing"));
+ }
+ }
+ }
}
- return reexchangeFuture;
+
+ return ValidateUtils.checkNotNull(kexFutureHolder.get(), "No current KEX future");
}
protected void checkRekey() throws IOException {
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/80a73a28/sshd-core/src/main/java/org/apache/sshd/common/session/Session.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/session/Session.java b/sshd-core/src/main/java/org/apache/sshd/common/session/Session.java
index ec35195..2a6a109 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/session/Session.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/session/Session.java
@@ -25,7 +25,7 @@ import org.apache.sshd.common.Closeable;
import org.apache.sshd.common.FactoryManager;
import org.apache.sshd.common.Service;
import org.apache.sshd.common.channel.ChannelListenerManager;
-import org.apache.sshd.common.future.SshFuture;
+import org.apache.sshd.common.future.KeyExchangeFuture;
import org.apache.sshd.common.io.IoSession;
import org.apache.sshd.common.io.IoWriteFuture;
import org.apache.sshd.common.kex.KexProposalOption;
@@ -184,11 +184,10 @@ public interface Session extends SessionListenerManager, ChannelListenerManager,
/**
* Initiate a new key exchange.
*
- * @return An {@link SshFuture} for awaiting the completion of the exchange
+ * @return An {@link KeyExchangeFuture} for awaiting the completion of the exchange
* @throws IOException If failed to negotiate keys
*/
- @SuppressWarnings("rawtypes")
- SshFuture reExchangeKeys() throws IOException;
+ KeyExchangeFuture reExchangeKeys() throws IOException;
/**
* Get the service of the specified type.
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/80a73a28/sshd-core/src/main/java/org/apache/sshd/server/kex/DHGServer.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/kex/DHGServer.java b/sshd-core/src/main/java/org/apache/sshd/server/kex/DHGServer.java
index d74adbb..29a807d 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/kex/DHGServer.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/kex/DHGServer.java
@@ -141,5 +141,4 @@ public class DHGServer extends AbstractDHServerKeyExchange {
session.writePacket(buffer);
return true;
}
-
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/80a73a28/sshd-core/src/test/java/org/apache/sshd/KeyReExchangeTest.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/test/java/org/apache/sshd/KeyReExchangeTest.java b/sshd-core/src/test/java/org/apache/sshd/KeyReExchangeTest.java
index 26f9645..571357f 100644
--- a/sshd-core/src/test/java/org/apache/sshd/KeyReExchangeTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/KeyReExchangeTest.java
@@ -23,18 +23,29 @@ import java.io.InputStream;
import java.io.OutputStream;
import java.io.PipedInputStream;
import java.io.PipedOutputStream;
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.lang.reflect.Proxy;
import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.List;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
-import com.jcraft.jsch.JSch;
import org.apache.sshd.client.SshClient;
import org.apache.sshd.client.channel.ChannelShell;
import org.apache.sshd.client.channel.ClientChannel;
import org.apache.sshd.client.session.ClientSession;
import org.apache.sshd.common.FactoryManagerUtils;
+import org.apache.sshd.common.NamedFactory;
+import org.apache.sshd.common.cipher.BuiltinCiphers;
+import org.apache.sshd.common.future.KeyExchangeFuture;
+import org.apache.sshd.common.kex.BuiltinDHFactories;
+import org.apache.sshd.common.kex.KeyExchange;
import org.apache.sshd.common.session.Session;
import org.apache.sshd.common.session.SessionListener;
+import org.apache.sshd.common.subsystem.sftp.SftpConstants;
import org.apache.sshd.server.ServerFactoryManager;
import org.apache.sshd.server.SshServer;
import org.apache.sshd.util.BaseTestSupport;
@@ -49,6 +60,8 @@ import org.junit.FixMethodOrder;
import org.junit.Test;
import org.junit.runners.MethodSorters;
+import com.jcraft.jsch.JSch;
+
/**
* Test key exchange algorithms.
*
@@ -60,6 +73,10 @@ public class KeyReExchangeTest extends BaseTestSupport {
private SshServer sshd;
private int port;
+ public KeyReExchangeTest() {
+ super();
+ }
+
@After
public void tearDown() throws Exception {
sshd.stop(true);
@@ -82,11 +99,107 @@ public class KeyReExchangeTest extends BaseTestSupport {
}
@Test
- public void testReExchangeFromClient() throws Exception {
+ public void testSwitchToNoneCipher() throws Exception {
+ setUp(0, 0);
+
+ sshd.getCipherFactories().add(BuiltinCiphers.none);
+ try (SshClient client = SshClient.setUpDefaultClient()) {
+ client.getCipherFactories().add(BuiltinCiphers.none);
+ client.start();
+
+ try (ClientSession session = client.connect(getCurrentTestName(), "localhost", port).verify(7L, TimeUnit.SECONDS).getSession()) {
+ session.addPasswordIdentity(getCurrentTestName());
+ session.auth().verify(5L, TimeUnit.SECONDS);
+
+ KeyExchangeFuture switchFuture = session.switchToNoneCipher();
+ switchFuture.verify(5L, TimeUnit.SECONDS);
+ try (ClientChannel channel = session.createSubsystemChannel(SftpConstants.SFTP_SUBSYSTEM_NAME)) {
+ channel.open().verify(5L, TimeUnit.SECONDS);
+ }
+ } finally {
+ client.stop();
+ }
+ }
+ }
+
+ @Test // see SSHD-558
+ public void testKexFutureExceptionPropagation() throws Exception {
+ setUp(0, 0);
+ sshd.getCipherFactories().add(BuiltinCiphers.none);
+
+ try (SshClient client = SshClient.setUpDefaultClient()) {
+ client.getCipherFactories().add(BuiltinCiphers.none);
+ // replace the original KEX factories with wrapped ones that we can fail intentionally
+ List<NamedFactory<KeyExchange>> kexFactories = new ArrayList<>();
+ final AtomicBoolean successfulInit = new AtomicBoolean(true);
+ final AtomicBoolean successfulNext = new AtomicBoolean(true);
+ final ClassLoader loader = getClass().getClassLoader();
+ final Class<?>[] interfaces = { KeyExchange.class };
+ for (final NamedFactory<KeyExchange> factory : client.getKeyExchangeFactories()) {
+ kexFactories.add(new NamedFactory<KeyExchange>() {
+ @Override
+ public String getName() {
+ return factory.getName();
+ }
+
+ @Override
+ public KeyExchange create() {
+ final KeyExchange proxiedInstance = factory.create();
+ return (KeyExchange) Proxy.newProxyInstance(loader, interfaces, new InvocationHandler() {
+ @Override
+ public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
+ String name = method.getName();
+ if ("init".equals(name) && (!successfulInit.get())) {
+ throw new UnsupportedOperationException("Intentionally failing 'init'");
+ } else if ("next".equals(name) && (!successfulNext.get())) {
+ throw new UnsupportedOperationException("Intentionally failing 'next'");
+ } else {
+ return method.invoke(proxiedInstance, args);
+ }
+ }
+ });
+ }
+ });
+ }
+ client.setKeyExchangeFactories(kexFactories);
+ client.start();
+
+ try {
+ try {
+ testKexFutureExceptionPropagation("init", successfulInit, client);
+ } finally {
+ successfulInit.set(true);
+ }
+
+ try {
+ testKexFutureExceptionPropagation("next", successfulNext, client);
+ } finally {
+ successfulNext.set(true);
+ }
+ } finally {
+ client.stop();
+ }
+ }
+ }
+
+ private void testKexFutureExceptionPropagation(String failureType, AtomicBoolean successFlag, SshClient client) throws Exception {
+ try (ClientSession session = client.connect(getCurrentTestName(), "localhost", port).verify(7L, TimeUnit.SECONDS).getSession()) {
+ session.addPasswordIdentity(getCurrentTestName());
+ session.auth().verify(5L, TimeUnit.SECONDS);
+
+ successFlag.set(false);
+ KeyExchangeFuture kexFuture = session.switchToNoneCipher();
+ assertTrue(failureType + ": failed to complete KEX on time", kexFuture.await(7L, TimeUnit.SECONDS));
+ assertNotNull(failureType + ": unexpected success", kexFuture.getException());
+ }
+ }
+
+ @Test
+ public void testReExchangeFromJschClient() throws Exception {
setUp(0, 0);
JSchLogger.init();
- JSch.setConfig("kex", "diffie-hellman-group-exchange-sha1");
+ JSch.setConfig("kex", BuiltinDHFactories.Constants.DIFFIE_HELLMAN_GROUP_EXCHANGE_SHA1);
JSch sch = new JSch();
com.jcraft.jsch.Session s = sch.getSession(getCurrentTestName(), "localhost", port);
try {
@@ -119,7 +232,7 @@ public class KeyReExchangeTest extends BaseTestSupport {
}
@Test
- public void testReExchangeFromNativeClient() throws Exception {
+ public void testReExchangeFromSshdClient() throws Exception {
setUp(0, 0);
try (SshClient client = SshClient.setUpDefaultClient()) {
@@ -155,7 +268,10 @@ public class KeyReExchangeTest extends BaseTestSupport {
for (int i = 0; i < 10; i++) {
teeOut.write(data);
teeOut.flush();
- session.reExchangeKeys();
+
+ KeyExchangeFuture kexFuture = session.reExchangeKeys();
+ assertTrue("Failed to complete KEX on time at iteration " + i, kexFuture.await(5L, TimeUnit.SECONDS));
+ assertNull("KEX exception signalled at iteration " + 1, kexFuture.getException());
}
teeOut.write("exit\n".getBytes(StandardCharsets.UTF_8));
teeOut.flush();
@@ -224,8 +340,10 @@ public class KeyReExchangeTest extends BaseTestSupport {
// ignored
}
});
+
+ byte[] data = sb.toString().getBytes(StandardCharsets.UTF_8);
for (int i = 0; i < 100; i++) {
- teeOut.write(sb.toString().getBytes(StandardCharsets.UTF_8));
+ teeOut.write(data);
teeOut.flush();
}
teeOut.write("exit\n".getBytes(StandardCharsets.UTF_8));
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/80a73a28/sshd-core/src/test/java/org/apache/sshd/client/ClientTest.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/test/java/org/apache/sshd/client/ClientTest.java b/sshd-core/src/test/java/org/apache/sshd/client/ClientTest.java
index 36cbbe3..7713b2d 100644
--- a/sshd-core/src/test/java/org/apache/sshd/client/ClientTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/client/ClientTest.java
@@ -74,7 +74,6 @@ import org.apache.sshd.common.channel.Channel;
import org.apache.sshd.common.channel.ChannelListener;
import org.apache.sshd.common.channel.ChannelListenerManager;
import org.apache.sshd.common.channel.TestChannelListener;
-import org.apache.sshd.common.cipher.BuiltinCiphers;
import org.apache.sshd.common.future.CloseFuture;
import org.apache.sshd.common.future.SshFutureListener;
import org.apache.sshd.common.io.IoReadFuture;
@@ -254,7 +253,7 @@ public class ClientTest extends BaseTestSupport {
assertObjectInstanceOf("Mismatched failure reason type", ChannelFailureException.class, reason);
String name = ((NamedResource) reason).getName();
- synchronized(failuresSet) {
+ synchronized (failuresSet) {
assertTrue("Re-signalled failure location: " + name, failuresSet.add(name));
}
}
@@ -271,7 +270,7 @@ public class ClientTest extends BaseTestSupport {
private void handleChannelEvent(String name, Channel channel) {
int id = channel.getId();
- synchronized(eventsMap) {
+ synchronized (eventsMap) {
if (eventsMap.put(name, id) != null) {
return; // already generated an exception for this event
}
@@ -300,7 +299,7 @@ public class ClientTest extends BaseTestSupport {
break; // 1st success means all methods have been invoked
}
} catch (IOException e) {
- synchronized(eventsMap) {
+ synchronized (eventsMap) {
eventsMap.remove("Closed"); // since it is called anyway but does not cause an IOException
assertTrue("Unexpected failure at retry #" + retryCount, eventsMap.size() < 3);
}
@@ -1233,24 +1232,6 @@ public class ClientTest extends BaseTestSupport {
}
@Test
- public void testSwitchToNoneCipher() throws Exception {
- sshd.getCipherFactories().add(BuiltinCiphers.none);
- client.getCipherFactories().add(BuiltinCiphers.none);
- client.start();
-
- try (ClientSession session = createTestClientSession()) {
- assertTrue("Failed to switch to NONE cipher on time", session.switchToNoneCipher().await(5L, TimeUnit.SECONDS));
-
- try (ClientChannel channel = session.createSubsystemChannel(SftpConstants.SFTP_SUBSYSTEM_NAME)) {
- channel.open().verify(5L, TimeUnit.SECONDS);
- }
- } finally {
- client.stop();
- }
- assertNull("Session closure not signalled", clientSessionHolder.get());
- }
-
- @Test
public void testCreateChannelByType() throws Exception {
client.start();
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/80a73a28/sshd-core/src/test/java/org/apache/sshd/client/kex/KexTest.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/test/java/org/apache/sshd/client/kex/KexTest.java b/sshd-core/src/test/java/org/apache/sshd/client/kex/KexTest.java
index 7ff8141..5ff1111 100644
--- a/sshd-core/src/test/java/org/apache/sshd/client/kex/KexTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/client/kex/KexTest.java
@@ -24,6 +24,7 @@ import java.io.OutputStream;
import java.io.PipedInputStream;
import java.io.PipedOutputStream;
import java.nio.charset.StandardCharsets;
+import java.util.Collection;
import java.util.Collections;
import java.util.concurrent.TimeUnit;
@@ -33,7 +34,6 @@ import org.apache.sshd.client.channel.ClientChannel;
import org.apache.sshd.client.session.ClientSession;
import org.apache.sshd.common.NamedFactory;
import org.apache.sshd.common.kex.BuiltinDHFactories;
-import org.apache.sshd.common.kex.DHFactory;
import org.apache.sshd.common.kex.KeyExchange;
import org.apache.sshd.server.SshServer;
import org.apache.sshd.util.BaseTestSupport;
@@ -42,10 +42,14 @@ import org.apache.sshd.util.EchoShellFactory;
import org.apache.sshd.util.TeeOutputStream;
import org.apache.sshd.util.Utils;
import org.junit.After;
+import org.junit.Assume;
import org.junit.Before;
import org.junit.FixMethodOrder;
import org.junit.Test;
+import org.junit.runner.RunWith;
import org.junit.runners.MethodSorters;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
/**
* Test client key exchange algorithms.
@@ -53,13 +57,20 @@ import org.junit.runners.MethodSorters;
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
@FixMethodOrder(MethodSorters.NAME_ASCENDING)
+@RunWith(Parameterized.class) // see https://github.com/junit-team/junit/wiki/Parameterized-tests
public class KexTest extends BaseTestSupport {
+ private final BuiltinDHFactories factory;
private SshServer sshd;
private int port;
- public KexTest() {
- super();
+ public KexTest(BuiltinDHFactories factory) {
+ this.factory = factory;
+ }
+
+ @Parameters(name = "Factory={0}")
+ public static Collection<Object[]> parameters() {
+ return parameterize(BuiltinDHFactories.VALUES);
}
@Before
@@ -80,29 +91,8 @@ public class KexTest extends BaseTestSupport {
}
@Test
- public void testClientKeyExchanges() throws Exception {
- Exception err = null;
-
- for (BuiltinDHFactories f : BuiltinDHFactories.VALUES) {
- if (!f.isSupported()) {
- System.out.println("Skip KEX=" + f.getName() + " - unsupported");
- continue;
- }
-
- try {
- testClient(f);
- } catch (Exception e) {
- System.err.println(e.getClass().getSimpleName() + " while test KEX=" + f.getName() + ": " + e.getMessage());
- err = e;
- }
- }
-
- if (err != null) {
- throw err;
- }
- }
-
- private void testClient(DHFactory factory) throws Exception {
+ public void testClientKeyExchange() throws Exception {
+ Assume.assumeTrue(factory.getName() + " not supported", factory.isSupported());
testClient(ClientBuilder.DH2KEX.transform(factory));
}