Codecov Report

Attention: Patch coverage is 63.47687% with 229 lines in your changes missing coverage. Please review.

Project coverage is 52.08%. Comparing base (cf238ac) to head (3fb1a80).
Report is 288 commits behind head on main.

@@            Coverage Diff             @@
##             main    #1870      +/-   ##
==========================================
- Coverage   57.93%   52.08%   -5.85%     
==========================================
  Files          50       73      +23     
  Lines        3119     5646    +2527     
==========================================
+ Hits         1807     2941    +1134     
- Misses       1154     2431    +1277     
- Partials      158      274     +116     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

i think this is ready for 👀 now. just a couple of notes.

1. the following use-cases are now covered:

• root ca -> leaf
• root ca -> intermediate ca -> leaf

2. the following kms providers are working:

• awskms
• azurekms
• gcpkms

3. hashivault was added, but has not been tested.

i think that about covers it.

cc @haydentherapper

Thanks @ianhundere, I’ll take a look at this later this week.

a nudge for 👀

cc @haydentherapper / @bobcallaway

@bobcallaway thanks for the fb especially w/ the gcpkms callout / i totally missed that one. below is a summary of the fb/changes implemented:

1. now using gcpkms instead of cloudkms to keep consistent w/ naming schemes used elsewhere in fulcio and sigstore etc:

createCmd.Flags().StringVar(&kmsType, "kms-type", "", "KMS provider type (awskms, gcpkms, azurekms)")

2. cli flags are now more specific to the kms svc used (e.g. gcp / azure / aws):

createCmd.Flags().StringVar(&kmsTenantID, "azure-tenant-id", "", "Azure KMS tenant ID")
createCmd.Flags().StringVar(&kmsCredsFile, "gcp-credentials-file", "", "Path to credentials file for GCP KMS")
createCmd.Flags().StringVar(&kmsRegion, "aws-region", "", "AWS KMS region")

3. made env vars more specific/consistent to the kms svc/flag used (e.g. gcp / azure / aws)

AWS_REGION
AZURE_TENANT_ID
GCP_CREDENTIALS_FILE

4. replaced kms signing from go.step.sm/crypto/kms/* to sigstore/pkg/signature/kms/*
5. added hashivault as a kms-type (not tested)

i'll implement the same fb w/ tsa.

cc @haydentherapper

ps also updated the readme/docs above. let me know if i should include that in some shape way or form in the repo itself.

another nudge for 👀 / thanks y'all

cc @haydentherapper / @bobcallaway

@haydentherapper okidoki, i implemented all your suggested feedback including templating.

the behavior now is that templates will be included via the embed package which are used as defaults / a user can still use their own templates to take advantage of https://pkg.go.dev/go.step.sm/crypto/x509util for advanced use-cases.

Thank you for the great work on this!

absolutely, the tsa PR is also up for review. have a good wknd!

@ianhundere Just realized one thing when reviewing the TSA PR. Should we remove the leaf certificate template processing from Fulcio? For the TSA, this is needed, as the leaf certificate signs timestamps. For Fulcio, Fulcio is the CA so it issues leaf certificates.

@haydentherapper ah, good point / so just to confirm, essentially remove all elements associated w/ leaf cert creation (e.g. leaf template, flags etc), correct ?

and should i do that in a separate PR ?

Sorry for two separate threads! I just replied on the TSA PR as well. I'd suggest that we make the leaf cert creation optional, and then point TSA operators to this repo, which will reduce code duplication. The only thing we need in the TSA repo then is the TSA templates for root/intermediate/leaf and a README.