PAM module for loading ZFS keys on login

[eric_metricspace.net]

Summary

This patch provides a PAM module that loads a user's authentication token as a ZFS key for a user-specific dataset on login. It also includes a patch to libzfs to allow a key to be provided directly in a key location, without the need to use a file.

This is intended to be used to allow users to have an encrypted ZFS home directory that is usually unmounted, but which has its keys loaded when the user logs in.

Test Plan

This was tested on a live system, with cases including excluded users, incorrect passwords, and correct passwords. It has also been converted to a test program and evaluated that way.

[delphij]

This proposed module have a lot of behavior that is very different from the Oracle and the OpenZFS implementation and since we are naming it the same way I don't think it's a good idea to diverge from the other implementations, especially the most commonly used parameters like homes= and the lack of mounting of the datasets.

Also note that this implementation is using libzfs instead of libzfs_core, and I think the latter is the preferred way for new code.

(The upstream OpenZFS PAM module was BSD licensed, by the way, is there any reason not to use it as a starting point, by the way?)

lib/libpam/modules/pam_zfs_key/pam_zfs_key.8

29

53

Loading ZFS key material would require either be root, or have delegated access for load-key. Is there any reason not to mount the dataset after a successful authentication now that the key is already loaded (if successful)?

95

This is a very strange choice. Normally hostname is configured as the FQDN and therefore it's unlikely to be a pool name.

Also note that this is different from both Oracle and OpenZFS's contributed version of pam_zfs_key, which is using the dataset prefix instead of the path (there isn't an obvious reason to avoid using a dataset prefix, as the dataset already knows where to mount and have all required information one would need for mounting).

102

[OPTIONAL] Can we just append username after the existing prefix instead of allowing it in arbitrary location(s)?

lib/libpam/modules/pam_zfs_key/pam_zfs_key.c

98

Please refactor the code to use openpam_get_option() instead of rolling your own.

155

Please replace this with openpam_get_option().

197

Please replace this with strdup() and check return.

244

location is not sensitive data, why is this necessary? (also note that this memset is likely to be optimized away anyways).

sys/contrib/openzfs/lib/libzfs/libzfs_crypto.c

709

Please replace this with strdup() and check return.

[eric_metricspace.net]

Abandoning, due to the existence of the upstream module in cddl

[zarychtam_plan-b.pwste.edu.pl]

In D31844#718844, @eric_metricspace.net wrote:

Abandoning, due to the existence of the upstream module in cddl

What about pam_zfs_key.8 ? Maybe you can submit it for another review ? Such a mangpage with examples could be handy.