Skip to content

Commit

Permalink
Better handling for future crypto parameters
Browse files Browse the repository at this point in the history
The intent is that this is like ENOTSUP, but specifically for when
something can't be done because we have no support for the requested
crypto parameters; eg unlocking a dataset or receiving a stream
encrypted with a suite we don't support.

Its not intended to be recoverable without upgrading ZFS itself.
If the request could be made to work by enabling a feature or modifying
some other configuration item, then some other code should be used.

load-key: In the future we might have more crypto suites (ie new values
for the `encryption` property. Right now trying to load a key on such
a future crypto suite will look up suite parameters off the end of the
crypto table, resulting in misbehaviour and/or crashes (or, with debug
enabled, trip the assertion in `zio_crypt_key_unwrap`).

Instead, lets check the value we got from the dataset, and if we can't
handle it, abort early.

recv: When receiving a raw stream encrypted with an unknown crypto
suite, `zfs recv` would report a generic `invalid backup stream`
(EINVAL). While technically correct, its not super helpful, so lets
ship a more specific error code and message.

Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Signed-off-by: Rob Norris <robn@despairlabs.com>
Closes openzfs#14577
  • Loading branch information
robn authored and lundman committed Mar 16, 2023
1 parent 1573da1 commit 61286f8
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 3 deletions.
3 changes: 2 additions & 1 deletion cmd/zdb/zdb.c
Original file line number Diff line number Diff line change
Expand Up @@ -3120,7 +3120,8 @@ zdb_load_key(objset_t *os)
if (err != 0)
fatal(
"couldn't load encryption key for %s: %s",
encroot, strerror(err));
encroot, err == ZFS_ERR_CRYPTO_NOTSUP ?
"crypto params not supported" : strerror(err));

ASSERT3U(dsl_dataset_get_keystatus(dd), ==, ZFS_KEYSTATUS_AVAILABLE);

Expand Down
1 change: 1 addition & 0 deletions contrib/pyzfs/libzfs_core/_constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ def enum(*sequential, **named):
'ZFS_ERR_VDEV_NOTSUP',
'ZFS_ERR_NOT_USER_NAMESPACE',
'ZFS_ERR_RESUME_EXISTS',
'ZFS_ERR_CRYPTO_NOTSUP',
],
{}
)
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 @@ -1569,6 +1569,7 @@ typedef enum {
ZFS_ERR_VDEV_NOTSUP,
ZFS_ERR_NOT_USER_NAMESPACE,
ZFS_ERR_RESUME_EXISTS,
ZFS_ERR_CRYPTO_NOTSUP,
} zfs_errno_t;

/*
Expand Down
5 changes: 5 additions & 0 deletions lib/libzfs/libzfs_crypto.c
Original file line number Diff line number Diff line change
Expand Up @@ -1417,6 +1417,11 @@ zfs_crypto_load_key(zfs_handle_t *zhp, boolean_t noop,
"Incorrect key provided for '%s'."),
zfs_get_name(zhp));
break;
case ZFS_ERR_CRYPTO_NOTSUP:
zfs_error_aux(zhp->zfs_hdl, dgettext(TEXT_DOMAIN,
"'%s' uses an unsupported encryption suite."),
zfs_get_name(zhp));
break;
}
goto error;
}
Expand Down
6 changes: 6 additions & 0 deletions lib/libzfs/libzfs_sendrecv.c
Original file line number Diff line number Diff line change
Expand Up @@ -5161,6 +5161,12 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const char *tosnap,
"stream."));
(void) zfs_error(hdl, EZFS_BADVERSION, errbuf);
break;
case ZFS_ERR_CRYPTO_NOTSUP:
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
"stream uses crypto parameters not compatible with "
"this pool"));
(void) zfs_error(hdl, EZFS_BADSTREAM, errbuf);
break;
case EDQUOT:
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
"destination %s space quota exceeded."), name);
Expand Down
16 changes: 14 additions & 2 deletions module/zfs/dsl_crypt.c
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,12 @@ dsl_crypto_key_open(objset_t *mos, dsl_wrapping_key_t *wkey,
if (ret != 0)
goto error;

/* handle a future crypto suite that we don't support */
if (crypt >= ZIO_CRYPT_FUNCTIONS) {
ret = (SET_ERROR(ZFS_ERR_CRYPTO_NOTSUP));
goto error;
}

ret = zap_lookup(mos, dckobj, DSL_CRYPTO_KEY_GUID, 8, 1, &guid);
if (ret != 0)
goto error;
Expand Down Expand Up @@ -2141,10 +2147,16 @@ dsl_crypto_recv_raw_key_check(dsl_dataset_t *ds, nvlist_t *nvl, dmu_tx_t *tx)
* wrapping key.
*/
ret = nvlist_lookup_uint64(nvl, DSL_CRYPTO_KEY_CRYPTO_SUITE, &intval);
if (ret != 0 || intval >= ZIO_CRYPT_FUNCTIONS ||
intval <= ZIO_CRYPT_OFF)
if (ret != 0 || intval <= ZIO_CRYPT_OFF)
return (SET_ERROR(EINVAL));

/*
* Flag a future crypto suite that we don't support differently, so
* we can return a more useful error to the user.
*/
if (intval >= ZIO_CRYPT_FUNCTIONS)
return (SET_ERROR(ZFS_ERR_CRYPTO_NOTSUP));

ret = nvlist_lookup_uint64(nvl, DSL_CRYPTO_KEY_GUID, &intval);
if (ret != 0)
return (SET_ERROR(EINVAL));
Expand Down

0 comments on commit 61286f8

Please sign in to comment.