-
Notifications
You must be signed in to change notification settings - Fork 602
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
Lean on Config.keyAlgorithms choosing between rsa-sha2-* and ssh-rsa #742
Conversation
…ong if container exited
…-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.
Is there anything that should be modified to have it merged? |
No, nothing was blocking it, it more just escaped my attention that it wasn't merged yet. Come to think of your starting coimment on this MR, it might be a good idea to add a factory method on DefaultConfig that adds the SSHRSA algorithm above the rest. So that it would be easy to adapt to the change for people. What do you think? |
13b4044
to
592b612
Compare
Added |
👍 Thanks! |
…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 <[email protected]> (cherry picked from commit 624747c)
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.
Apparently, I tried to find a way to determine if the server supports some key algorithm signature. I haven't found anything related in source code of OpenSSH. So, I decided to implement in SSHJ the same approach as OpenSSH client uses.
Also, if this patch is accepted, better to add a note to the changelog about a backward compatibility breakage. The default config stops working with old servers like Apache SSHD that doesn't support rsa-sha2-* signatures. It is still possible to connect to them, but an instance of DefaultConfig should be modified a bit:
KeyAlgorithms.SSHRSA
should have higher priority than other RSA-related algorithms, which can be done by callingConfig.setKeyAlgorithms
.