[PATCH 0/7] ima/evm: Small enhancements

[PATCH 0/7] ima/evm: Small enhancements

[Roberto Sassu]

This patch set provides some small enhancements for IMA and EVM.

Patch 1 avoids measurement and audit when access to the file will be denied
by IMA itself.

Patch 2 introduces a new policy keyword meta_immutable to protect the label
transition during binary execution.

Patch 3-5 add new hard-coded policies aiming at producing measurement or
enforcing access to files that likely are provided by software vendors.

Patch 6 increases the crypto resistance of EVM by allowing the choice of
the hash algorithm for the HMAC.

Patch 7 adds two new values for the evm= option in the kernel command line
to facilitate the setup of EVM.

Roberto Sassu (7):
  ima: Avoid measurement and audit if access to the file will be denied
  ima: Add meta_immutable appraisal type
  ima: Introduce exec_tcb and tmpfs policies
  ima: Introduce appraise_exec_tcb and appraise_tmpfs policies
  ima: Introduce appraise_exec_immutable policy
  evm: Allow choice of hash algorithm for HMAC
  evm: Extend evm= with allow_metadata_writes and complete values

 Documentation/ABI/testing/ima_policy          |  2 +-
 .../admin-guide/kernel-parameters.txt         | 36 +++++++-
 security/integrity/evm/Kconfig                | 34 +++++++
 security/integrity/evm/evm.h                  |  2 +
 security/integrity/evm/evm_crypto.c           | 55 ++++++++++--
 security/integrity/evm/evm_main.c             | 29 ++++--
 security/integrity/ima/ima_appraise.c         |  9 ++
 security/integrity/ima/ima_main.c             | 20 +++--
 security/integrity/ima/ima_policy.c           | 90 ++++++++++++++-----
 security/integrity/integrity.h                |  4 +-
 10 files changed, 232 insertions(+), 49 deletions(-)

-- 
2.26.2

[PATCH 1/7] ima: Avoid measurement and audit if access to the file will be denied

[Roberto Sassu]

Currently, IMA adds a measurement entry and an audit log even if access
to the file will be denied when appraisal is in enforce mode. This gives
the false indication to a verifier analyzing the data that file access
occurred. It also has the undesirable effect of unnecessarily revoking
access to TPM-protected objects sealed to PCRs.

Given that a potentially corrupted file will not be accessed by any user
space process, it is safe for the purpose of integrity evaluation to avoid
to record a measurement or audit log in this case.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima_main.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 9ef748ea829f..0faddcb8c71a 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -328,16 +328,14 @@ static int process_measurement(struct file *file, const struct cred *cred,
 	hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
 
 	rc = ima_collect_measurement(iint, file, buf, size, hash_algo, modsig);
-	if (rc != 0 && rc != -EBADF && rc != -EINVAL)
+	if (rc != 0 && rc != -EBADF && rc != -EINVAL) {
+		action &= ~(IMA_MEASURE | IMA_AUDIT);
 		goto out_locked;
+	}
 
 	if (!pathbuf)	/* ima_rdwr_violation possibly pre-fetched */
 		pathname = ima_d_path(&file->f_path, &pathbuf, filename);
 
-	if (action & IMA_MEASURE)
-		ima_store_measurement(iint, file, pathname,
-				      xattr_value, xattr_len, modsig, pcr,
-				      template_desc);
 	if (rc == 0 && (action & IMA_APPRAISE_SUBMASK)) {
 		rc = ima_check_blacklist(iint, modsig, pcr);
 		if (rc != -EPERM) {
@@ -351,15 +349,21 @@ static int process_measurement(struct file *file, const struct cred *cred,
 			rc = mmap_violation_check(func, file, &pathbuf,
 						  &pathname, filename);
 	}
-	if (action & IMA_AUDIT)
-		ima_audit_measurement(iint, pathname);
-
 	if ((file->f_flags & O_DIRECT) && (iint->flags & IMA_PERMIT_DIRECTIO))
 		rc = 0;
 out_locked:
 	if ((mask & MAY_WRITE) && test_bit(IMA_DIGSIG, &iint->atomic_flags) &&
 	     !(iint->flags & IMA_NEW_FILE))
 		rc = -EACCES;
+	if (must_appraise)
+		if (rc && (ima_appraise & IMA_APPRAISE_ENFORCE))
+			action &= ~(IMA_MEASURE | IMA_AUDIT);
+	if (action & IMA_MEASURE)
+		ima_store_measurement(iint, file, pathname,
+				      xattr_value, xattr_len, modsig, pcr,
+				      template_desc);
+	if (action & IMA_AUDIT)
+		ima_audit_measurement(iint, pathname);
 	mutex_unlock(&iint->mutex);
 	kfree(xattr_value);
 	ima_free_modsig(modsig);
-- 
2.26.2

[PATCH 2/7] ima: Add meta_immutable appraisal type

[Roberto Sassu]

Currently, IMA supports the appraise_type=imasig option in the policy to
require signed file content or metadata. This patch introduces the new
option appraise_type=meta_immutable to require that file metadata is also
immutable, i.e. it cannot have been produced by the system itself but only
from a vendor whose signing key is trusted by the kernel. Currently, this
requirement can be satisfied only by portable signatures.

The main purpose of this option is to ensure a proper label transition
during binary execution, when the target label depends on the label of the
binary being executed. Without it, an administrator might obtain a
different target label by changing the label of the executable.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 Documentation/ABI/testing/ima_policy  |  2 +-
 security/integrity/ima/ima_appraise.c |  9 +++++++++
 security/integrity/ima/ima_policy.c   | 13 ++++++++++---
 security/integrity/integrity.h        |  1 +
 4 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 070779e8d836..bc6597db7c78 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -42,7 +42,7 @@ Description:
 			fowner:= decimal value
 		  lsm:  are LSM specific
 		  option:
-			appraise_type:= [imasig] [imasig|modsig]
+			appraise_type:= [imasig] [imasig|modsig] [meta_immutable]
 			appraise_flag:= [check_blacklist]
 			Currently, blacklist check is only for files signed with appended
 			signature.
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 45e244fc2ef2..5814b8cbe86c 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -430,6 +430,15 @@ int ima_appraise_measurement(enum ima_hooks func,
 		WARN_ONCE(true, "Unexpected integrity status %d\n", status);
 	}
 
+	if ((iint->flags & IMA_META_IMMUTABLE_REQUIRED) &&
+	    status != INTEGRITY_PASS_IMMUTABLE) {
+		status = INTEGRITY_FAIL;
+		cause = "metadata-modifiable";
+		integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode,
+				    filename, op, cause, rc, 0);
+		goto out;
+	}
+
 	if (xattr_value)
 		rc = xattr_verify(func, iint, xattr_value, xattr_len, &status,
 				  &cause);
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 4f8cb155e4fd..33b5133645b3 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -1079,7 +1079,8 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
 		return false;
 
 	if (entry->action != APPRAISE &&
-	    entry->flags & (IMA_DIGSIG_REQUIRED | IMA_MODSIG_ALLOWED | IMA_CHECK_BLACKLIST))
+	    entry->flags & (IMA_DIGSIG_REQUIRED | IMA_MODSIG_ALLOWED |
+			    IMA_CHECK_BLACKLIST | IMA_META_IMMUTABLE_REQUIRED))
 		return false;
 
 	/*
@@ -1109,7 +1110,8 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
 				     IMA_UID | IMA_FOWNER | IMA_FSUUID |
 				     IMA_INMASK | IMA_EUID | IMA_PCR |
 				     IMA_FSNAME | IMA_DIGSIG_REQUIRED |
-				     IMA_PERMIT_DIRECTIO))
+				     IMA_PERMIT_DIRECTIO |
+				     IMA_META_IMMUTABLE_REQUIRED))
 			return false;
 
 		break;
@@ -1121,7 +1123,8 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
 				     IMA_INMASK | IMA_EUID | IMA_PCR |
 				     IMA_FSNAME | IMA_DIGSIG_REQUIRED |
 				     IMA_PERMIT_DIRECTIO | IMA_MODSIG_ALLOWED |
-				     IMA_CHECK_BLACKLIST))
+				     IMA_CHECK_BLACKLIST |
+				     IMA_META_IMMUTABLE_REQUIRED))
 			return false;
 
 		break;
@@ -1495,6 +1498,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 				 strcmp(args[0].from, "imasig|modsig") == 0)
 				entry->flags |= IMA_DIGSIG_REQUIRED |
 						IMA_MODSIG_ALLOWED;
+			else if (strcmp(args[0].from, "meta_immutable") == 0)
+				entry->flags |= IMA_META_IMMUTABLE_REQUIRED;
 			else
 				result = -EINVAL;
 			break;
@@ -1850,6 +1855,8 @@ int ima_policy_show(struct seq_file *m, void *v)
 	}
 	if (entry->flags & IMA_CHECK_BLACKLIST)
 		seq_puts(m, "appraise_flag=check_blacklist ");
+	if (entry->flags & IMA_META_IMMUTABLE_REQUIRED)
+		seq_puts(m, "appraise_type=meta_immutable ");
 	if (entry->flags & IMA_PERMIT_DIRECTIO)
 		seq_puts(m, "permit_directio ");
 	rcu_read_unlock();
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 547425c20e11..be501a63ae30 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -39,6 +39,7 @@
 #define IMA_FAIL_UNVERIFIABLE_SIGS	0x10000000
 #define IMA_MODSIG_ALLOWED	0x20000000
 #define IMA_CHECK_BLACKLIST	0x40000000
+#define IMA_META_IMMUTABLE_REQUIRED	0x80000000
 
 #define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
 				 IMA_HASH | IMA_APPRAISE_SUBMASK)
-- 
2.26.2

[PATCH 3/7] ima: Introduce exec_tcb and tmpfs policies

[Roberto Sassu]

This patch introduces two new hard-coded policies, named exec_tcb and
tmpfs. The first excludes the FILE_CHECK rules to measure only files
executed and mmapped. The second excludes the dont_measure rule for the
tmpfs filesystem for the ima_tcb, tcb and exec_tcb policies, as this could
be used as a way for an attacker to hide his actions. The new policies can
be selected by specifying them as a value of ima_policy= in the kernel
command line.

The benefit of using the exec_tcb policy, as opposed to the tcb policy, is
that most likely the measurement list will contain only immutable files,
recognizable from a set of reference values from software vendors. However,
this policy provides a less accurate view of the integrity of the system,
as opened files are excluded from measurement.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 .../admin-guide/kernel-parameters.txt         |  7 ++++
 security/integrity/ima/ima_policy.c           | 42 ++++++++++++-------
 2 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 04545725f187..88c2ba679c92 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1765,6 +1765,13 @@
 			mode bit set by either the effective uid (euid=0) or
 			uid=0.
 
+			The "exec_tcb" policy is similar to the "tcb" policy
+			except for opened files, which are not measured.
+
+			The "tmpfs" policy excludes the dont_measure rule for
+			the tmpfs filesystem, for the "ima_tcb", "tcb" and
+			"exec_tcb" policies.
+
 			The "appraise_tcb" policy appraises the integrity of
 			all files owned by root.
 
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 33b5133645b3..fff178abb004 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -35,6 +35,8 @@
 #define IMA_FSNAME	0x0200
 #define IMA_KEYRINGS	0x0400
 #define IMA_LABEL	0x0800
+#define IMA_SKIP_TMPFS	0x1000
+#define IMA_SKIP_OPEN	0x2000
 
 #define UNKNOWN		0
 #define MEASURE		0x0001	/* same as IMA_MEASURE */
@@ -105,7 +107,8 @@ static struct ima_rule_entry dont_measure_rules[] __ro_after_init = {
 	{.action = DONT_MEASURE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC},
 	{.action = DONT_MEASURE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC},
 	{.action = DONT_MEASURE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC},
-	{.action = DONT_MEASURE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC},
+	{.action = DONT_MEASURE, .fsmagic = TMPFS_MAGIC,
+	 .flags = IMA_FSMAGIC | IMA_SKIP_TMPFS},
 	{.action = DONT_MEASURE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC},
 	{.action = DONT_MEASURE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC},
 	{.action = DONT_MEASURE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC},
@@ -126,7 +129,7 @@ static struct ima_rule_entry original_measurement_rules[] __ro_after_init = {
 	 .flags = IMA_FUNC | IMA_MASK},
 	{.action = MEASURE, .func = FILE_CHECK, .mask = MAY_READ,
 	 .uid = GLOBAL_ROOT_UID, .uid_op = &uid_eq,
-	 .flags = IMA_FUNC | IMA_MASK | IMA_UID},
+	 .flags = IMA_FUNC | IMA_MASK | IMA_UID | IMA_SKIP_OPEN},
 	{.action = MEASURE, .func = MODULE_CHECK, .flags = IMA_FUNC},
 	{.action = MEASURE, .func = FIRMWARE_CHECK, .flags = IMA_FUNC},
 };
@@ -138,10 +141,10 @@ static struct ima_rule_entry default_measurement_rules[] __ro_after_init = {
 	 .flags = IMA_FUNC | IMA_MASK},
 	{.action = MEASURE, .func = FILE_CHECK, .mask = MAY_READ,
 	 .uid = GLOBAL_ROOT_UID, .uid_op = &uid_eq,
-	 .flags = IMA_FUNC | IMA_INMASK | IMA_EUID},
+	 .flags = IMA_FUNC | IMA_INMASK | IMA_EUID | IMA_SKIP_OPEN},
 	{.action = MEASURE, .func = FILE_CHECK, .mask = MAY_READ,
 	 .uid = GLOBAL_ROOT_UID, .uid_op = &uid_eq,
-	 .flags = IMA_FUNC | IMA_INMASK | IMA_UID},
+	 .flags = IMA_FUNC | IMA_INMASK | IMA_UID | IMA_SKIP_OPEN},
 	{.action = MEASURE, .func = MODULE_CHECK, .flags = IMA_FUNC},
 	{.action = MEASURE, .func = FIRMWARE_CHECK, .flags = IMA_FUNC},
 	{.action = MEASURE, .func = POLICY_CHECK, .flags = IMA_FUNC},
@@ -230,6 +233,7 @@ static int __init default_measure_policy_setup(char *str)
 }
 __setup("ima_tcb", default_measure_policy_setup);
 
+static unsigned int ima_measure_skip_flags __initdata;
 static bool ima_use_appraise_tcb __initdata;
 static bool ima_use_secure_boot __initdata;
 static bool ima_use_critical_data __initdata;
@@ -243,7 +247,12 @@ static int __init policy_setup(char *str)
 			continue;
 		if ((strcmp(p, "tcb") == 0) && !ima_policy)
 			ima_policy = DEFAULT_TCB;
-		else if (strcmp(p, "appraise_tcb") == 0)
+		else if ((strcmp(p, "tmpfs") == 0))
+			ima_measure_skip_flags |= IMA_SKIP_TMPFS;
+		else if ((strcmp(p, "exec_tcb") == 0) && !ima_policy) {
+			ima_policy = DEFAULT_TCB;
+			ima_measure_skip_flags |= IMA_SKIP_OPEN;
+		} else if (strcmp(p, "appraise_tcb") == 0)
 			ima_use_appraise_tcb = true;
 		else if (strcmp(p, "secure_boot") == 0)
 			ima_use_secure_boot = true;
@@ -739,13 +748,16 @@ static int ima_appraise_flag(enum ima_hooks func)
 }
 
 static void add_rules(struct ima_rule_entry *entries, int count,
-		      enum policy_rule_list policy_rule)
+		      enum policy_rule_list policy_rule, int skip_flags)
 {
 	int i = 0;
 
 	for (i = 0; i < count; i++) {
 		struct ima_rule_entry *entry;
 
+		if (entries[i].flags & skip_flags)
+			continue;
+
 		if (policy_rule & IMA_DEFAULT_POLICY)
 			list_add_tail(&entries[i].list, &ima_default_rules);
 
@@ -824,18 +836,18 @@ void __init ima_init_policy(void)
 	/* if !ima_policy, we load NO default rules */
 	if (ima_policy)
 		add_rules(dont_measure_rules, ARRAY_SIZE(dont_measure_rules),
-			  IMA_DEFAULT_POLICY);
+			  IMA_DEFAULT_POLICY, ima_measure_skip_flags);
 
 	switch (ima_policy) {
 	case ORIGINAL_TCB:
 		add_rules(original_measurement_rules,
 			  ARRAY_SIZE(original_measurement_rules),
-			  IMA_DEFAULT_POLICY);
+			  IMA_DEFAULT_POLICY, ima_measure_skip_flags);
 		break;
 	case DEFAULT_TCB:
 		add_rules(default_measurement_rules,
 			  ARRAY_SIZE(default_measurement_rules),
-			  IMA_DEFAULT_POLICY);
+			  IMA_DEFAULT_POLICY, ima_measure_skip_flags);
 	default:
 		break;
 	}
@@ -851,7 +863,7 @@ void __init ima_init_policy(void)
 		pr_info("No architecture policies found\n");
 	else
 		add_rules(arch_policy_entry, arch_entries,
-			  IMA_DEFAULT_POLICY | IMA_CUSTOM_POLICY);
+			  IMA_DEFAULT_POLICY | IMA_CUSTOM_POLICY, 0);
 
 	/*
 	 * Insert the builtin "secure_boot" policy rules requiring file
@@ -859,7 +871,7 @@ void __init ima_init_policy(void)
 	 */
 	if (ima_use_secure_boot)
 		add_rules(secure_boot_rules, ARRAY_SIZE(secure_boot_rules),
-			  IMA_DEFAULT_POLICY);
+			  IMA_DEFAULT_POLICY, 0);
 
 	/*
 	 * Insert the build time appraise rules requiring file signatures
@@ -871,21 +883,21 @@ void __init ima_init_policy(void)
 	if (build_appraise_entries) {
 		if (ima_use_secure_boot)
 			add_rules(build_appraise_rules, build_appraise_entries,
-				  IMA_CUSTOM_POLICY);
+				  IMA_CUSTOM_POLICY, 0);
 		else
 			add_rules(build_appraise_rules, build_appraise_entries,
-				  IMA_DEFAULT_POLICY | IMA_CUSTOM_POLICY);
+				  IMA_DEFAULT_POLICY | IMA_CUSTOM_POLICY, 0);
 	}
 
 	if (ima_use_appraise_tcb)
 		add_rules(default_appraise_rules,
 			  ARRAY_SIZE(default_appraise_rules),
-			  IMA_DEFAULT_POLICY);
+			  IMA_DEFAULT_POLICY, 0);
 
 	if (ima_use_critical_data)
 		add_rules(critical_data_rules,
 			  ARRAY_SIZE(critical_data_rules),
-			  IMA_DEFAULT_POLICY);
+			  IMA_DEFAULT_POLICY, 0);
 
 	ima_update_policy_flag();
 }
-- 
2.26.2

[PATCH 4/7] ima: Introduce appraise_exec_tcb and appraise_tmpfs policies

[Roberto Sassu]

This patch introduces two new hard-coded policies, named appraise_exec_tcb
and appraise_tmpfs. The first appraises files executed or mmapped, as
opposed to files owned by root, requiring in addition a digital signature.
The second appraises files in the tmpfs and ramfs filesystems, to avoid
that an attacker copies files there to avoid appraisal. The new policies
can be selected by specifying them as a value of ima_policy= in the kernel
command line.

Similarly to the exec_tcb policy, the appraise_exec_tcb policy can be used
to appraise only immutable files and avoid appraisal of mutable files which
might not have an HMAC or digital signature.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 .../admin-guide/kernel-parameters.txt         |  9 +++++
 security/integrity/ima/ima_policy.c           | 33 +++++++++++++++----
 2 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 88c2ba679c92..93c5f78905e2 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1775,6 +1775,15 @@
 			The "appraise_tcb" policy appraises the integrity of
 			all files owned by root.
 
+			The "appraise_exec_tcb" is similar to the "appraise_tcb"
+			policy except for files executed or mmapped, which are
+			appraised (with imasig requirement) instead of files
+			owned by root.
+
+			The "appraise_tmpfs" policy excludes the dont_appraise
+			rule for the tmpfs and ramfs filesystems for the
+			"appraise_tcb" and "appraise_exec_tcb" policies.
+
 			The "secure_boot" policy appraises the integrity
 			of files (eg. kexec kernel image, kernel modules,
 			firmware, policy, etc) based on file signatures.
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index fff178abb004..a45e494e06e8 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -154,8 +154,8 @@ static struct ima_rule_entry default_appraise_rules[] __ro_after_init = {
 	{.action = DONT_APPRAISE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC},
 	{.action = DONT_APPRAISE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC},
 	{.action = DONT_APPRAISE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC},
-	{.action = DONT_APPRAISE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC},
-	{.action = DONT_APPRAISE, .fsmagic = RAMFS_MAGIC, .flags = IMA_FSMAGIC},
+	{.action = DONT_APPRAISE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC | IMA_SKIP_TMPFS},
+	{.action = DONT_APPRAISE, .fsmagic = RAMFS_MAGIC, .flags = IMA_FSMAGIC | IMA_SKIP_TMPFS},
 	{.action = DONT_APPRAISE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC},
 	{.action = DONT_APPRAISE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC},
 	{.action = DONT_APPRAISE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC},
@@ -171,14 +171,21 @@ static struct ima_rule_entry default_appraise_rules[] __ro_after_init = {
 #endif
 #ifndef CONFIG_IMA_APPRAISE_SIGNED_INIT
 	{.action = APPRAISE, .fowner = GLOBAL_ROOT_UID, .fowner_op = &uid_eq,
-	 .flags = IMA_FOWNER},
+	 .flags = IMA_FOWNER | IMA_SKIP_OPEN},
 #else
 	/* force signature */
 	{.action = APPRAISE, .fowner = GLOBAL_ROOT_UID, .fowner_op = &uid_eq,
-	 .flags = IMA_FOWNER | IMA_DIGSIG_REQUIRED},
+	 .flags = IMA_FOWNER | IMA_DIGSIG_REQUIRED | IMA_SKIP_OPEN},
 #endif
 };
 
+static struct ima_rule_entry appraise_exec_rules[] __ro_after_init = {
+	{.action = APPRAISE, .func = BPRM_CHECK,
+	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+	{.action = APPRAISE, .func = MMAP_CHECK,
+	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+};
+
 static struct ima_rule_entry build_appraise_rules[] __ro_after_init = {
 #ifdef CONFIG_IMA_APPRAISE_REQUIRE_MODULE_SIGS
 	{.action = APPRAISE, .func = MODULE_CHECK,
@@ -234,6 +241,7 @@ static int __init default_measure_policy_setup(char *str)
 __setup("ima_tcb", default_measure_policy_setup);
 
 static unsigned int ima_measure_skip_flags __initdata;
+static unsigned int ima_appraise_skip_flags __initdata;
 static bool ima_use_appraise_tcb __initdata;
 static bool ima_use_secure_boot __initdata;
 static bool ima_use_critical_data __initdata;
@@ -254,7 +262,12 @@ static int __init policy_setup(char *str)
 			ima_measure_skip_flags |= IMA_SKIP_OPEN;
 		} else if (strcmp(p, "appraise_tcb") == 0)
 			ima_use_appraise_tcb = true;
-		else if (strcmp(p, "secure_boot") == 0)
+		else if ((strcmp(p, "appraise_tmpfs") == 0))
+			ima_appraise_skip_flags |= IMA_SKIP_TMPFS;
+		else if (strcmp(p, "appraise_exec_tcb") == 0) {
+			ima_use_appraise_tcb = true;
+			ima_appraise_skip_flags |= IMA_SKIP_OPEN;
+		} else if (strcmp(p, "secure_boot") == 0)
 			ima_use_secure_boot = true;
 		else if (strcmp(p, "critical_data") == 0)
 			ima_use_critical_data = true;
@@ -889,10 +902,16 @@ void __init ima_init_policy(void)
 				  IMA_DEFAULT_POLICY | IMA_CUSTOM_POLICY, 0);
 	}
 
-	if (ima_use_appraise_tcb)
+	if (ima_use_appraise_tcb) {
 		add_rules(default_appraise_rules,
 			  ARRAY_SIZE(default_appraise_rules),
-			  IMA_DEFAULT_POLICY, 0);
+			  IMA_DEFAULT_POLICY, ima_appraise_skip_flags);
+
+		if (ima_appraise_skip_flags & IMA_SKIP_OPEN)
+			add_rules(appraise_exec_rules,
+				  ARRAY_SIZE(appraise_exec_rules),
+				  IMA_DEFAULT_POLICY, 0);
+	}
 
 	if (ima_use_critical_data)
 		add_rules(critical_data_rules,
-- 
2.26.2

[PATCH 5/7] ima: Introduce appraise_exec_immutable policy

[Roberto Sassu]

This patch modifies the existing "appraise_exec_tcb" policy, by adding the
appraise_type=meta_immutable requirement for executed files. This policy
can be selected by specifying
ima_policy="appraise_exec_tcb|appraise_exec_immutable" in the kernel
command line.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 4 ++++
 security/integrity/ima/ima_policy.c             | 8 +++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 93c5f78905e2..265f7657f59d 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1780,6 +1780,10 @@
 			appraised (with imasig requirement) instead of files
 			owned by root.
 
+			The "appraise_exec_immutable" policy requires immutable
+			metadata for executed files, if the "appraise_exec_tcb"
+			policy is selected.
+
 			The "appraise_tmpfs" policy excludes the dont_appraise
 			rule for the tmpfs and ramfs filesystems for the
 			"appraise_tcb" and "appraise_exec_tcb" policies.
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index a45e494e06e8..6249817ebd04 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -243,6 +243,7 @@ __setup("ima_tcb", default_measure_policy_setup);
 static unsigned int ima_measure_skip_flags __initdata;
 static unsigned int ima_appraise_skip_flags __initdata;
 static bool ima_use_appraise_tcb __initdata;
+static bool ima_use_appraise_exec_immutable __initdata;
 static bool ima_use_secure_boot __initdata;
 static bool ima_use_critical_data __initdata;
 static bool ima_fail_unverifiable_sigs __ro_after_init;
@@ -267,7 +268,9 @@ static int __init policy_setup(char *str)
 		else if (strcmp(p, "appraise_exec_tcb") == 0) {
 			ima_use_appraise_tcb = true;
 			ima_appraise_skip_flags |= IMA_SKIP_OPEN;
-		} else if (strcmp(p, "secure_boot") == 0)
+		} else if (strcmp(p, "appraise_exec_immutable") == 0)
+			ima_use_appraise_exec_immutable = true;
+		else if (strcmp(p, "secure_boot") == 0)
 			ima_use_secure_boot = true;
 		else if (strcmp(p, "critical_data") == 0)
 			ima_use_critical_data = true;
@@ -913,6 +916,9 @@ void __init ima_init_policy(void)
 				  IMA_DEFAULT_POLICY, 0);
 	}
 
+	if (ima_use_appraise_exec_immutable)
+		appraise_exec_rules[0].flags |= IMA_META_IMMUTABLE_REQUIRED;
+
 	if (ima_use_critical_data)
 		add_rules(critical_data_rules,
 			  ARRAY_SIZE(critical_data_rules),
-- 
2.26.2

[PATCH 6/7] evm: Allow choice of hash algorithm for HMAC

[Roberto Sassu]

Commit 5feeb61183dd ("evm: Allow non-SHA1 digital signatures") introduced
the possibility to use a different hash algorithm for signatures, but kept
the algorithm for the HMAC hard-coded (SHA1). Switching to a different
algorithm for HMAC would require to change the code in different places.

This patch introduces a new global variable called evm_hash_algo, and
consistently uses it whenever EVM performs HMAC-related operations. This
variable can be modified at kernel build time with the new configuration
option called CONFIG_EVM_DEFAULT_HASH, or at run-time with the new option
evm_hash=.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 .../admin-guide/kernel-parameters.txt         |  8 +++
 security/integrity/evm/Kconfig                | 34 ++++++++++++
 security/integrity/evm/evm.h                  |  2 +
 security/integrity/evm/evm_crypto.c           | 55 +++++++++++++++++--
 security/integrity/evm/evm_main.c             | 13 +++--
 security/integrity/integrity.h                |  3 +-
 6 files changed, 105 insertions(+), 10 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 265f7657f59d..f61ce44c5d8e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1343,6 +1343,14 @@
 			Permit 'security.evm' to be updated regardless of
 			current integrity status.
 
+	evm_hash=	[EVM] Hash algorithm used to calculate the HMAC.
+			Format: { md5 | sha1 | rmd160 | sha256 | sha384
+				   | sha512 | ... }
+			default: "sha256"
+
+			The list of supported hash algorithms is defined
+			in crypto/hash_info.h.
+
 	failslab=
 	fail_usercopy=
 	fail_page_alloc=
diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
index a6e19d23e700..234077b24283 100644
--- a/security/integrity/evm/Kconfig
+++ b/security/integrity/evm/Kconfig
@@ -13,6 +13,40 @@ config EVM
 
 	  If you are unsure how to answer this question, answer N.
 
+choice
+	prompt "Default EVM hash algorithm"
+	default EVM_DEFAULT_HASH_SHA256
+	depends on EVM
+	help
+	   Select the default hash algorithm used for the HMAC. The compiled
+	   default hash algorithm can be overwritten using the kernel command
+	   line 'evm_hash=' option.
+
+	config EVM_DEFAULT_HASH_SHA1
+		bool "SHA1"
+		depends on CRYPTO_SHA1=y
+
+	config EVM_DEFAULT_HASH_SHA256
+		bool "SHA256 (default)"
+		depends on CRYPTO_SHA256=y
+
+	config EVM_DEFAULT_HASH_SHA512
+		bool "SHA512"
+		depends on CRYPTO_SHA512=y
+
+	config EVM_DEFAULT_HASH_WP512
+		bool "WP512"
+		depends on CRYPTO_WP512=y
+endchoice
+
+config EVM_DEFAULT_HASH
+	string
+	depends on EVM
+	default "sha1" if EVM_DEFAULT_HASH_SHA1
+	default "sha256" if EVM_DEFAULT_HASH_SHA256
+	default "sha512" if EVM_DEFAULT_HASH_SHA512
+	default "wp512" if EVM_DEFAULT_HASH_WP512
+
 config EVM_ATTR_FSUUID
 	bool "FSUUID (version 2)"
 	default y
diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
index f2fef2b5ed51..ae590f71ce7d 100644
--- a/security/integrity/evm/evm.h
+++ b/security/integrity/evm/evm.h
@@ -32,6 +32,7 @@ struct xattr_list {
 };
 
 extern int evm_initialized;
+extern enum hash_algo evm_hash_algo;
 
 #define EVM_ATTR_FSUUID		0x0001
 
@@ -49,6 +50,7 @@ struct evm_digest {
 } __packed;
 
 int evm_init_key(void);
+int __init evm_init_crypto(void);
 int evm_update_evmxattr(struct dentry *dentry,
 			const char *req_xattr_name,
 			const char *req_xattr_value,
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index d76b006cbcc4..b66264b53d5d 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -33,7 +33,24 @@ static DEFINE_MUTEX(mutex);
 
 static unsigned long evm_set_key_flags;
 
-static const char evm_hmac[] = "hmac(sha1)";
+enum hash_algo evm_hash_algo __ro_after_init = HASH_ALGO_SHA1;
+
+static int hash_setup_done;
+static int __init hash_setup(char *str)
+{
+	int i;
+
+	i = match_string(hash_algo_name, HASH_ALGO__LAST, str);
+	if (i < 0) {
+		pr_err("invalid hash algorithm \"%s\"", str);
+		return 1;
+	}
+
+	evm_hash_algo = i;
+	hash_setup_done = 1;
+	return 1;
+}
+__setup("evm_hash=", hash_setup);
 
 /**
  * evm_set_key() - set EVM HMAC key from the kernel
@@ -74,8 +91,12 @@ static struct shash_desc *init_desc(char type, uint8_t hash_algo)
 	long rc;
 	const char *algo;
 	struct crypto_shash **tfm, *tmp_tfm = NULL;
+	char evm_hmac[CRYPTO_MAX_ALG_NAME];
 	struct shash_desc *desc;
 
+	snprintf(evm_hmac, sizeof(evm_hmac), "hmac(%s)",
+		 hash_algo_name[evm_hash_algo]);
+
 	if (type == EVM_XATTR_HMAC) {
 		if (!(evm_initialized & EVM_INIT_HMAC)) {
 			pr_err_once("HMAC key is not set\n");
@@ -317,7 +338,7 @@ int evm_update_evmxattr(struct dentry *dentry, const char *xattr_name,
 	if (rc)
 		return -EPERM;
 
-	data.hdr.algo = HASH_ALGO_SHA1;
+	data.hdr.algo = evm_hash_algo;
 	rc = evm_calc_hmac(dentry, xattr_name, xattr_value,
 			   xattr_value_len, &data);
 	if (rc == 0) {
@@ -325,7 +346,8 @@ int evm_update_evmxattr(struct dentry *dentry, const char *xattr_name,
 		rc = __vfs_setxattr_noperm(&init_user_ns, dentry,
 					   XATTR_NAME_EVM,
 					   &data.hdr.xattr.data[1],
-					   SHA1_DIGEST_SIZE + 1, 0);
+					   hash_digest_size[evm_hash_algo] + 1,
+					   0);
 	} else if (rc == -ENODATA && (inode->i_opflags & IOP_XATTR)) {
 		rc = __vfs_removexattr(&init_user_ns, dentry, XATTR_NAME_EVM);
 	}
@@ -337,7 +359,7 @@ int evm_init_hmac(struct inode *inode, const struct xattr *lsm_xattr,
 {
 	struct shash_desc *desc;
 
-	desc = init_desc(EVM_XATTR_HMAC, HASH_ALGO_SHA1);
+	desc = init_desc(EVM_XATTR_HMAC, evm_hash_algo);
 	if (IS_ERR(desc)) {
 		pr_info("init_desc failed\n");
 		return PTR_ERR(desc);
@@ -350,7 +372,7 @@ int evm_init_hmac(struct inode *inode, const struct xattr *lsm_xattr,
 }
 
 /*
- * Get the key from the TPM for the SHA1-HMAC
+ * Get the key from the TPM for the HMAC
  */
 int evm_init_key(void)
 {
@@ -373,3 +395,26 @@ int evm_init_key(void)
 	key_put(evm_key);
 	return rc;
 }
+
+/*
+ * Configure the hash algorithm for the HMAC
+ */
+int __init evm_init_crypto(void)
+{
+	int i;
+
+	if (hash_setup_done)
+		return 0;
+
+	i = match_string(hash_algo_name, HASH_ALGO__LAST,
+			 CONFIG_EVM_DEFAULT_HASH);
+	if (i < 0) {
+		pr_err("invalid hash algorithm \"%s\"",
+		       CONFIG_EVM_DEFAULT_HASH);
+		return -EINVAL;
+	}
+
+	evm_hash_algo = i;
+
+	return 0;
+}
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 8e80af97021e..cb3754e0cc60 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -187,18 +187,18 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
 	/* check value type */
 	switch (xattr_data->type) {
 	case EVM_XATTR_HMAC:
-		if (xattr_len != sizeof(struct evm_xattr)) {
+		if (xattr_len != hash_digest_size[evm_hash_algo] + 1) {
 			evm_status = INTEGRITY_FAIL;
 			goto out;
 		}
 
-		digest.hdr.algo = HASH_ALGO_SHA1;
+		digest.hdr.algo = evm_hash_algo;
 		rc = evm_calc_hmac(dentry, xattr_name, xattr_value,
 				   xattr_value_len, &digest);
 		if (rc)
 			break;
 		rc = crypto_memneq(xattr_data->data, digest.digest,
-				   SHA1_DIGEST_SIZE);
+				   hash_digest_size[evm_hash_algo]);
 		if (rc)
 			rc = -EINVAL;
 		break;
@@ -722,7 +722,7 @@ int evm_inode_init_security(struct inode *inode,
 		goto out;
 
 	evm_xattr->value = xattr_data;
-	evm_xattr->value_len = sizeof(*xattr_data);
+	evm_xattr->value_len = hash_digest_size[evm_hash_algo] + 1;
 	evm_xattr->name = XATTR_EVM_SUFFIX;
 	return 0;
 out:
@@ -759,6 +759,11 @@ static int __init init_evm(void)
 		goto error;
 	}
 
+	error = evm_init_crypto();
+	if (error < 0) {
+		pr_info("Error initializing crypto\n");
+		goto error;
+	}
 error:
 	if (error != 0) {
 		if (!list_empty(&evm_config_xattrnames)) {
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index be501a63ae30..74919b638f52 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -17,6 +17,7 @@
 #include <crypto/sha1.h>
 #include <linux/key.h>
 #include <linux/audit.h>
+#include <crypto/hash_info.h>
 
 /* iint action cache flags */
 #define IMA_MEASURE		0x00000001
@@ -89,7 +90,7 @@ struct evm_ima_xattr_data {
 /* Only used in the EVM HMAC code. */
 struct evm_xattr {
 	struct evm_ima_xattr_data data;
-	u8 digest[SHA1_DIGEST_SIZE];
+	u8 digest[SHA512_DIGEST_SIZE];
 } __packed;
 
 #define IMA_MAX_DIGEST_SIZE	64
-- 
2.26.2

[PATCH 7/7] evm: Extend evm= with allow_metadata_writes and complete values

[Roberto Sassu]

With the patch 'evm: Ignore INTEGRITY_NOLABEL/INTEGRITY_NOXATTRS if
conditions are safe', the INTEGRITY_NOLABEL and INTEGRITY_NOXATTRS errors
can be ignored by setting the EVM_SETUP_COMPLETE flag. Also, the same
errors can be avoided by disabling EVM verification completely with the
EVM_ALLOW_METADATA_WRITES flag.

This patch makes it possible to set these initialization flags directly in
the kernel command line, so that no additional setup is required from the
initial ram disk. The new allowed values for evm= are:

allow_metadata_writes: permit metadata modificatons;
complete: don't allow further changes of the EVM mode.

While EVM_ALLOW_METADATA_WRITES will be applied directly by the kernel at
setup time, EVM_SETUP_COMPLETE will be applied only if a public key is
loaded by evm_load_x509().

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 Documentation/admin-guide/kernel-parameters.txt |  8 +++++---
 security/integrity/evm/evm_main.c               | 16 ++++++++++++----
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index f61ce44c5d8e..eaf08095df43 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1339,9 +1339,11 @@
 			has equivalent usage. See its documentation for details.
 
 	evm=		[EVM]
-			Format: { "fix" }
-			Permit 'security.evm' to be updated regardless of
-			current integrity status.
+			Format: { "fix" | "allow_metadata_writes" | "complete" }
+			fix: permit 'security.evm' to be updated regardless of
+			current integrity status;
+			allow_metadata_writes: permit metadata modificatons;
+			complete: don't allow further changes of the EVM mode.
 
 	evm_hash=	[EVM] Hash algorithm used to calculate the HMAC.
 			Format: { md5 | sha1 | rmd160 | sha256 | sha384
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index cb3754e0cc60..84a9b7a69b1f 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -56,17 +56,22 @@ static struct xattr_list evm_config_default_xattrnames[] = {
 
 LIST_HEAD(evm_config_xattrnames);
 
-static int evm_fixmode;
-static int __init evm_set_fixmode(char *str)
+static int evm_fixmode __ro_after_init;
+static int evm_complete __ro_after_init;
+static int __init evm_set_param(char *str)
 {
 	if (strncmp(str, "fix", 3) == 0)
 		evm_fixmode = 1;
+	else if (strncmp(str, "allow_metadata_writes", 21) == 0)
+		evm_initialized |= EVM_ALLOW_METADATA_WRITES;
+	else if (strncmp(str, "complete", 8) == 0)
+		evm_complete = 1;
 	else
 		pr_err("invalid \"%s\" mode", str);
 
 	return 0;
 }
-__setup("evm=", evm_set_fixmode);
+__setup("evm=", evm_set_param);
 
 static void __init evm_init_config(void)
 {
@@ -737,8 +742,11 @@ void __init evm_load_x509(void)
 	int rc;
 
 	rc = integrity_load_x509(INTEGRITY_KEYRING_EVM, CONFIG_EVM_X509_PATH);
-	if (!rc)
+	if (!rc) {
 		evm_initialized |= EVM_INIT_X509;
+		if (evm_complete)
+			evm_initialized |= EVM_SETUP_COMPLETE;
+	}
 }
 #endif
 
-- 
2.26.2