| Message ID | 20250217130839.2392666-2-pvorel@suse.cz (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] tst_security.sh: Fix SELinux detection | expand |
Hi! On 2/17/25 14:08, Petr Vorel wrote: > Adding TST_SETUP_EARLY to run test specific setup before loading policy. > That allows to avoid loading IMA policy (which usually request reboot) > if the test would be skipped anyway. > > Fixes: aac97cca96 ("ima_setup.sh: Allow to load predefined policy") > Signed-off-by: Petr Vorel <pvorel@suse.cz> > --- > testcases/kernel/security/integrity/ima/tests/ima_selinux.sh | 3 ++- > testcases/kernel/security/integrity/ima/tests/ima_setup.sh | 2 ++ > 2 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh b/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh > index 97c5d64ec5..577f7c2aca 100755 > --- a/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh > +++ b/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh > @@ -13,9 +13,10 @@ > > TST_NEEDS_CMDS="awk cut grep tail" > TST_CNT=2 > -TST_SETUP="setup" > TST_MIN_KVER="5.12" > > +TST_SETUP_EARLY="setup" > + > REQUIRED_POLICY_CONTENT='selinux.policy' > > setup() > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh > index 1f1c267c4b..2a9f64978e 100644 > --- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh > +++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh > @@ -265,6 +265,8 @@ ima_setup() > cd "$TST_MNTPOINT" > fi > > + [ -n "$TST_SETUP_EARLY" ] && $TST_SETUP_EARLY > + Why not doing ". ima_setup.sh" at the end of setup() ? > if ! verify_ima_policy; then > load_ima_policy > fi Andrea
Hi, > Adding TST_SETUP_EARLY to run test specific setup before loading policy. > That allows to avoid loading IMA policy (which usually request reboot) > if the test would be skipped anyway. > Fixes: aac97cca96 ("ima_setup.sh: Allow to load predefined policy") > Signed-off-by: Petr Vorel <pvorel@suse.cz> > --- > testcases/kernel/security/integrity/ima/tests/ima_selinux.sh | 3 ++- > testcases/kernel/security/integrity/ima/tests/ima_setup.sh | 2 ++ > 2 files changed, 4 insertions(+), 1 deletion(-) > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh b/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh > index 97c5d64ec5..577f7c2aca 100755 > --- a/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh > +++ b/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh > @@ -13,9 +13,10 @@ > TST_NEEDS_CMDS="awk cut grep tail" > TST_CNT=2 > -TST_SETUP="setup" > TST_MIN_KVER="5.12" > +TST_SETUP_EARLY="setup" Hm, unlike TST_SETUP_CALLER, which is defined in ima_setup.sh, using "TST_" prefixed variables in the test itself is catched by variable checker in tst_test.sh: ima_selinux 1 TWARN: Reserved variable TST_SETUP_EARLY used! I need to redefine the variable to not use "TST_" prefix (see the diff below). I'm sorry for the noise. Kind regards, Petr +++ testcases/kernel/security/integrity/ima/tests/ima_selinux.sh @@ -15,7 +15,7 @@ TST_NEEDS_CMDS="awk cut grep tail" TST_CNT=2 TST_MIN_KVER="5.12" -TST_SETUP_EARLY="setup" +IMA_SETUP_EARLY="setup" REQUIRED_POLICY_CONTENT='selinux.policy' diff --git testcases/kernel/security/integrity/ima/tests/ima_setup.sh testcases/kernel/security/integrity/ima/tests/ima_setup.sh index 2a9f64978e..befaf7f73c 100644 --- testcases/kernel/security/integrity/ima/tests/ima_setup.sh +++ testcases/kernel/security/integrity/ima/tests/ima_setup.sh @@ -265,7 +265,7 @@ ima_setup() cd "$TST_MNTPOINT" fi - [ -n "$TST_SETUP_EARLY" ] && $TST_SETUP_EARLY + [ -n "$IMA_SETUP_EARLY" ] && $IMA_SETUP_EARLY if ! verify_ima_policy; then load_ima_policy
Hi Andrea, > Hi! > On 2/17/25 14:08, Petr Vorel wrote: > > Adding TST_SETUP_EARLY to run test specific setup before loading policy. > > That allows to avoid loading IMA policy (which usually request reboot) > > if the test would be skipped anyway. > > Fixes: aac97cca96 ("ima_setup.sh: Allow to load predefined policy") > > Signed-off-by: Petr Vorel <pvorel@suse.cz> > > --- > > testcases/kernel/security/integrity/ima/tests/ima_selinux.sh | 3 ++- > > testcases/kernel/security/integrity/ima/tests/ima_setup.sh | 2 ++ > > 2 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh b/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh > > index 97c5d64ec5..577f7c2aca 100755 > > --- a/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh > > +++ b/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh > > @@ -13,9 +13,10 @@ > > TST_NEEDS_CMDS="awk cut grep tail" > > TST_CNT=2 > > -TST_SETUP="setup" > > TST_MIN_KVER="5.12" > > +TST_SETUP_EARLY="setup" > > + > > REQUIRED_POLICY_CONTENT='selinux.policy' > > setup() > > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh > > index 1f1c267c4b..2a9f64978e 100644 > > --- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh > > +++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh > > @@ -265,6 +265,8 @@ ima_setup() > > cd "$TST_MNTPOINT" > > fi > > + [ -n "$TST_SETUP_EARLY" ] && $TST_SETUP_EARLY > > + > Why not doing ". ima_setup.sh" at the end of setup() ? I do not understand what you mean. How would it help? Also since 04021637f4 ("tst_test.sh: Cleanup getopts usage") shell library is required to be run at the end of a shell test, just before tst_run. FYI at least for some tests (these with $REQUIRED_BUILTIN_POLICY, e.g. evm_overlay.sh) it makes sense to check /proc/cmdline content (e.g. run load_ima_policy()) before the test specific setup to quit early enough. But OTOH it's best to postpone loading the policy after other checks are done (e.g. here check for SELinux), because loading policy often requires reboot. Maybe best would be to do $REQUIRED_BUILTIN_POLICY check *before running the setup and keep loading the policy *after* the setup. Maybe I should postpone loading (see the diff below). Kind regards, Petr > > if ! verify_ima_policy; then > > load_ima_policy > > fi > Andrea diff --git testcases/kernel/security/integrity/ima/tests/ima_selinux.sh testcases/kernel/security/integrity/ima/tests/ima_selinux.sh index c8eb80b641..97c5d64ec5 100755 --- testcases/kernel/security/integrity/ima/tests/ima_selinux.sh +++ testcases/kernel/security/integrity/ima/tests/ima_selinux.sh @@ -13,10 +13,9 @@ TST_NEEDS_CMDS="awk cut grep tail" TST_CNT=2 +TST_SETUP="setup" TST_MIN_KVER="5.12" -IMA_SETUP_EARLY="setup" - REQUIRED_POLICY_CONTENT='selinux.policy' setup() diff --git testcases/kernel/security/integrity/ima/tests/ima_setup.sh testcases/kernel/security/integrity/ima/tests/ima_setup.sh index befaf7f73c..f7ec5aeb35 100644 --- testcases/kernel/security/integrity/ima/tests/ima_setup.sh +++ testcases/kernel/security/integrity/ima/tests/ima_setup.sh @@ -245,6 +245,8 @@ load_ima_policy() ima_setup() { + local load_policy + SECURITYFS="$(mount_helper securityfs $SYSFS/kernel/security)" IMA_DIR="$SECURITYFS/ima" @@ -265,13 +267,15 @@ ima_setup() cd "$TST_MNTPOINT" fi - [ -n "$IMA_SETUP_EARLY" ] && $IMA_SETUP_EARLY + verify_ima_policy + load_policy=$? + + [ -n "$TST_SETUP_CALLER" ] && $TST_SETUP_CALLER - if ! verify_ima_policy; then + if [ $load_policy -ne 0 ]; then load_ima_policy fi - [ -n "$TST_SETUP_CALLER" ] && $TST_SETUP_CALLER } ima_cleanup()
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh b/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh index 97c5d64ec5..577f7c2aca 100755 --- a/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh +++ b/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh @@ -13,9 +13,10 @@ TST_NEEDS_CMDS="awk cut grep tail" TST_CNT=2 -TST_SETUP="setup" TST_MIN_KVER="5.12" +TST_SETUP_EARLY="setup" + REQUIRED_POLICY_CONTENT='selinux.policy' setup() diff --git a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh index 1f1c267c4b..2a9f64978e 100644 --- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh +++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh @@ -265,6 +265,8 @@ ima_setup() cd "$TST_MNTPOINT" fi + [ -n "$TST_SETUP_EARLY" ] && $TST_SETUP_EARLY + if ! verify_ima_policy; then load_ima_policy fi
Adding TST_SETUP_EARLY to run test specific setup before loading policy. That allows to avoid loading IMA policy (which usually request reboot) if the test would be skipped anyway. Fixes: aac97cca96 ("ima_setup.sh: Allow to load predefined policy") Signed-off-by: Petr Vorel <pvorel@suse.cz> --- testcases/kernel/security/integrity/ima/tests/ima_selinux.sh | 3 ++- testcases/kernel/security/integrity/ima/tests/ima_setup.sh | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-)