x/crypto/ssh/knownhosts: incorrect parsing of known_hosts for ipv6

[alajmo]

What version of Go are you using (go version)?

$ go version
go version go1.18 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GOARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOVERSION="go1.18"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
$ ssh -V

OpenSSH_9.0p1 Debian-1, OpenSSL 1.1.1o  3 May 2022

What did you do?

package main

import (
	"fmt"
	"net"

	"golang.org/x/crypto/ssh"
	"golang.org/x/crypto/ssh/knownhosts"
)

func main() {
	hostKeyCallback, err := knownhosts.New("/home/samir/.ssh/known_hosts")
	if err != nil {
		fmt.Println(err)
		return
	}

	config := &ssh.ClientConfig{
		User: "test",
		Auth: []ssh.AuthMethod{ssh.Password("test")},
		HostKeyCallback: func(hostname string, remote net.Addr, key ssh.PublicKey) error {
			addr := []string{hostname}
			line := knownhosts.Line(addr, key)
			fmt.Println(line)

			return hostKeyCallback(hostname, remote, key)
		},
	}
	_, err = ssh.Dial("tcp", "[2001:3984:3989::10]:22", config)

	if err != nil {
		fmt.Println(err)
		return
	}
}

When running the above script, the line line := knownhosts.Line(addr, key) prints the following:

(The xyz is a just a placeholder)

[2001:3984:3989::10] ecdsa-sha2-nistp256 xyz

which I add to /home/samir/.ssh/known_hosts. I get the message ssh: handshake failed: knownhosts: key is unknown as well, which is expected.

What did you expect to see?

On the second re-run (with my hostkey added to known_hosts), I expect the connection to be established.

What did you see instead?

I see:

knownhosts: /home/samir/.ssh/known_hosts:1: address [2001:3984:3989::10]: missing port in address

If I add the :22 port to the IP, it works (this shouldn't work though, since it's the default port, should only work when port != 22):

[2001:3984:3989::10]:22 ecdsa-sha2-nistp256 xyz

And it works if I remove the brackets (this is the correct way and how ssh works):

2001:3984:3989::10 ecdsa-sha2-nistp256 xyz

The method Line says Line returns a line to add append to the known_hosts files., but the method New doesn't support parsing the known_hosts file without a port number when brackets [] are used.

So it should be:

1. If IPv6 and port == 22 return abcd:abcd:abcd:abcd
2. if IPv6 and port != 22 return [abcd:abcd:abcd:abcd]:33
3. If IPv4 and port == 22 return 127.0.0.1
4. If IPv4 and port != 22 return [127.0.0.1]:33

I think the Normalize function is the culprit in some of the errors:

https://cs.opensource.google/go/x/crypto/+/bc19a97f:ssh/knownhosts/knownhosts_test.go;l=329

The test cases are:

		"[abcd:abcd:abcd:abcd]":    "[abcd:abcd:abcd:abcd]",
		"[abcd:abcd:abcd:abcd]:22": "[abcd:abcd:abcd:abcd]",

They should be (removal of brackets on the right side):

		"[abcd:abcd:abcd:abcd]":    "abcd:abcd:abcd:abcd",
		"[abcd:abcd:abcd:abcd]:22": "abcd:abcd:abcd:abcd",

Also, a small note, the method Line has a grammar error:

It says Line returns a line to add append to the known_hosts files., but it should say:

1. Line returns a line to add to the known_hosts files., or
2. Line returns a line to append to the known_hosts files.

[cagedmantis]

//cc @FiloSottile @golang/security

[alajmo]

From https://man.openbsd.org/sshd.8:

A hostname or address may optionally be enclosed within ‘[’ and ‘]’ brackets then followed by ‘:’ and a non-standard port number.

So if the IP is enclosed with brackets [], then it MUST be followed by a colon : AND a non-standard port.

[evanelias]

For anyone who needs a quick work-around for writing ipv6 entries to known_hosts files, package github.com/skeema/knownhosts@v1.2.0 now includes patched versions of Normalize and Line with correct ipv6 behavior, thanks to a nice contribution from @lonnywong of the @trzsz project.

github.com/skeema/knownhosts is a thin wrapper around x/crypto/ssh/knownhosts, rather than a fork. It's battle-tested and adds several improvements not found in x/crypto/ssh/knownhosts. It's designed to be a nearly-drop-in replacement; you'll just need to cast back to ssh.HostKeyCallback if using the result of its New directly in ssh.ClientConfig.HostKeyCallback.

[gopherbot]

Change https://go.dev/cl/522255 mentions this issue: ssh/knownhosts: fix bracket normalisation