| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 10 Jun 2022 14:59:15 +0000
From: Roberto Sassu <roberto.sassu@...wei.com>
To: Daniel Borkmann <daniel@...earbox.net>,
"ast@...nel.org" <ast@...nel.org>,
"andrii@...nel.org" <andrii@...nel.org>,
"kpsingh@...nel.org" <kpsingh@...nel.org>
CC: "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
kernel test robot <lkp@...el.com>,
"john.fastabend@...il.com" <john.fastabend@...il.com>
Subject: RE: [PATCH v3 1/2] bpf: Add bpf_verify_signature() helper
> From: Daniel Borkmann [mailto:daniel@...earbox.net]
> 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@...wei.com>
> > Reported-by: kernel test robot <lkp@...el.com> (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);
> > }
Powered by blists - more mailing lists