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/08/01 08:10:45 UTC

[mina-sshd] 03/06: [SSHD-1004] Deprecate MD5-based and truncated HMAC algorithms from default setup.

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 04081d71ddb819cebc16a8b84be289b26a7f863e
Author: Lyor Goldstein <lg...@apache.org>
AuthorDate: Fri Jul 31 23:02:35 2020 +0300

    [SSHD-1004] Deprecate MD5-based and truncated HMAC algorithms from default setup.
---
 CHANGES.md                                            |  9 +++++----
 README.md                                             |  5 +++--
 .../main/java/org/apache/sshd/common/BaseBuilder.java |  5 +----
 .../java/org/apache/sshd/DefaultSetupTestSupport.java | 15 +++++++++++++++
 .../java/org/apache/sshd/common/SshBuilderTest.java   | 19 -------------------
 5 files changed, 24 insertions(+), 29 deletions(-)

diff --git a/CHANGES.md b/CHANGES.md
index 18b1509..96e094f 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -12,13 +12,9 @@
 
 ## Major code re-factoring
 
-* [SSHD-506](https://issues.apache.org/jira/browse/SSHD-506) Added support for AES-GCM ciphers.
-* [SSHD-1004](https://issues.apache.org/jira/browse/SSHD-1004) Deprecate DES, RC4 and Blowfish ciphers from default setup.
-* [SSHD-1004](https://issues.apache.org/jira/browse/SSHD-1004) Deprecate SHA-1 based key exchanges and signatures.
 * [SSHD-1034](https://issues.apache.org/jira/browse/SSHD-1034) Rename `org.apache.sshd.common.ForwardingFilter` to `Forwarder`.
 * [SSHD-1035](https://issues.apache.org/jira/browse/SSHD-1035) Move property definitions to common locations.
 * [SSHD-1038](https://issues.apache.org/jira/browse/SSHD-1038) Refactor packages from a module into a cleaner hierarchy.
-* [SSHD-1047](https://issues.apache.org/jira/browse/SSHD-1047) Support for SSH jumps.
 
 ## Minor code helpers
 
@@ -28,10 +24,15 @@
 
 ## Behavioral changes and enhancements
 
+* [SSHD-506](https://issues.apache.org/jira/browse/SSHD-506) Added support for AES-GCM ciphers.
+* [SSHD-1004](https://issues.apache.org/jira/browse/SSHD-1004) Deprecate DES, RC4 and Blowfish ciphers from default setup.
+* [SSHD-1004](https://issues.apache.org/jira/browse/SSHD-1004) Deprecate SHA-1 based key exchanges and signatures from default setup.
+* [SSHD-1004](https://issues.apache.org/jira/browse/SSHD-1004) Deprecate MD5-based and truncated HMAC algorithms from default setup.
 * [SSHD-1020](https://issues.apache.org/jira/browse/SSHD-1020) SSH connections getting closed abruptly with timeout exceptions.
 * [SSHD-1026](https://issues.apache.org/jira/browse/SSHD-1026) Improve build reproductibility.
 * [SSHD-1028](https://issues.apache.org/jira/browse/SSHD-1028) Fix SSH_MSG_DISCONNECT: Too many concurrent connections.
 * [SSHD-1032](https://issues.apache.org/jira/browse/SSHD-1032) Fix possible ArrayIndexOutOfBoundsException in ChannelAsyncOutputStream.
 * [SSHD-1033](https://issues.apache.org/jira/browse/SSHD-1033) Fix simultaneous usage of dynamic and local port forwarding.
 * [SSHD-1039](https://issues.apache.org/jira/browse/SSHD-1039) Fix support for some basic options in ssh/sshd cli.
+* [SSHD-1047](https://issues.apache.org/jira/browse/SSHD-1047) Support for SSH jumps.
 * [SSHD-1048](https://issues.apache.org/jira/browse/SSHD-1048) Wrap instead of rethrow IOException in Future.
diff --git a/README.md b/README.md
index 69eaa25..564d7a4 100644
--- a/README.md
+++ b/README.md
@@ -77,6 +77,7 @@ the unsafe settings must do so **explicitly**. The following settings have been
     * While it refers to Kerberos, it mentions weaknesses in DES as well.
 * [OpenSSH release notes](https://www.openssh.com/releasenotes.html) - usually a good indicator of de-facto practices
 * SHA-1 based key exchanges and signatures
+* MD5-based and truncated HMAC algorithms
 
 **Caveat:**: According to [RFC 8332 - section 3.31](https://tools.ietf.org/html/rfc8332#section-3.3)
 >>
@@ -88,8 +89,8 @@ the unsafe settings must do so **explicitly**. The following settings have been
 >> algorithms have been sufficiently widely adopted to warrant disabling "ssh-rsa", clients MAY default to one of
 >> the new algorithms.
 
-This means that users that encounter this problem must modify the supported security settings **explicitly** in
-order to avoid the issue.
+This means that users that encounter this (and related) problems must modify the supported security settings
+**explicitly** in order to avoid the issue.
 
 # [Release notes](./CHANGES.md)
 
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/BaseBuilder.java b/sshd-core/src/main/java/org/apache/sshd/common/BaseBuilder.java
index d89cadd..9c6f71f 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/BaseBuilder.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/BaseBuilder.java
@@ -108,10 +108,7 @@ public class BaseBuilder<T extends AbstractFactoryManager, S extends BaseBuilder
                     BuiltinMacs.hmacsha1etm,
                     BuiltinMacs.hmacsha256,
                     BuiltinMacs.hmacsha512,
-                    BuiltinMacs.hmacsha1,
-                    BuiltinMacs.hmacmd5,
-                    BuiltinMacs.hmacsha196,
-                    BuiltinMacs.hmacmd596));
+                    BuiltinMacs.hmacsha1));
 
     /**
      * Preferred {@link BuiltinSignatures} according to
diff --git a/sshd-core/src/test/java/org/apache/sshd/DefaultSetupTestSupport.java b/sshd-core/src/test/java/org/apache/sshd/DefaultSetupTestSupport.java
index 6c393f6..351f3e9 100644
--- a/sshd-core/src/test/java/org/apache/sshd/DefaultSetupTestSupport.java
+++ b/sshd-core/src/test/java/org/apache/sshd/DefaultSetupTestSupport.java
@@ -33,6 +33,8 @@ import org.apache.sshd.common.cipher.Cipher;
 import org.apache.sshd.common.helpers.AbstractFactoryManager;
 import org.apache.sshd.common.kex.BuiltinDHFactories;
 import org.apache.sshd.common.kex.KeyExchange;
+import org.apache.sshd.common.mac.BuiltinMacs;
+import org.apache.sshd.common.mac.Mac;
 import org.apache.sshd.common.signature.BuiltinSignatures;
 import org.apache.sshd.common.signature.Signature;
 import org.apache.sshd.common.util.GenericUtils;
@@ -99,6 +101,19 @@ public abstract class DefaultSetupTestSupport<M extends AbstractFactoryManager>
 
     }
 
+    @Test
+    public void testDefaultMacsList() {
+        assertSameNamedFactoriesListInstances(
+                Mac.class.getSimpleName(), BaseBuilder.DEFAULT_MAC_PREFERENCE, factory.getMacFactories());
+    }
+
+    @Test
+    public void testNoDeprecatedMacs() {
+        assertNoDeprecatedFactoryInstanceNames(
+                Mac.class.getSimpleName(), EnumSet.of(BuiltinMacs.hmacmd5, BuiltinMacs.hmacmd596, BuiltinMacs.hmacsha196),
+                factory.getMacFactories());
+    }
+
     protected static void assertSameNamedResourceListNames(
             String hint, List<? extends NamedResource> expected, List<? extends NamedResource> actual) {
         int len = GenericUtils.size(expected);
diff --git a/sshd-core/src/test/java/org/apache/sshd/common/SshBuilderTest.java b/sshd-core/src/test/java/org/apache/sshd/common/SshBuilderTest.java
index 736a94a..e28746f 100644
--- a/sshd-core/src/test/java/org/apache/sshd/common/SshBuilderTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/common/SshBuilderTest.java
@@ -19,13 +19,10 @@
 
 package org.apache.sshd.common;
 
-import java.util.Collection;
 import java.util.List;
-import java.util.Set;
 
 import org.apache.sshd.common.cipher.BuiltinCiphers;
 import org.apache.sshd.common.cipher.Cipher;
-import org.apache.sshd.common.mac.BuiltinMacs;
 import org.apache.sshd.common.util.GenericUtils;
 import org.apache.sshd.util.test.BaseTestSupport;
 import org.apache.sshd.util.test.NoIoTestCase;
@@ -45,22 +42,6 @@ public class SshBuilderTest extends BaseTestSupport {
     }
 
     /**
-     * Make sure that all values in {@link BuiltinMacs} are listed in {@link BaseBuilder#DEFAULT_MAC_PREFERENCE}
-     */
-    @Test
-    public void testAllBuiltinMacsListed() {
-        testAllInstancesListed(BuiltinMacs.VALUES, BaseBuilder.DEFAULT_MAC_PREFERENCE);
-    }
-
-    private static <
-            E extends Enum<E>> void testAllInstancesListed(Set<? extends E> expValues, Collection<? extends E> actValues) {
-        assertEquals("Mismatched actual values size", expValues.size(), actValues.size());
-        for (E expected : expValues) {
-            assertTrue(expected.name() + " not found in actual values", actValues.contains(expected));
-        }
-    }
-
-    /**
      * Make sure that {@link BaseBuilder#setUpDefaultCiphers(boolean)} returns the correct result - i.e., according to
      * the {@code ingoreUnsupported} parameter and in the defined preference order
      */