All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6][RFC] Add EFI secure key to key retention service
@ 2018-08-05  3:21 ` Lee, Chun-Yi
  0 siblings, 0 replies; 30+ messages in thread
From: Lee, Chun-Yi @ 2018-08-05  3:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-efi, x86, keyrings, linux-integrity, Lee, Chun-Yi,
	Kees Cook, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Rafael J. Wysocki, Pavel Machek, Chen Yu, Oliver Neukum,
	Ryan Chen, Ard Biesheuvel, David Howells, Mimi Zohar

When secure boot is enabled, only signed EFI binary can access
EFI boot service variable before ExitBootService. Which means that
the EFI boot service variable is secure.
   
This patch set add functions to EFI boot stub to generate a 512-bit
random number that it can be used as a root key for encryption and
authentication. This root key will be kept in EFI boot service variable.
EFI boot stub will read and transfer ERK (efi root key) to kernel.
    
At runtime, the ERK can be used to encrypted/authentication other
random number to generate EFI secure key. The EFI secure key can be
a new master key type for encrypted key. It's useful for hibernation
or evm.

Here is the proof of concept for using EFI secure key in hibernation:
  https://github.com/joeyli/linux-s4sign/commit/6311e97038974bc5de8121769fb4d34470009566

Cc: Kees Cook <keescook@chromium.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Chen Yu <yu.c.chen@intel.com>
Cc: Oliver Neukum <oneukum@suse.com>
Cc: Ryan Chen <yu.chen.surf@gmail.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>

Lee, Chun-Yi (6):
  x86/KASLR: make getting random long number function public
  efi: the function transfers status to string
  efi: generate efi root key in EFI boot stub
  key: add EFI secure key type
  key: add EFI secure key as a master key type
  key: enforce the secure boot checking when loading efi root key

 Documentation/admin-guide/kernel-parameters.txt |   6 +
 arch/x86/boot/compressed/Makefile               |   1 +
 arch/x86/boot/compressed/cpuflags.c             |   2 +-
 arch/x86/boot/compressed/eboot.c                |   2 +
 arch/x86/boot/compressed/efi_root_key.c         | 212 +++++++
 arch/x86/boot/compressed/kaslr.c                |  21 -
 arch/x86/boot/compressed/misc.c                 |  17 +
 arch/x86/boot/compressed/misc.h                 |  12 +-
 arch/x86/include/asm/efi.h                      |  13 +
 arch/x86/include/uapi/asm/bootparam.h           |   1 +
 arch/x86/kernel/setup.c                         |   3 +
 arch/x86/lib/kaslr.c                            |  61 +-
 arch/x86/lib/random.c                           |  68 +++
 drivers/firmware/efi/Kconfig                    |  31 +
 drivers/firmware/efi/Makefile                   |   1 +
 drivers/firmware/efi/efi-secure-key.c           | 748 ++++++++++++++++++++++++
 include/keys/efi-type.h                         |  57 ++
 include/linux/efi.h                             |  40 ++
 include/linux/kernel.h                          |   3 +-
 kernel/panic.c                                  |   1 +
 security/keys/encrypted-keys/encrypted.c        |  10 +
 21 files changed, 1226 insertions(+), 84 deletions(-)
 create mode 100644 arch/x86/boot/compressed/efi_root_key.c
 create mode 100644 arch/x86/lib/random.c
 create mode 100644 drivers/firmware/efi/efi-secure-key.c
 create mode 100644 include/keys/efi-type.h

-- 
2.13.6


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

* [PATCH 0/6][RFC] Add EFI secure key to key retention service
@ 2018-08-05  3:21 ` Lee, Chun-Yi
  0 siblings, 0 replies; 30+ messages in thread
From: Lee, Chun-Yi @ 2018-08-05  3:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-efi, x86, keyrings, linux-integrity, Lee, Chun-Yi,
	Kees Cook, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Rafael J. Wysocki, Pavel Machek, Chen Yu, Oliver Neukum,
	Ryan Chen, Ard Biesheuvel, David Howells, Mimi Zohar

When secure boot is enabled, only signed EFI binary can access
EFI boot service variable before ExitBootService. Which means that
the EFI boot service variable is secure.
   
This patch set add functions to EFI boot stub to generate a 512-bit
random number that it can be used as a root key for encryption and
authentication. This root key will be kept in EFI boot service variable.
EFI boot stub will read and transfer ERK (efi root key) to kernel.
    
At runtime, the ERK can be used to encrypted/authentication other
random number to generate EFI secure key. The EFI secure key can be
a new master key type for encrypted key. It's useful for hibernation
or evm.

Here is the proof of concept for using EFI secure key in hibernation:
  https://github.com/joeyli/linux-s4sign/commit/6311e97038974bc5de8121769fb4d34470009566

Cc: Kees Cook <keescook@chromium.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Chen Yu <yu.c.chen@intel.com>
Cc: Oliver Neukum <oneukum@suse.com>
Cc: Ryan Chen <yu.chen.surf@gmail.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>

Lee, Chun-Yi (6):
  x86/KASLR: make getting random long number function public
  efi: the function transfers status to string
  efi: generate efi root key in EFI boot stub
  key: add EFI secure key type
  key: add EFI secure key as a master key type
  key: enforce the secure boot checking when loading efi root key

 Documentation/admin-guide/kernel-parameters.txt |   6 +
 arch/x86/boot/compressed/Makefile               |   1 +
 arch/x86/boot/compressed/cpuflags.c             |   2 +-
 arch/x86/boot/compressed/eboot.c                |   2 +
 arch/x86/boot/compressed/efi_root_key.c         | 212 +++++++
 arch/x86/boot/compressed/kaslr.c                |  21 -
 arch/x86/boot/compressed/misc.c                 |  17 +
 arch/x86/boot/compressed/misc.h                 |  12 +-
 arch/x86/include/asm/efi.h                      |  13 +
 arch/x86/include/uapi/asm/bootparam.h           |   1 +
 arch/x86/kernel/setup.c                         |   3 +
 arch/x86/lib/kaslr.c                            |  61 +-
 arch/x86/lib/random.c                           |  68 +++
 drivers/firmware/efi/Kconfig                    |  31 +
 drivers/firmware/efi/Makefile                   |   1 +
 drivers/firmware/efi/efi-secure-key.c           | 748 ++++++++++++++++++++++++
 include/keys/efi-type.h                         |  57 ++
 include/linux/efi.h                             |  40 ++
 include/linux/kernel.h                          |   3 +-
 kernel/panic.c                                  |   1 +
 security/keys/encrypted-keys/encrypted.c        |  10 +
 21 files changed, 1226 insertions(+), 84 deletions(-)
 create mode 100644 arch/x86/boot/compressed/efi_root_key.c
 create mode 100644 arch/x86/lib/random.c
 create mode 100644 drivers/firmware/efi/efi-secure-key.c
 create mode 100644 include/keys/efi-type.h

-- 
2.13.6


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

* [PATCH 1/6] x86/KASLR: make getting random long number function public
  2018-08-05  3:21 ` Lee, Chun-Yi
@ 2018-08-05  3:21   ` Lee, Chun-Yi
  -1 siblings, 0 replies; 30+ messages in thread
From: Lee, Chun-Yi @ 2018-08-05  3:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-efi, x86, keyrings, linux-integrity, Lee, Chun-Yi,
	Kees Cook, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Rafael J. Wysocki, Pavel Machek, Chen Yu, Oliver Neukum,
	Ryan Chen, Ard Biesheuvel, David Howells, Mimi Zohar

Separating the functions for getting random long number from KASLR
to x86 library, then it can be used to generate random long for
EFI root key.

Cc: Kees Cook <keescook@chromium.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Chen Yu <yu.c.chen@intel.com>
Cc: Oliver Neukum <oneukum@suse.com>
Cc: Ryan Chen <yu.chen.surf@gmail.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
---
 arch/x86/boot/compressed/kaslr.c | 21 -------------
 arch/x86/boot/compressed/misc.c  | 17 ++++++++++
 arch/x86/boot/compressed/misc.h  |  6 ++++
 arch/x86/lib/kaslr.c             | 61 ++---------------------------------
 arch/x86/lib/random.c            | 68 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 93 insertions(+), 80 deletions(-)
 create mode 100644 arch/x86/lib/random.c

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index b87a7582853d..0f40d2178ebc 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -33,13 +33,11 @@
 #include "error.h"
 #include "../string.h"
 
-#include <generated/compile.h>
 #include <linux/module.h>
 #include <linux/uts.h>
 #include <linux/utsname.h>
 #include <linux/ctype.h>
 #include <linux/efi.h>
-#include <generated/utsrelease.h>
 #include <asm/efi.h>
 
 /* Macros used by the included decompressor code below. */
@@ -57,25 +55,6 @@ extern unsigned long get_cmd_line_ptr(void);
 /* Used by PAGE_KERN* macros: */
 pteval_t __default_kernel_pte_mask __read_mostly = ~0;
 
-/* Simplified build-specific string for starting entropy. */
-static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
-		LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
-
-static unsigned long rotate_xor(unsigned long hash, const void *area,
-				size_t size)
-{
-	size_t i;
-	unsigned long *ptr = (unsigned long *)area;
-
-	for (i = 0; i < size / sizeof(hash); i++) {
-		/* Rotate by odd number of bits and XOR. */
-		hash = (hash << ((sizeof(hash) * 8) - 7)) | (hash >> 7);
-		hash ^= ptr[i];
-	}
-
-	return hash;
-}
-
 /* Attempt to create a simple but unpredictable starting entropy. */
 static unsigned long get_boot_seed(void)
 {
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index 8dd1d5ccae58..eb0ab9cad4e4 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -426,3 +426,20 @@ void fortify_panic(const char *name)
 {
 	error("detected buffer overflow");
 }
+
+#if CONFIG_RANDOMIZE_BASE
+unsigned long rotate_xor(unsigned long hash, const void *area,
+			size_t size)
+{
+	size_t i;
+	unsigned long *ptr = (unsigned long *)area;
+
+	for (i = 0; i < size / sizeof(hash); i++) {
+		/* Rotate by odd number of bits and XOR. */
+		hash = (hash << ((sizeof(hash) * 8) - 7)) | (hash >> 7);
+		hash ^= ptr[i];
+	}
+
+	return hash;
+}
+#endif
diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index a423bdb42686..957f327ad83c 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -70,6 +70,8 @@ int cmdline_find_option_bool(const char *option);
 
 
 #if CONFIG_RANDOMIZE_BASE
+#include <generated/compile.h>
+#include <generated/utsrelease.h>
 /* kaslr.c */
 void choose_random_location(unsigned long input,
 			    unsigned long input_size,
@@ -78,6 +80,10 @@ void choose_random_location(unsigned long input,
 			    unsigned long *virt_addr);
 /* cpuflags.c */
 bool has_cpuflag(int flag);
+/* Simplified build-specific string for starting entropy. */
+static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
+		LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
+unsigned long rotate_xor(unsigned long hash, const void *area, size_t size);
 #else
 static inline void choose_random_location(unsigned long input,
 					  unsigned long input_size,
diff --git a/arch/x86/lib/kaslr.c b/arch/x86/lib/kaslr.c
index 79778ab200e4..29ed9bfde5a5 100644
--- a/arch/x86/lib/kaslr.c
+++ b/arch/x86/lib/kaslr.c
@@ -26,67 +26,10 @@
 #define get_boot_seed() kaslr_offset()
 #endif
 
-#define I8254_PORT_CONTROL	0x43
-#define I8254_PORT_COUNTER0	0x40
-#define I8254_CMD_READBACK	0xC0
-#define I8254_SELECT_COUNTER0	0x02
-#define I8254_STATUS_NOTREADY	0x40
-static inline u16 i8254(void)
-{
-	u16 status, timer;
-
-	do {
-		outb(I8254_PORT_CONTROL,
-		     I8254_CMD_READBACK | I8254_SELECT_COUNTER0);
-		status = inb(I8254_PORT_COUNTER0);
-		timer  = inb(I8254_PORT_COUNTER0);
-		timer |= inb(I8254_PORT_COUNTER0) << 8;
-	} while (status & I8254_STATUS_NOTREADY);
-
-	return timer;
-}
+#include "random.c"
 
 unsigned long kaslr_get_random_long(const char *purpose)
 {
-#ifdef CONFIG_X86_64
-	const unsigned long mix_const = 0x5d6008cbf3848dd3UL;
-#else
-	const unsigned long mix_const = 0x3f39e593UL;
-#endif
-	unsigned long raw, random = get_boot_seed();
-	bool use_i8254 = true;
-
-	debug_putstr(purpose);
 	debug_putstr(" KASLR using");
-
-	if (has_cpuflag(X86_FEATURE_RDRAND)) {
-		debug_putstr(" RDRAND");
-		if (rdrand_long(&raw)) {
-			random ^= raw;
-			use_i8254 = false;
-		}
-	}
-
-	if (has_cpuflag(X86_FEATURE_TSC)) {
-		debug_putstr(" RDTSC");
-		raw = rdtsc();
-
-		random ^= raw;
-		use_i8254 = false;
-	}
-
-	if (use_i8254) {
-		debug_putstr(" i8254");
-		random ^= i8254();
-	}
-
-	/* Circular multiply for better bit diffusion */
-	asm(_ASM_MUL "%3"
-	    : "=a" (random), "=d" (raw)
-	    : "a" (random), "rm" (mix_const));
-	random += raw;
-
-	debug_putstr("...\n");
-
-	return random;
+	return get_random_long(purpose);
 }
diff --git a/arch/x86/lib/random.c b/arch/x86/lib/random.c
new file mode 100644
index 000000000000..f2fe6a784c98
--- /dev/null
+++ b/arch/x86/lib/random.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <asm/io.h>
+#include <asm/archrandom.h>
+
+#define I8254_PORT_CONTROL	0x43
+#define I8254_PORT_COUNTER0	0x40
+#define I8254_CMD_READBACK	0xC0
+#define I8254_SELECT_COUNTER0	0x02
+#define I8254_STATUS_NOTREADY	0x40
+static inline u16 i8254(void)
+{
+	u16 status, timer;
+
+	do {
+		outb(I8254_PORT_CONTROL,
+		     I8254_CMD_READBACK | I8254_SELECT_COUNTER0);
+		status = inb(I8254_PORT_COUNTER0);
+		timer  = inb(I8254_PORT_COUNTER0);
+		timer |= inb(I8254_PORT_COUNTER0) << 8;
+	} while (status & I8254_STATUS_NOTREADY);
+
+	return timer;
+}
+
+static unsigned long get_random_long(const char *purpose)
+{
+#ifdef CONFIG_X86_64
+	const unsigned long mix_const = 0x5d6008cbf3848dd3UL;
+#else
+	const unsigned long mix_const = 0x3f39e593UL;
+#endif
+	unsigned long raw, random = get_boot_seed();
+	bool use_i8254 = true;
+
+	debug_putstr(purpose);
+
+	if (has_cpuflag(X86_FEATURE_RDRAND)) {
+		debug_putstr(" RDRAND");
+		if (rdrand_long(&raw)) {
+			random ^= raw;
+			use_i8254 = false;
+		}
+	}
+
+	if (has_cpuflag(X86_FEATURE_TSC)) {
+		debug_putstr(" RDTSC");
+		raw = rdtsc();
+
+		random ^= raw;
+		use_i8254 = false;
+	}
+
+	if (use_i8254) {
+		debug_putstr(" i8254");
+		random ^= i8254();
+	}
+
+	/* Circular multiply for better bit diffusion */
+	asm(_ASM_MUL "%3"
+	    : "=a" (random), "=d" (raw)
+	    : "a" (random), "rm" (mix_const));
+	random += raw;
+
+	debug_putstr("...\n");
+
+	return random;
+}
-- 
2.13.6


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

* [PATCH 1/6] x86/KASLR: make getting random long number function public
@ 2018-08-05  3:21   ` Lee, Chun-Yi
  0 siblings, 0 replies; 30+ messages in thread
From: Lee, Chun-Yi @ 2018-08-05  3:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-efi, x86, keyrings, linux-integrity, Lee, Chun-Yi,
	Kees Cook, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Rafael J. Wysocki, Pavel Machek, Chen Yu, Oliver Neukum,
	Ryan Chen, Ard Biesheuvel, David Howells, Mimi Zohar

Separating the functions for getting random long number from KASLR
to x86 library, then it can be used to generate random long for
EFI root key.

Cc: Kees Cook <keescook@chromium.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Chen Yu <yu.c.chen@intel.com>
Cc: Oliver Neukum <oneukum@suse.com>
Cc: Ryan Chen <yu.chen.surf@gmail.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
---
 arch/x86/boot/compressed/kaslr.c | 21 -------------
 arch/x86/boot/compressed/misc.c  | 17 ++++++++++
 arch/x86/boot/compressed/misc.h  |  6 ++++
 arch/x86/lib/kaslr.c             | 61 ++---------------------------------
 arch/x86/lib/random.c            | 68 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 93 insertions(+), 80 deletions(-)
 create mode 100644 arch/x86/lib/random.c

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index b87a7582853d..0f40d2178ebc 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -33,13 +33,11 @@
 #include "error.h"
 #include "../string.h"
 
-#include <generated/compile.h>
 #include <linux/module.h>
 #include <linux/uts.h>
 #include <linux/utsname.h>
 #include <linux/ctype.h>
 #include <linux/efi.h>
-#include <generated/utsrelease.h>
 #include <asm/efi.h>
 
 /* Macros used by the included decompressor code below. */
@@ -57,25 +55,6 @@ extern unsigned long get_cmd_line_ptr(void);
 /* Used by PAGE_KERN* macros: */
 pteval_t __default_kernel_pte_mask __read_mostly = ~0;
 
-/* Simplified build-specific string for starting entropy. */
-static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
-		LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
-
-static unsigned long rotate_xor(unsigned long hash, const void *area,
-				size_t size)
-{
-	size_t i;
-	unsigned long *ptr = (unsigned long *)area;
-
-	for (i = 0; i < size / sizeof(hash); i++) {
-		/* Rotate by odd number of bits and XOR. */
-		hash = (hash << ((sizeof(hash) * 8) - 7)) | (hash >> 7);
-		hash ^= ptr[i];
-	}
-
-	return hash;
-}
-
 /* Attempt to create a simple but unpredictable starting entropy. */
 static unsigned long get_boot_seed(void)
 {
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index 8dd1d5ccae58..eb0ab9cad4e4 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -426,3 +426,20 @@ void fortify_panic(const char *name)
 {
 	error("detected buffer overflow");
 }
+
+#if CONFIG_RANDOMIZE_BASE
+unsigned long rotate_xor(unsigned long hash, const void *area,
+			size_t size)
+{
+	size_t i;
+	unsigned long *ptr = (unsigned long *)area;
+
+	for (i = 0; i < size / sizeof(hash); i++) {
+		/* Rotate by odd number of bits and XOR. */
+		hash = (hash << ((sizeof(hash) * 8) - 7)) | (hash >> 7);
+		hash ^= ptr[i];
+	}
+
+	return hash;
+}
+#endif
diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index a423bdb42686..957f327ad83c 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -70,6 +70,8 @@ int cmdline_find_option_bool(const char *option);
 
 
 #if CONFIG_RANDOMIZE_BASE
+#include <generated/compile.h>
+#include <generated/utsrelease.h>
 /* kaslr.c */
 void choose_random_location(unsigned long input,
 			    unsigned long input_size,
@@ -78,6 +80,10 @@ void choose_random_location(unsigned long input,
 			    unsigned long *virt_addr);
 /* cpuflags.c */
 bool has_cpuflag(int flag);
+/* Simplified build-specific string for starting entropy. */
+static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
+		LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
+unsigned long rotate_xor(unsigned long hash, const void *area, size_t size);
 #else
 static inline void choose_random_location(unsigned long input,
 					  unsigned long input_size,
diff --git a/arch/x86/lib/kaslr.c b/arch/x86/lib/kaslr.c
index 79778ab200e4..29ed9bfde5a5 100644
--- a/arch/x86/lib/kaslr.c
+++ b/arch/x86/lib/kaslr.c
@@ -26,67 +26,10 @@
 #define get_boot_seed() kaslr_offset()
 #endif
 
-#define I8254_PORT_CONTROL	0x43
-#define I8254_PORT_COUNTER0	0x40
-#define I8254_CMD_READBACK	0xC0
-#define I8254_SELECT_COUNTER0	0x02
-#define I8254_STATUS_NOTREADY	0x40
-static inline u16 i8254(void)
-{
-	u16 status, timer;
-
-	do {
-		outb(I8254_PORT_CONTROL,
-		     I8254_CMD_READBACK | I8254_SELECT_COUNTER0);
-		status = inb(I8254_PORT_COUNTER0);
-		timer  = inb(I8254_PORT_COUNTER0);
-		timer |= inb(I8254_PORT_COUNTER0) << 8;
-	} while (status & I8254_STATUS_NOTREADY);
-
-	return timer;
-}
+#include "random.c"
 
 unsigned long kaslr_get_random_long(const char *purpose)
 {
-#ifdef CONFIG_X86_64
-	const unsigned long mix_const = 0x5d6008cbf3848dd3UL;
-#else
-	const unsigned long mix_const = 0x3f39e593UL;
-#endif
-	unsigned long raw, random = get_boot_seed();
-	bool use_i8254 = true;
-
-	debug_putstr(purpose);
 	debug_putstr(" KASLR using");
-
-	if (has_cpuflag(X86_FEATURE_RDRAND)) {
-		debug_putstr(" RDRAND");
-		if (rdrand_long(&raw)) {
-			random ^= raw;
-			use_i8254 = false;
-		}
-	}
-
-	if (has_cpuflag(X86_FEATURE_TSC)) {
-		debug_putstr(" RDTSC");
-		raw = rdtsc();
-
-		random ^= raw;
-		use_i8254 = false;
-	}
-
-	if (use_i8254) {
-		debug_putstr(" i8254");
-		random ^= i8254();
-	}
-
-	/* Circular multiply for better bit diffusion */
-	asm(_ASM_MUL "%3"
-	    : "=a" (random), "=d" (raw)
-	    : "a" (random), "rm" (mix_const));
-	random += raw;
-
-	debug_putstr("...\n");
-
-	return random;
+	return get_random_long(purpose);
 }
diff --git a/arch/x86/lib/random.c b/arch/x86/lib/random.c
new file mode 100644
index 000000000000..f2fe6a784c98
--- /dev/null
+++ b/arch/x86/lib/random.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <asm/io.h>
+#include <asm/archrandom.h>
+
+#define I8254_PORT_CONTROL	0x43
+#define I8254_PORT_COUNTER0	0x40
+#define I8254_CMD_READBACK	0xC0
+#define I8254_SELECT_COUNTER0	0x02
+#define I8254_STATUS_NOTREADY	0x40
+static inline u16 i8254(void)
+{
+	u16 status, timer;
+
+	do {
+		outb(I8254_PORT_CONTROL,
+		     I8254_CMD_READBACK | I8254_SELECT_COUNTER0);
+		status = inb(I8254_PORT_COUNTER0);
+		timer  = inb(I8254_PORT_COUNTER0);
+		timer |= inb(I8254_PORT_COUNTER0) << 8;
+	} while (status & I8254_STATUS_NOTREADY);
+
+	return timer;
+}
+
+static unsigned long get_random_long(const char *purpose)
+{
+#ifdef CONFIG_X86_64
+	const unsigned long mix_const = 0x5d6008cbf3848dd3UL;
+#else
+	const unsigned long mix_const = 0x3f39e593UL;
+#endif
+	unsigned long raw, random = get_boot_seed();
+	bool use_i8254 = true;
+
+	debug_putstr(purpose);
+
+	if (has_cpuflag(X86_FEATURE_RDRAND)) {
+		debug_putstr(" RDRAND");
+		if (rdrand_long(&raw)) {
+			random ^= raw;
+			use_i8254 = false;
+		}
+	}
+
+	if (has_cpuflag(X86_FEATURE_TSC)) {
+		debug_putstr(" RDTSC");
+		raw = rdtsc();
+
+		random ^= raw;
+		use_i8254 = false;
+	}
+
+	if (use_i8254) {
+		debug_putstr(" i8254");
+		random ^= i8254();
+	}
+
+	/* Circular multiply for better bit diffusion */
+	asm(_ASM_MUL "%3"
+	    : "=a" (random), "=d" (raw)
+	    : "a" (random), "rm" (mix_const));
+	random += raw;
+
+	debug_putstr("...\n");
+
+	return random;
+}
-- 
2.13.6


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

* [PATCH 2/6] efi: the function transfers status to string
  2018-08-05  3:21 ` Lee, Chun-Yi
@ 2018-08-05  3:21   ` Lee, Chun-Yi
  -1 siblings, 0 replies; 30+ messages in thread
From: Lee, Chun-Yi @ 2018-08-05  3:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-efi, x86, keyrings, linux-integrity, Lee, Chun-Yi,
	Kees Cook, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Rafael J. Wysocki, Pavel Machek, Chen Yu, Oliver Neukum,
	Ryan Chen, Ard Biesheuvel, David Howells, Mimi Zohar

This function can be used to transfer EFI status code to string
for printing out debug message.

Cc: Kees Cook <keescook@chromium.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Chen Yu <yu.c.chen@intel.com>
Cc: Oliver Neukum <oneukum@suse.com>
Cc: Ryan Chen <yu.chen.surf@gmail.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
---
 include/linux/efi.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/include/linux/efi.h b/include/linux/efi.h
index 56add823f190..744cf92fe18e 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1651,4 +1651,30 @@ struct linux_efi_tpm_eventlog {
 
 extern int efi_tpm_eventlog_init(void);
 
+#define EFI_STATUS_STR(_status) \
+case EFI_##_status: \
+	return "EFI_" __stringify(_status);
+
+static inline char *
+efi_status_to_str(efi_status_t status)
+{
+	switch (status) {
+	EFI_STATUS_STR(SUCCESS)
+	EFI_STATUS_STR(LOAD_ERROR)
+	EFI_STATUS_STR(INVALID_PARAMETER)
+	EFI_STATUS_STR(UNSUPPORTED)
+	EFI_STATUS_STR(BAD_BUFFER_SIZE)
+	EFI_STATUS_STR(BUFFER_TOO_SMALL)
+	EFI_STATUS_STR(NOT_READY)
+	EFI_STATUS_STR(DEVICE_ERROR)
+	EFI_STATUS_STR(WRITE_PROTECTED)
+	EFI_STATUS_STR(OUT_OF_RESOURCES)
+	EFI_STATUS_STR(NOT_FOUND)
+	EFI_STATUS_STR(ABORTED)
+	EFI_STATUS_STR(SECURITY_VIOLATION)
+	}
+
+	return "";
+}
+
 #endif /* _LINUX_EFI_H */
-- 
2.13.6


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

* [PATCH 2/6] efi: the function transfers status to string
@ 2018-08-05  3:21   ` Lee, Chun-Yi
  0 siblings, 0 replies; 30+ messages in thread
From: Lee, Chun-Yi @ 2018-08-05  3:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-efi, x86, keyrings, linux-integrity, Lee, Chun-Yi,
	Kees Cook, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Rafael J. Wysocki, Pavel Machek, Chen Yu, Oliver Neukum,
	Ryan Chen, Ard Biesheuvel, David Howells, Mimi Zohar

This function can be used to transfer EFI status code to string
for printing out debug message.

Cc: Kees Cook <keescook@chromium.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Chen Yu <yu.c.chen@intel.com>
Cc: Oliver Neukum <oneukum@suse.com>
Cc: Ryan Chen <yu.chen.surf@gmail.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
---
 include/linux/efi.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/include/linux/efi.h b/include/linux/efi.h
index 56add823f190..744cf92fe18e 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1651,4 +1651,30 @@ struct linux_efi_tpm_eventlog {
 
 extern int efi_tpm_eventlog_init(void);
 
+#define EFI_STATUS_STR(_status) \
+case EFI_##_status: \
+	return "EFI_" __stringify(_status);
+
+static inline char *
+efi_status_to_str(efi_status_t status)
+{
+	switch (status) {
+	EFI_STATUS_STR(SUCCESS)
+	EFI_STATUS_STR(LOAD_ERROR)
+	EFI_STATUS_STR(INVALID_PARAMETER)
+	EFI_STATUS_STR(UNSUPPORTED)
+	EFI_STATUS_STR(BAD_BUFFER_SIZE)
+	EFI_STATUS_STR(BUFFER_TOO_SMALL)
+	EFI_STATUS_STR(NOT_READY)
+	EFI_STATUS_STR(DEVICE_ERROR)
+	EFI_STATUS_STR(WRITE_PROTECTED)
+	EFI_STATUS_STR(OUT_OF_RESOURCES)
+	EFI_STATUS_STR(NOT_FOUND)
+	EFI_STATUS_STR(ABORTED)
+	EFI_STATUS_STR(SECURITY_VIOLATION)
+	}
+
+	return "";
+}
+
 #endif /* _LINUX_EFI_H */
-- 
2.13.6


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

* [PATCH 3/6] efi: generate efi root key in EFI boot stub
  2018-08-05  3:21 ` Lee, Chun-Yi
@ 2018-08-05  3:21   ` Lee, Chun-Yi
  -1 siblings, 0 replies; 30+ messages in thread
From: Lee, Chun-Yi @ 2018-08-05  3:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-efi, x86, keyrings, linux-integrity, Lee, Chun-Yi,
	Kees Cook, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Rafael J. Wysocki, Pavel Machek, Chen Yu, Oliver Neukum,
	Ryan Chen, Ard Biesheuvel, David Howells, Mimi Zohar

When secure boot is enabled, only signed EFI binary can access
EFI boot service variable before ExitBootService. Which means that
the EFI boot service variable is secure.

This patch adds functions to EFI boot stub to generate a 512-bit
random number that it can be used as a root key for encryption and
authentication. This root key will be kept in EFI boot service
variable. EFI boot stub reads and transfers ERK (efi root key) to
runtime kernel by setup data.

At runtime, the ERK can be used to encrypted/authentication
other random number to generate EFI secure key. The EFI secure key
can be a new master key type for encrypted key. It's useful for
EVM or hibernation.

Cc: Kees Cook <keescook@chromium.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Chen Yu <yu.c.chen@intel.com>
Cc: Oliver Neukum <oneukum@suse.com>
Cc: Ryan Chen <yu.chen.surf@gmail.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
---
 arch/x86/boot/compressed/Makefile       |   1 +
 arch/x86/boot/compressed/cpuflags.c     |   2 +-
 arch/x86/boot/compressed/eboot.c        |   2 +
 arch/x86/boot/compressed/efi_root_key.c | 212 ++++++++++++++++++++++++++++++++
 arch/x86/boot/compressed/misc.c         |   2 +-
 arch/x86/boot/compressed/misc.h         |  18 +--
 arch/x86/include/asm/efi.h              |  13 ++
 arch/x86/include/uapi/asm/bootparam.h   |   1 +
 arch/x86/kernel/setup.c                 |   3 +
 drivers/firmware/efi/Kconfig            |  23 ++++
 drivers/firmware/efi/Makefile           |   1 +
 drivers/firmware/efi/efi-secure-key.c   |  66 ++++++++++
 include/linux/efi.h                     |  14 +++
 13 files changed, 348 insertions(+), 10 deletions(-)
 create mode 100644 arch/x86/boot/compressed/efi_root_key.c
 create mode 100644 drivers/firmware/efi/efi-secure-key.c

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 169c2feda14a..923ea490cfe7 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -85,6 +85,7 @@ endif
 
 $(obj)/eboot.o: KBUILD_CFLAGS += -fshort-wchar -mno-red-zone
 
+vmlinux-objs-$(CONFIG_EFI_SECURE_KEY) += $(obj)/efi_root_key.o
 vmlinux-objs-$(CONFIG_EFI_STUB) += $(obj)/eboot.o $(obj)/efi_stub_$(BITS).o \
 	$(objtree)/drivers/firmware/efi/libstub/lib.a
 vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_thunk_$(BITS).o
diff --git a/arch/x86/boot/compressed/cpuflags.c b/arch/x86/boot/compressed/cpuflags.c
index 6448a8196d32..4df8f6d634c3 100644
--- a/arch/x86/boot/compressed/cpuflags.c
+++ b/arch/x86/boot/compressed/cpuflags.c
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0
-#ifdef CONFIG_RANDOMIZE_BASE
+#if defined(CONFIG_RANDOMIZE_BASE) || defined(CONFIG_EFI_SECURE_KEY)
 
 #include "../cpuflags.c"
 
diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index e98522ea6f09..fe252b45b17b 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -952,6 +952,8 @@ struct boot_params *efi_main(struct efi_config *c,
 
 	setup_efi_pci(boot_params);
 
+	efi_setup_root_key(sys_table, boot_params);
+
 	setup_quirks(boot_params);
 
 	status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
diff --git a/arch/x86/boot/compressed/efi_root_key.c b/arch/x86/boot/compressed/efi_root_key.c
new file mode 100644
index 000000000000..291aef8b583e
--- /dev/null
+++ b/arch/x86/boot/compressed/efi_root_key.c
@@ -0,0 +1,212 @@
+/* EFI root key generator
+ *
+ * Copyright (C) 2018 Lee, Chun-Yi <jlee@suse.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#include <linux/efi.h>
+#include <asm/efi.h>
+
+#include "misc.h"
+
+static efi_system_table_t *s_table;
+static struct boot_params *b_params;
+
+#ifdef DEBUG
+#define debug_putstr(__x)  efi_printk(s_table, (char *)__x)
+#else
+#define debug_putstr(__x)
+#endif
+
+static void efi_printk_status(char *reason, efi_status_t status)
+{
+	efi_printk(s_table, reason);
+	efi_printk(s_table, efi_status_to_str(status));
+	efi_printk(s_table, "\n");
+}
+
+static unsigned long get_boot_seed(void)
+{
+	unsigned long hash = 0;
+
+	hash = rotate_xor(hash, build_str, sizeof(build_str));
+	hash = rotate_xor(hash, b_params, sizeof(*b_params));
+
+	return hash;
+}
+
+#include "../../lib/random.c"
+
+static void generate_root_key(u8 key[], unsigned int size)
+{
+	unsigned int bfill = size;
+
+	if (key = NULL || !size)
+		return;
+
+	memset(key, 0, size);
+	while (bfill > 0) {
+		unsigned long entropy = 0;
+		unsigned int copy_len = 0;
+		entropy = get_random_long("EFI root key");
+		copy_len = (bfill < sizeof(entropy)) ? bfill : sizeof(entropy);
+		memcpy((void *)(key + size - bfill), &entropy, copy_len);
+		bfill -= copy_len;
+	}
+}
+
+#define get_efi_var(name, vendor, ...) \
+	efi_call_runtime(get_variable, \
+			(efi_char16_t *)(name), (efi_guid_t *)(vendor), \
+			__VA_ARGS__);
+#define set_efi_var(name, vendor, ...) \
+	efi_call_runtime(set_variable, \
+			(efi_char16_t *)(name), (efi_guid_t *)(vendor), \
+			__VA_ARGS__);
+
+static efi_char16_t const root_key_name[] = {
+	'R', 'o', 'o', 't', 'K', 'e', 'y', 0
+};
+#define ROOT_KEY_ATTRIBUTE	(EFI_VARIABLE_NON_VOLATILE | \
+				EFI_VARIABLE_BOOTSERVICE_ACCESS)
+
+static efi_status_t get_root_key(unsigned long *attributes,
+			unsigned long *key_size,
+			struct efi_rkey_setup_data *rkey_setup)
+{
+	void *key_data;
+	efi_status_t status;
+
+	status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
+				*key_size, &key_data);
+	if (status != EFI_SUCCESS) {
+		efi_printk_status("Failed to allocate mem: \n", status);
+		return status;
+	}
+	memset(key_data, 0, *key_size);
+	status = get_efi_var(root_key_name, &EFI_SECURE_GUID,
+			     attributes, key_size, key_data);
+	if (status != EFI_SUCCESS) {
+		efi_printk_status("Failed to get root key: ", status);
+		goto err;
+	}
+
+	memset(rkey_setup->root_key, 0, ROOT_KEY_SIZE);
+	memcpy(rkey_setup->root_key, key_data,
+	       (*key_size >= ROOT_KEY_SIZE) ? ROOT_KEY_SIZE : *key_size);
+err:
+	efi_call_early(free_pool, key_data);
+	return status;
+}
+
+static efi_status_t remove_root_key(unsigned long attributes)
+{
+	efi_status_t status;
+
+	status = set_efi_var(root_key_name,
+			     &EFI_SECURE_GUID, attributes, 0, NULL);
+	if (status = EFI_SUCCESS)
+		efi_printk(s_table, "Removed root key\n");
+	else
+		efi_printk_status("Failed to remove root key: ", status);
+
+	return status;
+}
+
+static efi_status_t create_root_key(struct efi_rkey_setup_data *rkey_setup)
+{
+	efi_status_t status;
+
+	efi_printk(s_table, "Create new root key\n");
+	generate_root_key(rkey_setup->root_key, ROOT_KEY_SIZE);
+	status = set_efi_var(root_key_name, &EFI_SECURE_GUID,
+			     ROOT_KEY_ATTRIBUTE, ROOT_KEY_SIZE,
+			     rkey_setup->root_key);
+	if (status != EFI_SUCCESS)
+		efi_printk_status("Failed to write root key: ", status);
+
+	return status;
+}
+
+static efi_status_t regen_root_key(struct efi_rkey_setup_data *rkey_setup)
+{
+	unsigned long attributes = 0;
+	unsigned long key_size = ROOT_KEY_SIZE;
+	efi_status_t status;
+
+	status = remove_root_key(attributes);
+	if (status = EFI_SUCCESS)
+		status = create_root_key(rkey_setup);
+	if (status = EFI_SUCCESS)
+		status = get_root_key(&attributes, &key_size, rkey_setup);
+}
+
+void efi_setup_root_key(efi_system_table_t *sys_table, struct boot_params *params)
+{
+	struct setup_data *setup_data, *rkey_setup_data;
+	unsigned long setup_size = 0;
+	unsigned long attributes = 0;
+	unsigned long key_size = 0;
+	struct efi_rkey_setup_data *rkey_setup;
+	efi_status_t status;
+
+	s_table = sys_table;
+	b_params = params;
+
+	setup_size = sizeof(struct setup_data) + sizeof(struct efi_rkey_setup_data);
+	status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
+				setup_size, &rkey_setup_data);
+	if (status != EFI_SUCCESS) {
+		efi_printk(s_table, "Failed to allocate mem for root key\n");
+		return;
+	}
+	memset(rkey_setup_data, 0, setup_size);
+	rkey_setup = (struct efi_rkey_setup_data *) rkey_setup_data->data;
+
+	/* detect the size of root key variable */
+	status = get_efi_var(root_key_name, &EFI_SECURE_GUID,
+			     &attributes, &key_size, NULL);
+	rkey_setup->detect_status = status;
+	switch (status) {
+	case EFI_BUFFER_TOO_SMALL:
+		status = get_root_key(&attributes, &key_size, rkey_setup);
+		if (status != EFI_SUCCESS)
+			break;
+		if (attributes != ROOT_KEY_ATTRIBUTE) {
+			efi_printk(sys_table, "Found a unqualified root key\n");
+			status = regen_root_key(rkey_setup);
+		}
+		break;
+
+	case EFI_NOT_FOUND:
+		status = create_root_key(rkey_setup);
+		if (status = EFI_SUCCESS) {
+			key_size = ROOT_KEY_SIZE;
+			status = get_root_key(&attributes, &key_size, rkey_setup);
+		}
+		break;
+
+	default:
+		efi_printk_status("Failed to detect root key's size: ", status);
+	}
+
+	rkey_setup->is_secure +		efi_get_secureboot(sys_table) = efi_secureboot_mode_enabled;
+	rkey_setup->key_size = key_size;
+	rkey_setup->final_status = status;
+
+	rkey_setup_data->type = SETUP_EFI_ROOT_KEY;
+	rkey_setup_data->len = sizeof(struct efi_rkey_setup_data);
+	rkey_setup_data->next = 0;
+	setup_data = (struct setup_data *)params->hdr.setup_data;
+	while (setup_data && setup_data->next)
+		setup_data = (struct setup_data *)setup_data->next;
+	if (setup_data)
+		setup_data->next = (unsigned long)rkey_setup_data;
+	else
+		params->hdr.setup_data = (unsigned long)rkey_setup_data;
+}
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index eb0ab9cad4e4..e01cf1bd2e92 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -427,7 +427,7 @@ void fortify_panic(const char *name)
 	error("detected buffer overflow");
 }
 
-#if CONFIG_RANDOMIZE_BASE
+#if defined(CONFIG_RANDOMIZE_BASE) || defined(CONFIG_EFI_SECURE_KEY)
 unsigned long rotate_xor(unsigned long hash, const void *area,
 			size_t size)
 {
diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index 957f327ad83c..c3950c4ffcd3 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -68,22 +68,24 @@ int cmdline_find_option(const char *option, char *buffer, int bufsize);
 int cmdline_find_option_bool(const char *option);
 #endif
 
-
-#if CONFIG_RANDOMIZE_BASE
+#if defined(CONFIG_RANDOMIZE_BASE) || defined(CONFIG_EFI_SECURE_KEY)
 #include <generated/compile.h>
 #include <generated/utsrelease.h>
-/* kaslr.c */
-void choose_random_location(unsigned long input,
-			    unsigned long input_size,
-			    unsigned long *output,
-			    unsigned long output_size,
-			    unsigned long *virt_addr);
 /* cpuflags.c */
 bool has_cpuflag(int flag);
 /* Simplified build-specific string for starting entropy. */
 static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
 		LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
 unsigned long rotate_xor(unsigned long hash, const void *area, size_t size);
+#endif
+
+#if CONFIG_RANDOMIZE_BASE
+/* kaslr.c */
+void choose_random_location(unsigned long input,
+			    unsigned long input_size,
+			    unsigned long *output,
+			    unsigned long output_size,
+			    unsigned long *virt_addr);
 #else
 static inline void choose_random_location(unsigned long input,
 					  unsigned long input_size,
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index cec5fae23eb3..a25a6da25467 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -172,6 +172,16 @@ static inline bool efi_runtime_supported(void)
 extern struct console early_efi_console;
 extern void parse_efi_setup(u64 phys_addr, u32 data_len);
 
+#ifdef CONFIG_EFI_SECURE_KEY
+extern void efi_setup_root_key(efi_system_table_t *table,
+				struct boot_params *params);
+extern void parse_efi_root_key_setup(u64 phys_addr, u32 data_len);
+#else
+static inline void efi_setup_root_key(efi_system_table_t *table,
+					struct boot_params *params) {}
+static inline void parse_efi_root_key_setup(u64 phys_addr, u32 data_len) {}
+#endif /* CONFIG_EFI_SECURE_KEY */
+
 extern void efifb_setup_from_dmi(struct screen_info *si, const char *opt);
 
 #ifdef CONFIG_EFI_MIXED
@@ -245,6 +255,9 @@ extern bool efi_reboot_required(void);
 
 #else
 static inline void parse_efi_setup(u64 phys_addr, u32 data_len) {}
+static inline void parse_efi_root_key_setup(u64 phys_addr, u32 data_len) {}
+static inline void efi_setup_root_key(efi_system_table_t *table,
+					struct boot_params *params) {}
 static inline bool efi_reboot_required(void)
 {
 	return false;
diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index a06cbf019744..586d824daefe 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -10,6 +10,7 @@
 #define SETUP_EFI			4
 #define SETUP_APPLE_PROPERTIES		5
 #define SETUP_JAILHOUSE			6
+#define SETUP_EFI_ROOT_KEY		7
 
 /* ram_size flags */
 #define RAMDISK_IMAGE_START_MASK	0x07FF
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 2f86d883dd95..0559abdc7648 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -420,6 +420,9 @@ static void __init parse_setup_data(void)
 		case SETUP_EFI:
 			parse_efi_setup(pa_data, data_len);
 			break;
+		case SETUP_EFI_ROOT_KEY:
+			parse_efi_root_key_setup(pa_data, data_len);
+			break;
 		default:
 			break;
 		}
diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
index 781a4a337557..048cf91ae8e8 100644
--- a/drivers/firmware/efi/Kconfig
+++ b/drivers/firmware/efi/Kconfig
@@ -164,6 +164,29 @@ config RESET_ATTACK_MITIGATION
 	  have been evicted, since otherwise it will trigger even on clean
 	  reboots.
 
+config EFI_SECURE_KEY
+	bool "EFI secure key"
+	default n
+	depends on KEYS && EFI_STUB && X86
+        select CRYPTO
+        select CRYPTO_HMAC
+        select CRYPTO_AES
+        select CRYPTO_CBC
+        select CRYPTO_SHA256
+        select CRYPTO_RNG
+	help
+	  This option enables the EFI secure key functions. EFI boot stub
+	  will generate a 512-bit random number that it can be used as
+	  a root key for encryption and authentication. The secure key will
+	  be kept in EFI boot service variable which is secure when secure
+	  boot is enabled.
+	  At runtime, the ERK (efi root key) can be used to encrypt and
+	  authenticate other random number for creating EFI secure key. The
+	  EFI secure key can be a master key type for encrypted key. It
+	  can be used by EVM and hibernation.
+
+	  If unsure, say N.
+
 endmenu
 
 config UEFI_CPER
diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
index 5f9f5039de50..d47149464f82 100644
--- a/drivers/firmware/efi/Makefile
+++ b/drivers/firmware/efi/Makefile
@@ -32,3 +32,4 @@ obj-$(CONFIG_ARM64)			+= $(arm-obj-y)
 obj-$(CONFIG_EFI_CAPSULE_LOADER)	+= capsule-loader.o
 obj-$(CONFIG_UEFI_CPER_ARM)		+= cper-arm.o
 obj-$(CONFIG_UEFI_CPER_X86)		+= cper-x86.o
+obj-$(CONFIG_EFI_SECURE_KEY)		+= efi-secure-key.o
diff --git a/drivers/firmware/efi/efi-secure-key.c b/drivers/firmware/efi/efi-secure-key.c
new file mode 100644
index 000000000000..e56d7d176e03
--- /dev/null
+++ b/drivers/firmware/efi/efi-secure-key.c
@@ -0,0 +1,66 @@
+/* EFI secure key
+ *
+ * Copyright (C) 2018 Lee, Chun-Yi <jlee@suse.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#include <linux/efi.h>
+#include <linux/memblock.h>
+#include <linux/security.h>
+
+static u8 root_key[ROOT_KEY_SIZE];
+static unsigned long rkey_size;
+static bool is_loaded;
+static bool is_secure;
+
+static void __init
+print_efi_rkey_setup_data(struct efi_rkey_setup_data *rkey_setup)
+{
+	pr_debug("EFI root key detection status: %s 0x%lx\n",
+		efi_status_to_str(rkey_setup->detect_status),
+		rkey_setup->detect_status);
+	pr_debug("EFI root key getting status: %s 0x%lx\n",
+		efi_status_to_str(rkey_setup->final_status),
+		rkey_setup->final_status);
+	pr_debug("EFI root key size: %ld\n", rkey_setup->key_size);
+
+	if (rkey_setup->final_status != EFI_SUCCESS) {
+		pr_warn("EFI root key getting failed: %s 0x%lx\n",
+			efi_status_to_str(rkey_setup->final_status),
+			rkey_setup->final_status);
+	} else if (rkey_setup->key_size < ROOT_KEY_SIZE) {
+		pr_warn(KERN_CONT "EFI root key size %ld is less than %d.\n",
+			rkey_setup->key_size, ROOT_KEY_SIZE);
+	}
+}
+
+void __init parse_efi_root_key_setup(u64 phys_addr, u32 data_len)
+{
+	struct efi_rkey_setup_data *rkey_setup;
+	void *setup_data;
+
+	setup_data = early_memremap(phys_addr, data_len);
+	rkey_setup = setup_data + sizeof(struct setup_data);
+	print_efi_rkey_setup_data(rkey_setup);
+
+	/* keep efi root key */
+	if (rkey_setup->final_status = EFI_SUCCESS) {
+		memcpy(root_key, rkey_setup->root_key, rkey_setup->key_size);
+		rkey_size = rkey_setup->key_size;
+		is_loaded = true;
+		is_secure = rkey_setup->is_secure;
+		pr_info("EFI root key is loaded.\n");
+		if (!is_secure) {
+			pr_warn("EFI root key is insecure when no secure boot.\n");
+		}
+	}
+
+	/* erase setup data */
+	memzero_explicit(setup_data,
+		sizeof(struct setup_data) + sizeof(struct efi_rkey_setup_data));
+	early_iounmap(setup_data, data_len);
+}
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 744cf92fe18e..f9fd273ef544 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1677,4 +1677,18 @@ efi_status_to_str(efi_status_t status)
 	return "";
 }
 
+#ifdef CONFIG_EFI_SECURE_KEY
+#define EFI_SECURE_GUID \
+	EFI_GUID(0x8c136d32, 0x039a, 0x4016, 0x8b, 0xb4, 0x9e, 0x98, 0x5e, 0x62, 0x78, 0x6f)
+#define ROOT_KEY_SIZE        64
+struct efi_rkey_setup_data {
+	bool is_secure;
+	unsigned long detect_status;
+	unsigned long final_status;
+	unsigned long key_size;
+	u8 root_key[ROOT_KEY_SIZE];
+};
+#else
+#define ROOT_KEY_SIZE        0
+#endif /* CONFIG_EFI_SECURE_KEY */
 #endif /* _LINUX_EFI_H */
-- 
2.13.6


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

* [PATCH 3/6] efi: generate efi root key in EFI boot stub
@ 2018-08-05  3:21   ` Lee, Chun-Yi
  0 siblings, 0 replies; 30+ messages in thread
From: Lee, Chun-Yi @ 2018-08-05  3:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-efi, x86, keyrings, linux-integrity, Lee, Chun-Yi,
	Kees Cook, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Rafael J. Wysocki, Pavel Machek, Chen Yu, Oliver Neukum,
	Ryan Chen, Ard Biesheuvel, David Howells, Mimi Zohar

When secure boot is enabled, only signed EFI binary can access
EFI boot service variable before ExitBootService. Which means that
the EFI boot service variable is secure.

This patch adds functions to EFI boot stub to generate a 512-bit
random number that it can be used as a root key for encryption and
authentication. This root key will be kept in EFI boot service
variable. EFI boot stub reads and transfers ERK (efi root key) to
runtime kernel by setup data.

At runtime, the ERK can be used to encrypted/authentication
other random number to generate EFI secure key. The EFI secure key
can be a new master key type for encrypted key. It's useful for
EVM or hibernation.

Cc: Kees Cook <keescook@chromium.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Chen Yu <yu.c.chen@intel.com>
Cc: Oliver Neukum <oneukum@suse.com>
Cc: Ryan Chen <yu.chen.surf@gmail.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
---
 arch/x86/boot/compressed/Makefile       |   1 +
 arch/x86/boot/compressed/cpuflags.c     |   2 +-
 arch/x86/boot/compressed/eboot.c        |   2 +
 arch/x86/boot/compressed/efi_root_key.c | 212 ++++++++++++++++++++++++++++++++
 arch/x86/boot/compressed/misc.c         |   2 +-
 arch/x86/boot/compressed/misc.h         |  18 +--
 arch/x86/include/asm/efi.h              |  13 ++
 arch/x86/include/uapi/asm/bootparam.h   |   1 +
 arch/x86/kernel/setup.c                 |   3 +
 drivers/firmware/efi/Kconfig            |  23 ++++
 drivers/firmware/efi/Makefile           |   1 +
 drivers/firmware/efi/efi-secure-key.c   |  66 ++++++++++
 include/linux/efi.h                     |  14 +++
 13 files changed, 348 insertions(+), 10 deletions(-)
 create mode 100644 arch/x86/boot/compressed/efi_root_key.c
 create mode 100644 drivers/firmware/efi/efi-secure-key.c

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 169c2feda14a..923ea490cfe7 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -85,6 +85,7 @@ endif
 
 $(obj)/eboot.o: KBUILD_CFLAGS += -fshort-wchar -mno-red-zone
 
+vmlinux-objs-$(CONFIG_EFI_SECURE_KEY) += $(obj)/efi_root_key.o
 vmlinux-objs-$(CONFIG_EFI_STUB) += $(obj)/eboot.o $(obj)/efi_stub_$(BITS).o \
 	$(objtree)/drivers/firmware/efi/libstub/lib.a
 vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_thunk_$(BITS).o
diff --git a/arch/x86/boot/compressed/cpuflags.c b/arch/x86/boot/compressed/cpuflags.c
index 6448a8196d32..4df8f6d634c3 100644
--- a/arch/x86/boot/compressed/cpuflags.c
+++ b/arch/x86/boot/compressed/cpuflags.c
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0
-#ifdef CONFIG_RANDOMIZE_BASE
+#if defined(CONFIG_RANDOMIZE_BASE) || defined(CONFIG_EFI_SECURE_KEY)
 
 #include "../cpuflags.c"
 
diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index e98522ea6f09..fe252b45b17b 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -952,6 +952,8 @@ struct boot_params *efi_main(struct efi_config *c,
 
 	setup_efi_pci(boot_params);
 
+	efi_setup_root_key(sys_table, boot_params);
+
 	setup_quirks(boot_params);
 
 	status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
diff --git a/arch/x86/boot/compressed/efi_root_key.c b/arch/x86/boot/compressed/efi_root_key.c
new file mode 100644
index 000000000000..291aef8b583e
--- /dev/null
+++ b/arch/x86/boot/compressed/efi_root_key.c
@@ -0,0 +1,212 @@
+/* EFI root key generator
+ *
+ * Copyright (C) 2018 Lee, Chun-Yi <jlee@suse.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#include <linux/efi.h>
+#include <asm/efi.h>
+
+#include "misc.h"
+
+static efi_system_table_t *s_table;
+static struct boot_params *b_params;
+
+#ifdef DEBUG
+#define debug_putstr(__x)  efi_printk(s_table, (char *)__x)
+#else
+#define debug_putstr(__x)
+#endif
+
+static void efi_printk_status(char *reason, efi_status_t status)
+{
+	efi_printk(s_table, reason);
+	efi_printk(s_table, efi_status_to_str(status));
+	efi_printk(s_table, "\n");
+}
+
+static unsigned long get_boot_seed(void)
+{
+	unsigned long hash = 0;
+
+	hash = rotate_xor(hash, build_str, sizeof(build_str));
+	hash = rotate_xor(hash, b_params, sizeof(*b_params));
+
+	return hash;
+}
+
+#include "../../lib/random.c"
+
+static void generate_root_key(u8 key[], unsigned int size)
+{
+	unsigned int bfill = size;
+
+	if (key == NULL || !size)
+		return;
+
+	memset(key, 0, size);
+	while (bfill > 0) {
+		unsigned long entropy = 0;
+		unsigned int copy_len = 0;
+		entropy = get_random_long("EFI root key");
+		copy_len = (bfill < sizeof(entropy)) ? bfill : sizeof(entropy);
+		memcpy((void *)(key + size - bfill), &entropy, copy_len);
+		bfill -= copy_len;
+	}
+}
+
+#define get_efi_var(name, vendor, ...) \
+	efi_call_runtime(get_variable, \
+			(efi_char16_t *)(name), (efi_guid_t *)(vendor), \
+			__VA_ARGS__);
+#define set_efi_var(name, vendor, ...) \
+	efi_call_runtime(set_variable, \
+			(efi_char16_t *)(name), (efi_guid_t *)(vendor), \
+			__VA_ARGS__);
+
+static efi_char16_t const root_key_name[] = {
+	'R', 'o', 'o', 't', 'K', 'e', 'y', 0
+};
+#define ROOT_KEY_ATTRIBUTE	(EFI_VARIABLE_NON_VOLATILE | \
+				EFI_VARIABLE_BOOTSERVICE_ACCESS)
+
+static efi_status_t get_root_key(unsigned long *attributes,
+			unsigned long *key_size,
+			struct efi_rkey_setup_data *rkey_setup)
+{
+	void *key_data;
+	efi_status_t status;
+
+	status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
+				*key_size, &key_data);
+	if (status != EFI_SUCCESS) {
+		efi_printk_status("Failed to allocate mem: \n", status);
+		return status;
+	}
+	memset(key_data, 0, *key_size);
+	status = get_efi_var(root_key_name, &EFI_SECURE_GUID,
+			     attributes, key_size, key_data);
+	if (status != EFI_SUCCESS) {
+		efi_printk_status("Failed to get root key: ", status);
+		goto err;
+	}
+
+	memset(rkey_setup->root_key, 0, ROOT_KEY_SIZE);
+	memcpy(rkey_setup->root_key, key_data,
+	       (*key_size >= ROOT_KEY_SIZE) ? ROOT_KEY_SIZE : *key_size);
+err:
+	efi_call_early(free_pool, key_data);
+	return status;
+}
+
+static efi_status_t remove_root_key(unsigned long attributes)
+{
+	efi_status_t status;
+
+	status = set_efi_var(root_key_name,
+			     &EFI_SECURE_GUID, attributes, 0, NULL);
+	if (status == EFI_SUCCESS)
+		efi_printk(s_table, "Removed root key\n");
+	else
+		efi_printk_status("Failed to remove root key: ", status);
+
+	return status;
+}
+
+static efi_status_t create_root_key(struct efi_rkey_setup_data *rkey_setup)
+{
+	efi_status_t status;
+
+	efi_printk(s_table, "Create new root key\n");
+	generate_root_key(rkey_setup->root_key, ROOT_KEY_SIZE);
+	status = set_efi_var(root_key_name, &EFI_SECURE_GUID,
+			     ROOT_KEY_ATTRIBUTE, ROOT_KEY_SIZE,
+			     rkey_setup->root_key);
+	if (status != EFI_SUCCESS)
+		efi_printk_status("Failed to write root key: ", status);
+
+	return status;
+}
+
+static efi_status_t regen_root_key(struct efi_rkey_setup_data *rkey_setup)
+{
+	unsigned long attributes = 0;
+	unsigned long key_size = ROOT_KEY_SIZE;
+	efi_status_t status;
+
+	status = remove_root_key(attributes);
+	if (status == EFI_SUCCESS)
+		status = create_root_key(rkey_setup);
+	if (status == EFI_SUCCESS)
+		status = get_root_key(&attributes, &key_size, rkey_setup);
+}
+
+void efi_setup_root_key(efi_system_table_t *sys_table, struct boot_params *params)
+{
+	struct setup_data *setup_data, *rkey_setup_data;
+	unsigned long setup_size = 0;
+	unsigned long attributes = 0;
+	unsigned long key_size = 0;
+	struct efi_rkey_setup_data *rkey_setup;
+	efi_status_t status;
+
+	s_table = sys_table;
+	b_params = params;
+
+	setup_size = sizeof(struct setup_data) + sizeof(struct efi_rkey_setup_data);
+	status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
+				setup_size, &rkey_setup_data);
+	if (status != EFI_SUCCESS) {
+		efi_printk(s_table, "Failed to allocate mem for root key\n");
+		return;
+	}
+	memset(rkey_setup_data, 0, setup_size);
+	rkey_setup = (struct efi_rkey_setup_data *) rkey_setup_data->data;
+
+	/* detect the size of root key variable */
+	status = get_efi_var(root_key_name, &EFI_SECURE_GUID,
+			     &attributes, &key_size, NULL);
+	rkey_setup->detect_status = status;
+	switch (status) {
+	case EFI_BUFFER_TOO_SMALL:
+		status = get_root_key(&attributes, &key_size, rkey_setup);
+		if (status != EFI_SUCCESS)
+			break;
+		if (attributes != ROOT_KEY_ATTRIBUTE) {
+			efi_printk(sys_table, "Found a unqualified root key\n");
+			status = regen_root_key(rkey_setup);
+		}
+		break;
+
+	case EFI_NOT_FOUND:
+		status = create_root_key(rkey_setup);
+		if (status == EFI_SUCCESS) {
+			key_size = ROOT_KEY_SIZE;
+			status = get_root_key(&attributes, &key_size, rkey_setup);
+		}
+		break;
+
+	default:
+		efi_printk_status("Failed to detect root key's size: ", status);
+	}
+
+	rkey_setup->is_secure =
+		efi_get_secureboot(sys_table) == efi_secureboot_mode_enabled;
+	rkey_setup->key_size = key_size;
+	rkey_setup->final_status = status;
+
+	rkey_setup_data->type = SETUP_EFI_ROOT_KEY;
+	rkey_setup_data->len = sizeof(struct efi_rkey_setup_data);
+	rkey_setup_data->next = 0;
+	setup_data = (struct setup_data *)params->hdr.setup_data;
+	while (setup_data && setup_data->next)
+		setup_data = (struct setup_data *)setup_data->next;
+	if (setup_data)
+		setup_data->next = (unsigned long)rkey_setup_data;
+	else
+		params->hdr.setup_data = (unsigned long)rkey_setup_data;
+}
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index eb0ab9cad4e4..e01cf1bd2e92 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -427,7 +427,7 @@ void fortify_panic(const char *name)
 	error("detected buffer overflow");
 }
 
-#if CONFIG_RANDOMIZE_BASE
+#if defined(CONFIG_RANDOMIZE_BASE) || defined(CONFIG_EFI_SECURE_KEY)
 unsigned long rotate_xor(unsigned long hash, const void *area,
 			size_t size)
 {
diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index 957f327ad83c..c3950c4ffcd3 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -68,22 +68,24 @@ int cmdline_find_option(const char *option, char *buffer, int bufsize);
 int cmdline_find_option_bool(const char *option);
 #endif
 
-
-#if CONFIG_RANDOMIZE_BASE
+#if defined(CONFIG_RANDOMIZE_BASE) || defined(CONFIG_EFI_SECURE_KEY)
 #include <generated/compile.h>
 #include <generated/utsrelease.h>
-/* kaslr.c */
-void choose_random_location(unsigned long input,
-			    unsigned long input_size,
-			    unsigned long *output,
-			    unsigned long output_size,
-			    unsigned long *virt_addr);
 /* cpuflags.c */
 bool has_cpuflag(int flag);
 /* Simplified build-specific string for starting entropy. */
 static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
 		LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
 unsigned long rotate_xor(unsigned long hash, const void *area, size_t size);
+#endif
+
+#if CONFIG_RANDOMIZE_BASE
+/* kaslr.c */
+void choose_random_location(unsigned long input,
+			    unsigned long input_size,
+			    unsigned long *output,
+			    unsigned long output_size,
+			    unsigned long *virt_addr);
 #else
 static inline void choose_random_location(unsigned long input,
 					  unsigned long input_size,
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index cec5fae23eb3..a25a6da25467 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -172,6 +172,16 @@ static inline bool efi_runtime_supported(void)
 extern struct console early_efi_console;
 extern void parse_efi_setup(u64 phys_addr, u32 data_len);
 
+#ifdef CONFIG_EFI_SECURE_KEY
+extern void efi_setup_root_key(efi_system_table_t *table,
+				struct boot_params *params);
+extern void parse_efi_root_key_setup(u64 phys_addr, u32 data_len);
+#else
+static inline void efi_setup_root_key(efi_system_table_t *table,
+					struct boot_params *params) {}
+static inline void parse_efi_root_key_setup(u64 phys_addr, u32 data_len) {}
+#endif /* CONFIG_EFI_SECURE_KEY */
+
 extern void efifb_setup_from_dmi(struct screen_info *si, const char *opt);
 
 #ifdef CONFIG_EFI_MIXED
@@ -245,6 +255,9 @@ extern bool efi_reboot_required(void);
 
 #else
 static inline void parse_efi_setup(u64 phys_addr, u32 data_len) {}
+static inline void parse_efi_root_key_setup(u64 phys_addr, u32 data_len) {}
+static inline void efi_setup_root_key(efi_system_table_t *table,
+					struct boot_params *params) {}
 static inline bool efi_reboot_required(void)
 {
 	return false;
diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index a06cbf019744..586d824daefe 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -10,6 +10,7 @@
 #define SETUP_EFI			4
 #define SETUP_APPLE_PROPERTIES		5
 #define SETUP_JAILHOUSE			6
+#define SETUP_EFI_ROOT_KEY		7
 
 /* ram_size flags */
 #define RAMDISK_IMAGE_START_MASK	0x07FF
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 2f86d883dd95..0559abdc7648 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -420,6 +420,9 @@ static void __init parse_setup_data(void)
 		case SETUP_EFI:
 			parse_efi_setup(pa_data, data_len);
 			break;
+		case SETUP_EFI_ROOT_KEY:
+			parse_efi_root_key_setup(pa_data, data_len);
+			break;
 		default:
 			break;
 		}
diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
index 781a4a337557..048cf91ae8e8 100644
--- a/drivers/firmware/efi/Kconfig
+++ b/drivers/firmware/efi/Kconfig
@@ -164,6 +164,29 @@ config RESET_ATTACK_MITIGATION
 	  have been evicted, since otherwise it will trigger even on clean
 	  reboots.
 
+config EFI_SECURE_KEY
+	bool "EFI secure key"
+	default n
+	depends on KEYS && EFI_STUB && X86
+        select CRYPTO
+        select CRYPTO_HMAC
+        select CRYPTO_AES
+        select CRYPTO_CBC
+        select CRYPTO_SHA256
+        select CRYPTO_RNG
+	help
+	  This option enables the EFI secure key functions. EFI boot stub
+	  will generate a 512-bit random number that it can be used as
+	  a root key for encryption and authentication. The secure key will
+	  be kept in EFI boot service variable which is secure when secure
+	  boot is enabled.
+	  At runtime, the ERK (efi root key) can be used to encrypt and
+	  authenticate other random number for creating EFI secure key. The
+	  EFI secure key can be a master key type for encrypted key. It
+	  can be used by EVM and hibernation.
+
+	  If unsure, say N.
+
 endmenu
 
 config UEFI_CPER
diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
index 5f9f5039de50..d47149464f82 100644
--- a/drivers/firmware/efi/Makefile
+++ b/drivers/firmware/efi/Makefile
@@ -32,3 +32,4 @@ obj-$(CONFIG_ARM64)			+= $(arm-obj-y)
 obj-$(CONFIG_EFI_CAPSULE_LOADER)	+= capsule-loader.o
 obj-$(CONFIG_UEFI_CPER_ARM)		+= cper-arm.o
 obj-$(CONFIG_UEFI_CPER_X86)		+= cper-x86.o
+obj-$(CONFIG_EFI_SECURE_KEY)		+= efi-secure-key.o
diff --git a/drivers/firmware/efi/efi-secure-key.c b/drivers/firmware/efi/efi-secure-key.c
new file mode 100644
index 000000000000..e56d7d176e03
--- /dev/null
+++ b/drivers/firmware/efi/efi-secure-key.c
@@ -0,0 +1,66 @@
+/* EFI secure key
+ *
+ * Copyright (C) 2018 Lee, Chun-Yi <jlee@suse.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#include <linux/efi.h>
+#include <linux/memblock.h>
+#include <linux/security.h>
+
+static u8 root_key[ROOT_KEY_SIZE];
+static unsigned long rkey_size;
+static bool is_loaded;
+static bool is_secure;
+
+static void __init
+print_efi_rkey_setup_data(struct efi_rkey_setup_data *rkey_setup)
+{
+	pr_debug("EFI root key detection status: %s 0x%lx\n",
+		efi_status_to_str(rkey_setup->detect_status),
+		rkey_setup->detect_status);
+	pr_debug("EFI root key getting status: %s 0x%lx\n",
+		efi_status_to_str(rkey_setup->final_status),
+		rkey_setup->final_status);
+	pr_debug("EFI root key size: %ld\n", rkey_setup->key_size);
+
+	if (rkey_setup->final_status != EFI_SUCCESS) {
+		pr_warn("EFI root key getting failed: %s 0x%lx\n",
+			efi_status_to_str(rkey_setup->final_status),
+			rkey_setup->final_status);
+	} else if (rkey_setup->key_size < ROOT_KEY_SIZE) {
+		pr_warn(KERN_CONT "EFI root key size %ld is less than %d.\n",
+			rkey_setup->key_size, ROOT_KEY_SIZE);
+	}
+}
+
+void __init parse_efi_root_key_setup(u64 phys_addr, u32 data_len)
+{
+	struct efi_rkey_setup_data *rkey_setup;
+	void *setup_data;
+
+	setup_data = early_memremap(phys_addr, data_len);
+	rkey_setup = setup_data + sizeof(struct setup_data);
+	print_efi_rkey_setup_data(rkey_setup);
+
+	/* keep efi root key */
+	if (rkey_setup->final_status == EFI_SUCCESS) {
+		memcpy(root_key, rkey_setup->root_key, rkey_setup->key_size);
+		rkey_size = rkey_setup->key_size;
+		is_loaded = true;
+		is_secure = rkey_setup->is_secure;
+		pr_info("EFI root key is loaded.\n");
+		if (!is_secure) {
+			pr_warn("EFI root key is insecure when no secure boot.\n");
+		}
+	}
+
+	/* erase setup data */
+	memzero_explicit(setup_data,
+		sizeof(struct setup_data) + sizeof(struct efi_rkey_setup_data));
+	early_iounmap(setup_data, data_len);
+}
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 744cf92fe18e..f9fd273ef544 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1677,4 +1677,18 @@ efi_status_to_str(efi_status_t status)
 	return "";
 }
 
+#ifdef CONFIG_EFI_SECURE_KEY
+#define EFI_SECURE_GUID \
+	EFI_GUID(0x8c136d32, 0x039a, 0x4016, 0x8b, 0xb4, 0x9e, 0x98, 0x5e, 0x62, 0x78, 0x6f)
+#define ROOT_KEY_SIZE        64
+struct efi_rkey_setup_data {
+	bool is_secure;
+	unsigned long detect_status;
+	unsigned long final_status;
+	unsigned long key_size;
+	u8 root_key[ROOT_KEY_SIZE];
+};
+#else
+#define ROOT_KEY_SIZE        0
+#endif /* CONFIG_EFI_SECURE_KEY */
 #endif /* _LINUX_EFI_H */
-- 
2.13.6


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

* [PATCH 4/6] key: add EFI secure key type
  2018-08-05  3:21 ` Lee, Chun-Yi
@ 2018-08-05  3:21   ` Lee, Chun-Yi
  -1 siblings, 0 replies; 30+ messages in thread
From: Lee, Chun-Yi @ 2018-08-05  3:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-efi, x86, keyrings, linux-integrity, Lee, Chun-Yi,
	Kees Cook, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Rafael J. Wysocki, Pavel Machek, Chen Yu, Oliver Neukum,
	Ryan Chen, Ard Biesheuvel, David Howells, Mimi Zohar

EFI secure key is a new key type that the key is encrypted and
authenticated by the EFI root key. The ERK (EFI root key) is
generated by EFI boot stub and be stored in EFI boot services
variable before ExitBootServices be called.

When user enabled the secure boot in firmware, the EFI root key is
secure which means that the encrypted EFI secure key is also secure.

A EFI secure key is generated by kernel and can be keep by user
space. It is also a new master key type like trusted key(TPM) or
user key. The EFI key can be used by hibernation encryption and
authentication. And it can also be a master key to generate a
encrypted key for EVM.

The layout of the efi key payload is:

	      -+-+-----------------------+
	       | |                       |
	       | |                       |
	       | |                       |
      key_len->| |         key           |
	       | |                       |
	       | |                       |
	      -+-+-----------------------+----+-
	       | |     key_len           |    |
	       | |(decrypted key length) |    |
	       | |     (string)          |    |
	       | |                       |    |
	       | +-----------------------|    |
 datablob_len->| |                       |    |
	       | |      ERK hash         | datablob
	       | |                       |    |
	       | +-----------------------|    |
	       | |          iv           |    |
	       | |(initialization vector)|    |
	       | +-----------------------|    |
	       | |                       |    |
	       | |                       |    |
	       | |                       |    |
	       | |    encrypted_key      |    |
	       | |                       |    |
	       | |                       |    |
	      -+-+-----------------------+----+-
		 |        hmac           |
		 |(signature of datablob)|
		 +-----------------------+

The datablob can be exported to user space with hmac.

As other key type, EFI key can be created by keyctl tool:

e.g. keyctl add efi key-name "new 128" @u

EFI secure key can also be dumped to a file by keyctl:
e.g. keyctl pipe $NUMBER > ~/tmp/key-name.blob

The $NUMBER is the key serial number.

Enroll the key blob back to kernel:
e.g. keyctl add efi key-name "load `cat ~/tmp/key-name.blob`" @u

Cc: Kees Cook <keescook@chromium.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Chen Yu <yu.c.chen@intel.com>
Cc: Oliver Neukum <oneukum@suse.com>
Cc: Ryan Chen <yu.chen.surf@gmail.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
---
 drivers/firmware/efi/efi-secure-key.c | 636 ++++++++++++++++++++++++++++++++++
 include/keys/efi-type.h               |  50 +++
 2 files changed, 686 insertions(+)
 create mode 100644 include/keys/efi-type.h

diff --git a/drivers/firmware/efi/efi-secure-key.c b/drivers/firmware/efi/efi-secure-key.c
index e56d7d176e03..5e72a8c9e13e 100644
--- a/drivers/firmware/efi/efi-secure-key.c
+++ b/drivers/firmware/efi/efi-secure-key.c
@@ -11,6 +11,16 @@
 #include <linux/efi.h>
 #include <linux/memblock.h>
 #include <linux/security.h>
+#include <linux/key-type.h>
+#include <linux/parser.h>
+#include <linux/random.h>
+#include <linux/scatterlist.h>
+#include <keys/efi-type.h>
+#include <keys/user-type.h>
+#include <crypto/algapi.h>
+#include <crypto/hash.h>
+#include <crypto/sha.h>
+#include <crypto/skcipher.h>
 
 static u8 root_key[ROOT_KEY_SIZE];
 static unsigned long rkey_size;
@@ -64,3 +74,629 @@ void __init parse_efi_root_key_setup(u64 phys_addr, u32 data_len)
 		sizeof(struct setup_data) + sizeof(struct efi_rkey_setup_data));
 	early_iounmap(setup_data, data_len);
 }
+
+#define ERK_HASH_SIZE SHA256_DIGEST_SIZE
+#define HMAC_HASH_SIZE SHA256_DIGEST_SIZE
+#define DKEY_SIZE SHA256_DIGEST_SIZE
+
+struct key_type key_type_efi;
+
+static const char hash_alg[] = "sha256";
+static const char hmac_alg[] = "hmac(sha256)";
+static struct crypto_shash *hash_tfm;
+
+static int calc_hash(struct crypto_shash *tfm, const u8 *buf,
+		     unsigned int buflen, u8 *digest)
+{
+	SHASH_DESC_ON_STACK(desc, tfm);
+	int ret;
+
+	desc->tfm = tfm;
+	desc->flags = 0;
+
+	ret = crypto_shash_digest(desc, buf, buflen, digest);
+	shash_desc_zero(desc);
+
+	return ret;
+}
+
+static int get_derived_key(const char *salt, u8 *derived_key)
+{
+	u8 *derived_buf;
+	unsigned int derived_buf_len;
+	int ret;
+
+	derived_buf_len = strlen(salt) + 1 + ROOT_KEY_SIZE;
+	if (derived_buf_len < DKEY_SIZE)
+		derived_buf_len = DKEY_SIZE;
+
+	derived_buf = kzalloc(derived_buf_len, GFP_KERNEL);
+	if (!derived_buf)
+		return -ENOMEM;
+
+	memcpy(derived_buf + strlen(derived_buf) + 1, root_key,
+		ROOT_KEY_SIZE);
+	ret = calc_hash(hash_tfm, derived_buf, derived_buf_len, derived_key);
+	memzero_explicit(derived_buf, derived_buf_len);
+
+	return ret;
+}
+
+static int calc_hmac(const u8 *buf, unsigned int buflen, u8 *digest)
+{
+	struct crypto_shash *tfm;
+	u8 *auth_key;
+	int ret;
+
+	auth_key = kzalloc(DKEY_SIZE, GFP_KERNEL);
+	if (!auth_key)
+		return -ENOMEM;
+
+	tfm = crypto_alloc_shash(hmac_alg, 0, CRYPTO_ALG_ASYNC);
+	if (IS_ERR(tfm)) {
+		pr_err("can't alloc %s transform: %ld\n",
+			hmac_alg, PTR_ERR(tfm));
+		ret = PTR_ERR(tfm);
+		goto tfm_fail;
+	}
+
+	ret = get_derived_key("AUTH_KEY", auth_key);
+	if (ret)
+		goto key_fail;
+
+	ret = crypto_shash_setkey(tfm, auth_key, DKEY_SIZE);
+	if (!ret)
+		ret = calc_hash(tfm, buf, buflen, digest);
+
+key_fail:
+	crypto_free_shash(tfm);
+tfm_fail:
+	memzero_explicit(auth_key, DKEY_SIZE);
+
+	return ret;
+}
+
+static const char blkcipher_alg[] = "cbc(aes)";
+static unsigned int ivsize;
+static int blksize;
+
+static int set_aes_sizes(void)
+{
+	struct crypto_skcipher *tfm;
+
+	tfm = crypto_alloc_skcipher(blkcipher_alg, 0, CRYPTO_ALG_ASYNC);
+	if (IS_ERR(tfm)) {
+		pr_err("failed to alloc_cipher (%ld)\n", PTR_ERR(tfm));
+		return PTR_ERR(tfm);
+	}
+	ivsize = crypto_skcipher_ivsize(tfm);
+	blksize = crypto_skcipher_blocksize(tfm);
+	crypto_free_skcipher(tfm);
+
+	return 0;
+}
+
+enum {
+	Opt_err = -1,
+	Opt_new, Opt_load,
+};
+
+static const match_table_t key_tokens = {
+	{Opt_new, "new"},
+	{Opt_load, "load"},
+	{Opt_err, NULL}
+};
+
+static struct efi_key_payload *efi_payload_alloc(struct key *key, char *key_len_str)
+{
+	struct efi_key_payload *ekp = NULL;
+	unsigned short encrypted_keylen;
+	unsigned short datablob_len;
+	unsigned short payload_len;
+	long key_len;
+	int ret;
+
+	ret = kstrtol(key_len_str, 10, &key_len);
+	if (ret < 0)
+		return ERR_PTR(ret);
+	encrypted_keylen = roundup(key_len, blksize);
+
+	/* efi_key_payload + key + datablob + hmac */
+	datablob_len = strlen(key_len_str) + 1 + ERK_HASH_SIZE + ivsize + encrypted_keylen;
+	payload_len = sizeof(*ekp) + key_len + datablob_len + HMAC_HASH_SIZE;
+
+	ret = key_payload_reserve(key, payload_len);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	ekp = kzalloc(payload_len, GFP_KERNEL);
+	if (!ekp)
+		return ERR_PTR(-ENOMEM);
+	ekp->key = ekp->payload_data;
+	ekp->datablob = ekp->key + key_len;
+	ekp->key_len_str = ekp->datablob;
+	ekp->erk_hash = ekp->key_len_str + strlen(key_len_str) + 1;
+	ekp->iv = ekp->erk_hash + ERK_HASH_SIZE;
+	ekp->encrypted_key = ekp->iv + ivsize;
+	ekp->hmac = ekp->encrypted_key + encrypted_keylen;
+	ekp->key_len = key_len;
+	ekp->datablob_len = datablob_len;
+
+	memcpy(ekp->key_len_str, key_len_str, strlen(key_len_str));
+
+	return ekp;
+}
+
+/*
+ * datablob_parse - parse the keyctl data and fill in the
+ *                  payload and options structures
+ *
+ * On success returns command number, otherwise -EINVAL.
+ */
+static int datablob_parse(char *datablob, char **key_len_str, char **hex_encoded_blob)
+{
+	substring_t args[MAX_OPT_ARGS];
+	long key_len;
+	int key_cmd;
+	int ret;
+	char *c;
+
+	/* main command */
+	c = strsep(&datablob, " \t");
+	if (!c)
+		return -EINVAL;
+	key_cmd = match_token(c, key_tokens, args);
+
+	/* first string argument is key length */
+	c = strsep(&datablob, " \t");
+	if (!c)
+		return -EINVAL;
+	*key_len_str = c;
+	ret = kstrtol(*key_len_str, 10, &key_len);
+	if (ret < 0 || key_len < MIN_KEY_SIZE || key_len > MAX_KEY_SIZE)
+		return -EINVAL;
+
+	switch (key_cmd) {
+	case Opt_new:
+		ret = Opt_new;
+		break;
+	case Opt_load:
+		*hex_encoded_blob = strsep(&datablob, " \t");
+		if (!*hex_encoded_blob) {
+			pr_info("hex blob is missing\n");
+			return -EINVAL;
+		}
+		if (strlen(*hex_encoded_blob) / 2 > MAX_BLOB_SIZE)
+			return -EINVAL;
+		ret = Opt_load;
+		break;
+	case Opt_err:
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+static int key_encrypt(struct efi_key_payload *ekp, size_t encrypted_keylen)
+{
+	struct scatterlist src[1], dst[1];
+	struct crypto_skcipher *tfm;
+	struct skcipher_request *req;
+	u8 *encrypted_key_tmp;
+	u8 *iv_tmp, *enc_key;
+	int ret;
+
+	encrypted_key_tmp = kzalloc(encrypted_keylen, GFP_KERNEL);
+	if (!encrypted_key_tmp)
+		return -ENOMEM;
+
+	enc_key = kzalloc(DKEY_SIZE, GFP_KERNEL);
+	if (!enc_key) {
+		ret = -ENOMEM;
+		goto key_fail;
+	}
+
+	iv_tmp = kmemdup(ekp->iv, ivsize, GFP_KERNEL);
+	if (!iv_tmp) {
+		ret = -ENOMEM;
+		goto iv_fail;
+	}
+
+	tfm = crypto_alloc_skcipher(blkcipher_alg, 0, CRYPTO_ALG_ASYNC);
+	if (IS_ERR(tfm)) {
+		ret = PTR_ERR(tfm);
+		pr_err("failed to allocate skcipher (%d)\n", ret);
+		goto tfm_fail;
+	}
+
+	ret = get_derived_key("ENC_KEY", enc_key);
+	if (ret) {
+		pr_err("failed to get encrypt key\n");
+		goto req_fail;
+	}
+
+	ret = crypto_skcipher_setkey(tfm, enc_key, DKEY_SIZE);
+	if (ret) {
+		pr_err("failed to setkey (%d)\n", ret);
+		goto req_fail;
+	}
+
+	req = skcipher_request_alloc(tfm, GFP_KERNEL);
+	if (!req) {
+		pr_err("failed to allocate request\n");
+		ret = -ENOMEM;
+		goto req_fail;
+	}
+
+	memcpy(iv_tmp, ekp->iv, ivsize);
+	sg_init_one(src, ekp->key, ekp->key_len);
+	sg_init_one(dst, encrypted_key_tmp, encrypted_keylen);
+	skcipher_request_set_crypt(req, src, dst, ekp->key_len, iv_tmp);
+	ret = crypto_skcipher_encrypt(req);
+	if (!ret)
+		memcpy(ekp->encrypted_key, encrypted_key_tmp, encrypted_keylen);
+
+	skcipher_request_free(req);
+req_fail:
+	crypto_free_skcipher(tfm);
+tfm_fail:
+	kzfree(iv_tmp);
+iv_fail:
+	memzero_explicit(enc_key, DKEY_SIZE);
+key_fail:
+	kzfree(encrypted_key_tmp);
+
+	return ret;
+}
+
+static int key_decrypt(struct efi_key_payload *ekp)
+{
+	struct scatterlist src[1], dst[1];
+	struct crypto_skcipher *tfm;
+	struct skcipher_request *req;
+	size_t encrypted_keylen;
+	u8 *decrypted_key_tmp;
+	u8 *enc_key, *iv_tmp;
+	int ret;
+
+	encrypted_keylen = roundup(ekp->key_len, blksize);
+
+	decrypted_key_tmp = kzalloc(ekp->key_len, GFP_KERNEL);
+	if (!decrypted_key_tmp)
+		return -ENOMEM;
+
+	enc_key = kzalloc(DKEY_SIZE, GFP_KERNEL);
+	if (!enc_key) {
+		ret = -ENOMEM;
+		goto key_fail;
+	}
+
+	iv_tmp = kmemdup(ekp->iv, ivsize, GFP_KERNEL);
+	if (!iv_tmp) {
+		ret = -ENOMEM;
+		goto iv_fail;
+	}
+
+	tfm = crypto_alloc_skcipher(blkcipher_alg, 0, CRYPTO_ALG_ASYNC);
+	if (IS_ERR(tfm)) {
+		ret = PTR_ERR(tfm);
+		pr_err("failed to allocate skcipher (%d)\n", ret);
+		goto tfm_fail;
+	}
+
+	ret = get_derived_key("ENC_KEY", enc_key);
+	if (ret) {
+		pr_err("failed to get encrypt key\n");
+		goto req_fail;
+	}
+
+	ret = crypto_skcipher_setkey(tfm, enc_key, DKEY_SIZE);
+	if (ret) {
+		pr_err("failed to setkey (%d)\n", ret);
+		goto req_fail;
+	}
+
+	req = skcipher_request_alloc(tfm, GFP_KERNEL);
+	if (!req) {
+		pr_err("failed to allocate request\n");
+		ret = -ENOMEM;
+		goto req_fail;
+	}
+
+	memcpy(iv_tmp, ekp->iv, ivsize);
+	sg_init_one(src, ekp->encrypted_key, encrypted_keylen);
+	sg_init_one(dst, decrypted_key_tmp, ekp->key_len);
+	skcipher_request_set_crypt(req, src, dst, ekp->key_len, iv_tmp);
+	ret = crypto_skcipher_decrypt(req);
+	if (!ret)
+		memcpy(ekp->key, decrypted_key_tmp, ekp->key_len);
+
+	skcipher_request_free(req);
+req_fail:
+	crypto_free_skcipher(tfm);
+tfm_fail:
+	kzfree(iv_tmp);
+iv_fail:
+	memzero_explicit(enc_key, DKEY_SIZE);
+key_fail:
+	kzfree(decrypted_key_tmp);
+
+	return ret;
+}
+
+/*
+ * Convert the ascii encoded blob to binary. And checking the hmac.
+ * The input blob format is:
+ * <erk hash> <encrypted iv> <encrypted key> <hmac>
+ */
+static int verify_hmac(struct efi_key_payload *ekp, char *hex_encoded_blob)
+{
+	u8 erk_hash[ERK_HASH_SIZE];
+	int max_encoded_blob_len;
+	size_t encrypted_keylen;
+	char *bufp;
+	u8 *hmac;
+	int ret;
+
+	/* check blob size */
+	max_encoded_blob_len = ekp->datablob_len + HMAC_HASH_SIZE
+				- strlen(ekp->key_len_str) - 1;
+	if (strlen(hex_encoded_blob) / 2 > max_encoded_blob_len)
+		return -EINVAL;
+
+	bufp = hex_encoded_blob;
+	ret = hex2bin(ekp->erk_hash, bufp, ERK_HASH_SIZE);
+	if (ret < 0)
+		return -EINVAL;
+
+	/* check the hash of ERK */
+	ret = calc_hash(hash_tfm, root_key, ROOT_KEY_SIZE, erk_hash);
+	if (ret < 0 || crypto_memneq(ekp->erk_hash, erk_hash, ERK_HASH_SIZE))
+		return -EINVAL;
+
+	/* iv */
+	bufp += ERK_HASH_SIZE * 2;
+	ret = hex2bin(ekp->iv, bufp, ivsize);
+	if (ret < 0)
+		return -EINVAL;
+
+	/* encrypted key */
+	bufp += ivsize * 2;
+	encrypted_keylen = roundup(ekp->key_len, blksize);
+	ret = hex2bin(ekp->encrypted_key, bufp, encrypted_keylen);
+	if (ret < 0)
+		return -EINVAL;
+
+	/* verify hmac */
+	bufp += encrypted_keylen * 2;
+	ret = hex2bin(ekp->hmac, bufp, HMAC_HASH_SIZE);
+	if (ret < 0)
+		return -EINVAL;
+
+	hmac = kzalloc(HMAC_HASH_SIZE, GFP_KERNEL);
+	if (!hmac)
+		return -ENOMEM;
+
+	ret = calc_hmac(ekp->datablob, ekp->datablob_len, hmac);
+	if (ret)
+		goto err;
+
+	ret = crypto_memneq(ekp->hmac, hmac, HMAC_HASH_SIZE);
+	if (ret) {
+		pr_warn("hmac signature does not match\n");
+		ret = -EINVAL;
+	}
+
+err:
+	kzfree(hmac);
+	return ret;
+}
+
+/*
+ * efi_instantiate - create a new efi key
+ *
+ * Decrypt an existing efi key blob or, for a new key, get a
+ * random key, then encrypt and creatse a efi key-type key,
+ * adding it to the specified keyring.
+ *
+ * e.g.
+ * keyctl add efi kmk-efi "new 128" @u
+ * keyctl add efi kmk-efi "load `cat kmk-efi.blob`" @u
+ *
+ * On success, return 0. Otherwise return errno.
+ */
+static int efi_instantiate(struct key *key,
+			   struct key_preparsed_payload *prep)
+{
+	struct efi_key_payload *ekp = NULL;
+	size_t datalen = prep->datalen;
+	char *datablob = NULL;
+	char *key_len_str = NULL;
+	char *hex_encoded_blob = NULL;
+	int key_cmd;
+	int ret = 0;
+
+	if (datalen <= 0 || datalen > 32767 || !prep->data)
+		return -EINVAL;
+
+	datablob = kzalloc(datalen + 1, GFP_KERNEL);
+	if (!datablob)
+		return -ENOMEM;
+	memcpy(datablob, prep->data, datalen);
+	datablob[datalen] = '\0';
+
+	key_cmd = datablob_parse(datablob, &key_len_str, &hex_encoded_blob);
+	if (key_cmd < 0) {
+		ret = key_cmd;
+		goto out;
+	}
+
+	ekp = efi_payload_alloc(key, key_len_str);
+	if (!ekp) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	switch (key_cmd) {
+	case Opt_load:
+		ret = verify_hmac(ekp, hex_encoded_blob);
+		if (ret)
+			break;
+		ret = key_decrypt(ekp);
+		break;
+	case Opt_new:
+		get_random_bytes(ekp->iv, ivsize);
+		get_random_bytes(ekp->key, ekp->key_len);
+		break;
+	default:
+		ret = -EINVAL;
+		goto out;
+	}
+out:
+	kzfree(datablob);
+	if (!ret)
+		rcu_assign_keypointer(key, ekp);
+	else
+		kzfree(ekp);
+	return ret;
+}
+
+/*
+ * efi_read_blob - read the encrypted blob data with hex format
+ *
+ * The resulting datablob format is:
+ * <key length string> <erk hash> <encrypted iv> <encrypted key> <hmac>
+ *
+ * On success, return the efi key datablob size.
+ */
+long efi_read_blob(const struct key *key, char __user *buffer,
+		   char *kbuffer, size_t buflen)
+{
+	struct efi_key_payload *ekp;
+	size_t asciiblob_len, encrypted_keylen;
+	char *ascii_buf;
+	char *bufp;
+	int i, len;
+	int ret;
+
+	if (!is_loaded)
+		return -EINVAL;
+
+	ekp = dereference_key_locked(key);
+	if (!ekp)
+		return -EINVAL;
+
+	/* datablob_len = key_len string length + 1 + ERK hash length + ivsize + encrypted_keylen
+	 * double size of ERK hash, iv, encrypted key, and hmac for ascii
+	 */
+	encrypted_keylen = roundup(ekp->key_len, blksize);
+	asciiblob_len = ekp->datablob_len + ERK_HASH_SIZE + ivsize + encrypted_keylen + HMAC_HASH_SIZE * 2;
+
+	if ((!buffer && !kbuffer) || buflen < asciiblob_len)
+		return asciiblob_len;
+
+	ascii_buf = kzalloc(asciiblob_len + 1, GFP_KERNEL);
+	if (!ascii_buf)
+		return -ENOMEM;
+
+	ascii_buf[asciiblob_len] = '\0';
+
+	/* copy key length string */
+	len = sprintf(ascii_buf, "%d ", ekp->key_len);
+
+	/* pack hash of ERK */
+	bufp = ascii_buf + len;
+	ret = calc_hash(hash_tfm, root_key, ROOT_KEY_SIZE, ekp->erk_hash);
+	if (ret)
+		goto err;
+	for (i = 0; i < ERK_HASH_SIZE; i++)
+		bufp = hex_byte_pack(bufp, ekp->erk_hash[i]);
+
+	/* pack iv */
+	for (i = 0; i < ivsize; i++)
+		bufp = hex_byte_pack(bufp, ekp->iv[i]);
+
+	/* encrypt and pack key */
+	ret = key_encrypt(ekp, encrypted_keylen);
+	if (ret)
+		goto err;
+	for (i = 0; i < ekp->key_len; i++)
+		bufp = hex_byte_pack(bufp, ekp->encrypted_key[i]);
+
+	/* generate and pack HMAC */
+	ret = calc_hmac(ekp->datablob, ekp->datablob_len, ekp->hmac);
+	if (ret)
+		goto err;
+	for (i = 0; i < HMAC_HASH_SIZE; i++)
+		bufp = hex_byte_pack(bufp, ekp->hmac[i]);
+
+	ret = asciiblob_len;
+	if (buffer) {
+		if (copy_to_user(buffer, ascii_buf, asciiblob_len) != 0)
+			ret = -EFAULT;
+	}
+	if (kbuffer) {
+		if (!memcpy(kbuffer, ascii_buf, asciiblob_len))
+			ret = -EFAULT;
+	}
+err:
+	kzfree(ascii_buf);
+	return ret;
+}
+EXPORT_SYMBOL(efi_read_blob);
+
+/*
+ * efi_read - format and copy the encrypted data to userspace
+ *
+ * The resulting datablob format is:
+ * <key length string> <erk hash> <encrypted iv> <encrypted key> <hmac>
+ *
+ * On success, return to userspace the encrypted key datablob size.
+ */
+static long efi_read(const struct key *key, char __user *buffer,
+			size_t buflen)
+{
+	return efi_read_blob(key, buffer, NULL, buflen);
+}
+
+/*
+ * efi_destroy - clear and free the key's payload
+ */
+static void efi_destroy(struct key *key)
+{
+	kzfree(key->payload.data[0]);
+}
+
+struct key_type key_type_efi = {
+	.name = "efi",
+	.instantiate = efi_instantiate,
+	.destroy = efi_destroy,
+	.describe = user_describe,
+	.read = efi_read,
+};
+EXPORT_SYMBOL_GPL(key_type_efi);
+
+static int __init init_efi_secure_key(void)
+{
+	int ret;
+
+	/* root_key must be loaded */
+	if (!is_loaded)
+		return 0;
+
+	hash_tfm = crypto_alloc_shash(hash_alg, 0, CRYPTO_ALG_ASYNC);
+	if (IS_ERR(hash_tfm)) {
+		pr_err("can't allocate %s transform: %ld\n",
+			hash_alg, PTR_ERR(hash_tfm));
+		return PTR_ERR(hash_tfm);
+	}
+
+	/* initial EFI key type */
+	ret = set_aes_sizes();
+	if (!ret)
+		ret = register_key_type(&key_type_efi);
+
+	return ret;
+}
+
+late_initcall(init_efi_secure_key);
diff --git a/include/keys/efi-type.h b/include/keys/efi-type.h
new file mode 100644
index 000000000000..57524b22d42f
--- /dev/null
+++ b/include/keys/efi-type.h
@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/* efi-type.h: EFI key type
+ *
+ * Copyright (C) 2018 Lee, Chun-Yi <jlee@suse.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#ifndef _KEYS_EFI_TYPE_H
+#define _KEYS_EFI_TYPE_H
+
+#include <linux/key.h>
+#include <linux/rcupdate.h>
+
+#define MIN_KEY_SIZE			32
+#define MAX_KEY_SIZE			128
+#define MAX_BLOB_SIZE			512
+
+struct efi_key_payload {
+	struct rcu_head rcu;
+	u8 *key;
+	u8 *datablob;			/* key_len(string) + ERK hash + iv + encrypted key */
+	char *key_len_str;
+	u8 *erk_hash;
+	u8 *iv;
+	u8 *encrypted_key;
+	u8 *hmac;
+	unsigned int key_len;
+	unsigned int datablob_len;
+	u8 payload_data[0];		/* key + datablob + hmac */
+};
+
+extern struct key_type key_type_efi;
+
+#if defined(CONFIG_EFI_SECURE_KEY)
+extern long efi_read_blob(const struct key *key, char __user *buffer,
+			  char *kbuffer, size_t buflen);
+#else
+inline long efi_read_blob(const struct key *key, char __user *buffer,
+			  char *kbuffer, size_t buflen)
+{
+	return 0;
+}
+#endif
+
+#endif /* _KEYS_EFI_TYPE_H */
-- 
2.13.6


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

* [PATCH 4/6] key: add EFI secure key type
@ 2018-08-05  3:21   ` Lee, Chun-Yi
  0 siblings, 0 replies; 30+ messages in thread
From: Lee, Chun-Yi @ 2018-08-05  3:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-efi, x86, keyrings, linux-integrity, Lee, Chun-Yi,
	Kees Cook, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Rafael J. Wysocki, Pavel Machek, Chen Yu, Oliver Neukum,
	Ryan Chen, Ard Biesheuvel, David Howells, Mimi Zohar

EFI secure key is a new key type that the key is encrypted and
authenticated by the EFI root key. The ERK (EFI root key) is
generated by EFI boot stub and be stored in EFI boot services
variable before ExitBootServices be called.

When user enabled the secure boot in firmware, the EFI root key is
secure which means that the encrypted EFI secure key is also secure.

A EFI secure key is generated by kernel and can be keep by user
space. It is also a new master key type like trusted key(TPM) or
user key. The EFI key can be used by hibernation encryption and
authentication. And it can also be a master key to generate a
encrypted key for EVM.

The layout of the efi key payload is:

	      -+-+-----------------------+
	       | |                       |
	       | |                       |
	       | |                       |
      key_len->| |         key           |
	       | |                       |
	       | |                       |
	      -+-+-----------------------+----+-
	       | |     key_len           |    |
	       | |(decrypted key length) |    |
	       | |     (string)          |    |
	       | |                       |    |
	       | +-----------------------|    |
 datablob_len->| |                       |    |
	       | |      ERK hash         | datablob
	       | |                       |    |
	       | +-----------------------|    |
	       | |          iv           |    |
	       | |(initialization vector)|    |
	       | +-----------------------|    |
	       | |                       |    |
	       | |                       |    |
	       | |                       |    |
	       | |    encrypted_key      |    |
	       | |                       |    |
	       | |                       |    |
	      -+-+-----------------------+----+-
		 |        hmac           |
		 |(signature of datablob)|
		 +-----------------------+

The datablob can be exported to user space with hmac.

As other key type, EFI key can be created by keyctl tool:

e.g. keyctl add efi key-name "new 128" @u

EFI secure key can also be dumped to a file by keyctl:
e.g. keyctl pipe $NUMBER > ~/tmp/key-name.blob

The $NUMBER is the key serial number.

Enroll the key blob back to kernel:
e.g. keyctl add efi key-name "load `cat ~/tmp/key-name.blob`" @u

Cc: Kees Cook <keescook@chromium.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Chen Yu <yu.c.chen@intel.com>
Cc: Oliver Neukum <oneukum@suse.com>
Cc: Ryan Chen <yu.chen.surf@gmail.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
---
 drivers/firmware/efi/efi-secure-key.c | 636 ++++++++++++++++++++++++++++++++++
 include/keys/efi-type.h               |  50 +++
 2 files changed, 686 insertions(+)
 create mode 100644 include/keys/efi-type.h

diff --git a/drivers/firmware/efi/efi-secure-key.c b/drivers/firmware/efi/efi-secure-key.c
index e56d7d176e03..5e72a8c9e13e 100644
--- a/drivers/firmware/efi/efi-secure-key.c
+++ b/drivers/firmware/efi/efi-secure-key.c
@@ -11,6 +11,16 @@
 #include <linux/efi.h>
 #include <linux/memblock.h>
 #include <linux/security.h>
+#include <linux/key-type.h>
+#include <linux/parser.h>
+#include <linux/random.h>
+#include <linux/scatterlist.h>
+#include <keys/efi-type.h>
+#include <keys/user-type.h>
+#include <crypto/algapi.h>
+#include <crypto/hash.h>
+#include <crypto/sha.h>
+#include <crypto/skcipher.h>
 
 static u8 root_key[ROOT_KEY_SIZE];
 static unsigned long rkey_size;
@@ -64,3 +74,629 @@ void __init parse_efi_root_key_setup(u64 phys_addr, u32 data_len)
 		sizeof(struct setup_data) + sizeof(struct efi_rkey_setup_data));
 	early_iounmap(setup_data, data_len);
 }
+
+#define ERK_HASH_SIZE SHA256_DIGEST_SIZE
+#define HMAC_HASH_SIZE SHA256_DIGEST_SIZE
+#define DKEY_SIZE SHA256_DIGEST_SIZE
+
+struct key_type key_type_efi;
+
+static const char hash_alg[] = "sha256";
+static const char hmac_alg[] = "hmac(sha256)";
+static struct crypto_shash *hash_tfm;
+
+static int calc_hash(struct crypto_shash *tfm, const u8 *buf,
+		     unsigned int buflen, u8 *digest)
+{
+	SHASH_DESC_ON_STACK(desc, tfm);
+	int ret;
+
+	desc->tfm = tfm;
+	desc->flags = 0;
+
+	ret = crypto_shash_digest(desc, buf, buflen, digest);
+	shash_desc_zero(desc);
+
+	return ret;
+}
+
+static int get_derived_key(const char *salt, u8 *derived_key)
+{
+	u8 *derived_buf;
+	unsigned int derived_buf_len;
+	int ret;
+
+	derived_buf_len = strlen(salt) + 1 + ROOT_KEY_SIZE;
+	if (derived_buf_len < DKEY_SIZE)
+		derived_buf_len = DKEY_SIZE;
+
+	derived_buf = kzalloc(derived_buf_len, GFP_KERNEL);
+	if (!derived_buf)
+		return -ENOMEM;
+
+	memcpy(derived_buf + strlen(derived_buf) + 1, root_key,
+		ROOT_KEY_SIZE);
+	ret = calc_hash(hash_tfm, derived_buf, derived_buf_len, derived_key);
+	memzero_explicit(derived_buf, derived_buf_len);
+
+	return ret;
+}
+
+static int calc_hmac(const u8 *buf, unsigned int buflen, u8 *digest)
+{
+	struct crypto_shash *tfm;
+	u8 *auth_key;
+	int ret;
+
+	auth_key = kzalloc(DKEY_SIZE, GFP_KERNEL);
+	if (!auth_key)
+		return -ENOMEM;
+
+	tfm = crypto_alloc_shash(hmac_alg, 0, CRYPTO_ALG_ASYNC);
+	if (IS_ERR(tfm)) {
+		pr_err("can't alloc %s transform: %ld\n",
+			hmac_alg, PTR_ERR(tfm));
+		ret = PTR_ERR(tfm);
+		goto tfm_fail;
+	}
+
+	ret = get_derived_key("AUTH_KEY", auth_key);
+	if (ret)
+		goto key_fail;
+
+	ret = crypto_shash_setkey(tfm, auth_key, DKEY_SIZE);
+	if (!ret)
+		ret = calc_hash(tfm, buf, buflen, digest);
+
+key_fail:
+	crypto_free_shash(tfm);
+tfm_fail:
+	memzero_explicit(auth_key, DKEY_SIZE);
+
+	return ret;
+}
+
+static const char blkcipher_alg[] = "cbc(aes)";
+static unsigned int ivsize;
+static int blksize;
+
+static int set_aes_sizes(void)
+{
+	struct crypto_skcipher *tfm;
+
+	tfm = crypto_alloc_skcipher(blkcipher_alg, 0, CRYPTO_ALG_ASYNC);
+	if (IS_ERR(tfm)) {
+		pr_err("failed to alloc_cipher (%ld)\n", PTR_ERR(tfm));
+		return PTR_ERR(tfm);
+	}
+	ivsize = crypto_skcipher_ivsize(tfm);
+	blksize = crypto_skcipher_blocksize(tfm);
+	crypto_free_skcipher(tfm);
+
+	return 0;
+}
+
+enum {
+	Opt_err = -1,
+	Opt_new, Opt_load,
+};
+
+static const match_table_t key_tokens = {
+	{Opt_new, "new"},
+	{Opt_load, "load"},
+	{Opt_err, NULL}
+};
+
+static struct efi_key_payload *efi_payload_alloc(struct key *key, char *key_len_str)
+{
+	struct efi_key_payload *ekp = NULL;
+	unsigned short encrypted_keylen;
+	unsigned short datablob_len;
+	unsigned short payload_len;
+	long key_len;
+	int ret;
+
+	ret = kstrtol(key_len_str, 10, &key_len);
+	if (ret < 0)
+		return ERR_PTR(ret);
+	encrypted_keylen = roundup(key_len, blksize);
+
+	/* efi_key_payload + key + datablob + hmac */
+	datablob_len = strlen(key_len_str) + 1 + ERK_HASH_SIZE + ivsize + encrypted_keylen;
+	payload_len = sizeof(*ekp) + key_len + datablob_len + HMAC_HASH_SIZE;
+
+	ret = key_payload_reserve(key, payload_len);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	ekp = kzalloc(payload_len, GFP_KERNEL);
+	if (!ekp)
+		return ERR_PTR(-ENOMEM);
+	ekp->key = ekp->payload_data;
+	ekp->datablob = ekp->key + key_len;
+	ekp->key_len_str = ekp->datablob;
+	ekp->erk_hash = ekp->key_len_str + strlen(key_len_str) + 1;
+	ekp->iv = ekp->erk_hash + ERK_HASH_SIZE;
+	ekp->encrypted_key = ekp->iv + ivsize;
+	ekp->hmac = ekp->encrypted_key + encrypted_keylen;
+	ekp->key_len = key_len;
+	ekp->datablob_len = datablob_len;
+
+	memcpy(ekp->key_len_str, key_len_str, strlen(key_len_str));
+
+	return ekp;
+}
+
+/*
+ * datablob_parse - parse the keyctl data and fill in the
+ *                  payload and options structures
+ *
+ * On success returns command number, otherwise -EINVAL.
+ */
+static int datablob_parse(char *datablob, char **key_len_str, char **hex_encoded_blob)
+{
+	substring_t args[MAX_OPT_ARGS];
+	long key_len;
+	int key_cmd;
+	int ret;
+	char *c;
+
+	/* main command */
+	c = strsep(&datablob, " \t");
+	if (!c)
+		return -EINVAL;
+	key_cmd = match_token(c, key_tokens, args);
+
+	/* first string argument is key length */
+	c = strsep(&datablob, " \t");
+	if (!c)
+		return -EINVAL;
+	*key_len_str = c;
+	ret = kstrtol(*key_len_str, 10, &key_len);
+	if (ret < 0 || key_len < MIN_KEY_SIZE || key_len > MAX_KEY_SIZE)
+		return -EINVAL;
+
+	switch (key_cmd) {
+	case Opt_new:
+		ret = Opt_new;
+		break;
+	case Opt_load:
+		*hex_encoded_blob = strsep(&datablob, " \t");
+		if (!*hex_encoded_blob) {
+			pr_info("hex blob is missing\n");
+			return -EINVAL;
+		}
+		if (strlen(*hex_encoded_blob) / 2 > MAX_BLOB_SIZE)
+			return -EINVAL;
+		ret = Opt_load;
+		break;
+	case Opt_err:
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+static int key_encrypt(struct efi_key_payload *ekp, size_t encrypted_keylen)
+{
+	struct scatterlist src[1], dst[1];
+	struct crypto_skcipher *tfm;
+	struct skcipher_request *req;
+	u8 *encrypted_key_tmp;
+	u8 *iv_tmp, *enc_key;
+	int ret;
+
+	encrypted_key_tmp = kzalloc(encrypted_keylen, GFP_KERNEL);
+	if (!encrypted_key_tmp)
+		return -ENOMEM;
+
+	enc_key = kzalloc(DKEY_SIZE, GFP_KERNEL);
+	if (!enc_key) {
+		ret = -ENOMEM;
+		goto key_fail;
+	}
+
+	iv_tmp = kmemdup(ekp->iv, ivsize, GFP_KERNEL);
+	if (!iv_tmp) {
+		ret = -ENOMEM;
+		goto iv_fail;
+	}
+
+	tfm = crypto_alloc_skcipher(blkcipher_alg, 0, CRYPTO_ALG_ASYNC);
+	if (IS_ERR(tfm)) {
+		ret = PTR_ERR(tfm);
+		pr_err("failed to allocate skcipher (%d)\n", ret);
+		goto tfm_fail;
+	}
+
+	ret = get_derived_key("ENC_KEY", enc_key);
+	if (ret) {
+		pr_err("failed to get encrypt key\n");
+		goto req_fail;
+	}
+
+	ret = crypto_skcipher_setkey(tfm, enc_key, DKEY_SIZE);
+	if (ret) {
+		pr_err("failed to setkey (%d)\n", ret);
+		goto req_fail;
+	}
+
+	req = skcipher_request_alloc(tfm, GFP_KERNEL);
+	if (!req) {
+		pr_err("failed to allocate request\n");
+		ret = -ENOMEM;
+		goto req_fail;
+	}
+
+	memcpy(iv_tmp, ekp->iv, ivsize);
+	sg_init_one(src, ekp->key, ekp->key_len);
+	sg_init_one(dst, encrypted_key_tmp, encrypted_keylen);
+	skcipher_request_set_crypt(req, src, dst, ekp->key_len, iv_tmp);
+	ret = crypto_skcipher_encrypt(req);
+	if (!ret)
+		memcpy(ekp->encrypted_key, encrypted_key_tmp, encrypted_keylen);
+
+	skcipher_request_free(req);
+req_fail:
+	crypto_free_skcipher(tfm);
+tfm_fail:
+	kzfree(iv_tmp);
+iv_fail:
+	memzero_explicit(enc_key, DKEY_SIZE);
+key_fail:
+	kzfree(encrypted_key_tmp);
+
+	return ret;
+}
+
+static int key_decrypt(struct efi_key_payload *ekp)
+{
+	struct scatterlist src[1], dst[1];
+	struct crypto_skcipher *tfm;
+	struct skcipher_request *req;
+	size_t encrypted_keylen;
+	u8 *decrypted_key_tmp;
+	u8 *enc_key, *iv_tmp;
+	int ret;
+
+	encrypted_keylen = roundup(ekp->key_len, blksize);
+
+	decrypted_key_tmp = kzalloc(ekp->key_len, GFP_KERNEL);
+	if (!decrypted_key_tmp)
+		return -ENOMEM;
+
+	enc_key = kzalloc(DKEY_SIZE, GFP_KERNEL);
+	if (!enc_key) {
+		ret = -ENOMEM;
+		goto key_fail;
+	}
+
+	iv_tmp = kmemdup(ekp->iv, ivsize, GFP_KERNEL);
+	if (!iv_tmp) {
+		ret = -ENOMEM;
+		goto iv_fail;
+	}
+
+	tfm = crypto_alloc_skcipher(blkcipher_alg, 0, CRYPTO_ALG_ASYNC);
+	if (IS_ERR(tfm)) {
+		ret = PTR_ERR(tfm);
+		pr_err("failed to allocate skcipher (%d)\n", ret);
+		goto tfm_fail;
+	}
+
+	ret = get_derived_key("ENC_KEY", enc_key);
+	if (ret) {
+		pr_err("failed to get encrypt key\n");
+		goto req_fail;
+	}
+
+	ret = crypto_skcipher_setkey(tfm, enc_key, DKEY_SIZE);
+	if (ret) {
+		pr_err("failed to setkey (%d)\n", ret);
+		goto req_fail;
+	}
+
+	req = skcipher_request_alloc(tfm, GFP_KERNEL);
+	if (!req) {
+		pr_err("failed to allocate request\n");
+		ret = -ENOMEM;
+		goto req_fail;
+	}
+
+	memcpy(iv_tmp, ekp->iv, ivsize);
+	sg_init_one(src, ekp->encrypted_key, encrypted_keylen);
+	sg_init_one(dst, decrypted_key_tmp, ekp->key_len);
+	skcipher_request_set_crypt(req, src, dst, ekp->key_len, iv_tmp);
+	ret = crypto_skcipher_decrypt(req);
+	if (!ret)
+		memcpy(ekp->key, decrypted_key_tmp, ekp->key_len);
+
+	skcipher_request_free(req);
+req_fail:
+	crypto_free_skcipher(tfm);
+tfm_fail:
+	kzfree(iv_tmp);
+iv_fail:
+	memzero_explicit(enc_key, DKEY_SIZE);
+key_fail:
+	kzfree(decrypted_key_tmp);
+
+	return ret;
+}
+
+/*
+ * Convert the ascii encoded blob to binary. And checking the hmac.
+ * The input blob format is:
+ * <erk hash> <encrypted iv> <encrypted key> <hmac>
+ */
+static int verify_hmac(struct efi_key_payload *ekp, char *hex_encoded_blob)
+{
+	u8 erk_hash[ERK_HASH_SIZE];
+	int max_encoded_blob_len;
+	size_t encrypted_keylen;
+	char *bufp;
+	u8 *hmac;
+	int ret;
+
+	/* check blob size */
+	max_encoded_blob_len = ekp->datablob_len + HMAC_HASH_SIZE
+				- strlen(ekp->key_len_str) - 1;
+	if (strlen(hex_encoded_blob) / 2 > max_encoded_blob_len)
+		return -EINVAL;
+
+	bufp = hex_encoded_blob;
+	ret = hex2bin(ekp->erk_hash, bufp, ERK_HASH_SIZE);
+	if (ret < 0)
+		return -EINVAL;
+
+	/* check the hash of ERK */
+	ret = calc_hash(hash_tfm, root_key, ROOT_KEY_SIZE, erk_hash);
+	if (ret < 0 || crypto_memneq(ekp->erk_hash, erk_hash, ERK_HASH_SIZE))
+		return -EINVAL;
+
+	/* iv */
+	bufp += ERK_HASH_SIZE * 2;
+	ret = hex2bin(ekp->iv, bufp, ivsize);
+	if (ret < 0)
+		return -EINVAL;
+
+	/* encrypted key */
+	bufp += ivsize * 2;
+	encrypted_keylen = roundup(ekp->key_len, blksize);
+	ret = hex2bin(ekp->encrypted_key, bufp, encrypted_keylen);
+	if (ret < 0)
+		return -EINVAL;
+
+	/* verify hmac */
+	bufp += encrypted_keylen * 2;
+	ret = hex2bin(ekp->hmac, bufp, HMAC_HASH_SIZE);
+	if (ret < 0)
+		return -EINVAL;
+
+	hmac = kzalloc(HMAC_HASH_SIZE, GFP_KERNEL);
+	if (!hmac)
+		return -ENOMEM;
+
+	ret = calc_hmac(ekp->datablob, ekp->datablob_len, hmac);
+	if (ret)
+		goto err;
+
+	ret = crypto_memneq(ekp->hmac, hmac, HMAC_HASH_SIZE);
+	if (ret) {
+		pr_warn("hmac signature does not match\n");
+		ret = -EINVAL;
+	}
+
+err:
+	kzfree(hmac);
+	return ret;
+}
+
+/*
+ * efi_instantiate - create a new efi key
+ *
+ * Decrypt an existing efi key blob or, for a new key, get a
+ * random key, then encrypt and creatse a efi key-type key,
+ * adding it to the specified keyring.
+ *
+ * e.g.
+ * keyctl add efi kmk-efi "new 128" @u
+ * keyctl add efi kmk-efi "load `cat kmk-efi.blob`" @u
+ *
+ * On success, return 0. Otherwise return errno.
+ */
+static int efi_instantiate(struct key *key,
+			   struct key_preparsed_payload *prep)
+{
+	struct efi_key_payload *ekp = NULL;
+	size_t datalen = prep->datalen;
+	char *datablob = NULL;
+	char *key_len_str = NULL;
+	char *hex_encoded_blob = NULL;
+	int key_cmd;
+	int ret = 0;
+
+	if (datalen <= 0 || datalen > 32767 || !prep->data)
+		return -EINVAL;
+
+	datablob = kzalloc(datalen + 1, GFP_KERNEL);
+	if (!datablob)
+		return -ENOMEM;
+	memcpy(datablob, prep->data, datalen);
+	datablob[datalen] = '\0';
+
+	key_cmd = datablob_parse(datablob, &key_len_str, &hex_encoded_blob);
+	if (key_cmd < 0) {
+		ret = key_cmd;
+		goto out;
+	}
+
+	ekp = efi_payload_alloc(key, key_len_str);
+	if (!ekp) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	switch (key_cmd) {
+	case Opt_load:
+		ret = verify_hmac(ekp, hex_encoded_blob);
+		if (ret)
+			break;
+		ret = key_decrypt(ekp);
+		break;
+	case Opt_new:
+		get_random_bytes(ekp->iv, ivsize);
+		get_random_bytes(ekp->key, ekp->key_len);
+		break;
+	default:
+		ret = -EINVAL;
+		goto out;
+	}
+out:
+	kzfree(datablob);
+	if (!ret)
+		rcu_assign_keypointer(key, ekp);
+	else
+		kzfree(ekp);
+	return ret;
+}
+
+/*
+ * efi_read_blob - read the encrypted blob data with hex format
+ *
+ * The resulting datablob format is:
+ * <key length string> <erk hash> <encrypted iv> <encrypted key> <hmac>
+ *
+ * On success, return the efi key datablob size.
+ */
+long efi_read_blob(const struct key *key, char __user *buffer,
+		   char *kbuffer, size_t buflen)
+{
+	struct efi_key_payload *ekp;
+	size_t asciiblob_len, encrypted_keylen;
+	char *ascii_buf;
+	char *bufp;
+	int i, len;
+	int ret;
+
+	if (!is_loaded)
+		return -EINVAL;
+
+	ekp = dereference_key_locked(key);
+	if (!ekp)
+		return -EINVAL;
+
+	/* datablob_len = key_len string length + 1 + ERK hash length + ivsize + encrypted_keylen
+	 * double size of ERK hash, iv, encrypted key, and hmac for ascii
+	 */
+	encrypted_keylen = roundup(ekp->key_len, blksize);
+	asciiblob_len = ekp->datablob_len + ERK_HASH_SIZE + ivsize + encrypted_keylen + HMAC_HASH_SIZE * 2;
+
+	if ((!buffer && !kbuffer) || buflen < asciiblob_len)
+		return asciiblob_len;
+
+	ascii_buf = kzalloc(asciiblob_len + 1, GFP_KERNEL);
+	if (!ascii_buf)
+		return -ENOMEM;
+
+	ascii_buf[asciiblob_len] = '\0';
+
+	/* copy key length string */
+	len = sprintf(ascii_buf, "%d ", ekp->key_len);
+
+	/* pack hash of ERK */
+	bufp = ascii_buf + len;
+	ret = calc_hash(hash_tfm, root_key, ROOT_KEY_SIZE, ekp->erk_hash);
+	if (ret)
+		goto err;
+	for (i = 0; i < ERK_HASH_SIZE; i++)
+		bufp = hex_byte_pack(bufp, ekp->erk_hash[i]);
+
+	/* pack iv */
+	for (i = 0; i < ivsize; i++)
+		bufp = hex_byte_pack(bufp, ekp->iv[i]);
+
+	/* encrypt and pack key */
+	ret = key_encrypt(ekp, encrypted_keylen);
+	if (ret)
+		goto err;
+	for (i = 0; i < ekp->key_len; i++)
+		bufp = hex_byte_pack(bufp, ekp->encrypted_key[i]);
+
+	/* generate and pack HMAC */
+	ret = calc_hmac(ekp->datablob, ekp->datablob_len, ekp->hmac);
+	if (ret)
+		goto err;
+	for (i = 0; i < HMAC_HASH_SIZE; i++)
+		bufp = hex_byte_pack(bufp, ekp->hmac[i]);
+
+	ret = asciiblob_len;
+	if (buffer) {
+		if (copy_to_user(buffer, ascii_buf, asciiblob_len) != 0)
+			ret = -EFAULT;
+	}
+	if (kbuffer) {
+		if (!memcpy(kbuffer, ascii_buf, asciiblob_len))
+			ret = -EFAULT;
+	}
+err:
+	kzfree(ascii_buf);
+	return ret;
+}
+EXPORT_SYMBOL(efi_read_blob);
+
+/*
+ * efi_read - format and copy the encrypted data to userspace
+ *
+ * The resulting datablob format is:
+ * <key length string> <erk hash> <encrypted iv> <encrypted key> <hmac>
+ *
+ * On success, return to userspace the encrypted key datablob size.
+ */
+static long efi_read(const struct key *key, char __user *buffer,
+			size_t buflen)
+{
+	return efi_read_blob(key, buffer, NULL, buflen);
+}
+
+/*
+ * efi_destroy - clear and free the key's payload
+ */
+static void efi_destroy(struct key *key)
+{
+	kzfree(key->payload.data[0]);
+}
+
+struct key_type key_type_efi = {
+	.name = "efi",
+	.instantiate = efi_instantiate,
+	.destroy = efi_destroy,
+	.describe = user_describe,
+	.read = efi_read,
+};
+EXPORT_SYMBOL_GPL(key_type_efi);
+
+static int __init init_efi_secure_key(void)
+{
+	int ret;
+
+	/* root_key must be loaded */
+	if (!is_loaded)
+		return 0;
+
+	hash_tfm = crypto_alloc_shash(hash_alg, 0, CRYPTO_ALG_ASYNC);
+	if (IS_ERR(hash_tfm)) {
+		pr_err("can't allocate %s transform: %ld\n",
+			hash_alg, PTR_ERR(hash_tfm));
+		return PTR_ERR(hash_tfm);
+	}
+
+	/* initial EFI key type */
+	ret = set_aes_sizes();
+	if (!ret)
+		ret = register_key_type(&key_type_efi);
+
+	return ret;
+}
+
+late_initcall(init_efi_secure_key);
diff --git a/include/keys/efi-type.h b/include/keys/efi-type.h
new file mode 100644
index 000000000000..57524b22d42f
--- /dev/null
+++ b/include/keys/efi-type.h
@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/* efi-type.h: EFI key type
+ *
+ * Copyright (C) 2018 Lee, Chun-Yi <jlee@suse.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#ifndef _KEYS_EFI_TYPE_H
+#define _KEYS_EFI_TYPE_H
+
+#include <linux/key.h>
+#include <linux/rcupdate.h>
+
+#define MIN_KEY_SIZE			32
+#define MAX_KEY_SIZE			128
+#define MAX_BLOB_SIZE			512
+
+struct efi_key_payload {
+	struct rcu_head rcu;
+	u8 *key;
+	u8 *datablob;			/* key_len(string) + ERK hash + iv + encrypted key */
+	char *key_len_str;
+	u8 *erk_hash;
+	u8 *iv;
+	u8 *encrypted_key;
+	u8 *hmac;
+	unsigned int key_len;
+	unsigned int datablob_len;
+	u8 payload_data[0];		/* key + datablob + hmac */
+};
+
+extern struct key_type key_type_efi;
+
+#if defined(CONFIG_EFI_SECURE_KEY)
+extern long efi_read_blob(const struct key *key, char __user *buffer,
+			  char *kbuffer, size_t buflen);
+#else
+inline long efi_read_blob(const struct key *key, char __user *buffer,
+			  char *kbuffer, size_t buflen)
+{
+	return 0;
+}
+#endif
+
+#endif /* _KEYS_EFI_TYPE_H */
-- 
2.13.6


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

* [PATCH 5/6] key: add EFI secure key as a master key type
  2018-08-05  3:21 ` Lee, Chun-Yi
@ 2018-08-05  3:21   ` Lee, Chun-Yi
  -1 siblings, 0 replies; 30+ messages in thread
From: Lee, Chun-Yi @ 2018-08-05  3:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-efi, x86, keyrings, linux-integrity, Lee, Chun-Yi,
	Kees Cook, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Rafael J. Wysocki, Pavel Machek, Chen Yu, Oliver Neukum,
	Ryan Chen, Ard Biesheuvel, David Howells, Mimi Zohar

EFI secure key can be a new master key type that it's used for
generate encrypted key.

Compared with trusted key or user key, the advantage of using
EFI master key is that it doesn't need TPM or password from user
space.

As other master key types, keyctl can be used to create new encrypted
key by EFI secure key. Using the "efi:" prefix string with master
key name:

e.g. keyctl add encrypted evm-key "new efi:kmk-efi 32" @u

Cc: Kees Cook <keescook@chromium.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Chen Yu <yu.c.chen@intel.com>
Cc: Oliver Neukum <oneukum@suse.com>
Cc: Ryan Chen <yu.chen.surf@gmail.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
---
 drivers/firmware/efi/efi-secure-key.c    | 21 +++++++++++++++++++++
 include/keys/efi-type.h                  |  7 +++++++
 security/keys/encrypted-keys/encrypted.c | 10 ++++++++++
 3 files changed, 38 insertions(+)

diff --git a/drivers/firmware/efi/efi-secure-key.c b/drivers/firmware/efi/efi-secure-key.c
index 5e72a8c9e13e..aa422ee87f70 100644
--- a/drivers/firmware/efi/efi-secure-key.c
+++ b/drivers/firmware/efi/efi-secure-key.c
@@ -676,6 +676,27 @@ struct key_type key_type_efi = {
 };
 EXPORT_SYMBOL_GPL(key_type_efi);
 
+/*
+ * request_efi_key - request the efi key
+ */
+struct key *request_efi_key(const char *master_desc,
+			    const u8 **master_key, size_t *master_keylen)
+{
+	struct efi_key_payload *epayload;
+	struct key *ekey;
+
+	ekey = request_key(&key_type_efi, master_desc, NULL);
+	if (IS_ERR(ekey))
+		goto error;
+
+	down_read(&ekey->sem);
+	epayload = ekey->payload.data[0];
+	*master_key = epayload->key;
+	*master_keylen = epayload->key_len;
+error:
+	return ekey;
+}
+
 static int __init init_efi_secure_key(void)
 {
 	int ret;
diff --git a/include/keys/efi-type.h b/include/keys/efi-type.h
index 57524b22d42f..bbe649f3eec0 100644
--- a/include/keys/efi-type.h
+++ b/include/keys/efi-type.h
@@ -39,12 +39,19 @@ extern struct key_type key_type_efi;
 #if defined(CONFIG_EFI_SECURE_KEY)
 extern long efi_read_blob(const struct key *key, char __user *buffer,
 			  char *kbuffer, size_t buflen);
+extern struct key *request_efi_key(const char *master_desc,
+			const u8 **master_key, size_t *master_keylen);
 #else
 inline long efi_read_blob(const struct key *key, char __user *buffer,
 			  char *kbuffer, size_t buflen)
 {
 	return 0;
 }
+static inline struct key *request_efi_key(const char *master_desc,
+			const u8 **master_key, size_t *master_keylen)
+{
+	return ERR_PTR(-EOPNOTSUPP);
+}
 #endif
 
 #endif /* _KEYS_EFI_TYPE_H */
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index d92cbf9687c3..b396506afdfc 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -24,6 +24,7 @@
 #include <keys/user-type.h>
 #include <keys/trusted-type.h>
 #include <keys/encrypted-type.h>
+#include <keys/efi-type.h>
 #include <linux/key-type.h>
 #include <linux/random.h>
 #include <linux/rcupdate.h>
@@ -40,6 +41,7 @@
 
 static const char KEY_TRUSTED_PREFIX[] = "trusted:";
 static const char KEY_USER_PREFIX[] = "user:";
+static const char KEY_EFI_PREFIX[] = "efi:";
 static const char hash_alg[] = "sha256";
 static const char hmac_alg[] = "hmac(sha256)";
 static const char blkcipher_alg[] = "cbc(aes)";
@@ -50,6 +52,7 @@ static int blksize;
 
 #define KEY_TRUSTED_PREFIX_LEN (sizeof (KEY_TRUSTED_PREFIX) - 1)
 #define KEY_USER_PREFIX_LEN (sizeof (KEY_USER_PREFIX) - 1)
+#define KEY_EFI_PREFIX_LEN (sizeof (KEY_EFI_PREFIX) - 1)
 #define KEY_ECRYPTFS_DESC_LEN 16
 #define HASH_SIZE SHA256_DIGEST_SIZE
 #define MAX_DATA_SIZE 4096
@@ -142,6 +145,8 @@ static int valid_master_desc(const char *new_desc, const char *orig_desc)
 		prefix_len = KEY_TRUSTED_PREFIX_LEN;
 	else if (!strncmp(new_desc, KEY_USER_PREFIX, KEY_USER_PREFIX_LEN))
 		prefix_len = KEY_USER_PREFIX_LEN;
+	else if (!strncmp(new_desc, KEY_EFI_PREFIX, KEY_EFI_PREFIX_LEN))
+		prefix_len = KEY_EFI_PREFIX_LEN;
 	else
 		return -EINVAL;
 
@@ -434,6 +439,11 @@ static struct key *request_master_key(struct encrypted_key_payload *epayload,
 		mkey = request_user_key(epayload->master_desc +
 					KEY_USER_PREFIX_LEN,
 					master_key, master_keylen);
+	} else if (!strncmp(epayload->master_desc, KEY_EFI_PREFIX,
+			    KEY_EFI_PREFIX_LEN)) {
+		mkey = request_efi_key(epayload->master_desc +
+					KEY_EFI_PREFIX_LEN,
+					master_key, master_keylen);
 	} else
 		goto out;
 
-- 
2.13.6


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

* [PATCH 5/6] key: add EFI secure key as a master key type
@ 2018-08-05  3:21   ` Lee, Chun-Yi
  0 siblings, 0 replies; 30+ messages in thread
From: Lee, Chun-Yi @ 2018-08-05  3:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-efi, x86, keyrings, linux-integrity, Lee, Chun-Yi,
	Kees Cook, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Rafael J. Wysocki, Pavel Machek, Chen Yu, Oliver Neukum,
	Ryan Chen, Ard Biesheuvel, David Howells, Mimi Zohar

EFI secure key can be a new master key type that it's used for
generate encrypted key.

Compared with trusted key or user key, the advantage of using
EFI master key is that it doesn't need TPM or password from user
space.

As other master key types, keyctl can be used to create new encrypted
key by EFI secure key. Using the "efi:" prefix string with master
key name:

e.g. keyctl add encrypted evm-key "new efi:kmk-efi 32" @u

Cc: Kees Cook <keescook@chromium.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Chen Yu <yu.c.chen@intel.com>
Cc: Oliver Neukum <oneukum@suse.com>
Cc: Ryan Chen <yu.chen.surf@gmail.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
---
 drivers/firmware/efi/efi-secure-key.c    | 21 +++++++++++++++++++++
 include/keys/efi-type.h                  |  7 +++++++
 security/keys/encrypted-keys/encrypted.c | 10 ++++++++++
 3 files changed, 38 insertions(+)

diff --git a/drivers/firmware/efi/efi-secure-key.c b/drivers/firmware/efi/efi-secure-key.c
index 5e72a8c9e13e..aa422ee87f70 100644
--- a/drivers/firmware/efi/efi-secure-key.c
+++ b/drivers/firmware/efi/efi-secure-key.c
@@ -676,6 +676,27 @@ struct key_type key_type_efi = {
 };
 EXPORT_SYMBOL_GPL(key_type_efi);
 
+/*
+ * request_efi_key - request the efi key
+ */
+struct key *request_efi_key(const char *master_desc,
+			    const u8 **master_key, size_t *master_keylen)
+{
+	struct efi_key_payload *epayload;
+	struct key *ekey;
+
+	ekey = request_key(&key_type_efi, master_desc, NULL);
+	if (IS_ERR(ekey))
+		goto error;
+
+	down_read(&ekey->sem);
+	epayload = ekey->payload.data[0];
+	*master_key = epayload->key;
+	*master_keylen = epayload->key_len;
+error:
+	return ekey;
+}
+
 static int __init init_efi_secure_key(void)
 {
 	int ret;
diff --git a/include/keys/efi-type.h b/include/keys/efi-type.h
index 57524b22d42f..bbe649f3eec0 100644
--- a/include/keys/efi-type.h
+++ b/include/keys/efi-type.h
@@ -39,12 +39,19 @@ extern struct key_type key_type_efi;
 #if defined(CONFIG_EFI_SECURE_KEY)
 extern long efi_read_blob(const struct key *key, char __user *buffer,
 			  char *kbuffer, size_t buflen);
+extern struct key *request_efi_key(const char *master_desc,
+			const u8 **master_key, size_t *master_keylen);
 #else
 inline long efi_read_blob(const struct key *key, char __user *buffer,
 			  char *kbuffer, size_t buflen)
 {
 	return 0;
 }
+static inline struct key *request_efi_key(const char *master_desc,
+			const u8 **master_key, size_t *master_keylen)
+{
+	return ERR_PTR(-EOPNOTSUPP);
+}
 #endif
 
 #endif /* _KEYS_EFI_TYPE_H */
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index d92cbf9687c3..b396506afdfc 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -24,6 +24,7 @@
 #include <keys/user-type.h>
 #include <keys/trusted-type.h>
 #include <keys/encrypted-type.h>
+#include <keys/efi-type.h>
 #include <linux/key-type.h>
 #include <linux/random.h>
 #include <linux/rcupdate.h>
@@ -40,6 +41,7 @@
 
 static const char KEY_TRUSTED_PREFIX[] = "trusted:";
 static const char KEY_USER_PREFIX[] = "user:";
+static const char KEY_EFI_PREFIX[] = "efi:";
 static const char hash_alg[] = "sha256";
 static const char hmac_alg[] = "hmac(sha256)";
 static const char blkcipher_alg[] = "cbc(aes)";
@@ -50,6 +52,7 @@ static int blksize;
 
 #define KEY_TRUSTED_PREFIX_LEN (sizeof (KEY_TRUSTED_PREFIX) - 1)
 #define KEY_USER_PREFIX_LEN (sizeof (KEY_USER_PREFIX) - 1)
+#define KEY_EFI_PREFIX_LEN (sizeof (KEY_EFI_PREFIX) - 1)
 #define KEY_ECRYPTFS_DESC_LEN 16
 #define HASH_SIZE SHA256_DIGEST_SIZE
 #define MAX_DATA_SIZE 4096
@@ -142,6 +145,8 @@ static int valid_master_desc(const char *new_desc, const char *orig_desc)
 		prefix_len = KEY_TRUSTED_PREFIX_LEN;
 	else if (!strncmp(new_desc, KEY_USER_PREFIX, KEY_USER_PREFIX_LEN))
 		prefix_len = KEY_USER_PREFIX_LEN;
+	else if (!strncmp(new_desc, KEY_EFI_PREFIX, KEY_EFI_PREFIX_LEN))
+		prefix_len = KEY_EFI_PREFIX_LEN;
 	else
 		return -EINVAL;
 
@@ -434,6 +439,11 @@ static struct key *request_master_key(struct encrypted_key_payload *epayload,
 		mkey = request_user_key(epayload->master_desc +
 					KEY_USER_PREFIX_LEN,
 					master_key, master_keylen);
+	} else if (!strncmp(epayload->master_desc, KEY_EFI_PREFIX,
+			    KEY_EFI_PREFIX_LEN)) {
+		mkey = request_efi_key(epayload->master_desc +
+					KEY_EFI_PREFIX_LEN,
+					master_key, master_keylen);
 	} else
 		goto out;
 
-- 
2.13.6


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

* [PATCH 6/6] key: enforce the secure boot checking when loading efi root key
  2018-08-05  3:21 ` Lee, Chun-Yi
@ 2018-08-05  3:21   ` Lee, Chun-Yi
  -1 siblings, 0 replies; 30+ messages in thread
From: Lee, Chun-Yi @ 2018-08-05  3:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-efi, x86, keyrings, linux-integrity, Lee, Chun-Yi,
	Kees Cook, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Rafael J. Wysocki, Pavel Machek, Chen Yu, Oliver Neukum,
	Ryan Chen, Ard Biesheuvel, David Howells, Mimi Zohar

The EFI root key is not completely secure insecure when the
secure boot is disabled because it can be changed by unsigned EFI
binary before system be handed over to kernel. But in some use
case user does not want the EFI secure key functions be blocked
when secure boot is disabled.

Like the kernel module verification, this patch adds a enforce
kernel configuration option that it can be used to enforce kernel
to checking the secure boot before loading efi root key. And user
can also use kernel parameter to enable it. When this option be
enabled, the EFI root key will not be loaded by kernel when secure
boot is diabled.

Without this option, kernel will be tainted but the EFI root key
can still be loaded.

Cc: Kees Cook <keescook@chromium.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Chen Yu <yu.c.chen@intel.com>
Cc: Oliver Neukum <oneukum@suse.com>
Cc: Ryan Chen <yu.chen.surf@gmail.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
---
 Documentation/admin-guide/kernel-parameters.txt |  6 +++++
 drivers/firmware/efi/Kconfig                    |  8 ++++++
 drivers/firmware/efi/efi-secure-key.c           | 33 ++++++++++++++++++++++---
 include/linux/kernel.h                          |  3 ++-
 kernel/panic.c                                  |  1 +
 5 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 533ff5c68970..7a9ac358793f 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1122,6 +1122,12 @@
 			vendor GUIDs, all of them will be loaded. See
 			Documentation/acpi/ssdt-overlays.txt for details.
 
+	efi-secure-key.sb_enforce [EFI; X86]
+			When EFI_SECURE_KEY is set, this means that
+			EFI root key will not be loaded when secure boot is
+			not enabled. Note that if EFI_SECURE_KEY_SB_ENFORCE
+			is set, that is always true, so this option does
+			nothing.
 
 	eisa_irq_edge=	[PARISC,HW]
 			See header of drivers/parisc/eisa.c.
diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
index 048cf91ae8e8..fba894a4e7b0 100644
--- a/drivers/firmware/efi/Kconfig
+++ b/drivers/firmware/efi/Kconfig
@@ -187,6 +187,14 @@ config EFI_SECURE_KEY
 
 	  If unsure, say N.
 
+config EFI_SECURE_KEY_SB_ENFORCE
+	bool "Force checking secure boot when loading EFI root key"
+	default y
+	depends on EFI_SECURE_KEY
+	help
+	  Skip EFI root key when secure boot is not enabled. Without this,
+	  EFI root key will simply taint the kernel when no secure boot.
+
 endmenu
 
 config UEFI_CPER
diff --git a/drivers/firmware/efi/efi-secure-key.c b/drivers/firmware/efi/efi-secure-key.c
index aa422ee87f70..417d73768887 100644
--- a/drivers/firmware/efi/efi-secure-key.c
+++ b/drivers/firmware/efi/efi-secure-key.c
@@ -15,6 +15,7 @@
 #include <linux/parser.h>
 #include <linux/random.h>
 #include <linux/scatterlist.h>
+#include <linux/moduleparam.h>
 #include <keys/efi-type.h>
 #include <keys/user-type.h>
 #include <crypto/algapi.h>
@@ -27,6 +28,9 @@ static unsigned long rkey_size;
 static bool is_loaded;
 static bool is_secure;
 
+static bool sb_enforce = IS_ENABLED(CONFIG_EFI_SECURE_KEY_SB_ENFORCE);
+module_param(sb_enforce, bool_enable_only, 0644);
+
 static void __init
 print_efi_rkey_setup_data(struct efi_rkey_setup_data *rkey_setup)
 {
@@ -59,11 +63,13 @@ void __init parse_efi_root_key_setup(u64 phys_addr, u32 data_len)
 
 	/* keep efi root key */
 	if (rkey_setup->final_status = EFI_SUCCESS) {
-		memcpy(root_key, rkey_setup->root_key, rkey_setup->key_size);
-		rkey_size = rkey_setup->key_size;
-		is_loaded = true;
 		is_secure = rkey_setup->is_secure;
-		pr_info("EFI root key is loaded.\n");
+		if (is_secure || !sb_enforce) {
+			memcpy(root_key, rkey_setup->root_key, rkey_setup->key_size);
+			rkey_size = rkey_setup->key_size;
+			is_loaded = true;
+			pr_info("EFI root key is loaded.\n");
+		}
 		if (!is_secure) {
 			pr_warn("EFI root key is insecure when no secure boot.\n");
 		}
@@ -75,6 +81,14 @@ void __init parse_efi_root_key_setup(u64 phys_addr, u32 data_len)
 	early_iounmap(setup_data, data_len);
 }
 
+static void __init clean_efi_root_key(void)
+{
+	memzero_explicit(root_key, ROOT_KEY_SIZE);
+	rkey_size = 0;
+	is_loaded = false;
+	is_secure = false;
+}
+
 #define ERK_HASH_SIZE SHA256_DIGEST_SIZE
 #define HMAC_HASH_SIZE SHA256_DIGEST_SIZE
 #define DKEY_SIZE SHA256_DIGEST_SIZE
@@ -705,6 +719,17 @@ static int __init init_efi_secure_key(void)
 	if (!is_loaded)
 		return 0;
 
+	if (!is_secure) {
+		if (sb_enforce) {
+			clean_efi_root_key();
+			pr_info("EFI root key is unloaded because insecure.\n");
+			return 0;
+		} else {
+			add_taint(TAINT_INSECURE_KEY, LOCKDEP_STILL_OK);
+			pr_warn("Tainted kernel because EFI root key is insecure.\n");
+		}
+	}
+
 	hash_tfm = crypto_alloc_shash(hash_alg, 0, CRYPTO_ALG_ASYNC);
 	if (IS_ERR(hash_tfm)) {
 		pr_err("can't allocate %s transform: %ld\n",
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 941dc0a5a877..b45716e54a97 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -565,7 +565,8 @@ extern enum system_states {
 #define TAINT_LIVEPATCH			15
 #define TAINT_AUX			16
 #define TAINT_RANDSTRUCT		17
-#define TAINT_FLAGS_COUNT		18
+#define TAINT_INSECURE_KEY		18
+#define TAINT_FLAGS_COUNT		19
 
 struct taint_flag {
 	char c_true;	/* character printed when tainted */
diff --git a/kernel/panic.c b/kernel/panic.c
index 8b2e002d52eb..d641098a814d 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -327,6 +327,7 @@ const struct taint_flag taint_flags[TAINT_FLAGS_COUNT] = {
 	[ TAINT_LIVEPATCH ]		= { 'K', ' ', true },
 	[ TAINT_AUX ]			= { 'X', ' ', true },
 	[ TAINT_RANDSTRUCT ]		= { 'T', ' ', true },
+	[TAINT_INSECURE_KEY]		= { 'Y', ' ', false },
 };
 
 /**
-- 
2.13.6


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

* [PATCH 6/6] key: enforce the secure boot checking when loading efi root key
@ 2018-08-05  3:21   ` Lee, Chun-Yi
  0 siblings, 0 replies; 30+ messages in thread
From: Lee, Chun-Yi @ 2018-08-05  3:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-efi, x86, keyrings, linux-integrity, Lee, Chun-Yi,
	Kees Cook, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Rafael J. Wysocki, Pavel Machek, Chen Yu, Oliver Neukum,
	Ryan Chen, Ard Biesheuvel, David Howells, Mimi Zohar

The EFI root key is not completely secure insecure when the
secure boot is disabled because it can be changed by unsigned EFI
binary before system be handed over to kernel. But in some use
case user does not want the EFI secure key functions be blocked
when secure boot is disabled.

Like the kernel module verification, this patch adds a enforce
kernel configuration option that it can be used to enforce kernel
to checking the secure boot before loading efi root key. And user
can also use kernel parameter to enable it. When this option be
enabled, the EFI root key will not be loaded by kernel when secure
boot is diabled.

Without this option, kernel will be tainted but the EFI root key
can still be loaded.

Cc: Kees Cook <keescook@chromium.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Chen Yu <yu.c.chen@intel.com>
Cc: Oliver Neukum <oneukum@suse.com>
Cc: Ryan Chen <yu.chen.surf@gmail.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
---
 Documentation/admin-guide/kernel-parameters.txt |  6 +++++
 drivers/firmware/efi/Kconfig                    |  8 ++++++
 drivers/firmware/efi/efi-secure-key.c           | 33 ++++++++++++++++++++++---
 include/linux/kernel.h                          |  3 ++-
 kernel/panic.c                                  |  1 +
 5 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 533ff5c68970..7a9ac358793f 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1122,6 +1122,12 @@
 			vendor GUIDs, all of them will be loaded. See
 			Documentation/acpi/ssdt-overlays.txt for details.
 
+	efi-secure-key.sb_enforce [EFI; X86]
+			When EFI_SECURE_KEY is set, this means that
+			EFI root key will not be loaded when secure boot is
+			not enabled. Note that if EFI_SECURE_KEY_SB_ENFORCE
+			is set, that is always true, so this option does
+			nothing.
 
 	eisa_irq_edge=	[PARISC,HW]
 			See header of drivers/parisc/eisa.c.
diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
index 048cf91ae8e8..fba894a4e7b0 100644
--- a/drivers/firmware/efi/Kconfig
+++ b/drivers/firmware/efi/Kconfig
@@ -187,6 +187,14 @@ config EFI_SECURE_KEY
 
 	  If unsure, say N.
 
+config EFI_SECURE_KEY_SB_ENFORCE
+	bool "Force checking secure boot when loading EFI root key"
+	default y
+	depends on EFI_SECURE_KEY
+	help
+	  Skip EFI root key when secure boot is not enabled. Without this,
+	  EFI root key will simply taint the kernel when no secure boot.
+
 endmenu
 
 config UEFI_CPER
diff --git a/drivers/firmware/efi/efi-secure-key.c b/drivers/firmware/efi/efi-secure-key.c
index aa422ee87f70..417d73768887 100644
--- a/drivers/firmware/efi/efi-secure-key.c
+++ b/drivers/firmware/efi/efi-secure-key.c
@@ -15,6 +15,7 @@
 #include <linux/parser.h>
 #include <linux/random.h>
 #include <linux/scatterlist.h>
+#include <linux/moduleparam.h>
 #include <keys/efi-type.h>
 #include <keys/user-type.h>
 #include <crypto/algapi.h>
@@ -27,6 +28,9 @@ static unsigned long rkey_size;
 static bool is_loaded;
 static bool is_secure;
 
+static bool sb_enforce = IS_ENABLED(CONFIG_EFI_SECURE_KEY_SB_ENFORCE);
+module_param(sb_enforce, bool_enable_only, 0644);
+
 static void __init
 print_efi_rkey_setup_data(struct efi_rkey_setup_data *rkey_setup)
 {
@@ -59,11 +63,13 @@ void __init parse_efi_root_key_setup(u64 phys_addr, u32 data_len)
 
 	/* keep efi root key */
 	if (rkey_setup->final_status == EFI_SUCCESS) {
-		memcpy(root_key, rkey_setup->root_key, rkey_setup->key_size);
-		rkey_size = rkey_setup->key_size;
-		is_loaded = true;
 		is_secure = rkey_setup->is_secure;
-		pr_info("EFI root key is loaded.\n");
+		if (is_secure || !sb_enforce) {
+			memcpy(root_key, rkey_setup->root_key, rkey_setup->key_size);
+			rkey_size = rkey_setup->key_size;
+			is_loaded = true;
+			pr_info("EFI root key is loaded.\n");
+		}
 		if (!is_secure) {
 			pr_warn("EFI root key is insecure when no secure boot.\n");
 		}
@@ -75,6 +81,14 @@ void __init parse_efi_root_key_setup(u64 phys_addr, u32 data_len)
 	early_iounmap(setup_data, data_len);
 }
 
+static void __init clean_efi_root_key(void)
+{
+	memzero_explicit(root_key, ROOT_KEY_SIZE);
+	rkey_size = 0;
+	is_loaded = false;
+	is_secure = false;
+}
+
 #define ERK_HASH_SIZE SHA256_DIGEST_SIZE
 #define HMAC_HASH_SIZE SHA256_DIGEST_SIZE
 #define DKEY_SIZE SHA256_DIGEST_SIZE
@@ -705,6 +719,17 @@ static int __init init_efi_secure_key(void)
 	if (!is_loaded)
 		return 0;
 
+	if (!is_secure) {
+		if (sb_enforce) {
+			clean_efi_root_key();
+			pr_info("EFI root key is unloaded because insecure.\n");
+			return 0;
+		} else {
+			add_taint(TAINT_INSECURE_KEY, LOCKDEP_STILL_OK);
+			pr_warn("Tainted kernel because EFI root key is insecure.\n");
+		}
+	}
+
 	hash_tfm = crypto_alloc_shash(hash_alg, 0, CRYPTO_ALG_ASYNC);
 	if (IS_ERR(hash_tfm)) {
 		pr_err("can't allocate %s transform: %ld\n",
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 941dc0a5a877..b45716e54a97 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -565,7 +565,8 @@ extern enum system_states {
 #define TAINT_LIVEPATCH			15
 #define TAINT_AUX			16
 #define TAINT_RANDSTRUCT		17
-#define TAINT_FLAGS_COUNT		18
+#define TAINT_INSECURE_KEY		18
+#define TAINT_FLAGS_COUNT		19
 
 struct taint_flag {
 	char c_true;	/* character printed when tainted */
diff --git a/kernel/panic.c b/kernel/panic.c
index 8b2e002d52eb..d641098a814d 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -327,6 +327,7 @@ const struct taint_flag taint_flags[TAINT_FLAGS_COUNT] = {
 	[ TAINT_LIVEPATCH ]		= { 'K', ' ', true },
 	[ TAINT_AUX ]			= { 'X', ' ', true },
 	[ TAINT_RANDSTRUCT ]		= { 'T', ' ', true },
+	[TAINT_INSECURE_KEY]		= { 'Y', ' ', false },
 };
 
 /**
-- 
2.13.6


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

* Re: [PATCH 0/6][RFC] Add EFI secure key to key retention service
  2018-08-05  3:21 ` Lee, Chun-Yi
@ 2018-08-05  7:25   ` Ard Biesheuvel
  -1 siblings, 0 replies; 30+ messages in thread
From: Ard Biesheuvel @ 2018-08-05  7:25 UTC (permalink / raw)
  To: Lee, Chun-Yi
  Cc: Linux Kernel Mailing List, linux-efi, the arch/x86 maintainers,
	keyrings, linux-integrity, Lee, Chun-Yi, Kees Cook,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Rafael J. Wysocki,
	Pavel Machek, Chen Yu, Oliver Neukum, Ryan Chen, David Howells,
	Mimi Zohar

Hello Chun,yi,

On 5 August 2018 at 05:21, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
> When secure boot is enabled, only signed EFI binary can access
> EFI boot service variable before ExitBootService. Which means that
> the EFI boot service variable is secure.
>

No it, isn't, and this is a very dangerous assumption to make.

'Secure' means different things to different people. 'Secure boot' is
a misnomer, since it is too vague: it should be called 'authenticated
boot', and the catch is that authentication using public-key crypto
does not involve secrets at all. The UEFI variable store was not
designed with confidentiality in mind, and assuming [given the
reputation of EFI on the implementation side] that you can use it to
keep secrets is rather unwise imho.

> This patch set add functions to EFI boot stub to generate a 512-bit
> random number that it can be used as a root key for encryption and
> authentication. This root key will be kept in EFI boot service variable.
> EFI boot stub will read and transfer ERK (efi root key) to kernel.
>
> At runtime, the ERK can be used to encrypted/authentication other
> random number to generate EFI secure key. The EFI secure key can be
> a new master key type for encrypted key. It's useful for hibernation
> or evm.
>
> Here is the proof of concept for using EFI secure key in hibernation:
>   https://github.com/joeyli/linux-s4sign/commit/6311e97038974bc5de8121769fb4d34470009566
>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Chen Yu <yu.c.chen@intel.com>
> Cc: Oliver Neukum <oneukum@suse.com>
> Cc: Ryan Chen <yu.chen.surf@gmail.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: David Howells <dhowells@redhat.com>
> Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
>
> Lee, Chun-Yi (6):
>   x86/KASLR: make getting random long number function public
>   efi: the function transfers status to string
>   efi: generate efi root key in EFI boot stub
>   key: add EFI secure key type
>   key: add EFI secure key as a master key type
>   key: enforce the secure boot checking when loading efi root key
>
>  Documentation/admin-guide/kernel-parameters.txt |   6 +
>  arch/x86/boot/compressed/Makefile               |   1 +
>  arch/x86/boot/compressed/cpuflags.c             |   2 +-
>  arch/x86/boot/compressed/eboot.c                |   2 +
>  arch/x86/boot/compressed/efi_root_key.c         | 212 +++++++
>  arch/x86/boot/compressed/kaslr.c                |  21 -
>  arch/x86/boot/compressed/misc.c                 |  17 +
>  arch/x86/boot/compressed/misc.h                 |  12 +-
>  arch/x86/include/asm/efi.h                      |  13 +
>  arch/x86/include/uapi/asm/bootparam.h           |   1 +
>  arch/x86/kernel/setup.c                         |   3 +
>  arch/x86/lib/kaslr.c                            |  61 +-
>  arch/x86/lib/random.c                           |  68 +++
>  drivers/firmware/efi/Kconfig                    |  31 +
>  drivers/firmware/efi/Makefile                   |   1 +
>  drivers/firmware/efi/efi-secure-key.c           | 748 ++++++++++++++++++++++++
>  include/keys/efi-type.h                         |  57 ++
>  include/linux/efi.h                             |  40 ++
>  include/linux/kernel.h                          |   3 +-
>  kernel/panic.c                                  |   1 +
>  security/keys/encrypted-keys/encrypted.c        |  10 +
>  21 files changed, 1226 insertions(+), 84 deletions(-)
>  create mode 100644 arch/x86/boot/compressed/efi_root_key.c
>  create mode 100644 arch/x86/lib/random.c
>  create mode 100644 drivers/firmware/efi/efi-secure-key.c
>  create mode 100644 include/keys/efi-type.h
>
> --
> 2.13.6
>

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

* Re: [PATCH 0/6][RFC] Add EFI secure key to key retention service
@ 2018-08-05  7:25   ` Ard Biesheuvel
  0 siblings, 0 replies; 30+ messages in thread
From: Ard Biesheuvel @ 2018-08-05  7:25 UTC (permalink / raw)
  To: Lee, Chun-Yi
  Cc: Linux Kernel Mailing List, linux-efi, the arch/x86 maintainers,
	keyrings, linux-integrity, Lee, Chun-Yi, Kees Cook,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Rafael J. Wysocki,
	Pavel Machek, Chen Yu, Oliver Neukum, Ryan Chen, David Howells,
	Mimi Zohar

Hello Chun,yi,

On 5 August 2018 at 05:21, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
> When secure boot is enabled, only signed EFI binary can access
> EFI boot service variable before ExitBootService. Which means that
> the EFI boot service variable is secure.
>

No it, isn't, and this is a very dangerous assumption to make.

'Secure' means different things to different people. 'Secure boot' is
a misnomer, since it is too vague: it should be called 'authenticated
boot', and the catch is that authentication using public-key crypto
does not involve secrets at all. The UEFI variable store was not
designed with confidentiality in mind, and assuming [given the
reputation of EFI on the implementation side] that you can use it to
keep secrets is rather unwise imho.

> This patch set add functions to EFI boot stub to generate a 512-bit
> random number that it can be used as a root key for encryption and
> authentication. This root key will be kept in EFI boot service variable.
> EFI boot stub will read and transfer ERK (efi root key) to kernel.
>
> At runtime, the ERK can be used to encrypted/authentication other
> random number to generate EFI secure key. The EFI secure key can be
> a new master key type for encrypted key. It's useful for hibernation
> or evm.
>
> Here is the proof of concept for using EFI secure key in hibernation:
>   https://github.com/joeyli/linux-s4sign/commit/6311e97038974bc5de8121769fb4d34470009566
>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Chen Yu <yu.c.chen@intel.com>
> Cc: Oliver Neukum <oneukum@suse.com>
> Cc: Ryan Chen <yu.chen.surf@gmail.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: David Howells <dhowells@redhat.com>
> Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
>
> Lee, Chun-Yi (6):
>   x86/KASLR: make getting random long number function public
>   efi: the function transfers status to string
>   efi: generate efi root key in EFI boot stub
>   key: add EFI secure key type
>   key: add EFI secure key as a master key type
>   key: enforce the secure boot checking when loading efi root key
>
>  Documentation/admin-guide/kernel-parameters.txt |   6 +
>  arch/x86/boot/compressed/Makefile               |   1 +
>  arch/x86/boot/compressed/cpuflags.c             |   2 +-
>  arch/x86/boot/compressed/eboot.c                |   2 +
>  arch/x86/boot/compressed/efi_root_key.c         | 212 +++++++
>  arch/x86/boot/compressed/kaslr.c                |  21 -
>  arch/x86/boot/compressed/misc.c                 |  17 +
>  arch/x86/boot/compressed/misc.h                 |  12 +-
>  arch/x86/include/asm/efi.h                      |  13 +
>  arch/x86/include/uapi/asm/bootparam.h           |   1 +
>  arch/x86/kernel/setup.c                         |   3 +
>  arch/x86/lib/kaslr.c                            |  61 +-
>  arch/x86/lib/random.c                           |  68 +++
>  drivers/firmware/efi/Kconfig                    |  31 +
>  drivers/firmware/efi/Makefile                   |   1 +
>  drivers/firmware/efi/efi-secure-key.c           | 748 ++++++++++++++++++++++++
>  include/keys/efi-type.h                         |  57 ++
>  include/linux/efi.h                             |  40 ++
>  include/linux/kernel.h                          |   3 +-
>  kernel/panic.c                                  |   1 +
>  security/keys/encrypted-keys/encrypted.c        |  10 +
>  21 files changed, 1226 insertions(+), 84 deletions(-)
>  create mode 100644 arch/x86/boot/compressed/efi_root_key.c
>  create mode 100644 arch/x86/lib/random.c
>  create mode 100644 drivers/firmware/efi/efi-secure-key.c
>  create mode 100644 include/keys/efi-type.h
>
> --
> 2.13.6
>

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

* Re: [PATCH 1/6] x86/KASLR: make getting random long number function public
  2018-08-05  3:21   ` Lee, Chun-Yi
@ 2018-08-05  8:16     ` Ard Biesheuvel
  -1 siblings, 0 replies; 30+ messages in thread
From: Ard Biesheuvel @ 2018-08-05  8:16 UTC (permalink / raw)
  To: Lee, Chun-Yi
  Cc: Linux Kernel Mailing List, linux-efi, the arch/x86 maintainers,
	keyrings, linux-integrity, Lee, Chun-Yi, Kees Cook,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Rafael J. Wysocki,
	Pavel Machek, Chen Yu, Oliver Neukum, Ryan Chen, David Howells,
	Mimi Zohar

On 5 August 2018 at 05:21, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
> Separating the functions for getting random long number from KASLR
> to x86 library, then it can be used to generate random long for
> EFI root key.
>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Chen Yu <yu.c.chen@intel.com>
> Cc: Oliver Neukum <oneukum@suse.com>
> Cc: Ryan Chen <yu.chen.surf@gmail.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: David Howells <dhowells@redhat.com>
> Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>

Not my call to make perhaps, but i'm nacking it anyway.

Playing games with counters and other low entropy inputs may have been
acceptable for kaslr on x86 when it was first introduced, but using it
to generate symmetric keys is really out of the question.

On modern x86, i suppose rdrand is a given, and these days we have
EFI_RNG_PROTOCOL as well (and an open source UEFI driver for ChasoKey,
btw - perhaps we should try and get MS to sign that?), although I'm
not sure whether any x86 support this out of the box now.

BTW using low entropy inputs on top of rdrand or EFI_RNG_PROTOCOL is
fine, if you're paranoid, but if you have neither of those, you should
really fail the call.


> ---
>  arch/x86/boot/compressed/kaslr.c | 21 -------------
>  arch/x86/boot/compressed/misc.c  | 17 ++++++++++
>  arch/x86/boot/compressed/misc.h  |  6 ++++
>  arch/x86/lib/kaslr.c             | 61 ++---------------------------------
>  arch/x86/lib/random.c            | 68 ++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 93 insertions(+), 80 deletions(-)
>  create mode 100644 arch/x86/lib/random.c
>
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index b87a7582853d..0f40d2178ebc 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -33,13 +33,11 @@
>  #include "error.h"
>  #include "../string.h"
>
> -#include <generated/compile.h>
>  #include <linux/module.h>
>  #include <linux/uts.h>
>  #include <linux/utsname.h>
>  #include <linux/ctype.h>
>  #include <linux/efi.h>
> -#include <generated/utsrelease.h>
>  #include <asm/efi.h>
>
>  /* Macros used by the included decompressor code below. */
> @@ -57,25 +55,6 @@ extern unsigned long get_cmd_line_ptr(void);
>  /* Used by PAGE_KERN* macros: */
>  pteval_t __default_kernel_pte_mask __read_mostly = ~0;
>
> -/* Simplified build-specific string for starting entropy. */
> -static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
> -               LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
> -
> -static unsigned long rotate_xor(unsigned long hash, const void *area,
> -                               size_t size)
> -{
> -       size_t i;
> -       unsigned long *ptr = (unsigned long *)area;
> -
> -       for (i = 0; i < size / sizeof(hash); i++) {
> -               /* Rotate by odd number of bits and XOR. */
> -               hash = (hash << ((sizeof(hash) * 8) - 7)) | (hash >> 7);
> -               hash ^= ptr[i];
> -       }
> -
> -       return hash;
> -}
> -
>  /* Attempt to create a simple but unpredictable starting entropy. */
>  static unsigned long get_boot_seed(void)
>  {
> diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
> index 8dd1d5ccae58..eb0ab9cad4e4 100644
> --- a/arch/x86/boot/compressed/misc.c
> +++ b/arch/x86/boot/compressed/misc.c
> @@ -426,3 +426,20 @@ void fortify_panic(const char *name)
>  {
>         error("detected buffer overflow");
>  }
> +
> +#if CONFIG_RANDOMIZE_BASE
> +unsigned long rotate_xor(unsigned long hash, const void *area,
> +                       size_t size)
> +{
> +       size_t i;
> +       unsigned long *ptr = (unsigned long *)area;
> +
> +       for (i = 0; i < size / sizeof(hash); i++) {
> +               /* Rotate by odd number of bits and XOR. */
> +               hash = (hash << ((sizeof(hash) * 8) - 7)) | (hash >> 7);
> +               hash ^= ptr[i];
> +       }
> +
> +       return hash;
> +}
> +#endif
> diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
> index a423bdb42686..957f327ad83c 100644
> --- a/arch/x86/boot/compressed/misc.h
> +++ b/arch/x86/boot/compressed/misc.h
> @@ -70,6 +70,8 @@ int cmdline_find_option_bool(const char *option);
>
>
>  #if CONFIG_RANDOMIZE_BASE
> +#include <generated/compile.h>
> +#include <generated/utsrelease.h>
>  /* kaslr.c */
>  void choose_random_location(unsigned long input,
>                             unsigned long input_size,
> @@ -78,6 +80,10 @@ void choose_random_location(unsigned long input,
>                             unsigned long *virt_addr);
>  /* cpuflags.c */
>  bool has_cpuflag(int flag);
> +/* Simplified build-specific string for starting entropy. */
> +static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
> +               LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
> +unsigned long rotate_xor(unsigned long hash, const void *area, size_t size);
>  #else
>  static inline void choose_random_location(unsigned long input,
>                                           unsigned long input_size,
> diff --git a/arch/x86/lib/kaslr.c b/arch/x86/lib/kaslr.c
> index 79778ab200e4..29ed9bfde5a5 100644
> --- a/arch/x86/lib/kaslr.c
> +++ b/arch/x86/lib/kaslr.c
> @@ -26,67 +26,10 @@
>  #define get_boot_seed() kaslr_offset()
>  #endif
>
> -#define I8254_PORT_CONTROL     0x43
> -#define I8254_PORT_COUNTER0    0x40
> -#define I8254_CMD_READBACK     0xC0
> -#define I8254_SELECT_COUNTER0  0x02
> -#define I8254_STATUS_NOTREADY  0x40
> -static inline u16 i8254(void)
> -{
> -       u16 status, timer;
> -
> -       do {
> -               outb(I8254_PORT_CONTROL,
> -                    I8254_CMD_READBACK | I8254_SELECT_COUNTER0);
> -               status = inb(I8254_PORT_COUNTER0);
> -               timer  = inb(I8254_PORT_COUNTER0);
> -               timer |= inb(I8254_PORT_COUNTER0) << 8;
> -       } while (status & I8254_STATUS_NOTREADY);
> -
> -       return timer;
> -}
> +#include "random.c"
>
>  unsigned long kaslr_get_random_long(const char *purpose)
>  {
> -#ifdef CONFIG_X86_64
> -       const unsigned long mix_const = 0x5d6008cbf3848dd3UL;
> -#else
> -       const unsigned long mix_const = 0x3f39e593UL;
> -#endif
> -       unsigned long raw, random = get_boot_seed();
> -       bool use_i8254 = true;
> -
> -       debug_putstr(purpose);
>         debug_putstr(" KASLR using");
> -
> -       if (has_cpuflag(X86_FEATURE_RDRAND)) {
> -               debug_putstr(" RDRAND");
> -               if (rdrand_long(&raw)) {
> -                       random ^= raw;
> -                       use_i8254 = false;
> -               }
> -       }
> -
> -       if (has_cpuflag(X86_FEATURE_TSC)) {
> -               debug_putstr(" RDTSC");
> -               raw = rdtsc();
> -
> -               random ^= raw;
> -               use_i8254 = false;
> -       }
> -
> -       if (use_i8254) {
> -               debug_putstr(" i8254");
> -               random ^= i8254();
> -       }
> -
> -       /* Circular multiply for better bit diffusion */
> -       asm(_ASM_MUL "%3"
> -           : "=a" (random), "=d" (raw)
> -           : "a" (random), "rm" (mix_const));
> -       random += raw;
> -
> -       debug_putstr("...\n");
> -
> -       return random;
> +       return get_random_long(purpose);
>  }
> diff --git a/arch/x86/lib/random.c b/arch/x86/lib/random.c
> new file mode 100644
> index 000000000000..f2fe6a784c98
> --- /dev/null
> +++ b/arch/x86/lib/random.c
> @@ -0,0 +1,68 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <asm/io.h>
> +#include <asm/archrandom.h>
> +
> +#define I8254_PORT_CONTROL     0x43
> +#define I8254_PORT_COUNTER0    0x40
> +#define I8254_CMD_READBACK     0xC0
> +#define I8254_SELECT_COUNTER0  0x02
> +#define I8254_STATUS_NOTREADY  0x40
> +static inline u16 i8254(void)
> +{
> +       u16 status, timer;
> +
> +       do {
> +               outb(I8254_PORT_CONTROL,
> +                    I8254_CMD_READBACK | I8254_SELECT_COUNTER0);
> +               status = inb(I8254_PORT_COUNTER0);
> +               timer  = inb(I8254_PORT_COUNTER0);
> +               timer |= inb(I8254_PORT_COUNTER0) << 8;
> +       } while (status & I8254_STATUS_NOTREADY);
> +
> +       return timer;
> +}
> +
> +static unsigned long get_random_long(const char *purpose)
> +{
> +#ifdef CONFIG_X86_64
> +       const unsigned long mix_const = 0x5d6008cbf3848dd3UL;
> +#else
> +       const unsigned long mix_const = 0x3f39e593UL;
> +#endif
> +       unsigned long raw, random = get_boot_seed();
> +       bool use_i8254 = true;
> +
> +       debug_putstr(purpose);
> +
> +       if (has_cpuflag(X86_FEATURE_RDRAND)) {
> +               debug_putstr(" RDRAND");
> +               if (rdrand_long(&raw)) {
> +                       random ^= raw;
> +                       use_i8254 = false;
> +               }
> +       }
> +
> +       if (has_cpuflag(X86_FEATURE_TSC)) {
> +               debug_putstr(" RDTSC");
> +               raw = rdtsc();
> +
> +               random ^= raw;
> +               use_i8254 = false;
> +       }
> +
> +       if (use_i8254) {
> +               debug_putstr(" i8254");
> +               random ^= i8254();
> +       }
> +
> +       /* Circular multiply for better bit diffusion */
> +       asm(_ASM_MUL "%3"
> +           : "=a" (random), "=d" (raw)
> +           : "a" (random), "rm" (mix_const));
> +       random += raw;
> +
> +       debug_putstr("...\n");
> +
> +       return random;
> +}
> --
> 2.13.6
>

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

* Re: [PATCH 1/6] x86/KASLR: make getting random long number function public
@ 2018-08-05  8:16     ` Ard Biesheuvel
  0 siblings, 0 replies; 30+ messages in thread
From: Ard Biesheuvel @ 2018-08-05  8:16 UTC (permalink / raw)
  To: Lee, Chun-Yi
  Cc: Linux Kernel Mailing List, linux-efi, the arch/x86 maintainers,
	keyrings, linux-integrity, Lee, Chun-Yi, Kees Cook,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Rafael J. Wysocki,
	Pavel Machek, Chen Yu, Oliver Neukum, Ryan Chen, David Howells,
	Mimi Zohar

On 5 August 2018 at 05:21, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
> Separating the functions for getting random long number from KASLR
> to x86 library, then it can be used to generate random long for
> EFI root key.
>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Chen Yu <yu.c.chen@intel.com>
> Cc: Oliver Neukum <oneukum@suse.com>
> Cc: Ryan Chen <yu.chen.surf@gmail.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: David Howells <dhowells@redhat.com>
> Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>

Not my call to make perhaps, but i'm nacking it anyway.

Playing games with counters and other low entropy inputs may have been
acceptable for kaslr on x86 when it was first introduced, but using it
to generate symmetric keys is really out of the question.

On modern x86, i suppose rdrand is a given, and these days we have
EFI_RNG_PROTOCOL as well (and an open source UEFI driver for ChasoKey,
btw - perhaps we should try and get MS to sign that?), although I'm
not sure whether any x86 support this out of the box now.

BTW using low entropy inputs on top of rdrand or EFI_RNG_PROTOCOL is
fine, if you're paranoid, but if you have neither of those, you should
really fail the call.


> ---
>  arch/x86/boot/compressed/kaslr.c | 21 -------------
>  arch/x86/boot/compressed/misc.c  | 17 ++++++++++
>  arch/x86/boot/compressed/misc.h  |  6 ++++
>  arch/x86/lib/kaslr.c             | 61 ++---------------------------------
>  arch/x86/lib/random.c            | 68 ++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 93 insertions(+), 80 deletions(-)
>  create mode 100644 arch/x86/lib/random.c
>
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index b87a7582853d..0f40d2178ebc 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -33,13 +33,11 @@
>  #include "error.h"
>  #include "../string.h"
>
> -#include <generated/compile.h>
>  #include <linux/module.h>
>  #include <linux/uts.h>
>  #include <linux/utsname.h>
>  #include <linux/ctype.h>
>  #include <linux/efi.h>
> -#include <generated/utsrelease.h>
>  #include <asm/efi.h>
>
>  /* Macros used by the included decompressor code below. */
> @@ -57,25 +55,6 @@ extern unsigned long get_cmd_line_ptr(void);
>  /* Used by PAGE_KERN* macros: */
>  pteval_t __default_kernel_pte_mask __read_mostly = ~0;
>
> -/* Simplified build-specific string for starting entropy. */
> -static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
> -               LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
> -
> -static unsigned long rotate_xor(unsigned long hash, const void *area,
> -                               size_t size)
> -{
> -       size_t i;
> -       unsigned long *ptr = (unsigned long *)area;
> -
> -       for (i = 0; i < size / sizeof(hash); i++) {
> -               /* Rotate by odd number of bits and XOR. */
> -               hash = (hash << ((sizeof(hash) * 8) - 7)) | (hash >> 7);
> -               hash ^= ptr[i];
> -       }
> -
> -       return hash;
> -}
> -
>  /* Attempt to create a simple but unpredictable starting entropy. */
>  static unsigned long get_boot_seed(void)
>  {
> diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
> index 8dd1d5ccae58..eb0ab9cad4e4 100644
> --- a/arch/x86/boot/compressed/misc.c
> +++ b/arch/x86/boot/compressed/misc.c
> @@ -426,3 +426,20 @@ void fortify_panic(const char *name)
>  {
>         error("detected buffer overflow");
>  }
> +
> +#if CONFIG_RANDOMIZE_BASE
> +unsigned long rotate_xor(unsigned long hash, const void *area,
> +                       size_t size)
> +{
> +       size_t i;
> +       unsigned long *ptr = (unsigned long *)area;
> +
> +       for (i = 0; i < size / sizeof(hash); i++) {
> +               /* Rotate by odd number of bits and XOR. */
> +               hash = (hash << ((sizeof(hash) * 8) - 7)) | (hash >> 7);
> +               hash ^= ptr[i];
> +       }
> +
> +       return hash;
> +}
> +#endif
> diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
> index a423bdb42686..957f327ad83c 100644
> --- a/arch/x86/boot/compressed/misc.h
> +++ b/arch/x86/boot/compressed/misc.h
> @@ -70,6 +70,8 @@ int cmdline_find_option_bool(const char *option);
>
>
>  #if CONFIG_RANDOMIZE_BASE
> +#include <generated/compile.h>
> +#include <generated/utsrelease.h>
>  /* kaslr.c */
>  void choose_random_location(unsigned long input,
>                             unsigned long input_size,
> @@ -78,6 +80,10 @@ void choose_random_location(unsigned long input,
>                             unsigned long *virt_addr);
>  /* cpuflags.c */
>  bool has_cpuflag(int flag);
> +/* Simplified build-specific string for starting entropy. */
> +static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
> +               LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
> +unsigned long rotate_xor(unsigned long hash, const void *area, size_t size);
>  #else
>  static inline void choose_random_location(unsigned long input,
>                                           unsigned long input_size,
> diff --git a/arch/x86/lib/kaslr.c b/arch/x86/lib/kaslr.c
> index 79778ab200e4..29ed9bfde5a5 100644
> --- a/arch/x86/lib/kaslr.c
> +++ b/arch/x86/lib/kaslr.c
> @@ -26,67 +26,10 @@
>  #define get_boot_seed() kaslr_offset()
>  #endif
>
> -#define I8254_PORT_CONTROL     0x43
> -#define I8254_PORT_COUNTER0    0x40
> -#define I8254_CMD_READBACK     0xC0
> -#define I8254_SELECT_COUNTER0  0x02
> -#define I8254_STATUS_NOTREADY  0x40
> -static inline u16 i8254(void)
> -{
> -       u16 status, timer;
> -
> -       do {
> -               outb(I8254_PORT_CONTROL,
> -                    I8254_CMD_READBACK | I8254_SELECT_COUNTER0);
> -               status = inb(I8254_PORT_COUNTER0);
> -               timer  = inb(I8254_PORT_COUNTER0);
> -               timer |= inb(I8254_PORT_COUNTER0) << 8;
> -       } while (status & I8254_STATUS_NOTREADY);
> -
> -       return timer;
> -}
> +#include "random.c"
>
>  unsigned long kaslr_get_random_long(const char *purpose)
>  {
> -#ifdef CONFIG_X86_64
> -       const unsigned long mix_const = 0x5d6008cbf3848dd3UL;
> -#else
> -       const unsigned long mix_const = 0x3f39e593UL;
> -#endif
> -       unsigned long raw, random = get_boot_seed();
> -       bool use_i8254 = true;
> -
> -       debug_putstr(purpose);
>         debug_putstr(" KASLR using");
> -
> -       if (has_cpuflag(X86_FEATURE_RDRAND)) {
> -               debug_putstr(" RDRAND");
> -               if (rdrand_long(&raw)) {
> -                       random ^= raw;
> -                       use_i8254 = false;
> -               }
> -       }
> -
> -       if (has_cpuflag(X86_FEATURE_TSC)) {
> -               debug_putstr(" RDTSC");
> -               raw = rdtsc();
> -
> -               random ^= raw;
> -               use_i8254 = false;
> -       }
> -
> -       if (use_i8254) {
> -               debug_putstr(" i8254");
> -               random ^= i8254();
> -       }
> -
> -       /* Circular multiply for better bit diffusion */
> -       asm(_ASM_MUL "%3"
> -           : "=a" (random), "=d" (raw)
> -           : "a" (random), "rm" (mix_const));
> -       random += raw;
> -
> -       debug_putstr("...\n");
> -
> -       return random;
> +       return get_random_long(purpose);
>  }
> diff --git a/arch/x86/lib/random.c b/arch/x86/lib/random.c
> new file mode 100644
> index 000000000000..f2fe6a784c98
> --- /dev/null
> +++ b/arch/x86/lib/random.c
> @@ -0,0 +1,68 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <asm/io.h>
> +#include <asm/archrandom.h>
> +
> +#define I8254_PORT_CONTROL     0x43
> +#define I8254_PORT_COUNTER0    0x40
> +#define I8254_CMD_READBACK     0xC0
> +#define I8254_SELECT_COUNTER0  0x02
> +#define I8254_STATUS_NOTREADY  0x40
> +static inline u16 i8254(void)
> +{
> +       u16 status, timer;
> +
> +       do {
> +               outb(I8254_PORT_CONTROL,
> +                    I8254_CMD_READBACK | I8254_SELECT_COUNTER0);
> +               status = inb(I8254_PORT_COUNTER0);
> +               timer  = inb(I8254_PORT_COUNTER0);
> +               timer |= inb(I8254_PORT_COUNTER0) << 8;
> +       } while (status & I8254_STATUS_NOTREADY);
> +
> +       return timer;
> +}
> +
> +static unsigned long get_random_long(const char *purpose)
> +{
> +#ifdef CONFIG_X86_64
> +       const unsigned long mix_const = 0x5d6008cbf3848dd3UL;
> +#else
> +       const unsigned long mix_const = 0x3f39e593UL;
> +#endif
> +       unsigned long raw, random = get_boot_seed();
> +       bool use_i8254 = true;
> +
> +       debug_putstr(purpose);
> +
> +       if (has_cpuflag(X86_FEATURE_RDRAND)) {
> +               debug_putstr(" RDRAND");
> +               if (rdrand_long(&raw)) {
> +                       random ^= raw;
> +                       use_i8254 = false;
> +               }
> +       }
> +
> +       if (has_cpuflag(X86_FEATURE_TSC)) {
> +               debug_putstr(" RDTSC");
> +               raw = rdtsc();
> +
> +               random ^= raw;
> +               use_i8254 = false;
> +       }
> +
> +       if (use_i8254) {
> +               debug_putstr(" i8254");
> +               random ^= i8254();
> +       }
> +
> +       /* Circular multiply for better bit diffusion */
> +       asm(_ASM_MUL "%3"
> +           : "=a" (random), "=d" (raw)
> +           : "a" (random), "rm" (mix_const));
> +       random += raw;
> +
> +       debug_putstr("...\n");
> +
> +       return random;
> +}
> --
> 2.13.6
>

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

* Re: [PATCH 2/6] efi: the function transfers status to string
  2018-08-05  3:21   ` Lee, Chun-Yi
@ 2018-08-05  8:17     ` Ard Biesheuvel
  -1 siblings, 0 replies; 30+ messages in thread
From: Ard Biesheuvel @ 2018-08-05  8:17 UTC (permalink / raw)
  To: Lee, Chun-Yi
  Cc: Linux Kernel Mailing List, linux-efi, the arch/x86 maintainers,
	keyrings, linux-integrity, Lee, Chun-Yi, Kees Cook,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Rafael J. Wysocki,
	Pavel Machek, Chen Yu, Oliver Neukum, Ryan Chen, David Howells,
	Mimi Zohar

On 5 August 2018 at 05:21, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
> This function can be used to transfer EFI status code to string
> for printing out debug message.
>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Chen Yu <yu.c.chen@intel.com>
> Cc: Oliver Neukum <oneukum@suse.com>
> Cc: Ryan Chen <yu.chen.surf@gmail.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: David Howells <dhowells@redhat.com>
> Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>

Very useful.

Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---
>  include/linux/efi.h | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 56add823f190..744cf92fe18e 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -1651,4 +1651,30 @@ struct linux_efi_tpm_eventlog {
>
>  extern int efi_tpm_eventlog_init(void);
>
> +#define EFI_STATUS_STR(_status) \
> +case EFI_##_status: \
> +       return "EFI_" __stringify(_status);
> +
> +static inline char *
> +efi_status_to_str(efi_status_t status)
> +{
> +       switch (status) {
> +       EFI_STATUS_STR(SUCCESS)
> +       EFI_STATUS_STR(LOAD_ERROR)
> +       EFI_STATUS_STR(INVALID_PARAMETER)
> +       EFI_STATUS_STR(UNSUPPORTED)
> +       EFI_STATUS_STR(BAD_BUFFER_SIZE)
> +       EFI_STATUS_STR(BUFFER_TOO_SMALL)
> +       EFI_STATUS_STR(NOT_READY)
> +       EFI_STATUS_STR(DEVICE_ERROR)
> +       EFI_STATUS_STR(WRITE_PROTECTED)
> +       EFI_STATUS_STR(OUT_OF_RESOURCES)
> +       EFI_STATUS_STR(NOT_FOUND)
> +       EFI_STATUS_STR(ABORTED)
> +       EFI_STATUS_STR(SECURITY_VIOLATION)
> +       }
> +
> +       return "";
> +}
> +
>  #endif /* _LINUX_EFI_H */
> --
> 2.13.6
>

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

* Re: [PATCH 2/6] efi: the function transfers status to string
@ 2018-08-05  8:17     ` Ard Biesheuvel
  0 siblings, 0 replies; 30+ messages in thread
From: Ard Biesheuvel @ 2018-08-05  8:17 UTC (permalink / raw)
  To: Lee, Chun-Yi
  Cc: Linux Kernel Mailing List, linux-efi, the arch/x86 maintainers,
	keyrings, linux-integrity, Lee, Chun-Yi, Kees Cook,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Rafael J. Wysocki,
	Pavel Machek, Chen Yu, Oliver Neukum, Ryan Chen, David Howells,
	Mimi Zohar

On 5 August 2018 at 05:21, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
> This function can be used to transfer EFI status code to string
> for printing out debug message.
>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Chen Yu <yu.c.chen@intel.com>
> Cc: Oliver Neukum <oneukum@suse.com>
> Cc: Ryan Chen <yu.chen.surf@gmail.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: David Howells <dhowells@redhat.com>
> Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>

Very useful.

Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---
>  include/linux/efi.h | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 56add823f190..744cf92fe18e 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -1651,4 +1651,30 @@ struct linux_efi_tpm_eventlog {
>
>  extern int efi_tpm_eventlog_init(void);
>
> +#define EFI_STATUS_STR(_status) \
> +case EFI_##_status: \
> +       return "EFI_" __stringify(_status);
> +
> +static inline char *
> +efi_status_to_str(efi_status_t status)
> +{
> +       switch (status) {
> +       EFI_STATUS_STR(SUCCESS)
> +       EFI_STATUS_STR(LOAD_ERROR)
> +       EFI_STATUS_STR(INVALID_PARAMETER)
> +       EFI_STATUS_STR(UNSUPPORTED)
> +       EFI_STATUS_STR(BAD_BUFFER_SIZE)
> +       EFI_STATUS_STR(BUFFER_TOO_SMALL)
> +       EFI_STATUS_STR(NOT_READY)
> +       EFI_STATUS_STR(DEVICE_ERROR)
> +       EFI_STATUS_STR(WRITE_PROTECTED)
> +       EFI_STATUS_STR(OUT_OF_RESOURCES)
> +       EFI_STATUS_STR(NOT_FOUND)
> +       EFI_STATUS_STR(ABORTED)
> +       EFI_STATUS_STR(SECURITY_VIOLATION)
> +       }
> +
> +       return "";
> +}
> +
>  #endif /* _LINUX_EFI_H */
> --
> 2.13.6
>

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

* Re: [PATCH 1/6] x86/KASLR: make getting random long number function public
  2018-08-05  8:16     ` Ard Biesheuvel
@ 2018-08-05 14:40       ` joeyli
  -1 siblings, 0 replies; 30+ messages in thread
From: joeyli @ 2018-08-05 14:40 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Lee, Chun-Yi, Linux Kernel Mailing List, linux-efi,
	the arch/x86 maintainers, keyrings, linux-integrity, Kees Cook,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Rafael J. Wysocki,
	Pavel Machek, Chen Yu, Oliver Neukum, Ryan Chen, David Howells,
	Mimi Zohar

Hi Ard,

On Sun, Aug 05, 2018 at 10:16:03AM +0200, Ard Biesheuvel wrote:
> On 5 August 2018 at 05:21, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
> > Separating the functions for getting random long number from KASLR
> > to x86 library, then it can be used to generate random long for
> > EFI root key.
> >
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: "H. Peter Anvin" <hpa@zytor.com>
> > Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> > Cc: Pavel Machek <pavel@ucw.cz>
> > Cc: Chen Yu <yu.c.chen@intel.com>
> > Cc: Oliver Neukum <oneukum@suse.com>
> > Cc: Ryan Chen <yu.chen.surf@gmail.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: David Howells <dhowells@redhat.com>
> > Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
> > Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
> 
> Not my call to make perhaps, but i'm nacking it anyway.
> 
> Playing games with counters and other low entropy inputs may have been
> acceptable for kaslr on x86 when it was first introduced, but using it
> to generate symmetric keys is really out of the question.
> 
> On modern x86, i suppose rdrand is a given, and these days we have
> EFI_RNG_PROTOCOL as well (and an open source UEFI driver for ChasoKey,
> btw - perhaps we should try and get MS to sign that?), although I'm
> not sure whether any x86 support this out of the box now.
> 
> BTW using low entropy inputs on top of rdrand or EFI_RNG_PROTOCOL is
> fine, if you're paranoid, but if you have neither of those, you should
> really fail the call.
> 

I agreed with your suggestion. I will add EFI_RNG_PROTOCOL and also checking
the existence of RDRAND and EFI_RNG_PROTOCOL.

Thanks for your view.

Joey Lee
 
> > ---
> >  arch/x86/boot/compressed/kaslr.c | 21 -------------
> >  arch/x86/boot/compressed/misc.c  | 17 ++++++++++
> >  arch/x86/boot/compressed/misc.h  |  6 ++++
> >  arch/x86/lib/kaslr.c             | 61 ++---------------------------------
> >  arch/x86/lib/random.c            | 68 ++++++++++++++++++++++++++++++++++++++++
> >  5 files changed, 93 insertions(+), 80 deletions(-)
> >  create mode 100644 arch/x86/lib/random.c
> >
> > diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> > index b87a7582853d..0f40d2178ebc 100644
> > --- a/arch/x86/boot/compressed/kaslr.c
> > +++ b/arch/x86/boot/compressed/kaslr.c
> > @@ -33,13 +33,11 @@
> >  #include "error.h"
> >  #include "../string.h"
> >
> > -#include <generated/compile.h>
> >  #include <linux/module.h>
> >  #include <linux/uts.h>
> >  #include <linux/utsname.h>
> >  #include <linux/ctype.h>
> >  #include <linux/efi.h>
> > -#include <generated/utsrelease.h>
> >  #include <asm/efi.h>
> >
> >  /* Macros used by the included decompressor code below. */
> > @@ -57,25 +55,6 @@ extern unsigned long get_cmd_line_ptr(void);
> >  /* Used by PAGE_KERN* macros: */
> >  pteval_t __default_kernel_pte_mask __read_mostly = ~0;
> >
> > -/* Simplified build-specific string for starting entropy. */
> > -static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
> > -               LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
> > -
> > -static unsigned long rotate_xor(unsigned long hash, const void *area,
> > -                               size_t size)
> > -{
> > -       size_t i;
> > -       unsigned long *ptr = (unsigned long *)area;
> > -
> > -       for (i = 0; i < size / sizeof(hash); i++) {
> > -               /* Rotate by odd number of bits and XOR. */
> > -               hash = (hash << ((sizeof(hash) * 8) - 7)) | (hash >> 7);
> > -               hash ^= ptr[i];
> > -       }
> > -
> > -       return hash;
> > -}
> > -
> >  /* Attempt to create a simple but unpredictable starting entropy. */
> >  static unsigned long get_boot_seed(void)
> >  {
> > diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
> > index 8dd1d5ccae58..eb0ab9cad4e4 100644
> > --- a/arch/x86/boot/compressed/misc.c
> > +++ b/arch/x86/boot/compressed/misc.c
> > @@ -426,3 +426,20 @@ void fortify_panic(const char *name)
> >  {
> >         error("detected buffer overflow");
> >  }
> > +
> > +#if CONFIG_RANDOMIZE_BASE
> > +unsigned long rotate_xor(unsigned long hash, const void *area,
> > +                       size_t size)
> > +{
> > +       size_t i;
> > +       unsigned long *ptr = (unsigned long *)area;
> > +
> > +       for (i = 0; i < size / sizeof(hash); i++) {
> > +               /* Rotate by odd number of bits and XOR. */
> > +               hash = (hash << ((sizeof(hash) * 8) - 7)) | (hash >> 7);
> > +               hash ^= ptr[i];
> > +       }
> > +
> > +       return hash;
> > +}
> > +#endif
> > diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
> > index a423bdb42686..957f327ad83c 100644
> > --- a/arch/x86/boot/compressed/misc.h
> > +++ b/arch/x86/boot/compressed/misc.h
> > @@ -70,6 +70,8 @@ int cmdline_find_option_bool(const char *option);
> >
> >
> >  #if CONFIG_RANDOMIZE_BASE
> > +#include <generated/compile.h>
> > +#include <generated/utsrelease.h>
> >  /* kaslr.c */
> >  void choose_random_location(unsigned long input,
> >                             unsigned long input_size,
> > @@ -78,6 +80,10 @@ void choose_random_location(unsigned long input,
> >                             unsigned long *virt_addr);
> >  /* cpuflags.c */
> >  bool has_cpuflag(int flag);
> > +/* Simplified build-specific string for starting entropy. */
> > +static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
> > +               LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
> > +unsigned long rotate_xor(unsigned long hash, const void *area, size_t size);
> >  #else
> >  static inline void choose_random_location(unsigned long input,
> >                                           unsigned long input_size,
> > diff --git a/arch/x86/lib/kaslr.c b/arch/x86/lib/kaslr.c
> > index 79778ab200e4..29ed9bfde5a5 100644
> > --- a/arch/x86/lib/kaslr.c
> > +++ b/arch/x86/lib/kaslr.c
> > @@ -26,67 +26,10 @@
> >  #define get_boot_seed() kaslr_offset()
> >  #endif
> >
> > -#define I8254_PORT_CONTROL     0x43
> > -#define I8254_PORT_COUNTER0    0x40
> > -#define I8254_CMD_READBACK     0xC0
> > -#define I8254_SELECT_COUNTER0  0x02
> > -#define I8254_STATUS_NOTREADY  0x40
> > -static inline u16 i8254(void)
> > -{
> > -       u16 status, timer;
> > -
> > -       do {
> > -               outb(I8254_PORT_CONTROL,
> > -                    I8254_CMD_READBACK | I8254_SELECT_COUNTER0);
> > -               status = inb(I8254_PORT_COUNTER0);
> > -               timer  = inb(I8254_PORT_COUNTER0);
> > -               timer |= inb(I8254_PORT_COUNTER0) << 8;
> > -       } while (status & I8254_STATUS_NOTREADY);
> > -
> > -       return timer;
> > -}
> > +#include "random.c"
> >
> >  unsigned long kaslr_get_random_long(const char *purpose)
> >  {
> > -#ifdef CONFIG_X86_64
> > -       const unsigned long mix_const = 0x5d6008cbf3848dd3UL;
> > -#else
> > -       const unsigned long mix_const = 0x3f39e593UL;
> > -#endif
> > -       unsigned long raw, random = get_boot_seed();
> > -       bool use_i8254 = true;
> > -
> > -       debug_putstr(purpose);
> >         debug_putstr(" KASLR using");
> > -
> > -       if (has_cpuflag(X86_FEATURE_RDRAND)) {
> > -               debug_putstr(" RDRAND");
> > -               if (rdrand_long(&raw)) {
> > -                       random ^= raw;
> > -                       use_i8254 = false;
> > -               }
> > -       }
> > -
> > -       if (has_cpuflag(X86_FEATURE_TSC)) {
> > -               debug_putstr(" RDTSC");
> > -               raw = rdtsc();
> > -
> > -               random ^= raw;
> > -               use_i8254 = false;
> > -       }
> > -
> > -       if (use_i8254) {
> > -               debug_putstr(" i8254");
> > -               random ^= i8254();
> > -       }
> > -
> > -       /* Circular multiply for better bit diffusion */
> > -       asm(_ASM_MUL "%3"
> > -           : "=a" (random), "=d" (raw)
> > -           : "a" (random), "rm" (mix_const));
> > -       random += raw;
> > -
> > -       debug_putstr("...\n");
> > -
> > -       return random;
> > +       return get_random_long(purpose);
> >  }
> > diff --git a/arch/x86/lib/random.c b/arch/x86/lib/random.c
> > new file mode 100644
> > index 000000000000..f2fe6a784c98
> > --- /dev/null
> > +++ b/arch/x86/lib/random.c
> > @@ -0,0 +1,68 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +#include <asm/io.h>
> > +#include <asm/archrandom.h>
> > +
> > +#define I8254_PORT_CONTROL     0x43
> > +#define I8254_PORT_COUNTER0    0x40
> > +#define I8254_CMD_READBACK     0xC0
> > +#define I8254_SELECT_COUNTER0  0x02
> > +#define I8254_STATUS_NOTREADY  0x40
> > +static inline u16 i8254(void)
> > +{
> > +       u16 status, timer;
> > +
> > +       do {
> > +               outb(I8254_PORT_CONTROL,
> > +                    I8254_CMD_READBACK | I8254_SELECT_COUNTER0);
> > +               status = inb(I8254_PORT_COUNTER0);
> > +               timer  = inb(I8254_PORT_COUNTER0);
> > +               timer |= inb(I8254_PORT_COUNTER0) << 8;
> > +       } while (status & I8254_STATUS_NOTREADY);
> > +
> > +       return timer;
> > +}
> > +
> > +static unsigned long get_random_long(const char *purpose)
> > +{
> > +#ifdef CONFIG_X86_64
> > +       const unsigned long mix_const = 0x5d6008cbf3848dd3UL;
> > +#else
> > +       const unsigned long mix_const = 0x3f39e593UL;
> > +#endif
> > +       unsigned long raw, random = get_boot_seed();
> > +       bool use_i8254 = true;
> > +
> > +       debug_putstr(purpose);
> > +
> > +       if (has_cpuflag(X86_FEATURE_RDRAND)) {
> > +               debug_putstr(" RDRAND");
> > +               if (rdrand_long(&raw)) {
> > +                       random ^= raw;
> > +                       use_i8254 = false;
> > +               }
> > +       }
> > +
> > +       if (has_cpuflag(X86_FEATURE_TSC)) {
> > +               debug_putstr(" RDTSC");
> > +               raw = rdtsc();
> > +
> > +               random ^= raw;
> > +               use_i8254 = false;
> > +       }
> > +
> > +       if (use_i8254) {
> > +               debug_putstr(" i8254");
> > +               random ^= i8254();
> > +       }
> > +
> > +       /* Circular multiply for better bit diffusion */
> > +       asm(_ASM_MUL "%3"
> > +           : "=a" (random), "=d" (raw)
> > +           : "a" (random), "rm" (mix_const));
> > +       random += raw;
> > +
> > +       debug_putstr("...\n");
> > +
> > +       return random;
> > +}
> > --
> > 2.13.6
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-efi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 1/6] x86/KASLR: make getting random long number function public
@ 2018-08-05 14:40       ` joeyli
  0 siblings, 0 replies; 30+ messages in thread
From: joeyli @ 2018-08-05 14:40 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Lee, Chun-Yi, Linux Kernel Mailing List, linux-efi,
	the arch/x86 maintainers, keyrings, linux-integrity, Kees Cook,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Rafael J. Wysocki,
	Pavel Machek, Chen Yu, Oliver Neukum, Ryan Chen, David Howells,
	Mimi Zohar

Hi Ard,

On Sun, Aug 05, 2018 at 10:16:03AM +0200, Ard Biesheuvel wrote:
> On 5 August 2018 at 05:21, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
> > Separating the functions for getting random long number from KASLR
> > to x86 library, then it can be used to generate random long for
> > EFI root key.
> >
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: "H. Peter Anvin" <hpa@zytor.com>
> > Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> > Cc: Pavel Machek <pavel@ucw.cz>
> > Cc: Chen Yu <yu.c.chen@intel.com>
> > Cc: Oliver Neukum <oneukum@suse.com>
> > Cc: Ryan Chen <yu.chen.surf@gmail.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: David Howells <dhowells@redhat.com>
> > Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
> > Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
> 
> Not my call to make perhaps, but i'm nacking it anyway.
> 
> Playing games with counters and other low entropy inputs may have been
> acceptable for kaslr on x86 when it was first introduced, but using it
> to generate symmetric keys is really out of the question.
> 
> On modern x86, i suppose rdrand is a given, and these days we have
> EFI_RNG_PROTOCOL as well (and an open source UEFI driver for ChasoKey,
> btw - perhaps we should try and get MS to sign that?), although I'm
> not sure whether any x86 support this out of the box now.
> 
> BTW using low entropy inputs on top of rdrand or EFI_RNG_PROTOCOL is
> fine, if you're paranoid, but if you have neither of those, you should
> really fail the call.
> 

I agreed with your suggestion. I will add EFI_RNG_PROTOCOL and also checking
the existence of RDRAND and EFI_RNG_PROTOCOL.

Thanks for your view.

Joey Lee
 
> > ---
> >  arch/x86/boot/compressed/kaslr.c | 21 -------------
> >  arch/x86/boot/compressed/misc.c  | 17 ++++++++++
> >  arch/x86/boot/compressed/misc.h  |  6 ++++
> >  arch/x86/lib/kaslr.c             | 61 ++---------------------------------
> >  arch/x86/lib/random.c            | 68 ++++++++++++++++++++++++++++++++++++++++
> >  5 files changed, 93 insertions(+), 80 deletions(-)
> >  create mode 100644 arch/x86/lib/random.c
> >
> > diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> > index b87a7582853d..0f40d2178ebc 100644
> > --- a/arch/x86/boot/compressed/kaslr.c
> > +++ b/arch/x86/boot/compressed/kaslr.c
> > @@ -33,13 +33,11 @@
> >  #include "error.h"
> >  #include "../string.h"
> >
> > -#include <generated/compile.h>
> >  #include <linux/module.h>
> >  #include <linux/uts.h>
> >  #include <linux/utsname.h>
> >  #include <linux/ctype.h>
> >  #include <linux/efi.h>
> > -#include <generated/utsrelease.h>
> >  #include <asm/efi.h>
> >
> >  /* Macros used by the included decompressor code below. */
> > @@ -57,25 +55,6 @@ extern unsigned long get_cmd_line_ptr(void);
> >  /* Used by PAGE_KERN* macros: */
> >  pteval_t __default_kernel_pte_mask __read_mostly = ~0;
> >
> > -/* Simplified build-specific string for starting entropy. */
> > -static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
> > -               LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
> > -
> > -static unsigned long rotate_xor(unsigned long hash, const void *area,
> > -                               size_t size)
> > -{
> > -       size_t i;
> > -       unsigned long *ptr = (unsigned long *)area;
> > -
> > -       for (i = 0; i < size / sizeof(hash); i++) {
> > -               /* Rotate by odd number of bits and XOR. */
> > -               hash = (hash << ((sizeof(hash) * 8) - 7)) | (hash >> 7);
> > -               hash ^= ptr[i];
> > -       }
> > -
> > -       return hash;
> > -}
> > -
> >  /* Attempt to create a simple but unpredictable starting entropy. */
> >  static unsigned long get_boot_seed(void)
> >  {
> > diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
> > index 8dd1d5ccae58..eb0ab9cad4e4 100644
> > --- a/arch/x86/boot/compressed/misc.c
> > +++ b/arch/x86/boot/compressed/misc.c
> > @@ -426,3 +426,20 @@ void fortify_panic(const char *name)
> >  {
> >         error("detected buffer overflow");
> >  }
> > +
> > +#if CONFIG_RANDOMIZE_BASE
> > +unsigned long rotate_xor(unsigned long hash, const void *area,
> > +                       size_t size)
> > +{
> > +       size_t i;
> > +       unsigned long *ptr = (unsigned long *)area;
> > +
> > +       for (i = 0; i < size / sizeof(hash); i++) {
> > +               /* Rotate by odd number of bits and XOR. */
> > +               hash = (hash << ((sizeof(hash) * 8) - 7)) | (hash >> 7);
> > +               hash ^= ptr[i];
> > +       }
> > +
> > +       return hash;
> > +}
> > +#endif
> > diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
> > index a423bdb42686..957f327ad83c 100644
> > --- a/arch/x86/boot/compressed/misc.h
> > +++ b/arch/x86/boot/compressed/misc.h
> > @@ -70,6 +70,8 @@ int cmdline_find_option_bool(const char *option);
> >
> >
> >  #if CONFIG_RANDOMIZE_BASE
> > +#include <generated/compile.h>
> > +#include <generated/utsrelease.h>
> >  /* kaslr.c */
> >  void choose_random_location(unsigned long input,
> >                             unsigned long input_size,
> > @@ -78,6 +80,10 @@ void choose_random_location(unsigned long input,
> >                             unsigned long *virt_addr);
> >  /* cpuflags.c */
> >  bool has_cpuflag(int flag);
> > +/* Simplified build-specific string for starting entropy. */
> > +static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
> > +               LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
> > +unsigned long rotate_xor(unsigned long hash, const void *area, size_t size);
> >  #else
> >  static inline void choose_random_location(unsigned long input,
> >                                           unsigned long input_size,
> > diff --git a/arch/x86/lib/kaslr.c b/arch/x86/lib/kaslr.c
> > index 79778ab200e4..29ed9bfde5a5 100644
> > --- a/arch/x86/lib/kaslr.c
> > +++ b/arch/x86/lib/kaslr.c
> > @@ -26,67 +26,10 @@
> >  #define get_boot_seed() kaslr_offset()
> >  #endif
> >
> > -#define I8254_PORT_CONTROL     0x43
> > -#define I8254_PORT_COUNTER0    0x40
> > -#define I8254_CMD_READBACK     0xC0
> > -#define I8254_SELECT_COUNTER0  0x02
> > -#define I8254_STATUS_NOTREADY  0x40
> > -static inline u16 i8254(void)
> > -{
> > -       u16 status, timer;
> > -
> > -       do {
> > -               outb(I8254_PORT_CONTROL,
> > -                    I8254_CMD_READBACK | I8254_SELECT_COUNTER0);
> > -               status = inb(I8254_PORT_COUNTER0);
> > -               timer  = inb(I8254_PORT_COUNTER0);
> > -               timer |= inb(I8254_PORT_COUNTER0) << 8;
> > -       } while (status & I8254_STATUS_NOTREADY);
> > -
> > -       return timer;
> > -}
> > +#include "random.c"
> >
> >  unsigned long kaslr_get_random_long(const char *purpose)
> >  {
> > -#ifdef CONFIG_X86_64
> > -       const unsigned long mix_const = 0x5d6008cbf3848dd3UL;
> > -#else
> > -       const unsigned long mix_const = 0x3f39e593UL;
> > -#endif
> > -       unsigned long raw, random = get_boot_seed();
> > -       bool use_i8254 = true;
> > -
> > -       debug_putstr(purpose);
> >         debug_putstr(" KASLR using");
> > -
> > -       if (has_cpuflag(X86_FEATURE_RDRAND)) {
> > -               debug_putstr(" RDRAND");
> > -               if (rdrand_long(&raw)) {
> > -                       random ^= raw;
> > -                       use_i8254 = false;
> > -               }
> > -       }
> > -
> > -       if (has_cpuflag(X86_FEATURE_TSC)) {
> > -               debug_putstr(" RDTSC");
> > -               raw = rdtsc();
> > -
> > -               random ^= raw;
> > -               use_i8254 = false;
> > -       }
> > -
> > -       if (use_i8254) {
> > -               debug_putstr(" i8254");
> > -               random ^= i8254();
> > -       }
> > -
> > -       /* Circular multiply for better bit diffusion */
> > -       asm(_ASM_MUL "%3"
> > -           : "=a" (random), "=d" (raw)
> > -           : "a" (random), "rm" (mix_const));
> > -       random += raw;
> > -
> > -       debug_putstr("...\n");
> > -
> > -       return random;
> > +       return get_random_long(purpose);
> >  }
> > diff --git a/arch/x86/lib/random.c b/arch/x86/lib/random.c
> > new file mode 100644
> > index 000000000000..f2fe6a784c98
> > --- /dev/null
> > +++ b/arch/x86/lib/random.c
> > @@ -0,0 +1,68 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +#include <asm/io.h>
> > +#include <asm/archrandom.h>
> > +
> > +#define I8254_PORT_CONTROL     0x43
> > +#define I8254_PORT_COUNTER0    0x40
> > +#define I8254_CMD_READBACK     0xC0
> > +#define I8254_SELECT_COUNTER0  0x02
> > +#define I8254_STATUS_NOTREADY  0x40
> > +static inline u16 i8254(void)
> > +{
> > +       u16 status, timer;
> > +
> > +       do {
> > +               outb(I8254_PORT_CONTROL,
> > +                    I8254_CMD_READBACK | I8254_SELECT_COUNTER0);
> > +               status = inb(I8254_PORT_COUNTER0);
> > +               timer  = inb(I8254_PORT_COUNTER0);
> > +               timer |= inb(I8254_PORT_COUNTER0) << 8;
> > +       } while (status & I8254_STATUS_NOTREADY);
> > +
> > +       return timer;
> > +}
> > +
> > +static unsigned long get_random_long(const char *purpose)
> > +{
> > +#ifdef CONFIG_X86_64
> > +       const unsigned long mix_const = 0x5d6008cbf3848dd3UL;
> > +#else
> > +       const unsigned long mix_const = 0x3f39e593UL;
> > +#endif
> > +       unsigned long raw, random = get_boot_seed();
> > +       bool use_i8254 = true;
> > +
> > +       debug_putstr(purpose);
> > +
> > +       if (has_cpuflag(X86_FEATURE_RDRAND)) {
> > +               debug_putstr(" RDRAND");
> > +               if (rdrand_long(&raw)) {
> > +                       random ^= raw;
> > +                       use_i8254 = false;
> > +               }
> > +       }
> > +
> > +       if (has_cpuflag(X86_FEATURE_TSC)) {
> > +               debug_putstr(" RDTSC");
> > +               raw = rdtsc();
> > +
> > +               random ^= raw;
> > +               use_i8254 = false;
> > +       }
> > +
> > +       if (use_i8254) {
> > +               debug_putstr(" i8254");
> > +               random ^= i8254();
> > +       }
> > +
> > +       /* Circular multiply for better bit diffusion */
> > +       asm(_ASM_MUL "%3"
> > +           : "=a" (random), "=d" (raw)
> > +           : "a" (random), "rm" (mix_const));
> > +       random += raw;
> > +
> > +       debug_putstr("...\n");
> > +
> > +       return random;
> > +}
> > --
> > 2.13.6
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-efi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 0/6][RFC] Add EFI secure key to key retention service
  2018-08-05  7:25   ` Ard Biesheuvel
@ 2018-08-05 16:31     ` joeyli
  -1 siblings, 0 replies; 30+ messages in thread
From: joeyli @ 2018-08-05 16:31 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Lee, Chun-Yi, Linux Kernel Mailing List, linux-efi,
	the arch/x86 maintainers, keyrings, linux-integrity, Kees Cook,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Rafael J. Wysocki,
	Pavel Machek, Chen Yu, Oliver Neukum, Ryan Chen, David Howells,
	Mimi Zohar

On Sun, Aug 05, 2018 at 09:25:56AM +0200, Ard Biesheuvel wrote:
> Hello Chun,yi,
> 
> On 5 August 2018 at 05:21, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
> > When secure boot is enabled, only signed EFI binary can access
> > EFI boot service variable before ExitBootService. Which means that
> > the EFI boot service variable is secure.
> >
> 
> No it, isn't, and this is a very dangerous assumption to make.
> 
> 'Secure' means different things to different people. 'Secure boot' is
> a misnomer, since it is too vague: it should be called 'authenticated
> boot', and the catch is that authentication using public-key crypto
> does not involve secrets at all. The UEFI variable store was not
> designed with confidentiality in mind, and assuming [given the
> reputation of EFI on the implementation side] that you can use it to
> keep secrets is rather unwise imho.
>

I agreed with you. Especially I can't refute the part of EFI
implementation, manufacturers can not be fully trusted. 

I am thinking a case... Some machines provide setup mode. If user
earses all manufacturer's reloaded keys and only enrolls their own
key. Which means that user fully controls the authentication
environment. Then the EFI boot service varible can be trusted by
the user. But this case is too strict for normal user.

Thanks for your review and comments. I will think more about your
suggestions.

Joey Lee  

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

* Re: [PATCH 0/6][RFC] Add EFI secure key to key retention service
@ 2018-08-05 16:31     ` joeyli
  0 siblings, 0 replies; 30+ messages in thread
From: joeyli @ 2018-08-05 16:31 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Lee, Chun-Yi, Linux Kernel Mailing List, linux-efi,
	the arch/x86 maintainers, keyrings, linux-integrity, Kees Cook,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Rafael J. Wysocki,
	Pavel Machek, Chen Yu, Oliver Neukum, Ryan Chen, David Howells,
	Mimi Zohar

On Sun, Aug 05, 2018 at 09:25:56AM +0200, Ard Biesheuvel wrote:
> Hello Chun,yi,
> 
> On 5 August 2018 at 05:21, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
> > When secure boot is enabled, only signed EFI binary can access
> > EFI boot service variable before ExitBootService. Which means that
> > the EFI boot service variable is secure.
> >
> 
> No it, isn't, and this is a very dangerous assumption to make.
> 
> 'Secure' means different things to different people. 'Secure boot' is
> a misnomer, since it is too vague: it should be called 'authenticated
> boot', and the catch is that authentication using public-key crypto
> does not involve secrets at all. The UEFI variable store was not
> designed with confidentiality in mind, and assuming [given the
> reputation of EFI on the implementation side] that you can use it to
> keep secrets is rather unwise imho.
>

I agreed with you. Especially I can't refute the part of EFI
implementation, manufacturers can not be fully trusted. 

I am thinking a case... Some machines provide setup mode. If user
earses all manufacturer's reloaded keys and only enrolls their own
key. Which means that user fully controls the authentication
environment. Then the EFI boot service varible can be trusted by
the user. But this case is too strict for normal user.

Thanks for your review and comments. I will think more about your
suggestions.

Joey Lee  

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

* Re: [PATCH 0/6][RFC] Add EFI secure key to key retention service
  2018-08-05  7:25   ` Ard Biesheuvel
@ 2018-08-05 17:47     ` James Bottomley
  -1 siblings, 0 replies; 30+ messages in thread
From: James Bottomley @ 2018-08-05 17:47 UTC (permalink / raw)
  To: Ard Biesheuvel, Lee, Chun-Yi
  Cc: Linux Kernel Mailing List, linux-efi, the arch/x86 maintainers,
	keyrings, linux-integrity, Lee, Chun-Yi, Kees Cook,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Rafael J. Wysocki,
	Pavel Machek, Chen Yu, Oliver Neukum, Ryan Chen, David Howells,
	Mimi Zohar

On Sun, 2018-08-05 at 09:25 +0200, Ard Biesheuvel wrote:
> Hello Chun,yi,
> 
> On 5 August 2018 at 05:21, Lee, Chun-Yi <joeyli.kernel@gmail.com>
> wrote:
> > When secure boot is enabled, only signed EFI binary can access
> > EFI boot service variable before ExitBootService. Which means that
> > the EFI boot service variable is secure.
> > 
> 
> No it, isn't, and this is a very dangerous assumption to make.
> 
> 'Secure' means different things to different people. 'Secure boot' is
> a misnomer, since it is too vague: it should be called 'authenticated
> boot', and the catch is that authentication using public-key crypto
> does not involve secrets at all.

Hang on, let's not throw the baby out with the bathwater here.

The design of "secure boot" is to create a boot time environment where
only trusted code may execute.  We rely on this trust guarantee when we
pivot from the EFI to the MoK root of trust in shim.

The reason we in Linux trust this guarantee is that it pertains to the
boot environment only, so any violation would allow Windows boot to be
compromised as well and we trust Microsoft's Business interests in
securing windows far enough to think this would be dealt with very
severely and it's an outcome the ODMs (who also add secure boot keys)
are worried enough about to be very careful.

The rub (and this is where I'm agreeing with Ard) is that any use case
we come up with where a violation wouldn't cause a problem in windows
is a use case where we cannot rely on the guarantee because Microsoft
no longer has a strong business interest in policing it.  This, for
instance, is why we don't populate the Linux trusted keyrings with the
secure boot keys (we may trust them in the boot environment where
compromise would be shared with windows but we can't trust them in the
Linux OS environment where it wouldn't).  So this means we have to be
very careful coming up with uses for secure boot that aren't strictly
rooted in the guarantee as enforced by the business interests of
Microsoft and the ODMs.

>  The UEFI variable store was not designed with confidentiality in
> mind, and assuming [given the reputation of EFI on the implementation
> side] that you can use it to keep secrets is rather unwise imho.

Agree completely here: Microsoft doesn't use UEFI variables for
confidentiality, so we shouldn't either.  If you want confidentiality,
use a TPM (like Microsoft does for the bitlocker key).

James


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

* Re: [PATCH 0/6][RFC] Add EFI secure key to key retention service
@ 2018-08-05 17:47     ` James Bottomley
  0 siblings, 0 replies; 30+ messages in thread
From: James Bottomley @ 2018-08-05 17:47 UTC (permalink / raw)
  To: Ard Biesheuvel, Lee, Chun-Yi
  Cc: Linux Kernel Mailing List, linux-efi, the arch/x86 maintainers,
	keyrings, linux-integrity, Lee, Chun-Yi, Kees Cook,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Rafael J. Wysocki,
	Pavel Machek, Chen Yu, Oliver Neukum, Ryan Chen, David Howells,
	Mimi Zohar

On Sun, 2018-08-05 at 09:25 +0200, Ard Biesheuvel wrote:
> Hello Chun,yi,
> 
> On 5 August 2018 at 05:21, Lee, Chun-Yi <joeyli.kernel@gmail.com>
> wrote:
> > When secure boot is enabled, only signed EFI binary can access
> > EFI boot service variable before ExitBootService. Which means that
> > the EFI boot service variable is secure.
> > 
> 
> No it, isn't, and this is a very dangerous assumption to make.
> 
> 'Secure' means different things to different people. 'Secure boot' is
> a misnomer, since it is too vague: it should be called 'authenticated
> boot', and the catch is that authentication using public-key crypto
> does not involve secrets at all.

Hang on, let's not throw the baby out with the bathwater here.

The design of "secure boot" is to create a boot time environment where
only trusted code may execute.  We rely on this trust guarantee when we
pivot from the EFI to the MoK root of trust in shim.

The reason we in Linux trust this guarantee is that it pertains to the
boot environment only, so any violation would allow Windows boot to be
compromised as well and we trust Microsoft's Business interests in
securing windows far enough to think this would be dealt with very
severely and it's an outcome the ODMs (who also add secure boot keys)
are worried enough about to be very careful.

The rub (and this is where I'm agreeing with Ard) is that any use case
we come up with where a violation wouldn't cause a problem in windows
is a use case where we cannot rely on the guarantee because Microsoft
no longer has a strong business interest in policing it.  This, for
instance, is why we don't populate the Linux trusted keyrings with the
secure boot keys (we may trust them in the boot environment where
compromise would be shared with windows but we can't trust them in the
Linux OS environment where it wouldn't).  So this means we have to be
very careful coming up with uses for secure boot that aren't strictly
rooted in the guarantee as enforced by the business interests of
Microsoft and the ODMs.

>  The UEFI variable store was not designed with confidentiality in
> mind, and assuming [given the reputation of EFI on the implementation
> side] that you can use it to keep secrets is rather unwise imho.

Agree completely here: Microsoft doesn't use UEFI variables for
confidentiality, so we shouldn't either.  If you want confidentiality,
use a TPM (like Microsoft does for the bitlocker key).

James


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

* Re: [PATCH 0/6][RFC] Add EFI secure key to key retention service
  2018-08-05 16:31     ` joeyli
@ 2018-08-05 19:00       ` Ard Biesheuvel
  -1 siblings, 0 replies; 30+ messages in thread
From: Ard Biesheuvel @ 2018-08-05 19:00 UTC (permalink / raw)
  To: joeyli
  Cc: Lee, Chun-Yi, Linux Kernel Mailing List, linux-efi,
	the arch/x86 maintainers, keyrings, linux-integrity, Kees Cook,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Rafael J. Wysocki,
	Pavel Machek, Chen Yu, Oliver Neukum, Ryan Chen, David Howells,
	Mimi Zohar

On 5 August 2018 at 18:31, joeyli <jlee@suse.com> wrote:
> On Sun, Aug 05, 2018 at 09:25:56AM +0200, Ard Biesheuvel wrote:
>> Hello Chun,yi,
>>
>> On 5 August 2018 at 05:21, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
>> > When secure boot is enabled, only signed EFI binary can access
>> > EFI boot service variable before ExitBootService. Which means that
>> > the EFI boot service variable is secure.
>> >
>>
>> No it, isn't, and this is a very dangerous assumption to make.
>>
>> 'Secure' means different things to different people. 'Secure boot' is
>> a misnomer, since it is too vague: it should be called 'authenticated
>> boot', and the catch is that authentication using public-key crypto
>> does not involve secrets at all. The UEFI variable store was not
>> designed with confidentiality in mind, and assuming [given the
>> reputation of EFI on the implementation side] that you can use it to
>> keep secrets is rather unwise imho.
>>
>
> I agreed with you. Especially I can't refute the part of EFI
> implementation, manufacturers can not be fully trusted.
>
> I am thinking a case... Some machines provide setup mode. If user
> earses all manufacturer's reloaded keys and only enrolls their own
> key. Which means that user fully controls the authentication
> environment. Then the EFI boot service varible can be trusted by
> the user.

This has nothing to do with trust but everything to do with confidentiality.

*Nothing* in the UEFI variable store stack has been designed or
implemented with confidentiality in mind. The contents of EFI
variables are readable in the clear from the SPI flash. Code that
handles variable store reads may leave unsanitized buffers behind. We
have efivarfs that needs to be modified to hide 'secret' variables,
but also, to kzfree() every allocation that is used in handling
varstore access etc etc.



> But this case is too strict for normal user.
>
> Thanks for your review and comments. I will think more about your
> suggestions.
>
> Joey Lee

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

* Re: [PATCH 0/6][RFC] Add EFI secure key to key retention service
@ 2018-08-05 19:00       ` Ard Biesheuvel
  0 siblings, 0 replies; 30+ messages in thread
From: Ard Biesheuvel @ 2018-08-05 19:00 UTC (permalink / raw)
  To: joeyli
  Cc: Lee, Chun-Yi, Linux Kernel Mailing List, linux-efi,
	the arch/x86 maintainers, keyrings, linux-integrity, Kees Cook,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Rafael J. Wysocki,
	Pavel Machek, Chen Yu, Oliver Neukum, Ryan Chen, David Howells,
	Mimi Zohar

On 5 August 2018 at 18:31, joeyli <jlee@suse.com> wrote:
> On Sun, Aug 05, 2018 at 09:25:56AM +0200, Ard Biesheuvel wrote:
>> Hello Chun,yi,
>>
>> On 5 August 2018 at 05:21, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
>> > When secure boot is enabled, only signed EFI binary can access
>> > EFI boot service variable before ExitBootService. Which means that
>> > the EFI boot service variable is secure.
>> >
>>
>> No it, isn't, and this is a very dangerous assumption to make.
>>
>> 'Secure' means different things to different people. 'Secure boot' is
>> a misnomer, since it is too vague: it should be called 'authenticated
>> boot', and the catch is that authentication using public-key crypto
>> does not involve secrets at all. The UEFI variable store was not
>> designed with confidentiality in mind, and assuming [given the
>> reputation of EFI on the implementation side] that you can use it to
>> keep secrets is rather unwise imho.
>>
>
> I agreed with you. Especially I can't refute the part of EFI
> implementation, manufacturers can not be fully trusted.
>
> I am thinking a case... Some machines provide setup mode. If user
> earses all manufacturer's reloaded keys and only enrolls their own
> key. Which means that user fully controls the authentication
> environment. Then the EFI boot service varible can be trusted by
> the user.

This has nothing to do with trust but everything to do with confidentiality.

*Nothing* in the UEFI variable store stack has been designed or
implemented with confidentiality in mind. The contents of EFI
variables are readable in the clear from the SPI flash. Code that
handles variable store reads may leave unsanitized buffers behind. We
have efivarfs that needs to be modified to hide 'secret' variables,
but also, to kzfree() every allocation that is used in handling
varstore access etc etc.



> But this case is too strict for normal user.
>
> Thanks for your review and comments. I will think more about your
> suggestions.
>
> Joey Lee

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

* Re: [PATCH 0/6][RFC] Add EFI secure key to key retention service
  2018-08-05 17:47     ` James Bottomley
@ 2018-08-06  6:00       ` joeyli
  -1 siblings, 0 replies; 30+ messages in thread
From: joeyli @ 2018-08-06  6:00 UTC (permalink / raw)
  To: James Bottomley
  Cc: Ard Biesheuvel, Lee, Chun-Yi, Linux Kernel Mailing List,
	linux-efi, the arch/x86 maintainers, keyrings, linux-integrity,
	Kees Cook, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Rafael J. Wysocki, Pavel Machek, Chen Yu, Oliver Neukum,
	Ryan Chen, David Howells, Mimi Zohar

Hi James,

On Sun, Aug 05, 2018 at 10:47:26AM -0700, James Bottomley wrote:
> On Sun, 2018-08-05 at 09:25 +0200, Ard Biesheuvel wrote:
> > Hello Chun,yi,
> > 
> > On 5 August 2018 at 05:21, Lee, Chun-Yi <joeyli.kernel@gmail.com>
> > wrote:
> > > When secure boot is enabled, only signed EFI binary can access
> > > EFI boot service variable before ExitBootService. Which means that
> > > the EFI boot service variable is secure.
> > > 
> > 
> > No it, isn't, and this is a very dangerous assumption to make.
> > 
> > 'Secure' means different things to different people. 'Secure boot' is
> > a misnomer, since it is too vague: it should be called 'authenticated
> > boot', and the catch is that authentication using public-key crypto
> > does not involve secrets at all.
> 
> Hang on, let's not throw the baby out with the bathwater here.
> 
> The design of "secure boot" is to create a boot time environment where
> only trusted code may execute.  We rely on this trust guarantee when we
> pivot from the EFI to the MoK root of trust in shim.
> 
> The reason we in Linux trust this guarantee is that it pertains to the
> boot environment only, so any violation would allow Windows boot to be
> compromised as well and we trust Microsoft's Business interests in
> securing windows far enough to think this would be dealt with very
> severely and it's an outcome the ODMs (who also add secure boot keys)
> are worried enough about to be very careful.
> 
> The rub (and this is where I'm agreeing with Ard) is that any use case
> we come up with where a violation wouldn't cause a problem in windows
> is a use case where we cannot rely on the guarantee because Microsoft
> no longer has a strong business interest in policing it.  This, for
> instance, is why we don't populate the Linux trusted keyrings with the
> secure boot keys (we may trust them in the boot environment where
> compromise would be shared with windows but we can't trust them in the
> Linux OS environment where it wouldn't).  So this means we have to be
> very careful coming up with uses for secure boot that aren't strictly
> rooted in the guarantee as enforced by the business interests of
> Microsoft and the ODMs.
>

Thank you for providing the view point from Microsoft bussiness ineterests.
I agreed with you. Honestly I didn't think this point before.
 
> >  The UEFI variable store was not designed with confidentiality in
> > mind, and assuming [given the reputation of EFI on the implementation
> > side] that you can use it to keep secrets is rather unwise imho.
> 
> Agree completely here: Microsoft doesn't use UEFI variables for
> confidentiality, so we shouldn't either.  If you want confidentiality,
> use a TPM (like Microsoft does for the bitlocker key).
>

OK~~ Then I will use TPM trusted key + encrypted key in hibernation
encryption/authentication.

Thanks for James and Ard's comments.

Joey Lee 

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

* Re: [PATCH 0/6][RFC] Add EFI secure key to key retention service
@ 2018-08-06  6:00       ` joeyli
  0 siblings, 0 replies; 30+ messages in thread
From: joeyli @ 2018-08-06  6:00 UTC (permalink / raw)
  To: James Bottomley
  Cc: Ard Biesheuvel, Lee, Chun-Yi, Linux Kernel Mailing List,
	linux-efi, the arch/x86 maintainers, keyrings, linux-integrity,
	Kees Cook, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Rafael J. Wysocki, Pavel Machek, Chen Yu, Oliver Neukum,
	Ryan Chen, David Howells, Mimi Zohar

Hi James,

On Sun, Aug 05, 2018 at 10:47:26AM -0700, James Bottomley wrote:
> On Sun, 2018-08-05 at 09:25 +0200, Ard Biesheuvel wrote:
> > Hello Chun,yi,
> > 
> > On 5 August 2018 at 05:21, Lee, Chun-Yi <joeyli.kernel@gmail.com>
> > wrote:
> > > When secure boot is enabled, only signed EFI binary can access
> > > EFI boot service variable before ExitBootService. Which means that
> > > the EFI boot service variable is secure.
> > > 
> > 
> > No it, isn't, and this is a very dangerous assumption to make.
> > 
> > 'Secure' means different things to different people. 'Secure boot' is
> > a misnomer, since it is too vague: it should be called 'authenticated
> > boot', and the catch is that authentication using public-key crypto
> > does not involve secrets at all.
> 
> Hang on, let's not throw the baby out with the bathwater here.
> 
> The design of "secure boot" is to create a boot time environment where
> only trusted code may execute.  We rely on this trust guarantee when we
> pivot from the EFI to the MoK root of trust in shim.
> 
> The reason we in Linux trust this guarantee is that it pertains to the
> boot environment only, so any violation would allow Windows boot to be
> compromised as well and we trust Microsoft's Business interests in
> securing windows far enough to think this would be dealt with very
> severely and it's an outcome the ODMs (who also add secure boot keys)
> are worried enough about to be very careful.
> 
> The rub (and this is where I'm agreeing with Ard) is that any use case
> we come up with where a violation wouldn't cause a problem in windows
> is a use case where we cannot rely on the guarantee because Microsoft
> no longer has a strong business interest in policing it.  This, for
> instance, is why we don't populate the Linux trusted keyrings with the
> secure boot keys (we may trust them in the boot environment where
> compromise would be shared with windows but we can't trust them in the
> Linux OS environment where it wouldn't).  So this means we have to be
> very careful coming up with uses for secure boot that aren't strictly
> rooted in the guarantee as enforced by the business interests of
> Microsoft and the ODMs.
>

Thank you for providing the view point from Microsoft bussiness ineterests.
I agreed with you. Honestly I didn't think this point before.
 
> >  The UEFI variable store was not designed with confidentiality in
> > mind, and assuming [given the reputation of EFI on the implementation
> > side] that you can use it to keep secrets is rather unwise imho.
> 
> Agree completely here: Microsoft doesn't use UEFI variables for
> confidentiality, so we shouldn't either.  If you want confidentiality,
> use a TPM (like Microsoft does for the bitlocker key).
>

OK~~ Then I will use TPM trusted key + encrypted key in hibernation
encryption/authentication.

Thanks for James and Ard's comments.

Joey Lee 

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

end of thread, other threads:[~2018-08-06  6:00 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-05  3:21 [PATCH 0/6][RFC] Add EFI secure key to key retention service Lee, Chun-Yi
2018-08-05  3:21 ` Lee, Chun-Yi
2018-08-05  3:21 ` [PATCH 1/6] x86/KASLR: make getting random long number function public Lee, Chun-Yi
2018-08-05  3:21   ` Lee, Chun-Yi
2018-08-05  8:16   ` Ard Biesheuvel
2018-08-05  8:16     ` Ard Biesheuvel
2018-08-05 14:40     ` joeyli
2018-08-05 14:40       ` joeyli
2018-08-05  3:21 ` [PATCH 2/6] efi: the function transfers status to string Lee, Chun-Yi
2018-08-05  3:21   ` Lee, Chun-Yi
2018-08-05  8:17   ` Ard Biesheuvel
2018-08-05  8:17     ` Ard Biesheuvel
2018-08-05  3:21 ` [PATCH 3/6] efi: generate efi root key in EFI boot stub Lee, Chun-Yi
2018-08-05  3:21   ` Lee, Chun-Yi
2018-08-05  3:21 ` [PATCH 4/6] key: add EFI secure key type Lee, Chun-Yi
2018-08-05  3:21   ` Lee, Chun-Yi
2018-08-05  3:21 ` [PATCH 5/6] key: add EFI secure key as a master " Lee, Chun-Yi
2018-08-05  3:21   ` Lee, Chun-Yi
2018-08-05  3:21 ` [PATCH 6/6] key: enforce the secure boot checking when loading efi root key Lee, Chun-Yi
2018-08-05  3:21   ` Lee, Chun-Yi
2018-08-05  7:25 ` [PATCH 0/6][RFC] Add EFI secure key to key retention service Ard Biesheuvel
2018-08-05  7:25   ` Ard Biesheuvel
2018-08-05 16:31   ` joeyli
2018-08-05 16:31     ` joeyli
2018-08-05 19:00     ` Ard Biesheuvel
2018-08-05 19:00       ` Ard Biesheuvel
2018-08-05 17:47   ` James Bottomley
2018-08-05 17:47     ` James Bottomley
2018-08-06  6:00     ` joeyli
2018-08-06  6:00       ` joeyli

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.