[PATCH v3 0/2] bpf: Add bpf_verify_signature() helper

[PATCH v3 0/2] bpf: Add bpf_verify_signature() helper

[Roberto Sassu]

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_signature(). Its job is simply to call the signature verification
function corresponding to the passed signature type, with the keyring selected
through the passed keyring identifier.

Since verify_pkcs7_signature() is doing crypto operations, it must be
called by a sleepable program. This restricts the set of functions that can
call the associated helper (for example, lsm.s/bpf is suitable,
fexit/array_map_update_elem is not).

The added test checks the ability of an eBPF program to verify module-style
appended signatures, as produced by the kernel tool sign-file, currently
used to sign kernel modules.

The patch set is organized as follows.

Patch 1 introduces the new helper. Patch 2 adds the test for the new
helper.

Changelog

v1:
 - Don't define new map flag but introduce simple wrapper of
   verify_pkcs7_signature() (suggested by Alexei and KP)

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)

Roberto Sassu (2):
  bpf: Add bpf_verify_signature() helper
  selftests/bpf: Add test for bpf_verify_signature() helper

 include/uapi/linux/bpf.h                      |  17 ++
 kernel/bpf/bpf_lsm.c                          |  46 ++++
 tools/include/uapi/linux/bpf.h                |  17 ++
 tools/testing/selftests/bpf/Makefile          |  11 +-
 tools/testing/selftests/bpf/config            |   1 +
 .../selftests/bpf/prog_tests/verify_sig.c     | 200 ++++++++++++++++++
 .../selftests/bpf/progs/test_verify_sig.c     | 160 ++++++++++++++
 .../testing/selftests/bpf/verify_sig_setup.sh | 100 +++++++++
 8 files changed, 549 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/verify_sig.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_verify_sig.c
 create mode 100755 tools/testing/selftests/bpf/verify_sig_setup.sh

-- 
2.25.1

[PATCH v3 1/2] bpf: Add bpf_verify_signature() helper

[Roberto Sassu]

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@huawei.com>
Reported-by: kernel test robot <lkp@intel.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;
+	}
+}
+
+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,
+	.arg3_type	= ARG_PTR_TO_MEM,
+	.arg4_type	= ARG_CONST_SIZE_OR_ZERO,
+	.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);
 	}
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index f4009dbdf62d..97521857e44a 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/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
-- 
2.25.1

[PATCH v3 2/2] selftests/bpf: Add test for bpf_verify_signature() helper

[Roberto Sassu]

Ensure that signature verification is performed successfully from an eBPF
program, with the new bpf_verify_signature() helper.

Generate a testing signature key and copy sign-file from scripts/ to the
eBPF selftests directory (if exists), so that the test is selfcontained.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 tools/testing/selftests/bpf/Makefile          |  11 +-
 tools/testing/selftests/bpf/config            |   1 +
 .../selftests/bpf/prog_tests/verify_sig.c     | 200 ++++++++++++++++++
 .../selftests/bpf/progs/test_verify_sig.c     | 160 ++++++++++++++
 .../testing/selftests/bpf/verify_sig_setup.sh | 100 +++++++++
 5 files changed, 469 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/verify_sig.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_verify_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 2d3c8c8f558a..238911b4e4b4 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -75,7 +75,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 +84,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
 
-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);
@@ -180,6 +180,10 @@ $(OUTPUT)/urandom_read: urandom_read.c urandom_read_aux.c $(OUTPUT)/liburandom_r
 		  liburandom_read.so $(LDLIBS)	       			       \
 		  -Wl,-rpath=. -Wl,--build-id=sha1 -o $@
 
+$(OUTPUT)/sign-file: ../../../../scripts/sign-file
+	$(call msg,SIGN-FILE,,$@)
+	$(Q)$(shell [ -f $< ] && cp -a $< $@)
+
 $(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
@@ -502,7 +506,8 @@ TRUNNER_EXTRA_SOURCES := test_progs.c cgroup_helpers.c trace_helpers.c	\
 			 cap_helpers.c
 TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read $(OUTPUT)/bpf_testmod.ko	\
 		       $(OUTPUT)/liburandom_read.so			\
-		       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 3b3edc0fc8a6..64b0957d7742 100644
--- a/tools/testing/selftests/bpf/config
+++ b/tools/testing/selftests/bpf/config
@@ -57,3 +57,4 @@ CONFIG_FPROBE=y
 CONFIG_IKCONFIG=y
 CONFIG_IKCONFIG_PROC=y
 CONFIG_MPTCP=y
+CONFIG_MODULE_SIG=y
diff --git a/tools/testing/selftests/bpf/prog_tests/verify_sig.c b/tools/testing/selftests/bpf/prog_tests/verify_sig.c
new file mode 100644
index 000000000000..165ab1a8ad98
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/verify_sig.c
@@ -0,0 +1,200 @@
+// 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 <sys/stat.h>
+#include <sys/wait.h>
+#include <test_progs.h>
+
+#include "test_verify_sig.skel.h"
+
+#define MAX_DATA_SIZE 4096
+
+struct data {
+	u8 payload[MAX_DATA_SIZE];
+};
+
+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(const char *tmp_dir, struct data *data_item)
+{
+	struct stat st;
+	char signed_file_template[] = "/tmp/signed_fileXXXXXX";
+	char path[PATH_MAX];
+	int ret, fd, child_status, child_pid;
+
+	fd = mkstemp(signed_file_template);
+	if (fd == -1)
+		return -errno;
+
+	ret = write(fd, "test", 4);
+
+	close(fd);
+
+	if (ret != 4) {
+		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", "sha256",
+			      path, path, signed_file_template, NULL);
+	}
+
+	waitpid(child_pid, &child_status, 0);
+
+	ret = WEXITSTATUS(child_status);
+	if (ret)
+		goto out;
+
+	ret = stat(signed_file_template, &st);
+	if (ret == -1) {
+		ret = -errno;
+		goto out;
+	}
+
+	if (st.st_size > sizeof(data_item->payload) - sizeof(u32)) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	*(u32 *)data_item->payload = __cpu_to_be32(st.st_size);
+
+	fd = open(signed_file_template, O_RDONLY);
+	if (fd == -1) {
+		ret = -errno;
+		goto out;
+	}
+
+	ret = read(fd, data_item->payload + sizeof(u32), st.st_size);
+
+	close(fd);
+
+	if (ret != st.st_size) {
+		ret = -EIO;
+		goto out;
+	}
+
+	ret = 0;
+out:
+	unlink(signed_file_template);
+	return ret;
+}
+
+void test_verify_sig(void)
+{
+	char tmp_dir_template[] = "/tmp/verify_sigXXXXXX";
+	char *tmp_dir;
+	struct test_verify_sig *skel = NULL;
+	struct bpf_map *map;
+	struct data data;
+	struct stat st;
+	u32 saved_len;
+	int ret, zero = 0;
+
+	if (libbpf_probe_bpf_helper(BPF_PROG_TYPE_KPROBE,
+			BPF_FUNC_verify_signature, NULL) == -EOPNOTSUPP) {
+		printf("%s:SKIP:bpf_verify_signature() helper not supported\n",
+		       __func__);
+		test__skip();
+		return;
+	}
+
+	if (stat("./sign-file", &st) == -1) {
+		printf("%s:SKIP:kernel modules are not signed\n", __func__);
+		test__skip();
+		return;
+	}
+
+	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;
+
+	skel = test_verify_sig__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "test_verify_sig__open_and_load"))
+		goto close_prog;
+
+	ret = test_verify_sig__attach(skel);
+	if (!ASSERT_OK(ret, "test_verify_sig__attach\n"))
+		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(tmp_dir, &data);
+	if (!ASSERT_OK(ret, "populate_data_item\n"))
+		goto close_prog;
+
+	skel->bss->monitored_pid = getpid();
+	skel->bss->keyring_id = 0xffff;
+
+	ret = bpf_map_update_elem(bpf_map__fd(map), &zero, &data, BPF_ANY);
+	if (!ASSERT_OK(ret, "bpf_map_update_elem\n"))
+		goto close_prog;
+
+	skel->bss->monitored_pid = getpid();
+	/* Search the verification key in the primary keyring (should fail). */
+	skel->bss->keyring_id = 0;
+
+	ret = bpf_map_update_elem(bpf_map__fd(map), &zero, &data, BPF_ANY);
+	if (!ASSERT_LT(ret, 0, "bpf_map_update_elem data_input\n"))
+		goto close_prog;
+
+	saved_len = *(__u32 *)data.payload;
+	*(__u32 *)data.payload = sizeof(data.payload);
+	ret = bpf_map_update_elem(bpf_map__fd(map), &zero, &data, BPF_ANY);
+	if (!ASSERT_LT(ret, 0, "bpf_map_update_elem data_input\n"))
+		goto close_prog;
+
+	*(__u32 *)data.payload = saved_len;
+	data.payload[sizeof(__u32)] = 'a';
+	ret = bpf_map_update_elem(bpf_map__fd(map), &zero, &data, BPF_ANY);
+	ASSERT_LT(ret, 0, "bpf_map_update_elem data_input\n");
+close_prog:
+	_run_setup_process(tmp_dir, "cleanup");
+
+	if (!skel)
+		return;
+
+	skel->bss->monitored_pid = 0;
+	test_verify_sig__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_verify_sig.c b/tools/testing/selftests/bpf/progs/test_verify_sig.c
new file mode 100644
index 000000000000..1c60c9b5c991
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_verify_sig.c
@@ -0,0 +1,160 @@
+// 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 <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include <bpf/bpf_endian.h>
+
+#define MAX_DATA_SIZE 4096
+
+#ifdef __BIG_ENDIAN__
+#define be32_to_cpu(x) (x)
+#else
+#define be32_to_cpu(x) ___bpf_swab32(x)
+#endif
+
+#define VERIFY_USE_SECONDARY_KEYRING (1UL)
+
+/* In stripped ARM and x86-64 modules, ~ is surprisingly rare. */
+#define MODULE_SIG_STRING "~Module signature appended~\n"
+
+typedef __u8 u8;
+typedef __u16 u16;
+typedef __u32 u32;
+typedef __u64 u64;
+
+enum pkey_id_type {
+	PKEY_ID_PGP,		/* OpenPGP generated key ID */
+	PKEY_ID_X509,		/* X.509 arbitrary subjectKeyIdentifier */
+	PKEY_ID_PKCS7,		/* Signature in PKCS#7 message */
+};
+
+/*
+ * 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 */
+};
+
+u32 monitored_pid;
+u32 keyring_id;
+
+struct data {
+	u8 payload[MAX_DATA_SIZE];
+};
+
+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";
+
+static int mod_check_sig(const struct module_signature *ms, size_t file_len)
+{
+	if (!ms)
+		return -ENOENT;
+
+	if (be32_to_cpu(ms->sig_len) >= file_len - sizeof(*ms))
+		return -EBADMSG;
+
+	if (ms->id_type != PKEY_ID_PKCS7)
+		return -ENOPKG;
+
+	if (ms->algo != 0 ||
+	    ms->hash != 0 ||
+	    ms->signer_len != 0 ||
+	    ms->key_id_len != 0 ||
+	    ms->__pad[0] != 0 ||
+	    ms->__pad[1] != 0 ||
+	    ms->__pad[2] != 0)
+		return -EBADMSG;
+
+	return 0;
+}
+
+SEC("lsm.s/bpf")
+int BPF_PROG(bpf, int cmd, union bpf_attr *attr, unsigned int size)
+{
+	const size_t marker_len = sizeof(MODULE_SIG_STRING) - 1;
+	char marker[sizeof(MODULE_SIG_STRING) - 1];
+	struct module_signature ms;
+	struct data *data_ptr;
+	u32 modlen;
+	u32 sig_len;
+	u64 value;
+	u8 *mod;
+	u32 pid;
+	int ret, zero = 0;
+
+	pid = bpf_get_current_pid_tgid() >> 32;
+	if (pid != monitored_pid)
+		return 0;
+
+	data_ptr = bpf_map_lookup_elem(&data_input, &zero);
+	if (!data_ptr)
+		return 0;
+
+	bpf_probe_read(&value, sizeof(value), &attr->value);
+
+	bpf_copy_from_user(data_ptr, sizeof(struct data),
+			   (void *)(unsigned long)value);
+
+	modlen = be32_to_cpu(*(u32 *)data_ptr->payload);
+	mod = data_ptr->payload + sizeof(u32);
+
+	if (modlen > sizeof(struct data) - sizeof(u32))
+		return -EINVAL;
+
+	if (modlen <= marker_len)
+		return -ENOENT;
+
+	modlen &= sizeof(struct data) - 1;
+	bpf_probe_read(marker, marker_len, (char *)mod + modlen - marker_len);
+
+	if (bpf_strncmp(marker, marker_len, MODULE_SIG_STRING))
+		return -ENOENT;
+
+	modlen -= marker_len;
+
+	if (modlen <= sizeof(ms))
+		return -EBADMSG;
+
+	bpf_probe_read(&ms, sizeof(ms), (char *)mod + (modlen - sizeof(ms)));
+
+	ret = mod_check_sig(&ms, modlen);
+	if (ret)
+		return ret;
+
+	sig_len = be32_to_cpu(ms.sig_len);
+	modlen -= sig_len + sizeof(ms);
+
+	modlen &= 0x3ff;
+	sig_len &= 0x3ff;
+
+	return bpf_verify_signature(mod, modlen, mod + modlen, sig_len,
+				    keyring_id + (PKEY_ID_PKCS7 << 16));
+}
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..48cb55abc4a4
--- /dev/null
+++ b/tools/testing/selftests/bpf/verify_sig_setup.sh
@@ -0,0 +1,100 @@
+#!/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.XXXX.log)"
+
+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
+
+	cat ${tmp_dir}/signing_key.der | keyctl padd asymmetric ebpf_testing_key @s
+}
+
+cleanup() {
+	local tmp_dir="$1"
+
+	keyctl unlink $(keyctl search @s asymmetric ebpf_testing_key) @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}"
-- 
2.25.1

Re: [PATCH v3 1/2] bpf: Add bpf_verify_signature() helper

[Daniel Borkmann]

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@huawei.com>
> Reported-by: kernel test robot <lkp@intel.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?

> +	}
> +}
> +
> +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?

> +	.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);
>   	}

RE: [PATCH v3 1/2] bpf: Add bpf_verify_signature() helper

[Roberto Sassu]

> From: Daniel Borkmann [mailto:daniel@iogearbox.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@huawei.com>
> > Reported-by: kernel test robot <lkp@intel.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);
> >   	}

Re: [PATCH v3 1/2] bpf: Add bpf_verify_signature() helper

[Daniel Borkmann]

On 6/10/22 4:59 PM, Roberto Sassu wrote:
>> From: Daniel Borkmann [mailto:daniel@iogearbox.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@huawei.com>
>>> Reported-by: kernel test robot <lkp@intel.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.

If realistically expected is really just two helpers, what speaks against a
bpf_verify_signature_pkcs7() and bpf_verify_signature_pgp() in that case, for
sake of better user experience?

Maybe one other angle.. if CONFIG_SYSTEM_DATA_VERIFICATION is enabled, it may
not be clear whether verify_pkcs7_signature() or a verify_pgp_signature() are
both always builtin. And then, we run into the issue again of more complex probing
for availability of the algs compared to simple ...

#if defined(CONFIG_SYSTEM_DATA_VERIFICATION) && defined(CONFIG_XYZ)
	case BPF_FUNC_verify_signature_xyz:
		return ..._proto;
#endif

... which bpftool and others easily understand.

>>> +	}
>>> +}
>>> +
>>> +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.

check_helper_mem_access() has:

      /* Allow zero-byte read from NULL, regardless of pointer type */
      if (zero_size_allowed && access_size == 0 &&
          register_is_null(reg))
              return 0;

So NULL/0 pair can be passed. Maybe good to add these corner cases to the test_progs
selftest additions then if it's needed.

>>> +	.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);
>>>    	}

Re: [PATCH v3 1/2] bpf: Add bpf_verify_signature() helper

[Alexei Starovoitov]

On Fri, Jun 10, 2022 at 6:59 AM Roberto Sassu <roberto.sassu@huawei.com> wrote:
>
> 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).
...
> + * 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).

Muxing all kinds of info in the 5th arg isn't great.
It's better to use dynptr here for data and sig.
It will free up two extra arguments.

Re: [PATCH v3 1/2] bpf: Add bpf_verify_signature() helper

[Alexei Starovoitov]

On Fri, Jun 10, 2022 at 6:59 AM Roberto Sassu <roberto.sassu@huawei.com> wrote:
> +       keyring = (keyring_id == U16_MAX) ?
> +                 cred->session_keyring : (struct key *)keyring_id;

This is too limiting.
bpf prog should be able to do what *key syscalls can do.
By doing lookup_user_key(id) -> keyring.
Maybe it's ok to have a special reserved id that does
cred->sessions_keyring as a shortcut, but that's an optimization.

Re: [PATCH v3 1/2] bpf: Add bpf_verify_signature() helper

[KP Singh]

On Fri, Jun 10, 2022 at 5:14 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 6/10/22 4:59 PM, Roberto Sassu wrote:
> >> From: Daniel Borkmann [mailto:daniel@iogearbox.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@huawei.com>
> >>> Reported-by: kernel test robot <lkp@intel.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.
>
> If realistically expected is really just two helpers, what speaks against a
> bpf_verify_signature_pkcs7() and bpf_verify_signature_pgp() in that case, for
> sake of better user experience?
>
> Maybe one other angle.. if CONFIG_SYSTEM_DATA_VERIFICATION is enabled, it may
> not be clear whether verify_pkcs7_signature() or a verify_pgp_signature() are
> both always builtin. And then, we run into the issue again of more complex probing
> for availability of the algs compared to simple ...
>
> #if defined(CONFIG_SYSTEM_DATA_VERIFICATION) && defined(CONFIG_XYZ)
>         case BPF_FUNC_verify_signature_xyz:
>                 return ..._proto;
> #endif
>
> ... which bpftool and others easily understand.
>
> >>> +   }
> >>> +}
> >>> +
> >>> +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.
>
> check_helper_mem_access() has:
>
>       /* Allow zero-byte read from NULL, regardless of pointer type */
>       if (zero_size_allowed && access_size == 0 &&
>           register_is_null(reg))
>               return 0;

Daniel, makes a fair point here. Alexei, what do you think?

I wonder if some "future" signature verification would need even more
/ different arguments so a unified bpf_verify_signature might get more
complex / not easy to extend.


>
> So NULL/0 pair can be passed. Maybe good to add these corner cases to the test_progs
> selftest additions then if it's needed.
>
> >>> +   .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);
> >>>     }
>

Re: [PATCH v3 1/2] bpf: Add bpf_verify_signature() helper

[Alexei Starovoitov]

On Fri, Jun 10, 2022 at 4:53 PM KP Singh <kpsingh@kernel.org> wrote:
> > >>> +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.
> >
> > check_helper_mem_access() has:
> >
> >       /* Allow zero-byte read from NULL, regardless of pointer type */
> >       if (zero_size_allowed && access_size == 0 &&
> >           register_is_null(reg))
> >               return 0;
>
> Daniel, makes a fair point here. Alexei, what do you think?
>
> I wonder if some "future" signature verification would need even more
> / different arguments so a unified bpf_verify_signature might get more
> complex / not easy to extend.

You mean a pkcs7 specific helper for now?
Makes sense.