From 115dc1e145ab8964c5b333858f8b7c1d2f60e504 Mon Sep 17 00:00:00 2001 From: Auke Zaaiman Date: Sun, 26 Mar 2023 10:23:43 +0200 Subject: [PATCH 1/2] GH-8581: Do not overwrite configuration of external provided SshClient Fixes spring-projects/spring-integration#8581 --- .../session/DefaultSftpSessionFactory.java | 63 +++++++++++-------- .../sftp/session/SftpSessionFactoryTests.java | 26 ++++++++ 2 files changed, 62 insertions(+), 27 deletions(-) diff --git a/spring-integration-sftp/src/main/java/org/springframework/integration/sftp/session/DefaultSftpSessionFactory.java b/spring-integration-sftp/src/main/java/org/springframework/integration/sftp/session/DefaultSftpSessionFactory.java index 53cf9c10f02..16d7a32368f 100644 --- a/spring-integration-sftp/src/main/java/org/springframework/integration/sftp/session/DefaultSftpSessionFactory.java +++ b/spring-integration-sftp/src/main/java/org/springframework/integration/sftp/session/DefaultSftpSessionFactory.java @@ -61,6 +61,7 @@ * @author Pat Turner * @author Artem Bilan * @author Krzysztof Debski + * @author Auke Zaaiman * * @since 2.0 */ @@ -335,36 +336,44 @@ private void doInitClient() throws IOException { if (this.port <= 0) { this.port = SshConstants.DEFAULT_PORT; } - ServerKeyVerifier serverKeyVerifier = - this.allowUnknownKeys ? AcceptAllServerKeyVerifier.INSTANCE : RejectAllServerKeyVerifier.INSTANCE; - if (this.knownHosts != null) { - serverKeyVerifier = new ResourceKnownHostsServerKeyVerifier(this.knownHosts); - } - this.sshClient.setServerKeyVerifier(serverKeyVerifier); - - this.sshClient.setPasswordIdentityProvider(PasswordIdentityProvider.wrapPasswords(this.password)); - if (this.privateKey != null) { - IoResource privateKeyResource = - new AbstractIoResource<>(Resource.class, this.privateKey) { - - @Override - public InputStream openInputStream() throws IOException { - return getResourceValue().getInputStream(); - } - }; - try { - Collection keys = - SecurityUtils.getKeyPairResourceParser() - .loadKeyPairs(null, privateKeyResource, - FilePasswordProvider.of(this.privateKeyPassphrase)); - this.sshClient.setKeyIdentityProvider(KeyIdentityProvider.wrapKeyPairs(keys)); + + doInitInnerClient(); + + this.sshClient.start(); + } + + private void doInitInnerClient() throws IOException { + if (this.isInnerClient) { + ServerKeyVerifier serverKeyVerifier = + this.allowUnknownKeys ? AcceptAllServerKeyVerifier.INSTANCE : RejectAllServerKeyVerifier.INSTANCE; + if (this.knownHosts != null) { + serverKeyVerifier = new ResourceKnownHostsServerKeyVerifier(this.knownHosts); } - catch (GeneralSecurityException ex) { - throw new IOException("Cannot load private key: " + this.privateKey.getFilename(), ex); + this.sshClient.setServerKeyVerifier(serverKeyVerifier); + + this.sshClient.setPasswordIdentityProvider(PasswordIdentityProvider.wrapPasswords(this.password)); + if (this.privateKey != null) { + IoResource privateKeyResource = + new AbstractIoResource<>(Resource.class, this.privateKey) { + + @Override + public InputStream openInputStream() throws IOException { + return getResourceValue().getInputStream(); + } + }; + try { + Collection keys = + SecurityUtils.getKeyPairResourceParser() + .loadKeyPairs(null, privateKeyResource, + FilePasswordProvider.of(this.privateKeyPassphrase)); + this.sshClient.setKeyIdentityProvider(KeyIdentityProvider.wrapKeyPairs(keys)); + } + catch (GeneralSecurityException ex) { + throw new IOException("Cannot load private key: " + this.privateKey.getFilename(), ex); + } } + this.sshClient.setUserInteraction(this.userInteraction); } - this.sshClient.setUserInteraction(this.userInteraction); - this.sshClient.start(); } @Override diff --git a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/SftpSessionFactoryTests.java b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/SftpSessionFactoryTests.java index eaa1c9ef43b..d4b8e104d18 100644 --- a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/SftpSessionFactoryTests.java +++ b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/SftpSessionFactoryTests.java @@ -23,6 +23,9 @@ import java.util.Collections; import java.util.List; +import org.apache.sshd.client.SshClient; +import org.apache.sshd.client.auth.password.PasswordIdentityProvider; +import org.apache.sshd.client.keyverifier.AcceptAllServerKeyVerifier; import org.apache.sshd.common.SshException; import org.apache.sshd.server.SshServer; import org.apache.sshd.server.keyprovider.SimpleGeneratorHostKeyProvider; @@ -35,10 +38,12 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.fail; import static org.awaitility.Awaitility.await; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; /** * @author Gary Russell * @author Artem Bilan + * @author Auke Zaaiman * * @since 3.0.2 */ @@ -126,4 +131,25 @@ public void concurrentGetSessionDoesntCauseFailure() throws IOException { } } + @Test + void externallyProvidedSshClientShouldNotHaveItsConfigurationOverwritten() throws IOException { + try (SshServer server = SshServer.setUpDefaultServer()) { + server.setPasswordAuthenticator((arg0, arg1, arg2) -> true); + server.setPort(0); + server.setKeyPairProvider(new SimpleGeneratorHostKeyProvider(new File("hostkey.ser").toPath())); + server.setSubsystemFactories(Collections.singletonList(new SftpSubsystemFactory())); + server.start(); + + SshClient externalClient = SshClient.setUpDefaultClient(); + externalClient.setServerKeyVerifier(AcceptAllServerKeyVerifier.INSTANCE); + externalClient.setPasswordIdentityProvider(PasswordIdentityProvider.wrapPasswords("pass")); + + DefaultSftpSessionFactory sftpSessionFactory = new DefaultSftpSessionFactory(externalClient, false); + sftpSessionFactory.setHost("localhost"); + sftpSessionFactory.setPort(server.getPort()); + sftpSessionFactory.setUser("user"); + + assertDoesNotThrow(() -> sftpSessionFactory.getSession().connect()); + } + } } From cb1f108b5dada1e399a9f0d43122602032bd9309 Mon Sep 17 00:00:00 2001 From: Auke Zaaiman Date: Tue, 28 Mar 2023 07:20:45 +0200 Subject: [PATCH 2/2] GH-8581: Do not overwrite configuration of external provided SshClient replace JUnit assertDoesNotThrow by AssertJ assertThatNoException in test --- .../integration/sftp/session/SftpSessionFactoryTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/SftpSessionFactoryTests.java b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/SftpSessionFactoryTests.java index d4b8e104d18..237b97739df 100644 --- a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/SftpSessionFactoryTests.java +++ b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/SftpSessionFactoryTests.java @@ -36,9 +36,9 @@ import org.springframework.core.task.SimpleAsyncTaskExecutor; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatNoException; import static org.assertj.core.api.Assertions.fail; import static org.awaitility.Awaitility.await; -import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; /** * @author Gary Russell @@ -149,7 +149,7 @@ void externallyProvidedSshClientShouldNotHaveItsConfigurationOverwritten() throw sftpSessionFactory.setPort(server.getPort()); sftpSessionFactory.setUser("user"); - assertDoesNotThrow(() -> sftpSessionFactory.getSession().connect()); + assertThatNoException().isThrownBy(() -> sftpSessionFactory.getSession()); } } }