From da7a88bceae32c66b54f4f1cbf331213663db219 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Sat, 3 Feb 2024 22:22:16 +0100 Subject: [ssh] Implement the "Ciphers" SSH config Upstream will remove the CBC algorithms aes128-cbc, aes192-cbc, and aes256-cbc from the server's KEX proposal in the next release. Removal of these algorithms by default in the client is planned for the release after that. These CBC algorithms were found vulnerable back in 2008,[1] and OpenSSH does not propose them: server-side since 2014, client-side since 2017. It is _highly_ unlikely that the removal of these algorithms by default would affect any JGit user. Nevertheless, let's give users a way to explicitly specify ciphers (including enabling deprecated algorithms) via their ~/.ssh/config file. [1] https://www.kb.cert.org/vuls/id/958563 Change-Id: I7444861df3a7f526277fef2485773a20ac74ae8a Signed-off-by: Thomas Wolf --- .../eclipse/jgit/transport/sshd/ApacheSshTest.java | 33 ++++++++++++ org.eclipse.jgit.ssh.apache/META-INF/MANIFEST.MF | 1 + .../internal/transport/sshd/JGitClientSession.java | 62 ++++++++++++++++++++-- 3 files changed, 92 insertions(+), 4 deletions(-) diff --git a/org.eclipse.jgit.ssh.apache.test/tst/org/eclipse/jgit/transport/sshd/ApacheSshTest.java b/org.eclipse.jgit.ssh.apache.test/tst/org/eclipse/jgit/transport/sshd/ApacheSshTest.java index a8fcca7b8e..873945780f 100644 --- a/org.eclipse.jgit.ssh.apache.test/tst/org/eclipse/jgit/transport/sshd/ApacheSshTest.java +++ b/org.eclipse.jgit.ssh.apache.test/tst/org/eclipse/jgit/transport/sshd/ApacheSshTest.java @@ -861,4 +861,37 @@ public class ApacheSshTest extends SshTestBase { verifyAuthLog(e.getMessage(), "log in"); } + @Test + public void testCipherModificationSingle() throws Exception { + cloneWith( + "ssh://" + TEST_USER + "@localhost:" + testPort + + "/doesntmatter", + defaultCloneDir, null, + "IdentityFile " + privateKey1.getAbsolutePath(), + "Ciphers aes192-ctr"); + } + + @Test + public void testCipherModificationAdd() throws Exception { + cloneWith( + "ssh://" + TEST_USER + "@localhost:" + testPort + + "/doesntmatter", + defaultCloneDir, null, + "IdentityFile " + privateKey1.getAbsolutePath(), + "Ciphers +3des-cbc"); + } + + @Test + public void testCipherModificationUnknown() throws Exception { + TransportException e = assertThrows(TransportException.class, + () -> cloneWith( + "ssh://" + TEST_USER + "@localhost:" + testPort + + "/doesntmatter", + defaultCloneDir, null, + "IdentityFile " + privateKey1.getAbsolutePath(), + // The server is not configured to use this deprecated + // algorithm + "Ciphers 3des-cbc")); + assertTrue(e.getLocalizedMessage().contains("3des-cbc")); + } } diff --git a/org.eclipse.jgit.ssh.apache/META-INF/MANIFEST.MF b/org.eclipse.jgit.ssh.apache/META-INF/MANIFEST.MF index 74e8c0488b..50a77bdbce 100644 --- a/org.eclipse.jgit.ssh.apache/META-INF/MANIFEST.MF +++ b/org.eclipse.jgit.ssh.apache/META-INF/MANIFEST.MF @@ -53,6 +53,7 @@ Import-Package: net.i2p.crypto.eddsa;version="[0.3.0,0.4.0)", org.apache.sshd.common;version="[2.12.0,2.13.0)", org.apache.sshd.common.auth;version="[2.12.0,2.13.0)", org.apache.sshd.common.channel;version="[2.12.0,2.13.0)", + org.apache.sshd.common.cipher;version="[2.12.0,2.13.0)", org.apache.sshd.common.compression;version="[2.12.0,2.13.0)", org.apache.sshd.common.config.keys;version="[2.12.0,2.13.0)", org.apache.sshd.common.config.keys.loader;version="[2.12.0,2.13.0)", diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitClientSession.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitClientSession.java index c19a04d7e5..32d6facbc3 100644 --- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitClientSession.java +++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitClientSession.java @@ -20,6 +20,7 @@ import java.security.PublicKey; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.EnumSet; import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashSet; @@ -28,6 +29,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.function.Supplier; +import java.util.stream.Collectors; import org.apache.sshd.client.ClientBuilder; import org.apache.sshd.client.ClientFactoryManager; @@ -36,8 +38,12 @@ import org.apache.sshd.client.keyverifier.ServerKeyVerifier; import org.apache.sshd.client.session.ClientSessionImpl; import org.apache.sshd.common.AttributeRepository; import org.apache.sshd.common.FactoryManager; +import org.apache.sshd.common.NamedFactory; import org.apache.sshd.common.NamedResource; import org.apache.sshd.common.PropertyResolver; +import org.apache.sshd.common.cipher.BuiltinCiphers; +import org.apache.sshd.common.cipher.Cipher; +import org.apache.sshd.common.cipher.CipherFactory; import org.apache.sshd.common.config.keys.KeyUtils; import org.apache.sshd.common.io.IoSession; import org.apache.sshd.common.io.IoWriteFuture; @@ -86,12 +92,21 @@ public class JGitClientSession extends ClientSessionImpl { */ private static final int DEFAULT_MAX_IDENTIFICATION_SIZE = 64 * 1024; - private static final AttributeKey INITIAL_KEX_DONE = new AttributeKey<>(); + /** + * Cipher implementations that we never ever want to use, even if Apache + * MINA SSHD has implementations for them. + */ + private static final Set FORBIDDEN_CIPHERS = EnumSet + .of(BuiltinCiphers.none); private HostConfigEntry hostConfig; private CredentialsProvider credentialsProvider; + private boolean isInitialKex = true; + + private List> ciphers; + private volatile StatefulProxyConnector proxyHandler; /** @@ -349,8 +364,7 @@ public class JGitClientSession extends ClientSessionImpl { protected String resolveSessionKexProposal(String hostKeyTypes) throws IOException { String kexMethods = String.join(",", determineKexProposal()); //$NON-NLS-1$ - Boolean isRekey = getAttribute(INITIAL_KEX_DONE); - if (isRekey == null || !isRekey.booleanValue()) { + if (isInitialKex) { // First time KexExtensionHandler extHandler = getKexExtensionHandler(); if (extHandler != null && extHandler.isKexExtensionsAvailable(this, @@ -361,7 +375,7 @@ public class JGitClientSession extends ClientSessionImpl { kexMethods += ',' + KexExtensions.CLIENT_KEX_EXTENSION; } } - setAttribute(INITIAL_KEX_DONE, Boolean.TRUE); + isInitialKex = false; } if (log.isDebugEnabled()) { log.debug(SshConstants.KEX_ALGORITHMS + ' ' + kexMethods); @@ -369,6 +383,46 @@ public class JGitClientSession extends ClientSessionImpl { return kexMethods; } + @Override + public List> getCipherFactories() { + if (ciphers == null) { + List> defaultCiphers = super.getCipherFactories(); + HostConfigEntry config = resolveAttribute( + JGitSshClient.HOST_CONFIG_ENTRY); + String algorithms = config.getProperty(SshConstants.CIPHERS); + if (!StringUtils.isEmptyOrNull(algorithms)) { + List defaultCipherNames = defaultCiphers + .stream().map(NamedFactory::getName) + .collect(Collectors.toCollection(ArrayList::new)); + Set allKnownCiphers = new HashSet<>(); + BuiltinCiphers.VALUES.stream() + .filter(c -> !FORBIDDEN_CIPHERS.contains(c)) + .filter(CipherFactory::isSupported) + .forEach(c -> allKnownCiphers.add(c.getName())); + BuiltinCiphers.getRegisteredExtensions().stream() + .filter(CipherFactory::isSupported) + .forEach(c -> allKnownCiphers.add(c.getName())); + List sessionCipherNames = modifyAlgorithmList( + defaultCipherNames, allKnownCiphers, algorithms, + SshConstants.CIPHERS); + if (sessionCipherNames.isEmpty()) { + log.warn(format(SshdText.get().configNoKnownAlgorithms, + SshConstants.CIPHERS, algorithms)); + ciphers = defaultCiphers; + } else { + List> sessionCiphers = new ArrayList<>( + sessionCipherNames.size()); + sessionCipherNames.forEach(name -> sessionCiphers + .add(BuiltinCiphers.resolveFactory(name))); + ciphers = sessionCiphers; + } + } else { + ciphers = defaultCiphers; + } + } + return ciphers; + } + /** * Modifies a given algorithm list according to a list from the ssh config, * including add ('+'), remove ('-') and reordering ('^') operators. -- cgit v1.2.3