Re: [PATCH] ima_tpm.sh: update test2 to detect integrity violations

[Petr Vorel <pvorel@suse.cz>]

Hi Mimi,

> Hi Petr,

> On Mon, 2026-08-17 at 12:55 +0200, Petr Vorel wrote:
> > Hi Mimi,

> > > <securityfs>/integrity/ima/violations reflects the number of
> > > integrity violations.  Include the "--ignore-violations" option,
> > > if there are any violations, on the initial IMA measurement list
> > > verification.

> > Thanks for your patch!

> > LGTM and it should be fixed. But there are some potential problems
> > (see bellow).

> And here I thought this was a simple performance improvement to execute evmctl
> with/without the --ignore-violations option once.

FYI back then I did not worry that much about performance (i.e. running evmctl
once or twice does not matter to me), I wanted to keep test coverage on vast
majority of distro versions (old and new). Maybe it's not that important.

> > > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> > > ---
> > >  .../security/integrity/ima/tests/ima_tpm.sh   | 20 ++++++++++++-------
> > >  1 file changed, 13 insertions(+), 7 deletions(-)

> > > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
> > > index 5d34d8679..acd8b6d30 100755
> > > --- a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
> > > +++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
> > > @@ -142,6 +142,8 @@ read_pcr_tpm2()
> > >  get_pcr10_aggregate()
> > >  {
> > >  	local cmd="evmctl -vv ima_measurement $BINARY_MEASUREMENTS"
> > > +	local violations="$IMA_DIR/violations"
> > > +	local num_violations=0
> > >  	local msg="$ERRMSG_EVMCTL"
> > >  	local res=TCONF
> > >  	local pcr ret
> > > @@ -151,16 +153,20 @@ get_pcr10_aggregate()
> > >  		res=TFAIL
> > >  	fi

> > > -	$cmd > hash.txt 2>&1
> > > -	ret=$?
> > > -	if [ $ret -ne 0 -a -z "$MISSING_EVMCTL" ]; then
> > > -		tst_res TFAIL "evmctl failed, trying with --ignore-violations"
> > You removed TFAIL (potential problem, see later).

> > > +	if [ ! -f "$violations" ]; then
> > > +		tst_res TINFO "missing $violations"
> > > +	else
> > > +		num_violations=$(cat "$violations")
> > > +	fi
> > > +
> > > +	if [ "$num_violations" -eq 0 ]; then
> > > +		$cmd > hash.txt 2>&1
> > > +		ret=$?
> > > +	else
> > > +		tst_res TINFO "ignoring $num_violations violations"
> > >  		cmd="$cmd --ignore-violations"
> > >  		$cmd > hash.txt 2>&1
> > >  		ret=$?
> > > -	elif [ $ret -ne 0 -a "$MISSING_EVMCTL" = 1 ]; then
> > > -		tst_res TFAIL "evmctl failed $msg"
> > And here again removed TFAIL (see later).

> > The main problem is that you removed the code when evmctl is not installed.

> If I'm understanding the code correctly, the original code executed evmctl
> whether it existed or not.  Let's fix that first.  My question is whether the
IMHO no.

> test should fail or be skipped?

The original intention in 7fd7c9febd [1] was to run without --ignore-violations
on any evmctl and if test fails (recorded always as TFAIL) and evmctl is new
enough rerun it with --ignore-violations.  (Simply not run for the second time
and just fail on missing evmctl or evmctl being too old to support
--ignore-violations.) Quoting the kernel commit for the reason:

	For old kernels which use SHA1/MD5, any evmctl version is required (evmctl
	ima_measurement was introduced in very old v0.7), but:
	* newer sysctl path /sys/class/tpm/tpm0/device/pcrs requires evmctl 1.1
	* using ima_policy=tcb requires 1.3.1 due --ignore-violations

	For evmctl >= 1.3.1 on failure we try to retest with --ignore-violations.

Is it a wrong approach? Looking on your patch it probably is wrong approach
(not taking /sys/kernel/security/ima/violations into account).

Also, as $MISSING_EVMCTL is taking into account in the results, it should be
safe.

[1] https://github.com/linux-test-project/ltp/commit/7fd7c9febdd7ed48ae3563923074bcabdfec3923

> > Therefore trying to rerun evmctl on failure on older release (e.g. 1.3) it will
> > fail due option have different name or not exist at all in evmctl < 1.2).
> > -a "$MISSING_EVMCTL" = 1 check had meaning "don't rerun with --ignore-violations
> > on old evmctl which does not have the option.

And $MISSING_EVMCTL is a bit misleading name, because 1 means either "evmctl not
installed at all" or "installed old evmctl version" (MISSING_SUITABLE_EVMCTL or
something would be more obvious).

> Thank you for the explanation.

+1, thank you too for your explanation.

> Before appending the "--ignore-violations" we should make sure it is supported.
Yes. And I hoped that code in the setup() resulting in $MISSING_EVMCTL did it
correctly.

> I guess for backwards compatibility we still want to verify the measurement
> list, knowing it will fail.

Yes.

> Hopefully with these two changes this patch will work properly.

Reviewed-by: Petr Vorel <pvorel@suse.cz>
(plan to merge tomorrow, in case there is any feedback)

BTW when run on new system (recent Tumbleweed) without evmctl the second TCONF
message is misleading a bit but let's ignore it:

ima_tpm 1 TINFO: TPM hardware support not enabled in kernel or no TPM chip found, testing TPM-bypass
ima_tpm 1 TCONF: 'evmctl' not found
ima_tpm 1 TCONF: algorithm not sha1 (sha256) => install evmctl >= 1.3.1

And other issue is that parsing kernel config in setup() should be replaced with
API function tst_check_kconfigs (less error-prone, it can ballback to
/proc/config.gz, etc). That's my TODO.

Kind regards,
Petr

> Mimi


> > FYI the code is a bit complicated, because here on TPM2 we require evmctl 1.3.1
> > to have --ignore-violations (renamed from --validate), which was released in
> > 2020 - too new for old enterprise distros to ignore; also TPM1 we require only
> > 1.1 from 2018, probably still too new. Once SLE12-SP3 EOL (in 1 year we may just
> > expect 1.3.1 to simplify).

> > > -		return
> > >  	fi

> > >  	[ $ret -ne 0 ] && tst_res TWARN "evmctl failed, trying to continue $msg"
> > Back to removed TFAIL. While this is OK as TWARN (some problem, but not related
> > to testing) we might end up to TBROK "Test didn't report any results" error in
> > tst_test.sh which quits test with TBROK "Test didn't report any results" if
> > there is no TPASS/TFAIL/TCONF message.

> > _tst_resstr()
> > {
> > 	echo "$TST_PASS$TST_FAIL$TST_CONF"
> > }

> > _tst_rescmp()
> > {
> > 	local res=$(_tst_resstr)

> > 	if [ "$1" = "$res" ]; then
> > 		tst_brk TBROK "Test didn't report any results"
> > 	fi
> > }

> > And this happen later in test2():
> > 	get_pcr10_aggregate > tmp.txt
> > 	pcr_aggregate="$(cat tmp.txt)"
> > 	if [ -z "$pcr_aggregate" ]; then
> > 		return
> > 	fi

> > Other option would be to print TFAIL message in test2():

> > 	get_pcr10_aggregate > tmp.txt
> > 	pcr_aggregate="$(cat tmp.txt)"
> > 	if [ -z "$pcr_aggregate" ]; then
> > 		tst_res TBROK "failed to get aggregate PCR-10"
> > 		return
> > 	fi

> > Lol, I'm disappointed how complicated and error prone I wrote back then.
> > Part of the problem is that quit with tst_brk does not work, when code which
> > does it is run in a subshell (via $(...) or `...) ), which quits subshell but
> > not the parent shell.