Detect and prevent mixed raw and non-raw sends

Currently, there is an issue in the raw receive code where
raw receives are allowed to happen on top of previously
non-raw received datasets. This is a problem because the
source-side dataset doesn't know about how the blocks on
the destination were encrypted. As a result, any MAC in
the objset's checksum-of-MACs tree that is a parent of both
blocks encrypted on the source and blocks encrypted by the
destination will be incorrect. This will result in
authentication errors when we decrypt the dataset.

This patch fixes this issue by adding a new check to the
raw receive code. The code now maintains an "IVset guid",
which acts as an identifier for the set of IVs used to
encrypt a given snapshot. When a snapshot is raw received,
the destination snapshot will take this value from the
DRR_BEGIN payload. Non-raw receives and normal "zfs snap"
operations will cause ZFS to generate a new IVset guid.
When a raw incremental stream is received, ZFS will check
that the "from" IVset guid in the stream matches that of
the "from" destination snapshot. If they do not match, the
code will error out the receive, preventing the problem.

This patch requires an on-disk format change to add the
IVset guids to snapshots and bookmarks. As a result, this
patch has errata handling and a tunable to help affected
users resolve the issue with as little interruption as
possible.

Reviewed-by: Paul Dagnelie <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Matt Ahrens <[email protected]>
Signed-off-by: Tom Caputi <[email protected]>
Closes #8308

1 files changed, 47 insertions, 15 deletions

diff --git a/module/zfs/dmu_recv.c b/module/zfs/dmu_recv.c
index 879460318..e49a0f4aa 100644
--- a/module/zfs/dmu_recv.c
+++ b/module/zfs/dmu_recv.c
@@ -72,7 +72,6 @@ typedef struct dmu_recv_begin_arg {
 	dmu_recv_cookie_t *drba_cookie;
 	cred_t *drba_cred;
 	dsl_crypto_params_t *drba_dcp;
-	uint64_t drba_snapobj;
 } dmu_recv_begin_arg_t;
 
 static int
@@ -128,7 +127,7 @@ recv_begin_check_existing_impl(dmu_recv_begin_arg_t *drba, dsl_dataset_t *ds,
 		dsl_dataset_t *snap;
 		uint64_t obj = dsl_dataset_phys(ds)->ds_prev_snap_obj;
 
-		/* Can't perform a raw receive on top of a non-raw receive */
+		/* Can't raw receive on top of an unencrypted dataset */
 		if (!encrypted && raw)
 			return (SET_ERROR(EINVAL));
 
@@ -155,7 +154,7 @@ recv_begin_check_existing_impl(dmu_recv_begin_arg_t *drba, dsl_dataset_t *ds,
 			return (SET_ERROR(ENODEV));
 
 		if (drba->drba_cookie->drc_force) {
-			drba->drba_snapobj = obj;
+			drba->drba_cookie->drc_fromsnapobj = obj;
 		} else {
 			/*
 			 * If we are not forcing, there must be no
@@ -165,7 +164,8 @@ recv_begin_check_existing_impl(dmu_recv_begin_arg_t *drba, dsl_dataset_t *ds,
 				dsl_dataset_rele(snap, FTAG);
 				return (SET_ERROR(ETXTBSY));
 			}
-			drba->drba_snapobj = ds->ds_prev->ds_object;
+			drba->drba_cookie->drc_fromsnapobj =
+			    ds->ds_prev->ds_object;
 		}
 
 		dsl_dataset_rele(snap, FTAG);
@@ -200,7 +200,7 @@ recv_begin_check_existing_impl(dmu_recv_begin_arg_t *drba, dsl_dataset_t *ds,
 				return (SET_ERROR(EINVAL));
 		}
 
-		drba->drba_snapobj = 0;
+		drba->drba_cookie->drc_fromsnapobj = 0;
 	}
 
 	return (0);
@@ -440,7 +440,7 @@ dmu_recv_begin_sync(void *arg, dmu_tx_t *tx)
 	 * the raw cmd set. Raw incremental recvs do not use a dcp
 	 * since the encryption parameters are already set in stone.
 	 */
-	if (dcp == NULL && drba->drba_snapobj == 0 &&
+	if (dcp == NULL && drba->drba_cookie->drc_fromsnapobj == 0 &&
 	    drba->drba_origin == NULL) {
 		ASSERT3P(dcp, ==, NULL);
 		dcp = &dummy_dcp;
@@ -454,15 +454,15 @@ dmu_recv_begin_sync(void *arg, dmu_tx_t *tx)
 		/* create temporary clone */
 		dsl_dataset_t *snap = NULL;
 
-		if (drba->drba_snapobj != 0) {
+		if (drba->drba_cookie->drc_fromsnapobj != 0) {
 			VERIFY0(dsl_dataset_hold_obj(dp,
-			    drba->drba_snapobj, FTAG, &snap));
+			    drba->drba_cookie->drc_fromsnapobj, FTAG, &snap));
 			ASSERT3P(dcp, ==, NULL);
 		}
 
 		dsobj = dsl_dataset_create_sync(ds->ds_dir, recv_clone_name,
 		    snap, crflags, drba->drba_cred, dcp, tx);
-		if (drba->drba_snapobj != 0)
+		if (drba->drba_cookie->drc_fromsnapobj != 0)
 			dsl_dataset_rele(snap, FTAG);
 		dsl_dataset_rele_flags(ds, dsflags, FTAG);
 	} else {
@@ -2526,11 +2526,16 @@ dmu_recv_stream(dmu_recv_cookie_t *drc, vnode_t *vp, offset_t *voffp,
 		 * the keynvl away until then.
 		 */
 		err = dsl_crypto_recv_raw(spa_name(ra->os->os_spa),
-		    drc->drc_ds->ds_object, drc->drc_drrb->drr_type,
-		    keynvl, drc->drc_newfs);
+		    drc->drc_ds->ds_object, drc->drc_fromsnapobj,
+		    drc->drc_drrb->drr_type, keynvl, drc->drc_newfs);
 		if (err != 0)
 			goto out;
 
+		/* see comment in dmu_recv_end_sync() */
+		drc->drc_ivset_guid = 0;
+		(void) nvlist_lookup_uint64(keynvl, "to_ivset_guid",
+		    &drc->drc_ivset_guid);
+
 		if (!drc->drc_newfs)
 			drc->drc_keynvl = fnvlist_dup(keynvl);
 	}
@@ -2591,10 +2596,10 @@ dmu_recv_stream(dmu_recv_cookie_t *drc, vnode_t *vp, offset_t *voffp,
 		    sizeof (struct receive_record_arg) + ra->rrd->payload_size);
 		ra->rrd = NULL;
 	}
-	if (ra->next_rrd == NULL)
-		ra->next_rrd = kmem_zalloc(sizeof (*ra->next_rrd), KM_SLEEP);
-	ra->next_rrd->eos_marker = B_TRUE;
-	bqueue_enqueue(&rwa->q, ra->next_rrd, 1);
+	ASSERT3P(ra->rrd, ==, NULL);
+	ra->rrd = kmem_zalloc(sizeof (*ra->rrd), KM_SLEEP);
+	ra->rrd->eos_marker = B_TRUE;
+	bqueue_enqueue(&rwa->q, ra->rrd, 1);
 
 	mutex_enter(&rwa->mutex);
 	while (!rwa->done) {
@@ -2635,6 +2640,14 @@ dmu_recv_stream(dmu_recv_cookie_t *drc, vnode_t *vp, offset_t *voffp,
 		err = rwa->err;
 
 out:
+	/*
+	 * If we hit an error before we started the receive_writer_thread
+	 * we need to clean up the next_rrd we create by processing the
+	 * DRR_BEGIN record.
+	 */
+	if (ra->next_rrd != NULL)
+		kmem_free(ra->next_rrd, sizeof (*ra->next_rrd));
+
 	nvlist_free(begin_nvl);
 	if ((featureflags & DMU_BACKUP_FEATURE_DEDUP) && (cleanup_fd != -1))
 		zfs_onexit_fd_rele(cleanup_fd);
@@ -2838,6 +2851,25 @@ dmu_recv_end_sync(void *arg, dmu_tx_t *tx)
 		drc->drc_newsnapobj =
 		    dsl_dataset_phys(drc->drc_ds)->ds_prev_snap_obj;
 	}
+
+	/*
+	 * If this is a raw receive, the crypt_keydata nvlist will include
+	 * a to_ivset_guid for us to set on the new snapshot. This value
+	 * will override the value generated by the snapshot code. However,
+	 * this value may not be present, because older implementations of
+	 * the raw send code did not include this value, and we are still
+	 * allowed to receive them if the zfs_disable_ivset_guid_check
+	 * tunable is set, in which case we will leave the newly-generated
+	 * value.
+	 */
+	if (drc->drc_raw && drc->drc_ivset_guid != 0) {
+		dmu_object_zapify(dp->dp_meta_objset, drc->drc_newsnapobj,
+		    DMU_OT_DSL_DATASET, tx);
+		VERIFY0(zap_update(dp->dp_meta_objset, drc->drc_newsnapobj,
+		    DS_FIELD_IVSET_GUID, sizeof (uint64_t), 1,
+		    &drc->drc_ivset_guid, tx));
+	}
+
 	zvol_create_minors(dp->dp_spa, drc->drc_tofs, B_TRUE);
 
 	/*