diff --git a/module/zfs/dsl_crypt.c b/module/zfs/dsl_crypt.c index abe724eed3..327d3ee91f 100644 --- a/module/zfs/dsl_crypt.c +++ b/module/zfs/dsl_crypt.c @@ -1418,10 +1418,17 @@ error: return (ret); } - +/* + * This function deals with the intricacies of updating wrapping + * key references and encryption roots recursively in the event + * of a call to 'zfs change-key' or 'zfs promote'. The 'skip' + * parameter should always be set to B_FALSE when called + * externally. + */ static void spa_keystore_change_key_sync_impl(uint64_t rddobj, uint64_t ddobj, - uint64_t new_rddobj, dsl_wrapping_key_t *wkey, dmu_tx_t *tx) + uint64_t new_rddobj, dsl_wrapping_key_t *wkey, boolean_t skip, + dmu_tx_t *tx) { zap_cursor_t *zc; zap_attribute_t *za; @@ -1435,7 +1442,7 @@ spa_keystore_change_key_sync_impl(uint64_t rddobj, uint64_t ddobj, /* hold the dd */ VERIFY0(dsl_dir_hold_obj(dp, ddobj, NULL, FTAG, &dd)); - /* ignore hidden dsl dirs */ + /* ignore special dsl dirs */ if (dd->dd_myname[0] == '$' || dd->dd_myname[0] == '%') { dsl_dir_rele(dd, FTAG); return; @@ -1446,7 +1453,7 @@ spa_keystore_change_key_sync_impl(uint64_t rddobj, uint64_t ddobj, * or if this dd is a clone. */ VERIFY0(dsl_dir_get_encryption_root_ddobj(dd, &curr_rddobj)); - if (curr_rddobj != rddobj || dsl_dir_is_clone(dd)) { + if (!skip && (curr_rddobj != rddobj || dsl_dir_is_clone(dd))) { dsl_dir_rele(dd, FTAG); return; } @@ -1454,19 +1461,23 @@ spa_keystore_change_key_sync_impl(uint64_t rddobj, uint64_t ddobj, /* * If we don't have a wrapping key just update the dck to reflect the * new encryption root. Otherwise rewrap the entire dck and re-sync it - * to disk. + * to disk. If skip is set, we don't do any of this work. */ - if (wkey == NULL) { - VERIFY0(zap_update(dp->dp_meta_objset, dd->dd_crypto_obj, - DSL_CRYPTO_KEY_ROOT_DDOBJ, 8, 1, &new_rddobj, tx)); - } else { - VERIFY0(spa_keystore_dsl_key_hold_dd(dp->dp_spa, dd, - FTAG, &dck)); - dsl_wrapping_key_hold(wkey, dck); - dsl_wrapping_key_rele(dck->dck_wkey, dck); - dck->dck_wkey = wkey; - dsl_crypto_key_sync(dck, tx); - spa_keystore_dsl_key_rele(dp->dp_spa, dck, FTAG); + if (!skip) { + if (wkey == NULL) { + VERIFY0(zap_update(dp->dp_meta_objset, + dd->dd_crypto_obj, + DSL_CRYPTO_KEY_ROOT_DDOBJ, 8, 1, + &new_rddobj, tx)); + } else { + VERIFY0(spa_keystore_dsl_key_hold_dd(dp->dp_spa, dd, + FTAG, &dck)); + dsl_wrapping_key_hold(wkey, dck); + dsl_wrapping_key_rele(dck->dck_wkey, dck); + dck->dck_wkey = wkey; + dsl_crypto_key_sync(dck, tx); + spa_keystore_dsl_key_rele(dp->dp_spa, dck, FTAG); + } } zc = kmem_alloc(sizeof (zap_cursor_t), KM_SLEEP); @@ -1478,7 +1489,27 @@ spa_keystore_change_key_sync_impl(uint64_t rddobj, uint64_t ddobj, zap_cursor_retrieve(zc, za) == 0; zap_cursor_advance(zc)) { spa_keystore_change_key_sync_impl(rddobj, - za->za_first_integer, new_rddobj, wkey, tx); + za->za_first_integer, new_rddobj, wkey, B_FALSE, tx); + } + zap_cursor_fini(zc); + + /* + * Recurse into all dsl dirs of clones. We utilize the skip parameter + * here so that we don't attempt to process the clones directly. This + * is because the clone and its origin share the same dck, which has + * already been updated. + */ + for (zap_cursor_init(zc, dp->dp_meta_objset, + dsl_dir_phys(dd)->dd_clones); + zap_cursor_retrieve(zc, za) == 0; + zap_cursor_advance(zc)) { + dsl_dataset_t *clone; + + VERIFY0(dsl_dataset_hold_obj(dp, za->za_first_integer, + FTAG, &clone)); + spa_keystore_change_key_sync_impl(rddobj, + clone->ds_dir->dd_object, new_rddobj, wkey, B_TRUE, tx); + dsl_dataset_rele(clone, FTAG); } zap_cursor_fini(zc); @@ -1558,7 +1589,7 @@ spa_keystore_change_key_sync(void *arg, dmu_tx_t *tx) /* recurse through all children and rewrap their keys */ spa_keystore_change_key_sync_impl(rddobj, ds->ds_dir->dd_object, - new_rddobj, wkey, tx); + new_rddobj, wkey, B_FALSE, tx); /* * All references to the old wkey should be released now (if it @@ -1736,7 +1767,7 @@ dsl_dataset_promote_crypt_sync(dsl_dir_t *target, dsl_dir_t *origin, rw_enter(&dp->dp_spa->spa_keystore.sk_wkeys_lock, RW_WRITER); spa_keystore_change_key_sync_impl(rddobj, origin->dd_object, - target->dd_object, NULL, tx); + target->dd_object, NULL, B_FALSE, tx); rw_exit(&dp->dp_spa->spa_keystore.sk_wkeys_lock); dsl_dataset_rele(targetds, FTAG); diff --git a/tests/runfiles/linux.run b/tests/runfiles/linux.run index e87e938ac6..6f015b3b31 100644 --- a/tests/runfiles/linux.run +++ b/tests/runfiles/linux.run @@ -123,7 +123,7 @@ 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_pbkdf2iters', 'zfs_change-key_clones'] tags = ['functional', 'cli_root', 'zfs_change-key'] [tests/functional/cli_root/zfs_clone] diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_change-key/Makefile.am b/tests/zfs-tests/tests/functional/cli_root/zfs_change-key/Makefile.am index 7c67e7239b..72d6e4700e 100644 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_change-key/Makefile.am +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_change-key/Makefile.am @@ -4,6 +4,7 @@ dist_pkgdata_SCRIPTS = \ cleanup.ksh \ zfs_change-key.ksh \ zfs_change-key_child.ksh \ + zfs_change-key_clones.ksh \ zfs_change-key_inherit.ksh \ zfs_change-key_format.ksh \ zfs_change-key_load.ksh \ diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_change-key/zfs_change-key_clones.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_change-key/zfs_change-key_clones.ksh new file mode 100755 index 0000000000..497fb99c81 --- /dev/null +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_change-key/zfs_change-key_clones.ksh @@ -0,0 +1,80 @@ +#!/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' should correctly update encryption roots with clones. +# +# STRATEGY: +# 1. Create an encrypted dataset +# 2. Create an encryption root child of the first dataset +# 3. Clone the child encryption root twice +# 4. Add inheriting children to the encryption root and each of the clones +# 5. Verify the encryption roots +# 6. Have the child encryption root inherit from its parent +# 7. Verify the encryption root for all datasets is now the parent dataset +# + +verify_runnable "both" + +function cleanup +{ + datasetexists $TESTPOOL/$TESTFS1 && \ + log_must zfs destroy -Rf $TESTPOOL/$TESTFS1 +} + +log_onexit cleanup + +log_assert "'zfs change-key' should correctly update encryption " \ + "roots with clones" + +log_must eval "echo $PASSPHRASE1 | zfs create -o encryption=on" \ + "-o keyformat=passphrase -o keylocation=prompt $TESTPOOL/$TESTFS1" +log_must eval "echo $PASSPHRASE2 | zfs create -o encryption=on" \ + "-o keyformat=passphrase -o keylocation=prompt $TESTPOOL/$TESTFS1/child" +log_must zfs snapshot $TESTPOOL/$TESTFS1/child@1 +log_must zfs clone $TESTPOOL/$TESTFS1/child@1 $TESTPOOL/$TESTFS1/clone1 +log_must zfs clone $TESTPOOL/$TESTFS1/child@1 $TESTPOOL/$TESTFS1/clone2 +log_must zfs create $TESTPOOL/$TESTFS1/child/A +log_must zfs create $TESTPOOL/$TESTFS1/clone1/B +log_must zfs create $TESTPOOL/$TESTFS1/clone2/C + +log_must verify_encryption_root $TESTPOOL/$TESTFS1 $TESTPOOL/$TESTFS1 +log_must verify_encryption_root $TESTPOOL/$TESTFS1/child $TESTPOOL/$TESTFS1/child +log_must verify_encryption_root $TESTPOOL/$TESTFS1/clone1 $TESTPOOL/$TESTFS1/child +log_must verify_encryption_root $TESTPOOL/$TESTFS1/clone2 $TESTPOOL/$TESTFS1/child +log_must verify_encryption_root $TESTPOOL/$TESTFS1/child/A $TESTPOOL/$TESTFS1/child +log_must verify_encryption_root $TESTPOOL/$TESTFS1/clone1/B $TESTPOOL/$TESTFS1/child +log_must verify_encryption_root $TESTPOOL/$TESTFS1/clone2/C $TESTPOOL/$TESTFS1/child + +log_must zfs change-key -i $TESTPOOL/$TESTFS1/child + +log_must verify_encryption_root $TESTPOOL/$TESTFS1 $TESTPOOL/$TESTFS1 +log_must verify_encryption_root $TESTPOOL/$TESTFS1/child $TESTPOOL/$TESTFS1 +log_must verify_encryption_root $TESTPOOL/$TESTFS1/clone1 $TESTPOOL/$TESTFS1 +log_must verify_encryption_root $TESTPOOL/$TESTFS1/clone2 $TESTPOOL/$TESTFS1 +log_must verify_encryption_root $TESTPOOL/$TESTFS1/child/A $TESTPOOL/$TESTFS1 +log_must verify_encryption_root $TESTPOOL/$TESTFS1/clone1/B $TESTPOOL/$TESTFS1 +log_must verify_encryption_root $TESTPOOL/$TESTFS1/clone2/C $TESTPOOL/$TESTFS1 + +log_pass "'zfs change-key' correctly updates encryption roots with clones" diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_promote/zfs_promote_encryptionroot.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_promote/zfs_promote_encryptionroot.ksh index 336c7b2538..2c7584d354 100755 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_promote/zfs_promote_encryptionroot.ksh +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_promote/zfs_promote_encryptionroot.ksh @@ -29,11 +29,12 @@ # 1. Create an encrypted dataset # 2. Clone the encryption root # 3. Clone the clone -# 4. Verify the encryption root of all three datasets is the origin +# 4. Add children to each of these three datasets +# 4. Verify the encryption root of all datasets is the origin # 5. Promote the clone of the clone -# 6. Verify the encryption root of all three datasets is still the origin -# 7. Promote the clone of the original encryption root -# 8. Verify the encryption root of all three datasets is the promoted dataset +# 6. Verify the encryption root of all datasets is still the origin +# 7. Promote the dataset again, so it is now the encryption root +# 8. Verify the encryption root of all datasets is the promoted dataset # verify_runnable "both" @@ -62,19 +63,31 @@ log_must zfs snap $snaproot log_must zfs clone $snaproot $TESTPOOL/clone1 log_must zfs snap $snapclone log_must zfs clone $snapclone $TESTPOOL/clone2 +log_must zfs create $TESTPOOL/$TESTFS1/child0 +log_must zfs create $TESTPOOL/clone1/child1 +log_must zfs create $TESTPOOL/clone2/child2 log_must verify_encryption_root $TESTPOOL/$TESTFS1 $TESTPOOL/$TESTFS1 log_must verify_encryption_root $TESTPOOL/clone1 $TESTPOOL/$TESTFS1 log_must verify_encryption_root $TESTPOOL/clone2 $TESTPOOL/$TESTFS1 +log_must verify_encryption_root $TESTPOOL/$TESTFS1/child0 $TESTPOOL/$TESTFS1 +log_must verify_encryption_root $TESTPOOL/clone1/child1 $TESTPOOL/$TESTFS1 +log_must verify_encryption_root $TESTPOOL/clone2/child2 $TESTPOOL/$TESTFS1 log_must zfs promote $TESTPOOL/clone2 log_must verify_encryption_root $TESTPOOL/$TESTFS1 $TESTPOOL/$TESTFS1 log_must verify_encryption_root $TESTPOOL/clone1 $TESTPOOL/$TESTFS1 log_must verify_encryption_root $TESTPOOL/clone2 $TESTPOOL/$TESTFS1 +log_must verify_encryption_root $TESTPOOL/$TESTFS1/child0 $TESTPOOL/$TESTFS1 +log_must verify_encryption_root $TESTPOOL/clone1/child1 $TESTPOOL/$TESTFS1 +log_must verify_encryption_root $TESTPOOL/clone2/child2 $TESTPOOL/$TESTFS1 log_must zfs promote $TESTPOOL/clone2 log_must verify_encryption_root $TESTPOOL/$TESTFS1 $TESTPOOL/clone2 log_must verify_encryption_root $TESTPOOL/clone1 $TESTPOOL/clone2 log_must verify_encryption_root $TESTPOOL/clone2 $TESTPOOL/clone2 +log_must verify_encryption_root $TESTPOOL/$TESTFS1/child0 $TESTPOOL/clone2 +log_must verify_encryption_root $TESTPOOL/clone1/child1 $TESTPOOL/clone2 +log_must verify_encryption_root $TESTPOOL/clone2/child2 $TESTPOOL/clone2 log_pass "ZFS promotes clones of an encryption root"