spring-integration icon indicating copy to clipboard operation
spring-integration copied to clipboard

[SFTP] [v6 Regression] ResourceKnownHostsServerKeyVerifier does not match by IP

Open jeanblanchard opened this issue 2 years ago • 17 comments

In what version(s) of Spring Integration are you seeing this issue?

  • 6.0.5 (Spring Boot 3.0.6)
  • Still in the main branch

Describe the bug

When using a DefaultSftpSessionFactory with knownHostsResource set, the known_hosts are matched only by hostname, and not by IP.

Even if host is set to an IP address in the configuration, only its reverse DNS is matched.

Exception from the log (redacted)

o.a.s.client.session.ClientSessionImpl   : exceptionCaught(ClientSessionImpl[[email protected]/203.0.113.1:2222])[state=Opened] SshException: Server key did not validate

To Reproduce

Configure a DefaultSftpSessionFactory, with host set to an IP address, and knownHostsResource set to a classpath known_host file with a public key configured for the IP.

Expected behavior

The known_hosts should be matched with the IP from the host config param, like it did in Spring Integration SFTP < 6.

Analyzing a bit more

ResourceKnownHostsServerKeyVerifier.resolveHostNetworkIdentities() calls SshdSocketAddress.toSshdSocketAddress, which always fetches the hostname from the connect address's IP.

This behavior appears to be copied from Apache Mina's KnownHostsServerKeyVerifier. It might well be on purpose on their side to only use hostnames, but this is a regression from the behavior in previous versions of Spring Integration SFTP.

Maybe ResourceKnownHostsServerKeyVerifier.resolveHostNetworkIdentities() should (in addition?) return the raw address from clientSession.getConnectAddress()?

jeanblanchard avatar Aug 01 '23 09:08 jeanblanchard

It is hard to test this with mocks, so I'd be glad to see more stack trace for that error to determine where exactly we should catch it and fallback to the IP address.

Correct me if I'm wrong.

  1. You have an entry in the known_hosts which has only an IP address: https://en.wikibooks.org/wiki/OpenSSH/Client_Configuration_Files#About_the_Contents_of_the_known_hosts_Files
  2. You configure exactly same IP address for an SFTP session.
  3. This IP address is resolved to its host name, but it doesn't match to the entry in the known_hosts because there is no host name indicator for the key.

As a workaround you can specify that host name in the known_hosts entry according to the doc mentioned above:

It is possible to use a comma-separated list of hosts in the host name field if a host has multiple names

Does it all make sense?

artembilan avatar Aug 01 '23 17:08 artembilan

Another workaround is to provide an externally configured SshClient:

	/**
	 * Instantiate based on the provided {@link SshClient}, e.g. some extension for HTTP/SOCKS.
	 * @param sshClient the {@link SshClient} instance.
	 * @param isSharedSession true if the session is to be shared.
	 */
	public DefaultSftpSessionFactory(SshClient sshClient, boolean isSharedSession) {

This way you can inject any custom setServerKeyVerifier(). But if you can do that, why just don't come back to us with a contribution for a fix you are asking here for? I'm telling this because I still don't see the whole picture where we are failing...

artembilan avatar Aug 03 '23 19:08 artembilan

Well, I have looked into the previous 5.5.x version which is based on JSCH SSH implementation, and it does nothing for the provided host or the entry from the known_hosts. It is just:

return hosts.regionMatches(true, i, _host, 0, hostlen);

Where _host is the value we provide for DefaultSftpSessionFactory and hosts is the host part of the known_hosts entry, like in the sample of the mentioned doc:

[anga.funkfeuer.at]:2022,[78.41.115.130]:2022 ssh-rsa AAAAB...fgTHaojQ==	

So, no way it is going to work in that version if there is no IP variant in the entry. Therefore it is hard to claim that it worked before since it has met your current requirements, but may fail in other use-cases.

But that is only if I look into a proper direction for what I'd like to hear from you back.

Thank

artembilan avatar Aug 03 '23 19:08 artembilan

We have the exact same issue. If i only downgrade from version 6.1.5 spring-integration-sftp to version 5.5.20 it works! i have normal known_hosts file with a host name without ip-adress.

Of course if have to change a bit the code because of the missing IntegrationFlows in version 6.1.5. for example if have to change:

@Bean
    public IntegrationFlow sftpSendFlow(SessionFactory<SftpClient.DirEntry> sftpSessionFactory) {
        return IntegrationFlow.from("sftpSendChannel").handle(Sftp.outboundAdapter(sftpSessionFactory, FileExistsMode.REPLACE).useTemporaryFileName(false)
                .remoteDirectoryExpression("headers['remoteDirectory']").fileNameGenerator(SftpEndpointFactory::createFileName)).get();
    }

back to

@Bean
    public IntegrationFlow sftpSendFlow(SessionFactory<ChannelSftp.LsEntry> sftpSessionFactory) {
        return IntegrationFlows.from("sftpSendChannel").handle(Sftp.outboundAdapter(sftpSessionFactory, FileExistsMode.REPLACE).useTemporaryFileName(false)
                .remoteDirectoryExpression("headers['remoteDirectory']").fileNameGenerator(SftpEndpointFactory::createFileName)).get();
    }

or change the

@Bean
    public SessionFactory<SftpClient.DirEntry> sftpSessionFactory(SftpDokumenterkennungConfig sftpDokumenterkennungConfig) {
        DefaultSftpSessionFactory defaultSftpSessionFactory = new DefaultSftpSessionFactory();
        defaultSftpSessionFactory.setKnownHostsResource(new ClassPathResource(KNOWN_HOSTS_FILE_NAME));
        defaultSftpSessionFactory.setHost(sftpDokumenterkennungConfig.getHost());
        defaultSftpSessionFactory.setPort(sftpDokumenterkennungConfig.getPort());
        defaultSftpSessionFactory.setUser(sftpDokumenterkennungConfig.getUser());
        defaultSftpSessionFactory.setPassword(sftpDokumenterkennungConfig.getPassword());

        return new CachingSessionFactory<>(defaultSftpSessionFactory);
    }

back to

@Bean
    public SessionFactory<ChannelSftp.LsEntry> sftpSessionFactory(SftpDokumenterkennungConfig sftpDokumenterkennungConfig) {
        DefaultSftpSessionFactory defaultSftpSessionFactory = new DefaultSftpSessionFactory();
        defaultSftpSessionFactory.setKnownHostsResource(new ClassPathResource(KNOWN_HOSTS_FILE_NAME));
        defaultSftpSessionFactory.setHost(sftpDokumenterkennungConfig.getHost());
        defaultSftpSessionFactory.setPort(sftpDokumenterkennungConfig.getPort());
        defaultSftpSessionFactory.setUser(sftpDokumenterkennungConfig.getUser());
        defaultSftpSessionFactory.setPassword(sftpDokumenterkennungConfig.getPassword());

        return new CachingSessionFactory<>(defaultSftpSessionFactory);
    }

but thats only very smal changes like i mentioned IntegrationFlows to IntegrationFlow or SftpClient.DirEntry to ChannelSftp.LsEntry

sebastianfilke avatar Feb 07 '24 10:02 sebastianfilke

Sorry, @sebastianfilke , but that's not helpful. What you show is a change between Spring Integration 5.5.x and 6.x. The real problem that we have migrated SFTP client from JCraft Jsch to Apache MINA and the last one does not support host resolution as it was in the former one. Therefore it would be great to have a fix in Apache MINA or as a workaround here in Spring Integration. However for the workaround I need to understand how to test the problem and know what to do to mitigate it. No one yet gave me the info is needed to work with.

Perhaps you can share what your known_hosts looks like and how you configure DefaultSftpSessionFactory to connect with that key.

Thanks for understanding!

artembilan avatar Feb 07 '24 16:02 artembilan

Hi @artembilan, is there any update on this topic?

I have the same issue as mentioned above, after migrating SB from 2.x to 3.2.

My known_hosts is structured as:

sftp.server.domain.com ssh-rsa AAAAB3Nz...

and set on DefaultSftpSessionFactory via sessionFactory.setKnownHostsResource(resource);

laguiar avatar May 17 '24 11:05 laguiar

Hi @laguiar !

Thank you for reaching out!

Would you mind to share more info? What is your DefaultSftpSessionFactory configuration? How does error looks like?

As you see in all the comments, no one has provided a proper environment to determine where we exactly are failing to find a path for fix.

artembilan avatar May 17 '24 14:05 artembilan

Hi @laguiar !

Thank you for reaching out!

Would you mind to share more info? What is your DefaultSftpSessionFactory configuration? How does error looks like?

As you see in all the comments, no one has provided a proper environment to determine where we exactly are failing to find a path for fix.

Sure... it's probably very standard, I don't have much customisation on this component.

    public DefaultSftpSessionFactory defineSftpSessionFactory() {
        var sessionFactory = new DefaultSftpSessionFactory();
        sessionFactory.setHost(host);
        sessionFactory.setPort(port);
        sessionFactory.setUser(user);
        sessionFactory.setPrivateKey(privateKey);
        sessionFactory.setPrivateKeyPassphrase(privateKeyPassphrase);
        sessionFactory.setTimeout(Math.toIntExact(timeout.toMillis()));
        sessionFactory.setAllowUnknownKeys(true);
        sessionFactory.setKnownHostsResource(knownHostsFile);
        var hostConfig = defineHostConfig();
        sessionFactory.setHostConfig(hostConfig);
        return sessionFactory;
    }

    private HostConfigEntry defineHostConfig() {
        var hostConfig = new HostConfigEntry(host, host, port, user);
        hostConfig.setProperty("MaxAuthTries", "3");
        hostConfig.setProperty("PreferredAuthentications", "publickey");
        return hostConfig;
    }

laguiar avatar May 17 '24 15:05 laguiar

And what is that host? What is the content of your knownHostsFile? What is the exception you got?

This is still so generic that it does not give any clues what might be wrong in the framework. And that's why this issue is still opened: no one provides any hints how to reproduce or what exactly doesn't work and has to be fixed respectively.

Thanks for understanding!

artembilan avatar May 17 '24 15:05 artembilan

@artembilan I just encountered a similar issue with my local integration tests. Not sure if it is the issue the original poster had, but it looks like @laguiar might at least be running into the same problem.

I believe my issue is related to this https://github.com/spring-projects/spring-integration/blob/4d33ea093c0bf3cdb01ffe07d17e3d60c51bfcfc/spring-integration-sftp/src/main/java/org/springframework/integration/sftp/session/DefaultSftpSessionFactory.java#L363-L367

Previously, if you specified both setAllowUnknownKeys as true and the setKnownHostsResource the key verification would be skipped regardless of what setKnownHostsResource was.

        // this session factory configuration works with 5.x.x but breaks with 6.x.x
        sessionFactory.setAllowUnknownKeys(true);
        sessionFactory.setKnownHostsResource(knownHostsFile); // knownHostsFile pointing to /dev/null

But now if the knownHosts file is ever not null, the allowUnknownKeys configuration is ignored.

For my case, the integration tests specified the knownHostsFile as /dev/null and allow unknown keys as true as we don't really care for the validations in our local integration tests. Setting the knownHostsFile to be null removed the error.

It's possible that users have a bad knownHosts file and are relying on setAllowUnknownKeys

jgormley6 avatar May 17 '24 18:05 jgormley6

@jgormley6 ,

thank you for sharing your experience, but I don't see how your problem is related to the original report. For your use-case see respective Javadocs:

	/**
	 * When no {@link #knownHosts} has been provided, set to true to unconditionally allow
	 * connecting to an unknown host or when a host's key has changed (see
	 * {@link #setKnownHostsResource(Resource) knownHosts}). Default false (since 4.2).
	 * Set to true if a knownHosts file is not provided.
	 * @param allowUnknownKeys true to allow connecting to unknown hosts.
	 * @since 4.1.7
	 */
	public void setAllowUnknownKeys(boolean allowUnknownKeys) {

So, the logic you show is correct and it really reflects those Javadocs.

I see an inconvenient with your configuration expectations, but that is what we might revise in the next 6.4 version.

Feel free to raise a new GH issue! However this one is about host/IP resolution problem which I'm still failed to compile from this discussion.

artembilan avatar May 17 '24 19:05 artembilan

And what is that host?

sftp.server.domain.com

What is the content of your knownHostsFile?

It's a Resource object pointing to a known_hosts file within the project resources folder. (format mentioned above)

What is the exception you got?

Caused by: org.apache.sshd.common.SshException: Server key did not validate

laguiar avatar May 19 '24 12:05 laguiar

@laguiar ,

may we have the whole stack trace to determine the relevance to our project?

artembilan avatar May 19 '24 12:05 artembilan

@artembilan sorry... I was replying from my phone.

This is the stacktrace:

org.springframework.messaging.MessageHandlingException: error occurred in message handler [bean 'businessObject.message-handler#0' for component 'businessObject.org.springframework.integration.config.ConsumerEndpointFactoryBean#0'; defined in: 'class path resource [com/org/Configuration.class]'; from source: 'bean method businessObject'], failedMessage=GenericMessage [payload={value}, headers={id=60a20-5e19-0f0a-141a, timestamp=1715848695857}]
	at org.springframework.integration.support.utils.IntegrationUtils.wrapInHandlingExceptionIfNecessary(IntegrationUtils.java:191)
	at org.springframework.integration.handler.AbstractMessageHandler.doHandleMessage(AbstractMessageHandler.java:108)
	at org.springframework.integration.handler.AbstractMessageHandler.handleWithMetrics(AbstractMessageHandler.java:90)
	at org.springframework.integration.handler.AbstractMessageHandler.handleMessage(AbstractMessageHandler.java:70)
	at org.springframework.integration.dispatcher.AbstractDispatcher.tryOptimizedDispatch(AbstractDispatcher.java:132)
	at org.springframework.integration.dispatcher.UnicastingDispatcher.doDispatch(UnicastingDispatcher.java:133)
	at org.springframework.integration.dispatcher.UnicastingDispatcher$1.run(UnicastingDispatcher.java:114)
	at org.springframework.integration.util.ErrorHandlingTaskExecutor.lambda$execute$0(ErrorHandlingTaskExecutor.java:57)
	at datadog.trace.bootstrap.instrumentation.java.concurrent.Wrapper.run(Wrapper.java:46)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at java.base/java.lang.Thread.run(Thread.java:840)
Caused by: org.springframework.messaging.MessagingException: Failed to execute on session
	at org.springframework.integration.file.remote.RemoteFileTemplate.execute(RemoteFileTemplate.java:461)
	at org.springframework.integration.file.remote.RemoteFileTemplate.send(RemoteFileTemplate.java:314)
	at org.springframework.integration.file.remote.RemoteFileTemplate.send(RemoteFileTemplate.java:302)
	at org.springframework.integration.file.remote.RemoteFileTemplate.send(RemoteFileTemplate.java:294)
	at org.springframework.integration.file.remote.handler.FileTransferringMessageHandler.handleMessageInternal(FileTransferringMessageHandler.java:207)
	at org.springframework.integration.handler.AbstractMessageHandler.doHandleMessage(AbstractMessageHandler.java:105)
	... 10 more
Caused by: java.lang.IllegalStateException: failed to create SFTP Session
	at org.springframework.integration.sftp.session.DefaultSftpSessionFactory.getSession(DefaultSftpSessionFactory.java:304)
	at org.springframework.integration.sftp.session.DefaultSftpSessionFactory.getSession(DefaultSftpSessionFactory.java:80)
	at org.springframework.integration.file.remote.RemoteFileTemplate.execute(RemoteFileTemplate.java:447)
	... 15 more
Caused by: org.apache.sshd.common.SshException: Server key did not validate
	at org.apache.sshd.common.future.AbstractSshFuture.verifyResult(AbstractSshFuture.java:141)
	at org.apache.sshd.client.future.DefaultAuthFuture.verify(DefaultAuthFuture.java:56)
	at org.apache.sshd.client.future.DefaultAuthFuture.verify(DefaultAuthFuture.java:35)
	at org.apache.sshd.common.future.VerifiableFuture.verify(VerifiableFuture.java:110)
	at org.springframework.integration.sftp.session.DefaultSftpSessionFactory.initClientSession(DefaultSftpSessionFactory.java:331)
	at org.springframework.integration.sftp.session.DefaultSftpSessionFactory.getSession(DefaultSftpSessionFactory.java:294)
	... 17 more
Caused by: org.apache.sshd.common.SshException: Server key did not validate

laguiar avatar May 21 '24 07:05 laguiar

Thanks, @laguiar !

Would you mind enable debug logging level for the org.apache.sshd.client? And of course org.springframework.integration.sftp. And then share those logs here before that stack trace. I see that it comes from the AbstractClientSession.checkKeys():

        if (!verified) {
            throw new SshException(SshConstants.SSH2_DISCONNECT_HOST_KEY_NOT_VERIFIABLE, "Server key did not validate");
        }

But would be great to have more info about those hosts and their keys to determine why they don't match.

artembilan avatar May 21 '24 14:05 artembilan

Would you mind enable debug logging level for the org.apache.sshd.client? And of course org.springframework.integration.sftp. And then share those logs here before that stack trace. I see that it comes from the AbstractClientSession.checkKeys():

Unfortunately, I won't be able to do that.

laguiar avatar May 28 '24 07:05 laguiar

@jgormley6 thank you for your suggestion, I had a malformed known_hosts

mjmacheli avatar Sep 02 '24 15:09 mjmacheli

Closed as Invalid since no any further input from community. Perhaps it has been fixed somehow in MINA, or those are just misconfigurations on end-user side.

artembilan avatar Sep 24 '25 16:09 artembilan