From 0a6f89cfa6f343caa90828f35c3b00d32d6014c5 Mon Sep 17 00:00:00 2001 From: Milan Broz Date: Sat, 23 Jan 2021 20:08:19 +0100 Subject: [PATCH] Fix dm-integrity HMAC recalculation problem. This patch adds support for Linux kernel (since version 5.11) dm-integrity fixes that disables integrity recalculation if keyed algorithms (HMAC) is used. Original dm-integrity superblock version <=4 is recalculation offset field not protected by HMAC. An attacker can move this pointer and force the kernel to recalculate the data area, ignoring original HMAC tags. N.B. dm-integrity was not intended to protect against intentional changes. Better use authenticated encryption (AEAD) in combination with dm-crypt. It is designed to protect against random data corruption caused by hardware or storage medium faults. Despite that, we try to keep the system secure if keyed algorithms are used. There are two possible keyed algorithms in dm-integrity - algorithm used to protect journal and superblock (--journal-integrity) and algorithms for protecting data (--integrity). The dm-integrity superblock is guarded by --journal-integrity, so if you want to protect data with HMAC, you should always also use HMAC for --journal-integrity. The keys are independent. If HMAC is used for data but not for the journal, recalculation is disabled by default. For new kernel dm-integrity, the HMAC option also uses salt in superblock to avoid an easy way to distinguish that the HMAC key is the same for two devices (if data are the same). The new HMAC and superblock are enabled automatically if the kernel supports it (you can see superblock version 5 and fix_hmac flag in dump command). If you need to use (insecure) backward compatibility, then two new integritysetup options are introduced: Use --integrity-legacy-recalc (instead of --integrity-recalc) to allow recalculation on legacy devices. Use --integrity-legacy-hmac in format action to force old insecure version format (with HMAC). Libcryptsetup API also introduces flags CRYPT_COMPAT_LEGACY_INTEGRITY_HMAC and CRYPT_COMPAT_LEGACY_INTEGRITY_RECALC to set these through crypt_set_compatibility() call. --- lib/integrity/integrity.c | 16 +++++++++++++--- lib/integrity/integrity.h | 2 ++ lib/libcryptsetup.h | 4 ++++ lib/libdevmapper.c | 29 ++++++++++++++++++++++++++++ lib/utils_dm.h | 3 +++ man/integritysetup.8 | 15 +++++++++++++++ src/integritysetup.c | 13 ++++++++++++- tests/integrity-compat-test | 38 ++++++++++++++++++++++++++++++++++++- 8 files changed, 115 insertions(+), 5 deletions(-) diff --git a/lib/integrity/integrity.c b/lib/integrity/integrity.c index 42e89c10..15005058 100644 --- a/lib/integrity/integrity.c +++ b/lib/integrity/integrity.c @@ -40,7 +40,7 @@ static int INTEGRITY_read_superblock(struct crypt_device *cd, if (read_lseek_blockwise(devfd, device_block_size(cd, device), device_alignment(device), sb, sizeof(*sb), offset) != sizeof(*sb) || memcmp(sb->magic, SB_MAGIC, sizeof(sb->magic)) || - sb->version < SB_VERSION_1 || sb->version > SB_VERSION_4) { + sb->version < SB_VERSION_1 || sb->version > SB_VERSION_5) { log_std(cd, "No integrity superblock detected on %s.\n", device_path(device)); r = -EINVAL; @@ -95,11 +95,12 @@ int INTEGRITY_dump(struct crypt_device *cd, struct device *device, uint64_t offs if (sb.version >= SB_VERSION_2 && (sb.flags & SB_FLAG_RECALCULATING)) log_std(cd, "recalc_sector %" PRIu64 "\n", sb.recalc_sector); log_std(cd, "log2_blocks_per_bitmap %u\n", sb.log2_blocks_per_bitmap_bit); - log_std(cd, "flags %s%s%s%s\n", + log_std(cd, "flags %s%s%s%s%s\n", sb.flags & SB_FLAG_HAVE_JOURNAL_MAC ? "have_journal_mac " : "", sb.flags & SB_FLAG_RECALCULATING ? "recalculating " : "", sb.flags & SB_FLAG_DIRTY_BITMAP ? "dirty_bitmap " : "", - sb.flags & SB_FLAG_FIXED_PADDING ? "fix_padding " : ""); + sb.flags & SB_FLAG_FIXED_PADDING ? "fix_padding " : "", + sb.flags & SB_FLAG_FIXED_HMAC ? "fix_hmac " : ""); return 0; } @@ -278,6 +279,15 @@ int INTEGRITY_activate_dmd_device(struct crypt_device *cd, return -ENOTSUP; } + if (r < 0 && (dmd->flags & CRYPT_ACTIVATE_RECALCULATE) && + !(crypt_get_compatibility(cd) & CRYPT_COMPAT_LEGACY_INTEGRITY_RECALC) && + (sb_flags & SB_FLAG_FIXED_HMAC) ? + (tgt->u.integrity.vk && !tgt->u.integrity.journal_integrity_key) : + (tgt->u.integrity.vk || tgt->u.integrity.journal_integrity_key)) { + log_err(cd, _("Kernel refuses to activate insecure recalculate option (see legacy activation options to override).")); + return -ENOTSUP; + } + return r; } diff --git a/lib/integrity/integrity.h b/lib/integrity/integrity.h index bfdfe58a..1f496120 100644 --- a/lib/integrity/integrity.h +++ b/lib/integrity/integrity.h @@ -35,11 +35,13 @@ struct crypt_dm_active_device; #define SB_VERSION_2 2 #define SB_VERSION_3 3 #define SB_VERSION_4 4 +#define SB_VERSION_5 5 #define SB_FLAG_HAVE_JOURNAL_MAC (1 << 0) #define SB_FLAG_RECALCULATING (1 << 1) /* V2 only */ #define SB_FLAG_DIRTY_BITMAP (1 << 2) /* V3 only */ #define SB_FLAG_FIXED_PADDING (1 << 3) /* V4 only */ +#define SB_FLAG_FIXED_HMAC (1 << 4) /* V5 only */ struct superblock { uint8_t magic[8]; diff --git a/lib/libcryptsetup.h b/lib/libcryptsetup.h index 09f5b8a8..4a956dcc 100644 --- a/lib/libcryptsetup.h +++ b/lib/libcryptsetup.h @@ -652,6 +652,10 @@ uint32_t crypt_get_compatibility(struct crypt_device *cd); /** dm-integrity device uses less effective (legacy) padding (old kernels) */ #define CRYPT_COMPAT_LEGACY_INTEGRITY_PADDING (1 << 0) +/** dm-integrity device does not protect superblock with HMAC (old kernels) */ +#define CRYPT_COMPAT_LEGACY_INTEGRITY_HMAC (1 << 1) +/** dm-integrity allow recalculating of volumes with HMAC keys (old kernels) */ +#define CRYPT_COMPAT_LEGACY_INTEGRITY_RECALC (1 << 2) /** * Convert to new type for already existing device. diff --git a/lib/libdevmapper.c b/lib/libdevmapper.c index bd5fb01c..72bf4952 100644 --- a/lib/libdevmapper.c +++ b/lib/libdevmapper.c @@ -239,6 +239,9 @@ static void _dm_set_integrity_compat(struct crypt_device *cd, if (_dm_satisfies_version(1, 6, 0, integrity_maj, integrity_min, integrity_patch)) _dm_flags |= DM_INTEGRITY_DISCARDS_SUPPORTED; + if (_dm_satisfies_version(1, 7, 0, integrity_maj, integrity_min, integrity_patch)) + _dm_flags |= DM_INTEGRITY_FIX_HMAC_SUPPORTED; + _dm_integrity_checked = true; } @@ -912,12 +915,25 @@ static char *get_dm_integrity_params(const struct dm_target *tgt, uint32_t flags strncat(features, feature, sizeof(features) - strlen(features) - 1); crypt_safe_free(hexkey); } + if (tgt->u.integrity.fix_padding) { num_options++; snprintf(feature, sizeof(feature), "fix_padding "); strncat(features, feature, sizeof(features) - strlen(features) - 1); } + if (tgt->u.integrity.fix_hmac) { + num_options++; + snprintf(feature, sizeof(feature), "fix_hmac "); + strncat(features, feature, sizeof(features) - strlen(features) - 1); + } + + if (tgt->u.integrity.legacy_recalc) { + num_options++; + snprintf(feature, sizeof(feature), "legacy_recalculate "); + strncat(features, feature, sizeof(features) - strlen(features) - 1); + } + if (flags & CRYPT_ACTIVATE_RECALCULATE) { num_options++; snprintf(feature, sizeof(feature), "recalculate "); @@ -2474,6 +2490,10 @@ static int _dm_target_query_integrity(struct crypt_device *cd, *act_flags |= CRYPT_ACTIVATE_RECALCULATE; } else if (!strcmp(arg, "fix_padding")) { tgt->u.integrity.fix_padding = true; + } else if (!strcmp(arg, "fix_hmac")) { + tgt->u.integrity.fix_hmac = true; + } else if (!strcmp(arg, "legacy_recalculate")) { + tgt->u.integrity.legacy_recalc = true; } else if (!strcmp(arg, "allow_discards")) { *act_flags |= CRYPT_ACTIVATE_ALLOW_DISCARDS; } else /* unknown option */ @@ -3060,6 +3080,15 @@ int dm_integrity_target_set(struct crypt_device *cd, !(crypt_get_compatibility(cd) & CRYPT_COMPAT_LEGACY_INTEGRITY_PADDING)) tgt->u.integrity.fix_padding = true; + if (!dm_flags(cd, DM_INTEGRITY, &dmi_flags) && + (dmi_flags & DM_INTEGRITY_FIX_HMAC_SUPPORTED) && + !(crypt_get_compatibility(cd) & CRYPT_COMPAT_LEGACY_INTEGRITY_HMAC)) + tgt->u.integrity.fix_hmac = true; + + /* This flag can be backported, just try to set it always */ + if (crypt_get_compatibility(cd) & CRYPT_COMPAT_LEGACY_INTEGRITY_RECALC) + tgt->u.integrity.legacy_recalc = true; + if (ip) { tgt->u.integrity.journal_size = ip->journal_size; tgt->u.integrity.journal_watermark = ip->journal_watermark; diff --git a/lib/utils_dm.h b/lib/utils_dm.h index 3c628a81..9a52aeba 100644 --- a/lib/utils_dm.h +++ b/lib/utils_dm.h @@ -72,6 +72,7 @@ static inline uint32_t act2dmflags(uint32_t act_flags) #define DM_INTEGRITY_DISCARDS_SUPPORTED (1 << 23) /* dm-integrity discards/TRIM option is supported */ #define DM_VERITY_PANIC_CORRUPTION_SUPPORTED (1 << 24) /* dm-verity panic on corruption */ #define DM_CRYPT_NO_WORKQUEUE_SUPPORTED (1 << 25) /* dm-crypt suppot for bypassing workqueues */ +#define DM_INTEGRITY_FIX_HMAC_SUPPORTED (1 << 26) /* hmac covers also superblock */ typedef enum { DM_CRYPT = 0, DM_VERITY, DM_INTEGRITY, DM_LINEAR, DM_ERROR, DM_ZERO, DM_UNKNOWN } dm_target_type; enum tdirection { TARGET_SET = 1, TARGET_QUERY }; @@ -149,6 +150,8 @@ struct dm_target { struct device *meta_device; bool fix_padding; + bool fix_hmac; + bool legacy_recalc; } integrity; struct { uint64_t offset; diff --git a/man/integritysetup.8 b/man/integritysetup.8 index 4e047aeb..a115501c 100644 --- a/man/integritysetup.8 +++ b/man/integritysetup.8 @@ -184,6 +184,21 @@ The dm-integrity target is available since Linux kernel version 4.12. Format and activation of an integrity device always require superuser privilege because the superblock is calculated and handled in dm-integrity kernel target. +.SH LEGACY COMPATIBILITY OPTIONS +.TP +\fBWARNING:\fR +Do not use these options until you need compatibility with specific old kernel. +.TP +.B "\-\-integrity\-legacy\-padding" +Use inefficient legacy padding. +.TP +.B "\-\-integrity\-legacy\-hmac" +Use old flawed HMAC calclation (also does not protect superblock). +.TP +.B "\-\-integrity\-legacy\-recalculate" +Allow insecure recalculating of volumes with HMAC keys (recalcualtion offset in superblock +is not protected). + .SH RETURN CODES Integritysetup returns 0 on success and a non-zero value on error. diff --git a/src/integritysetup.c b/src/integritysetup.c index 27dfc10a..7cec07e1 100644 --- a/src/integritysetup.c +++ b/src/integritysetup.c @@ -56,6 +56,8 @@ static int opt_integrity_nojournal = 0; static int opt_integrity_recovery = 0; static int opt_integrity_bitmap = 0; static int opt_integrity_legacy_padding = 0; +static int opt_integrity_legacy_hmac = 0; +static int opt_integrity_legacy_recalculate = 0; static int opt_integrity_recalculate = 0; static int opt_allow_discards = 0; @@ -254,6 +256,9 @@ static int action_format(int arg) if (opt_integrity_legacy_padding) crypt_set_compatibility(cd, CRYPT_COMPAT_LEGACY_INTEGRITY_PADDING); + if (opt_integrity_legacy_hmac) + crypt_set_compatibility(cd, CRYPT_COMPAT_LEGACY_INTEGRITY_HMAC); + r = crypt_format(cd, CRYPT_INTEGRITY, NULL, NULL, NULL, NULL, 0, ¶ms); if (r < 0) /* FIXME: call wipe signatures again */ goto out; @@ -319,7 +324,7 @@ static int action_open(int arg) if (opt_integrity_bitmap) activate_flags |= CRYPT_ACTIVATE_NO_JOURNAL_BITMAP; - if (opt_integrity_recalculate) + if (opt_integrity_recalculate || opt_integrity_legacy_recalculate) activate_flags |= CRYPT_ACTIVATE_RECALCULATE; if (opt_allow_discards) activate_flags |= CRYPT_ACTIVATE_ALLOW_DISCARDS; @@ -335,6 +340,9 @@ static int action_open(int arg) if (r) goto out; + if (opt_integrity_legacy_recalculate) + crypt_set_compatibility(cd, CRYPT_COMPAT_LEGACY_INTEGRITY_RECALC); + r = crypt_activate_by_volume_key(cd, action_argv[1], integrity_key, opt_integrity_key_size, activate_flags); out: @@ -584,6 +592,9 @@ int main(int argc, const char **argv) { "integrity-recalculate", '\0', POPT_ARG_NONE, &opt_integrity_recalculate, 0, N_("Recalculate initial tags automatically."), NULL }, { "integrity-legacy-padding", '\0', POPT_ARG_NONE, &opt_integrity_legacy_padding, 0, N_("Use inefficient legacy padding (old kernels)"), NULL }, + { "integrity-legacy-hmac", '\0', POPT_ARG_NONE, &opt_integrity_legacy_hmac, 0, N_("Do not protect superblock with HMAC (old kernels)"), NULL }, + { "integrity-legacy-recalculate",'\0',POPT_ARG_NONE, &opt_integrity_legacy_recalculate, 0, N_("Allow recalculating of volumes with HMAC keys (old kernels)"), NULL }, + { "allow-discards", '\0', POPT_ARG_NONE, &opt_allow_discards, 0, N_("Allow discards (aka TRIM) requests for device"), NULL }, POPT_TABLEEND }; diff --git a/tests/integrity-compat-test b/tests/integrity-compat-test index 0329d0a0..e1e79316 100755 --- a/tests/integrity-compat-test +++ b/tests/integrity-compat-test @@ -14,6 +14,7 @@ DEV_LOOP="" DEV=test123.img DEV2=test124.img KEY_FILE=key.img +KEY_FILE2=key2.img dmremove() { # device udevadm settle >/dev/null 2>&1 @@ -25,7 +26,7 @@ cleanup() { [ -b /dev/mapper/$DEV_NAME_BIG ] && dmremove $DEV_NAME_BIG [ -n "$DEV_LOOP" ] && losetup -d "$DEV_LOOP" DEV_LOOP="" - rm -f $DEV $DEV2 $KEY_FILE >/dev/null 2>&1 + rm -f $DEV $DEV2 $KEY_FILE $KEY_FILE2 >/dev/null 2>&1 } fail() @@ -60,11 +61,15 @@ function dm_integrity_features() [ $VER_MIN -gt 2 ] && { DM_INTEGRITY_BITMAP=1 } + [ $VER_MIN -gt 6 ] && { + DM_INTEGRITY_HMAC_FIX=1 + } } add_device() { cleanup dd if=/dev/urandom of=$KEY_FILE bs=1 count=512 >/dev/null 2>&1 + dd if=/dev/urandom of=$KEY_FILE2 bs=1 count=32 >/dev/null 2>&1 dd if=/dev/zero of=$DEV bs=1M count=32 >/dev/null 2>&1 dd if=/dev/zero of=$DEV2 bs=1M count=32 >/dev/null 2>&1 sync @@ -436,4 +441,35 @@ else echo "[N/A]" fi +echo -n "Fixed HMAC and legacy flags:" +if [ -n "$DM_INTEGRITY_HMAC_FIX" ] ; then + add_device + # only data HMAC + ARGS="--integrity hmac-sha256 --integrity-key-file $KEY_FILE --integrity-key-size 32" + $INTSETUP format -q $DEV --integrity-legacy-hmac --no-wipe --tag-size 32 $ARGS || fail "Cannot format device." + $INTSETUP open $DEV $DEV_NAME --integrity-recalculate $ARGS >/dev/null 2>&1 && fail "Cannot activate device." + $INTSETUP open $DEV $DEV_NAME --integrity-legacy-recalculate $ARGS || fail "Cannot activate device." + $INTSETUP close $DEV_NAME || fail "Cannot deactivate device." + # New version - must fail (no journal HMAC) + $INTSETUP format -q $DEV --no-wipe --tag-size 32 $ARGS || fail "Cannot format device." + $INTSETUP open $DEV $DEV_NAME --integrity-recalculate $ARGS >/dev/null 2>&1 && fail "Cannot activate device." + $INTSETUP open $DEV $DEV_NAME --integrity-legacy-recalculate $ARGS || fail "Cannot activate device." + $INTSETUP close $DEV_NAME || fail "Cannot deactivate device." + + # data and journal HMAC + ARGS="$ARGS --journal-integrity hmac-sha256 --journal-integrity-key-file $KEY_FILE2 --journal-integrity-key-size 32" + $INTSETUP format -q $DEV --integrity-legacy-hmac --no-wipe --tag-size 32 $ARGS || fail "Cannot format device." + $INTSETUP open $DEV $DEV_NAME --integrity-recalculate $ARGS >/dev/null 2>&1 && fail "Cannot activate device." + $INTSETUP open $DEV $DEV_NAME --integrity-legacy-recalculate $ARGS || fail "Cannot activate device." + $INTSETUP close $DEV_NAME || fail "Cannot deactivate device." + # New fixed version + $INTSETUP format -q $DEV --no-wipe --tag-size 32 $ARGS || fail "Cannot format device." + $INTSETUP dump $DEV | grep "flags" | grep -q "fix_hmac" || fail "Flag for HMAC not set." + $INTSETUP open $DEV $DEV_NAME --integrity-recalculate $ARGS || fail "Cannot activate device." + $INTSETUP close $DEV_NAME || fail "Cannot deactivate device." + echo "[OK]" +else + echo "[N/A]" +fi + cleanup