mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 00/13] add efi secure boot support
@ 2017-03-25  8:31 Jean-Christophe PLAGNIOL-VILLARD
  2017-03-26  2:44 ` [PATCH 01/13] bootm: move open to image_handler Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 1 reply; 22+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-25  8:31 UTC (permalink / raw)
  To: barebox

Hi,

	This patch series rework the secure boot support to make it generic
	and we could use it on EFI too

The following changes since commit 2846e53d2d41742348459676edab737edf90604a:

  arm: baltos: define baltos_sram_init() return type as void (2017-03-13 09:13:17 +0100)

are available in the git repository at:

  git://git.jcrosoft.org/barebox.git delivery/efi-sb

for you to fetch changes up to ff17500171e65d8c20c7c0acc803aa5a4d22d014:

  efi: enable sercure boot support (2017-03-15 03:09:09 +0800)

----------------------------------------------------------------
Jean-Christophe PLAGNIOL-VILLARD (13):
      bootm: move open to image_handler
      boot_verify: use a new error ESECVIOLATION
      bootm: make security generic
      boot: invert the secure boot forcing support
      move boot verify to generic code
      boot_verify: make it modifiable at start time
      go: only use it if boot signature is not required
      boot_verify: allow to force unsigned image to boot
      boot_verify: add password request support
      efi: add more security related guid for the efivars
      efi: fix lds for secure boot support
      efi: fix secure and setup mode report
      efi: enable sercure boot support

 arch/arm/lib/bootm.c                         |   3 +++
 arch/blackfin/lib/blackfin_linux.c           |   1 +
 arch/nios2/lib/bootm.c                       |   1 +
 arch/ppc/lib/ppclinux.c                      |   1 +
 arch/x86/Kconfig                             |   1 +
 arch/x86/mach-efi/elf_ia32_efi.lds.S         |  10 +++++++---
 arch/x86/mach-efi/elf_x86_64_efi.lds.S       |  10 ++++++----
 arch/x86/mach-efi/include/mach/barebox.lds.h |  14 +++++++++++++-
 commands/bootm.c                             |   6 +++---
 commands/go.c                                |   9 +++++++++
 common/Kconfig                               |  24 ++++++++++++++++++++----
 common/Makefile                              |   1 +
 common/boot_verify.c                         | 113 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 common/bootm.c                               | 123 ++++++++++++++++++++++++++++++++++++++-------------------------------------------------------------------------------------
 common/efi-guid.c                            |   6 ++++++
 common/efi/efi-image.c                       |   1 +
 common/efi/efi.c                             |   2 +-
 common/image-fit.c                           |  42 +++++++++++++++++++++++++++++-------------
 common/misc.c                                |   1 +
 common/password.c                            |  18 ++++++++++++++++++
 common/uimage.c                              |  33 +++++++++++++++++++++++++++++++++
 drivers/efi/efi-device.c                     |  17 +++++++++++++----
 include/asm-generic/barebox.lds.h            |   8 +++++---
 include/asm-generic/errno.h                  |   1 +
 include/boot_verify.h                        |  36 ++++++++++++++++++++++++++++++++++++
 include/bootm.h                              |  16 +++++-----------
 include/efi.h                                |  18 ++++++++++++++++++
 include/image-fit.h                          |   5 +++--
 include/image.h                              |   2 ++
 include/password.h                           |   6 ++++++
 30 files changed, 395 insertions(+), 134 deletions(-)

Best Regards,
J.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 01/13] bootm: move open to image_handler
  2017-03-25  8:31 [PATCH 00/13] add efi secure boot support Jean-Christophe PLAGNIOL-VILLARD
@ 2017-03-26  2:44 ` Jean-Christophe PLAGNIOL-VILLARD
  2017-03-26  2:44   ` [PATCH 02/13] boot_verify: use a new error ESECVIOLATION Jean-Christophe PLAGNIOL-VILLARD
                     ` (12 more replies)
  0 siblings, 13 replies; 22+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-26  2:44 UTC (permalink / raw)
  To: barebox

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 arch/arm/lib/bootm.c               |  2 +
 arch/blackfin/lib/blackfin_linux.c |  1 +
 arch/nios2/lib/bootm.c             |  1 +
 arch/ppc/lib/ppclinux.c            |  1 +
 common/bootm.c                     | 79 ++++++++++++--------------------------
 common/image-fit.c                 | 14 +++++++
 common/misc.c                      |  1 +
 common/uimage.c                    | 32 +++++++++++++++
 include/bootm.h                    |  1 +
 include/image-fit.h                |  1 +
 include/image.h                    |  2 +
 11 files changed, 80 insertions(+), 55 deletions(-)

diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index 8068a53be..204344f87 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -217,6 +217,7 @@ static int do_bootm_linux(struct image_data *data)
 
 static struct image_handler uimage_handler = {
 	.name = "ARM Linux uImage",
+	.open = uimage_bootm_open,
 	.bootm = do_bootm_linux,
 	.filetype = filetype_uimage,
 	.ih_os = IH_OS_LINUX,
@@ -579,6 +580,7 @@ BAREBOX_MAGICVAR(aimage_noverwrite_tags, "Disable overwrite of the tags addr wit
 
 static struct image_handler arm_fit_handler = {
         .name = "FIT image",
+	.open = fit_bootm_open,
         .bootm = do_bootm_linux,
         .filetype = filetype_oftree,
 };
diff --git a/arch/blackfin/lib/blackfin_linux.c b/arch/blackfin/lib/blackfin_linux.c
index 5ebd284d1..27002eadb 100644
--- a/arch/blackfin/lib/blackfin_linux.c
+++ b/arch/blackfin/lib/blackfin_linux.c
@@ -68,6 +68,7 @@ static int do_bootm_linux(struct image_data *idata)
 
 static struct image_handler handler = {
 	.name = "Blackfin Linux",
+	.open = uimage_bootm_open,
 	.bootm = do_bootm_linux,
 	.filetype = filetype_uimage,
 	.ih_os = IH_OS_LINUX,
diff --git a/arch/nios2/lib/bootm.c b/arch/nios2/lib/bootm.c
index 34908bde3..f1b3c2624 100644
--- a/arch/nios2/lib/bootm.c
+++ b/arch/nios2/lib/bootm.c
@@ -69,6 +69,7 @@ static int do_bootm_linux(struct image_data *idata)
 
 static struct image_handler handler = {
 	.name = "NIOS2 Linux",
+	.open = uimage_bootm_open,
 	.bootm = do_bootm_linux,
 	.filetype = filetype_uimage,
 	.ih_os = IH_OS_LINUX,
diff --git a/arch/ppc/lib/ppclinux.c b/arch/ppc/lib/ppclinux.c
index 3fca6b272..c882938fa 100644
--- a/arch/ppc/lib/ppclinux.c
+++ b/arch/ppc/lib/ppclinux.c
@@ -100,6 +100,7 @@ error:
 
 static struct image_handler handler = {
 	.name = "PowerPC Linux",
+	.open = uimage_bootm_open,
 	.bootm = do_bootm_linux,
 	.filetype = filetype_uimage,
 	.ih_os = IH_OS_LINUX,
diff --git a/common/bootm.c b/common/bootm.c
index 81625d915..64c933b3c 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -34,7 +34,7 @@ int register_image_handler(struct image_handler *handler)
 }
 
 static struct image_handler *bootm_find_handler(enum filetype filetype,
-		struct image_data *data)
+		struct image_data *data, int enforce_os)
 {
 	struct image_handler *handler;
 
@@ -42,9 +42,16 @@ static struct image_handler *bootm_find_handler(enum filetype filetype,
 		if (filetype != filetype_uimage &&
 				handler->filetype == filetype)
 			return handler;
-		if  (filetype == filetype_uimage &&
-				handler->ih_os == data->os->header.ih_os)
-			return handler;
+		if (filetype == filetype_uimage) {
+			/*
+			 * we can take the first one as open is the same
+			 * not matter the OS
+			 */
+			if (enforce_os && handler->ih_os == data->os->header.ih_os)
+				return handler;
+			else
+				return handler;
+		}
 	}
 
 	return NULL;
@@ -441,38 +448,6 @@ int bootm_get_os_size(struct image_data *data)
 	return -EINVAL;
 }
 
-static int bootm_open_os_uimage(struct image_data *data)
-{
-	int ret;
-
-	data->os = uimage_open(data->os_file);
-	if (!data->os)
-		return -EINVAL;
-
-	if (bootm_get_verify_mode() > BOOTM_VERIFY_NONE) {
-		ret = uimage_verify(data->os);
-		if (ret) {
-			printf("Checking data crc failed with %s\n",
-					strerror(-ret));
-			uimage_close(data->os);
-			return ret;
-		}
-	}
-
-	uimage_print_contents(data->os);
-
-	if (data->os->header.ih_arch != IH_ARCH) {
-		printf("Unsupported Architecture 0x%x\n",
-		       data->os->header.ih_arch);
-		return -EINVAL;
-	}
-
-	if (data->os_address == UIMAGE_SOME_ADDRESS)
-		data->os_address = data->os->header.ih_load;
-
-	return 0;
-}
-
 static void bootm_print_info(struct image_data *data)
 {
 	if (data->os_res)
@@ -548,6 +523,14 @@ int bootm_boot(struct bootm_data *bootm_data)
 		goto err_out;
 	}
 
+	handler = bootm_find_handler(os_type, data, 0);
+	if (!handler) {
+		printf("no image handler found for image type %s\n",
+			file_type_to_string(os_type));
+		ret = -ENODEV;
+		goto err_out;
+	}
+
 	if (IS_ENABLED(CONFIG_BOOTM_FORCE_SIGNED_IMAGES)) {
 		data->verify = BOOTM_VERIFY_SIGNATURE;
 
@@ -565,25 +548,11 @@ int bootm_boot(struct bootm_data *bootm_data)
 		}
 	}
 
-	if (IS_ENABLED(CONFIG_FITIMAGE) && os_type == filetype_oftree) {
-		struct fit_handle *fit;
-
-		fit = fit_open(data->os_file, data->os_part, data->verbose, data->verify);
-		if (IS_ERR(fit)) {
-			printf("Loading FIT image %s failed with: %s\n", data->os_file,
-			       strerrorp(fit));
-			ret = PTR_ERR(fit);
-			goto err_out;
-		}
-
-		data->os_fit = fit;
-	}
-
-	if (os_type == filetype_uimage) {
-		ret = bootm_open_os_uimage(data);
+	if (handler->open) {
+		ret = handler->open(data);
 		if (ret) {
-			printf("Loading OS image failed with: %s\n",
-					strerror(-ret));
+			printf("Loading OS image %s failed with: %s\n",
+					handler->name, strerror(-ret));
 			goto err_out;
 		}
 	}
@@ -610,7 +579,7 @@ int bootm_boot(struct bootm_data *bootm_data)
 	if (data->os_address == UIMAGE_SOME_ADDRESS)
 		data->os_address = UIMAGE_INVALID_ADDRESS;
 
-	handler = bootm_find_handler(os_type, data);
+	handler = bootm_find_handler(os_type, data, 1);
 	if (!handler) {
 		printf("no image handler found for image type %s\n",
 			file_type_to_string(os_type));
diff --git a/common/image-fit.c b/common/image-fit.c
index 6a01c614c..5c014d66b 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -583,6 +583,19 @@ struct fit_handle *fit_open(const char *filename, const char *config, bool verbo
 	return ERR_PTR(ret);
 }
 
+int fit_bootm_open(struct image_data *data)
+{
+	struct fit_handle *fit;
+
+	fit = fit_open(data->os_file, data->os_part, data->verbose, data->verify);
+	if (IS_ERR(fit))
+		return PTR_ERR(fit);
+
+	data->os_fit = fit;
+
+	return 0;
+}
+
 void fit_close(struct fit_handle *handle)
 {
 	if (handle->root)
@@ -604,6 +617,7 @@ static int do_bootm_sandbox_fit(struct image_data *data)
 
 static struct image_handler sandbox_fit_handler = {
 	.name = "FIT image",
+	.open = fit_bootm_open,
 	.bootm = do_bootm_sandbox_fit,
 	.filetype = filetype_oftree,
 };
diff --git a/common/misc.c b/common/misc.c
index f0f0b808b..60acbd009 100644
--- a/common/misc.c
+++ b/common/misc.c
@@ -101,6 +101,7 @@ const char *strerror(int errnum)
 	case	EISNAM		: str = "Is a named type file"; break;
 	case	EREMOTEIO	: str = "Remote I/O error"; break;
 #endif
+	case	ESECVIOLATION	: str = "Security Violation"; break;
 	default:
 		sprintf(errno_string, "error %d", errnum);
 		return errno_string;
diff --git a/common/uimage.c b/common/uimage.c
index 28a25bba2..72c868882 100644
--- a/common/uimage.c
+++ b/common/uimage.c
@@ -527,3 +527,35 @@ out:
 
 	return buf;
 }
+
+int uimage_bootm_open(struct image_data *data)
+{
+	int ret;
+
+	data->os = uimage_open(data->os_file);
+	if (!data->os)
+		return -EINVAL;
+
+	if (bootm_get_verify_mode() > BOOTM_VERIFY_NONE) {
+		ret = uimage_verify(data->os);
+		if (ret) {
+			printf("Checking data crc failed with %s\n",
+					strerror(-ret));
+			uimage_close(data->os);
+			return ret;
+		}
+	}
+
+	uimage_print_contents(data->os);
+
+	if (data->os->header.ih_arch != IH_ARCH) {
+		printf("Unsupported Architecture 0x%x\n",
+		       data->os->header.ih_arch);
+		return -EINVAL;
+	}
+
+	if (data->os_address == UIMAGE_SOME_ADDRESS)
+		data->os_address = data->os->header.ih_load;
+
+	return 0;
+}
diff --git a/include/bootm.h b/include/bootm.h
index 6e9777a9a..1c7a145c6 100644
--- a/include/bootm.h
+++ b/include/bootm.h
@@ -92,6 +92,7 @@ struct image_handler {
 
 	enum filetype filetype;
 	int (*bootm)(struct image_data *data);
+	int (*open)(struct image_data *data);
 };
 
 int register_image_handler(struct image_handler *handle);
diff --git a/include/image-fit.h b/include/image-fit.h
index c49f95826..e817ebfae 100644
--- a/include/image-fit.h
+++ b/include/image-fit.h
@@ -38,6 +38,7 @@ struct fit_handle {
 	unsigned long initrd_size;
 };
 
+int fit_bootm_open(struct image_data *data);
 struct fit_handle *fit_open(const char *filename, const char *config, bool verbose,
 			    enum bootm_verify verify);
 void fit_close(struct fit_handle *handle);
diff --git a/include/image.h b/include/image.h
index 3e75d49b8..9ed265658 100644
--- a/include/image.h
+++ b/include/image.h
@@ -229,6 +229,8 @@ struct uimage_handle_data {
 	ulong len;
 };
 
+struct image_data;
+int uimage_bootm_open(struct image_data *data);
 struct uimage_handle *uimage_open(const char *filename);
 void uimage_close(struct uimage_handle *handle);
 int uimage_verify(struct uimage_handle *handle);
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 02/13] boot_verify: use a new error ESECVIOLATION
  2017-03-26  2:44 ` [PATCH 01/13] bootm: move open to image_handler Jean-Christophe PLAGNIOL-VILLARD
@ 2017-03-26  2:44   ` Jean-Christophe PLAGNIOL-VILLARD
  2017-03-26  7:59     ` Michael Olbrich
  2017-03-26  2:44   ` [PATCH 03/13] bootm: make security generic Jean-Christophe PLAGNIOL-VILLARD
                     ` (11 subsequent siblings)
  12 siblings, 1 reply; 22+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-26  2:44 UTC (permalink / raw)
  To: barebox

so we can indentify it correctly

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 common/bootm.c              |  7 ++++---
 common/efi/efi.c            |  2 +-
 common/image-fit.c          | 12 ++++++------
 include/asm-generic/errno.h |  1 +
 4 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/common/bootm.c b/common/bootm.c
index 64c933b3c..53311ab1c 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -541,9 +541,10 @@ int bootm_boot(struct bootm_data *bootm_data)
 		data->oftree = NULL;
 		data->oftree_file = NULL;
 		data->initrd_file = NULL;
-		if (os_type != filetype_oftree) {
-			printf("Signed boot and image is no FIT image, aborting\n");
-			ret = -EINVAL;
+		if (!handler->is_secure_supported) {
+			printf("Signed boot and image %s does not support it",
+				handler->name);
+			ret = -ESECVIOLATION;
 			goto err_out;
 		}
 	}
diff --git a/common/efi/efi.c b/common/efi/efi.c
index 05c58250f..19ee96411 100644
--- a/common/efi/efi.c
+++ b/common/efi/efi.c
@@ -244,7 +244,7 @@ int efi_errno(efi_status_t err)
 	case EFI_TFTP_ERROR: ret = EINVAL; break;
 	case EFI_PROTOCOL_ERROR: ret = EPROTO; break;
 	case EFI_INCOMPATIBLE_VERSION: ret = EINVAL; break;
-	case EFI_SECURITY_VIOLATION: ret = EINVAL; break;
+	case EFI_SECURITY_VIOLATION: ret = ESECVIOLATION; break;
 	case EFI_CRC_ERROR: ret = EINVAL; break;
 	case EFI_END_OF_MEDIA: ret = EINVAL; break;
 	case EFI_END_OF_FILE: ret = EINVAL; break;
diff --git a/common/image-fit.c b/common/image-fit.c
index 5c014d66b..5750199c3 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -353,23 +353,23 @@ static int fit_verify_hash(struct device_node *hash, const void *data, int data_
 	value_read = of_get_property(hash, "value", &hash_len);
 	if (!value_read) {
 		pr_err("%s: \"value\" property not found\n", hash->full_name);
-		return -EINVAL;
+		return -ESECVIOLATION;
 	}
 
 	if (of_property_read_string(hash, "algo", &algo)) {
 		pr_err("%s: \"algo\" property not found\n", hash->full_name);
-		return -EINVAL;
+		return -ESECVIOLATION;
 	}
 
 	d = digest_alloc(algo);
 	if (!d) {
 		pr_err("%s: unsupported algo %s\n", hash->full_name, algo);
-		return -EINVAL;
+		return -ESECVIOLATION;
 	}
 
 	if (hash_len != digest_length(d)) {
 		pr_err("%s: invalid hash length %d\n", hash->full_name, hash_len);
-		ret = -EINVAL;
+		ret = -ESECVIOLATION;
 		goto err_digest_free;
 	}
 
@@ -381,7 +381,7 @@ static int fit_verify_hash(struct device_node *hash, const void *data, int data_
 
 	if (memcmp(value_read, value_calc, hash_len)) {
 		pr_info("%s: hash BAD\n", hash->full_name);
-		ret =  -EBADMSG;
+		ret =  -ESECVIOLATION;
 	} else {
 		pr_info("%s: hash OK\n", hash->full_name);
 		ret = 0;
@@ -431,7 +431,7 @@ static int fit_open_image(struct fit_handle *handle, const char *unit, const voi
 		if (handle->verify == BOOTM_VERIFY_AVAILABLE)
 			ret = 0;
 		else
-			ret = -EINVAL;
+			ret = -ESECVIOLATION;
 		for_each_child_of_node(image, hash) {
 			if (handle->verbose)
 				of_print_nodes(hash, 0);
diff --git a/include/asm-generic/errno.h b/include/asm-generic/errno.h
index 7d99a9537..45b2a2065 100644
--- a/include/asm-generic/errno.h
+++ b/include/asm-generic/errno.h
@@ -133,6 +133,7 @@
 #define	EKEYREJECTED	129	/* Key was rejected by service */
 
 /* Should never be seen by user programs */
+#define ESECVIOLATION	511
 #define ERESTARTSYS	512
 #define ERESTARTNOINTR	513
 #define ERESTARTNOHAND	514	/* restart if no handler.. */
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 03/13] bootm: make security generic
  2017-03-26  2:44 ` [PATCH 01/13] bootm: move open to image_handler Jean-Christophe PLAGNIOL-VILLARD
  2017-03-26  2:44   ` [PATCH 02/13] boot_verify: use a new error ESECVIOLATION Jean-Christophe PLAGNIOL-VILLARD
@ 2017-03-26  2:44   ` Jean-Christophe PLAGNIOL-VILLARD
  2017-03-26  2:44   ` [PATCH 04/13] boot: invert the secure boot forcing support Jean-Christophe PLAGNIOL-VILLARD
                     ` (10 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-26  2:44 UTC (permalink / raw)
  To: barebox

so other secure format can be used

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 arch/arm/lib/bootm.c | 1 +
 common/image-fit.c   | 1 +
 include/bootm.h      | 1 +
 3 files changed, 3 insertions(+)

diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index 204344f87..5b90705cd 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -580,6 +580,7 @@ BAREBOX_MAGICVAR(aimage_noverwrite_tags, "Disable overwrite of the tags addr wit
 
 static struct image_handler arm_fit_handler = {
         .name = "FIT image",
+	.is_secure_supported = 1,
 	.open = fit_bootm_open,
         .bootm = do_bootm_linux,
         .filetype = filetype_oftree,
diff --git a/common/image-fit.c b/common/image-fit.c
index 5750199c3..7563eb955 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -617,6 +617,7 @@ static int do_bootm_sandbox_fit(struct image_data *data)
 
 static struct image_handler sandbox_fit_handler = {
 	.name = "FIT image",
+	.is_secure_supported = 1,
 	.open = fit_bootm_open,
 	.bootm = do_bootm_sandbox_fit,
 	.filetype = filetype_oftree,
diff --git a/include/bootm.h b/include/bootm.h
index 1c7a145c6..27c9f571e 100644
--- a/include/bootm.h
+++ b/include/bootm.h
@@ -88,6 +88,7 @@ struct image_handler {
 
 	struct list_head list;
 
+	int is_secure_supported;
 	int ih_os;
 
 	enum filetype filetype;
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 04/13] boot: invert the secure boot forcing support
  2017-03-26  2:44 ` [PATCH 01/13] bootm: move open to image_handler Jean-Christophe PLAGNIOL-VILLARD
  2017-03-26  2:44   ` [PATCH 02/13] boot_verify: use a new error ESECVIOLATION Jean-Christophe PLAGNIOL-VILLARD
  2017-03-26  2:44   ` [PATCH 03/13] bootm: make security generic Jean-Christophe PLAGNIOL-VILLARD
@ 2017-03-26  2:44   ` Jean-Christophe PLAGNIOL-VILLARD
  2017-03-26  2:44   ` [PATCH 05/13] move boot verify to generic code Jean-Christophe PLAGNIOL-VILLARD
                     ` (9 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-26  2:44 UTC (permalink / raw)
  To: barebox

Add HAS_SECURE_BOOT as we will add other image format that support secure boot

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 common/Kconfig | 12 ++++++++----
 common/bootm.c |  6 +++---
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/common/Kconfig b/common/Kconfig
index f7ff04664..895814ee9 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -21,6 +21,9 @@ config HAS_KALLSYMS
 config HAS_MODULES
 	bool
 
+config HAS_SECURE_BOOT
+	bool
+
 config HAS_CACHE
 	bool
 	help
@@ -624,6 +627,7 @@ config BOOTM_FITIMAGE_SIGNATURE
 	prompt "support verifying signed FIT images"
 	depends on BOOTM_FITIMAGE
 	select FITIMAGE_SIGNATURE
+	select HAS_SECURE_BOOT
 	help
 	  Support verifying signed FIT images. This requires FIT images
 	  as described in:
@@ -631,14 +635,14 @@ config BOOTM_FITIMAGE_SIGNATURE
 	  Additionally the barebox device tree needs a /signature node with the
 	  public key with which the image has been signed.
 
-config BOOTM_FORCE_SIGNED_IMAGES
+config BOOT_FORCE_SIGNED_IMAGES
 	bool
 	prompt "Force booting of signed images"
-	depends on BOOTM_FITIMAGE_SIGNATURE
+	depends on HAS_SECURE_BOOT
 	help
 	  With this option enabled only signed images can be booted, unsigned images
-	  are refused to boot. Effectively this means only FIT images can be booted
-	  since they are the only supported image type that support signing.
+	  are refused to boot. Effectively this means only Signed images can
+	  be booted.
 
 config BLSPEC
 	depends on BLOCK
diff --git a/common/bootm.c b/common/bootm.c
index 53311ab1c..885b09f81 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -82,7 +82,7 @@ enum bootm_verify bootm_get_verify_mode(void)
 }
 
 static const char * const bootm_verify_names[] = {
-#ifndef CONFIG_BOOTM_FORCE_SIGNED_IMAGES
+#ifndef CONFIG_BOOT_FORCE_SIGNED_IMAGES
 	[BOOTM_VERIFY_NONE] = "none",
 	[BOOTM_VERIFY_HASH] = "hash",
 	[BOOTM_VERIFY_AVAILABLE] = "available",
@@ -531,7 +531,7 @@ int bootm_boot(struct bootm_data *bootm_data)
 		goto err_out;
 	}
 
-	if (IS_ENABLED(CONFIG_BOOTM_FORCE_SIGNED_IMAGES)) {
+	if (IS_ENABLED(CONFIG_BOOT_FORCE_SIGNED_IMAGES)) {
 		data->verify = BOOTM_VERIFY_SIGNATURE;
 
 		/*
@@ -635,7 +635,7 @@ static int bootm_init(void)
 		globalvar_add_simple("bootm.initrd.loadaddr", NULL);
 	}
 
-	if (IS_ENABLED(CONFIG_BOOTM_FORCE_SIGNED_IMAGES))
+	if (IS_ENABLED(CONFIG_BOOT_FORCE_SIGNED_IMAGES))
 		bootm_verify_mode = BOOTM_VERIFY_SIGNATURE;
 
 	globalvar_add_simple_int("bootm.verbose", &bootm_verbosity, "%u");
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 05/13] move boot verify to generic code
  2017-03-26  2:44 ` [PATCH 01/13] bootm: move open to image_handler Jean-Christophe PLAGNIOL-VILLARD
                     ` (2 preceding siblings ...)
  2017-03-26  2:44   ` [PATCH 04/13] boot: invert the secure boot forcing support Jean-Christophe PLAGNIOL-VILLARD
@ 2017-03-26  2:44   ` Jean-Christophe PLAGNIOL-VILLARD
  2017-03-26  2:44   ` [PATCH 06/13] boot_verify: make it modifiable at start time Jean-Christophe PLAGNIOL-VILLARD
                     ` (8 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-26  2:44 UTC (permalink / raw)
  To: barebox

so we can use it outside of bootm only

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 commands/bootm.c      |  6 +++---
 common/Kconfig        |  4 ++++
 common/Makefile       |  1 +
 common/boot_verify.c  | 35 +++++++++++++++++++++++++++++++++++
 common/bootm.c        | 29 +++--------------------------
 common/image-fit.c    | 14 +++++++-------
 common/uimage.c       |  2 +-
 include/boot_verify.h | 20 ++++++++++++++++++++
 include/bootm.h       | 14 +++-----------
 include/image-fit.h   |  4 ++--
 10 files changed, 79 insertions(+), 50 deletions(-)
 create mode 100644 common/boot_verify.c
 create mode 100644 include/boot_verify.h

diff --git a/commands/bootm.c b/commands/bootm.c
index c7cbdbe0f..b35aaa914 100644
--- a/commands/bootm.c
+++ b/commands/bootm.c
@@ -64,11 +64,11 @@ static int do_bootm(int argc, char *argv[])
 	while ((opt = getopt(argc, argv, BOOTM_OPTS)) > 0) {
 		switch(opt) {
 		case 'c':
-			if (data.verify < BOOTM_VERIFY_HASH)
-				data.verify = BOOTM_VERIFY_HASH;
+			if (data.verify < BOOT_VERIFY_HASH)
+				data.verify = BOOT_VERIFY_HASH;
 			break;
 		case 's':
-			data.verify = BOOTM_VERIFY_SIGNATURE;
+			data.verify = BOOT_VERIFY_SIGNATURE;
 			break;
 #ifdef CONFIG_BOOTM_INITRD
 		case 'L':
diff --git a/common/Kconfig b/common/Kconfig
index 895814ee9..00e98e859 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -24,6 +24,9 @@ config HAS_MODULES
 config HAS_SECURE_BOOT
 	bool
 
+config BOOT_VERIFY
+	bool
+
 config HAS_CACHE
 	bool
 	help
@@ -551,6 +554,7 @@ config TIMESTAMP
 
 menuconfig BOOTM
 	select UIMAGE
+	select BOOT_VERIFY
 	default y if COMMAND_SUPPORT
 	bool "bootm support"
 
diff --git a/common/Makefile b/common/Makefile
index 5f58c81d2..5d471a3a0 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_UBIFORMAT)		+= ubiformat.o
 obj-$(CONFIG_BAREBOX_UPDATE_IMX_NAND_FCB) += imx-bbu-nand-fcb.o
 obj-$(CONFIG_CONSOLE_RATP)	+= ratp.o
 obj-$(CONFIG_BOOT)		+= boot.o
+obj-$(CONFIG_BOOT_VERIFY)	+= boot_verify.o
 
 quiet_cmd_pwd_h = PWDH    $@
 ifdef CONFIG_PASSWORD
diff --git a/common/boot_verify.c b/common/boot_verify.c
new file mode 100644
index 000000000..afe929e68
--- /dev/null
+++ b/common/boot_verify.c
@@ -0,0 +1,35 @@
+#include <common.h>
+#include <boot_verify.h>
+#include <globalvar.h>
+#include <magicvar.h>
+#include <init.h>
+
+static enum boot_verify boot_verify_mode = BOOT_VERIFY_HASH;
+
+enum boot_verify boot_get_verify_mode(void)
+{
+	return boot_verify_mode;
+}
+
+static const char * const boot_verify_names[] = {
+#ifndef CONFIG_BOOT_FORCE_SIGNED_IMAGES
+	[BOOT_VERIFY_NONE] = "none",
+	[BOOT_VERIFY_HASH] = "hash",
+	[BOOT_VERIFY_AVAILABLE] = "available",
+#endif
+	[BOOT_VERIFY_SIGNATURE] = "signature",
+};
+
+static int init_boot_verify(void)
+{
+	if (IS_ENABLED(CONFIG_BOOT_FORCE_SIGNED_IMAGES))
+		boot_verify_mode = BOOT_VERIFY_SIGNATURE;
+
+	globalvar_add_simple_enum("boot.verify", (unsigned int *)&boot_verify_mode,
+				  boot_verify_names, ARRAY_SIZE(boot_verify_names));
+
+	return 0;
+}
+late_initcall(init_boot_verify);
+
+BAREBOX_MAGICVAR_NAMED(global_boot_verify, global.boot.verify, "boot default verify level");
diff --git a/common/bootm.c b/common/bootm.c
index 885b09f81..74202a829 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -69,27 +69,11 @@ void bootm_data_init_defaults(struct bootm_data *data)
 	getenv_ul("global.bootm.image.loadaddr", &data->os_address);
 	getenv_ul("global.bootm.initrd.loadaddr", &data->initrd_address);
 	data->initrd_file = getenv_nonempty("global.bootm.initrd");
-	data->verify = bootm_get_verify_mode();
+	data->verify = boot_get_verify_mode();
 	data->appendroot = bootm_appendroot;
 	data->verbose = bootm_verbosity;
 }
 
-static enum bootm_verify bootm_verify_mode = BOOTM_VERIFY_HASH;
-
-enum bootm_verify bootm_get_verify_mode(void)
-{
-	return bootm_verify_mode;
-}
-
-static const char * const bootm_verify_names[] = {
-#ifndef CONFIG_BOOT_FORCE_SIGNED_IMAGES
-	[BOOTM_VERIFY_NONE] = "none",
-	[BOOTM_VERIFY_HASH] = "hash",
-	[BOOTM_VERIFY_AVAILABLE] = "available",
-#endif
-	[BOOTM_VERIFY_SIGNATURE] = "signature",
-};
-
 static int uimage_part_num(const char *partname)
 {
 	if (!partname)
@@ -175,7 +159,7 @@ static int bootm_open_initrd_uimage(struct image_data *data)
 		if (!data->initrd)
 			return -EINVAL;
 
-		if (bootm_get_verify_mode() > BOOTM_VERIFY_NONE) {
+		if (boot_get_verify_mode() > BOOT_VERIFY_NONE) {
 			ret = uimage_verify(data->initrd);
 			if (ret) {
 				printf("Checking data crc failed with %s\n",
@@ -532,7 +516,7 @@ int bootm_boot(struct bootm_data *bootm_data)
 	}
 
 	if (IS_ENABLED(CONFIG_BOOT_FORCE_SIGNED_IMAGES)) {
-		data->verify = BOOTM_VERIFY_SIGNATURE;
+		data->verify = BOOT_VERIFY_SIGNATURE;
 
 		/*
 		 * When we only allow booting signed images make sure everything
@@ -635,14 +619,8 @@ static int bootm_init(void)
 		globalvar_add_simple("bootm.initrd.loadaddr", NULL);
 	}
 
-	if (IS_ENABLED(CONFIG_BOOT_FORCE_SIGNED_IMAGES))
-		bootm_verify_mode = BOOTM_VERIFY_SIGNATURE;
-
 	globalvar_add_simple_int("bootm.verbose", &bootm_verbosity, "%u");
 
-	globalvar_add_simple_enum("bootm.verify", (unsigned int *)&bootm_verify_mode,
-				  bootm_verify_names, ARRAY_SIZE(bootm_verify_names));
-
 	return 0;
 }
 late_initcall(bootm_init);
@@ -653,6 +631,5 @@ BAREBOX_MAGICVAR_NAMED(global_bootm_image_loadaddr, global.bootm.image.loadaddr,
 BAREBOX_MAGICVAR_NAMED(global_bootm_initrd, global.bootm.initrd, "bootm default initrd");
 BAREBOX_MAGICVAR_NAMED(global_bootm_initrd_loadaddr, global.bootm.initrd.loadaddr, "bootm default initrd loadaddr");
 BAREBOX_MAGICVAR_NAMED(global_bootm_oftree, global.bootm.oftree, "bootm default oftree");
-BAREBOX_MAGICVAR_NAMED(global_bootm_verify, global.bootm.verify, "bootm default verify level");
 BAREBOX_MAGICVAR_NAMED(global_bootm_verbose, global.bootm.verbose, "bootm default verbosity level (0=quiet)");
 BAREBOX_MAGICVAR_NAMED(global_bootm_appendroot, global.bootm.appendroot, "Add root= option to Kernel to mount rootfs from the device the Kernel comes from");
diff --git a/common/image-fit.c b/common/image-fit.c
index 7563eb955..53f3173fc 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -427,8 +427,8 @@ static int fit_open_image(struct fit_handle *handle, const char *unit, const voi
 		return -EINVAL;
 	}
 
-	if (handle->verify > BOOTM_VERIFY_NONE) {
-		if (handle->verify == BOOTM_VERIFY_AVAILABLE)
+	if (handle->verify > BOOT_VERIFY_NONE) {
+		if (handle->verify == BOOT_VERIFY_AVAILABLE)
 			ret = 0;
 		else
 			ret = -ESECVIOLATION;
@@ -461,13 +461,13 @@ static int fit_config_verify_signature(struct fit_handle *handle, struct device_
 		return 0;
 
 	switch (handle->verify) {
-	case BOOTM_VERIFY_NONE:
-	case BOOTM_VERIFY_HASH:
+	case BOOT_VERIFY_NONE:
+	case BOOT_VERIFY_HASH:
 		return 0;
-	case BOOTM_VERIFY_SIGNATURE:
+	case BOOT_VERIFY_SIGNATURE:
 		ret = -EINVAL;
 		break;
-	case BOOTM_VERIFY_AVAILABLE:
+	case BOOT_VERIFY_AVAILABLE:
 		ret = 0;
 		break;
 	}
@@ -542,7 +542,7 @@ static int fit_open_configuration(struct fit_handle *handle, const char *name)
 }
 
 struct fit_handle *fit_open(const char *filename, const char *config, bool verbose,
-			    enum bootm_verify verify)
+			    enum boot_verify verify)
 {
 	struct fit_handle *handle = NULL;
 	const char *desc = "(no description)";
diff --git a/common/uimage.c b/common/uimage.c
index 72c868882..d1947aa11 100644
--- a/common/uimage.c
+++ b/common/uimage.c
@@ -536,7 +536,7 @@ int uimage_bootm_open(struct image_data *data)
 	if (!data->os)
 		return -EINVAL;
 
-	if (bootm_get_verify_mode() > BOOTM_VERIFY_NONE) {
+	if (boot_get_verify_mode() > BOOT_VERIFY_NONE) {
 		ret = uimage_verify(data->os);
 		if (ret) {
 			printf("Checking data crc failed with %s\n",
diff --git a/include/boot_verify.h b/include/boot_verify.h
new file mode 100644
index 000000000..3a4436584
--- /dev/null
+++ b/include/boot_verify.h
@@ -0,0 +1,20 @@
+#ifndef __BOOT_VERIFY_H__
+#define __BOOT_VERIFY_H__
+
+enum boot_verify {
+	BOOT_VERIFY_NONE,
+	BOOT_VERIFY_HASH,
+	BOOT_VERIFY_AVAILABLE,
+	BOOT_VERIFY_SIGNATURE,
+};
+
+#ifndef CONFIG_BOOT_VERIFY
+static inline enum boot_verify boot_get_verify_mode(void)
+{
+	return BOOT_VERIFY_NONE;
+}
+#else
+enum boot_verify boot_get_verify_mode(void);
+#endif
+
+#endif /* __BOOT_VERIFY_H__ */
diff --git a/include/bootm.h b/include/bootm.h
index 27c9f571e..73b0c8294 100644
--- a/include/bootm.h
+++ b/include/bootm.h
@@ -3,21 +3,15 @@
 
 #include <image.h>
 #include <filetype.h>
+#include <boot_verify.h>
 #include <linux/list.h>
 
-enum bootm_verify {
-	BOOTM_VERIFY_NONE,
-	BOOTM_VERIFY_HASH,
-	BOOTM_VERIFY_SIGNATURE,
-	BOOTM_VERIFY_AVAILABLE,
-};
-
 struct bootm_data {
 	const char *os_file;
 	const char *initrd_file;
 	const char *oftree_file;
 	int verbose;
-	enum bootm_verify verify;
+	enum boot_verify verify;
 	bool force;
 	bool dryrun;
 	/*
@@ -77,7 +71,7 @@ struct image_data {
 	struct fdt_header *oftree;
 	struct resource *oftree_res;
 
-	enum bootm_verify verify;
+	enum boot_verify verify;
 	int verbose;
 	int force;
 	int dryrun;
@@ -120,8 +114,6 @@ int bootm_load_initrd(struct image_data *data, unsigned long load_address);
 int bootm_load_devicetree(struct image_data *data, unsigned long load_address);
 int bootm_get_os_size(struct image_data *data);
 
-enum bootm_verify bootm_get_verify_mode(void);
-
 #define UIMAGE_SOME_ADDRESS (UIMAGE_INVALID_ADDRESS - 1)
 
 #endif /* __BOOTM_H */
diff --git a/include/image-fit.h b/include/image-fit.h
index e817ebfae..bb69ce5af 100644
--- a/include/image-fit.h
+++ b/include/image-fit.h
@@ -26,7 +26,7 @@ struct fit_handle {
 	size_t size;
 
 	bool verbose;
-	enum bootm_verify verify;
+	enum boot_verify verify;
 
 	struct device_node *root;
 
@@ -40,7 +40,7 @@ struct fit_handle {
 
 int fit_bootm_open(struct image_data *data);
 struct fit_handle *fit_open(const char *filename, const char *config, bool verbose,
-			    enum bootm_verify verify);
+			    enum boot_verify verify);
 void fit_close(struct fit_handle *handle);
 
 #endif	/* __IMAGE_FIT_H__ */
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 06/13] boot_verify: make it modifiable at start time
  2017-03-26  2:44 ` [PATCH 01/13] bootm: move open to image_handler Jean-Christophe PLAGNIOL-VILLARD
                     ` (3 preceding siblings ...)
  2017-03-26  2:44   ` [PATCH 05/13] move boot verify to generic code Jean-Christophe PLAGNIOL-VILLARD
@ 2017-03-26  2:44   ` Jean-Christophe PLAGNIOL-VILLARD
  2017-03-26  8:16     ` Michael Olbrich
  2017-03-26  2:44   ` [PATCH 07/13] go: only use it if boot signature is not required Jean-Christophe PLAGNIOL-VILLARD
                     ` (7 subsequent siblings)
  12 siblings, 1 reply; 22+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-26  2:44 UTC (permalink / raw)
  To: barebox

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 commands/bootm.c      |  2 +-
 common/boot_verify.c  | 39 +++++++++++++++++++++++++++++++++------
 common/bootm.c        |  2 +-
 include/boot_verify.h | 15 ++++++++++++---
 4 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/commands/bootm.c b/commands/bootm.c
index b35aaa914..cb520a1ba 100644
--- a/commands/bootm.c
+++ b/commands/bootm.c
@@ -64,7 +64,7 @@ static int do_bootm(int argc, char *argv[])
 	while ((opt = getopt(argc, argv, BOOTM_OPTS)) > 0) {
 		switch(opt) {
 		case 'c':
-			if (data.verify < BOOT_VERIFY_HASH)
+			if (data.verify > BOOT_VERIFY_HASH)
 				data.verify = BOOT_VERIFY_HASH;
 			break;
 		case 's':
diff --git a/common/boot_verify.c b/common/boot_verify.c
index afe929e68..9cbeb7a65 100644
--- a/common/boot_verify.c
+++ b/common/boot_verify.c
@@ -11,22 +11,49 @@ enum boot_verify boot_get_verify_mode(void)
 	return boot_verify_mode;
 }
 
+/* keep it for the most secure to the less */
 static const char * const boot_verify_names[] = {
-#ifndef CONFIG_BOOT_FORCE_SIGNED_IMAGES
-	[BOOT_VERIFY_NONE] = "none",
-	[BOOT_VERIFY_HASH] = "hash",
-	[BOOT_VERIFY_AVAILABLE] = "available",
-#endif
 	[BOOT_VERIFY_SIGNATURE] = "signature",
+	[BOOT_VERIFY_AVAILABLE] = "available",
+	[BOOT_VERIFY_HASH] = "hash",
+	[BOOT_VERIFY_NONE] = "none",
 };
 
+/* allow architecture to overwrite it such as EFI */
+static int default_is_secure_mode(void)
+{
+	if (IS_ENABLED(CONFIG_BOOT_FORCE_SIGNED_IMAGES))
+		return 1;
+
+	return 0;
+}
+
+static int (*__is_secure_mode)(void) = default_is_secure_mode;
+
+int is_secure_mode(void)
+{
+	return __is_secure_mode();
+}
+
+void boot_set_is_secure_mode(int (*fn)(void))
+{
+	__is_secure_mode = fn;
+}
+
 static int init_boot_verify(void)
 {
+	int size;
+
 	if (IS_ENABLED(CONFIG_BOOT_FORCE_SIGNED_IMAGES))
 		boot_verify_mode = BOOT_VERIFY_SIGNATURE;
 
+	if (is_secure_mode())
+		size = 1;
+	else
+		size = ARRAY_SIZE(boot_verify_names);
+
 	globalvar_add_simple_enum("boot.verify", (unsigned int *)&boot_verify_mode,
-				  boot_verify_names, ARRAY_SIZE(boot_verify_names));
+				  boot_verify_names, size);
 
 	return 0;
 }
diff --git a/common/bootm.c b/common/bootm.c
index 74202a829..1558f3c5d 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -159,7 +159,7 @@ static int bootm_open_initrd_uimage(struct image_data *data)
 		if (!data->initrd)
 			return -EINVAL;
 
-		if (boot_get_verify_mode() > BOOT_VERIFY_NONE) {
+		if (boot_get_verify_mode() != BOOT_VERIFY_NONE) {
 			ret = uimage_verify(data->initrd);
 			if (ret) {
 				printf("Checking data crc failed with %s\n",
diff --git a/include/boot_verify.h b/include/boot_verify.h
index 3a4436584..ee830bf5c 100644
--- a/include/boot_verify.h
+++ b/include/boot_verify.h
@@ -2,10 +2,10 @@
 #define __BOOT_VERIFY_H__
 
 enum boot_verify {
-	BOOT_VERIFY_NONE,
-	BOOT_VERIFY_HASH,
-	BOOT_VERIFY_AVAILABLE,
 	BOOT_VERIFY_SIGNATURE,
+	BOOT_VERIFY_AVAILABLE,
+	BOOT_VERIFY_HASH,
+	BOOT_VERIFY_NONE,
 };
 
 #ifndef CONFIG_BOOT_VERIFY
@@ -13,8 +13,17 @@ static inline enum boot_verify boot_get_verify_mode(void)
 {
 	return BOOT_VERIFY_NONE;
 }
+
+static int inline is_secure_mode(void)
+{
+	return 0;
+}
+
+static void inline boot_set_is_secure_mode(int (*fn)(void)) {}
 #else
 enum boot_verify boot_get_verify_mode(void);
+int is_secure_mode(void);
+void boot_set_is_secure_mode(int (*fn)(void));
 #endif
 
 #endif /* __BOOT_VERIFY_H__ */
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 07/13] go: only use it if boot signature is not required
  2017-03-26  2:44 ` [PATCH 01/13] bootm: move open to image_handler Jean-Christophe PLAGNIOL-VILLARD
                     ` (4 preceding siblings ...)
  2017-03-26  2:44   ` [PATCH 06/13] boot_verify: make it modifiable at start time Jean-Christophe PLAGNIOL-VILLARD
@ 2017-03-26  2:44   ` Jean-Christophe PLAGNIOL-VILLARD
  2017-03-26  8:23     ` Michael Olbrich
  2017-03-26  2:44   ` [PATCH 08/13] boot_verify: allow to force unsigned image to boot Jean-Christophe PLAGNIOL-VILLARD
                     ` (6 subsequent siblings)
  12 siblings, 1 reply; 22+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-26  2:44 UTC (permalink / raw)
  To: barebox

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 commands/go.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/commands/go.c b/commands/go.c
index fb319b320..e0385a977 100644
--- a/commands/go.c
+++ b/commands/go.c
@@ -26,6 +26,7 @@
 #include <fcntl.h>
 #include <linux/ctype.h>
 #include <errno.h>
+#include <boot_verify.h>
 
 static int do_go(int argc, char *argv[])
 {
@@ -37,6 +38,9 @@ static int do_go(int argc, char *argv[])
 	if (argc < 2)
 		return COMMAND_ERROR_USAGE;
 
+	if (boot_get_verify_mode() < BOOT_VERIFY_AVAILABLE)
+		return -ESECVIOLATION;
+
 	if (!isdigit(*argv[1])) {
 		fd = open(argv[1], O_RDONLY);
 		if (fd < 0) {
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 08/13] boot_verify: allow to force unsigned image to boot
  2017-03-26  2:44 ` [PATCH 01/13] bootm: move open to image_handler Jean-Christophe PLAGNIOL-VILLARD
                     ` (5 preceding siblings ...)
  2017-03-26  2:44   ` [PATCH 07/13] go: only use it if boot signature is not required Jean-Christophe PLAGNIOL-VILLARD
@ 2017-03-26  2:44   ` Jean-Christophe PLAGNIOL-VILLARD
  2017-03-26  8:25     ` Michael Olbrich
  2017-03-26  2:45   ` [PATCH 09/13] boot_verify: add password request support Jean-Christophe PLAGNIOL-VILLARD
                     ` (5 subsequent siblings)
  12 siblings, 1 reply; 22+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-26  2:44 UTC (permalink / raw)
  To: barebox

request confirmation before booting an unsigned image

with a default timeout

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 commands/go.c         |  9 +++++++--
 common/Kconfig        |  8 ++++++++
 common/boot_verify.c  | 47 ++++++++++++++++++++++++++++++++++++++++++++---
 common/bootm.c        |  6 ++++++
 common/image-fit.c    |  1 +
 common/uimage.c       |  1 +
 include/boot_verify.h |  7 +++++++
 7 files changed, 74 insertions(+), 5 deletions(-)

diff --git a/commands/go.c b/commands/go.c
index e0385a977..919bcddc7 100644
--- a/commands/go.c
+++ b/commands/go.c
@@ -38,8 +38,13 @@ static int do_go(int argc, char *argv[])
 	if (argc < 2)
 		return COMMAND_ERROR_USAGE;
 
-	if (boot_get_verify_mode() < BOOT_VERIFY_AVAILABLE)
-		return -ESECVIOLATION;
+	if (boot_get_verify_mode() < BOOT_VERIFY_AVAILABLE) {
+		int is_sec;
+
+		is_sec = boot_can_start_unsigned();
+		if (is_sec)
+			return is_sec;
+	}
 
 	if (!isdigit(*argv[1])) {
 		fd = open(argv[1], O_RDONLY);
diff --git a/common/Kconfig b/common/Kconfig
index 00e98e859..2588651ae 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -648,6 +648,14 @@ config BOOT_FORCE_SIGNED_IMAGES
 	  are refused to boot. Effectively this means only Signed images can
 	  be booted.
 
+config BOOT_FORCE_USER_SIGNED_IMAGES
+	bool
+	prompt "Force booting of signed images or confirm them"
+	depends on HAS_SECURE_BOOT
+	help
+	  With this option enabled only signed images can be booted, unsigned images
+	  need a user confirmation to boot.
+
 config BLSPEC
 	depends on BLOCK
 	depends on FLEXIBLE_BOOTARGS
diff --git a/common/boot_verify.c b/common/boot_verify.c
index 9cbeb7a65..07ae07e16 100644
--- a/common/boot_verify.c
+++ b/common/boot_verify.c
@@ -1,9 +1,17 @@
+/*
+ * Copyright (c) 2016 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
+ * Copyright (c) 2017 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
+ *
+ * Under GPLv2 Only
+ */
 #include <common.h>
 #include <boot_verify.h>
+#include <console_countdown.h>
 #include <globalvar.h>
 #include <magicvar.h>
 #include <init.h>
 
+static unsigned int boot_verify_confirm_timeout = 10;
 static enum boot_verify boot_verify_mode = BOOT_VERIFY_HASH;
 
 enum boot_verify boot_get_verify_mode(void)
@@ -14,6 +22,7 @@ enum boot_verify boot_get_verify_mode(void)
 /* keep it for the most secure to the less */
 static const char * const boot_verify_names[] = {
 	[BOOT_VERIFY_SIGNATURE] = "signature",
+	[BOOT_VERIFY_SIGNATURE_USER] = "signature-user",
 	[BOOT_VERIFY_AVAILABLE] = "available",
 	[BOOT_VERIFY_HASH] = "hash",
 	[BOOT_VERIFY_NONE] = "none",
@@ -40,6 +49,29 @@ void boot_set_is_secure_mode(int (*fn)(void))
 	__is_secure_mode = fn;
 }
 
+int boot_can_start_unsigned(void)
+{
+	int ret;
+	char c;
+	int timeout = boot_verify_confirm_timeout;
+
+	if (!is_secure_mode())
+		return 0;
+
+	if (boot_verify_mode != BOOT_VERIFY_SIGNATURE_USER)
+		return -ESECVIOLATION;
+
+	printf("Are you sure you wish to run an unsigned binary\n");
+	printf("in a secure environment?\n");
+	printf("press y to confirm\n");
+
+	ret = console_countdown(timeout, CONSOLE_COUNTDOWN_ANYKEY, &c);
+	if (ret != -EINTR)
+		return -ESECVIOLATION;
+
+	return c == 'y' ? 0 : -ESECVIOLATION;
+}
+
 static int init_boot_verify(void)
 {
 	int size;
@@ -47,16 +79,25 @@ static int init_boot_verify(void)
 	if (IS_ENABLED(CONFIG_BOOT_FORCE_SIGNED_IMAGES))
 		boot_verify_mode = BOOT_VERIFY_SIGNATURE;
 
-	if (is_secure_mode())
-		size = 1;
-	else
+	if (is_secure_mode()) {
+		if (IS_ENABLED(CONFIG_BOOT_FORCE_USER_SIGNED_IMAGES))
+			size = 2;
+		else
+			size = 1;
+	} else {
 		size = ARRAY_SIZE(boot_verify_names);
+	}
 
 	globalvar_add_simple_enum("boot.verify", (unsigned int *)&boot_verify_mode,
 				  boot_verify_names, size);
 
+	globalvar_add_simple_int("boot.verify_confirm_timeout",
+				 &boot_verify_confirm_timeout, "%u");
+
 	return 0;
 }
 late_initcall(init_boot_verify);
 
 BAREBOX_MAGICVAR_NAMED(global_boot_verify, global.boot.verify, "boot default verify level");
+BAREBOX_MAGICVAR_NAMED(global_boot_verify_confirm_timeout, global.boot.verify_confirm_timeout,
+		"Secure Boot Comfirm timeout in seconds before booting an unsigned image");
diff --git a/common/bootm.c b/common/bootm.c
index 1558f3c5d..73a3a99dd 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -579,6 +579,12 @@ int bootm_boot(struct bootm_data *bootm_data)
 		printf("Passing control to %s handler\n", handler->name);
 	}
 
+	if (!handler->is_secure_supported && is_secure_mode()) {
+		ret = boot_can_start_unsigned();
+		if (ret)
+			goto err_out;
+	}
+
 	ret = handler->bootm(data);
 	if (data->dryrun)
 		printf("Dryrun. Aborted\n");
diff --git a/common/image-fit.c b/common/image-fit.c
index 53f3173fc..0df735062 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -465,6 +465,7 @@ static int fit_config_verify_signature(struct fit_handle *handle, struct device_
 	case BOOT_VERIFY_HASH:
 		return 0;
 	case BOOT_VERIFY_SIGNATURE:
+	case BOOT_VERIFY_SIGNATURE_USER:
 		ret = -EINVAL;
 		break;
 	case BOOT_VERIFY_AVAILABLE:
diff --git a/common/uimage.c b/common/uimage.c
index d1947aa11..f25341c15 100644
--- a/common/uimage.c
+++ b/common/uimage.c
@@ -30,6 +30,7 @@
 #include <rtc.h>
 #include <filetype.h>
 #include <memory.h>
+#include <bootm.h>
 
 static inline int uimage_is_multi_image(struct uimage_handle *handle)
 {
diff --git a/include/boot_verify.h b/include/boot_verify.h
index ee830bf5c..12dcfbfdc 100644
--- a/include/boot_verify.h
+++ b/include/boot_verify.h
@@ -3,6 +3,7 @@
 
 enum boot_verify {
 	BOOT_VERIFY_SIGNATURE,
+	BOOT_VERIFY_SIGNATURE_USER,
 	BOOT_VERIFY_AVAILABLE,
 	BOOT_VERIFY_HASH,
 	BOOT_VERIFY_NONE,
@@ -19,10 +20,16 @@ static int inline is_secure_mode(void)
 	return 0;
 }
 
+static int inline boot_can_start_unsigned(void)
+{
+	return 0;
+}
+
 static void inline boot_set_is_secure_mode(int (*fn)(void)) {}
 #else
 enum boot_verify boot_get_verify_mode(void);
 int is_secure_mode(void);
+int boot_can_start_unsigned(void);
 void boot_set_is_secure_mode(int (*fn)(void));
 #endif
 
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 09/13] boot_verify: add password request support
  2017-03-26  2:44 ` [PATCH 01/13] bootm: move open to image_handler Jean-Christophe PLAGNIOL-VILLARD
                     ` (6 preceding siblings ...)
  2017-03-26  2:44   ` [PATCH 08/13] boot_verify: allow to force unsigned image to boot Jean-Christophe PLAGNIOL-VILLARD
@ 2017-03-26  2:45   ` Jean-Christophe PLAGNIOL-VILLARD
  2017-03-27  6:11     ` Sascha Hauer
  2017-03-26  2:45   ` [PATCH 10/13] efi: add more security related guid for the efivars Jean-Christophe PLAGNIOL-VILLARD
                     ` (4 subsequent siblings)
  12 siblings, 1 reply; 22+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-26  2:45 UTC (permalink / raw)
  To: barebox

This will allow to let the user enter a password before booting more safe
than just a 'y'

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 common/boot_verify.c | 10 ++++++++++
 common/password.c    | 18 ++++++++++++++++++
 include/password.h   |  6 ++++++
 3 files changed, 34 insertions(+)

diff --git a/common/boot_verify.c b/common/boot_verify.c
index 07ae07e16..2faa8d56c 100644
--- a/common/boot_verify.c
+++ b/common/boot_verify.c
@@ -10,6 +10,7 @@
 #include <globalvar.h>
 #include <magicvar.h>
 #include <init.h>
+#include <password.h>
 
 static unsigned int boot_verify_confirm_timeout = 10;
 static enum boot_verify boot_verify_mode = BOOT_VERIFY_HASH;
@@ -63,6 +64,14 @@ int boot_can_start_unsigned(void)
 
 	printf("Are you sure you wish to run an unsigned binary\n");
 	printf("in a secure environment?\n");
+	if (IS_ENABLED(CONFIG_PASSWORD)) {
+		printf("enter password to confirm\n");
+		ret = request_password(timeout);
+		if (ret != -ENOTSUPP)
+			return -ESECVIOLATION;
+
+	}
+
 	printf("press y to confirm\n");
 
 	ret = console_countdown(timeout, CONSOLE_COUNTDOWN_ANYKEY, &c);
@@ -72,6 +81,7 @@ int boot_can_start_unsigned(void)
 	return c == 'y' ? 0 : -ESECVIOLATION;
 }
 
+
 static int init_boot_verify(void)
 {
 	int size;
diff --git a/common/password.c b/common/password.c
index d52b746f0..1147111cd 100644
--- a/common/password.c
+++ b/common/password.c
@@ -435,6 +435,24 @@ void login(void)
 	}
 }
 
+int request_password(int timeout)
+{
+	unsigned char passwd[PASSWD_MAX_LENGTH];
+	int ret;
+
+	if (!is_passwd_default_enable() && !is_passwd_env_enable())
+		return -ENOTSUPP;
+
+	ret = password(passwd, PASSWD_MAX_LENGTH, LOGIN_MODE, timeout);
+	if (ret < 0)
+		return ret;
+
+	if (check_passwd(passwd, ret) == 1)
+		return 0;
+
+	return -EINVAL;
+}
+
 static int login_global_init(void)
 {
 	login_fail_command = xstrdup("boot");
diff --git a/include/password.h b/include/password.h
index 8b9961815..5e8964929 100644
--- a/include/password.h
+++ b/include/password.h
@@ -31,10 +31,16 @@ int set_env_passwd(unsigned char *passwd, size_t length);
 
 #ifdef CONFIG_PASSWORD
 void login(void);
+int request_password(int timeout);
 #else
 static inline void login(void)
 {
 }
+
+static inline int request_password(int timeout)
+{
+	return 0;
+}
 #endif
 
 #endif /* __PASSWORD_H__ */
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 10/13] efi: add more security related guid for the efivars
  2017-03-26  2:44 ` [PATCH 01/13] bootm: move open to image_handler Jean-Christophe PLAGNIOL-VILLARD
                     ` (7 preceding siblings ...)
  2017-03-26  2:45   ` [PATCH 09/13] boot_verify: add password request support Jean-Christophe PLAGNIOL-VILLARD
@ 2017-03-26  2:45   ` Jean-Christophe PLAGNIOL-VILLARD
  2017-03-26  2:45   ` [PATCH 11/13] efi: fix lds for secure boot support Jean-Christophe PLAGNIOL-VILLARD
                     ` (3 subsequent siblings)
  12 siblings, 0 replies; 22+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-26  2:45 UTC (permalink / raw)
  To: barebox

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 common/efi-guid.c |  6 ++++++
 include/efi.h     | 18 ++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/common/efi-guid.c b/common/efi-guid.c
index 71aa21ddd..01b02bbb1 100644
--- a/common/efi-guid.c
+++ b/common/efi-guid.c
@@ -81,6 +81,12 @@ const char *efi_guid_string(efi_guid_t *g)
 	EFI_GUID_STRING( EFI_TCG_PROTOCOL_GUID, "TcgService", "TCGServices Protocol");
 	/* TPM 2.0 */
 	EFI_GUID_STRING( EFI_TCG2_PROTOCOL_GUID, "Tcg2Service", "TCG2Services Protocol");
+	EFI_GUID_STRING(EFI_VENDOR_KEYS_NV_VARIABLE_NAME_GUID, "VendorKeysNv", "Vendor Keys Non-Volatile");
+	EFI_GUID_STRING(EFI_AUTHENTICATED_VARIABLE_GUID, "AuthVar", "Authenticated Variable");
+	EFI_GUID_STRING(EFI_IMAGE_SECURITY_DATABASE_GUID, "ImageSecurityDB", "Image Security Database");
+	EFI_GUID_STRING(EFI_CERT_DB_GUID, "CertDB", "Certificate Database");
+	EFI_GUID_STRING(EFI_SECURE_BOOT_ENABLE_DISABLE_GUID, "SecureBootEnable", "Secure Boot Enable Disable");
+	EFI_GUID_STRING(EFI_CUSTOM_MODE_ENABLE_GUID, "CustomMode", "CustomMode");
 
 	/* File */
 	EFI_GUID_STRING(EFI_IDEBUSDXE_INF_GUID, "IdeBusDxe.inf", "EFI IdeBusDxe.inf File GUID");
diff --git a/include/efi.h b/include/efi.h
index e1fc134ee..31e7b283e 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -507,6 +507,24 @@ extern efi_runtime_services_t *RT;
 #define EFI_TCG2_PROTOCOL_GUID \
 	EFI_GUID(0x607f766c, 0x7455, 0x42be, 0x93, 0x0b, 0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f)
 
+#define EFI_VENDOR_KEYS_NV_VARIABLE_NAME_GUID \
+	EFI_GUID(0x9073e4e0, 0x60ec, 0x4b6e, 0x99, 0x3, 0x4c, 0x22, 0x3c, 0x26, 0x0f, 0x3c)
+
+#define EFI_AUTHENTICATED_VARIABLE_GUID \
+	EFI_GUID(0xaaf32c78, 0x947b, 0x439a, 0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3,0x77, 0x92)
+
+#define EFI_IMAGE_SECURITY_DATABASE_GUID \
+	EFI_GUID(0xd719b2cb, 0x3d3a, 0x4596, 0xa3, 0xbc, 0xda, 0xd0, 0xe, 0x67, 0x65, 0x6f)
+
+#define EFI_CERT_DB_GUID \
+	EFI_GUID(0xd9bee56e, 0x75dc, 0x49d9, 0xb4, 0xd7, 0xb5, 0x34, 0x21, 0xf, 0x63, 0x7a)
+
+#define EFI_SECURE_BOOT_ENABLE_DISABLE_GUID \
+	EFI_GUID(0xf0a30bc7, 0xaf08, 0x4556, 0x99, 0xc4, 0x00, 0x10, 0x9, 0xc9, 0x3a, 0x44)
+
+#define EFI_CUSTOM_MODE_ENABLE_GUID \
+	EFI_GUID(0xc076ec0c, 0x7028, 0x4399, 0xa0, 0x72, 0x71, 0xee, 0x5c, 0x44, 0x8b, 0x9f)
+
 extern efi_guid_t efi_file_info_id;
 extern efi_guid_t efi_simple_file_system_protocol_guid;
 extern efi_guid_t efi_device_path_protocol_guid;
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 11/13] efi: fix lds for secure boot support
  2017-03-26  2:44 ` [PATCH 01/13] bootm: move open to image_handler Jean-Christophe PLAGNIOL-VILLARD
                     ` (8 preceding siblings ...)
  2017-03-26  2:45   ` [PATCH 10/13] efi: add more security related guid for the efivars Jean-Christophe PLAGNIOL-VILLARD
@ 2017-03-26  2:45   ` Jean-Christophe PLAGNIOL-VILLARD
  2017-03-26  8:30     ` Michael Olbrich
  2017-03-26  2:45   ` [PATCH 12/13] efi: fix secure and setup mode report Jean-Christophe PLAGNIOL-VILLARD
                     ` (2 subsequent siblings)
  12 siblings, 1 reply; 22+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-26  2:45 UTC (permalink / raw)
  To: barebox

everythink need to be aligned to 4096

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 arch/x86/mach-efi/elf_ia32_efi.lds.S         | 10 +++++++---
 arch/x86/mach-efi/elf_x86_64_efi.lds.S       | 10 ++++++----
 arch/x86/mach-efi/include/mach/barebox.lds.h | 14 +++++++++++++-
 include/asm-generic/barebox.lds.h            |  8 +++++---
 4 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/arch/x86/mach-efi/elf_ia32_efi.lds.S b/arch/x86/mach-efi/elf_ia32_efi.lds.S
index 69f43f554..6d9cb973c 100644
--- a/arch/x86/mach-efi/elf_ia32_efi.lds.S
+++ b/arch/x86/mach-efi/elf_ia32_efi.lds.S
@@ -50,22 +50,23 @@ SECTIONS
 		*(COMMON)
 	}
 
-	. = ALIGN(64);
+	. = ALIGN(4096);
 
 	__barebox_initcalls_start = .;
 	__barebox_initcalls : { INITCALLS }
 	__barebox_initcalls_end = .;
+	. = ALIGN(4096);
 
 	__barebox_exitcalls_start = .;
 	__barebox_exitcalls : { EXITCALLS }
 	__barebox_exitcalls_end = .;
 
-	. = ALIGN(64);
+	. = ALIGN(4096);
 	__barebox_magicvar_start = .;
 	.barebox_magicvar : { BAREBOX_MAGICVARS }
 	__barebox_magicvar_end = .;
 
-	. = ALIGN(64);
+	. = ALIGN(4096);
 	__barebox_cmd_start = .;
 	__barebox_cmd : { BAREBOX_CMDS }
 	__barebox_cmd_end = .;
@@ -76,6 +77,9 @@ SECTIONS
 	.rel : {
 		*(.rel.data)
 		*(.rel.data.*)
+		*(.rela.barebox*)
+		*(.rela.initcall*)
+		*(.rela.exitcall*)
 		*(.rel.got)
 		*(.rel.stab)
 		*(.data.rel.ro.local)
diff --git a/arch/x86/mach-efi/elf_x86_64_efi.lds.S b/arch/x86/mach-efi/elf_x86_64_efi.lds.S
index 93d34d17a..8216d1d70 100644
--- a/arch/x86/mach-efi/elf_x86_64_efi.lds.S
+++ b/arch/x86/mach-efi/elf_x86_64_efi.lds.S
@@ -23,6 +23,7 @@ SECTIONS
 		*(.text)
 		*(.text.*)
 		*(.gnu.linkonce.t.*)
+		. = ALIGN(16);
 	}
 
 	_etext = .;
@@ -33,8 +34,8 @@ SECTIONS
 		*(.reloc)
 	}
 
-	. = ALIGN(4096);
 	_sdata = .;
+	. = ALIGN(4096);
 
 	.data : {
 		*(.rodata*)
@@ -52,22 +53,23 @@ SECTIONS
 		*(.rel.local)
 	}
 
-	. = ALIGN(64);
+	. = ALIGN(4096);
 
 	__barebox_initcalls_start = .;
 	__barebox_initcalls : { INITCALLS }
 	__barebox_initcalls_end = .;
+	. = ALIGN(4096);
 
 	__barebox_exitcalls_start = .;
 	__barebox_exitcalls : { EXITCALLS }
 	__barebox_exitcalls_end = .;
 
-	. = ALIGN(64);
+	. = ALIGN(4096);
 	__barebox_magicvar_start = .;
 	.barebox_magicvar : { BAREBOX_MAGICVARS }
 	__barebox_magicvar_end = .;
 
-	. = ALIGN(64);
+	. = ALIGN(4096);
 	__barebox_cmd_start = .;
 	__barebox_cmd : { BAREBOX_CMDS }
 	__barebox_cmd_end = .;
diff --git a/arch/x86/mach-efi/include/mach/barebox.lds.h b/arch/x86/mach-efi/include/mach/barebox.lds.h
index 40a8c178f..e7a3bb9cd 100644
--- a/arch/x86/mach-efi/include/mach/barebox.lds.h
+++ b/arch/x86/mach-efi/include/mach/barebox.lds.h
@@ -1 +1,13 @@
-/* empty */
+/*
+ * Copyright (C) 2017 Jean-Christophe PLAGNIOL-VILLARD <plagnio@jcrosoft.com>
+ *
+ * Under GPL v2
+ */
+
+#ifndef __EFI_MACH_BAREBOX_LDS_H__
+#define __EFI_MACH_BAREBOX_LDS_H__
+
+/* For secure boot we need all the section to be 4096 alligned */
+#define STRUCT_ALIGNMENT 4096
+
+#endif /* __EFI_MACH_BAREBOX_LDS_H__ */
diff --git a/include/asm-generic/barebox.lds.h b/include/asm-generic/barebox.lds.h
index c8a919b92..6c37751b3 100644
--- a/include/asm-generic/barebox.lds.h
+++ b/include/asm-generic/barebox.lds.h
@@ -3,15 +3,17 @@
  * Align to a 32 byte boundary equal to the
  * alignment gcc 4.5 uses for a struct
  */
-#define STRUCT_ALIGNMENT 32
-#define STRUCT_ALIGN() . = ALIGN(STRUCT_ALIGNMENT)
-
 #if defined CONFIG_X86 || \
 	defined CONFIG_ARCH_EP93XX || \
 	defined CONFIG_ARCH_ZYNQ
 #include <mach/barebox.lds.h>
 #endif
 
+#ifndef STRUCT_ALIGNMENT
+#define STRUCT_ALIGNMENT 32
+#endif
+#define STRUCT_ALIGN() . = ALIGN(STRUCT_ALIGNMENT)
+
 #ifndef PRE_IMAGE
 #define PRE_IMAGE
 #endif
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 12/13] efi: fix secure and setup mode report
  2017-03-26  2:44 ` [PATCH 01/13] bootm: move open to image_handler Jean-Christophe PLAGNIOL-VILLARD
                     ` (9 preceding siblings ...)
  2017-03-26  2:45   ` [PATCH 11/13] efi: fix lds for secure boot support Jean-Christophe PLAGNIOL-VILLARD
@ 2017-03-26  2:45   ` Jean-Christophe PLAGNIOL-VILLARD
  2017-03-26  2:45   ` [PATCH 13/13] efi: enable sercure boot support Jean-Christophe PLAGNIOL-VILLARD
  2017-03-26  7:57   ` [PATCH 01/13] bootm: move open to image_handler Michael Olbrich
  12 siblings, 0 replies; 22+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-26  2:45 UTC (permalink / raw)
  To: barebox

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 drivers/efi/efi-device.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/efi/efi-device.c b/drivers/efi/efi-device.c
index 6ed7f12b3..998bda7c6 100644
--- a/drivers/efi/efi-device.c
+++ b/drivers/efi/efi-device.c
@@ -365,7 +365,7 @@ static int efi_is_secure_boot(void)
 		free(val);
 	}
 
-	return ret != 1;
+	return ret != 0;
 }
 
 static int efi_is_setup_mode(void)
@@ -379,7 +379,7 @@ static int efi_is_setup_mode(void)
 		free(val);
 	}
 
-	return ret != 1;
+	return ret != 0;
 }
 
 static int efi_init_devices(void)
@@ -406,7 +406,7 @@ static int efi_init_devices(void)
 	dev_add_param_int_ro(efi_bus.dev, "fw_revision", efi_sys_table->fw_revision, "%u");
 	dev_add_param_int_ro(efi_bus.dev, "secure_boot", secure_boot, "%d");
 	dev_add_param_int_ro(efi_bus.dev, "secure_mode",
-			     secure_boot & setup_mode, "%u");
+			     secure_boot && !setup_mode, "%u");
 
 	efi_bus.dev->info = efi_businfo;
 
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 13/13] efi: enable sercure boot support
  2017-03-26  2:44 ` [PATCH 01/13] bootm: move open to image_handler Jean-Christophe PLAGNIOL-VILLARD
                     ` (10 preceding siblings ...)
  2017-03-26  2:45   ` [PATCH 12/13] efi: fix secure and setup mode report Jean-Christophe PLAGNIOL-VILLARD
@ 2017-03-26  2:45   ` Jean-Christophe PLAGNIOL-VILLARD
  2017-03-26  7:57   ` [PATCH 01/13] bootm: move open to image_handler Michael Olbrich
  12 siblings, 0 replies; 22+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-26  2:45 UTC (permalink / raw)
  To: barebox

This will ensure that we just start secured binary
without user confirmation

But for now on we only support EFI correctly signed image to start
Later will allow both.

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 arch/x86/Kconfig         |  1 +
 common/efi/efi-image.c   |  1 +
 drivers/efi/efi-device.c | 13 +++++++++++--
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 52ccf4894..65e4c8b7c 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -78,6 +78,7 @@ choice
 		select EFI_DEVICEPATH
 		select PRINTF_UUID
 		select CLOCKSOURCE_EFI_X86
+		select HAS_SECURE_BOOT
 
 	config X86_BIOS_BRINGUP
 		bool "16 bit BIOS"
diff --git a/common/efi/efi-image.c b/common/efi/efi-image.c
index 885348da4..6552d803d 100644
--- a/common/efi/efi-image.c
+++ b/common/efi/efi-image.c
@@ -270,6 +270,7 @@ static int do_bootm_efi(struct image_data *data)
 static struct image_handler efi_handle_tr = {
 	.name = "EFI Application",
 	.bootm = do_bootm_efi,
+	.is_secure_supported = 1,
 	.filetype = filetype_exe,
 };
 
diff --git a/drivers/efi/efi-device.c b/drivers/efi/efi-device.c
index 998bda7c6..0a6d7ca4e 100644
--- a/drivers/efi/efi-device.c
+++ b/drivers/efi/efi-device.c
@@ -26,6 +26,7 @@
 #include <linux/sizes.h>
 #include <wchar.h>
 #include <init.h>
+#include <boot_verify.h>
 #include <efi.h>
 #include <efi/efi.h>
 #include <efi/efi-device.h>
@@ -382,13 +383,20 @@ static int efi_is_setup_mode(void)
 	return ret != 0;
 }
 
+static int efi_is_secure_mode(void)
+{
+	int secure_boot = efi_is_secure_boot();
+	int setup_mode = efi_is_setup_mode();
+
+	return secure_boot && !setup_mode;
+}
+
 static int efi_init_devices(void)
 {
 	char *fw_vendor = NULL;
 	u16 sys_major = efi_sys_table->hdr.revision >> 16;
 	u16 sys_minor = efi_sys_table->hdr.revision & 0xffff;
 	int secure_boot = efi_is_secure_boot();
-	int setup_mode = efi_is_setup_mode();
 
 	fw_vendor = strdup_wchar_to_char((const wchar_t *)efi_sys_table->fw_vendor);
 
@@ -406,9 +414,10 @@ static int efi_init_devices(void)
 	dev_add_param_int_ro(efi_bus.dev, "fw_revision", efi_sys_table->fw_revision, "%u");
 	dev_add_param_int_ro(efi_bus.dev, "secure_boot", secure_boot, "%d");
 	dev_add_param_int_ro(efi_bus.dev, "secure_mode",
-			     secure_boot && !setup_mode, "%u");
+			     efi_is_secure_mode(), "%u");
 
 	efi_bus.dev->info = efi_businfo;
+	boot_set_is_secure_mode(efi_is_secure_mode);
 
 	efi_register_devices();
 
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 01/13] bootm: move open to image_handler
  2017-03-26  2:44 ` [PATCH 01/13] bootm: move open to image_handler Jean-Christophe PLAGNIOL-VILLARD
                     ` (11 preceding siblings ...)
  2017-03-26  2:45   ` [PATCH 13/13] efi: enable sercure boot support Jean-Christophe PLAGNIOL-VILLARD
@ 2017-03-26  7:57   ` Michael Olbrich
  12 siblings, 0 replies; 22+ messages in thread
From: Michael Olbrich @ 2017-03-26  7:57 UTC (permalink / raw)
  To: barebox

On Sun, Mar 26, 2017 at 04:44:52AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
>  arch/arm/lib/bootm.c               |  2 +
>  arch/blackfin/lib/blackfin_linux.c |  1 +
>  arch/nios2/lib/bootm.c             |  1 +
>  arch/ppc/lib/ppclinux.c            |  1 +
>  common/bootm.c                     | 79 ++++++++++++--------------------------
>  common/image-fit.c                 | 14 +++++++
>  common/misc.c                      |  1 +
>  common/uimage.c                    | 32 +++++++++++++++
>  include/bootm.h                    |  1 +
>  include/image-fit.h                |  1 +
>  include/image.h                    |  2 +
>  11 files changed, 80 insertions(+), 55 deletions(-)
> 
> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> index 8068a53be..204344f87 100644
> --- a/arch/arm/lib/bootm.c
> +++ b/arch/arm/lib/bootm.c
> @@ -217,6 +217,7 @@ static int do_bootm_linux(struct image_data *data)
>  
>  static struct image_handler uimage_handler = {
>  	.name = "ARM Linux uImage",
> +	.open = uimage_bootm_open,
>  	.bootm = do_bootm_linux,
>  	.filetype = filetype_uimage,
>  	.ih_os = IH_OS_LINUX,
> @@ -579,6 +580,7 @@ BAREBOX_MAGICVAR(aimage_noverwrite_tags, "Disable overwrite of the tags addr wit
>  
>  static struct image_handler arm_fit_handler = {
>          .name = "FIT image",
> +	.open = fit_bootm_open,
>          .bootm = do_bootm_linux,
>          .filetype = filetype_oftree,
>  };
> diff --git a/arch/blackfin/lib/blackfin_linux.c b/arch/blackfin/lib/blackfin_linux.c
> index 5ebd284d1..27002eadb 100644
> --- a/arch/blackfin/lib/blackfin_linux.c
> +++ b/arch/blackfin/lib/blackfin_linux.c
> @@ -68,6 +68,7 @@ static int do_bootm_linux(struct image_data *idata)
>  
>  static struct image_handler handler = {
>  	.name = "Blackfin Linux",
> +	.open = uimage_bootm_open,
>  	.bootm = do_bootm_linux,
>  	.filetype = filetype_uimage,
>  	.ih_os = IH_OS_LINUX,
> diff --git a/arch/nios2/lib/bootm.c b/arch/nios2/lib/bootm.c
> index 34908bde3..f1b3c2624 100644
> --- a/arch/nios2/lib/bootm.c
> +++ b/arch/nios2/lib/bootm.c
> @@ -69,6 +69,7 @@ static int do_bootm_linux(struct image_data *idata)
>  
>  static struct image_handler handler = {
>  	.name = "NIOS2 Linux",
> +	.open = uimage_bootm_open,
>  	.bootm = do_bootm_linux,
>  	.filetype = filetype_uimage,
>  	.ih_os = IH_OS_LINUX,
> diff --git a/arch/ppc/lib/ppclinux.c b/arch/ppc/lib/ppclinux.c
> index 3fca6b272..c882938fa 100644
> --- a/arch/ppc/lib/ppclinux.c
> +++ b/arch/ppc/lib/ppclinux.c
> @@ -100,6 +100,7 @@ error:
>  
>  static struct image_handler handler = {
>  	.name = "PowerPC Linux",
> +	.open = uimage_bootm_open,
>  	.bootm = do_bootm_linux,
>  	.filetype = filetype_uimage,
>  	.ih_os = IH_OS_LINUX,
> diff --git a/common/bootm.c b/common/bootm.c
> index 81625d915..64c933b3c 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -34,7 +34,7 @@ int register_image_handler(struct image_handler *handler)
>  }
>  
>  static struct image_handler *bootm_find_handler(enum filetype filetype,
> -		struct image_data *data)
> +		struct image_data *data, int enforce_os)
>  {
>  	struct image_handler *handler;
>  
> @@ -42,9 +42,16 @@ static struct image_handler *bootm_find_handler(enum filetype filetype,
>  		if (filetype != filetype_uimage &&
>  				handler->filetype == filetype)
>  			return handler;
> -		if  (filetype == filetype_uimage &&
> -				handler->ih_os == data->os->header.ih_os)
> -			return handler;
> +		if (filetype == filetype_uimage) {
> +			/*
> +			 * we can take the first one as open is the same
> +			 * not matter the OS
> +			 */
> +			if (enforce_os && handler->ih_os == data->os->header.ih_os)
> +				return handler;
> +			else
> +				return handler;
> +		}
>  	}
>  
>  	return NULL;
> @@ -441,38 +448,6 @@ int bootm_get_os_size(struct image_data *data)
>  	return -EINVAL;
>  }
>  
> -static int bootm_open_os_uimage(struct image_data *data)
> -{
> -	int ret;
> -
> -	data->os = uimage_open(data->os_file);
> -	if (!data->os)
> -		return -EINVAL;
> -
> -	if (bootm_get_verify_mode() > BOOTM_VERIFY_NONE) {
> -		ret = uimage_verify(data->os);
> -		if (ret) {
> -			printf("Checking data crc failed with %s\n",
> -					strerror(-ret));
> -			uimage_close(data->os);
> -			return ret;
> -		}
> -	}
> -
> -	uimage_print_contents(data->os);
> -
> -	if (data->os->header.ih_arch != IH_ARCH) {
> -		printf("Unsupported Architecture 0x%x\n",
> -		       data->os->header.ih_arch);
> -		return -EINVAL;
> -	}
> -
> -	if (data->os_address == UIMAGE_SOME_ADDRESS)
> -		data->os_address = data->os->header.ih_load;
> -
> -	return 0;
> -}
> -
>  static void bootm_print_info(struct image_data *data)
>  {
>  	if (data->os_res)
> @@ -548,6 +523,14 @@ int bootm_boot(struct bootm_data *bootm_data)
>  		goto err_out;
>  	}
>  
> +	handler = bootm_find_handler(os_type, data, 0);
> +	if (!handler) {
> +		printf("no image handler found for image type %s\n",
> +			file_type_to_string(os_type));
> +		ret = -ENODEV;
> +		goto err_out;
> +	}

I get that the handler needs to be found here for the open callback, but
what's 'enforce_os' for?
We need a handler that can start the image later anyways, so that's the
use-case for this?

> +
>  	if (IS_ENABLED(CONFIG_BOOTM_FORCE_SIGNED_IMAGES)) {
>  		data->verify = BOOTM_VERIFY_SIGNATURE;
>  
> @@ -565,25 +548,11 @@ int bootm_boot(struct bootm_data *bootm_data)
>  		}
>  	}
>  
> -	if (IS_ENABLED(CONFIG_FITIMAGE) && os_type == filetype_oftree) {
> -		struct fit_handle *fit;
> -
> -		fit = fit_open(data->os_file, data->os_part, data->verbose, data->verify);
> -		if (IS_ERR(fit)) {
> -			printf("Loading FIT image %s failed with: %s\n", data->os_file,
> -			       strerrorp(fit));
> -			ret = PTR_ERR(fit);
> -			goto err_out;
> -		}
> -
> -		data->os_fit = fit;
> -	}
> -
> -	if (os_type == filetype_uimage) {
> -		ret = bootm_open_os_uimage(data);
> +	if (handler->open) {
> +		ret = handler->open(data);
>  		if (ret) {
> -			printf("Loading OS image failed with: %s\n",
> -					strerror(-ret));
> +			printf("Loading OS image %s failed with: %s\n",
> +					handler->name, strerror(-ret));
>  			goto err_out;
>  		}
>  	}
> @@ -610,7 +579,7 @@ int bootm_boot(struct bootm_data *bootm_data)
>  	if (data->os_address == UIMAGE_SOME_ADDRESS)
>  		data->os_address = UIMAGE_INVALID_ADDRESS;
>  
> -	handler = bootm_find_handler(os_type, data);
> +	handler = bootm_find_handler(os_type, data, 1);
>  	if (!handler) {
>  		printf("no image handler found for image type %s\n",
>  			file_type_to_string(os_type));
> diff --git a/common/image-fit.c b/common/image-fit.c
> index 6a01c614c..5c014d66b 100644
> --- a/common/image-fit.c
> +++ b/common/image-fit.c
> @@ -583,6 +583,19 @@ struct fit_handle *fit_open(const char *filename, const char *config, bool verbo
>  	return ERR_PTR(ret);
>  }
>  
> +int fit_bootm_open(struct image_data *data)
> +{
> +	struct fit_handle *fit;
> +
> +	fit = fit_open(data->os_file, data->os_part, data->verbose, data->verify);
> +	if (IS_ERR(fit))
> +		return PTR_ERR(fit);
> +
> +	data->os_fit = fit;
> +
> +	return 0;
> +}
> +
>  void fit_close(struct fit_handle *handle)
>  {
>  	if (handle->root)
> @@ -604,6 +617,7 @@ static int do_bootm_sandbox_fit(struct image_data *data)
>  
>  static struct image_handler sandbox_fit_handler = {
>  	.name = "FIT image",
> +	.open = fit_bootm_open,
>  	.bootm = do_bootm_sandbox_fit,
>  	.filetype = filetype_oftree,
>  };
> diff --git a/common/misc.c b/common/misc.c
> index f0f0b808b..60acbd009 100644
> --- a/common/misc.c
> +++ b/common/misc.c
> @@ -101,6 +101,7 @@ const char *strerror(int errnum)
>  	case	EISNAM		: str = "Is a named type file"; break;
>  	case	EREMOTEIO	: str = "Remote I/O error"; break;
>  #endif
> +	case	ESECVIOLATION	: str = "Security Violation"; break;

ESECVIOLATION is introduced in the next patch, so this hunk should be in
that patch.

Regards,
Michael

>  	default:
>  		sprintf(errno_string, "error %d", errnum);
>  		return errno_string;
> diff --git a/common/uimage.c b/common/uimage.c
> index 28a25bba2..72c868882 100644
> --- a/common/uimage.c
> +++ b/common/uimage.c
> @@ -527,3 +527,35 @@ out:
>  
>  	return buf;
>  }
> +
> +int uimage_bootm_open(struct image_data *data)
> +{
> +	int ret;
> +
> +	data->os = uimage_open(data->os_file);
> +	if (!data->os)
> +		return -EINVAL;
> +
> +	if (bootm_get_verify_mode() > BOOTM_VERIFY_NONE) {
> +		ret = uimage_verify(data->os);
> +		if (ret) {
> +			printf("Checking data crc failed with %s\n",
> +					strerror(-ret));
> +			uimage_close(data->os);
> +			return ret;
> +		}
> +	}
> +
> +	uimage_print_contents(data->os);
> +
> +	if (data->os->header.ih_arch != IH_ARCH) {
> +		printf("Unsupported Architecture 0x%x\n",
> +		       data->os->header.ih_arch);
> +		return -EINVAL;
> +	}
> +
> +	if (data->os_address == UIMAGE_SOME_ADDRESS)
> +		data->os_address = data->os->header.ih_load;
> +
> +	return 0;
> +}
> diff --git a/include/bootm.h b/include/bootm.h
> index 6e9777a9a..1c7a145c6 100644
> --- a/include/bootm.h
> +++ b/include/bootm.h
> @@ -92,6 +92,7 @@ struct image_handler {
>  
>  	enum filetype filetype;
>  	int (*bootm)(struct image_data *data);
> +	int (*open)(struct image_data *data);
>  };
>  
>  int register_image_handler(struct image_handler *handle);
> diff --git a/include/image-fit.h b/include/image-fit.h
> index c49f95826..e817ebfae 100644
> --- a/include/image-fit.h
> +++ b/include/image-fit.h
> @@ -38,6 +38,7 @@ struct fit_handle {
>  	unsigned long initrd_size;
>  };
>  
> +int fit_bootm_open(struct image_data *data);
>  struct fit_handle *fit_open(const char *filename, const char *config, bool verbose,
>  			    enum bootm_verify verify);
>  void fit_close(struct fit_handle *handle);
> diff --git a/include/image.h b/include/image.h
> index 3e75d49b8..9ed265658 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -229,6 +229,8 @@ struct uimage_handle_data {
>  	ulong len;
>  };
>  
> +struct image_data;
> +int uimage_bootm_open(struct image_data *data);
>  struct uimage_handle *uimage_open(const char *filename);
>  void uimage_close(struct uimage_handle *handle);
>  int uimage_verify(struct uimage_handle *handle);
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 02/13] boot_verify: use a new error ESECVIOLATION
  2017-03-26  2:44   ` [PATCH 02/13] boot_verify: use a new error ESECVIOLATION Jean-Christophe PLAGNIOL-VILLARD
@ 2017-03-26  7:59     ` Michael Olbrich
  0 siblings, 0 replies; 22+ messages in thread
From: Michael Olbrich @ 2017-03-26  7:59 UTC (permalink / raw)
  To: barebox

On Sun, Mar 26, 2017 at 04:44:53AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> so we can indentify it correctly
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
>  common/bootm.c              |  7 ++++---
>  common/efi/efi.c            |  2 +-
>  common/image-fit.c          | 12 ++++++------
>  include/asm-generic/errno.h |  1 +
>  4 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/common/bootm.c b/common/bootm.c
> index 64c933b3c..53311ab1c 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -541,9 +541,10 @@ int bootm_boot(struct bootm_data *bootm_data)
>  		data->oftree = NULL;
>  		data->oftree_file = NULL;
>  		data->initrd_file = NULL;
> -		if (os_type != filetype_oftree) {
> -			printf("Signed boot and image is no FIT image, aborting\n");
> -			ret = -EINVAL;
> +		if (!handler->is_secure_supported) {

is_secure_supported is introduced on the next patch, so this hunk belongs
into that patch.

Regards,
Michael

> +			printf("Signed boot and image %s does not support it",
> +				handler->name);
> +			ret = -ESECVIOLATION;
>  			goto err_out;
>  		}
>  	}
> diff --git a/common/efi/efi.c b/common/efi/efi.c
> index 05c58250f..19ee96411 100644
> --- a/common/efi/efi.c
> +++ b/common/efi/efi.c
> @@ -244,7 +244,7 @@ int efi_errno(efi_status_t err)
>  	case EFI_TFTP_ERROR: ret = EINVAL; break;
>  	case EFI_PROTOCOL_ERROR: ret = EPROTO; break;
>  	case EFI_INCOMPATIBLE_VERSION: ret = EINVAL; break;
> -	case EFI_SECURITY_VIOLATION: ret = EINVAL; break;
> +	case EFI_SECURITY_VIOLATION: ret = ESECVIOLATION; break;
>  	case EFI_CRC_ERROR: ret = EINVAL; break;
>  	case EFI_END_OF_MEDIA: ret = EINVAL; break;
>  	case EFI_END_OF_FILE: ret = EINVAL; break;
> diff --git a/common/image-fit.c b/common/image-fit.c
> index 5c014d66b..5750199c3 100644
> --- a/common/image-fit.c
> +++ b/common/image-fit.c
> @@ -353,23 +353,23 @@ static int fit_verify_hash(struct device_node *hash, const void *data, int data_
>  	value_read = of_get_property(hash, "value", &hash_len);
>  	if (!value_read) {
>  		pr_err("%s: \"value\" property not found\n", hash->full_name);
> -		return -EINVAL;
> +		return -ESECVIOLATION;
>  	}
>  
>  	if (of_property_read_string(hash, "algo", &algo)) {
>  		pr_err("%s: \"algo\" property not found\n", hash->full_name);
> -		return -EINVAL;
> +		return -ESECVIOLATION;
>  	}
>  
>  	d = digest_alloc(algo);
>  	if (!d) {
>  		pr_err("%s: unsupported algo %s\n", hash->full_name, algo);
> -		return -EINVAL;
> +		return -ESECVIOLATION;
>  	}
>  
>  	if (hash_len != digest_length(d)) {
>  		pr_err("%s: invalid hash length %d\n", hash->full_name, hash_len);
> -		ret = -EINVAL;
> +		ret = -ESECVIOLATION;
>  		goto err_digest_free;
>  	}
>  
> @@ -381,7 +381,7 @@ static int fit_verify_hash(struct device_node *hash, const void *data, int data_
>  
>  	if (memcmp(value_read, value_calc, hash_len)) {
>  		pr_info("%s: hash BAD\n", hash->full_name);
> -		ret =  -EBADMSG;
> +		ret =  -ESECVIOLATION;
>  	} else {
>  		pr_info("%s: hash OK\n", hash->full_name);
>  		ret = 0;
> @@ -431,7 +431,7 @@ static int fit_open_image(struct fit_handle *handle, const char *unit, const voi
>  		if (handle->verify == BOOTM_VERIFY_AVAILABLE)
>  			ret = 0;
>  		else
> -			ret = -EINVAL;
> +			ret = -ESECVIOLATION;
>  		for_each_child_of_node(image, hash) {
>  			if (handle->verbose)
>  				of_print_nodes(hash, 0);
> diff --git a/include/asm-generic/errno.h b/include/asm-generic/errno.h
> index 7d99a9537..45b2a2065 100644
> --- a/include/asm-generic/errno.h
> +++ b/include/asm-generic/errno.h
> @@ -133,6 +133,7 @@
>  #define	EKEYREJECTED	129	/* Key was rejected by service */
>  
>  /* Should never be seen by user programs */
> +#define ESECVIOLATION	511
>  #define ERESTARTSYS	512
>  #define ERESTARTNOINTR	513
>  #define ERESTARTNOHAND	514	/* restart if no handler.. */
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 06/13] boot_verify: make it modifiable at start time
  2017-03-26  2:44   ` [PATCH 06/13] boot_verify: make it modifiable at start time Jean-Christophe PLAGNIOL-VILLARD
@ 2017-03-26  8:16     ` Michael Olbrich
  0 siblings, 0 replies; 22+ messages in thread
From: Michael Olbrich @ 2017-03-26  8:16 UTC (permalink / raw)
  To: barebox

On Sun, Mar 26, 2017 at 04:44:57AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
>  commands/bootm.c      |  2 +-
>  common/boot_verify.c  | 39 +++++++++++++++++++++++++++++++++------
>  common/bootm.c        |  2 +-
>  include/boot_verify.h | 15 ++++++++++++---
>  4 files changed, 47 insertions(+), 11 deletions(-)
> 
> diff --git a/commands/bootm.c b/commands/bootm.c
> index b35aaa914..cb520a1ba 100644
> --- a/commands/bootm.c
> +++ b/commands/bootm.c
> @@ -64,7 +64,7 @@ static int do_bootm(int argc, char *argv[])
>  	while ((opt = getopt(argc, argv, BOOTM_OPTS)) > 0) {
>  		switch(opt) {
>  		case 'c':
> -			if (data.verify < BOOT_VERIFY_HASH)
> +			if (data.verify > BOOT_VERIFY_HASH)

This is very confusing without a comment. It took me a while to figure out
that this does not actually change anything.
I think you could change the order in the array without modifying the enum.
Or at least comment on it in the commit message.

Regards,
Michael

>  				data.verify = BOOT_VERIFY_HASH;
>  			break;
>  		case 's':
> diff --git a/common/boot_verify.c b/common/boot_verify.c
> index afe929e68..9cbeb7a65 100644
> --- a/common/boot_verify.c
> +++ b/common/boot_verify.c
> @@ -11,22 +11,49 @@ enum boot_verify boot_get_verify_mode(void)
>  	return boot_verify_mode;
>  }
>  
> +/* keep it for the most secure to the less */
>  static const char * const boot_verify_names[] = {
> -#ifndef CONFIG_BOOT_FORCE_SIGNED_IMAGES
> -	[BOOT_VERIFY_NONE] = "none",
> -	[BOOT_VERIFY_HASH] = "hash",
> -	[BOOT_VERIFY_AVAILABLE] = "available",
> -#endif
>  	[BOOT_VERIFY_SIGNATURE] = "signature",
> +	[BOOT_VERIFY_AVAILABLE] = "available",
> +	[BOOT_VERIFY_HASH] = "hash",
> +	[BOOT_VERIFY_NONE] = "none",
>  };
>  
> +/* allow architecture to overwrite it such as EFI */
> +static int default_is_secure_mode(void)
> +{
> +	if (IS_ENABLED(CONFIG_BOOT_FORCE_SIGNED_IMAGES))
> +		return 1;
> +
> +	return 0;
> +}
> +
> +static int (*__is_secure_mode)(void) = default_is_secure_mode;
> +
> +int is_secure_mode(void)
> +{
> +	return __is_secure_mode();
> +}
> +
> +void boot_set_is_secure_mode(int (*fn)(void))
> +{
> +	__is_secure_mode = fn;
> +}
> +
>  static int init_boot_verify(void)
>  {
> +	int size;
> +
>  	if (IS_ENABLED(CONFIG_BOOT_FORCE_SIGNED_IMAGES))
>  		boot_verify_mode = BOOT_VERIFY_SIGNATURE;
>  
> +	if (is_secure_mode())
> +		size = 1;
> +	else
> +		size = ARRAY_SIZE(boot_verify_names);
> +
>  	globalvar_add_simple_enum("boot.verify", (unsigned int *)&boot_verify_mode,
> -				  boot_verify_names, ARRAY_SIZE(boot_verify_names));
> +				  boot_verify_names, size);
>  
>  	return 0;
>  }
> diff --git a/common/bootm.c b/common/bootm.c
> index 74202a829..1558f3c5d 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -159,7 +159,7 @@ static int bootm_open_initrd_uimage(struct image_data *data)
>  		if (!data->initrd)
>  			return -EINVAL;
>  
> -		if (boot_get_verify_mode() > BOOT_VERIFY_NONE) {
> +		if (boot_get_verify_mode() != BOOT_VERIFY_NONE) {
>  			ret = uimage_verify(data->initrd);
>  			if (ret) {
>  				printf("Checking data crc failed with %s\n",
> diff --git a/include/boot_verify.h b/include/boot_verify.h
> index 3a4436584..ee830bf5c 100644
> --- a/include/boot_verify.h
> +++ b/include/boot_verify.h
> @@ -2,10 +2,10 @@
>  #define __BOOT_VERIFY_H__
>  
>  enum boot_verify {
> -	BOOT_VERIFY_NONE,
> -	BOOT_VERIFY_HASH,
> -	BOOT_VERIFY_AVAILABLE,
>  	BOOT_VERIFY_SIGNATURE,
> +	BOOT_VERIFY_AVAILABLE,
> +	BOOT_VERIFY_HASH,
> +	BOOT_VERIFY_NONE,
>  };
>  
>  #ifndef CONFIG_BOOT_VERIFY
> @@ -13,8 +13,17 @@ static inline enum boot_verify boot_get_verify_mode(void)
>  {
>  	return BOOT_VERIFY_NONE;
>  }
> +
> +static int inline is_secure_mode(void)
> +{
> +	return 0;
> +}
> +
> +static void inline boot_set_is_secure_mode(int (*fn)(void)) {}
>  #else
>  enum boot_verify boot_get_verify_mode(void);
> +int is_secure_mode(void);
> +void boot_set_is_secure_mode(int (*fn)(void));
>  #endif
>  
>  #endif /* __BOOT_VERIFY_H__ */
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 07/13] go: only use it if boot signature is not required
  2017-03-26  2:44   ` [PATCH 07/13] go: only use it if boot signature is not required Jean-Christophe PLAGNIOL-VILLARD
@ 2017-03-26  8:23     ` Michael Olbrich
  2017-03-27 11:50       ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 1 reply; 22+ messages in thread
From: Michael Olbrich @ 2017-03-26  8:23 UTC (permalink / raw)
  To: barebox

On Sun, Mar 26, 2017 at 04:44:58AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

Does this realy help? If someone has access to the barebox shell, then
there are many ways to overwrite the secure boot check.

Michael

> ---
>  commands/go.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/commands/go.c b/commands/go.c
> index fb319b320..e0385a977 100644
> --- a/commands/go.c
> +++ b/commands/go.c
> @@ -26,6 +26,7 @@
>  #include <fcntl.h>
>  #include <linux/ctype.h>
>  #include <errno.h>
> +#include <boot_verify.h>
>  
>  static int do_go(int argc, char *argv[])
>  {
> @@ -37,6 +38,9 @@ static int do_go(int argc, char *argv[])
>  	if (argc < 2)
>  		return COMMAND_ERROR_USAGE;
>  
> +	if (boot_get_verify_mode() < BOOT_VERIFY_AVAILABLE)
> +		return -ESECVIOLATION;
> +
>  	if (!isdigit(*argv[1])) {
>  		fd = open(argv[1], O_RDONLY);
>  		if (fd < 0) {
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 08/13] boot_verify: allow to force unsigned image to boot
  2017-03-26  2:44   ` [PATCH 08/13] boot_verify: allow to force unsigned image to boot Jean-Christophe PLAGNIOL-VILLARD
@ 2017-03-26  8:25     ` Michael Olbrich
  0 siblings, 0 replies; 22+ messages in thread
From: Michael Olbrich @ 2017-03-26  8:25 UTC (permalink / raw)
  To: barebox

On Sun, Mar 26, 2017 at 04:44:59AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> request confirmation before booting an unsigned image
> 
> with a default timeout
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
>  commands/go.c         |  9 +++++++--
>  common/Kconfig        |  8 ++++++++
>  common/boot_verify.c  | 47 ++++++++++++++++++++++++++++++++++++++++++++---
>  common/bootm.c        |  6 ++++++
>  common/image-fit.c    |  1 +
>  common/uimage.c       |  1 +
>  include/boot_verify.h |  7 +++++++
>  7 files changed, 74 insertions(+), 5 deletions(-)
> 
> diff --git a/commands/go.c b/commands/go.c
> index e0385a977..919bcddc7 100644
> --- a/commands/go.c
> +++ b/commands/go.c
> @@ -38,8 +38,13 @@ static int do_go(int argc, char *argv[])
>  	if (argc < 2)
>  		return COMMAND_ERROR_USAGE;
>  
> -	if (boot_get_verify_mode() < BOOT_VERIFY_AVAILABLE)
> -		return -ESECVIOLATION;
> +	if (boot_get_verify_mode() < BOOT_VERIFY_AVAILABLE) {
> +		int is_sec;
> +
> +		is_sec = boot_can_start_unsigned();
> +		if (is_sec)
> +			return is_sec;
> +	}
>  
>  	if (!isdigit(*argv[1])) {
>  		fd = open(argv[1], O_RDONLY);
> diff --git a/common/Kconfig b/common/Kconfig
> index 00e98e859..2588651ae 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -648,6 +648,14 @@ config BOOT_FORCE_SIGNED_IMAGES
>  	  are refused to boot. Effectively this means only Signed images can
>  	  be booted.
>  
> +config BOOT_FORCE_USER_SIGNED_IMAGES
> +	bool
> +	prompt "Force booting of signed images or confirm them"
> +	depends on HAS_SECURE_BOOT
> +	help
> +	  With this option enabled only signed images can be booted, unsigned images
> +	  need a user confirmation to boot.
> +
>  config BLSPEC
>  	depends on BLOCK
>  	depends on FLEXIBLE_BOOTARGS
> diff --git a/common/boot_verify.c b/common/boot_verify.c
> index 9cbeb7a65..07ae07e16 100644
> --- a/common/boot_verify.c
> +++ b/common/boot_verify.c
> @@ -1,9 +1,17 @@
> +/*
> + * Copyright (c) 2016 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
> + * Copyright (c) 2017 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> + *
> + * Under GPLv2 Only
> + */

This belongs in the patch that introduces the file.

Michael

>  #include <common.h>
>  #include <boot_verify.h>
> +#include <console_countdown.h>
>  #include <globalvar.h>
>  #include <magicvar.h>
>  #include <init.h>
>  
> +static unsigned int boot_verify_confirm_timeout = 10;
>  static enum boot_verify boot_verify_mode = BOOT_VERIFY_HASH;
>  
>  enum boot_verify boot_get_verify_mode(void)
> @@ -14,6 +22,7 @@ enum boot_verify boot_get_verify_mode(void)
>  /* keep it for the most secure to the less */
>  static const char * const boot_verify_names[] = {
>  	[BOOT_VERIFY_SIGNATURE] = "signature",
> +	[BOOT_VERIFY_SIGNATURE_USER] = "signature-user",
>  	[BOOT_VERIFY_AVAILABLE] = "available",
>  	[BOOT_VERIFY_HASH] = "hash",
>  	[BOOT_VERIFY_NONE] = "none",
> @@ -40,6 +49,29 @@ void boot_set_is_secure_mode(int (*fn)(void))
>  	__is_secure_mode = fn;
>  }
>  
> +int boot_can_start_unsigned(void)
> +{
> +	int ret;
> +	char c;
> +	int timeout = boot_verify_confirm_timeout;
> +
> +	if (!is_secure_mode())
> +		return 0;
> +
> +	if (boot_verify_mode != BOOT_VERIFY_SIGNATURE_USER)
> +		return -ESECVIOLATION;
> +
> +	printf("Are you sure you wish to run an unsigned binary\n");
> +	printf("in a secure environment?\n");
> +	printf("press y to confirm\n");
> +
> +	ret = console_countdown(timeout, CONSOLE_COUNTDOWN_ANYKEY, &c);
> +	if (ret != -EINTR)
> +		return -ESECVIOLATION;
> +
> +	return c == 'y' ? 0 : -ESECVIOLATION;
> +}
> +
>  static int init_boot_verify(void)
>  {
>  	int size;
> @@ -47,16 +79,25 @@ static int init_boot_verify(void)
>  	if (IS_ENABLED(CONFIG_BOOT_FORCE_SIGNED_IMAGES))
>  		boot_verify_mode = BOOT_VERIFY_SIGNATURE;
>  
> -	if (is_secure_mode())
> -		size = 1;
> -	else
> +	if (is_secure_mode()) {
> +		if (IS_ENABLED(CONFIG_BOOT_FORCE_USER_SIGNED_IMAGES))
> +			size = 2;
> +		else
> +			size = 1;
> +	} else {
>  		size = ARRAY_SIZE(boot_verify_names);
> +	}
>  
>  	globalvar_add_simple_enum("boot.verify", (unsigned int *)&boot_verify_mode,
>  				  boot_verify_names, size);
>  
> +	globalvar_add_simple_int("boot.verify_confirm_timeout",
> +				 &boot_verify_confirm_timeout, "%u");
> +
>  	return 0;
>  }
>  late_initcall(init_boot_verify);
>  
>  BAREBOX_MAGICVAR_NAMED(global_boot_verify, global.boot.verify, "boot default verify level");
> +BAREBOX_MAGICVAR_NAMED(global_boot_verify_confirm_timeout, global.boot.verify_confirm_timeout,
> +		"Secure Boot Comfirm timeout in seconds before booting an unsigned image");
> diff --git a/common/bootm.c b/common/bootm.c
> index 1558f3c5d..73a3a99dd 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -579,6 +579,12 @@ int bootm_boot(struct bootm_data *bootm_data)
>  		printf("Passing control to %s handler\n", handler->name);
>  	}
>  
> +	if (!handler->is_secure_supported && is_secure_mode()) {
> +		ret = boot_can_start_unsigned();
> +		if (ret)
> +			goto err_out;
> +	}
> +
>  	ret = handler->bootm(data);
>  	if (data->dryrun)
>  		printf("Dryrun. Aborted\n");
> diff --git a/common/image-fit.c b/common/image-fit.c
> index 53f3173fc..0df735062 100644
> --- a/common/image-fit.c
> +++ b/common/image-fit.c
> @@ -465,6 +465,7 @@ static int fit_config_verify_signature(struct fit_handle *handle, struct device_
>  	case BOOT_VERIFY_HASH:
>  		return 0;
>  	case BOOT_VERIFY_SIGNATURE:
> +	case BOOT_VERIFY_SIGNATURE_USER:
>  		ret = -EINVAL;
>  		break;
>  	case BOOT_VERIFY_AVAILABLE:
> diff --git a/common/uimage.c b/common/uimage.c
> index d1947aa11..f25341c15 100644
> --- a/common/uimage.c
> +++ b/common/uimage.c
> @@ -30,6 +30,7 @@
>  #include <rtc.h>
>  #include <filetype.h>
>  #include <memory.h>
> +#include <bootm.h>
>  
>  static inline int uimage_is_multi_image(struct uimage_handle *handle)
>  {
> diff --git a/include/boot_verify.h b/include/boot_verify.h
> index ee830bf5c..12dcfbfdc 100644
> --- a/include/boot_verify.h
> +++ b/include/boot_verify.h
> @@ -3,6 +3,7 @@
>  
>  enum boot_verify {
>  	BOOT_VERIFY_SIGNATURE,
> +	BOOT_VERIFY_SIGNATURE_USER,
>  	BOOT_VERIFY_AVAILABLE,
>  	BOOT_VERIFY_HASH,
>  	BOOT_VERIFY_NONE,
> @@ -19,10 +20,16 @@ static int inline is_secure_mode(void)
>  	return 0;
>  }
>  
> +static int inline boot_can_start_unsigned(void)
> +{
> +	return 0;
> +}
> +
>  static void inline boot_set_is_secure_mode(int (*fn)(void)) {}
>  #else
>  enum boot_verify boot_get_verify_mode(void);
>  int is_secure_mode(void);
> +int boot_can_start_unsigned(void);
>  void boot_set_is_secure_mode(int (*fn)(void));
>  #endif
>  
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 11/13] efi: fix lds for secure boot support
  2017-03-26  2:45   ` [PATCH 11/13] efi: fix lds for secure boot support Jean-Christophe PLAGNIOL-VILLARD
@ 2017-03-26  8:30     ` Michael Olbrich
  0 siblings, 0 replies; 22+ messages in thread
From: Michael Olbrich @ 2017-03-26  8:30 UTC (permalink / raw)
  To: barebox

On Sun, Mar 26, 2017 at 04:45:02AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> everythink need to be aligned to 4096

As Lucas noted the last time, this deserves a comment why this is needed.
Just a reference to the EFI spec (I think?) or something like that.

> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
>  arch/x86/mach-efi/elf_ia32_efi.lds.S         | 10 +++++++---
>  arch/x86/mach-efi/elf_x86_64_efi.lds.S       | 10 ++++++----
>  arch/x86/mach-efi/include/mach/barebox.lds.h | 14 +++++++++++++-
>  include/asm-generic/barebox.lds.h            |  8 +++++---
>  4 files changed, 31 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/x86/mach-efi/elf_ia32_efi.lds.S b/arch/x86/mach-efi/elf_ia32_efi.lds.S
> index 69f43f554..6d9cb973c 100644
> --- a/arch/x86/mach-efi/elf_ia32_efi.lds.S
> +++ b/arch/x86/mach-efi/elf_ia32_efi.lds.S
> @@ -50,22 +50,23 @@ SECTIONS
>  		*(COMMON)
>  	}
>  
> -	. = ALIGN(64);
> +	. = ALIGN(4096);
>  
>  	__barebox_initcalls_start = .;
>  	__barebox_initcalls : { INITCALLS }
>  	__barebox_initcalls_end = .;
> +	. = ALIGN(4096);
>  
>  	__barebox_exitcalls_start = .;
>  	__barebox_exitcalls : { EXITCALLS }
>  	__barebox_exitcalls_end = .;
>  
> -	. = ALIGN(64);
> +	. = ALIGN(4096);
>  	__barebox_magicvar_start = .;
>  	.barebox_magicvar : { BAREBOX_MAGICVARS }
>  	__barebox_magicvar_end = .;
>  
> -	. = ALIGN(64);
> +	. = ALIGN(4096);
>  	__barebox_cmd_start = .;
>  	__barebox_cmd : { BAREBOX_CMDS }
>  	__barebox_cmd_end = .;
> @@ -76,6 +77,9 @@ SECTIONS
>  	.rel : {
>  		*(.rel.data)
>  		*(.rel.data.*)
> +		*(.rela.barebox*)
> +		*(.rela.initcall*)
> +		*(.rela.exitcall*)

This is unrelated and should be a different patch.

>  		*(.rel.got)
>  		*(.rel.stab)
>  		*(.data.rel.ro.local)
> diff --git a/arch/x86/mach-efi/elf_x86_64_efi.lds.S b/arch/x86/mach-efi/elf_x86_64_efi.lds.S
> index 93d34d17a..8216d1d70 100644
> --- a/arch/x86/mach-efi/elf_x86_64_efi.lds.S
> +++ b/arch/x86/mach-efi/elf_x86_64_efi.lds.S
> @@ -23,6 +23,7 @@ SECTIONS
>  		*(.text)
>  		*(.text.*)
>  		*(.gnu.linkonce.t.*)
> +		. = ALIGN(16);

Why 16 here?

Regards,
Michael

>  	}
>  
>  	_etext = .;
> @@ -33,8 +34,8 @@ SECTIONS
>  		*(.reloc)
>  	}
>  
> -	. = ALIGN(4096);
>  	_sdata = .;
> +	. = ALIGN(4096);
>  
>  	.data : {
>  		*(.rodata*)
> @@ -52,22 +53,23 @@ SECTIONS
>  		*(.rel.local)
>  	}
>  
> -	. = ALIGN(64);
> +	. = ALIGN(4096);
>  
>  	__barebox_initcalls_start = .;
>  	__barebox_initcalls : { INITCALLS }
>  	__barebox_initcalls_end = .;
> +	. = ALIGN(4096);
>  
>  	__barebox_exitcalls_start = .;
>  	__barebox_exitcalls : { EXITCALLS }
>  	__barebox_exitcalls_end = .;
>  
> -	. = ALIGN(64);
> +	. = ALIGN(4096);
>  	__barebox_magicvar_start = .;
>  	.barebox_magicvar : { BAREBOX_MAGICVARS }
>  	__barebox_magicvar_end = .;
>  
> -	. = ALIGN(64);
> +	. = ALIGN(4096);
>  	__barebox_cmd_start = .;
>  	__barebox_cmd : { BAREBOX_CMDS }
>  	__barebox_cmd_end = .;
> diff --git a/arch/x86/mach-efi/include/mach/barebox.lds.h b/arch/x86/mach-efi/include/mach/barebox.lds.h
> index 40a8c178f..e7a3bb9cd 100644
> --- a/arch/x86/mach-efi/include/mach/barebox.lds.h
> +++ b/arch/x86/mach-efi/include/mach/barebox.lds.h
> @@ -1 +1,13 @@
> -/* empty */
> +/*
> + * Copyright (C) 2017 Jean-Christophe PLAGNIOL-VILLARD <plagnio@jcrosoft.com>
> + *
> + * Under GPL v2
> + */
> +
> +#ifndef __EFI_MACH_BAREBOX_LDS_H__
> +#define __EFI_MACH_BAREBOX_LDS_H__
> +
> +/* For secure boot we need all the section to be 4096 alligned */
> +#define STRUCT_ALIGNMENT 4096
> +
> +#endif /* __EFI_MACH_BAREBOX_LDS_H__ */
> diff --git a/include/asm-generic/barebox.lds.h b/include/asm-generic/barebox.lds.h
> index c8a919b92..6c37751b3 100644
> --- a/include/asm-generic/barebox.lds.h
> +++ b/include/asm-generic/barebox.lds.h
> @@ -3,15 +3,17 @@
>   * Align to a 32 byte boundary equal to the
>   * alignment gcc 4.5 uses for a struct
>   */
> -#define STRUCT_ALIGNMENT 32
> -#define STRUCT_ALIGN() . = ALIGN(STRUCT_ALIGNMENT)
> -
>  #if defined CONFIG_X86 || \
>  	defined CONFIG_ARCH_EP93XX || \
>  	defined CONFIG_ARCH_ZYNQ
>  #include <mach/barebox.lds.h>
>  #endif
>  
> +#ifndef STRUCT_ALIGNMENT
> +#define STRUCT_ALIGNMENT 32
> +#endif
> +#define STRUCT_ALIGN() . = ALIGN(STRUCT_ALIGNMENT)
> +
>  #ifndef PRE_IMAGE
>  #define PRE_IMAGE
>  #endif
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 09/13] boot_verify: add password request support
  2017-03-26  2:45   ` [PATCH 09/13] boot_verify: add password request support Jean-Christophe PLAGNIOL-VILLARD
@ 2017-03-27  6:11     ` Sascha Hauer
  0 siblings, 0 replies; 22+ messages in thread
From: Sascha Hauer @ 2017-03-27  6:11 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox

On Sun, Mar 26, 2017 at 04:45:00AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> This will allow to let the user enter a password before booting more safe
> than just a 'y'
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
>  common/boot_verify.c | 10 ++++++++++
>  common/password.c    | 18 ++++++++++++++++++
>  include/password.h   |  6 ++++++
>  3 files changed, 34 insertions(+)
> 
> diff --git a/common/boot_verify.c b/common/boot_verify.c
> index 07ae07e16..2faa8d56c 100644
> --- a/common/boot_verify.c
> +++ b/common/boot_verify.c
> @@ -10,6 +10,7 @@
>  #include <globalvar.h>
>  #include <magicvar.h>
>  #include <init.h>
> +#include <password.h>
>  
>  static unsigned int boot_verify_confirm_timeout = 10;
>  static enum boot_verify boot_verify_mode = BOOT_VERIFY_HASH;
> @@ -63,6 +64,14 @@ int boot_can_start_unsigned(void)
>  
>  	printf("Are you sure you wish to run an unsigned binary\n");
>  	printf("in a secure environment?\n");
> +	if (IS_ENABLED(CONFIG_PASSWORD)) {
> +		printf("enter password to confirm\n");

This needs to be in request_password(), otherwise you may end up
printing this without a password ever being asked for.

> +		ret = request_password(timeout);
> +		if (ret != -ENOTSUPP)
> +			return -ESECVIOLATION;

Shouldn't you continue when the correct password is entered?

> +
> +	}
> +
>  	printf("press y to confirm\n");
>  
>  	ret = console_countdown(timeout, CONSOLE_COUNTDOWN_ANYKEY, &c);
> @@ -72,6 +81,7 @@ int boot_can_start_unsigned(void)
>  	return c == 'y' ? 0 : -ESECVIOLATION;
>  }
>  
> +
>  static int init_boot_verify(void)
>  {
>  	int size;
> diff --git a/common/password.c b/common/password.c
> index d52b746f0..1147111cd 100644
> --- a/common/password.c
> +++ b/common/password.c
> @@ -435,6 +435,24 @@ void login(void)
>  	}
>  }
>  
> +int request_password(int timeout)
> +{
> +	unsigned char passwd[PASSWD_MAX_LENGTH];
> +	int ret;
> +
> +	if (!is_passwd_default_enable() && !is_passwd_env_enable())
> +		return -ENOTSUPP;
> +
> +	ret = password(passwd, PASSWD_MAX_LENGTH, LOGIN_MODE, timeout);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (check_passwd(passwd, ret) == 1)
> +		return 0;
> +
> +	return -EINVAL;
> +}
> +
>  static int login_global_init(void)
>  {
>  	login_fail_command = xstrdup("boot");
> diff --git a/include/password.h b/include/password.h
> index 8b9961815..5e8964929 100644
> --- a/include/password.h
> +++ b/include/password.h
> @@ -31,10 +31,16 @@ int set_env_passwd(unsigned char *passwd, size_t length);
>  
>  #ifdef CONFIG_PASSWORD
>  void login(void);
> +int request_password(int timeout);
>  #else
>  static inline void login(void)
>  {
>  }
> +
> +static inline int request_password(int timeout)
> +{
> +	return 0;
> +}

You have a static inline wrapper for request_password(), why not use it
and drop the IS_ENABLED(CONFIG_PASSWORD) above when you use it?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 07/13] go: only use it if boot signature is not required
  2017-03-26  8:23     ` Michael Olbrich
@ 2017-03-27 11:50       ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 0 replies; 22+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-27 11:50 UTC (permalink / raw)
  To: Michael Olbrich; +Cc: barebox


> On 26 Mar 2017, at 4:23 PM, Michael Olbrich <m.olbrich@pengutronix.de> wrote:
> 
> On Sun, Mar 26, 2017 at 04:44:58AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
>> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> 
> Does this realy help? If someone has access to the barebox shell, then
> there are many ways to overwrite the secure boot check.

No have shell support does not mean been allow to by pass secure boot
As you think user interaction vs script

And do not forget the boot sequence can be change by the OS (user)

So we may endup to try to boot other images or boot sequence that use go

Best Regards,
J.


> Michael
> 
>> ---
>> commands/go.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>> 
>> diff --git a/commands/go.c b/commands/go.c
>> index fb319b320..e0385a977 100644
>> --- a/commands/go.c
>> +++ b/commands/go.c
>> @@ -26,6 +26,7 @@
>> #include <fcntl.h>
>> #include <linux/ctype.h>
>> #include <errno.h>
>> +#include <boot_verify.h>
>> 
>> static int do_go(int argc, char *argv[])
>> {
>> @@ -37,6 +38,9 @@ static int do_go(int argc, char *argv[])
>> 	if (argc < 2)
>> 		return COMMAND_ERROR_USAGE;
>> 
>> +	if (boot_get_verify_mode() < BOOT_VERIFY_AVAILABLE)
>> +		return -ESECVIOLATION;
>> +
>> 	if (!isdigit(*argv[1])) {
>> 		fd = open(argv[1], O_RDONLY);
>> 		if (fd < 0) {
>> -- 
>> 2.11.0
>> 
>> 
>> _______________________________________________
>> barebox mailing list
>> barebox@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/barebox
>> 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

end of thread, other threads:[~2017-03-27 13:13 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-25  8:31 [PATCH 00/13] add efi secure boot support Jean-Christophe PLAGNIOL-VILLARD
2017-03-26  2:44 ` [PATCH 01/13] bootm: move open to image_handler Jean-Christophe PLAGNIOL-VILLARD
2017-03-26  2:44   ` [PATCH 02/13] boot_verify: use a new error ESECVIOLATION Jean-Christophe PLAGNIOL-VILLARD
2017-03-26  7:59     ` Michael Olbrich
2017-03-26  2:44   ` [PATCH 03/13] bootm: make security generic Jean-Christophe PLAGNIOL-VILLARD
2017-03-26  2:44   ` [PATCH 04/13] boot: invert the secure boot forcing support Jean-Christophe PLAGNIOL-VILLARD
2017-03-26  2:44   ` [PATCH 05/13] move boot verify to generic code Jean-Christophe PLAGNIOL-VILLARD
2017-03-26  2:44   ` [PATCH 06/13] boot_verify: make it modifiable at start time Jean-Christophe PLAGNIOL-VILLARD
2017-03-26  8:16     ` Michael Olbrich
2017-03-26  2:44   ` [PATCH 07/13] go: only use it if boot signature is not required Jean-Christophe PLAGNIOL-VILLARD
2017-03-26  8:23     ` Michael Olbrich
2017-03-27 11:50       ` Jean-Christophe PLAGNIOL-VILLARD
2017-03-26  2:44   ` [PATCH 08/13] boot_verify: allow to force unsigned image to boot Jean-Christophe PLAGNIOL-VILLARD
2017-03-26  8:25     ` Michael Olbrich
2017-03-26  2:45   ` [PATCH 09/13] boot_verify: add password request support Jean-Christophe PLAGNIOL-VILLARD
2017-03-27  6:11     ` Sascha Hauer
2017-03-26  2:45   ` [PATCH 10/13] efi: add more security related guid for the efivars Jean-Christophe PLAGNIOL-VILLARD
2017-03-26  2:45   ` [PATCH 11/13] efi: fix lds for secure boot support Jean-Christophe PLAGNIOL-VILLARD
2017-03-26  8:30     ` Michael Olbrich
2017-03-26  2:45   ` [PATCH 12/13] efi: fix secure and setup mode report Jean-Christophe PLAGNIOL-VILLARD
2017-03-26  2:45   ` [PATCH 13/13] efi: enable sercure boot support Jean-Christophe PLAGNIOL-VILLARD
2017-03-26  7:57   ` [PATCH 01/13] bootm: move open to image_handler Michael Olbrich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox