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
The following commit(s) were added to refs/heads/master by this push:
new 2aadde9 [SSHD-1004] Deprecate DES, RC4 and Blowfish ciphers from
default setup
2aadde9 is described below
commit 2aadde9b01480a2a1d6d04b16e3a3b83a48292cd
Author: Lyor Goldstein <lgoldst...@apache.org>
AuthorDate: Fri Jul 31 10:39:09 2020 +0300
[SSHD-1004] Deprecate DES, RC4 and Blowfish ciphers from default setup
---
CHANGES.md | 1 +
README.md | 9 +++
.../java/org/apache/sshd/common/BaseBuilder.java | 9 +--
.../org/apache/sshd/DefaultSetupTestSupport.java | 90 ++++++++++++++++++++++
.../apache/sshd/client/ClientDefaultSetupTest.java | 34 ++++++++
.../org/apache/sshd/common/SshBuilderTest.java | 12 ---
.../sshd/common/cipher/BuiltinCiphersTest.java | 35 ---------
.../apache/sshd/server/ServerDefaultSetupTest.java | 34 ++++++++
8 files changed, 169 insertions(+), 55 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index b55e550..6477914 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -16,6 +16,7 @@
* [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-1035) Refactor
packages from a module into a cleaner hierarchy.
+* [SSHD-1004](https://issues.apache.org/jira/browse/SSHD-1035) Deprecate DES,
RC4 and Blowfish ciphers from default setup.
## Minor code helpers
diff --git a/README.md b/README.md
index 82778d7..e43adce 100644
--- a/README.md
+++ b/README.md
@@ -68,6 +68,15 @@ aes128-...@openssh.com, aes256-...@openssh.com
, ssh-rsa-cert-...@openssh.com, ssh-dss-cert-...@openssh.com,
ssh-ed25519-cert-...@openssh.com
, ecdsa-sha2-nistp256-cert-...@openssh.com,
ecdsa-sha2-nistp384-cert-...@openssh.com,
ecdsa-sha2-nistp521-cert-...@openssh.com
+**Note:** The above list contains all the supported security settings in the
code. However, in accordance with the latest recommendations
+the default client/server setup includes only the security settings that are
currently considered safe to use. Users who wish to include
+the unsafe settings must do so **explicitly**. The following settings have
been deprecated and are no longer included in the default setup:
+
+* [RFC 8758 - Deprecating RC4 in Secure Shell
(SSH)](https://tools.ietf.org/html/rfc8758)
+* [RFC 8429 - Deprecate Triple-DES (3DES) and RC4 in
Kerberos](https://tools.ietf.org/html/rfc8429)
+ * 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
+
# [Release notes](./CHANGES.md)
# Core requirements
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 ee352fc..821765a 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
@@ -74,16 +74,9 @@ public class BaseBuilder<T extends AbstractFactoryManager, S
extends BaseBuilder
BuiltinCiphers.aes256ctr,
BuiltinCiphers.aes128gcm,
BuiltinCiphers.aes256gcm,
- BuiltinCiphers.arcfour256,
- BuiltinCiphers.arcfour128,
BuiltinCiphers.aes128cbc,
- BuiltinCiphers.tripledescbc,
- BuiltinCiphers.blowfishcbc,
- // TODO add support for cast128-cbc cipher
BuiltinCiphers.aes192cbc,
- BuiltinCiphers.aes256cbc
- // TODO add support for arcfour cipher
- ));
+ BuiltinCiphers.aes256cbc));
/**
* The default {@link BuiltinDHFactories} setup in order of preference as
specified by
diff --git
a/sshd-core/src/test/java/org/apache/sshd/DefaultSetupTestSupport.java
b/sshd-core/src/test/java/org/apache/sshd/DefaultSetupTestSupport.java
new file mode 100644
index 0000000..bc5ef94
--- /dev/null
+++ b/sshd-core/src/test/java/org/apache/sshd/DefaultSetupTestSupport.java
@@ -0,0 +1,90 @@
+/*
+ * 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;
+
+import java.util.Collection;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
+
+import org.apache.sshd.common.BaseBuilder;
+import org.apache.sshd.common.NamedFactory;
+import org.apache.sshd.common.NamedResource;
+import org.apache.sshd.common.cipher.BuiltinCiphers;
+import org.apache.sshd.common.cipher.Cipher;
+import org.apache.sshd.common.helpers.AbstractFactoryManager;
+import org.apache.sshd.common.util.GenericUtils;
+import org.apache.sshd.util.test.BaseTestSupport;
+import org.apache.sshd.util.test.NoIoTestCase;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+/**
+ * Makes sure default client/server setup satisfies various conditions
+ *
+ * @param <M> The {@link AbstractFactoryManager} type being tested - can be
client or server
+ * @author <a href="mailto:d...@mina.apache.org";>Apache MINA SSHD
Project</a>
+ */
+@Category({ NoIoTestCase.class })
+public abstract class DefaultSetupTestSupport<M extends
AbstractFactoryManager> extends BaseTestSupport {
+ protected final M factory;
+
+ protected DefaultSetupTestSupport(M factory) {
+ this.factory = factory;
+ }
+
+ @Test
+ public void testDefaultCiphersList() {
+ assertNamedFactoriesList(Cipher.class.getSimpleName(),
BaseBuilder.DEFAULT_CIPHERS_PREFERENCE,
+ factory.getCipherFactories());
+ }
+
+ @Test // SSHD-1004
+ public void testNoDeprecatedCiphers() {
+ assertNoDeprecatedSettings(Cipher.class.getSimpleName(),
+ EnumSet.of(BuiltinCiphers.arcfour128,
BuiltinCiphers.arcfour256, BuiltinCiphers.tripledescbc,
+ BuiltinCiphers.blowfishcbc),
+ factory.getCipherFactories());
+ }
+
+ protected static <T, F extends NamedFactory<T>> void
assertNoDeprecatedSettings(
+ String hint, Collection<? extends F> unexpected, Collection<?
extends F> actual) {
+ Collection<String> disallowedNames = unexpected.stream()
+ .map(NamedResource::getName)
+ .collect(Collectors.toCollection(() -> new
TreeSet<>(String.CASE_INSENSITIVE_ORDER)));
+ for (F namedFactory : actual) {
+ String name = namedFactory.getName();
+ assertFalse(hint + " - disallowed: " + name,
disallowedNames.contains(name));
+ }
+ }
+
+ protected static <T, F extends NamedFactory<T>> void
assertNamedFactoriesList(
+ String hint, List<? extends F> expected, List<? extends F> actual)
{
+ int len = GenericUtils.size(expected);
+ assertEquals(hint + "[size]", len, GenericUtils.size(actual));
+
+ for (int index = 0; index < len; index++) {
+ F expFactory = expected.get(index);
+ F actFactory = actual.get(index);
+ assertSame(hint + "[" + index + "]", expFactory, actFactory);
+ }
+ }
+}
diff --git
a/sshd-core/src/test/java/org/apache/sshd/client/ClientDefaultSetupTest.java
b/sshd-core/src/test/java/org/apache/sshd/client/ClientDefaultSetupTest.java
new file mode 100644
index 0000000..b689506
--- /dev/null
+++ b/sshd-core/src/test/java/org/apache/sshd/client/ClientDefaultSetupTest.java
@@ -0,0 +1,34 @@
+/*
+ * 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.client;
+
+import org.apache.sshd.DefaultSetupTestSupport;
+import org.junit.FixMethodOrder;
+import org.junit.runners.MethodSorters;
+
+/**
+ * @author <a href="mailto:d...@mina.apache.org";>Apache MINA SSHD Project</a>
+ */
+@FixMethodOrder(MethodSorters.NAME_ASCENDING)
+public class ClientDefaultSetupTest extends DefaultSetupTestSupport<SshClient>
{
+ public ClientDefaultSetupTest() {
+ super(SshClient.setUpDefaultClient());
+ }
+}
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 1061ea9..8f23ba0 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
@@ -20,7 +20,6 @@
package org.apache.sshd.common;
import java.util.Collection;
-import java.util.EnumSet;
import java.util.List;
import java.util.Set;
@@ -49,17 +48,6 @@ public class SshBuilderTest extends BaseTestSupport {
}
/**
- * Make sure that all values in {@link BuiltinCiphers} are listed in
{@link BaseBuilder#DEFAULT_CIPHERS_PREFERENCE}
- */
- @Test
- public void testAllBuiltinCiphersListed() {
- Set<BuiltinCiphers> all = EnumSet.allOf(BuiltinCiphers.class);
- // The 'none' cipher is not listed as preferred - it is implied
- assertTrue("Missing " + BuiltinCiphers.Constants.NONE + " cipher in
all values", all.remove(BuiltinCiphers.none));
- testAllInstancesListed(all, BaseBuilder.DEFAULT_CIPHERS_PREFERENCE);
- }
-
- /**
* Make sure that all values in {@link BuiltinMacs} are listed in {@link
BaseBuilder#DEFAULT_MAC_PREFERENCE}
*/
@Test
diff --git
a/sshd-core/src/test/java/org/apache/sshd/common/cipher/BuiltinCiphersTest.java
b/sshd-core/src/test/java/org/apache/sshd/common/cipher/BuiltinCiphersTest.java
index 33f1810..57c1d0b 100644
---
a/sshd-core/src/test/java/org/apache/sshd/common/cipher/BuiltinCiphersTest.java
+++
b/sshd-core/src/test/java/org/apache/sshd/common/cipher/BuiltinCiphersTest.java
@@ -23,7 +23,6 @@ import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Collection;
import java.util.Collections;
import java.util.EnumSet;
import java.util.List;
@@ -31,14 +30,11 @@ import java.util.Objects;
import java.util.Random;
import java.util.Set;
-import org.apache.sshd.client.SshClient;
-import org.apache.sshd.common.FactoryManager;
import org.apache.sshd.common.NamedFactory;
import org.apache.sshd.common.NamedResource;
import org.apache.sshd.common.cipher.BuiltinCiphers.ParseResult;
import org.apache.sshd.common.util.GenericUtils;
import org.apache.sshd.common.util.buffer.BufferUtils;
-import org.apache.sshd.server.SshServer;
import org.apache.sshd.util.test.BaseTestSupport;
import org.junit.FixMethodOrder;
import org.junit.Test;
@@ -156,37 +152,6 @@ public class BuiltinCiphersTest extends BaseTestSupport {
}
}
- @Test
- public void testSshClientSupportedCiphersConfiguration() throws Exception {
- try (SshClient client = setupTestClient()) {
- testSupportedCiphersConfiguration(client);
- }
- }
-
- @Test
- public void testSshSercerSupportedCiphersConfiguration() throws Exception {
- try (SshServer server = setupTestServer()) {
- testSupportedCiphersConfiguration(server);
- }
- }
-
- private static <M extends FactoryManager> M
testSupportedCiphersConfiguration(M manager) {
- Collection<? extends NamedResource> factories =
manager.getCipherFactories();
- List<String> names = NamedResource.getNameList(factories);
- for (BuiltinCiphers c : BuiltinCiphers.VALUES) {
- if (BuiltinCiphers.none.equals(c)) {
- continue; // not always included by default + it is a dummy
cipher
- }
-
- // for now, all key sizes below 128 are supported in JVM(s)
- if (c.getKeySize() <= 128) {
- assertTrue("Supported cipher not configured by default: " + c,
names.contains(c.getName()));
- }
- }
-
- return manager;
- }
-
private static void testCipherEncryption(Random rnd, Cipher cipher) throws
Exception {
byte[] key = new byte[cipher.getKdfSize()];
rnd.nextBytes(key);
diff --git
a/sshd-core/src/test/java/org/apache/sshd/server/ServerDefaultSetupTest.java
b/sshd-core/src/test/java/org/apache/sshd/server/ServerDefaultSetupTest.java
new file mode 100644
index 0000000..efb4c6e
--- /dev/null
+++ b/sshd-core/src/test/java/org/apache/sshd/server/ServerDefaultSetupTest.java
@@ -0,0 +1,34 @@
+/*
+ * 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;
+
+import org.apache.sshd.DefaultSetupTestSupport;
+import org.junit.FixMethodOrder;
+import org.junit.runners.MethodSorters;
+
+/**
+ * @author <a href="mailto:d...@mina.apache.org";>Apache MINA SSHD Project</a>
+ */
+@FixMethodOrder(MethodSorters.NAME_ASCENDING)
+public class ServerDefaultSetupTest extends DefaultSetupTestSupport<SshServer>
{
+ public ServerDefaultSetupTest() {
+ super(SshServer.setUpDefaultServer());
+ }
+}
