Skip to content

Commit

Permalink
Encryption Stability and On-Disk Format Fixes
Browse files Browse the repository at this point in the history
The on-disk format for encrypted datasets protects not only
the encrypted and authenticated blocks themselves, 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 interpreted.

Unfortunately, the current on-disk format erroneously
includes some fields which are not portable and thus cannot
support raw sends. It is 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, the current code generates incompatible on-disk
formats on big endian and little endian systems due to an
issue with how block pointers are authenticated. Finally,
raw send streams do not currently include dn_maxblkid when
sending both the metadnode and normal dnodes which are
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 and ensures that these MACs are always
calculated in little endian order (regardless of the host
system's byte order). This patch also registers an errata
for the old on-disk format, which we detect 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.

Fixes ZOL # 6845

Signed-off-by: Tom Caputi <[email protected]>
  • Loading branch information
Tom Caputi authored and lundman committed Jan 10, 2018
1 parent 4bf474b commit 940e474
Show file tree
Hide file tree
Showing 26 changed files with 671 additions and 164 deletions.
4 changes: 2 additions & 2 deletions usr/src/cmd/zfs/zfs_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -348,8 +348,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"));
}
Expand Down
3 changes: 2 additions & 1 deletion usr/src/cmd/zstreamdump/zstreamdump.c
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ main(int argc, char *argv[])
if (verbose) {
(void) printf("OBJECT object = %llu type = %u "
"bonustype = %u blksz = %u bonuslen = %u "
"raw_bonuslen = %u flags = %u "
"raw_bonuslen = %u flags = %u maxblkid = %llu "
"indblkshift = %u nlevels = %u "
"nblkptr = %u\n",
(u_longlong_t)drro->drr_object,
Expand All @@ -454,6 +454,7 @@ main(int argc, char *argv[])
drro->drr_bonuslen,
drro->drr_raw_bonuslen,
drro->drr_flags,
(u_longlong_t)drro->drr_maxblkid,
drro->drr_indblkshift,
drro->drr_nlevels,
drro->drr_nblkptr);
Expand Down
6 changes: 6 additions & 0 deletions usr/src/pkg/manifests/system-test-zfstest.mf
Original file line number Diff line number Diff line change
Expand Up @@ -1572,6 +1572,8 @@ file \
mode=0444
file path=opt/zfs-tests/tests/functional/cli_root/zpool_import/cleanup \
mode=0555
file path=opt/zfs-tests/tests/functional/cli_root/zpool_import/cryptv0.dat.bz2 \
mode=0444
file path=opt/zfs-tests/tests/functional/cli_root/zpool_import/setup mode=0555
file \
path=opt/zfs-tests/tests/functional/cli_root/zpool_import/zpool_import.cfg \
Expand Down Expand Up @@ -1624,6 +1626,9 @@ file \
file \
path=opt/zfs-tests/tests/functional/cli_root/zpool_import/zpool_import_encrypted_load \
mode=0555
file \
path=opt/zfs-tests/tests/functional/cli_root/zpool_import/zpool_import_errata3 \
mode=0555
file \
path=opt/zfs-tests/tests/functional/cli_root/zpool_import/zpool_import_features_001_pos \
mode=0555
Expand Down Expand Up @@ -2442,6 +2447,7 @@ file path=opt/zfs-tests/tests/functional/rsend/send-c_volume mode=0555
file path=opt/zfs-tests/tests/functional/rsend/send-c_zstreamdump mode=0555
file path=opt/zfs-tests/tests/functional/rsend/send-cpL_varied_recsize \
mode=0555
file path=opt/zfs-tests/tests/functional/rsend/send_encrypted_files mode=0555
file path=opt/zfs-tests/tests/functional/rsend/send_encrypted_heirarchy \
mode=0555
file path=opt/zfs-tests/tests/functional/rsend/setup mode=0555
Expand Down
4 changes: 2 additions & 2 deletions usr/src/test/zfs-tests/runfiles/omnios.run
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ tests = ['zpool_import_001_pos', 'zpool_import_002_pos',
'zpool_import_features_003_pos', 'zpool_import_missing_001_pos',
'zpool_import_missing_002_pos', 'zpool_import_missing_003_pos',
'zpool_import_rename_001_pos', 'zpool_import_encrypted',
'zpool_import_encrypted_load']
'zpool_import_encrypted_load', 'zpool_import_errata3']

[/opt/zfs-tests/tests/functional/cli_root/zpool_labelclear]
tests = ['zpool_labelclear_active', 'zpool_labelclear_exported']
Expand Down Expand Up @@ -507,7 +507,7 @@ tests = ['rsend_001_pos', 'rsend_002_pos', 'rsend_003_pos', 'rsend_004_pos',
'send-c_lz4_disabled', 'send-c_recv_lz4_disabled',
'send-c_mixed_compression', 'send-c_stream_size_estimate', 'send-cD',
'send-c_embedded_blocks', 'send-c_resume', 'send-cpL_varied_recsize',
'send-c_recv_dedup', 'send_encrypted_heirarchy']
'send-c_recv_dedup', 'send_encrypted_heirarchy', 'send_encrypted_files']

[/opt/zfs-tests/tests/functional/scrub_mirror]
tests = ['scrub_mirror_001_pos', 'scrub_mirror_002_pos',
Expand Down
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
#!/bin/ksh -p
#
# CDDL HEADER START
#
# 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.
#
# CDDL HEADER END
#

#
# Copyright (c) 2017 Datto, Inc. All rights reserved.
#

. $STF_SUITE/include/libtest.shlib

#
# DESCRIPTION:
# 'zpool import' should import a pool with Errata #3 while preventing
# the user from performing read write operations
#
# STRATEGY:
# 1. Import a pre-packaged pool with Errata #3
# 2. Attempt to write to the effected datasets
# 3. Attempt to read from the effected datasets
# 4. Attempt to perform a raw send of the effected datasets
# 5. Perform a regular send of the datasets under a new encryption root
# 6. Verify the new datasets can be read from and written to
# 7. Destroy the old effected datasets
# 8. Reimport the pool and verify that the errata is no longer present
#

verify_runnable "global"

POOL_NAME=cryptv0
POOL_FILE=cryptv0.dat

function uncompress_pool
{

log_note "Creating pool from $POOL_FILE"
log_must bzcat \
$STF_SUITE/tests/functional/cli_root/zpool_import/$POOL_FILE.bz2 \
> /$TESTPOOL/$POOL_FILE
return 0
}

function cleanup
{
poolexists $POOL_NAME && log_must zpool destroy $POOL_NAME
[[ -e /$TESTPOOL/$POOL_FILE ]] && rm /$TESTPOOL/$POOL_FILE
return 0
}
log_onexit cleanup

log_assert "Verify that Errata 3 is properly handled"

uncompress_pool
log_must zpool import -d /$TESTPOOL/ $POOL_NAME
log_must eval "zpool status | grep -q Errata"
log_must eval "echo 'password' | zfs load-key $POOL_NAME/testfs"
log_must eval "echo 'password' | zfs load-key $POOL_NAME/testvol"

log_mustnot zfs mount $POOL_NAME/testfs
log_must zfs mount -o ro $POOL_NAME/testfs

old_mntpnt=$(get_prop mountpoint $POOL_NAME/testfs)
log_must eval "ls $old_mntpnt | grep -q testfile"
log_mustnot dd if=/dev/zero of=/dev/zvol/$POOL_NAME/testvol bs=512 count=1
log_must dd if=/dev/zvol/$POOL_NAME/testvol of=/dev/null bs=512 count=1
log_must eval "echo 'password' | zfs create \
-o encryption=on -o keyformat=passphrase -o keylocation=prompt \
cryptv0/encroot"
log_mustnot eval "zfs send -w $POOL_NAME/testfs@snap1 | \
zfs recv $POOL_NAME/encroot/testfs"
log_mustnot eval "zfs send -w $POOL_NAME/testvol@snap1 | \
zfs recv $POOL_NAME/encroot/testvol"

log_must eval "zfs send $POOL_NAME/testfs@snap1 | \
zfs recv $POOL_NAME/encroot/testfs"
log_must eval "zfs send $POOL_NAME/testvol@snap1 | \
zfs recv $POOL_NAME/encroot/testvol"
block_device_wait
log_must dd if=/dev/zero of=/dev/zvol/$POOL_NAME/encroot/testvol bs=512 count=1
new_mntpnt=$(get_prop mountpoint $POOL_NAME/encroot/testfs)
log_must eval "ls $new_mntpnt | grep -q testfile"
log_must zfs destroy -r $POOL_NAME/testfs
log_must zfs destroy -r $POOL_NAME/testvol

log_must zpool export $POOL_NAME
log_must zpool import -d /$TESTPOOL/ $POOL_NAME
log_mustnot eval "zpool status | grep -q Errata"
log_pass "Errata 3 is properly handled"
101 changes: 101 additions & 0 deletions usr/src/test/zfs-tests/tests/functional/rsend/send_encrypted_files.ksh
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
#!/bin/ksh -p
#
# CDDL HEADER START
#
# 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.
#
# CDDL HEADER END
#

#
# Copyright (c) 2017 by Datto Inc. All rights reserved.
#

. $STF_SUITE/tests/functional/rsend/rsend.kshlib

#
# DESCRIPTION:
#
#
# STRATEGY:
# 1. Create a new encrypted filesystem
# 2. Add an empty file to the filesystem
# 3. Add a small 512 byte file to the filesystem
# 4. Add a larger 32M file to the filesystem
# 5. Add a large sparse file to the filesystem
# 6. Add a file truncated to 4M to the filesystem
# 7. Add a sparse file with metadata compression disabled to the filesystem
# 8. Add and remove 1000 empty files to the filesystem
# 9. Snapshot the filesystem
# 10. Send and receive the filesystem, ensuring that it can be mounted
#

verify_runnable "both"

function set_metadata_compression_disabled # <0|1>
{
echo $1 > /sys/usr/src/uts/common/fs/zfs/parameters/zfs_mdcomp_disable
}

function cleanup
{
datasetexists $TESTPOOL/$TESTFS2 && \
log_must zfs destroy -r $TESTPOOL/$TESTFS2
datasetexists $TESTPOOL/recv && \
log_must zfs destroy -r $TESTPOOL/recv
[[ -f $keyfile ]] && log_must rm $keyfile
[[ -f $sendfile ]] && log_must rm $sendfile
}
log_onexit cleanup

log_assert "Verify 'zfs send -w' works with many different file layouts"

typeset keyfile=/$TESTPOOL/pkey
typeset sendfile=/$TESTPOOL/sendfile

log_must eval "echo 'password' > $keyfile"
log_must zfs create -o encryption=on -o keyformat=passphrase \
-o keylocation=file://$keyfile $TESTPOOL/$TESTFS2

log_must touch /$TESTPOOL/$TESTFS2/empty
log_must mkfile 512 /$TESTPOOL/$TESTFS2/small
log_must mkfile 32M /$TESTPOOL/$TESTFS2/full
log_must dd if=/dev/urandom of=/$TESTPOOL/$TESTFS2/sparse \
bs=512 count=1 seek=10G >/dev/null 2>&1
log_must mkfile 32M /$TESTPOOL/$TESTFS2/truncated
log_must truncate -s 4M /$TESTPOOL/$TESTFS2/truncated
sync

log_must set_metadata_compression_disabled 1
log_must dd if=/dev/urandom of=/$TESTPOOL/$TESTFS2/no_mdcomp \
count=1 bs=512 seek=10G >/dev/null 2>&1
sync
log_must set_metadata_compression_disabled 0

log_must mkdir -p /$TESTPOOL/$TESTFS2/dir
for i in {1..1000}; do
log_must mkfile 512 /$TESTPOOL/$TESTFS2/dir/file-$i
done
sync

for i in {1..1000}; do
log_must rm /$TESTPOOL/$TESTFS2/dir/file-$i
done
sync

log_must zfs snapshot $TESTPOOL/$TESTFS2@now
log_must eval "zfs send -wR $TESTPOOL/$TESTFS2@now > $sendfile"

log_must eval "zfs recv -F $TESTPOOL/recv < $sendfile"
log_must zfs load-key $TESTPOOL/recv

log_must zfs mount -a

log_pass "Verified 'zfs send -w' works with many different file layouts"
15 changes: 12 additions & 3 deletions usr/src/uts/common/fs/zfs/arc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1357,6 +1357,7 @@ hdr_full_cons(void *vbuf, void *unused, int kmflag)
arc_buf_hdr_t *hdr = vbuf;

bzero(hdr, HDR_FULL_SIZE);
hdr->b_l1hdr.b_byteswap = DMU_BSWAP_NUMFUNCS;
cv_init(&hdr->b_l1hdr.b_cv, NULL, CV_DEFAULT, NULL);
refcount_create(&hdr->b_l1hdr.b_refcnt);
mutex_init(&hdr->b_l1hdr.b_freeze_lock, NULL, MUTEX_DEFAULT, NULL);
Expand Down Expand Up @@ -3319,9 +3320,6 @@ arc_hdr_alloc_pabd(arc_buf_hdr_t *hdr, boolean_t alloc_rdata)
ASSERT(!HDR_SHARED_DATA(hdr) || alloc_rdata);
IMPLY(alloc_rdata, HDR_PROTECTED(hdr));

if (hdr->b_l1hdr.b_pabd == NULL && !HDR_HAS_RABD(hdr))
hdr->b_l1hdr.b_byteswap = DMU_BSWAP_NUMFUNCS;

if (alloc_rdata) {
size = HDR_GET_PSIZE(hdr);
ASSERT3P(hdr->b_crypt_hdr.b_rabd, ==, NULL);
Expand Down Expand Up @@ -6287,6 +6285,17 @@ arc_write_ready(zio_t *zio)
ASSERT3U(BP_GET_TYPE(zio->io_bp), !=, DMU_OT_INTENT_LOG);
ASSERT(HDR_PROTECTED(hdr));

if (BP_SHOULD_BYTESWAP(zio->io_bp)) {
if (BP_GET_LEVEL(zio->io_bp) > 0) {
hdr->b_l1hdr.b_byteswap = DMU_BSWAP_UINT64;
} else {
hdr->b_l1hdr.b_byteswap =
DMU_OT_BYTESWAP(BP_GET_TYPE(zio->io_bp));
}
} else {
hdr->b_l1hdr.b_byteswap = DMU_BSWAP_NUMFUNCS;
}

hdr->b_crypt_hdr.b_ot = BP_GET_TYPE(zio->io_bp);
hdr->b_crypt_hdr.b_dsobj = zio->io_bookmark.zb_objset;
zio_crypt_decode_params_bp(zio->io_bp, hdr->b_crypt_hdr.b_salt,
Expand Down
23 changes: 21 additions & 2 deletions usr/src/uts/common/fs/zfs/dmu.c
Original file line number Diff line number Diff line change
Expand Up @@ -915,7 +915,7 @@ dmu_free_long_object_impl(objset_t *os, uint64_t object, boolean_t raw)
err = dmu_object_dirty_raw(os, object, tx);
if (err == 0)
err = dmu_object_free(os, object, tx);

dmu_tx_commit(tx);
} else {
dmu_tx_abort(tx);
Expand Down Expand Up @@ -2035,6 +2035,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 (0);
}

void
dmu_object_set_checksum(objset_t *os, uint64_t object, uint8_t checksum,
dmu_tx_t *tx)
Expand Down Expand Up @@ -2220,8 +2237,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;
Expand Down
10 changes: 10 additions & 0 deletions usr/src/uts/common/fs/zfs/dmu_objset.c
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,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_incompatible_encryption_version(ds->ds_dir)) {
return (SET_ERROR(EROFS));
}

/* if we are decrypting, we can now check MACs in os->os_phys_buf */
Expand Down Expand Up @@ -2381,6 +2384,13 @@ dmu_objset_find(char *name, int func(const char *, void *), void *arg,
return (error);
}

boolean_t
dmu_objset_incompatible_encryption_version(objset_t *os)
{
return (dsl_dir_incompatible_encryption_version(
os->os_dsl_dataset->ds_dir));
}

void
dmu_objset_set_user(objset_t *os, void *user_ptr)
{
Expand Down
Loading

0 comments on commit 940e474

Please sign in to comment.