Re: [PATCH RFC v10 5/17] ipe: introduce 'boot_verified' as a trust provider

Paul Moore
2023-07-08T05:37:55Z

On Jun 28, 2023 Fan Wu <wufan@linux.microsoft.com> wrote:
>
> IPE is designed to provide system level trust guarantees, this usually
> implies that trust starts from bootup with a hardware root of trust,
> which validates the bootloader. After this, the bootloader verifies the
> kernel and the initramfs.
>
> As there's no currently supported integrity method for initramfs, and
> it's typically already verified by the bootloader, introduce a property
> that causes the first superblock to have an execution to be "pinned",
> which is typically initramfs.
>
> When the "pinned" device is unmounted, it will be "unpinned" and
> `boot_verified` property will always evaluate to false afterward.
>
> We use a pointer with a spin_lock to "pin" the device instead of rcu
> because rcu synchronization may sleep, which is not allowed when
> unmounting a device.
>
> Signed-off-by: Deven Bowers <deven.desai@linux.microsoft.com>
> Signed-off-by: Fan Wu <wufan@linux.microsoft.com>
> ---
> security/ipe/eval.c | 72 +++++++++++++++++++++++++++++++++++-
> security/ipe/eval.h | 2 +
> security/ipe/hooks.c | 12 ++++++
> security/ipe/hooks.h | 2 +
> security/ipe/ipe.c | 1 +
> security/ipe/policy.h | 2 +
> security/ipe/policy_parser.c | 37 +++++++++++++++++-
> 7 files changed, 126 insertions(+), 2 deletions(-)

The compilation errors continue into this patch.

> diff --git a/security/ipe/policy_parser.c b/security/ipe/policy_parser.c
> index 27e5767480b0..28c14adfe6d2 100644
> --- a/security/ipe/policy_parser.c
> +++ b/security/ipe/policy_parser.c
> @@ -265,6 +265,12 @@ static enum ipe_action_type parse_action(char *t)
> return match_token(t, action_tokens, args);
> }
>
> +static const match_table_t property_tokens = {
> + {__IPE_PROP_BOOT_VERIFIED_FALSE, "boot_verified=FALSE"},
> + {__IPE_PROP_BOOT_VERIFIED_TRUE, "boot_verified=TRUE"},
> + {__IPE_PROP_INVALID, NULL}
> +};
> +
> /**
> * parse_property - Parse the property type given a token string.
> * @t: Supplies the token string to be parsed.
> @@ -277,7 +283,36 @@ static enum ipe_action_type parse_action(char *t)
> */
> static int parse_property(char *t, struct ipe_rule *r)
> {
> - return -EBADMSG;
> + substring_t args[MAX_OPT_ARGS];
> + struct ipe_prop *p = NULL;
> + int rc = 0;
> + int token;
> +
> + p = kzalloc(sizeof(*p), GFP_KERNEL);
> + if (!p)
> + return -ENOMEM;
> +
> + token = match_token(t, property_tokens, args);
> +
> + switch (token) {
> + case __IPE_PROP_BOOT_VERIFIED_FALSE:
> + case __IPE_PROP_BOOT_VERIFIED_TRUE:
> + p->type = token;
> + break;
> + case __IPE_PROP_INVALID:

You generally don't need to explicitly specify a case if the code
immediately falls through to 'default'.

> + default:
> + rc = -EBADMSG;
> + break;
> + }
> + if (rc)
> + goto err;
> + list_add_tail(&p->next, &r->props);
> +
> +out:
> + return rc;
> +err:
> + kfree(p);
> + goto out;

Once again, don't use a goto when the jump destination simply does a
return, do the return directly.

> }
>
> /**
> --
> 2.25.1

--
paul-moore.com