Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Encryption and Raw Send Stability Improvements #6864

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cmd/zfs/zfs_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}
Expand Down
20 changes: 20 additions & 0 deletions cmd/zpool/zpool_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -2121,6 +2121,15 @@ show_import(nvlist_t *config)
"updating.\n"));
break;

case ZPOOL_ERRATA_ZOL_6845_ENCRYPTION:
(void) printf(gettext(" action: Existing "
"encrypted datasets contain an on-disk "
"incompatibility, which\n\tneeds to be "
"corrected. Backup these datasets to new "
"encrypted datasets\n\tand destroy the "
"old ones.\n"));
break;

default:
/*
* All errata must contain an action message.
Expand Down Expand Up @@ -6499,6 +6508,17 @@ status_callback(zpool_handle_t *zhp, void *data)
"run 'zpool scrub'.\n"));
break;

case ZPOOL_ERRATA_ZOL_6845_ENCRYPTION:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make it perfectly clear to anyone who hits this that the dataset can mounted readonly. Here's my suggested wording.

diff --git a/cmd/zpool/zpool_main.c b/cmd/zpool/zpool_main.c
index 14507b3..5d18ffe 100644
--- a/cmd/zpool/zpool_main.c
+++ b/cmd/zpool/zpool_main.c
@@ -6509,9 +6509,14 @@ status_callback(zpool_handle_t *zhp, void *data)
                        break;
 
                case ZPOOL_ERRATA_ZOL_6845_ENCRYPTION:
+                       (void) printf(gettext("\tExisting encrypted datasets "
+                           "contain an on-disk incompatibility\n\twhich "
+                           "needs to be corrected.\n"));
                        (void) printf(gettext("action: To correct the issue "
                            "backup existing encrypted datasets to new\n\t"
-                           "encrypted datasets and destroy the old ones.\n"));
+                           "encrypted datasets and destroy the old ones. "
+                           "'zfs mount -o ro' can\n\tbe used to temporarily "
+                           "mount existing encrypted datasets readonly.\n")); 
                        break;
 
                default:

Copy link
Contributor

@trisk trisk Jan 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also specify that a non-raw send from the old dataset can be used to convert it to the new format. Actually, calling it a "backup" might be confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@trisk
The errata page (which gets linked in zpool status) explains exactly what can and can't be done with datasets in this state and how best to deal with them. I'm not sure we should try to explain all of that here in the quick status blurb.

Actually, calling it a "backup" might be confusing.

How so?

(void) printf(gettext("\tExisting encrypted datasets "
"contain an on-disk incompatibility\n\twhich "
"needs to be corrected.\n"));
(void) printf(gettext("action: To correct the issue "
"backup existing encrypted datasets to new\n\t"
"encrypted datasets and destroy the old ones. "
"'zfs mount -o ro' can\n\tbe used to temporarily "
"mount existing encrypted datasets readonly.\n"));
break;

default:
/*
* All errata which allow the pool to be imported
Expand Down
6 changes: 5 additions & 1 deletion cmd/zstreamdump/zstreamdump.c
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,8 @@ main(int argc, char *argv[])
drro->drr_raw_bonuslen =
BSWAP_32(drro->drr_raw_bonuslen);
drro->drr_toguid = BSWAP_64(drro->drr_toguid);
drro->drr_maxblkid =
BSWAP_64(drro->drr_maxblkid);
}

payload_size = DRR_OBJECT_PAYLOAD_SIZE(drro);
Expand All @@ -451,7 +453,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,
Expand All @@ -461,6 +464,7 @@ main(int argc, char *argv[])
drro->drr_dn_slots,
drro->drr_raw_bonuslen,
drro->drr_flags,
(u_longlong_t)drro->drr_maxblkid,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't you need to byteswap this above?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch. will fix.

drro->drr_indblkshift,
drro->drr_nlevels,
drro->drr_nblkptr);
Expand Down
25 changes: 24 additions & 1 deletion cmd/ztest/ztest.c
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,8 @@ extern uint64_t metaslab_df_alloc_threshold;
extern unsigned long zfs_deadman_synctime_ms;
extern int metaslab_preload_limit;
extern boolean_t zfs_compressed_arc_enabled;
extern int zfs_abd_scatter_enabled;
extern int zfs_abd_scatter_enabled;
extern int dmu_object_alloc_chunk_shift;

static ztest_shared_opts_t *ztest_shared_opts;
static ztest_shared_opts_t ztest_opts;
Expand Down Expand Up @@ -314,6 +315,7 @@ static ztest_shared_callstate_t *ztest_shared_callstate;
ztest_func_t ztest_dmu_read_write;
ztest_func_t ztest_dmu_write_parallel;
ztest_func_t ztest_dmu_object_alloc_free;
ztest_func_t ztest_dmu_object_next_chunk;
ztest_func_t ztest_dmu_commit_callbacks;
ztest_func_t ztest_zap;
ztest_func_t ztest_zap_parallel;
Expand Down Expand Up @@ -361,6 +363,7 @@ ztest_info_t ztest_info[] = {
ZTI_INIT(ztest_dmu_read_write, 1, &zopt_always),
ZTI_INIT(ztest_dmu_write_parallel, 10, &zopt_always),
ZTI_INIT(ztest_dmu_object_alloc_free, 1, &zopt_always),
ZTI_INIT(ztest_dmu_object_next_chunk, 1, &zopt_sometimes),
ZTI_INIT(ztest_dmu_commit_callbacks, 1, &zopt_always),
ZTI_INIT(ztest_zap, 30, &zopt_always),
ZTI_INIT(ztest_zap_parallel, 100, &zopt_always),
Expand Down Expand Up @@ -4055,6 +4058,26 @@ ztest_dmu_object_alloc_free(ztest_ds_t *zd, uint64_t id)
umem_free(od, size);
}

/*
* Rewind the global allocator to verify object allocation backfilling.
*/
void
ztest_dmu_object_next_chunk(ztest_ds_t *zd, uint64_t id)
{
objset_t *os = zd->zd_os;
int dnodes_per_chunk = 1 << dmu_object_alloc_chunk_shift;
uint64_t object;

/*
* Rewind the global allocator randomly back to a lower object number
* to force backfilling and reclamation of recently freed dnodes.
*/
mutex_enter(&os->os_obj_lock);
object = ztest_random(os->os_obj_next_chunk);
os->os_obj_next_chunk = P2ALIGN(object, dnodes_per_chunk);
mutex_exit(&os->os_obj_lock);
}

#undef OD_ARRAY_SIZE
#define OD_ARRAY_SIZE 2

Expand Down
52 changes: 52 additions & 0 deletions contrib/dracut/90zfs/zfs-load-key.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
#!/bin/bash

# This script only gets executed on systemd systems, see mount-zfs.sh for non-systemd systems

# import the libs now that we know the pool imported
[ -f /lib/dracut-lib.sh ] && dracutlib=/lib/dracut-lib.sh
[ -f /usr/lib/dracut/modules.d/99base/dracut-lib.sh ] && dracutlib=/usr/lib/dracut/modules.d/99base/dracut-lib.sh
. "$dracutlib"

# load the kernel command line vars
[ -z "$root" ] && root=$(getarg root=)
# If root is not ZFS= or zfs: or rootfstype is not zfs then we are not supposed to handle it.
[ "${root##zfs:}" = "${root}" -a "${root##ZFS=}" = "${root}" -a "$rootfstype" != "zfs" ] && exit 0

# There is a race between the zpool import and the pre-mount hooks, so we wait for a pool to be imported
while true; do
zpool list -H | grep -q -v '^$' && break
[[ $(systemctl is-failed zfs-import-cache.service) == 'failed' ]] && exit 1
[[ $(systemctl is-failed zfs-import-scan.service) == 'failed' ]] && exit 1
sleep 0.1s
done

# run this after import as zfs-import-cache/scan service is confirmed good
if [[ "${root}" = "zfs:AUTO" ]] ; then
root=$(zpool list -H -o bootfs | awk '$1 != "-" {print; exit}')
else
root="${root##zfs:}"
root="${root##ZFS=}"
fi

# if pool encryption is active and the zfs command understands '-o encryption'
if [[ $(zpool list -H -o feature@encryption $(echo "${root}" | awk -F\/ '{print $1}')) == 'active' ]]; then
# check if root dataset has encryption enabled
if $(zfs list -H -o encryption "${root}" | grep -q -v off); then
# figure out where the root dataset has its key, the keylocation should not be none
while true; do
if [[ $(zfs list -H -o keylocation "${root}") == 'none' ]]; then
root=$(echo -n "${root}" | awk 'BEGIN{FS=OFS="/"}{NF--; print}')
[[ "${root}" == '' ]] && exit 1
else
break
fi
done
# decrypt them
TRY_COUNT=5
while [ $TRY_COUNT != 0 ]; do
zfs load-key "$root" <<< $(systemd-ask-password "Encrypted ZFS password for ${root}: ")
[[ $? == 0 ]] && break
((TRY_COUNT-=1))
done
fi
fi
7 changes: 7 additions & 0 deletions include/sys/dmu.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions include/sys/dmu_objset.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_incompatible_encryption_version(objset_t *os);

int dmu_fsname(const char *snapname, char *buf);

Expand Down
14 changes: 14 additions & 0 deletions include/sys/dnode.h
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,13 @@ typedef struct dnode_phys {
uint64_t dn_maxblkid; /* largest allocated block ID */
uint64_t dn_used; /* bytes (or sectors) of disk space */

/*
* Both dn_pad2 and dn_pad3 are protected by the block's MAC. This
* allows us to protect any fields that might be added here in the
* future. In either case, developers will want to check
* zio_crypt_init_uios_dnode() to ensure the new field is being
* protected properly.
*/
uint64_t dn_pad3[4];

/*
Expand Down Expand Up @@ -301,6 +308,7 @@ struct dnode {
uint8_t dn_rm_spillblk[TXG_SIZE]; /* for removing spill blk */
uint16_t dn_next_bonuslen[TXG_SIZE];
uint32_t dn_next_blksz[TXG_SIZE]; /* next block size in bytes */
uint64_t dn_next_maxblkid[TXG_SIZE]; /* next maxblkid in bytes */

/* protected by dn_dbufs_mtx; declared here to fill 32-bit hole */
uint32_t dn_dbufs_count; /* count of dn_dbufs */
Expand Down Expand Up @@ -416,6 +424,7 @@ int dnode_next_offset(dnode_t *dn, int flags, uint64_t *off,
int minlvl, uint64_t blkfill, uint64_t txg);
void dnode_evict_dbufs(dnode_t *dn);
void dnode_evict_bonus(dnode_t *dn);
void dnode_free_interior_slots(dnode_t *dn);

#define DNODE_IS_CACHEABLE(_dn) \
((_dn)->dn_objset->os_primary_cache == ZFS_CACHE_ALL || \
Expand Down Expand Up @@ -509,6 +518,11 @@ typedef struct dnode_stats {
* which had already been unlinked in an earlier txg.
*/
kstat_named_t dnode_hold_free_txg;
/*
* Number of times dnode_free_interior_slots() needed to retry
* acquiring a slot zrl lock due to contention.
*/
kstat_named_t dnode_free_interior_lock_retry;
/*
* Number of new dnodes allocated by dnode_allocate().
*/
Expand Down
3 changes: 2 additions & 1 deletion include/sys/dsl_crypt.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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_incompatible_encryption_version(dsl_dir_t *dd);

void spa_keystore_init(spa_keystore_t *sk);
void spa_keystore_fini(spa_keystore_t *sk);
Expand Down
1 change: 1 addition & 0 deletions include/sys/fs/zfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -891,6 +891,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;

/*
Expand Down
4 changes: 3 additions & 1 deletion include/sys/zfs_ioctl.h
Original file line number Diff line number Diff line change
Expand Up @@ -219,10 +219,12 @@ 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 */
uint8_t drr_indblkshift;
uint8_t drr_nlevels;
uint8_t drr_nblkptr;
uint8_t drr_pad[5];
uint64_t drr_maxblkid;
/* bonus content follows */
} drr_object;
struct drr_freeobjects {
Expand Down
11 changes: 8 additions & 3 deletions include/sys/zio_crypt.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down
19 changes: 11 additions & 8 deletions lib/libzfs/libzfs_crypto.c
Original file line number Diff line number Diff line change
Expand Up @@ -1054,7 +1054,7 @@ zfs_crypto_load_key(zfs_handle_t *zhp, boolean_t noop, char *alt_keylocation)
}

try_again:
/* fetching and deriving the key are correctible errors. set the flag */
/* fetching and deriving the key are correctable errors. set the flag */
correctible = B_TRUE;

/* get key material from key format and location */
Expand Down Expand Up @@ -1110,22 +1110,25 @@ zfs_crypto_load_key(zfs_handle_t *zhp, boolean_t noop, char *alt_keylocation)

error:
zfs_error(zhp->zfs_hdl, EZFS_CRYPTOFAILED, errbuf);
if (key_material != NULL)
if (key_material != NULL) {
free(key_material);
if (key_data != NULL)
key_material = NULL;
}
if (key_data != NULL) {
free(key_data);
key_data = NULL;
}

/*
* Here we decide if it is ok to allow the user to retry entering their
* key. The can_retry flag will be set if the user is entering their
* key from an interactive prompt. The correctible flag will only be
* set if an error that occured could be corrected by retrying. Both
* key from an interactive prompt. The correctable flag will only be
* set if an error that occurred could be corrected by retrying. Both
* flags are needed to allow the user to attempt key entry again
*/
if (can_retry && correctible && attempts <= MAX_KEY_PROMPT_ATTEMPTS) {
attempts++;
attempts++;
if (can_retry && correctible && attempts < MAX_KEY_PROMPT_ATTEMPTS)
goto try_again;
}

return (ret);
}
Expand Down
18 changes: 9 additions & 9 deletions lib/libzfs/libzfs_status.c
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,15 @@ check_status(nvlist_t *config, boolean_t isimport, zpool_errata_t *erratap)
if (find_vdev_problem(nvroot, vdev_removed))
return (ZPOOL_STATUS_REMOVED_DEV);

/*
* Informational errata available.
*/
(void) nvlist_lookup_uint64(config, ZPOOL_CONFIG_ERRATA, &errata);
if (errata) {
*erratap = errata;
return (ZPOOL_STATUS_ERRATA);
}

/*
* Outdated, but usable, version
*/
Expand Down Expand Up @@ -382,15 +391,6 @@ check_status(nvlist_t *config, boolean_t isimport, zpool_errata_t *erratap)
}
}

/*
* Informational errata available.
*/
(void) nvlist_lookup_uint64(config, ZPOOL_CONFIG_ERRATA, &errata);
if (errata) {
*erratap = errata;
return (ZPOOL_STATUS_ERRATA);
}

return (ZPOOL_STATUS_OK);
}

Expand Down
Loading