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

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,

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);

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 */
+	ASSERT0(ra->os->os_encrypted &&
-	if (ra->os->os_encrypted &&
+	    (featureflags & DMU_BACKUP_FEATURE_EMBED_DATA));
-	    (featureflags & DMU_BACKUP_FEATURE_EMBED_DATA)) {
-		err = SET_ERROR(EINVAL);
-		goto out;
-	}
 
 	/* if this stream is dedup'ed, set up the avl tree for guid mapping */
 	if (featureflags & DMU_BACKUP_FEATURE_DEDUP) {

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,

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));
 }
 
 /*