Skip to content
New issue

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

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

Already on GitHub? # to your account

HostKeyAlgorithms: add rsa-sha2-256 and rsa-sha2-512 for ssh-rsa #6

Merged
merged 1 commit into from
Mar 12, 2024

Conversation

lonnywong
Copy link
Contributor

  • Related to new conn [] failed: ssh: handshake failed trzsz/trzsz-ssh#93

  • Steps to reproduce:

    • Change /etc/ssh/sshd_config on the ssh server, add a configuration:
      HostKeyAlgorithms rsa-sha2-512,rsa-sha2-256
      
    • Restart the ssh server service ssh restart.
    • Delete the known host items of the server in ~/.ssh/known_hosts on the client.
    • Log in to the server using go ssh client, which should add a ssh-rsa item to ~/.ssh/known_hosts.
    • Log in to the server using go ssh client again, an error will occur:
      client offered: [ssh-rsa], server offered: [rsa-sha2-512 rsa-sha2-256]
      

@evanelias
Copy link
Contributor

Great debugging and fix, thank you @lonnywong, this is excellent!

One minor nit: should the 3 resulting rsa entries get returned in the opposite order, so that the most-secure algorithm gets listed first? I'm pondering this since ssh.ClientConfig.HostKeyAlgorithms is a sorted list by preference, first match between client and server wins. In other words, instead of expanding the rsa key's algorithms to "ssh-rsa", "rsa-sha2-256", "rsa-sha2-512", should it be ordered like "rsa-sha2-512", "rsa-sha2-256", "ssh-rsa" ?

To be clear, aside from how we expand ssh-rsa knownhosts entries, we can keep all other ordering as-is: that is, the returned order just matches the order of the entries in the knownhosts file.

Separately, CI is failing because of something with golint, probably because the Go version I have in the GitHub Actions config is old. I'll try bumping that version on my end momentarily; or if that fails I'll just finally remove golint since it is deprecated/unmaintained.

@evanelias
Copy link
Contributor

update: CI is now fixed on main branch, if you rebase your PR against latest main commit it should hopefully work now 👍

@lonnywong
Copy link
Contributor Author

@evanelias Now, the order is rsa-sha2-512, rsa-sha2-256, ssh-rsa.

@evanelias
Copy link
Contributor

Thanks, looks perfect! Merging momentarily and tagging a new version.

@evanelias evanelias merged commit e73fcfc into skeema:main Mar 12, 2024
1 check passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants