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 2020/07/22 08:49:39 UTC
[mina-sshd] 01/03: [SSHD-1040] Make server key available after KEX
completed
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 ad2b9b6574c5e3d3d42ef975bdf68af8e9c3b8fc
Author: Lyor Goldstein <lg...@apache.org>
AuthorDate: Wed Jul 22 11:02:41 2020 +0300
[SSHD-1040] Make server key available after KEX completed
---
CHANGES.md | 2 ++
.../sshd/client/ClientAuthenticationManager.java | 2 +-
.../client/kex/AbstractDHClientKeyExchange.java | 18 +++--------
.../java/org/apache/sshd/client/kex/DHGClient.java | 6 ++--
.../org/apache/sshd/client/kex/DHGEXClient.java | 13 +++++---
.../sshd/client/session/AbstractClientSession.java | 17 +++++++++-
.../apache/sshd/client/session/ClientSession.java | 8 +++++
.../org/apache/sshd/common/kex/KeyExchange.java | 8 -----
.../org/apache/sshd/common/session/Session.java | 5 ++-
.../common/session/helpers/AbstractSession.java | 2 +-
.../server/kex/AbstractDHServerKeyExchange.java | 12 --------
.../sshd/common/auth/AuthenticationTest.java | 36 ++++++++++++++++++++++
12 files changed, 86 insertions(+), 43 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index 1f843bc..4e8967d 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -18,6 +18,8 @@
## Minor code helpers
+* [SSHD-1040](https://issues.apache.org/jira/browse/SSHD-1040) Make server key available after KEX completed.
+
## Behavioral changes and enhancements
* [SSHD-1028](https://issues.apache.org/jira/browse/SSHD-1028) Fix SSH_MSG_DISCONNECT: Too many concurrent connections.
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/ClientAuthenticationManager.java b/sshd-core/src/main/java/org/apache/sshd/client/ClientAuthenticationManager.java
index 1965633..b42eae5 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/ClientAuthenticationManager.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/ClientAuthenticationManager.java
@@ -38,7 +38,7 @@ import org.apache.sshd.common.util.ValidateUtils;
/**
* Holds information required for the client to perform authentication with the server
- *
+ *
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public interface ClientAuthenticationManager
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/kex/AbstractDHClientKeyExchange.java b/sshd-core/src/main/java/org/apache/sshd/client/kex/AbstractDHClientKeyExchange.java
index a3a2a54..92f10e8 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/kex/AbstractDHClientKeyExchange.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/kex/AbstractDHClientKeyExchange.java
@@ -19,9 +19,7 @@
package org.apache.sshd.client.kex;
-import java.security.PublicKey;
-
-import org.apache.sshd.client.session.ClientSession;
+import org.apache.sshd.client.session.AbstractClientSession;
import org.apache.sshd.client.session.ClientSessionHolder;
import org.apache.sshd.common.kex.dh.AbstractDHKeyExchange;
import org.apache.sshd.common.session.Session;
@@ -33,20 +31,14 @@ import org.apache.sshd.common.util.ValidateUtils;
public abstract class AbstractDHClientKeyExchange
extends AbstractDHKeyExchange
implements ClientSessionHolder {
- protected PublicKey serverKey;
protected AbstractDHClientKeyExchange(Session session) {
- super(ValidateUtils.checkInstanceOf(session, ClientSession.class,
- "Using a client side KeyExchange on a server: %s", session));
- }
-
- @Override
- public final ClientSession getClientSession() {
- return (ClientSession) getSession();
+ super(ValidateUtils.checkInstanceOf(session, AbstractClientSession.class,
+ "Non-AbstractClientSession: %s", session));
}
@Override
- public PublicKey getServerKey() {
- return serverKey;
+ public final AbstractClientSession getClientSession() {
+ return (AbstractClientSession) getSession();
}
}
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 23c7b7e..559206b 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
@@ -25,6 +25,7 @@ import java.util.Collection;
import java.util.Objects;
import java.util.concurrent.TimeUnit;
+import org.apache.sshd.client.session.AbstractClientSession;
import org.apache.sshd.common.NamedFactory;
import org.apache.sshd.common.SshConstants;
import org.apache.sshd.common.SshException;
@@ -113,7 +114,7 @@ public class DHGClient extends AbstractDHClientKeyExchange {
@Override
@SuppressWarnings("checkstyle:VariableDeclarationUsageDistance")
public boolean next(int cmd, Buffer buffer) throws Exception {
- Session session = getSession();
+ AbstractClientSession session = getClientSession();
if (log.isDebugEnabled()) {
log.debug("next({})[{}] process command={}",
this, session, KeyExchange.getSimpleKexOpcodeName(cmd));
@@ -132,7 +133,7 @@ public class DHGClient extends AbstractDHClientKeyExchange {
k = dh.getK();
buffer = new ByteArrayBuffer(k_s);
- serverKey = buffer.getRawPublicKey();
+ PublicKey serverKey = buffer.getRawPublicKey();
PublicKey serverPublicHostKey = serverKey;
if (serverKey instanceof OpenSshCertificate) {
@@ -182,6 +183,7 @@ public class DHGClient extends AbstractDHClientKeyExchange {
"KeyExchange signature verification failed for key type=" + keyAlg);
}
+ session.setServerKey(serverKey);
return true;
}
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 166fd99..d02d7e6 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
@@ -20,8 +20,10 @@
package org.apache.sshd.client.kex;
import java.math.BigInteger;
+import java.security.PublicKey;
import java.util.Objects;
+import org.apache.sshd.client.session.AbstractClientSession;
import org.apache.sshd.common.NamedFactory;
import org.apache.sshd.common.SshConstants;
import org.apache.sshd.common.SshException;
@@ -58,8 +60,10 @@ public class DHGEXClient extends AbstractDHClientKeyExchange {
this.factory = Objects.requireNonNull(factory, "No factory");
// SSHD-941 give the user a chance to intervene in the choice
- min = CoreModuleProperties.PROP_DHGEX_CLIENT_MIN_KEY.get(session).orElse(SecurityUtils.MIN_DHGEX_KEY_SIZE);
- max = CoreModuleProperties.PROP_DHGEX_CLIENT_MAX_KEY.get(session).orElse(SecurityUtils.getMaxDHGroupExchangeKeySize());
+ min = CoreModuleProperties.PROP_DHGEX_CLIENT_MIN_KEY.get(session)
+ .orElse(SecurityUtils.MIN_DHGEX_KEY_SIZE);
+ max = CoreModuleProperties.PROP_DHGEX_CLIENT_MAX_KEY.get(session)
+ .orElse(SecurityUtils.getMaxDHGroupExchangeKeySize());
prf = CoreModuleProperties.PROP_DHGEX_CLIENT_PRF_KEY.get(session)
.orElse(Math.min(SecurityUtils.PREFERRED_DHGEX_KEY_SIZE, max));
}
@@ -117,7 +121,7 @@ public class DHGEXClient extends AbstractDHClientKeyExchange {
@Override
@SuppressWarnings("checkstyle:VariableDeclarationUsageDistance")
public boolean next(int cmd, Buffer buffer) throws Exception {
- Session session = getSession();
+ AbstractClientSession session = getClientSession();
boolean debugEnabled = log.isDebugEnabled();
if (debugEnabled) {
log.debug("next({})[{}] process command={} (expected={})",
@@ -166,7 +170,7 @@ public class DHGEXClient extends AbstractDHClientKeyExchange {
k = dh.getK();
buffer = new ByteArrayBuffer(k_s);
- serverKey = buffer.getRawPublicKey();
+ PublicKey serverKey = buffer.getRawPublicKey();
String keyAlg = KeyUtils.getKeyType(serverKey);
if (GenericUtils.isEmpty(keyAlg)) {
@@ -202,6 +206,7 @@ public class DHGEXClient extends AbstractDHClientKeyExchange {
SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
"KeyExchange signature verification failed for key type=" + keyAlg);
}
+ session.setServerKey(serverKey);
return true;
}
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 4a0312e..5421ee6 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
@@ -86,6 +86,7 @@ public abstract class AbstractClientSession extends AbstractSession implements C
private final AuthenticationIdentitiesProvider identitiesProvider;
private final AttributeRepository connectionContext;
+ private PublicKey serverKey;
private ServerKeyVerifier serverKeyVerifier;
private UserInteraction userInteraction;
private PasswordIdentityProvider passwordIdentityProvider;
@@ -124,6 +125,20 @@ public abstract class AbstractClientSession extends AbstractSession implements C
}
@Override
+ public PublicKey getServerKey() {
+ return serverKey;
+ }
+
+ public void setServerKey(PublicKey serverKey) {
+ if (log.isDebugEnabled()) {
+ log.debug("setServerKey({}) keyType={}, digest={}",
+ this, KeyUtils.getKeyType(serverKey), KeyUtils.getFingerPrint(serverKey));
+ }
+
+ this.serverKey = serverKey;
+ }
+
+ @Override
public ServerKeyVerifier getServerKeyVerifier() {
ClientFactoryManager manager = getFactoryManager();
return resolveEffectiveProvider(ServerKeyVerifier.class, serverKeyVerifier, manager.getServerKeyVerifier());
@@ -544,7 +559,7 @@ public abstract class AbstractClientSession extends AbstractSession implements C
ServerKeyVerifier serverKeyVerifier = Objects.requireNonNull(getServerKeyVerifier(), "No server key verifier");
IoSession networkSession = getIoSession();
SocketAddress remoteAddress = networkSession.getRemoteAddress();
- PublicKey serverKey = kex.getServerKey();
+ PublicKey serverKey = Objects.requireNonNull(getServerKey(), "No server key to verify");
boolean verified = false;
if (serverKey instanceof OpenSshCertificate) {
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 b7d21fa..86a8ccb 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
@@ -28,6 +28,7 @@ import java.nio.charset.StandardCharsets;
import java.rmi.RemoteException;
import java.rmi.ServerException;
import java.security.KeyPair;
+import java.security.PublicKey;
import java.time.Duration;
import java.util.Collection;
import java.util.Collections;
@@ -122,6 +123,13 @@ public interface ClientSession
AuthFuture auth() throws IOException;
/**
+ * Retrieves the server's key
+ *
+ * @return The server's {@link PublicKey} - {@code null} if KEX not started or not completed
+ */
+ PublicKey getServerKey();
+
+ /**
* Create a channel of the given type. Same as calling <code>createChannel(type, null)</code>.
*
* @param type The channel type
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/kex/KeyExchange.java b/sshd-core/src/main/java/org/apache/sshd/common/kex/KeyExchange.java
index 754b01f..a0ad22e 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/kex/KeyExchange.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/kex/KeyExchange.java
@@ -18,7 +18,6 @@
*/
package org.apache.sshd.common.kex;
-import java.security.PublicKey;
import java.util.Collections;
import java.util.NavigableMap;
@@ -85,13 +84,6 @@ public interface KeyExchange extends NamedResource, SessionHolder<Session> {
*/
byte[] getK();
- /**
- * Retrieves the server's key
- *
- * @return The server's {@link PublicKey}
- */
- PublicKey getServerKey();
-
static String getGroupKexOpcodeName(int cmd) {
String name = GROUP_KEX_OPCODES_MAP.get(cmd);
if (GenericUtils.isEmpty(name)) {
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 46a3fc4..047dac7 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
@@ -88,7 +88,7 @@ public interface Session
/**
* Prepare a new "clean" buffer while reserving the needed space (5 bytes) for the packet header.
- *
+ *
* @param cmd The SSH command to initialize the buffer with
* @param buffer The {@link Buffer} instance to initialize
* @return The initialized buffer
@@ -300,6 +300,9 @@ public interface Session
void setAuthenticated() throws IOException;
+ /**
+ * @return The current {@link KeyExchange} in progress - {@code null} if KEX not started or successfully completed
+ */
KeyExchange getKex();
/**
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 9583773..8e2655e 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
@@ -184,7 +184,7 @@ public abstract class AbstractSession extends SessionHelper {
protected final AtomicLong outBytesCount = new AtomicLong(0L);
protected final AtomicLong inBlocksCount = new AtomicLong(0L);
protected final AtomicLong outBlocksCount = new AtomicLong(0L);
- protected final AtomicReference<Instant> lastKeyTimeValue = new AtomicReference(Instant.now());
+ protected final AtomicReference<Instant> lastKeyTimeValue = new AtomicReference<>(Instant.now());
// we initialize them here in case super constructor calls some methods that use these values
protected long maxRekyPackets;
protected long maxRekeyBytes;
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/kex/AbstractDHServerKeyExchange.java b/sshd-core/src/main/java/org/apache/sshd/server/kex/AbstractDHServerKeyExchange.java
index 0cc375b..a27b3f1 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/kex/AbstractDHServerKeyExchange.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/kex/AbstractDHServerKeyExchange.java
@@ -19,10 +19,6 @@
package org.apache.sshd.server.kex;
-import java.security.KeyPair;
-import java.security.PublicKey;
-import java.util.Objects;
-
import org.apache.sshd.common.kex.dh.AbstractDHKeyExchange;
import org.apache.sshd.common.session.Session;
import org.apache.sshd.common.util.ValidateUtils;
@@ -44,12 +40,4 @@ public abstract class AbstractDHServerKeyExchange
public final ServerSession getServerSession() {
return (ServerSession) getSession();
}
-
- @Override
- public PublicKey getServerKey() {
- ServerSession session = getServerSession();
- KeyPair kpHost = Objects.requireNonNull(
- session.getHostKey(), "No server key pair available");
- return kpHost.getPublic();
- }
}
diff --git a/sshd-core/src/test/java/org/apache/sshd/common/auth/AuthenticationTest.java b/sshd-core/src/test/java/org/apache/sshd/common/auth/AuthenticationTest.java
index e57f17d..04149dd 100644
--- a/sshd-core/src/test/java/org/apache/sshd/common/auth/AuthenticationTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/common/auth/AuthenticationTest.java
@@ -51,6 +51,7 @@ import org.apache.sshd.common.config.keys.FilePasswordProvider;
import org.apache.sshd.common.config.keys.KeyUtils;
import org.apache.sshd.common.io.IoSession;
import org.apache.sshd.common.io.IoWriteFuture;
+import org.apache.sshd.common.kex.KeyExchange;
import org.apache.sshd.common.keyprovider.KeyIdentityProvider;
import org.apache.sshd.common.keyprovider.KeyPairProvider;
import org.apache.sshd.common.session.Session;
@@ -944,6 +945,41 @@ public class AuthenticationTest extends BaseTestSupport {
}
}
+ @Test // see SSHD-1040
+ public void testServerKeyAvailableAfterAuth() throws Exception {
+ KeyPairProvider keyPairProvider = sshd.getKeyPairProvider();
+ Iterable<KeyPair> availableKeys = keyPairProvider.loadKeys(null);
+ PublicKey actualKey = null;
+
+ try (SshClient client = setupTestClient()) {
+ client.start();
+
+ try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
+ .verify(CONNECT_TIMEOUT).getSession()) {
+ session.addPasswordIdentity(getCurrentTestName());
+ session.auth().verify(AUTH_TIMEOUT);
+
+ KeyExchange kex = session.getKex();
+ assertNull("KEX no nullified after completion", kex);
+
+ actualKey = session.getServerKey();
+ } finally {
+ client.stop();
+ }
+ }
+
+ assertNotNull("No server key extracted", actualKey);
+
+ for (KeyPair kp : availableKeys) {
+ PublicKey expectedKey = kp.getPublic();
+ if (KeyUtils.compareKeys(expectedKey, actualKey)) {
+ return;
+ }
+ }
+
+ fail("No matching server key found for " + actualKey);
+ }
+
private static void assertAuthenticationResult(String message, AuthFuture future, boolean expected) throws IOException {
assertTrue(message + ": failed to get result on time", future.await(AUTH_TIMEOUT));
assertEquals(message + ": mismatched authentication result", expected, future.isSuccess());