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
The following commit(s) were added to refs/heads/master by this push:
new 5a78e6dfe GH-493: Fix "blocksize" of arcfour ciphers
5a78e6dfe is described below
commit 5a78e6dfe37ec982de8eec7abf449e83b3c984ae
Author: Thomas Wolf <tw...@apache.org>
AuthorDate: Thu May 16 23:01:07 2024 +0200
GH-493: Fix "blocksize" of arcfour ciphers
Arcfour is a stream cipher. The padding must then be such that the total
SSH packet size is a multiple of 8. See RFC 4253: "the length of the
concatenation of 'packet_length', 'padding_length', 'payload', and
'random padding' MUST be a multiple of the cipher block size or 8,
whichever is larger. This constraint MUST be enforced, even when using
stream ciphers."[1]
Previous code had 16 and 32 as "block size" for these ciphers, which is
wrong. Set it to 8.
The wrong values 16 and 32 were added by mistake in commit 1166a5d2 in
version 2.2.0.
[1] https://datatracker.ietf.org/doc/html/rfc4253#page-7
Bug: https://github.com/apache/mina-sshd/issues/493
---
CHANGES.md | 2 +
.../apache/sshd/common/cipher/BuiltinCiphers.java | 4 +-
.../sshd/common/cipher/ArcFourOpenSshTest.java | 139 +++++++++++++++++++++
sshd-mina/pom.xml | 1 +
sshd-netty/pom.xml | 1 +
5 files changed, 145 insertions(+), 2 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index 50540d765..1cb4bfb2d 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -36,7 +36,9 @@
* [GH-455](https://github.com/apache/mina-sshd/issues/455) Fix `BaseCipher`:
make sure all bytes are processed
* [GH-470](https://github.com/apache/mina-sshd/issues/470) MontgomeryCurve:
synchronize access to KeyPairGenerator
* [GH-489](https://github.com/apache/mina-sshd/issues/489) SFTP v3 client:
better file type determination
+* [GH-493](https://github.com/apache/mina-sshd/issues/493) Fix arcfour128 and
arcfour256 ciphers
* [GH-500](https://github.com/apache/mina-sshd/issues/500) SFTP file system:
fix memory leak on exceptions
+* [GH-504](https://github.com/apache/mina-sshd/issues/504) Pass through
failure exception to `SessionListener.sessionNegotiationEnd()`
* [PR-472](https://github.com/apache/mina-sshd/pull/472) sshd-spring-sftp: fix
client start
* [PR-476](https://github.com/apache/mina-sshd/pull/476) Fix Android detection
diff --git
a/sshd-common/src/main/java/org/apache/sshd/common/cipher/BuiltinCiphers.java
b/sshd-common/src/main/java/org/apache/sshd/common/cipher/BuiltinCiphers.java
index bdacbaa95..0b9c49d5e 100644
---
a/sshd-common/src/main/java/org/apache/sshd/common/cipher/BuiltinCiphers.java
+++
b/sshd-common/src/main/java/org/apache/sshd/common/cipher/BuiltinCiphers.java
@@ -115,7 +115,7 @@ public enum BuiltinCiphers implements CipherFactory {
* @see <A
HREF="https://issues.apache.org/jira/browse/SSHD-1004";>SSHD-1004</A>
*/
@Deprecated
- arcfour128(Constants.ARCFOUR128, 8, 0, 16, "ARCFOUR", 128, "RC4", 16) {
+ arcfour128(Constants.ARCFOUR128, 8, 0, 16, "ARCFOUR", 128, "RC4", 8) {
@Override
public Cipher create() {
return new BaseRC4Cipher(getIVSize(), getKdfSize(), getKeySize(),
getCipherBlockSize());
@@ -126,7 +126,7 @@ public enum BuiltinCiphers implements CipherFactory {
* @see <A
HREF="https://issues.apache.org/jira/browse/SSHD-1004";>SSHD-1004</A>
*/
@Deprecated
- arcfour256(Constants.ARCFOUR256, 8, 0, 32, "ARCFOUR", 256, "RC4", 32) {
+ arcfour256(Constants.ARCFOUR256, 8, 0, 32, "ARCFOUR", 256, "RC4", 8) {
@Override
public Cipher create() {
return new BaseRC4Cipher(getIVSize(), getKdfSize(), getKeySize(),
getCipherBlockSize());
diff --git
a/sshd-core/src/test/java/org/apache/sshd/common/cipher/ArcFourOpenSshTest.java
b/sshd-core/src/test/java/org/apache/sshd/common/cipher/ArcFourOpenSshTest.java
new file mode 100644
index 000000000..0825b170d
--- /dev/null
+++
b/sshd-core/src/test/java/org/apache/sshd/common/cipher/ArcFourOpenSshTest.java
@@ -0,0 +1,139 @@
+/*
+ * 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.common.cipher;
+
+import java.security.Security;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import org.apache.sshd.client.SshClient;
+import org.apache.sshd.client.future.AuthFuture;
+import org.apache.sshd.client.session.ClientSession;
+import org.apache.sshd.common.keyprovider.FileKeyPairProvider;
+import org.apache.sshd.common.mac.BuiltinMacs;
+import org.apache.sshd.util.test.BaseTestSupport;
+import org.apache.sshd.util.test.CommonTestSupportUtils;
+import org.apache.sshd.util.test.ContainerTestCase;
+import org.bouncycastle.jce.provider.BouncyCastleProvider;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testcontainers.containers.GenericContainer;
+import org.testcontainers.containers.output.Slf4jLogConsumer;
+import org.testcontainers.containers.wait.strategy.Wait;
+import org.testcontainers.images.builder.ImageFromDockerfile;
+import org.testcontainers.utility.MountableFile;
+
+/**
+ * Test RC4 ciphers against OpenSSH 7.4.
+ *
+ * @author <a href="mailto:d...@mina.apache.org";>Apache MINA SSHD Project</a>
+ */
+@RunWith(Parameterized.class)
+@Category(ContainerTestCase.class)
+public class ArcFourOpenSshTest extends BaseTestSupport {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(ArcFourOpenSshTest.class);
+
+ // Re-use an already defined key
+ private static final String TEST_RESOURCES =
"org/apache/sshd/common/kex/extensions/client";
+
+ @Rule
+ public GenericContainer<?> sshdContainer = new GenericContainer<>(new
ImageFromDockerfile()
+ .withDockerfileFromBuilder(builder -> builder //
+ // Use old CentOS to get an OpenSSH that supports arcfour
+ .from("centos:7.9.2009") //
+ .run("yum install -y openssh-server") // Installs OpenSSH
7.4
+ // Enable deprecated ciphers
+ .run("echo 'Ciphers +arcfour128,arcfour256' >>
/etc/ssh/sshd_config")
+ .run("echo 'MACs
+hmac-md5,hmac-md5-96,hmac-sha1,hmac-sha1-96' >> /etc/ssh/sshd_config")
+ .run("/usr/sbin/sshd-keygen") // Generate multiple host
keys
+ .run("adduser bob") // Add a user
+ .run("echo \\\"123qweASD\\\" | passwd bob --stdin") //
Give it a password to unlock the user
+ .run("mkdir -p /home/bob/.ssh") // Create the SSH config
directory
+ .entryPoint("/entrypoint.sh") // Sets bob as owner of
anything under /home/bob and launches sshd
+ .build())) //
+
.withCopyFileToContainer(MountableFile.forClasspathResource(TEST_RESOURCES +
"/bob_key.pub"),
+ "/home/bob/.ssh/authorized_keys")
+ // entrypoint must be executable. Spotbugs doesn't
like 0777, so use hex
+ .withCopyFileToContainer(
+
MountableFile.forClasspathResource(TEST_RESOURCES + "/entrypoint.sh", 0x1ff),
+ "/entrypoint.sh")
+ .waitingFor(Wait.forLogMessage(".*Server listening
on :: port 22.*\\n", 1)).withExposedPorts(22) //
+ .withLogConsumer(new Slf4jLogConsumer(LOG));
+
+ private final BuiltinCiphers builtIn;
+
+ private final BuiltinMacs mac;
+
+ public ArcFourOpenSshTest(String providerName, BuiltinCiphers factory,
String name, BuiltinMacs mac, String macName) {
+ this.builtIn = factory;
+ this.mac = mac;
+ if ("BC".equals(providerName)) {
+ registerBouncyCastleProviderIfNecessary();
+ }
+ }
+
+ private static void registerBouncyCastleProviderIfNecessary() {
+ if (Security.getProvider(BouncyCastleProvider.PROVIDER_NAME) == null) {
+ Security.addProvider(new BouncyCastleProvider());
+ }
+ }
+
+ private static void addCipher(BuiltinCiphers cipherFactory, List<Object[]>
items) {
+ for (BuiltinMacs mac : BuiltinMacs.VALUES) {
+ items.add(new Object[] { "SunJCE", cipherFactory,
cipherFactory.getName(), mac, mac.getName() });
+ items.add(new Object[] { "BC", cipherFactory,
cipherFactory.getName(), mac, mac.getName() });
+ }
+ }
+
+ @SuppressWarnings("deprecation")
+ @Parameters(name = "{2} - {4} - {0}")
+ public static List<Object[]> getParameters() {
+ List<Object[]> items = new ArrayList<>();
+ addCipher(BuiltinCiphers.arcfour128, items);
+ addCipher(BuiltinCiphers.arcfour256, items);
+ return items;
+ }
+
+ @Test
+ public void testConnection() throws Exception {
+ FileKeyPairProvider keyPairProvider =
CommonTestSupportUtils.createTestKeyPairProvider(TEST_RESOURCES + "/bob_key");
+ SshClient client = setupTestClient();
+ client.setKeyIdentityProvider(keyPairProvider);
+ client.setCipherFactories(Collections.singletonList(builtIn));
+ client.setMacFactories(Collections.singletonList(mac));
+ client.start();
+
+ Integer actualPort = sshdContainer.getMappedPort(22);
+ String actualHost = sshdContainer.getHost();
+ try (ClientSession session = client.connect("bob", actualHost,
actualPort).verify(CONNECT_TIMEOUT).getSession()) {
+ AuthFuture authed = session.auth().verify(AUTH_TIMEOUT);
+ assertTrue(authed.isDone() && authed.isSuccess());
+ } finally {
+ client.stop();
+ }
+ }
+}
diff --git a/sshd-mina/pom.xml b/sshd-mina/pom.xml
index 81fde636b..fcdb73fd0 100644
--- a/sshd-mina/pom.xml
+++ b/sshd-mina/pom.xml
@@ -115,6 +115,7 @@
<!-- These tests use NIO2 explicitly -->
<exclude>**/Nio2ServiceTest.java</exclude>
<!-- testcontainers filesystem building from classpath
doesn't work from reusable test jar classpath -->
+ <exclude>**/ArcFourOpenSshTest.java</exclude>
<exclude>**/ClientOpenSSHCertificatesTest.java</exclude>
<exclude>**/SessionReKeyHostKeyExchangeTest.java</exclude>
<exclude>**/HostBoundPubKeyAuthTest.java</exclude>
diff --git a/sshd-netty/pom.xml b/sshd-netty/pom.xml
index ca6049f23..b26041a38 100644
--- a/sshd-netty/pom.xml
+++ b/sshd-netty/pom.xml
@@ -134,6 +134,7 @@
<!-- These tests use NIO2 explicitly -->
<exclude>**/Nio2ServiceTest.java</exclude>
<!-- testcontainers filesystem building from classpath
doesn't work from reusable test jar classpath -->
+ <exclude>**/ArcFourOpenSshTest.java</exclude>
<exclude>**/ClientOpenSSHCertificatesTest.java</exclude>
<exclude>**/SessionReKeyHostKeyExchangeTest.java</exclude>
<exclude>**/HostBoundPubKeyAuthTest.java</exclude>
