[PATCH 0/3] LTP tests: load predefined policy

[PATCH 0/3] LTP tests: load predefined policy

[Petr Vorel]

From: Petr Vorel <petr.vorel@gmail.com>

Hi Mimi, all,

this effort allows to load policy LTP provides as example
via LTP_IMA_LOAD_POLICY=1 environment variable (off by default).

This should allow better coverage for these who just run runtest/ima.
But it requires tooling which runs LTP to do the restart after each
test.

Kind regards,
Petr

Petr Vorel (3):
  ima: Add TCB policy as an example
  ima_setup.sh: Allow to load predefined policy
  ima_{kexec,keys,selinux}: Set minimal kernel version

 .../kernel/security/integrity/ima/README.md   |  6 +++
 .../ima/datafiles/ima_measurements/tcb.policy | 20 +++++++
 .../security/integrity/ima/tests/ima_kexec.sh |  1 +
 .../security/integrity/ima/tests/ima_keys.sh  |  1 +
 .../integrity/ima/tests/ima_measurements.sh   | 17 +++++-
 .../integrity/ima/tests/ima_selinux.sh        |  1 +
 .../security/integrity/ima/tests/ima_setup.sh | 52 ++++++++++++++++---
 7 files changed, 89 insertions(+), 9 deletions(-)
 create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_measurements/tcb.policy

-- 
2.45.2

[PATCH 1/3] ima: Add TCB policy as an example

[Petr Vorel]

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 .../ima/datafiles/ima_measurements/tcb.policy | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_measurements/tcb.policy

diff --git a/testcases/kernel/security/integrity/ima/datafiles/ima_measurements/tcb.policy b/testcases/kernel/security/integrity/ima/datafiles/ima_measurements/tcb.policy
new file mode 100644
index 0000000000..280e6af87c
--- /dev/null
+++ b/testcases/kernel/security/integrity/ima/datafiles/ima_measurements/tcb.policy
@@ -0,0 +1,20 @@
+dont_measure fsmagic=0x9fa0
+dont_measure fsmagic=0x62656572
+dont_measure fsmagic=0x64626720
+dont_measure fsmagic=0x1021994
+dont_measure fsmagic=0x1cd1
+dont_measure fsmagic=0x42494e4d
+dont_measure fsmagic=0x73636673
+dont_measure fsmagic=0xf97cff8c
+dont_measure fsmagic=0x43415d53
+dont_measure fsmagic=0x27e0eb
+dont_measure fsmagic=0x63677270
+dont_measure fsmagic=0x6e736673
+dont_measure fsmagic=0xde5e81e4
+measure func=MMAP_CHECK mask=MAY_EXEC
+measure func=BPRM_CHECK mask=MAY_EXEC
+measure func=FILE_CHECK mask=^MAY_READ euid=0
+measure func=FILE_CHECK mask=^MAY_READ uid=0
+measure func=MODULE_CHECK
+measure func=FIRMWARE_CHECK
+measure func=POLICY_CHECK
-- 
2.45.2

Re: [PATCH 1/3] ima: Add TCB policy as an example

[Mimi Zohar]

Hi Petr,

On Tue, 2024-11-26 at 18:38 +0100, Petr Vorel wrote:
> Signed-off-by: Petr Vorel <pvorel@suse.cz>

Except for the "dont_measure <tmpfs>" rule, the "dont_measure" rules are for
pseudo filesystems.  Including a "dont_measure <tmpfs>" policy rule was suppose
to be limited to the initramfs, and then replaced with an IMA custom policy. I
would either re-order the rules so that the "dont_measure" rules are only before
the two "func=FILE_CHECK" rules or perhaps remove the "dont_measure <tmpfs>"
policy rule.  The kernel builtin "tcb" policy should be updated as described
below.

> ---
>  .../ima/datafiles/ima_measurements/tcb.policy | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
>  create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_measurements/tcb.policy
> 
> diff --git a/testcases/kernel/security/integrity/ima/datafiles/ima_measurements/tcb.policy b/testcases/kernel/security/integrity/ima/datafiles/ima_measurements/tcb.policy
> new file mode 100644
> index 0000000000..280e6af87c
> --- /dev/null
> +++ b/testcases/kernel/security/integrity/ima/datafiles/ima_measurements/tcb.policy
> @@ -0,0 +1,20 @@
> +dont_measure fsmagic=0x9fa0
> +dont_measure fsmagic=0x62656572
> +dont_measure fsmagic=0x64626720
> +dont_measure fsmagic=0x1021994
> +dont_measure fsmagic=0x1cd1
> +dont_measure fsmagic=0x42494e4d
> +dont_measure fsmagic=0x73636673
> +dont_measure fsmagic=0xf97cff8c
> +dont_measure fsmagic=0x43415d53
> +dont_measure fsmagic=0x27e0eb
> +dont_measure fsmagic=0x63677270
> +dont_measure fsmagic=0x6e736673
> +dont_measure fsmagic=0xde5e81e4

Limit the affect of "dont_measure" rules to just the "func=FILE_CHECK" rules, by
moving them to before the "func=FILE_CHECK" rules.

> +measure func=MMAP_CHECK mask=MAY_EXEC
> +measure func=BPRM_CHECK mask=MAY_EXEC
> +measure func=FILE_CHECK mask=^MAY_READ euid=0
> +measure func=FILE_CHECK mask=^MAY_READ uid=0

Move above two "func=FILE_CHECK" rules to the end.

> +measure func=MODULE_CHECK
> +measure func=FIRMWARE_CHECK
> +measure func=POLICY_CHECK

thanks,

Mimi

Re: [PATCH 1/3] ima: Add TCB policy as an example

[Petr Vorel]

Hi Mimi,

> Hi Petr,

> On Tue, 2024-11-26 at 18:38 +0100, Petr Vorel wrote:
> > Signed-off-by: Petr Vorel <pvorel@suse.cz>

> Except for the "dont_measure <tmpfs>" rule, the "dont_measure" rules are for
> pseudo filesystems.  Including a "dont_measure <tmpfs>" policy rule was suppose
> to be limited to the initramfs, and then replaced with an IMA custom policy. I
> would either re-order the rules so that the "dont_measure" rules are only before
> the two "func=FILE_CHECK" rules or perhaps remove the "dont_measure <tmpfs>"
> policy rule.  The kernel builtin "tcb" policy should be updated as described
> below.


> > ---
> >  .../ima/datafiles/ima_measurements/tcb.policy | 20 +++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >  create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_measurements/tcb.policy

> > diff --git a/testcases/kernel/security/integrity/ima/datafiles/ima_measurements/tcb.policy b/testcases/kernel/security/integrity/ima/datafiles/ima_measurements/tcb.policy
> > new file mode 100644
> > index 0000000000..280e6af87c
> > --- /dev/null
> > +++ b/testcases/kernel/security/integrity/ima/datafiles/ima_measurements/tcb.policy
> > @@ -0,0 +1,20 @@
> > +dont_measure fsmagic=0x9fa0
> > +dont_measure fsmagic=0x62656572
> > +dont_measure fsmagic=0x64626720
> > +dont_measure fsmagic=0x1021994
> > +dont_measure fsmagic=0x1cd1
> > +dont_measure fsmagic=0x42494e4d
> > +dont_measure fsmagic=0x73636673
> > +dont_measure fsmagic=0xf97cff8c
> > +dont_measure fsmagic=0x43415d53
> > +dont_measure fsmagic=0x27e0eb
> > +dont_measure fsmagic=0x63677270
> > +dont_measure fsmagic=0x6e736673
> > +dont_measure fsmagic=0xde5e81e4

> Limit the affect of "dont_measure" rules to just the "func=FILE_CHECK" rules, by
> moving them to before the "func=FILE_CHECK" rules.

> > +measure func=MMAP_CHECK mask=MAY_EXEC
> > +measure func=BPRM_CHECK mask=MAY_EXEC
> > +measure func=FILE_CHECK mask=^MAY_READ euid=0
> > +measure func=FILE_CHECK mask=^MAY_READ uid=0

> Move above two "func=FILE_CHECK" rules to the end.

> > +measure func=MODULE_CHECK
> > +measure func=FIRMWARE_CHECK
> > +measure func=POLICY_CHECK

Thanks for your comments. You probably mean ima_policy=tcb from [1], right?

dont_measure fsmagic=0x9fa0          # PROC_SUPER_MAGIC
dont_measure fsmagic=0x62656572      # SYSFS_MAGIC
dont_measure fsmagic=0x64626720      # DEBUGFS_MAGIC
dont_measure fsmagic=0x1021994       # TMPFS_MAGIC
dont_measure fsmagic=0x1cd1          # DEVPTS_SUPER_MAGIC
dont_measure fsmagic=0x42494e4d      # BINFMTFS_MAGIC
dont_measure fsmagic=0x73636673      # SECURITYFS_MAGIC
dont_measure fsmagic=0xf97cff8c      # SELINUX_MAGIC
dont_measure fsmagic=0x43415d53      # SMACK_MAGIC
dont_measure fsmagic=0x27e0eb        # CGROUP_SUPER_MAGIC
dont_measure fsmagic=0x63677270      # CGROUP2_SUPER_MAGIC
dont_measure fsmagic=0x6e736673      # NSFS_MAGIC
dont_measure fsmagic=0xde5e81e4      # EFIVARFS_MAGIC
measure func=MMAP_CHECK mask=MAY_EXEC
measure func=BPRM_CHECK mask=MAY_EXEC           # binary executed
measure func=FILE_CHECK mask=^MAY_READ euid=0
measure func=FILE_CHECK mask=^MAY_READ uid=0    # root opened r/o, r/w
measure func=MODULE_CHECK
measure func=FIRMWARE_CHECK
measure func=POLICY_CHECK

without: dont_measure fsmagic=0x1021994       # TMPFS_MAGIC
(I would remove the comments from final policy file)

Or if not, feel free to send a patch with optimal policy.

Kind regards,
Petr

[1] https://ima-doc.readthedocs.io/en/latest/ima-policy.html#ima-tcb

> thanks,

> Mimi

[PATCH 2/3] ima_setup.sh: Allow to load predefined policy

[Petr Vorel]

environment variable LTP_IMA_LOAD_POLICY=1 tries to load example policy
if available. This should be used only if tooling running LTP tests
allows to reboot afterwards (because policy may be writable only once,
e.g. missing CONFIG_IMA_WRITE_POLICY=y, or policies can influence each
other).

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 .../kernel/security/integrity/ima/README.md   |  6 +++
 .../integrity/ima/tests/ima_measurements.sh   | 17 +++++-
 .../security/integrity/ima/tests/ima_setup.sh | 52 ++++++++++++++++---
 3 files changed, 66 insertions(+), 9 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/README.md b/testcases/kernel/security/integrity/ima/README.md
index 5b261a1914..a00b01b5fe 100644
--- a/testcases/kernel/security/integrity/ima/README.md
+++ b/testcases/kernel/security/integrity/ima/README.md
@@ -8,6 +8,12 @@ CONFIG_INTEGRITY=y
 CONFIG_IMA=y
 ```
 
+### Loading policy for testing (optional)
+Setting environment variable `LTP_IMA_LOAD_POLICY=1` tries to load example
+policy if available. This should be used only if tooling running LTP tests
+allows to reboot afterwards (because policy may be writable only once, e.g.
+missing `CONFIG_IMA_WRITE_POLICY=y`, or policies can influence each other).
+
 ### IMA measurement tests
 `ima_measurements.sh` require builtin IMA tcb policy to be loaded
 (`ima_policy=tcb` kernel parameter).
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
index 1da2aa6a51..b4205ab95f 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
@@ -1,7 +1,7 @@
 #!/bin/sh
 # SPDX-License-Identifier: GPL-2.0-or-later
 # Copyright (c) 2009 IBM Corporation
-# Copyright (c) 2018-2021 Petr Vorel <pvorel@suse.cz>
+# Copyright (c) 2018-2024 Petr Vorel <pvorel@suse.cz>
 # Author: Mimi Zohar <zohar@linux.ibm.com>
 #
 # Verify that measurements are added to the measurement list based on policy.
@@ -12,10 +12,23 @@ TST_CNT=3
 
 setup()
 {
-	require_ima_policy_cmdline "tcb"
+	local policy="tcb"
 
 	TEST_FILE="$PWD/test.txt"
 	[ -f "$IMA_POLICY" ] || tst_res TINFO "not using default policy"
+
+	if [ "$LTP_IMA_LOAD_POLICY" != 1 ]; then
+		require_ima_policy_cmdline $policy
+		return
+	elif check_ima_policy_cmdline $policy; then
+		return
+	fi
+
+	if ! check_ima_policy_cmdline $policy &&
+		! require_ima_policy_content '^measure func=FILE_CHECK mask=^MAY_READ uid=0' &&
+		! require_ima_policy_content 'measure func=POLICY_CHECK'; then
+		tst_brk TCONF "IMA measurement tests require builtin IMA $policy policy (e.g. ima_policy=$policy kernel parameter or it's equivalent)"
+	fi
 }
 
 check_iversion_support()
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
index df3fc5603f..e585418c17 100644
--- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
@@ -1,7 +1,7 @@
 #!/bin/sh
 # SPDX-License-Identifier: GPL-2.0-or-later
 # Copyright (c) 2009 IBM Corporation
-# Copyright (c) 2018-2020 Petr Vorel <pvorel@suse.cz>
+# Copyright (c) 2018-2024 Petr Vorel <pvorel@suse.cz>
 # Author: Mimi Zohar <zohar@linux.ibm.com>
 
 TST_TESTFUNC="test"
@@ -72,14 +72,20 @@ require_policy_readable()
 	fi
 }
 
-require_policy_writable()
+check_policy_writable()
 {
-	local err="IMA policy already loaded and kernel not configured to enable multiple writes to it (need CONFIG_IMA_WRITE_POLICY=y)"
-
-	[ -f $IMA_POLICY ] || tst_brk TCONF "$err"
-	# CONFIG_IMA_READ_POLICY
+	[ -f $IMA_POLICY ] || return 1
+	# workaround for kernels < v4.18 without fix
+	# ffb122de9a60b ("ima: Reflect correct permissions for policy")
 	echo "" 2> log > $IMA_POLICY
-	grep -q "Device or resource busy" log && tst_brk TCONF "$err"
+	grep -q "Device or resource busy" log && return 1
+	return 0
+}
+
+require_policy_writable()
+{
+	check_policy_writable || tst_brk TCONF \
+		"IMA policy already loaded and kernel not configured to enable multiple writes to it (need CONFIG_IMA_WRITE_POLICY=y)"
 }
 
 check_ima_policy_content()
@@ -158,6 +164,32 @@ print_ima_config()
 	tst_res TINFO "/proc/cmdline: $(cat /proc/cmdline)"
 }
 
+load_ima_policy()
+{
+	local policy="$(ls $TST_DATAROOT/*.policy 2>/dev/null)"
+
+	if [ "$LTP_IMA_LOAD_POLICY" != 1 -a "$policy" -a -f "$policy" ]; then
+		tst_res TINFO "NOTE: set LTP_IMA_LOAD_POLICY=1 to load policy for this test"
+		return
+	fi
+
+	if [ -z "$policy" -o ! -f "$policy" ]; then
+		tst_res TINFO "no policy for this test"
+		return
+	fi
+
+	tst_res TINFO "trying to load '$policy' policy:"
+	cat $policy
+	if ! check_policy_writable; then
+		tst_res TINFO "WARNING: IMA policy already loaded and kernel not configured to enable multiple writes to it (need CONFIG_IMA_WRITE_POLICY=y), reboot required"
+		return
+	fi
+
+	cat "$policy" 2> log > $IMA_POLICY
+	if grep -q "Device or resource busy" log; then
+		tst_brk TBROK "Loading policy failed"
+	fi
+}
 ima_setup()
 {
 	SECURITYFS="$(mount_helper securityfs $SYSFS/kernel/security)"
@@ -180,6 +212,8 @@ ima_setup()
 		cd "$TST_MNTPOINT"
 	fi
 
+	load_ima_policy
+
 	[ -n "$TST_SETUP_CALLER" ] && $TST_SETUP_CALLER
 }
 
@@ -192,6 +226,10 @@ ima_cleanup()
 	for dir in $UMOUNT; do
 		umount $dir
 	done
+
+	if [ "$LTP_IMA_LOAD_POLICY" = 1 ]; then
+		tst_res TINFO "WARNING: policy loaded via LTP_IMA_LOAD_POLICY=1, reboot recommended"
+	fi
 }
 
 set_digest_index()
-- 
2.45.2

Re: [PATCH 2/3] ima_setup.sh: Allow to load predefined policy

[Petr Vorel]

Hi Mimi, all,

...
> +load_ima_policy()
> +{
> +	local policy="$(ls $TST_DATAROOT/*.policy 2>/dev/null)"
> +
> +	if [ "$LTP_IMA_LOAD_POLICY" != 1 -a "$policy" -a -f "$policy" ]; then
> +		tst_res TINFO "NOTE: set LTP_IMA_LOAD_POLICY=1 to load policy for this test"
> +		return
> +	fi
> +
> +	if [ -z "$policy" -o ! -f "$policy" ]; then
> +		tst_res TINFO "no policy for this test"
FYI here needs to be a flag to not warn in the cleanup about needed reboot.

> +		return
> +	fi
> +
> +	tst_res TINFO "trying to load '$policy' policy:"
> +	cat $policy
> +	if ! check_policy_writable; then
> +		tst_res TINFO "WARNING: IMA policy already loaded and kernel not configured to enable multiple writes to it (need CONFIG_IMA_WRITE_POLICY=y), reboot required"
> +		return
> +	fi
> +
> +	cat "$policy" 2> log > $IMA_POLICY
> +	if grep -q "Device or resource busy" log; then
> +		tst_brk TBROK "Loading policy failed"
Also here.

diff, which should solve it.

Kind regards,
Petr

+++ testcases/kernel/security/integrity/ima/tests/ima_setup.sh
@@ -175,6 +175,7 @@ load_ima_policy()
 
 	if [ -z "$policy" -o ! -f "$policy" ]; then
 		tst_res TINFO "no policy for this test"
+		LTP_IMA_LOAD_POLICY=
 		return
 	fi
 
@@ -182,6 +183,7 @@ load_ima_policy()
 	cat $policy
 	if ! check_policy_writable; then
 		tst_res TINFO "WARNING: IMA policy already loaded and kernel not configured to enable multiple writes to it (need CONFIG_IMA_WRITE_POLICY=y), reboot required"
+		LTP_IMA_LOAD_POLICY=
 		return
 	fi
 

Re: [PATCH 2/3] ima_setup.sh: Allow to load predefined policy

[Mimi Zohar]

On Tue, 2024-11-26 at 18:38 +0100, Petr Vorel wrote:
> environment variable LTP_IMA_LOAD_POLICY=1 tries to load example policy
> if available. This should be used only if tooling running LTP tests
> allows to reboot afterwards (because policy may be writable only once,
> e.g. missing CONFIG_IMA_WRITE_POLICY=y, or policies can influence each
> other).

Thanks, Petr.  Allowing the policy to be updated only if permitted is a good
idea.  Even with the LTP_IMA_LOAD_POLICY=1 environment variable, the policy
might not be loaded.  For example, when secure boot is enabled and the kernel is
configured with CONFIG_IMA_ARCH_POLICY enabled, an "appraise func=POLICY_CHECK
appraise_type=imasig" rule is loaded, requiring the IMA policy itself to be
signed.

On failure to load a policy, the ima_conditionals.sh and ima_policy.sh tests say
"TINFO: SELinux enabled in enforcing mode, this may affect test results".  We
should stop blaming SELinux. :)

thanks,

Mimi

Re: [PATCH 2/3] ima_setup.sh: Allow to load predefined policy

[Petr Vorel]

Hi Mimi, all,

> On Tue, 2024-11-26 at 18:38 +0100, Petr Vorel wrote:
> > environment variable LTP_IMA_LOAD_POLICY=1 tries to load example policy
> > if available. This should be used only if tooling running LTP tests
> > allows to reboot afterwards (because policy may be writable only once,
> > e.g. missing CONFIG_IMA_WRITE_POLICY=y, or policies can influence each
> > other).

> Thanks, Petr.  Allowing the policy to be updated only if permitted is a good
> idea.  Even with the LTP_IMA_LOAD_POLICY=1 environment variable, the policy
> might not be loaded.  For example, when secure boot is enabled and the kernel is
> configured with CONFIG_IMA_ARCH_POLICY enabled, an "appraise func=POLICY_CHECK
> appraise_type=imasig" rule is loaded, requiring the IMA policy itself to be
> signed.

Yes, it's an attempt, which can fail for various reasons. I'll add this example
of failure to load the policy to the commit message and to the docs.

I'd like to detect if policy got updated to avoid wasting time with SUT reboot
when policy was not updated. But this probably will not be always possible
(e.g. (CONFIG_IMA_READ_POLICY not set).

> On failure to load a policy, the ima_conditionals.sh and ima_policy.sh tests say
> "TINFO: SELinux enabled in enforcing mode, this may affect test results".  We
> should stop blaming SELinux. :)

This info was added for LTP shell tests, which got often affected by
SELinux/Apparmor. Because IMA is written in LTP shell API it gets this.
Error message is printed on TBROK, TFAIL, TWARN. Is this the only part where you
would like to avoid the message? Or do you want to remove SELinux/Apparmor
warning from all IMA tests?

Kind regards,
Petr

> thanks,

> Mimi

Re: [PATCH 2/3] ima_setup.sh: Allow to load predefined policy

[Mimi Zohar]

On Wed, 2024-12-11 at 20:48 +0100, Petr Vorel wrote:
> Hi Mimi, all,
> 
> > On Tue, 2024-11-26 at 18:38 +0100, Petr Vorel wrote:
> > > environment variable LTP_IMA_LOAD_POLICY=1 tries to load example policy
> > > if available. This should be used only if tooling running LTP tests
> > > allows to reboot afterwards (because policy may be writable only once,
> > > e.g. missing CONFIG_IMA_WRITE_POLICY=y, or policies can influence each
> > > other).
> 
> > Thanks, Petr.  Allowing the policy to be updated only if permitted is a good
> > idea.  Even with the LTP_IMA_LOAD_POLICY=1 environment variable, the policy
> > might not be loaded.  For example, when secure boot is enabled and the kernel is
> > configured with CONFIG_IMA_ARCH_POLICY enabled, an "appraise func=POLICY_CHECK
> > appraise_type=imasig" rule is loaded, requiring the IMA policy itself to be
> > signed.
> 
> Yes, it's an attempt, which can fail for various reasons. I'll add this example
> of failure to load the policy to the commit message and to the docs.
> 
> I'd like to detect if policy got updated to avoid wasting time with SUT reboot
> when policy was not updated. But this probably will not be always possible
> (e.g. (CONFIG_IMA_READ_POLICY not set).

Why do you actually need to be able to read the policy, after updating it?
Either the policy update succeeded or failed.  For example in the signed policy
case, writing a valid policy rule via 'cat' on a system requiring the policy to
be signed always fails.

> 
> > On failure to load a policy, the ima_conditionals.sh and ima_policy.sh tests say
> > "TINFO: SELinux enabled in enforcing mode, this may affect test results".  We
> > should stop blaming SELinux. :)
> 
> This info was added for LTP shell tests, which got often affected by
> SELinux/Apparmor. Because IMA is written in LTP shell API it gets this.
> Error message is printed on TBROK, TFAIL, TWARN. Is this the only part where you
> would like to avoid the message? Or do you want to remove SELinux/Apparmor
> warning from all IMA tests?

You have a better feel than me as to whether SELinux/Apparmor could interfere
with these tests.  Unless we have some reason to blame SELinux/Apparmor, I would
remove it.

Mimi 

Re: [PATCH 2/3] ima_setup.sh: Allow to load predefined policy

[Petr Vorel]

> On Wed, 2024-12-11 at 20:48 +0100, Petr Vorel wrote:
> > Hi Mimi, all,

> > > On Tue, 2024-11-26 at 18:38 +0100, Petr Vorel wrote:
> > > > environment variable LTP_IMA_LOAD_POLICY=1 tries to load example policy
> > > > if available. This should be used only if tooling running LTP tests
> > > > allows to reboot afterwards (because policy may be writable only once,
> > > > e.g. missing CONFIG_IMA_WRITE_POLICY=y, or policies can influence each
> > > > other).

> > > Thanks, Petr.  Allowing the policy to be updated only if permitted is a good
> > > idea.  Even with the LTP_IMA_LOAD_POLICY=1 environment variable, the policy
> > > might not be loaded.  For example, when secure boot is enabled and the kernel is
> > > configured with CONFIG_IMA_ARCH_POLICY enabled, an "appraise func=POLICY_CHECK
> > > appraise_type=imasig" rule is loaded, requiring the IMA policy itself to be
> > > signed.

> > Yes, it's an attempt, which can fail for various reasons. I'll add this example
> > of failure to load the policy to the commit message and to the docs.

> > I'd like to detect if policy got updated to avoid wasting time with SUT reboot
> > when policy was not updated. But this probably will not be always possible
> > (e.g. (CONFIG_IMA_READ_POLICY not set).

> Why do you actually need to be able to read the policy, after updating it?
You're right that's not needed for detecting the update.
But it's needed for tests which would like to detect whether policy contain
required items. Because without CONFIG_IMA_READ_POLICY when tests fails due old
kernel or unsuitable policy we have no way to detect. And TBROK is here false
positive which needs to be resolved by testers. But that's IMHO unsolvable.

> Either the policy update succeeded or failed.  For example in the signed policy
> case, writing a valid policy rule via 'cat' on a system requiring the policy to
> be signed always fails.

> > > On failure to load a policy, the ima_conditionals.sh and ima_policy.sh tests say
> > > "TINFO: SELinux enabled in enforcing mode, this may affect test results".  We
> > > should stop blaming SELinux. :)

> > This info was added for LTP shell tests, which got often affected by
> > SELinux/Apparmor. Because IMA is written in LTP shell API it gets this.
> > Error message is printed on TBROK, TFAIL, TWARN. Is this the only part where you
> > would like to avoid the message? Or do you want to remove SELinux/Apparmor
> > warning from all IMA tests?

> You have a better feel than me as to whether SELinux/Apparmor could interfere
> with these tests.  Unless we have some reason to blame SELinux/Apparmor, I would
> remove it.

+1, I'll remove it for IMA in v2.

Kind regards,
Petr

> Mimi 

[PATCH 3/3] ima_{kexec,keys,selinux}: Set minimal kernel version

[Petr Vorel]

The functionality IMHO was not backported to the enterprise kernels.

This helps to avoid false positive in ima_kexec.sh:
ima_kexec 1 TWARN: policy not readable, it might not contain required
policy '^measure.*func=KEXEC_CMDLINE'
ima_kexec 1 TBROK: unable to find a correct measurement

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/security/integrity/ima/tests/ima_kexec.sh   | 1 +
 testcases/kernel/security/integrity/ima/tests/ima_keys.sh    | 1 +
 testcases/kernel/security/integrity/ima/tests/ima_selinux.sh | 1 +
 3 files changed, 3 insertions(+)

diff --git a/testcases/kernel/security/integrity/ima/tests/ima_kexec.sh b/testcases/kernel/security/integrity/ima/tests/ima_kexec.sh
index 62f05f5361..3446bc24bf 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_kexec.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_kexec.sh
@@ -11,6 +11,7 @@
 TST_NEEDS_CMDS="grep kexec sed"
 TST_CNT=3
 TST_SETUP="setup"
+TST_MIN_KVER="5.3"
 
 IMA_KEXEC_IMAGE="${IMA_KEXEC_IMAGE:-/boot/vmlinuz-$(uname -r)}"
 REQUIRED_POLICY='^measure.*func=KEXEC_CMDLINE'
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
index 793908d44a..ff32eb6c43 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
@@ -10,6 +10,7 @@ TST_NEEDS_CMDS="cmp cut grep sed"
 TST_CNT=2
 TST_SETUP=setup
 TST_CLEANUP=cleanup
+TST_MIN_KVER="5.6"
 
 FUNC_KEYCHECK='func=KEY_CHECK'
 REQUIRED_POLICY="^measure.*$FUNC_KEYCHECK"
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh b/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh
index f6e392822a..75f9ba84e4 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh
@@ -12,6 +12,7 @@
 TST_NEEDS_CMDS="awk cut grep tail"
 TST_CNT=2
 TST_SETUP="setup"
+TST_MIN_KVER="5.12"
 
 FUNC_CRITICAL_DATA='func=CRITICAL_DATA'
 REQUIRED_POLICY="^measure.*$FUNC_CRITICAL_DATA"
-- 
2.45.2

Re: [PATCH 3/3] ima_{kexec,keys,selinux}: Set minimal kernel version

[Mimi Zohar]

On Tue, 2024-11-26 at 18:38 +0100, Petr Vorel wrote:
> The functionality IMHO was not backported to the enterprise kernels.
> 
> This helps to avoid false positive in ima_kexec.sh:
> ima_kexec 1 TWARN: policy not readable, it might not contain required
> policy '^measure.*func=KEXEC_CMDLINE'
> ima_kexec 1 TBROK: unable to find a correct measurement
> 
> Signed-off-by: Petr Vorel <pvorel@suse.cz>

Looks good.

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

Re: [PATCH 3/3] ima_{kexec,keys,selinux}: Set minimal kernel version

[Petr Vorel]

Hi Mimi,

> On Tue, 2024-11-26 at 18:38 +0100, Petr Vorel wrote:
> > The functionality IMHO was not backported to the enterprise kernels.

> > This helps to avoid false positive in ima_kexec.sh:
> > ima_kexec 1 TWARN: policy not readable, it might not contain required
> > policy '^measure.*func=KEXEC_CMDLINE'
> > ima_kexec 1 TBROK: unable to find a correct measurement

> > Signed-off-by: Petr Vorel <pvorel@suse.cz>

> Looks good.

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

Thanks a lot, this patch merged.

Kind regards,
Petr