Remove AES-CBC ciphers from the server's default KEX proposal

CBC implementations have been historically susceptible to various
padding oracle attacks, and the use of CBC-mode ciphers in the SSH
protocol was found insecure in 2008 in another attack known as
CPNI-957037[1], VU#958563[2], or CVE-2008-5161[3]. Details were
published in [4] in 2009.

OpenSSH does not propose the CBC ciphers (unless explicitly enabled)
in servers since 2014, and has removed them from the client proposal
in 2017, too.

Before the full disclosure in 2009, OpenSSH had implemented some
mitigations against CPNI-957037, but given the nature of the attack
I'm not convinced they are effective. The attack leverages OpenSSH
as an oracle and it does not need to control the IV, so it should be
possible to perform the decryption attack offline using an older
unpatched SSH implementation.

TLS has deprecated CBC ciphers in TLS v1.2, and has removed them in
TLS v.1.3.

For clients, we keep the CBC ciphers by default for now to facilitate
connecting to legacy servers. I plan to remove them from the client's
default list in the next release.

[1] https://www.openssh.com/txt/cbc.adv
[2] https://www.kb.cert.org/vuls/id/958563
[3] https://cve.mitre.org/cgi-bin/cvename.cgi?name=2008-5161
[4] https://www.cs.umd.edu/~jkatz/security/downloads/PlaintextRecoverySSH.pdf

CHANGES.md

@@ -164,5 +164,28 @@ such excess data is silently discarded.
 
 ## Potential compatibility issues
 
+### AES-CBC ciphers removed from server's defaults
+
+The AES-CBC ciphers `aes128-cbc`, `aes192-cbc`, and `aes256-cbc` have been removed from the default
+list of cipher algorithms that a server proposes in the key exchange. OpenSSH has removed these
+cipher algorithms from the server proposal in 2014, and has removed them from the client proposal
+in 2017.
+
+The cipher implementations still exist but they are not enabled by default. Existing code that
+explicitly sets the cipher factories is unaffected. Code that relies on the default settings
+will newly create a server that does not support the CBC-mode ciphers. To enable the CBC-mode
+ciphers, one can use for instance
+
+```
+SshServer server = ServerBuilder.builder()
+   ...
+   .cipherFactories(BuiltinFactory.setUpFactories(false, BaseBuilder.DEFAULT_CIPHERS_PREFERENCES));
+   ...
+   .build();
+```
+
+For the SSH _client_, the CBC ciphers are still enabled by default to facilitate connecting to
+legacy servers. We plan to remove the CBC ciphers from the client's defaults in the next release.
+
 ## Major Code Re-factoring
 

sshd-core/src/main/java/org/apache/sshd/server/ServerBuilder.java

@@ -25,9 +25,11 @@
 import java.util.function.Function;
 
 import org.apache.sshd.common.BaseBuilder;
+import org.apache.sshd.common.BuiltinFactory;
 import org.apache.sshd.common.NamedFactory;
 import org.apache.sshd.common.channel.ChannelFactory;
 import org.apache.sshd.common.channel.RequestHandler;
+import org.apache.sshd.common.cipher.BuiltinCiphers;
 import org.apache.sshd.common.compression.BuiltinCompressions;
 import org.apache.sshd.common.compression.Compression;
 import org.apache.sshd.common.compression.CompressionFactory;
@@ -86,6 +88,23 @@ Arrays.<CompressionFactory> asList(
                     BuiltinCompressions.delayedZlib));
     public static final KexExtensionHandler DEFAULT_KEX_EXTENSION_HANDLER = DefaultServerKexExtensionHandler.INSTANCE;
 
+    /**
+     * Default list of ciphers for a server. This excludes the AES-CBC ciphers -- OpenSSH has stopped proposing them by
+     * default in 2014 (and removed them from the client proposal in 2017, too). CBC is susceptible to padding oracle
+     * attacks and other attacks and is thus not recommended anymore.
+     * <p>
+     * For clients, we do still include the CBC modes to better support connecting with legacy servers.
+     * </p>
+     */
+    public static final List<BuiltinCiphers> DEFAULT_SERVER_CIPHERS_PREFERENCE = Collections.unmodifiableList(
+            Arrays.asList(
+                    BuiltinCiphers.cc20p1305_openssh,
+                    BuiltinCiphers.aes128ctr,
+                    BuiltinCiphers.aes192ctr,
+                    BuiltinCiphers.aes256ctr,
+                    BuiltinCiphers.aes128gcm,
+                    BuiltinCiphers.aes256gcm));
+
     protected PublickeyAuthenticator pubkeyAuthenticator;
     protected KeyboardInteractiveAuthenticator interactiveAuthenticator;
 
@@ -105,6 +124,9 @@ public ServerBuilder publickeyAuthenticator(PublickeyAuthenticator auth) {
 
     @Override
     protected ServerBuilder fillWithDefaultValues() {
+        if (cipherFactories == null) {
+            cipherFactories(BuiltinFactory.setUpFactories(false, DEFAULT_SERVER_CIPHERS_PREFERENCE));
+        }
         super.fillWithDefaultValues();
 
         if (compressionFactories == null) {

sshd-core/src/test/java/org/apache/sshd/DefaultSetupTestSupport.java

@@ -38,6 +38,8 @@
 import org.apache.sshd.common.signature.BuiltinSignatures;
 import org.apache.sshd.common.signature.Signature;
 import org.apache.sshd.common.util.GenericUtils;
+import org.apache.sshd.server.ServerBuilder;
+import org.apache.sshd.server.SshServer;
 import org.apache.sshd.util.test.BaseTestSupport;
 import org.apache.sshd.util.test.NoIoTestCase;
 import org.junit.Test;
@@ -59,7 +61,10 @@ protected DefaultSetupTestSupport(M factory) {
 
     @Test
     public void testDefaultCiphersList() {
-        assertSameNamedFactoriesListInstances(Cipher.class.getSimpleName(), BaseBuilder.DEFAULT_CIPHERS_PREFERENCE,
+        assertSameNamedFactoriesListInstances(Cipher.class.getSimpleName(),
+                factory instanceof SshServer
+                        ? ServerBuilder.DEFAULT_SERVER_CIPHERS_PREFERENCE
+                        : BaseBuilder.DEFAULT_CIPHERS_PREFERENCE,
                 factory.getCipherFactories());
     }
 

sshd-core/src/test/java/org/apache/sshd/client/ClientSessionListenerTest.java

@@ -96,7 +96,7 @@ public static void tearDownClientAndServer() throws Exception {
     public void testSessionListenerCanModifyKEXNegotiation() throws Exception {
         Map<KexProposalOption, NamedResource> kexParams = new EnumMap<>(KexProposalOption.class);
         kexParams.put(KexProposalOption.ALGORITHMS, getLeastFavorite(KeyExchange.class, client.getKeyExchangeFactories()));
-        kexParams.put(KexProposalOption.C2SENC, getLeastFavorite(CipherFactory.class, client.getCipherFactories()));
+        kexParams.put(KexProposalOption.C2SENC, getLeastFavorite(CipherFactory.class, sshd.getCipherFactories()));
         kexParams.put(KexProposalOption.C2SMAC, getLeastFavorite(MacFactory.class, client.getMacFactories()));
 
         SessionListener listener = new SessionListener() {

sshd-core/src/test/java/org/apache/sshd/common/mac/MacCompatibilityTest.java

@@ -26,7 +26,9 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.concurrent.TimeUnit;
 
 import ch.ethz.ssh2.Connection;
@@ -162,29 +164,41 @@ public void testWithJSCH() throws Exception {
         Assume.assumeTrue("Known JSCH bug with " + macName, !BuiltinMacs.hmacsha512.equals(factory));
 
         JSch sch = new JSch();
-        JSch.setConfig("cipher.s2c", "aes128-cbc,3des-cbc,blowfish-cbc,aes192-cbc,aes256-cbc,none");
-        JSch.setConfig("cipher.c2s", "aes128-cbc,3des-cbc,blowfish-cbc,aes192-cbc,aes256-cbc,none");
-        JSch.setConfig("mac.s2c", macName);
-        JSch.setConfig("mac.c2s", macName);
-        JSch.setConfig(macName, jschMacClass);
-
-        com.jcraft.jsch.Session session = sch.getSession(getCurrentTestName(), TEST_LOCALHOST, port);
+        Map<String, String> values = new HashMap<>();
+        values.put("cipher.s2c", JSch.getConfig("cipher.s2c"));
+        values.put("cipher.c2s", JSch.getConfig("cipher.s2c"));
+        values.put("mac.s2c", JSch.getConfig("mac.s2c"));
+        values.put("mac.c2s", JSch.getConfig("mac.s2c"));
+        values.put(macName, JSch.getConfig(macName));
         try {
-            session.setUserInfo(new SimpleUserInfo(getCurrentTestName()));
-            session.connect();
+            JSch.setConfig("cipher.s2c", "aes128-ctr,aes192-ctr,aes256-ctr,none");
+            JSch.setConfig("cipher.c2s", "aes128-ctr,aes192-ctr,aes256-ctr,none");
+            JSch.setConfig("mac.s2c", macName);
+            JSch.setConfig("mac.c2s", macName);
+            JSch.setConfig(macName, jschMacClass);
+
+            com.jcraft.jsch.Session session = sch.getSession(getCurrentTestName(), TEST_LOCALHOST, port);
+            try {
+                session.setUserInfo(new SimpleUserInfo(getCurrentTestName()));
+                session.connect();
 
-            com.jcraft.jsch.Channel channel = session.openChannel(Channel.CHANNEL_SHELL);
-            channel.connect();
+                com.jcraft.jsch.Channel channel = session.openChannel(Channel.CHANNEL_SHELL);
+                channel.connect();
 
-            try (OutputStream stdin = channel.getOutputStream();
-                 InputStream stdout = channel.getInputStream();
-                 InputStream stderr = channel.getExtInputStream()) {
-                runShellTest(stdin, stdout);
+                try (OutputStream stdin = channel.getOutputStream();
+                     InputStream stdout = channel.getInputStream();
+                     InputStream stderr = channel.getExtInputStream()) {
+                    runShellTest(stdin, stdout);
+                } finally {
+                    channel.disconnect();
+                }
             } finally {
-                channel.disconnect();
+                session.disconnect();
             }
         } finally {
-            session.disconnect();
+            for (Map.Entry<String, String> item : values.entrySet()) {
+                JSch.setConfig(item.getKey(), item.getValue());
+            }
         }
     }