linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/6] Add support for architecture specific IMA policies
@ 2018-09-19  7:55 Nayna Jain
  2018-09-19  7:55 ` [PATCH v3 1/6] x86/ima: define arch_ima_get_secureboot Nayna Jain
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Nayna Jain @ 2018-09-19  7:55 UTC (permalink / raw)
  To: linux-integrity
  Cc: zohar, linux-security-module, linux-efi, linux-kernel, dhowells,
	jforbes, Nayna Jain

The architecture specific policy, introduced in this patch set, permits
different architectures to define IMA policy rules based on kernel
configuration and system runtime information.

For example, on x86, there are two methods of verifying the kexec'ed kernel
image signature - CONFIG_KEXEC_VERIFY_SIG and IMA appraisal policy
KEXEC_KERNEL_CHECK. CONFIG_KEXEC_VERIFY_SIG enforces the kexec_file_load
syscall to verify file signatures, but does not prevent the kexec_load
syscall. The IMA KEXEC_KERNEL_CHECK policy rule verifies the kexec'ed
kernel image, loaded via the kexec_file_load syscall, is validly signed and
prevents loading a kernel image via the kexec_load syscall. When secure
boot is enabled, the kexec'ed kernel image needs to be signed and the
signature verified. In this environment, either method of verifying the
kexec'ed kernel image is acceptable, as long as the kexec_load syscall is
disabled.

The previous version of this patchset introduced a new IMA policy rule to
disable the kexec_load syscall, when CONFIG_KEXEC_VERIFY_SIG was enabled,
however that is removed from this version by introducing a different
mechanism.

The patchset defines an arch_ima_get_secureboot() function to retrieve the
secureboot state of the system. If secureboot is enabled and
CONFIG_KEXEC_VERIFY_SIG is configured, it denies permission to kexec_load
syscall.

To support architecture specific policies, a new function
arch_get_ima_policy() is defined. This patch set defines IMA
KERNEL_KEXEC_POLICY rules for x86 only if CONFIG_KEXEC_VERIFY_SIG is
disabled and secure boot is enabled.

This patch set includes a patch, which refactors ima_init_policy() to
remove code duplication.

Changelog:

v3:
* x86/ima: define arch_ima_get_secureboot
	- Edited subject line, added x86.

* x86/ima: define arch_get_ima_policy() for x86
	- Fixed the error reported by kbuild test robot. The error was
	appearing when CONFIG_X86 is enabled, but CONFIG_IMA_ARCH_POLICY
	is disabled.

v2:
* ima: define arch_ima_get_secureboot
	- New Patch - to retrieve secureboot state of the system
* ima: prevent kexec_load syscall based on runtime secureboot flag
	- New Patch - disables kexec_load if KEXEC_VERIFY_SIG is
	  configured and secureboot is enabled
* ima: refactor ima_init_policy()
	- New Patch - cleans up the code duplication in
	  ima_init_policy(), adds new function add_rules()
* ima: add support for arch specific policies
	- modified ima_init_arch_policy() and ima_init_policy() to
	  use add_rules() from previous patch.
* ima: add support for external setting of ima_appraise
	- sets ima_appraise flag explicitly for arch_specific setting
* ima: add support for KEXEC_ORIG_KERNEL_CHECK
	- deleted the patch based on Seth's feedback
* x86/ima: define arch_get_ima_policy() for x86
	- removes the policy KEXEC_ORIG_KERNEL_CHECK based on
	  Seth's feedback.

Eric Richter (1):
  x86/ima: define arch_get_ima_policy() for x86

Nayna Jain (5):
  x86/ima: define arch_ima_get_secureboot
  ima: prevent kexec_load syscall based on runtime secureboot flag
  ima: refactor ima_init_policy()
  ima: add support for arch specific policies
  ima: add support for external setting of ima_appraise

 arch/x86/kernel/Makefile              |   2 +
 arch/x86/kernel/ima_arch.c            |  35 +++++++
 include/linux/ima.h                   |  18 ++++
 security/integrity/ima/Kconfig        |   8 ++
 security/integrity/ima/ima.h          |   5 +
 security/integrity/ima/ima_appraise.c |  11 ++-
 security/integrity/ima/ima_main.c     |  17 ++--
 security/integrity/ima/ima_policy.c   | 167 +++++++++++++++++++++++++---------
 8 files changed, 214 insertions(+), 49 deletions(-)
 create mode 100644 arch/x86/kernel/ima_arch.c

-- 
2.13.6


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

* [PATCH v3 1/6] x86/ima: define arch_ima_get_secureboot
  2018-09-19  7:55 [PATCH v3 0/6] Add support for architecture specific IMA policies Nayna Jain
@ 2018-09-19  7:55 ` Nayna Jain
  2018-09-19  7:55 ` [PATCH v3 2/6] ima: prevent kexec_load syscall based on runtime secureboot flag Nayna Jain
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Nayna Jain @ 2018-09-19  7:55 UTC (permalink / raw)
  To: linux-integrity
  Cc: zohar, linux-security-module, linux-efi, linux-kernel, dhowells,
	jforbes, Nayna Jain

Distros are concerned about totally disabling the kexec_load syscall.
As a compromise, the kexec_load syscall will only be disabled when
CONFIG_KEXEC_VERIFY_SIG is configured and the system is booted with
secureboot enabled.

This patch defines the new arch specific function called
arch_ima_get_secureboot() to retrieve the secureboot state of the system.

Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
Suggested-by: Seth Forshee <seth.forshee@canonical.com>
---
 arch/x86/kernel/Makefile   |  2 ++
 arch/x86/kernel/ima_arch.c | 17 +++++++++++++++++
 include/linux/ima.h        |  9 +++++++++
 3 files changed, 28 insertions(+)
 create mode 100644 arch/x86/kernel/ima_arch.c

diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 02d6f5cf4e70..f32406e51424 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -149,3 +149,5 @@ ifeq ($(CONFIG_X86_64),y)
 	obj-$(CONFIG_MMCONF_FAM10H)	+= mmconf-fam10h_64.o
 	obj-y				+= vsmp_64.o
 endif
+
+obj-$(CONFIG_IMA)			+= ima_arch.o
diff --git a/arch/x86/kernel/ima_arch.c b/arch/x86/kernel/ima_arch.c
new file mode 100644
index 000000000000..bb5a88d2b271
--- /dev/null
+++ b/arch/x86/kernel/ima_arch.c
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (C) 2018 IBM Corporation
+ */
+#include <linux/efi.h>
+#include <linux/ima.h>
+
+extern struct boot_params boot_params;
+
+bool arch_ima_get_secureboot(void)
+{
+	if (efi_enabled(EFI_BOOT) &&
+		(boot_params.secure_boot == efi_secureboot_mode_enabled))
+		return true;
+	else
+		return false;
+}
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 84806b54b50a..4852255aa4f4 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -30,6 +30,15 @@ extern void ima_post_path_mknod(struct dentry *dentry);
 extern void ima_add_kexec_buffer(struct kimage *image);
 #endif
 
+#ifdef CONFIG_X86
+extern bool arch_ima_get_secureboot(void);
+#else
+static inline bool arch_ima_get_secureboot(void)
+{
+	return false;
+}
+#endif
+
 #else
 static inline int ima_bprm_check(struct linux_binprm *bprm)
 {
-- 
2.13.6


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

* [PATCH v3 2/6] ima: prevent kexec_load syscall based on runtime secureboot flag
  2018-09-19  7:55 [PATCH v3 0/6] Add support for architecture specific IMA policies Nayna Jain
  2018-09-19  7:55 ` [PATCH v3 1/6] x86/ima: define arch_ima_get_secureboot Nayna Jain
@ 2018-09-19  7:55 ` Nayna Jain
  2018-09-19  7:55 ` [PATCH v3 3/6] ima: refactor ima_init_policy() Nayna Jain
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Nayna Jain @ 2018-09-19  7:55 UTC (permalink / raw)
  To: linux-integrity
  Cc: zohar, linux-security-module, linux-efi, linux-kernel, dhowells,
	jforbes, Nayna Jain

When CONFIG_KEXEC_VERIFY_SIG is enabled, the kexec_file_load syscall
requires the kexec'd kernel image to be signed. Distros are concerned
about totally disabling the kexec_load syscall. As a compromise, the
kexec_load syscall will only be disabled when CONFIG_KEXEC_VERIFY_SIG
is configured and the system is booted with secureboot enabled.

This patch disables the kexec_load syscall only for systems booted with
secureboot enabled.

Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
---
 security/integrity/ima/ima_main.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index dce0a8a217bb..bdb6e5563d05 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -505,20 +505,24 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
  */
 int ima_load_data(enum kernel_load_data_id id)
 {
-	bool sig_enforce;
+	bool ima_enforce, sig_enforce;
 
-	if ((ima_appraise & IMA_APPRAISE_ENFORCE) != IMA_APPRAISE_ENFORCE)
-		return 0;
+	ima_enforce =
+		(ima_appraise & IMA_APPRAISE_ENFORCE) == IMA_APPRAISE_ENFORCE;
 
 	switch (id) {
 	case LOADING_KEXEC_IMAGE:
-		if (ima_appraise & IMA_APPRAISE_KEXEC) {
+#ifdef CONFIG_KEXEC_VERIFY_SIG
+		if (arch_ima_get_secureboot())
+			return -EACCES;
+#endif
+		if (ima_enforce && (ima_appraise & IMA_APPRAISE_KEXEC)) {
 			pr_err("impossible to appraise a kernel image without a file descriptor; try using kexec_file_load syscall.\n");
 			return -EACCES;	/* INTEGRITY_UNKNOWN */
 		}
 		break;
 	case LOADING_FIRMWARE:
-		if (ima_appraise & IMA_APPRAISE_FIRMWARE) {
+		if (ima_enforce && (ima_appraise & IMA_APPRAISE_FIRMWARE)) {
 			pr_err("Prevent firmware sysfs fallback loading.\n");
 			return -EACCES;	/* INTEGRITY_UNKNOWN */
 		}
@@ -526,7 +530,8 @@ int ima_load_data(enum kernel_load_data_id id)
 	case LOADING_MODULE:
 		sig_enforce = is_module_sig_enforced();
 
-		if (!sig_enforce && (ima_appraise & IMA_APPRAISE_MODULES)) {
+		if (ima_enforce && (!sig_enforce
+				    && (ima_appraise & IMA_APPRAISE_MODULES))) {
 			pr_err("impossible to appraise a module without a file descriptor. sig_enforce kernel parameter might help\n");
 			return -EACCES;	/* INTEGRITY_UNKNOWN */
 		}
-- 
2.13.6


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

* [PATCH v3 3/6] ima: refactor ima_init_policy()
  2018-09-19  7:55 [PATCH v3 0/6] Add support for architecture specific IMA policies Nayna Jain
  2018-09-19  7:55 ` [PATCH v3 1/6] x86/ima: define arch_ima_get_secureboot Nayna Jain
  2018-09-19  7:55 ` [PATCH v3 2/6] ima: prevent kexec_load syscall based on runtime secureboot flag Nayna Jain
@ 2018-09-19  7:55 ` Nayna Jain
  2018-09-21  8:34   ` Dan Carpenter
  2018-09-19  7:55 ` [PATCH v3 4/6] ima: add support for arch specific policies Nayna Jain
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Nayna Jain @ 2018-09-19  7:55 UTC (permalink / raw)
  To: linux-integrity
  Cc: zohar, linux-security-module, linux-efi, linux-kernel, dhowells,
	jforbes, Nayna Jain

This patch removes the code duplication in ima_init_policy() by defining
a new function named add_rules().

Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
---
 security/integrity/ima/ima_policy.c | 98 +++++++++++++++++++++----------------
 1 file changed, 57 insertions(+), 41 deletions(-)

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 8c9499867c91..b58906a05736 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -58,6 +58,8 @@ enum lsm_rule_types { LSM_OBJ_USER, LSM_OBJ_ROLE, LSM_OBJ_TYPE,
 
 enum policy_types { ORIGINAL_TCB = 1, DEFAULT_TCB };
 
+enum policy_rule_list { IMA_DEFAULT_POLICY = 1, IMA_CUSTOM_POLICY };
+
 struct ima_rule_entry {
 	struct list_head list;
 	int action;
@@ -473,6 +475,33 @@ static int ima_appraise_flag(enum ima_hooks func)
 	return 0;
 }
 
+static void add_rules(struct ima_rule_entry *entries, int count,
+		      enum policy_rule_list file)
+{
+	int i = 0;
+
+	for (i = 0; i < count; i++) {
+		struct ima_rule_entry *entry;
+
+		if (file && IMA_DEFAULT_POLICY)
+			list_add_tail(&entries[i].list, &ima_default_rules);
+
+		if (file && IMA_CUSTOM_POLICY) {
+			entry = kmemdup(&entries[i], sizeof(*entry),
+					GFP_KERNEL);
+			if (!entry)
+				continue;
+
+			INIT_LIST_HEAD(&entry->list);
+			list_add_tail(&entry->list, &ima_policy_rules);
+		}
+		if (entries[i].action == APPRAISE)
+			temp_ima_appraise |= ima_appraise_flag(entries[i].func);
+		if (entries[i].func == POLICY_CHECK)
+			temp_ima_appraise |= IMA_APPRAISE_POLICY;
+	}
+}
+
 /**
  * ima_init_policy - initialize the default measure rules.
  *
@@ -481,28 +510,23 @@ static int ima_appraise_flag(enum ima_hooks func)
  */
 void __init ima_init_policy(void)
 {
-	int i, measure_entries, appraise_entries, secure_boot_entries;
-
-	/* if !ima_policy set entries = 0 so we load NO default rules */
-	measure_entries = ima_policy ? ARRAY_SIZE(dont_measure_rules) : 0;
-	appraise_entries = ima_use_appraise_tcb ?
-			 ARRAY_SIZE(default_appraise_rules) : 0;
-	secure_boot_entries = ima_use_secure_boot ?
-			ARRAY_SIZE(secure_boot_rules) : 0;
+	int build_appraise_entries;
 
-	for (i = 0; i < measure_entries; i++)
-		list_add_tail(&dont_measure_rules[i].list, &ima_default_rules);
+	/* if !ima_policy, we load NO default rules */
+	if (ima_policy)
+		add_rules(dont_measure_rules, ARRAY_SIZE(dont_measure_rules),
+			  IMA_DEFAULT_POLICY);
 
 	switch (ima_policy) {
 	case ORIGINAL_TCB:
-		for (i = 0; i < ARRAY_SIZE(original_measurement_rules); i++)
-			list_add_tail(&original_measurement_rules[i].list,
-				      &ima_default_rules);
+		add_rules(original_measurement_rules,
+			  ARRAY_SIZE(original_measurement_rules),
+			  IMA_DEFAULT_POLICY);
 		break;
 	case DEFAULT_TCB:
-		for (i = 0; i < ARRAY_SIZE(default_measurement_rules); i++)
-			list_add_tail(&default_measurement_rules[i].list,
-				      &ima_default_rules);
+		add_rules(default_measurement_rules,
+			  ARRAY_SIZE(default_measurement_rules),
+			  IMA_DEFAULT_POLICY);
 	default:
 		break;
 	}
@@ -511,38 +535,30 @@ void __init ima_init_policy(void)
 	 * Insert the builtin "secure_boot" policy rules requiring file
 	 * signatures, prior to any other appraise rules.
 	 */
-	for (i = 0; i < secure_boot_entries; i++) {
-		list_add_tail(&secure_boot_rules[i].list, &ima_default_rules);
-		temp_ima_appraise |=
-		    ima_appraise_flag(secure_boot_rules[i].func);
-	}
+	if (ima_use_secure_boot)
+		add_rules(secure_boot_rules, ARRAY_SIZE(secure_boot_rules),
+			  IMA_DEFAULT_POLICY);
 
 	/*
 	 * Insert the build time appraise rules requiring file signatures
 	 * for both the initial and custom policies, prior to other appraise
-	 * rules.
+	 * rules. As the secure boot rules includes all of the build time
+	 * rules, include either one or the other set of rules, but not both.
 	 */
-	for (i = 0; i < ARRAY_SIZE(build_appraise_rules); i++) {
-		struct ima_rule_entry *entry;
-
-		if (!secure_boot_entries)
-			list_add_tail(&build_appraise_rules[i].list,
-				      &ima_default_rules);
-
-		entry = kmemdup(&build_appraise_rules[i], sizeof(*entry),
-				GFP_KERNEL);
-		if (entry)
-			list_add_tail(&entry->list, &ima_policy_rules);
-		build_ima_appraise |=
-			ima_appraise_flag(build_appraise_rules[i].func);
+	build_appraise_entries = ARRAY_SIZE(build_appraise_rules);
+	if (build_appraise_entries) {
+		if (ima_use_secure_boot)
+			add_rules(build_appraise_rules, build_appraise_entries,
+				  IMA_CUSTOM_POLICY);
+		else
+			add_rules(build_appraise_rules, build_appraise_entries,
+				  IMA_DEFAULT_POLICY | IMA_CUSTOM_POLICY);
 	}
 
-	for (i = 0; i < appraise_entries; i++) {
-		list_add_tail(&default_appraise_rules[i].list,
-			      &ima_default_rules);
-		if (default_appraise_rules[i].func == POLICY_CHECK)
-			temp_ima_appraise |= IMA_APPRAISE_POLICY;
-	}
+	if (ima_use_appraise_tcb)
+		add_rules(default_appraise_rules,
+			  ARRAY_SIZE(default_appraise_rules),
+			  IMA_DEFAULT_POLICY);
 
 	ima_rules = &ima_default_rules;
 	ima_update_policy_flag();
-- 
2.13.6


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

* [PATCH v3 4/6] ima: add support for arch specific policies
  2018-09-19  7:55 [PATCH v3 0/6] Add support for architecture specific IMA policies Nayna Jain
                   ` (2 preceding siblings ...)
  2018-09-19  7:55 ` [PATCH v3 3/6] ima: refactor ima_init_policy() Nayna Jain
@ 2018-09-19  7:55 ` Nayna Jain
  2018-09-19  7:55 ` [PATCH v3 5/6] ima: add support for external setting of ima_appraise Nayna Jain
  2018-09-19  7:55 ` [PATCH v3 6/6] x86/ima: define arch_get_ima_policy() for x86 Nayna Jain
  5 siblings, 0 replies; 9+ messages in thread
From: Nayna Jain @ 2018-09-19  7:55 UTC (permalink / raw)
  To: linux-integrity
  Cc: zohar, linux-security-module, linux-efi, linux-kernel, dhowells,
	jforbes, Nayna Jain

Builtin IMA policies can be enabled on the boot command line, and replaced
with a custom policy, normally during early boot in the initramfs. Build
time IMA policy rules were recently added. These rules are automatically
enabled on boot and persist after loading a custom policy.

There is a need for yet another type of policy, an architecture specific
policy, which is derived at runtime during kernel boot, based on the
runtime secure boot flags.  Like the build time policy rules, these rules
persist after loading a custom policy.

This patch adds support for loading an architecture specific IMA policy.

Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
- Defined function to convert the arch policy strings to an array of
ima_entry_rules.  The memory can then be freed after loading a custom
policy.
- Rename ima_get_arch_policy to arch_get_ima_policy.
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
- Modified ima_init_arch_policy() and ima_init_policy() to use add_rules()
from previous patch.
Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
---
 include/linux/ima.h                 |  5 +++
 security/integrity/ima/ima_policy.c | 70 +++++++++++++++++++++++++++++++++++--
 2 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/include/linux/ima.h b/include/linux/ima.h
index 4852255aa4f4..350fa957f8a6 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -39,6 +39,11 @@ static inline bool arch_ima_get_secureboot(void)
 }
 #endif
 
+static inline const char * const *arch_get_ima_policy(void)
+{
+	return NULL;
+}
+
 #else
 static inline int ima_bprm_check(struct linux_binprm *bprm)
 {
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index b58906a05736..23f3aa214016 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -20,6 +20,7 @@
 #include <linux/rculist.h>
 #include <linux/genhd.h>
 #include <linux/seq_file.h>
+#include <linux/ima.h>
 
 #include "ima.h"
 
@@ -195,6 +196,9 @@ static struct ima_rule_entry secure_boot_rules[] __ro_after_init = {
 	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
 };
 
+/* An array of architecture specific rules */
+struct ima_rule_entry *arch_policy_entry __ro_after_init;
+
 static LIST_HEAD(ima_default_rules);
 static LIST_HEAD(ima_policy_rules);
 static LIST_HEAD(ima_temp_rules);
@@ -492,7 +496,6 @@ static void add_rules(struct ima_rule_entry *entries, int count,
 			if (!entry)
 				continue;
 
-			INIT_LIST_HEAD(&entry->list);
 			list_add_tail(&entry->list, &ima_policy_rules);
 		}
 		if (entries[i].action == APPRAISE)
@@ -502,6 +505,48 @@ static void add_rules(struct ima_rule_entry *entries, int count,
 	}
 }
 
+static int ima_parse_rule(char *rule, struct ima_rule_entry *entry);
+
+static int __init ima_init_arch_policy(void)
+{
+	const char * const *arch_rules;
+	const char * const *rules;
+	int arch_entries = 0;
+	int i = 0;
+
+	arch_rules = arch_get_ima_policy();
+	if (!arch_rules)
+		return arch_entries;
+
+	/* Get number of rules */
+	for (rules = arch_rules; *rules != NULL; rules++)
+		arch_entries++;
+
+	arch_policy_entry = kcalloc(arch_entries + 1,
+				    sizeof(*arch_policy_entry), GFP_KERNEL);
+	if (!arch_policy_entry)
+		return 0;
+
+	/* Convert each policy string rules to struct ima_rule_entry format */
+	for (rules = arch_rules, i = 0; *rules != NULL; rules++) {
+		char rule[255];
+		int result;
+
+		result = strlcpy(rule, *rules, sizeof(rule));
+
+		INIT_LIST_HEAD(&arch_policy_entry[i].list);
+		result = ima_parse_rule(rule, &arch_policy_entry[i]);
+		if (result) {
+			pr_warn("Skipping unknown architecture policy rule: %s\n", rule);
+			memset(&arch_policy_entry[i], 0,
+			       sizeof(*arch_policy_entry));
+			continue;
+		}
+		i++;
+	}
+	return i;
+}
+
 /**
  * ima_init_policy - initialize the default measure rules.
  *
@@ -510,7 +555,7 @@ static void add_rules(struct ima_rule_entry *entries, int count,
  */
 void __init ima_init_policy(void)
 {
-	int build_appraise_entries;
+	int build_appraise_entries, arch_entries;
 
 	/* if !ima_policy, we load NO default rules */
 	if (ima_policy)
@@ -532,6 +577,19 @@ void __init ima_init_policy(void)
 	}
 
 	/*
+	 * Based on runtime secure boot flags, insert arch specific measurement
+	 * and appraise rules requiring file signatures for both the initial
+	 * and custom policies, prior to other appraise rules.
+	 * (Highest priority)
+	 */
+	arch_entries = ima_init_arch_policy();
+	if (!arch_entries)
+		pr_info("No architecture policies found\n");
+	else
+		add_rules(arch_policy_entry, arch_entries,
+			  IMA_DEFAULT_POLICY | IMA_CUSTOM_POLICY);
+
+	/*
 	 * Insert the builtin "secure_boot" policy rules requiring file
 	 * signatures, prior to any other appraise rules.
 	 */
@@ -592,6 +650,14 @@ void ima_update_policy(void)
 	if (ima_rules != policy) {
 		ima_policy_flag = 0;
 		ima_rules = policy;
+
+		/*
+		 * IMA architecture specific policy rules are specified
+		 * as strings and converted to an array of ima_entry_rules
+		 * on boot.  After loading a custom policy, free the
+		 * architecture specific rules stored as an array.
+		 */
+		kfree(arch_policy_entry);
 	}
 	ima_update_policy_flag();
 }
-- 
2.13.6


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

* [PATCH v3 5/6] ima: add support for external setting of ima_appraise
  2018-09-19  7:55 [PATCH v3 0/6] Add support for architecture specific IMA policies Nayna Jain
                   ` (3 preceding siblings ...)
  2018-09-19  7:55 ` [PATCH v3 4/6] ima: add support for arch specific policies Nayna Jain
@ 2018-09-19  7:55 ` Nayna Jain
  2018-09-19  7:55 ` [PATCH v3 6/6] x86/ima: define arch_get_ima_policy() for x86 Nayna Jain
  5 siblings, 0 replies; 9+ messages in thread
From: Nayna Jain @ 2018-09-19  7:55 UTC (permalink / raw)
  To: linux-integrity
  Cc: zohar, linux-security-module, linux-efi, linux-kernel, dhowells,
	jforbes, Nayna Jain

The "ima_appraise" mode defaults to enforcing, unless configured to allow
the boot command line "ima_appraise" option. This patch explicitly sets the
"ima_appraise" mode for the arch specific policy setting.

Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
---
 security/integrity/ima/ima.h          |  5 +++++
 security/integrity/ima/ima_appraise.c | 11 +++++++++--
 security/integrity/ima/ima_policy.c   |  5 ++++-
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 588e4813370c..6e5fa7c42809 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -248,6 +248,7 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
 				 int xattr_len);
 int ima_read_xattr(struct dentry *dentry,
 		   struct evm_ima_xattr_data **xattr_value);
+void set_ima_appraise(char *str);
 
 #else
 static inline int ima_appraise_measurement(enum ima_hooks func,
@@ -290,6 +291,10 @@ static inline int ima_read_xattr(struct dentry *dentry,
 	return 0;
 }
 
+static inline void set_ima_appraise(char *str)
+{
+}
+
 #endif /* CONFIG_IMA_APPRAISE */
 
 /* LSM based policy rules require audit */
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 8bd7a0733e51..e061613bcb87 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -18,15 +18,22 @@
 
 #include "ima.h"
 
-static int __init default_appraise_setup(char *str)
+void set_ima_appraise(char *str)
 {
-#ifdef CONFIG_IMA_APPRAISE_BOOTPARAM
 	if (strncmp(str, "off", 3) == 0)
 		ima_appraise = 0;
 	else if (strncmp(str, "log", 3) == 0)
 		ima_appraise = IMA_APPRAISE_LOG;
 	else if (strncmp(str, "fix", 3) == 0)
 		ima_appraise = IMA_APPRAISE_FIX;
+	else if (strncmp(str, "enforce", 7) == 0)
+		ima_appraise = IMA_APPRAISE_ENFORCE;
+}
+
+static int __init default_appraise_setup(char *str)
+{
+#ifdef CONFIG_IMA_APPRAISE_BOOTPARAM
+	set_ima_appraise(str);
 #endif
 	return 1;
 }
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 23f3aa214016..c574841bcc9a 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -585,9 +585,12 @@ void __init ima_init_policy(void)
 	arch_entries = ima_init_arch_policy();
 	if (!arch_entries)
 		pr_info("No architecture policies found\n");
-	else
+	else {
 		add_rules(arch_policy_entry, arch_entries,
 			  IMA_DEFAULT_POLICY | IMA_CUSTOM_POLICY);
+		if (temp_ima_appraise)
+			set_ima_appraise("enforce");
+	}
 
 	/*
 	 * Insert the builtin "secure_boot" policy rules requiring file
-- 
2.13.6


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

* [PATCH v3 6/6] x86/ima: define arch_get_ima_policy() for x86
  2018-09-19  7:55 [PATCH v3 0/6] Add support for architecture specific IMA policies Nayna Jain
                   ` (4 preceding siblings ...)
  2018-09-19  7:55 ` [PATCH v3 5/6] ima: add support for external setting of ima_appraise Nayna Jain
@ 2018-09-19  7:55 ` Nayna Jain
  5 siblings, 0 replies; 9+ messages in thread
From: Nayna Jain @ 2018-09-19  7:55 UTC (permalink / raw)
  To: linux-integrity
  Cc: zohar, linux-security-module, linux-efi, linux-kernel, dhowells,
	jforbes, Eric Richter, Nayna Jain

From: Eric Richter <erichte@linux.vnet.ibm.com>

This patch implements an example arch-specific IMA policy for x86 to
enable measurement and appraisal of any kernel image loaded for kexec,
when CONFIG_KEXEC_VERIFY_SIG is not enabled.

For systems with CONFIG_KEXEC_VERIFY_SIG enabled, only the measurement
rule is enabled, not the IMA-appraisal rule.

Signed-off-by: Eric Richter <erichte@linux.vnet.ibm.com>
- Removed the policy KEXEC_ORIG_KERNEL_CHECK which was defined to
  disable the kexec_load syscall.
- arch_get_ima_policy() uses arch_ima_get_secureboot() to get secureboot
  state
Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
---
 arch/x86/kernel/ima_arch.c     | 18 ++++++++++++++++++
 include/linux/ima.h            |  4 ++++
 security/integrity/ima/Kconfig |  8 ++++++++
 3 files changed, 30 insertions(+)

diff --git a/arch/x86/kernel/ima_arch.c b/arch/x86/kernel/ima_arch.c
index bb5a88d2b271..245976e49a55 100644
--- a/arch/x86/kernel/ima_arch.c
+++ b/arch/x86/kernel/ima_arch.c
@@ -15,3 +15,21 @@ bool arch_ima_get_secureboot(void)
 	else
 		return false;
 }
+
+/* arch rules for audit and user mode */
+static const char * const sb_arch_rules[] = {
+#ifndef CONFIG_KEXEC_VERIFY_SIG
+	"appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig",
+#endif /* CONFIG_KEXEC_VERIFY_SIG */
+	"measure func=KEXEC_KERNEL_CHECK",
+	NULL
+};
+
+#ifdef CONFIG_IMA_ARCH_POLICY
+const char * const *arch_get_ima_policy(void)
+{
+	if (arch_ima_get_secureboot())
+		return sb_arch_rules;
+	return NULL;
+}
+#endif
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 350fa957f8a6..dabd3abdf671 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -39,10 +39,14 @@ static inline bool arch_ima_get_secureboot(void)
 }
 #endif
 
+#if defined(CONFIG_X86) && defined(CONFIG_IMA_ARCH_POLICY)
+extern const char * const *arch_get_ima_policy(void);
+#else
 static inline const char * const *arch_get_ima_policy(void)
 {
 	return NULL;
 }
+#endif
 
 #else
 static inline int ima_bprm_check(struct linux_binprm *bprm)
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 13b446328dda..97609a76aa14 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -157,6 +157,14 @@ config IMA_APPRAISE
 	  <http://linux-ima.sourceforge.net>
 	  If unsure, say N.
 
+config IMA_ARCH_POLICY
+        bool "Enable loading an IMA architecture specific policy"
+        depends on KEXEC_VERIFY_SIG || IMA_APPRAISE && INTEGRITY_ASYMMETRIC_KEYS
+        default n
+        help
+          This option enables loading an IMA architecture specific policy
+          based on run time secure boot flags.
+
 config IMA_APPRAISE_BUILD_POLICY
 	bool "IMA build time configured policy rules"
 	depends on IMA_APPRAISE && INTEGRITY_ASYMMETRIC_KEYS
-- 
2.13.6


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

* Re: [PATCH v3 3/6] ima: refactor ima_init_policy()
  2018-09-19  7:55 ` [PATCH v3 3/6] ima: refactor ima_init_policy() Nayna Jain
@ 2018-09-21  8:34   ` Dan Carpenter
  2018-09-24 11:10     ` Nayna Jain
  0 siblings, 1 reply; 9+ messages in thread
From: Dan Carpenter @ 2018-09-21  8:34 UTC (permalink / raw)
  To: kbuild, Nayna Jain
  Cc: kbuild-all, linux-integrity, zohar, linux-security-module,
	linux-efi, linux-kernel, dhowells, jforbes, Nayna Jain

Hi Nayna,

Thank you for the patch! Perhaps something to improve:

url:    https://github.com/0day-ci/linux/commits/Nayna-Jain/Add-support-for-architecture-specific-IMA-policies/20180920-035110

smatch warnings:
security/integrity/ima/ima_policy.c:489 add_rules() warn: should this be a bitwise op?

# https://github.com/0day-ci/linux/commit/84a2e186f940ebc6c34e6d276e55f665167a5bb8
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 84a2e186f940ebc6c34e6d276e55f665167a5bb8
vim +489 security/integrity/ima/ima_policy.c

6f0911a6 Mimi Zohar 2018-04-12  477  
84a2e186 Nayna Jain 2018-09-19  478  static void add_rules(struct ima_rule_entry *entries, int count,
84a2e186 Nayna Jain 2018-09-19  479  		      enum policy_rule_list file)
84a2e186 Nayna Jain 2018-09-19  480  {
84a2e186 Nayna Jain 2018-09-19  481  	int i = 0;
84a2e186 Nayna Jain 2018-09-19  482  
84a2e186 Nayna Jain 2018-09-19  483  	for (i = 0; i < count; i++) {
84a2e186 Nayna Jain 2018-09-19  484  		struct ima_rule_entry *entry;
84a2e186 Nayna Jain 2018-09-19  485  
84a2e186 Nayna Jain 2018-09-19  486  		if (file && IMA_DEFAULT_POLICY)
                                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^
84a2e186 Nayna Jain 2018-09-19  487  			list_add_tail(&entries[i].list, &ima_default_rules);
84a2e186 Nayna Jain 2018-09-19  488  
84a2e186 Nayna Jain 2018-09-19 @489  		if (file && IMA_CUSTOM_POLICY) {
                                                    ^^^^^^^^^^^^^^^^^^^^^^^^^

It does look like it should be "if (file & IMA_CUSTOM_POLICY) {" but I
haven't looked at the context besides what's here in this email.

84a2e186 Nayna Jain 2018-09-19  490  			entry = kmemdup(&entries[i], sizeof(*entry),
84a2e186 Nayna Jain 2018-09-19  491  					GFP_KERNEL);
84a2e186 Nayna Jain 2018-09-19  492  			if (!entry)
84a2e186 Nayna Jain 2018-09-19  493  				continue;
84a2e186 Nayna Jain 2018-09-19  494  
84a2e186 Nayna Jain 2018-09-19  495  			INIT_LIST_HEAD(&entry->list);
84a2e186 Nayna Jain 2018-09-19  496  			list_add_tail(&entry->list, &ima_policy_rules);
84a2e186 Nayna Jain 2018-09-19  497  		}
84a2e186 Nayna Jain 2018-09-19  498  		if (entries[i].action == APPRAISE)
84a2e186 Nayna Jain 2018-09-19  499  			temp_ima_appraise |= ima_appraise_flag(entries[i].func);
84a2e186 Nayna Jain 2018-09-19  500  		if (entries[i].func == POLICY_CHECK)
84a2e186 Nayna Jain 2018-09-19  501  			temp_ima_appraise |= IMA_APPRAISE_POLICY;
84a2e186 Nayna Jain 2018-09-19  502  	}
84a2e186 Nayna Jain 2018-09-19  503  }
84a2e186 Nayna Jain 2018-09-19  504  

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

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

* Re: [PATCH v3 3/6] ima: refactor ima_init_policy()
  2018-09-21  8:34   ` Dan Carpenter
@ 2018-09-24 11:10     ` Nayna Jain
  0 siblings, 0 replies; 9+ messages in thread
From: Nayna Jain @ 2018-09-24 11:10 UTC (permalink / raw)
  To: Dan Carpenter, kbuild
  Cc: kbuild-all, linux-integrity, zohar, linux-security-module,
	linux-efi, linux-kernel, dhowells, jforbes



On 09/21/2018 02:04 PM, Dan Carpenter wrote:
> Hi Nayna,
>
> Thank you for the patch! Perhaps something to improve:
>
> url:    https://github.com/0day-ci/linux/commits/Nayna-Jain/Add-support-for-architecture-specific-IMA-policies/20180920-035110
>
> smatch warnings:
> security/integrity/ima/ima_policy.c:489 add_rules() warn: should this be a bitwise op?
>
> # https://github.com/0day-ci/linux/commit/84a2e186f940ebc6c34e6d276e55f665167a5bb8
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout 84a2e186f940ebc6c34e6d276e55f665167a5bb8
> vim +489 security/integrity/ima/ima_policy.c
>
> 6f0911a6 Mimi Zohar 2018-04-12  477
> 84a2e186 Nayna Jain 2018-09-19  478  static void add_rules(struct ima_rule_entry *entries, int count,
> 84a2e186 Nayna Jain 2018-09-19  479  		      enum policy_rule_list file)
> 84a2e186 Nayna Jain 2018-09-19  480  {
> 84a2e186 Nayna Jain 2018-09-19  481  	int i = 0;
> 84a2e186 Nayna Jain 2018-09-19  482
> 84a2e186 Nayna Jain 2018-09-19  483  	for (i = 0; i < count; i++) {
> 84a2e186 Nayna Jain 2018-09-19  484  		struct ima_rule_entry *entry;
> 84a2e186 Nayna Jain 2018-09-19  485
> 84a2e186 Nayna Jain 2018-09-19  486  		if (file && IMA_DEFAULT_POLICY)
>                                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^
> 84a2e186 Nayna Jain 2018-09-19  487  			list_add_tail(&entries[i].list, &ima_default_rules);
> 84a2e186 Nayna Jain 2018-09-19  488
> 84a2e186 Nayna Jain 2018-09-19 @489  		if (file && IMA_CUSTOM_POLICY) {
>                                                      ^^^^^^^^^^^^^^^^^^^^^^^^^
>
> It does look like it should be "if (file & IMA_CUSTOM_POLICY) {" but I
> haven't looked at the context besides what's here in this email.
Thanks Dan for noticing this. Yes, I will fix it and post the v4 version.

Thanks & Regards,
     - Nayna


>
> 84a2e186 Nayna Jain 2018-09-19  490  			entry = kmemdup(&entries[i], sizeof(*entry),
> 84a2e186 Nayna Jain 2018-09-19  491  					GFP_KERNEL);
> 84a2e186 Nayna Jain 2018-09-19  492  			if (!entry)
> 84a2e186 Nayna Jain 2018-09-19  493  				continue;
> 84a2e186 Nayna Jain 2018-09-19  494
> 84a2e186 Nayna Jain 2018-09-19  495  			INIT_LIST_HEAD(&entry->list);
> 84a2e186 Nayna Jain 2018-09-19  496  			list_add_tail(&entry->list, &ima_policy_rules);
> 84a2e186 Nayna Jain 2018-09-19  497  		}
> 84a2e186 Nayna Jain 2018-09-19  498  		if (entries[i].action == APPRAISE)
> 84a2e186 Nayna Jain 2018-09-19  499  			temp_ima_appraise |= ima_appraise_flag(entries[i].func);
> 84a2e186 Nayna Jain 2018-09-19  500  		if (entries[i].func == POLICY_CHECK)
> 84a2e186 Nayna Jain 2018-09-19  501  			temp_ima_appraise |= IMA_APPRAISE_POLICY;
> 84a2e186 Nayna Jain 2018-09-19  502  	}
> 84a2e186 Nayna Jain 2018-09-19  503  }
> 84a2e186 Nayna Jain 2018-09-19  504
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
>


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

end of thread, other threads:[~2018-09-24 11:13 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-19  7:55 [PATCH v3 0/6] Add support for architecture specific IMA policies Nayna Jain
2018-09-19  7:55 ` [PATCH v3 1/6] x86/ima: define arch_ima_get_secureboot Nayna Jain
2018-09-19  7:55 ` [PATCH v3 2/6] ima: prevent kexec_load syscall based on runtime secureboot flag Nayna Jain
2018-09-19  7:55 ` [PATCH v3 3/6] ima: refactor ima_init_policy() Nayna Jain
2018-09-21  8:34   ` Dan Carpenter
2018-09-24 11:10     ` Nayna Jain
2018-09-19  7:55 ` [PATCH v3 4/6] ima: add support for arch specific policies Nayna Jain
2018-09-19  7:55 ` [PATCH v3 5/6] ima: add support for external setting of ima_appraise Nayna Jain
2018-09-19  7:55 ` [PATCH v3 6/6] x86/ima: define arch_get_ima_policy() for x86 Nayna Jain

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).