Archive-Team
/
zfs
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 103 Wiki Activity

Allow unencrypted children of encrypted datasets 

When encryption was first added to ZFS, we made a decision to
prevent users from creating unencrypted children of encrypted
datasets. The idea was to prevent users from inadvertently
leaving some of their data unencrypted. However, since the
release of 0.8.0, some legitimate reasons have been brought up
for this behavior to be allowed. This patch simply removes this
limitation from all code paths that had checks for it and updates
the tests accordingly.

Reviewed-by: Jason King <jason.king@joyent.com>
Reviewed-by: Sean Eric Fagan <sef@ixsystems.com>
Reviewed-by: Richard Laager <rlaager@wiktel.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tom Caputi <tcaputi@datto.com>
Closes #8737 
Closes #8870
This commit is contained in:
Tom Caputi 2019-06-20 15:29:51 -04:00 committed by Brian Behlendorf
parent 84b4201f32
commit da68988708
10 changed files with 63 additions and 163 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
include/sys/dsl_crypt.h
View File

@ -209,7 +209,6 @@ void dsl_dataset_create_crypt_sync(uint64_t dsobj, dsl_dir_t *dd,
struct dsl_dataset *origin, dsl_crypto_params_t *dcp, dmu_tx_t *tx);
uint64_t dsl_crypto_key_create_sync(uint64_t crypt, dsl_wrapping_key_t *wkey,
dmu_tx_t *tx);
int dmu_objset_clone_crypt_check(dsl_dir_t *parentdd, dsl_dir_t *origindd);
uint64_t dsl_crypto_key_clone_sync(dsl_dir_t *origindd, dmu_tx_t *tx);
void dsl_crypto_key_destroy_sync(uint64_t dckobj, dmu_tx_t *tx);

41
lib/libzfs/libzfs_crypto.c
View File

@ -740,14 +740,6 @@ zfs_crypto_create(libzfs_handle_t *hdl, char *parent_name, nvlist_t *props,
pcrypt = ZIO_CRYPT_OFF;
}
/* Check for encryption being explicitly truned off */
if (crypt == ZIO_CRYPT_OFF && pcrypt != ZIO_CRYPT_OFF) {
ret = EINVAL;
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
"Invalid encryption value. Dataset must be encrypted."));
goto out;
}
/* Get the inherited encryption property if we don't have it locally */
if (!local_crypt)
crypt = pcrypt;
@ -849,10 +841,7 @@ int
zfs_crypto_clone_check(libzfs_handle_t *hdl, zfs_handle_t *origin_zhp,
char *parent_name, nvlist_t *props)
{
int ret;
char errbuf[1024];
zfs_handle_t *pzhp = NULL;
uint64_t pcrypt, ocrypt;
(void) snprintf(errbuf, sizeof (errbuf),
dgettext(TEXT_DOMAIN, "Encryption clone error"));
@ -865,40 +854,12 @@ zfs_crypto_clone_check(libzfs_handle_t *hdl, zfs_handle_t *origin_zhp,
nvlist_exists(props, zfs_prop_to_name(ZFS_PROP_KEYLOCATION)) ||
nvlist_exists(props, zfs_prop_to_name(ZFS_PROP_ENCRYPTION)) ||
nvlist_exists(props, zfs_prop_to_name(ZFS_PROP_PBKDF2_ITERS))) {
ret = EINVAL;
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
"Encryption properties must inherit from origin dataset."));
goto out;
return (EINVAL);
}
/* get a reference to parent dataset, should never be NULL */
pzhp = make_dataset_handle(hdl, parent_name);
if (pzhp == NULL) {
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
"Failed to lookup parent."));
return (ENOENT);
}
/* Lookup parent's crypt */
pcrypt = zfs_prop_get_int(pzhp, ZFS_PROP_ENCRYPTION);
ocrypt = zfs_prop_get_int(origin_zhp, ZFS_PROP_ENCRYPTION);
/* all children of encrypted parents must be encrypted */
if (pcrypt != ZIO_CRYPT_OFF && ocrypt == ZIO_CRYPT_OFF) {
ret = EINVAL;
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
"Cannot create unencrypted clone as a child "
"of encrypted parent."));
goto out;
}
zfs_close(pzhp);
return (0);
out:
if (pzhp != NULL)
zfs_close(pzhp);
return (ret);
}
typedef struct loadkeys_cbdata {

13
lib/libzfs/libzfs_dataset.c
View File

@ -4675,16 +4675,9 @@ zfs_rename(zfs_handle_t *zhp, const char *target, boolean_t recursive,
"with the new name"));
(void) zfs_error(hdl, EZFS_EXISTS, errbuf);
} else if (errno == EACCES) {
if (zfs_prop_get_int(zhp, ZFS_PROP_ENCRYPTION) ==
ZIO_CRYPT_OFF) {
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
"cannot rename an unencrypted dataset to "
"be a decendent of an encrypted one"));
} else {
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
"cannot move encryption child outside of "
"its encryption root"));
}
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
"cannot move encrypted child outside of "
"its encryption root"));
(void) zfs_error(hdl, EZFS_CRYPTOFAILED, errbuf);
} else {
(void) zfs_standard_error(zhp->zfs_hdl, errno, errbuf);

48
lib/libzfs/libzfs_sendrecv.c
View File

@ -3425,7 +3425,7 @@ recv_fix_encryption_hierarchy(libzfs_handle_t *hdl, const char *destname,
is_clone = zhp->zfs_dmustats.dds_origin[0] != '\0';
(void) zfs_crypto_get_encryption_root(zhp, &is_encroot, NULL);
/* we don't need to do anything for unencrypted filesystems */
/* we don't need to do anything for unencrypted datasets */
if (crypt == ZIO_CRYPT_OFF) {
zfs_close(zhp);
continue;
@ -4813,34 +4813,6 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const char *tosnap,
goto out;
}
/*
* It is invalid to receive a properties stream that was
* unencrypted on the send side as a child of an encrypted
* parent. Technically there is nothing preventing this, but
* it would mean that the encryption=off property which is
* locally set on the send side would not be received correctly.
* We can infer encryption=off if the stream is not raw and
* properties were included since the send side will only ever
* send the encryption property in a raw nvlist header. This
* check will be avoided if the user specifically overrides
* the encryption property on the command line.
*/
if (!raw && rcvprops != NULL &&
!nvlist_exists(cmdprops,
zfs_prop_to_name(ZFS_PROP_ENCRYPTION))) {
uint64_t crypt;
crypt = zfs_prop_get_int(zhp, ZFS_PROP_ENCRYPTION);
if (crypt != ZIO_CRYPT_OFF) {
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
"parent '%s' must not be encrypted to "
"receive unenecrypted property"), name);
err = zfs_error(hdl, EZFS_BADPROP, errbuf);
zfs_close(zhp);
goto out;
}
}
zfs_close(zhp);
newfs = B_TRUE;
@ -4877,6 +4849,24 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const char *tosnap,
&oxprops, &wkeydata, &wkeylen, errbuf)) != 0)
goto out;
/*
* When sending with properties (zfs send -p), the encryption property
* is not included because it is a SETONCE property and therefore
* treated as read only. However, we are always able to determine its
* value because raw sends will include it in the DRR_BDEGIN payload
* and non-raw sends with properties are not allowed for encrypted
* datasets. Therefore, if this is a non-raw properties stream, we can
* infer that the value should be ZIO_CRYPT_OFF and manually add that
* to the received properties.
*/
if (stream_wantsnewfs && !raw && rcvprops != NULL &&
!nvlist_exists(cmdprops, zfs_prop_to_name(ZFS_PROP_ENCRYPTION))) {
if (oxprops == NULL)
oxprops = fnvlist_alloc();
fnvlist_add_uint64(oxprops,
zfs_prop_to_name(ZFS_PROP_ENCRYPTION), ZIO_CRYPT_OFF);
}
err = ioctl_err = lzc_receive_with_cmdprops(destsnap, rcvprops,
oxprops, wkeydata, wkeylen, origin, flags->force, flags->resumable,
raw, infd, drr_noswap, cleanup_fd, &read_bytes, &errflags,

7
module/zfs/dmu_objset.c
View File

@ -1349,13 +1349,6 @@ dmu_objset_clone_check(void *arg, dmu_tx_t *tx)
return (SET_ERROR(EINVAL));
}
error = dmu_objset_clone_crypt_check(pdd, origin->ds_dir);
if (error != 0) {
dsl_dataset_rele(origin, FTAG);
dsl_dir_rele(pdd, FTAG);
return (error);
}
dsl_dataset_rele(origin, FTAG);
dsl_dir_rele(pdd, FTAG);

24
module/zfs/dmu_recv.c
View File

@ -624,7 +624,7 @@ dmu_recv_begin_check(void *arg, dmu_tx_t *tx)
/* Open the parent of tofs */
ASSERT3U(strlen(tofs), <, sizeof (buf));
(void) strlcpy(buf, tofs, strrchr(tofs, '/') - tofs + 1);
error = dsl_dataset_hold_flags(dp, buf, dsflags, FTAG, &ds);
error = dsl_dataset_hold(dp, buf, FTAG, &ds);
if (error != 0)
return (error);
@ -642,13 +642,13 @@ dmu_recv_begin_check(void *arg, dmu_tx_t *tx)
error = dmu_objset_create_crypt_check(ds->ds_dir,
drba->drba_dcp, &will_encrypt);
if (error != 0) {
dsl_dataset_rele_flags(ds, dsflags, FTAG);
dsl_dataset_rele(ds, FTAG);
return (error);
}
if (will_encrypt &&
(featureflags & DMU_BACKUP_FEATURE_EMBED_DATA)) {
dsl_dataset_rele_flags(ds, dsflags, FTAG);
dsl_dataset_rele(ds, FTAG);
return (SET_ERROR(EINVAL));
}
}
@ -661,25 +661,25 @@ dmu_recv_begin_check(void *arg, dmu_tx_t *tx)
error = dsl_fs_ss_limit_check(ds->ds_dir, 1,
ZFS_PROP_FILESYSTEM_LIMIT, NULL, drba->drba_cred);
if (error != 0) {
dsl_dataset_rele_flags(ds, dsflags, FTAG);
dsl_dataset_rele(ds, FTAG);
return (error);
}
error = dsl_fs_ss_limit_check(ds->ds_dir, 1,
ZFS_PROP_SNAPSHOT_LIMIT, NULL, drba->drba_cred);
if (error != 0) {
dsl_dataset_rele_flags(ds, dsflags, FTAG);
dsl_dataset_rele(ds, FTAG);
return (error);
}
/* can't recv below anything but filesystems (eg. no ZVOLs) */
error = dmu_objset_from_ds(ds, &os);
if (error != 0) {
dsl_dataset_rele_flags(ds, dsflags, FTAG);
dsl_dataset_rele(ds, FTAG);
return (error);
}
if (dmu_objset_type(os) != DMU_OST_ZFS) {
dsl_dataset_rele_flags(ds, dsflags, FTAG);
dsl_dataset_rele(ds, FTAG);
return (SET_ERROR(ZFS_ERR_WRONG_PARENT));
}
@ -688,25 +688,25 @@ dmu_recv_begin_check(void *arg, dmu_tx_t *tx)
error = dsl_dataset_hold_flags(dp, drba->drba_origin,
dsflags, FTAG, &origin);
if (error != 0) {
dsl_dataset_rele_flags(ds, dsflags, FTAG);
dsl_dataset_rele(ds, FTAG);
return (error);
}
if (!origin->ds_is_snapshot) {
dsl_dataset_rele_flags(origin, dsflags, FTAG);
dsl_dataset_rele_flags(ds, dsflags, FTAG);
dsl_dataset_rele(ds, FTAG);
return (SET_ERROR(EINVAL));
}
if (dsl_dataset_phys(origin)->ds_guid != fromguid &&
fromguid != 0) {
dsl_dataset_rele_flags(origin, dsflags, FTAG);
dsl_dataset_rele_flags(ds, dsflags, FTAG);
dsl_dataset_rele(ds, FTAG);
return (SET_ERROR(ENODEV));
}
if (origin->ds_dir->dd_crypto_obj != 0 &&
(featureflags & DMU_BACKUP_FEATURE_EMBED_DATA)) {
dsl_dataset_rele_flags(origin, dsflags, FTAG);
dsl_dataset_rele_flags(ds, dsflags, FTAG);
dsl_dataset_rele(ds, FTAG);
return (SET_ERROR(EINVAL));
}
@ -729,7 +729,7 @@ dmu_recv_begin_check(void *arg, dmu_tx_t *tx)
dsl_dataset_rele_flags(origin, dsflags, FTAG);
}
dsl_dataset_rele_flags(ds, dsflags, FTAG);
dsl_dataset_rele(ds, FTAG);
error = 0;
}
return (error);

44
module/zfs/dsl_crypt.c
View File

@ -1610,15 +1610,8 @@ dsl_dir_rename_crypt_check(dsl_dir_t *dd, dsl_dir_t *newparent)
int ret;
uint64_t curr_rddobj, parent_rddobj;
if (dd->dd_crypto_obj == 0) {
/* children of encrypted parents must be encrypted */
if (newparent->dd_crypto_obj != 0) {
ret = SET_ERROR(EACCES);
goto error;
}
if (dd->dd_crypto_obj == 0)
return (0);
}
ret = dsl_dir_get_encryption_root_ddobj(dd, &curr_rddobj);
if (ret != 0)
@ -1747,34 +1740,6 @@ dsl_dataset_promote_crypt_sync(dsl_dir_t *target, dsl_dir_t *origin,
kmem_free(keylocation, ZAP_MAXVALUELEN);
}
int
dmu_objset_clone_crypt_check(dsl_dir_t *parentdd, dsl_dir_t *origindd)
{
int ret;
uint64_t pcrypt, crypt;
/*
* Check that we are not making an unencrypted child of an
* encrypted parent.
*/
ret = dsl_dir_get_crypt(parentdd, &pcrypt);
if (ret != 0)
return (ret);
ret = dsl_dir_get_crypt(origindd, &crypt);
if (ret != 0)
return (ret);
ASSERT3U(pcrypt, !=, ZIO_CRYPT_INHERIT);
ASSERT3U(crypt, !=, ZIO_CRYPT_INHERIT);
if (crypt == ZIO_CRYPT_OFF && pcrypt != ZIO_CRYPT_OFF)
return (SET_ERROR(EINVAL));
return (0);
}
int
dmu_objset_create_crypt_check(dsl_dir_t *parentdd, dsl_crypto_params_t *dcp,
boolean_t *will_encrypt)
@ -1805,13 +1770,6 @@ dmu_objset_create_crypt_check(dsl_dir_t *parentdd, dsl_crypto_params_t *dcp,
ASSERT3U(pcrypt, !=, ZIO_CRYPT_INHERIT);
ASSERT3U(crypt, !=, ZIO_CRYPT_INHERIT);
/*
* We can't create an unencrypted child of an encrypted parent
* under any circumstances.
*/
if (crypt == ZIO_CRYPT_OFF && pcrypt != ZIO_CRYPT_OFF)
return (SET_ERROR(EINVAL));
/* check for valid dcp with no encryption (inherited or local) */
if (crypt == ZIO_CRYPT_OFF) {
/* Must not specify encryption params */

20
tests/zfs-tests/tests/functional/cli_root/zfs_create/zfs_create_encrypted.ksh
View File

@ -51,10 +51,10 @@
# yes unspec 0 1 no no keyformat specified
# yes unspec 1 0 yes new encryption root, crypt inherited
# yes unspec 1 1 yes new encryption root, crypt inherited
# yes off 0 0 no unencrypted child of encrypted parent
# yes off 0 1 no unencrypted child of encrypted parent
# yes off 1 0 no unencrypted child of encrypted parent
# yes off 1 1 no unencrypted child of encrypted parent
# yes off 0 0 yes unencrypted child of encrypted parent
# yes off 0 1 no keylocation given, but crypt off
# yes off 1 0 no keyformat given, but crypt off
# yes off 1 1 no keyformat given, but crypt off
# yes on 0 0 yes inherited encryption, local crypt
# yes on 0 1 no no keyformat specified for new key
# yes on 1 0 yes new encryption root
@ -113,7 +113,9 @@ log_must eval "echo $PASSPHRASE | zfs create -o keyformat=passphrase" \
log_must eval "echo $PASSPHRASE | zfs create -o keyformat=passphrase" \
"-o keylocation=prompt $TESTPOOL/$TESTFS2/c4"
log_mustnot zfs create -o encryption=off $TESTPOOL/$TESTFS2/c5
log_must zfs create -o encryption=off $TESTPOOL/$TESTFS2/c5
log_must test "$(get_prop 'encryption' $TESTPOOL/$TESTFS2/c5)" == "off"
log_mustnot zfs create -o encryption=off -o keylocation=prompt \
$TESTPOOL/$TESTFS2/c5
log_mustnot zfs create -o encryption=off -o keyformat=passphrase \
@ -122,13 +124,13 @@ log_mustnot zfs create -o encryption=off -o keyformat=passphrase \
-o keylocation=prompt $TESTPOOL/$TESTFS2/c5
log_must eval "echo $PASSPHRASE | zfs create -o encryption=on" \
"$TESTPOOL/$TESTFS2/c5"
"$TESTPOOL/$TESTFS2/c6"
log_mustnot zfs create -o encryption=on -o keylocation=prompt \
$TESTPOOL/$TESTFS2/c6
$TESTPOOL/$TESTFS2/c7
log_must eval "echo $PASSPHRASE | zfs create -o encryption=on" \
"-o keyformat=passphrase $TESTPOOL/$TESTFS2/c6"
"-o keyformat=passphrase $TESTPOOL/$TESTFS2/c7"
log_must eval "echo $PASSPHRASE | zfs create -o encryption=on" \
"-o keyformat=passphrase -o keylocation=prompt $TESTPOOL/$TESTFS2/c7"
"-o keyformat=passphrase -o keylocation=prompt $TESTPOOL/$TESTFS2/c8"
log_pass "ZFS creates datasets only if they have a valid combination of" \
"encryption properties set."

14
tests/zfs-tests/tests/functional/cli_root/zfs_receive/zfs_receive_to_encrypted.ksh
View File

@ -46,7 +46,7 @@ function cleanup
log_onexit cleanup
log_assert "ZFS should receive to an encrypted child dataset"
log_assert "ZFS should receive encrypted filesystems into child dataset"
typeset passphrase="password"
typeset snap="$TESTPOOL/$TESTFS@snap"
@ -60,11 +60,13 @@ log_must eval "echo $passphrase | zfs create -o encryption=on" \
log_note "Verifying ZFS will receive to an encrypted child"
log_must eval "zfs send $snap | zfs receive $TESTPOOL/$TESTFS1/c1"
log_note "Verifying 'send -p' will not receive to an encrypted child"
log_mustnot eval "zfs send -p $snap | zfs receive $TESTPOOL/$TESTFS1/c2"
log_note "Verifying 'send -p' will receive to an encrypted child"
log_must eval "zfs send -p $snap | zfs receive $TESTPOOL/$TESTFS1/c2"
log_must test "$(get_prop 'encryption' $TESTPOOL/$TESTFS1/c2)" == "off"
log_note "Verifying 'send -R' will not receive to an encrypted child"
log_mustnot eval "zfs send -R $snap | zfs receive $TESTPOOL/$TESTFS1/c3"
log_note "Verifying 'send -R' will receive to an encrypted child"
log_must eval "zfs send -R $snap | zfs receive $TESTPOOL/$TESTFS1/c3"
log_must test "$(get_prop 'encryption' $TESTPOOL/$TESTFS1/c3)" == "off"
log_note "Verifying ZFS will not receive to an encrypted child when the" \
"parent key is unloaded"
@ -72,4 +74,4 @@ log_must zfs unmount $TESTPOOL/$TESTFS1
log_must zfs unload-key $TESTPOOL/$TESTFS1
log_mustnot eval "zfs send $snap | zfs receive $TESTPOOL/$TESTFS1/c4"
log_pass "ZFS can receive to an encrypted child dataset"
log_pass "ZFS can receive encrypted filesystems into child dataset"

14
tests/zfs-tests/tests/functional/cli_root/zfs_rename/zfs_rename_to_encrypted.ksh
View File

@ -23,12 +23,13 @@
#
# DESCRIPTION:
# 'zfs rename' should not rename an unencrypted dataset to a child
# 'zfs rename' should be able to move an unencrypted dataset to a child
# of an encrypted dataset
#
# STRATEGY:
# 1. Create an encrypted dataset
# 2. Attempt to rename the default dataset to a child of the encrypted dataset
# 2. Rename the default dataset to a child of the encrypted dataset
# 3. Confirm the child dataset doesn't have any encryption properties
#
verify_runnable "both"
@ -36,16 +37,17 @@ verify_runnable "both"
function cleanup
{
datasetexists $TESTPOOL/$TESTFS2 && \
log_must zfs destroy $TESTPOOL/$TESTFS2
log_must zfs destroy -r $TESTPOOL/$TESTFS2
}
log_onexit cleanup
log_assert "'zfs rename' should not rename an unencrypted dataset to a" \
log_assert "'zfs rename' should allow renaming an unencrypted dataset to a" \
"child of an encrypted dataset"
log_must eval "echo $PASSPHRASE | zfs create -o encryption=on" \
"-o keyformat=passphrase -o keylocation=prompt $TESTPOOL/$TESTFS2"
log_mustnot zfs rename $TESTPOOL/$TESTFS $TESTPOOL/$TESTFS2/$TESTFS
log_must zfs rename $TESTPOOL/$TESTFS $TESTPOOL/$TESTFS2/$TESTFS
log_must test "$(get_prop 'encryption' $TESTPOOL/$TESTFS2/$TESTFS)" == "off"
log_pass "'zfs rename' does not rename an unencrypted dataset to a child" \
log_pass "'zfs rename' allows renaming an unencrypted dataset to a child" \
"of an encrypted dataset"