Fix for openzfs#6845

The current on-disk format for encrypted datasets protects
not only the encrypted and authenticated blocks, but also
the order and interpretation of these blocks. In order to
make this work while maintaining the ability to do raw sends
the indirect bps maintain a secure checksum of all the MACs
in the block below it, along with a few other fields that
determine how the data is interpretted.

Unfortunately, the current on-disk format erroniously
includes some fields which are not portable and thus cannot
support raw sends. It is also not possible to easily work
around this issue due to a separate and much smaller bug
which causes indirect blocks for encrypted dnodes to not
be compressed, which conflicts with the previous bug. In
addition, raw send streams do not currently include
dn_maxblkid which is needed in order to ensure that we are
correctly maintaining the portable objset MAC.

This patch zero's out the offending fields when computing the
bp MAC (as they should have been) and registers an errata for
the on-disk format bug. We detect the errata by adding a
"version" field to newly created DSL Crypto Keys. We allow
datasets without a version (version 0) to only be mounted for
read so that they can easily be migrated. We also now include
dn_maxblkid in raw send streams to ensure the MAC can be
maintained correctly.

Signed-off-by: Tom Caputi <tcaputi@datto.com>

cmd/zfs/zfs_main.c

@@ -342,8 +342,8 @@ get_usage(zfs_help_t idx)
 		return (gettext("\tunload-key [-r] "
 		    "<-a | filesystem|volume>\n"));
 	case HELP_CHANGE_KEY:
-		return (gettext("\tchange-key [-l] [-o keyformat=<value>]"
-		    "\t    [-o keylocation=<value>] [-o pbkfd2iters=<value>]"
+		return (gettext("\tchange-key [-l] [-o keyformat=<value>]\n"
+		    "\t    [-o keylocation=<value>] [-o pbkfd2iters=<value>]\n"
 		    "\t    <filesystem|volume>\n"
 		    "\tchange-key -i [-l] <filesystem|volume>\n"));
 	}

cmd/zpool/zpool_main.c

@@ -2119,6 +2119,17 @@ show_import(nvlist_t *config)
 				    "updating.\n"));
 				break;
 
+			case ZPOOL_ERRATA_ZOL_6845_ENCRYPTION:
+				(void) printf(gettext(" action: existing "
+				    "encrypted datasets cannot be used with "
+				    "this\n\tversion of zfs due to an on-disk "
+				    "incompatibility which\n\tneeds to be "
+				    "corrected. Revert to an eariler"
+				    "version and\n\tbackup and destroy any "
+				    "existing encrypted datasets\n\tbefore "
+				    "updating.\n"));
+				break;
+
 			default:
 				/*
 				 * All errata must contain an action message.
@@ -6470,6 +6481,13 @@ status_callback(zpool_handle_t *zhp, void *data)
 			    "run 'zpool scrub'.\n"));
 			break;
 
+		case ZPOOL_ERRATA_ZOL_6845_ENCRYPTION:
+			(void) printf(gettext("action: To correct the issue "
+			    "revert to an earlier version and backup\n\tand "
+			    "destroy any existing encrypted datasets before "
+			    "updating.\n"));
+			break;
+
 		default:
 			/*
 			 * All errata which allow the pool to be imported

cmd/zstreamdump/zstreamdump.c

@@ -451,7 +451,8 @@ main(int argc, char *argv[])
 				(void) printf("OBJECT object = %llu type = %u "
 				    "bonustype = %u blksz = %u bonuslen = %u "
 				    "dn_slots = %u raw_bonuslen = %u "
-				    "flags = %u indblkshift = %u nlevels = %u "
+				    "flags = %u maxblkid = %llu "
+				    "indblkshift = %u nlevels = %u "
 				    "nblkptr = %u\n",
 				    (u_longlong_t)drro->drr_object,
 				    drro->drr_type,
@@ -461,6 +462,7 @@ main(int argc, char *argv[])
 				    drro->drr_dn_slots,
 				    drro->drr_raw_bonuslen,
 				    drro->drr_flags,
+				    (u_longlong_t)drro->drr_maxblkid,
 				    drro->drr_indblkshift,
 				    drro->drr_nlevels,
 				    drro->drr_nblkptr);

include/sys/dmu.h

@@ -434,6 +434,13 @@ int dmu_object_set_nlevels(objset_t *os, uint64_t object, int nlevels,
 int dmu_object_set_blocksize(objset_t *os, uint64_t object, uint64_t size,
     int ibs, dmu_tx_t *tx);
 
+/*
+ * Manually set the maxblkid on a dnode. This will adjust nlevels accordingly
+ * to accommodate the change.
+ */
+int dmu_object_set_maxblkid(objset_t *os, uint64_t object, uint64_t maxblkid,
+    dmu_tx_t *tx);
+
 /*
  * Set the checksum property on a dnode.  The new checksum algorithm will
  * apply to all newly written blocks; existing blocks will not be affected.

include/sys/dmu_objset.h

@@ -217,6 +217,7 @@ boolean_t dmu_objset_userobjused_enabled(objset_t *os);
 boolean_t dmu_objset_userobjspace_upgradable(objset_t *os);
 void dmu_objset_userobjspace_upgrade(objset_t *os);
 boolean_t dmu_objset_userobjspace_present(objset_t *os);
+boolean_t dmu_objset_bad_encryption_version(objset_t *os);
 
 int dmu_fsname(const char *snapname, char *buf);
 

include/sys/dsl_crypt.h

@@ -39,7 +39,7 @@
 #define	DSL_CRYPTO_KEY_HMAC_KEY		"DSL_CRYPTO_HMAC_KEY_1"
 #define	DSL_CRYPTO_KEY_ROOT_DDOBJ	"DSL_CRYPTO_ROOT_DDOBJ"
 #define	DSL_CRYPTO_KEY_REFCOUNT		"DSL_CRYPTO_REFCOUNT"
-
+#define	DSL_CRYPTO_KEY_VERSION		"DSL_CRYPTO_VERSION"
 
 /*
  * In-memory representation of a wrapping key. One of these structs will exist
@@ -169,6 +169,7 @@ int dsl_crypto_params_create_nvlist(dcp_cmd_t cmd, nvlist_t *props,
 void dsl_crypto_params_free(dsl_crypto_params_t *dcp, boolean_t unload);
 void dsl_dataset_crypt_stats(struct dsl_dataset *ds, nvlist_t *nv);
 int dsl_crypto_can_set_keylocation(const char *dsname, const char *keylocation);
+boolean_t dsl_dir_bad_encryption_version(dsl_dir_t *dd);
 
 void spa_keystore_init(spa_keystore_t *sk);
 void spa_keystore_fini(spa_keystore_t *sk);

include/sys/fs/zfs.h

@@ -890,6 +890,7 @@ typedef enum zpool_errata {
 	ZPOOL_ERRATA_NONE,
 	ZPOOL_ERRATA_ZOL_2094_SCRUB,
 	ZPOOL_ERRATA_ZOL_2094_ASYNC_DESTROY,
+	ZPOOL_ERRATA_ZOL_6845_ENCRYPTION,
 } zpool_errata_t;
 
 /*

include/sys/zfs_ioctl.h

@@ -219,7 +219,8 @@ typedef struct dmu_replay_record {
 			uint8_t drr_flags;
 			uint32_t drr_raw_bonuslen;
 			uint64_t drr_toguid;
-			/* only nonzero for raw streams */
+			/* only (possibly) nonzero for raw streams */
+			uint64_t drr_maxblkid;
 			uint8_t drr_indblkshift;
 			uint8_t drr_nlevels;
 			uint8_t drr_nblkptr;

include/sys/zio_crypt.h

@@ -36,6 +36,8 @@ struct zbookmark_phys;
 #define	MASTER_KEY_MAX_LEN	32
 #define	SHA512_HMAC_KEYLEN	64
 
+#define	ZIO_CRYPT_KEY_CURRENT_VERSION	1ULL
+
 typedef enum zio_crypt_type {
 	ZC_TYPE_NONE = 0,
 	ZC_TYPE_CCM,
@@ -64,6 +66,9 @@ typedef struct zio_crypt_key {
 	/* encryption algorithm */
 	uint64_t zk_crypt;
 
+	/* on-disk format version */
+	uint64_t zk_version;
+
 	/* GUID for uniquely identifying this key. Not encrypted on disk. */
 	uint64_t zk_guid;
 
@@ -104,9 +109,9 @@ int zio_crypt_key_get_salt(zio_crypt_key_t *key, uint8_t *salt_out);
 
 int zio_crypt_key_wrap(crypto_key_t *cwkey, zio_crypt_key_t *key, uint8_t *iv,
     uint8_t *mac, uint8_t *keydata_out, uint8_t *hmac_keydata_out);
-int zio_crypt_key_unwrap(crypto_key_t *cwkey, uint64_t crypt, uint64_t guid,
-    uint8_t *keydata, uint8_t *hmac_keydata, uint8_t *iv, uint8_t *mac,
-    zio_crypt_key_t *key);
+int zio_crypt_key_unwrap(crypto_key_t *cwkey, uint64_t crypt, uint64_t version,
+    uint64_t guid, uint8_t *keydata, uint8_t *hmac_keydata, uint8_t *iv,
+    uint8_t *mac, zio_crypt_key_t *key);
 int zio_crypt_generate_iv(uint8_t *ivbuf);
 int zio_crypt_generate_iv_salt_dedup(zio_crypt_key_t *key, uint8_t *data,
     uint_t datalen, uint8_t *ivbuf, uint8_t *salt);

module/zfs/dmu.c

@@ -2022,6 +2022,23 @@ dmu_object_set_blocksize(objset_t *os, uint64_t object, uint64_t size, int ibs,
 	return (err);
 }
 
+int
+dmu_object_set_maxblkid(objset_t *os, uint64_t object, uint64_t maxblkid,
+    dmu_tx_t *tx)
+{
+	dnode_t *dn;
+	int err;
+
+	err = dnode_hold(os, object, FTAG, &dn);
+	if (err)
+		return (err);
+	rw_enter(&dn->dn_struct_rwlock, RW_WRITER);
+	dnode_new_blkid(dn, maxblkid, tx, B_FALSE);
+	rw_exit(&dn->dn_struct_rwlock);
+	dnode_rele(dn, FTAG);
+	return (err);
+}
+
 void
 dmu_object_set_checksum(objset_t *os, uint64_t object, uint8_t checksum,
     dmu_tx_t *tx)
@@ -2207,8 +2224,10 @@ dmu_write_policy(objset_t *os, dnode_t *dn, int level, int wp, zio_prop_t *zp)
 			dedup = B_FALSE;
 		}
 
-		if (type == DMU_OT_DNODE || type == DMU_OT_OBJSET)
+		if (level <= 0 &&
+		    (type == DMU_OT_DNODE || type == DMU_OT_OBJSET)) {
 			compress = ZIO_COMPRESS_EMPTY;
+		}
 	}
 
 	zp->zp_compress = compress;
@@ -2483,6 +2502,7 @@ EXPORT_SYMBOL(dmu_object_size_from_db);
 EXPORT_SYMBOL(dmu_object_dnsize_from_db);
 EXPORT_SYMBOL(dmu_object_set_nlevels);
 EXPORT_SYMBOL(dmu_object_set_blocksize);
+EXPORT_SYMBOL(dmu_object_set_maxblkid);
 EXPORT_SYMBOL(dmu_object_set_checksum);
 EXPORT_SYMBOL(dmu_object_set_compress);
 EXPORT_SYMBOL(dmu_write_policy);

module/zfs/dmu_objset.c

@@ -660,6 +660,9 @@ dmu_objset_own_impl(dsl_dataset_t *ds, dmu_objset_type_t type,
 		return (SET_ERROR(EINVAL));
 	} else if (!readonly && dsl_dataset_is_snapshot(ds)) {
 		return (SET_ERROR(EROFS));
+	} else if (!readonly && decrypt &&
+	    dsl_dir_bad_encryption_version(ds->ds_dir)) {
+		return (SET_ERROR(EROFS));
 	}
 
 	/* if we are decrypting, we can now check MACs in os->os_phys_buf */
@@ -2625,6 +2628,12 @@ dmu_objset_find(char *name, int func(const char *, void *), void *arg,
 	return (error);
 }
 
+boolean_t
+dmu_objset_bad_encryption_version(objset_t *os)
+{
+	return (dsl_dir_bad_encryption_version(os->os_dsl_dataset->ds_dir));
+}
+
 void
 dmu_objset_set_user(objset_t *os, void *user_ptr)
 {

module/zfs/dmu_send.c

@@ -570,6 +570,7 @@ dump_dnode(dmu_sendarg_t *dsp, const blkptr_t *bp, uint64_t object,
 			drro->drr_flags |= DRR_RAW_BYTESWAP;
 
 		/* needed for reconstructing dnp on recv side */
+		drro->drr_maxblkid = dnp->dn_maxblkid;
 		drro->drr_indblkshift = dnp->dn_indblkshift;
 		drro->drr_nlevels = dnp->dn_nlevels;
 		drro->drr_nblkptr = dnp->dn_nblkptr;
@@ -2538,6 +2539,8 @@ receive_object(struct receive_writer_arg *rwa, struct drr_object *drro,
 		    drro->drr_blksz, drro->drr_indblkshift, tx));
 		VERIFY0(dmu_object_set_nlevels(rwa->os, drro->drr_object,
 		    drro->drr_nlevels, tx));
+		VERIFY0(dmu_object_set_maxblkid(rwa->os, drro->drr_object,
+		    drro->drr_maxblkid, tx));
 	}
 
 	if (data != NULL) {

module/zfs/dnode_sync.c

@@ -699,6 +699,18 @@ dnode_sync(dnode_t *dn, dmu_tx_t *tx)
 		mutex_exit(&dn->dn_mtx);
 	}
 
+	/* Must be done after dnode_sync_free_range */
+	mutex_enter(&dn->dn_mtx);
+	if (dn->dn_maxblkid > dnp->dn_maxblkid) {
+		dnp->dn_maxblkid = dn->dn_maxblkid;
+
+		/* assert that maxblkid is possible with nlevels and ibs */
+		ASSERT3U(dnp->dn_maxblkid, <=, dn->dn_nblkptr *
+		    (1ULL << ((dn->dn_indblkshift - SPA_BLKPTRSHIFT) *
+		    (dn->dn_nlevels - 1))));
+	}
+	mutex_exit(&dn->dn_mtx);
+
 	if (freeing_dnode) {
 		dn->dn_objset->os_freed_dnodes++;
 		dnode_sync_free(dn, tx);