crypto/ssh/knownhosts: check more than one key

[caarlos0]

I believe this fixes https://github.com/golang/go/issues/36126 .

The problem was that it was keeping only the first known key of each type found. If you have a server advertising multiple keys of the same type, you might get a missmatch key error.

Per sshd(8) man page, it should allow reapeatable hosts with different host keys, although it don't specify anything about hosts being from different types:

It is permissible (but not recommended) to have several lines or
different host keys for the same names.  This will inevitably happen when
short forms of host names from different domains are put in the file.  It
is possible that the files contain conflicting information;
authentication is accepted if valid information can be found from either
file.

So, this changes knownhosts behavior to accept any of the keys for a given host, regardless of type.

closes https://github.com/golang/go/issues/36126

[gopherbot]

This PR (HEAD: adcf924af74cf42728a996edb00152400cc40cd8) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/crypto/+/478535 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off) See the Wiki page for more info

[gopherbot]

Message from Gopher Robot:

Patch Set 1:

Congratulations on opening your first change. Thank you for your contribution!

Next steps: A maintainer will review your change and provide feedback. See https://go.dev/doc/contribute#review for more info and tips to get your patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be surprising to people new to the project. The careful, iterative review process is our way of helping mentor contributors and ensuring that their contributions have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which little code gets reviewed or merged. If a reviewer responds with a comment like R=go1.11 or adds a tag like "wait-release", it means that this CL will be reviewed as part of the next development cycle. See https://go.dev/s/release for more details.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/478535. After addressing review feedback, remember to publish your drafts!

[lonnywong]

Any progress? Love to see this merged. Thanks!

[evanelias]

One potential downside to this PR is it changes the documented behavior of knownhosts.KeyError.Want, re: "For each key algorithm there can be one hostkey"

type KeyError struct {
	// Want holds the accepted host keys. For each key algorithm,
	// there can be one hostkey.  If Want is empty, the host is
	// unknown. If Want is non-empty, there was a mismatch, which
	// can signify a MITM attack.
	Want []KnownKey
}

@lonnywong out of curiosity, do you really have 2 keys of the same type for the same hostname (same situation as https://github.com/golang/go/issues/36126, which would be solved by this PR)? Or do you have a server presenting multiple hostkeys of different types, some of which aren't in the client knownhosts file (see https://github.com/golang/go/issues/29286)?

Both issues result in the same user-facing error on the client side, but they have different root causes. In my experience, the latter situation is much more common.

I have a widely-used package, github.com/skeema/knownhosts, which uses some trickery to provide a workaround for https://github.com/golang/go/issues/29286, although it doesn't help with https://github.com/golang/go/issues/36126.

Just mentioning this here because github.com/skeema/knownhosts currently assumes the "For each key algorithm there can be one hostkey" logic remains the case, although I don't think it would break anything if that changes -- pretty sure it would just lead to potential duplicate entries in ssh.ClientConfig.HostKeyAlgorithms which hopefully is harmless? Anyway that should be easy to solve if it is a problem. Not sure if other packages out there depend on that logic more strictly though.

[lonnywong]

@evanelias Thanks very much for your help. My case is multiple hostkeys of different types. I didn’t find https://github.com/golang/go/issues/29286 before. I’ll try your awesome package https://github.com/skeema/knownhosts this weekend. Thanks again.

[gopherbot]

Message from Gopher Robot:

Patch Set 1:

Congratulations on opening your first change. Thank you for your contribution!

Next steps: A maintainer will review your change and provide feedback. See https://go.dev/doc/contribute#review for more info and tips to get your patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be surprising to people new to the project. The careful, iterative review process is our way of helping mentor contributors and ensuring that their contributions have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which little code gets reviewed or merged. If a reviewer responds with a comment like R=go1.11 or adds a tag like "wait-release", it means that this CL will be reviewed as part of the next development cycle. See https://go.dev/s/release for more details.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/478535. After addressing review feedback, remember to publish your drafts!

[caarlos0]

anything I can do to help move this forward?