zfs load-key: don't enforce minimum passphrase length #12765

AttilaFueloep · 2021-11-15T09:23:26Z

Motivation and Context

The pam_zfs_key pam(8) module does not enforce a minimum length while changing the passphrase of the users encrypted home dataset. This can lead to a situation where the key for that dataset can't be loaded with zfs-load-key(8) since it enforces a minimum length. Please see #12651 and #12656 for further details.

Please note that zfs-load-key(8) still enforces the maximum passphrase length. I think this is reasonable since it avoids a DOS condition and more recent linux pam(8) enforces a maximum password length of 512 as well [1].

Closes #12651
Closes #12656

Description

Since, as opposed to zfs-change-key(8), there is no compelling reason for zfs-load-key(8) to enforce a minimum passphrase length, remove that check. A test to verify that loading short keys succeeds was added to the pam_zfs_key tests. While there, fix a small cleanup bug.

How Has This Been Tested?

Ran the added test to verify that loading short keys fails on master and succeeds with 97ec67e applied.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Performance enhancement (non-breaking change which improves efficiency)
Code cleanup (non-breaking change which makes code smaller or more readable)
Breaking change (fix or feature that would cause existing functionality to change)
Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
Documentation (a change to man pages or other documentation)

Checklist:

My code follows the OpenZFS code style requirements.
I have updated the documentation accordingly.
I have read the contributing document.
I have added tests to cover my changes.
I have run the ZFS Test Suite with this change applied.
All commit messages are properly formatted and contain Signed-off-by.

Remove the generated pam service config file `/etc/pam.d/pam_zfs_key_test` on test cleanup, since the tests shouldn't alter system state. While here, move the pam service config file name into a variable. Signed-off-by: Attila Fülöp <[email protected]>

The pam_zfs_key pam module does not enforce a minimum password length while changing the user password and thus the users home dataset passphrase. To not end up with a dateset `zfs load-key` can't load the key for, `zfs load-key` should not enforce a minimum passphrase length. This adds a test for that. Signed-off-by: Attila Fülöp <[email protected]>

The restriction that an encryption key must be at least MIN_PASSPHRASE_LEN characters long make sense when changing the encryption key, but not when loading: as this restriction is not enforced in the libraries, it is possible to bypass zfs change-key's restrictions and end up with a key that becomes impossible to load with zfs load-key, for example through pam_zfs_key. Signed-off-by: Harald van Dijk <[email protected]>

Signed-off-by: Attila Fülöp <[email protected]>

AttilaFueloep · 2021-11-15T09:24:25Z

Picked up from #12656.

AttilaFueloep · 2021-11-15T10:01:05Z

@felixdoerre Would you mind having a look?

AttilaFueloep · 2021-11-15T16:04:09Z

The kernel.org build failure is a known issue and the CentOS 8 test failure seems unrelated.

The pam_zfs_key pam module does not enforce a minimum password length while changing the user password and thus the users home dataset passphrase. To not end up with a dateset `zfs load-key` can't load the key for, `zfs load-key` should not enforce a minimum passphrase length. This adds a test for that. Reviewed-by: Felix Dörre <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Attila Fülöp <[email protected]> Closes #12765 Closes #12651 Closes #12656

The restriction that an encryption key must be at least MIN_PASSPHRASE_LEN characters long make sense when changing the encryption key, but not when loading: as this restriction is not enforced in the libraries, it is possible to bypass zfs change-key's restrictions and end up with a key that becomes impossible to load with zfs load-key, for example through pam_zfs_key. Reviewed-by: Felix Dörre <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Harald van Dijk <[email protected]> Closes #12765

Reviewed-by: Felix Dörre <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Attila Fülöp <[email protected]> Closes #12765

Remove the generated pam service config file `/etc/pam.d/pam_zfs_key_test` on test cleanup, since the tests shouldn't alter system state. While here, move the pam service config file name into a variable. Reviewed-by: Felix Dörre <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Attila Fülöp <[email protected]> Closes openzfs#12765

The pam_zfs_key pam module does not enforce a minimum password length while changing the user password and thus the users home dataset passphrase. To not end up with a dateset `zfs load-key` can't load the key for, `zfs load-key` should not enforce a minimum passphrase length. This adds a test for that. Reviewed-by: Felix Dörre <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Attila Fülöp <[email protected]> Closes openzfs#12765 Closes openzfs#12651 Closes openzfs#12656

The restriction that an encryption key must be at least MIN_PASSPHRASE_LEN characters long make sense when changing the encryption key, but not when loading: as this restriction is not enforced in the libraries, it is possible to bypass zfs change-key's restrictions and end up with a key that becomes impossible to load with zfs load-key, for example through pam_zfs_key. Reviewed-by: Felix Dörre <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Harald van Dijk <[email protected]> Closes openzfs#12765

Reviewed-by: Felix Dörre <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Attila Fülöp <[email protected]> Closes openzfs#12765

Remove the generated pam service config file `/etc/pam.d/pam_zfs_key_test` on test cleanup, since the tests shouldn't alter system state. While here, move the pam service config file name into a variable. Reviewed-by: Felix Dörre <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Attila Fülöp <[email protected]> Closes openzfs#12765

The pam_zfs_key pam module does not enforce a minimum password length while changing the user password and thus the users home dataset passphrase. To not end up with a dateset `zfs load-key` can't load the key for, `zfs load-key` should not enforce a minimum passphrase length. This adds a test for that. Reviewed-by: Felix Dörre <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Attila Fülöp <[email protected]> Closes openzfs#12765 Closes openzfs#12651 Closes openzfs#12656

The restriction that an encryption key must be at least MIN_PASSPHRASE_LEN characters long make sense when changing the encryption key, but not when loading: as this restriction is not enforced in the libraries, it is possible to bypass zfs change-key's restrictions and end up with a key that becomes impossible to load with zfs load-key, for example through pam_zfs_key. Reviewed-by: Felix Dörre <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Harald van Dijk <[email protected]> Closes openzfs#12765

Reviewed-by: Felix Dörre <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Attila Fülöp <[email protected]> Closes openzfs#12765

ggzengel · 2022-08-30T08:41:39Z

I have an old pool with short keys and can't load them.
When will this patch be merged?
What can I do to enter a valid passphrase?

# zfs version
zfs-2.1.5-1~bpo11+1
zfs-kmod-2.1.5-1~bpo11+1
# lsb_release -a
No LSB modules are available.
Distributor ID: Debian
Description:    Debian GNU/Linux 11 (bullseye)
Release:        11
Codename:       bullseye

felixdoerre · 2022-08-30T11:43:53Z

This patch was already merged, and specifically the changes you seek are contained in master in commit 85638aa. However this change was not backported to zfs-2.1 and therefore isn't contained in any zfs release yet. Interestingly the problem of having an old pool with short key didn't come up before (this pr is dedicated to short keys being set/entered with the pam module). You could use the pam module to work around that:
Create a user with the corresponding home and short key as password, install the pam_zfs_key module, login to that user, and the dataset should be unlocked. Sadly I am not aware of any other workarounds.

AttilaFueloep and others added 4 commits November 14, 2021 17:36

get_key_material: fix style

1aa10d6

Signed-off-by: Attila Fülöp <[email protected]>

AttilaFueloep mentioned this pull request Nov 15, 2021

get_key_material: skip validation when loading keys #12656

Closed

13 tasks

felixdoerre approved these changes Nov 15, 2021

View reviewed changes

behlendorf approved these changes Nov 30, 2021

View reviewed changes

behlendorf added the Status: Accepted Ready to integrate (reviewed, tested) label Nov 30, 2021

behlendorf closed this in 307db92 Nov 30, 2021

behlendorf pushed a commit that referenced this pull request Nov 30, 2021

get_key_material: fix style

861dca0

Reviewed-by: Felix Dörre <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Attila Fülöp <[email protected]> Closes #12765

nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022

get_key_material: fix style

f8e54bd

Reviewed-by: Felix Dörre <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Attila Fülöp <[email protected]> Closes openzfs#12765

nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022

get_key_material: fix style

df77cc8

Reviewed-by: Felix Dörre <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Attila Fülöp <[email protected]> Closes openzfs#12765

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

zfs load-key: don't enforce minimum passphrase length #12765

zfs load-key: don't enforce minimum passphrase length #12765

AttilaFueloep commented Nov 15, 2021

AttilaFueloep commented Nov 15, 2021

AttilaFueloep commented Nov 15, 2021

AttilaFueloep commented Nov 15, 2021

ggzengel commented Aug 30, 2022

felixdoerre commented Aug 30, 2022

zfs load-key: don't enforce minimum passphrase length #12765

zfs load-key: don't enforce minimum passphrase length #12765

Conversation

AttilaFueloep commented Nov 15, 2021

Motivation and Context

Description

How Has This Been Tested?

Types of changes

Checklist:

AttilaFueloep commented Nov 15, 2021

AttilaFueloep commented Nov 15, 2021

AttilaFueloep commented Nov 15, 2021

ggzengel commented Aug 30, 2022

felixdoerre commented Aug 30, 2022