-
Notifications
You must be signed in to change notification settings - Fork 137
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
Implement basic AWS CloudHSM support for root CA creation + rewrite "FulcioCA" to "PKCS11CA" #187
Conversation
wow, now that is how to write up a PR! This looks good to me Mark, only thing I would add is to make additions to the README (such you have aboe for using libcloudhsm_pkcs11), I understand this may be pending though as its an WIP PR. @dlorenc / @bobcallaway on ideas on the root CA being stored locally, I see no is security impact as the priv key is in the HSM. This is not to different to what we have with the GCP public good instance. I guess the main aspect is if someone wants this to be highly available. I am ok with addressing that later when we get user feedback to suggest adoption, we can then look to use an ec2 bucket api. |
cmd/app/root.go
Outdated
@@ -46,7 +46,8 @@ func Execute() { | |||
|
|||
func init() { | |||
rootCmd.PersistentFlags().StringVar(&logType, "log_type", "dev", "logger type to use (dev/prod)") | |||
rootCmd.PersistentFlags().String("ca", "", "googleca | fulcioca") | |||
rootCmd.PersistentFlags().String("ca", "", "googleca | pkcs11ca") | |||
rootCmd.PersistentFlags().String("aws_hsm_root_ca_path", "", "Path to root CA on disk (only used with AWS HSM)") | |||
rootCmd.PersistentFlags().String("gcp_private_ca_parent", "", "private ca parent: /projects/<project>/locations/<location>/<name> (only used with --ca googleca)") | |||
rootCmd.PersistentFlags().String("hsm-caroot-id", "", "HSM ID for Root CA (only used with --ca fulcio)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rootCmd.PersistentFlags().String("hsm-caroot-id", "", "HSM ID for Root CA (only used with --ca fulcio)") | |
rootCmd.PersistentFlags().String("hsm-caroot-id", "", "HSM ID for Root CA (only used with --ca pkcs11ca)") |
} | ||
block, _ := pem.Decode(pubPEMData) | ||
if block == nil || block.Type != "CERTIFICATE" { | ||
log.Logger.Fatal("failed to decode PEM block containing certificate") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the intent here is good, but would cause a dropped connection to the client. If we did this once at initialization, that address this issue as well
5072e69
to
df8793c
Compare
@lukehinds @bobcallaway I've just pushed a few changes that address most of the minor comments people have made. I've also amended my setup guide and put it in its own On another note, I think that it would be helpful to reorganize/simplify the various pieces of documentation in a follow up PR. As is, they're kind of just sitting around. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor nits, and opened separate issue for fixing per-transaction loading. otherwise LGTM
cmd/app/createca.go
Outdated
if viper.GetString("hsm") == "aws" { | ||
log.Logger.Info("Running in AWS mode; skipping root CA storage in HSM") | ||
if !viper.IsSet("out") { | ||
log.Logger.Info("WARNING: --out is not set. Root CA will not be saved.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this log level be a bit higher? If you're putting WARNING
in the message, should it at least be log.Logger.Warn
?
cmd/app/createca.go
Outdated
err = p11Ctx.ImportCertificateWithLabel(rootID, []byte(LABEL), certParse) | ||
if err != nil { | ||
log.Logger.Fatal(err) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
err = p11Ctx.ImportCertificateWithLabel(rootID, []byte(LABEL), certParse) | |
if err != nil { | |
log.Logger.Fatal(err) | |
} | |
if err = p11Ctx.ImportCertificateWithLabel(rootID, []byte(LABEL), certParse); err != nil { | |
log.Logger.Fatal(err) | |
} |
cmd/app/createca.go
Outdated
@@ -149,6 +156,7 @@ func init() { | |||
createcaCmd.PersistentFlags().String("street-address", "", "Locality name for root CA") | |||
createcaCmd.PersistentFlags().String("postal-code", "", "Locality name for root CA") | |||
createcaCmd.PersistentFlags().String("out", "", "output root CA to file") | |||
createcaCmd.PersistentFlags().String("hsm", "softhsm", "The HSM provider to use. Valid values: aws, softhsm (default)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
createcaCmd.PersistentFlags().String("hsm", "softhsm", "The HSM provider to use. Valid values: aws, softhsm (default)") | |
createcaCmd.PersistentFlags().String("hsm", "softhsm", "The HSM provider to use. Valid values: softhsm (default), aws") |
This adds basic support for self-provisioning a root CA with AWS CloudHSM through the existing PKCS11 support. Since the "FulcioCA" option now encompasses both BYOD HSMs and AWS CloudHSM, it's been renamed to the more all-encompassing "PKCS11CA". Signed-off-by: Mark Bestavros <mbestavr@redhat.com>
df8793c
to
46a0e09
Compare
Thanks @bobcallaway for the feedback - @lukehinds ready to merge? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, will merge this week just in case @bobcallaway has any further notes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
This adds basic support for self-provisioning a root CA with AWS CloudHSM through the existing PKCS11 support. Since the "FulcioCA" option now encompasses both BYOD HSMs and AWS CloudHSM, it's been renamed to the more all-encompassing "PKCS11CA".
Signed-off-by: Mark Bestavros mbestavr@redhat.com
How it works
AWS CloudHSM provides a PKCS#11 compliant library for interacting with instances of its CloudHSM product. For the most part, this serves as a drop-in replacement for the
SoftHSM
PKCS#11 library; as long as keys are set up beforehand, just as Fulcio expects to be done when using SoftHSM, it (mostly) works with no code changes.The exception is that CloudHSM does not support certificate storage on their HSMs:
In its current state, Fulcio does attempt to store the root CA it creates as part of the
createca
command within the HSM it's using, and this causes an error when running with AWS CloudHSM.I've explored many possible workarounds for this limitation, and run into just as many dead ends. While those might change in the future, for now, this is the most simple working implementation of something that works with AWS: effectively, during root CA creation when running on AWS, instead of attempting to store the root CA in the HSM or in some other certificate storage service, it's just stored on-disk as a PEM file. Then, later on, when running
fulcio serve
, another option is exposed to load the root CA from disk instead of the HSM. This works around the lack of certificate support on AWS' HSMs.Setup
AWS provides a rather comprehensive setup guide for getting a CloudHSM instance deployed and setting up an EC2 instance that can interact with it. For a complete setup, you'll want to make sure the CloudHSM Management Utility, Key Management Utility, and CloudHSM's PKCS#11 library are all installed on your EC2 instance.
First, you'll need to make sure your HSM is set up using the Management Utility using this guide. Next, you should provision keys within the HSM for Fulcio to use. Do this with the Key Management Utility:
loginHSM -u CU -s <username> -p <password>
genECCKeyPair -i 14 -l PKCS11CA -id 1
With these steps done, your HSM is set up!
Next: clone Fulcio with this patch included onto the EC2 instance that has the AWS PKCS11 library installed. You'll also need to adjust
config/crypto11.conf
as such, in order to make it work with the CloudHSM setup:The
Path
variable replaces thesofthsm
PKCS11 library with AWS CloudHSM's. On AWS CloudHSM, theTokenLabel
must always becavium
- or things won't work. ThePin
takes the format of a crypto user and corresponding password on the HSM, as seen here.With this done, you're ready to use Fulcio! Here are some example commands:
This command creates a new root CA using the private key stored in AWS CloudHSM and stores it in the
myrootCA.pem
file locally.And this command uses the generated
myrootCA.pem
file to run the Fulcio server.