[PATCH bpf-next v3 0/3] Implement bpf_ima_inode_hash

[PATCH bpf-next v3 0/3] Implement bpf_ima_inode_hash

[KP Singh]

From: KP Singh <kpsingh@google.com>

# v2 -> v3

- Fixed an issue pointed out by Alexei, the helper should only be
  exposed to sleepable hooks.
- Update the selftests to constrain the IMA policy udpate to a loopback
  filesystem specifically created for the test. Also, split this out
  from the LSM test. I dropped the Ack from this last patch since this
  is a re-write.

KP Singh (3):
  ima: Implement ima_inode_hash
  bpf: Add a BPF helper for getting the IMA hash of an inode
  bpf: Add a selftest for bpf_ima_inode_hash

 include/linux/ima.h                           |  6 ++
 include/uapi/linux/bpf.h                      | 11 +++
 kernel/bpf/bpf_lsm.c                          | 26 ++++++
 scripts/bpf_helpers_doc.py                    |  2 +
 security/integrity/ima/ima_main.c             | 78 ++++++++++++------
 tools/include/uapi/linux/bpf.h                | 11 +++
 tools/testing/selftests/bpf/config            |  4 +
 tools/testing/selftests/bpf/ima_setup.sh      | 80 +++++++++++++++++++
 .../selftests/bpf/prog_tests/test_ima.c       | 74 +++++++++++++++++
 tools/testing/selftests/bpf/progs/ima.c       | 28 +++++++
 10 files changed, 296 insertions(+), 24 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/ima_setup.sh
 create mode 100644 tools/testing/selftests/bpf/prog_tests/test_ima.c
 create mode 100644 tools/testing/selftests/bpf/progs/ima.c

-- 
2.29.2.454.gaff20da3a2-goog

[PATCH bpf-next v3 1/3] ima: Implement ima_inode_hash

[KP Singh]

From: KP Singh <kpsingh@google.com>

This is in preparation to add a helper for BPF LSM programs to use
IMA hashes when attached to LSM hooks. There are LSM hooks like
inode_unlink which do not have a struct file * argument and cannot
use the existing ima_file_hash API.

An inode based API is, therefore, useful in LSM based detections like an
executable trying to delete itself which rely on the inode_unlink LSM
hook.

Moreover, the ima_file_hash function does nothing with the struct file
pointer apart from calling file_inode on it and converting it to an
inode.

Signed-off-by: KP Singh <kpsingh@google.com>
---
 include/linux/ima.h               |  6 +++
 security/integrity/ima/ima_main.c | 78 +++++++++++++++++++++----------
 2 files changed, 60 insertions(+), 24 deletions(-)

diff --git a/include/linux/ima.h b/include/linux/ima.h
index 8fa7bcfb2da2..7233a2751754 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -29,6 +29,7 @@ extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
 			      enum kernel_read_file_id id);
 extern void ima_post_path_mknod(struct dentry *dentry);
 extern int ima_file_hash(struct file *file, char *buf, size_t buf_size);
+extern int ima_inode_hash(struct inode *inode, char *buf, size_t buf_size);
 extern void ima_kexec_cmdline(int kernel_fd, const void *buf, int size);
 
 #ifdef CONFIG_IMA_KEXEC
@@ -115,6 +116,11 @@ static inline int ima_file_hash(struct file *file, char *buf, size_t buf_size)
 	return -EOPNOTSUPP;
 }
 
+static inline int ima_inode_hash(struct inode *inode, char *buf, size_t buf_size)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline void ima_kexec_cmdline(int kernel_fd, const void *buf, int size) {}
 #endif /* CONFIG_IMA */
 
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 2d1af8899cab..cb2deaa188e7 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -501,37 +501,14 @@ int ima_file_check(struct file *file, int mask)
 }
 EXPORT_SYMBOL_GPL(ima_file_check);
 
-/**
- * ima_file_hash - return the stored measurement if a file has been hashed and
- * is in the iint cache.
- * @file: pointer to the file
- * @buf: buffer in which to store the hash
- * @buf_size: length of the buffer
- *
- * On success, return the hash algorithm (as defined in the enum hash_algo).
- * If buf is not NULL, this function also outputs the hash into buf.
- * If the hash is larger than buf_size, then only buf_size bytes will be copied.
- * It generally just makes sense to pass a buffer capable of holding the largest
- * possible hash: IMA_MAX_DIGEST_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 parameters are incorrect, return -EINVAL.
- */
-int ima_file_hash(struct file *file, char *buf, size_t buf_size)
+static int __ima_inode_hash(struct inode *inode, char *buf, size_t buf_size)
 {
-	struct inode *inode;
 	struct integrity_iint_cache *iint;
 	int hash_algo;
 
-	if (!file)
-		return -EINVAL;
-
 	if (!ima_policy_flag)
 		return -EOPNOTSUPP;
 
-	inode = file_inode(file);
 	iint = integrity_iint_find(inode);
 	if (!iint)
 		return -EOPNOTSUPP;
@@ -558,8 +535,61 @@ int ima_file_hash(struct file *file, char *buf, size_t buf_size)
 
 	return hash_algo;
 }
+
+/**
+ * ima_file_hash - return the stored measurement if a file has been hashed and
+ * is in the iint cache.
+ * @file: pointer to the file
+ * @buf: buffer in which to store the hash
+ * @buf_size: length of the buffer
+ *
+ * On success, return the hash algorithm (as defined in the enum hash_algo).
+ * If buf is not NULL, this function also outputs the hash into buf.
+ * If the hash is larger than buf_size, then only buf_size bytes will be copied.
+ * It generally just makes sense to pass a buffer capable of holding the largest
+ * possible hash: IMA_MAX_DIGEST_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 parameters are incorrect, return -EINVAL.
+ */
+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);
+}
 EXPORT_SYMBOL_GPL(ima_file_hash);
 
+/**
+ * ima_inode_hash - return the stored measurement if the inode has been hashed
+ * and is in the iint cache.
+ * @inode: pointer to the inode
+ * @buf: buffer in which to store the hash
+ * @buf_size: length of the buffer
+ *
+ * On success, return the hash algorithm (as defined in the enum hash_algo).
+ * If buf is not NULL, this function also outputs the hash into buf.
+ * If the hash is larger than buf_size, then only buf_size bytes will be copied.
+ * It generally just makes sense to pass a buffer capable of holding the largest
+ * possible hash: IMA_MAX_DIGEST_SIZE.
+ * The hash returned is based on the entire contents, including the appended
+ * signature.
+ *
+ * If IMA is disabled or if no measurement is available, return -EOPNOTSUPP.
+ * If the parameters are incorrect, return -EINVAL.
+ */
+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);
+}
+EXPORT_SYMBOL_GPL(ima_inode_hash);
+
 /**
  * ima_post_create_tmpfile - mark newly created tmpfile as new
  * @file : newly created tmpfile
-- 
2.29.2.454.gaff20da3a2-goog

Re: [PATCH bpf-next v3 1/3] ima: Implement ima_inode_hash

[Yonghong Song]

On 11/24/20 7:12 AM, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
> 
> This is in preparation to add a helper for BPF LSM programs to use
> IMA hashes when attached to LSM hooks. There are LSM hooks like
> inode_unlink which do not have a struct file * argument and cannot
> use the existing ima_file_hash API.
> 
> An inode based API is, therefore, useful in LSM based detections like an
> executable trying to delete itself which rely on the inode_unlink LSM
> hook.
> 
> Moreover, the ima_file_hash function does nothing with the struct file
> pointer apart from calling file_inode on it and converting it to an
> inode.
> 
> Signed-off-by: KP Singh <kpsingh@google.com>

There is no change for this patch compared to previous version,
so you can carry my Ack.

Acked-by: Yonghong Song <yhs@fb.com>

Re: [PATCH bpf-next v3 1/3] ima: Implement ima_inode_hash

[KP Singh]

On Tue, Nov 24, 2020 at 6:35 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 11/24/20 7:12 AM, KP Singh wrote:
> > From: KP Singh <kpsingh@google.com>
> >
> > This is in preparation to add a helper for BPF LSM programs to use
> > IMA hashes when attached to LSM hooks. There are LSM hooks like
> > inode_unlink which do not have a struct file * argument and cannot
> > use the existing ima_file_hash API.
> >
> > An inode based API is, therefore, useful in LSM based detections like an
> > executable trying to delete itself which rely on the inode_unlink LSM
> > hook.
> >
> > Moreover, the ima_file_hash function does nothing with the struct file
> > pointer apart from calling file_inode on it and converting it to an
> > inode.
> >
> > Signed-off-by: KP Singh <kpsingh@google.com>
>
> There is no change for this patch compared to previous version,
> so you can carry my Ack.
>
> Acked-by: Yonghong Song <yhs@fb.com>

I am guessing:

*  We need an Ack from Mimi/James.
* As regards to which tree, I guess bpf-next would be better since the
BPF helper and the selftest depends on it

Re: [PATCH bpf-next v3 1/3] ima: Implement ima_inode_hash

[Daniel Borkmann]

On 11/25/20 1:04 PM, KP Singh wrote:
> On Tue, Nov 24, 2020 at 6:35 PM Yonghong Song <yhs@fb.com> wrote:
>> On 11/24/20 7:12 AM, KP Singh wrote:
>>> From: KP Singh <kpsingh@google.com>
>>>
>>> This is in preparation to add a helper for BPF LSM programs to use
>>> IMA hashes when attached to LSM hooks. There are LSM hooks like
>>> inode_unlink which do not have a struct file * argument and cannot
>>> use the existing ima_file_hash API.
>>>
>>> An inode based API is, therefore, useful in LSM based detections like an
>>> executable trying to delete itself which rely on the inode_unlink LSM
>>> hook.
>>>
>>> Moreover, the ima_file_hash function does nothing with the struct file
>>> pointer apart from calling file_inode on it and converting it to an
>>> inode.
>>>
>>> Signed-off-by: KP Singh <kpsingh@google.com>
>>
>> There is no change for this patch compared to previous version,
>> so you can carry my Ack.
>>
>> Acked-by: Yonghong Song <yhs@fb.com>
> 
> I am guessing:
> 
> *  We need an Ack from Mimi/James.

Yes.

> * As regards to which tree, I guess bpf-next would be better since the
> BPF helper and the selftest depends on it

Yep, bpf-next is my preference as otherwise we're running into unnecessary
merge conflicts.

Thanks,
Daniel

Re: [PATCH bpf-next v3 1/3] ima: Implement ima_inode_hash

[Mimi Zohar]

On Tue, 2020-11-24 at 15:12 +0000, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
> 
> This is in preparation to add a helper for BPF LSM programs to use
> IMA hashes when attached to LSM hooks. There are LSM hooks like
> inode_unlink which do not have a struct file * argument and cannot
> use the existing ima_file_hash API.
> 
> An inode based API is, therefore, useful in LSM based detections like an
> executable trying to delete itself which rely on the inode_unlink LSM
> hook.
> 
> Moreover, the ima_file_hash function does nothing with the struct file
> pointer apart from calling file_inode on it and converting it to an
> inode.
> 
> Signed-off-by: KP Singh <kpsingh@google.com>


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

[PATCH bpf-next v3 2/3] bpf: Add a BPF helper for getting the IMA hash of an inode

[KP Singh]

From: KP Singh <kpsingh@google.com>

Provide a wrapper function to get the IMA hash of an inode. This helper
is useful in fingerprinting files (e.g executables on execution) and
using these fingerprints in detections like an executable unlinking
itself.

Since the ima_inode_hash can sleep, it's only allowed for sleepable
LSM hooks.

Signed-off-by: KP Singh <kpsingh@google.com>
---
 include/uapi/linux/bpf.h       | 11 +++++++++++
 kernel/bpf/bpf_lsm.c           | 26 ++++++++++++++++++++++++++
 scripts/bpf_helpers_doc.py     |  2 ++
 tools/include/uapi/linux/bpf.h | 11 +++++++++++
 4 files changed, 50 insertions(+)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 3ca6146f001a..c3458ec1f30a 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -3807,6 +3807,16 @@ union bpf_attr {
  * 		See: **clock_gettime**\ (**CLOCK_MONOTONIC_COARSE**)
  * 	Return
  * 		Current *ktime*.
+ *
+ * long bpf_ima_inode_hash(struct inode *inode, void *dst, u32 size)
+ *	Description
+ *		Returns the stored IMA hash of the *inode* (if it's avaialable).
+ *		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 IMA is disabled or **-EINVAL** if
+ *		invalid arguments are passed.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -3970,6 +3980,7 @@ union bpf_attr {
 	FN(get_current_task_btf),	\
 	FN(bprm_opts_set),		\
 	FN(ktime_get_coarse_ns),	\
+	FN(ima_inode_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 b4f27a874092..70e5e0b6d69d 100644
--- a/kernel/bpf/bpf_lsm.c
+++ b/kernel/bpf/bpf_lsm.c
@@ -15,6 +15,7 @@
 #include <net/bpf_sk_storage.h>
 #include <linux/bpf_local_storage.h>
 #include <linux/btf_ids.h>
+#include <linux/ima.h>
 
 /* For every LSM hook that allows attachment of BPF programs, declare a nop
  * function where a BPF program can be attached.
@@ -75,6 +76,29 @@ const static struct bpf_func_proto bpf_bprm_opts_set_proto = {
 	.arg2_type	= ARG_ANYTHING,
 };
 
+BPF_CALL_3(bpf_ima_inode_hash, struct inode *, inode, void *, dst, u32, size)
+{
+	return ima_inode_hash(inode, dst, size);
+}
+
+static bool bpf_ima_inode_hash_allowed(const struct bpf_prog *prog)
+{
+	return bpf_lsm_is_sleepable_hook(prog->aux->attach_btf_id);
+}
+
+BTF_ID_LIST_SINGLE(bpf_ima_inode_hash_btf_ids, struct, inode)
+
+const static struct bpf_func_proto bpf_ima_inode_hash_proto = {
+	.func		= bpf_ima_inode_hash,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_BTF_ID,
+	.arg1_btf_id	= &bpf_ima_inode_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)
 {
@@ -97,6 +121,8 @@ bpf_lsm_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_task_storage_delete_proto;
 	case BPF_FUNC_bprm_opts_set:
 		return &bpf_bprm_opts_set_proto;
+	case BPF_FUNC_ima_inode_hash:
+		return prog->aux->sleepable ? &bpf_ima_inode_hash_proto : NULL;
 	default:
 		return tracing_prog_func_proto(func_id, prog);
 	}
diff --git a/scripts/bpf_helpers_doc.py b/scripts/bpf_helpers_doc.py
index c5bc947a70ad..8b829748d488 100755
--- a/scripts/bpf_helpers_doc.py
+++ b/scripts/bpf_helpers_doc.py
@@ -436,6 +436,7 @@ class PrinterHelpers(Printer):
             'struct xdp_md',
             'struct path',
             'struct btf_ptr',
+            'struct inode',
     ]
     known_types = {
             '...',
@@ -480,6 +481,7 @@ class PrinterHelpers(Printer):
             'struct task_struct',
             'struct path',
             'struct btf_ptr',
+            'struct inode',
     }
     mapped_types = {
             'u8': '__u8',
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 3ca6146f001a..c3458ec1f30a 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -3807,6 +3807,16 @@ union bpf_attr {
  * 		See: **clock_gettime**\ (**CLOCK_MONOTONIC_COARSE**)
  * 	Return
  * 		Current *ktime*.
+ *
+ * long bpf_ima_inode_hash(struct inode *inode, void *dst, u32 size)
+ *	Description
+ *		Returns the stored IMA hash of the *inode* (if it's avaialable).
+ *		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 IMA is disabled or **-EINVAL** if
+ *		invalid arguments are passed.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -3970,6 +3980,7 @@ union bpf_attr {
 	FN(get_current_task_btf),	\
 	FN(bprm_opts_set),		\
 	FN(ktime_get_coarse_ns),	\
+	FN(ima_inode_hash),		\
 	/* */
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
-- 
2.29.2.454.gaff20da3a2-goog

Re: [PATCH bpf-next v3 2/3] bpf: Add a BPF helper for getting the IMA hash of an inode

[Yonghong Song]

On 11/24/20 7:12 AM, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
> 
> Provide a wrapper function to get the IMA hash of an inode. This helper
> is useful in fingerprinting files (e.g executables on execution) and
> using these fingerprints in detections like an executable unlinking
> itself.
> 
> Since the ima_inode_hash can sleep, it's only allowed for sleepable
> LSM hooks.
> 
> Signed-off-by: KP Singh <kpsingh@google.com>

Acked-by: Yonghong Song <yhs@fb.com>

[PATCH bpf-next v3 3/3] bpf: Add a selftest for bpf_ima_inode_hash

[KP Singh]

From: KP Singh <kpsingh@google.com>

The test does the following:

- Mounts a loopback filesystem and appends the IMA policy to measure
  executions only on this file-system. Restricting the IMA policy to a
  particular filesystem prevents a system-wide IMA policy change.
- Executes an executable copied to this loopback filesystem.
- Calls the bpf_ima_inode_hash in the bprm_committed_creds hook and
  checks if the call succeeded and checks if a hash was calculated.

The test shells out to the added ima_setup.sh script as the setup is
better handled in a shell script and is more complicated to do in the
test program or even shelling out individual commands from C.

The list of required configs (i.e. IMA, SECURITYFS,
IMA_{WRITE,READ}_POLICY) for running this test are also updated.

Signed-off-by: KP Singh <kpsingh@google.com>
---
 tools/testing/selftests/bpf/config            |  4 +
 tools/testing/selftests/bpf/ima_setup.sh      | 80 +++++++++++++++++++
 .../selftests/bpf/prog_tests/test_ima.c       | 74 +++++++++++++++++
 tools/testing/selftests/bpf/progs/ima.c       | 28 +++++++
 4 files changed, 186 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/ima_setup.sh
 create mode 100644 tools/testing/selftests/bpf/prog_tests/test_ima.c
 create mode 100644 tools/testing/selftests/bpf/progs/ima.c

diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
index 2118e23ac07a..365bf9771b07 100644
--- a/tools/testing/selftests/bpf/config
+++ b/tools/testing/selftests/bpf/config
@@ -39,3 +39,7 @@ CONFIG_BPF_JIT=y
 CONFIG_BPF_LSM=y
 CONFIG_SECURITY=y
 CONFIG_LIRC=y
+CONFIG_IMA=y
+CONFIG_SECURITYFS=y
+CONFIG_IMA_WRITE_POLICY=y
+CONFIG_IMA_READ_POLICY=y
diff --git a/tools/testing/selftests/bpf/ima_setup.sh b/tools/testing/selftests/bpf/ima_setup.sh
new file mode 100644
index 000000000000..15490ccc5e55
--- /dev/null
+++ b/tools/testing/selftests/bpf/ima_setup.sh
@@ -0,0 +1,80 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+set -e
+set -u
+
+IMA_POLICY_FILE="/sys/kernel/security/ima/policy"
+TEST_BINARY="/bin/true"
+
+usage()
+{
+        echo "Usage: $0 <setup|cleanup|run> <existing_tmp_dir>"
+        exit 1
+}
+
+setup()
+{
+        local tmp_dir="$1"
+        local mount_img="${tmp_dir}/test.img"
+        local mount_dir="${tmp_dir}/mnt"
+        local copied_bin_path="${mount_dir}/$(basename ${TEST_BINARY})"
+        mkdir -p ${mount_dir}
+
+        dd if=/dev/zero of="${mount_img}" bs=1M count=10
+
+        local loop_device="$(losetup --find --show ${mount_img})"
+
+        mkfs.ext4 "${loop_device}"
+        mount "${loop_device}" "${mount_dir}"
+
+        cp "${TEST_BINARY}" "${mount_dir}"
+        local mount_uuid="$(blkid -s UUID -o value ${loop_device})"
+        echo "measure func=BPRM_CHECK fsuuid=${mount_uuid}" > ${IMA_POLICY_FILE}
+}
+
+cleanup() {
+        local tmp_dir="$1"
+        local mount_img="${tmp_dir}/test.img"
+        local mount_dir="${tmp_dir}/mnt"
+
+        local loop_devices=$(losetup -j ${mount_img} -O NAME --noheadings)
+        for loop_dev in "${loop_devices}"; do
+                losetup -d $loop_dev
+        done
+
+        umount ${mount_dir}
+        rm -rf ${tmp_dir}
+}
+
+run()
+{
+        local tmp_dir="$1"
+        local mount_dir="${tmp_dir}/mnt"
+        local copied_bin_path="${mount_dir}/$(basename ${TEST_BINARY})"
+
+        exec "${copied_bin_path}"
+}
+
+main()
+{
+        [[ $# -ne 2 ]] && usage
+
+        local action="$1"
+        local tmp_dir="$2"
+
+        [[ ! -d "${tmp_dir}" ]] && echo "Directory ${tmp_dir} doesn't exist" && exit 1
+
+        if [[ "${action}" == "setup" ]]; then
+                setup "${tmp_dir}"
+        elif [[ "${action}" == "cleanup" ]]; then
+                cleanup "${tmp_dir}"
+        elif [[ "${action}" == "run" ]]; then
+                run "${tmp_dir}"
+        else
+                echo "Unknown action: ${action}"
+                exit 1
+        fi
+}
+
+main "$@"
diff --git a/tools/testing/selftests/bpf/prog_tests/test_ima.c b/tools/testing/selftests/bpf/prog_tests/test_ima.c
new file mode 100644
index 000000000000..61fca681d524
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/test_ima.c
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright (C) 2020 Google LLC.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <sys/wait.h>
+#include <test_progs.h>
+
+#include "ima.skel.h"
+
+static int run_measured_process(const char *measured_dir, u32 *monitored_pid)
+{
+	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,
+		       NULL);
+		exit(errno);
+
+	} else if (child_pid > 0) {
+		waitpid(child_pid, &child_status, 0);
+		return WEXITSTATUS(child_status);
+	}
+
+	return -EINVAL;
+}
+
+void test_test_ima(void)
+{
+	char measured_dir_template[] = "/tmp/ima_measuredXXXXXX";
+	const char *measured_dir;
+	char cmd[256];
+
+	int err, duration = 0;
+	struct ima *skel = NULL;
+
+	skel = ima__open_and_load();
+	if (CHECK(!skel, "skel_load", "skeleton failed\n"))
+		goto close_prog;
+
+	err = ima__attach(skel);
+	if (CHECK(err, "attach", "attach failed: %d\n", err))
+		goto close_prog;
+
+	measured_dir = mkdtemp(measured_dir_template);
+	if (CHECK(measured_dir == NULL, "mkdtemp", "err %d\n", errno))
+		goto close_prog;
+
+	snprintf(cmd, sizeof(cmd), "./ima_setup.sh setup %s", measured_dir);
+	if (CHECK_FAIL(system(cmd)))
+		goto close_clean;
+
+	err = run_measured_process(measured_dir, &skel->bss->monitored_pid);
+	if (CHECK(err, "run_measured_process", "err = %d\n", err))
+		goto close_clean;
+
+	CHECK(skel->data->ima_hash_ret < 0, "ima_hash_ret",
+	      "ima_hash_ret = %ld\n", skel->data->ima_hash_ret);
+
+	CHECK(skel->bss->ima_hash == 0, "ima_hash",
+	      "ima_hash = %lu\n", skel->bss->ima_hash);
+
+close_clean:
+	snprintf(cmd, sizeof(cmd), "./ima_setup.sh cleanup %s", measured_dir);
+	CHECK_FAIL(system(cmd));
+close_prog:
+	ima__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/ima.c b/tools/testing/selftests/bpf/progs/ima.c
new file mode 100644
index 000000000000..86b21aff4bc5
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/ima.c
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright 2020 Google LLC.
+ */
+
+#include "vmlinux.h"
+#include <errno.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+long ima_hash_ret = -1;
+u64 ima_hash = 0;
+u32 monitored_pid = 0;
+
+char _license[] SEC("license") = "GPL";
+
+SEC("lsm.s/bprm_committed_creds")
+int BPF_PROG(ima, struct linux_binprm *bprm)
+{
+	u32 pid = bpf_get_current_pid_tgid() >> 32;
+
+	if (pid == monitored_pid)
+		ima_hash_ret = bpf_ima_inode_hash(bprm->file->f_inode,
+						  &ima_hash, sizeof(ima_hash));
+
+	return 0;
+}
-- 
2.29.2.454.gaff20da3a2-goog

Re: [PATCH bpf-next v3 3/3] bpf: Add a selftest for bpf_ima_inode_hash

[Yonghong Song]

On 11/24/20 7:12 AM, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
> 
> The test does the following:
> 
> - Mounts a loopback filesystem and appends the IMA policy to measure
>    executions only on this file-system. Restricting the IMA policy to a
>    particular filesystem prevents a system-wide IMA policy change.
> - Executes an executable copied to this loopback filesystem.
> - Calls the bpf_ima_inode_hash in the bprm_committed_creds hook and
>    checks if the call succeeded and checks if a hash was calculated.
> 
> The test shells out to the added ima_setup.sh script as the setup is
> better handled in a shell script and is more complicated to do in the
> test program or even shelling out individual commands from C.
> 
> The list of required configs (i.e. IMA, SECURITYFS,
> IMA_{WRITE,READ}_POLICY) for running this test are also updated.
> 
> Signed-off-by: KP Singh <kpsingh@google.com>

Ack from bpf perspective,

Acked-by: Yonghong Song <yhs@fb.com>

LSM/IMA experts may want to check ima_setup.sh as well.

> ---
>   tools/testing/selftests/bpf/config            |  4 +
>   tools/testing/selftests/bpf/ima_setup.sh      | 80 +++++++++++++++++++
>   .../selftests/bpf/prog_tests/test_ima.c       | 74 +++++++++++++++++
>   tools/testing/selftests/bpf/progs/ima.c       | 28 +++++++
>   4 files changed, 186 insertions(+)
>   create mode 100644 tools/testing/selftests/bpf/ima_setup.sh
>   create mode 100644 tools/testing/selftests/bpf/prog_tests/test_ima.c
>   create mode 100644 tools/testing/selftests/bpf/progs/ima.c
> 
> diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
> index 2118e23ac07a..365bf9771b07 100644
> --- a/tools/testing/selftests/bpf/config
> +++ b/tools/testing/selftests/bpf/config
> @@ -39,3 +39,7 @@ CONFIG_BPF_JIT=y
>   CONFIG_BPF_LSM=y
>   CONFIG_SECURITY=y
>   CONFIG_LIRC=y
> +CONFIG_IMA=y
> +CONFIG_SECURITYFS=y
> +CONFIG_IMA_WRITE_POLICY=y
> +CONFIG_IMA_READ_POLICY=y
> diff --git a/tools/testing/selftests/bpf/ima_setup.sh b/tools/testing/selftests/bpf/ima_setup.sh
> new file mode 100644
> index 000000000000..15490ccc5e55
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/ima_setup.sh
> @@ -0,0 +1,80 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +
> +set -e
> +set -u
> +
> +IMA_POLICY_FILE="/sys/kernel/security/ima/policy"
> +TEST_BINARY="/bin/true"
> +
> +usage()
> +{
> +        echo "Usage: $0 <setup|cleanup|run> <existing_tmp_dir>"
> +        exit 1
> +}
> +
> +setup()
> +{
> +        local tmp_dir="$1"
> +        local mount_img="${tmp_dir}/test.img"
> +        local mount_dir="${tmp_dir}/mnt"
> +        local copied_bin_path="${mount_dir}/$(basename ${TEST_BINARY})"
> +        mkdir -p ${mount_dir}
> +
> +        dd if=/dev/zero of="${mount_img}" bs=1M count=10
> +
> +        local loop_device="$(losetup --find --show ${mount_img})"
> +
> +        mkfs.ext4 "${loop_device}"
> +        mount "${loop_device}" "${mount_dir}"
> +
> +        cp "${TEST_BINARY}" "${mount_dir}"
> +        local mount_uuid="$(blkid -s UUID -o value ${loop_device})"
> +        echo "measure func=BPRM_CHECK fsuuid=${mount_uuid}" > ${IMA_POLICY_FILE}
> +}
> +
> +cleanup() {
> +        local tmp_dir="$1"
> +        local mount_img="${tmp_dir}/test.img"
> +        local mount_dir="${tmp_dir}/mnt"
> +
> +        local loop_devices=$(losetup -j ${mount_img} -O NAME --noheadings)
> +        for loop_dev in "${loop_devices}"; do
> +                losetup -d $loop_dev
> +        done
> +
> +        umount ${mount_dir}
> +        rm -rf ${tmp_dir}
> +}
> +
> +run()
> +{
> +        local tmp_dir="$1"
> +        local mount_dir="${tmp_dir}/mnt"
> +        local copied_bin_path="${mount_dir}/$(basename ${TEST_BINARY})"
> +
> +        exec "${copied_bin_path}"
> +}
> +
> +main()
> +{
> +        [[ $# -ne 2 ]] && usage
> +
> +        local action="$1"
> +        local tmp_dir="$2"
> +
> +        [[ ! -d "${tmp_dir}" ]] && echo "Directory ${tmp_dir} doesn't exist" && exit 1
> +
> +        if [[ "${action}" == "setup" ]]; then
> +                setup "${tmp_dir}"
> +        elif [[ "${action}" == "cleanup" ]]; then
> +                cleanup "${tmp_dir}"
> +        elif [[ "${action}" == "run" ]]; then
> +                run "${tmp_dir}"
> +        else
> +                echo "Unknown action: ${action}"
> +                exit 1
> +        fi
> +}
> +
> +main "$@"
> diff --git a/tools/testing/selftests/bpf/prog_tests/test_ima.c b/tools/testing/selftests/bpf/prog_tests/test_ima.c
> new file mode 100644
> index 000000000000..61fca681d524
[...]

Re: [PATCH bpf-next v3 3/3] bpf: Add a selftest for bpf_ima_inode_hash

[Mimi Zohar]

On Tue, 2020-11-24 at 15:12 +0000, KP Singh wrote:
> diff --git a/tools/testing/selftests/bpf/ima_setup.sh b/tools/testing/selftests/bpf/ima_setup.sh
> new file mode 100644
> index 000000000000..15490ccc5e55
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/ima_setup.sh
> @@ -0,0 +1,80 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +
> +set -e
> +set -u
> +
> +IMA_POLICY_FILE="/sys/kernel/security/ima/policy"
> +TEST_BINARY="/bin/true"
> +
> +usage()
> +{
> +        echo "Usage: $0 <setup|cleanup|run> <existing_tmp_dir>"
> +        exit 1
> +}
> +
> +setup()
> +{
> +        local tmp_dir="$1"
> +        local mount_img="${tmp_dir}/test.img"
> +        local mount_dir="${tmp_dir}/mnt"
> +        local copied_bin_path="${mount_dir}/$(basename ${TEST_BINARY})"
> +        mkdir -p ${mount_dir}
> +
> +        dd if=/dev/zero of="${mount_img}" bs=1M count=10
> +
> +        local loop_device="$(losetup --find --show ${mount_img})"
> +
> +        mkfs.ext4 "${loop_device}"
> +        mount "${loop_device}" "${mount_dir}"
> +
> +        cp "${TEST_BINARY}" "${mount_dir}"
> +        local mount_uuid="$(blkid -s UUID -o value ${loop_device})"
> +        echo "measure func=BPRM_CHECK fsuuid=${mount_uuid}" > ${IMA_POLICY_FILE}

Anyone using IMA, normally define policy rules requiring the policy
itself to be signed.   Instead of writing the policy rules, write the
signed policy file pathname.  Refer to dracut commit 479b5cd9
("98integrity: support validating the IMA policy file signature").

Both enabling IMA_APPRAISE_REQUIRE_POLICY_SIGS and the builtin
"appraise_tcb" policy require loading a signed policy.

Mimi

Re: [PATCH bpf-next v3 3/3] bpf: Add a selftest for bpf_ima_inode_hash

[KP Singh]

On Wed, Nov 25, 2020 at 3:20 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Tue, 2020-11-24 at 15:12 +0000, KP Singh wrote:
> > diff --git a/tools/testing/selftests/bpf/ima_setup.sh b/tools/testing/selftests/bpf/ima_setup.sh
> > new file mode 100644
> > index 000000000000..15490ccc5e55
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/ima_setup.sh
> > @@ -0,0 +1,80 @@
> > +#!/bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +set -e
> > +set -u
> > +
> > +IMA_POLICY_FILE="/sys/kernel/security/ima/policy"
> > +TEST_BINARY="/bin/true"
> > +
> > +usage()
> > +{
> > +        echo "Usage: $0 <setup|cleanup|run> <existing_tmp_dir>"
> > +        exit 1
> > +}
> > +
> > +setup()
> > +{
> > +        local tmp_dir="$1"
> > +        local mount_img="${tmp_dir}/test.img"
> > +        local mount_dir="${tmp_dir}/mnt"
> > +        local copied_bin_path="${mount_dir}/$(basename ${TEST_BINARY})"
> > +        mkdir -p ${mount_dir}
> > +
> > +        dd if=/dev/zero of="${mount_img}" bs=1M count=10
> > +
> > +        local loop_device="$(losetup --find --show ${mount_img})"
> > +
> > +        mkfs.ext4 "${loop_device}"
> > +        mount "${loop_device}" "${mount_dir}"
> > +
> > +        cp "${TEST_BINARY}" "${mount_dir}"
> > +        local mount_uuid="$(blkid -s UUID -o value ${loop_device})"
> > +        echo "measure func=BPRM_CHECK fsuuid=${mount_uuid}" > ${IMA_POLICY_FILE}
>
> Anyone using IMA, normally define policy rules requiring the policy
> itself to be signed.   Instead of writing the policy rules, write the

The goal of this self test is to not fully test the IMA functionality but check
if the BPF helper works and returns a hash with the minimal possible IMA
config dependencies. And it seems like we can accomplish this by simply
writing the policy to securityfs directly.

From what I noticed, IMA_APPRAISE_REQUIRE_POLICY_SIGS
requires configuring a lot of other kernel options
(IMA_APPRAISE, ASYMMETRIC_KEYS etc.) that seem
like too much for bpf self tests to depend on.

I guess we can independently add selftests for IMA  which represent
a more real IMA configuration.  Hope this sounds reasonable?

> signed policy file pathname.  Refer to dracut commit 479b5cd9
> ("98integrity: support validating the IMA policy file signature").
>
> Both enabling IMA_APPRAISE_REQUIRE_POLICY_SIGS and the builtin
> "appraise_tcb" policy require loading a signed policy.

Thanks for the pointers.

- KP

>



> Mimi
>

Re: [PATCH bpf-next v3 3/3] bpf: Add a selftest for bpf_ima_inode_hash

[Mimi Zohar]

On Wed, 2020-11-25 at 03:55 +0100, KP Singh wrote:
> On Wed, Nov 25, 2020 at 3:20 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Tue, 2020-11-24 at 15:12 +0000, KP Singh wrote:
> > > diff --git a/tools/testing/selftests/bpf/ima_setup.sh b/tools/testing/selftests/bpf/ima_setup.sh
> > > new file mode 100644
> > > index 000000000000..15490ccc5e55
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/bpf/ima_setup.sh
> > > @@ -0,0 +1,80 @@
> > > +#!/bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +
> > > +set -e
> > > +set -u
> > > +
> > > +IMA_POLICY_FILE="/sys/kernel/security/ima/policy"
> > > +TEST_BINARY="/bin/true"
> > > +
> > > +usage()
> > > +{
> > > +        echo "Usage: $0 <setup|cleanup|run> <existing_tmp_dir>"
> > > +        exit 1
> > > +}
> > > +
> > > +setup()
> > > +{
> > > +        local tmp_dir="$1"
> > > +        local mount_img="${tmp_dir}/test.img"
> > > +        local mount_dir="${tmp_dir}/mnt"
> > > +        local copied_bin_path="${mount_dir}/$(basename ${TEST_BINARY})"
> > > +        mkdir -p ${mount_dir}
> > > +
> > > +        dd if=/dev/zero of="${mount_img}" bs=1M count=10
> > > +
> > > +        local loop_device="$(losetup --find --show ${mount_img})"
> > > +
> > > +        mkfs.ext4 "${loop_device}"
> > > +        mount "${loop_device}" "${mount_dir}"
> > > +
> > > +        cp "${TEST_BINARY}" "${mount_dir}"
> > > +        local mount_uuid="$(blkid -s UUID -o value ${loop_device})"
> > > +        echo "measure func=BPRM_CHECK fsuuid=${mount_uuid}" > ${IMA_POLICY_FILE}
> >
> > Anyone using IMA, normally define policy rules requiring the policy
> > itself to be signed.   Instead of writing the policy rules, write the
> 
> The goal of this self test is to not fully test the IMA functionality but check
> if the BPF helper works and returns a hash with the minimal possible IMA
> config dependencies. And it seems like we can accomplish this by simply
> writing the policy to securityfs directly.
> 
> From what I noticed, IMA_APPRAISE_REQUIRE_POLICY_SIGS
> requires configuring a lot of other kernel options
> (IMA_APPRAISE, ASYMMETRIC_KEYS etc.) that seem
> like too much for bpf self tests to depend on.
> 
> I guess we can independently add selftests for IMA  which represent
> a more real IMA configuration.  Hope this sounds reasonable?

Sure.  My point was that writing the policy rule might fail.

Mimi
> 
> > signed policy file pathname.  Refer to dracut commit 479b5cd9
> > ("98integrity: support validating the IMA policy file signature").
> >
> > Both enabling IMA_APPRAISE_REQUIRE_POLICY_SIGS and the builtin
> > "appraise_tcb" policy require loading a signed policy.
> 
> Thanks for the pointers.

Re: [PATCH bpf-next v3 3/3] bpf: Add a selftest for bpf_ima_inode_hash

[Mimi Zohar]

On Tue, 2020-11-24 at 15:12 +0000, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
> 
> The test does the following:
> 
> - Mounts a loopback filesystem and appends the IMA policy to measure
>   executions only on this file-system. Restricting the IMA policy to a
>   particular filesystem prevents a system-wide IMA policy change.
> - Executes an executable copied to this loopback filesystem.
> - Calls the bpf_ima_inode_hash in the bprm_committed_creds hook and
>   checks if the call succeeded and checks if a hash was calculated.
> 
> The test shells out to the added ima_setup.sh script as the setup is
> better handled in a shell script and is more complicated to do in the
> test program or even shelling out individual commands from C.
> 
> The list of required configs (i.e. IMA, SECURITYFS,
> IMA_{WRITE,READ}_POLICY) for running this test are also updated.
> 
> Signed-off-by: KP Singh <kpsingh@google.com>

Suggested-by: Mimi Zohar <zohar@linux.ibm.com> (limit policy rule to
loopback mount)

> ---
>  tools/testing/selftests/bpf/config            |  4 +
>  tools/testing/selftests/bpf/ima_setup.sh      | 80 +++++++++++++++++++
>  .../selftests/bpf/prog_tests/test_ima.c       | 74 +++++++++++++++++
>  tools/testing/selftests/bpf/progs/ima.c       | 28 +++++++
>  4 files changed, 186 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/ima_setup.sh
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/test_ima.c
>  create mode 100644 tools/testing/selftests/bpf/progs/ima.c
> 
> diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
> index 2118e23ac07a..365bf9771b07 100644
> --- a/tools/testing/selftests/bpf/config
> +++ b/tools/testing/selftests/bpf/config
> @@ -39,3 +39,7 @@ CONFIG_BPF_JIT=y
>  CONFIG_BPF_LSM=y
>  CONFIG_SECURITY=y
>  CONFIG_LIRC=y
> +CONFIG_IMA=y
> +CONFIG_SECURITYFS=y
> +CONFIG_IMA_WRITE_POLICY=y
> +CONFIG_IMA_READ_POLICY=y
> diff --git a/tools/testing/selftests/bpf/ima_setup.sh b/tools/testing/selftests/bpf/ima_setup.sh
> new file mode 100644
> index 000000000000..15490ccc5e55
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/ima_setup.sh
> @@ -0,0 +1,80 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +
> +set -e
> +set -u
> +
> +IMA_POLICY_FILE="/sys/kernel/security/ima/policy"
> +TEST_BINARY="/bin/true"
> +
> +usage()
> +{
> +        echo "Usage: $0 <setup|cleanup|run> <existing_tmp_dir>"
> +        exit 1
> +}
> +
> +setup()
> +{
> +        local tmp_dir="$1"
> +        local mount_img="${tmp_dir}/test.img"
> +        local mount_dir="${tmp_dir}/mnt"
> +        local copied_bin_path="${mount_dir}/$(basename ${TEST_BINARY})"
> +        mkdir -p ${mount_dir}
> +
> +        dd if=/dev/zero of="${mount_img}" bs=1M count=10
> +
> +        local loop_device="$(losetup --find --show ${mount_img})"
> +
> +        mkfs.ext4 "${loop_device}"
> +        mount "${loop_device}" "${mount_dir}"
> +
> +        cp "${TEST_BINARY}" "${mount_dir}"
> +        local mount_uuid="$(blkid -s UUID -o value ${loop_device})"
> +        echo "measure func=BPRM_CHECK fsuuid=${mount_uuid}" > ${IMA_POLICY_FILE}
> +}
> +
> +cleanup() {
> +        local tmp_dir="$1"
> +        local mount_img="${tmp_dir}/test.img"
> +        local mount_dir="${tmp_dir}/mnt"
> +
> +        local loop_devices=$(losetup -j ${mount_img} -O NAME --noheadings)
> +        for loop_dev in "${loop_devices}"; do
> +                losetup -d $loop_dev
> +        done
> +
> +        umount ${mount_dir}
> +        rm -rf ${tmp_dir}
> +}
> +
> +run()
> +{
> +        local tmp_dir="$1"
> +        local mount_dir="${tmp_dir}/mnt"
> +        local copied_bin_path="${mount_dir}/$(basename ${TEST_BINARY})"
> +
> +        exec "${copied_bin_path}"
> +}
> +
> +main()
> +{
> +        [[ $# -ne 2 ]] && usage
> +
> +        local action="$1"
> +        local tmp_dir="$2"
> +
> +        [[ ! -d "${tmp_dir}" ]] && echo "Directory ${tmp_dir} doesn't exist" && exit 1
> +
> +        if [[ "${action}" == "setup" ]]; then
> +                setup "${tmp_dir}"
> +        elif [[ "${action}" == "cleanup" ]]; then
> +                cleanup "${tmp_dir}"
> +        elif [[ "${action}" == "run" ]]; then
> +                run "${tmp_dir}"
> +        else
> +                echo "Unknown action: ${action}"
> +                exit 1
> +        fi
> +}
> +
> +main "$@"
> diff --git a/tools/testing/selftests/bpf/prog_tests/test_ima.c b/tools/testing/selftests/bpf/prog_tests/test_ima.c
> new file mode 100644
> index 000000000000..61fca681d524
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/test_ima.c
> @@ -0,0 +1,74 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Copyright (C) 2020 Google LLC.
> + */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <sys/wait.h>
> +#include <test_progs.h>
> +
> +#include "ima.skel.h"
> +
> +static int run_measured_process(const char *measured_dir, u32 *monitored_pid)
> +{
> +	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,
> +		       NULL);
> +		exit(errno);
> +
> +	} else if (child_pid > 0) {
> +		waitpid(child_pid, &child_status, 0);
> +		return WEXITSTATUS(child_status);
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +void test_test_ima(void)
> +{
> +	char measured_dir_template[] = "/tmp/ima_measuredXXXXXX";
> +	const char *measured_dir;
> +	char cmd[256];
> +
> +	int err, duration = 0;
> +	struct ima *skel = NULL;
> +
> +	skel = ima__open_and_load();
> +	if (CHECK(!skel, "skel_load", "skeleton failed\n"))
> +		goto close_prog;
> +
> +	err = ima__attach(skel);
> +	if (CHECK(err, "attach", "attach failed: %d\n", err))
> +		goto close_prog;
> +
> +	measured_dir = mkdtemp(measured_dir_template);
> +	if (CHECK(measured_dir == NULL, "mkdtemp", "err %d\n", errno))
> +		goto close_prog;
> +
> +	snprintf(cmd, sizeof(cmd), "./ima_setup.sh setup %s", measured_dir);
> +	if (CHECK_FAIL(system(cmd)))
> +		goto close_clean;
> +
> +	err = run_measured_process(measured_dir, &skel->bss->monitored_pid);
> +	if (CHECK(err, "run_measured_process", "err = %d\n", err))
> +		goto close_clean;
> +
> +	CHECK(skel->data->ima_hash_ret < 0, "ima_hash_ret",
> +	      "ima_hash_ret = %ld\n", skel->data->ima_hash_ret);
> +
> +	CHECK(skel->bss->ima_hash == 0, "ima_hash",
> +	      "ima_hash = %lu\n", skel->bss->ima_hash);
> +
> +close_clean:
> +	snprintf(cmd, sizeof(cmd), "./ima_setup.sh cleanup %s", measured_dir);
> +	CHECK_FAIL(system(cmd));
> +close_prog:
> +	ima__destroy(skel);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/ima.c b/tools/testing/selftests/bpf/progs/ima.c
> new file mode 100644
> index 000000000000..86b21aff4bc5
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/ima.c
> @@ -0,0 +1,28 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Copyright 2020 Google LLC.
> + */
> +
> +#include "vmlinux.h"
> +#include <errno.h>
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +
> +long ima_hash_ret = -1;
> +u64 ima_hash = 0;
> +u32 monitored_pid = 0;
> +
> +char _license[] SEC("license") = "GPL";
> +
> +SEC("lsm.s/bprm_committed_creds")
> +int BPF_PROG(ima, struct linux_binprm *bprm)
> +{
> +	u32 pid = bpf_get_current_pid_tgid() >> 32;
> +
> +	if (pid == monitored_pid)
> +		ima_hash_ret = bpf_ima_inode_hash(bprm->file->f_inode,
> +						  &ima_hash, sizeof(ima_hash));
> +
> +	return 0;
> +}

Re: [PATCH bpf-next v3 3/3] bpf: Add a selftest for bpf_ima_inode_hash

[Yonghong Song]

On 11/24/20 7:12 AM, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
> 
> The test does the following:
> 
> - Mounts a loopback filesystem and appends the IMA policy to measure
>    executions only on this file-system. Restricting the IMA policy to a
>    particular filesystem prevents a system-wide IMA policy change.
> - Executes an executable copied to this loopback filesystem.
> - Calls the bpf_ima_inode_hash in the bprm_committed_creds hook and
>    checks if the call succeeded and checks if a hash was calculated.
> 
> The test shells out to the added ima_setup.sh script as the setup is
> better handled in a shell script and is more complicated to do in the
> test program or even shelling out individual commands from C.
> 
> The list of required configs (i.e. IMA, SECURITYFS,
> IMA_{WRITE,READ}_POLICY) for running this test are also updated.
> 
> Signed-off-by: KP Singh <kpsingh@google.com>
> ---
>   tools/testing/selftests/bpf/config            |  4 +
>   tools/testing/selftests/bpf/ima_setup.sh      | 80 +++++++++++++++++++
>   .../selftests/bpf/prog_tests/test_ima.c       | 74 +++++++++++++++++
>   tools/testing/selftests/bpf/progs/ima.c       | 28 +++++++
>   4 files changed, 186 insertions(+)
>   create mode 100644 tools/testing/selftests/bpf/ima_setup.sh
>   create mode 100644 tools/testing/selftests/bpf/prog_tests/test_ima.c
>   create mode 100644 tools/testing/selftests/bpf/progs/ima.c
> 
> diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
> index 2118e23ac07a..365bf9771b07 100644
> --- a/tools/testing/selftests/bpf/config
> +++ b/tools/testing/selftests/bpf/config
> @@ -39,3 +39,7 @@ CONFIG_BPF_JIT=y
>   CONFIG_BPF_LSM=y
>   CONFIG_SECURITY=y
>   CONFIG_LIRC=y
> +CONFIG_IMA=y
> +CONFIG_SECURITYFS=y
> +CONFIG_IMA_WRITE_POLICY=y
> +CONFIG_IMA_READ_POLICY=y
> diff --git a/tools/testing/selftests/bpf/ima_setup.sh b/tools/testing/selftests/bpf/ima_setup.sh
> new file mode 100644
> index 000000000000..15490ccc5e55
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/ima_setup.sh
> @@ -0,0 +1,80 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +
> +set -e
> +set -u
> +
> +IMA_POLICY_FILE="/sys/kernel/security/ima/policy"
> +TEST_BINARY="/bin/true"
> +
> +usage()
> +{
> +        echo "Usage: $0 <setup|cleanup|run> <existing_tmp_dir>"
> +        exit 1
> +}
> +
> +setup()
> +{
> +        local tmp_dir="$1"
> +        local mount_img="${tmp_dir}/test.img"
> +        local mount_dir="${tmp_dir}/mnt"
> +        local copied_bin_path="${mount_dir}/$(basename ${TEST_BINARY})"
> +        mkdir -p ${mount_dir}
> +
> +        dd if=/dev/zero of="${mount_img}" bs=1M count=10
> +
> +        local loop_device="$(losetup --find --show ${mount_img})"
> +
> +        mkfs.ext4 "${loop_device}"
> +        mount "${loop_device}" "${mount_dir}"
> +
> +        cp "${TEST_BINARY}" "${mount_dir}"
> +        local mount_uuid="$(blkid -s UUID -o value ${loop_device})"
> +        echo "measure func=BPRM_CHECK fsuuid=${mount_uuid}" > ${IMA_POLICY_FILE}
> +}
> +
> +cleanup() {
> +        local tmp_dir="$1"
> +        local mount_img="${tmp_dir}/test.img"
> +        local mount_dir="${tmp_dir}/mnt"
> +
> +        local loop_devices=$(losetup -j ${mount_img} -O NAME --noheadings)
> +        for loop_dev in "${loop_devices}"; do
> +                losetup -d $loop_dev
> +        done
> +
> +        umount ${mount_dir}
> +        rm -rf ${tmp_dir}
> +}
> +
> +run()
> +{
> +        local tmp_dir="$1"
> +        local mount_dir="${tmp_dir}/mnt"
> +        local copied_bin_path="${mount_dir}/$(basename ${TEST_BINARY})"
> +
> +        exec "${copied_bin_path}"
> +}
> +
> +main()
> +{
> +        [[ $# -ne 2 ]] && usage
> +
> +        local action="$1"
> +        local tmp_dir="$2"
> +
> +        [[ ! -d "${tmp_dir}" ]] && echo "Directory ${tmp_dir} doesn't exist" && exit 1
> +
> +        if [[ "${action}" == "setup" ]]; then
> +                setup "${tmp_dir}"
> +        elif [[ "${action}" == "cleanup" ]]; then
> +                cleanup "${tmp_dir}"
> +        elif [[ "${action}" == "run" ]]; then
> +                run "${tmp_dir}"
> +        else
> +                echo "Unknown action: ${action}"
> +                exit 1
> +        fi
> +}
> +
> +main "$@"


> diff --git a/tools/testing/selftests/bpf/prog_tests/test_ima.c b/tools/testing/selftests/bpf/prog_tests/test_ima.c
> new file mode 100644
> index 000000000000..61fca681d524
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/test_ima.c
> @@ -0,0 +1,74 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Copyright (C) 2020 Google LLC.
> + */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <sys/wait.h>
> +#include <test_progs.h>
> +
> +#include "ima.skel.h"
> +
> +static int run_measured_process(const char *measured_dir, u32 *monitored_pid)
> +{
> +	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,
> +		       NULL);
> +		exit(errno);

Running test_progs-no-alu32, the test failed as:

root@arch-fb-vm1:~/net-next/net-next/tools/testing/selftests/bpf 
./test_progs-no_alu32 -t test_ima 

sh: ./ima_setup.sh: No such file or directory 

sh: ./ima_setup.sh: No such file or directory 

test_test_ima:PASS:skel_load 0 nsec 

test_test_ima:PASS:attach 0 nsec 

test_test_ima:PASS:mkdtemp 0 nsec 

test_test_ima:FAIL:56 

test_test_ima:FAIL:71 

#114 test_ima:FAIL 

Summary: 0/0 PASSED, 0 SKIPPED, 1 FAILED

Although the file is indeed in this directory:
root@arch-fb-vm1:~/net-next/net-next/tools/testing/selftests/bpf ls 
ima_setup.sh
ima_setup.sh

I think the execution actually tries to get file from
no_alu32 directory to avoid reusing the same files in
.../testing/selftests/bpf for -mcpu=v3 purpose.

The following change, which copies ima_setup.sh to
no_alu32 directory, seems fixing the issue:

TRUNNER_EXTRA_SOURCES := test_progs.c cgroup_helpers.c trace_helpers.c 
     \
                          network_helpers.c testing_helpers.c            \
                          btf_helpers.c  flow_dissector_load.h
  TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read                          \
+                      ima_setup.sh                                     \
                        $(wildcard progs/btf_dump_test_case_*.c)
  TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE
  TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS)

Could you do a followup on this?

> +
> +	} else if (child_pid > 0) {
> +		waitpid(child_pid, &child_status, 0);
> +		return WEXITSTATUS(child_status);
> +	}
> +
> +	return -EINVAL;
> +}
> +
[...]

Re: [PATCH bpf-next v3 3/3] bpf: Add a selftest for bpf_ima_inode_hash

[KP Singh]

[...]

> > +             exit(errno);
>
> Running test_progs-no-alu32, the test failed as:
>
> root@arch-fb-vm1:~/net-next/net-next/tools/testing/selftests/bpf
> ./test_progs-no_alu32 -t test_ima

Note to self: Also start testing test_progs-no_alu32

>
> sh: ./ima_setup.sh: No such file or directory
>
> sh: ./ima_setup.sh: No such file or directory
>
> test_test_ima:PASS:skel_load 0 nsec
>
> test_test_ima:PASS:attach 0 nsec
>
> test_test_ima:PASS:mkdtemp 0 nsec
>
> test_test_ima:FAIL:56
>
> test_test_ima:FAIL:71
>
> #114 test_ima:FAIL
>
> Summary: 0/0 PASSED, 0 SKIPPED, 1 FAILED
>
> Although the file is indeed in this directory:
> root@arch-fb-vm1:~/net-next/net-next/tools/testing/selftests/bpf ls
> ima_setup.sh
> ima_setup.sh
>
> I think the execution actually tries to get file from
> no_alu32 directory to avoid reusing the same files in
> .../testing/selftests/bpf for -mcpu=v3 purpose.
>
> The following change, which copies ima_setup.sh to
> no_alu32 directory, seems fixing the issue:

Thanks!

>
> TRUNNER_EXTRA_SOURCES := test_progs.c cgroup_helpers.c trace_helpers.c
>      \
>                           network_helpers.c testing_helpers.c            \
>                           btf_helpers.c  flow_dissector_load.h
>   TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read                          \
> +                      ima_setup.sh                                     \
>                         $(wildcard progs/btf_dump_test_case_*.c)
>   TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE
>   TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS)
>
> Could you do a followup on this?

Yes, I will send out a fix today.

- KP

Re: [PATCH bpf-next v3 3/3] bpf: Add a selftest for bpf_ima_inode_hash

[Andrii Nakryiko]

On Tue, Nov 24, 2020 at 7:16 AM KP Singh <kpsingh@chromium.org> wrote:
>
> From: KP Singh <kpsingh@google.com>
>
> The test does the following:
>
> - Mounts a loopback filesystem and appends the IMA policy to measure
>   executions only on this file-system. Restricting the IMA policy to a
>   particular filesystem prevents a system-wide IMA policy change.
> - Executes an executable copied to this loopback filesystem.
> - Calls the bpf_ima_inode_hash in the bprm_committed_creds hook and
>   checks if the call succeeded and checks if a hash was calculated.
>
> The test shells out to the added ima_setup.sh script as the setup is
> better handled in a shell script and is more complicated to do in the
> test program or even shelling out individual commands from C.
>
> The list of required configs (i.e. IMA, SECURITYFS,
> IMA_{WRITE,READ}_POLICY) for running this test are also updated.
>
> Signed-off-by: KP Singh <kpsingh@google.com>
> ---
>  tools/testing/selftests/bpf/config            |  4 +
>  tools/testing/selftests/bpf/ima_setup.sh      | 80 +++++++++++++++++++
>  .../selftests/bpf/prog_tests/test_ima.c       | 74 +++++++++++++++++
>  tools/testing/selftests/bpf/progs/ima.c       | 28 +++++++
>  4 files changed, 186 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/ima_setup.sh
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/test_ima.c
>  create mode 100644 tools/testing/selftests/bpf/progs/ima.c
>

[...]

> +cleanup() {
> +        local tmp_dir="$1"
> +        local mount_img="${tmp_dir}/test.img"
> +        local mount_dir="${tmp_dir}/mnt"
> +
> +        local loop_devices=$(losetup -j ${mount_img} -O NAME --noheadings)

libbpf and kernel-patches CIs are using BusyBox environment which has
losetup that doesn't support -j option. Is there some way to work
around that? What we have is this:

BusyBox v1.31.1 () multi-call binary.

Usage: losetup [-rP] [-o OFS] {-f|LOOPDEV} FILE: associate loop devices

    losetup -c LOOPDEV: reread file size

    losetup -d LOOPDEV: disassociate

    losetup -a: show status

    losetup -f: show next free loop device

    -o OFS    Start OFS bytes into FILE

    -P    Scan for partitions

    -r    Read-only

    -f    Show/use next free loop device


> +        for loop_dev in "${loop_devices}"; do
> +                losetup -d $loop_dev
> +        done
> +
> +        umount ${mount_dir}
> +        rm -rf ${tmp_dir}
> +}
> +

[...]

Re: [PATCH bpf-next v3 3/3] bpf: Add a selftest for bpf_ima_inode_hash

[KP Singh]

On Fri, Nov 27, 2020 at 5:29 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Tue, Nov 24, 2020 at 7:16 AM KP Singh <kpsingh@chromium.org> wrote:
> >
> > From: KP Singh <kpsingh@google.com>
> >

[...]

>
> > +cleanup() {
> > +        local tmp_dir="$1"
> > +        local mount_img="${tmp_dir}/test.img"
> > +        local mount_dir="${tmp_dir}/mnt"
> > +
> > +        local loop_devices=$(losetup -j ${mount_img} -O NAME --noheadings)
>
> libbpf and kernel-patches CIs are using BusyBox environment which has
> losetup that doesn't support -j option. Is there some way to work
> around that? What we have is this:
>
> BusyBox v1.31.1 () multi-call binary.
>
> Usage: losetup [-rP] [-o OFS] {-f|LOOPDEV} FILE: associate loop devices
>
>     losetup -c LOOPDEV: reread file size
>
>     losetup -d LOOPDEV: disassociate
>
>     losetup -a: show status

I can try to grep and parse the status output as a fallback. Will send another
fix.

- KP

>
>     losetup -f: show next free loop device
>
>     -o OFS    Start OFS bytes into FILE
>
>     -P    Scan for partitions
>
>     -r    Read-only
>
>     -f    Show/use next free loop device
>
>
> > +        for loop_dev in "${loop_devices}"; do

[...]

Re: [PATCH bpf-next v3 0/3] Implement bpf_ima_inode_hash

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (refs/heads/master):

On Tue, 24 Nov 2020 15:12:07 +0000 you wrote:
> From: KP Singh <kpsingh@google.com>
> 
> # v2 -> v3
> 
> - Fixed an issue pointed out by Alexei, the helper should only be
>   exposed to sleepable hooks.
> - Update the selftests to constrain the IMA policy udpate to a loopback
>   filesystem specifically created for the test. Also, split this out
>   from the LSM test. I dropped the Ack from this last patch since this
>   is a re-write.
> 
> [...]

Here is the summary with links:
  - [bpf-next,v3,1/3] ima: Implement ima_inode_hash
    https://git.kernel.org/bpf/bpf-next/c/403319be5de5
  - [bpf-next,v3,2/3] bpf: Add a BPF helper for getting the IMA hash of an inode
    https://git.kernel.org/bpf/bpf-next/c/27672f0d280a
  - [bpf-next,v3,3/3] bpf: Add a selftest for bpf_ima_inode_hash
    https://git.kernel.org/bpf/bpf-next/c/898eeb122e8a

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