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 <[email protected]>

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)
+	}
+}