This is an automated email from the ASF dual-hosted git repository.
twolf pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
commit edb2b95121ff19866e34245abdee8460cea1b4ca
Author: Thomas Wolf <tw...@apache.org>
AuthorDate: Fri Feb 2 00:38:36 2024 +0100
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 | 23 +++++++++++
.../java/org/apache/sshd/server/ServerBuilder.java | 22 ++++++++++
.../org/apache/sshd/DefaultSetupTestSupport.java | 7 +++-
.../sshd/client/ClientSessionListenerTest.java | 2 +-
.../sshd/common/mac/MacCompatibilityTest.java | 48 ++++++++++++++--------
5 files changed, 83 insertions(+), 19 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index 405eb4225..e6e60e08e 100644
--- a/CHANGES.md
+++ b/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
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/ServerBuilder.java
b/sshd-core/src/main/java/org/apache/sshd/server/ServerBuilder.java
index 4fe4961e3..5a7ed835c 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/ServerBuilder.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/ServerBuilder.java
@@ -25,9 +25,11 @@ import java.util.List;
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 @@ public class ServerBuilder extends BaseBuilder<SshServer,
ServerBuilder> {
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 class ServerBuilder extends BaseBuilder<SshServer,
ServerBuilder> {
@Override
protected ServerBuilder fillWithDefaultValues() {
+ if (cipherFactories == null) {
+ cipherFactories(BuiltinFactory.setUpFactories(false,
DEFAULT_SERVER_CIPHERS_PREFERENCE));
+ }
super.fillWithDefaultValues();
if (compressionFactories == null) {
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 c2f82be7a..45f810db8 100644
--- a/sshd-core/src/test/java/org/apache/sshd/DefaultSetupTestSupport.java
+++ b/sshd-core/src/test/java/org/apache/sshd/DefaultSetupTestSupport.java
@@ -38,6 +38,8 @@ 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;
+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 @@ public abstract class DefaultSetupTestSupport<M extends
AbstractFactoryManager>
@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());
}
diff --git
a/sshd-core/src/test/java/org/apache/sshd/client/ClientSessionListenerTest.java
b/sshd-core/src/test/java/org/apache/sshd/client/ClientSessionListenerTest.java
index 585c5afd3..e1b73e822 100644
---
a/sshd-core/src/test/java/org/apache/sshd/client/ClientSessionListenerTest.java
+++
b/sshd-core/src/test/java/org/apache/sshd/client/ClientSessionListenerTest.java
@@ -96,7 +96,7 @@ public class ClientSessionListenerTest extends
BaseTestSupport {
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() {
diff --git
a/sshd-core/src/test/java/org/apache/sshd/common/mac/MacCompatibilityTest.java
b/sshd-core/src/test/java/org/apache/sshd/common/mac/MacCompatibilityTest.java
index 692a22953..319d27406 100644
---
a/sshd-core/src/test/java/org/apache/sshd/common/mac/MacCompatibilityTest.java
+++
b/sshd-core/src/test/java/org/apache/sshd/common/mac/MacCompatibilityTest.java
@@ -26,7 +26,9 @@ import java.nio.file.Path;
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 class MacCompatibilityTest extends BaseTestSupport
{
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());
+ }
}
}
