Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 22 Jun 2022 18:27:38 -0700
From:   John Fastabend <john.fastabend@gmail.com>
To:     Roberto Sassu <roberto.sassu@huawei.com>,
        John Fastabend <john.fastabend@gmail.com>,
        "ast@kernel.org" <ast@kernel.org>,
        "daniel@iogearbox.net" <daniel@iogearbox.net>,
        "andrii@kernel.org" <andrii@kernel.org>,
        "kpsingh@kernel.org" <kpsingh@kernel.org>,
        "songliubraving@fb.com" <songliubraving@fb.com>,
        "kafai@fb.com" <kafai@fb.com>, "yhs@fb.com" <yhs@fb.com>
Cc:     "dhowells@redhat.com" <dhowells@redhat.com>,
        "keyrings@vger.kernel.org" <keyrings@vger.kernel.org>,
        "bpf@vger.kernel.org" <bpf@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kselftest@vger.kernel.org" <linux-kselftest@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        kernel test robot <lkp@intel.com>
Message-ID: <62b3c18ab4dda_6a3b220812@john.notmuch>
In-Reply-To: <03b67c7a6161428c9ff8a5dde0450402@huawei.com>
References: <20220621163757.760304-1-roberto.sassu@huawei.com>
 <20220621163757.760304-4-roberto.sassu@huawei.com>
 <62b245e22effa_1627420871@john.notmuch>
 <03b67c7a6161428c9ff8a5dde0450402@huawei.com>
Subject: RE: [PATCH v5 3/5] bpf: Add bpf_verify_pkcs7_signature() helper
Mime-Version: 1.0
Content-Type: text/plain;
 charset=utf-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

Roberto Sassu wrote:
> > From: John Fastabend [mailto:john.fastabend@gmail.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@huawei.com>
> > > Reported-by: kernel test robot <lkp@intel.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.

Yes something like this.