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 <lgoldst...@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 'lsets...@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:d...@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:d...@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) {