From cc22b88a5b1105deddaf1d954373decc6d158706 Mon Sep 17 00:00:00 2001 From: Kaveh Shamsi Date: Tue, 26 Jul 2022 16:57:08 -0400 Subject: [PATCH] Replaces Jsch with Apache MINA in config-server (#2113) * Bumps jgit to 6.2.x * FileBasedSshSessionFacotry sets the properties only for relevant host Co-authored-by: Ryan Baxter --- spring-cloud-config-dependencies/pom.xml | 2 +- .../server/ssh/FileBasedSshSessionFactory.java | 15 +++++++++++---- .../FileBasedSshTransportConfigCallback.java | 3 ++- .../ssh/PropertyBasedSshSessionFactory.java | 13 +++++++++++++ .../ssh/FileBasedSshSessionFactoryTest.java | 17 ++++++++++++++++- .../PropertyBasedSshSessionFactoryTest.java | 18 +++++++++++++++++- 6 files changed, 60 insertions(+), 8 deletions(-) diff --git a/spring-cloud-config-dependencies/pom.xml b/spring-cloud-config-dependencies/pom.xml index c776061e..4b1e5170 100644 --- a/spring-cloud-config-dependencies/pom.xml +++ b/spring-cloud-config-dependencies/pom.xml @@ -15,7 +15,7 @@ spring-cloud-config-dependencies Spring Cloud Config Dependencies - 5.13.1.202206130422-r + 6.2.0.202206071550-r 3.0.0-M2 2.1.1.RELEASE diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/ssh/FileBasedSshSessionFactory.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/ssh/FileBasedSshSessionFactory.java index 94d81a90..a236c3c6 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/ssh/FileBasedSshSessionFactory.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/ssh/FileBasedSshSessionFactory.java @@ -17,6 +17,7 @@ package org.springframework.cloud.config.server.ssh; import java.io.File; +import java.util.Map; import org.eclipse.jgit.annotations.NonNull; import org.eclipse.jgit.internal.transport.ssh.OpenSshConfigFile; @@ -33,10 +34,11 @@ public class FileBasedSshSessionFactory extends SshdSessionFactory { private static final String NO_OPTION = "no"; - private final JGitEnvironmentProperties sshUriProperties; + private final Map sshKeysByHostname; - public FileBasedSshSessionFactory(JGitEnvironmentProperties sshUriProperties) { - this.sshUriProperties = sshUriProperties; + public FileBasedSshSessionFactory(Map sshKeysByHostname) { + this.sshKeysByHostname = sshKeysByHostname; + assert this.sshKeysByHostname.entrySet().size() > 0; } @Override @@ -47,8 +49,13 @@ public class FileBasedSshSessionFactory extends SshdSessionFactory { public HostEntry lookup(@NonNull String hostName, int port, String userName) { HostEntry hostEntry = super.lookup(hostName, port, userName); + JGitEnvironmentProperties sshProperties = sshKeysByHostname.get(hostName); + if (sshProperties == null) { + return hostEntry; + } + hostEntry.setValue(STRICT_HOST_KEY_CHECKING, - sshUriProperties.isStrictHostKeyChecking() ? YES_OPTION : NO_OPTION); + sshProperties.isStrictHostKeyChecking() ? YES_OPTION : NO_OPTION); return hostEntry; } diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/ssh/FileBasedSshTransportConfigCallback.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/ssh/FileBasedSshTransportConfigCallback.java index fa408c64..246e7d43 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/ssh/FileBasedSshTransportConfigCallback.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/ssh/FileBasedSshTransportConfigCallback.java @@ -43,7 +43,8 @@ public class FileBasedSshTransportConfigCallback implements TransportConfigCallb @Override public void configure(Transport transport) { if (transport instanceof SshTransport) { - ((SshTransport) transport).setSshSessionFactory(new FileBasedSshSessionFactory(sshUriProperties)); + ((SshTransport) transport).setSshSessionFactory(new FileBasedSshSessionFactory( + new SshUriPropertyProcessor(this.sshUriProperties).getSshKeysByHostname())); } } diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/ssh/PropertyBasedSshSessionFactory.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/ssh/PropertyBasedSshSessionFactory.java index 24a57e71..388b32d3 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/ssh/PropertyBasedSshSessionFactory.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/ssh/PropertyBasedSshSessionFactory.java @@ -40,6 +40,7 @@ import org.eclipse.jgit.internal.transport.ssh.OpenSshConfigFile; import org.eclipse.jgit.internal.transport.sshd.OpenSshServerKeyDatabase; import org.eclipse.jgit.transport.CredentialsProvider; import org.eclipse.jgit.transport.SshConfigStore; +import org.eclipse.jgit.transport.SshConstants; import org.eclipse.jgit.transport.sshd.JGitKeyCache; import org.eclipse.jgit.transport.sshd.ProxyData; import org.eclipse.jgit.transport.sshd.ProxyDataFactory; @@ -88,6 +89,18 @@ public class PropertyBasedSshSessionFactory extends SshdSessionFactory { return updateIfNeeded(hostEntry, hostName); } + @Override + public HostConfig lookupDefault(String hostName, int port, String userName) { + OpenSshConfigFile.HostEntry hostEntry = new OpenSshConfigFile.HostEntry(); + + hostEntry.setValue(SshConstants.HOST_NAME, hostName); + hostEntry.setValue(SshConstants.PORT, + Integer.toString(port > 0 ? port : SshConstants.SSH_DEFAULT_PORT)); + hostEntry.setValue(SshConstants.USER, userName); + + return updateIfNeeded(hostEntry, hostName); + } + private OpenSshConfigFile.HostEntry updateIfNeeded(OpenSshConfigFile.HostEntry hostEntry, String hostName) { JGitEnvironmentProperties sshProperties = sshKeysByHostname.get(hostName); if (sshProperties == null) { diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/ssh/FileBasedSshSessionFactoryTest.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/ssh/FileBasedSshSessionFactoryTest.java index 17dcbc26..0fc31172 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/ssh/FileBasedSshSessionFactoryTest.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/ssh/FileBasedSshSessionFactoryTest.java @@ -17,6 +17,8 @@ package org.springframework.cloud.config.server.ssh; import java.io.File; +import java.util.HashMap; +import java.util.Map; import org.eclipse.jgit.transport.SshConfigStore; import org.junit.Test; @@ -58,6 +60,17 @@ public class FileBasedSshSessionFactoryTest { assertThat(sshConfig.getValue("StrictHostKeyChecking")).isEqualTo("yes"); } + @Test + public void sshConfigurationIsDoneForRelevantHostOnly() { + JGitEnvironmentProperties sshKey = new JGitEnvironmentProperties(); + sshKey.setUri("ssh://gitlab.example.local:3322/somerepo.git"); + setupSessionFactory(sshKey); + + SshConfigStore.HostConfig sshConfig = getSshHostConfig("another.host"); + + assertThat(sshConfig.getValue("StrictHostKeyChecking")).isNull(); + } + @Test public void handlesNullConfigFile() { JGitEnvironmentProperties sshKey = new JGitEnvironmentProperties(); @@ -75,7 +88,9 @@ public class FileBasedSshSessionFactoryTest { } private void setupSessionFactory(JGitEnvironmentProperties sshKey) { - this.factory = new FileBasedSshSessionFactory(sshKey); + Map sshKeysByHostname = new HashMap<>(); + sshKeysByHostname.put(SshUriPropertyProcessor.getHostname(sshKey.getUri()), sshKey); + this.factory = new FileBasedSshSessionFactory(sshKeysByHostname); } } diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/ssh/PropertyBasedSshSessionFactoryTest.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/ssh/PropertyBasedSshSessionFactoryTest.java index 7b060505..4af85cb4 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/ssh/PropertyBasedSshSessionFactoryTest.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/ssh/PropertyBasedSshSessionFactoryTest.java @@ -209,7 +209,7 @@ public class PropertyBasedSshSessionFactoryTest { } @Test - public void proxySettingsIsUsed() throws Exception { + public void proxySettingsIsUsed() { JGitEnvironmentProperties sshProperties = new JGitEnvironmentProperties(); sshProperties.setUri("ssh://gitlab.example.local:3322/somerepo.git"); sshProperties.setPrivateKey(PRIVATE_KEY); @@ -231,6 +231,17 @@ public class PropertyBasedSshSessionFactoryTest { assertThat(proxyData.getProxy().address().toString()).containsPattern("host\\.domain.*:8080"); } + @Test + public void defaultSshConfigIsSet() { + setupSessionFactory(new JGitEnvironmentProperties()); + + SshConfigStore.HostConfig sshConfig = getDefaultSshHostConfig("host.name", 123, "user.name"); + + assertThat(sshConfig.getValue("HostName")).isEqualTo("host.name"); + assertThat(sshConfig.getValue("Port")).isEqualTo("123"); + assertThat(sshConfig.getValue("User")).isEqualTo("user.name"); + } + @Test public void sshConfigFileIsNotUsed() { setupSessionFactory(new JGitEnvironmentProperties()); @@ -309,6 +320,11 @@ public class PropertyBasedSshSessionFactoryTest { "userName"); } + private SshConfigStore.HostConfig getDefaultSshHostConfig(String hostName, int port, String username) { + return factory.createSshConfigStore(new File("dummy"), new File("dummy"), "localUserName") + .lookupDefault(hostName, port, username); + } + private void setupSessionFactory(JGitEnvironmentProperties sshKey) { Map sshKeysByHostname = new HashMap<>(); sshKeysByHostname.put(SshUriPropertyProcessor.getHostname(sshKey.getUri()), sshKey);