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.
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
refactor: Rewrite luks tpm2 script. #302
refactor: Rewrite luks tpm2 script. #302
Changes from all commits
4a70d22
eb26c37
8d3f306
cb49743
39b3add
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This validation can be more thorough. For example, if the user accidentally enrolled multiple tokens, or has no backup passphrase.
In the latter case, I believe systemd-cryptenroll will throw an error, which would break the script if not handled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the same logic that we had previously. We can put a warning at the top to make sure you still have a decryption method besides the tpm.
We could have a warning during the enrollment to always ensure that you have a password or recovery key to ensure you have access to the data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@m2Giles I don't see this logic in the previous script, unless you mean the backing up of the previous crypttab?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do like the check.
I think the simple thing will be checking if passphrase is in slot 0. If not it's safe to assume the user manually configured the device.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@m2Giles yeah that's a reasonable assumption.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@m2Giles I think this script should be as foolproof as possible, so if there's any sign that the user has manually touched cryptsetup, we should fail and say "it looks like you've got this covered :)"
otherwise you open this up to needing to support and/or work around custom configurations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirming a comment I made elsewhere,
systemd-cryptenroll
will NOT wipe the tpm2 slot if it's the last remaining slot.Granted, if a user is living dangerously if they get to this state, and I think it would have to be by use of having a tpm2 slot and recover-key slot both setup by systemd-cryptenroll, and then having wiped the recovery slot.
Similarly,
cryptsetup luksKillSlot
prevents removing of a slot using that slot's passphrase.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As an aside, if a user does reach the state of only a tpm2 slot... a passphrase can be re-added:
cryptsetup luksAddKey --token-id 2 --token-type systemd-tpm2 /dev/disk/by-uuid/d4f1e47c-a653-4cd5-a6ae-bfc62c0a6852