All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Vorel <pvorel@suse.cz>
To: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
Cc: zohar@linux.ibm.com, paul@paul-moore.com,
	stephen.smalley.work@gmail.com, tusharsu@linux.microsoft.com,
	ltp@lists.linux.it, linux-integrity@vger.kernel.org,
	selinux@vger.kernel.org
Subject: Re: [PATCH] IMA: Add test for selinux measurement
Date: Tue, 23 Feb 2021 19:00:16 +0100	[thread overview]
Message-ID: <YDVCsNAfn+Ot6QIB@pevik> (raw)
In-Reply-To: <20210222023805.12846-1-nramas@linux.microsoft.com>

Hi Lakshmi,

> +++ b/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh
...
> +validate_policy_capabilities()
> +{
> +	local measured_cap measured_value expected_value
> +	local result=1
> +	local inx=7
> +
> +	# Policy capabilities flags start from "network_peer_controls"
> +	# in the measured SELinux state at offset 7 for 'awk'
> +	while [ $inx -lt 20 ]; do
> +		measured_cap=$(echo $1 | awk -F'[=;]' -v inx="$inx" '{print $inx}')
> +		inx=$(( $inx + 1 ))
> +
> +		measured_value=$(echo $1 | awk -F'[=;]' -v inx="$inx" '{print $inx}')
> +		expected_value=$(cat "$SELINUX_DIR/policy_capabilities/$measured_cap")
> +		if [ "$measured_value" != "$expected_value" ];then
> +			tst_res TWARN "$measured_cap: expected: $expected_value, got: $digest"
We rarely use TWARN in the tests, only when the error is not related to the test result.
Otherwise we use TFAIL.

The rest LGTM.
Reviewed-by: Petr Vorel <pvorel@suse.cz>

I did few formatting and style changes:
https://github.com/pevik/ltp/commits/ima/selinux.v2.fixes
(branch ima/selinux.v2.fixes), see diff below.

As we discuss, I'm going tom merge test when patchset is merged in maintainers tree,
please ping me. And ideally we should mention kernel commit hash as a comment in
the test.

Thanks a lot!

Kind regards,
Petr

diff --git testcases/kernel/security/integrity/ima/tests/ima_selinux.sh testcases/kernel/security/integrity/ima/tests/ima_selinux.sh
index e5060a5e3..ed758631b 100755
--- testcases/kernel/security/integrity/ima/tests/ima_selinux.sh
+++ testcases/kernel/security/integrity/ima/tests/ima_selinux.sh
@@ -13,16 +13,14 @@ TST_SETUP="setup"
 . ima_setup.sh
 
 FUNC_CRITICAL_DATA='func=CRITICAL_DATA'
-REQUIRED_POLICY="^measure.*($FUNC_CRITICAL_DATA)"
+REQUIRED_POLICY="^measure.*$FUNC_CRITICAL_DATA"
 
 setup()
 {
-	SELINUX_DIR=$(tst_get_selinux_dir)
-	if [ -z "$SELINUX_DIR" ]; then
-		tst_brk TCONF "SELinux is not enabled"
-		return
-	fi
+	tst_require_selinux_enabled
 	require_ima_policy_content "$REQUIRED_POLICY" '-E' > $TST_TMPDIR/policy.txt
+
+	SELINUX_DIR=$(tst_get_selinux_dir)
 }
 
 # Format of the measured SELinux state data.
@@ -41,16 +39,16 @@ validate_policy_capabilities()
 	# in the measured SELinux state at offset 7 for 'awk'
 	while [ $inx -lt 20 ]; do
 		measured_cap=$(echo $1 | awk -F'[=;]' -v inx="$inx" '{print $inx}')
-		inx=$(( $inx + 1 ))
+		inx=$(($inx + 1))
 
 		measured_value=$(echo $1 | awk -F'[=;]' -v inx="$inx" '{print $inx}')
 		expected_value=$(cat "$SELINUX_DIR/policy_capabilities/$measured_cap")
-		if [ "$measured_value" != "$expected_value" ];then
+		if [ "$measured_value" != "$expected_value" ]; then
 			tst_res TWARN "$measured_cap: expected: $expected_value, got: $digest"
 			result=0
 		fi
 
-		inx=$(( $inx + 1 ))
+		inx=$(($inx + 1))
 	done
 
 	return $result
@@ -109,7 +107,6 @@ test2()
 	local initialized_value
 	local enforced_value expected_enforced_value
 	local checkreqprot_value expected_checkreqprot_value
-	local result
 
 	tst_res TINFO "verifying SELinux state measurement"
 
@@ -149,27 +146,25 @@ test2()
 	measured_data=$(cat $state_file)
 	enforced_value=$(echo $measured_data | awk -F'[=;]' '{print $4}')
 	expected_enforced_value=$(cat $SELINUX_DIR/enforce)
-	if [ "$expected_enforced_value" != "$enforced_value" ];then
+	if [ "$expected_enforced_value" != "$enforced_value" ]; then
 		tst_res TFAIL "enforce: expected: $expected_enforced_value, got: $enforced_value"
 		return
 	fi
 
 	checkreqprot_value=$(echo $measured_data | awk -F'[=;]' '{print $6}')
 	expected_checkreqprot_value=$(cat $SELINUX_DIR/checkreqprot)
-	if [ "$expected_checkreqprot_value" != "$checkreqprot_value" ];then
+	if [ "$expected_checkreqprot_value" != "$checkreqprot_value" ]; then
 		tst_res TFAIL "checkreqprot: expected: $expected_checkreqprot_value, got: $checkreqprot_value"
 		return
 	fi
 
 	initialized_value=$(echo $measured_data | awk -F'[=;]' '{print $2}')
-	if [ "$initialized_value" != "1" ];then
+	if [ "$initialized_value" != "1" ]; then
 		tst_res TFAIL "initialized: expected 1, got: $initialized_value"
 		return
 	fi
 
-	validate_policy_capabilities $measured_data
-	result=$?
-	if [ $result = 0 ]; then
+	if validate_policy_capabilities $measured_data; then
 		tst_res TFAIL "policy capabilities did not match"
 		return
 	fi

WARNING: multiple messages have this Message-ID (diff)
From: Petr Vorel <pvorel@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH] IMA: Add test for selinux measurement
Date: Tue, 23 Feb 2021 19:00:16 +0100	[thread overview]
Message-ID: <YDVCsNAfn+Ot6QIB@pevik> (raw)
In-Reply-To: <20210222023805.12846-1-nramas@linux.microsoft.com>

Hi Lakshmi,

> +++ b/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh
...
> +validate_policy_capabilities()
> +{
> +	local measured_cap measured_value expected_value
> +	local result=1
> +	local inx=7
> +
> +	# Policy capabilities flags start from "network_peer_controls"
> +	# in the measured SELinux state at offset 7 for 'awk'
> +	while [ $inx -lt 20 ]; do
> +		measured_cap=$(echo $1 | awk -F'[=;]' -v inx="$inx" '{print $inx}')
> +		inx=$(( $inx + 1 ))
> +
> +		measured_value=$(echo $1 | awk -F'[=;]' -v inx="$inx" '{print $inx}')
> +		expected_value=$(cat "$SELINUX_DIR/policy_capabilities/$measured_cap")
> +		if [ "$measured_value" != "$expected_value" ];then
> +			tst_res TWARN "$measured_cap: expected: $expected_value, got: $digest"
We rarely use TWARN in the tests, only when the error is not related to the test result.
Otherwise we use TFAIL.

The rest LGTM.
Reviewed-by: Petr Vorel <pvorel@suse.cz>

I did few formatting and style changes:
https://github.com/pevik/ltp/commits/ima/selinux.v2.fixes
(branch ima/selinux.v2.fixes), see diff below.

As we discuss, I'm going tom merge test when patchset is merged in maintainers tree,
please ping me. And ideally we should mention kernel commit hash as a comment in
the test.

Thanks a lot!

Kind regards,
Petr

diff --git testcases/kernel/security/integrity/ima/tests/ima_selinux.sh testcases/kernel/security/integrity/ima/tests/ima_selinux.sh
index e5060a5e3..ed758631b 100755
--- testcases/kernel/security/integrity/ima/tests/ima_selinux.sh
+++ testcases/kernel/security/integrity/ima/tests/ima_selinux.sh
@@ -13,16 +13,14 @@ TST_SETUP="setup"
 . ima_setup.sh
 
 FUNC_CRITICAL_DATA='func=CRITICAL_DATA'
-REQUIRED_POLICY="^measure.*($FUNC_CRITICAL_DATA)"
+REQUIRED_POLICY="^measure.*$FUNC_CRITICAL_DATA"
 
 setup()
 {
-	SELINUX_DIR=$(tst_get_selinux_dir)
-	if [ -z "$SELINUX_DIR" ]; then
-		tst_brk TCONF "SELinux is not enabled"
-		return
-	fi
+	tst_require_selinux_enabled
 	require_ima_policy_content "$REQUIRED_POLICY" '-E' > $TST_TMPDIR/policy.txt
+
+	SELINUX_DIR=$(tst_get_selinux_dir)
 }
 
 # Format of the measured SELinux state data.
@@ -41,16 +39,16 @@ validate_policy_capabilities()
 	# in the measured SELinux state at offset 7 for 'awk'
 	while [ $inx -lt 20 ]; do
 		measured_cap=$(echo $1 | awk -F'[=;]' -v inx="$inx" '{print $inx}')
-		inx=$(( $inx + 1 ))
+		inx=$(($inx + 1))
 
 		measured_value=$(echo $1 | awk -F'[=;]' -v inx="$inx" '{print $inx}')
 		expected_value=$(cat "$SELINUX_DIR/policy_capabilities/$measured_cap")
-		if [ "$measured_value" != "$expected_value" ];then
+		if [ "$measured_value" != "$expected_value" ]; then
 			tst_res TWARN "$measured_cap: expected: $expected_value, got: $digest"
 			result=0
 		fi
 
-		inx=$(( $inx + 1 ))
+		inx=$(($inx + 1))
 	done
 
 	return $result
@@ -109,7 +107,6 @@ test2()
 	local initialized_value
 	local enforced_value expected_enforced_value
 	local checkreqprot_value expected_checkreqprot_value
-	local result
 
 	tst_res TINFO "verifying SELinux state measurement"
 
@@ -149,27 +146,25 @@ test2()
 	measured_data=$(cat $state_file)
 	enforced_value=$(echo $measured_data | awk -F'[=;]' '{print $4}')
 	expected_enforced_value=$(cat $SELINUX_DIR/enforce)
-	if [ "$expected_enforced_value" != "$enforced_value" ];then
+	if [ "$expected_enforced_value" != "$enforced_value" ]; then
 		tst_res TFAIL "enforce: expected: $expected_enforced_value, got: $enforced_value"
 		return
 	fi
 
 	checkreqprot_value=$(echo $measured_data | awk -F'[=;]' '{print $6}')
 	expected_checkreqprot_value=$(cat $SELINUX_DIR/checkreqprot)
-	if [ "$expected_checkreqprot_value" != "$checkreqprot_value" ];then
+	if [ "$expected_checkreqprot_value" != "$checkreqprot_value" ]; then
 		tst_res TFAIL "checkreqprot: expected: $expected_checkreqprot_value, got: $checkreqprot_value"
 		return
 	fi
 
 	initialized_value=$(echo $measured_data | awk -F'[=;]' '{print $2}')
-	if [ "$initialized_value" != "1" ];then
+	if [ "$initialized_value" != "1" ]; then
 		tst_res TFAIL "initialized: expected 1, got: $initialized_value"
 		return
 	fi
 
-	validate_policy_capabilities $measured_data
-	result=$?
-	if [ $result = 0 ]; then
+	if validate_policy_capabilities $measured_data; then
 		tst_res TFAIL "policy capabilities did not match"
 		return
 	fi

  reply	other threads:[~2021-02-23 18:01 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-22  2:38 [PATCH] IMA: Add test for selinux measurement Lakshmi Ramasubramanian
2021-02-22  2:38 ` [LTP] " Lakshmi Ramasubramanian
2021-02-23 18:00 ` Petr Vorel [this message]
2021-02-23 18:00   ` Petr Vorel
2021-02-23 18:26   ` Lakshmi Ramasubramanian
2021-02-23 18:26     ` [LTP] " Lakshmi Ramasubramanian
2021-02-23 22:14     ` Petr Vorel
2021-02-23 22:14       ` [LTP] " Petr Vorel
2021-02-23 23:01       ` Lakshmi Ramasubramanian
2021-02-23 23:01         ` [LTP] " Lakshmi Ramasubramanian
2021-03-16 11:54 ` Petr Vorel
2021-03-16 11:54   ` [LTP] " Petr Vorel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YDVCsNAfn+Ot6QIB@pevik \
    --to=pvorel@suse.cz \
    --cc=linux-integrity@vger.kernel.org \
    --cc=ltp@lists.linux.it \
    --cc=nramas@linux.microsoft.com \
    --cc=paul@paul-moore.com \
    --cc=selinux@vger.kernel.org \
    --cc=stephen.smalley.work@gmail.com \
    --cc=tusharsu@linux.microsoft.com \
    --cc=zohar@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.