diff mbox series

security/ima: limit the scope of the LTP policy rules based on the UUID

Message ID 20221006164342.68763-1-zohar@linux.ibm.com
State Changes Requested
Delegated to: Petr Vorel
Headers show
Series security/ima: limit the scope of the LTP policy rules based on the UUID | expand

Commit Message

Mimi Zohar Oct. 6, 2022, 4:43 p.m. UTC
The LTP policy rules either replace or extend the global IMA policy. As a
result, the ordering of the LTP IMA tests is important and affects the
ability of re-running the tests.  For example, ima_conditionals.sh
defines a rule to measure user files, while ima_measuremnets.sh verifies
not measuring user files.  Not limiting the LTP IMA policy scope could
also affect the running system.

To allow the LTP tests to be re-run without rebooting the system, limit the
scope of the LTP policy rules to the loopback mounted filesystem based on
the UUID.

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 .../security/integrity/ima/tests/ima_conditionals.sh       | 2 +-
 .../kernel/security/integrity/ima/tests/ima_policy.sh      | 7 ++++++-
 testcases/kernel/security/integrity/ima/tests/ima_setup.sh | 4 ++++
 3 files changed, 11 insertions(+), 2 deletions(-)

Comments

Petr Vorel Oct. 6, 2022, 9:02 p.m. UTC | #1
Hi Mimi,

> The LTP policy rules either replace or extend the global IMA policy. As a
> result, the ordering of the LTP IMA tests is important and affects the
> ability of re-running the tests.  For example, ima_conditionals.sh
> defines a rule to measure user files, while ima_measuremnets.sh verifies
> not measuring user files.  Not limiting the LTP IMA policy scope could
> also affect the running system.

> To allow the LTP tests to be re-run without rebooting the system, limit the
> scope of the LTP policy rules to the loopback mounted filesystem based on
> the UUID.
Thanks a lot for this, that'll be a great simplification for IMA testing.
I'll have a deeper look tomorrow, but what we need is to ima_setup.sh is to
always have loopback device. ATM it's just only if TMPDIR is tmpfs.
See patch below (untested, I'll test it tomorrow).

Also is the kernel code path very different to use UUID from the current code?
If yes, we might want also to keep the old behavior enabled with some environment
variable (the default would be to use UUID). Or not worth of keeping it?

I'd also wish to have simple C implementation instead requesting blkid
(although util-linux is very common, it's an extra dependency).
I might write simple C code which finds which UUID in /dev/disk/by-uuid/ is for
loop device should be pretty simple code. But for now it's ok to use blkid,
although it should be added into TST_NEEDS_CMDS.

...
> diff --git a/testcases/kernel/security/integrity/ima/tests/ima_conditionals.sh b/testcases/kernel/security/integrity/ima/tests/ima_conditionals.sh
> index 0d50db906..d5c5f3ebe 100755
> --- a/testcases/kernel/security/integrity/ima/tests/ima_conditionals.sh
> +++ b/testcases/kernel/security/integrity/ima/tests/ima_conditionals.sh
> @@ -28,7 +28,7 @@ verify_measurement()
>  	ROD rm -f $test_file

>  	tst_res TINFO "verify measuring user files when requested via $request"
> -	ROD echo "measure $request=$value" \> $IMA_POLICY
> +	ROD echo "measure $FSUUID $request=$value" \> $IMA_POLICY
>  	ROD echo "$(cat /proc/uptime) $request test" \> $test_file

>  	case "$request" in
> diff --git a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
> index af1fb0028..95e7331a4 100755
> --- a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
> +++ b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
> @@ -27,7 +27,12 @@ load_policy()
>  	exec 2>/dev/null 4>$IMA_POLICY
>  	[ $? -eq 0 ] || exit 1

> -	cat $1 >&4 2> /dev/null
> +	if [ -n "$FSUUID" ]; then
Interesting, would it be correct if there is no UUID with my changes below (i.e.
always use the loop device)? Actually, do we also want to have way to disable
loop device (obviously only on TMPDIR not being tmpfs).
> +		sed "s/measure /measure $FSUUID /" $1 >&4 2> /dev/null
> +	else
> +		cat $1 >&4 2> /dev/null
> +	fi
> +
>  	ret=$?
>  	exec 4>&-

> diff --git a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
> index df3fc5603..016a68cb2 100644
> --- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
> +++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
> @@ -178,6 +178,10 @@ ima_setup()
>  	if [ "$TST_MOUNT_DEVICE" = 1 ]; then
>  		tst_res TINFO "\$TMPDIR is on tmpfs => run on loop device"
>  		cd "$TST_MNTPOINT"
> +
> +		loopdev=$(mount | grep $TST_MNTPOINT | cut -f1 -d' ')
We have $TST_DEVICE for this.

> +		FSUUID="fsuuid=$(blkid | grep $loopdev | cut -f2 -d'"')"
> +		tst_res TINFO "LTP IMA policy rules based on $FSUUID"
>  	fi

>  	[ -n "$TST_SETUP_CALLER" ] && $TST_SETUP_CALLER

Proposed (not yet tested) changes.

Kind regards,
Petr

diff --git testcases/kernel/security/integrity/ima/tests/ima_setup.sh testcases/kernel/security/integrity/ima/tests/ima_setup.sh
index 016a68cb2..dd88fbc71 100644
--- testcases/kernel/security/integrity/ima/tests/ima_setup.sh
+++ testcases/kernel/security/integrity/ima/tests/ima_setup.sh
@@ -11,9 +11,7 @@ TST_CLEANUP_CALLER="$TST_CLEANUP"
 TST_CLEANUP="ima_cleanup"
 TST_NEEDS_ROOT=1
 TST_MOUNT_DEVICE=1
-
-# TST_MOUNT_DEVICE can be unset, therefore specify explicitly
-TST_NEEDS_TMPDIR=1
+TST_NEEDS_CMDS="$TST_NEEDS_CMDS blkid"
 
 SYSFS="/sys"
 UMOUNT=
@@ -179,8 +177,7 @@ ima_setup()
 		tst_res TINFO "\$TMPDIR is on tmpfs => run on loop device"
 		cd "$TST_MNTPOINT"
 
-		loopdev=$(mount | grep $TST_MNTPOINT | cut -f1 -d' ')
-		FSUUID="fsuuid=$(blkid | grep $loopdev | cut -f2 -d'"')"
+		FSUUID="fsuuid=$(blkid | grep $TST_DEVICE | cut -f2 -d'"')"
 		tst_res TINFO "LTP IMA policy rules based on $FSUUID"
 	fi
 
@@ -339,10 +336,4 @@ require_evmctl()
 	fi
 }
 
-# loop device is needed to use only for tmpfs
-TMPDIR="${TMPDIR:-/tmp}"
-if tst_supported_fs -d $TMPDIR -s "tmpfs"; then
-	unset TST_MOUNT_DEVICE
-fi
-
 . tst_test.sh
Mimi Zohar Oct. 6, 2022, 10:55 p.m. UTC | #2
Hi Petr,

On Thu, 2022-10-06 at 23:02 +0200, Petr Vorel wrote:
> Hi Mimi,
> 
> > The LTP policy rules either replace or extend the global IMA policy. As a
> > result, the ordering of the LTP IMA tests is important and affects the
> > ability of re-running the tests.  For example, ima_conditionals.sh
> > defines a rule to measure user files, while ima_measuremnets.sh verifies
> > not measuring user files.  Not limiting the LTP IMA policy scope could
> > also affect the running system.
> 
> > To allow the LTP tests to be re-run without rebooting the system, limit the
> > scope of the LTP policy rules to the loopback mounted filesystem based on
> > the UUID.
> Thanks a lot for this, that'll be a great simplification for IMA testing.

By limiting the scope of the IMA policy rules, not everything would
have to be signed on the file system, which brings us one step closer
to defining LTP appraise policy rules.

> I'll have a deeper look tomorrow, but what we need is to ima_setup.sh is to
> always have loopback device. ATM it's just only if TMPDIR is tmpfs.
> See patch below (untested, I'll test it tomorrow).

Agreed.   Seems to be working.  :)

> 
> Also is the kernel code path very different to use UUID from the current code?

The code path is the same - either the policy rule matches or it
doesn't.  Previously, however, the test files being measured could have
been located on any filesystem.  With this change, the test files now
have to be on the UUID filesystem.

> If yes, we might want also to keep the old behavior enabled with some environment
> variable (the default would be to use UUID). Or not worth of keeping it?

Instead of keeping the old behavior, how about defining additional file
tests that do not match the new UUID policy rule?   These files will
not be measured.

> I'd also wish to have simple C implementation instead requesting blkid
> (although util-linux is very common, it's an extra dependency).
> I might write simple C code which finds which UUID in /dev/disk/by-uuid/ is for
> loop device should be pretty simple code. But for now it's ok to use blkid,
> although it should be added into TST_NEEDS_CMDS.

Sure.  I posted this patch more as a proof of concept than anything
else.

> ...
> > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_conditionals.sh b/testcases/kernel/security/integrity/ima/tests/ima_conditionals.sh
> > index 0d50db906..d5c5f3ebe 100755
> > --- a/testcases/kernel/security/integrity/ima/tests/ima_conditionals.sh
> > +++ b/testcases/kernel/security/integrity/ima/tests/ima_conditionals.sh
> > @@ -28,7 +28,7 @@ verify_measurement()
> >  	ROD rm -f $test_file
> 
> >  	tst_res TINFO "verify measuring user files when requested via $request"
> > -	ROD echo "measure $request=$value" \> $IMA_POLICY
> > +	ROD echo "measure $FSUUID $request=$value" \> $IMA_POLICY
> >  	ROD echo "$(cat /proc/uptime) $request test" \> $test_file
> 
> >  	case "$request" in
> > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
> > index af1fb0028..95e7331a4 100755
> > --- a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
> > +++ b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
> > @@ -27,7 +27,12 @@ load_policy()
> >  	exec 2>/dev/null 4>$IMA_POLICY
> >  	[ $? -eq 0 ] || exit 1
> 
> > -	cat $1 >&4 2> /dev/null
> > +	if [ -n "$FSUUID" ]; then
> Interesting, would it be correct if there is no UUID with my changes below (i.e.
> always use the loop device)? Actually, do we also want to have way to disable
> loop device (obviously only on TMPDIR not being tmpfs).

If/when using a non loopback device, there should at least be a major
warning that the global policy has been modified.

> > +		sed "s/measure /measure $FSUUID /" $1 >&4 2> /dev/null
> > +	else
> > +		cat $1 >&4 2> /dev/null
> > +	fi
> > +
> >  	ret=$?
> >  	exec 4>&-
> 
> > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
> > index df3fc5603..016a68cb2 100644
> > --- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
> > +++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
> > @@ -178,6 +178,10 @@ ima_setup()
> >  	if [ "$TST_MOUNT_DEVICE" = 1 ]; then
> >  		tst_res TINFO "\$TMPDIR is on tmpfs => run on loop device"
> >  		cd "$TST_MNTPOINT"
> > +
> > +		loopdev=$(mount | grep $TST_MNTPOINT | cut -f1 -d' ')
> We have $TST_DEVICE for this.
> 
> > +		FSUUID="fsuuid=$(blkid | grep $loopdev | cut -f2 -d'"')"
> > +		tst_res TINFO "LTP IMA policy rules based on $FSUUID"
> >  	fi
> 
> >  	[ -n "$TST_SETUP_CALLER" ] && $TST_SETUP_CALLER
> 
> Proposed (not yet tested) changes.
> 

Thanks, the proposed changes seem to be working.

thanks,

Mimi

> 
> diff --git testcases/kernel/security/integrity/ima/tests/ima_setup.sh testcases/kernel/security/integrity/ima/tests/ima_setup.sh
> index 016a68cb2..dd88fbc71 100644
> --- testcases/kernel/security/integrity/ima/tests/ima_setup.sh
> +++ testcases/kernel/security/integrity/ima/tests/ima_setup.sh
> @@ -11,9 +11,7 @@ TST_CLEANUP_CALLER="$TST_CLEANUP"
>  TST_CLEANUP="ima_cleanup"
>  TST_NEEDS_ROOT=1
>  TST_MOUNT_DEVICE=1
> -
> -# TST_MOUNT_DEVICE can be unset, therefore specify explicitly
> -TST_NEEDS_TMPDIR=1
> +TST_NEEDS_CMDS="$TST_NEEDS_CMDS blkid"
>  
>  SYSFS="/sys"
>  UMOUNT=
> @@ -179,8 +177,7 @@ ima_setup()
>  		tst_res TINFO "\$TMPDIR is on tmpfs => run on loop device"
>  		cd "$TST_MNTPOINT"
>  
> -		loopdev=$(mount | grep $TST_MNTPOINT | cut -f1 -d' ')
> -		FSUUID="fsuuid=$(blkid | grep $loopdev | cut -f2 -d'"')"
> +		FSUUID="fsuuid=$(blkid | grep $TST_DEVICE | cut -f2 -d'"')"
>  		tst_res TINFO "LTP IMA policy rules based on $FSUUID"
>  	fi
>  
> @@ -339,10 +336,4 @@ require_evmctl()
>  	fi
>  }
>  
> -# loop device is needed to use only for tmpfs
> -TMPDIR="${TMPDIR:-/tmp}"
> -if tst_supported_fs -d $TMPDIR -s "tmpfs"; then
> -	unset TST_MOUNT_DEVICE
> -fi
> -
>  . tst_test.sh
Petr Vorel Oct. 7, 2022, 5:27 a.m. UTC | #3
Hi Mimi,

> Hi Petr,

> On Thu, 2022-10-06 at 23:02 +0200, Petr Vorel wrote:
> > Hi Mimi,

> > > The LTP policy rules either replace or extend the global IMA policy. As a
> > > result, the ordering of the LTP IMA tests is important and affects the
> > > ability of re-running the tests.  For example, ima_conditionals.sh
> > > defines a rule to measure user files, while ima_measuremnets.sh verifies
> > > not measuring user files.  Not limiting the LTP IMA policy scope could
> > > also affect the running system.

> > > To allow the LTP tests to be re-run without rebooting the system, limit the
> > > scope of the LTP policy rules to the loopback mounted filesystem based on
> > > the UUID.
> > Thanks a lot for this, that'll be a great simplification for IMA testing.

> By limiting the scope of the IMA policy rules, not everything would
> have to be signed on the file system, which brings us one step closer
> to defining LTP appraise policy rules.

> > I'll have a deeper look tomorrow, but what we need is to ima_setup.sh is to
> > always have loopback device. ATM it's just only if TMPDIR is tmpfs.
> > See patch below (untested, I'll test it tomorrow).

> Agreed.   Seems to be working.  :)
Thanks!

> > Also is the kernel code path very different to use UUID from the current code?

> The code path is the same - either the policy rule matches or it
> doesn't.  Previously, however, the test files being measured could have
> been located on any filesystem.  With this change, the test files now
> have to be on the UUID filesystem.

Good to know. Also, we have new feature in shell API: $TST_ALL_FILESYSTEMS (it
has been for long time for C API as .all_filesystems, which loops the test over
various filesystems: ext2, ext3, ext4, xfs, btrfs, vfat, exfat, ntfs, tmpfs.
Test therefore takes much longer, but it's worth for tests which can behave
differently on various filesystems. I suppose IMA does not need it, right?

> > If yes, we might want also to keep the old behavior enabled with some environment
> > variable (the default would be to use UUID). Or not worth of keeping it?

> Instead of keeping the old behavior, how about defining additional file
> tests that do not match the new UUID policy rule?   These files will
> not be measured.
Correct measurement outside of the loop device? I.e. something in $TST_TMPDIR?
(i.e. /tmp/foo - test unique working directory, $TST_MNTPOINT is mounted on
/tmp/foo/mntpoint, so that we still have working place outside mounted loop device).
Do you mean trying to measure something what expects to fail?

> > I'd also wish to have simple C implementation instead requesting blkid
> > (although util-linux is very common, it's an extra dependency).
> > I might write simple C code which finds which UUID in /dev/disk/by-uuid/ is for
> > loop device should be pretty simple code. But for now it's ok to use blkid,
> > although it should be added into TST_NEEDS_CMDS.

> Sure.  I posted this patch more as a proof of concept than anything
> else.
+1

> > ...
> > > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_conditionals.sh b/testcases/kernel/security/integrity/ima/tests/ima_conditionals.sh
> > > index 0d50db906..d5c5f3ebe 100755
> > > --- a/testcases/kernel/security/integrity/ima/tests/ima_conditionals.sh
> > > +++ b/testcases/kernel/security/integrity/ima/tests/ima_conditionals.sh
> > > @@ -28,7 +28,7 @@ verify_measurement()
> > >  	ROD rm -f $test_file

> > >  	tst_res TINFO "verify measuring user files when requested via $request"
> > > -	ROD echo "measure $request=$value" \> $IMA_POLICY
> > > +	ROD echo "measure $FSUUID $request=$value" \> $IMA_POLICY
> > >  	ROD echo "$(cat /proc/uptime) $request test" \> $test_file

> > >  	case "$request" in
> > > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
> > > index af1fb0028..95e7331a4 100755
> > > --- a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
> > > +++ b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
> > > @@ -27,7 +27,12 @@ load_policy()
> > >  	exec 2>/dev/null 4>$IMA_POLICY
> > >  	[ $? -eq 0 ] || exit 1

> > > -	cat $1 >&4 2> /dev/null
> > > +	if [ -n "$FSUUID" ]; then
> > Interesting, would it be correct if there is no UUID with my changes below (i.e.
> > always use the loop device)? Actually, do we also want to have way to disable
> > loop device (obviously only on TMPDIR not being tmpfs).

> If/when using a non loopback device, there should at least be a major
> warning that the global policy has been modified.
OK not quiting whole test with TBROK, but add TWARN (test continue, but later
exits with non-zero).

> > > +		sed "s/measure /measure $FSUUID /" $1 >&4 2> /dev/null
> > > +	else
> > > +		cat $1 >&4 2> /dev/null
> > > +	fi
> > > +
> > >  	ret=$?
> > >  	exec 4>&-

> > > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
> > > index df3fc5603..016a68cb2 100644
> > > --- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
> > > +++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
> > > @@ -178,6 +178,10 @@ ima_setup()
> > >  	if [ "$TST_MOUNT_DEVICE" = 1 ]; then
> > >  		tst_res TINFO "\$TMPDIR is on tmpfs => run on loop device"
> > >  		cd "$TST_MNTPOINT"
> > > +
> > > +		loopdev=$(mount | grep $TST_MNTPOINT | cut -f1 -d' ')
> > We have $TST_DEVICE for this.

> > > +		FSUUID="fsuuid=$(blkid | grep $loopdev | cut -f2 -d'"')"
> > > +		tst_res TINFO "LTP IMA policy rules based on $FSUUID"
> > >  	fi

> > >  	[ -n "$TST_SETUP_CALLER" ] && $TST_SETUP_CALLER

> > Proposed (not yet tested) changes.


> Thanks, the proposed changes seem to be working.
Thanks a lot for testing. I give it try today and merge it today or early next
week.

Kind regards,
Petr

> thanks,

> Mimi
Mimi Zohar Oct. 7, 2022, 12:56 p.m. UTC | #4
Hi Petr,

On Fri, 2022-10-07 at 07:27 +0200, Petr Vorel wrote:

> > > Also is the kernel code path very different to use UUID from the current code?
> 
> > The code path is the same - either the policy rule matches or it
> > doesn't.  Previously, however, the test files being measured could have
> > been located on any filesystem.  With this change, the test files now
> > have to be on the UUID filesystem.
> 
> Good to know. Also, we have new feature in shell API: $TST_ALL_FILESYSTEMS (it
> has been for long time for C API as .all_filesystems, which loops the test over
> various filesystems: ext2, ext3, ext4, xfs, btrfs, vfat, exfat, ntfs, tmpfs.
> Test therefore takes much longer, but it's worth for tests which can behave
> differently on various filesystems. I suppose IMA does not need it, right?

Nice!  IMA code paths are different on filesystems with/without
i_version support.   With the proposed i_version kernel
changes, ima_measurement.sh test2 is really important.

On filesystems without i_version support, after a file has been opened
for write, on fput IMA assumes the file has been modified.  On next
access, the file is re-verified/re-measured.

I'm not sure if ima_measurement.sh test2, which is limited to
filesystems with i_version support, should be extended or a new test
defined to detect whether a file is properly re-measured after it has
been modified on all filesystems, even those without i_version support.
 
> 
> > > If yes, we might want also to keep the old behavior enabled with some environment
> > > variable (the default would be to use UUID). Or not worth of keeping it?
> 
> > Instead of keeping the old behavior, how about defining additional file
> > tests that do not match the new UUID policy rule?   These files will
> > not be measured.
> Correct measurement outside of the loop device? I.e. something in $TST_TMPDIR?
> (i.e. /tmp/foo - test unique working directory, $TST_MNTPOINT is mounted on
> /tmp/foo/mntpoint, so that we still have working place outside mounted loop device).
> Do you mean trying to measure something what expects to fail?

Yes, there shouldn't be a new measurement.

> > > > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
> > > > index af1fb0028..95e7331a4 100755
> > > > --- a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
> > > > +++ b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
> > > > @@ -27,7 +27,12 @@ load_policy()
> > > >  	exec 2>/dev/null 4>$IMA_POLICY
> > > >  	[ $? -eq 0 ] || exit 1
> 
> > > > -	cat $1 >&4 2> /dev/null
> > > > +	if [ -n "$FSUUID" ]; then
> > > Interesting, would it be correct if there is no UUID with my changes below (i.e.
> > > always use the loop device)? Actually, do we also want to have way to disable
> > > loop device (obviously only on TMPDIR not being tmpfs).
> 
> > If/when using a non loopback device, there should at least be a major
> > warning that the global policy has been modified.
> OK not quiting whole test with TBROK, but add TWARN (test continue, but later
> exits with non-zero).

Sounds good.

thanks,

Mimi
Petr Vorel Oct. 10, 2022, 10:41 a.m. UTC | #5
> Hi Petr,

> On Fri, 2022-10-07 at 07:27 +0200, Petr Vorel wrote:

> > > > Also is the kernel code path very different to use UUID from the current code?

> > > The code path is the same - either the policy rule matches or it
> > > doesn't.  Previously, however, the test files being measured could have
> > > been located on any filesystem.  With this change, the test files now
> > > have to be on the UUID filesystem.

> > Good to know. Also, we have new feature in shell API: $TST_ALL_FILESYSTEMS (it
> > has been for long time for C API as .all_filesystems, which loops the test over
> > various filesystems: ext2, ext3, ext4, xfs, btrfs, vfat, exfat, ntfs, tmpfs.
> > Test therefore takes much longer, but it's worth for tests which can behave
> > differently on various filesystems. I suppose IMA does not need it, right?

> Nice!  IMA code paths are different on filesystems with/without
> i_version support.   With the proposed i_version kernel
> changes, ima_measurement.sh test2 is really important.

> On filesystems without i_version support, after a file has been opened
> for write, on fput IMA assumes the file has been modified.  On next
> access, the file is re-verified/re-measured.

> I'm not sure if ima_measurement.sh test2, which is limited to
> filesystems with i_version support, should be extended or a new test
> defined to detect whether a file is properly re-measured after it has
> been modified on all filesystems, even those without i_version support.


> > > > If yes, we might want also to keep the old behavior enabled with some environment
> > > > variable (the default would be to use UUID). Or not worth of keeping it?

> > > Instead of keeping the old behavior, how about defining additional file
> > > tests that do not match the new UUID policy rule?   These files will
> > > not be measured.
> > Correct measurement outside of the loop device? I.e. something in $TST_TMPDIR?
> > (i.e. /tmp/foo - test unique working directory, $TST_MNTPOINT is mounted on
> > /tmp/foo/mntpoint, so that we still have working place outside mounted loop device).
> > Do you mean trying to measure something what expects to fail?

> Yes, there shouldn't be a new measurement.

> > > > > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
> > > > > index af1fb0028..95e7331a4 100755
> > > > > --- a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
> > > > > +++ b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
> > > > > @@ -27,7 +27,12 @@ load_policy()
> > > > >  	exec 2>/dev/null 4>$IMA_POLICY
> > > > >  	[ $? -eq 0 ] || exit 1

> > > > > -	cat $1 >&4 2> /dev/null
> > > > > +	if [ -n "$FSUUID" ]; then
> > > > Interesting, would it be correct if there is no UUID with my changes below (i.e.
> > > > always use the loop device)? Actually, do we also want to have way to disable
> > > > loop device (obviously only on TMPDIR not being tmpfs).

BTW using fsuuid= depends on v3.9, on commit:
85865c1fa189 ("ima: add policy support for file system uuid")

v3.9 is quite old, it shouldn't be a problem, but it'd be better to add TST_MIN_KVER="3.9"

I'll send v2, just for you to check the changes.

Kind regards,
Petr

> > > If/when using a non loopback device, there should at least be a major
> > > warning that the global policy has been modified.
> > OK not quiting whole test with TBROK, but add TWARN (test continue, but later
> > exits with non-zero).

> Sounds good.

> thanks,

> Mimi
Petr Vorel Oct. 10, 2022, 11:43 a.m. UTC | #6
Hi Mimi,

FYI I have problems with ima_violations.sh, when run whole runtest/ima:

tst_device.c:89: TINFO: Found free device 0 '/dev/loop0'
ima_violations 1 TINFO: Formatting ext3 with opts='/dev/loop0'
ima_violations 1 TINFO: Mounting device: mount -t ext3 /dev/loop0 /tmp/LTP_ima_violations.Og149san78/mntpoint
ima_violations 1 TINFO: timeout per run is 0h 5m 0s
ima_violations 1 TINFO: IMA kernel config:
ima_violations 1 TINFO: CONFIG_IMA=y
ima_violations 1 TINFO: CONFIG_IMA_MEASURE_PCR_IDX=10
ima_violations 1 TINFO: CONFIG_IMA_LSM_RULES=y
ima_violations 1 TINFO: CONFIG_IMA_NG_TEMPLATE=y
ima_violations 1 TINFO: CONFIG_IMA_DEFAULT_TEMPLATE="ima-ng"
ima_violations 1 TINFO: CONFIG_IMA_DEFAULT_HASH_SHA256=y
ima_violations 1 TINFO: CONFIG_IMA_DEFAULT_HASH="sha256"
ima_violations 1 TINFO: CONFIG_IMA_READ_POLICY=y
ima_violations 1 TINFO: CONFIG_IMA_APPRAISE=y
ima_violations 1 TINFO: CONFIG_IMA_APPRAISE_BOOTPARAM=y
ima_violations 1 TINFO: CONFIG_IMA_APPRAISE_MODSIG=y
ima_violations 1 TINFO: CONFIG_IMA_TRUSTED_KEYRING=y
ima_violations 1 TINFO: CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY=y
ima_violations 1 TINFO: CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS=y
ima_violations 1 TINFO: CONFIG_IMA_QUEUE_EARLY_BOOT_KEYS=y
ima_violations 1 TINFO: /proc/cmdline: BOOT_IMAGE=/boot/vmlinuz-5.19.12-1-default root=UUID=6de93d21-b5ed-4aa4-a9e4-00a7ab77c6d9 splash=silent video=1024x768 plymouth.ignore-serial-consoles console=ttyS0 console=tty kernel.softlockup_panic=1 resume=/dev/disk/by-uuid/570474ff-969e-41e6-883e-ecff2fd15015 security=apparmor mitigations=auto ignore_loglevel lsm=integrity ima_policy=tcb
ima_violations 1 TINFO: $TMPDIR is on tmpfs => run on loop device
ima_violations 1 TINFO: LTP IMA policy rules based on fsuuid=7ab2cd65-3060-4dbc-b786-72703604a33e
ima_violations 1 TINFO: using log /var/log/audit/audit.log
ima_violations 1 TINFO: verify open writers violation
ima_violations 1 TFAIL: open_writers violation not added
ima_violations 2 TINFO: verify ToMToU violation
ima_violations 2 TFAIL: ToMToU violation not added
ima_violations 3 TINFO: verify open_writers using mmapped files
tst_test.c:1526: TINFO: Timeout per run is 0h 00m 30s
ima_mmap.c:38: TINFO: sleep 3s
ima_violations 3 TFAIL: open_writers violation not added
ima_mmap.c:41: TPASS: test completed

My fix [1] does not help. Problems are with my changes or just with your
original patch. Continue debugging.

Kind regards,
Petr

[1] https://patchwork.ozlabs.org/project/ltp/patch/20221010085944.26814-1-pvorel@suse.cz/
Mimi Zohar Oct. 12, 2022, 2:47 a.m. UTC | #7
On Mon, 2022-10-10 at 13:43 +0200, Petr Vorel wrote:
> Hi Mimi,
> 
> FYI I have problems with ima_violations.sh, when run whole runtest/ima:
> 
> tst_device.c:89: TINFO: Found free device 0 '/dev/loop0'
> ima_violations 1 TINFO: Formatting ext3 with opts='/dev/loop0'
> ima_violations 1 TINFO: Mounting device: mount -t ext3 /dev/loop0 /tmp/LTP_ima_violations.Og149san78/mntpoint
> ima_violations 1 TINFO: timeout per run is 0h 5m 0s
> ima_violations 1 TINFO: IMA kernel config:
> ima_violations 1 TINFO: CONFIG_IMA=y
> ima_violations 1 TINFO: CONFIG_IMA_MEASURE_PCR_IDX=10
> ima_violations 1 TINFO: CONFIG_IMA_LSM_RULES=y
> ima_violations 1 TINFO: CONFIG_IMA_NG_TEMPLATE=y
> ima_violations 1 TINFO: CONFIG_IMA_DEFAULT_TEMPLATE="ima-ng"
> ima_violations 1 TINFO: CONFIG_IMA_DEFAULT_HASH_SHA256=y
> ima_violations 1 TINFO: CONFIG_IMA_DEFAULT_HASH="sha256"
> ima_violations 1 TINFO: CONFIG_IMA_READ_POLICY=y
> ima_violations 1 TINFO: CONFIG_IMA_APPRAISE=y
> ima_violations 1 TINFO: CONFIG_IMA_APPRAISE_BOOTPARAM=y
> ima_violations 1 TINFO: CONFIG_IMA_APPRAISE_MODSIG=y
> ima_violations 1 TINFO: CONFIG_IMA_TRUSTED_KEYRING=y
> ima_violations 1 TINFO: CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY=y
> ima_violations 1 TINFO: CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS=y
> ima_violations 1 TINFO: CONFIG_IMA_QUEUE_EARLY_BOOT_KEYS=y

> ima_violations 1 TINFO: /proc/cmdline: BOOT_IMAGE=/boot/vmlinuz-
> 5.19.12-1-default root=UUID=6de93d21-b5ed-4aa4-a9e4-00a7ab77c6d9
> splash=silent video=1024x768 plymouth.ignore-serial-consoles
> console=ttyS0 console=tty kernel.softlockup_panic=1
> resume=/dev/disk/by-uuid/570474ff-969e-41e6-883e-ecff2fd15015
> security=apparmor mitigations=auto ignore_loglevel lsm=integrity
> ima_policy=tcb

I would use either use the original "security=" or the new "lsm=" boot
command line option.

> ima_violations 1 TINFO: $TMPDIR is on tmpfs => run on loop device
> ima_violations 1 TINFO: LTP IMA policy rules based on fsuuid=7ab2cd65-3060-4dbc-b786-72703604a33e
> ima_violations 1 TINFO: using log /var/log/audit/audit.log
> ima_violations 1 TINFO: verify open writers violation
> ima_violations 1 TFAIL: open_writers violation not added
> ima_violations 2 TINFO: verify ToMToU violation
> ima_violations 2 TFAIL: ToMToU violation not added
> ima_violations 3 TINFO: verify open_writers using mmapped files
> tst_test.c:1526: TINFO: Timeout per run is 0h 00m 30s
> ima_mmap.c:38: TINFO: sleep 3s
> ima_violations 3 TFAIL: open_writers violation not added
> ima_mmap.c:41: TPASS: test completed
> 
> My fix [1] does not help. Problems are with my changes or just with your
> original patch. Continue debugging.
> Kind regards,
> Petr
> 
> [1] https://patchwork.ozlabs.org/project/ltp/patch/20221010085944.26814-1-pvorel@suse.cz/

Only the ima_conditionals.sh and ima_policy.sh tests define policy
rules based on fsuuid.  The other tests are still based on the builtin
"ima_policy=tcb" rules.

Without seeing the output of "cat /sys/kernel/security/ima/policy" it's
hard to understand what's causing these errors.

thanks,

Mimi
Petr Vorel Oct. 12, 2022, 11:54 a.m. UTC | #8
Hi Mimi,

> On Mon, 2022-10-10 at 13:43 +0200, Petr Vorel wrote:
> > Hi Mimi,

> > FYI I have problems with ima_violations.sh, when run whole runtest/ima:

> > tst_device.c:89: TINFO: Found free device 0 '/dev/loop0'
> > ima_violations 1 TINFO: Formatting ext3 with opts='/dev/loop0'
> > ima_violations 1 TINFO: Mounting device: mount -t ext3 /dev/loop0 /tmp/LTP_ima_violations.Og149san78/mntpoint
> > ima_violations 1 TINFO: timeout per run is 0h 5m 0s
> > ima_violations 1 TINFO: IMA kernel config:
> > ima_violations 1 TINFO: CONFIG_IMA=y
> > ima_violations 1 TINFO: CONFIG_IMA_MEASURE_PCR_IDX=10
> > ima_violations 1 TINFO: CONFIG_IMA_LSM_RULES=y
> > ima_violations 1 TINFO: CONFIG_IMA_NG_TEMPLATE=y
> > ima_violations 1 TINFO: CONFIG_IMA_DEFAULT_TEMPLATE="ima-ng"
> > ima_violations 1 TINFO: CONFIG_IMA_DEFAULT_HASH_SHA256=y
> > ima_violations 1 TINFO: CONFIG_IMA_DEFAULT_HASH="sha256"
> > ima_violations 1 TINFO: CONFIG_IMA_READ_POLICY=y
> > ima_violations 1 TINFO: CONFIG_IMA_APPRAISE=y
> > ima_violations 1 TINFO: CONFIG_IMA_APPRAISE_BOOTPARAM=y
> > ima_violations 1 TINFO: CONFIG_IMA_APPRAISE_MODSIG=y
> > ima_violations 1 TINFO: CONFIG_IMA_TRUSTED_KEYRING=y
> > ima_violations 1 TINFO: CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY=y
> > ima_violations 1 TINFO: CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS=y
> > ima_violations 1 TINFO: CONFIG_IMA_QUEUE_EARLY_BOOT_KEYS=y

> > ima_violations 1 TINFO: /proc/cmdline: BOOT_IMAGE=/boot/vmlinuz-
> > 5.19.12-1-default root=UUID=6de93d21-b5ed-4aa4-a9e4-00a7ab77c6d9
> > splash=silent video=1024x768 plymouth.ignore-serial-consoles
> > console=ttyS0 console=tty kernel.softlockup_panic=1
> > resume=/dev/disk/by-uuid/570474ff-969e-41e6-883e-ecff2fd15015
> > security=apparmor mitigations=auto ignore_loglevel lsm=integrity
> > ima_policy=tcb

> I would use either use the original "security=" or the new "lsm=" boot
> command line option.
FYI lsm= ima_policy=tcb would break booting, although on 5.19 I don't se the
warning lsm asking to have integrity among the values. That's the old problem,
it'd be good to move integrity off the security hook as you suggested [2].
Therefore only "security=" or without both "lsm" or "security" kernel boots.

BTW security=apparmor is in the result of the setup from openSUSE installer. It
got back to using security, due previously mentioned problem with boot..

Although "security=apparmor lsm=integrity ima_policy=tcb" might not be a good
idea, it does not cause ima_violations.sh. Actually with all of these fails
ima_violations.sh fails:
* lsm=integrity ima_policy=tcb (without security=apparmor)
* security= ima_policy=tcb
* ima_policy=tcb

For some reason ima_violations.sh works, when run as the first test after boot
(at least with only "ima_policy=tcb" setup), but not when whole ima runtest file
is run (as there are tests run before it).  I'm still trying to figure out
what's wrong.  What do you use for running LTP IMA tests? And do you run whole
runtest file?

Questions:
* which kernel cmdline options (IMA related) makes sense to use on testing IMA?
* is it feasible to have cmdline setup which which would be suitable
for all tests (running ima runtest file) + any of these tests? At least have a
detection and TCONF instead of failure.

> > ima_violations 1 TINFO: $TMPDIR is on tmpfs => run on loop device
> > ima_violations 1 TINFO: LTP IMA policy rules based on fsuuid=7ab2cd65-3060-4dbc-b786-72703604a33e
> > ima_violations 1 TINFO: using log /var/log/audit/audit.log
> > ima_violations 1 TINFO: verify open writers violation
> > ima_violations 1 TFAIL: open_writers violation not added
> > ima_violations 2 TINFO: verify ToMToU violation
> > ima_violations 2 TFAIL: ToMToU violation not added
> > ima_violations 3 TINFO: verify open_writers using mmapped files
> > tst_test.c:1526: TINFO: Timeout per run is 0h 00m 30s
> > ima_mmap.c:38: TINFO: sleep 3s
> > ima_violations 3 TFAIL: open_writers violation not added
> > ima_mmap.c:41: TPASS: test completed

> > My fix [1] does not help. Problems are with my changes or just with your
> > original patch. Continue debugging.
> > Kind regards,
> > Petr

> > [1] https://patchwork.ozlabs.org/project/ltp/patch/20221010085944.26814-1-pvorel@suse.cz/

> Only the ima_conditionals.sh and ima_policy.sh tests define policy
> rules based on fsuuid.  The other tests are still based on the builtin
> "ima_policy=tcb" rules.
Ah, correct. What was the reason not to transform the rest?
Maybe my following patch which uses loop device for all tests wasn't a good
idea.

> Without seeing the output of "cat /sys/kernel/security/ima/policy" it's
> hard to understand what's causing these errors.
It's empty. with fsuuid based setup I'll try to use policy examples, so that it
won't TCONF, but that's a next step after we solve this.

Kind regards,
Petr

[2] https://lore.kernel.org/linux-integrity/cacde31235f08eeec698c63025a0eef81e10fe71.camel@linux.ibm.com/

> thanks,

> Mimi
Mimi Zohar Oct. 12, 2022, 1:02 p.m. UTC | #9
Hi Petr,

On Wed, 2022-10-12 at 13:54 +0200, Petr Vorel wrote:

> For some reason ima_violations.sh works, when run as the first test after boot
> (at least with only "ima_policy=tcb" setup), but not when whole ima runtest file
> is run (as there are tests run before it).  I'm still trying to figure out
> what's wrong.

Sounds like initially the tests are run with the builtin "tcb" policy. 
Loading any IMA policy rules replaces the existing builtin policy with
the new custom policy.
Petr Vorel Oct. 12, 2022, 2:39 p.m. UTC | #10
Hi Mimi,

> Hi Petr,

> On Wed, 2022-10-12 at 13:54 +0200, Petr Vorel wrote:

> > For some reason ima_violations.sh works, when run as the first test after boot
> > (at least with only "ima_policy=tcb" setup), but not when whole ima runtest file
> > is run (as there are tests run before it).  I'm still trying to figure out
> > what's wrong.

> Sounds like initially the tests are run with the builtin "tcb" policy. 
Yes, since LTP does not support reboot and IMA ima_measurements.sh requires
ima_policy=tcb, I configured VM to run all tests with ima_policy=tcb.

> Loading any IMA policy rules replaces the existing builtin policy with
> the new custom policy.

Yes, done in ima_policy.sh, which is the second test (valid policy: measure.policy).
Thus only ima_measurements.sh and ima_policy.sh are run with ima_policy=tcb.
I haven't had a time to look into ascii_runtime_measurements, but this changed
with fsuuid= (previously was working, now vails in ima_violations.sh).
I'll have look soon (I'm wasting your time if I ask before proper debugging).

As I wrote before, it'd be great if 1) running whole runtest/ima worked (either
TPASS or TCONF detect missing something in kernel or in kernel params, ...).
2) running any single tests also TPASS or TCONF.

Testers then could run tests with a different setup (builtin policies, custom
policies, ...).

Kind regards,
Petr
Petr Vorel Dec. 15, 2022, 6:39 p.m. UTC | #11
Hi Mimi,

I'm sorry, it took me long time to look into the issue.

> Only the ima_conditionals.sh and ima_policy.sh tests define policy
> rules based on fsuuid.  The other tests are still based on the builtin
> "ima_policy=tcb" rules.
Yes.

> Without seeing the output of "cat /sys/kernel/security/ima/policy" it's
> hard to understand what's causing these errors.

1) In current master content of /sys/kernel/security/ima/policy when booted with
ima_policy=tcb, which is required by ima_policy.sh:
dont_measure fsmagic=0x9fa0 
dont_measure fsmagic=0x62656572 
dont_measure fsmagic=0x64626720 
dont_measure fsmagic=0x1021994 
dont_measure fsmagic=0x1cd1 
dont_measure fsmagic=0x42494e4d 
dont_measure fsmagic=0x73636673 
dont_measure fsmagic=0xf97cff8c 
dont_measure fsmagic=0x43415d53 
dont_measure fsmagic=0x27e0eb 
dont_measure fsmagic=0x63677270 
dont_measure fsmagic=0x6e736673 
dont_measure fsmagic=0xde5e81e4 
measure func=MMAP_CHECK mask=MAY_EXEC 
measure func=BPRM_CHECK mask=MAY_EXEC 
measure func=FILE_CHECK mask=^MAY_READ euid=0 
measure func=FILE_CHECK mask=^MAY_READ uid=0 
measure func=MODULE_CHECK 
measure func=FIRMWARE_CHECK 
measure func=POLICY_CHECK 

2) if I run ima_violations.sh now (in runtest/ima is run after ima_policy.sh.),
policy does not change as test does not touch it and test pass.

3) Then run ima_policy.sh, the policy changed:
dont_measure fsmagic=0x9fa0 
dont_measure fsmagic=0x62656572 
dont_measure fsmagic=0x64626720 
dont_measure fsmagic=0x1021994 
dont_measure fsmagic=0x73636673 
measure func=MMAP_CHECK mask=MAY_EXEC 
measure func=BPRM_CHECK mask=MAY_EXEC 
measure func=FILE_CHECK mask=MAY_READ uid=0 

4) Obviously the policy file is the same after repeated run of ima_policy.sh.

Now with your patchset.
The policy file after 1) and 2) is obviously the same.

3) Now ima_policy.sh with your patchset:
ima_policy 1 TINFO: Formatting ext3 with opts='/dev/loop0'
ima_policy 1 TINFO: Mounting device: mount -t ext3 /dev/loop0 /tmp/LTP_ima_policy.c0pGs7haPF/mntpoint 
ima_policy 1 TINFO: timeout per run is 0h 5m 0s
ima_policy 1 TINFO: IMA kernel config:
ima_policy 1 TINFO: CONFIG_IMA=y
ima_policy 1 TINFO: CONFIG_IMA_MEASURE_PCR_IDX=10
ima_policy 1 TINFO: CONFIG_IMA_LSM_RULES=y
ima_policy 1 TINFO: CONFIG_IMA_NG_TEMPLATE=y
ima_policy 1 TINFO: CONFIG_IMA_DEFAULT_TEMPLATE="ima-ng"
ima_policy 1 TINFO: CONFIG_IMA_DEFAULT_HASH_SHA256=y
ima_policy 1 TINFO: CONFIG_IMA_DEFAULT_HASH="sha256"
ima_policy 1 TINFO: CONFIG_IMA_READ_POLICY=y
ima_policy 1 TINFO: CONFIG_IMA_APPRAISE=y
ima_policy 1 TINFO: CONFIG_IMA_APPRAISE_BOOTPARAM=y
ima_policy 1 TINFO: CONFIG_IMA_APPRAISE_MODSIG=y
ima_policy 1 TINFO: CONFIG_IMA_TRUSTED_KEYRING=y
ima_policy 1 TINFO: CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY=y
ima_policy 1 TINFO: CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS=y
ima_policy 1 TINFO: CONFIG_IMA_QUEUE_EARLY_BOOT_KEYS=y
ima_policy 1 TINFO: /proc/cmdline: BOOT_IMAGE=/boot/vmlinuz-6.0.7-1-default root=UUID=6de93d21-b5ed-4aa4-a9e4-00a7ab77c6d9 splash=silent video=1024x768 plymouth.ignore-serial-consoles console=ttyS0 console=tty kernel.softlockup_panic=1 resume=/dev/disk/by-uuid/570474ff-969e-41e6-883e-ecff2fd15015 mitigations=auto ignore_loglevel lsm=integrity ima_policy=tcb
ima_policy 1 TINFO: $TMPDIR is on tmpfs => run on loop device
ima_policy 1 TINFO: LTP IMA policy rules based on fsuuid=7e3ca343-13c0-4464-8375-489cd3a57c0f
ima_policy 1 TINFO: verify that invalid policy isn't loaded
ima_policy 1 TPASS: didn't load invalid policy
ima_policy 2 TINFO: verify that policy file is not opened concurrently and able to loaded multiple times
ima_policy 2 TPASS: policy was loaded just by one process and able to loaded multiple times

The default ima_policy=tcb is changed by fsuuid in ima_policy.sh:
dont_measure fsmagic=0x9fa0 fsuuid=7e3ca343-13c0-4464-8375-489cd3a57c0f 
dont_measure fsmagic=0x62656572 fsuuid=7e3ca343-13c0-4464-8375-489cd3a57c0f 
dont_measure fsmagic=0x64626720 fsuuid=7e3ca343-13c0-4464-8375-489cd3a57c0f 
dont_measure fsmagic=0x1021994 fsuuid=7e3ca343-13c0-4464-8375-489cd3a57c0f 
dont_measure fsmagic=0x73636673 fsuuid=7e3ca343-13c0-4464-8375-489cd3a57c0f 
measure func=MMAP_CHECK mask=MAY_EXEC fsuuid=7e3ca343-13c0-4464-8375-489cd3a57c0f 
measure func=BPRM_CHECK mask=MAY_EXEC fsuuid=7e3ca343-13c0-4464-8375-489cd3a57c0f 
measure func=FILE_CHECK mask=MAY_READ fsuuid=7e3ca343-13c0-4464-8375-489cd3a57c0f uid=0 

4) running ima_violations.sh after ima_policy.sh no longer works, because
there is nothing new in /var/log/audit/audit.log. I don't know why, but
ima_violations.sh requires either the default ima_policy=tcb policy or policy
created by ima_policy.sh *without* fsuuid.

FYI below is content of /var/log/audit/audit.log.

Also looking at things twice, fsuuid does not help testing much.
Because main blocker for testing is not the scope of the policy, but write once
policy - CONFIG_IMA_WRITE_POLICY not being set on distro kernels thus repeated
write of the policy will need reboot.

Rebooting actually might be possible sooner or later with new runltp-ng from
Andrea [1] (the feature is not here yet, but will be sooner or later). runltp-ng
is close to upstream, there was first attempt [2].

Other option would be to prepare policy which would be suitable for all tests,
with help of fsuuid. But that has drawback:
Currently we use LTP API to mount directories on loop device after test has started.
These devices are temporary, e.g.
/tmp/LTP_ima_violations.pEvyfJO7Af/mntpoint/test.txt will be unmounted and
deleted after each test run. But for fsuuid we'd need to first permanently
mount the devices to get their UUID. Therefore there would have to be some
special setup script needed to be run for all tests. This has proven to be
problematic in the past. I'd have to extend the API to create something permanent.

Other question is whether IMA is tested by anybody with different build configuration.

I also looked to kselftest, but the only tests I found are some bpf related
(bpf_ima_*_hash tests: bpf/ima_setup.sh, used by bpf/prog_tests/test_ima.c,
bpf/progs/ima.c, which require also CONFIG_IMA_WRITE_POLICY, thus not usable on
distro kernels). There are tests in ima-evm-utils, but I thought they target to test evmctl,
although that's actually some testing of whole kernel and userspace IMA.

Kind regards,
Petr

[1] https://github.com/acerv/runltp-ng
[2] https://lore.kernel.org/ltp/20221202103011.12206-1-andrea.cervesato@suse.com/

=== current master ===
# PATH="/opt/ltp/testcases/bin:$PATH" ./ima_violations.sh
type=INTEGRITY_PCR msg=audit(1670857707.178:126): pid=1438 uid=0 auid=0 ses=3 op=invalid_pcr cause=open_writers comm="grep" name="/var/log/audit/audit.log" dev="vda2" ino=8182 res=1 errno=0
type=INTEGRITY_PCR msg=audit(1670857707.186:127): pid=1374 uid=0 auid=0 ses=3 op=invalid_pcr cause=open_writers comm="ima_violations." name="/tmp/LTP_ima_violations.pEvyfJO7Af/mntpoint/test.txt" dev="loop0" ino=6073 res=1 errno=0
type=INTEGRITY_PCR msg=audit(1670857707.190:128): pid=1441 uid=0 auid=0 ses=3 op=invalid_pcr cause=open_writers comm="grep" name="/var/log/audit/audit.log" dev="vda2" ino=8182 res=1 errno=0
type=INTEGRITY_PCR msg=audit(1670857707.202:129): pid=1445 uid=0 auid=0 ses=3 op=invalid_pcr cause=open_writers comm="grep" name="/var/log/audit/audit.log" dev="vda2" ino=8182 res=1 errno=0
type=INTEGRITY_PCR msg=audit(1670857707.206:130): pid=1374 uid=0 auid=0 ses=3 op=invalid_pcr cause=ToMToU comm="ima_violations." name="/tmp/LTP_ima_violations.pEvyfJO7Af/mntpoint/test.txt" dev="loop0" ino=6073 res=1 errno=0
type=INTEGRITY_PCR msg=audit(1670857707.210:131): pid=1448 uid=0 auid=0 ses=3 op=invalid_pcr cause=open_writers comm="grep" name="/var/log/audit/audit.log" dev="vda2" ino=8182 res=1 errno=0
type=INTEGRITY_PCR msg=audit(1670857707.218:132): pid=1452 uid=0 auid=0 ses=3 op=invalid_pcr cause=open_writers comm="grep" name="/var/log/audit/audit.log" dev="vda2" ino=8182 res=1 errno=0
type=INTEGRITY_PCR msg=audit(1670857708.230:133): pid=1374 uid=0 auid=0 ses=3 op=invalid_pcr cause=open_writers comm="ima_violations." name="/tmp/LTP_ima_violations.pEvyfJO7Af/mntpoint/test.txt" dev="loop0" ino=6073 res=1 errno=0
type=INTEGRITY_PCR msg=audit(1670857708.246:134): pid=1458 uid=0 auid=0 ses=3 op=invalid_pcr cause=open_writers comm="grep" name="/var/log/audit/audit.log" dev="vda2" ino=8182 res=1 errno=0
=> test passes

# PATH="/opt/ltp/testcases/bin:$PATH" ./ima_policy.sh
type=INTEGRITY_STATUS msg=audit(1670857735.235:139): pid=1476 uid=0 auid=0 ses=3 op=policy_update cause=failed comm="ima_policy.sh" res=0 errno=0
type=INTEGRITY_STATUS msg=audit(1670857735.243:140): pid=1476 uid=0 auid=0 ses=3 op=policy_update cause=failed comm="ima_policy.sh" res=0 errno=0
type=INTEGRITY_POLICY_RULE msg=audit(1670857735.247:141): action=dont_measure fsmagic=0x9fa0 res=1
type=INTEGRITY_POLICY_RULE msg=audit(1670857735.247:142): action=dont_measure fsmagic=0x62656572 res=1
type=INTEGRITY_POLICY_RULE msg=audit(1670857735.247:143): action=dont_measure fsmagic=0x64626720 res=1
type=INTEGRITY_POLICY_RULE msg=audit(1670857735.247:144): action=dont_measure fsmagic=0x01021994 res=1
type=INTEGRITY_POLICY_RULE msg=audit(1670857735.247:145): UNKNOWN=dnt_measure res=0
type=INTEGRITY_STATUS msg=audit(1670857735.247:146): pid=1519 uid=0 auid=0 ses=3 op=update_policy cause=invalid-policy comm="cat" res=0 errno=0
type=INTEGRITY_STATUS msg=audit(1670857735.251:147): pid=1518 uid=0 auid=0 ses=3 op=policy_update cause=failed comm="ima_policy.sh" res=0 errno=0
type=INTEGRITY_STATUS msg=audit(1670857735.259:148): pid=1476 uid=0 auid=0 ses=3 op=policy_update cause=failed comm="ima_policy.sh" res=0 errno=0
type=INTEGRITY_POLICY_RULE msg=audit(1670857735.263:149): action=dont_measure fsmagic=0x9fa0 res=1
type=INTEGRITY_POLICY_RULE msg=audit(1670857735.263:150): action=dont_measure fsmagic=0x62656572 res=1
type=INTEGRITY_POLICY_RULE msg=audit(1670857735.263:151): action=dont_measure fsmagic=0x64626720 res=1
type=INTEGRITY_POLICY_RULE msg=audit(1670857735.263:152): action=dont_measure fsmagic=0x01021994 res=1
type=INTEGRITY_POLICY_RULE msg=audit(1670857735.263:153): action=dont_measure fsmagic=0x73636673 res=1
type=INTEGRITY_POLICY_RULE msg=audit(1670857735.263:154): action=measure func=FILE_MMAP mask=MAY_EXEC res=1
type=INTEGRITY_POLICY_RULE msg=audit(1670857735.263:155): action=measure func=BPRM_CHECK mask=MAY_EXEC res=1
type=INTEGRITY_POLICY_RULE msg=audit(1670857735.263:156): action=measure func=FILE_CHECK mask=MAY_READ uid=0 res=1
type=INTEGRITY_STATUS msg=audit(1670857735.267:157): pid=1523 uid=0 auid=0 ses=3 op=policy_update cause=completed comm="ima_policy.sh" res=1 errno=0

# PATH="/opt/ltp/testcases/bin:$PATH" ./ima_violations.sh
type=INTEGRITY_PCR msg=audit(1670857748.427:158): pid=1581 uid=0 auid=0 ses=3 op=invalid_pcr cause=open_writers comm="grep" name="/var/log/audit/audit.log" dev="vda2" ino=8182 res=1 errno=0
type=INTEGRITY_PCR msg=audit(1670857748.435:159): pid=1539 uid=0 auid=0 ses=3 op=invalid_pcr cause=open_writers comm="ima_violations." name="/tmp/LTP_ima_violations.F0oDEcjtAh/mntpoint/test.txt" dev="loop0" ino=6073 res=1 errno=0
type=INTEGRITY_PCR msg=audit(1670857748.439:160): pid=1584 uid=0 auid=0 ses=3 op=invalid_pcr cause=open_writers comm="grep" name="/var/log/audit/audit.log" dev="vda2" ino=8182 res=1 errno=0
type=INTEGRITY_PCR msg=audit(1670857748.451:161): pid=1588 uid=0 auid=0 ses=3 op=invalid_pcr cause=open_writers comm="grep" name="/var/log/audit/audit.log" dev="vda2" ino=8182 res=1 errno=0
type=INTEGRITY_PCR msg=audit(1670857748.451:162): pid=1539 uid=0 auid=0 ses=3 op=invalid_pcr cause=ToMToU comm="ima_violations." name="/tmp/LTP_ima_violations.F0oDEcjtAh/mntpoint/test.txt" dev="loop0" ino=6073 res=1 errno=0
type=INTEGRITY_PCR msg=audit(1670857748.455:163): pid=1591 uid=0 auid=0 ses=3 op=invalid_pcr cause=open_writers comm="grep" name="/var/log/audit/audit.log" dev="vda2" ino=8182 res=1 errno=0
type=INTEGRITY_PCR msg=audit(1670857748.467:164): pid=1595 uid=0 auid=0 ses=3 op=invalid_pcr cause=open_writers comm="grep" name="/var/log/audit/audit.log" dev="vda2" ino=8182 res=1 errno=0
type=INTEGRITY_PCR msg=audit(1670857749.475:165): pid=1539 uid=0 auid=0 ses=3 op=invalid_pcr cause=open_writers comm="ima_violations." name="/tmp/LTP_ima_violations.F0oDEcjtAh/mntpoint/test.txt" dev="loop0" ino=6073 res=1 errno=0
type=INTEGRITY_PCR msg=audit(1670857749.487:166): pid=1601 uid=0 auid=0 ses=3 op=invalid_pcr cause=open_writers comm="grep" name="/var/log/audit/audit.log" dev="vda2" ino=8182 res=1 errno=0
=> again, test passes, logs produced by ima_violations.sh are the same as were on previous run

=== your v1 patchset ===
# PATH="/opt/ltp/testcases/bin:$PATH" ./ima_violations.sh
type=INTEGRITY_PCR msg=audit(1670857298.171:130): pid=1504 uid=0 auid=0 ses=1 op=invalid_pcr cause=open_writers comm="grep" name="/var/log/audit/audit.log" dev="vda2" ino=8182 res=1 errno=0
type=INTEGRITY_PCR msg=audit(1670857298.179:131): pid=1434 uid=0 auid=0 ses=1 op=invalid_pcr cause=open_writers comm="ima_violations." name="/tmp/LTP_ima_violations.gLp6AVWCuS/mntpoint/test.txt" dev="loop0" ino=6073 res=1 errno=0
type=INTEGRITY_PCR msg=audit(1670857298.183:132): pid=1507 uid=0 auid=0 ses=1 op=invalid_pcr cause=open_writers comm="grep" name="/var/log/audit/audit.log" dev="vda2" ino=8182 res=1 errno=0
type=INTEGRITY_PCR msg=audit(1670857298.191:133): pid=1511 uid=0 auid=0 ses=1 op=invalid_pcr cause=open_writers comm="grep" name="/var/log/audit/audit.log" dev="vda2" ino=8182 res=1 errno=0
type=INTEGRITY_PCR msg=audit(1670857298.195:134): pid=1434 uid=0 auid=0 ses=1 op=invalid_pcr cause=ToMToU comm="ima_violations." name="/tmp/LTP_ima_violations.gLp6AVWCuS/mntpoint/test.txt" dev="loop0" ino=6073 res=1 errno=0
type=INTEGRITY_PCR msg=audit(1670857298.199:135): pid=1514 uid=0 auid=0 ses=1 op=invalid_pcr cause=open_writers comm="grep" name="/var/log/audit/audit.log" dev="vda2" ino=8182 res=1 errno=0
type=INTEGRITY_PCR msg=audit(1670857298.211:136): pid=1518 uid=0 auid=0 ses=1 op=invalid_pcr cause=open_writers comm="grep" name="/var/log/audit/audit.log" dev="vda2" ino=8182 res=1 errno=0
type=INTEGRITY_PCR msg=audit(1670857299.223:137): pid=1434 uid=0 auid=0 ses=1 op=invalid_pcr cause=open_writers comm="ima_violations." name="/tmp/LTP_ima_violations.gLp6AVWCuS/mntpoint/test.txt" dev="loop0" ino=6073 res=1 errno=0
type=INTEGRITY_PCR msg=audit(1670857299.235:138): pid=1524 uid=0 auid=0 ses=1 op=invalid_pcr cause=open_writers comm="grep" name="/var/log/audit/audit.log" dev="vda2" ino=8182 res=1 errno=0
=> so far so good

# PATH="/opt/ltp/testcases/bin:$PATH" ./ima_policy.sh
type=INTEGRITY_STATUS msg=audit(1670857346.435:139): pid=1541 uid=0 auid=0 ses=1 op=policy_update cause=failed comm="ima_policy.sh" res=0 errno=0
type=INTEGRITY_STATUS msg=audit(1670857346.443:140): pid=1541 uid=0 auid=0 ses=1 op=policy_update cause=failed comm="ima_policy.sh" res=0 errno=0
type=INTEGRITY_POLICY_RULE msg=audit(1670857346.447:141): action=dont_measure fsuuid=23a85f4a-3f53-49e4-9079-2c7e5ba1c9fa fsmagic=0x9fa0 res=1
type=INTEGRITY_POLICY_RULE msg=audit(1670857346.447:142): action=dont_measure fsuuid=23a85f4a-3f53-49e4-9079-2c7e5ba1c9fa fsmagic=0x62656572 res=1
type=INTEGRITY_POLICY_RULE msg=audit(1670857346.447:143): action=dont_measure fsuuid=23a85f4a-3f53-49e4-9079-2c7e5ba1c9fa fsmagic=0x64626720 res=1
type=INTEGRITY_POLICY_RULE msg=audit(1670857346.447:144): action=dont_measure fsuuid=23a85f4a-3f53-49e4-9079-2c7e5ba1c9fa fsmagic=0x01021994 res=1
type=INTEGRITY_POLICY_RULE msg=audit(1670857346.447:145): UNKNOWN=dnt_measure res=0
type=INTEGRITY_STATUS msg=audit(1670857346.447:146): pid=1594 uid=0 auid=0 ses=1 op=update_policy cause=invalid-policy comm="sed" res=0 errno=0
type=INTEGRITY_STATUS msg=audit(1670857346.451:147): pid=1593 uid=0 auid=0 ses=1 op=policy_update cause=failed comm="ima_policy.sh" res=0 errno=0
type=INTEGRITY_STATUS msg=audit(1670857346.455:148): pid=1541 uid=0 auid=0 ses=1 op=policy_update cause=failed comm="ima_policy.sh" res=0 errno=0
type=INTEGRITY_POLICY_RULE msg=audit(1670857346.463:149): action=dont_measure fsuuid=23a85f4a-3f53-49e4-9079-2c7e5ba1c9fa fsmagic=0x9fa0 res=1
type=INTEGRITY_POLICY_RULE msg=audit(1670857346.463:150): action=dont_measure fsuuid=23a85f4a-3f53-49e4-9079-2c7e5ba1c9fa fsmagic=0x62656572 res=1
type=INTEGRITY_POLICY_RULE msg=audit(1670857346.463:151): action=dont_measure fsuuid=23a85f4a-3f53-49e4-9079-2c7e5ba1c9fa fsmagic=0x64626720 res=1
type=INTEGRITY_POLICY_RULE msg=audit(1670857346.463:152): action=dont_measure fsuuid=23a85f4a-3f53-49e4-9079-2c7e5ba1c9fa fsmagic=0x01021994 res=1
type=INTEGRITY_POLICY_RULE msg=audit(1670857346.463:153): action=dont_measure fsuuid=23a85f4a-3f53-49e4-9079-2c7e5ba1c9fa fsmagic=0x73636673 res=1
type=INTEGRITY_POLICY_RULE msg=audit(1670857346.463:154): action=measure fsuuid=23a85f4a-3f53-49e4-9079-2c7e5ba1c9fa func=FILE_MMAP mask=MAY_EXEC res=1
type=INTEGRITY_POLICY_RULE msg=audit(1670857346.463:155): action=measure fsuuid=23a85f4a-3f53-49e4-9079-2c7e5ba1c9fa func=BPRM_CHECK mask=MAY_EXEC res=1
type=INTEGRITY_POLICY_RULE msg=audit(1670857346.463:156): action=measure fsuuid=23a85f4a-3f53-49e4-9079-2c7e5ba1c9fa func=FILE_CHECK mask=MAY_READ uid=0 res=1
type=INTEGRITY_STATUS msg=audit(1670857346.463:157): pid=1598 uid=0 auid=0 ses=1 op=policy_update cause=completed comm="ima_policy.sh" res=1 errno=0
=> obviously, there is fsuuid=... and comm="sed" instead of comm="cat"

# PATH="/opt/ltp/testcases/bin:$PATH" ./ima_violations.sh
tst_device.c:93: TINFO: Found free device 0 '/dev/loop0'
ima_violations 1 TINFO: Formatting ext3 with opts='/dev/loop0'
ima_violations 1 TINFO: Mounting device: mount -t ext3 /dev/loop0 /tmp/LTP_ima_violations.6QLCCXonjt/mntpoint 
ima_violations 1 TINFO: timeout per run is 0h 5m 0s
ima_violations 1 TINFO: IMA kernel config:
ima_violations 1 TINFO: CONFIG_IMA=y
ima_violations 1 TINFO: CONFIG_IMA_MEASURE_PCR_IDX=10
ima_violations 1 TINFO: CONFIG_IMA_LSM_RULES=y
ima_violations 1 TINFO: CONFIG_IMA_NG_TEMPLATE=y
ima_violations 1 TINFO: CONFIG_IMA_DEFAULT_TEMPLATE="ima-ng"
ima_violations 1 TINFO: CONFIG_IMA_DEFAULT_HASH_SHA256=y
ima_violations 1 TINFO: CONFIG_IMA_DEFAULT_HASH="sha256"
ima_violations 1 TINFO: CONFIG_IMA_READ_POLICY=y
ima_violations 1 TINFO: CONFIG_IMA_APPRAISE=y
ima_violations 1 TINFO: CONFIG_IMA_APPRAISE_BOOTPARAM=y
ima_violations 1 TINFO: CONFIG_IMA_APPRAISE_MODSIG=y
ima_violations 1 TINFO: CONFIG_IMA_TRUSTED_KEYRING=y
ima_violations 1 TINFO: CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY=y
ima_violations 1 TINFO: CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS=y
ima_violations 1 TINFO: CONFIG_IMA_QUEUE_EARLY_BOOT_KEYS=y
ima_violations 1 TINFO: /proc/cmdline: BOOT_IMAGE=/boot/vmlinuz-6.0.7-1-default root=UUID=6de93d21-b5ed-4aa4-a9e4-00a7ab77c6d9 splash=silent video=1024x768 plymouth.ignore-serial-consoles console=ttyS0 console=tty kernel.softlockup_panic=1 resume=/dev/disk/by-uuid/570474ff-969e-41e6-883e-ecff2fd15015 mitigations=auto ignore_loglevel lsm=integrity ima_policy=tcb
ima_violations 1 TINFO: $TMPDIR is on tmpfs => run on loop device
ima_violations 1 TINFO: LTP IMA policy rules based on fsuuid=b1314073-12d0-49c7-bbd0-19bcbaa37182
ima_violations 1 TINFO: using log /var/log/audit/audit.log
ima_violations 1 TINFO: verify open writers violation
ima_violations 1 TFAIL: open_writers violation not added
ima_violations 2 TINFO: verify ToMToU violation
ima_violations 2 TFAIL: ToMToU violation not added
ima_violations 3 TINFO: verify open_writers using mmapped files
tst_test.c:1558: TINFO: Timeout per run is 0h 00m 30s
ima_mmap.c:38: TINFO: sleep 3s
ima_violations 3 TFAIL: open_writers violation not added
ima_mmap.c:41: TPASS: test completed
=> There is no outpout in /var/log/audit/audit.log, thus test is correct that
it fails. On a previous (good) run, 2 new violations are added to
/sys/kernel/security/ima/violations, on the following (bad) run no new
violation is added (obviously).

> thanks,

> Mimi
Mimi Zohar Dec. 15, 2022, 11:29 p.m. UTC | #12
On Thu, 2022-12-15 at 19:39 +0100, Petr Vorel wrote:
> Hi Mimi,
> 
> I'm sorry, it took me long time to look into the issue.
> 
> > Only the ima_conditionals.sh and ima_policy.sh tests define policy
> > rules based on fsuuid.  The other tests are still based on the builtin
> > "ima_policy=tcb" rules.
> Yes.

< trimmed >

> 4) running ima_violations.sh after ima_policy.sh no longer works, because
> there is nothing new in /var/log/audit/audit.log. I don't know why, but
> ima_violations.sh requires either the default ima_policy=tcb policy or policy
> created by ima_policy.sh *without* fsuuid.

Violations occur when a file in policy is already opened for read and
is being opened for write, or the reverse.  After the builtin policy is
replaced with the custom policy based on the UUID, running the
violation test fails because the UUID is reset by the call to
ima_setup().  So the file being opened doesn't match any policy rule.

> FYI below is content of /var/log/audit/audit.log.
> 
> Also looking at things twice, fsuuid does not help testing much.
> Because main blocker for testing is not the scope of the policy, but write once
> policy - CONFIG_IMA_WRITE_POLICY not being set on distro kernels thus repeated
> write of the policy will need reboot.

Oh, I didn't realize this.  Fedora (and RHEL) enable
CONFIG_IMA_WRITE_POLICY.

> Rebooting actually might be possible sooner or later with new runltp-ng from
> Andrea [1] (the feature is not here yet, but will be sooner or later). runltp-ng
> is close to upstream, there was first attempt [2].

Let's try to avoid this solution as much as possible.

> Other option would be to prepare policy which would be suitable for all tests,
> with help of fsuuid.

Ok, I'll look into this.

> But that has drawback:
> Currently we use LTP API to mount directories on loop device after test has started.
> These devices are temporary, e.g.
> /tmp/LTP_ima_violations.pEvyfJO7Af/mntpoint/test.txt will be unmounted and
> deleted after each test run. But for fsuuid we'd need to first permanently
> mount the devices to get their UUID. Therefore there would have to be some
> special setup script needed to be run for all tests. This has proven to be
> problematic in the past. I'd have to extend the API to create something permanent.

Instead of ima_setup() setting the UUID to a new different value, if
additional rules cannot be written (require_policy_writable) the UUID
could be set to the existing policy rules UUID.

Thanks,

Mimi
Petr Vorel Dec. 16, 2022, 8:08 a.m. UTC | #13
> On Thu, 2022-12-15 at 19:39 +0100, Petr Vorel wrote:
> > Hi Mimi,

> > I'm sorry, it took me long time to look into the issue.

> > > Only the ima_conditionals.sh and ima_policy.sh tests define policy
> > > rules based on fsuuid.  The other tests are still based on the builtin
> > > "ima_policy=tcb" rules.
> > Yes.

> < trimmed >

> > 4) running ima_violations.sh after ima_policy.sh no longer works, because
> > there is nothing new in /var/log/audit/audit.log. I don't know why, but
> > ima_violations.sh requires either the default ima_policy=tcb policy or policy
> > created by ima_policy.sh *without* fsuuid.

> Violations occur when a file in policy is already opened for read and
> is being opened for write, or the reverse.  After the builtin policy is
> replaced with the custom policy based on the UUID, running the
> violation test fails because the UUID is reset by the call to
> ima_setup().  So the file being opened doesn't match any policy rule.
Yes, I was just surprised that test ima_violations.sh really depends on
ima_policy=tcb or policy created by ima_policy.sh test.

This also means if somebody wants to test just ima_violations.sh,
he would need to have ima_policy=tcb. Dependency between IMA tests
is something LTP does not expect nor want.

Unfortunately due failures described above it'd be not good to accept this
patch.  But it's a very nice proof of concept how to use fsuuid, thank you.

> > FYI below is content of /var/log/audit/audit.log.

> > Also looking at things twice, fsuuid does not help testing much.
> > Because main blocker for testing is not the scope of the policy, but write once
> > policy - CONFIG_IMA_WRITE_POLICY not being set on distro kernels thus repeated
> > write of the policy will need reboot.

> Oh, I didn't realize this.  Fedora (and RHEL) enable
> CONFIG_IMA_WRITE_POLICY.
Yes, but this config is 'n' by default. I wonder: shouldn't it be 'y' by
default, so that people who does not care get it enabled? Or is it a security
concern to get it enabled during just on boot by systemd?

Even if it were 'y' by default, there still might be some distros/people who
disable it... This is the source of all complications.

> > Rebooting actually might be possible sooner or later with new runltp-ng from
> > Andrea [1] (the feature is not here yet, but will be sooner or later). runltp-ng
> > is close to upstream, there was first attempt [2].

> Let's try to avoid this solution as much as possible.
Sure, reboot is complication.

But you need to accept exact order of the tests. Or have detection,
but the detection does not work on disabled CONFIG_IMA_READ_POLICY
(which is the default when CONFIG_IMA_WRITE_POLICY is not set - again people can
have it unset). Actually ima_kexec.sh has broken detection - this gives TWARN
and TBROK (= failure) on disabled CONFIG_IMA_READ_POLICY:

		if [ "$policy_readable" != 1 ]; then
			tst_res TWARN "policy not readable, it might not contain required policy '$REQUIRED_POLICY'"
			res=TBROK
		fi
		tst_brk $res "unable to find a correct measurement"

Exact order is first test anything with ima_policy=tcb, then have custom policy.
Also, one should run tests with no IMA setup, then with ima_policy=tcb. Then
there are other unsupported/untested policies: ima_policy=secure_boot and
ima_policy=appraise_tcb, supporting them would obviously require reboot.

> > Other option would be to prepare policy which would be suitable for all tests,
> > with help of fsuuid.

> Ok, I'll look into this.
Thank you, very much appreciated!

> > But that has drawback:
> > Currently we use LTP API to mount directories on loop device after test has started.
> > These devices are temporary, e.g.
> > /tmp/LTP_ima_violations.pEvyfJO7Af/mntpoint/test.txt will be unmounted and
> > deleted after each test run. But for fsuuid we'd need to first permanently
> > mount the devices to get their UUID. Therefore there would have to be some
> > special setup script needed to be run for all tests. This has proven to be
> > problematic in the past. I'd have to extend the API to create something permanent.

> Instead of ima_setup() setting the UUID to a new different value, if
> additional rules cannot be written (require_policy_writable) the UUID
> could be set to the existing policy rules UUID.
Not sure if I understand you. FYI no mounted directory exists before test run,
nothing survives after test finishes.
Test is supposed to write *only* to it's temporary directory $TST_TMPDIR, which
is created by TST_NEEDS_TMPDIR=1 after test has started, this is the directory
which gets deleted.

Kind regards,
Petr
diff mbox series

Patch

diff --git a/testcases/kernel/security/integrity/ima/tests/ima_conditionals.sh b/testcases/kernel/security/integrity/ima/tests/ima_conditionals.sh
index 0d50db906..d5c5f3ebe 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_conditionals.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_conditionals.sh
@@ -28,7 +28,7 @@  verify_measurement()
 	ROD rm -f $test_file
 
 	tst_res TINFO "verify measuring user files when requested via $request"
-	ROD echo "measure $request=$value" \> $IMA_POLICY
+	ROD echo "measure $FSUUID $request=$value" \> $IMA_POLICY
 	ROD echo "$(cat /proc/uptime) $request test" \> $test_file
 
 	case "$request" in
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
index af1fb0028..95e7331a4 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
@@ -27,7 +27,12 @@  load_policy()
 	exec 2>/dev/null 4>$IMA_POLICY
 	[ $? -eq 0 ] || exit 1
 
-	cat $1 >&4 2> /dev/null
+	if [ -n "$FSUUID" ]; then
+		sed "s/measure /measure $FSUUID /" $1 >&4 2> /dev/null
+	else
+		cat $1 >&4 2> /dev/null
+	fi
+
 	ret=$?
 	exec 4>&-
 
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
index df3fc5603..016a68cb2 100644
--- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
@@ -178,6 +178,10 @@  ima_setup()
 	if [ "$TST_MOUNT_DEVICE" = 1 ]; then
 		tst_res TINFO "\$TMPDIR is on tmpfs => run on loop device"
 		cd "$TST_MNTPOINT"
+
+		loopdev=$(mount | grep $TST_MNTPOINT | cut -f1 -d' ')
+		FSUUID="fsuuid=$(blkid | grep $loopdev | cut -f2 -d'"')"
+		tst_res TINFO "LTP IMA policy rules based on $FSUUID"
 	fi
 
 	[ -n "$TST_SETUP_CALLER" ] && $TST_SETUP_CALLER