The CONFIG_DM_VERITY_VERIFY_ROOTHASH_SIG introduced by Jaskaran was intended to be used to allow an LSM to enforce verifications for all dm-verity volumes. However, with it's current implementation, this signature verification occurs after the merkel-tree is validated, as a result the signature can pass initial verification by passing a matching root-hash and signature. This results in an unreadable block_device, but that has passed signature validation (and subsequently, would be marked as verified). This change moves the signature verification to after the merkel-tree has finished validation. Signed-off-by: Deven Bowers <deven.desai@xxxxxxxxxxxxxxxxxxx> --- drivers/md/dm-verity-target.c | 42 ++++----- drivers/md/dm-verity-verify-sig.c | 140 ++++++++++++++++++++++-------- drivers/md/dm-verity-verify-sig.h | 20 ++--- drivers/md/dm-verity.h | 2 +- 4 files changed, 132 insertions(+), 72 deletions(-) diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c index eec9f252e935..c507f3a4e237 100644 --- a/drivers/md/dm-verity-target.c +++ b/drivers/md/dm-verity-target.c @@ -471,9 +471,9 @@ static int verity_verify_io(struct dm_verity_io *io) struct bvec_iter start; unsigned b; struct crypto_wait wait; + int r; for (b = 0; b < io->n_blocks; b++) { - int r; sector_t cur_block = io->block + b; struct ahash_request *req = verity_io_hash_req(v, io); @@ -530,6 +530,16 @@ static int verity_verify_io(struct dm_verity_io *io) return -EIO; } + /* + * At this point, the merkel tree has finished validating. + * if signature was specified, validate the signature here. + */ + r = verity_verify_root_hash(v); + if (r < 0) { + DMERR_LIMIT("signature mismatch"); + return r; + } + return 0; } @@ -728,7 +738,7 @@ static void verity_status(struct dm_target *ti, status_type_t type, args++; if (v->validated_blocks) args++; - if (v->signature_key_desc) + if (v->sig) args += DM_VERITY_ROOT_HASH_VERIFICATION_OPTS; if (!args) return; @@ -751,9 +761,9 @@ static void verity_status(struct dm_target *ti, status_type_t type, if (v->validated_blocks) DMEMIT(" " DM_VERITY_OPT_AT_MOST_ONCE); sz = verity_fec_status_table(v, sz, result, maxlen); - if (v->signature_key_desc) + if (v->sig) DMEMIT(" " DM_VERITY_ROOT_HASH_VERIFICATION_OPT_SIG_KEY - " %s", v->signature_key_desc); + " %s", v->sig->signature_key_desc); break; } } @@ -819,7 +829,7 @@ static void verity_dtr(struct dm_target *ti) verity_fec_dtr(v); - kfree(v->signature_key_desc); + verity_verify_dtr(v); kfree(v); } @@ -876,8 +886,7 @@ static int verity_alloc_zero_digest(struct dm_verity *v) return r; } -static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v, - struct dm_verity_sig_opts *verify_args) +static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v) { int r; unsigned argc; @@ -927,9 +936,7 @@ static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v, return r; continue; } else if (verity_verify_is_sig_opt_arg(arg_name)) { - r = verity_verify_sig_parse_opt_args(as, v, - verify_args, - &argc, arg_name); + r = verity_verify_sig_parse_opt_args(as, v, &argc); if (r) return r; continue; @@ -960,7 +967,6 @@ static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v, static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv) { struct dm_verity *v; - struct dm_verity_sig_opts verify_args = {0}; struct dm_arg_set as; unsigned int num; unsigned long long num_ll; @@ -1128,20 +1134,11 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv) as.argc = argc; as.argv = argv; - r = verity_parse_opt_args(&as, v, &verify_args); + r = verity_parse_opt_args(&as, v); if (r < 0) goto bad; } - /* Root hash signature is a optional parameter*/ - r = verity_verify_root_hash(root_hash_digest_to_validate, - strlen(root_hash_digest_to_validate), - verify_args.sig, - verify_args.sig_size); - if (r < 0) { - ti->error = "Root hash verification failed"; - goto bad; - } v->hash_per_block_bits = __fls((1 << v->hash_dev_block_bits) / v->digest_size); @@ -1207,13 +1204,10 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv) ti->per_io_data_size = roundup(ti->per_io_data_size, __alignof__(struct dm_verity_io)); - verity_verify_sig_opts_cleanup(&verify_args); - return 0; bad: - verity_verify_sig_opts_cleanup(&verify_args); verity_dtr(ti); return r; diff --git a/drivers/md/dm-verity-verify-sig.c b/drivers/md/dm-verity-verify-sig.c index 614e43db93aa..27dac8aa2e5a 100644 --- a/drivers/md/dm-verity-verify-sig.c +++ b/drivers/md/dm-verity-verify-sig.c @@ -22,6 +22,16 @@ MODULE_PARM_DESC(require_signatures, #define DM_VERITY_IS_SIG_FORCE_ENABLED() \ (require_signatures != false) +static void destroy_verity_sig(struct dm_verity_sig *sig_info) +{ + if (!sig_info) + return; + + kfree(sig_info->sig); + kfree(sig_info->signature_key_desc); + kfree(sig_info); +} + bool verity_verify_is_sig_opt_arg(const char *arg_name) { return (!strcasecmp(arg_name, @@ -29,7 +39,7 @@ bool verity_verify_is_sig_opt_arg(const char *arg_name) } static int verity_verify_get_sig_from_key(const char *key_desc, - struct dm_verity_sig_opts *sig_opts) + struct dm_verity_sig *sig_info) { struct key *key; const struct user_key_payload *ukp; @@ -48,14 +58,14 @@ static int verity_verify_get_sig_from_key(const char *key_desc, goto end; } - sig_opts->sig = kmalloc(ukp->datalen, GFP_KERNEL); - if (!sig_opts->sig) { + sig_info->sig = kmalloc(ukp->datalen, GFP_KERNEL); + if (!sig_info->sig) { ret = -ENOMEM; goto end; } - sig_opts->sig_size = ukp->datalen; + sig_info->sig_size = ukp->datalen; - memcpy(sig_opts->sig, ukp->data, sig_opts->sig_size); + memcpy(sig_info->sig, ukp->data, sig_info->sig_size); end: up_read(&key->sem); @@ -64,70 +74,128 @@ static int verity_verify_get_sig_from_key(const char *key_desc, return ret; } +/** + * Parse any signature verification arguments. + * This function will populate v->sig, it is the caller's + * responsibility to free this structure via verity_verify_dtr + * + * @as: argument set passed in to parse + * @v: verity context structure. Should have a NULL v->sig member. + * @argc: current argument number + */ int verity_verify_sig_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v, - struct dm_verity_sig_opts *sig_opts, - unsigned int *argc, - const char *arg_name) + unsigned int *argc) { struct dm_target *ti = v->ti; + struct dm_verity_sig *sig_info = NULL; int ret = 0; const char *sig_key = NULL; if (!*argc) { ti->error = DM_VERITY_VERIFY_ERR("Signature key not specified"); - return -EINVAL; + ret = -EINVAL; + goto cleanup; + } + + sig_info = kzalloc(sizeof(*sig_info), GFP_KERNEL); + if (!sig_info) { + ret = -ENOMEM; + goto cleanup; } sig_key = dm_shift_arg(as); (*argc)--; - ret = verity_verify_get_sig_from_key(sig_key, sig_opts); - if (ret < 0) + ret = verity_verify_get_sig_from_key(sig_key, sig_info); + if (ret < 0) { ti->error = DM_VERITY_VERIFY_ERR("Invalid key specified"); + goto cleanup; + } - v->signature_key_desc = kstrdup(sig_key, GFP_KERNEL); - if (!v->signature_key_desc) - return -ENOMEM; + sig_info->signature_key_desc = kstrdup(sig_key, GFP_KERNEL); + if (!sig_info->signature_key_desc) { + ret = -ENOMEM; + goto cleanup; + } + v->sig = sig_info; + sig_info = NULL; +cleanup: + if (sig_info) + destroy_verity_sig(sig_info); return ret; } -/* +/** * verify_verify_roothash - Verify the root hash of the verity hash device * using builtin trusted keys. * - * @root_hash: For verity, the roothash/data to be verified. - * @root_hash_len: Size of the roothash/data to be verified. - * @sig_data: The trusted signature that verifies the roothash/data. - * @sig_len: Size of the signature. + * @v: dm_verity structure containing all context for the dm_verity + * operation. * */ -int verity_verify_root_hash(const void *root_hash, size_t root_hash_len, - const void *sig_data, size_t sig_len) +int verity_verify_root_hash(const struct dm_verity *v) { - int ret; + int ret = 0; + char *root_hash = NULL; + size_t root_hash_size = 0; + struct dm_verity_sig *sig_target = NULL; + + if (!v || !v->ti || !v->root_digest || v->digest_size == 0) { + ret = -EINVAL; + goto cleanup; + } + + sig_target = v->sig; + + if (!sig_target || !sig_target->sig || sig_target->sig_size == 0) { + if (DM_VERITY_IS_SIG_FORCE_ENABLED()) { + ret = -ENOKEY; + goto cleanup; + } else { + goto cleanup; + } + } - if (!root_hash || root_hash_len == 0) - return -EINVAL; + /* + * If signature has passed validation once, assume + * that future signatures will pass. + */ + if (sig_target->passed) + goto cleanup; - if (!sig_data || sig_len == 0) { - if (DM_VERITY_IS_SIG_FORCE_ENABLED()) - return -ENOKEY; - else - return 0; + root_hash_size = v->digest_size * 2; + root_hash = kzalloc(root_hash_size, GFP_KERNEL); + if (!root_hash) { + ret = -ENOMEM; + goto cleanup; } - ret = verify_pkcs7_signature(root_hash, root_hash_len, sig_data, - sig_len, NULL, VERIFYING_UNSPECIFIED_SIGNATURE, - NULL, NULL); + bin2hex(root_hash, v->root_digest, v->digest_size); + + ret = verify_pkcs7_signature(root_hash, root_hash_size, v->sig->sig, + v->sig->sig_size, NULL, + VERIFYING_UNSPECIFIED_SIGNATURE, NULL, + NULL); + if (ret != 0) + goto cleanup; + sig_target->passed = true; +cleanup: + kfree(root_hash); return ret; } -void verity_verify_sig_opts_cleanup(struct dm_verity_sig_opts *sig_opts) +/** + * Performs destruction / cleanup of a valid dm_verity_sig struct + * + * @v: dm_verity structure containing the dm_verity_sig struct to + * be freed. + */ + +void verity_verify_dtr(struct dm_verity *v) { - kfree(sig_opts->sig); - sig_opts->sig = NULL; - sig_opts->sig_size = 0; + destroy_verity_sig(v->sig); + v->sig = NULL; } diff --git a/drivers/md/dm-verity-verify-sig.h b/drivers/md/dm-verity-verify-sig.h index 19b1547aa741..81158d7702a3 100644 --- a/drivers/md/dm-verity-verify-sig.h +++ b/drivers/md/dm-verity-verify-sig.h @@ -11,31 +11,30 @@ #define DM_VERITY_ROOT_HASH_VERIFICATION "DM Verity Sig Verification" #define DM_VERITY_ROOT_HASH_VERIFICATION_OPT_SIG_KEY "root_hash_sig_key_desc" -struct dm_verity_sig_opts { +struct dm_verity_sig { + char *signature_key_desc; unsigned int sig_size; u8 *sig; + bool passed; }; #ifdef CONFIG_DM_VERITY_VERIFY_ROOTHASH_SIG #define DM_VERITY_ROOT_HASH_VERIFICATION_OPTS 2 -int verity_verify_root_hash(const void *data, size_t data_len, - const void *sig_data, size_t sig_len); +int verity_verify_root_hash(const struct dm_verity *v); bool verity_verify_is_sig_opt_arg(const char *arg_name); int verity_verify_sig_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v, - struct dm_verity_sig_opts *sig_opts, - unsigned int *argc, const char *arg_name); + unsigned int *argc); -void verity_verify_sig_opts_cleanup(struct dm_verity_sig_opts *sig_opts); +void verity_verify_dtr(struct dm_verity *v); #else #define DM_VERITY_ROOT_HASH_VERIFICATION_OPTS 0 -int verity_verify_root_hash(const void *data, size_t data_len, - const void *sig_data, size_t sig_len) +int verity_verify_root_hash(const struct dm_verity *v) { return 0; } @@ -46,13 +45,12 @@ bool verity_verify_is_sig_opt_arg(const char *arg_name) } int verity_verify_sig_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v, - struct dm_verity_sig_opts *sig_opts, - unsigned int *argc, const char *arg_name) + unsigned int *argc) { return -EINVAL; } -void verity_verify_sig_opts_cleanup(struct dm_verity_sig_opts *sig_opts) +void verity_verify_dtr(struct dm_verity *v) { } diff --git a/drivers/md/dm-verity.h b/drivers/md/dm-verity.h index 641b9e3a399b..995c495decad 100644 --- a/drivers/md/dm-verity.h +++ b/drivers/md/dm-verity.h @@ -64,7 +64,7 @@ struct dm_verity { struct dm_verity_fec *fec; /* forward error correction */ unsigned long *validated_blocks; /* bitset blocks validated */ - char *signature_key_desc; /* signature keyring reference */ + struct dm_verity_sig *sig; /* signature verification */ }; struct dm_verity_io { -- 2.27.0 -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel