Fix for #6706

This patch resolves an issue where raw sends would fail to send
encryption parameters if the wrapping key was unloaded and reloaded
before the data was sent and the dataset wass not an encryption root.
The code attempted to lookup the values from the wrapping key which
was not being initialized upon reload. This change forces the code to
lookup the correct value from the encryption root's DSL Crypto Key.
Unfortunately, this issue led to the on-disk DSL Crypto Key for some
non-encryption root datasets being left with zeroed out encryption
parameters. However, this should not present a problem since these
values are never looked at and are overrwritten upon changing keys.

This patch also fixes an issue where raw, resumable sends were not
being cleaned up appropriately if an invalid DSL Crypto Key was
received.

Signed-off-by: Tom Caputi <[email protected]>

2 files changed, 72 insertions, 18 deletions

diff --git a/module/zfs/dmu_send.c b/module/zfs/dmu_send.c
index 4318a7815..cc6b97d53 100644
--- a/module/zfs/dmu_send.c
+++ b/module/zfs/dmu_send.c
@@ -2064,7 +2064,8 @@ dmu_recv_resume_begin_sync(void *arg, dmu_tx_t *tx)
 	dsl_dataset_phys(ds)->ds_flags |= DS_FLAG_INCONSISTENT;
 
 	rrw_enter(&ds->ds_bp_rwlock, RW_READER, FTAG);
-	ASSERT(!BP_IS_HOLE(dsl_dataset_get_blkptr(ds)));
+	ASSERT(!BP_IS_HOLE(dsl_dataset_get_blkptr(ds)) ||
+	    drba->drba_cookie->drc_raw);
 	rrw_exit(&ds->ds_bp_rwlock, FTAG);
 
 	drba->drba_cookie->drc_ds = ds;
@@ -2958,6 +2959,7 @@ receive_object_range(struct receive_writer_arg *rwa,
 static void
 dmu_recv_cleanup_ds(dmu_recv_cookie_t *drc)
 {
+	dsl_dataset_t *ds = drc->drc_ds;
 	ds_hold_flags_t dsflags = (drc->drc_raw) ? 0 : DS_HOLD_FLAG_DECRYPT;
 
 	/*
@@ -2967,14 +2969,17 @@ dmu_recv_cleanup_ds(dmu_recv_cookie_t *drc)
 	 * that the user accounting code will not attempt to do anything
 	 * after we stopped receiving the dataset.
 	 */
-	txg_wait_synced(drc->drc_ds->ds_dir->dd_pool, 0);
+	txg_wait_synced(ds->ds_dir->dd_pool, 0);
 
-	if (drc->drc_resumable) {
-		dsl_dataset_disown(drc->drc_ds, dsflags, dmu_recv_tag);
+	rrw_enter(&ds->ds_bp_rwlock, RW_READER, FTAG);
+	if (drc->drc_resumable && !BP_IS_HOLE(dsl_dataset_get_blkptr(ds))) {
+		rrw_exit(&ds->ds_bp_rwlock, FTAG);
+		dsl_dataset_disown(ds, dsflags, dmu_recv_tag);
 	} else {
 		char name[ZFS_MAX_DATASET_NAME_LEN];
-		dsl_dataset_name(drc->drc_ds, name);
-		dsl_dataset_disown(drc->drc_ds, dsflags, dmu_recv_tag);
+		rrw_exit(&ds->ds_bp_rwlock, FTAG);
+		dsl_dataset_name(ds, name);
+		dsl_dataset_disown(ds, dsflags, dmu_recv_tag);
 		(void) dsl_destroy_head(name);
 	}
 }
diff --git a/module/zfs/dsl_crypt.c b/module/zfs/dsl_crypt.c
index 3dcdde4d0..a71142682 100644
--- a/module/zfs/dsl_crypt.c
+++ b/module/zfs/dsl_crypt.c
@@ -723,6 +723,7 @@ spa_keystore_load_wkey(const char *dsname, dsl_crypto_params_t *dcp,
 	dsl_crypto_key_t *dck = NULL;
 	dsl_wrapping_key_t *wkey = dcp->cp_wkey;
 	dsl_pool_t *dp = NULL;
+	uint64_t keyformat, salt, iters;
 
 	/*
 	 * We don't validate the wrapping key's keyformat, salt, or iters
@@ -757,8 +758,36 @@ spa_keystore_load_wkey(const char *dsname, dsl_crypto_params_t *dcp,
 	if (ret != 0)
 		goto error;
 
+	/* initialize the wkey encryption parameters from the DSL Crypto Key */
+	ret = zap_lookup(dp->dp_meta_objset, dd->dd_crypto_obj,
+	    zfs_prop_to_name(ZFS_PROP_KEYFORMAT), 8, 1, &keyformat);
+	if (ret != 0)
+		goto error;
+
+	ret = zap_lookup(dp->dp_meta_objset, dd->dd_crypto_obj,
+	    zfs_prop_to_name(ZFS_PROP_PBKDF2_SALT), 8, 1, &salt);
+	if (ret != 0)
+		goto error;
+
+	ret = zap_lookup(dp->dp_meta_objset, dd->dd_crypto_obj,
+	    zfs_prop_to_name(ZFS_PROP_PBKDF2_ITERS), 8, 1, &iters);
+	if (ret != 0)
+		goto error;
+
+	ASSERT3U(keyformat, <, ZFS_KEYFORMAT_FORMATS);
+	ASSERT3U(keyformat, !=, ZFS_KEYFORMAT_NONE);
+	IMPLY(keyformat == ZFS_KEYFORMAT_PASSPHRASE, iters != 0);
+	IMPLY(keyformat == ZFS_KEYFORMAT_PASSPHRASE, salt != 0);
+	IMPLY(keyformat != ZFS_KEYFORMAT_PASSPHRASE, iters == 0);
+	IMPLY(keyformat != ZFS_KEYFORMAT_PASSPHRASE, salt == 0);
+
+	wkey->wk_keyformat = keyformat;
+	wkey->wk_salt = salt;
+	wkey->wk_iters = iters;
+
 	/*
-	 * At this point we have verified the key. We can simply cleanup and
+	 * At this point we have verified the wkey and confirmed that it can
+	 * be used to decrypt a DSL Crypto Key. We can simply cleanup and
 	 * return if this is all the user wanted to do.
 	 */
 	if (noop)
@@ -2176,6 +2205,7 @@ dsl_crypto_populate_key_nvlist(dsl_dataset_t *ds, nvlist_t **nvl_out)
 	uint64_t rddobj;
 	nvlist_t *nvl = NULL;
 	uint64_t dckobj = ds->ds_dir->dd_crypto_obj;
+	dsl_dir_t *rdd = NULL;
 	dsl_pool_t *dp = ds->ds_dir->dd_pool;
 	objset_t *mos = dp->dp_meta_objset;
 	uint64_t crypt = 0, guid = 0, format = 0, iters = 0, salt = 0;
@@ -2194,10 +2224,6 @@ dsl_crypto_populate_key_nvlist(dsl_dataset_t *ds, nvlist_t **nvl_out)
 		goto error;
 
 	/* lookup values from the DSL Crypto Key */
-	ret = dsl_dir_get_encryption_root_ddobj(ds->ds_dir, &rddobj);
-	if (ret != 0)
-		goto error;
-
 	ret = zap_lookup(mos, dckobj, DSL_CRYPTO_KEY_CRYPTO_SUITE, 8, 1,
 	    &crypt);
 	if (ret != 0)
@@ -2227,24 +2253,43 @@ dsl_crypto_populate_key_nvlist(dsl_dataset_t *ds, nvlist_t **nvl_out)
 	if (ret != 0)
 		goto error;
 
-	/* lookup wrapping key properties */
-	ret = zap_lookup(dp->dp_meta_objset, dckobj,
-	    zfs_prop_to_name(ZFS_PROP_KEYFORMAT), 8, 1, &format);
+	/*
+	 * Lookup wrapping key properties. An early version of the code did
+	 * not correctly add these values to the wrapping key or the DSL
+	 * Crypto Key on disk for non encryption roots, so to be safe we
+	 * always take the slightly circuitous route of looking it up from
+	 * the encryption root's key.
+	 */
+	ret = dsl_dir_get_encryption_root_ddobj(ds->ds_dir, &rddobj);
 	if (ret != 0)
 		goto error;
 
+	dsl_pool_config_enter(dp, FTAG);
+
+	ret = dsl_dir_hold_obj(dp, rddobj, NULL, FTAG, &rdd);
+	if (ret != 0)
+		goto error_unlock;
+
+	ret = zap_lookup(dp->dp_meta_objset, rdd->dd_crypto_obj,
+	    zfs_prop_to_name(ZFS_PROP_KEYFORMAT), 8, 1, &format);
+	if (ret != 0)
+		goto error_unlock;
+
 	if (format == ZFS_KEYFORMAT_PASSPHRASE) {
-		ret = zap_lookup(dp->dp_meta_objset, dckobj,
+		ret = zap_lookup(dp->dp_meta_objset, rdd->dd_crypto_obj,
 		    zfs_prop_to_name(ZFS_PROP_PBKDF2_ITERS), 8, 1, &iters);
 		if (ret != 0)
-			goto error;
+			goto error_unlock;
 
-		ret = zap_lookup(dp->dp_meta_objset, dckobj,
+		ret = zap_lookup(dp->dp_meta_objset, rdd->dd_crypto_obj,
 		    zfs_prop_to_name(ZFS_PROP_PBKDF2_SALT), 8, 1, &salt);
 		if (ret != 0)
-			goto error;
+			goto error_unlock;
 	}
 
+	dsl_dir_rele(rdd, FTAG);
+	dsl_pool_config_exit(dp, FTAG);
+
 	fnvlist_add_uint64(nvl, DSL_CRYPTO_KEY_CRYPTO_SUITE, crypt);
 	fnvlist_add_uint64(nvl, DSL_CRYPTO_KEY_GUID, guid);
 	VERIFY0(nvlist_add_uint8_array(nvl, DSL_CRYPTO_KEY_MASTER_KEY,
@@ -2270,7 +2315,11 @@ dsl_crypto_populate_key_nvlist(dsl_dataset_t *ds, nvlist_t **nvl_out)
 	*nvl_out = nvl;
 	return (0);
 
+error_unlock:
+	dsl_pool_config_exit(dp, FTAG);
 error:
+	if (rdd != NULL)
+		dsl_dir_rele(rdd, FTAG);
 	nvlist_free(nvl);
 
 	*nvl_out = NULL;