Skip to content
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.

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

Fix matching of pubkeys to key algorithms #607

Merged
merged 9 commits into from
Jul 28, 2020

Conversation

fmeum
Copy link
Contributor

@fmeum fmeum commented Jun 23, 2020

Allow all configured key algorithms for pubkey authentication, even if
these algorithms are not supported as host key algorithms by the
server.

Preference is given to the modern rsa-sha2-* signature algorithms if
the server indicates support for them as host keys signature
algorithms.

Should fix #600.

Allow all configured key algorithms for pubkey authentication, even if
these algorithms are not supported as host key algorithms by the
server.

Preference is given to the modern rsa-sha2-* signature algorithms if
the server indicates support for them as host keys signature
algorithms.
@fmeum fmeum requested a review from hierynomus as a code owner June 23, 2020 12:42
@hierynomus
Copy link
Owner

Is this according to the RFC? This code was changed pretty recently and I think I then interpreted the RFC correctly. But I might be mistaken of course.

@fmeum
Copy link
Contributor Author

fmeum commented Jun 23, 2020

Is this according to the RFC? This code was changed pretty recently and I think I then interpreted the RFC correctly. But I might be mistaken of course.

I am quite sure that pubkey algorithms are not negotiated, only host key algorithms are. There is an extension with which the server can announce support for pubkey types though which could be more reliable than the rough feature detection implemented here based on the host key type.

@moritz31
Copy link
Contributor

I face exactly this issue. Authentication happens over an ed25519 key but for signature the server only provides rsa, which will not work with the current implementation

@fmeum
Copy link
Contributor Author

fmeum commented Jul 28, 2020

@moritz31 If possible, could you check whether the issue you are facing is resolved by my PR?

@fmeum
Copy link
Contributor Author

fmeum commented Jul 28, 2020

I'll make a minor improvement to the tests.

@fmeum
Copy link
Contributor Author

fmeum commented Jul 28, 2020

I'll make a minor improvement to the tests.

Nvm, everything is fine. It should just be noted that the SSH server in the docker image should not add both ecdsa-sha2-384 and ecdsa-sha2-521 host keys, as otherwise the tests can't verify any longer that #600 has been fixed.

@fmeum fmeum closed this Jul 28, 2020
@fmeum fmeum reopened this Jul 28, 2020
@fmeum
Copy link
Contributor Author

fmeum commented Jul 28, 2020

The flaky part seems to be the build step, not the integration test step. But everything passed now.

@hierynomus
Copy link
Owner

yes, the plugins repository was throwing 500 errors...

@hierynomus hierynomus merged commit 6becee1 into hierynomus:master Jul 28, 2020
@fmeum fmeum deleted the negotiation_fix branch July 28, 2020 10:30
vladimirlagunov added a commit to vladimirlagunov/sshj that referenced this pull request Nov 11, 2021
…-sha2-* and ssh-rsa

Previously, there was a heuristic that was choosing rsa-sha2-512 after receiving a host key of type RSA. It didn't work well when a server doesn't have an RSA host key.

OpenSSH 8.8 introduced a breaking change: it removed ssh-rsa from the default list of supported public key signature algorithms. SSHJ was unable to connect to OpenSSH 8.8 server if the server has an EcDSA or Ed25519 host key.

Current behaviour behaves the same as OpenSSH 8.8 client does. SSHJ doesn't try to determine rsa-sha2-* support on the fly. Instead, it looks only on `Config.getKeyAlgorithms()`, which may or may not contain ssh-rsa and rsa-sha2-* in any order.

Sorry, this commit mostly reverts changes from hierynomus#607.
hierynomus added a commit that referenced this pull request Dec 6, 2021
…742)

* Improve SshdContainer: log `docker build` to stdout, don't wait too long if container exited

* Fix #740: Lean on Config.keyAlgorithms choosing between rsa-sha2-* and ssh-rsa

Previously, there was a heuristic that was choosing rsa-sha2-512 after receiving a host key of type RSA. It didn't work well when a server doesn't have an RSA host key.

OpenSSH 8.8 introduced a breaking change: it removed ssh-rsa from the default list of supported public key signature algorithms. SSHJ was unable to connect to OpenSSH 8.8 server if the server has an EcDSA or Ed25519 host key.

Current behaviour behaves the same as OpenSSH 8.8 client does. SSHJ doesn't try to determine rsa-sha2-* support on the fly. Instead, it looks only on `Config.getKeyAlgorithms()`, which may or may not contain ssh-rsa and rsa-sha2-* in any order.

Sorry, this commit mostly reverts changes from #607.

* Introduce ConfigImpl.prioritizeSshRsaKeyAlgorithm to deal with broken backward compatibility

Co-authored-by: Jeroen van Erp <jeroen@hierynomus.com>
vladimirlagunov added a commit to vladimirlagunov/sshj that referenced this pull request Jan 21, 2022
…ierynomus#742)

* Improve SshdContainer: log `docker build` to stdout, don't wait too long if container exited

* Fix hierynomus#740: Lean on Config.keyAlgorithms choosing between rsa-sha2-* and ssh-rsa

Previously, there was a heuristic that was choosing rsa-sha2-512 after receiving a host key of type RSA. It didn't work well when a server doesn't have an RSA host key.

OpenSSH 8.8 introduced a breaking change: it removed ssh-rsa from the default list of supported public key signature algorithms. SSHJ was unable to connect to OpenSSH 8.8 server if the server has an EcDSA or Ed25519 host key.

Current behaviour behaves the same as OpenSSH 8.8 client does. SSHJ doesn't try to determine rsa-sha2-* support on the fly. Instead, it looks only on `Config.getKeyAlgorithms()`, which may or may not contain ssh-rsa and rsa-sha2-* in any order.

Sorry, this commit mostly reverts changes from hierynomus#607.

* Introduce ConfigImpl.prioritizeSshRsaKeyAlgorithm to deal with broken backward compatibility

Co-authored-by: Jeroen van Erp <jeroen@hierynomus.com>
(cherry picked from commit 624747c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Public key authentication only possible with host key types
4 participants