[PATCH] ima_violations.sh: Another fix of condition evaluation

[PATCH] ima_violations.sh: Another fix of condition evaluation

[Petr Vorel]

c0c35509f9 was not enough to fix evaluation against empty
$expected_violations:

ima_violations 1 TINFO: verify open writers violation
/opt/ltp/testcases/bin/ima_violations.sh: line 96: [: 0: unary operator expected

Therefore split checks into two if.

Also improvements (readability)
* shorten line length with saving subtraction into variable
* evaluate empty variable with ${:-}

Fixes: 726ed71905 ("ima_violations.sh: Update validate() to support multiple violations")
Reported-by: Martin Doucha <mdoucha@suse.cz>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
NOTE: this was found on old SLES 4.4 based kernel which does not log
validations. But missing validations might be just a Secure Boot related
setup problem:

$ mokutil --sb-state
Secure Boot: EFI variables not supported on SUT

Events are logged when Secure Boot is off:
$ mokutil --sb-state
SecureBoot disabled

Or maybe violations worked differently on the old kernel (I remember
only 6.15 change).

Kind regards,
Petr

 .../integrity/ima/tests/ima_violations.sh     | 21 ++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
index 1d2f1d9447..a8476e6b59 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
@@ -87,23 +87,30 @@ validate()
 	local search="$3"
 	local expected_violations="$4"
 	local max_attempt=3
-	local count2 i num_violations_new
+	local count2 diff i num_violations_new pass
 
 	for i in $(seq 1 $max_attempt); do
 		read num_violations_new < $IMA_VIOLATIONS
 		count2="$(get_count $search)"
-		if [ -z "$expected_violations" -a $(($num_violations_new - $num_violations)) -gt 0 ] || \
-		   [ $(($num_violations_new - $num_violations)) -eq $expected_violations ]; then
-			[ -z "$expected_violations" ] && expected_violations=1
+		diff=$(($num_violations_new - $num_violations))
+
+		if [ "$expected_violations" ] && [ $diff -eq $expected_violations ]; then
+			pass=1
+		fi
+		if [ -z "$expected_violations" ] && [ $diff -gt 0 ]; then
+			pass=1
+		fi
+
+		if [ "$pass" = 1 ]; then
 			if [ $count2 -gt $count ]; then
-				tst_res TPASS "$expected_violations $search violation(s) added"
+				tst_res TPASS "${expected_violations:-1} $search violation(s) added"
 				return
 			else
 				tst_res TINFO "$search not found in $LOG ($i/$max_attempt attempt)..."
 				tst_sleep 1s
 			fi
-		elif [ $(($num_violations_new - $num_violations)) -gt 0 ]; then
-			tst_res $IMA_FAIL "$search too many violations added: $num_violations_new - $num_violations"
+		elif [ $diff -gt 0 ]; then
+			tst_res $IMA_FAIL "$search too many violations added: $diff ($num_violations_new - $num_violations)"
 			return
 		else
 			tst_res $IMA_FAIL "$search violation not added"
-- 
2.51.0

Re: [PATCH] ima_violations.sh: Another fix of condition evaluation

[Mimi Zohar]

On Thu, 2025-12-11 at 12:10 +0100, Petr Vorel wrote:
> c0c35509f9 was not enough to fix evaluation against empty
> $expected_violations:
> 
> ima_violations 1 TINFO: verify open writers violation
> /opt/ltp/testcases/bin/ima_violations.sh: line 96: [: 0: unary operator expected
> 
> Therefore split checks into two if.
> 
> Also improvements (readability)
> * shorten line length with saving subtraction into variable
> * evaluate empty variable with ${:-}
> 
> Fixes: 726ed71905 ("ima_violations.sh: Update validate() to support multiple violations")
> Reported-by: Martin Doucha <mdoucha@suse.cz>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>

Thanks, Martin, Petr.  LGTM

> ---
> NOTE: this was found on old SLES 4.4 based kernel which does not log
> validations. But missing validations might be just a Secure Boot related
> setup problem:
> 
> $ mokutil --sb-state
> Secure Boot: EFI variables not supported on SUT
> 
> Events are logged when Secure Boot is off:
> $ mokutil --sb-state
> SecureBoot disabled
> 
> Or maybe violations worked differently on the old kernel (I remember
> only 6.15 change).

Violations only occur when there are policy rules containing "func=FILE_CHECK"
defined.  The secure boot mode should only affects the arch specific policies,
which do not include "func=FILE_CHECK" rules.  There is a slight difference
between the builtin the original "ima_tcb" and newer "ima_policy=tcb" policies,
which might affect violations.

-- 
thanks, 

Mimi

Re: [LTP] [PATCH] ima_violations.sh: Another fix of condition evaluation

[Andrea Cervesato]

Hi!

On Thu Dec 11, 2025 at 12:10 PM CET, Petr Vorel wrote:
> c0c35509f9 was not enough to fix evaluation against empty
> $expected_violations:
>
> ima_violations 1 TINFO: verify open writers violation
> /opt/ltp/testcases/bin/ima_violations.sh: line 96: [: 0: unary operator expected
>
> Therefore split checks into two if.
>
> Also improvements (readability)
> * shorten line length with saving subtraction into variable
> * evaluate empty variable with ${:-}
>
> Fixes: 726ed71905 ("ima_violations.sh: Update validate() to support multiple violations")
> Reported-by: Martin Doucha <mdoucha@suse.cz>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> NOTE: this was found on old SLES 4.4 based kernel which does not log
> validations. But missing validations might be just a Secure Boot related
> setup problem:
>
> $ mokutil --sb-state
> Secure Boot: EFI variables not supported on SUT
>
> Events are logged when Secure Boot is off:
> $ mokutil --sb-state
> SecureBoot disabled
>
> Or maybe violations worked differently on the old kernel (I remember
> only 6.15 change).
>
> Kind regards,
> Petr
>
>  .../integrity/ima/tests/ima_violations.sh     | 21 ++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
> index 1d2f1d9447..a8476e6b59 100755
> --- a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
> +++ b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
> @@ -87,23 +87,30 @@ validate()
>  	local search="$3"
>  	local expected_violations="$4"
>  	local max_attempt=3
> -	local count2 i num_violations_new
> +	local count2 diff i num_violations_new pass
>  
>  	for i in $(seq 1 $max_attempt); do
>  		read num_violations_new < $IMA_VIOLATIONS
>  		count2="$(get_count $search)"
> -		if [ -z "$expected_violations" -a $(($num_violations_new - $num_violations)) -gt 0 ] || \
> -		   [ $(($num_violations_new - $num_violations)) -eq $expected_violations ]; then
> -			[ -z "$expected_violations" ] && expected_violations=1
> +		diff=$(($num_violations_new - $num_violations))
> +
> +		if [ "$expected_violations" ] && [ $diff -eq $expected_violations ]; then
> +			pass=1
> +		fi
> +		if [ -z "$expected_violations" ] && [ $diff -gt 0 ]; then
> +			pass=1
> +		fi

Maybe readability can be improved (well..shell scripts are pretty ugly
by nature anyway):

	diff=$((num_violations_new - num_violations))

	if [ "$expected_violations" ]; then
		[ $diff -eq $expected_violations ] && pass=1
	else
		[ $diff -gt 0 ] && pass=1
	fi

> +
> +		if [ "$pass" = 1 ]; then
>  			if [ $count2 -gt $count ]; then
> -				tst_res TPASS "$expected_violations $search violation(s) added"
> +				tst_res TPASS "${expected_violations:-1} $search violation(s) added"
>  				return
>  			else
>  				tst_res TINFO "$search not found in $LOG ($i/$max_attempt attempt)..."
>  				tst_sleep 1s
>  			fi
> -		elif [ $(($num_violations_new - $num_violations)) -gt 0 ]; then
> -			tst_res $IMA_FAIL "$search too many violations added: $num_violations_new - $num_violations"
> +		elif [ $diff -gt 0 ]; then
> +			tst_res $IMA_FAIL "$search too many violations added: $diff ($num_violations_new - $num_violations)"
>  			return
>  		else
>  			tst_res $IMA_FAIL "$search violation not added"

-- 
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

Re: [LTP] [PATCH] ima_violations.sh: Another fix of condition evaluation

[Petr Vorel]

> Hi!

> On Thu Dec 11, 2025 at 12:10 PM CET, Petr Vorel wrote:
> > c0c35509f9 was not enough to fix evaluation against empty
> > $expected_violations:

> > ima_violations 1 TINFO: verify open writers violation
> > /opt/ltp/testcases/bin/ima_violations.sh: line 96: [: 0: unary operator expected

> > Therefore split checks into two if.

> > Also improvements (readability)
> > * shorten line length with saving subtraction into variable
> > * evaluate empty variable with ${:-}

> > Fixes: 726ed71905 ("ima_violations.sh: Update validate() to support multiple violations")
> > Reported-by: Martin Doucha <mdoucha@suse.cz>
> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > ---
> > NOTE: this was found on old SLES 4.4 based kernel which does not log
> > validations. But missing validations might be just a Secure Boot related
> > setup problem:

> > $ mokutil --sb-state
> > Secure Boot: EFI variables not supported on SUT

> > Events are logged when Secure Boot is off:
> > $ mokutil --sb-state
> > SecureBoot disabled

> > Or maybe violations worked differently on the old kernel (I remember
> > only 6.15 change).

> > Kind regards,
> > Petr

> >  .../integrity/ima/tests/ima_violations.sh     | 21 ++++++++++++-------
> >  1 file changed, 14 insertions(+), 7 deletions(-)

> > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
> > index 1d2f1d9447..a8476e6b59 100755
> > --- a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
> > +++ b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
> > @@ -87,23 +87,30 @@ validate()
> >  	local search="$3"
> >  	local expected_violations="$4"
> >  	local max_attempt=3
> > -	local count2 i num_violations_new
> > +	local count2 diff i num_violations_new pass

> >  	for i in $(seq 1 $max_attempt); do
> >  		read num_violations_new < $IMA_VIOLATIONS
> >  		count2="$(get_count $search)"
> > -		if [ -z "$expected_violations" -a $(($num_violations_new - $num_violations)) -gt 0 ] || \
> > -		   [ $(($num_violations_new - $num_violations)) -eq $expected_violations ]; then
> > -			[ -z "$expected_violations" ] && expected_violations=1
> > +		diff=$(($num_violations_new - $num_violations))
> > +
> > +		if [ "$expected_violations" ] && [ $diff -eq $expected_violations ]; then
> > +			pass=1
> > +		fi
> > +		if [ -z "$expected_violations" ] && [ $diff -gt 0 ]; then
> > +			pass=1
> > +		fi

> Maybe readability can be improved (well..shell scripts are pretty ugly
> by nature anyway):

> 	diff=$((num_violations_new - num_violations))

> 	if [ "$expected_violations" ]; then
> 		[ $diff -eq $expected_violations ] && pass=1
> 	else
> 		[ $diff -gt 0 ] && pass=1
> 	fi

Thanks, makes sense, I'll modify it before merge.
Feel free to add your RBT/TBT tags if you wish (as you spent time looking into
this).

Kind regards,
Petr

> > +
> > +		if [ "$pass" = 1 ]; then
> >  			if [ $count2 -gt $count ]; then
> > -				tst_res TPASS "$expected_violations $search violation(s) added"
> > +				tst_res TPASS "${expected_violations:-1} $search violation(s) added"
> >  				return
> >  			else
> >  				tst_res TINFO "$search not found in $LOG ($i/$max_attempt attempt)..."
> >  				tst_sleep 1s
> >  			fi
> > -		elif [ $(($num_violations_new - $num_violations)) -gt 0 ]; then
> > -			tst_res $IMA_FAIL "$search too many violations added: $num_violations_new - $num_violations"
> > +		elif [ $diff -gt 0 ]; then
> > +			tst_res $IMA_FAIL "$search too many violations added: $diff ($num_violations_new - $num_violations)"
> >  			return
> >  		else
> >  			tst_res $IMA_FAIL "$search violation not added"

Re: [PATCH] ima_violations.sh: Another fix of condition evaluation

[Petr Vorel]

> On Thu, 2025-12-11 at 12:10 +0100, Petr Vorel wrote:
> > c0c35509f9 was not enough to fix evaluation against empty
> > $expected_violations:

> > ima_violations 1 TINFO: verify open writers violation
> > /opt/ltp/testcases/bin/ima_violations.sh: line 96: [: 0: unary operator expected

> > Therefore split checks into two if.

> > Also improvements (readability)
> > * shorten line length with saving subtraction into variable
> > * evaluate empty variable with ${:-}

> > Fixes: 726ed71905 ("ima_violations.sh: Update validate() to support multiple violations")
> > Reported-by: Martin Doucha <mdoucha@suse.cz>
> > Signed-off-by: Petr Vorel <pvorel@suse.cz>

> Thanks, Martin, Petr.  LGTM

Thanks! FYI merged with simpler code suggested by Andrea.

> > ---
> > NOTE: this was found on old SLES 4.4 based kernel which does not log
> > validations. But missing validations might be just a Secure Boot related
> > setup problem:

> > $ mokutil --sb-state
> > Secure Boot: EFI variables not supported on SUT

> > Events are logged when Secure Boot is off:
> > $ mokutil --sb-state
> > SecureBoot disabled

> > Or maybe violations worked differently on the old kernel (I remember
> > only 6.15 change).

> Violations only occur when there are policy rules containing "func=FILE_CHECK"
> defined.  The secure boot mode should only affects the arch specific policies,
> which do not include "func=FILE_CHECK" rules.  There is a slight difference
> between the builtin the original "ima_tcb" and newer "ima_policy=tcb" policies,
> which might affect violations.

Thanks for the hints! I'll have to dig more into tthe problem to see what is
wrong.

Anyway at least for example policy I see the differences between the old [1] and
new [2]:

-measure func=FILE_CHECK mask=MAY_READ uid=0
+measure func=FILE_CHECK mask=^MAY_READ euid=0
+measure func=FILE_CHECK mask=^MAY_READ uid=0    # root opened r/o, r/w

I guess I'll just add for older kernels this example policy:
measure func=FILE_CHECK mask=MAY_READ uid=0

Kind regards,
Petr

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