From 63a26454ba43ca3af5020f25d2e59c7ee15bf13d Mon Sep 17 00:00:00 2001 From: George Amanakis Date: Fri, 21 Jan 2022 20:41:17 +0100 Subject: [PATCH] Introduce a flag to skip comparing the local mac when raw sending Raw receiving a snapshot back to the originating dataset is currently impossible because of user accounting being present in the originating dataset. One solution would be resetting user accounting when raw receiving on the receiving dataset. However, to recalculate it we would have to dirty all dnodes, which may not be preferable on big datasets. Instead, we rely on the os_phys flag OBJSET_FLAG_USERACCOUNTING_COMPLETE to indicate that user accounting is incomplete when raw receiving. Thus, on the next mount of the receiving dataset the local mac protecting user accounting is zeroed out. The flag is then cleared when user accounting of the raw received snapshot is calculated. Reviewed-by: Brian Behlendorf Signed-off-by: George Amanakis Closes #12981 Closes #10523 Closes #11221 Closes #11294 Closes #12594 Issue #11300 --- module/os/freebsd/zfs/zio_crypt.c | 17 ++- module/os/linux/zfs/zio_crypt.c | 17 ++- module/zfs/dnode_sync.c | 9 +- module/zfs/dsl_crypt.c | 9 +- tests/runfiles/common.run | 3 +- .../tests/functional/userquota/Makefile.am | 3 +- .../userquota/userspace_send_encrypted.ksh | 119 ++++++++++++++++++ 7 files changed, 163 insertions(+), 14 deletions(-) create mode 100755 tests/zfs-tests/tests/functional/userquota/userspace_send_encrypted.ksh diff --git a/module/os/freebsd/zfs/zio_crypt.c b/module/os/freebsd/zfs/zio_crypt.c index c55c1ac25..ea120bcb5 100644 --- a/module/os/freebsd/zfs/zio_crypt.c +++ b/module/os/freebsd/zfs/zio_crypt.c @@ -1065,11 +1065,26 @@ zio_crypt_do_objset_hmacs(zio_crypt_key_t *key, void *data, uint_t datalen, bcopy(raw_portable_mac, portable_mac, ZIO_OBJSET_MAC_LEN); + /* + * This is necessary here as we check next whether + * OBJSET_FLAG_USERACCOUNTING_COMPLETE is set in order to + * decide if the local_mac should be zeroed out. That flag will always + * be set by dmu_objset_id_quota_upgrade_cb() and + * dmu_objset_userspace_upgrade_cb() if useraccounting has been + * completed. + */ + intval = osp->os_flags; + if (should_bswap) + intval = BSWAP_64(intval); + boolean_t uacct_incomplete = + !(intval & OBJSET_FLAG_USERACCOUNTING_COMPLETE); + /* * The local MAC protects the user, group and project accounting. * If these objects are not present, the local MAC is zeroed out. */ - if ((datalen >= OBJSET_PHYS_SIZE_V3 && + if (uacct_incomplete || + (datalen >= OBJSET_PHYS_SIZE_V3 && 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) || diff --git a/module/os/linux/zfs/zio_crypt.c b/module/os/linux/zfs/zio_crypt.c index 9f8b9f53e..a979f7e20 100644 --- a/module/os/linux/zfs/zio_crypt.c +++ b/module/os/linux/zfs/zio_crypt.c @@ -1203,11 +1203,26 @@ zio_crypt_do_objset_hmacs(zio_crypt_key_t *key, void *data, uint_t datalen, bcopy(raw_portable_mac, portable_mac, ZIO_OBJSET_MAC_LEN); + /* + * This is necessary here as we check next whether + * OBJSET_FLAG_USERACCOUNTING_COMPLETE is set in order to + * decide if the local_mac should be zeroed out. That flag will always + * be set by dmu_objset_id_quota_upgrade_cb() and + * dmu_objset_userspace_upgrade_cb() if useraccounting has been + * completed. + */ + intval = osp->os_flags; + if (should_bswap) + intval = BSWAP_64(intval); + boolean_t uacct_incomplete = + !(intval & OBJSET_FLAG_USERACCOUNTING_COMPLETE); + /* * The local MAC protects the user, group and project accounting. * If these objects are not present, the local MAC is zeroed out. */ - if ((datalen >= OBJSET_PHYS_SIZE_V3 && + if (uacct_incomplete || + (datalen >= OBJSET_PHYS_SIZE_V3 && 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) || diff --git a/module/zfs/dnode_sync.c b/module/zfs/dnode_sync.c index dd37e3af7..f574130e5 100644 --- a/module/zfs/dnode_sync.c +++ b/module/zfs/dnode_sync.c @@ -655,8 +655,13 @@ dnode_sync(dnode_t *dn, dmu_tx_t *tx) DNODE_FLAG_USEROBJUSED_ACCOUNTED; mutex_exit(&dn->dn_mtx); dmu_objset_userquota_get_ids(dn, B_FALSE, tx); - } else { - /* Once we account for it, we should always account for it */ + } else if (!(os->os_encrypted && dmu_objset_is_receiving(os))) { + /* + * Once we account for it, we should always account for it, + * except for the case of a raw receive. We will not be able + * to account for it until the receiving dataset has been + * mounted. + */ ASSERT(!(dn->dn_phys->dn_flags & DNODE_FLAG_USERUSED_ACCOUNTED)); ASSERT(!(dn->dn_phys->dn_flags & diff --git a/module/zfs/dsl_crypt.c b/module/zfs/dsl_crypt.c index 26d4c2fe7..1ea184de3 100644 --- a/module/zfs/dsl_crypt.c +++ b/module/zfs/dsl_crypt.c @@ -2007,14 +2007,6 @@ dsl_crypto_recv_raw_objset_check(dsl_dataset_t *ds, dsl_dataset_t *fromds, if (ret != 0) return (ret); - /* - * Useraccounting is not portable and must be done with the keys loaded. - * Therefore, whenever we do any kind of receive the useraccounting - * must not be present. - */ - ASSERT0(os->os_flags & OBJSET_FLAG_USERACCOUNTING_COMPLETE); - ASSERT0(os->os_flags & OBJSET_FLAG_USEROBJACCOUNTING_COMPLETE); - mdn = DMU_META_DNODE(os); /* @@ -2106,6 +2098,7 @@ dsl_crypto_recv_raw_objset_sync(dsl_dataset_t *ds, dmu_objset_type_t ostype, arc_release(os->os_phys_buf, &os->os_phys_buf); bcopy(portable_mac, os->os_phys->os_portable_mac, ZIO_OBJSET_MAC_LEN); bzero(os->os_phys->os_local_mac, ZIO_OBJSET_MAC_LEN); + os->os_flags &= ~OBJSET_FLAG_USERACCOUNTING_COMPLETE; os->os_next_write_raw[tx->tx_txg & TXG_MASK] = B_TRUE; /* set metadnode compression and checksum */ diff --git a/tests/runfiles/common.run b/tests/runfiles/common.run index 0f0eab050..1c0042af1 100644 --- a/tests/runfiles/common.run +++ b/tests/runfiles/common.run @@ -906,7 +906,8 @@ tests = [ 'userquota_004_pos', 'userquota_005_neg', 'userquota_006_pos', 'userquota_007_pos', 'userquota_008_pos', 'userquota_009_pos', 'userquota_010_pos', 'userquota_011_pos', 'userquota_012_neg', - 'userspace_001_pos', 'userspace_002_pos', 'userspace_encrypted'] + 'userspace_001_pos', 'userspace_002_pos', 'userspace_encrypted', + 'userspace_send_encrypted'] tags = ['functional', 'userquota'] [tests/functional/vdev_zaps] diff --git a/tests/zfs-tests/tests/functional/userquota/Makefile.am b/tests/zfs-tests/tests/functional/userquota/Makefile.am index 9100e4ada..2c94d3e15 100644 --- a/tests/zfs-tests/tests/functional/userquota/Makefile.am +++ b/tests/zfs-tests/tests/functional/userquota/Makefile.am @@ -21,7 +21,8 @@ dist_pkgdata_SCRIPTS = \ userspace_001_pos.ksh \ userspace_002_pos.ksh \ userspace_003_pos.ksh \ - userspace_encrypted.ksh + userspace_encrypted.ksh \ + userspace_send_encrypted.ksh dist_pkgdata_DATA = \ userquota.cfg \ diff --git a/tests/zfs-tests/tests/functional/userquota/userspace_send_encrypted.ksh b/tests/zfs-tests/tests/functional/userquota/userspace_send_encrypted.ksh new file mode 100755 index 000000000..e9ef0c426 --- /dev/null +++ b/tests/zfs-tests/tests/functional/userquota/userspace_send_encrypted.ksh @@ -0,0 +1,119 @@ +#!/bin/ksh -p +# +# This file and its contents are supplied under the terms of the +# Common Development and Distribution License ("CDDL"), version 1.0. +# You may only use this file in accordance with the terms of version +# 1.0 of the CDDL. +# +# A full copy of the text of the CDDL should have accompanied this +# source. A copy of the CDDL is also available via the Internet at +# http://www.illumos.org/license/CDDL. +# + +# +# Copyright 2021, George Amanakis . All rights reserved. +# + +. $STF_SUITE/include/libtest.shlib +. $STF_SUITE/tests/functional/userquota/userquota_common.kshlib + +# +# DESCRIPTION: +# Sending raw encrypted datasets back to the source dataset succeeds. +# +# +# STRATEGY: +# 1. Create encrypted source dataset, set userquota and write a file +# 2. Create base snapshot +# 3. Write new file, snapshot, get userspace +# 4. Raw send both snapshots +# 5. Destroy latest snapshot at source and rollback +# 6. Unmount, unload key from source +# 7. Raw send latest snapshot back to source +# 8. Mount both source and target datasets +# 9. Verify encrypted datasets support 'zfs userspace' and 'zfs groupspace' +# and the accounting is done correctly +# + +function cleanup +{ + destroy_pool $POOLNAME + rm -f $FILEDEV +} + +log_onexit cleanup + +FILEDEV="$TEST_BASE_DIR/userspace_encrypted" +POOLNAME="testpool$$" +ENC_SOURCE="$POOLNAME/source" +ENC_TARGET="$POOLNAME/target" + +log_assert "Sending raw encrypted datasets back to the source dataset succeeds." + +# Setup pool and create source +truncate -s 200m $FILEDEV +log_must zpool create -o feature@encryption=enabled $POOLNAME \ + $FILEDEV +log_must eval "echo 'password' | zfs create -o encryption=on" \ + "-o keyformat=passphrase -o keylocation=prompt " \ + "$ENC_SOURCE" + +# Set user quota and write file +log_must zfs set userquota@$QUSER1=50m $ENC_SOURCE +mkmount_writable $ENC_SOURCE +mntpnt=$(get_prop mountpoint $ENC_SOURCE) +log_must user_run $QUSER1 mkfile 10m /$mntpnt/file1 +sync + +# Snapshot +log_must zfs snap $ENC_SOURCE@base + +# Write new file, snapshot, get userspace +log_must user_run $QUSER1 mkfile 20m /$mntpnt/file2 +log_must zfs snap $ENC_SOURCE@s1 + +# Raw send both snapshots +log_must eval "zfs send -w $ENC_SOURCE@base | zfs recv " \ + "$ENC_TARGET" +log_must eval "zfs send -w -i @base $ENC_SOURCE@s1 | zfs recv " \ + "$ENC_TARGET" + +# Destroy latest snapshot at source and rollback +log_must zfs destroy $ENC_SOURCE@s1 +log_must zfs rollback $ENC_SOURCE@base +rollback_uspace=$(zfs userspace -Hp $ENC_SOURCE | \ + awk "/$QUSER1/"' {printf "%d\n", $4 / 1024 / 1024}') + +# Unmount, unload key +log_must zfs umount $ENC_SOURCE +log_must zfs unload-key -a + +# Raw send latest snapshot back to source +log_must eval "zfs send -w -i @base $ENC_TARGET@s1 | zfs recv " \ + "$ENC_SOURCE" + +# Mount encrypted datasets and verify they support 'zfs userspace' and +# 'zfs groupspace' and the accounting is done correctly +log_must eval "echo 'password' | zfs load-key $ENC_SOURCE" +log_must eval "echo 'password' | zfs load-key $ENC_TARGET" +log_must zfs mount $ENC_SOURCE +log_must zfs mount $ENC_TARGET +sync + +sleep 5 + +src_uspace=$(zfs userspace -Hp $ENC_SOURCE | \ + awk "/$QUSER1/"' {printf "%d\n", $4 / 1024 / 1024}') +tgt_uspace=$(zfs userspace -Hp $ENC_TARGET | \ + awk "/$QUSER1/"' {printf "%d\n", $4 / 1024 / 1024}') +log_must test "$src_uspace" -eq "$tgt_uspace" +log_must test "$rollback_uspace" -ne "$src_uspace" + +src_uquota=$(zfs userspace -Hp $ENC_SOURCE | awk "/$QUSER1/"' {print $5}') +tgt_uquota=$(zfs userspace -Hp $ENC_TARGET | awk "/$QUSER1/"' {print $5}') +log_must test "$src_uquota" -eq "$tgt_uquota" + +# Cleanup +cleanup + +log_pass "Sending raw encrypted datasets back to the source dataset succeeds."