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 basic ACME device-attest-01 support #712

Merged
merged 43 commits into from
Apr 6, 2023
Merged

Add basic ACME device-attest-01 support #712

merged 43 commits into from
Apr 6, 2023

Conversation

hslatman
Copy link
Member

@hslatman hslatman commented Aug 4, 2022

The --attestation-uri and --attestation-ca-url flags can be used when requesting a
certificate from step-ca. The ACME device-attest-01 challenge method will be
used to authorize the certificate request when --attestation-uri indicates usage
of the TPM KMS (tpmkms).

The example below will perform the ACME device-attest-01 challenge verification
by including an AK certificate in the response to the ACME server, configured with
an ACME provisioner called attestation.

step ca certificate my-unique-id id.crt --attestation-uri tpmkms:name=first-key --provisioner attestation --attestation-ca-url https://attca:port

If no valid AK certificate is available, the client will try to enroll with the Attestation CA
at https://attca:port first. If an AK is already available, the existing certificate and private
key will be used. The AK private key to be used will be derived from the EK public key.

The private key for the certificate issued by the ACME server will be attested by the AK and
persisted in a step context aware directory, so that it can be reloaded when required.

The ACME provisioner must be configured with the trusted root of the Attestation CA, so
that the AK certificate can be verified.

Two additional flags are present to help connecting to the Attestation CA:

  • --attestation-ca-root can be used to provide the path to a file with trusted root CAs to
    use when a TLS connection is made to the Attestation CA.
  • --attestation-ca-insecure can be used to disable TLS validation when connecting
    to the Attestation CA.

The `--permanent-identifier` flag can be used when requesting a
certificate from `step-ca`. The ACME `device-attest-01` challenge
method will be used to authorize the certificate request.
@github-actions github-actions bot added the needs triage Waiting for discussion / prioritization by team label Aug 4, 2022
utils/cautils/tpm.go Fixed Show fixed Hide fixed
@hslatman hslatman marked this pull request as ready for review April 6, 2023 15:42
@hslatman hslatman requested review from maraino and dopey April 6, 2023 15:42
@hslatman
Copy link
Member Author

hslatman commented Apr 6, 2023

@maraino: There are still things to be improved w.r.t. using the CreateAttestor interface, but I think what exists today can be fit to work like that while being backwards compatible. It just needs a bit more work from my side. I would propose to do that when we gather some feedback from the field on how this is currently used.

Copy link
Collaborator

@maraino maraino left a comment

Choose a reason for hiding this comment

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

Added a few comments.

go.mod Show resolved Hide resolved

tpmAttestationCABaseURL := clictx.String("attestation-ca-url")
if tpmAttestationCABaseURL == "" {
return fmt.Errorf("flag %q cannot be empty", "--attestation-ca-url")
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should be consistent with these types of errors and use:

Suggested change
return fmt.Errorf("flag %q cannot be empty", "--attestation-ca-url")
return errs.RequiredFlag(ctx, "attestation-ca-url")

utils/cautils/tpm.go Outdated Show resolved Hide resolved
Comment on lines +158 to +165
cli.StringFlag{
Name: "attestation-ca-url",
Usage: "The base url of the Attestation CA to use",
},
cli.StringFlag{
Name: "attestation-ca-root",
Usage: "The path to the PEM <file> with trusted roots when connecting to the Attestation CA",
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

In the future, we might want to add some defaults for this, ok for now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or we can bootstrap them.

utils/cautils/tpm.go Outdated Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

Although this does not require CGO support, having some of this logic out of the CLI would be nice. I think right now is ok to have it here, but we should think about other options.

Right now, with a YubiKey, the attestation statement is on step kms attest, but in that case is because it requires CGO. We will need to integrate this there too.

Other options are to create an ACMEAttester interface or perhaps just an extension to the Attester interface in the crypto KMS package. So we can use it in step-kms-plugin as well as here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I agree that it would be nice to have it all behind a single interface. The thing that made it less simple, at least for me, is the online Attestation CA involved: there are more parameters that the step kms attest with a TPM has to know about. And it probably also needs to keep track of some state, independent of the CLI. The TPMStore seems like a good candidate for that, but it somewhat muddies its responsibility, so I'm not entirely sure about it yet.

utils/cautils/tpm.go Outdated Show resolved Hide resolved
@hslatman hslatman merged commit adccf6e into master Apr 6, 2023
@hslatman hslatman deleted the acme-attestation branch April 6, 2023 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage Waiting for discussion / prioritization by team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants