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

Add flag to verify OIDC issuer in certificate #1308

Merged
merged 1 commit into from
Jan 14, 2022

Conversation

haydentherapper
Copy link
Contributor

@haydentherapper haydentherapper commented Jan 13, 2022

Summary

This PR adds support for --cert-oidc-issuer to verify the OIDC issuer of a Fulcio certificate. In combination with --cert-email, users should be able to verify and pin the expected identity of the Fulcio certificate.

Also added certificate generation test utilities. These might be more useful in sigstore/sigstore.

Ticket Link

Ref #556

Release Note

* Added `--cert-oidc-issuer` to verify the OIDC issuer of a Fulcio certificate

@dlorenc
Copy link
Member

dlorenc commented Jan 13, 2022

Nice!

@mattmoor FYI for the policy task force

@mattmoor
Copy link
Member

Awesome. Looks like it needs a rebase

Copy link
Member

@mattmoor mattmoor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great to see 👍

Comment on lines 35 to 36
cmd.Flags().StringVar(&o.CertEmail, "cert-email", "",
"the email expected in a valid Fulcio certificate")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should consider changing this since only a handful of things use email, and the rest use URIs.

Doesn't have to be here (probably shouldn't be), but something related that's been bothering me.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we change this to CertSubject? #1313

With github too, it's difficult to get the whole job workflow ref (maybe you only want to verify it came from A workflow in the org/repo -- could we do partial matches?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of that info is in extensions already (I think you added it!), and I think this is where it may make more sense to pass some payload with all the data to something like Cue/Rego for policy may be more manageable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a comment in the bug, I think a cert subject flag would be great to handle both email and URIs.

Given that there is ongoing work for policies (correct?), I agree that we should probably avoid adding complex policy checks to flags, and keep flag-based checks simple.

cmd/cosign/cli/options/certificate.go Outdated Show resolved Hide resolved
This adds support for --cert-oidc-issuer. In combination
with --cert-email, users should be able to verify and pin
the expected identity of the Fulcio certificate.

Also added certificate generation test utilities.

Signed-off-by: Hayden Blauzvern <[email protected]>
@haydentherapper
Copy link
Contributor Author

Rebased and updated with flag doc change. I also removed x509.ExtKeyUsage(x509.KeyUsageDigitalSignature) from the list of EKUs used during cert verification, because digital signature is not an extended key usage.

@dlorenc dlorenc merged commit 079e28d into sigstore:main Jan 14, 2022
@github-actions github-actions bot added this to the v1.5.0 milestone Jan 14, 2022
@haydentherapper haydentherapper deleted the issuer branch January 14, 2022 18:34
mlieberman85 pushed a commit to mlieberman85/cosign that referenced this pull request May 6, 2022
This adds support for --cert-oidc-issuer. In combination
with --cert-email, users should be able to verify and pin
the expected identity of the Fulcio certificate.

Also added certificate generation test utilities.

Signed-off-by: Hayden Blauzvern <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants