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

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:

If IPv6 and port == 22 return abcd:abcd:abcd:abcd
if IPv6 and port != 22 return [abcd:abcd:abcd:abcd]:33
If IPv4 and port == 22 return 127.0.0.1
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 3. Line returns a line to append to the known_hosts files.

Comment From: cagedmantis

//cc @FiloSottile @golang/security

Comment From: 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.

Comment From: 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.

Comment From: gopherbot

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

Comment From: prubel

This still seems like a problem and there's a patch that's been sitting there for nearly 2 years. What's stopping those fixes from going in (or being rejected)?

Comment From: rolandshoemaker

cc @drakkan

Comment From: drakkan

I ran some tests using the proposed patch, and it fails when the input is something like 2001:db8::1:2200, I'm not sure if this case needs to be supported. I think we don't have to handle malformed inputs like [2001:db8::1 maybe we should document what we handle