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 certificate chain flag for signing #1656

Merged
merged 3 commits into from
Mar 26, 2022

Conversation

haydentherapper
Copy link
Contributor

This allows users to pass their own certificate chain to include in the
OCI signature. The chain is checked for validity using the provided
certificate.

Also refactored the check for matching public keys using a method from
sigstore/sigstore, comparing the certificate's key with the provided
key. Also added this check when extracting the PKCS11 certificate.

Certificate chains must be PEM-encoded. I changed the text of the
certificate flag to also specify a preference for PEM encoding, but
didn't remove the code that handles DER encoding for backwards
compatibility.

Signed-off-by: Hayden Blauzvern hblauzvern@google.com

Summary

Ticket Link

Ref #1554

Release Note

Added cert-chain flag for including certificate chain in OCI signature on signing

@haydentherapper
Copy link
Contributor Author

cc @bburky @dlorenc

@codecov-commenter
Copy link

codecov-commenter commented Mar 24, 2022

Codecov Report

Merging #1656 (9892326) into main (bdef009) will increase coverage by 0.97%.
The diff coverage is 28.57%.

@@            Coverage Diff             @@
##             main    #1656      +/-   ##
==========================================
+ Coverage   28.08%   29.06%   +0.97%     
==========================================
  Files         139      139              
  Lines        8025     8189     +164     
==========================================
+ Hits         2254     2380     +126     
- Misses       5523     5546      +23     
- Partials      248      263      +15     
Impacted Files Coverage Δ
cmd/cosign/cli/attest.go 0.00% <0.00%> (ø)
cmd/cosign/cli/options/attest.go 0.00% <0.00%> (ø)
cmd/cosign/cli/options/sign.go 0.00% <0.00%> (ø)
cmd/cosign/cli/policy_init.go 1.43% <0.00%> (ø)
cmd/cosign/cli/sign.go 0.00% <0.00%> (ø)
cmd/cosign/cli/sign/sign_blob.go 0.00% <0.00%> (ø)
cmd/cosign/cli/sign/sign.go 11.83% <42.30%> (+10.20%) ⬆️
pkg/cosign/kubernetes/webhook/validator.go 80.49% <0.00%> (-1.22%) ⬇️
pkg/apis/config/image_policies.go 69.38% <0.00%> (ø)
pkg/cosign/kubernetes/webhook/validation.go 80.21% <0.00%> (+1.64%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bdef009...9892326. Read the comment docs.

This allows users to pass their own certificate chain to include in the
OCI signature. The chain is checked for validity using the provided
certificate.

Also refactored the check for matching public keys using a method from
sigstore/sigstore, comparing the certificate's key with the provided
key. Also added this check when extracting the PKCS11 certificate.

Certificate chains must be PEM-encoded. I changed the text of the
certificate flag to also specify a preference for PEM encoding, but
didn't remove the code that handles DER encoding for backwards
compatibility.

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
@haydentherapper
Copy link
Contributor Author

@dlorenc Are you familiar with ./hack/update-codegen.sh? I ran it, but it had a few errors, and ended up generating a bunch of code files in vendors/ rather than updating the licenses.

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
@haydentherapper
Copy link
Contributor Author

Figured out the issue, looks like some part of the script needs a license binary to be installed via go install instead of go get.

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
@bburky
Copy link
Contributor

bburky commented Mar 25, 2022

I tested with a couple configurations:

# The following works great and adds the chain annotation:
cosign sign --key "awskms:///..." --cert cert.pem  --cert-chain chain.pem "localhost:5000/$image"  
# The following does _not_ work. The certificate is automatically extracted from the smartcard, but the provided chain is not added as an annotation:
cosign sign --key "pkcs11:..." --cert-chain chain.pem "localhost:5000/$image"  

In general, I've noticed the (optional) --cert flag doesn't seem to work for --key "pkcs11:..." and this could be related. The cert provided from the token is used unconditionally, the cert on the command line is not used. It should probably be an error to pass --cert, or it should be used instead of the one on the token.

Perhaps the cosign sign --key "pkcs11:..." --cert-chain chain.pem behavior is related? It seems that the provided cert chain is ignored.

@haydentherapper
Copy link
Contributor Author

Thanks for testing this out. Currently, with a pkcs11 key, it attempts to fetch a certificate from the token and then returns early. You're correct, it's currently ignoring the cert and cert chain flags. I just need to change it to not exit early.

@dlorenc dlorenc merged commit db90d13 into sigstore:main Mar 26, 2022
@github-actions github-actions bot added this to the v1.7.0 milestone Mar 26, 2022
@haydentherapper haydentherapper deleted the chain-sign branch March 28, 2022 17:17
mlieberman85 pushed a commit to mlieberman85/cosign that referenced this pull request May 6, 2022
* Add certificate chain flag for signing

This allows users to pass their own certificate chain to include in the
OCI signature. The chain is checked for validity using the provided
certificate.

Also refactored the check for matching public keys using a method from
sigstore/sigstore, comparing the certificate's key with the provided
key. Also added this check when extracting the PKCS11 certificate.

Certificate chains must be PEM-encoded. I changed the text of the
certificate flag to also specify a preference for PEM encoding, but
didn't remove the code that handles DER encoding for backwards
compatibility.

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>

* Adding 3rd party licenses

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>

* Added check for empty chain

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
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.

None yet

4 participants