> From: Daniel Borkmann [mailto:daniel@xxxxxxxxxxxxx] > Sent: Friday, June 10, 2022 4:49 PM > On 6/10/22 3:59 PM, Roberto Sassu wrote: > > Add the bpf_verify_signature() helper, to give eBPF security modules the > > ability to check the validity of a signature against supplied data, by > > using 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 specify the identifier of the keyring containing the keys > > for signature verification: 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); > > 0xffff for the session keyring (for testing purposes). > > > > The caller should also specify the type of signature. Currently only PKCS#7 > > is supported. > > > > Since the maximum number of parameters of an eBPF helper is 5, the keyring > > and signature types share one (keyring ID: low 16 bits, signature type: > > high 16 bits). > > > > Signed-off-by: Roberto Sassu <roberto.sassu@xxxxxxxxxx> > > Reported-by: kernel test robot <lkp@xxxxxxxxx> (cast warning) > > --- > > include/uapi/linux/bpf.h | 17 +++++++++++++ > > kernel/bpf/bpf_lsm.c | 46 ++++++++++++++++++++++++++++++++++ > > tools/include/uapi/linux/bpf.h | 17 +++++++++++++ > > 3 files changed, 80 insertions(+) > > > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > > index f4009dbdf62d..97521857e44a 100644 > > --- a/include/uapi/linux/bpf.h > > +++ b/include/uapi/linux/bpf.h > > @@ -5249,6 +5249,22 @@ union bpf_attr { > > * Pointer to the underlying dynptr data, NULL if the dynptr is > > * read-only, if the dynptr is invalid, or if the offset and length > > * is out of bounds. > > + * > > + * long bpf_verify_signature(u8 *data, u32 datalen, u8 *sig, u32 siglen, u32 > info) > > + * Description > > + * Verify a signature of length *siglen* against the supplied data > > + * with length *datalen*. *info* contains the keyring identifier > > + * (low 16 bits) and the signature type (high 16 bits). The keyring > > + * identifier can have the following values (some defined in > > + * verification.h): 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); 0xffff for > > + * the session keyring (for testing purposes). > > + * Return > > + * 0 on success, a negative value on error. > > */ > > #define __BPF_FUNC_MAPPER(FN) \ > > FN(unspec), \ > > @@ -5455,6 +5471,7 @@ union bpf_attr { > > FN(dynptr_read), \ > > FN(dynptr_write), \ > > FN(dynptr_data), \ > > + FN(verify_signature), \ > > /* */ > > > > /* 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..20bd850ea3ee 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/module_signature.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,46 @@ 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_signature, u8 *, data, u32, datalen, u8 *, sig, > > + u32, siglen, u32, info) > > +{ > > + unsigned long keyring_id = info & U16_MAX; > > + enum pkey_id_type id_type = info >> 16; > > + const struct cred *cred = current_cred(); > > + struct key *keyring; > > + > > + if (keyring_id > (unsigned long)VERIFY_USE_PLATFORM_KEYRING && > > + keyring_id != U16_MAX) > > + return -EINVAL; > > + > > + keyring = (keyring_id == U16_MAX) ? > > + cred->session_keyring : (struct key *)keyring_id; > > + > > + switch (id_type) { > > + case PKEY_ID_PKCS7: > > + return verify_pkcs7_signature(data, datalen, sig, siglen, > > + keyring, > > + > VERIFYING_UNSPECIFIED_SIGNATURE, > > + NULL, NULL); > > + default: > > + return -EOPNOTSUPP; > > Question to you & KP: > > > Can we keep the helper generic so that it can be extended to more types of > > signatures and pass the signature type as an enum? > > How many different signature types do we expect, say, in the next 6mo, to land > here? Just thinking out loud whether it is better to keep it simple as with the > last iteration where we have a helper specific to pkcs7, and if needed in future > we add others. We only have the last reg as auxillary arg where we need to > squeeze > all info into it now. What if for other, future signature types this won't suffice? I would add at least another for PGP, assuming that the code will be upstreamed. But I agree, the number should not be that high. > > + } > > +} > > + > > +static const struct bpf_func_proto bpf_verify_signature_proto = { > > + .func = bpf_verify_signature, > > + .gpl_only = false, > > + .ret_type = RET_INTEGER, > > + .arg1_type = ARG_PTR_TO_MEM, > > + .arg2_type = ARG_CONST_SIZE_OR_ZERO, > > Can verify_pkcs7_signature() handle null/0 len for data* args? Shouldn't ARG_PTR_TO_MEM require valid memory? 0 len should not be a problem. Thanks Roberto HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Yang Xi, Li He > > + .arg3_type = ARG_PTR_TO_MEM, > > + .arg4_type = ARG_CONST_SIZE_OR_ZERO, > > Ditto for sig* args? > > > + .arg5_type = ARG_ANYTHING, > > + .allowed = bpf_ima_inode_hash_allowed, > > +}; > > +#endif > > + > > static const struct bpf_func_proto * > > bpf_lsm_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) > > { > > @@ -158,6 +200,10 @@ bpf_lsm_func_proto(enum bpf_func_id func_id, > const struct bpf_prog *prog) > > return prog->aux->sleepable ? &bpf_ima_file_hash_proto : > NULL; > > case BPF_FUNC_get_attach_cookie: > > return bpf_prog_has_trampoline(prog) ? > &bpf_get_attach_cookie_proto : NULL; > > +#ifdef CONFIG_SYSTEM_DATA_VERIFICATION > > + case BPF_FUNC_verify_signature: > > + return prog->aux->sleepable ? &bpf_verify_signature_proto : > NULL; > > +#endif > > default: > > return tracing_prog_func_proto(func_id, prog); > > }
