ssh/knownhosts: treat only ASCII space and tab as whitespace The previous implementation used bytes.TrimSpace, which strips all Unicode whitespace categories (e.g., non-breaking spaces). However, OpenSSH's known_hosts parser (hostfile.c) strictly treats only ASCII space (0x20) and horizontal tab (0x09) as separators. This discrepancy meant the Go parser might interpret fields differently than OpenSSH, potentially treating parts of a key or hostname as separators if they contained Unicode whitespace. This change replaces bytes.TrimSpace with a local trimSpace helper that only trims " \t", ensuring parsing behavior consistent with the reference implementation. 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: Ia536889636de2c167d2507c01e3f1b7c033c9a8f Reviewed-on: https://go-review.googlesource.com/c/crypto/+/782426 Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Filippo Valsorda <filippo@golang.org> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Junyang Shao <shaojunyang@google.com>
diff --git a/ssh/knownhosts/knownhosts.go b/ssh/knownhosts/knownhosts.go index e57cf5b..1ef1c35 100644 --- a/ssh/knownhosts/knownhosts.go +++ b/ssh/knownhosts/knownhosts.go
@@ -178,7 +178,7 @@ return string(line), nil } - return string(line[:i]), bytes.TrimSpace(line[i:]) + return string(line[:i]), trimSpace(line[i:]) } func parseLine(line []byte) (marker, host string, key ssh.PublicKey, err error) { @@ -387,7 +387,7 @@ for scanner.Scan() { lineNum++ line := scanner.Bytes() - line = bytes.TrimSpace(line) + line = trimSpace(line) if len(line) == 0 || line[0] == '#' { continue } @@ -535,3 +535,10 @@ func (h *hashedHost) match(a addr) bool { return bytes.Equal(hashHost(Normalize(a.String()), h.salt), h.hash) } + +// trimSpace removes leading and trailing ASCII whitespace (space and tab). It +// is used instead of bytes.TrimSpace to match OpenSSH behavior, which strictly +// parses only ASCII space (0x20) and tab (0x09) as whitespace. +func trimSpace(in []byte) []byte { + return bytes.Trim(in, " \t") +}
diff --git a/ssh/knownhosts/knownhosts_test.go b/ssh/knownhosts/knownhosts_test.go index 4d3ca59..8e1fba2 100644 --- a/ssh/knownhosts/knownhosts_test.go +++ b/ssh/knownhosts/knownhosts_test.go
@@ -8,6 +8,7 @@ "bytes" "crypto/ed25519" "crypto/rand" + "encoding/base64" "fmt" "net" "reflect" @@ -407,3 +408,41 @@ t.Errorf("should have passed the check, got %v", err) } } + +func TestUnicodeSpace(t *testing.T) { + line := fmt.Sprintf("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 Unicode space, expected error due to strict ASCII parsing") + } +} + +func TestUnicodeSpaceTrimming(t *testing.T) { + const unicodeSpace = " " + line := fmt.Sprintf("%sserver.org %s", unicodeSpace, edKeyStr) + + db := newHostKeyDB() + err := db.Read(bytes.NewBufferString(line), "testdb") + + if err != nil { + t.Fatalf("Read failed: %v", err) + } + + lines := db.lines + if len(lines) != 1 { + t.Fatalf("Expected 1 line, got %d", len(lines)) + } + + cleanAddr := addr{host: "server.org", port: "22"} + dirtyAddr := addr{host: unicodeSpace + "server.org", port: "22"} + + if lines[0].match(cleanAddr) { + t.Errorf("Matched clean host 'server.org', implying Unicode space was trimmed") + } + if !lines[0].match(dirtyAddr) { + t.Errorf("Did not match dirty host, implying parsing issue") + } +}