[v17,07/12] bpf: Add bpf_verify_pkcs7_signature() kfunc

Commit Message

Roberto Sassu Sept. 9, 2022, 12:07 p.m. UTC

From: Roberto Sassu <[e-mail]>

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 <[e-mail]>
Acked-by: KP Singh <[e-mail]>
---
 kernel/trace/bpf_trace.c | 45 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

Comments

Song Liu Sept. 9, 2022, 4:06 p.m. UTC | #1

On Fri, Sep 9, 2022 at 1:09 PM Roberto Sassu
<[e-mail]> wrote:
>
> From: Roberto Sassu <[e-mail]>
>
> 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 <[e-mail]>
> Acked-by: KP Singh <[e-mail]>

Acked-by: Song Liu <[e-mail]>

[...]

KP Singh Sept. 11, 2022, 11:40 a.m. UTC | #2

On Fri, Sep 9, 2022 at 2:09 PM Roberto Sassu
<[e-mail]> wrote:
>
> From: Roberto Sassu <[e-mail]>
>
> 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 <[e-mail]>
> Acked-by: KP Singh <[e-mail]>
> ---
>  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 ab183dbaa8d1..9df53c40cffd 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);
> +}

This seems to work if the data that needs to be verified
and the signature is allocated onto the map.

For BPF program signing, the signature will be void * pointer (and length)
in a struct in the kernel

+++ b/include/uapi/linux/bpf.h
@@ -1383,6 +1383,8 @@ union bpf_attr {
                __aligned_u64   fd_array;       /* array of FDs */
                __aligned_u64   core_relos;
                __u32           core_relo_rec_size; /* sizeof(struct
bpf_core_relo) */
+               __aligned_u64   signature;
+               __u32           signature_size;
        };

Something like this in the bpf_prog_aux struct which is passed to
security_bpf_prog_alloc.

Now creating a dynptr to use with this kfunc does not work:

   bpf_dynptr_from_mem(aux->signature, aux->signature_size, 0, &sig_ptr);

So one has to copy kernel data into a map and then create dynptrs.
Would you be able to update
the dynptr logic to handle this case too? (follow up is okay too).

- KP


> +#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
>

Kumar Kartikeya Dwivedi Sept. 11, 2022, 9:08 p.m. UTC | #3

On Sun, 11 Sept 2022 at 13:41, KP Singh <[e-mail]> wrote:
>
> On Fri, Sep 9, 2022 at 2:09 PM Roberto Sassu
> <[e-mail]> wrote:
> >
> > From: Roberto Sassu <[e-mail]>
> >
> > 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 <[e-mail]>
> > Acked-by: KP Singh <[e-mail]>
> > ---
> >  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 ab183dbaa8d1..9df53c40cffd 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);
> > +}
>
> This seems to work if the data that needs to be verified
> and the signature is allocated onto the map.
>
> For BPF program signing, the signature will be void * pointer (and length)
> in a struct in the kernel
>
> +++ b/include/uapi/linux/bpf.h
> @@ -1383,6 +1383,8 @@ union bpf_attr {
>                 __aligned_u64   fd_array;       /* array of FDs */
>                 __aligned_u64   core_relos;
>                 __u32           core_relo_rec_size; /* sizeof(struct
> bpf_core_relo) */
> +               __aligned_u64   signature;
> +               __u32           signature_size;
>         };
>
> Something like this in the bpf_prog_aux struct which is passed to
> security_bpf_prog_alloc.
>
> Now creating a dynptr to use with this kfunc does not work:
>
>    bpf_dynptr_from_mem(aux->signature, aux->signature_size, 0, &sig_ptr);
>
> So one has to copy kernel data into a map and then create dynptrs.
> Would you be able to update
> the dynptr logic to handle this case too? (follow up is okay too).
>

ISTM it needs the feature first before it can be added.
To make it work like map_val, value_size(which is a constant) to pass
to bpf_dynptr_from_mem,
verifier will have to mark load of aux->signature as PTR_TO_MEM with the known
constant size, and then mark_reg_known for scalar reg for aux->signature_size.
Since we need to know that 0 <= r2 <= r1.mem_size.
This would require some work on the btf_struct_access handling.

It cannot be made to work in the general case of void * and len.
There might also be other better options (like kernel itself preparing
read only bpf_dynptr struct in bpf_prog_aux for the signature) so you
can pass its address directly to the kfunc.

Patch

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index ab183dbaa8d1..9df53c40cffd 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 = {