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)?
> > 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.
Maybe I should do this instead: consider again related actions and
combinations of actions that include appraise.
> +
> > +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.
Actually, this was the situation for the MMAP_CHECK and
MMAP_CHECK_REQPROT hooks test.
Roberto
> > +desc="different actions, same func"
> > +rule="appraise func=FILE_CHECK"
> > +ima_policy="dont_measure func=FILE_CHECK"
> > +expect_pass check_result "$desc" "$rule" "$ima_policy" 2
>
> Similarly, an "appraise" rule should not be impacted by an existing
> "dont_measure" rule.
>
> > +desc="different actions, same func"
> > +rule="measure func=FILE_CHECK"
> > +ima_policy="dont_measure func=FILE_CHECK"
> > +expect_pass check_result "$desc" "$rule" "$ima_policy" 2
>
> Right, measure/dont_measure rules for the same func hook overlap.
>
> > +
> > +desc="different actions, same func, different policy options"
> > +rule="measure func=FILE_CHECK"
> > +ima_policy="dont_measure func=FILE_CHECK permit_directio"
> > +expect_pass check_result "$desc" "$rule" "$ima_policy" 2
>
> Right, any combination of measure rules or measure/dont_measure rules
> for the same func hook should overlap, if one rule is more restrictive
> than the other.
>
> > +desc="different actions, same func, different policy options"
> > +rule="measure func=FILE_CHECK permit_directio"
> > +ima_policy="dont_measure func=FILE_CHECK"
> > +expect_pass check_result "$desc" "$rule" "$ima_policy" 2
> > +
> > +desc="same actions, same func, same mask with different modifier"
> > +rule="measure func=FILE_CHECK mask=MAY_EXEC"
> > +ima_policy="measure func=FILE_CHECK mask=^MAY_EXEC"
> > +expect_pass check_result "$desc" "$rule" "$ima_policy" 2
> > +
> > +desc="same actions, same func, different mask with same modifier"
> > +rule="measure func=FILE_CHECK mask=^MAY_READ"
> > +ima_policy="measure func=FILE_CHECK mask=^MAY_EXEC"
> > +expect_pass check_result "$desc" "$rule" "$ima_policy" 2
>
> Right, these rules are equally restrictive, but would overlap when a
> file is opened RW.
>
