From: Roberto Sassu <[email protected]>
Add the bpf_verify_pkcs7_signature() kfunc, to give eBPF security modules
the ability to check the validity of a signature against supplied data, by
using user-provided or system-provided keys as trust anchor.
The new kfunc makes it possible to enforce mandatory policies, as eBPF
programs might be allowed to make security decisions only based on data
sources the system administrator approves.
The caller should provide the data to be verified and the signature as eBPF
dynamic pointers (to minimize the number of parameters) and a bpf_key
structure containing a reference to the keyring with keys trusted for
signature verification, obtained from bpf_lookup_user_key() or
bpf_lookup_system_key().
For bpf_key structures obtained from the former lookup function,
bpf_verify_pkcs7_signature() completes the permission check deferred by
that function by calling key_validate(). key_task_permission() is already
called by the PKCS#7 code.
Signed-off-by: Roberto Sassu <[email protected]>
Acked-by: KP Singh <[email protected]>
---
kernel/trace/bpf_trace.c | 45 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 7a7023704ac2..8e2c026b0a58 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1294,12 +1294,57 @@ void bpf_key_put(struct bpf_key *bkey)
kfree(bkey);
}
+#ifdef CONFIG_SYSTEM_DATA_VERIFICATION
+/**
+ * bpf_verify_pkcs7_signature - verify a PKCS#7 signature
+ * @data_ptr: data to verify
+ * @sig_ptr: signature of the data
+ * @trusted_keyring: keyring with keys trusted for signature verification
+ *
+ * Verify the PKCS#7 signature *sig_ptr* against the supplied *data_ptr*
+ * with keys in a keyring referenced by *trusted_keyring*.
+ *
+ * Return: 0 on success, a negative value on error.
+ */
+int bpf_verify_pkcs7_signature(struct bpf_dynptr_kern *data_ptr,
+ struct bpf_dynptr_kern *sig_ptr,
+ struct bpf_key *trusted_keyring)
+{
+ int ret;
+
+ if (trusted_keyring->has_ref) {
+ /*
+ * Do the permission check deferred in bpf_lookup_user_key().
+ * See bpf_lookup_user_key() for more details.
+ *
+ * A call to key_task_permission() here would be redundant, as
+ * it is already done by keyring_search() called by
+ * find_asymmetric_key().
+ */
+ ret = key_validate(trusted_keyring->key);
+ if (ret < 0)
+ return ret;
+ }
+
+ return verify_pkcs7_signature(data_ptr->data,
+ bpf_dynptr_get_size(data_ptr),
+ sig_ptr->data,
+ bpf_dynptr_get_size(sig_ptr),
+ trusted_keyring->key,
+ VERIFYING_UNSPECIFIED_SIGNATURE, NULL,
+ NULL);
+}
+#endif /* CONFIG_SYSTEM_DATA_VERIFICATION */
+
__diag_pop();
BTF_SET8_START(key_sig_kfunc_set)
BTF_ID_FLAGS(func, bpf_lookup_user_key, KF_ACQUIRE | KF_RET_NULL | KF_SLEEPABLE)
BTF_ID_FLAGS(func, bpf_lookup_system_key, KF_ACQUIRE | KF_RET_NULL)
BTF_ID_FLAGS(func, bpf_key_put, KF_RELEASE)
+#ifdef CONFIG_SYSTEM_DATA_VERIFICATION
+BTF_ID_FLAGS(func, bpf_verify_pkcs7_signature, KF_SLEEPABLE)
+#endif
BTF_SET8_END(key_sig_kfunc_set)
static const struct btf_kfunc_id_set bpf_key_sig_kfunc_set = {
--
2.25.1
On Mon, 5 Sept 2022 at 16:35, Roberto Sassu
<[email protected]> wrote:
>
> From: Roberto Sassu <[email protected]>
>
> Add the bpf_verify_pkcs7_signature() kfunc, to give eBPF security modules
> the ability to check the validity of a signature against supplied data, by
> using user-provided or system-provided keys as trust anchor.
>
> The new kfunc makes it possible to enforce mandatory policies, as eBPF
> programs might be allowed to make security decisions only based on data
> sources the system administrator approves.
>
> The caller should provide the data to be verified and the signature as eBPF
> dynamic pointers (to minimize the number of parameters) and a bpf_key
> structure containing a reference to the keyring with keys trusted for
> signature verification, obtained from bpf_lookup_user_key() or
> bpf_lookup_system_key().
>
> For bpf_key structures obtained from the former lookup function,
> bpf_verify_pkcs7_signature() completes the permission check deferred by
> that function by calling key_validate(). key_task_permission() is already
> called by the PKCS#7 code.
>
> Signed-off-by: Roberto Sassu <[email protected]>
> Acked-by: KP Singh <[email protected]>
> ---
> kernel/trace/bpf_trace.c | 45 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 45 insertions(+)
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 7a7023704ac2..8e2c026b0a58 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -1294,12 +1294,57 @@ void bpf_key_put(struct bpf_key *bkey)
> kfree(bkey);
> }
>
> +#ifdef CONFIG_SYSTEM_DATA_VERIFICATION
> +/**
> + * bpf_verify_pkcs7_signature - verify a PKCS#7 signature
> + * @data_ptr: data to verify
> + * @sig_ptr: signature of the data
> + * @trusted_keyring: keyring with keys trusted for signature verification
> + *
> + * Verify the PKCS#7 signature *sig_ptr* against the supplied *data_ptr*
> + * with keys in a keyring referenced by *trusted_keyring*.
> + *
> + * Return: 0 on success, a negative value on error.
> + */
> +int bpf_verify_pkcs7_signature(struct bpf_dynptr_kern *data_ptr,
> + struct bpf_dynptr_kern *sig_ptr,
> + struct bpf_key *trusted_keyring)
> +{
> + int ret;
> +
> + if (trusted_keyring->has_ref) {
> + /*
> + * Do the permission check deferred in bpf_lookup_user_key().
> + * See bpf_lookup_user_key() for more details.
> + *
> + * A call to key_task_permission() here would be redundant, as
> + * it is already done by keyring_search() called by
> + * find_asymmetric_key().
> + */
> + ret = key_validate(trusted_keyring->key);
> + if (ret < 0)
> + return ret;
> + }
> +
> + return verify_pkcs7_signature(data_ptr->data,
> + bpf_dynptr_get_size(data_ptr),
> + sig_ptr->data,
> + bpf_dynptr_get_size(sig_ptr),
MIssing check for data_ptr->data == NULL before making this call? Same
for sig_ptr.
> + trusted_keyring->key,
> + VERIFYING_UNSPECIFIED_SIGNATURE, NULL,
> + NULL);
> +}
> +#endif /* CONFIG_SYSTEM_DATA_VERIFICATION */
> +
> __diag_pop();
>
> BTF_SET8_START(key_sig_kfunc_set)
> BTF_ID_FLAGS(func, bpf_lookup_user_key, KF_ACQUIRE | KF_RET_NULL | KF_SLEEPABLE)
> BTF_ID_FLAGS(func, bpf_lookup_system_key, KF_ACQUIRE | KF_RET_NULL)
> BTF_ID_FLAGS(func, bpf_key_put, KF_RELEASE)
> +#ifdef CONFIG_SYSTEM_DATA_VERIFICATION
> +BTF_ID_FLAGS(func, bpf_verify_pkcs7_signature, KF_SLEEPABLE)
> +#endif
> BTF_SET8_END(key_sig_kfunc_set)
>
> static const struct btf_kfunc_id_set bpf_key_sig_kfunc_set = {
> --
> 2.25.1
>
On Tue, 2022-09-06 at 04:57 +0200, Kumar Kartikeya Dwivedi wrote:
> On Mon, 5 Sept 2022 at 16:35, Roberto Sassu
> <[email protected]> wrote:
> > From: Roberto Sassu <[email protected]>
> >
> > Add the bpf_verify_pkcs7_signature() kfunc, to give eBPF security
> > modules
> > the ability to check the validity of a signature against supplied
> > data, by
> > using user-provided or system-provided keys as trust anchor.
> >
> > The new kfunc makes it possible to enforce mandatory policies, as
> > eBPF
> > programs might be allowed to make security decisions only based on
> > data
> > sources the system administrator approves.
> >
> > The caller should provide the data to be verified and the signature
> > as eBPF
> > dynamic pointers (to minimize the number of parameters) and a
> > bpf_key
> > structure containing a reference to the keyring with keys trusted
> > for
> > signature verification, obtained from bpf_lookup_user_key() or
> > bpf_lookup_system_key().
> >
> > For bpf_key structures obtained from the former lookup function,
> > bpf_verify_pkcs7_signature() completes the permission check
> > deferred by
> > that function by calling key_validate(). key_task_permission() is
> > already
> > called by the PKCS#7 code.
> >
> > Signed-off-by: Roberto Sassu <[email protected]>
> > Acked-by: KP Singh <[email protected]>
> > ---
> > kernel/trace/bpf_trace.c | 45
> > ++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 45 insertions(+)
> >
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index 7a7023704ac2..8e2c026b0a58 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -1294,12 +1294,57 @@ void bpf_key_put(struct bpf_key *bkey)
> > kfree(bkey);
> > }
> >
> > +#ifdef CONFIG_SYSTEM_DATA_VERIFICATION
> > +/**
> > + * bpf_verify_pkcs7_signature - verify a PKCS#7 signature
> > + * @data_ptr: data to verify
> > + * @sig_ptr: signature of the data
> > + * @trusted_keyring: keyring with keys trusted for signature
> > verification
> > + *
> > + * Verify the PKCS#7 signature *sig_ptr* against the supplied
> > *data_ptr*
> > + * with keys in a keyring referenced by *trusted_keyring*.
> > + *
> > + * Return: 0 on success, a negative value on error.
> > + */
> > +int bpf_verify_pkcs7_signature(struct bpf_dynptr_kern *data_ptr,
> > + struct bpf_dynptr_kern *sig_ptr,
> > + struct bpf_key *trusted_keyring)
> > +{
> > + int ret;
> > +
> > + if (trusted_keyring->has_ref) {
> > + /*
> > + * Do the permission check deferred in
> > bpf_lookup_user_key().
> > + * See bpf_lookup_user_key() for more details.
> > + *
> > + * A call to key_task_permission() here would be
> > redundant, as
> > + * it is already done by keyring_search() called by
> > + * find_asymmetric_key().
> > + */
> > + ret = key_validate(trusted_keyring->key);
> > + if (ret < 0)
> > + return ret;
> > + }
> > +
> > + return verify_pkcs7_signature(data_ptr->data,
> > + bpf_dynptr_get_size(data_ptr)
> > ,
> > + sig_ptr->data,
> > + bpf_dynptr_get_size(sig_ptr),
>
> MIssing check for data_ptr->data == NULL before making this call?
> Same
> for sig_ptr.
Patch 3 requires the dynptrs to be initialized. Isn't enough?
Thanks
Roberto
On Tue, 6 Sept 2022 at 10:08, Roberto Sassu
<[email protected]> wrote:
>
> On Tue, 2022-09-06 at 04:57 +0200, Kumar Kartikeya Dwivedi wrote:
> > On Mon, 5 Sept 2022 at 16:35, Roberto Sassu
> > <[email protected]> wrote:
> > > From: Roberto Sassu <[email protected]>
> > >
> > > Add the bpf_verify_pkcs7_signature() kfunc, to give eBPF security
> > > modules
> > > the ability to check the validity of a signature against supplied
> > > data, by
> > > using user-provided or system-provided keys as trust anchor.
> > >
> > > The new kfunc makes it possible to enforce mandatory policies, as
> > > eBPF
> > > programs might be allowed to make security decisions only based on
> > > data
> > > sources the system administrator approves.
> > >
> > > The caller should provide the data to be verified and the signature
> > > as eBPF
> > > dynamic pointers (to minimize the number of parameters) and a
> > > bpf_key
> > > structure containing a reference to the keyring with keys trusted
> > > for
> > > signature verification, obtained from bpf_lookup_user_key() or
> > > bpf_lookup_system_key().
> > >
> > > For bpf_key structures obtained from the former lookup function,
> > > bpf_verify_pkcs7_signature() completes the permission check
> > > deferred by
> > > that function by calling key_validate(). key_task_permission() is
> > > already
> > > called by the PKCS#7 code.
> > >
> > > Signed-off-by: Roberto Sassu <[email protected]>
> > > Acked-by: KP Singh <[email protected]>
> > > ---
> > > kernel/trace/bpf_trace.c | 45
> > > ++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 45 insertions(+)
> > >
> > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > > index 7a7023704ac2..8e2c026b0a58 100644
> > > --- a/kernel/trace/bpf_trace.c
> > > +++ b/kernel/trace/bpf_trace.c
> > > @@ -1294,12 +1294,57 @@ void bpf_key_put(struct bpf_key *bkey)
> > > kfree(bkey);
> > > }
> > >
> > > +#ifdef CONFIG_SYSTEM_DATA_VERIFICATION
> > > +/**
> > > + * bpf_verify_pkcs7_signature - verify a PKCS#7 signature
> > > + * @data_ptr: data to verify
> > > + * @sig_ptr: signature of the data
> > > + * @trusted_keyring: keyring with keys trusted for signature
> > > verification
> > > + *
> > > + * Verify the PKCS#7 signature *sig_ptr* against the supplied
> > > *data_ptr*
> > > + * with keys in a keyring referenced by *trusted_keyring*.
> > > + *
> > > + * Return: 0 on success, a negative value on error.
> > > + */
> > > +int bpf_verify_pkcs7_signature(struct bpf_dynptr_kern *data_ptr,
> > > + struct bpf_dynptr_kern *sig_ptr,
> > > + struct bpf_key *trusted_keyring)
> > > +{
> > > + int ret;
> > > +
> > > + if (trusted_keyring->has_ref) {
> > > + /*
> > > + * Do the permission check deferred in
> > > bpf_lookup_user_key().
> > > + * See bpf_lookup_user_key() for more details.
> > > + *
> > > + * A call to key_task_permission() here would be
> > > redundant, as
> > > + * it is already done by keyring_search() called by
> > > + * find_asymmetric_key().
> > > + */
> > > + ret = key_validate(trusted_keyring->key);
> > > + if (ret < 0)
> > > + return ret;
> > > + }
> > > +
> > > + return verify_pkcs7_signature(data_ptr->data,
> > > + bpf_dynptr_get_size(data_ptr)
> > > ,
> > > + sig_ptr->data,
> > > + bpf_dynptr_get_size(sig_ptr),
> >
> > MIssing check for data_ptr->data == NULL before making this call?
> > Same
> > for sig_ptr.
>
> Patch 3 requires the dynptrs to be initialized. Isn't enough?
>
No, it seems even initialized dynptr can be NULL at runtime. Look at
both ringbuf_submit_dynptr and ringbuf_discard_dynptr.
The verifier won't know after ringbuf_reserve_dynptr whether it set it
to NULL or some valid pointer.
dynptr_init is basically that stack slot is now STACK_DYNPTR, it says
nothing more about the dynptr.
As far as testing this goes, you can pass invalid parameters to
ringbuf_reserve_dynptr to have it set to NULL, then make sure your
helper returns an error at runtime for it.
> Thanks
>
> Roberto
>
On Wed, 2022-09-07 at 04:28 +0200, Kumar Kartikeya Dwivedi wrote:
> On Tue, 6 Sept 2022 at 10:08, Roberto Sassu
> <[email protected]> wrote:
> > On Tue, 2022-09-06 at 04:57 +0200, Kumar Kartikeya Dwivedi wrote:
> > > On Mon, 5 Sept 2022 at 16:35, Roberto Sassu
> > > <[email protected]> wrote:
> > > > From: Roberto Sassu <[email protected]>
> > > >
> > > > Add the bpf_verify_pkcs7_signature() kfunc, to give eBPF
> > > > security
> > > > modules
> > > > the ability to check the validity of a signature against
> > > > supplied
> > > > data, by
> > > > using user-provided or system-provided keys as trust anchor.
> > > >
> > > > The new kfunc makes it possible to enforce mandatory policies,
> > > > as
> > > > eBPF
> > > > programs might be allowed to make security decisions only based
> > > > on
> > > > data
> > > > sources the system administrator approves.
> > > >
> > > > The caller should provide the data to be verified and the
> > > > signature
> > > > as eBPF
> > > > dynamic pointers (to minimize the number of parameters) and a
> > > > bpf_key
> > > > structure containing a reference to the keyring with keys
> > > > trusted
> > > > for
> > > > signature verification, obtained from bpf_lookup_user_key() or
> > > > bpf_lookup_system_key().
> > > >
> > > > For bpf_key structures obtained from the former lookup
> > > > function,
> > > > bpf_verify_pkcs7_signature() completes the permission check
> > > > deferred by
> > > > that function by calling key_validate(). key_task_permission()
> > > > is
> > > > already
> > > > called by the PKCS#7 code.
> > > >
> > > > Signed-off-by: Roberto Sassu <[email protected]>
> > > > Acked-by: KP Singh <[email protected]>
> > > > ---
> > > > kernel/trace/bpf_trace.c | 45
> > > > ++++++++++++++++++++++++++++++++++++++++
> > > > 1 file changed, 45 insertions(+)
> > > >
> > > > diff --git a/kernel/trace/bpf_trace.c
> > > > b/kernel/trace/bpf_trace.c
> > > > index 7a7023704ac2..8e2c026b0a58 100644
> > > > --- a/kernel/trace/bpf_trace.c
> > > > +++ b/kernel/trace/bpf_trace.c
> > > > @@ -1294,12 +1294,57 @@ void bpf_key_put(struct bpf_key *bkey)
> > > > kfree(bkey);
> > > > }
> > > >
> > > > +#ifdef CONFIG_SYSTEM_DATA_VERIFICATION
> > > > +/**
> > > > + * bpf_verify_pkcs7_signature - verify a PKCS#7 signature
> > > > + * @data_ptr: data to verify
> > > > + * @sig_ptr: signature of the data
> > > > + * @trusted_keyring: keyring with keys trusted for signature
> > > > verification
> > > > + *
> > > > + * Verify the PKCS#7 signature *sig_ptr* against the supplied
> > > > *data_ptr*
> > > > + * with keys in a keyring referenced by *trusted_keyring*.
> > > > + *
> > > > + * Return: 0 on success, a negative value on error.
> > > > + */
> > > > +int bpf_verify_pkcs7_signature(struct bpf_dynptr_kern
> > > > *data_ptr,
> > > > + struct bpf_dynptr_kern *sig_ptr,
> > > > + struct bpf_key *trusted_keyring)
> > > > +{
> > > > + int ret;
> > > > +
> > > > + if (trusted_keyring->has_ref) {
> > > > + /*
> > > > + * Do the permission check deferred in
> > > > bpf_lookup_user_key().
> > > > + * See bpf_lookup_user_key() for more details.
> > > > + *
> > > > + * A call to key_task_permission() here would
> > > > be
> > > > redundant, as
> > > > + * it is already done by keyring_search()
> > > > called by
> > > > + * find_asymmetric_key().
> > > > + */
> > > > + ret = key_validate(trusted_keyring->key);
> > > > + if (ret < 0)
> > > > + return ret;
> > > > + }
> > > > +
> > > > + return verify_pkcs7_signature(data_ptr->data,
> > > > + bpf_dynptr_get_size(data_
> > > > ptr)
> > > > ,
> > > > + sig_ptr->data,
> > > > + bpf_dynptr_get_size(sig_p
> > > > tr),
> > >
> > > MIssing check for data_ptr->data == NULL before making this call?
> > > Same
> > > for sig_ptr.
> >
> > Patch 3 requires the dynptrs to be initialized. Isn't enough?
> >
>
> No, it seems even initialized dynptr can be NULL at runtime. Look at
> both ringbuf_submit_dynptr and ringbuf_discard_dynptr.
> The verifier won't know after ringbuf_reserve_dynptr whether it set
> it
> to NULL or some valid pointer.
>
> dynptr_init is basically that stack slot is now STACK_DYNPTR, it says
> nothing more about the dynptr.
>
> As far as testing this goes, you can pass invalid parameters to
> ringbuf_reserve_dynptr to have it set to NULL, then make sure your
> helper returns an error at runtime for it.
I see, thanks.
I did a quick test. Pass 1 as flags argument to bpf_dynptr_from_mem()
(not supported), and see how bpf_verify_pkcs7_signature() handles it.
Everything seems good, the ASN1 parser called by pkcs7_parse_message()
correctly handles zero length.
So, I will add just this test, right?
Thanks
Roberto
On Wed, 7 Sept 2022 at 14:20, Roberto Sassu
<[email protected]> wrote:
>
> On Wed, 2022-09-07 at 04:28 +0200, Kumar Kartikeya Dwivedi wrote:
> > On Tue, 6 Sept 2022 at 10:08, Roberto Sassu
> > <[email protected]> wrote:
> > > On Tue, 2022-09-06 at 04:57 +0200, Kumar Kartikeya Dwivedi wrote:
> > > > On Mon, 5 Sept 2022 at 16:35, Roberto Sassu
> > > > <[email protected]> wrote:
> > > > > From: Roberto Sassu <[email protected]>
> > > > >
> > > > > Add the bpf_verify_pkcs7_signature() kfunc, to give eBPF
> > > > > security
> > > > > modules
> > > > > the ability to check the validity of a signature against
> > > > > supplied
> > > > > data, by
> > > > > using user-provided or system-provided keys as trust anchor.
> > > > >
> > > > > The new kfunc makes it possible to enforce mandatory policies,
> > > > > as
> > > > > eBPF
> > > > > programs might be allowed to make security decisions only based
> > > > > on
> > > > > data
> > > > > sources the system administrator approves.
> > > > >
> > > > > The caller should provide the data to be verified and the
> > > > > signature
> > > > > as eBPF
> > > > > dynamic pointers (to minimize the number of parameters) and a
> > > > > bpf_key
> > > > > structure containing a reference to the keyring with keys
> > > > > trusted
> > > > > for
> > > > > signature verification, obtained from bpf_lookup_user_key() or
> > > > > bpf_lookup_system_key().
> > > > >
> > > > > For bpf_key structures obtained from the former lookup
> > > > > function,
> > > > > bpf_verify_pkcs7_signature() completes the permission check
> > > > > deferred by
> > > > > that function by calling key_validate(). key_task_permission()
> > > > > is
> > > > > already
> > > > > called by the PKCS#7 code.
> > > > >
> > > > > Signed-off-by: Roberto Sassu <[email protected]>
> > > > > Acked-by: KP Singh <[email protected]>
> > > > > ---
> > > > > kernel/trace/bpf_trace.c | 45
> > > > > ++++++++++++++++++++++++++++++++++++++++
> > > > > 1 file changed, 45 insertions(+)
> > > > >
> > > > > diff --git a/kernel/trace/bpf_trace.c
> > > > > b/kernel/trace/bpf_trace.c
> > > > > index 7a7023704ac2..8e2c026b0a58 100644
> > > > > --- a/kernel/trace/bpf_trace.c
> > > > > +++ b/kernel/trace/bpf_trace.c
> > > > > @@ -1294,12 +1294,57 @@ void bpf_key_put(struct bpf_key *bkey)
> > > > > kfree(bkey);
> > > > > }
> > > > >
> > > > > +#ifdef CONFIG_SYSTEM_DATA_VERIFICATION
> > > > > +/**
> > > > > + * bpf_verify_pkcs7_signature - verify a PKCS#7 signature
> > > > > + * @data_ptr: data to verify
> > > > > + * @sig_ptr: signature of the data
> > > > > + * @trusted_keyring: keyring with keys trusted for signature
> > > > > verification
> > > > > + *
> > > > > + * Verify the PKCS#7 signature *sig_ptr* against the supplied
> > > > > *data_ptr*
> > > > > + * with keys in a keyring referenced by *trusted_keyring*.
> > > > > + *
> > > > > + * Return: 0 on success, a negative value on error.
> > > > > + */
> > > > > +int bpf_verify_pkcs7_signature(struct bpf_dynptr_kern
> > > > > *data_ptr,
> > > > > + struct bpf_dynptr_kern *sig_ptr,
> > > > > + struct bpf_key *trusted_keyring)
> > > > > +{
> > > > > + int ret;
> > > > > +
> > > > > + if (trusted_keyring->has_ref) {
> > > > > + /*
> > > > > + * Do the permission check deferred in
> > > > > bpf_lookup_user_key().
> > > > > + * See bpf_lookup_user_key() for more details.
> > > > > + *
> > > > > + * A call to key_task_permission() here would
> > > > > be
> > > > > redundant, as
> > > > > + * it is already done by keyring_search()
> > > > > called by
> > > > > + * find_asymmetric_key().
> > > > > + */
> > > > > + ret = key_validate(trusted_keyring->key);
> > > > > + if (ret < 0)
> > > > > + return ret;
> > > > > + }
> > > > > +
> > > > > + return verify_pkcs7_signature(data_ptr->data,
> > > > > + bpf_dynptr_get_size(data_
> > > > > ptr)
> > > > > ,
> > > > > + sig_ptr->data,
> > > > > + bpf_dynptr_get_size(sig_p
> > > > > tr),
> > > >
> > > > MIssing check for data_ptr->data == NULL before making this call?
> > > > Same
> > > > for sig_ptr.
> > >
> > > Patch 3 requires the dynptrs to be initialized. Isn't enough?
> > >
> >
> > No, it seems even initialized dynptr can be NULL at runtime. Look at
> > both ringbuf_submit_dynptr and ringbuf_discard_dynptr.
> > The verifier won't know after ringbuf_reserve_dynptr whether it set
> > it
> > to NULL or some valid pointer.
> >
> > dynptr_init is basically that stack slot is now STACK_DYNPTR, it says
> > nothing more about the dynptr.
> >
> > As far as testing this goes, you can pass invalid parameters to
> > ringbuf_reserve_dynptr to have it set to NULL, then make sure your
> > helper returns an error at runtime for it.
>
> I see, thanks.
>
> I did a quick test. Pass 1 as flags argument to bpf_dynptr_from_mem()
> (not supported), and see how bpf_verify_pkcs7_signature() handles it.
>
> Everything seems good, the ASN1 parser called by pkcs7_parse_message()
> correctly handles zero length.
>
> So, I will add just this test, right?
>
Yeah, if it handles it correctly, just adding a test to make sure it
stays that way in the future would be fine.