linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH ima-evm-utils 1/2] Add --hmackey option for evmctl
@ 2023-03-24 18:11 Roberto Sassu
  2023-03-24 18:11 ` [PATCH ima-evm-utils 2/2] Add simple test to check EVM HMAC calculation Roberto Sassu
  2023-04-24 17:44 ` [PATCH ima-evm-utils 1/2] Add --hmackey option for evmctl Mimi Zohar
  0 siblings, 2 replies; 5+ messages in thread
From: Roberto Sassu @ 2023-03-24 18:11 UTC (permalink / raw)
  To: zohar, dmitry.kasatkin
  Cc: linux-integrity, vt, pvorel, stefanb, Roberto Sassu

From: Roberto Sassu <roberto.sassu@huawei.com>

Add the --hmackey option, to specify an alternative path for the file
containing the HMAC key. By default evmctl looks in
/etc/keys/evm-key-plain.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 src/evmctl.c | 7 ++++++-
 src/imaevm.h | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index 304b70de40f..df38e763041 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -1413,7 +1413,8 @@ static int cmd_hmac_evm(struct command *cmd)
 			return err;
 	}
 
-	return hmac_evm(file, "/etc/keys/evm-key-plain");
+	return hmac_evm(file, imaevm_params.hmackeyfile ? :
+			"/etc/keys/evm-key-plain");
 }
 
 static int ima_fix(const char *path)
@@ -2940,6 +2941,7 @@ static struct option opts[] = {
 	{"keyid-from-cert", 1, 0, 145},
 	{"veritysig", 0, 0, 146},
 	{"hwtpm", 0, 0, 147},
+	{"hmackey", 1, 0, 148},
 	{}
 
 };
@@ -3185,6 +3187,9 @@ int main(int argc, char *argv[])
 		case 147:
 			hwtpm = 1;
 			break;
+		case 148:
+			imaevm_params.hmackeyfile = optarg;
+			break;
 		case '?':
 			exit(1);
 			break;
diff --git a/src/imaevm.h b/src/imaevm.h
index 78e7ed5e89d..18d7b0e447e 100644
--- a/src/imaevm.h
+++ b/src/imaevm.h
@@ -221,6 +221,7 @@ struct libimaevm_params {
 	const char *keypass;
 	uint32_t keyid;		/* keyid overriding value, unless 0. (Host order.) */
 	ENGINE *eng;
+	const char *hmackeyfile;
 };
 
 struct RSA_ASN1_template {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH ima-evm-utils 2/2] Add simple test to check EVM HMAC calculation
  2023-03-24 18:11 [PATCH ima-evm-utils 1/2] Add --hmackey option for evmctl Roberto Sassu
@ 2023-03-24 18:11 ` Roberto Sassu
  2023-04-24 12:22   ` Mimi Zohar
  2023-04-24 17:44 ` [PATCH ima-evm-utils 1/2] Add --hmackey option for evmctl Mimi Zohar
  1 sibling, 1 reply; 5+ messages in thread
From: Roberto Sassu @ 2023-03-24 18:11 UTC (permalink / raw)
  To: zohar, dmitry.kasatkin
  Cc: linux-integrity, vt, pvorel, stefanb, Roberto Sassu

From: Roberto Sassu <roberto.sassu@huawei.com>

Add a simple test to ensure that the kernel and evmctl provide the same
result for the HMAC calculation.

This test requires that, unless the UML kernel is used, the
TST_EVM_CHANGE_MODE environment variable is set to 1.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 kernel-configs/base |   4 +-
 tests/Makefile.am   |   2 +-
 tests/evm_hmac.test | 170 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 174 insertions(+), 2 deletions(-)
 create mode 100755 tests/evm_hmac.test

diff --git a/kernel-configs/base b/kernel-configs/base
index 7acbd5b3b2a..0d8714d8f98 100644
--- a/kernel-configs/base
+++ b/kernel-configs/base
@@ -46,11 +46,13 @@ CONFIG_TMPFS_XATTR=y
 CONFIG_CONFIGFS_FS=y
 CONFIG_KEYS=y
 CONFIG_ENCRYPTED_KEYS=y
+CONFIG_USER_DECRYPTED_DATA=y
 CONFIG_SECURITY=y
 CONFIG_SECURITYFS=y
 CONFIG_SECURITY_NETWORK=y
 CONFIG_SECURITY_PATH=y
-CONFIG_LSM="lockdown,yama,loadpin,safesetid,integrity,bpf"
+CONFIG_SECURITY_SMACK=y
+CONFIG_LSM="lockdown,yama,loadpin,safesetid,smack,integrity,bpf"
 CONFIG_CRYPTO_AEAD2=y
 CONFIG_CRYPTO_SKCIPHER=y
 CONFIG_CRYPTO_SKCIPHER2=y
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 03aa5b76088..a28f671398f 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -3,7 +3,7 @@ TESTS = $(check_SCRIPTS)
 
 check_SCRIPTS += ima_hash.test sign_verify.test boot_aggregate.test \
 		 fsverity.test portable_signatures.test ima_policy_check.test \
-		 mmap_check.test
+		 mmap_check.test evm_hmac.test
 
 check_PROGRAMS := test_mmap
 
diff --git a/tests/evm_hmac.test b/tests/evm_hmac.test
new file mode 100755
index 00000000000..de8b6a9a4d9
--- /dev/null
+++ b/tests/evm_hmac.test
@@ -0,0 +1,170 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (C) 2023 Roberto Sassu <roberto.sassu@huawei.com>
+#
+# Check if the kernel and evmctl provide the same result for HMAC calculation.
+
+trap '_report_exit_and_cleanup _cleanup_env cleanup' SIGINT SIGTERM SIGSEGV EXIT
+
+# Base VERBOSE on the environment variable, if set.
+VERBOSE="${VERBOSE:-0}"
+TST_EVM_CHANGE_MODE="${TST_EVM_CHANGE_MODE:-0}"
+
+# From security/integrity/evm/evm.h in kernel source directory
+(( EVM_INIT_HMAC=0x0001 ))
+
+cd "$(dirname "$0")" || exit 1
+export PATH=$PWD/../src:$PATH
+export LD_LIBRARY_PATH=$LD_LIBRARY_PATH
+. ./functions.sh
+_require evmctl
+
+cleanup() {
+	if [ "$g_loop_mounted" = "1" ]; then
+		popd > /dev/null || exit "$FAIL"
+		umount "$g_mountpoint"
+	fi
+
+	if [ -n "$g_dev" ]; then
+		losetup -d "$g_dev"
+	fi
+
+	if [ -n "$g_image" ]; then
+		rm -f "$g_image"
+	fi
+
+	if [ -n "$g_mountpoint" ]; then
+		rm -Rf "$g_mountpoint"
+	fi
+}
+
+get_xattr() {
+	local format="hex"
+
+	if [ "$1" = "security.selinux" ]; then
+		format="text"
+	fi
+
+	getfattr -n "$1" -e "$format" -d "$2" 2> /dev/null | awk -F "=" '$1 == "'"$1"'" {if ("'"$format"'" == "hex") v=substr($2, 3); else { split($2, temp, "\""); v=temp[2] }; print v}'
+}
+
+IMA_UUID="28b23254-9467-44c0-b6ba-34b12e85a26f"
+
+# The purpose of this test is to verify if the kernel and evmctl produce the
+# same HMAC.
+check_evm_hmac() {
+	local evm_xattr evm_xattr_evmctl test_file_digest
+
+	echo "Test: ${FUNCNAME[0]} (evm_hash: $1, evm_value: $g_evm_value, algo: $1, fs: $2)"
+
+	if ! touch test-file; then
+		echo "${RED}Cannot create test-file${NORM}"
+		return "$FAIL"
+	fi
+
+	# Compare HMAC calculated by the kernel with that calculated by evmctl.
+	evm_xattr="$(get_xattr security.evm test-file)"
+	test_file_digest=$("$1"sum test-file | awk '{print $1}')
+	# evm_xattr has an extra byte at the beginning for the xattr type.
+	if [ "${#evm_xattr}" != $(( ${#test_file_digest} + 2 )) ]; then
+		echo "${RED}Unexpected size of security.evm${NORM}"
+		return "$FAIL"
+	fi
+
+	evm_xattr_evmctl="$(evmctl hmac -v -n test-file --uuid=$IMA_UUID -a "$1" --hmackey "$g_hmackey" 2>&1 | awk -F " " '$1 == "hmac:" {print $2}')"
+	if [ "$evm_xattr" != "02$evm_xattr_evmctl" ]; then
+		echo "${RED}security.evm mismatch between the kernel and evmctl${NORM}"
+		return "$FAIL"
+	fi
+
+	return "$OK"
+}
+
+cleanup_evm_hmac() {
+	rm -f test-file
+}
+
+_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
+
+# Assume that the EVM mode can be changed in a new environment.
+if [ -z "$TST_ENV" ] && [ "$TST_EVM_CHANGE_MODE" -eq 0 ]; then
+	echo "${CYAN}TST_EVM_CHANGE_MODE env variable must be set to 1${NORM}"
+	exit "$SKIP"
+fi
+
+g_lsm_init_xattr=$(awk '$1 ~ /(smack|selinux)/' < /sys/kernel/security/lsm)
+if [ -z "$g_lsm_init_xattr" ]; then
+	echo "${CYAN}Either SMACK or SELinux must be active in the system${NORM}"
+	exit "$SKIP"
+fi
+
+g_mountpoint="$(mktemp -d)"
+g_image="$(mktemp)"
+
+if [ -z "$g_mountpoint" ]; then
+	echo "${RED}Mountpoint directory not created${NORM}"
+	exit "$FAIL"
+fi
+
+if [ "$(whoami)" != "root" ]; then
+	echo "${CYAN}This script must be executed as root${NORM}"
+	exit "$SKIP"
+fi
+
+if ! dd if=/dev/zero of="$g_image" bs=1M count=10 &> /dev/null; then
+	echo "${RED}Cannot create test image${NORM}"
+	exit "$FAIL"
+fi
+
+g_dev="$(losetup -f "$g_image" --show)"
+if [ -z "$g_dev" ]; then
+	echo "${RED}Cannot create loop device${NORM}"
+	exit "$FAIL"
+fi
+
+if ! mkfs.ext4 -U $IMA_UUID -b 4096 "$g_dev" &> /dev/null; then
+	echo "${RED}Cannot format $g_dev${NORM}"
+	exit "$FAIL"
+fi
+
+if ! mount -o i_version "$g_dev" "$g_mountpoint"; then
+	echo "${RED}Cannot mount loop device${NORM}"
+	exit "$FAIL"
+fi
+
+g_loop_mounted=1
+chmod 777 "$g_mountpoint"
+pushd "$g_mountpoint" > /dev/null || exit "$FAIL"
+
+if [ -f /sys/kernel/security/evm ]; then
+	g_evm_value=$(cat /sys/kernel/security/evm)
+fi
+
+g_hmackey_data="abcdefABCDEF1234567890aaaaaaaaaaabcdefABCDEF1234567890aaaaaaaaaa"
+
+g_hmackey="$(mktemp)"
+echo $g_hmackey_data | xxd -r -p > "$g_hmackey"
+
+if [ -n "$g_evm_value" ] && [ $((g_evm_value & EVM_INIT_HMAC)) -ne $EVM_INIT_HMAC ]; then
+	g_evm_id="$(keyctl add encrypted evm-key "new enc32 user:kmk 32 $g_hmackey_data" @u)"
+	if ! echo "$EVM_INIT_HMAC" | tee /sys/kernel/security/evm &> /dev/null; then
+		# Retry with sudo -i, to force search in the root user keyring.
+		if ! echo "$EVM_INIT_HMAC" | sudo -i tee /sys/kernel/security/evm &> /dev/null; then
+			keyctl unlink "$g_evm_id"
+			echo "${RED}Failed to initialize EVM${NORM}"
+			exit "$FAIL"
+		fi
+	fi
+
+	g_evm_value=$(cat /sys/kernel/security/evm)
+fi
+
+expect_pass check_evm_hmac sha1 ext4
+cleanup_evm_hmac
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH ima-evm-utils 2/2] Add simple test to check EVM HMAC calculation
  2023-03-24 18:11 ` [PATCH ima-evm-utils 2/2] Add simple test to check EVM HMAC calculation Roberto Sassu
@ 2023-04-24 12:22   ` Mimi Zohar
  2023-04-24 17:17     ` Roberto Sassu
  0 siblings, 1 reply; 5+ messages in thread
From: Mimi Zohar @ 2023-04-24 12:22 UTC (permalink / raw)
  To: Roberto Sassu, dmitry.kasatkin
  Cc: linux-integrity, vt, pvorel, stefanb, Roberto Sassu

Hi Roberto,

On Fri, 2023-03-24 at 19:11 +0100, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
> 
> Add a simple test to ensure that the kernel and evmctl provide the same
> result for the HMAC calculation.

Based on the LSM discussions, including a test for a file in a Smack
transmuting directory would be nice.

> This test requires that, unless the UML kernel is used, the
> TST_EVM_CHANGE_MODE environment variable is set to 1.

Agreed requring permission to enable EVM HMAC with a well known HMAC
value is 
a good idea.   Please update the patch description with an explanation.

> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
>  kernel-configs/base |   4 +-
>  tests/Makefile.am   |   2 +-
>  tests/evm_hmac.test | 170 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 174 insertions(+), 2 deletions(-)
>  create mode 100755 tests/evm_hmac.test
> 
> diff --git a/kernel-configs/base b/kernel-configs/base
> index 7acbd5b3b2a..0d8714d8f98 100644
> --- a/kernel-configs/base
> +++ b/kernel-configs/base
> @@ -46,11 +46,13 @@ CONFIG_TMPFS_XATTR=y
>  CONFIG_CONFIGFS_FS=y
>  CONFIG_KEYS=y
>  CONFIG_ENCRYPTED_KEYS=y
> +CONFIG_USER_DECRYPTED_DATA=y
>  CONFIG_SECURITY=y
>  CONFIG_SECURITYFS=y
>  CONFIG_SECURITY_NETWORK=y
>  CONFIG_SECURITY_PATH=y
> -CONFIG_LSM="lockdown,yama,loadpin,safesetid,integrity,bpf"
> +CONFIG_SECURITY_SMACK=y
> +CONFIG_LSM="lockdown,yama,loadpin,safesetid,smack,integrity,bpf"
>  CONFIG_CRYPTO_AEAD2=y
>  CONFIG_CRYPTO_SKCIPHER=y
>  CONFIG_CRYPTO_SKCIPHER2=y
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 03aa5b76088..a28f671398f 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -3,7 +3,7 @@ TESTS = $(check_SCRIPTS)
>  
>  check_SCRIPTS += ima_hash.test sign_verify.test boot_aggregate.test \
>  		 fsverity.test portable_signatures.test ima_policy_check.test \
> -		 mmap_check.test
> +		 mmap_check.test evm_hmac.test
>  
>  check_PROGRAMS := test_mmap
>  
> diff --git a/tests/evm_hmac.test b/tests/evm_hmac.test
> new file mode 100755
> index 00000000000..de8b6a9a4d9
> --- /dev/null
> +++ b/tests/evm_hmac.test
> @@ -0,0 +1,170 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Copyright (C) 2023 Roberto Sassu <roberto.sassu@huawei.com>
> +#
> +# Check if the kernel and evmctl provide the same result for HMAC calculation.
> +
> +trap '_report_exit_and_cleanup _cleanup_env cleanup' SIGINT SIGTERM SIGSEGV EXIT
> +
> +# Base VERBOSE on the environment variable, if set.
> +VERBOSE="${VERBOSE:-0}"
> +TST_EVM_CHANGE_MODE="${TST_EVM_CHANGE_MODE:-0}"
> +
> +# From security/integrity/evm/evm.h in kernel source directory
> +(( EVM_INIT_HMAC=0x0001 ))
> +
> +cd "$(dirname "$0")" || exit 1
> +export PATH=$PWD/../src:$PATH
> +export LD_LIBRARY_PATH=$LD_LIBRARY_PATH
> +. ./functions.sh
> +_require evmctl
> +
> +cleanup() {
> +	if [ "$g_loop_mounted" = "1" ]; then
> +		popd > /dev/null || exit "$FAIL"
> +		umount "$g_mountpoint"
> +	fi
> +
> +	if [ -n "$g_dev" ]; then
> +		losetup -d "$g_dev"
> +	fi
> +
> +	if [ -n "$g_image" ]; then
> +		rm -f "$g_image"
> +	fi
> +
> +	if [ -n "$g_mountpoint" ]; then
> +		rm -Rf "$g_mountpoint"
> +	fi
> +}
> +
> +get_xattr() {
> +	local format="hex"
> +
> +	if [ "$1" = "security.selinux" ]; then
> +		format="text"
> +	fi
> +
> +	getfattr -n "$1" -e "$format" -d "$2" 2> /dev/null | awk -F "=" '$1 == "'"$1"'" {if ("'"$format"'" == "hex") v=substr($2, 3); else { split($2, temp, "\""); v=temp[2] }; print v}'
> +}
> +
> +IMA_UUID="28b23254-9467-44c0-b6ba-34b12e85a26f"
> +
> +# The purpose of this test is to verify if the kernel and evmctl produce the
> +# same HMAC.
> +check_evm_hmac() {
> +	local evm_xattr evm_xattr_evmctl test_file_digest
> +
> +	echo "Test: ${FUNCNAME[0]} (evm_hash: $1, evm_value: $g_evm_value, algo: $1, fs: $2)"
> +
> +	if ! touch test-file; then
> +		echo "${RED}Cannot create test-file${NORM}"
> +		return "$FAIL"
> +	fi
> +
> +	# Compare HMAC calculated by the kernel with that calculated by evmctl.
> +	evm_xattr="$(get_xattr security.evm test-file)"
> +	test_file_digest=$("$1"sum test-file | awk '{print $1}')
> +	# evm_xattr has an extra byte at the beginning for the xattr type.
> +	if [ "${#evm_xattr}" != $(( ${#test_file_digest} + 2 )) ]; then
> +		echo "${RED}Unexpected size of security.evm${NORM}"
> +		return "$FAIL"
> +	fi
> +
> +	evm_xattr_evmctl="$(evmctl hmac -v -n test-file --uuid=$IMA_UUID -a "$1" --hmackey "$g_hmackey" 2>&1 | awk -F " " '$1 == "hmac:" {print $2}')"
> +	if [ "$evm_xattr" != "02$evm_xattr_evmctl" ]; then
> +		echo "${RED}security.evm mismatch between the kernel and evmctl${NORM}"
> +		return "$FAIL"
> +	fi
> +
> +	return "$OK"
> +}
> +
> +cleanup_evm_hmac() {
> +	rm -f test-file
> +}
> +
> +_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
> +
> +# Assume that the EVM mode can be changed in a new environment.
> +if [ -z "$TST_ENV" ] && [ "$TST_EVM_CHANGE_MODE" -eq 0 ]; then
> +	echo "${CYAN}TST_EVM_CHANGE_MODE env variable must be set to 1${NORM}"
> +	exit "$SKIP"
> +fi
> +
> +g_lsm_init_xattr=$(awk '$1 ~ /(smack|selinux)/' < /sys/kernel/security/lsm)
> +if [ -z "$g_lsm_init_xattr" ]; then
> +	echo "${CYAN}Either SMACK or SELinux must be active in the system${NORM}"
> +	exit "$SKIP"
> +fi
> +
> +g_mountpoint="$(mktemp -d)"
> +g_image="$(mktemp)"
> +
> +if [ -z "$g_mountpoint" ]; then
> +	echo "${RED}Mountpoint directory not created${NORM}"
> +	exit "$FAIL"
> +fi
> +
> +if [ "$(whoami)" != "root" ]; then
> +	echo "${CYAN}This script must be executed as root${NORM}"
> +	exit "$SKIP"
> +fi
> +
> +if ! dd if=/dev/zero of="$g_image" bs=1M count=10 &> /dev/null; then
> +	echo "${RED}Cannot create test image${NORM}"
> +	exit "$FAIL"
> +fi
> +
> +g_dev="$(losetup -f "$g_image" --show)"
> +if [ -z "$g_dev" ]; then
> +	echo "${RED}Cannot create loop device${NORM}"
> +	exit "$FAIL"
> +fi
> +
> +if ! mkfs.ext4 -U $IMA_UUID -b 4096 "$g_dev" &> /dev/null; then
> +	echo "${RED}Cannot format $g_dev${NORM}"
> +	exit "$FAIL"
> +fi
> +
> +if ! mount -o i_version "$g_dev" "$g_mountpoint"; then
> +	echo "${RED}Cannot mount loop device${NORM}"
> +	exit "$FAIL"
> +fi
> +
> +g_loop_mounted=1
> +chmod 777 "$g_mountpoint"
> +pushd "$g_mountpoint" > /dev/null || exit "$FAIL"
> +
> +if [ -f /sys/kernel/security/evm ]; then
> +	g_evm_value=$(cat /sys/kernel/security/evm)
> +fi
> +
> +g_hmackey_data="abcdefABCDEF1234567890aaaaaaaaaaabcdefABCDEF1234567890aaaaaaaaaa"
> +
> +g_hmackey="$(mktemp)"
> +echo $g_hmackey_data | xxd -r -p > "$g_hmackey"
> +
> +if [ -n "$g_evm_value" ] && [ $((g_evm_value & EVM_INIT_HMAC)) -ne $EVM_INIT_HMAC ]; then
> +	g_evm_id="$(keyctl add encrypted evm-key "new enc32 user:kmk 32 $g_hmackey_data" @u)"

The above command fails on kernels without encrypted key support for
user provided decrypted data.

Required commits:
5adedd42245a ("KEYS: encrypted: fix key instantiation with user-
provided data")
cd3bc044af48 ("KEYS: encrypted: Instantiate key with user-provided
decrypted data")

> +	if ! echo "$EVM_INIT_HMAC" | tee /sys/kernel/security/evm &> /dev/null; then
> +		# Retry with sudo -i, to force search in the root user keyring.
> +		if ! echo "$EVM_INIT_HMAC" | sudo -i tee /sys/kernel/security/evm &> /dev/null; then
> +			keyctl unlink "$g_evm_id"
> +			echo "${RED}Failed to initialize EVM${NORM}"
> +			exit "$FAIL"
> +		fi
> +	fi
> +
> +	g_evm_value=$(cat /sys/kernel/security/evm)
> +fi
> +
> +expect_pass check_evm_hmac sha1 ext4
> +cleanup_evm_hmac

-- 
thanks,

Mimi



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH ima-evm-utils 2/2] Add simple test to check EVM HMAC calculation
  2023-04-24 12:22   ` Mimi Zohar
@ 2023-04-24 17:17     ` Roberto Sassu
  0 siblings, 0 replies; 5+ messages in thread
From: Roberto Sassu @ 2023-04-24 17:17 UTC (permalink / raw)
  To: Mimi Zohar, dmitry.kasatkin
  Cc: linux-integrity, vt, pvorel, stefanb, Roberto Sassu

On Mon, 2023-04-24 at 08:22 -0400, Mimi Zohar wrote:
> Hi Roberto,
> 
> On Fri, 2023-03-24 at 19:11 +0100, Roberto Sassu wrote:
> > From: Roberto Sassu <roberto.sassu@huawei.com>
> > 
> > Add a simple test to ensure that the kernel and evmctl provide the same
> > result for the HMAC calculation.
> 
> Based on the LSM discussions, including a test for a file in a Smack
> transmuting directory would be nice.

Hi Mimi

ok, will add a test.

> > This test requires that, unless the UML kernel is used, the
> > TST_EVM_CHANGE_MODE environment variable is set to 1.
> 
> Agreed requring permission to enable EVM HMAC with a well known HMAC
> value is 
> a good idea.   Please update the patch description with an explanation.

Ok.

> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > ---
> >  kernel-configs/base |   4 +-
> >  tests/Makefile.am   |   2 +-
> >  tests/evm_hmac.test | 170 ++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 174 insertions(+), 2 deletions(-)
> >  create mode 100755 tests/evm_hmac.test
> > 
> > diff --git a/kernel-configs/base b/kernel-configs/base
> > index 7acbd5b3b2a..0d8714d8f98 100644
> > --- a/kernel-configs/base
> > +++ b/kernel-configs/base
> > @@ -46,11 +46,13 @@ CONFIG_TMPFS_XATTR=y
> >  CONFIG_CONFIGFS_FS=y
> >  CONFIG_KEYS=y
> >  CONFIG_ENCRYPTED_KEYS=y
> > +CONFIG_USER_DECRYPTED_DATA=y
> >  CONFIG_SECURITY=y
> >  CONFIG_SECURITYFS=y
> >  CONFIG_SECURITY_NETWORK=y
> >  CONFIG_SECURITY_PATH=y
> > -CONFIG_LSM="lockdown,yama,loadpin,safesetid,integrity,bpf"
> > +CONFIG_SECURITY_SMACK=y
> > +CONFIG_LSM="lockdown,yama,loadpin,safesetid,smack,integrity,bpf"
> >  CONFIG_CRYPTO_AEAD2=y
> >  CONFIG_CRYPTO_SKCIPHER=y
> >  CONFIG_CRYPTO_SKCIPHER2=y
> > diff --git a/tests/Makefile.am b/tests/Makefile.am
> > index 03aa5b76088..a28f671398f 100644
> > --- a/tests/Makefile.am
> > +++ b/tests/Makefile.am
> > @@ -3,7 +3,7 @@ TESTS = $(check_SCRIPTS)
> >  
> >  check_SCRIPTS += ima_hash.test sign_verify.test boot_aggregate.test \
> >  		 fsverity.test portable_signatures.test ima_policy_check.test \
> > -		 mmap_check.test
> > +		 mmap_check.test evm_hmac.test
> >  
> >  check_PROGRAMS := test_mmap
> >  
> > diff --git a/tests/evm_hmac.test b/tests/evm_hmac.test
> > new file mode 100755
> > index 00000000000..de8b6a9a4d9
> > --- /dev/null
> > +++ b/tests/evm_hmac.test
> > @@ -0,0 +1,170 @@
> > +#!/bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +#
> > +# Copyright (C) 2023 Roberto Sassu <roberto.sassu@huawei.com>
> > +#
> > +# Check if the kernel and evmctl provide the same result for HMAC calculation.
> > +
> > +trap '_report_exit_and_cleanup _cleanup_env cleanup' SIGINT SIGTERM SIGSEGV EXIT
> > +
> > +# Base VERBOSE on the environment variable, if set.
> > +VERBOSE="${VERBOSE:-0}"
> > +TST_EVM_CHANGE_MODE="${TST_EVM_CHANGE_MODE:-0}"
> > +
> > +# From security/integrity/evm/evm.h in kernel source directory
> > +(( EVM_INIT_HMAC=0x0001 ))
> > +
> > +cd "$(dirname "$0")" || exit 1
> > +export PATH=$PWD/../src:$PATH
> > +export LD_LIBRARY_PATH=$LD_LIBRARY_PATH
> > +. ./functions.sh
> > +_require evmctl
> > +
> > +cleanup() {
> > +	if [ "$g_loop_mounted" = "1" ]; then
> > +		popd > /dev/null || exit "$FAIL"
> > +		umount "$g_mountpoint"
> > +	fi
> > +
> > +	if [ -n "$g_dev" ]; then
> > +		losetup -d "$g_dev"
> > +	fi
> > +
> > +	if [ -n "$g_image" ]; then
> > +		rm -f "$g_image"
> > +	fi
> > +
> > +	if [ -n "$g_mountpoint" ]; then
> > +		rm -Rf "$g_mountpoint"
> > +	fi
> > +}
> > +
> > +get_xattr() {
> > +	local format="hex"
> > +
> > +	if [ "$1" = "security.selinux" ]; then
> > +		format="text"
> > +	fi
> > +
> > +	getfattr -n "$1" -e "$format" -d "$2" 2> /dev/null | awk -F "=" '$1 == "'"$1"'" {if ("'"$format"'" == "hex") v=substr($2, 3); else { split($2, temp, "\""); v=temp[2] }; print v}'
> > +}
> > +
> > +IMA_UUID="28b23254-9467-44c0-b6ba-34b12e85a26f"
> > +
> > +# The purpose of this test is to verify if the kernel and evmctl produce the
> > +# same HMAC.
> > +check_evm_hmac() {
> > +	local evm_xattr evm_xattr_evmctl test_file_digest
> > +
> > +	echo "Test: ${FUNCNAME[0]} (evm_hash: $1, evm_value: $g_evm_value, algo: $1, fs: $2)"
> > +
> > +	if ! touch test-file; then
> > +		echo "${RED}Cannot create test-file${NORM}"
> > +		return "$FAIL"
> > +	fi
> > +
> > +	# Compare HMAC calculated by the kernel with that calculated by evmctl.
> > +	evm_xattr="$(get_xattr security.evm test-file)"
> > +	test_file_digest=$("$1"sum test-file | awk '{print $1}')
> > +	# evm_xattr has an extra byte at the beginning for the xattr type.
> > +	if [ "${#evm_xattr}" != $(( ${#test_file_digest} + 2 )) ]; then
> > +		echo "${RED}Unexpected size of security.evm${NORM}"
> > +		return "$FAIL"
> > +	fi
> > +
> > +	evm_xattr_evmctl="$(evmctl hmac -v -n test-file --uuid=$IMA_UUID -a "$1" --hmackey "$g_hmackey" 2>&1 | awk -F " " '$1 == "hmac:" {print $2}')"
> > +	if [ "$evm_xattr" != "02$evm_xattr_evmctl" ]; then
> > +		echo "${RED}security.evm mismatch between the kernel and evmctl${NORM}"
> > +		return "$FAIL"
> > +	fi
> > +
> > +	return "$OK"
> > +}
> > +
> > +cleanup_evm_hmac() {
> > +	rm -f test-file
> > +}
> > +
> > +_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
> > +
> > +# Assume that the EVM mode can be changed in a new environment.
> > +if [ -z "$TST_ENV" ] && [ "$TST_EVM_CHANGE_MODE" -eq 0 ]; then
> > +	echo "${CYAN}TST_EVM_CHANGE_MODE env variable must be set to 1${NORM}"
> > +	exit "$SKIP"
> > +fi
> > +
> > +g_lsm_init_xattr=$(awk '$1 ~ /(smack|selinux)/' < /sys/kernel/security/lsm)
> > +if [ -z "$g_lsm_init_xattr" ]; then
> > +	echo "${CYAN}Either SMACK or SELinux must be active in the system${NORM}"
> > +	exit "$SKIP"
> > +fi
> > +
> > +g_mountpoint="$(mktemp -d)"
> > +g_image="$(mktemp)"
> > +
> > +if [ -z "$g_mountpoint" ]; then
> > +	echo "${RED}Mountpoint directory not created${NORM}"
> > +	exit "$FAIL"
> > +fi
> > +
> > +if [ "$(whoami)" != "root" ]; then
> > +	echo "${CYAN}This script must be executed as root${NORM}"
> > +	exit "$SKIP"
> > +fi
> > +
> > +if ! dd if=/dev/zero of="$g_image" bs=1M count=10 &> /dev/null; then
> > +	echo "${RED}Cannot create test image${NORM}"
> > +	exit "$FAIL"
> > +fi
> > +
> > +g_dev="$(losetup -f "$g_image" --show)"
> > +if [ -z "$g_dev" ]; then
> > +	echo "${RED}Cannot create loop device${NORM}"
> > +	exit "$FAIL"
> > +fi
> > +
> > +if ! mkfs.ext4 -U $IMA_UUID -b 4096 "$g_dev" &> /dev/null; then
> > +	echo "${RED}Cannot format $g_dev${NORM}"
> > +	exit "$FAIL"
> > +fi
> > +
> > +if ! mount -o i_version "$g_dev" "$g_mountpoint"; then
> > +	echo "${RED}Cannot mount loop device${NORM}"
> > +	exit "$FAIL"
> > +fi
> > +
> > +g_loop_mounted=1
> > +chmod 777 "$g_mountpoint"
> > +pushd "$g_mountpoint" > /dev/null || exit "$FAIL"
> > +
> > +if [ -f /sys/kernel/security/evm ]; then
> > +	g_evm_value=$(cat /sys/kernel/security/evm)
> > +fi
> > +
> > +g_hmackey_data="abcdefABCDEF1234567890aaaaaaaaaaabcdefABCDEF1234567890aaaaaaaaaa"
> > +
> > +g_hmackey="$(mktemp)"
> > +echo $g_hmackey_data | xxd -r -p > "$g_hmackey"
> > +
> > +if [ -n "$g_evm_value" ] && [ $((g_evm_value & EVM_INIT_HMAC)) -ne $EVM_INIT_HMAC ]; then
> > +	g_evm_id="$(keyctl add encrypted evm-key "new enc32 user:kmk 32 $g_hmackey_data" @u)"
> 
> The above command fails on kernels without encrypted key support for
> user provided decrypted data.
> 
> Required commits:
> 5adedd42245a ("KEYS: encrypted: fix key instantiation with user-
> provided data")
> cd3bc044af48 ("KEYS: encrypted: Instantiate key with user-provided
> decrypted data")

Ok, thanks. Will add those commits as dependency for the tests.

Roberto

> > +	if ! echo "$EVM_INIT_HMAC" | tee /sys/kernel/security/evm &> /dev/null; then
> > +		# Retry with sudo -i, to force search in the root user keyring.
> > +		if ! echo "$EVM_INIT_HMAC" | sudo -i tee /sys/kernel/security/evm &> /dev/null; then
> > +			keyctl unlink "$g_evm_id"
> > +			echo "${RED}Failed to initialize EVM${NORM}"
> > +			exit "$FAIL"
> > +		fi
> > +	fi
> > +
> > +	g_evm_value=$(cat /sys/kernel/security/evm)
> > +fi
> > +
> > +expect_pass check_evm_hmac sha1 ext4
> > +cleanup_evm_hmac


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH ima-evm-utils 1/2] Add --hmackey option for evmctl
  2023-03-24 18:11 [PATCH ima-evm-utils 1/2] Add --hmackey option for evmctl Roberto Sassu
  2023-03-24 18:11 ` [PATCH ima-evm-utils 2/2] Add simple test to check EVM HMAC calculation Roberto Sassu
@ 2023-04-24 17:44 ` Mimi Zohar
  1 sibling, 0 replies; 5+ messages in thread
From: Mimi Zohar @ 2023-04-24 17:44 UTC (permalink / raw)
  To: Roberto Sassu, dmitry.kasatkin
  Cc: linux-integrity, vt, pvorel, stefanb, Roberto Sassu

On Fri, 2023-03-24 at 19:11 +0100, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
> 
> Add the --hmackey option, to specify an alternative path for the file
> containing the HMAC key. By default evmctl looks in
> /etc/keys/evm-key-plain.
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

Please document the change in the evmctl.c usage and the README.

Please note that when DEBUG is not enabled, "evmctl hmac" is not
defined.   Patch 2/2 assumes that "evmctl hmac" is configured.

-- 
thanks,

Mimi


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-04-24 17:45 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-24 18:11 [PATCH ima-evm-utils 1/2] Add --hmackey option for evmctl Roberto Sassu
2023-03-24 18:11 ` [PATCH ima-evm-utils 2/2] Add simple test to check EVM HMAC calculation Roberto Sassu
2023-04-24 12:22   ` Mimi Zohar
2023-04-24 17:17     ` Roberto Sassu
2023-04-24 17:44 ` [PATCH ima-evm-utils 1/2] Add --hmackey option for evmctl Mimi Zohar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).