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

x/crypto: document how to unmarshal ssh certs #22046

Closed
cviecco opened this issue Sep 26, 2017 · 13 comments
Closed

x/crypto: document how to unmarshal ssh certs #22046

cviecco opened this issue Sep 26, 2017 · 13 comments

Comments

@cviecco
Copy link

@cviecco cviecco commented Sep 26, 2017

What did you do?

While writing an application that generates ssh certificates I wanted to audit its output, by verifying the signed ssh certificate against signing policies.

What did you expect to see?

A public function that unmarshalls a []byte into an *ssh.Certificate. This is 98% done already with the private parseCert function.

What did you see instead?

No public function to unmarshal ssh certificates (a private one exists) and the Unmarshall (which is the mirror of Marshall and not suitable for my purposes ( see: #21491 )).

@gopherbot gopherbot added this to the Unreleased milestone Sep 26, 2017
@cviecco
Copy link
Author

@cviecco cviecco commented Sep 26, 2017

The code for this function would be trivial:

func ParseCertificate(wireBytes []byte) (*Certificate, error) {
algo, in, ok := parseString(wireBytes)
if !ok {
return nil, errShortRead
}
return parseCert(in, certToPrivAlgo(string(algo)))
}

@mdempsky mdempsky changed the title x/crypto: propsal: Add public method to unmarshal ssh certificates. proposal: x/crypto: add public method to unmarshal ssh certificates. Sep 26, 2017
@gopherbot gopherbot added the Proposal label Sep 26, 2017
@rsc
Copy link
Contributor

@rsc rsc commented Oct 2, 2017

/cc @hanwen @x1ddos @FiloSottile @agl (recent Reviewers of golang.org/x/net/ssh)

@hanwen
Copy link
Contributor

@hanwen hanwen commented Oct 2, 2017

doesn't ParsePublicKey work?

btw - what does "verifying the signed ssh certificate against signing policies" mean?

@cviecco
Copy link
Author

@cviecco cviecco commented Oct 2, 2017

No, parse publicKey gets me the public key, not the other members of the certificate like the ValidPrincipals, ValidAfter, ValidBefore etc.

what does "verifying the signed ssh certificate against signing policies" mean?
After I generate a certificate ( (*Certificate) SignCert ) and give it to an external user ( (*Certificate) Marshal) I wan to take the marshalled representation and verify that the ValidBefore, ValidPrincipals match what I expect.
Think certificate transparency for ssh certs.

@hanwen
Copy link
Contributor

@hanwen hanwen commented Oct 3, 2017

public key is an interface. You can cast to ssh.Certificate, see https://gist.github.com/hanwen/ec620792123ffee5c2cdfbcc33fab0da

SignCert just signs the certificate in place (setting nonce, signature and signaturekey). You can (and should!) check the fields of the certificate that you want to sign before you call SignCert.

Marshal does not modify the certificate, so there is no need to go through a Marshal/Unmarshal step when you do this check.

@cviecco
Copy link
Author

@cviecco cviecco commented Oct 3, 2017

I guess I explained myself poorly. I have two processes: P1: generates ssh certs and writes the marshalled certificate to a log. P2: Audits P1 by unmarshalling the generated certificates(in the log) and looking at the fields.

The publicKey interface is not enough for auditing purposes. This is an ask for the inverse of (*Certficiate)Marsall just like the crypto/x509's ParseCertificate.

@hanwen
Copy link
Contributor

@hanwen hanwen commented Nov 13, 2017

"The publicKey interface is not enough for auditing purposes." - you can cast the PublicKey to a certificate and audit it to your heart's desire.

I don't want to provide syntactic sugar for what is essentially a one-liner, but maybe you can think of a way to clarify the docs?

@rgooch
Copy link

@rgooch rgooch commented Nov 13, 2017

We don't know what that one-liner is. Can you please post sample code which performs this conversion?

@hanwen
Copy link
Contributor

@hanwen hanwen commented Nov 13, 2017

pk, .. , err := ssh.ParsePublicKey( .. )

cert := pk.(*ssh.Certificate)

@rgooch
Copy link

@rgooch rgooch commented Nov 13, 2017

Wow. Thank you for showing this. I'm happy to report that this works for us. I have to say, this is basically non-discoverable. I would suggest adding a note to the documentation for ssh.ParsePublicKey saying that - given the correct input - will return *ssh.Certificate as the concrete type.

@gopherbot
Copy link

@gopherbot gopherbot commented Jan 22, 2018

Change https://golang.org/cl/88895 mentions this issue: ssh: clarify how to parse out Certificates

@rsc rsc changed the title proposal: x/crypto: add public method to unmarshal ssh certificates. x/crypto: document how to unmarshal ssh certs Jan 29, 2018
@IxDay
Copy link

@IxDay IxDay commented Jan 16, 2019

Is there a way to add some part of this response to the documentation ?
It took me way too long to find the proper way to parse a certificate file in order to get a public key out of it.

bored-engineer pushed a commit to bored-engineer/ssh that referenced this issue Oct 13, 2019
Fixes golang/go#22046

Change-Id: I9a9aff37ba0fd0ca1f5fa1a212c66b812f6b9f70
Reviewed-on: https://go-review.googlesource.com/88895
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
bored-engineer pushed a commit to bored-engineer/ssh that referenced this issue Oct 13, 2019
Fixes golang/go#22046

Change-Id: I9a9aff37ba0fd0ca1f5fa1a212c66b812f6b9f70
Reviewed-on: https://go-review.googlesource.com/88895
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
bored-engineer pushed a commit to bored-engineer/ssh that referenced this issue Oct 13, 2019
Fixes golang/go#22046

Change-Id: I9a9aff37ba0fd0ca1f5fa1a212c66b812f6b9f70
Reviewed-on: https://go-review.googlesource.com/88895
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@golang golang locked and limited conversation to collaborators Jan 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

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