skeema / knownhosts

Go SSH known_hosts wrapper with host key lookup
Apache License 2.0
32 stars 4 forks source link

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

Closed lonnywong closed 8 months ago

lonnywong commented 8 months ago
evanelias commented 8 months ago

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 commented 8 months ago

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

lonnywong commented 8 months ago

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

evanelias commented 8 months ago

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