BPF List
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Blaise Boscaccy" <bboscaccy@linux.microsoft.com>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH v7 10/10] ipe: Add BPF program load policy enforcement via Hornet integration
Date: Fri, 08 May 2026 04:21:41 +0000	[thread overview]
Message-ID: <20260508042142.4AC13C2BCC9@smtp.kernel.org> (raw)
In-Reply-To: <20260507191416.2984054-11-bboscaccy@linux.microsoft.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The `IPE_PROP_BPF_KERNEL_FALSE` property evaluates to `true` when `CONFIG_IPE_PROP_BPF_SIGNATURE` is disabled, violating IPE's fail-closed design.
- [High] `ipe_bpf_prog_load_post_integrity()` completely ignores the `lsmid` parameter, allowing other LSMs to silently overwrite Hornet's integrity verdict.
- [Medium] The documentation provides an insecure policy example that allows unprivileged users to bypass the `bpf_signature=OK` check using their own keyrings.
--

commit 341207097f16b88e2b641d2d93566b204484bce3
Author: Blaise Boscaccy <bboscaccy@linux.microsoft.com>

ipe: Add BPF program load policy enforcement via Hornet integration

Add support for the bpf_prog_load_post_integrity LSM hook, enabling IPE
to make policy decisions about BPF program loading based on integrity
verdicts provided by the Hornet LSM.

> diff --git a/Documentation/admin-guide/LSM/ipe.rst b/Documentation/admin-guide/LSM/ipe.rst
> index a756d81585317..d68ba9d98859e 100644
> --- a/Documentation/admin-guide/LSM/ipe.rst
> +++ b/Documentation/admin-guide/LSM/ipe.rst
[ ... ]
> @@ -788,6 +896,58 @@ Allow execution of a specific fs-verity file
>  
>     op=EXECUTE fsverity_digest=sha256:fd88f2b8824e197f850bf4c5109bea5cf0ee38104f710843bb72da796ba5af9e action=ALLOW
>  
> +Allow only signed BPF programs
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +::
> +
> +   policy_name=Allow_Signed_BPF policy_version=0.0.0
> +   DEFAULT action=ALLOW
> +
> +   DEFAULT op=BPF_PROG_LOAD action=DENY
> +   op=BPF_PROG_LOAD bpf_kernel=TRUE action=ALLOW
> +   op=BPF_PROG_LOAD bpf_signature=OK action=ALLOW

Since Hornet verifies the program signature against the user-provided
keyring ID, could an unprivileged user bypass this policy by creating a
personal user keyring, signing a malicious BPF program with their own key,
and passing their keyring ID?

Would it be better to recommend explicitly restricting the bpf_keyring
property in this example, perhaps to bpf_keyring=BUILTIN?

[ ... ]
> diff --git a/security/ipe/eval.c b/security/ipe/eval.c
> index 21439c5be3364..705c4ecfda696 100644
> --- a/security/ipe/eval.c
> +++ b/security/ipe/eval.c
[ ... ]
> @@ -297,6 +362,32 @@ static bool evaluate_property(const struct ipe_eval_ctx *const ctx,
[ ... ]
> +	case IPE_PROP_BPF_KERNEL_FALSE:
> +		return !evaluate_bpf_kernel(ctx);

When CONFIG_IPE_PROP_BPF_SIGNATURE is disabled, the stub for
evaluate_bpf_kernel() unconditionally returns false:

    static bool evaluate_bpf_kernel(const struct ipe_eval_ctx *const ctx)
    {
        return false;
    }

This causes !evaluate_bpf_kernel(ctx) to return true.

Does this inadvertently cause any IPE policy rule using bpf_kernel=FALSE
to always match when the feature is disabled? Should this regression be
addressed with a dedicated stub that returns false when disabled to maintain
the fail-closed design?

[ ... ]
> diff --git a/security/ipe/hooks.c b/security/ipe/hooks.c
> index 0ae54a880405a..9271e129a2cf2 100644
> --- a/security/ipe/hooks.c
> +++ b/security/ipe/hooks.c
[ ... ]
> +#ifdef CONFIG_IPE_PROP_BPF_SIGNATURE
> +/**
> + * ipe_bpf_prog_load_post_integrity() - Store integrity verdict in per-prog blob.
[ ... ]
> + */
> +int ipe_bpf_prog_load_post_integrity(struct bpf_prog *prog,
> +				     union bpf_attr *attr,
> +				     struct bpf_token *token,
> +				     bool kernel,
> +				     const struct lsm_id *lsmid,
> +				     enum lsm_integrity_verdict verdict)
> +{
> +	struct ipe_bpf_prog *blob = ipe_bpf_prog(prog);
> +
> +	blob->verdict = verdict;

Should this check the lsmid parameter before storing the verdict?

In a multi-LSM environment, it looks like any other integrity provider
calling this hook could silently overwrite the verdict previously set by
Hornet, potentially replacing an LSM_INT_VERDICT_BADSIG verdict with an
LSM_INT_VERDICT_OK or LSM_INT_VERDICT_NONE verdict.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260507191416.2984054-1-bboscaccy@linux.microsoft.com?part=10

  reply	other threads:[~2026-05-08  4:21 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-07 19:13 [PATCH v7 00/10] Reintroduce Hornet LSM Blaise Boscaccy
2026-05-07 19:13 ` [PATCH v7 01/10] crypto: pkcs7: add flag for validated trust on a signed info block Blaise Boscaccy
2026-05-07 23:51   ` sashiko-bot
2026-05-07 19:13 ` [PATCH v7 02/10] crypto: pkcs7: add ability to extract signed attributes by OID Blaise Boscaccy
2026-05-08  0:14   ` sashiko-bot
2026-05-07 19:13 ` [PATCH v7 03/10] crypto: pkcs7: add tests for pkcs7_get_authattr Blaise Boscaccy
2026-05-08  0:35   ` sashiko-bot
2026-05-07 19:13 ` [PATCH v7 04/10] lsm: framework for BPF integrity verification Blaise Boscaccy
2026-05-08  1:09   ` sashiko-bot
2026-05-07 19:13 ` [PATCH v7 05/10] lsm: security: Add additional enum values for bpf integrity checks Blaise Boscaccy
2026-05-07 19:14 ` [PATCH v7 06/10] security: Hornet LSM Blaise Boscaccy
2026-05-08  2:07   ` sashiko-bot
2026-05-07 19:14 ` [PATCH v7 07/10] hornet: Introduce gen_sig Blaise Boscaccy
2026-05-08  2:22   ` sashiko-bot
2026-05-07 19:14 ` [PATCH v7 08/10] hornet: Add a light skeleton data extractor scripts Blaise Boscaccy
2026-05-08  2:35   ` sashiko-bot
2026-05-07 19:14 ` [PATCH v7 09/10] selftests/hornet: Add a selftest for the Hornet LSM Blaise Boscaccy
2026-05-08  2:58   ` sashiko-bot
2026-05-07 19:14 ` [PATCH v7 10/10] ipe: Add BPF program load policy enforcement via Hornet integration Blaise Boscaccy
2026-05-08  4:21   ` sashiko-bot [this message]
2026-05-08 18:40   ` Fan Wu
2026-05-07 20:57 ` [PATCH v7 00/10] Reintroduce Hornet LSM Paul Moore
2026-05-07 21:58   ` Eric Biggers
2026-05-07 22:22     ` Paul Moore

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260508042142.4AC13C2BCC9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bboscaccy@linux.microsoft.com \
    --cc=bpf@vger.kernel.org \
    --cc=sashiko@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox