LinuxPPC-Dev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v7 0/8] powerpc: Enabling IMA arch specific secure boot policies
@ 2019-10-08  1:14 Nayna Jain
  2019-10-08  1:14 ` [PATCH v7 1/8] powerpc: detect the secure boot mode of the system Nayna Jain
                   ` (7 more replies)
  0 siblings, 8 replies; 14+ messages in thread
From: Nayna Jain @ 2019-10-08  1:14 UTC (permalink / raw)
  To: linuxppc-dev, linux-efi, linux-integrity
  Cc: Ard Biesheuvel, Eric Ricther, Nayna Jain, linux-kernel,
	Mimi Zohar, Claudio Carvalho, Matthew Garret, Greg Kroah-Hartman,
	Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, George Wilson

This patchset extends the previous version of the patchset[1] by adding
the support for checking against the binary blacklisted hashes.

IMA subsystem supports custom, built-in, arch-specific policies to define
the files to be measured and appraised. These policies are honored based
on the priority where arch-specific policies is the highest and custom
is the lowest.

PowerNV systems uses the linux based bootloader and kexec the Host OS.
It rely on IMA for signature verification of the kernel before doing the
kexec. This patchset adds support for powerpc arch specific ima policies
that are defined based on system's OS secureboot and trustedboot state.
The OS secureboot and trustedboot state are determined via device-tree
properties.

The verification needs to be done only for the binaries which are not
blacklisted. The kernel currently checks against the blacklisted keys.
However that results in blacklisting all the binaries that are signed by
that key. In order to prevent single binary from loading, it is required
to support checking against blacklisting of the binary hash. This patchset
adds the support in IMA to check against blacklisted hashes for the files
signed by appended signature.

[1] http://patchwork.ozlabs.org/cover/1149262/ 

Changelog:
v7:
* Removes patch related to dt-bindings as per input from Rob Herring. 
* fixes Patch 1/8 to use new device-tree updates as per Oliver
  feedback to device-tree documentation in skiboot mailing list.
(https://lists.ozlabs.org/pipermail/skiboot/2019-September/015329.html)
* Includes feedbacks from Mimi, Thiago
  * moves function get_powerpc_fw_sb_node() from Patch 1 to Patch 3 
  * fixes Patch 2/8 to use CONFIG_MODULE_SIG_FORCE.
  * updates Patch description in Patch 5/8
  * adds a new patch to add wrapper is_binary_blacklisted()
  * removes the patch that deprecated permit_directio

v6:
* includes feedbacks from Michael Ellerman on the patchset v5
  * removed email ids from comments
  * add the doc for the device-tree
  * renames the secboot.c to secure_boot.c and secboot.h to secure_boot.h
  * other code specific fixes
* split the patches to differentiate between secureboot and trustedboot
state of the system
* adds the patches to support the blacklisting of the binary hash.

v5:
* secureboot state is now read via device tree entry rather than OPAL
secure variables
* ima arch policies are updated to use policy based template for
measurement rules

v4:
* Fixed the build issue as reported by Satheesh Rajendran.

v3:
* OPAL APIs in Patch 1 are updated to provide generic interface based on
key/keylen. This patchset updates kernel OPAL APIs to be compatible with
generic interface.
* Patch 2 is cleaned up to use new OPAL APIs.
* Since OPAL can support different types of backend which can vary in the
variable interpretation, the Patch 2 is updated to add a check for the
backend version
* OPAL API now expects consumer to first check the supported backend version
before calling other secvar OPAL APIs. This check is now added in patch 2.
* IMA policies in Patch 3 is updated to specify appended signature and
per policy template.
* The patches now are free of any EFIisms.

v2:

* Removed Patch 1: powerpc/include: Override unneeded early ioremap
functions
* Updated Subject line and patch description of the Patch 1 of this series
* Removed dependency of OPAL_SECVAR on EFI, CPU_BIG_ENDIAN and UCS2_STRING
* Changed OPAL APIs from static to non-static. Added opal-secvar.h for the
same
* Removed EFI hooks from opal_secvar.c
* Removed opal_secvar_get_next(), opal_secvar_enqueue() and
opal_query_variable_info() function
* get_powerpc_sb_mode() in secboot.c now directly calls OPAL Runtime API
rather than via EFI hooks.
* Fixed log messages in get_powerpc_sb_mode() function.
* Added dependency for PPC_SECURE_BOOT on configs PPC64 and OPAL_SECVAR
* Replaced obj-$(CONFIG_IMA) with obj-$(CONFIG_PPC_SECURE_BOOT) in
arch/powerpc/kernel/Makefile
*** BLURB HERE ***

Nayna Jain (8):
  powerpc: detect the secure boot mode of the system
  powerpc: add support to initialize ima policy rules
  powerpc: detect the trusted boot state of the system
  powerpc/ima: add measurement rules to ima arch specific policy
  ima: make process_buffer_measurement() generic
  certs: add wrapper function to check blacklisted binary hash
  ima: check against blacklisted hashes for files with modsig
  powerpc/ima: update ima arch policy to check for blacklist

 Documentation/ABI/testing/ima_policy   |  1 +
 arch/powerpc/Kconfig                   | 12 ++++
 arch/powerpc/include/asm/secure_boot.h | 35 ++++++++++++
 arch/powerpc/kernel/Makefile           |  2 +
 arch/powerpc/kernel/ima_arch.c         | 72 ++++++++++++++++++++++++
 arch/powerpc/kernel/secure_boot.c      | 77 ++++++++++++++++++++++++++
 certs/blacklist.c                      |  9 +++
 include/keys/system_keyring.h          |  6 ++
 include/linux/ima.h                    |  3 +-
 security/integrity/ima/ima.h           | 12 ++++
 security/integrity/ima/ima_appraise.c  | 39 +++++++++++++
 security/integrity/ima/ima_main.c      | 41 +++++++-------
 security/integrity/ima/ima_policy.c    | 10 +++-
 security/integrity/integrity.h         |  1 +
 14 files changed, 298 insertions(+), 22 deletions(-)
 create mode 100644 arch/powerpc/include/asm/secure_boot.h
 create mode 100644 arch/powerpc/kernel/ima_arch.c
 create mode 100644 arch/powerpc/kernel/secure_boot.c

-- 
2.20.1


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

* [PATCH v7 1/8] powerpc: detect the secure boot mode of the system
  2019-10-08  1:14 [PATCH v7 0/8] powerpc: Enabling IMA arch specific secure boot policies Nayna Jain
@ 2019-10-08  1:14 ` Nayna Jain
  2019-10-08  1:14 ` [PATCH v7 2/8] powerpc: add support to initialize ima policy rules Nayna Jain
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Nayna Jain @ 2019-10-08  1:14 UTC (permalink / raw)
  To: linuxppc-dev, linux-efi, linux-integrity
  Cc: Ard Biesheuvel, Eric Ricther, Nayna Jain, linux-kernel,
	Mimi Zohar, Claudio Carvalho, Matthew Garret, Greg Kroah-Hartman,
	Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, George Wilson

Secure boot on PowerNV defines different IMA policies based on the secure
boot state of the system.

This patch defines a function to detect the secure boot state of the
system.

The PPC_SECURE_BOOT config represents the base enablement of secureboot
on POWER.

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
 arch/powerpc/Kconfig                   | 10 ++++++
 arch/powerpc/include/asm/secure_boot.h | 29 ++++++++++++++++++
 arch/powerpc/kernel/Makefile           |  2 ++
 arch/powerpc/kernel/secure_boot.c      | 42 ++++++++++++++++++++++++++
 4 files changed, 83 insertions(+)
 create mode 100644 arch/powerpc/include/asm/secure_boot.h
 create mode 100644 arch/powerpc/kernel/secure_boot.c

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 3e56c9c2f16e..b4a221886fcf 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -934,6 +934,16 @@ config PPC_MEM_KEYS
 
 	  If unsure, say y.
 
+config PPC_SECURE_BOOT
+	prompt "Enable secure boot support"
+	bool
+	depends on PPC_POWERNV
+	help
+	  Systems with firmware secure boot enabled needs to define security
+	  policies to extend secure boot to the OS. This config allows user
+	  to enable OS secure boot on systems that have firmware support for
+	  it. If in doubt say N.
+
 endmenu
 
 config ISA_DMA_API
diff --git a/arch/powerpc/include/asm/secure_boot.h b/arch/powerpc/include/asm/secure_boot.h
new file mode 100644
index 000000000000..23d2ef2f1f7b
--- /dev/null
+++ b/arch/powerpc/include/asm/secure_boot.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Secure boot definitions
+ *
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Nayna Jain
+ */
+#ifndef _ASM_POWER_SECURE_BOOT_H
+#define _ASM_POWER_SECURE_BOOT_H
+
+#ifdef CONFIG_PPC_SECURE_BOOT
+
+bool is_powerpc_os_secureboot_enabled(void);
+struct device_node *get_powerpc_os_sb_node(void);
+
+#else
+
+static inline bool is_powerpc_os_secureboot_enabled(void)
+{
+	return false;
+}
+
+static inline struct device_node *get_powerpc_os_sb_node(void)
+{
+	return NULL;
+}
+
+#endif
+#endif
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index a7ca8fe62368..e2a54fa240ac 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -161,6 +161,8 @@ ifneq ($(CONFIG_PPC_POWERNV)$(CONFIG_PPC_SVM),)
 obj-y				+= ucall.o
 endif
 
+obj-$(CONFIG_PPC_SECURE_BOOT)	+= secure_boot.o
+
 # Disable GCOV, KCOV & sanitizers in odd or sensitive code
 GCOV_PROFILE_prom_init.o := n
 KCOV_INSTRUMENT_prom_init.o := n
diff --git a/arch/powerpc/kernel/secure_boot.c b/arch/powerpc/kernel/secure_boot.c
new file mode 100644
index 000000000000..0488dbcab6b9
--- /dev/null
+++ b/arch/powerpc/kernel/secure_boot.c
@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Nayna Jain
+ */
+#include <linux/types.h>
+#include <linux/of.h>
+#include <asm/secure_boot.h>
+
+struct device_node *get_powerpc_os_sb_node(void)
+{
+	return of_find_compatible_node(NULL, NULL, "ibm,secvar-v1");
+}
+
+bool is_powerpc_os_secureboot_enabled(void)
+{
+	struct device_node *node;
+
+	node = get_powerpc_os_sb_node();
+	if (!node)
+		goto disabled;
+
+	if (!of_device_is_available(node)) {
+		pr_err("Secure variables support is in error state, fail secure\n");
+		goto enabled;
+	}
+
+	/*
+	 * secureboot is enabled if os-secure-enforcing property exists,
+	 * else disabled.
+	 */
+	if (!of_find_property(node, "os-secure-enforcing", NULL))
+		goto disabled;
+
+enabled:
+	pr_info("secureboot mode enabled\n");
+	return true;
+
+disabled:
+	pr_info("secureboot mode disabled\n");
+	return false;
+}
-- 
2.20.1


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

* [PATCH v7 2/8] powerpc: add support to initialize ima policy rules
  2019-10-08  1:14 [PATCH v7 0/8] powerpc: Enabling IMA arch specific secure boot policies Nayna Jain
  2019-10-08  1:14 ` [PATCH v7 1/8] powerpc: detect the secure boot mode of the system Nayna Jain
@ 2019-10-08  1:14 ` Nayna Jain
  2019-10-11 13:12   ` Mimi Zohar
  2019-10-08  1:14 ` [PATCH v7 3/8] powerpc: detect the trusted boot state of the system Nayna Jain
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Nayna Jain @ 2019-10-08  1:14 UTC (permalink / raw)
  To: linuxppc-dev, linux-efi, linux-integrity
  Cc: Ard Biesheuvel, Eric Ricther, Nayna Jain, linux-kernel,
	Mimi Zohar, Claudio Carvalho, Matthew Garret, Greg Kroah-Hartman,
	Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, George Wilson

PowerNV systems uses kernel based bootloader, thus its secure boot
implementation uses kernel IMA security subsystem to verify the kernel
before kexec. Since the verification policy might differ based on the
secure boot mode of the system, the policies are defined at runtime.

This patch implements the arch-specific support to define the IMA policy
rules based on the runtime secure boot mode of the system.

This patch provides arch-specific IMA policies if PPC_SECURE_BOOT
config is enabled.

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
 arch/powerpc/Kconfig           |  2 ++
 arch/powerpc/kernel/Makefile   |  2 +-
 arch/powerpc/kernel/ima_arch.c | 33 +++++++++++++++++++++++++++++++++
 include/linux/ima.h            |  3 ++-
 4 files changed, 38 insertions(+), 2 deletions(-)
 create mode 100644 arch/powerpc/kernel/ima_arch.c

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index b4a221886fcf..deb19ec6ba3d 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -938,6 +938,8 @@ config PPC_SECURE_BOOT
 	prompt "Enable secure boot support"
 	bool
 	depends on PPC_POWERNV
+	depends on IMA
+	depends on IMA_ARCH_POLICY
 	help
 	  Systems with firmware secure boot enabled needs to define security
 	  policies to extend secure boot to the OS. This config allows user
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index e2a54fa240ac..e8eb2955b7d5 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -161,7 +161,7 @@ ifneq ($(CONFIG_PPC_POWERNV)$(CONFIG_PPC_SVM),)
 obj-y				+= ucall.o
 endif
 
-obj-$(CONFIG_PPC_SECURE_BOOT)	+= secure_boot.o
+obj-$(CONFIG_PPC_SECURE_BOOT)	+= secure_boot.o ima_arch.o
 
 # Disable GCOV, KCOV & sanitizers in odd or sensitive code
 GCOV_PROFILE_prom_init.o := n
diff --git a/arch/powerpc/kernel/ima_arch.c b/arch/powerpc/kernel/ima_arch.c
new file mode 100644
index 000000000000..c22d82965eb4
--- /dev/null
+++ b/arch/powerpc/kernel/ima_arch.c
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Nayna Jain
+ */
+
+#include <linux/ima.h>
+#include <asm/secure_boot.h>
+
+bool arch_ima_get_secureboot(void)
+{
+	return is_powerpc_os_secureboot_enabled();
+}
+
+/* Defines IMA appraise rules for secureboot */
+static const char *const arch_rules[] = {
+	"appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig",
+#if !IS_ENABLED(CONFIG_MODULE_SIG_FORCE)
+	"appraise func=MODULE_CHECK appraise_type=imasig|modsig",
+#endif
+	NULL
+};
+
+/*
+ * Returns the relevant IMA arch policies based on the system secureboot state.
+ */
+const char *const *arch_get_ima_policy(void)
+{
+	if (is_powerpc_os_secureboot_enabled())
+		return arch_rules;
+
+	return NULL;
+}
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 1c37f17f7203..6d904754d858 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -29,7 +29,8 @@ extern void ima_kexec_cmdline(const void *buf, int size);
 extern void ima_add_kexec_buffer(struct kimage *image);
 #endif
 
-#if (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390)
+#if (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390) \
+	|| defined(CONFIG_PPC_SECURE_BOOT)
 extern bool arch_ima_get_secureboot(void);
 extern const char * const *arch_get_ima_policy(void);
 #else
-- 
2.20.1


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

* [PATCH v7 3/8] powerpc: detect the trusted boot state of the system
  2019-10-08  1:14 [PATCH v7 0/8] powerpc: Enabling IMA arch specific secure boot policies Nayna Jain
  2019-10-08  1:14 ` [PATCH v7 1/8] powerpc: detect the secure boot mode of the system Nayna Jain
  2019-10-08  1:14 ` [PATCH v7 2/8] powerpc: add support to initialize ima policy rules Nayna Jain
@ 2019-10-08  1:14 ` Nayna Jain
  2019-10-08  1:14 ` [PATCH v7 4/8] powerpc/ima: add measurement rules to ima arch specific policy Nayna Jain
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Nayna Jain @ 2019-10-08  1:14 UTC (permalink / raw)
  To: linuxppc-dev, linux-efi, linux-integrity
  Cc: Ard Biesheuvel, Eric Ricther, Nayna Jain, linux-kernel,
	Mimi Zohar, Claudio Carvalho, Matthew Garret, Greg Kroah-Hartman,
	Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, George Wilson

PowerNV systems enables the IMA measurement rules only if the
trusted boot is enabled on the system.

This patch adds the function to detect if the system has trusted
boot enabled.

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
 arch/powerpc/include/asm/secure_boot.h |  6 +++++
 arch/powerpc/kernel/secure_boot.c      | 35 ++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/arch/powerpc/include/asm/secure_boot.h b/arch/powerpc/include/asm/secure_boot.h
index 23d2ef2f1f7b..ecd08515e301 100644
--- a/arch/powerpc/include/asm/secure_boot.h
+++ b/arch/powerpc/include/asm/secure_boot.h
@@ -12,6 +12,7 @@
 
 bool is_powerpc_os_secureboot_enabled(void);
 struct device_node *get_powerpc_os_sb_node(void);
+bool is_powerpc_trustedboot_enabled(void);
 
 #else
 
@@ -25,5 +26,10 @@ static inline struct device_node *get_powerpc_os_sb_node(void)
 	return NULL;
 }
 
+static inline bool is_powerpc_os_trustedboot_enabled(void)
+{
+	return false;
+}
+
 #endif
 #endif
diff --git a/arch/powerpc/kernel/secure_boot.c b/arch/powerpc/kernel/secure_boot.c
index 0488dbcab6b9..9d5ac1b39e46 100644
--- a/arch/powerpc/kernel/secure_boot.c
+++ b/arch/powerpc/kernel/secure_boot.c
@@ -7,6 +7,27 @@
 #include <linux/of.h>
 #include <asm/secure_boot.h>
 
+static const char * const fwsecureboot_compat[] = {
+	"ibm,secureboot-v1",
+	"ibm,secureboot-v2",
+	NULL,
+};
+
+static struct device_node *get_powerpc_fw_sb_node(void)
+{
+	struct device_node *node;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(fwsecureboot_compat); ++i) {
+		node = of_find_compatible_node(NULL, NULL,
+					       fwsecureboot_compat[i]);
+		if (node)
+			return node;
+	}
+
+	return NULL;
+}
+
 struct device_node *get_powerpc_os_sb_node(void)
 {
 	return of_find_compatible_node(NULL, NULL, "ibm,secvar-v1");
@@ -40,3 +61,17 @@ bool is_powerpc_os_secureboot_enabled(void)
 	pr_info("secureboot mode disabled\n");
 	return false;
 }
+
+bool is_powerpc_trustedboot_enabled(void)
+{
+	struct device_node *node;
+
+	node = get_powerpc_fw_sb_node();
+	if (node && (of_find_property(node, "trusted-enabled", NULL))) {
+		pr_info("trustedboot mode enabled\n");
+		return true;
+	}
+
+	pr_info("trustedboot mode disabled\n");
+	return false;
+}
-- 
2.20.1


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

* [PATCH v7 4/8] powerpc/ima: add measurement rules to ima arch specific policy
  2019-10-08  1:14 [PATCH v7 0/8] powerpc: Enabling IMA arch specific secure boot policies Nayna Jain
                   ` (2 preceding siblings ...)
  2019-10-08  1:14 ` [PATCH v7 3/8] powerpc: detect the trusted boot state of the system Nayna Jain
@ 2019-10-08  1:14 ` Nayna Jain
  2019-10-08  1:14 ` [PATCH v7 5/8] ima: make process_buffer_measurement() generic Nayna Jain
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Nayna Jain @ 2019-10-08  1:14 UTC (permalink / raw)
  To: linuxppc-dev, linux-efi, linux-integrity
  Cc: Ard Biesheuvel, Eric Ricther, Nayna Jain, linux-kernel,
	Mimi Zohar, Claudio Carvalho, Matthew Garret, Greg Kroah-Hartman,
	Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, George Wilson

This patch adds the measurement rules to the arch specific policies on
trusted boot enabled systems.

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
 arch/powerpc/kernel/ima_arch.c | 45 +++++++++++++++++++++++++++++++---
 1 file changed, 42 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/ima_arch.c b/arch/powerpc/kernel/ima_arch.c
index c22d82965eb4..88bfe4a1a9a5 100644
--- a/arch/powerpc/kernel/ima_arch.c
+++ b/arch/powerpc/kernel/ima_arch.c
@@ -12,8 +12,19 @@ bool arch_ima_get_secureboot(void)
 	return is_powerpc_os_secureboot_enabled();
 }
 
-/* Defines IMA appraise rules for secureboot */
+/*
+ * The "arch_rules" contains both the securebot and trustedboot rules for adding
+ * the kexec kernel image and kernel modules file hashes to the IMA measurement
+ * list and verifying the file signatures against known good values.
+ *
+ * The "appraise_type=imasig|modsig" option allows the good signature to be
+ * stored as an xattr or as an appended signature. The "template=ima-modsig"
+ * option includes the appended signature, when available, in the IMA
+ * measurement list.
+ */
 static const char *const arch_rules[] = {
+	"measure func=KEXEC_KERNEL_CHECK template=ima-modsig",
+	"measure func=MODULE_CHECK template=ima-modsig",
 	"appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig",
 #if !IS_ENABLED(CONFIG_MODULE_SIG_FORCE)
 	"appraise func=MODULE_CHECK appraise_type=imasig|modsig",
@@ -22,12 +33,40 @@ static const char *const arch_rules[] = {
 };
 
 /*
- * Returns the relevant IMA arch policies based on the system secureboot state.
+ * The "measure_rules" are enabled only on "trustedboot" enabled systems.
+ * These rules add the kexec kernel image and kernel modules file hashes to
+ * the IMA measurement list.
+ */
+static const char *const measure_rules[] = {
+	"measure func=KEXEC_KERNEL_CHECK",
+	"measure func=MODULE_CHECK",
+	NULL
+};
+
+/*
+ * Returns the relevant IMA arch policies based on the system secureboot
+ * and trustedboot state.
  */
 const char *const *arch_get_ima_policy(void)
 {
-	if (is_powerpc_os_secureboot_enabled())
+	const char *const *rules;
+	int offset = 0;
+
+	for (rules = arch_rules; *rules != NULL; rules++) {
+		if (strncmp(*rules, "appraise", 8) == 0)
+			break;
+		offset++;
+	}
+
+	if (is_powerpc_os_secureboot_enabled()
+	    && is_powerpc_trustedboot_enabled())
 		return arch_rules;
 
+	if (is_powerpc_os_secureboot_enabled())
+		return arch_rules + offset;
+
+	if (is_powerpc_trustedboot_enabled())
+		return measure_rules;
+
 	return NULL;
 }
-- 
2.20.1


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

* [PATCH v7 5/8] ima: make process_buffer_measurement() generic
  2019-10-08  1:14 [PATCH v7 0/8] powerpc: Enabling IMA arch specific secure boot policies Nayna Jain
                   ` (3 preceding siblings ...)
  2019-10-08  1:14 ` [PATCH v7 4/8] powerpc/ima: add measurement rules to ima arch specific policy Nayna Jain
@ 2019-10-08  1:14 ` Nayna Jain
  2019-10-11 13:14   ` Mimi Zohar
  2019-10-08  1:14 ` [PATCH v7 6/8] certs: add wrapper function to check blacklisted binary hash Nayna Jain
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Nayna Jain @ 2019-10-08  1:14 UTC (permalink / raw)
  To: linuxppc-dev, linux-efi, linux-integrity
  Cc: Ard Biesheuvel, Eric Ricther, Nayna Jain, linux-kernel,
	Mimi Zohar, Claudio Carvalho, Matthew Garret, Greg Kroah-Hartman,
	Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, George Wilson

An additional measurement record is needed to indicate the blacklisted
binary. The record will measure the blacklisted binary hash.

This patch makes the function process_buffer_measurement() generic to be
called by the blacklisting function. It modifies the function to handle
more than just the KEXEC_CMDLINE.

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
 security/integrity/ima/ima.h      |  3 +++
 security/integrity/ima/ima_main.c | 29 ++++++++++++++---------------
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 3689081aaf38..ed86c1f70d7f 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -217,6 +217,9 @@ void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
 			   struct evm_ima_xattr_data *xattr_value,
 			   int xattr_len, const struct modsig *modsig, int pcr,
 			   struct ima_template_desc *template_desc);
+void process_buffer_measurement(const void *buf, int size,
+				const char *eventname, int pcr,
+				struct ima_template_desc *template_desc);
 void ima_audit_measurement(struct integrity_iint_cache *iint,
 			   const unsigned char *filename);
 int ima_alloc_init_template(struct ima_event_data *event_data,
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 60027c643ecd..77115e884496 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -626,14 +626,14 @@ int ima_load_data(enum kernel_load_data_id id)
  * @buf: pointer to the buffer that needs to be added to the log.
  * @size: size of buffer(in bytes).
  * @eventname: event name to be used for the buffer entry.
- * @cred: a pointer to a credentials structure for user validation.
- * @secid: the secid of the task to be validated.
+ * @pcr: pcr to extend the measurement
+ * @template_desc: template description
  *
  * Based on policy, the buffer is measured into the ima log.
  */
-static void process_buffer_measurement(const void *buf, int size,
-				       const char *eventname,
-				       const struct cred *cred, u32 secid)
+void process_buffer_measurement(const void *buf, int size,
+				const char *eventname, int pcr,
+				struct ima_template_desc *template_desc)
 {
 	int ret = 0;
 	struct ima_template_entry *entry = NULL;
@@ -642,19 +642,11 @@ static void process_buffer_measurement(const void *buf, int size,
 					    .filename = eventname,
 					    .buf = buf,
 					    .buf_len = size};
-	struct ima_template_desc *template_desc = NULL;
 	struct {
 		struct ima_digest_data hdr;
 		char digest[IMA_MAX_DIGEST_SIZE];
 	} hash = {};
 	int violation = 0;
-	int pcr = CONFIG_IMA_MEASURE_PCR_IDX;
-	int action = 0;
-
-	action = ima_get_action(NULL, cred, secid, 0, KEXEC_CMDLINE, &pcr,
-				&template_desc);
-	if (!(action & IMA_MEASURE))
-		return;
 
 	iint.ima_hash = &hash.hdr;
 	iint.ima_hash->algo = ima_hash_algo;
@@ -686,12 +678,19 @@ static void process_buffer_measurement(const void *buf, int size,
  */
 void ima_kexec_cmdline(const void *buf, int size)
 {
+	int pcr = CONFIG_IMA_MEASURE_PCR_IDX;
+	struct ima_template_desc *template_desc = NULL;
+	int action;
 	u32 secid;
 
 	if (buf && size != 0) {
 		security_task_getsecid(current, &secid);
-		process_buffer_measurement(buf, size, "kexec-cmdline",
-					   current_cred(), secid);
+		action = ima_get_action(NULL, current_cred(), secid, 0,
+					KEXEC_CMDLINE, &pcr, &template_desc);
+		if (!(action & IMA_MEASURE))
+			return;
+		process_buffer_measurement(buf, size, "kexec-cmdline", pcr,
+					   template_desc);
 	}
 }
 
-- 
2.20.1


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

* [PATCH v7 6/8] certs: add wrapper function to check blacklisted binary hash
  2019-10-08  1:14 [PATCH v7 0/8] powerpc: Enabling IMA arch specific secure boot policies Nayna Jain
                   ` (4 preceding siblings ...)
  2019-10-08  1:14 ` [PATCH v7 5/8] ima: make process_buffer_measurement() generic Nayna Jain
@ 2019-10-08  1:14 ` Nayna Jain
  2019-10-11 13:18   ` Mimi Zohar
  2019-10-08  1:14 ` [PATCH v7 7/8] ima: check against blacklisted hashes for files with modsig Nayna Jain
  2019-10-08  1:14 ` [PATCH v7 8/8] powerpc/ima: update ima arch policy to check for blacklist Nayna Jain
  7 siblings, 1 reply; 14+ messages in thread
From: Nayna Jain @ 2019-10-08  1:14 UTC (permalink / raw)
  To: linuxppc-dev, linux-efi, linux-integrity
  Cc: Ard Biesheuvel, Eric Ricther, Nayna Jain, linux-kernel,
	Mimi Zohar, Claudio Carvalho, Matthew Garret, Greg Kroah-Hartman,
	Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, George Wilson

The existing is_hash_blacklisted() function returns -EKEYREJECTED
error code for both the blacklisted keys and binaries.

This patch adds a wrapper function is_binary_blacklisted() to check
against binary hashes and returns -EPERM.

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
 certs/blacklist.c             | 9 +++++++++
 include/keys/system_keyring.h | 6 ++++++
 2 files changed, 15 insertions(+)

diff --git a/certs/blacklist.c b/certs/blacklist.c
index ec00bf337eb6..6514f9ebc943 100644
--- a/certs/blacklist.c
+++ b/certs/blacklist.c
@@ -135,6 +135,15 @@ int is_hash_blacklisted(const u8 *hash, size_t hash_len, const char *type)
 }
 EXPORT_SYMBOL_GPL(is_hash_blacklisted);
 
+int is_binary_blacklisted(const u8 *hash, size_t hash_len)
+{
+	if (is_hash_blacklisted(hash, hash_len, "bin") == -EKEYREJECTED)
+		return -EPERM;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(is_binary_blacklisted);
+
 /*
  * Initialise the blacklist
  */
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index c1a96fdf598b..fb8b07daa9d1 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -35,12 +35,18 @@ extern int restrict_link_by_builtin_and_secondary_trusted(
 extern int mark_hash_blacklisted(const char *hash);
 extern int is_hash_blacklisted(const u8 *hash, size_t hash_len,
 			       const char *type);
+extern int is_binary_blacklisted(const u8 *hash, size_t hash_len);
 #else
 static inline int is_hash_blacklisted(const u8 *hash, size_t hash_len,
 				      const char *type)
 {
 	return 0;
 }
+
+static inline int is_binary_blacklisted(const u8 *hash, size_t hash_len)
+{
+	return 0;
+}
 #endif
 
 #ifdef CONFIG_IMA_BLACKLIST_KEYRING
-- 
2.20.1


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

* [PATCH v7 7/8] ima: check against blacklisted hashes for files with modsig
  2019-10-08  1:14 [PATCH v7 0/8] powerpc: Enabling IMA arch specific secure boot policies Nayna Jain
                   ` (5 preceding siblings ...)
  2019-10-08  1:14 ` [PATCH v7 6/8] certs: add wrapper function to check blacklisted binary hash Nayna Jain
@ 2019-10-08  1:14 ` Nayna Jain
  2019-10-11 13:19   ` Mimi Zohar
  2019-10-08  1:14 ` [PATCH v7 8/8] powerpc/ima: update ima arch policy to check for blacklist Nayna Jain
  7 siblings, 1 reply; 14+ messages in thread
From: Nayna Jain @ 2019-10-08  1:14 UTC (permalink / raw)
  To: linuxppc-dev, linux-efi, linux-integrity
  Cc: Ard Biesheuvel, Eric Ricther, Nayna Jain, linux-kernel,
	Mimi Zohar, Claudio Carvalho, Matthew Garret, Greg Kroah-Hartman,
	Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, George Wilson

Asymmetric private keys are used to sign multiple files. The kernel
currently support checking against the blacklisted keys. However, if the
public key is blacklisted, any file signed by the blacklisted key will
automatically fail signature verification. We might not want to blacklist
all the files signed by a particular key, but just a single file.
Blacklisting the public key is not fine enough granularity.

This patch adds support for blacklisting binaries with appended signatures,
based on the IMA policy.  Defined is a new policy option
"appraise_flag=check_blacklist".

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
 Documentation/ABI/testing/ima_policy  |  1 +
 security/integrity/ima/ima.h          |  9 +++++++
 security/integrity/ima/ima_appraise.c | 39 +++++++++++++++++++++++++++
 security/integrity/ima/ima_main.c     | 12 ++++++---
 security/integrity/ima/ima_policy.c   | 10 +++++--
 security/integrity/integrity.h        |  1 +
 6 files changed, 66 insertions(+), 6 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 29ebe9afdac4..4c97afcc0f3c 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -25,6 +25,7 @@ Description:
 			lsm:	[[subj_user=] [subj_role=] [subj_type=]
 				 [obj_user=] [obj_role=] [obj_type=]]
 			option:	[[appraise_type=]] [template=] [permit_directio]
+				[appraise_flag=[check_blacklist]]
 		base: 	func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
 				[FIRMWARE_CHECK]
 				[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index ed86c1f70d7f..63e20ccc91ce 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -256,6 +256,8 @@ int ima_policy_show(struct seq_file *m, void *v);
 #define IMA_APPRAISE_KEXEC	0x40
 
 #ifdef CONFIG_IMA_APPRAISE
+int ima_check_blacklist(struct integrity_iint_cache *iint,
+			const struct modsig *modsig, int action, int pcr);
 int ima_appraise_measurement(enum ima_hooks func,
 			     struct integrity_iint_cache *iint,
 			     struct file *file, const unsigned char *filename,
@@ -271,6 +273,13 @@ int ima_read_xattr(struct dentry *dentry,
 		   struct evm_ima_xattr_data **xattr_value);
 
 #else
+static inline int ima_check_blacklist(struct integrity_iint_cache *iint,
+				      const struct modsig *modsig, int action,
+				      int pcr)
+{
+	return 0;
+}
+
 static inline int ima_appraise_measurement(enum ima_hooks func,
 					   struct integrity_iint_cache *iint,
 					   struct file *file,
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 136ae4e0ee92..fe34d64a684c 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -12,6 +12,7 @@
 #include <linux/magic.h>
 #include <linux/ima.h>
 #include <linux/evm.h>
+#include <keys/system_keyring.h>
 
 #include "ima.h"
 
@@ -303,6 +304,44 @@ static int modsig_verify(enum ima_hooks func, const struct modsig *modsig,
 	return rc;
 }
 
+/*
+ * ima_blacklist_measurement - Checks whether the binary is blacklisted. If
+ * yes, then adds the hash of the blacklisted binary to the measurement list.
+ *
+ * Returns -EPERM if the hash is blacklisted.
+ */
+int ima_check_blacklist(struct integrity_iint_cache *iint,
+			const struct modsig *modsig, int action, int pcr)
+{
+	enum hash_algo hash_algo;
+	const u8 *digest = NULL;
+	u32 digestsize = 0;
+	u32 secid;
+	int rc = 0;
+	struct ima_template_desc *template_desc;
+
+	template_desc = lookup_template_desc("ima-buf");
+	template_desc_init_fields(template_desc->fmt, &(template_desc->fields),
+				  &(template_desc->num_fields));
+
+	if (!(iint->flags & IMA_CHECK_BLACKLIST))
+		return 0;
+
+	if (iint->flags & IMA_MODSIG_ALLOWED) {
+		security_task_getsecid(current, &secid);
+		ima_get_modsig_digest(modsig, &hash_algo, &digest, &digestsize);
+		rc = is_binary_blacklisted(digest, digestsize);
+
+		/* Returns -EPERM on blacklisted hash found */
+		if ((rc == -EPERM) && (iint->flags & IMA_MEASURE))
+			process_buffer_measurement(digest, digestsize,
+						   "blacklisted-hash", pcr,
+						   template_desc);
+	}
+
+	return rc;
+}
+
 /*
  * ima_appraise_measurement - appraise file measurement
  *
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 77115e884496..40d30ab17cbe 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -335,10 +335,14 @@ static int process_measurement(struct file *file, const struct cred *cred,
 				      xattr_value, xattr_len, modsig, pcr,
 				      template_desc);
 	if (rc == 0 && (action & IMA_APPRAISE_SUBMASK)) {
-		inode_lock(inode);
-		rc = ima_appraise_measurement(func, iint, file, pathname,
-					      xattr_value, xattr_len, modsig);
-		inode_unlock(inode);
+		rc = ima_check_blacklist(iint, modsig, action, pcr);
+		if (rc != -EPERM) {
+			inode_lock(inode);
+			rc = ima_appraise_measurement(func, iint, file,
+						      pathname, xattr_value,
+						      xattr_len, modsig);
+			inode_unlock(inode);
+		}
 		if (!rc)
 			rc = mmap_violation_check(func, file, &pathbuf,
 						  &pathname, filename);
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 5380aca2b351..bfaae7a8443a 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -765,8 +765,8 @@ enum {
 	Opt_fsuuid, Opt_uid_eq, Opt_euid_eq, Opt_fowner_eq,
 	Opt_uid_gt, Opt_euid_gt, Opt_fowner_gt,
 	Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt,
-	Opt_appraise_type, Opt_permit_directio,
-	Opt_pcr, Opt_template, Opt_err
+	Opt_appraise_type, Opt_appraise_flag,
+	Opt_permit_directio, Opt_pcr, Opt_template, Opt_err
 };
 
 static const match_table_t policy_tokens = {
@@ -798,6 +798,7 @@ static const match_table_t policy_tokens = {
 	{Opt_euid_lt, "euid<%s"},
 	{Opt_fowner_lt, "fowner<%s"},
 	{Opt_appraise_type, "appraise_type=%s"},
+	{Opt_appraise_flag, "appraise_flag=%s"},
 	{Opt_permit_directio, "permit_directio"},
 	{Opt_pcr, "pcr=%s"},
 	{Opt_template, "template=%s"},
@@ -1172,6 +1173,11 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 			else
 				result = -EINVAL;
 			break;
+		case Opt_appraise_flag:
+			ima_log_string(ab, "appraise_flag", args[0].from);
+			if (strstr(args[0].from, "blacklist"))
+				entry->flags |= IMA_CHECK_BLACKLIST;
+			break;
 		case Opt_permit_directio:
 			entry->flags |= IMA_PERMIT_DIRECTIO;
 			break;
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index d9323d31a3a8..73fc286834d7 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -32,6 +32,7 @@
 #define EVM_IMMUTABLE_DIGSIG	0x08000000
 #define IMA_FAIL_UNVERIFIABLE_SIGS	0x10000000
 #define IMA_MODSIG_ALLOWED	0x20000000
+#define IMA_CHECK_BLACKLIST	0x40000000
 
 #define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
 				 IMA_HASH | IMA_APPRAISE_SUBMASK)
-- 
2.20.1


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

* [PATCH v7 8/8] powerpc/ima: update ima arch policy to check for blacklist
  2019-10-08  1:14 [PATCH v7 0/8] powerpc: Enabling IMA arch specific secure boot policies Nayna Jain
                   ` (6 preceding siblings ...)
  2019-10-08  1:14 ` [PATCH v7 7/8] ima: check against blacklisted hashes for files with modsig Nayna Jain
@ 2019-10-08  1:14 ` Nayna Jain
  2019-10-11 13:19   ` Mimi Zohar
  7 siblings, 1 reply; 14+ messages in thread
From: Nayna Jain @ 2019-10-08  1:14 UTC (permalink / raw)
  To: linuxppc-dev, linux-efi, linux-integrity
  Cc: Ard Biesheuvel, Eric Ricther, Nayna Jain, linux-kernel,
	Mimi Zohar, Claudio Carvalho, Matthew Garret, Greg Kroah-Hartman,
	Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, George Wilson

This patch updates the arch specific policies for PowernV systems
to add check against blacklisted binary hashes before doing the
verification.

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
 arch/powerpc/kernel/ima_arch.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/ima_arch.c b/arch/powerpc/kernel/ima_arch.c
index 88bfe4a1a9a5..4fa41537b846 100644
--- a/arch/powerpc/kernel/ima_arch.c
+++ b/arch/powerpc/kernel/ima_arch.c
@@ -25,9 +25,9 @@ bool arch_ima_get_secureboot(void)
 static const char *const arch_rules[] = {
 	"measure func=KEXEC_KERNEL_CHECK template=ima-modsig",
 	"measure func=MODULE_CHECK template=ima-modsig",
-	"appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig",
+	"appraise func=KEXEC_KERNEL_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig",
 #if !IS_ENABLED(CONFIG_MODULE_SIG_FORCE)
-	"appraise func=MODULE_CHECK appraise_type=imasig|modsig",
+	"appraise func=MODULE_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig",
 #endif
 	NULL
 };
-- 
2.20.1


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

* Re: [PATCH v7 2/8] powerpc: add support to initialize ima policy rules
  2019-10-08  1:14 ` [PATCH v7 2/8] powerpc: add support to initialize ima policy rules Nayna Jain
@ 2019-10-11 13:12   ` Mimi Zohar
  0 siblings, 0 replies; 14+ messages in thread
From: Mimi Zohar @ 2019-10-11 13:12 UTC (permalink / raw)
  To: Nayna Jain, linuxppc-dev, linux-efi, linux-integrity
  Cc: Ard Biesheuvel, Eric Ricther, linux-kernel, Claudio Carvalho,
	Matthew Garret, Greg Kroah-Hartman, Paul Mackerras, Jeremy Kerr,
	Elaine Palmer, Oliver O'Halloran, George Wilson

On Mon, 2019-10-07 at 21:14 -0400, Nayna Jain wrote:
> PowerNV systems uses kernel based bootloader, thus its secure boot
> implementation uses kernel IMA security subsystem to verify the kernel
> before kexec. 

^use a Linux based bootloader, which rely on the IMA subsystem to
enforce different secure boot modes.

> Since the verification policy might differ based on the
> secure boot mode of the system, the policies are defined at runtime.

^the policies need to be defined at runtime.
> 
> This patch implements the arch-specific support to define the IMA policy
> rules based on the runtime secure boot mode of the system.
> 
> This patch provides arch-specific IMA policies if PPC_SECURE_BOOT
> config is enabled.
> 
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> ---
>  arch/powerpc/Kconfig           |  2 ++
>  arch/powerpc/kernel/Makefile   |  2 +-
>  arch/powerpc/kernel/ima_arch.c | 33 +++++++++++++++++++++++++++++++++
>  include/linux/ima.h            |  3 ++-
>  4 files changed, 38 insertions(+), 2 deletions(-)
>  create mode 100644 arch/powerpc/kernel/ima_arch.c
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index b4a221886fcf..deb19ec6ba3d 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -938,6 +938,8 @@ config PPC_SECURE_BOOT
>  	prompt "Enable secure boot support"
>  	bool
>  	depends on PPC_POWERNV
> +	depends on IMA
> +	depends on IMA_ARCH_POLICY

As IMA_ARCH_POLICY is dependent on IMA, I don't see a need for
depending on both IMA and IMA_ARCH_POLICY.

Mimi


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

* Re: [PATCH v7 5/8] ima: make process_buffer_measurement() generic
  2019-10-08  1:14 ` [PATCH v7 5/8] ima: make process_buffer_measurement() generic Nayna Jain
@ 2019-10-11 13:14   ` Mimi Zohar
  0 siblings, 0 replies; 14+ messages in thread
From: Mimi Zohar @ 2019-10-11 13:14 UTC (permalink / raw)
  To: Nayna Jain, linuxppc-dev, linux-efi, linux-integrity
  Cc: Ard Biesheuvel, Eric Ricther, Prakhar Srivastava, linux-kernel,
	Claudio Carvalho, Matthew Garret, Greg Kroah-Hartman,
	Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, George Wilson

[Cc'ing Prakhar Srivastava]

On Mon, 2019-10-07 at 21:14 -0400, Nayna Jain wrote:
> An additional measurement record is needed to indicate the blacklisted
> binary. The record will measure the blacklisted binary hash.
> 
> This patch makes the function process_buffer_measurement() generic to be
> called by the blacklisting function. It modifies the function to handle
> more than just the KEXEC_CMDLINE.

The purpose of this patch is to make process_buffer_measurement() more
generic.  The patch description should simply say,
process_buffer_measurement() is limited to measuring the kexec boot
command line.  This patch makes process_buffer_measurement() more
generic, allowing it to measure other types of buffer data (eg.
blacklisted binary hashes).

Mimi

> 
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> ---
>  security/integrity/ima/ima.h      |  3 +++
>  security/integrity/ima/ima_main.c | 29 ++++++++++++++---------------
>  2 files changed, 17 insertions(+), 15 deletions(-)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 3689081aaf38..ed86c1f70d7f 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -217,6 +217,9 @@ void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
>  			   struct evm_ima_xattr_data *xattr_value,
>  			   int xattr_len, const struct modsig *modsig, int pcr,
>  			   struct ima_template_desc *template_desc);
> +void process_buffer_measurement(const void *buf, int size,
> +				const char *eventname, int pcr,
> +				struct ima_template_desc *template_desc);
>  void ima_audit_measurement(struct integrity_iint_cache *iint,
>  			   const unsigned char *filename);
>  int ima_alloc_init_template(struct ima_event_data *event_data,
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 60027c643ecd..77115e884496 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -626,14 +626,14 @@ int ima_load_data(enum kernel_load_data_id id)
>   * @buf: pointer to the buffer that needs to be added to the log.
>   * @size: size of buffer(in bytes).
>   * @eventname: event name to be used for the buffer entry.
> - * @cred: a pointer to a credentials structure for user validation.
> - * @secid: the secid of the task to be validated.
> + * @pcr: pcr to extend the measurement
> + * @template_desc: template description
>   *
>   * Based on policy, the buffer is measured into the ima log.
>   */
> -static void process_buffer_measurement(const void *buf, int size,
> -				       const char *eventname,
> -				       const struct cred *cred, u32 secid)
> +void process_buffer_measurement(const void *buf, int size,
> +				const char *eventname, int pcr,
> +				struct ima_template_desc *template_desc)
>  {
>  	int ret = 0;
>  	struct ima_template_entry *entry = NULL;
> @@ -642,19 +642,11 @@ static void process_buffer_measurement(const void *buf, int size,
>  					    .filename = eventname,
>  					    .buf = buf,
>  					    .buf_len = size};
> -	struct ima_template_desc *template_desc = NULL;
>  	struct {
>  		struct ima_digest_data hdr;
>  		char digest[IMA_MAX_DIGEST_SIZE];
>  	} hash = {};
>  	int violation = 0;
> -	int pcr = CONFIG_IMA_MEASURE_PCR_IDX;
> -	int action = 0;
> -
> -	action = ima_get_action(NULL, cred, secid, 0, KEXEC_CMDLINE, &pcr,
> -				&template_desc);
> -	if (!(action & IMA_MEASURE))
> -		return;
>  
>  	iint.ima_hash = &hash.hdr;
>  	iint.ima_hash->algo = ima_hash_algo;
> @@ -686,12 +678,19 @@ static void process_buffer_measurement(const void *buf, int size,
>   */
>  void ima_kexec_cmdline(const void *buf, int size)
>  {
> +	int pcr = CONFIG_IMA_MEASURE_PCR_IDX;
> +	struct ima_template_desc *template_desc = NULL;
> +	int action;
>  	u32 secid;
>  
>  	if (buf && size != 0) {
>  		security_task_getsecid(current, &secid);
> -		process_buffer_measurement(buf, size, "kexec-cmdline",
> -					   current_cred(), secid);
> +		action = ima_get_action(NULL, current_cred(), secid, 0,
> +					KEXEC_CMDLINE, &pcr, &template_desc);
> +		if (!(action & IMA_MEASURE))
> +			return;
> +		process_buffer_measurement(buf, size, "kexec-cmdline", pcr,
> +					   template_desc);
>  	}
>  }
>  


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

* Re: [PATCH v7 6/8] certs: add wrapper function to check blacklisted binary hash
  2019-10-08  1:14 ` [PATCH v7 6/8] certs: add wrapper function to check blacklisted binary hash Nayna Jain
@ 2019-10-11 13:18   ` Mimi Zohar
  0 siblings, 0 replies; 14+ messages in thread
From: Mimi Zohar @ 2019-10-11 13:18 UTC (permalink / raw)
  To: Nayna Jain, linuxppc-dev, linux-efi, linux-integrity
  Cc: Ard Biesheuvel, Eric Ricther, linux-kernel, Claudio Carvalho,
	Matthew Garret, Greg Kroah-Hartman, Paul Mackerras, Jeremy Kerr,
	Elaine Palmer, Oliver O'Halloran, George Wilson

On Mon, 2019-10-07 at 21:14 -0400, Nayna Jain wrote:
> The existing is_hash_blacklisted() function returns -EKEYREJECTED
> error code for both the blacklisted keys and binaries.
> 
> This patch adds a wrapper function is_binary_blacklisted() to check
> against binary hashes and returns -EPERM.    
> 
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>

This patch description describes what you're doing, not the
motivation.

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>

> ---
>  certs/blacklist.c             | 9 +++++++++
>  include/keys/system_keyring.h | 6 ++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/certs/blacklist.c b/certs/blacklist.c
> index ec00bf337eb6..6514f9ebc943 100644
> --- a/certs/blacklist.c
> +++ b/certs/blacklist.c
> @@ -135,6 +135,15 @@ int is_hash_blacklisted(const u8 *hash, size_t hash_len, const char *type)
>  }
>  EXPORT_SYMBOL_GPL(is_hash_blacklisted);
>  
> +int is_binary_blacklisted(const u8 *hash, size_t hash_len)
> +{
> +	if (is_hash_blacklisted(hash, hash_len, "bin") == -EKEYREJECTED)
> +		return -EPERM;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(is_binary_blacklisted);
> +
>  /*
>   * Initialise the blacklist
>   */
> diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
> index c1a96fdf598b..fb8b07daa9d1 100644
> --- a/include/keys/system_keyring.h
> +++ b/include/keys/system_keyring.h
> @@ -35,12 +35,18 @@ extern int restrict_link_by_builtin_and_secondary_trusted(
>  extern int mark_hash_blacklisted(const char *hash);
>  extern int is_hash_blacklisted(const u8 *hash, size_t hash_len,
>  			       const char *type);
> +extern int is_binary_blacklisted(const u8 *hash, size_t hash_len);
>  #else
>  static inline int is_hash_blacklisted(const u8 *hash, size_t hash_len,
>  				      const char *type)
>  {
>  	return 0;
>  }
> +
> +static inline int is_binary_blacklisted(const u8 *hash, size_t hash_len)
> +{
> +	return 0;
> +}
>  #endif
>  
>  #ifdef CONFIG_IMA_BLACKLIST_KEYRING


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

* Re: [PATCH v7 8/8] powerpc/ima: update ima arch policy to check for blacklist
  2019-10-08  1:14 ` [PATCH v7 8/8] powerpc/ima: update ima arch policy to check for blacklist Nayna Jain
@ 2019-10-11 13:19   ` Mimi Zohar
  0 siblings, 0 replies; 14+ messages in thread
From: Mimi Zohar @ 2019-10-11 13:19 UTC (permalink / raw)
  To: Nayna Jain, linuxppc-dev, linux-efi, linux-integrity
  Cc: Ard Biesheuvel, Eric Ricther, linux-kernel, Claudio Carvalho,
	Matthew Garret, Greg Kroah-Hartman, Paul Mackerras, Jeremy Kerr,
	Elaine Palmer, Oliver O'Halloran, George Wilson

On Mon, 2019-10-07 at 21:14 -0400, Nayna Jain wrote:
> This patch updates the arch specific policies for PowernV systems
> to add check against blacklisted binary hashes before doing the
> verification.

This sentence explains how you're doing something.  A simple tweak in
the wording provides the motivation.

^to make sure that the binary hash is not blacklisted.

> 
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>

> ---
>  arch/powerpc/kernel/ima_arch.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/ima_arch.c b/arch/powerpc/kernel/ima_arch.c
> index 88bfe4a1a9a5..4fa41537b846 100644
> --- a/arch/powerpc/kernel/ima_arch.c
> +++ b/arch/powerpc/kernel/ima_arch.c
> @@ -25,9 +25,9 @@ bool arch_ima_get_secureboot(void)
>  static const char *const arch_rules[] = {
>  	"measure func=KEXEC_KERNEL_CHECK template=ima-modsig",
>  	"measure func=MODULE_CHECK template=ima-modsig",
> -	"appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig",
> +	"appraise func=KEXEC_KERNEL_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig",
>  #if !IS_ENABLED(CONFIG_MODULE_SIG_FORCE)
> -	"appraise func=MODULE_CHECK appraise_type=imasig|modsig",
> +	"appraise func=MODULE_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig",
>  #endif
>  	NULL
>  };


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

* Re: [PATCH v7 7/8] ima: check against blacklisted hashes for files with modsig
  2019-10-08  1:14 ` [PATCH v7 7/8] ima: check against blacklisted hashes for files with modsig Nayna Jain
@ 2019-10-11 13:19   ` Mimi Zohar
  0 siblings, 0 replies; 14+ messages in thread
From: Mimi Zohar @ 2019-10-11 13:19 UTC (permalink / raw)
  To: Nayna Jain, linuxppc-dev, linux-efi, linux-integrity
  Cc: Ard Biesheuvel, Eric Ricther, linux-kernel, Claudio Carvalho,
	Matthew Garret, Greg Kroah-Hartman, Paul Mackerras, Jeremy Kerr,
	Elaine Palmer, Oliver O'Halloran, George Wilson

On Mon, 2019-10-07 at 21:14 -0400, Nayna Jain wrote:
> Asymmetric private keys are used to sign multiple files. The kernel
> currently support checking against the blacklisted keys. However, if the
> public key is blacklisted, any file signed by the blacklisted key will
> automatically fail signature verification. We might not want to blacklist
> all the files signed by a particular key, but just a single file.
> Blacklisting the public key is not fine enough granularity.
> 
> This patch adds support for blacklisting binaries with appended signatures,
> based on the IMA policy.  Defined is a new policy option
> "appraise_flag=check_blacklist".

The blacklisted hash is not the same as the file hash, but is the file
hash without the appended signature.  Are there tools for calculating
the blacklisted hash?  Can you provide an example?

> 
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> ---
>  Documentation/ABI/testing/ima_policy  |  1 +
>  security/integrity/ima/ima.h          |  9 +++++++
>  security/integrity/ima/ima_appraise.c | 39 +++++++++++++++++++++++++++
>  security/integrity/ima/ima_main.c     | 12 ++++++---
>  security/integrity/ima/ima_policy.c   | 10 +++++--
>  security/integrity/integrity.h        |  1 +
>  6 files changed, 66 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
> index 29ebe9afdac4..4c97afcc0f3c 100644
> --- a/Documentation/ABI/testing/ima_policy
> +++ b/Documentation/ABI/testing/ima_policy
> @@ -25,6 +25,7 @@ Description:
>  			lsm:	[[subj_user=] [subj_role=] [subj_type=]
>  				 [obj_user=] [obj_role=] [obj_type=]]
>  			option:	[[appraise_type=]] [template=] [permit_directio]
> +				[appraise_flag=[check_blacklist]]
>  		base: 	func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
>  				[FIRMWARE_CHECK]
>  				[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index ed86c1f70d7f..63e20ccc91ce 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -256,6 +256,8 @@ int ima_policy_show(struct seq_file *m, void *v);
>  #define IMA_APPRAISE_KEXEC	0x40
>  
>  #ifdef CONFIG_IMA_APPRAISE
> +int ima_check_blacklist(struct integrity_iint_cache *iint,
> +			const struct modsig *modsig, int action, int pcr);
>  int ima_appraise_measurement(enum ima_hooks func,
>  			     struct integrity_iint_cache *iint,
>  			     struct file *file, const unsigned char *filename,
> @@ -271,6 +273,13 @@ int ima_read_xattr(struct dentry *dentry,
>  		   struct evm_ima_xattr_data **xattr_value);
>  
>  #else
> +static inline int ima_check_blacklist(struct integrity_iint_cache *iint,
> +				      const struct modsig *modsig, int action,
> +				      int pcr)
> +{
> +	return 0;
> +}
> +
>  static inline int ima_appraise_measurement(enum ima_hooks func,
>  					   struct integrity_iint_cache *iint,
>  					   struct file *file,
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index 136ae4e0ee92..fe34d64a684c 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -12,6 +12,7 @@
>  #include <linux/magic.h>
>  #include <linux/ima.h>
>  #include <linux/evm.h>
> +#include <keys/system_keyring.h>
>  
>  #include "ima.h"
>  
> @@ -303,6 +304,44 @@ static int modsig_verify(enum ima_hooks func, const struct modsig *modsig,
>  	return rc;
>  }
>  
> +/*
> + * ima_blacklist_measurement - Checks whether the binary is blacklisted. If
> + * yes, then adds the hash of the blacklisted binary to the measurement list.
> + *
> + * Returns -EPERM if the hash is blacklisted.
> + */
> +int ima_check_blacklist(struct integrity_iint_cache *iint,
> +			const struct modsig *modsig, int action, int pcr)
> +{
> +	enum hash_algo hash_algo;
> +	const u8 *digest = NULL;
> +	u32 digestsize = 0;
> +	u32 secid;
> +	int rc = 0;
> +	struct ima_template_desc *template_desc;
> +
> +	template_desc = lookup_template_desc("ima-buf");
> +	template_desc_init_fields(template_desc->fmt, &(template_desc->fields),
> +				  &(template_desc->num_fields));

Before using template_desc, make sure that template_desc isn't NULL.
 For completeness, check the return code of
template_desc_init_fields()

> +
> +	if (!(iint->flags & IMA_CHECK_BLACKLIST))
> +		return 0;

Move this check before getting the template_desc and make sure that
modsig isn't NULL.

> +
> +	if (iint->flags & IMA_MODSIG_ALLOWED) {
> +		security_task_getsecid(current, &secid);

secid isn't being used.

> +		ima_get_modsig_digest(modsig, &hash_algo, &digest, &digestsize);
> +		rc = is_binary_blacklisted(digest, digestsize);
> +
> +		/* Returns -EPERM on blacklisted hash found */

Now that it is returning a sane errno, the comment isn't needed.

Mimi

> +		if ((rc == -EPERM) && (iint->flags & IMA_MEASURE))
> +			process_buffer_measurement(digest, digestsize,
> +						   "blacklisted-hash", pcr,
> +						   template_desc);
> +	}
> +
> +	return rc;
> +}
> +
>  /*
>   * ima_appraise_measurement - appraise file measurement
>   *
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 77115e884496..40d30ab17cbe 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -335,10 +335,14 @@ static int process_measurement(struct file *file, const struct cred *cred,
>  				      xattr_value, xattr_len, modsig, pcr,
>  				      template_desc);
>  	if (rc == 0 && (action & IMA_APPRAISE_SUBMASK)) {
> -		inode_lock(inode);
> -		rc = ima_appraise_measurement(func, iint, file, pathname,
> -					      xattr_value, xattr_len, modsig);
> -		inode_unlock(inode);
> +		rc = ima_check_blacklist(iint, modsig, action, pcr);
> +		if (rc != -EPERM) {
> +			inode_lock(inode);
> +			rc = ima_appraise_measurement(func, iint, file,
> +						      pathname, xattr_value,
> +						      xattr_len, modsig);
> +			inode_unlock(inode);
> +		}
>  		if (!rc)
>  			rc = mmap_violation_check(func, file, &pathbuf,
>  						  &pathname, filename);
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 5380aca2b351..bfaae7a8443a 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -765,8 +765,8 @@ enum {
>  	Opt_fsuuid, Opt_uid_eq, Opt_euid_eq, Opt_fowner_eq,
>  	Opt_uid_gt, Opt_euid_gt, Opt_fowner_gt,
>  	Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt,
> -	Opt_appraise_type, Opt_permit_directio,
> -	Opt_pcr, Opt_template, Opt_err
> +	Opt_appraise_type, Opt_appraise_flag,
> +	Opt_permit_directio, Opt_pcr, Opt_template, Opt_err
>  };
>  
>  static const match_table_t policy_tokens = {
> @@ -798,6 +798,7 @@ static const match_table_t policy_tokens = {
>  	{Opt_euid_lt, "euid<%s"},
>  	{Opt_fowner_lt, "fowner<%s"},
>  	{Opt_appraise_type, "appraise_type=%s"},
> +	{Opt_appraise_flag, "appraise_flag=%s"},
>  	{Opt_permit_directio, "permit_directio"},
>  	{Opt_pcr, "pcr=%s"},
>  	{Opt_template, "template=%s"},
> @@ -1172,6 +1173,11 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>  			else
>  				result = -EINVAL;
>  			break;
> +		case Opt_appraise_flag:
> +			ima_log_string(ab, "appraise_flag", args[0].from);
> +			if (strstr(args[0].from, "blacklist"))
> +				entry->flags |= IMA_CHECK_BLACKLIST;
> +			break;
>  		case Opt_permit_directio:
>  			entry->flags |= IMA_PERMIT_DIRECTIO;
>  			break;
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index d9323d31a3a8..73fc286834d7 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -32,6 +32,7 @@
>  #define EVM_IMMUTABLE_DIGSIG	0x08000000
>  #define IMA_FAIL_UNVERIFIABLE_SIGS	0x10000000
>  #define IMA_MODSIG_ALLOWED	0x20000000
> +#define IMA_CHECK_BLACKLIST	0x40000000
>  
>  #define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
>  				 IMA_HASH | IMA_APPRAISE_SUBMASK)


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

end of thread, back to index

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-08  1:14 [PATCH v7 0/8] powerpc: Enabling IMA arch specific secure boot policies Nayna Jain
2019-10-08  1:14 ` [PATCH v7 1/8] powerpc: detect the secure boot mode of the system Nayna Jain
2019-10-08  1:14 ` [PATCH v7 2/8] powerpc: add support to initialize ima policy rules Nayna Jain
2019-10-11 13:12   ` Mimi Zohar
2019-10-08  1:14 ` [PATCH v7 3/8] powerpc: detect the trusted boot state of the system Nayna Jain
2019-10-08  1:14 ` [PATCH v7 4/8] powerpc/ima: add measurement rules to ima arch specific policy Nayna Jain
2019-10-08  1:14 ` [PATCH v7 5/8] ima: make process_buffer_measurement() generic Nayna Jain
2019-10-11 13:14   ` Mimi Zohar
2019-10-08  1:14 ` [PATCH v7 6/8] certs: add wrapper function to check blacklisted binary hash Nayna Jain
2019-10-11 13:18   ` Mimi Zohar
2019-10-08  1:14 ` [PATCH v7 7/8] ima: check against blacklisted hashes for files with modsig Nayna Jain
2019-10-11 13:19   ` Mimi Zohar
2019-10-08  1:14 ` [PATCH v7 8/8] powerpc/ima: update ima arch policy to check for blacklist Nayna Jain
2019-10-11 13:19   ` Mimi Zohar

LinuxPPC-Dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linuxppc-dev/0 linuxppc-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linuxppc-dev linuxppc-dev/ https://lore.kernel.org/linuxppc-dev \
		linuxppc-dev@lists.ozlabs.org linuxppc-dev@ozlabs.org linuxppc-dev@archiver.kernel.org
	public-inbox-index linuxppc-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.ozlabs.lists.linuxppc-dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox