lkml.org 
[lkml]   [2020]   [Apr]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[RFC PATCH v2 06/12] dm-verity: move signature check after tree validation
    Date
    From: Deven Bowers <deven.desai@linux.microsoft.com>

    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@linux.microsoft.com>
    ---
    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..9d7b34672c80 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.26.0
    \
     
     \ /
      Last update: 2020-04-07 00:16    [W:2.631 / U:0.860 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site