ssh/knownhosts: check more than one key

The problem was that it was keeping only the first known key of each
type found.
If you have a server advertising multiple keys of the same type,
you might get a missmatch key error.

Per `sshd(8)` man page, it should allow reapeatable hosts with
different host keys, although it don't specify anything about
hosts being from different types:

>     It is permissible (but not recommended) to have several lines or
>     different host keys for the same names.  This will inevitably happen when
>     short forms of host names from different domains are put in the file.  It
>     is possible that the files contain conflicting information;
>     authentication is accepted if valid information can be found from either
>     file.

So, this changes `knownhosts` behavior to accept any of the keys for a
given host, regardless of type.

Fixes #36126

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

Author: caarlos0

ssh/knownhosts/knownhosts.go

@@ -302,8 +302,8 @@ func (k *KnownKey) String() string {
 // applications can offer an interactive prompt to the user.
 type KeyError struct {
 	// Want holds the accepted host keys. For each key algorithm,
-	// there can be one hostkey.  If Want is empty, the host is
-	// unknown. If Want is non-empty, there was a mismatch, which
+	// there can be multiple hostkeys.  If Want is empty, the host
+	// is unknown. If Want is non-empty, there was a mismatch, which
 	// can signify a MITM attack.
 	Want []KnownKey
 }
@@ -358,34 +358,20 @@ func (db *hostKeyDB) checkAddr(a addr, remoteKey ssh.PublicKey) error {
 	// is just a key for the IP address, but not for the
 	// hostname?
 
-	// Algorithm => key.
-	knownKeys := map[string]KnownKey{}
-	for _, l := range db.lines {
-		if l.match(a) {
-			typ := l.knownKey.Key.Type()
-			if _, ok := knownKeys[typ]; !ok {
-				knownKeys[typ] = l.knownKey
-			}
-		}
-	}
-
 	keyErr := &KeyError{}
-	for _, v := range knownKeys {
-		keyErr.Want = append(keyErr.Want, v)
-	}
 
-	// Unknown remote host.
-	if len(knownKeys) == 0 {
-		return keyErr
-	}
+	for _, l := range db.lines {
+		if !l.match(a) {
+			continue
+		}
 
-	// If the remote host starts using a different, unknown key type, we
-	// also interpret that as a mismatch.
-	if known, ok := knownKeys[remoteKey.Type()]; !ok || !keyEq(known.Key, remoteKey) {
-		return keyErr
+		keyErr.Want = append(keyErr.Want, l.knownKey)
+		if keyEq(l.knownKey.Key, remoteKey) {
+			return nil
+		}
 	}
 
-	return nil
+	return keyErr
 }
 
 // The Read function parses file contents.

ssh/knownhosts/knownhosts_test.go

@@ -201,17 +201,6 @@ func TestHostNamePrecedence(t *testing.T) {
 	}
 }
 
-func TestDBOrderingPrecedenceKeyType(t *testing.T) {
-	str := fmt.Sprintf("server.org,%s %s\nserver.org,%s %s", testAddr, edKeyStr, testAddr, alternateEdKeyStr)
-	db := testDB(t, str)
-
-	if err := db.check("server.org:22", testAddr, alternateEdKey); err == nil {
-		t.Errorf("check succeeded")
-	} else if _, ok := err.(*KeyError); !ok {
-		t.Errorf("got %T, want *KeyError", err)
-	}
-}
-
 func TestNegate(t *testing.T) {
 	str := fmt.Sprintf("%s,!server.org %s", testAddr, edKeyStr)
 	db := testDB(t, str)
@@ -354,3 +343,16 @@ func TestHashedHostkeyCheck(t *testing.T) {
 		t.Errorf("got error %v, want %v", got, want)
 	}
 }
+
+func TestIssue36126(t *testing.T) {
+	str := fmt.Sprintf("server.org,%s %s\nserver.org,%s %s", testAddr, edKeyStr, testAddr, alternateEdKeyStr)
+	db := testDB(t, str)
+
+	if err := db.check("server.org:22", testAddr, edKey); err != nil {
+		t.Errorf("should have passed the check, got %v", err)
+	}
+
+	if err := db.check("server.org:22", testAddr, alternateEdKey); err != nil {
+		t.Errorf("should have passed the check, got %v", err)
+	}
+}