Project dnode should be protected by local MAC

This patch corrects a small security issue with 9c5167d1. When the
project dnode was added to the objset_phys_t, it was not included
in the local MAC for cryptographic protection, allowing an attacker
to modify this data without the consent of the key holder. This
patch does represent an on-disk format change for anyone using
project dnodes on an encrypted dataset.

Signed-off-by: Tom Caputi <tcaputi@datto.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #7177

diff --git a/module/zfs/zio_crypt.c b/module/zfs/zio_crypt.c
index 823e6b8d66eaf986b9b56aba9e525ab3afb37666..155bce9fb433f5de514d5f0885ecb9cafc7e8565 100644 (file)
--- a/module/zfs/zio_crypt.c
+++ b/module/zfs/zio_crypt.c
@@ -1095,9 +1095,9 @@ error:
 
 /*
  * objset_phys_t blocks introduce a number of exceptions to the normal
- * authentication process. objset_phys_t's contain 2 seperate HMACS for
+ * authentication process. objset_phys_t's contain 2 separate HMACS for
  * protecting the integrity of their data. The portable_mac protects the
- * the metadnode. This MAC can be sent with a raw send and protects against
+ * metadnode. This MAC can be sent with a raw send and protects against
  * reordering of data within the metadnode. The local_mac protects the user
  * accounting objects which are not sent from one system to another.
  *
@@ -1199,8 +1199,10 @@ zio_crypt_do_objset_hmacs(zio_crypt_key_t *key, void *data, uint_t datalen,
         * The local MAC protects the user and group accounting. If these
         * objects are not present, the local MAC is zeroed out.
         */
-       if (osp->os_userused_dnode.dn_type == DMU_OT_NONE &&
-           osp->os_groupused_dnode.dn_type == DMU_OT_NONE) {
+       if (datalen >= OBJSET_PHYS_SIZE_V2 &&
+           osp->os_userused_dnode.dn_type == DMU_OT_NONE &&
+           osp->os_groupused_dnode.dn_type == DMU_OT_NONE &&
+           osp->os_projectused_dnode.dn_type == DMU_OT_NONE) {
                bzero(local_mac, ZIO_OBJSET_MAC_LEN);
                return (0);
        }
@@ -1231,15 +1233,27 @@ zio_crypt_do_objset_hmacs(zio_crypt_key_t *key, void *data, uint_t datalen,
        }
 
        /* add in fields from the user accounting dnodes */
-       ret = zio_crypt_do_dnode_hmac_updates(ctx, key->zk_version,
-           should_bswap, &osp->os_userused_dnode);
-       if (ret)
-               goto error;
+       if (osp->os_userused_dnode.dn_type != DMU_OT_NONE) {
+               ret = zio_crypt_do_dnode_hmac_updates(ctx, key->zk_version,
+                   should_bswap, &osp->os_userused_dnode);
+               if (ret)
+                       goto error;
+       }
 
-       ret = zio_crypt_do_dnode_hmac_updates(ctx, key->zk_version,
-           should_bswap, &osp->os_groupused_dnode);
-       if (ret)
-               goto error;
+       if (osp->os_groupused_dnode.dn_type != DMU_OT_NONE) {
+               ret = zio_crypt_do_dnode_hmac_updates(ctx, key->zk_version,
+                   should_bswap, &osp->os_groupused_dnode);
+               if (ret)
+                       goto error;
+       }
+
+       if (osp->os_projectused_dnode.dn_type != DMU_OT_NONE &&
+           datalen >= OBJSET_PHYS_SIZE_V3) {
+               ret = zio_crypt_do_dnode_hmac_updates(ctx, key->zk_version,
+                   should_bswap, &osp->os_projectused_dnode);
+               if (ret)
+                       goto error;
+       }
 
        /* store the final digest in a temporary buffer and copy what we need */
        cd.cd_length = SHA512_DIGEST_LENGTH;