Do not merge (depends on storage-ng) - Supporting LUKS2 encryption by TPM2 device (jsc#PED-10703) #713
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
schubi2
changed the title
jreidinger
reviewed
Mar 28, 2025
| output.strip | ||
| end | ||
|
|
||
| # Enabe TPM2, if it is required |
jreidinger
reviewed
Mar 28, 2025
|
|
||
| begin | ||
| Yast::Execute.on_target!("keyctl", "padd", "user", "cryptenroll", "@u", | ||
| stdout: :capture, |
There was a problem hiding this comment.
why stdout is captured, when it is not used?
jreidinger
reviewed
Mar 28, 2025
| if entry.empty? | ||
| "" | ||
| else | ||
| entry.first["title"] |
There was a problem hiding this comment.
this looks strange as you basically search for first entry, so it should look like
entry = @data.find { |d| d["id"] == @default }
entry ? entry["title"] : ""
jreidinger
reviewed
Mar 28, 2025
| log.warn "Invalid value #{value} trying to set as default. Fallback to default" | ||
| value = "" | ||
| entry = @data.select { |d| d["title"] == value } | ||
| if entry.empty? |
There was a problem hiding this comment.
same here. The used method should not be select, but find that return first entry or nil.
jreidinger
reviewed
Mar 28, 2025
| @@ -155,7 +157,7 @@ def packages | |||
| res = super | |||
| res << ("grub2-" + grub2bls_architecture + "-efi-bls") | |||
| res << "sdbootutil" | |||
| res << "shim" if secure_boot | |||
| res << "shim" | |||
There was a problem hiding this comment.
why this change is needed? I think there are architectures where shim is no available, but as it is grub2-bls architectures are already limited, os not sure.
jreidinger
reviewed
Mar 28, 2025
| def check_tpm2 | ||
| return true unless Y2Storage::StorageManager.instance.encryption_use_tpm2 | ||
|
|
||
| add_new_problem(_("This bootloader cannot handle encryption supported by a TPM2 device. ")) |
There was a problem hiding this comment.
so even our patched grub2 cannot handle tpm2 device?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
We would like to give the user an easy way to use TPM2 devices for LUKS2 encrypted
partitions. (jsc#PED-10703)
The user can select TPM2 support for encryption in yast-storage-ng:
yast/yast-storage-ng#1406
Solution
Enrolling TPM2 with the given password, if the user has selected it.
Proposal: Selecting yast2-bls bootloader which supports TPM2.
Testing
Screenshots