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 KMS-backed signer #496

Merged
merged 1 commit into from
Apr 9, 2022

Conversation

haydentherapper
Copy link
Contributor

This CA implementation will use an on-disk certificate chain and a
remote KMS signer to sign certificates. There is validation on server
startup that the provided chain matches the provided key.

I've also added a utility to generate the intermediate certificate by
calling GCP CA Service. This will be used to set up Fulcio.

This also refactors the code to add an intermediate CA struct that
implements the common methods. This makes it simple to add new
intermediate CA types, with each only needing to provide a method to
fetch a signer and certificate chain.

Updated sigstore/sigstore to pull in the latest change to compare public keys.

Tested with:
go run pkg/ca/intermediateca/fetch_ca_cert/fetch_ca_cert.go --kms-key="gcpkms://projects/<project>/locations/us-central1/keyRings/test-key-ring/cryptoKeys/ca-key/versions/1" --gcp-ca-parent="projects/<project>/locations/us-west1/caPools/<pool>" --output="chain.crt.pem"

go run main.go serve --port 5555 --ca kmsca --ct-log-url="" --kms-key="gcpkms://projects/<project>/locations/us-central1/keyRings/test-key-ring/cryptoKeys/ca-key/versions/1" --cert-chain-path="chain.crt.pem"

Made a call using a script based on the load testing tool, got back a certificate chain.

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

Summary

Ticket Link

Fixes #489

Release Note

Added a KMS-backed intermediate CA implementation

@haydentherapper
Copy link
Contributor Author

haydentherapper commented Mar 31, 2022

This is a good compromise between the various solutions. While an ephemeral key and certificate would be great, as discussed in #494, this doesn't work well with replicated instances. Moving to a KMS-backed signer gives us the ability to limit and audit signing key access. KMS will also enable us to have far more QPS than CA Service, and controlling the signer will let us implement embedded SCTs (PR coming right up!).

With this design, we'll issue intermediates for the same lifetime as the root. I'd like to include the intermediate in the TUF targets to ease client discovery. Also this simplifies the revocation story - If the KMS signer is compromised, we simply remove the intermediate certificate from the TUF target metadata and add a new one. This design also mitigates an issue with verify-blob not being able to find the intermediate cert for verification, since it will now be embedded with TUF.

@haydentherapper
Copy link
Contributor Author

cc @nsmith5 (Can't add you as a reviewer for some reason)

@haydentherapper
Copy link
Contributor Author

Will investigate failing test - It's not failing locally for me.

@haydentherapper
Copy link
Contributor Author

haydentherapper commented Mar 31, 2022

One last remark: I haven't added tests for kmsca.go because this relies on sigstore/sigstore's KMS library, and there's no mocks or fakes for this.

@codecov-commenter
Copy link

codecov-commenter commented Mar 31, 2022

Codecov Report

Merging #496 (72f22e6) into main (765a06a) will decrease coverage by 4.40%.
The diff coverage is 72.44%.

@@            Coverage Diff             @@
##             main     #496      +/-   ##
==========================================
- Coverage   47.97%   43.57%   -4.41%     
==========================================
  Files          14       18       +4     
  Lines        1063     1221     +158     
==========================================
+ Hits          510      532      +22     
- Misses        480      615     +135     
- Partials       73       74       +1     
Impacted Files Coverage Δ
pkg/ca/kmsca/kmsca.go 48.00% <48.00%> (ø)
pkg/ca/fileca/load.go 68.96% <66.66%> (+0.83%) ⬆️
pkg/ca/intermediateca/intermediateca.go 80.95% <80.95%> (ø)
pkg/ca/fileca/fileca.go 46.87% <100.00%> (+11.02%) ⬆️
pkg/api/client.go 75.22% <0.00%> (-1.92%) ⬇️
pkg/ca/x509ca/common.go 3.53% <0.00%> (ø)
pkg/ca/x509ca/x509ca.go 0.00% <0.00%> (ø)

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 765a06a...72f22e6. Read the comment docs.

@haydentherapper
Copy link
Contributor Author

Failing test resolved, I switched to using a directory generated by the test library.

@dlorenc
Copy link
Member

dlorenc commented Mar 31, 2022

Nice work!

@haydentherapper
Copy link
Contributor Author

Added tests for kmsca.go. Temporarily added a replace to my fork of sigstore/sigstore until sigstore/sigstore#361 is submitted.

@haydentherapper haydentherapper force-pushed the intermediate branch 2 times, most recently from 93fe5f1 to cb422ed Compare April 1, 2022 16:28
cmd/app/serve.go Show resolved Hide resolved
cmd/app/serve.go Outdated Show resolved Hide resolved
cmd/app/serve.go Outdated Show resolved Hide resolved
@priyawadhwa
Copy link
Contributor

This looks great!

@haydentherapper
Copy link
Contributor Author

PR should be ready to go!

Copy link
Contributor

@nsmith5 nsmith5 left a comment

Choose a reason for hiding this comment

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

Awesome! Really like the refactor into pkg/intermediateca

pkg/ca/intermediateca/fetch_ca_cert/fetch_ca_cert.go Outdated Show resolved Hide resolved
pkg/ca/intermediateca/fetch_ca_cert/fetch_ca_cert.go Outdated Show resolved Hide resolved
pkg/ca/kmsca/kmsca.go Show resolved Hide resolved
pkg/test/cert_utils.go Outdated Show resolved Hide resolved
Copy link
Contributor

@nsmith5 nsmith5 left a comment

Choose a reason for hiding this comment

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

Awesome work :D Just a nit about Kms versus KMS, but otherwise LGTM

pkg/ca/kmsca/kmsca.go Outdated Show resolved Hide resolved
@haydentherapper
Copy link
Contributor Author

Thanks! PR should be good to go unless there's any other comments.

This CA implementation will use an on-disk certificate chain and a
remote KMS signer to sign certificates. There is validation on server
startup that the provided chain matches the provided key.

I've also added a utility to generate the intermediate certificate by
calling GCP CA Service. This will be used to set up Fulcio.

This also refactors the code to add an intermediate CA struct that
implements the common methods. This makes it simple to add new
intermediate CA types, with each only needing to provide a method to
fetch a signer and certificate chain.

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
@dlorenc dlorenc merged commit 4db32b6 into sigstore:main Apr 9, 2022
@haydentherapper haydentherapper deleted the intermediate branch April 9, 2022 17:22
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.

Fulcio as an intermediate CA
5 participants