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 2019/09/23 17:21:23 UTC
[mina-sshd] branch master updated (11b33de -> 6eb232f)
This is an automated email from the ASF dual-hosted git repository.
lgoldstein pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git.
from 11b33de [SSHD-942] Added ChannelIdTrackingUnknownChannelReferenceHandler
new 6e63c9d [SSHD-941] Lazy-load and cache internal moduli used in Diffie-Helman group key exchange
new 6eb232f [SSHD-941] Allow user to override min./max. key sizes for a specific session Diffi-Helman group exchange via session properties
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
CHANGES.md | 13 ++-
README.md | 1 +
.../java/org/apache/sshd/client/kex/DHGClient.java | 11 ++-
.../org/apache/sshd/client/kex/DHGEXClient.java | 31 ++++++-
.../org/apache/sshd/server/kex/DHGEXServer.java | 96 ++++++++++++++--------
.../java/org/apache/sshd/server/kex/DHGServer.java | 3 +-
.../java/org/apache/sshd/server/kex/Moduli.java | 51 +++++++++++-
.../org/apache/sshd/server/kex/ModuliTest.java | 52 +++++++-----
8 files changed, 191 insertions(+), 67 deletions(-)
copy sshd-common/src/test/java/org/apache/sshd/common/channel/PtyModeTest.java => sshd-core/src/test/java/org/apache/sshd/server/kex/ModuliTest.java (51%)
[mina-sshd] 01/02: [SSHD-941] Lazy-load and cache internal moduli
used in Diffie-Helman group key exchange
Posted by lg...@apache.org.
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 6e63c9d0d42a33fb98a82d4b5dac66da08398db3
Author: Lyor Goldstein <lg...@apache.org>
AuthorDate: Mon Sep 23 09:12:49 2019 +0300
[SSHD-941] Lazy-load and cache internal moduli used in Diffie-Helman group key exchange
---
CHANGES.md | 3 +
README.md | 1 +
.../org/apache/sshd/client/kex/DHGEXClient.java | 18 ++++-
.../org/apache/sshd/server/kex/DHGEXServer.java | 87 ++++++++++++++--------
.../java/org/apache/sshd/server/kex/Moduli.java | 51 ++++++++++++-
.../org/apache/sshd/server/kex/ModuliTest.java | 70 +++++++++++++++++
6 files changed, 192 insertions(+), 38 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index 1de1064..70c4c90 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -40,6 +40,9 @@ peer version data is received.
by tracking the initialized channels identifiers and being lenient only if command is received for a channel that was
initialized in the past.
+* The internal moduli used in Diffie-Hellman group exchange are **cached** - lazy-loaded the 1st time such an exchange
+occurs. The cache can be invalidated (and thus force a re-load) by invoking `Moduli#clearInternalModuliCache`.
+
## Behavioral changes and enhancements
* [SSHD-926](https://issues.apache.org/jira/browse/SSHD-930) - Add support for OpenSSH 'lsetstat@openssh.com' SFTP protocol extension.
diff --git a/README.md b/README.md
index 49b4e38..ac35e7b 100644
--- a/README.md
+++ b/README.md
@@ -17,6 +17,7 @@ based applications requiring SSH support.
* [RFC 4335 - The Secure Shell (SSH) Session Channel Break Extension](https://tools.ietf.org/html/rfc4335)
* [RFC 4344 - The Secure Shell (SSH) Transport Layer Encryption Modes](https://tools.ietf.org/html/rfc4344)
* [RFC 4345 - Improved Arcfour Modes for the Secure Shell (SSH) Transport Layer Protocol](https://tools.ietf.org/html/rfc4345)
+* [RFC 4419 - Diffie-Hellman Group Exchange for the Secure Shell (SSH) Transport Layer Protocol](https://tools.ietf.org/html/rfc4419)
* [RFC 4716 - The Secure Shell (SSH) Public Key File Format](https://tools.ietf.org/html/rfc4716)
* [RFC 5480 - Elliptic Curve Cryptography Subject Public Key Information](https://tools.ietf.org/html/rfc5480)
* [RFC 6668 - SHA-2 Data Integrity Verification for the Secure Shell (SSH) Transport Layer Protocol](https://tools.ietf.org/html/rfc6668)
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 597079b..ced2125 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
@@ -87,8 +87,10 @@ public class DHGEXClient extends AbstractDHClientKeyExchange {
public void init(Session s, byte[] v_s, byte[] v_c, byte[] i_s, byte[] i_c) throws Exception {
super.init(s, v_s, v_c, i_s, i_c);
if (log.isDebugEnabled()) {
- log.debug("init({}) Send SSH_MSG_KEX_DH_GEX_REQUEST", s);
+ log.debug("init({})[{}] Send SSH_MSG_KEX_DH_GEX_REQUEST - min={}, prf={}, max={}",
+ this, s, min, prf, max);
}
+
Buffer buffer = s.createBuffer(SshConstants.SSH_MSG_KEX_DH_GEX_REQUEST, Integer.SIZE);
buffer.putInt(min);
buffer.putInt(prf);
@@ -104,8 +106,9 @@ public class DHGEXClient extends AbstractDHClientKeyExchange {
Session session = getSession();
boolean debugEnabled = log.isDebugEnabled();
if (debugEnabled) {
- log.debug("next({})[{}] process command={}",
- this, session, KeyExchange.getGroupKexOpcodeName(cmd));
+ log.debug("next({})[{}] process command={} (expected={})",
+ this, session, KeyExchange.getGroupKexOpcodeName(cmd),
+ KeyExchange.getGroupKexOpcodeName(expected));
}
if (cmd != expected) {
@@ -126,6 +129,7 @@ public class DHGEXClient extends AbstractDHClientKeyExchange {
if (debugEnabled) {
log.debug("next({})[{}] Send SSH_MSG_KEX_DH_GEX_INIT", this, session);
}
+
buffer = session.createBuffer(
SshConstants.SSH_MSG_KEX_DH_GEX_INIT, e.length + Byte.SIZE);
buffer.putMPInt(e);
@@ -135,6 +139,11 @@ public class DHGEXClient extends AbstractDHClientKeyExchange {
}
if (cmd == SshConstants.SSH_MSG_KEX_DH_GEX_REPLY) {
+ if (debugEnabled) {
+ log.debug("next({})[{}] validate SSH_MSG_KEX_DH_GEX_REPLY - min={}, prf={}, max={}",
+ this, session, min, prf, max);
+ }
+
byte[] k_s = buffer.getBytes();
f = buffer.getMPIntAsBytes();
byte[] sig = buffer.getBytes();
@@ -146,7 +155,8 @@ public class DHGEXClient extends AbstractDHClientKeyExchange {
String keyAlg = KeyUtils.getKeyType(serverKey);
if (GenericUtils.isEmpty(keyAlg)) {
- throw new SshException("Unsupported server key type");
+ throw new SshException("Unsupported server key type: " + serverKey.getAlgorithm()
+ + " [" + serverKey.getFormat() + "]");
}
buffer = new ByteArrayBuffer();
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/kex/DHGEXServer.java b/sshd-core/src/main/java/org/apache/sshd/server/kex/DHGEXServer.java
index 914c5b0..55d791f 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/kex/DHGEXServer.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/kex/DHGEXServer.java
@@ -55,7 +55,6 @@ import org.apache.sshd.server.session.ServerSession;
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public class DHGEXServer extends AbstractDHServerKeyExchange {
-
protected final DHFactory factory;
protected DHG dh;
protected int min;
@@ -73,7 +72,7 @@ public class DHGEXServer extends AbstractDHServerKeyExchange {
return factory.getName();
}
- public static KeyExchangeFactory newFactory(final DHFactory factory) {
+ public static KeyExchangeFactory newFactory(DHFactory factory) {
return new KeyExchangeFactory() {
@Override
public KeyExchange create() {
@@ -105,10 +104,13 @@ public class DHGEXServer extends AbstractDHServerKeyExchange {
ServerSession session = getServerSession();
boolean debugEnabled = log.isDebugEnabled();
if (debugEnabled) {
- log.debug("next({})[{}] process command={}", this, session, KeyExchange.getGroupKexOpcodeName(cmd));
+ log.debug("next({})[{}] process command={} (expected={})",
+ this, session, KeyExchange.getGroupKexOpcodeName(cmd),
+ KeyExchange.getGroupKexOpcodeName(expected));
}
- if (cmd == SshConstants.SSH_MSG_KEX_DH_GEX_REQUEST_OLD && expected == SshConstants.SSH_MSG_KEX_DH_GEX_REQUEST) {
+ if ((cmd == SshConstants.SSH_MSG_KEX_DH_GEX_REQUEST_OLD)
+ && (expected == SshConstants.SSH_MSG_KEX_DH_GEX_REQUEST)) {
oldRequest = true;
min = SecurityUtils.MIN_DHGEX_KEY_SIZE;
prf = buffer.getInt();
@@ -116,15 +118,17 @@ public class DHGEXServer extends AbstractDHServerKeyExchange {
if ((max < min) || (prf < min) || (max < prf)) {
throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
- "Protocol error: bad parameters " + min + " !< " + prf + " !< " + max);
+ "Protocol error: bad parameters " + min + " !< " + prf + " !< " + max);
}
+
dh = chooseDH(min, prf, max);
f = dh.getE();
hash = dh.getHash();
hash.init();
if (debugEnabled) {
- log.debug("next({})[{}] send SSH_MSG_KEX_DH_GEX_GROUP", this, session);
+ log.debug("next({})[{}] send (old request) SSH_MSG_KEX_DH_GEX_GROUP - min={}, prf={}, max={}",
+ this, session, min, prf, max);
}
buffer = session.createBuffer(SshConstants.SSH_MSG_KEX_DH_GEX_GROUP);
@@ -136,21 +140,25 @@ public class DHGEXServer extends AbstractDHServerKeyExchange {
return false;
}
- if (cmd == SshConstants.SSH_MSG_KEX_DH_GEX_REQUEST && expected == SshConstants.SSH_MSG_KEX_DH_GEX_REQUEST) {
+ if ((cmd == SshConstants.SSH_MSG_KEX_DH_GEX_REQUEST)
+ && (expected == SshConstants.SSH_MSG_KEX_DH_GEX_REQUEST)) {
min = buffer.getInt();
prf = buffer.getInt();
max = buffer.getInt();
+
if ((prf < min) || (max < prf)) {
throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
- "Protocol error: bad parameters " + min + " !< " + prf + " !< " + max);
+ "Protocol error: bad parameters " + min + " !< " + prf + " !< " + max);
}
+
dh = chooseDH(min, prf, max);
f = dh.getE();
hash = dh.getHash();
hash.init();
if (debugEnabled) {
- log.debug("next({})[{}] Send SSH_MSG_KEX_DH_GEX_GROUP", this, session);
+ log.debug("next({})[{}] Send SSH_MSG_KEX_DH_GEX_GROUP - min={}, prf={}, max={}",
+ this, session, min, prf, max);
}
buffer = session.createBuffer(SshConstants.SSH_MSG_KEX_DH_GEX_GROUP);
buffer.putMPInt(dh.getP());
@@ -163,8 +171,8 @@ public class DHGEXServer extends AbstractDHServerKeyExchange {
if (cmd != expected) {
throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
- "Protocol error: expected packet " + KeyExchange.getGroupKexOpcodeName(expected)
- + ", got " + KeyExchange.getGroupKexOpcodeName(cmd));
+ "Protocol error: expected packet " + KeyExchange.getGroupKexOpcodeName(expected)
+ + ", got " + KeyExchange.getGroupKexOpcodeName(cmd));
}
if (cmd == SshConstants.SSH_MSG_KEX_DH_GEX_INIT) {
@@ -190,6 +198,7 @@ public class DHGEXServer extends AbstractDHServerKeyExchange {
buffer.putBytes(i_c);
buffer.putBytes(i_s);
buffer.putBytes(k_s);
+
if (oldRequest) {
buffer.putInt(prf);
} else {
@@ -197,6 +206,7 @@ public class DHGEXServer extends AbstractDHServerKeyExchange {
buffer.putInt(prf);
buffer.putInt(max);
}
+
buffer.putMPInt(dh.getP());
buffer.putMPInt(dh.getG());
buffer.putMPInt(e);
@@ -220,10 +230,12 @@ public class DHGEXServer extends AbstractDHServerKeyExchange {
// Send response
if (debugEnabled) {
- log.debug("next({})[{}] Send SSH_MSG_KEX_DH_GEX_REPLY", this, session);
+ log.debug("next({})[{}] Send SSH_MSG_KEX_DH_GEX_REPLY - old={}, min={}, prf={}, max={}",
+ this, session, oldRequest, min, prf, max);
}
- buffer = session.prepareBuffer(SshConstants.SSH_MSG_KEX_DH_GEX_REPLY, BufferUtils.clear(buffer));
+ buffer = session.prepareBuffer(
+ SshConstants.SSH_MSG_KEX_DH_GEX_REPLY, BufferUtils.clear(buffer));
buffer.putBytes(k_s);
buffer.putBytes(f);
buffer.putBytes(sigH);
@@ -235,20 +247,23 @@ public class DHGEXServer extends AbstractDHServerKeyExchange {
}
protected DHG chooseDH(int min, int prf, int max) throws Exception {
- List<Moduli.DhGroup> groups = loadModuliGroups();
-
+ int maxDHGroupExchangeKeySize = SecurityUtils.getMaxDHGroupExchangeKeySize();
min = Math.max(min, SecurityUtils.MIN_DHGEX_KEY_SIZE);
prf = Math.max(prf, SecurityUtils.MIN_DHGEX_KEY_SIZE);
- prf = Math.min(prf, SecurityUtils.getMaxDHGroupExchangeKeySize());
- max = Math.min(max, SecurityUtils.getMaxDHGroupExchangeKeySize());
- int bestSize = 0;
+ prf = Math.min(prf, maxDHGroupExchangeKeySize);
+ max = Math.min(max, maxDHGroupExchangeKeySize);
+
+ List<Moduli.DhGroup> groups = loadModuliGroups();
+ Session session = getServerSession();
List<Moduli.DhGroup> selected = new ArrayList<>();
+ int bestSize = 0;
boolean traceEnabled = log.isTraceEnabled();
for (Moduli.DhGroup group : groups) {
int size = group.getSize();
if ((size < min) || (size > max)) {
if (traceEnabled) {
- log.trace("chooseDH - skip group={} - size not in range [{}-{}]", group, min, max);
+ log.trace("chooseDH({})[{}] - skip group={} - size not in range [{}-{}]",
+ this, session, group, min, max);
}
continue;
}
@@ -256,22 +271,24 @@ public class DHGEXServer extends AbstractDHServerKeyExchange {
if (((size > prf) && (size < bestSize)) || ((size > bestSize) && (bestSize < prf))) {
bestSize = size;
if (traceEnabled) {
- log.trace("chooseDH(prf={}, min={}, max={}) new best size={} from group={}", prf, min, max, bestSize, group);
+ log.trace("chooseDH({})[{}][prf={}, min={}, max={}] new best size={} from group={}",
+ this, session, prf, min, max, bestSize, group);
}
selected.clear();
}
if (size == bestSize) {
if (traceEnabled) {
- log.trace("chooseDH(prf={}, min={}, max={}) selected {}", prf, min, max, group);
+ log.trace("chooseDH({})[{}][prf={}, min={}, max={}] selected {}",
+ this, session, prf, min, max, group);
}
selected.add(group);
}
}
- ServerSession session = getServerSession();
if (selected.isEmpty()) {
- log.warn("chooseDH({})[{}] No suitable primes found, defaulting to DHG1", this, session);
+ log.warn("chooseDH({})[{}][prf={}, min={}, max={}] No suitable primes found, defaulting to DHG1",
+ this, session, prf, min, max);
return getDH(new BigInteger(DHGroupData.getP1()), new BigInteger(DHGroupData.getG()));
}
@@ -280,42 +297,48 @@ public class DHGEXServer extends AbstractDHServerKeyExchange {
Random random = Objects.requireNonNull(factory.create(), "No random generator");
int which = random.random(selected.size());
Moduli.DhGroup group = selected.get(which);
+ if (traceEnabled) {
+ log.trace("chooseDH({})[{}][prf={}, min={}, max={}] selected {}",
+ this, session, prf, min, max, group);
+ }
return getDH(group.getP(), group.getG());
}
protected List<Moduli.DhGroup> loadModuliGroups() throws IOException {
- ServerSession session = getServerSession();
+ Session session = getServerSession();
String moduliStr = session.getString(ServerFactoryManager.MODULI_URL);
List<Moduli.DhGroup> groups = null;
- URL moduli;
if (!GenericUtils.isEmpty(moduliStr)) {
try {
- moduli = new URL(moduliStr);
+ URL moduli = new URL(moduliStr);
groups = Moduli.parseModuli(moduli);
} catch (IOException e) { // OK - use internal moduli
- log.warn("Error (" + e.getClass().getSimpleName() + ") loading external moduli from " + moduliStr + ": " + e.getMessage());
+ log.warn("loadModuliGroups({})[{}] Error ({}) loading external moduli from {}: {}",
+ this, session, e.getClass().getSimpleName(), moduliStr, e.getMessage());
}
}
if (groups == null) {
- moduliStr = "/org/apache/sshd/moduli";
+ moduliStr = Moduli.INTERNAL_MODULI_RESPATH;
try {
- moduli = getClass().getResource(moduliStr);
+ URL moduli = getClass().getResource(moduliStr);
if (moduli == null) {
throw new FileNotFoundException("Missing internal moduli file");
}
moduliStr = moduli.toExternalForm();
- groups = Moduli.parseModuli(moduli);
+ groups = Moduli.loadInternalModuli(moduli);
} catch (IOException e) {
- log.warn("Error (" + e.getClass().getSimpleName() + ") loading internal moduli from " + moduliStr + ": " + e.getMessage());
+ log.warn("loadModuliGroups({})[{}] Error ({}) loading internal moduli from {}: {}",
+ this, session, e.getClass().getSimpleName(), moduliStr, e.getMessage());
throw e; // this time we MUST throw the exception
}
}
if (log.isDebugEnabled()) {
- log.debug("Loaded moduli groups from {}", moduliStr);
+ log.debug("loadModuliGroups({})[{}] Loaded moduli groups from {}",
+ this, session, moduliStr);
}
return groups;
}
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/kex/Moduli.java b/sshd-core/src/main/java/org/apache/sshd/server/kex/Moduli.java
index adb7098..481b697 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/kex/Moduli.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/kex/Moduli.java
@@ -19,14 +19,21 @@
package org.apache.sshd.server.kex;
import java.io.BufferedReader;
+import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStreamReader;
import java.math.BigInteger;
import java.net.URL;
import java.nio.charset.StandardCharsets;
+import java.util.AbstractMap.SimpleImmutableEntry;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.List;
+import java.util.Map;
import java.util.Objects;
+import java.util.concurrent.atomic.AtomicReference;
+
+import org.apache.sshd.common.util.GenericUtils;
/**
* Helper class to load DH group primes from a file.
@@ -34,6 +41,10 @@ import java.util.Objects;
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public final class Moduli {
+ /**
+ * Resource path of internal moduli file
+ */
+ public static final String INTERNAL_MODULI_RESPATH = "/org/apache/sshd/moduli";
public static final int MODULI_TYPE_SAFE = 2;
public static final int MODULI_TESTS_COMPOSITE = 0x01;
@@ -63,20 +74,53 @@ public final class Moduli {
@Override
public String toString() {
- return "[size=" + getSize() + ",G=" + getG() + ",P=" + getP() + "]";
+ return "[size=" + getSize() + ", G=" + getG() + ", P=" + getP() + "]";
}
}
+ private static final AtomicReference<Map.Entry<String, List<DhGroup>>> INTERNAL_MODULI_HOLDER = new AtomicReference<>();
+
// Private constructor
private Moduli() {
throw new UnsupportedOperationException("No instance allowed");
}
+ public static Map.Entry<String, List<DhGroup>> clearInternalModuliCache() {
+ return INTERNAL_MODULI_HOLDER.getAndSet(null);
+ }
+
+ public static List<DhGroup> loadInternalModuli(URL url) throws IOException {
+ if (url == null) {
+ throw new FileNotFoundException("No internal moduli resource specified");
+ }
+
+ String moduliStr = url.toExternalForm();
+ Map.Entry<String, List<DhGroup>> lastModuli = INTERNAL_MODULI_HOLDER.get();
+ String lastResource = (lastModuli == null) ? null : lastModuli.getKey();
+ if (Objects.equals(lastResource, moduliStr)) {
+ return lastModuli.getValue();
+ }
+
+ List<DhGroup> groups = parseModuli(url);
+ if (GenericUtils.isEmpty(groups)) {
+ groups = Collections.emptyList();
+ } else {
+ groups = Collections.unmodifiableList(groups);
+ }
+
+ INTERNAL_MODULI_HOLDER.set(new SimpleImmutableEntry<>(moduliStr, groups));
+ return groups;
+ }
+
public static List<DhGroup> parseModuli(URL url) throws IOException {
List<DhGroup> groups = new ArrayList<>();
try (BufferedReader r = new BufferedReader(new InputStreamReader(url.openStream(), StandardCharsets.UTF_8))) {
for (String line = r.readLine(); line != null; line = r.readLine()) {
line = line.trim();
+ if (line.isEmpty()) {
+ continue;
+ }
+
if (line.startsWith("#")) {
continue;
}
@@ -105,7 +149,10 @@ public final class Moduli {
continue;
}
- DhGroup group = new DhGroup(Integer.parseInt(parts[4]) + 1, new BigInteger(parts[5], 16), new BigInteger(parts[6], 16));
+ DhGroup group = new DhGroup(
+ Integer.parseInt(parts[4]) + 1,
+ new BigInteger(parts[5], 16),
+ new BigInteger(parts[6], 16));
groups.add(group);
}
diff --git a/sshd-core/src/test/java/org/apache/sshd/server/kex/ModuliTest.java b/sshd-core/src/test/java/org/apache/sshd/server/kex/ModuliTest.java
new file mode 100644
index 0000000..ef37662
--- /dev/null
+++ b/sshd-core/src/test/java/org/apache/sshd/server/kex/ModuliTest.java
@@ -0,0 +1,70 @@
+/*
+ * 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.server.kex;
+
+import java.io.IOException;
+import java.net.URL;
+import java.util.List;
+
+import org.apache.sshd.common.util.GenericUtils;
+import org.apache.sshd.server.kex.Moduli.DhGroup;
+import org.apache.sshd.util.test.JUnitTestSupport;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.FixMethodOrder;
+import org.junit.Test;
+import org.junit.runners.MethodSorters;
+
+/**
+ * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ */
+@FixMethodOrder(MethodSorters.NAME_ASCENDING)
+public class ModuliTest extends JUnitTestSupport {
+ public ModuliTest() {
+ super();
+ }
+
+ @BeforeClass
+ @AfterClass
+ public static void clearInternalModuliCache() {
+ Moduli.clearInternalModuliCache();
+ }
+
+ @Before
+ @After
+ public void clearCache() {
+ clearInternalModuliCache();
+ }
+
+ @Test
+ public void testLoadInternalModuli() throws IOException {
+ URL moduli = getClass().getResource(Moduli.INTERNAL_MODULI_RESPATH);
+ assertNotNull("Missing internal moduli resource", moduli);
+
+ List<DhGroup> expected = Moduli.loadInternalModuli(moduli);
+ assertTrue("No moduli groups parsed", GenericUtils.isNotEmpty(expected));
+
+ for (int index = 1; index <= Byte.SIZE; index++) {
+ List<DhGroup> actual = Moduli.loadInternalModuli(moduli);
+ assertSame("Mismatched cached instance at retry #" + index, expected, actual);
+ }
+ }
+}
[mina-sshd] 02/02: [SSHD-941] Allow user to override min./max. key
sizes for a specific session Diffi-Helman group exchange via session
properties
Posted by lg...@apache.org.
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 6eb232f3a0b92749fad3c314d47d78a61a85b401
Author: Lyor Goldstein <lg...@apache.org>
AuthorDate: Mon Sep 23 09:41:46 2019 +0300
[SSHD-941] Allow user to override min./max. key sizes for a specific session Diffi-Helman group exchange via session properties
---
CHANGES.md | 10 +++++++++-
.../main/java/org/apache/sshd/client/kex/DHGClient.java | 11 ++++++++---
.../main/java/org/apache/sshd/client/kex/DHGEXClient.java | 13 +++++++++++++
.../main/java/org/apache/sshd/server/kex/DHGEXServer.java | 15 ++++++++++-----
.../main/java/org/apache/sshd/server/kex/DHGServer.java | 3 ++-
5 files changed, 42 insertions(+), 10 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index 70c4c90..2ec72a5 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -43,6 +43,10 @@ initialized in the past.
* The internal moduli used in Diffie-Hellman group exchange are **cached** - lazy-loaded the 1st time such an exchange
occurs. The cache can be invalidated (and thus force a re-load) by invoking `Moduli#clearInternalModuliCache`.
+* `DHGEXClient#init` implementation allows overriding the min./max. key sizes for a specific session Diffi-Helman group
+exchange via properties - see `DHGEXClient#PROP_DHGEX_CLIENT_MIN/MAX_KEY`. Similar applies for `DHGEXServer` but only for
+the message type=30.
+
## Behavioral changes and enhancements
* [SSHD-926](https://issues.apache.org/jira/browse/SSHD-930) - Add support for OpenSSH 'lsetstat@openssh.com' SFTP protocol extension.
@@ -50,8 +54,12 @@ occurs. The cache can be invalidated (and thus force a re-load) by invoking `Mod
* [SSHD-930](https://issues.apache.org/jira/browse/SSHD-930) - Added configuration allowing the user to specify whether client should wait
for the server's identification before sending its own.
-* [SSHD-931](https://issues.apache.org/jira/browse/SSHD-931) - Using an executor supplier instead of a specific instance in `SftpSubsystemFactory`
+* [SSHD-931](https://issues.apache.org/jira/browse/SSHD-931) - Using an executor supplier instead of a specific instance in `SftpSubsystemFactory`.
* [SSHD-934](https://issues.apache.org/jira/browse/SSHD-934) - Fixed ECDSA public key encoding into OpenSSH format.
* [SSHD-937](https://issues.apache.org/jira/browse/SSHD-937) - Provide session instance when creating a subsystem, user authentication, channel.
+
+* [SSHD-941](https://issues.apache.org/jira/browse/SSHD-941) - Allow user to override min./max. key sizes for a specific session Diffi-Helman group
+exchange via properties.
+
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 cf00249..bed61f3 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
@@ -102,8 +102,10 @@ public class DHGClient extends AbstractDHClientKeyExchange {
public boolean next(int cmd, Buffer buffer) throws Exception {
Session session = getSession();
if (log.isDebugEnabled()) {
- log.debug("next({})[{}] process command={}", this, session, KeyExchange.getSimpleKexOpcodeName(cmd));
+ log.debug("next({})[{}] process command={}",
+ this, session, KeyExchange.getSimpleKexOpcodeName(cmd));
}
+
if (cmd != SshConstants.SSH_MSG_KEXDH_REPLY) {
throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
"Protocol error: expected packet SSH_MSG_KEXDH_REPLY, got " + KeyExchange.getSimpleKexOpcodeName(cmd));
@@ -119,7 +121,8 @@ public class DHGClient extends AbstractDHClientKeyExchange {
serverKey = buffer.getRawPublicKey();
String keyAlg = KeyUtils.getKeyType(serverKey);
if (GenericUtils.isEmpty(keyAlg)) {
- throw new SshException("Unsupported server key type: " + serverKey.getAlgorithm());
+ throw new SshException("Unsupported server key type: " + serverKey.getAlgorithm()
+ + "[" + serverKey.getFormat() + "]");
}
buffer = new ByteArrayBuffer();
@@ -140,8 +143,10 @@ 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 for key type=" + keyAlg);
+ throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
+ "KeyExchange signature verification failed for key type=" + keyAlg);
}
+
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 ced2125..2e23683 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
@@ -23,6 +23,7 @@ import java.math.BigInteger;
import java.util.Objects;
import org.apache.sshd.common.NamedFactory;
+import org.apache.sshd.common.PropertyResolverUtils;
import org.apache.sshd.common.SshConstants;
import org.apache.sshd.common.SshException;
import org.apache.sshd.common.config.keys.KeyUtils;
@@ -42,6 +43,9 @@ import org.apache.sshd.common.util.security.SecurityUtils;
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public class DHGEXClient extends AbstractDHClientKeyExchange {
+ public static final String PROP_DHGEX_CLIENT_MIN_KEY = "dhgex-client-min";
+ public static final String PROP_DHGEX_CLIENT_MAX_KEY = "dhgex-client-max";
+
protected final DHFactory factory;
protected byte expected;
protected int min = SecurityUtils.MIN_DHGEX_KEY_SIZE;
@@ -86,10 +90,19 @@ public class DHGEXClient extends AbstractDHClientKeyExchange {
@Override
public void init(Session s, byte[] v_s, byte[] v_c, byte[] i_s, byte[] i_c) throws Exception {
super.init(s, v_s, v_c, i_s, i_c);
+
+ // SSHD-941 give the user a chance to intervene in the choice
+ min = PropertyResolverUtils.getIntProperty(s, PROP_DHGEX_CLIENT_MIN_KEY, min);
+ max = PropertyResolverUtils.getIntProperty(s, PROP_DHGEX_CLIENT_MAX_KEY, max);
+ prf = Math.min(SecurityUtils.PREFERRED_DHGEX_KEY_SIZE, max);
if (log.isDebugEnabled()) {
log.debug("init({})[{}] Send SSH_MSG_KEX_DH_GEX_REQUEST - min={}, prf={}, max={}",
this, s, min, prf, max);
}
+ if ((max < min) || (prf < min) || (max < prf)) {
+ throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
+ "Protocol error: bad parameters " + min + " !< " + prf + " !< " + max);
+ }
Buffer buffer = s.createBuffer(SshConstants.SSH_MSG_KEX_DH_GEX_REQUEST, Integer.SIZE);
buffer.putInt(min);
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/kex/DHGEXServer.java b/sshd-core/src/main/java/org/apache/sshd/server/kex/DHGEXServer.java
index 55d791f..7a4b8f2 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/kex/DHGEXServer.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/kex/DHGEXServer.java
@@ -31,6 +31,7 @@ import java.util.Objects;
import org.apache.sshd.common.Factory;
import org.apache.sshd.common.FactoryManager;
import org.apache.sshd.common.NamedFactory;
+import org.apache.sshd.common.PropertyResolverUtils;
import org.apache.sshd.common.SshConstants;
import org.apache.sshd.common.SshException;
import org.apache.sshd.common.kex.DHFactory;
@@ -55,6 +56,9 @@ import org.apache.sshd.server.session.ServerSession;
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public class DHGEXServer extends AbstractDHServerKeyExchange {
+ public static final String PROP_DHGEX_SERVER_MIN_KEY = "dhgex-server-min";
+ public static final String PROP_DHGEX_SERVER_MAX_KEY = "dhgex-server-max";
+
protected final DHFactory factory;
protected DHG dh;
protected int min;
@@ -112,9 +116,11 @@ public class DHGEXServer extends AbstractDHServerKeyExchange {
if ((cmd == SshConstants.SSH_MSG_KEX_DH_GEX_REQUEST_OLD)
&& (expected == SshConstants.SSH_MSG_KEX_DH_GEX_REQUEST)) {
oldRequest = true;
- min = SecurityUtils.MIN_DHGEX_KEY_SIZE;
+ min = PropertyResolverUtils.getIntProperty(
+ session, PROP_DHGEX_SERVER_MIN_KEY, SecurityUtils.MIN_DHGEX_KEY_SIZE);
prf = buffer.getInt();
- max = SecurityUtils.getMaxDHGroupExchangeKeySize();
+ max = PropertyResolverUtils.getIntProperty(
+ session, PROP_DHGEX_SERVER_MAX_KEY, SecurityUtils.getMaxDHGroupExchangeKeySize());
if ((max < min) || (prf < min) || (max < prf)) {
throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
@@ -307,7 +313,6 @@ public class DHGEXServer extends AbstractDHServerKeyExchange {
protected List<Moduli.DhGroup> loadModuliGroups() throws IOException {
Session session = getServerSession();
String moduliStr = session.getString(ServerFactoryManager.MODULI_URL);
-
List<Moduli.DhGroup> groups = null;
if (!GenericUtils.isEmpty(moduliStr)) {
try {
@@ -337,8 +342,8 @@ public class DHGEXServer extends AbstractDHServerKeyExchange {
}
if (log.isDebugEnabled()) {
- log.debug("loadModuliGroups({})[{}] Loaded moduli groups from {}",
- this, session, moduliStr);
+ log.debug("loadModuliGroups({})[{}] Loaded {} moduli groups from {}",
+ this, session, GenericUtils.size(groups), moduliStr);
}
return groups;
}
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 3dfefad..d74353b 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
@@ -88,7 +88,8 @@ public class DHGServer extends AbstractDHServerKeyExchange {
public boolean next(int cmd, Buffer buffer) throws Exception {
ServerSession session = getServerSession();
if (log.isDebugEnabled()) {
- log.debug("next({})[{}] process command={}", this, session, KeyExchange.getSimpleKexOpcodeName(cmd));
+ log.debug("next({})[{}] process command={}",
+ this, session, KeyExchange.getSimpleKexOpcodeName(cmd));
}
if (cmd != SshConstants.SSH_MSG_KEXDH_INIT) {