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 intermediate CA implementation with in-memory signer #494

Closed
wants to merge 1 commit into from

Conversation

haydentherapper
Copy link
Contributor

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

Summary

Ticket Link

Ref #489

Release Note


@haydentherapper
Copy link
Contributor Author

cc @bobcallaway
Looking for comments on the usage of goroutines and mutexes. I've tested locally, this is working.

Not in love with the amount of reader locks. The goroutine communication seems to be in line with the recommended style.

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

Codecov Report

Merging #494 (d9fbd8c) into main (61c9601) will increase coverage by 0.28%.
The diff coverage is n/a.

❗ Current head d9fbd8c differs from pull request most recent head b38b0db. Consider uploading reports for the commit b38b0db to get more accurate results

@@            Coverage Diff             @@
##             main     #494      +/-   ##
==========================================
+ Coverage   47.97%   48.25%   +0.28%     
==========================================
  Files          14       14              
  Lines        1063     1063              
==========================================
+ Hits          510      513       +3     
+ Misses        480      478       -2     
+ Partials       73       72       -1     
Impacted Files Coverage Δ
pkg/ca/fileca/load.go 71.42% <0.00%> (+3.29%) ⬆️

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 61c9601...b38b0db. Read the comment docs.

@@ -53,7 +54,7 @@ func newServeCmd() *cobra.Command {

cmd.Flags().StringVarP(&serveCmdConfigFilePath, "config", "c", "", "config file containing all settings")
cmd.Flags().String("log_type", "dev", "logger type to use (dev/prod)")
cmd.Flags().String("ca", "", "googleca | pkcs11ca | fileca | ephemeralca (for testing)")
cmd.Flags().String("ca", "", "googleca | pkcs11ca | fileca | intermediateca | ephemeralca (for testing)")
Copy link
Member

Choose a reason for hiding this comment

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

given the dependency on google CA service for intermediateca, does it make sense to have the name reflect that somehow?

@nsmith5
Copy link
Contributor

nsmith5 commented Mar 30, 2022

If scaled horizontally, this would result in several different in memory signers and different responses on /api/v1/rootCert or /api/v2/trustBundle` (soon to come) right?

@haydentherapper
Copy link
Contributor Author

Yep, it would. Intermediates would change, but the root would be the same.

@nsmith5
Copy link
Contributor

nsmith5 commented Mar 30, 2022

Brace for noob question 😆 : I don't understand how verification would work if that was the case. Only the leaf code signing certificate is uploaded to Rekor.

With the binary I want to verify in hand, I can hash and look it up in the Rekor log. That gives me a signature and leaf cert. To verify the leaf certificate chains to the Fulcio root I need the full chain of CA certificates back the the Fulcio root right? How does a verifier obtain the ephemeral intermediate certificate that are part of that chain in this case?

If it has been rotated since the issuing event there is no way I can grab it from /api/v1/rootCert and if it hasn't I have a 1/n chance of receiving it from /api/v1/rootCert where n is the number of fulcio replicas

@haydentherapper
Copy link
Contributor Author

Question away!

The certificate chain doesn't/shouldn't get verified based on what's returned by Rekor. (Separately, I am actually working on support for adding the cert chain, but there's a larger question around how to search when you don't know if an entry contains just the cert chain or the cert). The verification key/cert/chain comes from either CLI input or the OCI object (expect in one case for verify-blob, discussed below).

The ephemeral intermediate CA certificate needs to be persisted alongside the leaf certificate. This is done currently in the OCI signature. For other use-cases, it's up to the client to figure out how to store the cert.

Rekor shouldn't be used as storage for certificates. For example, the way that verify-blob fetches a certificate given an artifact isn't ideal. Rekor is meant as a witness to an event, and to find that proof, you need to know the contents of the Rekor entry (the signature, artifact hash, and verification key). Searching means you will accept someone else's proof and verification key.

As for the comment about api/v1/rootCert, lemme think on this. Could build an aggregator that calls all instances of Fulcio to build a trust bundle maybe.

@haydentherapper
Copy link
Contributor Author

High level comment: Intermediate CA certs are meant to be untrusted and just used for chain building from a leaf to a root. It's always up to the client to store these in the same way they store leaf certificates.

@nsmith5
Copy link
Contributor

nsmith5 commented Mar 30, 2022

Ahhhh ok I think I understand whats up here. Limme check my understanding:

  • When requesting a code signing certificate the client will grab the leaf and the intermediate(s) (basically everything it needs to chain up that final Fulcio root certificate)
  • The leaf and intermediate will be stored alongside the signature

Verifiers can then check that this chains back to Fulcio root cert because they have the intermediates in hand already and then can also check the signature because they have the leaf in hand.

I think this makes sense, but I can foresee some modifications to the API and perhaps clients to make it function as expected. I recall then when I did the fileca stuff it serves up the intermediate and root chain on /api/v1/rootCert. It assumes there is just the one intermedate shared by all replicas and I'm not sure that the clients grabbed that intermedate and stored it along side the signature. That said, I test a lot with sign-blob and not OCI signature so maybe that already works as expected there.

@haydentherapper
Copy link
Contributor Author

Cosign will not need any updates besides figuring out verify-blob, I've wrapped up the work for verifying intermediates. External clients may need changes, hence why I want to do this before 1.0. Even with a static intermediate CA certificate, clients may need to be updated if they're not handling the certificate chain returned by Fulcio.

Thanks for pointing out replicas, this is something I'll need to consider. I do quite like the idea of an ephemeral key, but without coordination between the replicas, this might not be feasible.

@haydentherapper
Copy link
Contributor Author

Thanks for the comments y'all. I want to unblock this work and I don't see a clear path forward to having replicas share the same ephemeral cert and key.

I'll be opening a new PR that adds a CA backed by KMS with an on-disk certificate chain.

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