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

rsc commented Oct 2, 2017

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

@hanwen
Copy link
Contributor

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 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 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 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 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 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 commented Nov 13, 2017

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

cert := pk.(*ssh.Certificate)

@rgooch
Copy link

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

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 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.

@hanwen
Copy link
Contributor

hanwen commented Jan 16, 2019

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
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 28, 2022
Fixes golang/go#22046

Change-Id: I9a9aff37ba0fd0ca1f5fa1a212c66b812f6b9f70
Reviewed-on: https://go-review.googlesource.com/88895
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
Fixes golang/go#22046

Change-Id: I9a9aff37ba0fd0ca1f5fa1a212c66b812f6b9f70
Reviewed-on: https://go-review.googlesource.com/88895
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
LewiGoddard pushed a commit to LewiGoddard/crypto that referenced this issue Feb 16, 2023
Fixes golang/go#22046

Change-Id: I9a9aff37ba0fd0ca1f5fa1a212c66b812f6b9f70
Reviewed-on: https://go-review.googlesource.com/88895
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
BiiChris pushed a commit to BiiChris/crypto that referenced this issue Sep 15, 2023
Fixes golang/go#22046

Change-Id: I9a9aff37ba0fd0ca1f5fa1a212c66b812f6b9f70
Reviewed-on: https://go-review.googlesource.com/88895
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
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

6 participants