From: Alexei Starovoitov alexei.starovoitov@gmail.com Sent: Friday, June 17, 2022 5:46 AM
Adding in CC the keyring mailing list and David.
Sort summary: we are adding an eBPF helper, to let eBPF programs verify PKCS#7 signatures. The helper simply calls verify_pkcs7_signature().
The problem is how to pass the key for verification.
For hardcoded keyring IDs, it is easy, pass 0, 1 or 2 for respectively the built-in, secondary and platform keyring.
If you want to pass another keyring, you need to do a lookup, which returns a key with reference count increased.
While in the kernel you can call key_put() to decrease the reference count, that is not guaranteed with an eBPF program, if the developer forgets about it. What probably is necessary, is to add the capability to the verifier to check whether the reference count is decreased, or adding a callback mechanism to call automatically key_put() when the eBPF program is terminated.
Is there an alternative solution?
Thanks
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Yang Xi, Li He
On Tue, Jun 14, 2022 at 03:06:19PM +0200, Roberto Sassu wrote:
Add the bpf_request_key_by_id() helper, so that an eBPF program can obtain a suitable key pointer to pass to the bpf_verify_pkcs7_signature() helper, to be introduced in a later patch.
The passed identifier can have the following values: 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); ULONG_MAX for the session keyring (for testing purposes).
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com
include/uapi/linux/bpf.h | 17 +++++++++++++++++ kernel/bpf/bpf_lsm.c | 30 ++++++++++++++++++++++++++++++ scripts/bpf_doc.py | 2 ++ tools/include/uapi/linux/bpf.h | 17 +++++++++++++++++ 4 files changed, 66 insertions(+)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index f4009dbdf62d..dfd93e0e0759 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 isread-only, if the dynptr is invalid, or if the offset and lengthis out of bounds.
- struct key *bpf_request_key_by_id(unsigned long id)
- Description
Request a keyring by *id*.
*id* can have the following values (some defined in
verification.h): 0 for the primary keyring (immutable keyringof
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 akexec'ed
kerned image and, possibly, the initramfs signature);ULONG_MAX
for the session keyring (for testing purposes).It's never ok to add something like this to uapi 'for testing purposes'. If it's not useful in general it should not be a part of api.
- Return
A non-NULL pointer if *id* is valid and not 0, a NULL pointer*/
otherwise.#define __BPF_FUNC_MAPPER(FN) \ FN(unspec), \ @@ -5455,6 +5471,7 @@ union bpf_attr { FN(dynptr_read), \ FN(dynptr_write), \ FN(dynptr_data), \
- FN(request_key_by_id), \ /* */
/* 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..e1911812398b 100644 --- a/kernel/bpf/bpf_lsm.c +++ b/kernel/bpf/bpf_lsm.c @@ -16,6 +16,7 @@ #include <linux/bpf_local_storage.h> #include <linux/btf_ids.h> #include <linux/ima.h> +#include <linux/verification.h>
/* For every LSM hook that allows attachment of BPF programs, declare a
nop
- function where a BPF program can be attached.
@@ -132,6 +133,31 @@ static const struct bpf_func_proto
bpf_get_attach_cookie_proto = {
.arg1_type = ARG_PTR_TO_CTX, };
+#ifdef CONFIG_KEYS +BTF_ID_LIST_SINGLE(bpf_request_key_by_id_btf_ids, struct, key)
+BPF_CALL_1(bpf_request_key_by_id, unsigned long, id) {
- const struct cred *cred = current_cred();
- if (id > (unsigned long)VERIFY_USE_PLATFORM_KEYRING && id !=
ULONG_MAX)
return (unsigned long)NULL;- if (id == ULONG_MAX)
return (unsigned long)cred->session_keyring;- return id;
It needs to do a proper lookup. Why cannot it do lookup_user_key ? The helper needs 'flags' arg too. Please think hard of extensibility and long term usefulness of api. At present this api feels like it was 'let me just hack something quickly'. Not ok.