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/knownhosts: interactive host key verification #40298

Closed
drognisep opened this issue Jul 20, 2020 · 2 comments
Closed

proposal: x/crypto/ssh/knownhosts: interactive host key verification #40298

drognisep opened this issue Jul 20, 2020 · 2 comments

Comments

@drognisep
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.14.4 windows/amd64

I'd like to submit a PR to include the option of an interactive prompt for host key verification in the case where the host key is not present in the known_hosts sources. Adding a prompt like what exists with the ssh CLI tool would be nice for other interactive tools that use this package.

Example:

The authenticity of host 'localhost (127.0.0.1)' can't be established.
ECDSA key fingerprint is SHA256:un8VXgUIyeqFIkd/l+avJyKOQvIvuthI8wkva0OmclU.
Are you sure you want to continue connecting (yes/no/[fingerprint])?

Current function:

func New(files ...string) (ssh.HostKeyCallback, error)

There are a few different ways I could see this going, so I'm requesting feedback on what would be most preferable.

Approach 1
I think this would be most easily done by adding a function to the x/crypto/ssh/knownhosts package to specify that verification should be interactive. This could be added at this line.

Proposed added function:

// NewInteractive creates a HostKeyCallback with an extended HostKeyFallback that
// shows an interactive prompt. If the user accepts the host key signature, then the
// key will be added to all sources specified by the 'files' parameter.
func NewInteractive(files ...string) (ssh.HostKeyCallback, error)

Approach 2
Alternatively, an option to include a handler for this functionality could be added so the user can make their own determinations about how to handle this situation. This may be preferable for some, but puts the extended verification process in the hands of the devs using this package. This is arguable not ideal considering that the existing verification processes are behind unexported functions, which seems to indicate that you're not trying to expose people to much of the complexity of host cert/key verification.

// UnknownHostKeyHandler allows the caller to provide an additional handler that gets called
// in the event that the host key is not revoked and not found in any of the provided known_hosts
// file locations.
type UnknownHostKeyHandler func(hostname string, remote net.Addr, key PublicKey) error

// NewInteractive enables the use of an UnknownHostKeyHandler.
func NewInteractive(unknownHandler UnknownHostKeyHandler, files ...string) (ssh.HostKeyCallback, error) {
    /* ... */
}

Approach 3
Another way to extend this could be with a configuration object that is passed to a new New function. This would provide an extension point for later use and provide the desired interactive behavior. I don't think there's a whole lot that would need to be added to this package and still fit within its scope, but it's an option nonetheless.

// InteractivePrompt uses the provided information to construct an interactive prompt string
// that is displayed to the user.
type InteractivePrompt func(hostname string, remote net.Addr, key PublicKey) string

// HostVerificationConfig allows callers to customize host verification.
type HostVerificationConfig struct {
    InteractiveHostKey bool,
    Prompt InteractivePrompt,
}

// NewCustom allows the caller to provide HostVerificationConfig to customize the host verification process.
func NewCustom(verificationConfig HostVerificationConfig, files ...string) (ssh.HostKeyCallback, error) {
    /* ... */
}
@gopherbot gopherbot added this to the Unreleased milestone Jul 20, 2020
@toothrot toothrot changed the title x/crypto/ssh/knownhosts proposal: interactive host key verification x/crypto/ssh/knownhosts: proposal: interactive host key verification Jul 21, 2020
@toothrot toothrot modified the milestones: Unreleased, Proposal Jul 21, 2020
@toothrot
Copy link
Contributor

/cc @hanwen @FiloSottile

@drognisep
Copy link
Author

@toothrot @hanwen @FiloSottile

I think the interesting part of this would be the write back. Currently, it looks like all of the known host information is aggregated into one source, so it would have to be changed to include the file paths so a newly accepted/verified host key could be appended. This piece doesn't seem like a massive change beyond new functionality, as it wouldn't affect any of the existing logic except initialization, where the array of file paths could be initialized.

However, in the case where the user has selected multiple sources of verified host key information, is there a "right" source to update? The best answer I can come up with is to add the new verified host key to all sources. The rationale behind this is that the user has selected multiple sources of verified hosts as "correct". When the user accepts/verifies a host key, adding that key to only one source doesn't make a lot of sense because it would seem to indicate that one of the sources is "more correct" than the others, even though such a concept does not exist in aggregate comparison. If one of the files is the "wrong" one to add the host key to, what does that say about the validity of any of the information in that file?

Looking forward to hearing from you all. 😁

@ianlancetaylor ianlancetaylor changed the title x/crypto/ssh/knownhosts: proposal: interactive host key verification proposal: x/crypto/ssh/knownhosts: interactive host key verification Aug 7, 2020
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Aug 7, 2020
@ianlancetaylor ianlancetaylor removed this from Incoming in Proposals (old) Mar 22, 2021
@golang golang locked and limited conversation to collaborators Sep 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants