x/crypto/ssh: host key algorithm selection prefers DSA over ED25519

[ORYLY]

When a server provides both a DSA host key and an ED25519 host key, the Go ssh library will select DSA instead. But DSA has been deprecated in OpenSSH (and in other libraries, I suppose).

This is because in crypto/ssh/common.go, ED25519 is at the end of supportedHostKeyAlgos, which is supposed to be in preference order:

var supportedHostKeyAlgos = []string{
	CertSigAlgoRSASHA2512v01, CertSigAlgoRSASHA2256v01,
	CertSigAlgoRSAv01, CertAlgoDSAv01, CertAlgoECDSA256v01,
	CertAlgoECDSA384v01, CertAlgoECDSA521v01, CertAlgoED25519v01,

	KeyAlgoECDSA256, KeyAlgoECDSA384, KeyAlgoECDSA521,
	SigAlgoRSASHA2512, SigAlgoRSASHA2256,
	SigAlgoRSA, KeyAlgoDSA,

	KeyAlgoED25519, // <-- lowest preference, unlike the other algo list constants that place ED25519 much higher
}

[dmitshur]

CC @neild, @rolandshoemaker.

[reedloden]

I believe the equivalent in OpenSSH (SSH_ALLOWED_CA_SIGALGS) has ED25519 first, as per https://github.com/openssh/openssh-portable/blob/0d96b1506b2f4757fefa5d1f884d49e96a6fd4c3/myproposal.h#L81-L89.

[drakkan]

Hello,

I definitely agree that we should update our algorithms to remove the insecure ones (dsa, sha-1 etc.), but since this is a backward incompatible change we need a proposal.

In this specific case let's assume we have a client like this

config := &ssh.ClientConfig{
	....
	HostKeyCallback: ssh.FixedHostKey(pub),
       ....

with a server supporting DSA and ED25519, it expects to validate the DSA key, if we switch the order we'll break it. (We should also have a host key callback that allows multiple host keys to be validated, but that's another proposal and I plan to add it in the fix for #37245 that I'm working on).

After this CL is merged, we'll have a clear distinction between secure and insecure algorithm and I'll propose to switch to the (currently) secure algorithms.

For host keys we'll use this one

supportedHostKeyAlgos = []string{
		CertAlgoRSASHA256v01, CertAlgoRSASHA512v01,
		CertAlgoECDSA256v01, CertAlgoECDSA384v01, CertAlgoECDSA521v01,
		CertAlgoED25519v01,

		KeyAlgoRSASHA256, KeyAlgoRSASHA512,
		KeyAlgoECDSA256, KeyAlgoECDSA384, KeyAlgoECDSA521,
		KeyAlgoED25519,
	}

I think the proposal is also the right place to discuss the ordering of algorithms and perhaps change it.