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

gpt-auto-generator: enable TPM2 unlocking in gpt-auto-generator #30185

Merged
merged 1 commit into from
Nov 28, 2023

Conversation

poettering
Copy link
Member

@poettering poettering commented Nov 24, 2023

If we detect a TPM, let's also unlock the disk with it, if it has an enrollment for that.

See: #30176

@github-actions github-actions bot added the please-review PR is ready for (re-)review by a maintainer label Nov 24, 2023

This comment was marked as off-topic.

@poettering
Copy link
Member Author

So this should fix the specific issue in #30176, and we probably should merge this, before v255.

But we actually have a bigger problem in general, and I am not sure how to address this best: since token module support was added to cryptsetup we'd try a "blanket" activation via token before doing the finer grained stuff. Butt that's a mess, because it will pull in our own token modules, and run them before we get to do so explicitly. This is a mess because this way we don't get the chance to pass our various settings into the token modules, hence they run without...

We probably should tweak the blanket activation to do so only for tokens that are not ours, i.e. foreign.

Man this is all so complex...

@thesamesam
Copy link
Contributor

That fixes it for me, thanks.

@yuwata yuwata added this to the v255 milestone Nov 24, 2023
Copy link
Member

@YHNdnzj YHNdnzj left a comment

Choose a reason for hiding this comment

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

Just some cosmetic stuff

src/gpt-auto-generator/gpt-auto-generator.c Outdated Show resolved Hide resolved
@YHNdnzj YHNdnzj added good-to-merge/with-minor-suggestions and removed please-review PR is ready for (re-)review by a maintainer labels Nov 25, 2023
@SoapGentoo
Copy link
Contributor

any reason not to merge this?

If we detect a TPM, let's also unlock the disk with it, if it has an
enrollment for that.

Fixes: systemd#30176
@YHNdnzj YHNdnzj added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed good-to-merge/with-minor-suggestions labels Nov 28, 2023
@bluca bluca merged commit 0d5f59a into systemd:main Nov 28, 2023
42 of 46 checks passed
@github-actions github-actions bot removed the good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed label Nov 28, 2023
@gdamjan
Copy link
Contributor

gdamjan commented Jan 2, 2024

Ah nice,
this should've been mentioned in the NEWS file.

Goodbye setting rd.luks.name=b598…=root rd.luks.options=b598…=tpm2-device=auto in my kernel command line. this was the only place where I had to specify the boot device in the UKI world.

@ElvishJerricco
Copy link
Contributor

If I'm understanding correctly, the way efi_measured_uki is used here means this only works if you boot with a sd-stub UKI. I mean I guess it can be argued that trying to TPM-unlock the root fs from a Type 1 boot entry is maybe not the best idea, but it still seems like it would make sense to have. At the very least, a TPM+pin LUKS slot is more secure than just the pin.

bluca added a commit to bluca/systemd that referenced this pull request Jun 3, 2024
…ailable

After systemd#30185 every attach operation
has a TPM device configured, even if it is not used, so attach_luks_or_plain_or_bitlk()
attempts to use it (first in the list), fails and falls back to the loop,
but the pass/keyfile parameters are cleared out. If multiple methods
are configured and the ones tried first fail with EAGAIN, try immediately
the next one, without returning to the loop.

Follow-up for 0d5f59a
bluca added a commit to bluca/systemd that referenced this pull request Jun 3, 2024
…ailable

After systemd#30185 every attach operation
has a TPM device configured, even if it is not used, so attach_luks_or_plain_or_bitlk()
attempts to use it (first in the list), fails and falls back to the loop,
but the pass/keyfile parameters are cleared out. If multiple methods
are configured and the ones tried first fail with EAGAIN, try immediately
the next one, without returning to the loop.

Fixes systemd#30425

Follow-up for 0d5f59a
poettering added a commit to poettering/systemd that referenced this pull request Jun 4, 2024
If we couldn't unlock a device with the chosen unlock path, let's not
fall back to the lowest one right away, but only flush out one path, and
try the next.

Fixes: systemd#30425
Follow-up-for: systemd#30185
Alternative-to: systemd#33183
poettering added a commit to poettering/systemd that referenced this pull request Jun 4, 2024
If we couldn't unlock a device with the chosen unlock path, let's not
fall back to the lowest one right away, but only flush out one path, and
try the next.

Fixes: systemd#30425
Follow-up-for: systemd#30185
Alternative-to: systemd#33183
bluca pushed a commit to poettering/systemd that referenced this pull request Jun 4, 2024
If we couldn't unlock a device with the chosen unlock path, let's not
fall back to the lowest one right away, but only flush out one path, and
try the next.

Fixes: systemd#30425
Follow-up-for: systemd#30185
Alternative-to: systemd#33183
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

8 participants