Navigation Menu

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

Jump to bottom

x/crypto/ssh: add support for hostkeys@openssh.com #37245

Open
FiloSottile opened this issue Feb 16, 2020 · 18 comments
Open

x/crypto/ssh: add support for hostkeys@openssh.com #37245

FiloSottile opened this issue Feb 16, 2020 · 18 comments
Labels
FeatureRequest help wanted NeedsFix The path to resolution is known, but the work has not been done. Proposal Proposal-Accepted Proposal-Crypto Proposal related to crypto packages or other security issues
Milestone
Backlog

Comments

@FiloSottile
Copy link
Contributor

The hostkeys@openssh.com extension lets a server notify a client of all its host keys to enable a smooth transition if they have UpdateHostKeys enabled. We should offer it server-side.
@FiloSottile FiloSottile added help wanted NeedsFix The path to resolution is known, but the work has not been done. FeatureRequest labels Feb 16, 2020
@gopherbot gopherbot added this to the Unreleased milestone Feb 16, 2020
@aphistic
Copy link

I'm interested in taking this on, as it would be helpful for our implementations. I've reviewed the extension details in the openssh PROTOCOL file and it's relatively light on specifics, but seems to be enough to implement it.

One question I have on the x/crypto/ssh side of things: You mention implementation on server-side, but if memory serves all the tests in that package use the client to test the server. Since there won't be a client-side implementation (maybe a config callback could be added that would be called when changes are found?), how would you expect the server side to be tested? Just sending raw messages from a client to the server and checking the expected responses?

@drakkan
Copy link
Member

drakkan commented Sep 14, 2023

Hi @aphistic,

I'm a little short on time right now but this is also on my TODO.
I think we should also add a client implementation consisting of callbacks to implement in the application using crypto/ssh.

@aphistic
Copy link

Hey @drakkan!

Is this something where you'd want to collaborate on it, or I should put it off? Do you have any thoughts on the implementation of it that I should implement?

We currently have a fork of x/crypto we're maintaining with support for RFC8332 and RFC8308 (server-sig-algs), but also has a couple of modifications made to allow us to introspect things about the connection (host key alg or kex alg chosen for a connection) and allowed us to implement the host key extension in our service directly.

Since the recent release of x/crypto now implements the features we originally created the fork for, we're looking to get back to using the upstream version. In order to use that, though, we'd need to have the host keys extension implemented. That's where the desire to work on this comes from.

@drakkan
Copy link
Member

drakkan commented Sep 14, 2023

Hi @aphistic,

yes I would like to collobarate but I'm currently busy with other things, if this is urgent for you feel free to start. I haven't had time to think about the implementation details yet. I just added this issue to my TODO and I marked it as low priority

Is your crypto fork public? I think it would be a good starting point.

I recently became a crypto/ssh maintainer and I'm trying to help implement the common features we all need 😄

I'm currently working on these issues

@achal1012
Copy link

Hello, I would like to contribute and help here if possible, I am also interested in this feature. @aphistic can you share the host key implementation that you have?

@drakkan drakkan mentioned this issue Jan 20, 2024
Open
@drakkan
Copy link
Member

drakkan commented Jan 27, 2024

Hello,

I propose the following API changes:

// HostKeysProve sends a hostkeys-prove@openssh.com message to request the
// server prove ownership of the private half of the key and validates the
// received signatures.
func (c *Client) HostKeysProve(publicKeys []PublicKey) error 

// HostKeys00Callback is the function type used to receive the host keys sent by
// servers using the hostkeys-00@openssh.com global message.
type HostKeys00Callback func(publicKeys []PublicKey)

type ClientConfig struct {
    ....
    // HostKeys00Callback is called after the authentication if the server sends
    // a hostkeys-00@openssh.com message. The client configuraton can supply
    // this callback to get notified about the received public keys.
    HostKeys00Callback HostKeys00Callback

No server-side API changes are needed because the hostkeys-00@openssh.com request can always be sent after user-authentication has completed. We only need to allow to accept multiple host keys with the same key format in AddHostKey.

We can also add something like FixedHostKeys to simplify verification for multiple host keys but I think this should be a separate commit

@gopherbot
Copy link

Change https://go.dev/cl/559055 mentions this issue: ssh: add support for hostkeys@openssh.com

@drakkan
Copy link
Member

drakkan commented Jan 27, 2024

Help with testing the above CL and thoughts on it are greatly appreciated. Thank you!

@drakkan
Copy link
Member

drakkan commented Feb 3, 2024 •
edited

CL updated with new API

// HostKeyUpdate represents a public key sent from servers using the
// hostkeys-00@openssh.com global message.
type HostKeyUpdate struct {
	key PublicKey
}

// Fingerprint returns the key's fingerprint as unpadded base64 encoded sha256
// hash.
func (h *HostKeyUpdate) Fingerprint() string {
	return FingerprintSHA256(h.key)
}

// KeyType returns the key type.
func (h *HostKeyUpdate) KeyType() string {
	return h.key.Type()
}

// Equal reports whether the public part of the host key matches the one
// supplied.
func (h *HostKeyUpdate) Equal(key PublicKey) bool {
	return bytes.Equal(h.key.Marshal(), key.Marshal())
}

// PublicKey sends a hostkeys-prove@openssh.com message to request the server
// prove ownership of the private half of the key and returns the public key if
// the verification is successful.
func (h *HostKeyUpdate) PublicKey(client *Client) (PublicKey, error) {
	if err := client.hostKeysProve([]PublicKey{h.key}); err != nil {
		return nil, err
	}
	return h.key, nil
}


// HostKeysUpdateCallback is the function type used to receive the host keys
// sent by servers using the hostkeys-00@openssh.com global message.
type HostKeysUpdateCallback func(keysUpdate []HostKeyUpdate)

type ClientConfig struct {
    ....
    // HostKeysUpdateCallback is called after the authentication if the server
    // sends a hostkeys-00@openssh.com message. The client configuration can
    // supply this callback to get notified about the received public keys.
    HostKeysUpdateCallback HostKeysUpdateCallback
    ....

The purpose of these changes is to allow clients to get public keys only after servers prove ownership.
Clients can use Fingerprint, KeyType and Equal methods to check if they already know the public key.

Also updated the check in AddHostKey to avoid adding the same host key multiple times. I think this was the purpose of the previous code. If we add the same host key multiple times OpenSSH returns an error like this

client_input_hostkeys: received duplicated ssh-rsa host key

@FiloSottile
Copy link
Contributor Author 

type HostKeysUpdateCallback func(keysUpdate []HostKeyUpdate)

we should make this

type HostKeysUpdateCallback func([]*HostKeyUpdate)

@FiloSottile
Copy link
Contributor Author

Also, (*HostKeyUpdate).PublicKey shouldn't take a Client, which instead should be saved in HostKeyUpdate.

@drakkan
Copy link
Member

drakkan commented Feb 6, 2024

@FiloSottile thank you, CL updated

// HostKeyUpdate represents a public key sent from servers using the
// hostkeys-00@openssh.com global message.
type HostKeyUpdate struct {
	key       PublicKey
	mux       *mux
	sessionID []byte
}

// Fingerprint returns the key's fingerprint as unpadded base64 encoded sha256
// hash. This is the same format returned by [FingerprintSHA256].
func (h *HostKeyUpdate) Fingerprint() string {
	return FingerprintSHA256(h.key)
}

// KeyType returns the key type.
func (h *HostKeyUpdate) KeyType() string {
	return h.key.Type()
}

// Equal reports whether the public part of the host key matches the one
// supplied.
func (h *HostKeyUpdate) Equal(key PublicKey) bool {
	return bytes.Equal(h.key.Marshal(), key.Marshal())
}

// PublicKey sends a hostkeys-prove@openssh.com message to request the server
// prove ownership of the private half of the key and returns the public key if
// the verification is successful.
func (h *HostKeyUpdate) PublicKey() (PublicKey, error) {
	if err := h.mux.hostKeysProve([]PublicKey{h.key}, h.sessionID); err != nil {
		return nil, err
	}
	return h.key, nil
}

// HostKeysUpdateCallback is the function type used to receive the host keys
// sent by servers using the hostkeys-00@openssh.com global message.
type HostKeysUpdateCallback func(keysUpdate []*HostKeyUpdate)

type ClientConfig struct {
    ....
    // HostKeysUpdateCallback is called after the authentication if the server
    // sends a hostkeys-00@openssh.com message. The client configuration can
    // supply this callback to get notified about the received public keys.
    HostKeysUpdateCallback HostKeysUpdateCallback
    ....

@drakkan
Copy link
Member

drakkan commented Feb 14, 2024

Proposed API

// HostKeyUpdate represents a public key sent from servers using the
// hostkeys-00@openssh.com global message.
type HostKeyUpdate struct {}

// Fingerprint returns the key's fingerprint as unpadded base64 encoded sha256
// hash. This is the same format returned by [FingerprintSHA256].
func (h *HostKeyUpdate) Fingerprint() string 

// KeyType returns the key type.
func (h *HostKeyUpdate) KeyType() string 

// Equal reports whether the public part of the host key matches the one
// supplied.
func (h *HostKeyUpdate) Equal(key PublicKey) bool 

// PublicKey sends a hostkeys-prove@openssh.com message to request the server
// prove ownership of the private half of the key and returns the public key if
// the verification is successful.
func (h *HostKeyUpdate) PublicKey() (PublicKey, error)

// HostKeysUpdateCallback is the function type used to receive the host keys
// sent by servers using the hostkeys-00@openssh.com global message.
type HostKeysUpdateCallback func(keysUpdate []*HostKeyUpdate)

type ClientConfig struct {
    ....
    // HostKeysUpdateCallback is called after the authentication if the server
    // sends a hostkeys-00@openssh.com message. The client configuration can
    // supply this callback to get notified about the received public keys.
    HostKeysUpdateCallback HostKeysUpdateCallback
    ....

cc @golang/proposal-review

@ericwq ericwq mentioned this issue Feb 19, 2024
Closed
@FiloSottile FiloSottile changed the title x/crypto/ssh: add support for hostkeys@openssh.com proposal: x/crypto/ssh: add support for hostkeys@openssh.com Feb 27, 2024
@FiloSottile FiloSottile added the Proposal-Crypto Proposal related to crypto packages or other security issues label Feb 27, 2024
@FiloSottile FiloSottile modified the milestones: Unreleased, Proposal Feb 27, 2024
@rsc
Copy link
Contributor

rsc commented Mar 8, 2024

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Mar 13, 2024

Any comments on the API in #37245 (comment)?

I assume the empty struct is not really empty but has unexported fields.

@drakkan
Copy link
Member

drakkan commented Mar 13, 2024

@rsc, I confirm that the struct has unexported fields used to immplement the public methods

Here are the struct fields in the proposed CL

type HostKeyUpdate struct {
	key       PublicKey
	mux       *mux
	sessionID []byte
}

key is used to implement Fingerprint, KeyType and Equal methods. mux and sessionID are used to send the hostkeys-prove@openssh.com to the server to verify the public key before returning it.

The public key is not exposed directly to prevent users from trusting it without sending the hostkeys-prove@openssh.com request.

@rsc
Copy link
Contributor

rsc commented Mar 27, 2024

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

The proposal details are in #37245 (comment).

@rsc
Copy link
Contributor

rsc commented Apr 4, 2024

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

The proposal details are in #37245 (comment).

@rsc rsc changed the title proposal: x/crypto/ssh: add support for hostkeys@openssh.com x/crypto/ssh: add support for hostkeys@openssh.com Apr 4, 2024
@rsc rsc modified the milestones: Proposal, Backlog Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
FeatureRequest help wanted NeedsFix The path to resolution is known, but the work has not been done. Proposal Proposal-Accepted Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
Proposals
Status: Accepted
Milestone
Backlog
Development

No branches or pull requests
6 participants
@rsc @aphistic @drakkan @FiloSottile @achal1012 @gopherbot