Messages in this thread | | | Subject | Re: [PATCH v6 4/5] bpf: Add bpf_verify_pkcs7_signature() helper | From | Daniel Borkmann <> | Date | Wed, 6 Jul 2022 18:03:49 +0200 |
| |
On 6/28/22 2:27 PM, Roberto Sassu wrote: > Add the bpf_verify_pkcs7_signature() helper, 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 helper 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 both the data to be verified and the signature as > eBPF dynamic pointers (to minimize the number of parameters). > > The caller should also provide a trusted keyring serial, together with key > lookup-specific flags, to determine which keys can be used for signature > verification. Alternatively, the caller could specify zero as serial value > (not valid, serials must be positive), and provide instead a special > keyring ID. > > Key lookup flags are defined in include/linux/key.h and can be: 1, to > request that special keyrings be created if referred to directly; 2 to > permit partially constructed keys to be found. > > Special IDs are defined in include/linux/verification.h and can be: 0 for > the primary keyring (immutable keyring of system keys); 1 for both the > primary and secondary keyring (where keys can be added only if they are > vouched for by existing keys in those keyrings); 2 for the platform keyring > (primarily used by the integrity subsystem to verify a kexec'ed kerned > image and, possibly, the initramfs signature). > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> > Reported-by: kernel test robot <lkp@intel.com> (cast warning)
nit: Given this a new feature not a fix to existing code, there is no need to add the above reported-by from kbuild bot.
> --- > include/uapi/linux/bpf.h | 24 +++++++++++++ > kernel/bpf/bpf_lsm.c | 63 ++++++++++++++++++++++++++++++++++ > tools/include/uapi/linux/bpf.h | 24 +++++++++++++ > 3 files changed, 111 insertions(+) > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > index e81362891596..b4f5ad863281 100644 > --- a/include/uapi/linux/bpf.h > +++ b/include/uapi/linux/bpf.h > @@ -5325,6 +5325,29 @@ union bpf_attr { > * **-EACCES** if the SYN cookie is not valid. > * > * **-EPROTONOSUPPORT** if CONFIG_IPV6 is not builtin. > + * > + * long bpf_verify_pkcs7_signature(struct bpf_dynptr *data_ptr, struct bpf_dynptr *sig_ptr, u32 trusted_keyring_serial, unsigned long lookup_flags, unsigned long trusted_keyring_id)
nit: for the args instead of ulong, just do u64
> + * Description > + * Verify the PKCS#7 signature *sig_ptr* against the supplied > + * *data_ptr* with keys in a keyring with serial > + * *trusted_keyring_serial*, searched with *lookup_flags*, if the > + * parameter value is positive, or alternatively in a keyring with > + * special ID *trusted_keyring_id* if *trusted_keyring_serial* is > + * zero. > + * > + * *lookup_flags* are defined in include/linux/key.h and can be: 1, > + * to request that special keyrings be created if referred to > + * directly; 2 to permit partially constructed keys to be found. > + * > + * Special IDs are defined in include/linux/verification.h and can > + * be: 0 for the primary keyring (immutable keyring of system > + * keys); 1 for both the primary and secondary keyring (where keys > + * can be added only if they are vouched for by existing keys in > + * those keyrings); 2 for the platform keyring (primarily used by > + * the integrity subsystem to verify a kexec'ed kerned image and, > + * possibly, the initramfs signature). > + * Return > + * 0 on success, a negative value on error. > */ > #define __BPF_FUNC_MAPPER(FN) \ > FN(unspec), \ > @@ -5535,6 +5558,7 @@ union bpf_attr { > FN(tcp_raw_gen_syncookie_ipv6), \ > FN(tcp_raw_check_syncookie_ipv4), \ > FN(tcp_raw_check_syncookie_ipv6), \ > + FN(verify_pkcs7_signature), \
(Needs rebase)
> /* */ > > /* integer value in 'imm' field of BPF_CALL instruction selects which helper > diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c > index c1351df9f7ee..401bda01ad84 100644 > --- a/kernel/bpf/bpf_lsm.c > +++ b/kernel/bpf/bpf_lsm.c > @@ -16,6 +16,8 @@ > #include <linux/bpf_local_storage.h> > #include <linux/btf_ids.h> > #include <linux/ima.h> > +#include <linux/verification.h> > +#include <linux/key.h> > > /* For every LSM hook that allows attachment of BPF programs, declare a nop > * function where a BPF program can be attached. > @@ -132,6 +134,62 @@ static const struct bpf_func_proto bpf_get_attach_cookie_proto = { > .arg1_type = ARG_PTR_TO_CTX, > }; > > +#ifdef CONFIG_SYSTEM_DATA_VERIFICATION > +BPF_CALL_5(bpf_verify_pkcs7_signature, struct bpf_dynptr_kern *, data_ptr, > + struct bpf_dynptr_kern *, sig_ptr, u32, trusted_keyring_serial, > + unsigned long, lookup_flags, unsigned long, trusted_keyring_id) > +{ > + key_ref_t trusted_keyring_ref; > + struct key *trusted_keyring; > + int ret; > + > + /* Keep in sync with defs in include/linux/key.h. */ > + if (lookup_flags > KEY_LOOKUP_PARTIAL) > + return -EINVAL;
iiuc, the KEY_LOOKUP_* is a mask, so you could also combine the two, e.g. KEY_LOOKUP_CREATE | KEY_LOOKUP_PARTIAL. I haven't seen you mentioning anything specific on why it is not allowed. What's the rationale, if it's intentional if should probably be documented?
At minimum I also think the helper description needs to be improved for people to understand enough w/o reading through the kernel source, e.g. wrt lookup_flags since I haven't seen it in your selftests either ... when does a user need to use the given flags.
nit: when both trusted_keyring_serial and trusted_keyring_id are passed to the helper, then this should be rejected as invalid argument? (Kind of feels a bit like we're cramming two things in one helper.. KP, thoughts? :))
> + /* Keep in sync with defs in include/linux/verification.h. */ > + if (trusted_keyring_id > (unsigned long)VERIFY_USE_PLATFORM_KEYRING) > + return -EINVAL; > + > + if (trusted_keyring_serial) { > + trusted_keyring_ref = lookup_user_key(trusted_keyring_serial, > + lookup_flags, > + KEY_NEED_SEARCH); > + if (IS_ERR(trusted_keyring_ref)) > + return PTR_ERR(trusted_keyring_ref); > + > + trusted_keyring = key_ref_to_ptr(trusted_keyring_ref); > + goto verify; > + } > + > + trusted_keyring = (struct key *)trusted_keyring_id; > +verify: > + ret = verify_pkcs7_signature(data_ptr->data, > + bpf_dynptr_get_size(data_ptr), > + sig_ptr->data, > + bpf_dynptr_get_size(sig_ptr), > + trusted_keyring, > + VERIFYING_UNSPECIFIED_SIGNATURE, NULL, > + NULL); > + if (trusted_keyring_serial) > + key_put(trusted_keyring); > + > + return ret; > +} > + > +static const struct bpf_func_proto bpf_verify_pkcs7_signature_proto = { > + .func = bpf_verify_pkcs7_signature, > + .gpl_only = false, > + .ret_type = RET_INTEGER, > + .arg1_type = ARG_PTR_TO_DYNPTR | DYNPTR_TYPE_LOCAL, > + .arg2_type = ARG_PTR_TO_DYNPTR | DYNPTR_TYPE_LOCAL, > + .arg3_type = ARG_ANYTHING, > + .arg4_type = ARG_ANYTHING, > + .arg5_type = ARG_ANYTHING, > + .allowed = bpf_ima_inode_hash_allowed, > +}; > +#endif /* CONFIG_SYSTEM_DATA_VERIFICATION */ > +
| |