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

2020-11-27 Thread KP Singh
On Fri, Nov 27, 2020 at 5:29 AM Andrii Nakryiko
 wrote:
>
> On Tue, Nov 24, 2020 at 7:16 AM KP Singh  wrote:
> >
> > From: KP Singh 
> >

[...]

>
> > +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 OFSStart OFS bytes into FILE
>
> -PScan for partitions
>
> -rRead-only
>
> -fShow/use next free loop device
>
>
> > +for loop_dev in "${loop_devices}"; do

[...]


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

2020-11-26 Thread Andrii Nakryiko
On Tue, Nov 24, 2020 at 7:16 AM KP Singh  wrote:
>
> From: KP Singh 
>
> 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 
> ---
>  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 OFSStart OFS bytes into FILE

-PScan for partitions

-rRead-only

-fShow/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

2020-11-26 Thread 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

2020-11-25 Thread Yonghong Song




On 11/24/20 7:12 AM, KP Singh wrote:

From: KP Singh 

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 
---
  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 ..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  "
+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 ..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 
+#include 
+#include 
+#include 
+#include 
+
+#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 


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

2020-11-25 Thread Mimi Zohar
On Tue, 2020-11-24 at 15:12 +, KP Singh wrote:
> From: KP Singh 
> 
> 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 

Suggested-by: Mimi Zohar  (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 ..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  "
> +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 ..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 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#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", 

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

2020-11-24 Thread 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  wrote:
> >
> > On Tue, 2020-11-24 at 15:12 +, 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 ..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  "
> > > +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

2020-11-24 Thread KP Singh
On Wed, Nov 25, 2020 at 3:20 AM Mimi Zohar  wrote:
>
> On Tue, 2020-11-24 at 15:12 +, 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 ..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  "
> > +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

2020-11-24 Thread Mimi Zohar
On Tue, 2020-11-24 at 15:12 +, 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 ..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  "
> +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

2020-11-24 Thread Yonghong Song




On 11/24/20 7:12 AM, KP Singh wrote:

From: KP Singh 

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 


Ack from bpf perspective,

Acked-by: Yonghong Song 

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 ..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  "
+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 ..61fca681d524

[...]


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

2020-11-24 Thread KP Singh
From: KP Singh 

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 
---
 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 ..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  "
+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 ..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 
+#include 
+#include 
+#include 
+#include 
+
+#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, _status, 0);
+   return WEXITSTATUS(child_status);
+   }
+
+   return -EINVAL;
+}
+
+void test_test_ima(void)
+{
+   char measured_dir_template[] = "/tmp/ima_measuredXX";
+   const char *measured_dir;
+   char cmd[256];
+