[PATCH 1/2] tst_security.sh: Fix SELinux detection

[PATCH 1/2] tst_security.sh: Fix SELinux detection

[Petr Vorel]

Some SLES15 versions create /selinux directory which fails the detection
if SELinux is actually not enabled. Therefore detect if directory
actually contains the 'enforce' file.

Also drop /selinux directory detection and detect only /sys/fs/selinux,
/sys/fs/selinux mount point was added in kernel 3.0 in commit
7a627e3b9a2b ("SELINUX: add /sys/fs/selinux mount point to put selinuxfs")
14 years is enough, kernel 3.0 is not even supported in current LTP and
we don't even support /selinux in C API (tst_security.c).

Fixes: e7b804df65 ("shell: Add tst_security.sh helper")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/lib/tst_security.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/testcases/lib/tst_security.sh b/testcases/lib/tst_security.sh
index 05640234ea..356c28fc73 100644
--- a/testcases/lib/tst_security.sh
+++ b/testcases/lib/tst_security.sh
@@ -127,8 +127,7 @@ tst_get_selinux_dir()
 {
 	local dir="/sys/fs/selinux"
 
-	[ -d "$dir" ] || dir="/selinux"
-	[ -d "$dir" ] && echo "$dir"
+	[ -f "$dir/enforce" ] && echo "$dir"
 }
 
 # Get SELinux enforce file path
-- 
2.47.2

[PATCH 2/2] ima_selinux.sh: Detect SELinux before loading policy

[Petr Vorel]

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
+
 	if ! verify_ima_policy; then
 		load_ima_policy
 	fi
-- 
2.47.2

Re: [LTP] [PATCH 2/2] ima_selinux.sh: Detect SELinux before loading policy

[Andrea Cervesato]

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

Re: [LTP] [PATCH 2/2] ima_selinux.sh: Detect SELinux before loading policy

[Petr Vorel]

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()

Re: [PATCH 2/2] ima_selinux.sh: Detect SELinux before loading policy

[Petr Vorel]

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

Re: [LTP] [PATCH 1/2] tst_security.sh: Fix SELinux detection

[Andrea Cervesato]

Hi!

Reviewed-by: Andrea Cervesato <andrea.cervesato@suse.com>

On 2/17/25 14:08, Petr Vorel wrote:
> Some SLES15 versions create /selinux directory which fails the detection
> if SELinux is actually not enabled. Therefore detect if directory
> actually contains the 'enforce' file.
>
> Also drop /selinux directory detection and detect only /sys/fs/selinux,
> /sys/fs/selinux mount point was added in kernel 3.0 in commit
> 7a627e3b9a2b ("SELINUX: add /sys/fs/selinux mount point to put selinuxfs")
> 14 years is enough, kernel 3.0 is not even supported in current LTP and
> we don't even support /selinux in C API (tst_security.c).
>
> Fixes: e7b804df65 ("shell: Add tst_security.sh helper")
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>   testcases/lib/tst_security.sh | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/testcases/lib/tst_security.sh b/testcases/lib/tst_security.sh
> index 05640234ea..356c28fc73 100644
> --- a/testcases/lib/tst_security.sh
> +++ b/testcases/lib/tst_security.sh
> @@ -127,8 +127,7 @@ tst_get_selinux_dir()
>   {
>   	local dir="/sys/fs/selinux"
>   
> -	[ -d "$dir" ] || dir="/selinux"
> -	[ -d "$dir" ] && echo "$dir"
> +	[ -f "$dir/enforce" ] && echo "$dir"
>   }
>   
>   # Get SELinux enforce file path

Re: [LTP] [PATCH 1/2] tst_security.sh: Fix SELinux detection

[Petr Vorel]

Hi Andrea,

> Hi!

Thanks for your review, merged as a needed fix.

Kind regards,
Petr

> Reviewed-by: Andrea Cervesato <andrea.cervesato@suse.com>

> On 2/17/25 14:08, Petr Vorel wrote:
> > Some SLES15 versions create /selinux directory which fails the detection
> > if SELinux is actually not enabled. Therefore detect if directory
> > actually contains the 'enforce' file.

> > Also drop /selinux directory detection and detect only /sys/fs/selinux,
> > /sys/fs/selinux mount point was added in kernel 3.0 in commit
> > 7a627e3b9a2b ("SELINUX: add /sys/fs/selinux mount point to put selinuxfs")
> > 14 years is enough, kernel 3.0 is not even supported in current LTP and
> > we don't even support /selinux in C API (tst_security.c).

> > Fixes: e7b804df65 ("shell: Add tst_security.sh helper")
> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > ---
> >   testcases/lib/tst_security.sh | 3 +--
> >   1 file changed, 1 insertion(+), 2 deletions(-)

> > diff --git a/testcases/lib/tst_security.sh b/testcases/lib/tst_security.sh
> > index 05640234ea..356c28fc73 100644
> > --- a/testcases/lib/tst_security.sh
> > +++ b/testcases/lib/tst_security.sh
> > @@ -127,8 +127,7 @@ tst_get_selinux_dir()
> >   {
> >   	local dir="/sys/fs/selinux"
> > -	[ -d "$dir" ] || dir="/selinux"
> > -	[ -d "$dir" ] && echo "$dir"
> > +	[ -f "$dir/enforce" ] && echo "$dir"
> >   }
> >   # Get SELinux enforce file path