Re: [PATCH v2 1/3] bpf: Add bpf_verify_pkcs7_signature() helper

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/8/22 1:12 PM, Roberto Sassu wrote:
Add the bpf_verify_pkcs7_signature() helper, to give the ability to eBPF
security modules to check the validity of a PKCS#7 signature against
supplied data.

Use the 'keyring' parameter to select the keyring containing the
verification key: 0 for the primary keyring, 1 for the primary and
secondary keyrings, 2 for the platform keyring.

Signed-off-by: Roberto Sassu <roberto.sassu@xxxxxxxxxx>
---
  include/uapi/linux/bpf.h       |  8 ++++++++
  kernel/bpf/bpf_lsm.c           | 32 ++++++++++++++++++++++++++++++++
  tools/include/uapi/linux/bpf.h |  8 ++++++++
  3 files changed, 48 insertions(+)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index f4009dbdf62d..40d0fc0d9493 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -5249,6 +5249,13 @@ 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_pkcs7_signature(u8 *data, u32 datalen, u8 *sig, u32 siglen, u64 keyring)
+ *	Description
+ *		Verify the PKCS#7 *sig* with length *siglen*, on *data* with
+ *		length *datalen*, with key in *keyring*.

Could you also add a description for users about the keyring argument and guidance on when
they should use which in their programs? Above is a bit too terse, imho.

+ *	Return
+ *		0 on success, a negative value on error.
   */
  #define __BPF_FUNC_MAPPER(FN)		\
  	FN(unspec),			\
@@ -5455,6 +5462,7 @@ union bpf_attr {
  	FN(dynptr_read),		\
  	FN(dynptr_write),		\
  	FN(dynptr_data),		\
+	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..1cda43cb541a 100644
--- a/kernel/bpf/bpf_lsm.c
+++ b/kernel/bpf/bpf_lsm.c
@@ -16,6 +16,7 @@
  #include <linux/bpf_local_storage.h>
  #include <linux/btf_ids.h>
  #include <linux/ima.h>
+#include <linux/verification.h>
/* For every LSM hook that allows attachment of BPF programs, declare a nop
   * function where a BPF program can be attached.
@@ -132,6 +133,35 @@ static const struct bpf_func_proto bpf_get_attach_cookie_proto = {
  	.arg1_type	= ARG_PTR_TO_CTX,
  };
+BPF_CALL_5(bpf_verify_pkcs7_signature, u8 *, data, u32, datalen, u8 *, sig,
+	   u32, siglen, u64, keyring)
+{
+	int ret = -EOPNOTSUPP;
+
+#ifdef CONFIG_SYSTEM_DATA_VERIFICATION
+	if (keyring > (unsigned long)VERIFY_USE_PLATFORM_KEYRING)
+		return -EINVAL;
+
+	ret = verify_pkcs7_signature(data, datalen, sig, siglen,
+				     (struct key *)keyring,
+				     VERIFYING_UNSPECIFIED_SIGNATURE, NULL,
+				     NULL);
+#endif
+	return ret;
+}

Looks great! One small nit, I would move all of the BPF_CALL and _proto under the
#ifdef CONFIG_SYSTEM_DATA_VERIFICATION ...

+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_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,
+};
+
  static const struct bpf_func_proto *
  bpf_lsm_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
  {
@@ -158,6 +188,8 @@ 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;
+	case BPF_FUNC_verify_pkcs7_signature:
+		return prog->aux->sleepable ? &bpf_verify_pkcs7_signature_proto : NULL;

... same here:

#ifdef CONFIG_SYSTEM_DATA_VERIFICATION
	case BPF_FUNC_verify_pkcs7_signature:
		return prog->aux->sleepable ? &bpf_verify_pkcs7_signature_proto : NULL;
#endif

So that bpftool or other feature probes can check for its availability. Otherwise, apps have
a hard time checking whether bpf_verify_pkcs7_signature() helper is available for use or not.

  	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..40d0fc0d9493 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -5249,6 +5249,13 @@ 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_pkcs7_signature(u8 *data, u32 datalen, u8 *sig, u32 siglen, u64 keyring)
+ *	Description
+ *		Verify the PKCS#7 *sig* with length *siglen*, on *data* with
+ *		length *datalen*, with key in *keyring*.
+ *	Return
+ *		0 on success, a negative value on error.
   */
  #define __BPF_FUNC_MAPPER(FN)		\
  	FN(unspec),			\
@@ -5455,6 +5462,7 @@ union bpf_attr {
  	FN(dynptr_read),		\
  	FN(dynptr_write),		\
  	FN(dynptr_data),		\
+	FN(verify_pkcs7_signature),	\
  	/* */
/* integer value in 'imm' field of BPF_CALL instruction selects which helper





[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux