All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mimi Zohar <zohar@linux.ibm.com>
To: Petr Vorel <pvorel@suse.cz>
Cc: ltp@lists.linux.it, linux-integrity@vger.kernel.org
Subject: Re: [PATCH] ima_tpm.sh: update test2 to detect integrity violations
Date: Mon, 17 Aug 2026 18:13:32 -0400	[thread overview]
Message-ID: <2fb152d374a14a0e30b20de73d74a90b195e0793.camel@linux.ibm.com> (raw)
In-Reply-To: <20260817105556.GA1951950@pevik>

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.

> 
> > 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
test should fail or be skipped?

> 
> 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.

Thank you for the explanation.

Before appending the "--ignore-violations" we should make sure it is supported.
I guess for backwards compatibility we still want to verify the measurement
list, knowing it will fail.

Hopefully with these two changes this patch will work properly.

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.

WARNING: multiple messages have this Message-ID (diff)
From: Mimi Zohar <zohar@linux.ibm.com>
To: Petr Vorel <pvorel@suse.cz>
Cc: linux-integrity@vger.kernel.org, ltp@lists.linux.it
Subject: Re: [LTP] [PATCH] ima_tpm.sh: update test2 to detect integrity violations
Date: Mon, 17 Aug 2026 18:13:32 -0400	[thread overview]
Message-ID: <2fb152d374a14a0e30b20de73d74a90b195e0793.camel@linux.ibm.com> (raw)
In-Reply-To: <20260817105556.GA1951950@pevik>

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.

> 
> > 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
test should fail or be skipped?

> 
> 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.

Thank you for the explanation.

Before appending the "--ignore-violations" we should make sure it is supported.
I guess for backwards compatibility we still want to verify the measurement
list, knowing it will fail.

Hopefully with these two changes this patch will work properly.

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.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

  reply	other threads:[~2026-08-17 22:13 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-14 13:57 [PATCH] ima_tpm.sh: update test2 to detect integrity violations Mimi Zohar
2026-08-14 13:57 ` [LTP] " Mimi Zohar
2026-08-14 14:52 ` [LTP] " linuxtestproject.agent
2026-08-17 10:55 ` [LTP] [PATCH] " Petr Vorel
2026-08-17 10:55   ` Petr Vorel
2026-08-17 22:13   ` Mimi Zohar [this message]
2026-08-17 22:13     ` [LTP] " Mimi Zohar
2026-08-18  8:20     ` Petr Vorel
2026-08-18  8:20       ` [LTP] " Petr Vorel
2026-08-20  9:18     ` Petr Vorel
2026-08-20  9:18       ` Petr Vorel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2fb152d374a14a0e30b20de73d74a90b195e0793.camel@linux.ibm.com \
    --to=zohar@linux.ibm.com \
    --cc=linux-integrity@vger.kernel.org \
    --cc=ltp@lists.linux.it \
    --cc=pvorel@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.