golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.71k stars 17.5k forks source link

proposal: x/crypto/ssh: allow the client to automatically detect host key algorithms #68619

Open drakkan opened 1 month ago

drakkan commented 1 month ago

Proposal Details

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 provided using ClientConfig.HostKeyCallback.

If our users don't set ClientConfig.HostKeyAlgorithms we should try to obtain the expected algorithms from the configured ClientConfig.HostKeyCallback.

To enable this automatic detection, I propose to add a new error that HostKeyCallback implementations can return to inform about supported algorithms:

// HostKeyCallbackError may be returned from [HostKeyCallback] implementations
// to inform about supported host key algorithms. If no [ClientConfig]
// HostKeyAlgorithms are set, the [Client] will execute the callback with a
// non-existent key type to discover and use the expected algorithms. For
// ssh-rsa key format we suggest returning sha-2 variants of the algorithms.
type HostKeyCallbackError struct {
    // HostKeyAlgorithms lists the public key algorithms that the callback will
    // accept.
    HostKeyAlgorithms []string
}

It is preferable to return HostKeyAlgorithms and not the key formats because this way implementations can decide, for example, to return only the sha-2 variants for the ssh-rsa key format.

if ClientConfig.HostKeyAlgorithms is not set, our client will execute the callback passing a sentinel, non-existent, key type and, if the error returned is an HostKeyCallbackError, the returned host key algorithms are used.

We should return HostKeyCallbackError in our internal HostKeyCallback implementations:

gabyhelp commented 1 month ago

Related Issues and Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

FiloSottile commented 1 month ago

I suggest a slightly different semantic: the HostKeyCallback error (let's call it UnknownKeyError) would be returned for any unknown key, and have a field KnownTypes []string.

The caller then filters the algorithms to only the ones compatible with those key types. (Host key callbacks don't usually know anything about algorithms, so let's make the error also only concern itself with key types. This also avoids the mistake of returning [ssh-rsa] and accidentally disabling SHA-2.)

Our client will call the callback proactively with a fake key to filter the ClientConfig.HostKeyAlgorithms (or the default). Our FixedHostKey will return the error. knownhosts.KeyError will add a As method filling out a UnknownKeyError.

drakkan commented 1 month ago

@FiloSottile thank you. Here is the updated proposal

// UnknownKeyError can be returned from [HostKeyCallback] implementations when
// they receive an unknow key to inform about supported key formats. Our
// [Client] will execute the callback, before connecting, with a non-existent
// key type to discover and use the supported key formats.
type UnknownKeyError struct {
    // KnownTypes lists the public key formats that the callback will accept.
    KnownTypes []string
}
imirkin commented 1 week ago

This also avoids the mistake of returning [ssh-rsa] and accidentally disabling SHA-2.)

Just ran into this particular mistake myself with some code that was written prior to the SHA-2 support, so something which encourages this to work would be desirable. I was thinking that maybe in addition to ClientConfig.HostKeyAlgorithms, there could be a ClientConfig.HostKeyTypes. But any approach that doesn't require the user of the ssh library to know about different algorithms for the same key type would be great.