One of the desirable features in security is the ability to restrict import of data to a given system based on data authenticity. If data import can be restricted, it would be possible to enforce a system-wide policy based on the signing keys the system owner trusts.
This feature is widely used in the kernel. For example, if the restriction is enabled, kernel modules can be plugged in only if they are signed with a key whose public part is in the primary or secondary keyring.
For eBPF, it can be useful as well. For example, it might be useful to authenticate data an eBPF program makes security decisions on.
After a discussion in the eBPF mailing list, it was decided that the stated goal should be accomplished by introducing a new helper: bpf_verify_pkcs7_signature(), dedicated to verify PKCS#7 signatures. More helpers will be introduced later, as necessary.
The job of bpf_verify_pkcs7_signature() is to retrieve the trusted keyring from function parameters, and to perform signature verification by calling verify_pkcs7_signature().
Data and signature can be provided to the new helper with two dynamic pointers, to reduce the number of parameters. The keyring can be provided by its serial, or by its special ID defined in verification.h, if the serial is zero (not a valid value). bpf_verify_pkcs7_signature() also accepts key lookup-specific flags, passed to lookup_user_key() when the helper searches the keyring by its serial.
While passing the keyring serial to bpf_verify_pkcs7_signature() provides great flexibility, it seems suboptimal in terms of security guarantees, as even if the eBPF program is assumed to be trusted, that serial might come from untrusted user space not choosing one that the system administrator approves to enforce a mandatory policy. The same goal could be instead more easily achieved by setting a hardcoded keyring ID in the signed eBPF program, to be passed to bpf_verify_pkcs7_signature().
bpf_verify_pkcs7_signature() can be called only from sleepable programs, because of memory allocation (with lookup flag KEY_LOOKUP_CREATE) and crypto operations. For example, the lsm.s/bpf attach point is suitable, fexit/array_map_update_elem is not.
A test was added to check the ability of bpf_verify_pkcs7_signature() to verify PKCS#7 signatures from the session keyring, a newly-created keyring, and from the primary and secondary keyring (taking the tcp_bic.ko kernel module for the verification). The test does not fail if that kernel module is not found (needs support from the CI).
A consideration was made on whether bpf_verify_pkcs7_signature() should be a simple wrapper, doing as little as possible, or whether it could have more complex logic. Having a simple and flexible wrapper requires two additional helpers, bpf_lookup_user_key() and bpf_key_put(), to search and acquire a key reference, pass that key to the wrapper, and release the reference. More care is also required on the eBPF verifier side, to ensure that an eBPF program always releases an acquired reference.
While that gives eBPF developers the greatest flexibility to use the helpers as necessary, it does not match the security of the solution of retrieving the key and using it within the same function, as for example in security/keys/keyctl.c. The risk is that an eBPF program requests a key for a purpose, and then uses the key in a different way with one of the available key-related helpers (to be added in the future).
struct key is not like a file descriptor, carrying permissions requested during an open, that can be revalidated at the time a read or write is performed. It is more close to a struct inode, the function using the key cannot know reliably which permission was requested at lookup time.
For that reason, the key lookup and usage cannot be separated, as the kernel will guarantee (also to other MAC mechanisms) that once a key has been requested with a specific purpose, it will be used accordingly, beyond the control of eBFP programs.
The patch set is organized as follows.
Patch 1 exports bpf_dynptr_get_size(), to obtain the real size of data carried by a dynamic pointer. Patch 2 makes available for new eBPF helpers some key-related definitions. Patch 3 fixes the helper prototype regular expression to accept unsigned as type prefix. Patch 4 introduces the bpf_verify_pkcs7_signature() helper and patch 5 adds the corresponding test.
Changelog
v5: - Move KEY_LOOKUP_ to include/linux/key.h for validation of bpf_verify_pkcs7_signature() parameter - Remove bpf_lookup_user_key() and bpf_key_put() helpers, and the corresponding tests - Replace struct key parameter of bpf_verify_pkcs7_signature() with the keyring serial and lookup flags - Call lookup_user_key() and key_put() in bpf_verify_pkcs7_signature() code, to ensure that the retrieved key is used according to the permission requested at lookup time - Clarified keyring precedence in the description of bpf_verify_pkcs7_signature() (suggested by John) - Remove newline in the second argument of ASSERT_ - Fix helper prototype regular expression in bpf_doc.py
v4: - Remove bpf_request_key_by_id(), don't return an invalid pointer that other helpers can use - Pass the keyring ID (without ULONG_MAX, suggested by Alexei) to bpf_verify_pkcs7_signature() - Introduce bpf_lookup_user_key() and bpf_key_put() helpers (suggested by Alexei) - Add lookup_key_norelease test, to ensure that the verifier blocks eBPF programs which don't decrement the key reference count - Parse raw PKCS#7 signature instead of module-style signature in the verify_pkcs7_signature test (suggested by Alexei) - Parse kernel module in user space and pass raw PKCS#7 signature to the eBPF program for signature verification
v3: - Rename bpf_verify_signature() back to bpf_verify_pkcs7_signature() to avoid managing different parameters for each signature verification function in one helper (suggested by Daniel) - Use dynamic pointers and export bpf_dynptr_get_size() (suggested by Alexei) - Introduce bpf_request_key_by_id() to give more flexibility to the caller of bpf_verify_pkcs7_signature() to retrieve the appropriate keyring (suggested by Alexei) - Fix test by reordering the gcc command line, always compile sign-file - Improve helper support check mechanism in the test
v2: - Rename bpf_verify_pkcs7_signature() to a more generic bpf_verify_signature() and pass the signature type (suggested by KP) - Move the helper and prototype declaration under #ifdef so that user space can probe for support for the helper (suggested by Daniel) - Describe better the keyring types (suggested by Daniel) - Include linux/bpf.h instead of vmlinux.h to avoid implicit or redeclaration - Make the test selfcontained (suggested by Alexei)
v1: - Don't define new map flag but introduce simple wrapper of verify_pkcs7_signature() (suggested by Alexei and KP)
Roberto Sassu (5): bpf: Export bpf_dynptr_get_size() KEYS: Move KEY_LOOKUP_ to include/linux/key.h scripts: Handle unsigned type prefix in bpf_doc.py bpf: Add bpf_verify_pkcs7_signature() helper selftests/bpf: Add test for bpf_verify_pkcs7_signature() helper
include/linux/bpf.h | 1 + include/linux/key.h | 3 + include/uapi/linux/bpf.h | 24 ++ kernel/bpf/bpf_lsm.c | 63 +++ kernel/bpf/helpers.c | 2 +- scripts/bpf_doc.py | 2 +- security/keys/internal.h | 2 - tools/include/uapi/linux/bpf.h | 24 ++ tools/testing/selftests/bpf/Makefile | 14 +- tools/testing/selftests/bpf/config | 2 + .../bpf/prog_tests/verify_pkcs7_sig.c | 359 ++++++++++++++++++ .../bpf/progs/test_verify_pkcs7_sig.c | 79 ++++ .../testing/selftests/bpf/verify_sig_setup.sh | 104 +++++ 13 files changed, 672 insertions(+), 7 deletions(-) create mode 100644 tools/testing/selftests/bpf/prog_tests/verify_pkcs7_sig.c create mode 100644 tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c create mode 100755 tools/testing/selftests/bpf/verify_sig_setup.sh
Export bpf_dynptr_get_size(), so that kernel code dealing with eBPF dynamic pointers can obtain the real size of data carried by this data structure.
Reviewed-by: Joanne Koong joannelkoong@gmail.com Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- include/linux/bpf.h | 1 + kernel/bpf/helpers.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index d05e1495a06e..3ec0f167e9d7 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -2521,5 +2521,6 @@ void bpf_dynptr_init(struct bpf_dynptr_kern *ptr, void *data, enum bpf_dynptr_type type, u32 offset, u32 size); void bpf_dynptr_set_null(struct bpf_dynptr_kern *ptr); int bpf_dynptr_check_size(u32 size); +u32 bpf_dynptr_get_size(struct bpf_dynptr_kern *ptr);
#endif /* _LINUX_BPF_H */ diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index a1c84d256f83..3f5ff8dbd3cb 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -1430,7 +1430,7 @@ static void bpf_dynptr_set_type(struct bpf_dynptr_kern *ptr, enum bpf_dynptr_typ ptr->size |= type << DYNPTR_TYPE_SHIFT; }
-static u32 bpf_dynptr_get_size(struct bpf_dynptr_kern *ptr) +u32 bpf_dynptr_get_size(struct bpf_dynptr_kern *ptr) { return ptr->size & DYNPTR_SIZE_MASK; }
In preparation for the patch that introduces the bpf_verify_pkcs7_signature() eBPF helper, move KEY_LOOKUP_ definitions to include/linux/key.h, to be able to validate the helper parameters.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- include/linux/key.h | 3 +++ security/keys/internal.h | 2 -- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/linux/key.h b/include/linux/key.h index 7febc4881363..a297e075038c 100644 --- a/include/linux/key.h +++ b/include/linux/key.h @@ -88,6 +88,9 @@ enum key_need_perm { KEY_DEFER_PERM_CHECK, /* Special: permission check is deferred */ };
+#define KEY_LOOKUP_CREATE 0x01 +#define KEY_LOOKUP_PARTIAL 0x02 + struct seq_file; struct user_struct; struct signal_struct; diff --git a/security/keys/internal.h b/security/keys/internal.h index 9b9cf3b6fcbb..3c1e7122076b 100644 --- a/security/keys/internal.h +++ b/security/keys/internal.h @@ -165,8 +165,6 @@ extern struct key *request_key_and_link(struct key_type *type,
extern bool lookup_user_key_possessed(const struct key *key, const struct key_match_data *match_data); -#define KEY_LOOKUP_CREATE 0x01 -#define KEY_LOOKUP_PARTIAL 0x02
extern long join_session_keyring(const char *name); extern void key_change_session_keyring(struct callback_head *twork);
While unsigned long is an accepted parameter type, the regular expression validating helper prototypes does not correctly take into account types composed by multiple words.
The regular expression:
((const )?(struct )?(\w+|...)( **\w+)?)
accepts only const and struct as prefix before the type. The following part of the regular expression expects a word with [a-zA-Z0-9_] characters (without space), so it would get just unsigned. Parsing words with \w+ in greedy mode makes the regular expression work even if the type is composed by two words, but not always. It wouldn't have been the case in possessive mode \w++ (don't give back characters to match the regular expression).
Simply adding unsigned as possible prefix is not correct, as the struct unsigned combination is not legal. Make instead struct and unsigned as alternatives, so that the following new combinations are legal:
unsigned type struct type const unsigned type
and not:
struct unsigned type
The regular expression is a preliminary check. The type, other than being legal, must be also present in the known_types array.
Don't mention the change in the regular expression description, as it is assumed that type implies also multiple words types.
At this point, don't switch from greedy to possessive mode (\w+ -> \w++) to avoid partial parsing of the type of helper parameters, as this functionality has only been added recently in Python 3.11.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- scripts/bpf_doc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/bpf_doc.py b/scripts/bpf_doc.py index a0ec321469bd..25e79d811487 100755 --- a/scripts/bpf_doc.py +++ b/scripts/bpf_doc.py @@ -124,7 +124,7 @@ class HeaderParser(object): # - Same as above, with "const" and/or "struct" in front of type # - "..." (undefined number of arguments, for bpf_trace_printk()) # There is at least one term ("void"), and at most five arguments. - p = re.compile(' * ?((.+) **\w+((((const )?(struct )?(\w+|...)( **\w+)?)(, )?){1,5}))$') + p = re.compile(' * ?((.+) **\w+((((const )?((struct )|(unsigned )?)(\w+|...)( **\w+)?)(, )?){1,5}))$') capture = p.match(self.line) if not capture: raise NoHelperFound
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 trusted keyring serial, together with key lookup-specific flags, to determine which keys can be used for signature verification. Alternatively, the caller could specify zero as serial value (not valid, serials must be positive), and provide instead a special keyring ID.
Key lookup flags are defined in include/linux/key.h and can be: 1, to request that special keyrings be created if referred to directly; 2 to permit partially constructed keys to be found.
Special IDs are defined in include/linux/verification.h and can be: 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).
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com Reported-by: kernel test robot lkp@intel.com (cast warning) --- include/uapi/linux/bpf.h | 24 +++++++++++++ kernel/bpf/bpf_lsm.c | 63 ++++++++++++++++++++++++++++++++++ tools/include/uapi/linux/bpf.h | 24 +++++++++++++ 3 files changed, 111 insertions(+)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index e81362891596..b4f5ad863281 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -5325,6 +5325,29 @@ union bpf_attr { * **-EACCES** if the SYN cookie is not valid. * * **-EPROTONOSUPPORT** if CONFIG_IPV6 is not builtin. + * + * long bpf_verify_pkcs7_signature(struct bpf_dynptr *data_ptr, struct bpf_dynptr *sig_ptr, u32 trusted_keyring_serial, unsigned long lookup_flags, unsigned long trusted_keyring_id) + * Description + * Verify the PKCS#7 signature *sig_ptr* against the supplied + * *data_ptr* with keys in a keyring with serial + * *trusted_keyring_serial*, searched with *lookup_flags*, if the + * parameter value is positive, or alternatively in a keyring with + * special ID *trusted_keyring_id* if *trusted_keyring_serial* is + * zero. + * + * *lookup_flags* are defined in include/linux/key.h and can be: 1, + * to request that special keyrings be created if referred to + * directly; 2 to permit partially constructed keys to be found. + * + * Special IDs are defined in include/linux/verification.h and can + * be: 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. */ #define __BPF_FUNC_MAPPER(FN) \ FN(unspec), \ @@ -5535,6 +5558,7 @@ union bpf_attr { FN(tcp_raw_gen_syncookie_ipv6), \ FN(tcp_raw_check_syncookie_ipv4), \ FN(tcp_raw_check_syncookie_ipv6), \ + FN(verify_pkcs7_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..401bda01ad84 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/key.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,62 @@ 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_pkcs7_signature, struct bpf_dynptr_kern *, data_ptr, + struct bpf_dynptr_kern *, sig_ptr, u32, trusted_keyring_serial, + unsigned long, lookup_flags, unsigned long, trusted_keyring_id) +{ + key_ref_t trusted_keyring_ref; + struct key *trusted_keyring; + int ret; + + /* Keep in sync with defs in include/linux/key.h. */ + if (lookup_flags > KEY_LOOKUP_PARTIAL) + return -EINVAL; + + /* Keep in sync with defs in include/linux/verification.h. */ + if (trusted_keyring_id > (unsigned long)VERIFY_USE_PLATFORM_KEYRING) + return -EINVAL; + + if (trusted_keyring_serial) { + trusted_keyring_ref = lookup_user_key(trusted_keyring_serial, + lookup_flags, + KEY_NEED_SEARCH); + if (IS_ERR(trusted_keyring_ref)) + return PTR_ERR(trusted_keyring_ref); + + trusted_keyring = key_ref_to_ptr(trusted_keyring_ref); + goto verify; + } + + trusted_keyring = (struct key *)trusted_keyring_id; +verify: + ret = verify_pkcs7_signature(data_ptr->data, + bpf_dynptr_get_size(data_ptr), + sig_ptr->data, + bpf_dynptr_get_size(sig_ptr), + trusted_keyring, + VERIFYING_UNSPECIFIED_SIGNATURE, NULL, + NULL); + if (trusted_keyring_serial) + key_put(trusted_keyring); + + return ret; +} + +static const struct bpf_func_proto bpf_verify_pkcs7_signature_proto = { + .func = bpf_verify_pkcs7_signature, + .gpl_only = false, + .ret_type = RET_INTEGER, + .arg1_type = ARG_PTR_TO_DYNPTR | DYNPTR_TYPE_LOCAL, + .arg2_type = ARG_PTR_TO_DYNPTR | DYNPTR_TYPE_LOCAL, + .arg3_type = ARG_ANYTHING, + .arg4_type = ARG_ANYTHING, + .arg5_type = ARG_ANYTHING, + .allowed = bpf_ima_inode_hash_allowed, +}; +#endif /* CONFIG_SYSTEM_DATA_VERIFICATION */ + static const struct bpf_func_proto * bpf_lsm_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) { @@ -158,6 +216,11 @@ 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_pkcs7_signature: + return prog->aux->sleepable ? + &bpf_verify_pkcs7_signature_proto : NULL; +#endif /* CONFIG_SYSTEM_DATA_VERIFICATION */ default: return tracing_prog_func_proto(func_id, prog); } diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index e81362891596..b4f5ad863281 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -5325,6 +5325,29 @@ union bpf_attr { * **-EACCES** if the SYN cookie is not valid. * * **-EPROTONOSUPPORT** if CONFIG_IPV6 is not builtin. + * + * long bpf_verify_pkcs7_signature(struct bpf_dynptr *data_ptr, struct bpf_dynptr *sig_ptr, u32 trusted_keyring_serial, unsigned long lookup_flags, unsigned long trusted_keyring_id) + * Description + * Verify the PKCS#7 signature *sig_ptr* against the supplied + * *data_ptr* with keys in a keyring with serial + * *trusted_keyring_serial*, searched with *lookup_flags*, if the + * parameter value is positive, or alternatively in a keyring with + * special ID *trusted_keyring_id* if *trusted_keyring_serial* is + * zero. + * + * *lookup_flags* are defined in include/linux/key.h and can be: 1, + * to request that special keyrings be created if referred to + * directly; 2 to permit partially constructed keys to be found. + * + * Special IDs are defined in include/linux/verification.h and can + * be: 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. */ #define __BPF_FUNC_MAPPER(FN) \ FN(unspec), \ @@ -5535,6 +5558,7 @@ union bpf_attr { FN(tcp_raw_gen_syncookie_ipv6), \ FN(tcp_raw_check_syncookie_ipv4), \ FN(tcp_raw_check_syncookie_ipv6), \ + FN(verify_pkcs7_signature), \ /* */
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
Ensure that signature verification is performed successfully from an eBPF program, with the new bpf_verify_pkcs7_signature() helper.
Generate a testing signature key and compile sign-file from scripts/, so that the test is selfcontained. Also, search for the tcb_bic.ko kernel module, parse it in user space to extract the raw PKCS#7 signature and send it to the eBPF program for signature verification. If tcb_bic.ko is not found, the test does not fail.
The additional verification of a kernel module is necessary to test lookup of a keyring with a special ID (primary, secondary or platform) since, if kernel modules are signed, the public key is also added at kernel build time to one of those keyrings.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- tools/testing/selftests/bpf/Makefile | 14 +- tools/testing/selftests/bpf/config | 2 + .../bpf/prog_tests/verify_pkcs7_sig.c | 359 ++++++++++++++++++ .../bpf/progs/test_verify_pkcs7_sig.c | 79 ++++ .../testing/selftests/bpf/verify_sig_setup.sh | 104 +++++ 5 files changed, 555 insertions(+), 3 deletions(-) create mode 100644 tools/testing/selftests/bpf/prog_tests/verify_pkcs7_sig.c create mode 100644 tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c create mode 100755 tools/testing/selftests/bpf/verify_sig_setup.sh
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 4fbd88a8ed9e..cad3607e9a6f 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -14,6 +14,7 @@ BPFTOOLDIR := $(TOOLSDIR)/bpf/bpftool APIDIR := $(TOOLSINCDIR)/uapi GENDIR := $(abspath ../../../../include/generated) GENHDR := $(GENDIR)/autoconf.h +HOSTPKG_CONFIG := pkg-config
ifneq ($(wildcard $(GENHDR)),) GENFLAGS := -DHAVE_GENHDR @@ -75,7 +76,7 @@ TEST_PROGS := test_kmod.sh \ test_xsk.sh
TEST_PROGS_EXTENDED := with_addr.sh \ - with_tunnels.sh ima_setup.sh \ + with_tunnels.sh ima_setup.sh verify_sig_setup.sh \ test_xdp_vlan.sh test_bpftool.py
# Compile but not part of 'make run_tests' @@ -84,7 +85,7 @@ TEST_GEN_PROGS_EXTENDED = test_sock_addr test_skb_cgroup_id_user \ test_lirc_mode2_user xdping test_cpp runqslower bench bpf_testmod.ko \ xdpxceiver xdp_redirect_multi xdp_synproxy
-TEST_CUSTOM_PROGS = $(OUTPUT)/urandom_read +TEST_CUSTOM_PROGS = $(OUTPUT)/urandom_read $(OUTPUT)/sign-file
# Emit succinct information message describing current building step # $1 - generic step name (e.g., CC, LINK, etc); @@ -189,6 +190,12 @@ $(OUTPUT)/urandom_read: urandom_read.c urandom_read_aux.c $(OUTPUT)/liburandom_r -fuse-ld=$(LLD) -Wl,-znoseparate-code \ -Wl,-rpath=. -Wl,--build-id=sha1 -o $@
+$(OUTPUT)/sign-file: ../../../../scripts/sign-file.c + $(call msg,SIGN-FILE,,$@) + $(Q)$(CC) $(shell $(HOSTPKG_CONFIG)--cflags libcrypto 2> /dev/null) \ + $< -o $@ \ + $(shell $(HOSTPKG_CONFIG) --libs libcrypto 2> /dev/null || echo -lcrypto) + $(OUTPUT)/bpf_testmod.ko: $(VMLINUX_BTF) $(wildcard bpf_testmod/Makefile bpf_testmod/*.[ch]) $(call msg,MOD,,$@) $(Q)$(RM) bpf_testmod/bpf_testmod.ko # force re-compilation @@ -512,7 +519,8 @@ TRUNNER_EXTRA_SOURCES := test_progs.c cgroup_helpers.c trace_helpers.c \ TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read $(OUTPUT)/bpf_testmod.ko \ $(OUTPUT)/liburandom_read.so \ $(OUTPUT)/xdp_synproxy \ - ima_setup.sh \ + $(OUTPUT)/sign-file \ + ima_setup.sh verify_sig_setup.sh \ $(wildcard progs/btf_dump_test_case_*.c) TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS) -DENABLE_ATOMICS_TESTS diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config index c05904d631ec..76b65acd897e 100644 --- a/tools/testing/selftests/bpf/config +++ b/tools/testing/selftests/bpf/config @@ -63,3 +63,5 @@ CONFIG_NETFILTER_XT_MATCH_STATE=y CONFIG_IP_NF_FILTER=y CONFIG_IP_NF_TARGET_SYNPROXY=y CONFIG_IP_NF_RAW=y +CONFIG_MODULE_SIG=y +CONFIG_KEYS=y diff --git a/tools/testing/selftests/bpf/prog_tests/verify_pkcs7_sig.c b/tools/testing/selftests/bpf/prog_tests/verify_pkcs7_sig.c new file mode 100644 index 000000000000..85552dc48333 --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/verify_pkcs7_sig.c @@ -0,0 +1,359 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* + * Copyright (C) 2022 Huawei Technologies Duesseldorf GmbH + * + * Author: Roberto Sassu roberto.sassu@huawei.com + */ + +#include <stdio.h> +#include <errno.h> +#include <stdlib.h> +#include <unistd.h> +#include <endian.h> +#include <limits.h> +#include <sys/stat.h> +#include <sys/wait.h> +#include <sys/mman.h> +#include <linux/keyctl.h> +#include <test_progs.h> + +#include "test_verify_pkcs7_sig.skel.h" + +#define MAX_DATA_SIZE (1024 * 1024) +#define MAX_SIG_SIZE 1024 +#define LOG_BUF_SIZE 16384 + +/* In stripped ARM and x86-64 modules, ~ is surprisingly rare. */ +#define MODULE_SIG_STRING "~Module signature appended~\n" + +/* + * Module signature information block. + * + * The constituents of the signature section are, in order: + * + * - Signer's name + * - Key identifier + * - Signature data + * - Information block + */ +struct module_signature { + u8 algo; /* Public-key crypto algorithm [0] */ + u8 hash; /* Digest algorithm [0] */ + u8 id_type; /* Key identifier type [PKEY_ID_PKCS7] */ + u8 signer_len; /* Length of signer's name [0] */ + u8 key_id_len; /* Length of key identifier [0] */ + u8 __pad[3]; + __be32 sig_len; /* Length of signature data */ +}; + +struct data { + u8 data[MAX_DATA_SIZE]; + u32 data_len; + u8 sig[MAX_SIG_SIZE]; + u32 sig_len; +}; + +static int _run_setup_process(const char *setup_dir, const char *cmd) +{ + int child_pid, child_status; + + child_pid = fork(); + if (child_pid == 0) { + execlp("./verify_sig_setup.sh", "./verify_sig_setup.sh", cmd, + setup_dir, NULL); + exit(errno); + + } else if (child_pid > 0) { + waitpid(child_pid, &child_status, 0); + return WEXITSTATUS(child_status); + } + + return -EINVAL; +} + +static int populate_data_item_str(const char *tmp_dir, struct data *data_item) +{ + struct stat st; + char data_template[] = "/tmp/dataXXXXXX"; + char path[PATH_MAX]; + int ret, fd, child_status, child_pid; + + data_item->data_len = 4; + memcpy(data_item->data, "test", data_item->data_len); + + fd = mkstemp(data_template); + if (fd == -1) + return -errno; + + ret = write(fd, data_item->data, data_item->data_len); + + close(fd); + + if (ret != data_item->data_len) { + ret = -EIO; + goto out; + } + + child_pid = fork(); + + if (child_pid == -1) { + ret = -errno; + goto out; + } + + if (child_pid == 0) { + snprintf(path, sizeof(path), "%s/signing_key.pem", tmp_dir); + + return execlp("./sign-file", "./sign-file", "-d", "sha256", + path, path, data_template, NULL); + } + + waitpid(child_pid, &child_status, 0); + + ret = WEXITSTATUS(child_status); + if (ret) + goto out; + + snprintf(path, sizeof(path), "%s.p7s", data_template); + + ret = stat(path, &st); + if (ret == -1) { + ret = -errno; + goto out; + } + + if (st.st_size > sizeof(data_item->sig)) { + ret = -EINVAL; + goto out_sig; + } + + data_item->sig_len = st.st_size; + + fd = open(path, O_RDONLY); + if (fd == -1) { + ret = -errno; + goto out_sig; + } + + ret = read(fd, data_item->sig, data_item->sig_len); + + close(fd); + + if (ret != data_item->sig_len) { + ret = -EIO; + goto out_sig; + } + + ret = 0; +out_sig: + unlink(path); +out: + unlink(data_template); + return ret; +} + +static int populate_data_item_mod(struct data *data_item) +{ + char mod_path[PATH_MAX], *mod_path_ptr; + struct stat st; + void *mod; + FILE *fp; + struct module_signature ms; + int ret, fd, modlen, marker_len, sig_len; + + data_item->data_len = 0; + + if (stat("/lib/modules", &st) == -1) + return 0; + + /* Requires CONFIG_TCP_CONG_BIC=m. */ + fp = popen("find /lib/modules/$(uname -r) -name tcp_bic.ko", "r"); + if (!fp) + return 0; + + mod_path_ptr = fgets(mod_path, sizeof(mod_path), fp); + pclose(fp); + + if (!mod_path_ptr) + return 0; + + mod_path_ptr = strchr(mod_path, '\n'); + if (!mod_path_ptr) + return 0; + + *mod_path_ptr = '\0'; + + if (stat(mod_path, &st) == -1) + return 0; + + modlen = st.st_size; + marker_len = sizeof(MODULE_SIG_STRING) - 1; + + fd = open(mod_path, O_RDONLY); + if (fd == -1) + return -errno; + + mod = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0); + + close(fd); + + if (mod == MAP_FAILED) + return -errno; + + if (strncmp(mod + modlen - marker_len, MODULE_SIG_STRING, marker_len)) { + ret = -EINVAL; + goto out; + } + + modlen -= marker_len; + + memcpy(&ms, mod + (modlen - sizeof(ms)), sizeof(ms)); + + sig_len = __be32_to_cpu(ms.sig_len); + modlen -= sig_len + sizeof(ms); + + if (modlen > sizeof(data_item->data)) { + ret = -E2BIG; + goto out; + } + + memcpy(data_item->data, mod, modlen); + data_item->data_len = modlen; + + if (sig_len > sizeof(data_item->sig)) { + ret = -E2BIG; + goto out; + } + + memcpy(data_item->sig, mod + modlen, sig_len); + data_item->sig_len = sig_len; + ret = 0; +out: + munmap(mod, st.st_size); + return ret; +} + +void test_verify_pkcs7_sig(void) +{ + char tmp_dir_template[] = "/tmp/verify_sigXXXXXX"; + char *tmp_dir; + char *buf = NULL; + struct test_verify_pkcs7_sig *skel = NULL; + struct bpf_map *map; + struct data data; + int ret, zero = 0; + + LIBBPF_OPTS(bpf_object_open_opts, opts); + + /* Trigger creation of session keyring. */ + syscall(__NR_request_key, "keyring", "_uid.0", NULL, + KEY_SPEC_SESSION_KEYRING); + + tmp_dir = mkdtemp(tmp_dir_template); + if (!ASSERT_OK_PTR(tmp_dir, "mkdtemp")) + return; + + ret = _run_setup_process(tmp_dir, "setup"); + if (!ASSERT_OK(ret, "_run_setup_process")) + goto close_prog; + + buf = malloc(LOG_BUF_SIZE); + if (!ASSERT_OK_PTR(buf, "malloc")) + goto close_prog; + + opts.kernel_log_buf = buf; + opts.kernel_log_size = LOG_BUF_SIZE; + opts.kernel_log_level = 1; + + skel = test_verify_pkcs7_sig__open_opts(&opts); + if (!ASSERT_OK_PTR(skel, "test_verify_pkcs7_sig__open_opts")) + goto close_prog; + + ret = test_verify_pkcs7_sig__load(skel); + + if (ret < 0 && strstr(buf, "unknown func bpf_verify_pkcs7_signature")) { + printf( + "%s:SKIP:bpf_verify_pkcs7_signature() helper not supported\n", + __func__); + test__skip(); + goto close_prog; + } + + if (!ASSERT_OK(ret, "test_verify_pkcs7_sig__load")) + goto close_prog; + + ret = test_verify_pkcs7_sig__attach(skel); + if (!ASSERT_OK(ret, "test_verify_pkcs7_sig__attach")) + goto close_prog; + + map = bpf_object__find_map_by_name(skel->obj, "data_input"); + if (!ASSERT_OK_PTR(map, "data_input not found")) + goto close_prog; + + ret = populate_data_item_str(tmp_dir, &data); + if (!ASSERT_OK(ret, "populate_data_item_str")) + goto close_prog; + + skel->bss->monitored_pid = getpid(); + skel->bss->trusted_keyring_serial = KEY_SPEC_SESSION_KEYRING; + + ret = bpf_map_update_elem(bpf_map__fd(map), &zero, &data, BPF_ANY); + if (!ASSERT_OK(ret, "bpf_map_update_elem")) + goto close_prog; + + /* Search the verification key in the testing keyring. */ + skel->bss->trusted_keyring_serial = syscall(__NR_request_key, + "keyring", + "ebpf_testing_keyring", + NULL, + KEY_SPEC_SESSION_KEYRING); + + ret = bpf_map_update_elem(bpf_map__fd(map), &zero, &data, BPF_ANY); + if (!ASSERT_OK(ret, "bpf_map_update_elem")) + goto close_prog; + + /* Corrupt data (signature verification should fail). */ + data.data[0] = 'a'; + ret = bpf_map_update_elem(bpf_map__fd(map), &zero, &data, BPF_ANY); + if (!ASSERT_LT(ret, 0, "bpf_map_update_elem data_input")) + goto close_prog; + + ret = populate_data_item_mod(&data); + if (!ASSERT_OK(ret, "populate_data_item_mod")) + goto close_prog; + + if (data.data_len) { + skel->bss->trusted_keyring_serial = 0; + skel->bss->trusted_keyring_id = 0; + + ret = bpf_map_update_elem(bpf_map__fd(map), &zero, &data, + BPF_ANY); + if (!ASSERT_OK(ret, "bpf_map_update_elem")) + goto close_prog; + + skel->bss->trusted_keyring_id = 1; + + ret = bpf_map_update_elem(bpf_map__fd(map), &zero, &data, + BPF_ANY); + if (!ASSERT_OK(ret, "bpf_map_update_elem")) + goto close_prog; + + /* Verification key is not in the platform keyring. */ + skel->bss->trusted_keyring_id = 2; + + ret = bpf_map_update_elem(bpf_map__fd(map), &zero, &data, + BPF_ANY); + ASSERT_LT(ret, 0, "bpf_map_update_elem"); + } + +close_prog: + _run_setup_process(tmp_dir, "cleanup"); + free(buf); + + if (!skel) + return; + + skel->bss->monitored_pid = 0; + test_verify_pkcs7_sig__destroy(skel); +} diff --git a/tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c b/tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c new file mode 100644 index 000000000000..e9937f1ec541 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c @@ -0,0 +1,79 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* + * Copyright (C) 2022 Huawei Technologies Duesseldorf GmbH + * + * Author: Roberto Sassu roberto.sassu@huawei.com + */ + +#include <errno.h> +#include <stdlib.h> +#include <limits.h> +#include <linux/bpf.h> +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_tracing.h> + +#define MAX_DATA_SIZE (1024 * 1024) +#define MAX_SIG_SIZE 1024 + +typedef __u8 u8; +typedef __u16 u16; +typedef __u32 u32; +typedef __u64 u64; + +u32 monitored_pid; +u32 trusted_keyring_serial; +unsigned long trusted_keyring_id; + +struct data { + u8 data[MAX_DATA_SIZE]; + u32 data_len; + u8 sig[MAX_SIG_SIZE]; + u32 sig_len; +}; + +struct { + __uint(type, BPF_MAP_TYPE_ARRAY); + __uint(max_entries, 1); + __type(key, __u32); + __type(value, struct data); +} data_input SEC(".maps"); + +char _license[] SEC("license") = "GPL"; + +SEC("lsm.s/bpf") +int BPF_PROG(bpf, int cmd, union bpf_attr *attr, unsigned int size) +{ + struct bpf_dynptr data_ptr, sig_ptr; + struct data *data_val; + u32 pid; + u64 value; + int ret, zero = 0; + + pid = bpf_get_current_pid_tgid() >> 32; + if (pid != monitored_pid) + return 0; + + data_val = bpf_map_lookup_elem(&data_input, &zero); + if (!data_val) + return 0; + + bpf_probe_read(&value, sizeof(value), &attr->value); + + bpf_copy_from_user(data_val, sizeof(struct data), + (void *)(unsigned long)value); + + if (data_val->data_len > sizeof(data_val->data)) + return -EINVAL; + + bpf_dynptr_from_mem(data_val->data, data_val->data_len, 0, &data_ptr); + + if (data_val->sig_len > sizeof(data_val->sig)) + return -EINVAL; + + bpf_dynptr_from_mem(data_val->sig, data_val->sig_len, 0, &sig_ptr); + + return bpf_verify_pkcs7_signature(&data_ptr, &sig_ptr, + trusted_keyring_serial, 0, + trusted_keyring_id); +} diff --git a/tools/testing/selftests/bpf/verify_sig_setup.sh b/tools/testing/selftests/bpf/verify_sig_setup.sh new file mode 100755 index 000000000000..ba08922b4a27 --- /dev/null +++ b/tools/testing/selftests/bpf/verify_sig_setup.sh @@ -0,0 +1,104 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 + +set -e +set -u +set -o pipefail + +VERBOSE="${SELFTESTS_VERBOSE:=0}" +LOG_FILE="$(mktemp /tmp/verify_sig_setup.log.XXXXXX)" + +x509_genkey_content="\ +[ req ] +default_bits = 2048 +distinguished_name = req_distinguished_name +prompt = no +string_mask = utf8only +x509_extensions = myexts + +[ req_distinguished_name ] +CN = eBPF Signature Verification Testing Key + +[ myexts ] +basicConstraints=critical,CA:FALSE +keyUsage=digitalSignature +subjectKeyIdentifier=hash +authorityKeyIdentifier=keyid +" + +usage() +{ + echo "Usage: $0 <setup|cleanup <existing_tmp_dir>" + exit 1 +} + +setup() +{ + local tmp_dir="$1" + + echo "${x509_genkey_content}" > ${tmp_dir}/x509.genkey + + openssl req -new -nodes -utf8 -sha256 -days 36500 \ + -batch -x509 -config ${tmp_dir}/x509.genkey \ + -outform PEM -out ${tmp_dir}/signing_key.pem \ + -keyout ${tmp_dir}/signing_key.pem 2>&1 + + openssl x509 -in ${tmp_dir}/signing_key.pem -out \ + ${tmp_dir}/signing_key.der -outform der + + key_id=$(cat ${tmp_dir}/signing_key.der | keyctl padd asymmetric ebpf_testing_key @s) + + keyring_id=$(keyctl newring ebpf_testing_keyring @s) + keyctl link $key_id $keyring_id +} + +cleanup() { + local tmp_dir="$1" + + keyctl unlink $(keyctl search @s asymmetric ebpf_testing_key) @s + keyctl unlink $(keyctl search @s keyring ebpf_testing_keyring) @s + rm -rf ${tmp_dir} +} + +catch() +{ + local exit_code="$1" + local log_file="$2" + + if [[ "${exit_code}" -ne 0 ]]; then + cat "${log_file}" >&3 + fi + + rm -f "${log_file}" + exit ${exit_code} +} + +main() +{ + [[ $# -ne 2 ]] && usage + + local action="$1" + local tmp_dir="$2" + + [[ ! -d "${tmp_dir}" ]] && echo "Directory ${tmp_dir} doesn't exist" && exit 1 + + if [[ "${action}" == "setup" ]]; then + setup "${tmp_dir}" + elif [[ "${action}" == "cleanup" ]]; then + cleanup "${tmp_dir}" + else + echo "Unknown action: ${action}" + exit 1 + fi +} + +trap 'catch "$?" "${LOG_FILE}"' EXIT + +if [[ "${VERBOSE}" -eq 0 ]]; then + # Save the stderr to 3 so that we can output back to + # it incase of an error. + exec 3>&2 1>"${LOG_FILE}" 2>&1 +fi + +main "$@" +rm -f "${LOG_FILE}"
From: Roberto Sassu Sent: Tuesday, June 28, 2022 2:28 PM Ensure that signature verification is performed successfully from an eBPF program, with the new bpf_verify_pkcs7_signature() helper.
Generate a testing signature key and compile sign-file from scripts/, so that the test is selfcontained. Also, search for the tcb_bic.ko kernel module, parse it in user space to extract the raw PKCS#7 signature and send it to the eBPF program for signature verification. If tcb_bic.ko is not found, the test does not fail.
Ops, tcp_bic.ko.
Roberto
On 6/28/22 2:27 PM, 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 trusted keyring serial, together with key lookup-specific flags, to determine which keys can be used for signature verification. Alternatively, the caller could specify zero as serial value (not valid, serials must be positive), and provide instead a special keyring ID.
Key lookup flags are defined in include/linux/key.h and can be: 1, to request that special keyrings be created if referred to directly; 2 to permit partially constructed keys to be found.
Special IDs are defined in include/linux/verification.h and can be: 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).
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com Reported-by: kernel test robot lkp@intel.com (cast warning)
nit: Given this a new feature not a fix to existing code, there is no need to add the above reported-by from kbuild bot.
include/uapi/linux/bpf.h | 24 +++++++++++++ kernel/bpf/bpf_lsm.c | 63 ++++++++++++++++++++++++++++++++++ tools/include/uapi/linux/bpf.h | 24 +++++++++++++ 3 files changed, 111 insertions(+)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index e81362891596..b4f5ad863281 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -5325,6 +5325,29 @@ union bpf_attr {
**-EACCES** if the SYN cookie is not valid.**-EPROTONOSUPPORT** if CONFIG_IPV6 is not builtin.
- long bpf_verify_pkcs7_signature(struct bpf_dynptr *data_ptr, struct bpf_dynptr *sig_ptr, u32 trusted_keyring_serial, unsigned long lookup_flags, unsigned long trusted_keyring_id)
nit: for the args instead of ulong, just do u64
- Description
Verify the PKCS#7 signature *sig_ptr* against the supplied
*data_ptr* with keys in a keyring with serial
*trusted_keyring_serial*, searched with *lookup_flags*, if the
parameter value is positive, or alternatively in a keyring with
special ID *trusted_keyring_id* if *trusted_keyring_serial* is
zero.
*lookup_flags* are defined in include/linux/key.h and can be: 1,
to request that special keyrings be created if referred to
directly; 2 to permit partially constructed keys to be found.
Special IDs are defined in include/linux/verification.h and can
be: 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
*/ #define __BPF_FUNC_MAPPER(FN) \ FN(unspec), \
0 on success, a negative value on error.@@ -5535,6 +5558,7 @@ union bpf_attr { FN(tcp_raw_gen_syncookie_ipv6), \ FN(tcp_raw_check_syncookie_ipv4), \ FN(tcp_raw_check_syncookie_ipv6), \
- FN(verify_pkcs7_signature), \
(Needs rebase)
/* */ /* 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..401bda01ad84 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/key.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,62 @@ 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_pkcs7_signature, struct bpf_dynptr_kern *, data_ptr,
struct bpf_dynptr_kern *, sig_ptr, u32, trusted_keyring_serial,unsigned long, lookup_flags, unsigned long, trusted_keyring_id)+{
- key_ref_t trusted_keyring_ref;
- struct key *trusted_keyring;
- int ret;
- /* Keep in sync with defs in include/linux/key.h. */
- if (lookup_flags > KEY_LOOKUP_PARTIAL)
return -EINVAL;
iiuc, the KEY_LOOKUP_* is a mask, so you could also combine the two, e.g. KEY_LOOKUP_CREATE | KEY_LOOKUP_PARTIAL. I haven't seen you mentioning anything specific on why it is not allowed. What's the rationale, if it's intentional if should probably be documented?
At minimum I also think the helper description needs to be improved for people to understand enough w/o reading through the kernel source, e.g. wrt lookup_flags since I haven't seen it in your selftests either ... when does a user need to use the given flags.
nit: when both trusted_keyring_serial and trusted_keyring_id are passed to the helper, then this should be rejected as invalid argument? (Kind of feels a bit like we're cramming two things in one helper.. KP, thoughts? :))
- /* Keep in sync with defs in include/linux/verification.h. */
- if (trusted_keyring_id > (unsigned long)VERIFY_USE_PLATFORM_KEYRING)
return -EINVAL;- if (trusted_keyring_serial) {
trusted_keyring_ref = lookup_user_key(trusted_keyring_serial,lookup_flags,KEY_NEED_SEARCH);if (IS_ERR(trusted_keyring_ref))return PTR_ERR(trusted_keyring_ref);trusted_keyring = key_ref_to_ptr(trusted_keyring_ref);goto verify;- }
- trusted_keyring = (struct key *)trusted_keyring_id;
+verify:
- ret = verify_pkcs7_signature(data_ptr->data,
bpf_dynptr_get_size(data_ptr),sig_ptr->data,bpf_dynptr_get_size(sig_ptr),trusted_keyring,VERIFYING_UNSPECIFIED_SIGNATURE, NULL,NULL);- if (trusted_keyring_serial)
key_put(trusted_keyring);- return ret;
+}
+static const struct bpf_func_proto bpf_verify_pkcs7_signature_proto = {
- .func = bpf_verify_pkcs7_signature,
- .gpl_only = false,
- .ret_type = RET_INTEGER,
- .arg1_type = ARG_PTR_TO_DYNPTR | DYNPTR_TYPE_LOCAL,
- .arg2_type = ARG_PTR_TO_DYNPTR | DYNPTR_TYPE_LOCAL,
- .arg3_type = ARG_ANYTHING,
- .arg4_type = ARG_ANYTHING,
- .arg5_type = ARG_ANYTHING,
- .allowed = bpf_ima_inode_hash_allowed,
+}; +#endif /* CONFIG_SYSTEM_DATA_VERIFICATION */
On Wed, Jul 6, 2022 at 6:04 PM Daniel Borkmann daniel@iogearbox.net wrote:
On 6/28/22 2:27 PM, 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 trusted keyring serial, together with key lookup-specific flags, to determine which keys can be used for signature verification. Alternatively, the caller could specify zero as serial value (not valid, serials must be positive), and provide instead a special keyring ID.
Key lookup flags are defined in include/linux/key.h and can be: 1, to request that special keyrings be created if referred to directly; 2 to permit partially constructed keys to be found.
Special IDs are defined in include/linux/verification.h and can be: 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).
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com Reported-by: kernel test robot lkp@intel.com (cast warning)
nit: Given this a new feature not a fix to existing code, there is no need to add the above reported-by from kbuild bot.
include/uapi/linux/bpf.h | 24 +++++++++++++ kernel/bpf/bpf_lsm.c | 63 ++++++++++++++++++++++++++++++++++ tools/include/uapi/linux/bpf.h | 24 +++++++++++++ 3 files changed, 111 insertions(+)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index e81362891596..b4f5ad863281 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -5325,6 +5325,29 @@ union bpf_attr {
**-EACCES** if the SYN cookie is not valid.**-EPROTONOSUPPORT** if CONFIG_IPV6 is not builtin.
- long bpf_verify_pkcs7_signature(struct bpf_dynptr *data_ptr, struct bpf_dynptr *sig_ptr, u32 trusted_keyring_serial, unsigned long lookup_flags, unsigned long trusted_keyring_id)
nit: for the args instead of ulong, just do u64
- Description
Verify the PKCS#7 signature *sig_ptr* against the supplied
*data_ptr* with keys in a keyring with serial
*trusted_keyring_serial*, searched with *lookup_flags*, if the
parameter value is positive, or alternatively in a keyring with
special ID *trusted_keyring_id* if *trusted_keyring_serial* is
zero.
*lookup_flags* are defined in include/linux/key.h and can be: 1,
to request that special keyrings be created if referred to
directly; 2 to permit partially constructed keys to be found.
Special IDs are defined in include/linux/verification.h and can
be: 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
*/ #define __BPF_FUNC_MAPPER(FN) \ FN(unspec), \
0 on success, a negative value on error.@@ -5535,6 +5558,7 @@ union bpf_attr { FN(tcp_raw_gen_syncookie_ipv6), \ FN(tcp_raw_check_syncookie_ipv4), \ FN(tcp_raw_check_syncookie_ipv6), \
FN(verify_pkcs7_signature), \(Needs rebase)
/* *//* 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..401bda01ad84 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/key.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,62 @@ 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_pkcs7_signature, struct bpf_dynptr_kern *, data_ptr,
struct bpf_dynptr_kern *, sig_ptr, u32, trusted_keyring_serial,unsigned long, lookup_flags, unsigned long, trusted_keyring_id)+{
key_ref_t trusted_keyring_ref;struct key *trusted_keyring;int ret;/* Keep in sync with defs in include/linux/key.h. */if (lookup_flags > KEY_LOOKUP_PARTIAL)return -EINVAL;iiuc, the KEY_LOOKUP_* is a mask, so you could also combine the two, e.g. KEY_LOOKUP_CREATE | KEY_LOOKUP_PARTIAL. I haven't seen you mentioning anything specific on why it is not allowed. What's the rationale, if it's intentional if should probably be documented?
I think this was a part of the digilim threat model (only allow limited lookup operations), but this seems to be conflating the policy into the implementation of the helper.
Roberto, can this not be implemented in digilim as a BPF LSM check that attaches to the key_permission LSM hook?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/incl...
At minimum I also think the helper description needs to be improved for people to understand enough w/o reading through the kernel source, e.g. wrt lookup_flags since I haven't seen it in your selftests either ... when does a user need to use the given flags.
nit: when both trusted_keyring_serial and trusted_keyring_id are passed to the helper, then this should be rejected as invalid argument? (Kind of feels a bit like we're cramming two things in one helper.. KP, thoughts? :))
EINVAL when both are passed seems reasonable. The signature (pun?) of the does seem to get bloated, but I am not sure if it's worth adding two helpers here.
/* Keep in sync with defs in include/linux/verification.h. */if (trusted_keyring_id > (unsigned long)VERIFY_USE_PLATFORM_KEYRING)return -EINVAL;if (trusted_keyring_serial) {trusted_keyring_ref = lookup_user_key(trusted_keyring_serial,lookup_flags,KEY_NEED_SEARCH);if (IS_ERR(trusted_keyring_ref))return PTR_ERR(trusted_keyring_ref);trusted_keyring = key_ref_to_ptr(trusted_keyring_ref);goto verify;}trusted_keyring = (struct key *)trusted_keyring_id;+verify:
ret = verify_pkcs7_signature(data_ptr->data,bpf_dynptr_get_size(data_ptr),sig_ptr->data,bpf_dynptr_get_size(sig_ptr),trusted_keyring,VERIFYING_UNSPECIFIED_SIGNATURE, NULL,NULL);if (trusted_keyring_serial)key_put(trusted_keyring);return ret;+}
+static const struct bpf_func_proto bpf_verify_pkcs7_signature_proto = {
.func = bpf_verify_pkcs7_signature,.gpl_only = false,.ret_type = RET_INTEGER,.arg1_type = ARG_PTR_TO_DYNPTR | DYNPTR_TYPE_LOCAL,.arg2_type = ARG_PTR_TO_DYNPTR | DYNPTR_TYPE_LOCAL,.arg3_type = ARG_ANYTHING,.arg4_type = ARG_ANYTHING,.arg5_type = ARG_ANYTHING,.allowed = bpf_ima_inode_hash_allowed,+}; +#endif /* CONFIG_SYSTEM_DATA_VERIFICATION */
From: KP Singh [mailto:kpsingh@kernel.org] Sent: Thursday, July 7, 2022 1:49 AM On Wed, Jul 6, 2022 at 6:04 PM Daniel Borkmann daniel@iogearbox.net wrote:
On 6/28/22 2:27 PM, 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 trusted keyring serial, together with key lookup-specific flags, to determine which keys can be used for signature verification. Alternatively, the caller could specify zero as serial value (not valid, serials must be positive), and provide instead a special keyring ID.
Key lookup flags are defined in include/linux/key.h and can be: 1, to request that special keyrings be created if referred to directly; 2 to permit partially constructed keys to be found.
Special IDs are defined in include/linux/verification.h and can be: 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).
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com Reported-by: kernel test robot lkp@intel.com (cast warning)
nit: Given this a new feature not a fix to existing code, there is no need to add the above reported-by from kbuild bot.
Ok.
include/uapi/linux/bpf.h | 24 +++++++++++++ kernel/bpf/bpf_lsm.c | 63 ++++++++++++++++++++++++++++++++++ tools/include/uapi/linux/bpf.h | 24 +++++++++++++ 3 files changed, 111 insertions(+)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index e81362891596..b4f5ad863281 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -5325,6 +5325,29 @@ union bpf_attr {
**-EACCES** if the SYN cookie is not valid.**-EPROTONOSUPPORT** if CONFIG_IPV6 is not builtin.
- long bpf_verify_pkcs7_signature(struct bpf_dynptr *data_ptr, struct
bpf_dynptr *sig_ptr, u32 trusted_keyring_serial, unsigned long lookup_flags, unsigned long trusted_keyring_id)
nit: for the args instead of ulong, just do u64
Ok.
- Description
Verify the PKCS#7 signature *sig_ptr* against the supplied
*data_ptr* with keys in a keyring with serial
*trusted_keyring_serial*, searched with *lookup_flags*, if the
parameter value is positive, or alternatively in a keyring with
special ID *trusted_keyring_id* if *trusted_keyring_serial* is
zero.
*lookup_flags* are defined in include/linux/key.h and can be: 1,
to request that special keyrings be created if referred to
directly; 2 to permit partially constructed keys to be found.
Special IDs are defined in include/linux/verification.h and can
be: 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
*/ #define __BPF_FUNC_MAPPER(FN) \ FN(unspec), \
0 on success, a negative value on error.@@ -5535,6 +5558,7 @@ union bpf_attr { FN(tcp_raw_gen_syncookie_ipv6), \ FN(tcp_raw_check_syncookie_ipv4), \ FN(tcp_raw_check_syncookie_ipv6), \
FN(verify_pkcs7_signature), \(Needs rebase)
/* *//* 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..401bda01ad84 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/key.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,62 @@ 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_pkcs7_signature, struct bpf_dynptr_kern *,
data_ptr,
struct bpf_dynptr_kern *, sig_ptr, u32, trusted_keyring_serial,unsigned long, lookup_flags, unsigned long, trusted_keyring_id)+{
key_ref_t trusted_keyring_ref;struct key *trusted_keyring;int ret;/* Keep in sync with defs in include/linux/key.h. */if (lookup_flags > KEY_LOOKUP_PARTIAL)return -EINVAL;iiuc, the KEY_LOOKUP_* is a mask, so you could also combine the two, e.g. KEY_LOOKUP_CREATE | KEY_LOOKUP_PARTIAL. I haven't seen you
mentioning anything
specific on why it is not allowed. What's the rationale, if it's intentional if should probably be documented?
It was a mistake. Will fix it.
I think this was a part of the digilim threat model (only allow limited lookup operations), but this seems to be conflating the policy into the implementation of the helper.
Uhm, yes, but these flags should not affect that requirement.
As long as I can select the primary or secondary keyring reliably with the pre-determined IDs, that should be fine.
Roberto, can this not be implemented in digilim as a BPF LSM check that attaches to the key_permission LSM hook?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/incl... nux/lsm_hooks.h#n1158
The pre-determined IDs handled by verify_pkcs7_signature() are more effective in selecting the proper key.
At minimum I also think the helper description needs to be improved for
people
to understand enough w/o reading through the kernel source, e.g. wrt
lookup_flags
since I haven't seen it in your selftests either ... when does a user need to use the given flags.
nit: when both trusted_keyring_serial and trusted_keyring_id are passed to the helper, then this should be rejected as invalid argument? (Kind of feels a bit like we're cramming two things in one helper.. KP, thoughts? :))
EINVAL when both are passed seems reasonable. The signature (pun?) of the does seem to get bloated, but I am not sure if it's worth adding two helpers here.
Ok for EINVAL. Should I change the trusted_keyring_id type to signed, and use -1 when it should not be specified?
Thanks
Roberto
/* Keep in sync with defs in include/linux/verification.h. */if (trusted_keyring_id > (unsignedlong)VERIFY_USE_PLATFORM_KEYRING)
return -EINVAL;if (trusted_keyring_serial) {trusted_keyring_ref = lookup_user_key(trusted_keyring_serial,lookup_flags,KEY_NEED_SEARCH);if (IS_ERR(trusted_keyring_ref))return PTR_ERR(trusted_keyring_ref);trusted_keyring = key_ref_to_ptr(trusted_keyring_ref);goto verify;}trusted_keyring = (struct key *)trusted_keyring_id;+verify:
ret = verify_pkcs7_signature(data_ptr->data,bpf_dynptr_get_size(data_ptr),sig_ptr->data,bpf_dynptr_get_size(sig_ptr),trusted_keyring,VERIFYING_UNSPECIFIED_SIGNATURE, NULL,NULL);if (trusted_keyring_serial)key_put(trusted_keyring);return ret;+}
+static const struct bpf_func_proto bpf_verify_pkcs7_signature_proto = {
.func = bpf_verify_pkcs7_signature,.gpl_only = false,.ret_type = RET_INTEGER,.arg1_type = ARG_PTR_TO_DYNPTR | DYNPTR_TYPE_LOCAL,.arg2_type = ARG_PTR_TO_DYNPTR | DYNPTR_TYPE_LOCAL,.arg3_type = ARG_ANYTHING,.arg4_type = ARG_ANYTHING,.arg5_type = ARG_ANYTHING,.allowed = bpf_ima_inode_hash_allowed,+}; +#endif /* CONFIG_SYSTEM_DATA_VERIFICATION */
From: Roberto Sassu [mailto:roberto.sassu@huawei.com] Sent: Thursday, July 7, 2022 1:01 PM
From: KP Singh [mailto:kpsingh@kernel.org] Sent: Thursday, July 7, 2022 1:49 AM On Wed, Jul 6, 2022 at 6:04 PM Daniel Borkmann daniel@iogearbox.net wrote:
[...]
nit: when both trusted_keyring_serial and trusted_keyring_id are passed to
the
helper, then this should be rejected as invalid argument? (Kind of feels a bit like we're cramming two things in one helper.. KP, thoughts? :))
EINVAL when both are passed seems reasonable. The signature (pun?) of the does seem to get bloated, but I am not sure if it's worth adding two helpers here.
Ok for EINVAL. Should I change the trusted_keyring_id type to signed, and use -1 when it should not be specified?
I still have the impression that a helper for lookup_user_key() is the preferred solution, despite the access control concern.
David, may ask if this is the correct way to use the key subsystem API when permission check is deferred? key_permission() is currently not defined outside the key subsystem.
The first three functions will be exposed by the kernel to eBPF programs and can be called at any time. bpf_<key_helper> is a generic helper dealing with a key.
BPF_CALL_2(bpf_lookup_user_key, u32, serial, unsigned long, flags) { ... key_ref = lookup_user_key(serial, flags, KEY_DEFER_PERM_CHECK); ... return (unsigned long)key_ref_to_ptr(key_ref); }
BPF_CALL_X(bpf_<key_helper>, struct key, *key, ...) { ret = key_read_state(key); ... ret = key_validate(key); ... ret = key_permission(key_ref, <key helper-specific permission>); ... }
BPF_CALL_1(bpf_key_put, struct key *, key) { key_put(key); return 0; }
An eBPF program would do for example:
SEC("lsm.s/bpf") int BPF_PROG(bpf, int cmd, union bpf_attr *attr, unsigned int size) { struct key *key; ... key = bpf_lookup_user_key(serial, flags); ... ret = bpf_key_helper(key, ...); ... key_put(key); }
Thanks
Roberto
linux-kselftest-mirror@lists.linaro.org
-
Daniel Borkmann -
KP Singh -
Roberto Sassu