Check encrypted dataset + embedded recv earlier

This patch fixes a bug where attempting to receive a send stream
with embedded data into an encrypted dataset would not cleanup
that dataset when the error was reached. The check was moved into
dmu_recv_begin_check(), preventing this issue.

Reviewed-by: Jorgen Lundman <lundman@lundman.net>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Richard Elling <Richard.Elling@RichardElling.com>
Signed-off-by: Tom Caputi <tcaputi@datto.com>
Closes #7650

diff --git a/include/sys/dsl_crypt.h b/include/sys/dsl_crypt.h
index e92ae364c8b51c27ff2b93f37d8673d41f1698d9..8766ce51ea9af655d30eeea43967bd08c3888b5b 100644 (file)
--- a/include/sys/dsl_crypt.h
+++ b/include/sys/dsl_crypt.h
@@ -202,7 +202,7 @@ int dsl_dataset_promote_crypt_check(dsl_dir_t *target, dsl_dir_t *origin);
 void dsl_dataset_promote_crypt_sync(dsl_dir_t *target, dsl_dir_t *origin,
     dmu_tx_t *tx);
 int dmu_objset_create_crypt_check(dsl_dir_t *parentdd,
-    dsl_crypto_params_t *dcp);
+    dsl_crypto_params_t *dcp, boolean_t *will_encrypt);
 void dsl_dataset_create_crypt_sync(uint64_t dsobj, dsl_dir_t *dd,
     struct dsl_dataset *origin, dsl_crypto_params_t *dcp, dmu_tx_t *tx);
 uint64_t dsl_crypto_key_create_sync(uint64_t crypt, dsl_wrapping_key_t *wkey,

diff --git a/module/zfs/dmu_objset.c b/module/zfs/dmu_objset.c
index 5b18ed5cc6b21ea013955cd884fe133ea81703ba..9adda320fff977f7dd083392c319843af87a709f 100644 (file)
--- a/module/zfs/dmu_objset.c
+++ b/module/zfs/dmu_objset.c
@@ -1118,7 +1118,7 @@ dmu_objset_create_check(void *arg, dmu_tx_t *tx)
                return (SET_ERROR(EEXIST));
        }
 
-       error = dmu_objset_create_crypt_check(pdd, doca->doca_dcp);
+       error = dmu_objset_create_crypt_check(pdd, doca->doca_dcp, NULL);
        if (error != 0) {
                dsl_dir_rele(pdd, FTAG);
                return (error);

diff --git a/module/zfs/dmu_send.c b/module/zfs/dmu_send.c
index 80a4843b3ed4f486ca8746f05c0cf2486d8e1b74..61d4ebd4ff7b1c26d36bc53eab23638286a786a0 100644 (file)
--- a/module/zfs/dmu_send.c
+++ b/module/zfs/dmu_send.c
@@ -1539,6 +1539,7 @@ recv_begin_check_existing_impl(dmu_recv_begin_arg_t *drba, dsl_dataset_t *ds,
        dsl_pool_t *dp = ds->ds_dir->dd_pool;
        boolean_t encrypted = ds->ds_dir->dd_crypto_obj != 0;
        boolean_t raw = (featureflags & DMU_BACKUP_FEATURE_RAW) != 0;
+       boolean_t embed = (featureflags & DMU_BACKUP_FEATURE_EMBED_DATA) != 0;
 
        /* temporary clone name must not exist */
        error = zap_lookup(dp->dp_meta_objset,
@@ -1576,6 +1577,10 @@ recv_begin_check_existing_impl(dmu_recv_begin_arg_t *drba, dsl_dataset_t *ds,
                if (!encrypted && raw)
                        return (SET_ERROR(EINVAL));
 
+               /* Encryption is incompatible with embedded data */
+               if (encrypted && embed)
+                       return (SET_ERROR(EINVAL));
+
                /* Find snapshot in this dir that matches fromguid. */
                while (obj != 0) {
                        error = dsl_dataset_hold_obj(dp, obj, FTAG,
@@ -1623,11 +1628,21 @@ recv_begin_check_existing_impl(dmu_recv_begin_arg_t *drba, dsl_dataset_t *ds,
                if ((!encrypted && raw) || encrypted)
                        return (SET_ERROR(EINVAL));
 
-               if ((featureflags & DMU_BACKUP_FEATURE_RAW) == 0) {
+               /*
+                * Perform the same encryption checks we would if
+                * we were creating a new dataset from scratch.
+                */
+               if (!raw) {
+                       boolean_t will_encrypt;
+
                        error = dmu_objset_create_crypt_check(
-                           ds->ds_dir->dd_parent, drba->drba_dcp);
+                           ds->ds_dir->dd_parent, drba->drba_dcp,
+                           &will_encrypt);
                        if (error != 0)
                                return (error);
+
+                       if (will_encrypt && embed)
+                               return (SET_ERROR(EINVAL));
                }
 
                drba->drba_snapobj = 0;
@@ -1700,6 +1715,10 @@ dmu_recv_begin_check(void *arg, dmu_tx_t *tx)
                /* raw receives require the encryption feature */
                if (!spa_feature_is_enabled(dp->dp_spa, SPA_FEATURE_ENCRYPTION))
                        return (SET_ERROR(ENOTSUP));
+
+               /* embedded data is incompatible with encryption and raw recv */
+               if (featureflags & DMU_BACKUP_FEATURE_EMBED_DATA)
+                       return (SET_ERROR(EINVAL));
        } else {
                dsflags |= DS_HOLD_FLAG_DECRYPT;
        }
@@ -1747,12 +1766,27 @@ dmu_recv_begin_check(void *arg, dmu_tx_t *tx)
 
                if ((featureflags & DMU_BACKUP_FEATURE_RAW) == 0 &&
                    drba->drba_origin == NULL) {
+                       boolean_t will_encrypt;
+
+                       /*
+                        * Check that we aren't breaking any encryption rules
+                        * and that we have all the parameters we need to
+                        * create an encrypted dataset if necessary. If we are
+                        * making an encrypted dataset the stream can't have
+                        * embedded data.
+                        */
                        error = dmu_objset_create_crypt_check(ds->ds_dir,
-                           drba->drba_dcp);
+                           drba->drba_dcp, &will_encrypt);
                        if (error != 0) {
                                dsl_dataset_rele_flags(ds, dsflags, FTAG);
                                return (error);
                        }
+
+                       if (will_encrypt &&
+                           (featureflags & DMU_BACKUP_FEATURE_EMBED_DATA)) {
+                               dsl_dataset_rele_flags(ds, dsflags, FTAG);
+                               return (SET_ERROR(EINVAL));
+                       }
                }
 
                /*
@@ -1794,6 +1828,12 @@ dmu_recv_begin_check(void *arg, dmu_tx_t *tx)
                                dsl_dataset_rele_flags(ds, dsflags, FTAG);
                                return (SET_ERROR(ENODEV));
                        }
+                       if (origin->ds_dir->dd_crypto_obj != 0 &&
+                           (featureflags & DMU_BACKUP_FEATURE_EMBED_DATA)) {
+                               dsl_dataset_rele_flags(origin, dsflags, FTAG);
+                               dsl_dataset_rele_flags(ds, dsflags, FTAG);
+                               return (SET_ERROR(EINVAL));
+                       }
                        dsl_dataset_rele_flags(origin,
                            dsflags, FTAG);
                }
@@ -3786,12 +3826,8 @@ dmu_recv_stream(dmu_recv_cookie_t *drc, vnode_t *vp, offset_t *voffp,
        featureflags = DMU_GET_FEATUREFLAGS(drc->drc_drrb->drr_versioninfo);
        ra->featureflags = featureflags;
 
-       /* embedded data is incompatible with encrypted datasets */
-       if (ra->os->os_encrypted &&
-           (featureflags & DMU_BACKUP_FEATURE_EMBED_DATA)) {
-               err = SET_ERROR(EINVAL);
-               goto out;
-       }
+       ASSERT0(ra->os->os_encrypted &&
+           (featureflags & DMU_BACKUP_FEATURE_EMBED_DATA));
 
        /* if this stream is dedup'ed, set up the avl tree for guid mapping */
        if (featureflags & DMU_BACKUP_FEATURE_DEDUP) {

diff --git a/module/zfs/dsl_crypt.c b/module/zfs/dsl_crypt.c
index 38e6123d32d30cd11576616586e5dfb5d894d4ed..f0878c934771bf4df8a0b21a3a35734957ed4a6e 100644 (file)
--- a/module/zfs/dsl_crypt.c
+++ b/module/zfs/dsl_crypt.c
@@ -1715,12 +1715,16 @@ dmu_objset_clone_crypt_check(dsl_dir_t *parentdd, dsl_dir_t *origindd)
 
 
 int
-dmu_objset_create_crypt_check(dsl_dir_t *parentdd, dsl_crypto_params_t *dcp)
+dmu_objset_create_crypt_check(dsl_dir_t *parentdd, dsl_crypto_params_t *dcp,
+    boolean_t *will_encrypt)
 {
        int ret;
        uint64_t pcrypt, crypt;
        dsl_crypto_params_t dummy_dcp = { 0 };
 
+       if (will_encrypt != NULL)
+               *will_encrypt = B_FALSE;
+
        if (dcp == NULL)
                dcp = &dummy_dcp;
 
@@ -1758,10 +1762,13 @@ dmu_objset_create_crypt_check(dsl_dir_t *parentdd, dsl_crypto_params_t *dcp)
                return (0);
        }
 
+       if (will_encrypt != NULL)
+               *will_encrypt = B_TRUE;
+
        /*
         * We will now definitely be encrypting. Check the feature flag. When
         * creating the pool the caller will check this for us since we won't
-        * technically have the fetaure activated yet.
+        * technically have the feature activated yet.
         */
        if (parentdd != NULL &&
            !spa_feature_is_enabled(parentdd->dd_pool->dp_spa,

diff --git a/module/zfs/spa.c b/module/zfs/spa.c
index 537e1906874439e4810352c66baea37653fdbc5f..0d0cb556cc788a6a9b15cc2d426c0607431c12b5 100644 (file)
--- a/module/zfs/spa.c
+++ b/module/zfs/spa.c
@@ -4959,7 +4959,7 @@ spa_create_check_encryption_params(dsl_crypto_params_t *dcp,
            !has_encryption)
                return (SET_ERROR(ENOTSUP));
 
-       return (dmu_objset_create_crypt_check(NULL, dcp));
+       return (dmu_objset_create_crypt_check(NULL, dcp, NULL));
 }
 
 /*