Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
alajmo opened this issue Jun 20, 2022 · 4 comments · May be fixed by golang/crypto#268
Open

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

alajmo opened this issue Jun 20, 2022 · 4 comments · May be fixed by golang/crypto#268
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@alajmo
Copy link

alajmo commented Jun 20, 2022

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.
@mdlayher mdlayher changed the title affected/package: golang.org/x/crypto/ssh/knownhosts, incorrect parsing of known_hosts for ipv6 x/crypto/ssh/knownhosts: incorrect parsing of known_hosts for ipv6 Jun 20, 2022
@gopherbot gopherbot added this to the Unreleased milestone Jun 20, 2022
@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 24, 2022
@cagedmantis
Copy link
Contributor

//cc @FiloSottile @golang/security

@alajmo
Copy link
Author

alajmo commented Aug 24, 2022

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
Copy link

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.

pacien added a commit to pacien/golang-crypto that referenced this issue Aug 23, 2023
1. When using the bracketed syntax, the port is mandatory, even if
   it's the default one. Otherwise, OpenSSH rejects it with:
   "address [abcd:abcd:abcd:abcd]: missing port in address".
   See sshd(8): SSH_KNOWN_HOSTS FILE FORMAT.

2. Brackets are not necessary when using the default port,
   even for IPv6 addresses.

Fixes golang/go#53463
pacien pushed a commit to pacien/golang-crypto that referenced this issue Aug 23, 2023
1. When using the bracketed syntax, the port is mandatory, even if
   it's the default one. Otherwise, OpenSSH rejects it with:
   "address [abcd:abcd:abcd:abcd]: missing port in address".
   See sshd(8): SSH_KNOWN_HOSTS FILE FORMAT.

2. Brackets are not necessary when using the default port,
   even for IPv6 addresses.

Fixes golang/go#53463
@gopherbot
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants