| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <d62907da62b5e0b25c9d7bd4b3119a3d1827bd29.camel@huaweicloud.com>
Date: Tue, 31 Jan 2023 15:00:08 +0100
From: Roberto Sassu <roberto.sassu@...weicloud.com>
To: Fan Wu <wufan@...ux.microsoft.com>, corbet@....net,
zohar@...ux.ibm.com, jmorris@...ei.org, serge@...lyn.com,
tytso@....edu, ebiggers@...nel.org, axboe@...nel.dk,
agk@...hat.com, snitzer@...nel.org, eparis@...hat.com,
paul@...l-moore.com
Cc: linux-doc@...r.kernel.org, linux-integrity@...r.kernel.org,
linux-security-module@...r.kernel.org,
linux-fscrypt@...r.kernel.org, linux-block@...r.kernel.org,
dm-devel@...hat.com, linux-audit@...hat.com,
roberto.sassu@...wei.com, linux-kernel@...r.kernel.org,
Deven Bowers <deven.desai@...ux.microsoft.com>
Subject: Re: [RFC PATCH v9 13/16] ipe: enable support for fs-verity as a
trust provider
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.desai@...ux.microsoft.com>
> Signed-off-by: Fan Wu <wufan@...ux.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:
Powered by blists - more mailing lists