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

Include tpm2_getcap as required tpm2 pin binary #503

Conversation

sarroutbi
Copy link
Collaborator

No description provided.

@jlebon
Copy link
Contributor

jlebon commented Nov 29, 2024

For context, Ignition traditionally has pulled in all the Clevis/TPM-related executables into the initramfs related to binding since that's understandably not covered by the dracut modules here:

https://github.com/coreos/ignition/blob/e54b43f7ca60ac1a7ff7ff078359ad72458dda00/dracut/30ignition/module-setup.sh#L49-L61

If that's the case here (i.e. that tpm2_getcap is only required at binding time), then it probably makes more sense to leave this in Ignition only. If it's also required for unlocking, then it should live here.

Relatedly, it'd be great if we could move that Ignition list into a separate Clevis dracut module here that would be easier to keep up to date. Though probably to do this well we'd want to add CI testing as well. This assumes there's interest beyond Ignition in doing binding from the initramfs.

@sarroutbi
Copy link
Collaborator Author

For context, Ignition traditionally has pulled in all the Clevis/TPM-related executables into the initramfs related to binding since that's understandably not covered by the dracut modules here:

https://github.com/coreos/ignition/blob/e54b43f7ca60ac1a7ff7ff078359ad72458dda00/dracut/30ignition/module-setup.sh#L49-L61

Thanks for the explanation

If that's the case here (i.e. that tpm2_getcap is only required at binding time), then it probably makes more sense to leave this in Ignition only. If it's also required for unlocking, then it should live here.

In case clevis-pin-tpm2 is not used, tpm2_getcap is needed for unlocking

Relatedly, it'd be great if we could move that Ignition list into a separate Clevis dracut module here that would be easier to keep up to date. Though probably to do this well we'd want to add CI testing as well. This assumes there's interest beyond Ignition in doing binding from the initramfs.
We would welcome a PR for that if you find it interesting

Copy link
Contributor

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

In that case, LGTM! I noticed you backported this to Fedora already. Can we also get it in c9s/c10s? (We're hitting it there too: openshift/os#1656).

@sarroutbi
Copy link
Collaborator Author

sarroutbi commented Nov 29, 2024

In that case, LGTM! I noticed you backported this to Fedora already. Can we also get it in c9s/c10s? (We're hitting it there too: openshift/os#1656).

It is already there:
https://gitlab.com/redhat/centos-stream/rpms/clevis/-/merge_requests/43
https://gitlab.com/redhat/centos-stream/rpms/clevis/-/merge_requests/42

You should have it in next clevis release, but I can not ensure it. QA is still verifying it.

@sergio-correia sergio-correia merged commit 29d0a4e into latchset:master Nov 30, 2024
12 checks passed
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.

3 participants