From 2637dda8f80dbd49dd0512c74108ff977dfb8c7b Mon Sep 17 00:00:00 2001 From: Tom Caputi Date: Tue, 3 Oct 2017 13:18:45 -0400 Subject: [PATCH] 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 --- module/zfs/dmu_send.c | 17 ++++++---- module/zfs/dsl_crypt.c | 73 +++++++++++++++++++++++++++++++++++------- 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; -- 2.40.0