Page MenuHomeFreeBSD

Allow autounmountd to unload ZFS keys
Needs RevisionPublic

Authored by eric_metricspace.net on Aug 30 2021, 11:31 AM.

Details

Summary

This patch adds a '-c' option to autounmountd, which will cause it to attempt to unload zfs encryption keys when unmounting a ZFS filesystem.

This is part of a set of functionality allowing users to have encrypted ZFS partitions managed by autofs, whose keys will only be loaded while in use (this is a requirement on many high-security systems).

Test Plan

Tested on real hardware with an automounted home directory, and shown to be working.

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

delphij requested changes to this revision.Sep 6 2021, 12:43 AM
delphij added a subscriber: delphij.

In general I don't think it's a good idea to add an option to opt-in unload of crypto keys: it should always be done unless explicitly opted out (or can't be opted out), so I'd recommend either removing the option and default to be safe, or make it an opt-out option (when specified, do not unload crypto key).

This revision now requires changes to proceed.Sep 6 2021, 12:43 AM

In general I don't think it's a good idea to add an option to opt-in unload of crypto keys: it should always be done unless explicitly opted out (or can't be opted out), so I'd recommend either removing the option and default to be safe, or make it an opt-out option (when specified, do not unload crypto key).

I'm not sure it's a good idea to abruptly change behavior in this way. I think it would be a better idea to introduce the ability as opt-in, and then change it to opt-out after announcing in advance.

In general I don't think it's a good idea to add an option to opt-in unload of crypto keys: it should always be done unless explicitly opted out (or can't be opted out), so I'd recommend either removing the option and default to be safe, or make it an opt-out option (when specified, do not unload crypto key).

I'm not sure it's a good idea to abruptly change behavior in this way. I think it would be a better idea to introduce the ability as opt-in, and then change it to opt-out after announcing in advance.

I'm not very convinced that this is a behavior change (please do correct me if I was wrong). Here is my thought: the change seems to affect ZFS case only, and for ZFS the regular usage is that they are mounted by zfs mount -a, and my understanding is that this change is intended to be used with the PAM module to provide more automation to the load/unload key process.

If the assumption above was right, since there isn't current working way of loading ZFS encrypted dataset with this new workflow, we are not changing any existing behavior.

In general I don't think it's a good idea to add an option to opt-in unload of crypto keys: it should always be done unless explicitly opted out (or can't be opted out), so I'd recommend either removing the option and default to be safe, or make it an opt-out option (when specified, do not unload crypto key).

I'm not sure it's a good idea to abruptly change behavior in this way. I think it would be a better idea to introduce the ability as opt-in, and then change it to opt-out after announcing in advance.

I'm not very convinced that this is a behavior change (please do correct me if I was wrong). Here is my thought: the change seems to affect ZFS case only, and for ZFS the regular usage is that they are mounted by zfs mount -a, and my understanding is that this change is intended to be used with the PAM module to provide more automation to the load/unload key process.

If the assumption above was right, since there isn't current working way of loading ZFS encrypted dataset with this new workflow, we are not changing any existing behavior.

But what about other encrypted ZFS volumes that are managed with autofs? Admittedly I don't know how common this is, but introducing an opt-out flag abruptly would break those cases. Perhaps we ought to take the question to the mailing lists.

rew added a subscriber: rew.

Does it make sense to have autofs unload zfs keys if it doesn't even know how to load them?

Also, autounmountd(8) isn't the only place an automounted filesystem can be unmounted. For example, see unmount_by_statfs() in usr.sbin/autofs/automount.c. I have a patch that centralizes unmount_by_statfs() and unmount_by_fsid().

Without digging deeper into this, I think there's some other details that still need to be hashed out:

  1. do we really need to drag in libzfs or can we accomplish what we need to by using auto_popen().
  2. autofs uses mount(8) and unmount(2) to mount/unmount filesystem - which works fine when the mountpoint is set to legacy.
  3. if autofs is going to unload zfs keys, it should know how to load them.

As a side note, it's been discussed in the past that using auto_popen() for unmount'ing is a non-starter - but possibly it could be leveraged to unload the keys after a successful unmount.

As far as the extra option to unload zfs keys - I don't think that's necessary. If autofs is managing the filesystem and the zfs keys are loaded, autofs should automatically unload the zfs keys after the filesystem is unmounted.

From my perspective, there's some more work to do here...

I've added trasz as a reviewer as he is the author/maintainer of autofs.

In D31725#719350, @rew wrote:

Does it make sense to have autofs unload zfs keys if it doesn't even know how to load them?

Yes. They are to be loaded by a PAM module. See answer to (2).

Also, autounmountd(8) isn't the only place an automounted filesystem can be unmounted. For example, see unmount_by_statfs() in usr.sbin/autofs/automount.c. I have a patch that centralizes unmount_by_statfs() and unmount_by_fsid().

It's not clear what the ask is here. Also, you do not necessarily want to unload *all* zfs keys when the partition is unloaded, as this may break existing systems. autofs, on the other hand, has a good reason to do this, as it's often used as the primary mechanism for dynamically mounting and unmounting filesystems (particularly home directories, which is the primary use case here)

Without digging deeper into this, I think there's some other details that still need to be hashed out:

  1. do we really need to drag in libzfs or can we accomplish what we need to by using auto_popen().

No. The MS_CRYPT flag is a libzfs-specific thing, and is only accessible through libzfs.

  1. autofs uses mount(8) and unmount(2) to mount/unmount filesystem - which works fine when the mountpoint is set to legacy.

That doesn't unload ZFS keys. The only way to do that is through libzfs.

  1. if autofs is going to unload zfs keys, it should know how to load them.

This doesn't make any sense. There is no principled, secure way for autofs to obtain the keys in order to load them.

As I explained in the description, this is intended to be used in conjunction with PAM to load a user's auth token as a key, allowing their encrypted home directory to be loaded when they log in, and unloaded once they've fully logged out. This in turn is a means to implement a common requirement on high-security systems.

As far as the extra option to unload zfs keys - I don't think that's necessary. If autofs is managing the filesystem and the zfs keys are loaded, autofs should automatically unload the zfs keys after the filesystem is unmounted.

That does not happen. This has been experimentally verified.

From my perspective, there's some more work to do here...

The suggested work seems to be based on false assumptions. If there is anything else , please let me know.

As I explained in the description, this is intended to be used in conjunction with PAM to load a user's auth token as a key, allowing their encrypted home directory to be loaded when they log in, and unloaded once they've fully logged out. This in turn is a means to implement a common requirement on high-security systems.

According to the OpenZFS commit, the zfs_key pam module already unmounts the dataset and unloads the key when a session is closed.

For reference: https://github.com/openzfs/zfs/commit/221e67040fc47c15b3da2afb09bb48f1e9700fb9

In D31725#719491, @rew wrote:

As I explained in the description, this is intended to be used in conjunction with PAM to load a user's auth token as a key, allowing their encrypted home directory to be loaded when they log in, and unloaded once they've fully logged out. This in turn is a means to implement a common requirement on high-security systems.

According to the OpenZFS commit, the zfs_key pam module already unmounts the dataset and unloads the key when a session is closed.

For reference: https://github.com/openzfs/zfs/commit/221e67040fc47c15b3da2afb09bb48f1e9700fb9

There is a known issue with this failing with EBUSY, due to lingering processes that still hold open files on the partition, preventing it from being unmounted and therefore also preventing the keys from being unloaded.

This is the problem with trying to do that in PAM: you get one shot, and if it fails, then there's no way to circle back and try again. The better way is to let autounmountd clean up the mount, and unload the keys when it does.

Sounds like a bug in the upstream module. I’d prefer to see that fixed rather than using autofs to work around the zfs_key pam module bug. Or see if it’s feasible to implement a pure autofs solution (i.e., autofs loads and unloads the key for a given dataset). If neither of the above options are possible, I’m more inclined to oppose this change until a more streamlined solution can be found.

With that said, my original comments are not based on false assumptions:

  • You’ve not accounted for all cases where an automounted filesystem can be unmounted. autounmountd(8) is not the only program that can unmount an automounted filesystem; automount(8) can also unmount them.
  • You’ve also not taken into consideration whether the mountpoint for a ZFS dataset is set to legacy or not. When set to legacy, mount/unmount should be used as opposed to ZFS’s mount/unmount.
  • As delphij said, the command line option to opt-in of unloading the ZFS key is not correct. The default should be to unload the key.
  • It's not clear where the "automounted" mount option gets set when using the zfs_key pam module.

I think first step might be to get autofs to properly mount/unmount ZFS datasets that have non-legacy mountpoints. Then try to add the key support after that. I'd recommend getting trasz's opinion on whether those changes would be welcome/worth it before working on it though.

In D31725#720207, @rew wrote:

Sounds like a bug in the upstream module. I’d prefer to see that fixed rather than using autofs to work around the zfs_key pam module bug. Or see if it’s feasible to implement a pure autofs solution (i.e., autofs loads and unloads the key for a given dataset). If neither of the above options are possible, I’m more inclined to oppose this change until a more streamlined solution can be found.

That isn't how these systems work...

First, autofs can't load keys, because it has no way to obtain them. It would need some means of interacting with users or else receiving an auth token.

Second, PAM cannot wait around to unmount a filesystem, because there is no principled way to guarantee that no programs survived the session and are still holding open file descriptors on the encrypted filesystem. It's not a "bug", per se, but a fundamental limitation. If anything, the bug was trying to have PAM unload the ZFS keys at all. The only way to do that properly is with something like autounmountd.

Admittedly this is all a not-great result POSIX interfaces failing to foresee such a use case, but it's what we have to work with.

With that said, my original comments are not based on false assumptions:

Many of these points still are. See below.

  • You’ve not accounted for all cases where an automounted filesystem can be unmounted. autounmountd(8) is not the only program that can unmount an automounted filesystem; automount(8) can also unmount them.
  • You’ve also not taken into consideration whether the mountpoint for a ZFS dataset is set to legacy or not. When set to legacy, mount/unmount should be used as opposed to ZFS’s mount/unmount.

Ok, I can deal with these issues.

  • As delphij said, the command line option to opt-in of unloading the ZFS key is not correct. The default should be to unload the key.

Doing that will change behavior. I really think that ought to be asked about in a more public forum first.

  • It's not clear where the "automounted" mount option gets set when using the zfs_key pam module.

The automounted option gets set by autofs when it mounts the filesystem. PAM doesn't mount the filesystems. The ZFS keys are loaded by PAM, which enables the filesystems to be mounted.

I think first step might be to get autofs to properly mount/unmount ZFS datasets that have non-legacy mountpoints.

What's the point of doing that? Unless you're managing crypto keys, the standard mount/unmount calls work fine with ZFS filesystems.