Skip to content

Commit

Permalink
HostKeyAlgorithms: ensure result never contains duplicates
Browse files Browse the repository at this point in the history
Currently the behavior of HostKeyAlgorithms never contains duplicates, only by
virtue of golang.org/x/crypto/ssh/knownhosts exposing a maximum of one key per
algorithm in its KeyError.Want slice.

However, that upstream behavior could theoretically change in the future,
especially since golang.org/x/crypto is versioned as a pre-v1 module, and the
one-key-per-type behavior is only documented as a comment (e.g. not part of
any type or function signature).

This commit makes our HostKeyAlgorithms function more robust / future-proof
by ensuring that its result does not contain duplicates, regardless of
upstream behavior.

This means if golang/go#28870 is solved (for example
by golang/crypto#254), there should not be any harm to
our behavior here in github.com/skeema/knownhosts.
  • Loading branch information
evanelias committed Sep 18, 2023
1 parent 2442217 commit 3a35d9f
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 4 deletions.
15 changes: 13 additions & 2 deletions knownhosts.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,19 @@ func (hkcb HostKeyCallback) HostKeys(hostWithPort string) (keys []ssh.PublicKey)
// known_hosts entries (for different key types), the result will be sorted by
// known_hosts filename and line number.
func (hkcb HostKeyCallback) HostKeyAlgorithms(hostWithPort string) (algos []string) {
for _, key := range hkcb.HostKeys(hostWithPort) {
algos = append(algos, key.Type())
// We ensure that algos never contains duplicates. This is done for robustness
// even though currently golang.org/x/crypto/ssh/knownhosts never exposes
// multiple keys of the same type. This way our behavior here is unaffected
// even if https://github.com/golang/go/issues/28870 is implemented, for
// example by https://github.com/golang/crypto/pull/254.
hostKeys := hkcb.HostKeys(hostWithPort)
seen := make(map[string]struct{}, len(hostKeys))
for _, key := range hostKeys {
typ := key.Type()
if _, already := seen[typ]; !already {
algos = append(algos, typ)
seen[typ] = struct{}{}
}
}
return algos
}
Expand Down
4 changes: 2 additions & 2 deletions knownhosts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,15 +248,15 @@ func getTestKnownHosts(t *testing.T) string {

// writeTestKnownHosts generates the test known_hosts file and returns the
// file path to it. The generated file contains several hosts with a mix of
// key types; each known host has between 1 and 3 different known host keys.
// key types; each known host has between 1 and 4 different known host keys.
// If generating or writing the file fails, the test fails.
func writeTestKnownHosts(t *testing.T) string {
t.Helper()
hosts := map[string][]ssh.PublicKey{
"only-rsa.example.test:22": {generatePubKeyRSA(t)},
"only-ecdsa.example.test:22": {generatePubKeyECDSA(t)},
"only-ed25519.example.test:22": {generatePubKeyEd25519(t)},
"multi.example.test:2233": {generatePubKeyRSA(t), generatePubKeyECDSA(t), generatePubKeyEd25519(t)},
"multi.example.test:2233": {generatePubKeyRSA(t), generatePubKeyECDSA(t), generatePubKeyEd25519(t), generatePubKeyEd25519(t)},
"192.168.1.102:2222": {generatePubKeyECDSA(t), generatePubKeyEd25519(t)},
"[fe80::abc:abc:abcd:abcd]:22": {generatePubKeyEd25519(t), generatePubKeyRSA(t)},
}
Expand Down

0 comments on commit 3a35d9f

Please sign in to comment.