Re: [LTP] [PATCH] security/ima: limit the scope of the LTP policy rules based on the UUID

[Petr Vorel <pvorel@suse.cz>]

> 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


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