Better handling for future crypto parameters

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 #14577

diff --git a/cmd/zdb/zdb.c b/cmd/zdb/zdb.c
index 6c8d737130eec190a1c21a4e40b693f20cab4a1a..c6198ee264168aaa69281bb7db9d69b3689f00b2 100644 (file)
--- a/cmd/zdb/zdb.c
+++ b/cmd/zdb/zdb.c
@@ -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);
 

diff --git a/contrib/pyzfs/libzfs_core/_constants.py b/contrib/pyzfs/libzfs_core/_constants.py
index 4db1de8d9a6cb37067fb8fe99d61b61a6b3b72f9..5ee422dfa8030dfd5c06f7b14346259b56d88ca8 100644 (file)
--- a/contrib/pyzfs/libzfs_core/_constants.py
+++ b/contrib/pyzfs/libzfs_core/_constants.py
@@ -102,6 +102,7 @@ zfs_errno = enum_with_offset(1024, [
         'ZFS_ERR_VDEV_NOTSUP',
         'ZFS_ERR_NOT_USER_NAMESPACE',
         'ZFS_ERR_RESUME_EXISTS',
+        'ZFS_ERR_CRYPTO_NOTSUP',
     ],
     {}
 )

diff --git a/include/sys/fs/zfs.h b/include/sys/fs/zfs.h
index da2d0521655c1a7d9abb93c0cd51c40fb2124aa8..e869685c5e2d3eaeaccc6f17a1f60dda48c857d7 100644 (file)
--- a/include/sys/fs/zfs.h
+++ b/include/sys/fs/zfs.h
@@ -1545,6 +1545,7 @@ typedef enum {
        ZFS_ERR_VDEV_NOTSUP,
        ZFS_ERR_NOT_USER_NAMESPACE,
        ZFS_ERR_RESUME_EXISTS,
+       ZFS_ERR_CRYPTO_NOTSUP,
 } zfs_errno_t;
 
 /*

diff --git a/lib/libzfs/libzfs_crypto.c b/lib/libzfs/libzfs_crypto.c
index 3ef88370108204491a72e3477be56da94e7edb2c..e8351b22ff9673b12593c804bd29f984c5584b3d 100644 (file)
--- a/lib/libzfs/libzfs_crypto.c
+++ b/lib/libzfs/libzfs_crypto.c
@@ -1407,6 +1407,11 @@ try_again:
                            "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;
        }

diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c
index 66a22e333663d3c5c1f5b7b87b061b2fd0eeb179..4b6e06df69bef81be0b2a58f9b027a56839d23b6 100644 (file)
--- a/lib/libzfs/libzfs_sendrecv.c
+++ b/lib/libzfs/libzfs_sendrecv.c
@@ -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);

diff --git a/module/zfs/dsl_crypt.c b/module/zfs/dsl_crypt.c
index 408d038b4de476db7b1f0329fdd4f4d90b4dda70..dd936e74b1d9456dafabd15ccf18a398659dfc09 100644 (file)
--- a/module/zfs/dsl_crypt.c
+++ b/module/zfs/dsl_crypt.c
@@ -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;
@@ -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));