Skip to content

Improve systemd-ask-password integration#413

Merged
koverstreet merged 2 commits intokoverstreet:masterfrom
beviu:systemd-ask-password
Sep 21, 2025
Merged

Improve systemd-ask-password integration#413
koverstreet merged 2 commits intokoverstreet:masterfrom
beviu:systemd-ask-password

Conversation

@beviu
Copy link

@beviu beviu commented Sep 20, 2025

For users that mount the same encrypted bcachefs FS multiple times at boot, this patch makes it so the passphrase is only asked once.

The first commit makes it so systemd-ask-password is invoked when stdin is /dev/null, instead of using our own passphrase prompting code. This is the case when a bcachefs FS is mounted by systemd: by default, systemd executes the mount helper with stdin set to /dev/null.

The second commit fixes a race condition where when multiple bcachefs mount commands are executed in parallel, all of them will see that there is no key in the keyring and start an instance of systemd-ask-password. We can ask systemd-ask-password to try to reuse an existing passphrase in the keyring. systemd-ask-password is smart and knows to check again every time another systemd-ask-password instance completes, so as soon as the first systemd-ask-password command is complete, the others ones will return as well.

src/key.rs Outdated

fn is_dev_null(fd: BorrowedFd) -> io::Result<bool> {
let mut buf: MaybeUninit<libc::stat> = MaybeUninit::uninit();
let ret = unsafe { libc::fstat(fd.as_raw_fd(), buf.as_mut_ptr()) };
Copy link
Owner

Choose a reason for hiding this comment

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

are you sure we don't have a better fstat wrapper available?

Copy link
Author

Choose a reason for hiding this comment

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

rustix has one and is already in the dependencies. I've changed the code to use rustix's wrapper.

When mount.bcachefs is started with stdin set to /dev/null (such as when
it is started by systemd during boot), try to use systemd-ask-password
to ask for the passphrase.
Before asking for a passphrase, mount.bcachefs searches for an existing
key in the keyring containing a passphrase for the filesystem. This
means that the user only has to enter the passphrase once when mounting
the filesystem multiple times.

However, if the key appears in between the check and the time when
systemd-ask-password queries the user for a password, the existing key
will not be reused. Also, when multiple instances of mount.bcachefs
are started in parallel for the same filesystem (such as during boot),
each of them will see that the key is not in the keyring and start an
instance of systemd-ask-password, meaning that the user will be queried
multiple times.

Fix the race condition by passing the --keyname and --accept-cached
options to systemd-ask-password which also makes it try itself to
retreive a cached password from the keyring before querying the user
for a password.
@beviu beviu force-pushed the systemd-ask-password branch from f95c331 to 69cb642 Compare September 21, 2025 19:52
@koverstreet koverstreet merged commit 0a3e0e6 into koverstreet:master Sep 21, 2025
4 of 12 checks passed
@ElvishJerricco
Copy link

I think it'd be better to follow the convention that systemd-cryptsetup uses. They just use the constant cryptsetup for the keyname unless a different name is specified by the user in /etc/crypttab. The benefit of this is sharing the passphrase. Multiple file systems will be unlocked by a shared passphrase, and e.g. you can configure GDM autologin and auto-unlocking the GNOME keyring all based on the FDE passphrase, because the pam_gnome_keyring PAM module can use the cryptsetup key from the kernel keyring. When the shared password doesn't work, things just fallback to prompting again. If users want to use a different key name we can just add an option parsed by the mount helper.

@beviu
Copy link
Author

beviu commented Sep 23, 2025

I see bcachefs mount as the equivalent to LUKS' cryptsetup open, not systemd-cryptsetup attach. I also feel like the user might not expect bcachefs mount to interact with systemd-cryptsetup by default. On the other hand, it would be great if you could just have the bcachefs systemd mount units integrate with the LUKS systemd units automatically.

Note that the kernel looks for the bcachefs:<uuid> key so bcachefs-tools can't get rid of that (unless the kernel is modified). But it could still check the passphrases in the cryptsetup key and if the FS unlocks successfully, then add the passphrase to the cryptsetup key.

I wonder that the maintainers of bcachefs-tools think. I can try to make a PR this week.

@ElvishJerricco
Copy link

I see bcachefs mount as the equivalent to LUKS' cryptsetup open, not systemd-cryptsetup attach. I also feel like the user might not expect bcachefs mount to interact with systemd-cryptsetup by default. On the other hand, it would be great if you could just have the bcachefs systemd mount units integrate with the LUKS systemd units automatically.

Right, if we're going to be doing the systemd-ask-password in the mount helper, then that necessitates the mount helper being where we put our systemd integrations relating to keys. For instance, I've been wanting to add a --credential= flag to the ask-pass command so that systemd credentials can be used to provide the key.

Note that the kernel looks for the bcachefs:<uuid> key so bcachefs-tools can't get rid of that

Right well I'm not suggesting that. In this PR, you've added a new key to the keyring, which is just <uuid> without the leading bcachefs:. I'm just saying that could be cryptsetup instead so that it's shared among the various things that see that key as the primary bootup passphrase.

@koverstreet
Copy link
Owner

As long as we're talking about systemd integration, awhile back we were talking about abusing systemd-ask-password for prompting the user for whether to mount degraded (on workstations, not servers). Anyone have thoughts on that or want to take that on too?

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