Skip to content

ResourceKnownHostsServerKeyVerifier does not consider the specified keytypes #8674

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
ruedih opened this issue Jul 14, 2023 · 3 comments · Fixed by #8675
Closed

ResourceKnownHostsServerKeyVerifier does not consider the specified keytypes #8674

ruedih opened this issue Jul 14, 2023 · 3 comments · Fixed by #8675
Assignees
@artembilan
Labels
(EOL) for: backport-to-6.1.x in: sftp type: bug
Milestone
6.2.0-M1

Comments

@ruedih
Copy link

ruedih commented Jul 14, 2023

In what version(s) of Spring Integration are you seeing this issue?
6.0.5.
6.1.1 should still be affected as the code was not changed

Describe the bug
The ResourceKnownHostsServerKeyVerifier in Spring Integration Sftp seems to always select the first matching key from the provided known_host file and seems not to take into account the specified keytype.

To Reproduce

To reproduce the issue, add multiple keytypes for the same host and port. Then experiment with the order.

Expected behavior

The keytype should be considered, and the findKnownHostEntry method should select the matching host, port and keytype.

**Related Code **
You can refer to the code at https://github.com/spring-projects/spring-integration/blob/v6.1.1/spring-integration-sftp/src/main/java/org/springframework/integration/sftp/session/ResourceKnownHostsServerKeyVerifier.java#L122
@ruedih ruedih added status: waiting-for-triage The issue need to be evaluated and its future decided type: bug labels Jul 14, 2023
@artembilan
Copy link
Member

Yeah... We blindly modeled this one after KnownHostsServerKeyVerifier in our currently used version of sshd-core.
Looks like we have just missed their TODO over there in the code:

// TODO allow for several candidates and check if ANY of them matches the key and has 'revoked' marker
HostEntryPair match = findKnownHostEntry(clientSession, remoteAddress, knownHosts);

Now in the latest version I see they have it fixed:

https://github.com/apache/mina-sshd/blob/master/sshd-core/src/main/java/org/apache/sshd/client/keyverifier/KnownHostsServerKeyVerifier.java#L273C8-L273C8

Will fix ours shortly respectively.

Thank you for the pointer!

NOTE: we cannot upgrade to the latest MINA since it looks like they have a bug in path resolution for remote directory creation.
Works OK on Windows though.

@artembilan artembilan added this to the 6.2.0-M1 milestone Jul 14, 2023
@artembilan artembilan added in: sftp backport 6.0.x (EOL) for: backport-to-6.1.x and removed status: waiting-for-triage The issue need to be evaluated and its future decided labels Jul 14, 2023
@artembilan artembilan self-assigned this Jul 14, 2023
artembilan added a commit to artembilan/spring-integration that referenced this issue Jul 14, 2023
@artembilan
spring-projectsGH-8674: Fix ServerKeyVerifier impl for key type
d77f6f4 
Fixes spring-projects#8674

The `ResourceKnownHostsServerKeyVerifier` does not take into account that several different
keys can be present in the known hosts resource for the same host/port

* Fix `ResourceKnownHostsServerKeyVerifier` to find a list of knows host for the requested session.
Then iterate of this result to match the key type first and then compare keys and their `revoked` marker

**Cherry-pick to `6.1.x` & `6.0.x`**
@artembilan artembilan mentioned this issue Jul 14, 2023
Merged
@artembilan
Copy link
Member

Please, find the fix in the linked PR.

@ruedih
Copy link
Author

ruedih commented Jul 16, 2023

Hey @artembilan ,

thanks for your super fast resolution of the issue. 🥇 :)

garyrussell pushed a commit that referenced this issue Jul 17, 2023
@artembilan
GH-8674: Fix ServerKeyVerifier impl for key type (#8675)
d4535ca 
Fixes #8674

The `ResourceKnownHostsServerKeyVerifier` does not take into account that several different
keys can be present in the known hosts resource for the same host/port

* Fix `ResourceKnownHostsServerKeyVerifier` to find a list of knows host for the requested session.
Then iterate of this result to match the key type first and then compare keys and their `revoked` marker

**Cherry-pick to `6.1.x` & `6.0.x`**
garyrussell pushed a commit that referenced this issue Jul 17, 2023
@artembilan @garyrussell
GH-8674: Fix ServerKeyVerifier impl for key type (#8675)
90dfed5 
Fixes #8674

The `ResourceKnownHostsServerKeyVerifier` does not take into account that several different
keys can be present in the known hosts resource for the same host/port

* Fix `ResourceKnownHostsServerKeyVerifier` to find a list of knows host for the requested session.
Then iterate of this result to match the key type first and then compare keys and their `revoked` marker

**Cherry-pick to `6.1.x` & `6.0.x`**
garyrussell pushed a commit that referenced this issue Jul 17, 2023
@artembilan @garyrussell
GH-8674: Fix ServerKeyVerifier impl for key type (#8675)
c267bd5 
Fixes #8674

The `ResourceKnownHostsServerKeyVerifier` does not take into account that several different
keys can be present in the known hosts resource for the same host/port

* Fix `ResourceKnownHostsServerKeyVerifier` to find a list of knows host for the requested session.
Then iterate of this result to match the key type first and then compare keys and their `revoked` marker

**Cherry-pick to `6.1.x` & `6.0.x`**
@artembilan artembilan mentioned this issue Aug 14, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@artembilan artembilan

Labels
(EOL) for: backport-to-6.1.x in: sftp type: bug
Projects
None yet
Milestone
6.2.0-M1
Development

Successfully merging a pull request may close this issue.

GH-8674: Fix ServerKeyVerifier impl for key type artembilan/spring-integration
2 participants
@artembilan @ruedih