PAM: Add fault tolerance to loading keys and mounting datasets #13050
Conversation
If an encrypted ZFS dataset is not unlocked and mounted on the first login session - as might be the case when a passphrase is not used for authentication - try again on subsequent sessions, whenever the needed tokens are provided. This change comes with performance considerations: checking whether the dataset is already mounted requires iterating through /proc/self/mounts, making the check Ο(n) for n mounts. The prior implementation checked a count value within a file, for far less runtime variability. Signed-off-by: Chris Lindee <chris.lindee+github@gmail.com>
If, for some reason, the key is already loaded for an unmounted dataset, proceed with mounting the filesystem instead of failing. Signed-off-by: Chris Lindee <chris.lindee+github@gmail.com>
|
@felixdoerre would you mind taking a look at this. |
| zfs_key_config_free(&config); | ||
| return (PAM_SUCCESS); | ||
| } | ||
| (void) zfs_key_config_modify_session_counter(pamh, &config, 1); | ||
|
|
||
| const pw_password_t *token = pw_get(pamh); |
There was a problem hiding this comment.
So do I read this correctly that we ask pam for the password even if the dataset is already mounted and even after the first session is already opened? So if e.g. root uses su to change to a user they will be prompted for a password even if that user's home is already unlocked?
There was a problem hiding this comment.
You are correct on both counts. Even before this change, people were experiencing this problem on the first session (see #11226). But, of course, once you exit all sessions the next one becomes "first" again; thus, even without this patch, we would expect many su users to run into this repetitively. Indeed, admins who only do one session for a user at a time (i.e. they su, and then exit that su before doing another su) might notice absolutely no change in behavior from this patch.
There was a problem hiding this comment.
Actually I would like to have the "is_mounted" check before we are requesting the password from pam. Because when opening up sessions from root via su for another user that's already unlocked, there is no need to ask for the password again, and asking for the password and without needing it seems wired to me.
There was a problem hiding this comment.
I think a larger discussion on the module needs to occur vis-à-vis #11222. There are multiple possible solutions: it may involve not prompting in the session interface, adding a configuration item regarding password prompts, doing as you suggest, or something else entirely.
For the best solution to #11222 (and #11226), we should look at the use cases and prior art (e.g. pam_ecryptfs, pam_mount). As it stands, I think the workaround I posted in #11222 should suffice.
There was a problem hiding this comment.
To initially create this module, I looked at pam_mount, and we even asked its creator to review pam_zfs_key, and there seems to be no completely satisfying solution to e.g. unmounting reliably inside pam alone (considering that processes might keep the mount busy when the user terminates their last session).
As for asking for a password inside su: I think generally that's good behavior, as whoever is root at that moment has the chance to unlock e.g. a home directory before switching to that user. Additionally that password prompt is optional, so it is at most an annoyance. For pam_mount that would also be default behavior (that can be disabled via pam option).
However asking for a password in a situation where the module can detect on its own that there will be no use for it seems still wrong.
Motivation and Context
Several PAM services can fail to provide the appropriate authtok to pam_zfs_key.so for unlocking and mounting a dataset, e.g.
sshdandsudo. When logging in with an authorized SSH key, the default action is to not prompt for a subsequent password, leaving pam_zfs_key.so with a failed PAM conversation. With thesudo -iu <user>command in its default configuration, the requested password is for the invoking user, not the password of the user being switched to; this means the password to unlock the dataset will not be presented (notably, however, if the invoking user's credentials are cached forsudo, thensudoactually prompts for the dataset password with a prompt of[sudo] password for %p:-- the%pis literal).Regardless of how, failing to unlock and mount the dataset on the first session means that the dataset will remain locked until every session is closed and the user starts again from a new "first" session with the appropriate token. In concert with systemd/systemd#8598 (see #13025), a user (or an admin acting as the user) can accidentally prevent a dataset from unlocking until the machine reboots (or a system administrator removes the right file).
Description
Instead of attempting to unlock and mount at the start of only the first session, check if the dataset is already unlocked or mounted on every session start and, if not, attempt the necessary actions to unlock or mount it. This provides fault-tolerance and allows SSH key users to unlock their home directory with
su -c true - $USERafter a successful key-based login.Note: This change comes with performance considerations: checking whether the dataset is already mounted requires iterating through /proc/self/mounts, making the check Ο(n) for n mounts. The prior implementation checked a count value within a file, for far less runtime variability.
Additionally, the PR addresses a related edge case where a dataset's key is already loaded, but the filesystem is not yet mounted. If pam_zfs_key.so fails to load a key because one already exists, try mounting the filesystem instead of dying with an error because a key - one that didn't need to be loaded - couldn't be loaded.
How Has This Been Tested?
Test Environment: Fedora 34 Server on VM
keystatusisunavailablesu - $USER; verifiedkeystatusisavailablekeystatusisavailablesu - $USER; verifiedkeystatusisavailableand no errors in syslog for this loginkeystatusisavailablekeystatusisavailablekeystatusisavailablekeystatusisunavailable(from a separate user)keystatusisunavailablezfs load-key; verifiedkeystatusisavailableand filesystem is not mountedsu - $USER; verified filesystem is mountedTypes of changes
Checklist:
Signed-off-by.