On Thu, 2023-02-16 at 09:39 +0100, Roberto Sassu wrote:
> On Wed, 2023-02-15 at 22:22 -0500, Mimi Zohar wrote:
> > Hi Roberto,
> >
> > > diff --git a/tests/ima_policy_check.awk b/tests/ima_policy_check.awk
> > > new file mode 100755
> > > index 00000000000..73107d01083
> > > --- /dev/null
> > > +++ b/tests/ima_policy_check.awk
> > > @@ -0,0 +1,176 @@
> > > +#! /usr/bin/gawk -f
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +#
> > > +# Copyright (C) 2023 Roberto Sassu <roberto.sassu@xxxxxxxxxx>
> > > +#
> > > +# Check a new rule against the loaded IMA policy.
> > > +#
> > > +# Documentation/ABI/testing/ima_policy (Linux kernel)
> > > +# base: [[func=] [mask=] [fsmagic=] [fsuuid=] [fsname=]
> > > +# [uid=] [euid=] [gid=] [egid=]
> > > +# [fowner=] [fgroup=]]
> > > +# lsm: [[subj_user=] [subj_role=] [subj_type=]
> > > +# [obj_user=] [obj_role=] [obj_type=]]
> > > +# option: [digest_type=] [template=] [permit_directio]
> > > +# [appraise_type=] [appraise_flag=]
> > > +# [appraise_algos=] [keyrings=]
> > > +#
> > > +# Rules don't overlap if there is at least one policy keyword (in base or lsm)
> > > +# providing a different value.
> >
> > The above comment needs to be updated to reflect the overlapping tests.
>
> Not sure what is missing. Maybe: rules don't overlap also when they are
> equivalent (they have the same keys and values)?
The above "overlap" definition doesn't take into account one rule being
more restrictive (having more "keys" than the other.)
>
> > > Currently, the < > operators and the ^ modifier
> > > +# are not supported and overlap is asserted even if intervals are disjoint.
> > > +# Also, despite the MMAP_CHECK and MMAP_CHECK_REQPROT hooks have different
> > > +# names, they are basically the same hook but with different behavior depending
> > > +# on external factors, so also in this case overlap has to be asserted. Finally,
> > > +# the existing aliases PATH_CHECK and FILE_MMAP are converted to the current
> > > +# hook names, respectively FILE_CHECK and MMAP_CHECK.
> > > +#
> > > +# Rule equivalence is determined by checking each key/value pair, regardless of
> > > +# their order. However, the action must always be at the beginning of the rules.
> > > +# Rules with aliases are considered equivalent.
> > > +#
> > > +# Return a bit mask with the following values:
> > > +# - 1: invalid new rule;
> > > +# - 2: overlap of the new rule with an existing rule in the IMA policy;
> > > +# - 4: new rule exists in the IMA policy.
> > >
> > > diff --git a/tests/ima_policy_check.test b/tests/ima_policy_check.test
> > > new file mode 100755
> > > index 00000000000..ba8747a74b1
> > > --- /dev/null
> > > +++ b/tests/ima_policy_check.test
> > > @@ -0,0 +1,225 @@
> > > +#!/bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +#
> > > +# Copyright (C) 2023 Roberto Sassu <roberto.sassu@xxxxxxxxxx>
> > > +#
> > > +# Test for ima_policy_check.awk
> > > +
> > > +trap '_report_exit_and_cleanup' SIGINT SIGTERM EXIT
> > > +
> > > +cd "$(dirname "$0")" || exit 1
> > > +. ./functions.sh
> > > +
> > > +export PATH=$PWD:$PATH
> > > +
> > > +check_result() {
> > > + local result
> > > +
> > > + echo -e "\nTest: $1"
> > > + echo "New rule: $2"
> > > + echo "IMA policy: $3"
> > > +
> > > + echo -n "Result (expect $4): "
> > > +
> > > + echo -e "$2\n$3" | ima_policy_check.awk
> > > + result=$?
> > > +
> > > + if [ "$result" -ne "$4" ]; then
> > > + echo "${RED}$result${NORM}"
> > > + return "$FAIL"
> > > + fi
> > > +
> > > + echo "${GREEN}$result${NORM}"
> > > + return "$OK"
> > > +}
> > > +
> > > +# Basic checks.
> > > +desc="empty IMA policy"
> > > +rule="measure func=FILE_CHECK"
> > > +ima_policy=""
> > > +expect_pass check_result "$desc" "$rule" "$ima_policy" 0
> >
> > Include the comment, before the tests, as to what the expected return
> > values mean:
> > # Return a bit mask with the following values:
> > # - 1: invalid new rule;
> > # - 2: overlap of the new rule with an existing rule in the IMA policy;
> > # - 4: new rule exists in the IMA policy.
>
> Ok.
>
> > > +desc="Empty new rule"
> > > +rule=""
> > > +ima_policy=""
> > > +expect_pass check_result "$desc" "$rule" "$ima_policy" 1
> > > +
> > > +desc="Wrong func"
> >
> > "FILE_CHECK" is actually fine, but the condition keyword "fun" is
> > invalid.
>
> Ok, will fix the description.
>
> > > +rule="measure fun=FILE_CHECK"
> > > +ima_policy=""
> > > +expect_pass check_result "$desc" "$rule" "$ima_policy" 1
> > > +
> > > +desc="Missing action"
> > > +rule="func=FILE_CHECK"
> > > +ima_policy=""
> > > +expect_pass check_result "$desc" "$rule" "$ima_policy" 1
> > > +
> > > +# Non-overlapping rules.
> > > +desc="Non-overlapping by func"
> > > +rule="measure func=FILE_CHECK"
> > > +ima_policy="appraise func=MMAP_CHECK"
> > > +expect_pass check_result "$desc" "$rule" "$ima_policy" 0
> >
> > All of the non-overlapping tests are non-overlapping by action as well.
> > Is this intentional?
>
> Yes. Originally, I was considering only related actions (with/without
> dont_). But then, appraise rules could interfer with the rest too.
Ok. To clarify, an "appraise" rule on an earlier hook (e.g.
file_check), could prevent a "measure" policy rule on a later hook
(e.g. bprm_check, mmap_check).
>
> Maybe I should do this instead: consider again related actions and
> combinations of actions that include appraise.
Agreed
>
> > +
> > > +desc="Non-overlapping by uid, func is equal"
> > > +rule="measure func=FILE_CHECK uid=0"
> > > +ima_policy="appraise uid=1 func=FILE_CHECK"
> > > +expect_pass check_result "$desc" "$rule" "$ima_policy" 0
> > > +desc="Non-overlapping by uid, func is equal, same policy options"
> > > +rule="measure func=FILE_CHECK uid=0 permit_directio"
> > > +ima_policy="appraise uid=1 func=FILE_CHECK permit_directio"
> > > +expect_pass check_result "$desc" "$rule" "$ima_policy" 0
> > > +
> > > +desc="Non-overlapping by mask, func and uid are equal, same policy options"
> > > +rule="measure func=FILE_CHECK uid=0 permit_directio mask=MAY_READ"
> > > +ima_policy="appraise uid=0 mask=MAY_EXEC func=FILE_CHECK permit_directio"
> > > +expect_pass check_result "$desc" "$rule" "$ima_policy" 0
> > > +
> > > +desc="Non-overlapping by mask, func and uid are equal, different policy options"
> > > +rule="measure func=FILE_CHECK uid=0 permit_directio mask=MAY_READ"
> > > +ima_policy="appraise uid=0 mask=MAY_EXEC func=FILE_CHECK"
> > > +expect_pass check_result "$desc" "$rule" "$ima_policy" 0
> > > +
> > > +# Overlapping and different rules.
> > > +desc="same actions, different keywords"
> > > +rule="appraise func=FILE_CHECK"
> > > +ima_policy="appraise uid=0"
> > > +expect_pass check_result "$desc" "$rule" "$ima_policy" 2
> > > +
> > > +desc="different actions, same func"
> > > +rule="appraise func=FILE_CHECK"
> > > +ima_policy="measure func=FILE_CHECK"
> > > +expect_pass check_result "$desc" "$rule" "$ima_policy" 2
> >
> > Ok, a "measure" rule overlapping with an existing "appraise" rule could
> > impact a test, but the reverse an "appraise" rule overlapping with an
> > existing "measure" rule should not impact tests. So overlapping rules
> > are not necessarily interferring.
>
> Uhm, probably it does, when you reexecute the tests again and the
> appraise rule is already added. ima_match_policy() parses the policy
> until actmask is cleared.
Ok
> Actually, this was the situation for the MMAP_CHECK and
> MMAP_CHECK_REQPROT hooks test.
--
thanks,
Mimi
