From b2af9f487af64f04d51082361db2fa806b4242f7 Mon Sep 17 00:00:00 2001 From: andy Date: Thu, 25 Jan 2024 04:32:35 -0800 Subject: [PATCH] Implement -f option to force inherit key with zfs change-key -if This implements functionality already available in pyzfs i.e. libzfs_core.lzc_change_key(b"tank/enc/data", "force_inherit") Unlike zfs change-key -i, the key is not required to be loaded. Work-around to #15687. Signed-off-by: andy --- cmd/zfs/zfs_main.c | 17 +++- include/libzfs.h | 3 +- lib/libzfs/libzfs.abi | 1 + lib/libzfs/libzfs_crypto.c | 10 ++- man/man8/zfs-load-key.8 | 10 ++- tests/runfiles/common.run | 3 +- tests/runfiles/sanity.run | 3 +- tests/zfs-tests/tests/Makefile.am | 1 + .../zfs_change-key_inherit-force.ksh | 88 +++++++++++++++++++ 9 files changed, 124 insertions(+), 12 deletions(-) create mode 100755 tests/zfs-tests/tests/functional/cli_root/zfs_change-key/zfs_change-key_inherit-force.ksh diff --git a/cmd/zfs/zfs_main.c b/cmd/zfs/zfs_main.c index f67f6114d0..0d84b47829 100644 --- a/cmd/zfs/zfs_main.c +++ b/cmd/zfs/zfs_main.c @@ -417,7 +417,7 @@ get_usage(zfs_help_t idx) return (gettext("\tchange-key [-l] [-o keyformat=]\n" "\t [-o keylocation=] [-o pbkdf2iters=]\n" "\t \n" - "\tchange-key -i [-l] \n")); + "\tchange-key -i [-lf] \n")); case HELP_VERSION: return (gettext("\tversion\n")); case HELP_REDACT: @@ -8373,11 +8373,11 @@ zfs_do_change_key(int argc, char **argv) { int c, ret; uint64_t keystatus; - boolean_t loadkey = B_FALSE, inheritkey = B_FALSE; + boolean_t loadkey = B_FALSE, inheritkey = B_FALSE, force = B_FALSE; zfs_handle_t *zhp = NULL; nvlist_t *props = fnvlist_alloc(); - while ((c = getopt(argc, argv, "lio:")) != -1) { + while ((c = getopt(argc, argv, "lifo:")) != -1) { switch (c) { case 'l': loadkey = B_TRUE; @@ -8385,6 +8385,9 @@ zfs_do_change_key(int argc, char **argv) case 'i': inheritkey = B_TRUE; break; + case 'f': + force = B_TRUE; + break; case 'o': if (!parseprop(props, optarg)) { nvlist_free(props); @@ -8404,6 +8407,12 @@ zfs_do_change_key(int argc, char **argv) usage(B_FALSE); } + if (force && !inheritkey) { + (void) fprintf(stderr, + gettext("Force option only applies to inherit\n")); + usage(B_FALSE); + } + argc -= optind; argv += optind; @@ -8437,7 +8446,7 @@ zfs_do_change_key(int argc, char **argv) zfs_refresh_properties(zhp); } - ret = zfs_crypto_rewrap(zhp, props, inheritkey); + ret = zfs_crypto_rewrap(zhp, props, inheritkey, force); if (ret != 0) { nvlist_free(props); zfs_close(zhp); diff --git a/include/libzfs.h b/include/libzfs.h index 4f06b5d3c2..8940a1606a 100644 --- a/include/libzfs.h +++ b/include/libzfs.h @@ -578,7 +578,8 @@ _LIBZFS_H int zfs_crypto_clone_check(libzfs_handle_t *, zfs_handle_t *, char *, _LIBZFS_H int zfs_crypto_attempt_load_keys(libzfs_handle_t *, const char *); _LIBZFS_H int zfs_crypto_load_key(zfs_handle_t *, boolean_t, const char *); _LIBZFS_H int zfs_crypto_unload_key(zfs_handle_t *); -_LIBZFS_H int zfs_crypto_rewrap(zfs_handle_t *, nvlist_t *, boolean_t); +_LIBZFS_H int zfs_crypto_rewrap(zfs_handle_t *, nvlist_t *, boolean_t, + boolean_t); typedef struct zprop_list { int pl_prop; diff --git a/lib/libzfs/libzfs.abi b/lib/libzfs/libzfs.abi index 7c39b134d1..07422555de 100644 --- a/lib/libzfs/libzfs.abi +++ b/lib/libzfs/libzfs.abi @@ -3542,6 +3542,7 @@ + diff --git a/lib/libzfs/libzfs_crypto.c b/lib/libzfs/libzfs_crypto.c index 8f2a50d55e..318e883850 100644 --- a/lib/libzfs/libzfs_crypto.c +++ b/lib/libzfs/libzfs_crypto.c @@ -1583,7 +1583,8 @@ error: } int -zfs_crypto_rewrap(zfs_handle_t *zhp, nvlist_t *raw_props, boolean_t inheritkey) +zfs_crypto_rewrap(zfs_handle_t *zhp, nvlist_t *raw_props, boolean_t inheritkey, + boolean_t forceinherit) { int ret; char errbuf[ERRBUFLEN]; @@ -1600,6 +1601,9 @@ zfs_crypto_rewrap(zfs_handle_t *zhp, nvlist_t *raw_props, boolean_t inheritkey) char prop_keylocation[MAXNAMELEN]; char parent_name[ZFS_MAX_DATASET_NAME_LEN]; + if (inheritkey && forceinherit) + cmd = DCP_CMD_FORCE_INHERIT; + (void) snprintf(errbuf, sizeof (errbuf), dgettext(TEXT_DOMAIN, "Key change error")); @@ -1759,7 +1763,7 @@ zfs_crypto_rewrap(zfs_handle_t *zhp, nvlist_t *raw_props, boolean_t inheritkey) /* check that the parent's key is loaded */ pkeystatus = zfs_prop_get_int(pzhp, ZFS_PROP_KEYSTATUS); - if (pkeystatus == ZFS_KEYSTATUS_UNAVAILABLE) { + if (!forceinherit && pkeystatus == ZFS_KEYSTATUS_UNAVAILABLE) { zfs_error_aux(pzhp->zfs_hdl, dgettext(TEXT_DOMAIN, "Parent key must be loaded.")); ret = EACCES; @@ -1769,7 +1773,7 @@ zfs_crypto_rewrap(zfs_handle_t *zhp, nvlist_t *raw_props, boolean_t inheritkey) /* check that the key is loaded */ keystatus = zfs_prop_get_int(zhp, ZFS_PROP_KEYSTATUS); - if (keystatus == ZFS_KEYSTATUS_UNAVAILABLE) { + if (!forceinherit && keystatus == ZFS_KEYSTATUS_UNAVAILABLE) { zfs_error_aux(zhp->zfs_hdl, dgettext(TEXT_DOMAIN, "Key must be loaded.")); ret = EACCES; diff --git a/man/man8/zfs-load-key.8 b/man/man8/zfs-load-key.8 index f68d6e985a..4a258b5d6c 100644 --- a/man/man8/zfs-load-key.8 +++ b/man/man8/zfs-load-key.8 @@ -56,7 +56,7 @@ .Nm zfs .Cm change-key .Fl i -.Op Fl l +.Op Fl lf .Ar filesystem . .Sh DESCRIPTION @@ -159,7 +159,7 @@ Unloads the keys for all encryption roots in all imported pools. .Nm zfs .Cm change-key .Fl i -.Op Fl l +.Op Fl lf .Ar filesystem .Xc Changes the user's key (e.g. a passphrase) used to access a dataset. @@ -217,6 +217,12 @@ Indicates that zfs should make inherit the key of its parent. Note that this command can only be run on an encryption root that has an encrypted parent. +.It Fl f +When used with +.Fl i +the key will be force-inherited. +This should only be used when it is known that the parent is a replica of the +original encryptionroot and has the same key. .El .El .Ss Encryption diff --git a/tests/runfiles/common.run b/tests/runfiles/common.run index 7f0cce2523..6280cfd39a 100644 --- a/tests/runfiles/common.run +++ b/tests/runfiles/common.run @@ -175,7 +175,8 @@ tags = ['functional', 'cli_root', 'zfs_bookmark'] [tests/functional/cli_root/zfs_change-key] tests = ['zfs_change-key', 'zfs_change-key_child', 'zfs_change-key_format', 'zfs_change-key_inherit', 'zfs_change-key_load', 'zfs_change-key_location', - 'zfs_change-key_pbkdf2iters', 'zfs_change-key_clones'] + 'zfs_change-key_pbkdf2iters', 'zfs_change-key_clones', + 'zfs_change-key_inherit-force'] tags = ['functional', 'cli_root', 'zfs_change-key'] [tests/functional/cli_root/zfs_clone] diff --git a/tests/runfiles/sanity.run b/tests/runfiles/sanity.run index ab41c05b84..f2585ae134 100644 --- a/tests/runfiles/sanity.run +++ b/tests/runfiles/sanity.run @@ -104,7 +104,8 @@ tags = ['functional', 'cli_root', 'zfs_bookmark'] [tests/functional/cli_root/zfs_change-key] tests = ['zfs_change-key', 'zfs_change-key_child', 'zfs_change-key_format', 'zfs_change-key_inherit', 'zfs_change-key_load', 'zfs_change-key_location', - 'zfs_change-key_pbkdf2iters', 'zfs_change-key_clones'] + 'zfs_change-key_pbkdf2iters', 'zfs_change-key_clones', + 'zfs_change-key_inherit-force'] tags = ['functional', 'cli_root', 'zfs_change-key'] [tests/functional/cli_root/zfs_clone] diff --git a/tests/zfs-tests/tests/Makefile.am b/tests/zfs-tests/tests/Makefile.am index 4040e60434..20d07dfc8c 100644 --- a/tests/zfs-tests/tests/Makefile.am +++ b/tests/zfs-tests/tests/Makefile.am @@ -634,6 +634,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \ functional/cli_root/zfs_change-key/zfs_change-key_child.ksh \ functional/cli_root/zfs_change-key/zfs_change-key_clones.ksh \ functional/cli_root/zfs_change-key/zfs_change-key_format.ksh \ + functional/cli_root/zfs_change-key/zfs_change-key_inherit-force.ksh \ functional/cli_root/zfs_change-key/zfs_change-key_inherit.ksh \ functional/cli_root/zfs_change-key/zfs_change-key.ksh \ functional/cli_root/zfs_change-key/zfs_change-key_load.ksh \ diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_change-key/zfs_change-key_inherit-force.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_change-key/zfs_change-key_inherit-force.ksh new file mode 100755 index 0000000000..546e508207 --- /dev/null +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_change-key/zfs_change-key_inherit-force.ksh @@ -0,0 +1,88 @@ +#!/bin/ksh -p +# +# CDDL HEADER START +# +# This file and its contents are supplied under the terms of the +# Common Development and Distribution License ("CDDL"), version 1.0. +# You may only use this file in accordance with the terms of version +# 1.0 of the CDDL. +# +# A full copy of the text of the CDDL should have accompanied this +# source. A copy of the CDDL is also available via the Internet at +# http://www.illumos.org/license/CDDL. +# +# CDDL HEADER END +# + +# +# Copyright (c) 2017 Datto, Inc. All rights reserved. +# + +. $STF_SUITE/include/libtest.shlib +. $STF_SUITE/tests/functional/cli_root/zfs_load-key/zfs_load-key_common.kshlib + +# +# DESCRIPTION: +# 'zfs change-key -if' should cause a dataset to inherit its parent key +# without the key being loaded +# +# STRATEGY: +# 1. Create a parent encrypted dataset +# 2. Create a child dataset +# 3. Create a copy of the parent dataset +# 4. Send a copy of the child to the copy of the parent +# 5. Attempt to force inherit the parent key without the keys being loaded +# 6. Verify the key is inherited +# 7. Load the parent key +# 8. Verify the key is available for parent and child +# 9. Attempt to mount the datasets +# + +verify_runnable "both" + +function cleanup +{ + datasetexists $TESTPOOL/$TESTFS1 && \ + destroy_dataset $TESTPOOL/$TESTFS1 -r + + datasetexists $TESTPOOL/$TESTFS2 && \ + destroy_dataset $TESTPOOL/$TESTFS2 -r +} + +log_onexit cleanup + +log_assert "'zfs change-key -if' should cause a dataset to inherit its" \ + "parent key" + +log_must eval "echo $PASSPHRASE | zfs create -o encryption=on" \ + "-o keyformat=passphrase -o keylocation=prompt $TESTPOOL/$TESTFS1" +log_must eval "echo $PASSPHRASE1 | zfs create $TESTPOOL/$TESTFS1/child" +log_must verify_encryption_root $TESTPOOL/$TESTFS1/child "$TESTPOOL/$TESTFS1" + +log_must zfs snapshot -r $TESTPOOL/$TESTFS1@snap + +log_must eval "zfs send -w $TESTPOOL/$TESTFS1@snap | zfs receive $TESTPOOL/$TESTFS2" +log_must verify_encryption_root $TESTPOOL/$TESTFS2 $TESTPOOL/$TESTFS2 +log_must key_unavailable $TESTPOOL/$TESTFS2 + +log_must eval "zfs send -w $TESTPOOL/$TESTFS1/child@snap | zfs receive $TESTPOOL/$TESTFS2/child" +log_must verify_encryption_root $TESTPOOL/$TESTFS2/child $TESTPOOL/$TESTFS2/child +log_must key_unavailable $TESTPOOL/$TESTFS2/child + +log_must_not zfs change-key -i $TESTPOOL/$TESTFS2/child +log_must_not zfs change-key -f $TESTPOOL/$TESTFS2/child +log_must zfs change-key -if $TESTPOOL/$TESTFS2/child +log_must verify_encryption_root $TESTPOOL/$TESTFS2/child "$TESTPOOL/$TESTFS2" + +log_must key_unavailable $TESTPOOL/$TESTFS2 +log_must key_unavailable $TESTPOOL/$TESTFS2/child + +log_must eval "echo $PASSPHRASE | zfs load-key $TESTPOOL/$TESTFS2" + +log_must key_available $TESTPOOL/$TESTFS2 +log_must key_available $TESTPOOL/$TESTFS2/child + +log_must zfs mount $TESTPOOL/$TESTFS2 +log_must zfs mount $TESTPOOL/$TESTFS2/child + +log_pass "'zfs change-key -if' causes a dataset to inherit its parent key" \ No newline at end of file