Skip to content

Commit 3a35d9f

Browse files
committed
HostKeyAlgorithms: ensure result never contains duplicates
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.
1 parent 2442217 commit 3a35d9f

File tree

2 files changed

+15
-4
lines changed

2 files changed

+15
-4
lines changed

knownhosts.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,19 @@ func (hkcb HostKeyCallback) HostKeys(hostWithPort string) (keys []ssh.PublicKey)
6969
// known_hosts entries (for different key types), the result will be sorted by
7070
// known_hosts filename and line number.
7171
func (hkcb HostKeyCallback) HostKeyAlgorithms(hostWithPort string) (algos []string) {
72-
for _, key := range hkcb.HostKeys(hostWithPort) {
73-
algos = append(algos, key.Type())
72+
// We ensure that algos never contains duplicates. This is done for robustness
73+
// even though currently golang.org/x/crypto/ssh/knownhosts never exposes
74+
// multiple keys of the same type. This way our behavior here is unaffected
75+
// even if https://github.com/golang/go/issues/28870 is implemented, for
76+
// example by https://github.com/golang/crypto/pull/254.
77+
hostKeys := hkcb.HostKeys(hostWithPort)
78+
seen := make(map[string]struct{}, len(hostKeys))
79+
for _, key := range hostKeys {
80+
typ := key.Type()
81+
if _, already := seen[typ]; !already {
82+
algos = append(algos, typ)
83+
seen[typ] = struct{}{}
84+
}
7485
}
7586
return algos
7687
}

knownhosts_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -248,15 +248,15 @@ func getTestKnownHosts(t *testing.T) string {
248248

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

0 commit comments

Comments
 (0)