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

Update attestation for a key #419

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Update attestation for a key #419

wants to merge 2 commits into from

Conversation

areed
Copy link
Contributor

@areed areed commented Feb 6, 2024

Currently a new key has to be created every time a new certificate is authorized with the acme device attestation protocol. This allows a new attestation to be generated with fresh qualifying data from the CA.

Name of feature:

Pain or issue this feature alleviates:

Why is this important to the project (if not answered above):

Is there documentation on how to use this feature? If so, where?

In what environments or workflows is this feature supported?

Linux

In what environments or workflows is this feature explicitly NOT supported (if any)?

Windows

Supporting links/other PRs/issues:

💔Thank you!

Currently a new key has to be created every time a new certificate is
authorized with the acme device attestation protocol. This allows a new
attestation to be generated with fresh qualifying data from the CA.
@hslatman
Copy link
Member

hslatman commented Feb 7, 2024

The test error is likely caused by not using the ctx and opening the TPM using the existing logic. There's a bit of plumbing to ensure the right command channel is set in all cases, incl. the in-process simulator for testing. See https://github.com/smallstep/crypto/blob/8684dc8bf25a0502f4c01cba205218c3cb09495a/tpm/key.go#L212C2-L215C30 for an example.

It's nice to see the new tpm2 API being used, but I'm not sure if it's compatible with the existing plumbing as described above. If it is: great; otherwise would advise to use the legacy package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants