Message-ID: <acc5495c0bef99f255c4e16f47c2a23dd450d5ad.camel@linux.ibm.com>
Subject: Re: [PATCH ima-evm-utils v2] Add tests for MMAP_CHECK and
 MMAP_CHECK_REQPROT hooks
From:   Mimi Zohar <zohar@linux.ibm.com>
To:     Roberto Sassu <roberto.sassu@huaweicloud.com>,
        dmitry.kasatkin@gmail.com, jmorris@namei.org, serge@hallyn.com
Cc:     linux-integrity@vger.kernel.org,
        linux-security-module@vger.kernel.org,
        linux-kernel@vger.kernel.org, stefanb@linux.ibm.com,
        viro@zeniv.linux.org.uk, pvorel@suse.cz,
        Roberto Sassu <roberto.sassu@huawei.com>
Date:   Wed, 01 Feb 2023 12:54:26 -0500
In-Reply-To: <4a01d975f20f842284da4f46da4eee8c0091f354.camel@huaweicloud.com>
References: <20230131174245.2343342-1-roberto.sassu@huaweicloud.com>
         <20230131174245.2343342-3-roberto.sassu@huaweicloud.com>
         <c5d2e56c923645ec6e0ac8de15123c123db271ee.camel@linux.ibm.com>
         <4a01d975f20f842284da4f46da4eee8c0091f354.camel@huaweicloud.com>
Content-Type: text/plain; charset="ISO-8859-15"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Wed, 2023-02-01 at 18:29 +0100, Roberto Sassu wrote:
> On Tue, 2023-01-31 at 22:02 -0500, Mimi Zohar wrote:
> > Hi Roberto,
> > 
> > > +check_mmap() {
> > > +	local hook="$1"
> > > +	local arg="$2"
> > > +	local test_file fowner rule result test_file_entry
> > > +
> > > +	echo -e "\nTest: ${FUNCNAME[0]} (hook=\"$hook\", test_mmap arg: \"$arg\")"
> > > +
> > > +	if ! test_file=$(mktemp -p "$PWD"); then
> > > +		echo "${RED}Cannot write $test_file${NORM}"
> > > +		return "$HARDFAIL"
> > > +	fi
> > > +
> > > +	fowner="$MMAP_CHECK_FOWNER"
> > > +	rule="$MEASURE_MMAP_CHECK_RULE"
> > > +
> > > +	if [ "$hook" = "MMAP_CHECK_REQPROT" ]; then
> > > +		fowner="$MMAP_CHECK_REQPROT_FOWNER"
> > > +		rule="$MEASURE_MMAP_CHECK_REQPROT_RULE"
> > > +	fi
> > > +
> > > +	if ! chown "$fowner" "$test_file"; then
> > > +		echo "${RED}Cannot change owner of $test_file${NORM}"
> > > +		return "$HARDFAIL"
> > > +	fi
> > > +
> > > +	check_load_ima_rule "$rule"
> > > +	result=$?
> > > +	if [ $result -ne "$OK" ]; then
> > > +		return $result
> > > +	fi
> > > +
> > > +	test_mmap "$test_file" "$arg"
> > > +
> > > +	if [ "$TFAIL" != "yes" ]; then
> > > +		echo -n "Result (expect found): "
> > > +	else
> > > +		echo -n "Result (expect not found): "
> > > +	fi
> > > +
> > > +	test_file_entry=$(awk '$5 == "'"$test_file"'"' < /sys/kernel/security/ima/ascii_runtime_measurements)
> > > +	if [ -z "$test_file_entry" ]; then
> > > +		echo "not found"
> > > +		return "$FAIL"
> > > +	fi
> > 
> > Using temporary files should prevent having multiple records.  Having a
> > verbose option to show the actual matching measurement list record
> > would be nice.
> > 
> > > +
> > > +	echo "found"
> > > +	return "$OK"
> > > +}
> > > +
> > 
> > 
> > > +
> > > +# Run in the new environment if TST_ENV is set.
> > > +_run_env "$TST_KERNEL" "$PWD/$(basename "$0")" "TST_ENV=$TST_ENV TST_KERNEL=$TST_KERNEL PATH=$PATH LD_LIBRARY_PATH=$LD_LIBRARY_PATH VERBOSE=$VERBOSE"
> > > +
> > > +# Exit from the creator of the new environment.
> > > +_exit_env "$TST_KERNEL"
> > > +
> > > +# Mount filesystems in the new environment.
> > > +_init_env
> > > +
> > > +if [ "$(whoami)" != "root" ]; then
> > > +	echo "${CYAN}This script must be executed as root${NORM}"
> > > +	exit "$SKIP"
> > > +fi
> > > +
> > > +if [ ! -f /sys/kernel/security/ima/policy ]; then
> > > +	echo "${CYAN}IMA policy file not found${NORM}"
> > > +	exit "$SKIP"
> > > +fi
> > > +
> > > +if ! cat /sys/kernel/security/ima/policy &> /dev/null; then
> > > +	echo "${CYAN}IMA policy file is not readable${NORM}"
> > > +	exit "$SKIP"
> > > +fi
> > 
> > An existing policy with an mmap rule would affect this test.  Check to
> > see if one already exists.
> 
> Not sure about this. We are specifying the filesystem UUID and the file
> owner. Should be enough to avoid interferences. Also
> check_load_ima_rule() avoids duplicated rules.

Since the policy rules are walked sequentially, existing mmap rules
without the uuid or owner qualifiers will interfere with the test
policy rules.   To see the problem load a custom policy containing an
mmap rule, without any qualifiers.  Then run the mmap_check test.

> 
> > > +
> > > +if [ -n "$TST_KEY_PATH" ]; then
> > > +	if [ "${TST_KEY_PATH:0:1}" != "/" ]; then
> > > +		echo "${RED}Absolute path required for the signing key${NORM}"
> > > +		exit "$FAIL"
> > > +	fi
> > > 
>