ssh/knownhosts: reject lines with multiple or unknown markers

Previously, parseLine would accept lines with multiple markers (e.g.,
"@cert-authority @revoked ...") or unknown markers (e.g., "@unknown ...").
In these cases, the second or unknown marker was incorrectly parsed as
the hostname. This often resulted in confusing downstream errors (like
"illegal base64 data") because field positions were shifted.

OpenSSH's hostfile.c strictly enforces a limit of one marker per line
and validates that markers must be exactly "@cert-authority" or
"@revoked".

This change adds a check to ensure that the parsed hostname does not
start with '@'. This effectively catches both multiple markers (where
the second marker becomes the host) and unknown markers (which are not
consumed as markers and thus become the host), enforcing strict
compliance with the OpenSSH format.

This issue was found during a security audit by NCC Group Cryptography
Services, sponsored by Teleport, and was assessed and is being fixed as
a non-security bug.

Change-Id: I13bb04d1f2610483ad9c6d1020e5100b6feded90
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/782428
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Mark Freeman <markfreeman@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>

ssh/knownhosts/knownhosts.go

diff --git a/ssh/knownhosts/knownhosts.go b/ssh/knownhosts/knownhosts.go
index c7eda05..cf520ad 100644
--- a/ssh/knownhosts/knownhosts.go
+++ b/ssh/knownhosts/knownhosts.go
@@ -188,6 +188,12 @@
 	}
 
 	host, line = nextWord(line)
+	// If the extracted 'host' starts with '@', it means we either encountered
+	// a second marker (e.g., "@cert-authority @revoked") or an unknown marker
+	// (e.g., "@unknown"). Both are invalid.
+	if len(host) > 0 && host[0] == '@' {
+		return "", "", nil, fmt.Errorf("knownhosts: unexpected marker: %q", host)
+	}
 	if len(line) == 0 {
 		return "", "", nil, errors.New("knownhosts: missing host pattern")
 	}

ssh/knownhosts/knownhosts_test.go

diff --git a/ssh/knownhosts/knownhosts_test.go b/ssh/knownhosts/knownhosts_test.go
index f3af2da..5e51447 100644
--- a/ssh/knownhosts/knownhosts_test.go
+++ b/ssh/knownhosts/knownhosts_test.go
@@ -462,3 +462,41 @@
 		t.Fatalf("got error %q, want to contain %q", err.Error(), expectedErr)
 	}
 }
+
+func TestMultipleMarkers(t *testing.T) {
+	lineDouble := fmt.Sprintf("@cert-authority @revoked server.org %s %s", edKey.Type(), base64.StdEncoding.EncodeToString(edKey.Marshal()))
+	db := newHostKeyDB()
+	err := db.Read(bytes.NewBufferString(lineDouble), "testdb")
+	if err == nil {
+		t.Fatal("Read succeeded on line with multiple markers, expected error")
+	}
+	expectedError := "unexpected marker"
+	if !strings.Contains(err.Error(), expectedError) {
+		t.Errorf("got error %q, want it to contain %q", err.Error(), expectedError)
+	}
+
+	lineUnknown := fmt.Sprintf("@unknown-marker server.org %s %s", edKey.Type(), base64.StdEncoding.EncodeToString(edKey.Marshal()))
+	err = db.Read(bytes.NewBufferString(lineUnknown), "testdb")
+	if err == nil {
+		t.Fatal("Read succeeded on line with unknown marker, expected error")
+	}
+	if !strings.Contains(err.Error(), expectedError) {
+		t.Errorf("got error %q, want it to contain %q", err.Error(), expectedError)
+	}
+}
+
+func TestUnknownMarker(t *testing.T) {
+	line := fmt.Sprintf("@unknown-marker server.org %s %s", edKey.Type(), base64.StdEncoding.EncodeToString(edKey.Marshal()))
+
+	db := newHostKeyDB()
+	err := db.Read(bytes.NewBufferString(line), "testdb")
+
+	if err == nil {
+		t.Fatal("Read succeeded on line with unknown marker, expected error")
+	}
+
+	expectedError := "unexpected marker"
+	if !strings.Contains(err.Error(), expectedError) {
+		t.Errorf("got error %q, want it to contain %q", err.Error(), expectedError)
+	}
+}