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: add ssh.ParseHostKey #41709

Closed
elansys-kc opened this issue Sep 30, 2020 · 8 comments
Closed

proposal: x/crypto/ssh: add ssh.ParseHostKey #41709

elansys-kc opened this issue Sep 30, 2020 · 8 comments

Comments

@elansys-kc
Copy link

@elansys-kc elansys-kc commented Sep 30, 2020

I don't necessarily wish to retrieve a servers publicKey hostkey via go on all platforms in my app and receive it OOB.

I was going to write my own function but noticed that ssh.ParseAuthorizedKeys works just fine.

Does a copy of or wrapper for ssh.ParseAuthorizedKeys called ssh.ParseHostKeys make sense for this library?

@gopherbot gopherbot added this to the Unreleased milestone Sep 30, 2020
@andybons andybons changed the title x/crypto/ssh Feature Request: ssh.ParseHostKey proposal: x/crypto/ssh: add ssh.ParseHostKey Oct 1, 2020
@gopherbot gopherbot added the Proposal label Oct 1, 2020
@andybons
Copy link
Member

@andybons andybons commented Oct 1, 2020

@hanwen
Copy link
Contributor

@hanwen hanwen commented Oct 2, 2020

@FiloSottile
Copy link
Member

@FiloSottile FiloSottile commented Oct 2, 2020

I'm not sure I understand, server public keys are stored in known_hosts files, for which there is ParseKnownHosts.

@elansys-kc
Copy link
Author

@elansys-kc elansys-kc commented Oct 9, 2020

My reasoning is as follows:

I don't wish to be tied to a hostname or IP address as I may wish to use broadcast DNS if MDNS or DNS fail.

I am signing the host key as retrieved from the OpenSSH server filesystem, via a provisioning http service, and so need to retrieve additional files, pk/sig.

Additionally, go is unlikely to have access to storage on all devices like android/ios and so simply reading a string equating to the server side host key format, seems to be the neatest option?

I also read that knownHosts parsing can fail on corrupted lines and so figured storing a single host key may be simpler but that would rather be reason for a pull request, if not already fixed.

Whilst ssh.ParseHostKey, could simply mention "generally not needed". I can see that it may be a little confusing to document it's existence, particularly with knownhosts being in a separate package.

Perhaps it is better to simply copy and maintain the function myself?

@elansys-kc
Copy link
Author

@elansys-kc elansys-kc commented Oct 9, 2020

Actually on the server side, I assume that generating and signing a known hosts format file with host */wildcard would work. Seems a bit like jumping through hoops but may have some unforeseen benefits. Sorry for the noise.

@elansys-kc elansys-kc closed this Oct 9, 2020
@elansys-kc
Copy link
Author

@elansys-kc elansys-kc commented Oct 9, 2020

It seems that ssh.ParseAuthorizedKeys and so ssh.ParseHostKeys may be needed to create a known hosts file using go, in the first place?

e.g. create a known hosts file from "/etc/ssh/ssh_host_ed25519_key.pub"

Should I simply write a custom function?

@elansys-kc elansys-kc reopened this Oct 9, 2020
@elansys-kc
Copy link
Author

@elansys-kc elansys-kc commented Oct 12, 2020

I shall just use the following until and if a stdlib function is created.

// SSHParseHostKey : Parses a host key as generated by ssh-keygen and returns a
//                public key type useable by "golang.org/x/crypto/ssh"
func SSHParseHostKey(file []byte) (out ssh.PublicKey, comment, keyType string, err error) {

	hostKeyFileStr := string(file)
	hostKeySplit := strings.Split(hostKeyFileStr, " ")
	if len(hostKeySplit) != 3	{
		err = library.LogPrint(nil, err, "invalid file input: was the file(",file,") generated by ssh-keygen?")
		return nil, "", "", err
	}
	keyType = hostKeySplit[0]
	hostKeyb64 := hostKeySplit[1]
	comment = hostKeySplit[2]

	hostKeyWire, err := base64.StdEncoding.DecodeString(hostKeyb64)
	if err != nil {
		//library.LogPrint(nil, err, "failed to b64 decode hostkey, was the file (",file,") generated by ssh-keygen?");
		return nil, "", "", err
	}

	out, err = ssh.ParsePublicKey(hostKeyWire)
	if err != nil {
		//library.LogPrint(nil, err, "failed to parse the decoded hostkey, was the file (",file,") generated by ssh-keygen?");
		return nil, "", "", err
	}
	return out, comment, keyType, err
}

If a ssh.ParseHostsKeys isn't deemed a useful function for the stdlib then I have no objection to this issue simply being closed.

@ianlancetaylor ianlancetaylor added this to Incoming in Proposals Oct 14, 2020
@FiloSottile
Copy link
Member

@FiloSottile FiloSottile commented Oct 14, 2020

Generally, host keys are stored as known_hosts files, so while I see your use case, I think it would add more confusion than convenience to support a different format for them. As you pointed out, you can also just use ParseAuthorizedKeys, as there is nothing client- or host-key specific in the format.

Moreover, if you're interested in just storing a simple compact representation of a single key, you could just store the base64 portion (which includes the key type) and just call base64.StdEncoding.DecodeString and ssh.ParsePublicKey without the string splitting.

@kevinburke kevinburke moved this from Incoming to Likely Decline in Proposals Nov 2, 2020
@rsc rsc moved this from Likely Decline to Declined in Proposals Nov 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Proposals
Declined
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.