linux-kernel - RE: [PATCH v5 3/5] bpf: Add bpf_verify_pkcs7

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <03b67c7a6161428c9ff8a5dde0450402@huawei.com>
Date:   Wed, 22 Jun 2022 09:54:18 +0000
From:   Roberto Sassu <roberto.sassu@...wei.com>
To:     John Fastabend <john.fastabend@...il.com>,
        "ast@...nel.org" <ast@...nel.org>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        "andrii@...nel.org" <andrii@...nel.org>,
        "kpsingh@...nel.org" <kpsingh@...nel.org>,
        "songliubraving@...com" <songliubraving@...com>,
        "kafai@...com" <kafai@...com>, "yhs@...com" <yhs@...com>
CC:     "dhowells@...hat.com" <dhowells@...hat.com>,
        "keyrings@...r.kernel.org" <keyrings@...r.kernel.org>,
        "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>
Subject: RE: [PATCH v5 3/5] bpf: Add bpf_verify_pkcs7_signature() helper

> From: John Fastabend [mailto:john.fastabend@...il.com]
> Sent: Wednesday, June 22, 2022 12:28 AM
> 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 keyring pointer obtained with
> > bpf_lookup_user_key() or, alternatively, a keyring ID with values defined
> > in verification.h. While the first choice gives users more flexibility, the
> > second offers better security guarantees, as the keyring selection will not
> > depend on possibly untrusted user space but on the kernel itself.
> >
> > Defined keyring IDs are: 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).
> >
> > Note: since the keyring ID assignment is understood only by
> > verify_pkcs7_signature(), it must be passed directly to the corresponding
> > helper, rather than to a separate new helper returning a struct key pointer
> > with the keyring ID as a pointer value. If such pointer is passed to any
> > other helper which does not check its validity, an illegal memory access
> > could occur.
> >
> > 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           | 39 ++++++++++++++++++++++++++++++++++
> >  tools/include/uapi/linux/bpf.h | 17 +++++++++++++++
> >  3 files changed, 73 insertions(+)
> >
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 7bbcf2cd105d..524bed4d7170 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -5339,6 +5339,22 @@ union bpf_attr {
> >   *		bpf_lookup_user_key() helper.
> >   *	Return
> >   *		0
> > + *
> > + * long bpf_verify_pkcs7_signature(struct bpf_dynptr *data_ptr, struct
> bpf_dynptr *sig_ptr, struct key *trusted_keys, unsigned long keyring_id)
> > + *	Description
> > + *		Verify the PKCS#7 signature *sig* against the supplied *data*
> > + *		with keys in *trusted_keys* or in a keyring with ID
> > + *		*keyring_id*.
> 
> Would be nice to give precedence here so that its obvious order between
> trusted_keys and keyring_id.

Did you mean to add at the end of the sentence:

or in a keyring with ID *keyring_id*, if *trusted_keys* is NULL.

Thanks

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Yang Xi, Li He

> > + *
> > + *		*keyring_id* can have the following values 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).
> > + *	Return
> > + *		0 on success, a negative value on error.
> >   */