[PATCH v3 0/9] bpf-lsm: Extend interoperability with IMA

[PATCH v3 0/9] bpf-lsm: Extend interoperability with IMA

[Roberto Sassu]

Extend the interoperability with IMA, to give wider flexibility for the
implementation of integrity-focused LSMs based on eBPF.

Patch 1 fixes some style issues.

Patches 2-6 give the ability to eBPF-based LSMs to take advantage of the
measurement capability of IMA without needing to setup a policy in IMA
(those LSMs might implement the policy capability themselves).

Patches 7-9 allow eBPF-based LSMs to evaluate files read by the kernel.

Changelog

v2:
- Add better description to patch 1 (suggested by Shuah)
- Recalculate digest if it is not fresh (when IMA_COLLECTED flag not set)
- Move declaration of bpf_ima_file_hash() at the end (suggested by
  Yonghong)
- Add tests to check if the digest has been recalculated
- Add deny test for bpf_kernel_read_file()
- Add description to tests

v1:
- Modify ima_file_hash() only and allow the usage of the function with the
  modified behavior by eBPF-based LSMs through the new function
  bpf_ima_file_hash() (suggested by Mimi)
- Make bpf_lsm_kernel_read_file() sleepable so that bpf_ima_inode_hash()
  and bpf_ima_file_hash() can be called inside the implementation of
  eBPF-based LSMs for this hook

Roberto Sassu (9):
  ima: Fix documentation-related warnings in ima_main.c
  ima: Always return a file measurement in ima_file_hash()
  bpf-lsm: Introduce new helper bpf_ima_file_hash()
  selftests/bpf: Move sample generation code to ima_test_common()
  selftests/bpf: Add test for bpf_ima_file_hash()
  selftests/bpf: Check if the digest is refreshed after a file write
  bpf-lsm: Make bpf_lsm_kernel_read_file() as sleepable
  selftests/bpf: Add test for bpf_lsm_kernel_read_file()
  selftests/bpf: Check that bpf_kernel_read_file() denies reading IMA
    policy

 include/uapi/linux/bpf.h                      |  11 ++
 kernel/bpf/bpf_lsm.c                          |  21 +++
 security/integrity/ima/ima_main.c             |  57 ++++---
 tools/include/uapi/linux/bpf.h                |  11 ++
 tools/testing/selftests/bpf/ima_setup.sh      |  35 +++-
 .../selftests/bpf/prog_tests/test_ima.c       | 149 +++++++++++++++++-
 tools/testing/selftests/bpf/progs/ima.c       |  66 +++++++-
 7 files changed, 321 insertions(+), 29 deletions(-)

-- 
2.32.0

[PATCH v3 1/9] ima: Fix documentation-related warnings in ima_main.c

[Roberto Sassu]

Fix the following warnings in ima_main.c, displayed with W=n make argument:

security/integrity/ima/ima_main.c:432: warning: Function parameter or
                          member 'vma' not described in 'ima_file_mprotect'
security/integrity/ima/ima_main.c:636: warning: Function parameter or
                  member 'inode' not described in 'ima_post_create_tmpfile'
security/integrity/ima/ima_main.c:636: warning: Excess function parameter
                            'file' description in 'ima_post_create_tmpfile'
security/integrity/ima/ima_main.c:843: warning: Function parameter or
                     member 'load_id' not described in 'ima_post_load_data'
security/integrity/ima/ima_main.c:843: warning: Excess function parameter
                                   'id' description in 'ima_post_load_data'

Also, fix some style issues in the description of ima_post_create_tmpfile()
and ima_post_path_mknod().

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima_main.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 8c6e4514d494..946ba8a12eab 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -418,6 +418,7 @@ int ima_file_mmap(struct file *file, unsigned long prot)
 
 /**
  * ima_file_mprotect - based on policy, limit mprotect change
+ * @vma: vm_area_struct protection is set to
  * @prot: contains the protection that will be applied by the kernel.
  *
  * Files can be mmap'ed read/write and later changed to execute to circumvent
@@ -610,8 +611,8 @@ EXPORT_SYMBOL_GPL(ima_inode_hash);
 
 /**
  * ima_post_create_tmpfile - mark newly created tmpfile as new
- * @mnt_userns:	user namespace of the mount the inode was found from
- * @file : newly created tmpfile
+ * @mnt_userns: user namespace of the mount the inode was found from
+ * @inode: inode of the newly created tmpfile
  *
  * No measuring, appraising or auditing of newly created tmpfiles is needed.
  * Skip calling process_measurement(), but indicate which newly, created
@@ -643,7 +644,7 @@ void ima_post_create_tmpfile(struct user_namespace *mnt_userns,
 
 /**
  * ima_post_path_mknod - mark as a new inode
- * @mnt_userns:	user namespace of the mount the inode was found from
+ * @mnt_userns: user namespace of the mount the inode was found from
  * @dentry: newly created dentry
  *
  * Mark files created via the mknodat syscall as new, so that the
@@ -814,8 +815,8 @@ int ima_load_data(enum kernel_load_data_id id, bool contents)
  * ima_post_load_data - appraise decision based on policy
  * @buf: pointer to in memory file contents
  * @size: size of in memory file contents
- * @id: kernel load data caller identifier
- * @description: @id-specific description of contents
+ * @load_id: kernel load data caller identifier
+ * @description: @load_id-specific description of contents
  *
  * Measure/appraise/audit in memory buffer based on policy.  Policy rules
  * are written in terms of a policy identifier.
-- 
2.32.0

Re: [PATCH v3 1/9] ima: Fix documentation-related warnings in ima_main.c

[Mimi Zohar]

On Wed, 2022-03-02 at 12:13 +0100, Roberto Sassu wrote:
> Fix the following warnings in ima_main.c, displayed with W=n make argument:
> 
> security/integrity/ima/ima_main.c:432: warning: Function parameter or
>                           member 'vma' not described in 'ima_file_mprotect'
> security/integrity/ima/ima_main.c:636: warning: Function parameter or
>                   member 'inode' not described in 'ima_post_create_tmpfile'
> security/integrity/ima/ima_main.c:636: warning: Excess function parameter
>                             'file' description in 'ima_post_create_tmpfile'
> security/integrity/ima/ima_main.c:843: warning: Function parameter or
>                      member 'load_id' not described in 'ima_post_load_data'
> security/integrity/ima/ima_main.c:843: warning: Excess function parameter
>                                    'id' description in 'ima_post_load_data'
> 
> Also, fix some style issues in the description of ima_post_create_tmpfile()
> and ima_post_path_mknod().
> 
> Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>

[PATCH v3 2/9] ima: Always return a file measurement in ima_file_hash()

[Roberto Sassu]

__ima_inode_hash() checks if a digest has been already calculated by
looking for the integrity_iint_cache structure associated to the passed
inode.

Users of ima_file_hash() (e.g. eBPF) might be interested in obtaining the
information without having to setup an IMA policy so that the digest is
always available at the time they call this function.

In addition, they likely expect the digest to be fresh, e.g. recalculated
by IMA after a file write. Although getting the digest from the
bprm_committed_creds hook (as in the eBPF test) ensures that the digest is
fresh, as the IMA hook is executed before that hook, this is not always the
case (e.g. for the mmap_file hook).

Call ima_collect_measurement() in __ima_inode_hash(), if the file
descriptor is available (passed by ima_file_hash()) and the digest is not
available/not fresh, and store the file measurement in a temporary
integrity_iint_cache structure.

This change does not cause memory usage increase, due to using the
temporary integrity_iint_cache structure, and due to freeing the
ima_digest_data structure inside integrity_iint_cache before exiting from
__ima_inode_hash().

For compatibility reasons, the behavior of ima_inode_hash() remains
unchanged.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima_main.c | 46 ++++++++++++++++++++++---------
 1 file changed, 33 insertions(+), 13 deletions(-)

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 946ba8a12eab..ed1a82f1def3 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -520,20 +520,38 @@ int ima_file_check(struct file *file, int mask)
 }
 EXPORT_SYMBOL_GPL(ima_file_check);
 
-static int __ima_inode_hash(struct inode *inode, char *buf, size_t buf_size)
+static int __ima_inode_hash(struct inode *inode, struct file *file, char *buf,
+			    size_t buf_size)
 {
-	struct integrity_iint_cache *iint;
-	int hash_algo;
+	struct integrity_iint_cache *iint = NULL, tmp_iint;
+	int rc, hash_algo;
 
-	if (!ima_policy_flag)
-		return -EOPNOTSUPP;
+	if (ima_policy_flag) {
+		iint = integrity_iint_find(inode);
+		if (iint)
+			mutex_lock(&iint->mutex);
+	}
+
+	if ((!iint || !(iint->flags & IMA_COLLECTED)) && file) {
+		if (iint)
+			mutex_unlock(&iint->mutex);
+
+		memset(&tmp_iint, 0, sizeof(tmp_iint));
+		tmp_iint.inode = inode;
+		mutex_init(&tmp_iint.mutex);
+
+		rc = ima_collect_measurement(&tmp_iint, file, NULL, 0,
+					     ima_hash_algo, NULL);
+		if (rc < 0)
+			return -EOPNOTSUPP;
+
+		iint = &tmp_iint;
+		mutex_lock(&iint->mutex);
+	}
 
-	iint = integrity_iint_find(inode);
 	if (!iint)
 		return -EOPNOTSUPP;
 
-	mutex_lock(&iint->mutex);
-
 	/*
 	 * ima_file_hash can be called when ima_collect_measurement has still
 	 * not been called, we might not always have a hash.
@@ -552,12 +570,14 @@ static int __ima_inode_hash(struct inode *inode, char *buf, size_t buf_size)
 	hash_algo = iint->ima_hash->algo;
 	mutex_unlock(&iint->mutex);
 
+	if (iint == &tmp_iint)
+		kfree(iint->ima_hash);
+
 	return hash_algo;
 }
 
 /**
- * ima_file_hash - return the stored measurement if a file has been hashed and
- * is in the iint cache.
+ * ima_file_hash - return a measurement of the file
  * @file: pointer to the file
  * @buf: buffer in which to store the hash
  * @buf_size: length of the buffer
@@ -570,7 +590,7 @@ static int __ima_inode_hash(struct inode *inode, char *buf, size_t buf_size)
  * The file hash returned is based on the entire file, including the appended
  * signature.
  *
- * If IMA is disabled or if no measurement is available, return -EOPNOTSUPP.
+ * If the measurement cannot be performed, return -EOPNOTSUPP.
  * If the parameters are incorrect, return -EINVAL.
  */
 int ima_file_hash(struct file *file, char *buf, size_t buf_size)
@@ -578,7 +598,7 @@ int ima_file_hash(struct file *file, char *buf, size_t buf_size)
 	if (!file)
 		return -EINVAL;
 
-	return __ima_inode_hash(file_inode(file), buf, buf_size);
+	return __ima_inode_hash(file_inode(file), file, buf, buf_size);
 }
 EXPORT_SYMBOL_GPL(ima_file_hash);
 
@@ -605,7 +625,7 @@ int ima_inode_hash(struct inode *inode, char *buf, size_t buf_size)
 	if (!inode)
 		return -EINVAL;
 
-	return __ima_inode_hash(inode, buf, buf_size);
+	return __ima_inode_hash(inode, NULL, buf, buf_size);
 }
 EXPORT_SYMBOL_GPL(ima_inode_hash);
 
-- 
2.32.0

Re: [PATCH v3 2/9] ima: Always return a file measurement in ima_file_hash()

[Mimi Zohar]

On Wed, 2022-03-02 at 12:13 +0100, Roberto Sassu wrote:
> __ima_inode_hash() checks if a digest has been already calculated by
> looking for the integrity_iint_cache structure associated to the passed
> inode.
> 
> Users of ima_file_hash() (e.g. eBPF) might be interested in obtaining the
> information without having to setup an IMA policy so that the digest is
> always available at the time they call this function.
> 
> In addition, they likely expect the digest to be fresh, e.g. recalculated
> by IMA after a file write. Although getting the digest from the
> bprm_committed_creds hook (as in the eBPF test) ensures that the digest is
> fresh, as the IMA hook is executed before that hook, this is not always the
> case (e.g. for the mmap_file hook).
> 
> Call ima_collect_measurement() in __ima_inode_hash(), if the file
> descriptor is available (passed by ima_file_hash()) and the digest is not
> available/not fresh, and store the file measurement in a temporary
> integrity_iint_cache structure.
> 
> This change does not cause memory usage increase, due to using the
> temporary integrity_iint_cache structure, and due to freeing the
> ima_digest_data structure inside integrity_iint_cache before exiting from
> __ima_inode_hash().
> 
> For compatibility reasons, the behavior of ima_inode_hash() remains
> unchanged.
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

The patch itself is fine, but with great hesitancy due to the existing
eBPF integrity gaps and how these functions are planned to be used,

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>

[PATCH v3 3/9] bpf-lsm: Introduce new helper bpf_ima_file_hash()

[Roberto Sassu]

ima_file_hash() has been modified to calculate the measurement of a file on
demand, if it has not been already performed by IMA or the measurement is
not fresh. For compatibility reasons, ima_inode_hash() remains unchanged.

Keep the same approach in eBPF and introduce the new helper
bpf_ima_file_hash() to take advantage of the modified behavior of
ima_file_hash().

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 include/uapi/linux/bpf.h       | 11 +++++++++++
 kernel/bpf/bpf_lsm.c           | 20 ++++++++++++++++++++
 tools/include/uapi/linux/bpf.h | 11 +++++++++++
 3 files changed, 42 insertions(+)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index b0383d371b9a..87d996e954d4 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -5018,6 +5018,16 @@ union bpf_attr {
  *
  *	Return
  *		The number of arguments of the traced function.
+ *
+ * long bpf_ima_file_hash(struct file *file, void *dst, u32 size)
+ *	Description
+ *		Returns a calculated IMA hash of the *file*.
+ *		If the hash is larger than *size*, then only *size*
+ *		bytes will be copied to *dst*
+ *	Return
+ *		The **hash_algo** is returned on success,
+ *		**-EOPNOTSUP** if the hash calculation failed or **-EINVAL** if
+ *		invalid arguments are passed.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -5206,6 +5216,7 @@ union bpf_attr {
 	FN(get_func_arg),		\
 	FN(get_func_ret),		\
 	FN(get_func_arg_cnt),		\
+	FN(ima_file_hash),		\
 	/* */
 
 /* 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 9e4ecc990647..e8d27af5bbcc 100644
--- a/kernel/bpf/bpf_lsm.c
+++ b/kernel/bpf/bpf_lsm.c
@@ -99,6 +99,24 @@ static const struct bpf_func_proto bpf_ima_inode_hash_proto = {
 	.allowed	= bpf_ima_inode_hash_allowed,
 };
 
+BPF_CALL_3(bpf_ima_file_hash, struct file *, file, void *, dst, u32, size)
+{
+	return ima_file_hash(file, dst, size);
+}
+
+BTF_ID_LIST_SINGLE(bpf_ima_file_hash_btf_ids, struct, file)
+
+static const struct bpf_func_proto bpf_ima_file_hash_proto = {
+	.func		= bpf_ima_file_hash,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_BTF_ID,
+	.arg1_btf_id	= &bpf_ima_file_hash_btf_ids[0],
+	.arg2_type	= ARG_PTR_TO_UNINIT_MEM,
+	.arg3_type	= ARG_CONST_SIZE,
+	.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)
 {
@@ -121,6 +139,8 @@ bpf_lsm_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_bprm_opts_set_proto;
 	case BPF_FUNC_ima_inode_hash:
 		return prog->aux->sleepable ? &bpf_ima_inode_hash_proto : NULL;
+	case BPF_FUNC_ima_file_hash:
+		return prog->aux->sleepable ? &bpf_ima_file_hash_proto : NULL;
 	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 b0383d371b9a..87d996e954d4 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -5018,6 +5018,16 @@ union bpf_attr {
  *
  *	Return
  *		The number of arguments of the traced function.
+ *
+ * long bpf_ima_file_hash(struct file *file, void *dst, u32 size)
+ *	Description
+ *		Returns a calculated IMA hash of the *file*.
+ *		If the hash is larger than *size*, then only *size*
+ *		bytes will be copied to *dst*
+ *	Return
+ *		The **hash_algo** is returned on success,
+ *		**-EOPNOTSUP** if the hash calculation failed or **-EINVAL** if
+ *		invalid arguments are passed.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -5206,6 +5216,7 @@ union bpf_attr {
 	FN(get_func_arg),		\
 	FN(get_func_ret),		\
 	FN(get_func_arg_cnt),		\
+	FN(ima_file_hash),		\
 	/* */
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
-- 
2.32.0

[PATCH v3 4/9] selftests/bpf: Move sample generation code to ima_test_common()

[Roberto Sassu]

Move sample generator code to ima_test_common() so that the new function
can be called by multiple LSM hooks.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 tools/testing/selftests/bpf/progs/ima.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/ima.c b/tools/testing/selftests/bpf/progs/ima.c
index 96060ff4ffc6..b5a0de50d1b4 100644
--- a/tools/testing/selftests/bpf/progs/ima.c
+++ b/tools/testing/selftests/bpf/progs/ima.c
@@ -18,8 +18,7 @@ struct {
 
 char _license[] SEC("license") = "GPL";
 
-SEC("lsm.s/bprm_committed_creds")
-void BPF_PROG(ima, struct linux_binprm *bprm)
+static void ima_test_common(struct file *file)
 {
 	u64 ima_hash = 0;
 	u64 *sample;
@@ -28,7 +27,7 @@ void BPF_PROG(ima, struct linux_binprm *bprm)
 
 	pid = bpf_get_current_pid_tgid() >> 32;
 	if (pid == monitored_pid) {
-		ret = bpf_ima_inode_hash(bprm->file->f_inode, &ima_hash,
+		ret = bpf_ima_inode_hash(file->f_inode, &ima_hash,
 					 sizeof(ima_hash));
 		if (ret < 0 || ima_hash == 0)
 			return;
@@ -43,3 +42,9 @@ void BPF_PROG(ima, struct linux_binprm *bprm)
 
 	return;
 }
+
+SEC("lsm.s/bprm_committed_creds")
+void BPF_PROG(bprm_committed_creds, struct linux_binprm *bprm)
+{
+	ima_test_common(bprm->file);
+}
-- 
2.32.0

[PATCH v3 5/9] selftests/bpf: Add test for bpf_ima_file_hash()

[Roberto Sassu]

Add new test to ensure that bpf_ima_file_hash() returns the digest of the
executed files.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 .../selftests/bpf/prog_tests/test_ima.c       | 43 +++++++++++++++++--
 tools/testing/selftests/bpf/progs/ima.c       | 10 ++++-
 2 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/test_ima.c b/tools/testing/selftests/bpf/prog_tests/test_ima.c
index 97d8a6f84f4a..acc911ba63fd 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_ima.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_ima.c
@@ -13,6 +13,8 @@
 
 #include "ima.skel.h"
 
+#define MAX_SAMPLES 2
+
 static int run_measured_process(const char *measured_dir, u32 *monitored_pid)
 {
 	int child_pid, child_status;
@@ -32,14 +34,25 @@ static int run_measured_process(const char *measured_dir, u32 *monitored_pid)
 	return -EINVAL;
 }
 
-static u64 ima_hash_from_bpf;
+static u64 ima_hash_from_bpf[MAX_SAMPLES];
+static int ima_hash_from_bpf_idx;
 
 static int process_sample(void *ctx, void *data, size_t len)
 {
-	ima_hash_from_bpf = *((u64 *)data);
+	if (ima_hash_from_bpf_idx >= MAX_SAMPLES)
+		return -ENOSPC;
+
+	ima_hash_from_bpf[ima_hash_from_bpf_idx++] = *((u64 *)data);
 	return 0;
 }
 
+static void test_init(struct ima__bss *bss)
+{
+	ima_hash_from_bpf_idx = 0;
+
+	bss->use_ima_file_hash = false;
+}
+
 void test_test_ima(void)
 {
 	char measured_dir_template[] = "/tmp/ima_measuredXXXXXX";
@@ -72,13 +85,35 @@ void test_test_ima(void)
 	if (CHECK(err, "failed to run command", "%s, errno = %d\n", cmd, errno))
 		goto close_clean;
 
+	/*
+	 * Test #1
+	 * - Goal: obtain a sample with the bpf_ima_inode_hash() helper
+	 * - Expected result:  1 sample (/bin/true)
+	 */
+	test_init(skel->bss);
 	err = run_measured_process(measured_dir, &skel->bss->monitored_pid);
-	if (CHECK(err, "run_measured_process", "err = %d\n", err))
+	if (CHECK(err, "run_measured_process #1", "err = %d\n", err))
 		goto close_clean;
 
 	err = ring_buffer__consume(ringbuf);
 	ASSERT_EQ(err, 1, "num_samples_or_err");
-	ASSERT_NEQ(ima_hash_from_bpf, 0, "ima_hash");
+	ASSERT_NEQ(ima_hash_from_bpf[0], 0, "ima_hash");
+
+	/*
+	 * Test #2
+	 * - Goal: obtain samples with the bpf_ima_file_hash() helper
+	 * - Expected result: 2 samples (./ima_setup.sh, /bin/true)
+	 */
+	test_init(skel->bss);
+	skel->bss->use_ima_file_hash = true;
+	err = run_measured_process(measured_dir, &skel->bss->monitored_pid);
+	if (CHECK(err, "run_measured_process #2", "err = %d\n", err))
+		goto close_clean;
+
+	err = ring_buffer__consume(ringbuf);
+	ASSERT_EQ(err, 2, "num_samples_or_err");
+	ASSERT_NEQ(ima_hash_from_bpf[0], 0, "ima_hash");
+	ASSERT_NEQ(ima_hash_from_bpf[1], 0, "ima_hash");
 
 close_clean:
 	snprintf(cmd, sizeof(cmd), "./ima_setup.sh cleanup %s", measured_dir);
diff --git a/tools/testing/selftests/bpf/progs/ima.c b/tools/testing/selftests/bpf/progs/ima.c
index b5a0de50d1b4..e0b073dcfb5d 100644
--- a/tools/testing/selftests/bpf/progs/ima.c
+++ b/tools/testing/selftests/bpf/progs/ima.c
@@ -18,6 +18,8 @@ struct {
 
 char _license[] SEC("license") = "GPL";
 
+bool use_ima_file_hash;
+
 static void ima_test_common(struct file *file)
 {
 	u64 ima_hash = 0;
@@ -27,8 +29,12 @@ static void ima_test_common(struct file *file)
 
 	pid = bpf_get_current_pid_tgid() >> 32;
 	if (pid == monitored_pid) {
-		ret = bpf_ima_inode_hash(file->f_inode, &ima_hash,
-					 sizeof(ima_hash));
+		if (!use_ima_file_hash)
+			ret = bpf_ima_inode_hash(file->f_inode, &ima_hash,
+						 sizeof(ima_hash));
+		else
+			ret = bpf_ima_file_hash(file, &ima_hash,
+						sizeof(ima_hash));
 		if (ret < 0 || ima_hash == 0)
 			return;
 
-- 
2.32.0

[PATCH v3 6/9] selftests/bpf: Check if the digest is refreshed after a file write

[Roberto Sassu]

Verify that bpf_ima_inode_hash() returns a non-fresh digest after a file
write, and that bpf_ima_file_hash() returns a fresh digest. Verification is
done by requesting the digest from the bprm_creds_for_exec hook, called
before ima_bprm_check().

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 tools/testing/selftests/bpf/ima_setup.sh      | 24 ++++++-
 .../selftests/bpf/prog_tests/test_ima.c       | 72 ++++++++++++++++++-
 tools/testing/selftests/bpf/progs/ima.c       | 11 +++
 3 files changed, 103 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/bpf/ima_setup.sh b/tools/testing/selftests/bpf/ima_setup.sh
index 8e62581113a3..a3de1cd43ba0 100755
--- a/tools/testing/selftests/bpf/ima_setup.sh
+++ b/tools/testing/selftests/bpf/ima_setup.sh
@@ -12,7 +12,7 @@ LOG_FILE="$(mktemp /tmp/ima_setup.XXXX.log)"
 
 usage()
 {
-	echo "Usage: $0 <setup|cleanup|run> <existing_tmp_dir>"
+	echo "Usage: $0 <setup|cleanup|run|modify-bin|restore-bin> <existing_tmp_dir>"
 	exit 1
 }
 
@@ -77,6 +77,24 @@ run()
 	exec "${copied_bin_path}"
 }
 
+modify_bin()
+{
+	local tmp_dir="$1"
+	local mount_dir="${tmp_dir}/mnt"
+	local copied_bin_path="${mount_dir}/$(basename ${TEST_BINARY})"
+
+	echo "mod" >> "${copied_bin_path}"
+}
+
+restore_bin()
+{
+	local tmp_dir="$1"
+	local mount_dir="${tmp_dir}/mnt"
+	local copied_bin_path="${mount_dir}/$(basename ${TEST_BINARY})"
+
+	truncate -s -4 "${copied_bin_path}"
+}
+
 catch()
 {
 	local exit_code="$1"
@@ -105,6 +123,10 @@ main()
 		cleanup "${tmp_dir}"
 	elif [[ "${action}" == "run" ]]; then
 		run "${tmp_dir}"
+	elif [[ "${action}" == "modify-bin" ]]; then
+		modify_bin "${tmp_dir}"
+	elif [[ "${action}" == "restore-bin" ]]; then
+		restore_bin "${tmp_dir}"
 	else
 		echo "Unknown action: ${action}"
 		exit 1
diff --git a/tools/testing/selftests/bpf/prog_tests/test_ima.c b/tools/testing/selftests/bpf/prog_tests/test_ima.c
index acc911ba63fd..a0cfba0b4273 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_ima.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_ima.c
@@ -13,16 +13,17 @@
 
 #include "ima.skel.h"
 
-#define MAX_SAMPLES 2
+#define MAX_SAMPLES 4
 
-static int run_measured_process(const char *measured_dir, u32 *monitored_pid)
+static int _run_measured_process(const char *measured_dir, u32 *monitored_pid,
+				 const char *cmd)
 {
 	int child_pid, child_status;
 
 	child_pid = fork();
 	if (child_pid == 0) {
 		*monitored_pid = getpid();
-		execlp("./ima_setup.sh", "./ima_setup.sh", "run", measured_dir,
+		execlp("./ima_setup.sh", "./ima_setup.sh", cmd, measured_dir,
 		       NULL);
 		exit(errno);
 
@@ -34,6 +35,11 @@ static int run_measured_process(const char *measured_dir, u32 *monitored_pid)
 	return -EINVAL;
 }
 
+static int run_measured_process(const char *measured_dir, u32 *monitored_pid)
+{
+	return _run_measured_process(measured_dir, monitored_pid, "run");
+}
+
 static u64 ima_hash_from_bpf[MAX_SAMPLES];
 static int ima_hash_from_bpf_idx;
 
@@ -51,6 +57,7 @@ static void test_init(struct ima__bss *bss)
 	ima_hash_from_bpf_idx = 0;
 
 	bss->use_ima_file_hash = false;
+	bss->enable_bprm_creds_for_exec = false;
 }
 
 void test_test_ima(void)
@@ -58,6 +65,7 @@ void test_test_ima(void)
 	char measured_dir_template[] = "/tmp/ima_measuredXXXXXX";
 	struct ring_buffer *ringbuf = NULL;
 	const char *measured_dir;
+	u64 bin_true_sample;
 	char cmd[256];
 
 	int err, duration = 0;
@@ -114,6 +122,64 @@ void test_test_ima(void)
 	ASSERT_EQ(err, 2, "num_samples_or_err");
 	ASSERT_NEQ(ima_hash_from_bpf[0], 0, "ima_hash");
 	ASSERT_NEQ(ima_hash_from_bpf[1], 0, "ima_hash");
+	bin_true_sample = ima_hash_from_bpf[1];
+
+	/*
+	 * Test #3
+	 * - Goal: confirm that bpf_ima_inode_hash() returns a non-fresh digest
+	 * - Expected result: 2 samples (/bin/true: non-fresh, fresh)
+	 */
+	test_init(skel->bss);
+
+	err = _run_measured_process(measured_dir, &skel->bss->monitored_pid,
+				    "modify-bin");
+	if (CHECK(err, "modify-bin #3", "err = %d\n", err))
+		goto close_clean;
+
+	skel->bss->enable_bprm_creds_for_exec = true;
+	err = run_measured_process(measured_dir, &skel->bss->monitored_pid);
+	if (CHECK(err, "run_measured_process #3", "err = %d\n", err))
+		goto close_clean;
+
+	err = ring_buffer__consume(ringbuf);
+	ASSERT_EQ(err, 2, "num_samples_or_err");
+	ASSERT_NEQ(ima_hash_from_bpf[0], 0, "ima_hash");
+	ASSERT_NEQ(ima_hash_from_bpf[1], 0, "ima_hash");
+	ASSERT_EQ(ima_hash_from_bpf[0], bin_true_sample, "sample_equal_or_err");
+	/* IMA refreshed the digest. */
+	ASSERT_NEQ(ima_hash_from_bpf[1], bin_true_sample,
+		   "sample_different_or_err");
+
+	/*
+	 * Test #4
+	 * - Goal: verify that bpf_ima_file_hash() returns a fresh digest
+	 * - Expected result: 4 samples (./ima_setup.sh: fresh, fresh;
+	 *                               /bin/true: fresh, fresh)
+	 */
+	test_init(skel->bss);
+	skel->bss->use_ima_file_hash = true;
+	skel->bss->enable_bprm_creds_for_exec = true;
+	err = run_measured_process(measured_dir, &skel->bss->monitored_pid);
+	if (CHECK(err, "run_measured_process #4", "err = %d\n", err))
+		goto close_clean;
+
+	err = ring_buffer__consume(ringbuf);
+	ASSERT_EQ(err, 4, "num_samples_or_err");
+	ASSERT_NEQ(ima_hash_from_bpf[0], 0, "ima_hash");
+	ASSERT_NEQ(ima_hash_from_bpf[1], 0, "ima_hash");
+	ASSERT_NEQ(ima_hash_from_bpf[2], 0, "ima_hash");
+	ASSERT_NEQ(ima_hash_from_bpf[3], 0, "ima_hash");
+	ASSERT_NEQ(ima_hash_from_bpf[2], bin_true_sample,
+		   "sample_different_or_err");
+	ASSERT_EQ(ima_hash_from_bpf[3], ima_hash_from_bpf[2],
+		  "sample_equal_or_err");
+
+	skel->bss->use_ima_file_hash = false;
+	skel->bss->enable_bprm_creds_for_exec = false;
+	err = _run_measured_process(measured_dir, &skel->bss->monitored_pid,
+				    "restore-bin");
+	if (CHECK(err, "restore-bin #3", "err = %d\n", err))
+		goto close_clean;
 
 close_clean:
 	snprintf(cmd, sizeof(cmd), "./ima_setup.sh cleanup %s", measured_dir);
diff --git a/tools/testing/selftests/bpf/progs/ima.c b/tools/testing/selftests/bpf/progs/ima.c
index e0b073dcfb5d..9633e5f2453d 100644
--- a/tools/testing/selftests/bpf/progs/ima.c
+++ b/tools/testing/selftests/bpf/progs/ima.c
@@ -19,6 +19,7 @@ struct {
 char _license[] SEC("license") = "GPL";
 
 bool use_ima_file_hash;
+bool enable_bprm_creds_for_exec;
 
 static void ima_test_common(struct file *file)
 {
@@ -54,3 +55,13 @@ void BPF_PROG(bprm_committed_creds, struct linux_binprm *bprm)
 {
 	ima_test_common(bprm->file);
 }
+
+SEC("lsm.s/bprm_creds_for_exec")
+int BPF_PROG(bprm_creds_for_exec, struct linux_binprm *bprm)
+{
+	if (!enable_bprm_creds_for_exec)
+		return 0;
+
+	ima_test_common(bprm->file);
+	return 0;
+}
-- 
2.32.0

[PATCH v3 7/9] bpf-lsm: Make bpf_lsm_kernel_read_file() as sleepable

[Roberto Sassu]

Make bpf_lsm_kernel_read_file() as sleepable, so that bpf_ima_inode_hash()
or bpf_ima_file_hash() can be called inside the implementation of this
hook.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 kernel/bpf/bpf_lsm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
index e8d27af5bbcc..064eccba641d 100644
--- a/kernel/bpf/bpf_lsm.c
+++ b/kernel/bpf/bpf_lsm.c
@@ -187,6 +187,7 @@ BTF_ID(func, bpf_lsm_inode_setxattr)
 BTF_ID(func, bpf_lsm_inode_symlink)
 BTF_ID(func, bpf_lsm_inode_unlink)
 BTF_ID(func, bpf_lsm_kernel_module_request)
+BTF_ID(func, bpf_lsm_kernel_read_file)
 BTF_ID(func, bpf_lsm_kernfs_init_security)
 
 #ifdef CONFIG_KEYS
-- 
2.32.0

[PATCH v3 8/9] selftests/bpf: Add test for bpf_lsm_kernel_read_file()

[Roberto Sassu]

Test the ability of bpf_lsm_kernel_read_file() to call the sleepable
functions bpf_ima_inode_hash() or bpf_ima_file_hash() to obtain a
measurement of a loaded IMA policy.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 tools/testing/selftests/bpf/ima_setup.sh      | 13 ++++++++++++-
 .../selftests/bpf/prog_tests/test_ima.c       | 19 +++++++++++++++++++
 tools/testing/selftests/bpf/progs/ima.c       | 18 ++++++++++++++++++
 3 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/ima_setup.sh b/tools/testing/selftests/bpf/ima_setup.sh
index a3de1cd43ba0..8ecead4ccad0 100755
--- a/tools/testing/selftests/bpf/ima_setup.sh
+++ b/tools/testing/selftests/bpf/ima_setup.sh
@@ -12,7 +12,7 @@ LOG_FILE="$(mktemp /tmp/ima_setup.XXXX.log)"
 
 usage()
 {
-	echo "Usage: $0 <setup|cleanup|run|modify-bin|restore-bin> <existing_tmp_dir>"
+	echo "Usage: $0 <setup|cleanup|run|modify-bin|restore-bin|load-policy> <existing_tmp_dir>"
 	exit 1
 }
 
@@ -51,6 +51,7 @@ setup()
 
 	ensure_mount_securityfs
 	echo "measure func=BPRM_CHECK fsuuid=${mount_uuid}" > ${IMA_POLICY_FILE}
+	echo "measure func=BPRM_CHECK fsuuid=${mount_uuid}" > ${mount_dir}/policy_test
 }
 
 cleanup() {
@@ -95,6 +96,14 @@ restore_bin()
 	truncate -s -4 "${copied_bin_path}"
 }
 
+load_policy()
+{
+	local tmp_dir="$1"
+	local mount_dir="${tmp_dir}/mnt"
+
+	echo ${mount_dir}/policy_test > ${IMA_POLICY_FILE} 2> /dev/null
+}
+
 catch()
 {
 	local exit_code="$1"
@@ -127,6 +136,8 @@ main()
 		modify_bin "${tmp_dir}"
 	elif [[ "${action}" == "restore-bin" ]]; then
 		restore_bin "${tmp_dir}"
+	elif [[ "${action}" == "load-policy" ]]; then
+		load_policy "${tmp_dir}"
 	else
 		echo "Unknown action: ${action}"
 		exit 1
diff --git a/tools/testing/selftests/bpf/prog_tests/test_ima.c b/tools/testing/selftests/bpf/prog_tests/test_ima.c
index a0cfba0b4273..b13a141c4220 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_ima.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_ima.c
@@ -58,6 +58,7 @@ static void test_init(struct ima__bss *bss)
 
 	bss->use_ima_file_hash = false;
 	bss->enable_bprm_creds_for_exec = false;
+	bss->enable_kernel_read_file = false;
 }
 
 void test_test_ima(void)
@@ -181,6 +182,24 @@ void test_test_ima(void)
 	if (CHECK(err, "restore-bin #3", "err = %d\n", err))
 		goto close_clean;
 
+	/*
+	 * Test #5
+	 * - Goal: obtain a sample from the kernel_read_file hook
+	 * - Expected result: 2 samples (./ima_setup.sh, policy_test)
+	 */
+	test_init(skel->bss);
+	skel->bss->use_ima_file_hash = true;
+	skel->bss->enable_kernel_read_file = true;
+	err = _run_measured_process(measured_dir, &skel->bss->monitored_pid,
+				    "load-policy");
+	if (CHECK(err, "run_measured_process #5", "err = %d\n", err))
+		goto close_clean;
+
+	err = ring_buffer__consume(ringbuf);
+	ASSERT_EQ(err, 2, "num_samples_or_err");
+	ASSERT_NEQ(ima_hash_from_bpf[0], 0, "ima_hash");
+	ASSERT_NEQ(ima_hash_from_bpf[1], 0, "ima_hash");
+
 close_clean:
 	snprintf(cmd, sizeof(cmd), "./ima_setup.sh cleanup %s", measured_dir);
 	err = system(cmd);
diff --git a/tools/testing/selftests/bpf/progs/ima.c b/tools/testing/selftests/bpf/progs/ima.c
index 9633e5f2453d..e3ce943c5c3d 100644
--- a/tools/testing/selftests/bpf/progs/ima.c
+++ b/tools/testing/selftests/bpf/progs/ima.c
@@ -20,6 +20,7 @@ char _license[] SEC("license") = "GPL";
 
 bool use_ima_file_hash;
 bool enable_bprm_creds_for_exec;
+bool enable_kernel_read_file;
 
 static void ima_test_common(struct file *file)
 {
@@ -65,3 +66,20 @@ int BPF_PROG(bprm_creds_for_exec, struct linux_binprm *bprm)
 	ima_test_common(bprm->file);
 	return 0;
 }
+
+SEC("lsm.s/kernel_read_file")
+int BPF_PROG(kernel_read_file, struct file *file, enum kernel_read_file_id id,
+	     bool contents)
+{
+	if (!enable_kernel_read_file)
+		return 0;
+
+	if (!contents)
+		return 0;
+
+	if (id != READING_POLICY)
+		return 0;
+
+	ima_test_common(file);
+	return 0;
+}
-- 
2.32.0

[PATCH v3 9/9] selftests/bpf: Check that bpf_kernel_read_file() denies reading IMA policy

[Roberto Sassu]

Check that bpf_kernel_read_file() denies the reading of an IMA policy, by
ensuring that ima_setup.sh exits with an error.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 .../selftests/bpf/prog_tests/test_ima.c        | 17 +++++++++++++++++
 tools/testing/selftests/bpf/progs/ima.c        | 18 ++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/test_ima.c b/tools/testing/selftests/bpf/prog_tests/test_ima.c
index b13a141c4220..b13feceb38f1 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_ima.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_ima.c
@@ -59,6 +59,7 @@ static void test_init(struct ima__bss *bss)
 	bss->use_ima_file_hash = false;
 	bss->enable_bprm_creds_for_exec = false;
 	bss->enable_kernel_read_file = false;
+	bss->test_deny = false;
 }
 
 void test_test_ima(void)
@@ -200,6 +201,22 @@ void test_test_ima(void)
 	ASSERT_NEQ(ima_hash_from_bpf[0], 0, "ima_hash");
 	ASSERT_NEQ(ima_hash_from_bpf[1], 0, "ima_hash");
 
+	/*
+	 * Test #6
+	 * - Goal: ensure that the kernel_read_file hook denies an operation
+	 * - Expected result: 0 samples
+	 */
+	test_init(skel->bss);
+	skel->bss->enable_kernel_read_file = true;
+	skel->bss->test_deny = true;
+	err = _run_measured_process(measured_dir, &skel->bss->monitored_pid,
+				    "load-policy");
+	if (CHECK(!err, "run_measured_process #6", "err = %d\n", err))
+		goto close_clean;
+
+	err = ring_buffer__consume(ringbuf);
+	ASSERT_EQ(err, 0, "num_samples_or_err");
+
 close_clean:
 	snprintf(cmd, sizeof(cmd), "./ima_setup.sh cleanup %s", measured_dir);
 	err = system(cmd);
diff --git a/tools/testing/selftests/bpf/progs/ima.c b/tools/testing/selftests/bpf/progs/ima.c
index e3ce943c5c3d..e16a2c208481 100644
--- a/tools/testing/selftests/bpf/progs/ima.c
+++ b/tools/testing/selftests/bpf/progs/ima.c
@@ -21,6 +21,7 @@ char _license[] SEC("license") = "GPL";
 bool use_ima_file_hash;
 bool enable_bprm_creds_for_exec;
 bool enable_kernel_read_file;
+bool test_deny;
 
 static void ima_test_common(struct file *file)
 {
@@ -51,6 +52,17 @@ static void ima_test_common(struct file *file)
 	return;
 }
 
+static int ima_test_deny(void)
+{
+	u32 pid;
+
+	pid = bpf_get_current_pid_tgid() >> 32;
+	if (pid == monitored_pid && test_deny)
+		return -EPERM;
+
+	return 0;
+}
+
 SEC("lsm.s/bprm_committed_creds")
 void BPF_PROG(bprm_committed_creds, struct linux_binprm *bprm)
 {
@@ -71,6 +83,8 @@ SEC("lsm.s/kernel_read_file")
 int BPF_PROG(kernel_read_file, struct file *file, enum kernel_read_file_id id,
 	     bool contents)
 {
+	int ret;
+
 	if (!enable_kernel_read_file)
 		return 0;
 
@@ -80,6 +94,10 @@ int BPF_PROG(kernel_read_file, struct file *file, enum kernel_read_file_id id,
 	if (id != READING_POLICY)
 		return 0;
 
+	ret = ima_test_deny();
+	if (ret < 0)
+		return ret;
+
 	ima_test_common(file);
 	return 0;
 }
-- 
2.32.0

Re: [PATCH v3 0/9] bpf-lsm: Extend interoperability with IMA

[Alexei Starovoitov]

On Wed, Mar 02, 2022 at 12:13:55PM +0100, Roberto Sassu wrote:
> Extend the interoperability with IMA, to give wider flexibility for the
> implementation of integrity-focused LSMs based on eBPF.
> 
> Patch 1 fixes some style issues.
> 
> Patches 2-6 give the ability to eBPF-based LSMs to take advantage of the
> measurement capability of IMA without needing to setup a policy in IMA
> (those LSMs might implement the policy capability themselves).
> 
> Patches 7-9 allow eBPF-based LSMs to evaluate files read by the kernel.
> 
> Changelog
> 
> v2:
> - Add better description to patch 1 (suggested by Shuah)
> - Recalculate digest if it is not fresh (when IMA_COLLECTED flag not set)
> - Move declaration of bpf_ima_file_hash() at the end (suggested by
>   Yonghong)
> - Add tests to check if the digest has been recalculated
> - Add deny test for bpf_kernel_read_file()
> - Add description to tests
> 
> v1:
> - Modify ima_file_hash() only and allow the usage of the function with the
>   modified behavior by eBPF-based LSMs through the new function
>   bpf_ima_file_hash() (suggested by Mimi)
> - Make bpf_lsm_kernel_read_file() sleepable so that bpf_ima_inode_hash()
>   and bpf_ima_file_hash() can be called inside the implementation of
>   eBPF-based LSMs for this hook
> 
> Roberto Sassu (9):
>   ima: Fix documentation-related warnings in ima_main.c
>   ima: Always return a file measurement in ima_file_hash()
>   bpf-lsm: Introduce new helper bpf_ima_file_hash()
>   selftests/bpf: Move sample generation code to ima_test_common()
>   selftests/bpf: Add test for bpf_ima_file_hash()
>   selftests/bpf: Check if the digest is refreshed after a file write
>   bpf-lsm: Make bpf_lsm_kernel_read_file() as sleepable
>   selftests/bpf: Add test for bpf_lsm_kernel_read_file()
>   selftests/bpf: Check that bpf_kernel_read_file() denies reading IMA
>     policy

We have to land this set through bpf-next.
Please get the Acks for patches 1 and 2, so we can proceed.

RE: [PATCH v3 0/9] bpf-lsm: Extend interoperability with IMA

[Roberto Sassu]

> From: Alexei Starovoitov [mailto:alexei.starovoitov@gmail.com]
> Sent: Wednesday, March 2, 2022 11:21 PM
> On Wed, Mar 02, 2022 at 12:13:55PM +0100, Roberto Sassu wrote:
> > Extend the interoperability with IMA, to give wider flexibility for the
> > implementation of integrity-focused LSMs based on eBPF.
> >
> > Patch 1 fixes some style issues.
> >
> > Patches 2-6 give the ability to eBPF-based LSMs to take advantage of the
> > measurement capability of IMA without needing to setup a policy in IMA
> > (those LSMs might implement the policy capability themselves).
> >
> > Patches 7-9 allow eBPF-based LSMs to evaluate files read by the kernel.
> >
> > Changelog
> >
> > v2:
> > - Add better description to patch 1 (suggested by Shuah)
> > - Recalculate digest if it is not fresh (when IMA_COLLECTED flag not set)
> > - Move declaration of bpf_ima_file_hash() at the end (suggested by
> >   Yonghong)
> > - Add tests to check if the digest has been recalculated
> > - Add deny test for bpf_kernel_read_file()
> > - Add description to tests
> >
> > v1:
> > - Modify ima_file_hash() only and allow the usage of the function with the
> >   modified behavior by eBPF-based LSMs through the new function
> >   bpf_ima_file_hash() (suggested by Mimi)
> > - Make bpf_lsm_kernel_read_file() sleepable so that bpf_ima_inode_hash()
> >   and bpf_ima_file_hash() can be called inside the implementation of
> >   eBPF-based LSMs for this hook
> >
> > Roberto Sassu (9):
> >   ima: Fix documentation-related warnings in ima_main.c
> >   ima: Always return a file measurement in ima_file_hash()
> >   bpf-lsm: Introduce new helper bpf_ima_file_hash()
> >   selftests/bpf: Move sample generation code to ima_test_common()
> >   selftests/bpf: Add test for bpf_ima_file_hash()
> >   selftests/bpf: Check if the digest is refreshed after a file write
> >   bpf-lsm: Make bpf_lsm_kernel_read_file() as sleepable
> >   selftests/bpf: Add test for bpf_lsm_kernel_read_file()
> >   selftests/bpf: Check that bpf_kernel_read_file() denies reading IMA
> >     policy
> 
> We have to land this set through bpf-next.
> Please get the Acks for patches 1 and 2, so we can proceed.

Ok. Mimi, do you have time to have a look at those patches?

Thanks

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

Re: [PATCH v3 0/9] bpf-lsm: Extend interoperability with IMA

[Mimi Zohar]

[Cc'ing Florent, Kees]

Hi Alexei,

On Wed, 2022-03-02 at 14:20 -0800, Alexei Starovoitov wrote:
> On Wed, Mar 02, 2022 at 12:13:55PM +0100, Roberto Sassu wrote:
> > Extend the interoperability with IMA, to give wider flexibility for the
> > implementation of integrity-focused LSMs based on eBPF.
> > 
> > Patch 1 fixes some style issues.
> > 
> > Patches 2-6 give the ability to eBPF-based LSMs to take advantage of the
> > measurement capability of IMA without needing to setup a policy in IMA
> > (those LSMs might implement the policy capability themselves).
> > 
> > Patches 7-9 allow eBPF-based LSMs to evaluate files read by the kernel.
> > 
> > Changelog
> > 
> > v2:
> > - Add better description to patch 1 (suggested by Shuah)
> > - Recalculate digest if it is not fresh (when IMA_COLLECTED flag not set)
> > - Move declaration of bpf_ima_file_hash() at the end (suggested by
> >   Yonghong)
> > - Add tests to check if the digest has been recalculated
> > - Add deny test for bpf_kernel_read_file()
> > - Add description to tests
> > 
> > v1:
> > - Modify ima_file_hash() only and allow the usage of the function with the
> >   modified behavior by eBPF-based LSMs through the new function
> >   bpf_ima_file_hash() (suggested by Mimi)
> > - Make bpf_lsm_kernel_read_file() sleepable so that bpf_ima_inode_hash()
> >   and bpf_ima_file_hash() can be called inside the implementation of
> >   eBPF-based LSMs for this hook
> > 
> > Roberto Sassu (9):
> >   ima: Fix documentation-related warnings in ima_main.c
> >   ima: Always return a file measurement in ima_file_hash()
> >   bpf-lsm: Introduce new helper bpf_ima_file_hash()
> >   selftests/bpf: Move sample generation code to ima_test_common()
> >   selftests/bpf: Add test for bpf_ima_file_hash()
> >   selftests/bpf: Check if the digest is refreshed after a file write
> >   bpf-lsm: Make bpf_lsm_kernel_read_file() as sleepable
> >   selftests/bpf: Add test for bpf_lsm_kernel_read_file()
> >   selftests/bpf: Check that bpf_kernel_read_file() denies reading IMA
> >     policy
> 
> We have to land this set through bpf-next.
> Please get the Acks for patches 1 and 2, so we can proceed.

Each year in the LSS integrity status update talk, I've mentioned the
eBPF integrity gaps.  I finally reached out to KP, Florent Revest, Kees
and others, letting them know that I'm concerned about the eBPF module
integrity gaps.  True there is a difference between signing the eBPF
source modules versus the eBPF generated output, but IMA could at least
verify the integrity of the source eBPF modules making sure they are
measured, the module hash audited, and are properly signed.

Before expanding the ima_file_hash() or ima_inode_hash() usage, I'd
appreciate someone adding the IMA support to measure, appraise, and
audit eBPF modules.  I realize that closing the eBPF integrity gaps is
orthogonal to this patch set, but this patch set is not only extending
the ima_file_hash()/ima_inode_hash() usage, but will be used to
circumvent IMA.  As per usual, IMA is policy based, allowing those
interested in eBPF module integrity to define IMA policy rules.

thanks,

Mimi

Re: [PATCH v3 0/9] bpf-lsm: Extend interoperability with IMA

[KP Singh]

On Thu, Mar 3, 2022 at 5:05 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> [Cc'ing Florent, Kees]
>
> Hi Alexei,
>
> On Wed, 2022-03-02 at 14:20 -0800, Alexei Starovoitov wrote:
> > On Wed, Mar 02, 2022 at 12:13:55PM +0100, Roberto Sassu wrote:
> > > Extend the interoperability with IMA, to give wider flexibility for the
> > > implementation of integrity-focused LSMs based on eBPF.
> > >
> > > Patch 1 fixes some style issues.
> > >
> > > Patches 2-6 give the ability to eBPF-based LSMs to take advantage of the
> > > measurement capability of IMA without needing to setup a policy in IMA
> > > (those LSMs might implement the policy capability themselves).
> > >
> > > Patches 7-9 allow eBPF-based LSMs to evaluate files read by the kernel.
> > >
> > > Changelog
> > >
> > > v2:
> > > - Add better description to patch 1 (suggested by Shuah)
> > > - Recalculate digest if it is not fresh (when IMA_COLLECTED flag not set)
> > > - Move declaration of bpf_ima_file_hash() at the end (suggested by
> > >   Yonghong)
> > > - Add tests to check if the digest has been recalculated
> > > - Add deny test for bpf_kernel_read_file()
> > > - Add description to tests
> > >
> > > v1:
> > > - Modify ima_file_hash() only and allow the usage of the function with the
> > >   modified behavior by eBPF-based LSMs through the new function
> > >   bpf_ima_file_hash() (suggested by Mimi)
> > > - Make bpf_lsm_kernel_read_file() sleepable so that bpf_ima_inode_hash()
> > >   and bpf_ima_file_hash() can be called inside the implementation of
> > >   eBPF-based LSMs for this hook
> > >
> > > Roberto Sassu (9):
> > >   ima: Fix documentation-related warnings in ima_main.c
> > >   ima: Always return a file measurement in ima_file_hash()
> > >   bpf-lsm: Introduce new helper bpf_ima_file_hash()
> > >   selftests/bpf: Move sample generation code to ima_test_common()
> > >   selftests/bpf: Add test for bpf_ima_file_hash()
> > >   selftests/bpf: Check if the digest is refreshed after a file write
> > >   bpf-lsm: Make bpf_lsm_kernel_read_file() as sleepable
> > >   selftests/bpf: Add test for bpf_lsm_kernel_read_file()
> > >   selftests/bpf: Check that bpf_kernel_read_file() denies reading IMA
> > >     policy
> >
> > We have to land this set through bpf-next.
> > Please get the Acks for patches 1 and 2, so we can proceed.
>

Hi Mimi,

> Each year in the LSS integrity status update talk, I've mentioned the
> eBPF integrity gaps.  I finally reached out to KP, Florent Revest, Kees

Thanks for bringing this up and it's very timely because we have been
having discussion around eBPF program signing and delineating that
from eBPF program integrity use-cases.

My plan is to travel to LSS (travel and visa permitting) and we can discuss
it more there.

If you prefer we can also discuss it before in one of the BPF office hours:

https://docs.google.com/spreadsheets/d/1LfrDXZ9-fdhvPEp_LHkxAMYyxxpwBXjywWa0AejEveU/edit#gid=0

> and others, letting them know that I'm concerned about the eBPF module
> integrity gaps.  True there is a difference between signing the eBPF
> source modules versus the eBPF generated output, but IMA could at least
> verify the integrity of the source eBPF modules making sure they are
> measured, the module hash audited, and are properly signed.
>
> Before expanding the ima_file_hash() or ima_inode_hash() usage, I'd
> appreciate someone adding the IMA support to measure, appraise, and
> audit eBPF modules.  I realize that closing the eBPF integrity gaps is
> orthogonal to this patch set, but this patch set is not only extending

This really is orthogonal and IMHO it does not seem rational to block this
patchset on it.

> the ima_file_hash()/ima_inode_hash() usage, but will be used to
> circumvent IMA.  As per usual, IMA is policy based, allowing those

I don't think they are being used to circumvent IMA but for totally
different use-cases (e.g. as a data point for detecting attacks).

- KP

> interested in eBPF module integrity to define IMA policy rules.
>
> thanks,
>
> Mimi
>

Re: [PATCH v3 0/9] bpf-lsm: Extend interoperability with IMA

[Mimi Zohar]

On Thu, 2022-03-03 at 17:17 +0100, KP Singh wrote:
> On Thu, Mar 3, 2022 at 5:05 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > [Cc'ing Florent, Kees]
> >
> > Hi Alexei,
> >
> > On Wed, 2022-03-02 at 14:20 -0800, Alexei Starovoitov wrote:
> > > On Wed, Mar 02, 2022 at 12:13:55PM +0100, Roberto Sassu wrote:
> > > > Extend the interoperability with IMA, to give wider flexibility for the
> > > > implementation of integrity-focused LSMs based on eBPF.
> > > >
> > > > Patch 1 fixes some style issues.
> > > >
> > > > Patches 2-6 give the ability to eBPF-based LSMs to take advantage of the
> > > > measurement capability of IMA without needing to setup a policy in IMA
> > > > (those LSMs might implement the policy capability themselves).
> > > >
> > > > Patches 7-9 allow eBPF-based LSMs to evaluate files read by the kernel.
> > > >
> > > > Changelog
> > > >
> > > > v2:
> > > > - Add better description to patch 1 (suggested by Shuah)
> > > > - Recalculate digest if it is not fresh (when IMA_COLLECTED flag not set)
> > > > - Move declaration of bpf_ima_file_hash() at the end (suggested by
> > > >   Yonghong)
> > > > - Add tests to check if the digest has been recalculated
> > > > - Add deny test for bpf_kernel_read_file()
> > > > - Add description to tests
> > > >
> > > > v1:
> > > > - Modify ima_file_hash() only and allow the usage of the function with the
> > > >   modified behavior by eBPF-based LSMs through the new function
> > > >   bpf_ima_file_hash() (suggested by Mimi)
> > > > - Make bpf_lsm_kernel_read_file() sleepable so that bpf_ima_inode_hash()
> > > >   and bpf_ima_file_hash() can be called inside the implementation of
> > > >   eBPF-based LSMs for this hook
> > > >
> > > > Roberto Sassu (9):
> > > >   ima: Fix documentation-related warnings in ima_main.c
> > > >   ima: Always return a file measurement in ima_file_hash()
> > > >   bpf-lsm: Introduce new helper bpf_ima_file_hash()
> > > >   selftests/bpf: Move sample generation code to ima_test_common()
> > > >   selftests/bpf: Add test for bpf_ima_file_hash()
> > > >   selftests/bpf: Check if the digest is refreshed after a file write
> > > >   bpf-lsm: Make bpf_lsm_kernel_read_file() as sleepable
> > > >   selftests/bpf: Add test for bpf_lsm_kernel_read_file()
> > > >   selftests/bpf: Check that bpf_kernel_read_file() denies reading IMA
> > > >     policy
> > >
> > > We have to land this set through bpf-next.
> > > Please get the Acks for patches 1 and 2, so we can proceed.
> >
> 
> Hi Mimi,
> 
> > Each year in the LSS integrity status update talk, I've mentioned the
> > eBPF integrity gaps.  I finally reached out to KP, Florent Revest, Kees
> 
> Thanks for bringing this up and it's very timely because we have been
> having discussion around eBPF program signing and delineating that
> from eBPF program integrity use-cases.
> 
> My plan is to travel to LSS (travel and visa permitting) and we can discuss
> it more there.
> 
> If you prefer we can also discuss it before in one of the BPF office hours:
> 
> https://docs.google.com/spreadsheets/d/1LfrDXZ9-fdhvPEp_LHkxAMYyxxpwBXjywWa0AejEveU/edit#gid=0

Sounds good.

> 
> > and others, letting them know that I'm concerned about the eBPF module
> > integrity gaps.  True there is a difference between signing the eBPF
> > source modules versus the eBPF generated output, but IMA could at least
> > verify the integrity of the source eBPF modules making sure they are
> > measured, the module hash audited, and are properly signed.
> >
> > Before expanding the ima_file_hash() or ima_inode_hash() usage, I'd
> > appreciate someone adding the IMA support to measure, appraise, and
> > audit eBPF modules.  I realize that closing the eBPF integrity gaps is
> > orthogonal to this patch set, but this patch set is not only extending
> 
> This really is orthogonal and IMHO it does not seem rational to block this
> patchset on it.
> 
> > the ima_file_hash()/ima_inode_hash() usage, but will be used to
> > circumvent IMA.  As per usual, IMA is policy based, allowing those
> 
> I don't think they are being used to circumvent IMA but for totally
> different use-cases (e.g. as a data point for detecting attacks).
> 
> 
> > interested in eBPF module integrity to define IMA policy rules.

That might be true for your usecase, but not Roberto's.  From the cover
letter above, Roberto was honest in saying:

Patches 2-6 give the ability to eBPF-based LSMs to take advantage of
the measurement capability of IMA without needing to setup a policy in
IMA (those LSMs might implement the policy capability themselves).

-- 
thanks,

Mimi

Re: [PATCH v3 0/9] bpf-lsm: Extend interoperability with IMA

[KP Singh]

On Thu, Mar 3, 2022 at 5:30 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Thu, 2022-03-03 at 17:17 +0100, KP Singh wrote:
> > On Thu, Mar 3, 2022 at 5:05 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > >
> > > [Cc'ing Florent, Kees]
> > >
> > > Hi Alexei,
> > >
> > > On Wed, 2022-03-02 at 14:20 -0800, Alexei Starovoitov wrote:
> > > > On Wed, Mar 02, 2022 at 12:13:55PM +0100, Roberto Sassu wrote:
> > > > > Extend the interoperability with IMA, to give wider flexibility for the
> > > > > implementation of integrity-focused LSMs based on eBPF.
> > > > >
> > > > > Patch 1 fixes some style issues.
> > > > >
> > > > > Patches 2-6 give the ability to eBPF-based LSMs to take advantage of the
> > > > > measurement capability of IMA without needing to setup a policy in IMA
> > > > > (those LSMs might implement the policy capability themselves).
> > > > >
> > > > > Patches 7-9 allow eBPF-based LSMs to evaluate files read by the kernel.
> > > > >
> > > > > Changelog
> > > > >
> > > > > v2:
> > > > > - Add better description to patch 1 (suggested by Shuah)
> > > > > - Recalculate digest if it is not fresh (when IMA_COLLECTED flag not set)
> > > > > - Move declaration of bpf_ima_file_hash() at the end (suggested by
> > > > >   Yonghong)
> > > > > - Add tests to check if the digest has been recalculated
> > > > > - Add deny test for bpf_kernel_read_file()
> > > > > - Add description to tests
> > > > >
> > > > > v1:
> > > > > - Modify ima_file_hash() only and allow the usage of the function with the
> > > > >   modified behavior by eBPF-based LSMs through the new function
> > > > >   bpf_ima_file_hash() (suggested by Mimi)
> > > > > - Make bpf_lsm_kernel_read_file() sleepable so that bpf_ima_inode_hash()
> > > > >   and bpf_ima_file_hash() can be called inside the implementation of
> > > > >   eBPF-based LSMs for this hook
> > > > >
> > > > > Roberto Sassu (9):
> > > > >   ima: Fix documentation-related warnings in ima_main.c
> > > > >   ima: Always return a file measurement in ima_file_hash()
> > > > >   bpf-lsm: Introduce new helper bpf_ima_file_hash()
> > > > >   selftests/bpf: Move sample generation code to ima_test_common()
> > > > >   selftests/bpf: Add test for bpf_ima_file_hash()
> > > > >   selftests/bpf: Check if the digest is refreshed after a file write
> > > > >   bpf-lsm: Make bpf_lsm_kernel_read_file() as sleepable
> > > > >   selftests/bpf: Add test for bpf_lsm_kernel_read_file()
> > > > >   selftests/bpf: Check that bpf_kernel_read_file() denies reading IMA
> > > > >     policy
> > > >
> > > > We have to land this set through bpf-next.
> > > > Please get the Acks for patches 1 and 2, so we can proceed.
> > >
> >
> > Hi Mimi,
> >
> > > Each year in the LSS integrity status update talk, I've mentioned the
> > > eBPF integrity gaps.  I finally reached out to KP, Florent Revest, Kees
> >
> > Thanks for bringing this up and it's very timely because we have been
> > having discussion around eBPF program signing and delineating that
> > from eBPF program integrity use-cases.
> >
> > My plan is to travel to LSS (travel and visa permitting) and we can discuss
> > it more there.
> >
> > If you prefer we can also discuss it before in one of the BPF office hours:
> >
> > https://docs.google.com/spreadsheets/d/1LfrDXZ9-fdhvPEp_LHkxAMYyxxpwBXjywWa0AejEveU/edit#gid=0
>
> Sounds good.
>
> >
> > > and others, letting them know that I'm concerned about the eBPF module
> > > integrity gaps.  True there is a difference between signing the eBPF
> > > source modules versus the eBPF generated output, but IMA could at least
> > > verify the integrity of the source eBPF modules making sure they are
> > > measured, the module hash audited, and are properly signed.
> > >
> > > Before expanding the ima_file_hash() or ima_inode_hash() usage, I'd
> > > appreciate someone adding the IMA support to measure, appraise, and
> > > audit eBPF modules.  I realize that closing the eBPF integrity gaps is
> > > orthogonal to this patch set, but this patch set is not only extending
> >
> > This really is orthogonal and IMHO it does not seem rational to block this
> > patchset on it.
> >
> > > the ima_file_hash()/ima_inode_hash() usage, but will be used to
> > > circumvent IMA.  As per usual, IMA is policy based, allowing those
> >
> > I don't think they are being used to circumvent IMA but for totally
> > different use-cases (e.g. as a data point for detecting attacks).
> >
> >
> > > interested in eBPF module integrity to define IMA policy rules.
>
> That might be true for your usecase, but not Roberto's.  From the cover
> letter above, Roberto was honest in saying:
>
> Patches 2-6 give the ability to eBPF-based LSMs to take advantage of
> the measurement capability of IMA without needing to setup a policy in
> IMA (those LSMs might implement the policy capability themselves).

Currently to use the helper bpf_ima_inode_hash in LSM progs
one needs to have a basic IMA policy in place just to get a hash,
even if one does not need to use the whole feature-set provided by IMA.
These patches would be quite beneficial for this user-journey.

Even Robert's use case is to implement IMA policies in BPF this is still
fundamentally different from IMA doing integrity measurement for BPF
and blocking this patch-set on the latter does not seem rational and
I don't see how implementing integrity for BPF would avoid your
concerns.



>
> --
> thanks,
>
> Mimi
>

Re: [PATCH v3 0/9] bpf-lsm: Extend interoperability with IMA

[Mimi Zohar]

On Thu, 2022-03-03 at 19:14 +0100, KP Singh wrote:
> 
> Even Robert's use case is to implement IMA policies in BPF this is still
> fundamentally different from IMA doing integrity measurement for BPF
> and blocking this patch-set on the latter does not seem rational and
> I don't see how implementing integrity for BPF would avoid your
> concerns.

eBPF modules are an entire class of files currently not being measured,
audited, or appraised.  This is an integrity gap that needs to be
closed.  The purpose would be to at least measure and verify the
integrity of the eBPF module that is going to be used in lieu of
traditional IMA.

-- 
thanks,

Mimi

Re: [PATCH v3 0/9] bpf-lsm: Extend interoperability with IMA

[Alexei Starovoitov]

On Thu, Mar 3, 2022 at 11:13 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Thu, 2022-03-03 at 19:14 +0100, KP Singh wrote:
> >
> > Even Robert's use case is to implement IMA policies in BPF this is still
> > fundamentally different from IMA doing integrity measurement for BPF
> > and blocking this patch-set on the latter does not seem rational and
> > I don't see how implementing integrity for BPF would avoid your
> > concerns.
>
> eBPF modules are an entire class of files currently not being measured,
> audited, or appraised.  This is an integrity gap that needs to be
> closed.  The purpose would be to at least measure and verify the
> integrity of the eBPF module that is going to be used in lieu of
> traditional IMA.

Mimi,

. There is no such thing as "eBPF modules". There are BPF programs.
They cannot be signed the same way as kernel modules.
We've been working on providing a way to sign them for more
than a year now. That work is still ongoing.

. IMA cannot be used for integrity check of BPF programs for the same
reasons why kernel module like signing cannot be used.

. This patch set is orthogonal.

Re: [PATCH v3 0/9] bpf-lsm: Extend interoperability with IMA

[Mimi Zohar]

On Thu, 2022-03-03 at 14:39 -0800, Alexei Starovoitov wrote:

> . There is no such thing as "eBPF modules". There are BPF programs.
> They cannot be signed the same way as kernel modules.
> We've been working on providing a way to sign them for more
> than a year now. That work is still ongoing.
> 
> . IMA cannot be used for integrity check of BPF programs for the same
> reasons why kernel module like signing cannot be used.

I assume the issue isn't where the signature is stored (e.g. appended,
xattr), but of calculating the hash.  Where is the discussion taking
place?   Are there any summaries of what has been discussed?

FYI, IMA isn't limited to measuring files.  Support was added for
buffer measurements (e.g kexec boot command line, certificates) and
measuring kernel critical data (e.g. SELinux in memory policy & state,
device mapper).

thanks,

Mimi

Re: [PATCH v3 0/9] bpf-lsm: Extend interoperability with IMA

[KP Singh]

On Mon, Mar 7, 2022 at 3:57 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Thu, 2022-03-03 at 14:39 -0800, Alexei Starovoitov wrote:
>
> > . There is no such thing as "eBPF modules". There are BPF programs.
> > They cannot be signed the same way as kernel modules.
> > We've been working on providing a way to sign them for more
> > than a year now. That work is still ongoing.
> >
> > . IMA cannot be used for integrity check of BPF programs for the same
> > reasons why kernel module like signing cannot be used.
>
> I assume the issue isn't where the signature is stored (e.g. appended,
> xattr), but of calculating the hash.  Where is the discussion taking

This has the relevant background: https://lwn.net/Articles/853489/

We had some more discussions in one of our BSC meeting:

https://github.com/ebpf-io/bsc/blob/master/minutes.md

and we expect the discussions to continue over conferences this year
 (e.g. LSF/MM/BPF, Linux Plumbers). As I mentioned on another thread
we don't have to wait for conferences and we can discuss this in the BPF
office hours. Please feel free to add an agenda at:

https://docs.google.com/spreadsheets/d/1LfrDXZ9-fdhvPEp_LHkxAMYyxxpwBXjywWa0AejEveU/edit#gid=0

(best is to give some notice so that interested folks can join).

> place?   Are there any summaries of what has been discussed?
>
> FYI, IMA isn't limited to measuring files.  Support was added for
> buffer measurements (e.g kexec boot command line, certificates) and
> measuring kernel critical data (e.g. SELinux in memory policy & state,
> device mapper).

Nice. I need to look at how this is implemented.

- KP

>
> thanks,
>
> Mimi
>

Re: [PATCH v3 0/9] bpf-lsm: Extend interoperability with IMA

[Mimi Zohar]

On Mon, 2022-03-07 at 14:17 +0100, KP Singh wrote:
> On Mon, Mar 7, 2022 at 3:57 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Thu, 2022-03-03 at 14:39 -0800, Alexei Starovoitov wrote:
> >
> > > . There is no such thing as "eBPF modules". There are BPF programs.
> > > They cannot be signed the same way as kernel modules.
> > > We've been working on providing a way to sign them for more
> > > than a year now. That work is still ongoing.
> > >
> > > . IMA cannot be used for integrity check of BPF programs for the same
> > > reasons why kernel module like signing cannot be used.
> >
> > I assume the issue isn't where the signature is stored (e.g. appended,
> > xattr), but of calculating the hash.  Where is the discussion taking
> 
> This has the relevant background: https://lwn.net/Articles/853489/

Thanks, Jon!

> 
> We had some more discussions in one of our BSC meeting:
> 
> https://github.com/ebpf-io/bsc/blob/master/minutes.md
> 
> and we expect the discussions to continue over conferences this year
>  (e.g. LSF/MM/BPF, Linux Plumbers). As I mentioned on another thread
> we don't have to wait for conferences and we can discuss this in the BPF
> office hours. Please feel free to add an agenda at:
> 
> https://docs.google.com/spreadsheets/d/1LfrDXZ9-fdhvPEp_LHkxAMYyxxpwBXjywWa0AejEveU/edit#gid=0
> 
> (best is to give some notice so that interested folks can join).

Right, but probably a good idea to understand the issues at least at a
high level, before a meeting.

> 
> > place?   Are there any summaries of what has been discussed?
> >
> > FYI, IMA isn't limited to measuring files.  Support was added for
> > buffer measurements (e.g kexec boot command line, certificates) and
> > measuring kernel critical data (e.g. SELinux in memory policy & state,
> > device mapper).
> 
> Nice. I need to look at how this is implemented.

ima_measure_critical_data() is of kernel state info, so signature
verification is currently not needed or supported, only measurement.

thanks,

Mimi

Re: [PATCH v3 0/9] bpf-lsm: Extend interoperability with IMA

[Mimi Zohar]

On Wed, 2022-03-02 at 12:13 +0100, Roberto Sassu wrote:
> Extend the interoperability with IMA, to give wider flexibility for the
> implementation of integrity-focused LSMs based on eBPF.
> 
> Patch 1 fixes some style issues.
> 
> Patches 2-6 give the ability to eBPF-based LSMs to take advantage of the
> measurement capability of IMA without needing to setup a policy in IMA
> (those LSMs might implement the policy capability themselves).
> 
> Patches 7-9 allow eBPF-based LSMs to evaluate files read by the kernel.

The tests seem to only work when neither a builtin IMA policy or a
custom policy is previously loaded.

thanks,

Mimi

RE: [PATCH v3 0/9] bpf-lsm: Extend interoperability with IMA

[Roberto Sassu]

> From: Mimi Zohar [mailto:zohar@linux.ibm.com]
> Sent: Sunday, March 6, 2022 8:24 PM
> On Wed, 2022-03-02 at 12:13 +0100, Roberto Sassu wrote:
> > Extend the interoperability with IMA, to give wider flexibility for the
> > implementation of integrity-focused LSMs based on eBPF.
> >
> > Patch 1 fixes some style issues.
> >
> > Patches 2-6 give the ability to eBPF-based LSMs to take advantage of the
> > measurement capability of IMA without needing to setup a policy in IMA
> > (those LSMs might implement the policy capability themselves).
> >
> > Patches 7-9 allow eBPF-based LSMs to evaluate files read by the kernel.
> 
> The tests seem to only work when neither a builtin IMA policy or a
> custom policy is previously loaded.

Hi Mimi

unfortunately yes. If there are more generic rules,
the number of samples differs from that expected.

For example, if you have an existing rule like:

measure func=BPRM_CHECK mask=MAY_EXEC

you will have:

test_test_ima:PASS:run_measured_process #1 0 nsec
test_test_ima:FAIL:num_samples_or_err unexpected
                num_samples_or_err: actual 2 != expected 1

Test #1 fails because also ima_setup.sh is measured.

Thanks

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

Re: [PATCH v3 0/9] bpf-lsm: Extend interoperability with IMA

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Wed, 2 Mar 2022 12:13:55 +0100 you wrote:
> Extend the interoperability with IMA, to give wider flexibility for the
> implementation of integrity-focused LSMs based on eBPF.
> 
> Patch 1 fixes some style issues.
> 
> Patches 2-6 give the ability to eBPF-based LSMs to take advantage of the
> measurement capability of IMA without needing to setup a policy in IMA
> (those LSMs might implement the policy capability themselves).
> 
> [...]

Here is the summary with links:
  - [v3,1/9] ima: Fix documentation-related warnings in ima_main.c
    https://git.kernel.org/bpf/bpf-next/c/bae60eefb95c
  - [v3,2/9] ima: Always return a file measurement in ima_file_hash()
    https://git.kernel.org/bpf/bpf-next/c/280fe8367b0d
  - [v3,3/9] bpf-lsm: Introduce new helper bpf_ima_file_hash()
    https://git.kernel.org/bpf/bpf-next/c/174b16946e39
  - [v3,4/9] selftests/bpf: Move sample generation code to ima_test_common()
    https://git.kernel.org/bpf/bpf-next/c/2746de3c53d6
  - [v3,5/9] selftests/bpf: Add test for bpf_ima_file_hash()
    https://git.kernel.org/bpf/bpf-next/c/27a77d0d460c
  - [v3,6/9] selftests/bpf: Check if the digest is refreshed after a file write
    https://git.kernel.org/bpf/bpf-next/c/91e8fa254dbd
  - [v3,7/9] bpf-lsm: Make bpf_lsm_kernel_read_file() as sleepable
    https://git.kernel.org/bpf/bpf-next/c/df6b3039fa11
  - [v3,8/9] selftests/bpf: Add test for bpf_lsm_kernel_read_file()
    https://git.kernel.org/bpf/bpf-next/c/e6dcf7bbf37c
  - [v3,9/9] selftests/bpf: Check that bpf_kernel_read_file() denies reading IMA policy
    https://git.kernel.org/bpf/bpf-next/c/7bae42b68d7f

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html