From be9a5c355c819ac0f2aca1f8c30dc75164e10322 Mon Sep 17 00:00:00 2001 From: Tom Caputi Date: Wed, 2 May 2018 18:36:20 -0400 Subject: [PATCH] Add support for decryption faults in zinject This patch adds the ability for zinject to trigger decryption and authentication faults in the ZIO and ARC layers. This functionality is exposed via the new "decrypt" error type, which may be provided for "data" object types. This patch also refactors some of the core encryption / decryption functions so that they have consistent prototypes, handle errors consistently, and do not have unused arguments. Reviewed-by: Brian Behlendorf Signed-off-by: Tom Caputi Closes #7474 --- cmd/zinject/zinject.c | 32 ++++- include/sys/arc_impl.h | 2 +- include/sys/dsl_crypt.h | 7 +- include/sys/zfs_ioctl.h | 1 + include/sys/zio.h | 2 + include/sys/zio_crypt.h | 13 +- man/man8/zinject.8 | 3 +- module/zfs/arc.c | 117 +++++++----------- module/zfs/dsl_crypt.c | 33 +++-- module/zfs/zio.c | 21 +++- module/zfs/zio_crypt.c | 16 +-- module/zfs/zio_inject.c | 32 ++++- tests/runfiles/linux.run | 2 +- .../tests/functional/fault/Makefile.am | 1 + .../tests/functional/fault/decrypt_fault.ksh | 55 ++++++++ 15 files changed, 221 insertions(+), 116 deletions(-) create mode 100755 tests/zfs-tests/tests/functional/fault/decrypt_fault.ksh diff --git a/cmd/zinject/zinject.c b/cmd/zinject/zinject.c index d66a959bc4..d6298f8f49 100644 --- a/cmd/zinject/zinject.c +++ b/cmd/zinject/zinject.c @@ -116,9 +116,9 @@ * specified. * * The '-e' option takes a string describing the errno to simulate. This must - * be either 'io' or 'checksum'. In most cases this will result in the same - * behavior, but RAID-Z will produce a different set of ereports for this - * situation. + * be one of 'io', 'checksum', or 'decrypt'. In most cases this will result + * in the same behavior, but RAID-Z will produce a different set of ereports + * for this situation. * * The '-a', '-u', and '-m' flags toggle internal flush behavior. If '-a' is * specified, then the ARC cache is flushed appropriately. If '-u' is @@ -299,8 +299,8 @@ usage(void) "\t\tinterpreted depending on the '-t' option.\n" "\n" "\t\t-q\tQuiet mode. Only print out the handler number added.\n" - "\t\t-e\tInject a specific error. Must be either 'io' or\n" - "\t\t\t'checksum'. Default is 'io'.\n" + "\t\t-e\tInject a specific error. Must be one of 'io',\n" + "\t\t\t'checksum', or decrypt. Default is 'io'.\n" "\t\t-l\tInject error at a particular block level. Default is " "0.\n" "\t\t-m\tAutomatically remount underlying filesystem.\n" @@ -774,6 +774,8 @@ main(int argc, char **argv) error = EIO; } else if (strcasecmp(optarg, "checksum") == 0) { error = ECKSUM; + } else if (strcasecmp(optarg, "decrypt") == 0) { + error = EACCES; } else if (strcasecmp(optarg, "nxio") == 0) { error = ENXIO; } else if (strcasecmp(optarg, "dtl") == 0) { @@ -1130,7 +1132,25 @@ main(int argc, char **argv) return (1); } - record.zi_cmd = ZINJECT_DATA_FAULT; + if (error == EACCES) { + if (type != TYPE_DATA) { + (void) fprintf(stderr, "decryption errors " + "may only be injected for 'data' types\n"); + libzfs_fini(g_zfs); + return (1); + } + + record.zi_cmd = ZINJECT_DECRYPT_FAULT; + /* + * Internally, ZFS actually uses ECKSUM for decryption + * errors since EACCES is used to indicate the key was + * not found. + */ + error = ECKSUM; + } else { + record.zi_cmd = ZINJECT_DATA_FAULT; + } + if (translate_record(type, argv[0], range, level, &record, pool, dataset) != 0) { libzfs_fini(g_zfs); diff --git a/include/sys/arc_impl.h b/include/sys/arc_impl.h index a923449d96..52863bba4e 100644 --- a/include/sys/arc_impl.h +++ b/include/sys/arc_impl.h @@ -96,7 +96,7 @@ struct arc_callback { boolean_t acb_encrypted; boolean_t acb_compressed; boolean_t acb_noauth; - uint64_t acb_dsobj; + zbookmark_phys_t acb_zb; zio_t *acb_zio_dummy; zio_t *acb_zio_head; arc_callback_t *acb_next; diff --git a/include/sys/dsl_crypt.h b/include/sys/dsl_crypt.h index efa3839f4e..e92ae364c8 100644 --- a/include/sys/dsl_crypt.h +++ b/include/sys/dsl_crypt.h @@ -216,8 +216,9 @@ int spa_do_crypt_mac_abd(boolean_t generate, spa_t *spa, uint64_t dsobj, abd_t *abd, uint_t datalen, uint8_t *mac); int spa_do_crypt_objset_mac_abd(boolean_t generate, spa_t *spa, uint64_t dsobj, abd_t *abd, uint_t datalen, boolean_t byteswap); -int spa_do_crypt_abd(boolean_t encrypt, spa_t *spa, uint64_t dsobj, - const blkptr_t *bp, uint64_t txgid, uint_t datalen, abd_t *pabd, - abd_t *cabd, uint8_t *iv, uint8_t *mac, uint8_t *salt, boolean_t *no_crypt); +int spa_do_crypt_abd(boolean_t encrypt, spa_t *spa, const zbookmark_phys_t *zb, + dmu_object_type_t ot, boolean_t dedup, boolean_t bswap, uint8_t *salt, + uint8_t *iv, uint8_t *mac, uint_t datalen, abd_t *pabd, abd_t *cabd, + boolean_t *no_crypt); #endif diff --git a/include/sys/zfs_ioctl.h b/include/sys/zfs_ioctl.h index ab562d24bf..06e3cb24a4 100644 --- a/include/sys/zfs_ioctl.h +++ b/include/sys/zfs_ioctl.h @@ -391,6 +391,7 @@ typedef enum zinject_type { ZINJECT_IGNORED_WRITES, ZINJECT_PANIC, ZINJECT_DELAY_IO, + ZINJECT_DECRYPT_FAULT, } zinject_type_t; typedef struct zfs_share { diff --git a/include/sys/zio.h b/include/sys/zio.h index dc6841d839..25c12fbcc8 100644 --- a/include/sys/zio.h +++ b/include/sys/zio.h @@ -640,6 +640,8 @@ extern int zio_inject_list_next(int *id, char *name, size_t buflen, struct zinject_record *record); extern int zio_clear_fault(int id); extern void zio_handle_panic_injection(spa_t *spa, char *tag, uint64_t type); +extern int zio_handle_decrypt_injection(spa_t *spa, const zbookmark_phys_t *zb, + uint64_t type, int error); extern int zio_handle_fault_injection(zio_t *zio, int error); extern int zio_handle_device_injection(vdev_t *vd, zio_t *zio, int error); extern int zio_handle_device_injections(vdev_t *vd, zio_t *zio, int err1, diff --git a/include/sys/zio_crypt.h b/include/sys/zio_crypt.h index 57b4c1e7c3..d54e2fe192 100644 --- a/include/sys/zio_crypt.h +++ b/include/sys/zio_crypt.h @@ -132,12 +132,13 @@ int zio_crypt_do_hmac(zio_crypt_key_t *key, uint8_t *data, uint_t datalen, uint8_t *digestbuf, uint_t digestlen); int zio_crypt_do_objset_hmacs(zio_crypt_key_t *key, void *data, uint_t datalen, boolean_t byteswap, uint8_t *portable_mac, uint8_t *local_mac); -int zio_do_crypt_data(boolean_t encrypt, zio_crypt_key_t *key, uint8_t *salt, - dmu_object_type_t ot, uint8_t *iv, uint8_t *mac, uint_t datalen, - boolean_t byteswap, uint8_t *plainbuf, uint8_t *cipherbuf, +int zio_do_crypt_data(boolean_t encrypt, zio_crypt_key_t *key, + dmu_object_type_t ot, boolean_t byteswap, uint8_t *salt, uint8_t *iv, + uint8_t *mac, uint_t datalen, uint8_t *plainbuf, uint8_t *cipherbuf, + boolean_t *no_crypt); +int zio_do_crypt_abd(boolean_t encrypt, zio_crypt_key_t *key, + dmu_object_type_t ot, boolean_t byteswap, uint8_t *salt, uint8_t *iv, + uint8_t *mac, uint_t datalen, abd_t *pabd, abd_t *cabd, boolean_t *no_crypt); -int zio_do_crypt_abd(boolean_t encrypt, zio_crypt_key_t *key, uint8_t *salt, - dmu_object_type_t ot, uint8_t *iv, uint8_t *mac, uint_t datalen, - boolean_t byteswap, abd_t *pabd, abd_t *cabd, boolean_t *no_crypt); #endif diff --git a/man/man8/zinject.8 b/man/man8/zinject.8 index c5916e342c..7b664415f5 100644 --- a/man/man8/zinject.8 +++ b/man/man8/zinject.8 @@ -108,13 +108,14 @@ A vdev specified by path or GUID. .BI "\-e" " device_error" Specify .BR "checksum" " for an ECKSUM error," +.BR "decrypt" " for a data decryption error," .BR "corrupt" " to flip a bit in the data after a read," .BR "dtl" " for an ECHILD error," .BR "io" " for an EIO error where reopening the device will succeed, or" .BR "nxio" " for an ENXIO error where reopening the device will fail." For EIO and ENXIO, the "failed" reads or writes still occur. The probe simply sets the error value reported by the I/O pipeline so it appears the read or -write failed. +write failed. Decryption errors only currently work with file data. .TP .BI "\-f" " frequency" Only inject errors a fraction of the time. Expressed as a real number diff --git a/module/zfs/arc.c b/module/zfs/arc.c index fa7f62d990..6662e0fae3 100644 --- a/module/zfs/arc.c +++ b/module/zfs/arc.c @@ -1922,10 +1922,9 @@ error: * also decompress the data. */ static int -arc_hdr_decrypt(arc_buf_hdr_t *hdr, spa_t *spa, uint64_t dsobj) +arc_hdr_decrypt(arc_buf_hdr_t *hdr, spa_t *spa, const zbookmark_phys_t *zb) { int ret; - dsl_crypto_key_t *dck = NULL; abd_t *cabd = NULL; void *tmp = NULL; boolean_t no_crypt = B_FALSE; @@ -1936,25 +1935,9 @@ arc_hdr_decrypt(arc_buf_hdr_t *hdr, spa_t *spa, uint64_t dsobj) arc_hdr_alloc_abd(hdr, B_FALSE); - /* - * We must be careful to use the passed-in dsobj value here and - * not the value in b_dsobj. b_dsobj is meant to be a best guess for - * the L2ARC, which has the luxury of being able to fail without real - * consequences (the data simply won't make it to the L2ARC). In - * reality, the dsobj stored in the header may belong to a dataset - * that has been unmounted or otherwise disowned, meaning the key - * won't be accessible via that dsobj anymore. - */ - ret = spa_keystore_lookup_key(spa, dsobj, FTAG, &dck); - if (ret != 0) { - ret = SET_ERROR(EACCES); - goto error; - } - - ret = zio_do_crypt_abd(B_FALSE, &dck->dck_key, - hdr->b_crypt_hdr.b_salt, hdr->b_crypt_hdr.b_ot, - hdr->b_crypt_hdr.b_iv, hdr->b_crypt_hdr.b_mac, - HDR_GET_PSIZE(hdr), bswap, hdr->b_l1hdr.b_pabd, + ret = spa_do_crypt_abd(B_FALSE, spa, zb, hdr->b_crypt_hdr.b_ot, + B_FALSE, bswap, hdr->b_crypt_hdr.b_salt, hdr->b_crypt_hdr.b_iv, + hdr->b_crypt_hdr.b_mac, HDR_GET_PSIZE(hdr), hdr->b_l1hdr.b_pabd, hdr->b_crypt_hdr.b_rabd, &no_crypt); if (ret != 0) goto error; @@ -1994,14 +1977,10 @@ arc_hdr_decrypt(arc_buf_hdr_t *hdr, spa_t *spa, uint64_t dsobj) hdr->b_l1hdr.b_pabd = cabd; } - spa_keystore_dsl_key_rele(spa, dck, FTAG); - return (0); error: arc_hdr_free_abd(hdr, B_FALSE); - if (dck != NULL) - spa_keystore_dsl_key_rele(spa, dck, FTAG); if (cabd != NULL) arc_free_data_buf(hdr, cabd, arc_hdr_size(hdr), hdr); @@ -2015,7 +1994,7 @@ error: */ static int arc_fill_hdr_crypt(arc_buf_hdr_t *hdr, kmutex_t *hash_lock, spa_t *spa, - uint64_t dsobj, boolean_t noauth) + const zbookmark_phys_t *zb, boolean_t noauth) { int ret; @@ -2029,7 +2008,7 @@ arc_fill_hdr_crypt(arc_buf_hdr_t *hdr, kmutex_t *hash_lock, spa_t *spa, * The caller requested authenticated data but our data has * not been authenticated yet. Verify the MAC now if we can. */ - ret = arc_hdr_authenticate(hdr, spa, dsobj); + ret = arc_hdr_authenticate(hdr, spa, zb->zb_objset); if (ret != 0) goto error; } else if (HDR_HAS_RABD(hdr) && hdr->b_l1hdr.b_pabd == NULL) { @@ -2038,7 +2017,7 @@ arc_fill_hdr_crypt(arc_buf_hdr_t *hdr, kmutex_t *hash_lock, spa_t *spa, * unencrypted version was requested we take this opportunity * to store the decrypted version in the header for future use. */ - ret = arc_hdr_decrypt(hdr, spa, dsobj); + ret = arc_hdr_decrypt(hdr, spa, zb); if (ret != 0) goto error; } @@ -2094,7 +2073,8 @@ arc_buf_untransform_in_place(arc_buf_t *buf, kmutex_t *hash_lock) * the correct-sized data buffer. */ static int -arc_buf_fill(arc_buf_t *buf, spa_t *spa, uint64_t dsobj, arc_fill_flags_t flags) +arc_buf_fill(arc_buf_t *buf, spa_t *spa, const zbookmark_phys_t *zb, + arc_fill_flags_t flags) { int error = 0; arc_buf_hdr_t *hdr = buf->b_hdr; @@ -2131,7 +2111,7 @@ arc_buf_fill(arc_buf_t *buf, spa_t *spa, uint64_t dsobj, arc_fill_flags_t flags) */ if (HDR_PROTECTED(hdr)) { error = arc_fill_hdr_crypt(hdr, hash_lock, spa, - dsobj, !!(flags & ARC_FILL_NOAUTH)); + zb, !!(flags & ARC_FILL_NOAUTH)); if (error != 0) { arc_hdr_set_flags(hdr, ARC_FLAG_IO_ERROR); return (error); @@ -2272,13 +2252,14 @@ arc_untransform(arc_buf_t *buf, spa_t *spa, const zbookmark_phys_t *zb, if (in_place) flags |= ARC_FILL_IN_PLACE; - ret = arc_buf_fill(buf, spa, zb->zb_objset, flags); + ret = arc_buf_fill(buf, spa, zb, flags); if (ret == ECKSUM) { /* * Convert authentication and decryption errors to EIO * (and generate an ereport) before leaving the ARC. */ ret = SET_ERROR(EIO); + spa_log_error(spa, zb); zfs_ereport_post(FM_EREPORT_ZFS_AUTHENTICATION, spa, NULL, zb, NULL, 0, 0); } @@ -2813,8 +2794,8 @@ arc_can_share(arc_buf_hdr_t *hdr, arc_buf_t *buf) * copy was made successfully, or an error code otherwise. */ static int -arc_buf_alloc_impl(arc_buf_hdr_t *hdr, spa_t *spa, uint64_t dsobj, void *tag, - boolean_t encrypted, boolean_t compressed, boolean_t noauth, +arc_buf_alloc_impl(arc_buf_hdr_t *hdr, spa_t *spa, const zbookmark_phys_t *zb, + void *tag, boolean_t encrypted, boolean_t compressed, boolean_t noauth, boolean_t fill, arc_buf_t **ret) { arc_buf_t *buf; @@ -2906,7 +2887,8 @@ arc_buf_alloc_impl(arc_buf_hdr_t *hdr, spa_t *spa, uint64_t dsobj, void *tag, * decompress the data. */ if (fill) { - return (arc_buf_fill(buf, spa, dsobj, flags)); + ASSERT3P(zb, !=, NULL); + return (arc_buf_fill(buf, spa, zb, flags)); } return (0); @@ -3588,7 +3570,7 @@ arc_alloc_buf(spa_t *spa, void *tag, arc_buf_contents_t type, int32_t size) ASSERT(!MUTEX_HELD(HDR_LOCK(hdr))); arc_buf_t *buf = NULL; - VERIFY0(arc_buf_alloc_impl(hdr, spa, 0, tag, B_FALSE, B_FALSE, + VERIFY0(arc_buf_alloc_impl(hdr, spa, NULL, tag, B_FALSE, B_FALSE, B_FALSE, B_FALSE, &buf)); arc_buf_thaw(buf); @@ -3613,7 +3595,7 @@ arc_alloc_compressed_buf(spa_t *spa, void *tag, uint64_t psize, uint64_t lsize, ASSERT(!MUTEX_HELD(HDR_LOCK(hdr))); arc_buf_t *buf = NULL; - VERIFY0(arc_buf_alloc_impl(hdr, spa, 0, tag, B_FALSE, + VERIFY0(arc_buf_alloc_impl(hdr, spa, NULL, tag, B_FALSE, B_TRUE, B_FALSE, B_FALSE, &buf)); arc_buf_thaw(buf); ASSERT3P(hdr->b_l1hdr.b_freeze_cksum, ==, NULL); @@ -3667,7 +3649,7 @@ arc_alloc_raw_buf(spa_t *spa, void *tag, uint64_t dsobj, boolean_t byteorder, * arc_write_ready(). */ buf = NULL; - VERIFY0(arc_buf_alloc_impl(hdr, spa, dsobj, tag, B_TRUE, B_TRUE, + VERIFY0(arc_buf_alloc_impl(hdr, spa, NULL, tag, B_TRUE, B_TRUE, B_FALSE, B_FALSE, &buf)); arc_buf_thaw(buf); ASSERT3P(hdr->b_l1hdr.b_freeze_cksum, ==, NULL); @@ -5814,7 +5796,7 @@ arc_read_done(zio_t *zio) continue; int error = arc_buf_alloc_impl(hdr, zio->io_spa, - acb->acb_dsobj, acb->acb_private, acb->acb_encrypted, + &acb->acb_zb, acb->acb_private, acb->acb_encrypted, acb->acb_compressed, acb->acb_noauth, B_TRUE, &acb->acb_buf); if (error != 0) { @@ -5829,7 +5811,7 @@ arc_read_done(zio_t *zio) * encryption key wasn't loaded */ ASSERT((zio->io_flags & ZIO_FLAG_SPECULATIVE) || - error != ENOENT); + error != EACCES); /* * If we failed to decrypt, report an error now (as the zio @@ -5838,11 +5820,10 @@ arc_read_done(zio_t *zio) if (error == ECKSUM) { ASSERT(BP_IS_PROTECTED(bp)); error = SET_ERROR(EIO); - spa_log_error(zio->io_spa, &zio->io_bookmark); if ((zio->io_flags & ZIO_FLAG_SPECULATIVE) == 0) { + spa_log_error(zio->io_spa, &acb->acb_zb); zfs_ereport_post(FM_EREPORT_ZFS_AUTHENTICATION, - zio->io_spa, NULL, &zio->io_bookmark, zio, - 0, 0); + zio->io_spa, NULL, &acb->acb_zb, zio, 0, 0); } } @@ -6004,7 +5985,7 @@ top: acb->acb_compressed = compressed_read; acb->acb_encrypted = encrypted_read; acb->acb_noauth = noauth_read; - acb->acb_dsobj = zb->zb_objset; + acb->acb_zb = *zb; if (pio != NULL) acb->acb_zio_dummy = zio_null(pio, spa, NULL, NULL, NULL, zio_flags); @@ -6049,18 +6030,22 @@ top: ASSERT(!BP_IS_EMBEDDED(bp) || !BP_IS_HOLE(bp)); /* Get a buf with the desired data in it. */ - rc = arc_buf_alloc_impl(hdr, spa, zb->zb_objset, - private, encrypted_read, compressed_read, - noauth_read, B_TRUE, &buf); + rc = arc_buf_alloc_impl(hdr, spa, zb, private, + encrypted_read, compressed_read, noauth_read, + B_TRUE, &buf); if (rc == ECKSUM) { /* * Convert authentication and decryption errors - * to EIO (and generate an ereport) before - * leaving the ARC. + * to EIO (and generate an ereport if needed) + * before leaving the ARC. */ rc = SET_ERROR(EIO); - zfs_ereport_post(FM_EREPORT_ZFS_AUTHENTICATION, - spa, NULL, zb, NULL, 0, 0); + if ((zio_flags & ZIO_FLAG_SPECULATIVE) == 0) { + spa_log_error(spa, zb); + zfs_ereport_post( + FM_EREPORT_ZFS_AUTHENTICATION, + spa, NULL, zb, NULL, 0, 0); + } } if (rc != 0) { (void) remove_reference(hdr, hash_lock, @@ -6071,7 +6056,7 @@ top: /* assert any errors weren't due to unloaded keys */ ASSERT((zio_flags & ZIO_FLAG_SPECULATIVE) || - rc != ENOENT); + rc != EACCES); } else if (*arc_flags & ARC_FLAG_PREFETCH && refcount_count(&hdr->b_l1hdr.b_refcnt) == 0) { arc_hdr_set_flags(hdr, ARC_FLAG_PREFETCH); @@ -6223,7 +6208,7 @@ top: acb->acb_compressed = compressed_read; acb->acb_encrypted = encrypted_read; acb->acb_noauth = noauth_read; - acb->acb_dsobj = zb->zb_objset; + acb->acb_zb = *zb; ASSERT3P(hdr->b_l1hdr.b_acb, ==, NULL); hdr->b_l1hdr.b_acb = acb; @@ -8078,7 +8063,6 @@ l2arc_untransform(zio_t *zio, l2arc_read_callback_t *cb) spa_t *spa = zio->io_spa; arc_buf_hdr_t *hdr = cb->l2rcb_hdr; blkptr_t *bp = zio->io_bp; - dsl_crypto_key_t *dck = NULL; uint8_t salt[ZIO_DATA_SALT_LEN]; uint8_t iv[ZIO_DATA_IV_LEN]; uint8_t mac[ZIO_DATA_MAC_LEN]; @@ -8099,31 +8083,20 @@ l2arc_untransform(zio_t *zio, l2arc_read_callback_t *cb) * until arc_read_done(). */ if (BP_IS_ENCRYPTED(bp)) { - abd_t *eabd = arc_get_data_abd(hdr, - arc_hdr_size(hdr), hdr); + abd_t *eabd = arc_get_data_abd(hdr, arc_hdr_size(hdr), hdr); zio_crypt_decode_params_bp(bp, salt, iv); zio_crypt_decode_mac_bp(bp, mac); - ret = spa_keystore_lookup_key(spa, - cb->l2rcb_zb.zb_objset, FTAG, &dck); + ret = spa_do_crypt_abd(B_FALSE, spa, &cb->l2rcb_zb, + BP_GET_TYPE(bp), BP_GET_DEDUP(bp), BP_SHOULD_BYTESWAP(bp), + salt, iv, mac, HDR_GET_PSIZE(hdr), eabd, + hdr->b_l1hdr.b_pabd, &no_crypt); if (ret != 0) { arc_free_data_abd(hdr, eabd, arc_hdr_size(hdr), hdr); goto error; } - ret = zio_do_crypt_abd(B_FALSE, &dck->dck_key, - salt, BP_GET_TYPE(bp), iv, mac, HDR_GET_PSIZE(hdr), - BP_SHOULD_BYTESWAP(bp), eabd, hdr->b_l1hdr.b_pabd, - &no_crypt); - if (ret != 0) { - arc_free_data_abd(hdr, eabd, arc_hdr_size(hdr), hdr); - spa_keystore_dsl_key_rele(spa, dck, FTAG); - goto error; - } - - spa_keystore_dsl_key_rele(spa, dck, FTAG); - /* * If we actually performed decryption, replace b_pabd * with the decrypted data. Otherwise we can just throw @@ -8529,9 +8502,9 @@ l2arc_apply_transforms(spa_t *spa, arc_buf_hdr_t *hdr, uint64_t asize, goto error; ret = zio_do_crypt_abd(B_TRUE, &dck->dck_key, - hdr->b_crypt_hdr.b_salt, hdr->b_crypt_hdr.b_ot, - hdr->b_crypt_hdr.b_iv, mac, psize, bswap, to_write, - eabd, &no_crypt); + hdr->b_crypt_hdr.b_ot, bswap, hdr->b_crypt_hdr.b_salt, + hdr->b_crypt_hdr.b_iv, mac, psize, to_write, eabd, + &no_crypt); if (ret != 0) goto error; diff --git a/module/zfs/dsl_crypt.c b/module/zfs/dsl_crypt.c index 579b32c428..e2067448cd 100644 --- a/module/zfs/dsl_crypt.c +++ b/module/zfs/dsl_crypt.c @@ -2661,23 +2661,23 @@ error: * these fields to populate pabd (the plaintext). */ int -spa_do_crypt_abd(boolean_t encrypt, spa_t *spa, uint64_t dsobj, - const blkptr_t *bp, uint64_t txgid, uint_t datalen, abd_t *pabd, - abd_t *cabd, uint8_t *iv, uint8_t *mac, uint8_t *salt, boolean_t *no_crypt) +spa_do_crypt_abd(boolean_t encrypt, spa_t *spa, const zbookmark_phys_t *zb, + dmu_object_type_t ot, boolean_t dedup, boolean_t bswap, uint8_t *salt, + uint8_t *iv, uint8_t *mac, uint_t datalen, abd_t *pabd, abd_t *cabd, + boolean_t *no_crypt) { int ret; - dmu_object_type_t ot = BP_GET_TYPE(bp); dsl_crypto_key_t *dck = NULL; uint8_t *plainbuf = NULL, *cipherbuf = NULL; ASSERT(spa_feature_is_active(spa, SPA_FEATURE_ENCRYPTION)); - ASSERT(!BP_IS_EMBEDDED(bp)); - ASSERT(BP_IS_ENCRYPTED(bp)); /* look up the key from the spa's keystore */ - ret = spa_keystore_lookup_key(spa, dsobj, FTAG, &dck); - if (ret != 0) + ret = spa_keystore_lookup_key(spa, zb->zb_objset, FTAG, &dck); + if (ret != 0) { + ret = SET_ERROR(EACCES); return (ret); + } if (encrypt) { plainbuf = abd_borrow_buf_copy(pabd, datalen); @@ -2696,7 +2696,7 @@ spa_do_crypt_abd(boolean_t encrypt, spa_t *spa, uint64_t dsobj, * at allocation time in zio_alloc_zil(). On decryption, we simply use * the provided values. */ - if (encrypt && ot != DMU_OT_INTENT_LOG && !BP_GET_DEDUP(bp)) { + if (encrypt && ot != DMU_OT_INTENT_LOG && !dedup) { ret = zio_crypt_key_get_salt(&dck->dck_key, salt); if (ret != 0) goto error; @@ -2704,7 +2704,7 @@ spa_do_crypt_abd(boolean_t encrypt, spa_t *spa, uint64_t dsobj, ret = zio_crypt_generate_iv(iv); if (ret != 0) goto error; - } else if (encrypt && BP_GET_DEDUP(bp)) { + } else if (encrypt && dedup) { ret = zio_crypt_generate_iv_salt_dedup(&dck->dck_key, plainbuf, datalen, iv, salt); if (ret != 0) @@ -2712,8 +2712,17 @@ spa_do_crypt_abd(boolean_t encrypt, spa_t *spa, uint64_t dsobj, } /* call lower level function to perform encryption / decryption */ - ret = zio_do_crypt_data(encrypt, &dck->dck_key, salt, ot, iv, mac, - datalen, BP_SHOULD_BYTESWAP(bp), plainbuf, cipherbuf, no_crypt); + ret = zio_do_crypt_data(encrypt, &dck->dck_key, ot, bswap, salt, iv, + mac, datalen, plainbuf, cipherbuf, no_crypt); + + /* + * Handle injected decryption faults. Unfortunately, we cannot inject + * faults for dnode blocks because we might trigger the panic in + * dbuf_prepare_encrypted_dnode_leaf(), which exists because syncing + * context is not prepared to handle malicious decryption failures. + */ + if (zio_injection_enabled && !encrypt && ot != DMU_OT_DNODE && ret == 0) + ret = zio_handle_decrypt_injection(spa, zb, ot, ECKSUM); if (ret != 0) goto error; diff --git a/module/zfs/zio.c b/module/zfs/zio.c index b585368be5..6822505f18 100644 --- a/module/zfs/zio.c +++ b/module/zfs/zio.c @@ -449,6 +449,10 @@ zio_decrypt(zio_t *zio, abd_t *data, uint64_t size) } abd_copy(data, zio->io_abd, size); + if (zio_injection_enabled && ot != DMU_OT_DNODE && ret == 0) { + ret = zio_handle_decrypt_injection(spa, + &zio->io_bookmark, ot, ECKSUM); + } if (ret != 0) goto error; @@ -468,6 +472,10 @@ zio_decrypt(zio_t *zio, abd_t *data, uint64_t size) zio_crypt_decode_mac_bp(bp, mac); ret = spa_do_crypt_mac_abd(B_FALSE, spa, dsobj, zio->io_abd, size, mac); + if (zio_injection_enabled && ret == 0) { + ret = zio_handle_decrypt_injection(spa, + &zio->io_bookmark, ot, ECKSUM); + } } abd_copy(data, zio->io_abd, size); @@ -487,8 +495,9 @@ zio_decrypt(zio_t *zio, abd_t *data, uint64_t size) zio_crypt_decode_mac_bp(bp, mac); } - ret = spa_do_crypt_abd(B_FALSE, spa, dsobj, bp, bp->blk_birth, - size, data, zio->io_abd, iv, mac, salt, &no_crypt); + ret = spa_do_crypt_abd(B_FALSE, spa, &zio->io_bookmark, BP_GET_TYPE(bp), + BP_GET_DEDUP(bp), BP_SHOULD_BYTESWAP(bp), salt, iv, mac, size, data, + zio->io_abd, &no_crypt); if (no_crypt) abd_copy(data, zio->io_abd, size); @@ -499,7 +508,7 @@ zio_decrypt(zio_t *zio, abd_t *data, uint64_t size) error: /* assert that the key was found unless this was speculative */ - ASSERT(ret != ENOENT || (zio->io_flags & ZIO_FLAG_SPECULATIVE)); + ASSERT(ret != EACCES || (zio->io_flags & ZIO_FLAG_SPECULATIVE)); /* * If there was a decryption / authentication error return EIO as @@ -508,6 +517,7 @@ error: if (ret == ECKSUM) { zio->io_error = SET_ERROR(EIO); if ((zio->io_flags & ZIO_FLAG_SPECULATIVE) == 0) { + spa_log_error(spa, &zio->io_bookmark); zfs_ereport_post(FM_EREPORT_ZFS_AUTHENTICATION, spa, NULL, &zio->io_bookmark, zio, 0, 0); } @@ -3906,8 +3916,9 @@ zio_encrypt(zio_t *zio) } /* Perform the encryption. This should not fail */ - VERIFY0(spa_do_crypt_abd(B_TRUE, spa, dsobj, bp, zio->io_txg, - psize, zio->io_abd, eabd, iv, mac, salt, &no_crypt)); + VERIFY0(spa_do_crypt_abd(B_TRUE, spa, &zio->io_bookmark, + BP_GET_TYPE(bp), BP_GET_DEDUP(bp), BP_SHOULD_BYTESWAP(bp), + salt, iv, mac, psize, zio->io_abd, eabd, &no_crypt)); /* encode encryption metadata into the bp */ if (ot == DMU_OT_INTENT_LOG) { diff --git a/module/zfs/zio_crypt.c b/module/zfs/zio_crypt.c index d9e88404f8..2e61767829 100644 --- a/module/zfs/zio_crypt.c +++ b/module/zfs/zio_crypt.c @@ -1860,9 +1860,9 @@ error: * Primary encryption / decryption entrypoint for zio data. */ int -zio_do_crypt_data(boolean_t encrypt, zio_crypt_key_t *key, uint8_t *salt, - dmu_object_type_t ot, uint8_t *iv, uint8_t *mac, uint_t datalen, - boolean_t byteswap, uint8_t *plainbuf, uint8_t *cipherbuf, +zio_do_crypt_data(boolean_t encrypt, zio_crypt_key_t *key, + dmu_object_type_t ot, boolean_t byteswap, uint8_t *salt, uint8_t *iv, + uint8_t *mac, uint_t datalen, uint8_t *plainbuf, uint8_t *cipherbuf, boolean_t *no_crypt) { int ret; @@ -1984,9 +1984,9 @@ error: * linear buffers. */ int -zio_do_crypt_abd(boolean_t encrypt, zio_crypt_key_t *key, uint8_t *salt, - dmu_object_type_t ot, uint8_t *iv, uint8_t *mac, uint_t datalen, - boolean_t byteswap, abd_t *pabd, abd_t *cabd, boolean_t *no_crypt) +zio_do_crypt_abd(boolean_t encrypt, zio_crypt_key_t *key, dmu_object_type_t ot, + boolean_t byteswap, uint8_t *salt, uint8_t *iv, uint8_t *mac, + uint_t datalen, abd_t *pabd, abd_t *cabd, boolean_t *no_crypt) { int ret; void *ptmp, *ctmp; @@ -1999,8 +1999,8 @@ zio_do_crypt_abd(boolean_t encrypt, zio_crypt_key_t *key, uint8_t *salt, ctmp = abd_borrow_buf_copy(cabd, datalen); } - ret = zio_do_crypt_data(encrypt, key, salt, ot, iv, mac, - datalen, byteswap, ptmp, ctmp, no_crypt); + ret = zio_do_crypt_data(encrypt, key, ot, byteswap, salt, iv, mac, + datalen, ptmp, ctmp, no_crypt); if (ret != 0) goto error; diff --git a/module/zfs/zio_inject.c b/module/zfs/zio_inject.c index 62ca41bf49..26f255c7b2 100644 --- a/module/zfs/zio_inject.c +++ b/module/zfs/zio_inject.c @@ -123,7 +123,7 @@ freq_triggered(uint32_t frequency) * Returns true if the given record matches the I/O in progress. */ static boolean_t -zio_match_handler(zbookmark_phys_t *zb, uint64_t type, +zio_match_handler(const zbookmark_phys_t *zb, uint64_t type, zinject_record_t *record, int error) { /* @@ -178,6 +178,36 @@ zio_handle_panic_injection(spa_t *spa, char *tag, uint64_t type) rw_exit(&inject_lock); } +/* + * Inject a decryption failure. Decryption failures can occur in + * both the ARC and the ZIO layers. + */ +int +zio_handle_decrypt_injection(spa_t *spa, const zbookmark_phys_t *zb, + uint64_t type, int error) +{ + int ret = 0; + inject_handler_t *handler; + + rw_enter(&inject_lock, RW_READER); + + for (handler = list_head(&inject_handlers); handler != NULL; + handler = list_next(&inject_handlers, handler)) { + + if (spa != handler->zi_spa || + handler->zi_record.zi_cmd != ZINJECT_DECRYPT_FAULT) + continue; + + if (zio_match_handler(zb, type, &handler->zi_record, error)) { + ret = error; + break; + } + } + + rw_exit(&inject_lock); + return (ret); +} + /* * Determine if the I/O in question should return failure. Returns the errno * to be returned to the caller. diff --git a/tests/runfiles/linux.run b/tests/runfiles/linux.run index 62dfcf44e5..1d0b2ef915 100644 --- a/tests/runfiles/linux.run +++ b/tests/runfiles/linux.run @@ -518,7 +518,7 @@ tags = ['functional', 'exec'] [tests/functional/fault] tests = ['auto_online_001_pos', 'auto_replace_001_pos', 'auto_spare_001_pos', 'auto_spare_002_pos', 'auto_spare_ashift', 'auto_spare_multiple', - 'scrub_after_resilver'] + 'scrub_after_resilver', 'decrypt_fault'] tags = ['functional', 'fault'] [tests/functional/features/async_destroy] diff --git a/tests/zfs-tests/tests/functional/fault/Makefile.am b/tests/zfs-tests/tests/functional/fault/Makefile.am index 3c0e31d2a1..1153ad8d66 100644 --- a/tests/zfs-tests/tests/functional/fault/Makefile.am +++ b/tests/zfs-tests/tests/functional/fault/Makefile.am @@ -8,6 +8,7 @@ dist_pkgdata_SCRIPTS = \ auto_spare_002_pos.ksh \ auto_spare_ashift.ksh \ auto_spare_multiple.ksh \ + decrypt_fault.ksh \ scrub_after_resilver.ksh dist_pkgdata_DATA = \ diff --git a/tests/zfs-tests/tests/functional/fault/decrypt_fault.ksh b/tests/zfs-tests/tests/functional/fault/decrypt_fault.ksh new file mode 100755 index 0000000000..10008f22ee --- /dev/null +++ b/tests/zfs-tests/tests/functional/fault/decrypt_fault.ksh @@ -0,0 +1,55 @@ +#!/bin/ksh -p +# +# 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. +# + +# +# Copyright (c) 2018 by Lawrence Livermore National Security, LLC. +# All rights reserved. +# + +. $STF_SUITE/include/libtest.shlib +. $STF_SUITE/tests/functional/fault/fault.cfg + +# +# DESCRIPTION: +# Test that injected decryption errors are handled correctly. +# +# STRATEGY: +# 1. Create an encrypted dataset with an test file +# 2. Inject decryption errors on the file 20% of the time +# 3. Read the file to confirm that errors are handled correctly +# 4. Confirm that the decryption injection was added to the ZED logs +# + +log_assert "Testing that injected decryption errors are handled correctly" + +function cleanup +{ + log_must zinject -c all + default_cleanup_noexit +} + +log_onexit cleanup + +default_mirror_setup_noexit $DISK1 $DISK2 +log_must eval "echo 'password' | zfs create -o encryption=on \ + -o keyformat=passphrase -o keylocation=prompt $TESTPOOL/fs" +mntpt=$(get_prop mountpoint $TESTPOOL/fs) +log_must mkfile 32M $mntpt/file1 + +log_must zinject -a -t data -e decrypt -f 20 $mntpt/file1 +log_must zfs umount $TESTPOOL/fs +log_must zfs mount $TESTPOOL/fs + +log_mustnot eval "cat $mntpt/file1 > /dev/null" +log_must eval "zpool events $TESTPOOL | grep -q 'authentication'" + +log_pass "Injected decryption errors are handled correctly"