On Mon, 2023-01-30 at 14:57 -0800, Fan Wu wrote:
> Enable IPE policy authors to indicate trust for a singular fsverity
> file, identified by the digest information, through "fsverity_digest"
> and all files using fsverity's builtin signatures via
> "fsverity_signature".
> 
> This enables file-level integrity claims to be expressed in IPE,
> allowing individual files to be authorized, giving some flexibility
> for policy authors. Such file-level claims are important to be expressed
> for enforcing the integrity of packages, as well as address some of the
> scalability issues in a sole dm-verity based solution (# of loop back
> devices, etc).
> 
> This solution cannot be done in userspace as the minimum threat that
> IPE should mitigate is an attacker downloads malicious payload with
> all required dependencies. These dependencies can lack the userspace
> check, bypassing the protection entirely. A similar attack succeeds if
> the userspace component is replaced with a version that does not
> perform the check. As a result, this can only be done in the common
> entry point - the kernel.
> 
> Signed-off-by: Deven Bowers <deven.de...@linux.microsoft.com>
> Signed-off-by: Fan Wu <wu...@linux.microsoft.com>
> ---
> v1-v6:
>   + Not present
> 
> v7:
>   Introduced
> 
> v8:
>   * Undo squash of 08/12, 10/12 - separating drivers/md/ from security/
>   * Use common-audit function for fsverity_signature.
>   + Change fsverity implementation to use fsverity_get_digest
>   + prevent unnecessary copy of fs-verity signature data, instead
>     just check for presence of signature data.
>   + Remove free_inode_security hook, as the digest is now acquired
>     at runtime instead of via LSM blob.
> 
> v9:
>   + Adapt to the new parser
> ---
>  security/ipe/Kconfig         |  11 ++++
>  security/ipe/audit.c         |  23 +++++++
>  security/ipe/eval.c          | 112 +++++++++++++++++++++++++++++++++++
>  security/ipe/eval.h          |  10 ++++
>  security/ipe/hooks.c         |  30 ++++++++++
>  security/ipe/hooks.h         |   7 +++
>  security/ipe/ipe.c           |  13 ++++
>  security/ipe/ipe.h           |   3 +
>  security/ipe/policy.h        |   3 +
>  security/ipe/policy_parser.c |   8 +++
>  10 files changed, 220 insertions(+)
> 
> diff --git a/security/ipe/Kconfig b/security/ipe/Kconfig
> index 16e835ce61b0..dd9a066dd35a 100644
> --- a/security/ipe/Kconfig
> +++ b/security/ipe/Kconfig
> @@ -32,6 +32,17 @@ config IPE_PROP_DM_VERITY
>  
>         If unsure, answer Y.
>  
> +config IPE_PROP_FS_VERITY
> +     bool "Enable property for fs-verity files"
> +     depends on FS_VERITY && FS_VERITY_BUILTIN_SIGNATURES
> +     help
> +       This option enables the usage of properties "fsverity_signature"
> +       and "fsverity_digest". These properties evaluates to TRUE when
> +       a file is fsverity enabled and with a signed digest or its
> +       diegst matches the supplied value in the policy.
> +
> +       if unsure, answer Y.
> +
>  endmenu
>  
>  endif
> diff --git a/security/ipe/audit.c b/security/ipe/audit.c
> index 769ba95d9b0d..16d81645e53c 100644
> --- a/security/ipe/audit.c
> +++ b/security/ipe/audit.c
> @@ -46,6 +46,11 @@ static const char *const audit_prop_names[ipe_prop_max] = {
>       "dmverity_signature=FALSE",
>       "dmverity_signature=TRUE",
>  #endif /* CONFIG_IPE_PROP_DM_VERITY */
> +#ifdef CONFIG_IPE_PROP_FS_VERITY
> +     "fsverity_digest=",
> +     "fsverity_signature=FALSE",
> +     "fsverity_signature=TRUE"
> +#endif /* CONFIG_IPE_PROP_FS_VERITY */
>  };
>  
>  #ifdef CONFIG_IPE_PROP_DM_VERITY
> @@ -64,6 +69,22 @@ static void audit_dmv_roothash(struct audit_buffer *ab, 
> const void *rh)
>  }
>  #endif /* CONFIG_IPE_PROP_DM_VERITY */
>  
> +#ifdef CONFIG_IPE_PROP_FS_VERITY
> +/**
> + * audit_fsv_digest - audit a digest of a fsverity file.
> + * @ab: Supplies a poniter to the audit_buffer to append to.
> + * @d: Supplies a pointer to the digest structure.
> + */
> +static void audit_fsv_digest(struct audit_buffer *ab, const void *d)
> +{
> +     ipe_digest_audit(ab, d);
> +}
> +#else
> +static void audit_fsv_digest(struct audit_buffer *ab, const void *d)
> +{
> +}
> +#endif /* CONFIG_IPE_PROP_DM_VERITY */
> +
>  /**
>   * audit_rule - audit an IPE policy rule approximation.
>   * @ab: Supplies a poniter to the audit_buffer to append to.
> @@ -79,6 +100,8 @@ static void audit_rule(struct audit_buffer *ab, const 
> struct ipe_rule *r)
>               audit_log_format(ab, "%s", audit_prop_names[ptr->type]);
>               if (ptr->type == ipe_prop_dmv_roothash)
>                       audit_dmv_roothash(ab, ptr->value);
> +             if (ptr->type == ipe_prop_fsv_digest)
> +                     audit_fsv_digest(ab, ptr->value);
>  
>               audit_log_format(ab, " ");
>       }
> diff --git a/security/ipe/eval.c b/security/ipe/eval.c
> index 538af4195ba7..210d3926c0a8 100644
> --- a/security/ipe/eval.c
> +++ b/security/ipe/eval.c
> @@ -81,6 +81,23 @@ static void build_ipe_bdev_ctx(struct ipe_eval_ctx *ctx, 
> const struct inode *con
>  }
>  #endif /* CONFIG_IPE_PROP_DM_VERITY */
>  
> +#ifdef CONFIG_IPE_PROP_FS_VERITY
> +/**
> + * build_ipe_inode_ctx - Build inode fields of an evaluation context.
> + * @ctx: Supplies a pointer to the context to be populdated.
> + * @ino: Supplies the inode struct of the file triggered IPE event.
> + */
> +static void build_ipe_inode_ctx(struct ipe_eval_ctx *ctx, const struct inode 
> *const ino)
> +{
> +     ctx->ino = ino;
> +     ctx->ipe_inode = ipe_inode(ctx->ino);
> +}
> +#else
> +static void build_ipe_inode_ctx(struct ipe_eval_ctx *ctx, const struct inode 
> *const ino)
> +{
> +}
> +#endif /* CONFIG_IPE_PROP_FS_VERITY */
> +
>  /**
>   * build_eval_ctx - Build an evaluation context.
>   * @ctx: Supplies a pointer to the context to be populdated.
> @@ -99,6 +116,7 @@ void build_eval_ctx(struct ipe_eval_ctx *ctx,
>       if (file) {
>               ino = d_real_inode(file->f_path.dentry);
>               build_ipe_bdev_ctx(ctx, ino);
> +             build_ipe_inode_ctx(ctx, ino);
>       }
>  }
>  
> @@ -171,6 +189,91 @@ static bool evaluate_dmv_sig_true(const struct 
> ipe_eval_ctx *const ctx,
>  }
>  #endif /* CONFIG_IPE_PROP_DM_VERITY */
>  
> +#ifdef CONFIG_IPE_PROP_FS_VERITY
> +/**
> + * evaluate_fsv_digest - Analyze @ctx against a fsv digest property.
> + * @ctx: Supplies a pointer to the context being evaluated.
> + * @p: Supplies a pointer to the property being evaluated.
> + *
> + * Return:
> + * * true    - The current @ctx match the @p
> + * * false   - The current @ctx doesn't match the @p
> + */
> +static bool evaluate_fsv_digest(const struct ipe_eval_ctx *const ctx,
> +                             struct ipe_prop *p)
> +{
> +     enum hash_algo alg;
> +     u8 digest[FS_VERITY_MAX_DIGEST_SIZE];
> +
> +     if (!ctx->ino)
> +             return false;
> +     if (fsverity_get_digest((struct inode *)ctx->ino,
> +                             digest,
> +                             &alg)) {
> +             return false;
> +     }
> +
> +     return ipe_digest_eval(p->value,
> +                            digest,
> +                            hash_digest_size[alg],
> +                            hash_algo_name[alg]);
> +}
> +
> +/**
> + * evaluate_fsv_sig_false - Analyze @ctx against a fsv sig false property.
> + * @ctx: Supplies a pointer to the context being evaluated.
> + * @p: Supplies a pointer to the property being evaluated.
> + *
> + * Return:
> + * * true    - The current @ctx match the @p
> + * * false   - The current @ctx doesn't match the @p
> + */
> +static bool evaluate_fsv_sig_false(const struct ipe_eval_ctx *const ctx,
> +                                struct ipe_prop *p)
> +{
> +     return !ctx->ino ||
> +            !IS_VERITY(ctx->ino) ||
> +            !ctx->ipe_inode ||
> +            !ctx->ipe_inode->fs_verity_signed;
> +}
> +
> +/**
> + * evaluate_fsv_sig_true - Analyze @ctx against a fsv sig true property.
> + * @ctx: Supplies a pointer to the context being evaluated.
> + * @p: Supplies a pointer to the property being evaluated.
> + *
> + * Return:
> + * * true - The current @ctx match the @p
> + * * false - The current @ctx doesn't match the @p
> + */
> +static bool evaluate_fsv_sig_true(const struct ipe_eval_ctx *const ctx,
> +                               struct ipe_prop *p)
> +{
> +     return ctx->ino &&
> +            IS_VERITY(ctx->ino) &&
> +            ctx->ipe_inode &&
> +            ctx->ipe_inode->fs_verity_signed;
> +}

Isn't better to just define one function and prepend a ! in
evaluate_property()?

Not sure about the usefulness of the fsverity_signature= property as it
is. I would at minimum allow to specify which keyring signatures are
verified against, and ensure that the keyring has a restriction.

And maybe I would call fsverity_verify_signature() directly, after
extending it to pass the desired keyring.

I would also split this patch in two, one for fsverity_digest= and one
for fsverity_signature=.

Roberto

> +#else
> +static bool evaluate_fsv_digest(const struct ipe_eval_ctx *const ctx,
> +                             struct ipe_prop *p)
> +{
> +     return false;
> +}
> +
> +static bool evaluate_fsv_sig_false(const struct ipe_eval_ctx *const ctx,
> +                                struct ipe_prop *p)
> +{
> +     return false;
> +}
> +
> +static bool evaluate_fsv_sig_true(const struct ipe_eval_ctx *const ctx,
> +                               struct ipe_prop *p)
> +{
> +     return false;
> +}
> +#endif /* CONFIG_IPE_PROP_FS_VERITY */
> +
>  /**
>   * evaluate_property - Analyze @ctx against a property.
>   * @ctx: Supplies a pointer to the context to be evaluated.
> @@ -201,6 +304,15 @@ static bool evaluate_property(const struct ipe_eval_ctx 
> *const ctx,
>       case ipe_prop_dmv_sig_true:
>               eval = evaluate_dmv_sig_true(ctx, p);
>               break;
> +     case ipe_prop_fsv_digest:
> +             eval = evaluate_fsv_digest(ctx, p);
> +             break;
> +     case ipe_prop_fsv_sig_false:
> +             eval = evaluate_fsv_sig_false(ctx, p);
> +             break;
> +     case ipe_prop_fsv_sig_true:
> +             eval = evaluate_fsv_sig_true(ctx, p);
> +             break;
>       default:
>               eval = false;
>       }
> diff --git a/security/ipe/eval.h b/security/ipe/eval.h
> index 4fd832c6893e..d3dce4f04cb4 100644
> --- a/security/ipe/eval.h
> +++ b/security/ipe/eval.h
> @@ -26,6 +26,12 @@ struct ipe_bdev {
>  };
>  #endif /* CONFIG_IPE_PROP_DM_VERITY */
>  
> +#ifdef CONFIG_IPE_PROP_FS_VERITY
> +struct ipe_inode {
> +     bool fs_verity_signed;
> +};
> +#endif /* CONFIG_IPE_PROP_FS_VERITY */
> +
>  struct ipe_eval_ctx {
>       enum ipe_op_type op;
>  
> @@ -34,6 +40,10 @@ struct ipe_eval_ctx {
>  #ifdef CONFIG_IPE_PROP_DM_VERITY
>       const struct ipe_bdev *ipe_bdev;
>  #endif /* CONFIG_IPE_PROP_DM_VERITY */
> +#ifdef CONFIG_IPE_PROP_FS_VERITY
> +     const struct inode *ino;
> +     const struct ipe_inode *ipe_inode;
> +#endif /* CONFIG_IPE_PROP_FS_VERITY */
>  };
>  
>  enum ipe_match {
> diff --git a/security/ipe/hooks.c b/security/ipe/hooks.c
> index 735375d2f858..836f08240372 100644
> --- a/security/ipe/hooks.c
> +++ b/security/ipe/hooks.c
> @@ -243,3 +243,33 @@ int ipe_bdev_setsecurity(struct block_device *bdev, 
> const char *key,
>       return -EOPNOTSUPP;
>  }
>  #endif /* CONFIG_IPE_PROP_DM_VERITY */
> +
> +#ifdef CONFIG_IPE_PROP_FS_VERITY
> +/**
> + * ipe_inode_setsecurity - Sets fields of a inode security blob from @key.
> + * @inode: The inode to source the security blob from.
> + * @name: The name representing the information to be stored.
> + * @value: The value to be stored.
> + * @size: The size of @value.
> + * @flags: unused
> + *
> + * Saves fsverity signature & digest into inode security blob
> + *
> + * Return:
> + * * 0       - OK
> + * * !0      - Error
> + */
> +int ipe_inode_setsecurity(struct inode *inode, const char *name,
> +                       const void *value, size_t size,
> +                       int flags)
> +{
> +     struct ipe_inode *inode_sec = ipe_inode(inode);
> +
> +     if (!strcmp(name, FS_VERITY_INODE_SEC_NAME)) {
> +             inode_sec->fs_verity_signed = size > 0 && value;
> +             return 0;
> +     }
> +
> +     return -EOPNOTSUPP;
> +}
> +#endif /* CONFIG_CONFIG_IPE_PROP_FS_VERITY */
> diff --git a/security/ipe/hooks.h b/security/ipe/hooks.h
> index 16611a149158..654aba584a44 100644
> --- a/security/ipe/hooks.h
> +++ b/security/ipe/hooks.h
> @@ -8,6 +8,7 @@
>  #include <linux/fs.h>
>  #include <linux/binfmts.h>
>  #include <linux/security.h>
> +#include <linux/fsverity.h>
>  #include <linux/device-mapper.h>
>  
>  void ipe_sb_free_security(struct super_block *mnt_sb);
> @@ -32,4 +33,10 @@ int ipe_bdev_setsecurity(struct block_device *bdev, const 
> char *key,
>                        const void *value, size_t len);
>  #endif /* CONFIG_IPE_PROP_DM_VERITY */
>  
> +#ifdef CONFIG_IPE_PROP_FS_VERITY
> +int ipe_inode_setsecurity(struct inode *inode, const char *name,
> +                       const void *value, size_t size,
> +                       int flags);
> +#endif /* CONFIG_IPE_PROP_FS_VERITY */
> +
>  #endif /* IPE_HOOKS_H */
> diff --git a/security/ipe/ipe.c b/security/ipe/ipe.c
> index 5612cb3cf1e5..705ce9a003de 100644
> --- a/security/ipe/ipe.c
> +++ b/security/ipe/ipe.c
> @@ -13,6 +13,9 @@ static struct lsm_blob_sizes ipe_blobs __lsm_ro_after_init 
> = {
>  #ifdef CONFIG_IPE_PROP_DM_VERITY
>               .lbs_bdev = sizeof(struct ipe_bdev),
>  #endif /* CONFIG_IPE_PROP_DM_VERITY */
> +#ifdef CONFIG_IPE_PROP_FS_VERITY
> +             .lbs_inode = sizeof(struct ipe_inode),
> +#endif /* CONFIG_IPE_PROP_FS_VERITY */
>  };
>  
>  #ifdef CONFIG_IPE_PROP_DM_VERITY
> @@ -22,6 +25,13 @@ struct ipe_bdev *ipe_bdev(struct block_device *b)
>  }
>  #endif /* CONFIG_IPE_PROP_DM_VERITY */
>  
> +#ifdef CONFIG_IPE_PROP_FS_VERITY
> +struct ipe_inode *ipe_inode(const struct inode *inode)
> +{
> +     return inode->i_security + ipe_blobs.lbs_inode;
> +}
> +#endif /* CONFIG_IPE_PROP_FS_VERITY */
> +
>  static struct security_hook_list ipe_hooks[] __lsm_ro_after_init = {
>       LSM_HOOK_INIT(sb_free_security, ipe_sb_free_security),
>       LSM_HOOK_INIT(bprm_check_security, ipe_bprm_check_security),
> @@ -33,6 +43,9 @@ static struct security_hook_list ipe_hooks[] 
> __lsm_ro_after_init = {
>       LSM_HOOK_INIT(bdev_free_security, ipe_bdev_free_security),
>       LSM_HOOK_INIT(bdev_setsecurity, ipe_bdev_setsecurity),
>  #endif
> +#ifdef CONFIG_IPE_PROP_FS_VERITY
> +     LSM_HOOK_INIT(inode_setsecurity, ipe_inode_setsecurity),
> +#endif /* CONFIG_IPE_PROP_FS_VERITY */
>  };
>  
>  /**
> diff --git a/security/ipe/ipe.h b/security/ipe/ipe.h
> index c2594a45b8f2..138fda645ecf 100644
> --- a/security/ipe/ipe.h
> +++ b/security/ipe/ipe.h
> @@ -15,5 +15,8 @@ extern bool ipe_enabled;
>  #ifdef CONFIG_IPE_PROP_DM_VERITY
>  struct ipe_bdev *ipe_bdev(struct block_device *b);
>  #endif /* CONFIG_IPE_PROP_DM_VERITY */
> +#ifdef CONFIG_IPE_PROP_FS_VERITY
> +struct ipe_inode *ipe_inode(const struct inode *inode);
> +#endif /* CONFIG_IPE_PROP_FS_VERITY */
>  
>  #endif /* IPE_H */
> diff --git a/security/ipe/policy.h b/security/ipe/policy.h
> index 324eb76c6067..50b8f4c49bc7 100644
> --- a/security/ipe/policy.h
> +++ b/security/ipe/policy.h
> @@ -31,6 +31,9 @@ enum ipe_prop_type {
>       ipe_prop_dmv_roothash,
>       ipe_prop_dmv_sig_false,
>       ipe_prop_dmv_sig_true,
> +     ipe_prop_fsv_digest,
> +     ipe_prop_fsv_sig_false,
> +     ipe_prop_fsv_sig_true,
>       ipe_prop_max
>  };
>  
> diff --git a/security/ipe/policy_parser.c b/security/ipe/policy_parser.c
> index 50a6a763e842..799ee7fda974 100644
> --- a/security/ipe/policy_parser.c
> +++ b/security/ipe/policy_parser.c
> @@ -273,6 +273,11 @@ static const match_table_t property_tokens = {
>       {ipe_prop_dmv_sig_false,        "dmverity_signature=FALSE"},
>       {ipe_prop_dmv_sig_true,         "dmverity_signature=TRUE"},
>  #endif /* CONFIG_IPE_PROP_DM_VERITY */
> +#ifdef CONFIG_IPE_PROP_FS_VERITY
> +     {ipe_prop_fsv_digest,           "fsverity_digest=%s"},
> +     {ipe_prop_fsv_sig_false,        "fsverity_signature=FALSE"},
> +     {ipe_prop_fsv_sig_true,         "fsverity_signature=TRUE"},
> +#endif /* CONFIG_IPE_PROP_FS_VERITY */
>       {ipe_prop_max,                  NULL}
>  };
>  
> @@ -304,6 +309,7 @@ int parse_property(char *t, struct ipe_rule *r)
>  
>       switch (token) {
>       case ipe_prop_dmv_roothash:
> +     case ipe_prop_fsv_digest:
>               dup = match_strdup(&args[0]);
>               if (!dup) {
>                       rc = -ENOMEM;
> @@ -315,6 +321,8 @@ int parse_property(char *t, struct ipe_rule *r)
>       case ipe_prop_boot_verified_true:
>       case ipe_prop_dmv_sig_false:
>       case ipe_prop_dmv_sig_true:
> +     case ipe_prop_fsv_sig_false:
> +     case ipe_prop_fsv_sig_true:
>               p->type = token;
>               break;
>       case ipe_prop_max:

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Reply via email to