From 4359973586f2a37f5509d04966024f8d034df142 Mon Sep 17 00:00:00 2001 From: Milan Broz Date: Sat, 13 Feb 2021 18:55:36 +0100 Subject: [PATCH] Fix dm-verity FEC calculation if stored in the same image with hashes. FEC (Forward Error Correction) data should cover the whole data area, hashes (Merkle tree) and optionally additional metadata (located after hash area). Unfortunately, if FEC data is stored in the same file as hash, the calculation wrongly used the whole file size thus overlaps with FEC area itself. This produces unusable and too large FEC data. (There is not a problem if FEC image is a separate image.) This patch fixes the problem, introducing FEC blocks calculation as: -If hash device is in a separate image, metadata covers the whole rest of the image after hash area. (Unchanged behaviour.) -If hash and FEC device is in the image, metadata ends on the FEC area offset. This should probably fix several issues reported with FEC wrong calculations. Fixes: #554 --- lib/libdevmapper.c | 8 +++--- lib/utils_dm.h | 7 ++--- lib/verity/verity.c | 2 +- lib/verity/verity.h | 4 +++ lib/verity/verity_fec.c | 59 ++++++++++++++++++++++++++++++---------- tests/verity-compat-test | 25 ++++++++++++----- 6 files changed, 75 insertions(+), 30 deletions(-) diff --git a/lib/libdevmapper.c b/lib/libdevmapper.c index 767be6d3..416fd367 100644 --- a/lib/libdevmapper.c +++ b/lib/libdevmapper.c @@ -727,7 +727,7 @@ static char *get_dm_verity_params(const struct dm_target *tgt, uint32_t flags) snprintf(fec_features, sizeof(fec_features)-1, " use_fec_from_device %s fec_start %" PRIu64 " fec_blocks %" PRIu64 " fec_roots %" PRIu32, device_block_path(tgt->u.verity.fec_device), tgt->u.verity.fec_offset, - vp->data_size + tgt->u.verity.hash_blocks, vp->fec_roots); + tgt->u.verity.fec_blocks, vp->fec_roots); } else *fec_features = '\0'; @@ -3021,8 +3021,8 @@ err: int dm_verity_target_set(struct dm_target *tgt, uint64_t seg_offset, uint64_t seg_size, struct device *data_device, struct device *hash_device, struct device *fec_device, - const char *root_hash, uint32_t root_hash_size, const char *root_hash_sig_key_desc, - uint64_t hash_offset_block, uint64_t hash_blocks, struct crypt_params_verity *vp) + const char *root_hash, uint32_t root_hash_size, const char* root_hash_sig_key_desc, + uint64_t hash_offset_block, uint64_t fec_blocks, struct crypt_params_verity *vp) { if (!data_device || !hash_device || !vp) return -EINVAL; @@ -3040,7 +3040,7 @@ int dm_verity_target_set(struct dm_target *tgt, uint64_t seg_offset, uint64_t se tgt->u.verity.root_hash_sig_key_desc = root_hash_sig_key_desc; tgt->u.verity.hash_offset = hash_offset_block; tgt->u.verity.fec_offset = vp->fec_area_offset / vp->hash_block_size; - tgt->u.verity.hash_blocks = hash_blocks; + tgt->u.verity.fec_blocks = fec_blocks; tgt->u.verity.vp = vp; return 0; diff --git a/lib/utils_dm.h b/lib/utils_dm.h index cf07e0b0..c47219d0 100644 --- a/lib/utils_dm.h +++ b/lib/utils_dm.h @@ -122,9 +122,8 @@ struct dm_target { const char *root_hash_sig_key_desc; uint64_t hash_offset; /* hash offset in blocks (not header) */ - uint64_t hash_blocks; /* size of hash device (in hash blocks) */ uint64_t fec_offset; /* FEC offset in blocks (not header) */ - uint64_t fec_blocks; /* size of FEC device (in hash blocks) */ + uint64_t fec_blocks; /* FEC blocks covering data + hash + padding (foreign metadata)*/ struct crypt_params_verity *vp; } verity; struct { @@ -191,8 +190,8 @@ int dm_crypt_target_set(struct dm_target *tgt, uint64_t seg_offset, uint64_t seg uint32_t tag_size, uint32_t sector_size); int dm_verity_target_set(struct dm_target *tgt, uint64_t seg_offset, uint64_t seg_size, struct device *data_device, struct device *hash_device, struct device *fec_device, - const char *root_hash, uint32_t root_hash_size, const char *root_hash_sig_key_desc, - uint64_t hash_offset_block, uint64_t hash_blocks, struct crypt_params_verity *vp); + const char *root_hash, uint32_t root_hash_size, const char* root_hash_sig_key_desc, + uint64_t hash_offset_block, uint64_t fec_blocks, struct crypt_params_verity *vp); int dm_integrity_target_set(struct crypt_device *cd, struct dm_target *tgt, uint64_t seg_offset, uint64_t seg_size, struct device *meta_device, diff --git a/lib/verity/verity.c b/lib/verity/verity.c index 4d197847..d0295103 100644 --- a/lib/verity/verity.c +++ b/lib/verity/verity.c @@ -310,7 +310,7 @@ int VERITY_activate(struct crypt_device *cd, crypt_metadata_device(cd), fec_device, root_hash, root_hash_size, signature_description, VERITY_hash_offset_block(verity_hdr), - VERITY_hash_blocks(cd, verity_hdr), verity_hdr); + VERITY_FEC_blocks(cd, fec_device, verity_hdr), verity_hdr); if (r) return r; diff --git a/lib/verity/verity.h b/lib/verity/verity.h index 335ec427..2269649b 100644 --- a/lib/verity/verity.h +++ b/lib/verity/verity.h @@ -71,6 +71,10 @@ uint64_t VERITY_hash_offset_block(struct crypt_params_verity *params); uint64_t VERITY_hash_blocks(struct crypt_device *cd, struct crypt_params_verity *params); +uint64_t VERITY_FEC_blocks(struct crypt_device *cd, + struct device *fec_device, + struct crypt_params_verity *params); + int VERITY_UUID_generate(struct crypt_device *cd, char **uuid_string); #endif diff --git a/lib/verity/verity_fec.c b/lib/verity/verity_fec.c index 220aeac9..f185574f 100644 --- a/lib/verity/verity_fec.c +++ b/lib/verity/verity_fec.c @@ -134,8 +134,11 @@ static int FEC_process_inputs(struct crypt_device *cd, /* calculate the total area covered by error correction codes */ ctx.size = 0; - for (n = 0; n < ctx.ninputs; ++n) + for (n = 0; n < ctx.ninputs; ++n) { + log_dbg(cd, "FEC input %s, offset %" PRIu64 " [bytes], length %" PRIu64 " [bytes]", + device_path(ctx.inputs[n].device), ctx.inputs[n].start, ctx.inputs[n].count); ctx.size += ctx.inputs[n].count; + } /* each byte in a data block is covered by a different code */ ctx.blocks = FEC_div_round_up(ctx.size, ctx.block_size); @@ -203,8 +206,7 @@ int VERITY_FEC_process(struct crypt_device *cd, struct device *fec_device, int check_fec, unsigned int *errors) { - int r; - int fd = -1; + int r = -EIO, fd = -1; struct fec_input_device inputs[FEC_INPUT_DEVICES] = { { .device = crypt_data_device(cd), @@ -214,7 +216,8 @@ int VERITY_FEC_process(struct crypt_device *cd, },{ .device = crypt_metadata_device(cd), .fd = -1, - .start = VERITY_hash_offset_block(params) * params->data_block_size + .start = VERITY_hash_offset_block(params) * params->data_block_size, + .count = (VERITY_FEC_blocks(cd, fec_device, params) - params->data_size) * params->data_block_size } }; @@ -230,7 +233,10 @@ int VERITY_FEC_process(struct crypt_device *cd, return -EINVAL; } - r = -EIO; + if (!inputs[0].count || !inputs[1].count) { + log_err(cd, _("Invalid FEC segment length.")); + return -EINVAL; + } if (check_fec) fd = open(device_path(fec_device), O_RDONLY); @@ -259,15 +265,6 @@ int VERITY_FEC_process(struct crypt_device *cd, goto out; } - /* cover the entire hash device starting from hash_offset */ - r = device_size(inputs[1].device, &inputs[1].count); - if (r) { - log_err(cd, _("Failed to determine size for device %s."), - device_path(inputs[1].device)); - goto out; - } - inputs[1].count -= inputs[1].start; - r = FEC_process_inputs(cd, params, inputs, FEC_INPUT_DEVICES, fd, check_fec, errors); out: if (inputs[0].fd != -1) @@ -279,3 +276,37 @@ out: return r; } + +uint64_t VERITY_FEC_blocks(struct crypt_device *cd, + struct device *fec_device, + struct crypt_params_verity *params) +{ + uint64_t blocks = 0; + + /* + * FEC covers this data: + * | protected data | hash area | padding (optional foreign metadata) | + * + * If hash device is in a separate image, metadata covers the whole rest of the image after hash area. + * If hash and FEC device is in the image, metadata ends on the FEC area offset. + */ + if (device_is_identical(crypt_metadata_device(cd), fec_device) > 0) { + log_dbg(cd, "FEC and hash device is the same."); + blocks = params->fec_area_offset; + } else { + /* cover the entire hash device starting from hash_offset */ + if (device_size(crypt_metadata_device(cd), &blocks)) { + log_err(cd, _("Failed to determine size for device %s."), + device_path(crypt_metadata_device(cd))); + return 0; + } + } + + blocks /= params->data_block_size; + blocks -= VERITY_hash_offset_block(params); + + /* Protected data */ + blocks += params->data_size; + + return blocks; +} diff --git a/tests/verity-compat-test b/tests/verity-compat-test index f41a4f52..26df5805 100755 --- a/tests/verity-compat-test +++ b/tests/verity-compat-test @@ -254,14 +254,20 @@ function check_fec() HASH_REPAIRED=${ARR[0]} $VERITYSETUP close $DEV_NAME - rm $1 $2 $3 $IMG_TMP > /dev/null 2>&1 if [ "$HASH_ORIG" != "$HASH_REPAIRED" ]; then - echo -n "[correction failed]" - return 1 - fi - - echo "[file was repaired][OK]" + echo -n "[kernel correction failed]" + $VERITYSETUP verify $1 $2 $ROOT_HASH --fec-device=$3 $PARAMS >/dev/null 2>&1 && fail "Userspace verify should fail" + echo -n "[userspace verify failed]" + RET=1 + else + echo -n "[repaired in kernel]" + $VERITYSETUP verify $1 $2 $ROOT_HASH --fec-device=$3 $PARAMS >/dev/null 2>&1 || fail "Userspace verify failed" + echo "[userspace verify][OK]" + RET=0 + fi + rm $1 $2 $3 $IMG_TMP > /dev/null 2>&1 + return $RET } function check_option() # $1 size, $2 hash, $3 salt, $4 version, $5 hash, $6 CLI option, $7 status option @@ -477,6 +483,11 @@ if check_version 1 3; then [ "$RET" -eq "3" ] && break [ "$RET" -eq "0" ] || fail "FEC repair failed" + (check_fec $IMG $IMG $IMG 512 500 50000 2457600 4915200 $(($RANDOM % 23 + 2)) $(($INDEX * 4)) 'n' $SALT) || fail "FEC repair failed" + (check_fec $IMG $IMG $IMG 512 500 50000 2457600 4915200 $(($RANDOM % 23 + 2)) $(($INDEX * 4)) 'y' $SALT) || fail "FEC repair failed" + (check_fec $IMG $IMG $IMG 4096 64 6250 4194304 8388608 $(($RANDOM % 23 + 2)) $(($INDEX * 4)) 'n' $SALT) || fail "FEC repair failed" + (check_fec $IMG $IMG $IMG 4096 64 6250 4194304 8388608 $(($RANDOM % 23 + 2)) $(($INDEX * 4)) 'y' $SALT) || fail "FEC repair failed" + (check_fec $IMG $IMG_HASH $FEC_DEV 4096 30 30 0 0 $(($RANDOM % 23 + 2)) $(($INDEX * 4)) 'n' $SALT) || fail "FEC repair failed" (check_fec $IMG $IMG_HASH $FEC_DEV 4096 35 35 0 0 $(($RANDOM % 23 + 2)) $(($INDEX * 4))) || fail "FEC repair failed" (check_fec $IMG $IMG_HASH $FEC_DEV 512 2000 2000 0 0 $(($RANDOM % 23 + 2)) $(($INDEX * 4))) || fail "FEC repair failed" @@ -494,7 +505,7 @@ checkUserSpaceRepair 400 512 2 256000 0 2 50 checkUserSpaceRepair 500 512 2 2457600 4915200 1 1 checkUserSpaceRepair -1 4096 2 0 0 3 10 checkUserSpaceRepair 400 4096 2 2048000 0 2 1 -#checkUserSpaceRepair 500 4096 2 2457600 4915200 1 2 # FIXME +checkUserSpaceRepair 500 4096 2 2457600 4915200 1 2 echo -n "Verity concurrent opening tests:" prepare 8192 1024