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-4 gives 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 5-6 allows eBPF-based LSMs to evaluate files read by the kernel.
Changelog
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 (6): 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: Add test for bpf_ima_file_hash() bpf-lsm: Make bpf_lsm_kernel_read_file() as sleepable selftests/bpf: Add test for bpf_lsm_kernel_read_file()
include/uapi/linux/bpf.h | 11 +++++ kernel/bpf/bpf_lsm.c | 21 +++++++++ security/integrity/ima/ima_main.c | 47 ++++++++++++------- tools/include/uapi/linux/bpf.h | 11 +++++ tools/testing/selftests/bpf/ima_setup.sh | 2 + .../selftests/bpf/prog_tests/test_ima.c | 30 ++++++++++-- tools/testing/selftests/bpf/progs/ima.c | 34 ++++++++++++-- 7 files changed, 132 insertions(+), 24 deletions(-)
Fix some warnings in ima_main.c, displayed with W=n make argument.
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.
On 2/15/22 5:40 AM, Roberto Sassu wrote:
Fix some warnings in ima_main.c, displayed with W=n make argument.
Thank you for fixing these. Doc builds are full of them and few less is welcome.
Adding the warns or summary of them to change log will be good.
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.
Reviewed-by: Shuah Khan skhan@linuxfoundation.org
thanks, -- Shuah
From: Shuah Khan [mailto:skhan@linuxfoundation.org] Sent: Tuesday, February 15, 2022 4:46 PM On 2/15/22 5:40 AM, Roberto Sassu wrote:
Fix some warnings in ima_main.c, displayed with W=n make argument.
Thank you for fixing these. Doc builds are full of them and few less is welcome.
Adding the warns or summary of them to change log will be good.
Hi Shuah
ok, I will add a brief description of what I fixed in the next version of the patch set.
Thanks
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Zhong Ronghua
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.
Reviewed-by: Shuah Khan skhan@linuxfoundation.org
thanks, -- Shuah
__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.
Call ima_collect_measurement() in __ima_inode_hash(), if the file descriptor is available (passed by ima_file_hash()), 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 | 36 +++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 11 deletions(-)
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 946ba8a12eab..3562a212a5ba 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -520,15 +520,27 @@ 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 && file) { + memset(&tmp_iint, 0, sizeof(tmp_iint)); + tmp_iint.inode = inode; + + rc = ima_collect_measurement(&tmp_iint, file, NULL, 0, + ima_hash_algo, NULL); + if (rc < 0) + return -EOPNOTSUPP; + + iint = &tmp_iint; + }
- iint = integrity_iint_find(inode); if (!iint) return -EOPNOTSUPP;
@@ -552,12 +564,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 +584,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 +592,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 +619,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);
ima_file_hash() has been modified to calculate the measurement of a file on demand, if it has not been already performed by IMA. 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..ba33d5718d6b 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -4648,6 +4648,16 @@ union bpf_attr { * **-EOPNOTSUP** if IMA is disabled or **-EINVAL** if * invalid arguments are passed. * + * 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. + * * struct socket *bpf_sock_from_file(struct file *file) * Description * If the given file represents a socket, returns the associated @@ -5182,6 +5192,7 @@ union bpf_attr { FN(bprm_opts_set), \ FN(ktime_get_coarse_ns), \ FN(ima_inode_hash), \ + FN(ima_file_hash), \ FN(sock_from_file), \ FN(check_mtu), \ FN(for_each_map_elem), \ 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..ba33d5718d6b 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -4648,6 +4648,16 @@ union bpf_attr { * **-EOPNOTSUP** if IMA is disabled or **-EINVAL** if * invalid arguments are passed. * + * 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. + * * struct socket *bpf_sock_from_file(struct file *file) * Description * If the given file represents a socket, returns the associated @@ -5182,6 +5192,7 @@ union bpf_attr { FN(bprm_opts_set), \ FN(ktime_get_coarse_ns), \ FN(ima_inode_hash), \ + FN(ima_file_hash), \ FN(sock_from_file), \ FN(check_mtu), \ FN(for_each_map_elem), \
On 2/15/22 4:40 AM, Roberto Sassu wrote:
ima_file_hash() has been modified to calculate the measurement of a file on demand, if it has not been already performed by IMA. 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..ba33d5718d6b 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -4648,6 +4648,16 @@ union bpf_attr {
**-EOPNOTSUP** if IMA is disabled or **-EINVAL** ifinvalid arguments are passed.
- 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.
- struct socket *bpf_sock_from_file(struct file *file)
- Description
If the given file represents a socket, returns the associated@@ -5182,6 +5192,7 @@ union bpf_attr { FN(bprm_opts_set), \ FN(ktime_get_coarse_ns), \ FN(ima_inode_hash), \
- FN(ima_file_hash), \
Please put the above FN(ima_file_hash) to the end of the list. Otherwise, we have a backward compatability issue.
FN(sock_from_file), \ FN(check_mtu), \ FN(for_each_map_elem), \ 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
[...]
From: Yonghong Song [mailto:yhs@fb.com] Sent: Tuesday, February 15, 2022 6:03 PM On 2/15/22 4:40 AM, Roberto Sassu wrote:
ima_file_hash() has been modified to calculate the measurement of a file on demand, if it has not been already performed by IMA. 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..ba33d5718d6b 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -4648,6 +4648,16 @@ union bpf_attr {
**-EOPNOTSUP** if IMA is disabled or **-EINVAL** ifinvalid arguments are passed.
- 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.
- struct socket *bpf_sock_from_file(struct file *file)
- Description
If the given file represents a socket, returns the associated@@ -5182,6 +5192,7 @@ union bpf_attr { FN(bprm_opts_set), \ FN(ktime_get_coarse_ns), \ FN(ima_inode_hash), \
- FN(ima_file_hash), \
Please put the above FN(ima_file_hash) to the end of the list. Otherwise, we have a backward compatability issue.
Hi Yonghong
sure, will do.
Thanks
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Zhong Ronghua
FN(sock_from_file), \ FN(check_mtu), \ FN(for_each_map_elem), \ 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
[...]
Modify the existing IMA test to call bpf_ima_file_hash() and update the expected result accordingly.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- .../selftests/bpf/prog_tests/test_ima.c | 29 ++++++++++++++++--- tools/testing/selftests/bpf/progs/ima.c | 10 +++++-- 2 files changed, 33 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..62bf0e830453 100644 --- a/tools/testing/selftests/bpf/prog_tests/test_ima.c +++ b/tools/testing/selftests/bpf/prog_tests/test_ima.c @@ -13,9 +13,10 @@
#include "ima.skel.h"
-static int run_measured_process(const char *measured_dir, u32 *monitored_pid) +static int run_measured_process(const char *measured_dir, u32 *monitored_pid, + bool *use_ima_file_hash) { - int child_pid, child_status; + int err, child_pid, child_status;
child_pid = fork(); if (child_pid == 0) { @@ -24,6 +25,21 @@ static int run_measured_process(const char *measured_dir, u32 *monitored_pid) NULL); exit(errno);
+ } else if (child_pid > 0) { + waitpid(child_pid, &child_status, 0); + err = WEXITSTATUS(child_status); + if (err) + return err; + } + + child_pid = fork(); + if (child_pid == 0) { + *monitored_pid = getpid(); + *use_ima_file_hash = true; + execlp("./ima_setup.sh", "./ima_setup.sh", "run", measured_dir, + NULL); + exit(errno); + } else if (child_pid > 0) { waitpid(child_pid, &child_status, 0); return WEXITSTATUS(child_status); @@ -72,12 +88,17 @@ void test_test_ima(void) if (CHECK(err, "failed to run command", "%s, errno = %d\n", cmd, errno)) goto close_clean;
- err = run_measured_process(measured_dir, &skel->bss->monitored_pid); + err = run_measured_process(measured_dir, &skel->bss->monitored_pid, + &skel->bss->use_ima_file_hash); if (CHECK(err, "run_measured_process", "err = %d\n", err)) goto close_clean;
err = ring_buffer__consume(ringbuf); - ASSERT_EQ(err, 1, "num_samples_or_err"); + /* + * 1 sample with use_ima_file_hash = false + * 2 samples with use_ima_file_hash = true (./ima_setup.sh, /bin/true) + */ + ASSERT_EQ(err, 3, "num_samples_or_err"); ASSERT_NEQ(ima_hash_from_bpf, 0, "ima_hash");
close_clean: diff --git a/tools/testing/selftests/bpf/progs/ima.c b/tools/testing/selftests/bpf/progs/ima.c index 96060ff4ffc6..9bb63f96cfc0 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; + SEC("lsm.s/bprm_committed_creds") void BPF_PROG(ima, struct linux_binprm *bprm) { @@ -28,8 +30,12 @@ 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, - sizeof(ima_hash)); + if (!use_ima_file_hash) + ret = bpf_ima_inode_hash(bprm->file->f_inode, &ima_hash, + sizeof(ima_hash)); + else + ret = bpf_ima_file_hash(bprm->file, &ima_hash, + sizeof(ima_hash)); if (ret < 0 || ima_hash == 0) return;
On 2/15/22 5:40 AM, Roberto Sassu wrote:
Modify the existing IMA test to call bpf_ima_file_hash() and update the expected result accordingly.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com
.../selftests/bpf/prog_tests/test_ima.c | 29 ++++++++++++++++--- tools/testing/selftests/bpf/progs/ima.c | 10 +++++-- 2 files changed, 33 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..62bf0e830453 100644 --- a/tools/testing/selftests/bpf/prog_tests/test_ima.c +++ b/tools/testing/selftests/bpf/prog_tests/test_ima.c @@ -13,9 +13,10 @@ #include "ima.skel.h" -static int run_measured_process(const char *measured_dir, u32 *monitored_pid) +static int run_measured_process(const char *measured_dir, u32 *monitored_pid,
bool *use_ima_file_hash)
- int child_pid, child_status;
- int err, child_pid, child_status;
child_pid = fork(); if (child_pid == 0) { @@ -24,6 +25,21 @@ static int run_measured_process(const char *measured_dir, u32 *monitored_pid) NULL); exit(errno);
- } else if (child_pid > 0) {
waitpid(child_pid, &child_status, 0);err = WEXITSTATUS(child_status);if (err)return err;- }
- child_pid = fork();
- if (child_pid == 0) {
*monitored_pid = getpid();*use_ima_file_hash = true;execlp("./ima_setup.sh", "./ima_setup.sh", "run", measured_dir,NULL);exit(errno);- } else if (child_pid > 0) { waitpid(child_pid, &child_status, 0); return WEXITSTATUS(child_status);
@@ -72,12 +88,17 @@ void test_test_ima(void) if (CHECK(err, "failed to run command", "%s, errno = %d\n", cmd, errno)) goto close_clean;
- err = run_measured_process(measured_dir, &skel->bss->monitored_pid);
- err = run_measured_process(measured_dir, &skel->bss->monitored_pid,
&skel->bss->use_ima_file_hash);err = ring_buffer__consume(ringbuf);
- ASSERT_EQ(err, 1, "num_samples_or_err");
- /*
* 1 sample with use_ima_file_hash = false* 2 samples with use_ima_file_hash = true (./ima_setup.sh, /bin/true)*/- ASSERT_EQ(err, 3, "num_samples_or_err"); ASSERT_NEQ(ima_hash_from_bpf, 0, "ima_hash");
close_clean: diff --git a/tools/testing/selftests/bpf/progs/ima.c b/tools/testing/selftests/bpf/progs/ima.c index 96060ff4ffc6..9bb63f96cfc0 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;
This can be statis.
SEC("lsm.s/bprm_committed_creds") void BPF_PROG(ima, struct linux_binprm *bprm) { @@ -28,8 +30,12 @@ void BPF_PROG(ima, struct linux_binprm *bprm) pid = bpf_get_current_pid_tgid() >> 32; if (pid == monitored_pid) {
I also noticed monitored_pid is defined in several bpf. Potentially could be made static. This isn't introduced in this patch though.
ret = bpf_ima_inode_hash(bprm->file->f_inode, &ima_hash,sizeof(ima_hash));
if (!use_ima_file_hash)ret = bpf_ima_inode_hash(bprm->file->f_inode, &ima_hash,sizeof(ima_hash));elseret = bpf_ima_file_hash(bprm->file, &ima_hash,sizeof(ima_hash));
thanks, -- Shuah
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
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 | 2 ++ .../selftests/bpf/prog_tests/test_ima.c | 3 +- tools/testing/selftests/bpf/progs/ima.c | 28 ++++++++++++++++--- 3 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/bpf/ima_setup.sh b/tools/testing/selftests/bpf/ima_setup.sh index 8e62581113a3..82530f19f85a 100755 --- a/tools/testing/selftests/bpf/ima_setup.sh +++ b/tools/testing/selftests/bpf/ima_setup.sh @@ -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() { @@ -74,6 +75,7 @@ run() local mount_dir="${tmp_dir}/mnt" local copied_bin_path="${mount_dir}/$(basename ${TEST_BINARY})"
+ echo ${mount_dir}/policy_test > ${IMA_POLICY_FILE} exec "${copied_bin_path}" }
diff --git a/tools/testing/selftests/bpf/prog_tests/test_ima.c b/tools/testing/selftests/bpf/prog_tests/test_ima.c index 62bf0e830453..c4a62d7b70df 100644 --- a/tools/testing/selftests/bpf/prog_tests/test_ima.c +++ b/tools/testing/selftests/bpf/prog_tests/test_ima.c @@ -97,8 +97,9 @@ void test_test_ima(void) /* * 1 sample with use_ima_file_hash = false * 2 samples with use_ima_file_hash = true (./ima_setup.sh, /bin/true) + * 1 sample with use_ima_file_hash = true (IMA policy) */ - ASSERT_EQ(err, 3, "num_samples_or_err"); + ASSERT_EQ(err, 4, "num_samples_or_err"); ASSERT_NEQ(ima_hash_from_bpf, 0, "ima_hash");
close_clean: diff --git a/tools/testing/selftests/bpf/progs/ima.c b/tools/testing/selftests/bpf/progs/ima.c index 9bb63f96cfc0..9b4c03f30a1c 100644 --- a/tools/testing/selftests/bpf/progs/ima.c +++ b/tools/testing/selftests/bpf/progs/ima.c @@ -20,8 +20,7 @@ char _license[] SEC("license") = "GPL";
bool use_ima_file_hash;
-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; @@ -31,10 +30,10 @@ void BPF_PROG(ima, struct linux_binprm *bprm) pid = bpf_get_current_pid_tgid() >> 32; if (pid == monitored_pid) { if (!use_ima_file_hash) - ret = bpf_ima_inode_hash(bprm->file->f_inode, &ima_hash, + ret = bpf_ima_inode_hash(file->f_inode, &ima_hash, sizeof(ima_hash)); else - ret = bpf_ima_file_hash(bprm->file, &ima_hash, + ret = bpf_ima_file_hash(file, &ima_hash, sizeof(ima_hash)); if (ret < 0 || ima_hash == 0) return; @@ -49,3 +48,24 @@ void BPF_PROG(ima, struct linux_binprm *bprm)
return; } + +SEC("lsm.s/bprm_committed_creds") +void BPF_PROG(ima, struct linux_binprm *bprm) +{ + ima_test_common(bprm->file); +} + +SEC("lsm.s/kernel_read_file") +int BPF_PROG(kernel_read_file, struct file *file, enum kernel_read_file_id id, + bool contents) +{ + if (!contents) + return 0; + + if (id != READING_POLICY) + return 0; + + ima_test_common(file); + + return 0; +}
On 2/15/22 5:40 AM, Roberto Sassu wrote:
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 | 2 ++ .../selftests/bpf/prog_tests/test_ima.c | 3 +- tools/testing/selftests/bpf/progs/ima.c | 28 ++++++++++++++++--- 3 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/bpf/ima_setup.sh b/tools/testing/selftests/bpf/ima_setup.sh index 8e62581113a3..82530f19f85a 100755 --- a/tools/testing/selftests/bpf/ima_setup.sh +++ b/tools/testing/selftests/bpf/ima_setup.sh @@ -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() { @@ -74,6 +75,7 @@ run() local mount_dir="${tmp_dir}/mnt" local copied_bin_path="${mount_dir}/$(basename ${TEST_BINARY})"
- echo ${mount_dir}/policy_test > ${IMA_POLICY_FILE} exec "${copied_bin_path}" }
diff --git a/tools/testing/selftests/bpf/prog_tests/test_ima.c b/tools/testing/selftests/bpf/prog_tests/test_ima.c index 62bf0e830453..c4a62d7b70df 100644 --- a/tools/testing/selftests/bpf/prog_tests/test_ima.c +++ b/tools/testing/selftests/bpf/prog_tests/test_ima.c @@ -97,8 +97,9 @@ void test_test_ima(void) /* * 1 sample with use_ima_file_hash = false * 2 samples with use_ima_file_hash = true (./ima_setup.sh, /bin/true)
* 1 sample with use_ima_file_hash = true (IMA policy)
- ASSERT_EQ(err, 3, "num_samples_or_err");
- ASSERT_EQ(err, 4, "num_samples_or_err"); ASSERT_NEQ(ima_hash_from_bpf, 0, "ima_hash");
close_clean: diff --git a/tools/testing/selftests/bpf/progs/ima.c b/tools/testing/selftests/bpf/progs/ima.c index 9bb63f96cfc0..9b4c03f30a1c 100644 --- a/tools/testing/selftests/bpf/progs/ima.c +++ b/tools/testing/selftests/bpf/progs/ima.c @@ -20,8 +20,7 @@ char _license[] SEC("license") = "GPL"; bool use_ima_file_hash; -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; @@ -31,10 +30,10 @@ void BPF_PROG(ima, struct linux_binprm *bprm) pid = bpf_get_current_pid_tgid() >> 32; if (pid == monitored_pid) { if (!use_ima_file_hash)
ret = bpf_ima_inode_hash(bprm->file->f_inode, &ima_hash,
ret = bpf_ima_inode_hash(file->f_inode, &ima_hash, sizeof(ima_hash));
ret = bpf_ima_file_hash(bprm->file, &ima_hash,
ret = bpf_ima_file_hash(file, &ima_hash, sizeof(ima_hash));
Is this considered an error? Does it make sense for this test to be void type and not return the error to its callers? One of the callers below seems to care for return values.
return;@@ -49,3 +48,24 @@ void BPF_PROG(ima, struct linux_binprm *bprm) return; }
+SEC("lsm.s/bprm_committed_creds") +void BPF_PROG(ima, struct linux_binprm *bprm) +{
- ima_test_common(bprm->file);
+}
+SEC("lsm.s/kernel_read_file") +int BPF_PROG(kernel_read_file, struct file *file, enum kernel_read_file_id id,
bool contents)+{
- if (!contents)
return 0;- if (id != READING_POLICY)
return 0;- ima_test_common(file);
This one here.
- return 0;
+}
thanks, -- Shuah
From: Shuah Khan [mailto:skhan@linuxfoundation.org] Sent: Tuesday, February 15, 2022 5:11 PM On 2/15/22 5:40 AM, Roberto Sassu wrote:
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 | 2 ++ .../selftests/bpf/prog_tests/test_ima.c | 3 +- tools/testing/selftests/bpf/progs/ima.c | 28 ++++++++++++++++--- 3 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/bpf/ima_setup.sh
b/tools/testing/selftests/bpf/ima_setup.sh
index 8e62581113a3..82530f19f85a 100755 --- a/tools/testing/selftests/bpf/ima_setup.sh +++ b/tools/testing/selftests/bpf/ima_setup.sh @@ -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() { @@ -74,6 +75,7 @@ run() local mount_dir="${tmp_dir}/mnt" local copied_bin_path="${mount_dir}/$(basename ${TEST_BINARY})"
- echo ${mount_dir}/policy_test > ${IMA_POLICY_FILE} exec "${copied_bin_path}" }
diff --git a/tools/testing/selftests/bpf/prog_tests/test_ima.c
b/tools/testing/selftests/bpf/prog_tests/test_ima.c
index 62bf0e830453..c4a62d7b70df 100644 --- a/tools/testing/selftests/bpf/prog_tests/test_ima.c +++ b/tools/testing/selftests/bpf/prog_tests/test_ima.c @@ -97,8 +97,9 @@ void test_test_ima(void) /* * 1 sample with use_ima_file_hash = false * 2 samples with use_ima_file_hash = true (./ima_setup.sh, /bin/true)
* 1 sample with use_ima_file_hash = true (IMA policy)
- ASSERT_EQ(err, 3, "num_samples_or_err");
ASSERT_EQ(err, 4, "num_samples_or_err"); ASSERT_NEQ(ima_hash_from_bpf, 0, "ima_hash");
close_clean:
diff --git a/tools/testing/selftests/bpf/progs/ima.c
b/tools/testing/selftests/bpf/progs/ima.c
index 9bb63f96cfc0..9b4c03f30a1c 100644 --- a/tools/testing/selftests/bpf/progs/ima.c +++ b/tools/testing/selftests/bpf/progs/ima.c @@ -20,8 +20,7 @@ char _license[] SEC("license") = "GPL";
bool use_ima_file_hash;
-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; @@ -31,10 +30,10 @@ void BPF_PROG(ima, struct linux_binprm *bprm) pid = bpf_get_current_pid_tgid() >> 32; if (pid == monitored_pid) { if (!use_ima_file_hash)
ret = bpf_ima_inode_hash(bprm->file->f_inode,&ima_hash,
ret = bpf_ima_inode_hash(file->f_inode, &ima_hash, sizeof(ima_hash));
ret = bpf_ima_file_hash(bprm->file, &ima_hash,
ret = bpf_ima_file_hash(file, &ima_hash, sizeof(ima_hash));Is this considered an error? Does it make sense for this test to be void type and not return the error to its callers? One of the callers below seems to care for return values.
The user space side of the test (test_ima.c) seems to check the number of samples obtained from the ring buffer. A failure here would result in the sample not being sent to that component.
Another test, as you suggest, could be to ensure that the kernel_read_file hook is able to deny operations. I would check this in a separate test.
Thanks
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Zhong Ronghua
return;@@ -49,3 +48,24 @@ void BPF_PROG(ima, struct linux_binprm *bprm)
return; }
+SEC("lsm.s/bprm_committed_creds") +void BPF_PROG(ima, struct linux_binprm *bprm) +{
- ima_test_common(bprm->file);
+}
+SEC("lsm.s/kernel_read_file") +int BPF_PROG(kernel_read_file, struct file *file, enum kernel_read_file_id id,
bool contents)+{
- if (!contents)
return 0;- if (id != READING_POLICY)
return 0;- ima_test_common(file);
This one here.
- return 0;
+}
thanks, -- Shuah
From: Roberto Sassu Sent: Tuesday, February 15, 2022 1:41 PM 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-4 gives 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 5-6 allows eBPF-based LSMs to evaluate files read by the kernel.
Hi everyone
I published the new DIGLIM eBPF, that takes advantage of the new features introduced with this patch set:
https://github.com/robertosassu/diglim-ebpf
the eBPF program is in ebpf/diglim_kern.c
If you could have a look and give me some comments or suggestions, it would be very appreciated!
Thanks
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Zhong Ronghua
Changelog
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 (6): 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: Add test for bpf_ima_file_hash() bpf-lsm: Make bpf_lsm_kernel_read_file() as sleepable selftests/bpf: Add test for bpf_lsm_kernel_read_file()
include/uapi/linux/bpf.h | 11 +++++ kernel/bpf/bpf_lsm.c | 21 +++++++++ security/integrity/ima/ima_main.c | 47 ++++++++++++------- tools/include/uapi/linux/bpf.h | 11 +++++ tools/testing/selftests/bpf/ima_setup.sh | 2 + .../selftests/bpf/prog_tests/test_ima.c | 30 ++++++++++-- tools/testing/selftests/bpf/progs/ima.c | 34 ++++++++++++-- 7 files changed, 132 insertions(+), 24 deletions(-)
-- 2.32.0
Hi Roberto,
On Tue, 2022-02-15 at 13:40 +0100, Roberto Sassu wrote:
Extend the interoperability with IMA, to give wider flexibility for the implementation of integrity-focused LSMs based on eBPF.
I've previously requested adding eBPF module measurements and signature verification support in IMA. There seemed to be some interest, but nothing has been posted.
thanks,
Mimi
From: Mimi Zohar [mailto:zohar@linux.ibm.com] Sent: Friday, February 25, 2022 1:22 AM Hi Roberto,
On Tue, 2022-02-15 at 13:40 +0100, Roberto Sassu wrote:
Extend the interoperability with IMA, to give wider flexibility for the implementation of integrity-focused LSMs based on eBPF.
I've previously requested adding eBPF module measurements and signature verification support in IMA. There seemed to be some interest, but nothing has been posted.
Hi Mimi
for my use case, DIGLIM eBPF, IMA integrity verification is needed until the binary carrying the eBPF program is executed as the init process. I've been thinking to use an appended signature to overcome the limitation of lack of xattrs in the initial ram disk.
At that point, the LSM is attached and it can enforce an execution policy, allowing or denying execution and mmap of files depending on the digest lists (reference values) read by the user space side.
After the LSM is attached, IMA's job would be just to calculate the file digests (currently, I'm using an audit policy to ensure that the digest is available when the eBPF program calls bpf_ima_inode_hash()).
The main benefit of this patch set is that the audit policy would not be required and digests are calculated only when requested by the eBPF program.
Thanks
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Zhong Ronghua
On Fri, 2022-02-25 at 08:41 +0000, Roberto Sassu wrote:
From: Mimi Zohar [mailto:zohar@linux.ibm.com] Sent: Friday, February 25, 2022 1:22 AM Hi Roberto,
On Tue, 2022-02-15 at 13:40 +0100, Roberto Sassu wrote:
Extend the interoperability with IMA, to give wider flexibility for the implementation of integrity-focused LSMs based on eBPF.
I've previously requested adding eBPF module measurements and signature verification support in IMA. There seemed to be some interest, but nothing has been posted.
Hi Mimi
for my use case, DIGLIM eBPF, IMA integrity verification is needed until the binary carrying the eBPF program is executed as the init process. I've been thinking to use an appended signature to overcome the limitation of lack of xattrs in the initial ram disk.
I would still like to see xattrs supported in the initial ram disk. Assuming you're still interested in pursuing it, someone would need to review and upstream it. Greg?
At that point, the LSM is attached and it can enforce an execution policy, allowing or denying execution and mmap of files depending on the digest lists (reference values) read by the user space side.
After the LSM is attached, IMA's job would be just to calculate the file digests (currently, I'm using an audit policy to ensure that the digest is available when the eBPF program calls bpf_ima_inode_hash()).
The main benefit of this patch set is that the audit policy would not be required and digests are calculated only when requested by the eBPF program.
Roberto, there's an existing eBPF integrity gap that needs to be closed, perhaps not for your usecase, but in general. Is that something you can look into?
thanks,
Mimi
On Fri, Feb 25, 2022 at 02:11:04PM -0500, Mimi Zohar wrote:
On Fri, 2022-02-25 at 08:41 +0000, Roberto Sassu wrote:
From: Mimi Zohar [mailto:zohar@linux.ibm.com] Sent: Friday, February 25, 2022 1:22 AM Hi Roberto,
On Tue, 2022-02-15 at 13:40 +0100, Roberto Sassu wrote:
Extend the interoperability with IMA, to give wider flexibility for the implementation of integrity-focused LSMs based on eBPF.
I've previously requested adding eBPF module measurements and signature verification support in IMA. There seemed to be some interest, but nothing has been posted.
Hi Mimi
for my use case, DIGLIM eBPF, IMA integrity verification is needed until the binary carrying the eBPF program is executed as the init process. I've been thinking to use an appended signature to overcome the limitation of lack of xattrs in the initial ram disk.
I would still like to see xattrs supported in the initial ram disk. Assuming you're still interested in pursuing it, someone would need to review and upstream it. Greg?
Me? How about the filesystem maintainers and developers? :)
There's a reason we never added xattrs support to ram disks, but I can't remember why...
thanks,
gre gk-h
On Sat, 2022-02-26 at 09:07 +0100, Greg Kroah-Hartman wrote:
On Fri, Feb 25, 2022 at 02:11:04PM -0500, Mimi Zohar wrote:
On Fri, 2022-02-25 at 08:41 +0000, Roberto Sassu wrote:
From: Mimi Zohar [mailto:zohar@linux.ibm.com] Sent: Friday, February 25, 2022 1:22 AM Hi Roberto,
On Tue, 2022-02-15 at 13:40 +0100, Roberto Sassu wrote:
Extend the interoperability with IMA, to give wider flexibility for the implementation of integrity-focused LSMs based on eBPF.
I've previously requested adding eBPF module measurements and signature verification support in IMA. There seemed to be some interest, but nothing has been posted.
Hi Mimi
for my use case, DIGLIM eBPF, IMA integrity verification is needed until the binary carrying the eBPF program is executed as the init process. I've been thinking to use an appended signature to overcome the limitation of lack of xattrs in the initial ram disk.
I would still like to see xattrs supported in the initial ram disk. Assuming you're still interested in pursuing it, someone would need to review and upstream it. Greg?
Me? How about the filesystem maintainers and developers? :)
There's a reason we never added xattrs support to ram disks, but I can't remember why...
CPIO 'newc' format doesn't support xattrs.
thanks,
Mimi
From: Mimi Zohar [mailto:zohar@linux.ibm.com] Sent: Friday, February 25, 2022 8:11 PM On Fri, 2022-02-25 at 08:41 +0000, Roberto Sassu wrote:
From: Mimi Zohar [mailto:zohar@linux.ibm.com] Sent: Friday, February 25, 2022 1:22 AM Hi Roberto,
On Tue, 2022-02-15 at 13:40 +0100, Roberto Sassu wrote:
Extend the interoperability with IMA, to give wider flexibility for the implementation of integrity-focused LSMs based on eBPF.
I've previously requested adding eBPF module measurements and signature verification support in IMA. There seemed to be some interest, but nothing has been posted.
Hi Mimi
for my use case, DIGLIM eBPF, IMA integrity verification is needed until the binary carrying the eBPF program is executed as the init process. I've been thinking to use an appended signature to overcome the limitation of lack of xattrs in the initial ram disk.
I would still like to see xattrs supported in the initial ram disk. Assuming you're still interested in pursuing it, someone would need to review and upstream it. Greg?
I could revise this work. However, since appended signatures would work too, I would propose to extend this appraisal mode to executables, if it is fine for you.
At that point, the LSM is attached and it can enforce an execution policy, allowing or denying execution and mmap of files depending on the digest lists (reference values) read by the user space side.
After the LSM is attached, IMA's job would be just to calculate the file digests (currently, I'm using an audit policy to ensure that the digest is available when the eBPF program calls bpf_ima_inode_hash()).
The main benefit of this patch set is that the audit policy would not be required and digests are calculated only when requested by the eBPF program.
Roberto, there's an existing eBPF integrity gap that needs to be closed, perhaps not for your usecase, but in general. Is that something you can look into?
It could be possible I look into it.
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Zhong Ronghua
From: Roberto Sassu Sent: Monday, February 28, 2022 10:08 AM
From: Mimi Zohar [mailto:zohar@linux.ibm.com] Sent: Friday, February 25, 2022 8:11 PM On Fri, 2022-02-25 at 08:41 +0000, Roberto Sassu wrote:
From: Mimi Zohar [mailto:zohar@linux.ibm.com] Sent: Friday, February 25, 2022 1:22 AM Hi Roberto,
On Tue, 2022-02-15 at 13:40 +0100, Roberto Sassu wrote:
Extend the interoperability with IMA, to give wider flexibility for the implementation of integrity-focused LSMs based on eBPF.
I've previously requested adding eBPF module measurements and signature verification support in IMA. There seemed to be some interest, but nothing has been posted.
Hi Mimi
for my use case, DIGLIM eBPF, IMA integrity verification is needed until the binary carrying the eBPF program is executed as the init process. I've been thinking to use an appended signature to overcome the limitation of lack of xattrs in the initial ram disk.
I would still like to see xattrs supported in the initial ram disk. Assuming you're still interested in pursuing it, someone would need to review and upstream it. Greg?
I could revise this work. However, since appended signatures would work too, I would propose to extend this appraisal mode to executables, if it is fine for you.
Regarding this patch set, I kindly ask if you could accept it, after I make the changes suggested.
The changes are simple, and waiting another kernel cycle seems too long.
Thanks
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Zhong Ronghua
At that point, the LSM is attached and it can enforce an execution policy, allowing or denying execution and mmap of files depending on the digest lists (reference values) read by the user space side.
After the LSM is attached, IMA's job would be just to calculate the file digests (currently, I'm using an audit policy to ensure that the digest is available when the eBPF program calls bpf_ima_inode_hash()).
The main benefit of this patch set is that the audit policy would not be required and digests are calculated only when requested by the eBPF program.
Roberto, there's an existing eBPF integrity gap that needs to be closed, perhaps not for your usecase, but in general. Is that something you can look into?
It could be possible I look into it.
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Zhong Ronghua
On Mon, Feb 28, 2022 at 09:12:35AM +0000, Roberto Sassu wrote:
From: Roberto Sassu Sent: Monday, February 28, 2022 10:08 AM
From: Mimi Zohar [mailto:zohar@linux.ibm.com] Sent: Friday, February 25, 2022 8:11 PM On Fri, 2022-02-25 at 08:41 +0000, Roberto Sassu wrote:
From: Mimi Zohar [mailto:zohar@linux.ibm.com] Sent: Friday, February 25, 2022 1:22 AM Hi Roberto,
On Tue, 2022-02-15 at 13:40 +0100, Roberto Sassu wrote:
Extend the interoperability with IMA, to give wider flexibility for the implementation of integrity-focused LSMs based on eBPF.
I've previously requested adding eBPF module measurements and signature verification support in IMA. There seemed to be some interest, but nothing has been posted.
Hi Mimi
for my use case, DIGLIM eBPF, IMA integrity verification is needed until the binary carrying the eBPF program is executed as the init process. I've been thinking to use an appended signature to overcome the limitation of lack of xattrs in the initial ram disk.
I would still like to see xattrs supported in the initial ram disk. Assuming you're still interested in pursuing it, someone would need to review and upstream it. Greg?
I could revise this work. However, since appended signatures would work too, I would propose to extend this appraisal mode to executables, if it is fine for you.
Regarding this patch set, I kindly ask if you could accept it, after I make the changes suggested.
The changes are simple, and waiting another kernel cycle seems too long.
3 months is not a long time, get it right first, there is no deadline here.
thanks,
greg k-h
linux-kselftest-mirror@lists.linaro.org
-
Greg Kroah-Hartman -
Mimi Zohar -
Roberto Sassu -
Shuah Khan -
Yonghong Song