-
Notifications
You must be signed in to change notification settings - Fork 402
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
fix(crypt): close crypt devices to release encryption keys get on shutdown #2471
base: master
Are you sure you want to change the base?
Conversation
Please fix commit message to e.g.
|
@LaszloGombos alright, just updated it. |
|
Updated once more |
@adrelanos I appreciate your thought/review on this. Thanks ! |
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.
lgtm
modules.d/90crypt/module-setup.sh
Outdated
@@ -96,8 +96,12 @@ install() { | |||
fi | |||
|
|||
inst_hook cmdline 30 "$moddir/parse-crypt.sh" | |||
inst_hook shutdown 24 "$moddir/crypt-shutdown.sh" | |||
if type "cryptsetup" > /dev/null 2>&1; then |
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.
Perhaps the following is easier to read (and type) instead, but it is certainly just a matter of taste
inst_multiple -o cryptsetup
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.
Yes, I didn't know of this simple way of optional includes, as this is my first time fiddling with dracut. Thanks for bringing this up @LaszloGombos , I will update it accordingly for simplicity.
Is this actually an issue also with systemd-cryptsetup ? https://www.freedesktop.org/software/systemd/man/crypttab.html#x-initrd.attach |
Promising option, I've just tried adding it to my crypttab. However, it looks like it didn't change anything. At the time crypt-shutdown.sh got run, the encrypted device was still attached. |
Not sure if this is helpful, but just in case - systemd/systemd#25538 |
Works nice with a Debian bookworm installed using Debian Installer, which sets up an encrypted LVM with unencrypted /boot. Broken with Debian bookworm (actually Kicksecure) installed with Calamares, which sets up an encrypted /boot but without LVM. Debugged as much as I could. Got some udev issue. Maybe something a simple as a missing dracut module?
Modified the script to gather more debug output.
No output. Good.
No output. Good.
So there is really nothing still mounted. The issue is |
The latest commit should fix above mentioned udev issues. At this stage, udev has already been shut down, but may have left the control socket behind. Cryptsetup makes use of dmsetup, which in turn checks whether this socket exists. When dmsetup finds this socket file, it will try to synchronize with udev. But since udev is not running, it will wait forever. Simply removing the leftover socket file fixed it for me. |
Great! Works for me. |
Could this be reviewed please? |
I better approach would be to call |
@LaszloGombos agreed, but I did not find any reference on a delayed detaching in
|
Thanks. I would recommend to try the following first
|
I've created a pull request in systemd that would implement deferred detaching, see systemd/systemd#32650 and the latest commit makes use of this new feature, if |
Changes
This pull request adds a shutdown hook to close encrypted devices and wipe their encryption keys from kernel memory.
Cryptsetup may not be installed on every system, if systemd-cryptsetup is used. That's why I made this feature optional in order not to break any running systems.
Checklist
Fixes #997 #1888