Re: [PATCH v2 6/8] IMA: Add example policy for ima_violations.sh

[Petr Vorel <pvorel@suse.cz>]

> On Tue, 2024-12-31 at 13:23 +0100, Petr Vorel wrote:
> > Hi Mimi,

> > > Hi Petr,

> > > On Fri, 2024-12-13 at 23:20 +0100, Petr Vorel wrote:
> > > > Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
> > > > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > > > ---
> > > >  .../integrity/ima/datafiles/ima_violations/violations.policy     | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >  create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_violations/violations.policy

> > > > diff --git a/testcases/kernel/security/integrity/ima/datafiles/ima_violations/violations.policy b/testcases/kernel/security/integrity/ima/datafiles/ima_violations/violations.policy
> > > > new file mode 100644
> > > > index 0000000000..5734c7617f
> > > > --- /dev/null
> > > > +++ b/testcases/kernel/security/integrity/ima/datafiles/ima_violations/violations.policy
> > > > @@ -0,0 +1 @@
> > > > +func=FILE_CHECK

> > > "[PATCH v2 1/8] IMA: Add TCB policy as an example for ima_measurements.sh"
> > > contains two rules to measure files opened by root on file open.

> > > measure func=FILE_CHECK mask=^MAY_READ euid=0
> > > measure func=FILE_CHECK mask=^MAY_READ uid=0

> > > If the 'tcb' or equivalent policy is loaded, there is no need to load another
> > > policy rule. 

> > I guess I'll move check for builtin policy loaded via kernel command line
> > parameter also to ima_setup.sh to avoid loading example policy when there is a
> > required builtin policy loaded.


> Between the builtin and arch specific policies, most of the rules are already
> defined.  The exception is measuring the boot command line.  Perhaps we should
> update the arch specific policy to include it with the other kexec rules.

> The arch specific policy may include the rule that requires the IMA policy to be
> signed.

> > I also wonder what is a common approach - don't
> > try to load custom example policy when there is builtin policy loaded?

> How about first checking if the rule exists when there is a builtin or
> equivalent custom policy loaded, before loading the example test policy?


> > My goal was to allow more broad IMA testing based on different setup:

> Very good.

> > * running tests with ima_policy=tcb builtin policy (current approach). Many
> > tests will be skipped due missing required policy content.

> Ok.  Remember even with "ima_policy=tcb" specified on the boot command line, the
> results will differ depending on whether the arch specific policy is loaded.

> > * running tests without any builtin policy + load a custom policy + reboot via
> > LTP_IMA_LOAD_POLICY=1 (this patchset), but this should be probably be done only
> > if required (or even none) builtin policy is loaded.

> Good.  The first patch introduces the equivalent custom policy to
> "ima_policy=tcb".  By "load a custom policy" are you referring to this policy or
> a specific policy test rule?

I refer to this policy. Maybe better would be "policy content required by the test"
or "test example policy".

My point is to allow testing without forcing ima_policy=tcb setup (some tooling
might not allow easily to add kernel cmdline parameters). Also, mixing test
example policy with ima_policy=tcb may result a different measurements, right?

If the above assumption is correct I would like to have testing *with*
ima_policy=tcb without loading any test example policy and *without*
ima_policy=tcb but loading test example policy via LTP_IMA_LOAD_POLICY=1.

> > * Ideally not require CONFIG_IMA_READ_POLICY=y as some distros does not have it
> > (but then it is hard to detect whether failures are real bugs or just false
> > positives due not having a proper policy). Maybe convert TBROK/TFAIL to TCONF if

I'm sorry, I was wrong here, I meant to ask: convert TFAIL to either TBROK or TCONF,
e.g. my patch [1].

> > policy content is required but cannot be read due CONFIG_IMA_READ_POLICY (and
> > custom policy with proper content was not loaded).

> Probably the latter option of converting from TBROK/TFAIL to TCONF is
> preferable.  Why fail a test without knowing it will fail.

Because on distros without CONFIG_IMA_READ_POLICY=y we never get notified about
the failure (maybe kernel is broken when it fails but nobody notices TCONF).
But although there is a slight difference between TFAIL and TBROK [2], I agree
that TCONF is probably the best (nobody wants to deal with false positives),
which is handled in my patch [1].

But instead of this I'll try for all tests which require to have certain policy
content (currently all but ima_conditionals.sh): if LTP_IMA_LOAD_POLICY=1 set
try to load example policy even policy content cannot be checked (TCONF when
policy fails to be loaded or if LTP_IMA_LOAD_POLICY not set).

Kind regards,
Petr

> > But you may have an idea what is more useful (brings more test coverage).

> There are two main problems:
> - Not being able to read the policy.
> - Only being able to load a signed policy.

> I think between your above ordering and a new test to see if the policy needs to
> be signed, it's the best we can do for now.

> As mentioned in my 2/8 response, a new package containing pre-defined custom
> policies that are signed by the distro would resolve the latter problem.


> Thanks,

> Mimi

[1] https://patchwork.ozlabs.org/project/ltp/patch/20241213222014.1580991-9-pvorel@suse.cz/
[2] https://linux-test-project.readthedocs.io/en/latest/developers/api_c_tests.html#tst-res-flags-constants