RE: [PATCH v3 1/2] bpf: Add bpf_verify_signature() helper

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 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);
> >   	}




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux