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

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

Open
drakkan opened this issue Jul 27, 2024 · 4 comments
Labels
Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Milestone

Comments

@drakkan
Copy link
Member

drakkan commented Jul 27, 2024

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:

  • ssh.FixedHostKey
  • knownhosts.New
@gopherbot gopherbot added this to the Proposal milestone Jul 27, 2024
@ianlancetaylor ianlancetaylor added the Proposal-Crypto Proposal related to crypto packages or other security issues label Jul 27, 2024
@FiloSottile
Copy link
Contributor

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

drakkan commented Aug 3, 2024

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

imirkin commented Aug 27, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
Status: Incoming
Development

No branches or pull requests

6 participants