x/crypto/ssh: client requires first hostkey to match, knownhosts doesn't expose available key types

[beiriannydd]

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

$ go version
go version go1.11.3 linux/amd64

Does this issue reproduce with the latest release?

This is the latest docker release (yes)

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

go env Output

$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/fsalwin/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/Users/fsalwin/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build745071247=/tmp/go-build -gno-record-gcc-switches"

What did you do?

	config := &ssh.ClientConfig{
		User:            GetRemoteUsername(host),
		HostKeyCallback: knownHostsCallback,
		Timeout:         30 * time.Second,
		Auth:            []ssh.AuthMethod{},
	}

knownHostsCallback is an ssh.HostKeysCallback.

2 machines:
machine A has ecdsa-sha2-nistp256 key for remote host in known hosts file
machine B has ssh-ed25519 key for remote host in known hosts file

If machine A connects to the remote host, the remote host sends the ecdsa-sha2-nistp256 key and is allowed to continue the handshake
If machine B connects to the remote host, the remote host sends the ecdsa-sha2-nistp256 key and is rejected because the callback returns an error.

If I set the host key list to prefer ["ssh-ed25519","ecdsa-sha2-nistp256"...]
Machine A fails
Machine B succeeds

What did you expect to see?

Since there is no mechanism for discovering the available hostkeys for a host published in ssh/knownhosts , I expected it to try each hostkey in the preference list in turn, failing only if all the keys had been tried.

What did you see instead?

SSH Client ceases handshake after receiving the first error response from the HostKeysCallback.

Since only the method on the database is returned by knownhosts.New() it is not easy to add another method accessing the same hostKeyDB instance.

I propose adding an initializing function for the database from knownhosts which returns the database - NewDB to complement New which would return a hostkeys interface.

There would be published methods on the receiver:

type KnownHostDB interface {
	// HostKeyCallback is knownhosts.New without the DB initialization.
	HostKeyCallback() ssh.HostKeyCallback
	// HostKeyAlgorithms takes an address and returns a list of matching key types.
	HostKeyAlgorithms(address string) ([]string, error)
}
// NewDB is knownhosts.New without the callback code
func NewDB(filename string) (KnownHostDB, error)

That way you can just use:

	knownHosts := knownhosts.NewDB(knownhostfile)
	// just to cut down on example code, the error is ignored.
	algos, _ := knownHosts.HostKeyAlgorithms(host)
	config := &ssh.ClientConfig{
		User:            GetRemoteUsername(host),
		HostKeyCallback: knownHosts.HostKeysCallback(),
		Timeout:         30 * time.Second,
		Auth:            []ssh.AuthMethod{},
		HostKeyAlgorithms: algos,
	}

Alternatively if the protocol allows for it, the host key algorithms could be tried in turn until you run out of algorithms (fail) or you have a match (success)

[beiriannydd]

golang/crypto#68

[agnivade]

If you are proposing to add new API, then this should be a proposal, rather than a bug report.

/cc @hanwen

[hanwen]

you can have only one hostkey algorithm per connection attempt, so it doesn't make sense to automate this, I think.

We could look at exposing the DB in knownhosts. I think that should just make the existing types public, rather than introducing an interface.

[beiriannydd]

db is a local inside the New() method currently, are you suggesting making it create a global, or having a different method which returns a db, which you call instead of .New()?
If you return the db then you need a different method to construct the CertChecker for use as the callback function.

[hanwen]

different method.

[beiriannydd]

So it sounds like the only thing you don't like about my solution is that it provides the interface. I guess the wonderful thing about Go is that you don't have to know that you respond to an interface, so as long as we call the methods something sane, people who need to be able to access file based knownhosts and eg. registry based knownhosts can create their own interface.

[hanwen]

interfaces should be defined where they are consumed, and functions should return concrete types if possible. See https://github.com/golang/go/wiki/CodeReviewComments#interfaces

I know that the SSH package violates this guidelines in some places, but unfortunately, we can't fix it without breaking callers. However, I don't want to introduce more instances of this pattern.

[TheoBrigitte]

I've run into this issue recently, where x/crypto/ssh fails to match host key for github.com with the one in my known_hosts file. But my regular openssh client does work correctly.

known_hosts contains a host key for ssh-ed25519 algorithm.

$ cat /home/theo/.ssh/known_hosts|grep github.com
github.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOMqqnkVzrm0SdG6UOoqKLsabgH5C9okWi0dh2l9GKJl

openssh client do use the correct ssh-ed25519 host key algorithm
openssh-client.log

golang/x/crypto fail and use the ecdsa-sha2-nistp256 host key algorithm
debug.log (running a slightly modified version with more debug log)

---

Current workaround: manually get the host key with ecdsa-sha2-nistp256 algorithm

$ sed -i '/^github.com/d' ~/.ssh/known_hosts
$ ssh -o HostKeyAlgorithms=ecdsa-sha2-nistp256 git@github.com

[rolandshoemaker]

This seems like intended behavior? If ClientConfig.HostKeyAlgorithms is not set, a reasonable default is set for acceptable host key type, which may be one for which you do not have a matching host key. In general if you are providing a ClientConfig.HostKeyCallback, it seems like you should be setting ClientConfig.HostKeyAlgorithms to a list of algorithms that your callback will accept keys for.

For @TheoBrigitte, is this fixed by setting ClientConfig.HostKeyAlgorithms = []string{KeyAlgoED25519}?

[TheoBrigitte]

For @TheoBrigitte, is this fixed by setting ClientConfig.HostKeyAlgorithms = []string{KeyAlgoED25519}?

Yes this fixes it, but ideally I would not have to set this manually, and it should be auto-discovered as in openssh client.