* [PATCH 0/5] EFI Secure boot support
@ 2017-03-09 14:31 Jean-Christophe PLAGNIOL-VILLARD
2017-03-09 14:34 ` [PATCH 1/5] efi: add more security related guid for the efivars Jean-Christophe PLAGNIOL-VILLARD
0 siblings, 1 reply; 19+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-09 14:31 UTC (permalink / raw)
To: barebox
Hi,
This patch serie is the first one to add the secure boot support
to barebox on efi
For now on this will allow you to execute only properly signed EFI
Application.
And request confirmation for non signed binary other than EFI
Later will add non signed EFI Application execution request at user
request and MoK Support, add other goodies.
Please pull
The following changes since commit 071eb906506d478a3d0b04460625e64edbcd1294:
efi: fix secure and setup mode report (2017-03-04 13:20:38 +0800)
are available in the git repository at:
git://git.jcrosoft.org/barebox.git delivery/efi-sb
for you to fetch changes up to 99e61a9be467fab5b5127b1233c4c4e70288e84c:
efi: enable sercure boot support (2017-03-04 13:20:38 +0800)
----------------------------------------------------------------
Jean-Christophe PLAGNIOL-VILLARD (2):
boot: if we are in secure boot mode
efi: enable sercure boot support
arch/x86/Kconfig | 1 +
commands/go.c | 7 +++++++
common/Kconfig | 8 ++++++++
common/Makefile | 1 +
common/bootm.c | 7 +++++++
common/efi/efi-image.c | 1 +
common/secure_boot.c | 43 +++++++++++++++++++++++++++++++++++++++++++
drivers/efi/efi-device.c | 9 +++++++++
include/bootm.h | 1 +
include/secure_boot.h | 25 +++++++++++++++++++++++++
10 files changed, 103 insertions(+)
create mode 100644 common/secure_boot.c
create mode 100644 include/secure_boot.h
Best Regards,
J.
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH 1/5] efi: add more security related guid for the efivars 2017-03-09 14:31 [PATCH 0/5] EFI Secure boot support Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-09 14:34 ` Jean-Christophe PLAGNIOL-VILLARD 2017-03-09 14:34 ` [PATCH 2/5] efi: fix lds for secure boot support Jean-Christophe PLAGNIOL-VILLARD ` (3 more replies) 0 siblings, 4 replies; 19+ messages in thread From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-09 14:34 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] 19+ messages in thread
* [PATCH 2/5] efi: fix lds for secure boot support 2017-03-09 14:34 ` [PATCH 1/5] efi: add more security related guid for the efivars Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-09 14:34 ` Jean-Christophe PLAGNIOL-VILLARD 2017-03-09 17:24 ` Lucas Stach 2017-03-09 14:34 ` [PATCH 3/5] efi: fix secure and setup mode report Jean-Christophe PLAGNIOL-VILLARD ` (2 subsequent siblings) 3 siblings, 1 reply; 19+ messages in thread From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-09 14:34 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] 19+ messages in thread
* Re: [PATCH 2/5] efi: fix lds for secure boot support 2017-03-09 14:34 ` [PATCH 2/5] efi: fix lds for secure boot support Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-09 17:24 ` Lucas Stach 2017-03-10 10:17 ` Jean-Christophe PLAGNIOL-VILLARD 0 siblings, 1 reply; 19+ messages in thread From: Lucas Stach @ 2017-03-09 17:24 UTC (permalink / raw) To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox Am Donnerstag, den 09.03.2017, 15:34 +0100 schrieb Jean-Christophe PLAGNIOL-VILLARD: > everythink need to be aligned to 4096 Why? The commit message isn't really telling anything. Regards, Lucas > > 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 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/5] efi: fix lds for secure boot support 2017-03-09 17:24 ` Lucas Stach @ 2017-03-10 10:17 ` Jean-Christophe PLAGNIOL-VILLARD 2017-03-10 11:05 ` Lucas Stach 0 siblings, 1 reply; 19+ messages in thread From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-10 10:17 UTC (permalink / raw) To: Lucas Stach; +Cc: barebox > On Mar 10, 2017, at 1:24 AM, Lucas Stach <l.stach@pengutronix.de> wrote: > > Am Donnerstag, den 09.03.2017, 15:34 +0100 schrieb Jean-Christophe > PLAGNIOL-VILLARD: >> everythink need to be aligned to 4096 > > Why? The commit message isn't really telling anything. This is a requierment by EFI Best Regards, J. > > Regards, > Lucas > >> >> 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 > > _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/5] efi: fix lds for secure boot support 2017-03-10 10:17 ` Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-10 11:05 ` Lucas Stach 2017-03-10 13:54 ` Jean-Christophe PLAGNIOL-VILLARD 0 siblings, 1 reply; 19+ messages in thread From: Lucas Stach @ 2017-03-10 11:05 UTC (permalink / raw) To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox Am Freitag, den 10.03.2017, 18:17 +0800 schrieb Jean-Christophe PLAGNIOL-VILLARD: > > On Mar 10, 2017, at 1:24 AM, Lucas Stach <l.stach@pengutronix.de> wrote: > > > > Am Donnerstag, den 09.03.2017, 15:34 +0100 schrieb Jean-Christophe > > PLAGNIOL-VILLARD: > >> everythink need to be aligned to 4096 > > > > Why? The commit message isn't really telling anything. > This is a requierment by EFI This is in no way an EFI requirement. Googling tells me that the signing procedure for EFI secure boot is built around a PE binary. PE in turn is based on the COFF binary format which, unlike ELF, has no section descriptions in the header and therefore requires the sections to be placed page aligned (4K on x86, 4K or 64K on ARM64). All of the above is what should have been included in the commit message, to allow other people to understand the commit and not require them to google their way to the justification of this commit. Regards, Lucas > Best Regards, > J. > > > > Regards, > > Lucas > > > >> > >> 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 > > > > > _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/5] efi: fix lds for secure boot support 2017-03-10 11:05 ` Lucas Stach @ 2017-03-10 13:54 ` Jean-Christophe PLAGNIOL-VILLARD 2017-03-10 13:57 ` Lucas Stach 2017-03-10 14:13 ` Jean-Christophe PLAGNIOL-VILLARD 0 siblings, 2 replies; 19+ messages in thread From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-10 13:54 UTC (permalink / raw) To: Lucas Stach; +Cc: barebox On 12:05 Fri 10 Mar , Lucas Stach wrote: > Am Freitag, den 10.03.2017, 18:17 +0800 schrieb Jean-Christophe > PLAGNIOL-VILLARD: > > > On Mar 10, 2017, at 1:24 AM, Lucas Stach <l.stach@pengutronix.de> wrote: > > > > > > Am Donnerstag, den 09.03.2017, 15:34 +0100 schrieb Jean-Christophe > > > PLAGNIOL-VILLARD: > > >> everythink need to be aligned to 4096 > > > > > > Why? The commit message isn't really telling anything. > > This is a requierment by EFI > > This is in no way an EFI requirement. > > Googling tells me that the signing procedure for EFI secure boot is > built around a PE binary. PE in turn is based on the COFF binary format > which, unlike ELF, has no section descriptions in the header and > therefore requires the sections to be placed page aligned (4K on x86, 4K > or 64K on ARM64). No COFF does not require to have section "page aligned" which is wrong here The is a Requirement by EFI from secure boot Otherwise EFI will not work today > > All of the above is what should have been included in the commit > message, to allow other people to understand the commit and not require > them to google their way to the justification of this commit. And read the Spec is required to undrestant EFI no? Or I need to put the spec in the commit too? Best Regards, J > > Regards, > Lucas > > > Best Regards, > > J. > > > > > > Regards, > > > Lucas > > > > > >> > > >> 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 > > > > > > > > > > _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/5] efi: fix lds for secure boot support 2017-03-10 13:54 ` Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-10 13:57 ` Lucas Stach 2017-03-10 14:13 ` Jean-Christophe PLAGNIOL-VILLARD 1 sibling, 0 replies; 19+ messages in thread From: Lucas Stach @ 2017-03-10 13:57 UTC (permalink / raw) To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox Am Freitag, den 10.03.2017, 14:54 +0100 schrieb Jean-Christophe PLAGNIOL-VILLARD: > On 12:05 Fri 10 Mar , Lucas Stach wrote: > > Am Freitag, den 10.03.2017, 18:17 +0800 schrieb Jean-Christophe > > PLAGNIOL-VILLARD: > > > > On Mar 10, 2017, at 1:24 AM, Lucas Stach <l.stach@pengutronix.de> wrote: > > > > > > > > Am Donnerstag, den 09.03.2017, 15:34 +0100 schrieb Jean-Christophe > > > > PLAGNIOL-VILLARD: > > > >> everythink need to be aligned to 4096 > > > > > > > > Why? The commit message isn't really telling anything. > > > This is a requierment by EFI > > > > This is in no way an EFI requirement. > > > > Googling tells me that the signing procedure for EFI secure boot is > > built around a PE binary. PE in turn is based on the COFF binary format > > which, unlike ELF, has no section descriptions in the header and > > therefore requires the sections to be placed page aligned (4K on x86, 4K > > or 64K on ARM64). > No COFF does not require to have section "page aligned" which is wrong here > The is a Requirement by EFI from secure boot > > Otherwise EFI will not work today > > > > > All of the above is what should have been included in the commit > > message, to allow other people to understand the commit and not require > > them to google their way to the justification of this commit. > And read the Spec is required to undrestant EFI no? > Or I need to put the spec in the commit too? > At least a pointer to the relevant part of the spec would be appreciated. That we are even having this conversation shows that your commit messages are a bit too terse. Being a bit more verbose would be really helpful for other folks trying to understand your changes. Thanks, Lucas _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/5] efi: fix lds for secure boot support 2017-03-10 13:54 ` Jean-Christophe PLAGNIOL-VILLARD 2017-03-10 13:57 ` Lucas Stach @ 2017-03-10 14:13 ` Jean-Christophe PLAGNIOL-VILLARD 1 sibling, 0 replies; 19+ messages in thread From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-10 14:13 UTC (permalink / raw) To: Lucas Stach; +Cc: barebox On 14:54 Fri 10 Mar , Jean-Christophe PLAGNIOL-VILLARD wrote: > On 12:05 Fri 10 Mar , Lucas Stach wrote: > > Am Freitag, den 10.03.2017, 18:17 +0800 schrieb Jean-Christophe > > PLAGNIOL-VILLARD: > > > > On Mar 10, 2017, at 1:24 AM, Lucas Stach <l.stach@pengutronix.de> wrote: > > > > > > > > Am Donnerstag, den 09.03.2017, 15:34 +0100 schrieb Jean-Christophe > > > > PLAGNIOL-VILLARD: > > > >> everythink need to be aligned to 4096 > > > > > > > > Why? The commit message isn't really telling anything. > > > This is a requierment by EFI > > > > This is in no way an EFI requirement. > > > > Googling tells me that the signing procedure for EFI secure boot is > > built around a PE binary. PE in turn is based on the COFF binary format > > which, unlike ELF, has no section descriptions in the header and > > therefore requires the sections to be placed page aligned (4K on x86, 4K > > or 64K on ARM64). > No COFF does not require to have section "page aligned" which is wrong here This is the job of the loader to aligned them in memory but on EFI it's required to sign the binary. Best Regards, J. _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/5] efi: fix secure and setup mode report 2017-03-09 14:34 ` [PATCH 1/5] efi: add more security related guid for the efivars Jean-Christophe PLAGNIOL-VILLARD 2017-03-09 14:34 ` [PATCH 2/5] efi: fix lds for secure boot support Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-09 14:34 ` Jean-Christophe PLAGNIOL-VILLARD 2017-03-13 7:34 ` Sascha Hauer 2017-03-09 14:34 ` [PATCH 4/5] boot: if we are in secure boot mode Jean-Christophe PLAGNIOL-VILLARD 2017-03-09 14:34 ` [PATCH 5/5] efi: enable sercure boot support Jean-Christophe PLAGNIOL-VILLARD 3 siblings, 1 reply; 19+ messages in thread From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-09 14:34 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..7029bfb31 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] 19+ messages in thread
* Re: [PATCH 3/5] efi: fix secure and setup mode report 2017-03-09 14:34 ` [PATCH 3/5] efi: fix secure and setup mode report Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-13 7:34 ` Sascha Hauer 2017-03-14 8:15 ` Jean-Christophe PLAGNIOL-VILLARD 0 siblings, 1 reply; 19+ messages in thread From: Sascha Hauer @ 2017-03-13 7:34 UTC (permalink / raw) To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox On Thu, Mar 09, 2017 at 03:34:08PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote: > 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..7029bfb31 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"); While a bitwise 'and' operator surely works with booleans here you should still use a logical 'and' operator here. 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] 19+ messages in thread
* Re: [PATCH 3/5] efi: fix secure and setup mode report 2017-03-13 7:34 ` Sascha Hauer @ 2017-03-14 8:15 ` Jean-Christophe PLAGNIOL-VILLARD 0 siblings, 0 replies; 19+ messages in thread From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-14 8:15 UTC (permalink / raw) To: Sascha Hauer; +Cc: barebox On 08:34 Mon 13 Mar , Sascha Hauer wrote: > On Thu, Mar 09, 2017 at 03:34:08PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote: > > 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..7029bfb31 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"); > > While a bitwise 'and' operator surely works with booleans here you should still > use a logical 'and' operator here. ok Best Regards, J. > > 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] 19+ messages in thread
* [PATCH 4/5] boot: if we are in secure boot mode 2017-03-09 14:34 ` [PATCH 1/5] efi: add more security related guid for the efivars Jean-Christophe PLAGNIOL-VILLARD 2017-03-09 14:34 ` [PATCH 2/5] efi: fix lds for secure boot support Jean-Christophe PLAGNIOL-VILLARD 2017-03-09 14:34 ` [PATCH 3/5] efi: fix secure and setup mode report Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-09 14:34 ` Jean-Christophe PLAGNIOL-VILLARD 2017-03-13 7:50 ` Sascha Hauer 2017-03-13 7:55 ` Sascha Hauer 2017-03-09 14:34 ` [PATCH 5/5] efi: enable sercure boot support Jean-Christophe PLAGNIOL-VILLARD 3 siblings, 2 replies; 19+ messages in thread From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-09 14:34 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 | 7 +++++++ common/Kconfig | 8 ++++++++ common/Makefile | 1 + common/bootm.c | 7 +++++++ common/secure_boot.c | 43 +++++++++++++++++++++++++++++++++++++++++++ include/bootm.h | 1 + include/secure_boot.h | 25 +++++++++++++++++++++++++ 7 files changed, 92 insertions(+) create mode 100644 common/secure_boot.c create mode 100644 include/secure_boot.h diff --git a/commands/go.c b/commands/go.c index fb319b320..61c9ce43f 100644 --- a/commands/go.c +++ b/commands/go.c @@ -26,6 +26,7 @@ #include <fcntl.h> #include <linux/ctype.h> #include <errno.h> +#include <secure_boot.h> static int do_go(int argc, char *argv[]) { @@ -37,6 +38,12 @@ static int do_go(int argc, char *argv[]) if (argc < 2) return COMMAND_ERROR_USAGE; + rcode = secure_boot_start_unsigned(); + if (rcode) + return rcode ? 1 : 0; + + rcode = 1; + if (!isdigit(*argv[1])) { fd = open(argv[1], O_RDONLY); if (fd < 0) { diff --git a/common/Kconfig b/common/Kconfig index f7ff04664..9e7d027a2 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 @@ -184,6 +187,11 @@ config NVVAR while global variables can be changed during runtime without changing the default. +config SECURE_BOOT_TIMEOUT + prompt "Secure Boot unsigned boot timeout" + int + default 3 + menu "memory layout" source "pbl/Kconfig" diff --git a/common/Makefile b/common/Makefile index 5f58c81d2..e57cc2c15 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_HAS_SECURE_BOOT) += secure_boot.o quiet_cmd_pwd_h = PWDH $@ ifdef CONFIG_PASSWORD diff --git a/common/bootm.c b/common/bootm.c index 81625d915..0ebf65681 100644 --- a/common/bootm.c +++ b/common/bootm.c @@ -23,6 +23,7 @@ #include <environment.h> #include <linux/stat.h> #include <magicvar.h> +#include <secure_boot.h> static LIST_HEAD(handler_list); @@ -625,6 +626,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 = secure_boot_start_unsigned(); + if (ret) + goto err_out; + } + ret = handler->bootm(data); if (data->dryrun) printf("Dryrun. Aborted\n"); diff --git a/common/secure_boot.c b/common/secure_boot.c new file mode 100644 index 000000000..e625ef4e1 --- /dev/null +++ b/common/secure_boot.c @@ -0,0 +1,43 @@ +/* + * Copyright (c) 2017 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> + * + * Under GPLv2 Only + */ +#include <common.h> +#include <secure_boot.h> +#include <console_countdown.h> +#include <globalvar.h> +#include <magicvar.h> +#include <init.h> + +static unsigned int sb_confirm_timeout = CONFIG_SECURE_BOOT_TIMEOUT; + +int secure_boot_start_unsigned(void) +{ + int ret; + char str[2] = { }; + int timeout = sb_confirm_timeout; + + if (!is_secure_mode()) + return 0; + + 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, str); + if (ret != -EINTR) + return -EINVAL; + + return str[0] == 'y' ? 0 : -EINVAL; +} + +static int init_sb_confirm_timeout(void) +{ + return globalvar_add_simple_int("sb.confirm_timeout", + &sb_confirm_timeout, "%u"); +} +late_initcall(init_sb_confirm_timeout); + +BAREBOX_MAGICVAR_NAMED(global_sb_confirm_timeout, global.sb.confirm_timeout, + "Secure Boot Comfirm timeout in seconds before booting an unsigned image"); diff --git a/include/bootm.h b/include/bootm.h index 6e9777a9a..12a61c007 100644 --- a/include/bootm.h +++ b/include/bootm.h @@ -89,6 +89,7 @@ struct image_handler { struct list_head list; int ih_os; + int is_secure_supported; enum filetype filetype; int (*bootm)(struct image_data *data); diff --git a/include/secure_boot.h b/include/secure_boot.h new file mode 100644 index 000000000..3aa071012 --- /dev/null +++ b/include/secure_boot.h @@ -0,0 +1,25 @@ +/* + * Copyright (c) 2017 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> + * + * Under GPLv2 Only + */ + +#ifndef __SECURE_BOOT_H__ +#define __SECURE_BOOT_H__ + +#ifdef CONFIG_HAS_SECURE_BOOT +int is_secure_mode(void); +int secure_boot_start_unsigned(void); +#else +static int inline is_secure_mode(void) +{ + return 0; +} + +static int inline secure_boot_start_unsigned(void) +{ + return 0; +} +#endif + +#endif /* __SECURE_BOOT_H__ */ -- 2.11.0 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/5] boot: if we are in secure boot mode 2017-03-09 14:34 ` [PATCH 4/5] boot: if we are in secure boot mode Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-13 7:50 ` Sascha Hauer 2017-03-14 8:14 ` Jean-Christophe PLAGNIOL-VILLARD 2017-03-13 7:55 ` Sascha Hauer 1 sibling, 1 reply; 19+ messages in thread From: Sascha Hauer @ 2017-03-13 7:50 UTC (permalink / raw) To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox On Thu, Mar 09, 2017 at 03:34:09PM +0100, 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 | 7 +++++++ > common/Kconfig | 8 ++++++++ > common/Makefile | 1 + > common/bootm.c | 7 +++++++ > common/secure_boot.c | 43 +++++++++++++++++++++++++++++++++++++++++++ > include/bootm.h | 1 + > include/secure_boot.h | 25 +++++++++++++++++++++++++ > 7 files changed, 92 insertions(+) > create mode 100644 common/secure_boot.c > create mode 100644 include/secure_boot.h > > diff --git a/commands/go.c b/commands/go.c > index fb319b320..61c9ce43f 100644 > --- a/commands/go.c > +++ b/commands/go.c > @@ -26,6 +26,7 @@ > #include <fcntl.h> > #include <linux/ctype.h> > #include <errno.h> > +#include <secure_boot.h> > > static int do_go(int argc, char *argv[]) > { > @@ -37,6 +38,12 @@ static int do_go(int argc, char *argv[]) > if (argc < 2) > return COMMAND_ERROR_USAGE; > > + rcode = secure_boot_start_unsigned(); > + if (rcode) > + return rcode ? 1 : 0; When rcode is true, then if rcode is true, return 1, otherwise return 0? > + > + rcode = 1; > + > if (!isdigit(*argv[1])) { > fd = open(argv[1], O_RDONLY); > if (fd < 0) { > diff --git a/common/Kconfig b/common/Kconfig > index f7ff04664..9e7d027a2 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 > @@ -184,6 +187,11 @@ config NVVAR > while global variables can be changed during runtime without changing the > default. > > +config SECURE_BOOT_TIMEOUT > + prompt "Secure Boot unsigned boot timeout" > + int > + default 3 > + > menu "memory layout" > > source "pbl/Kconfig" > diff --git a/common/Makefile b/common/Makefile > index 5f58c81d2..e57cc2c15 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_HAS_SECURE_BOOT) += secure_boot.o > > quiet_cmd_pwd_h = PWDH $@ > ifdef CONFIG_PASSWORD > diff --git a/common/bootm.c b/common/bootm.c > index 81625d915..0ebf65681 100644 > --- a/common/bootm.c > +++ b/common/bootm.c > @@ -23,6 +23,7 @@ > #include <environment.h> > #include <linux/stat.h> > #include <magicvar.h> > +#include <secure_boot.h> > > static LIST_HEAD(handler_list); > > @@ -625,6 +626,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 = secure_boot_start_unsigned(); > + if (ret) > + goto err_out; > + } > + > ret = handler->bootm(data); > if (data->dryrun) > printf("Dryrun. Aborted\n"); > diff --git a/common/secure_boot.c b/common/secure_boot.c > new file mode 100644 > index 000000000..e625ef4e1 > --- /dev/null > +++ b/common/secure_boot.c > @@ -0,0 +1,43 @@ > +/* > + * Copyright (c) 2017 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> > + * > + * Under GPLv2 Only > + */ > +#include <common.h> > +#include <secure_boot.h> > +#include <console_countdown.h> > +#include <globalvar.h> > +#include <magicvar.h> > +#include <init.h> > + > +static unsigned int sb_confirm_timeout = CONFIG_SECURE_BOOT_TIMEOUT; Use globalvar_add_simple_int instead of putting this into the config. > + > +int secure_boot_start_unsigned(void) This function name is very misleading. It sounds like it starts an unsigned image, but this function doesn't start anything. Second guess is that it returns a boolean whether it's allowed to start an unsigned image, but actually it returns the opposite. > +{ > + int ret; > + char str[2] = { }; > + int timeout = sb_confirm_timeout; > + > + if (!is_secure_mode()) > + return 0; > + > + 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, str); console_countdown puts exactly one character in *str, no need to pass an array. > + if (ret != -EINTR) > + return -EINVAL; > + > + return str[0] == 'y' ? 0 : -EINVAL; > +} > + > +static int init_sb_confirm_timeout(void) > +{ > + return globalvar_add_simple_int("sb.confirm_timeout", > + &sb_confirm_timeout, "%u"); > +} Ok, you already made it a globalvar. No need to put the default into kconfig then. 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] 19+ messages in thread
* Re: [PATCH 4/5] boot: if we are in secure boot mode 2017-03-13 7:50 ` Sascha Hauer @ 2017-03-14 8:14 ` Jean-Christophe PLAGNIOL-VILLARD 0 siblings, 0 replies; 19+ messages in thread From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-14 8:14 UTC (permalink / raw) To: Sascha Hauer; +Cc: barebox On 08:50 Mon 13 Mar , Sascha Hauer wrote: > On Thu, Mar 09, 2017 at 03:34:09PM +0100, 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 | 7 +++++++ > > common/Kconfig | 8 ++++++++ > > common/Makefile | 1 + > > common/bootm.c | 7 +++++++ > > common/secure_boot.c | 43 +++++++++++++++++++++++++++++++++++++++++++ > > include/bootm.h | 1 + > > include/secure_boot.h | 25 +++++++++++++++++++++++++ > > 7 files changed, 92 insertions(+) > > create mode 100644 common/secure_boot.c > > create mode 100644 include/secure_boot.h > > > > diff --git a/commands/go.c b/commands/go.c > > index fb319b320..61c9ce43f 100644 > > --- a/commands/go.c > > +++ b/commands/go.c > > @@ -26,6 +26,7 @@ > > #include <fcntl.h> > > #include <linux/ctype.h> > > #include <errno.h> > > +#include <secure_boot.h> > > > > static int do_go(int argc, char *argv[]) > > { > > @@ -37,6 +38,12 @@ static int do_go(int argc, char *argv[]) > > if (argc < 2) > > return COMMAND_ERROR_USAGE; > > > > + rcode = secure_boot_start_unsigned(); > > + if (rcode) > > + return rcode ? 1 : 0; > > When rcode is true, then if rcode is true, return 1, otherwise return 0? I forget we could return a negative error on command too > > > + > > + rcode = 1; > > + > > if (!isdigit(*argv[1])) { > > fd = open(argv[1], O_RDONLY); > > if (fd < 0) { > > diff --git a/common/Kconfig b/common/Kconfig > > index f7ff04664..9e7d027a2 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 > > @@ -184,6 +187,11 @@ config NVVAR > > while global variables can be changed during runtime without changing the > > default. > > > > +config SECURE_BOOT_TIMEOUT > > + prompt "Secure Boot unsigned boot timeout" > > + int > > + default 3 > > + > > menu "memory layout" > > > > source "pbl/Kconfig" > > diff --git a/common/Makefile b/common/Makefile > > index 5f58c81d2..e57cc2c15 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_HAS_SECURE_BOOT) += secure_boot.o > > > > quiet_cmd_pwd_h = PWDH $@ > > ifdef CONFIG_PASSWORD > > diff --git a/common/bootm.c b/common/bootm.c > > index 81625d915..0ebf65681 100644 > > --- a/common/bootm.c > > +++ b/common/bootm.c > > @@ -23,6 +23,7 @@ > > #include <environment.h> > > #include <linux/stat.h> > > #include <magicvar.h> > > +#include <secure_boot.h> > > > > static LIST_HEAD(handler_list); > > > > @@ -625,6 +626,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 = secure_boot_start_unsigned(); > > + if (ret) > > + goto err_out; > > + } > > + > > ret = handler->bootm(data); > > if (data->dryrun) > > printf("Dryrun. Aborted\n"); > > diff --git a/common/secure_boot.c b/common/secure_boot.c > > new file mode 100644 > > index 000000000..e625ef4e1 > > --- /dev/null > > +++ b/common/secure_boot.c > > @@ -0,0 +1,43 @@ > > +/* > > + * Copyright (c) 2017 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> > > + * > > + * Under GPLv2 Only > > + */ > > +#include <common.h> > > +#include <secure_boot.h> > > +#include <console_countdown.h> > > +#include <globalvar.h> > > +#include <magicvar.h> > > +#include <init.h> > > + > > +static unsigned int sb_confirm_timeout = CONFIG_SECURE_BOOT_TIMEOUT; > > Use globalvar_add_simple_int instead of putting this into the config. > > > + > > +int secure_boot_start_unsigned(void) > > This function name is very misleading. It sounds like it starts an > unsigned image, but this function doesn't start anything. Second guess > is that it returns a boolean whether it's allowed to start an unsigned > image, but actually it returns the opposite. secure_boot_can_start_unsigned could be better it return 0 if ok an error otherwise as we can return an security violation or other error Best Regards, J. _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/5] boot: if we are in secure boot mode 2017-03-09 14:34 ` [PATCH 4/5] boot: if we are in secure boot mode Jean-Christophe PLAGNIOL-VILLARD 2017-03-13 7:50 ` Sascha Hauer @ 2017-03-13 7:55 ` Sascha Hauer 2017-03-14 8:07 ` Jean-Christophe PLAGNIOL-VILLARD 1 sibling, 1 reply; 19+ messages in thread From: Sascha Hauer @ 2017-03-13 7:55 UTC (permalink / raw) To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox On Thu, Mar 09, 2017 at 03:34:09PM +0100, 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 | 7 +++++++ > common/Kconfig | 8 ++++++++ > common/Makefile | 1 + > common/bootm.c | 7 +++++++ > common/secure_boot.c | 43 +++++++++++++++++++++++++++++++++++++++++++ > include/bootm.h | 1 + > include/secure_boot.h | 25 +++++++++++++++++++++++++ > 7 files changed, 92 insertions(+) > create mode 100644 common/secure_boot.c > create mode 100644 include/secure_boot.h > > diff --git a/commands/go.c b/commands/go.c > index fb319b320..61c9ce43f 100644 > --- a/commands/go.c > +++ b/commands/go.c > @@ -26,6 +26,7 @@ > #include <fcntl.h> > #include <linux/ctype.h> > #include <errno.h> > +#include <secure_boot.h> > > static int do_go(int argc, char *argv[]) > { > @@ -37,6 +38,12 @@ static int do_go(int argc, char *argv[]) > if (argc < 2) > return COMMAND_ERROR_USAGE; > > + rcode = secure_boot_start_unsigned(); > + if (rcode) > + return rcode ? 1 : 0; > + > + rcode = 1; > + > if (!isdigit(*argv[1])) { > fd = open(argv[1], O_RDONLY); > if (fd < 0) { > diff --git a/common/Kconfig b/common/Kconfig > index f7ff04664..9e7d027a2 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 > @@ -184,6 +187,11 @@ config NVVAR > while global variables can be changed during runtime without changing the > default. > > +config SECURE_BOOT_TIMEOUT > + prompt "Secure Boot unsigned boot timeout" > + int > + default 3 > + > menu "memory layout" > > source "pbl/Kconfig" > diff --git a/common/Makefile b/common/Makefile > index 5f58c81d2..e57cc2c15 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_HAS_SECURE_BOOT) += secure_boot.o > > quiet_cmd_pwd_h = PWDH $@ > ifdef CONFIG_PASSWORD > diff --git a/common/bootm.c b/common/bootm.c > index 81625d915..0ebf65681 100644 > --- a/common/bootm.c > +++ b/common/bootm.c > @@ -23,6 +23,7 @@ > #include <environment.h> > #include <linux/stat.h> > #include <magicvar.h> > +#include <secure_boot.h> > > static LIST_HEAD(handler_list); > > @@ -625,6 +626,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 = secure_boot_start_unsigned(); > + if (ret) > + goto err_out; > + } We already have CONFIG_BOOTM_FORCE_SIGNED_IMAGES which admittedly is not the most generic implementation, but in the end we shouldn't have two different ways how secure boot is integrated in the bootm code. 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] 19+ messages in thread
* Re: [PATCH 4/5] boot: if we are in secure boot mode 2017-03-13 7:55 ` Sascha Hauer @ 2017-03-14 8:07 ` Jean-Christophe PLAGNIOL-VILLARD 2017-03-14 9:48 ` Sascha Hauer 0 siblings, 1 reply; 19+ messages in thread From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-14 8:07 UTC (permalink / raw) To: Sascha Hauer; +Cc: barebox On 08:55 Mon 13 Mar , Sascha Hauer wrote: > On Thu, Mar 09, 2017 at 03:34:09PM +0100, 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 | 7 +++++++ > > common/Kconfig | 8 ++++++++ > > common/Makefile | 1 + > > common/bootm.c | 7 +++++++ > > common/secure_boot.c | 43 +++++++++++++++++++++++++++++++++++++++++++ > > include/bootm.h | 1 + > > include/secure_boot.h | 25 +++++++++++++++++++++++++ > > 7 files changed, 92 insertions(+) > > create mode 100644 common/secure_boot.c > > create mode 100644 include/secure_boot.h > > > > diff --git a/commands/go.c b/commands/go.c > > index fb319b320..61c9ce43f 100644 > > --- a/commands/go.c > > +++ b/commands/go.c > > @@ -26,6 +26,7 @@ > > #include <fcntl.h> > > #include <linux/ctype.h> > > #include <errno.h> > > +#include <secure_boot.h> > > > > static int do_go(int argc, char *argv[]) > > { > > @@ -37,6 +38,12 @@ static int do_go(int argc, char *argv[]) > > if (argc < 2) > > return COMMAND_ERROR_USAGE; > > > > + rcode = secure_boot_start_unsigned(); > > + if (rcode) > > + return rcode ? 1 : 0; > > + > > + rcode = 1; > > + > > if (!isdigit(*argv[1])) { > > fd = open(argv[1], O_RDONLY); > > if (fd < 0) { > > diff --git a/common/Kconfig b/common/Kconfig > > index f7ff04664..9e7d027a2 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 > > @@ -184,6 +187,11 @@ config NVVAR > > while global variables can be changed during runtime without changing the > > default. > > > > +config SECURE_BOOT_TIMEOUT > > + prompt "Secure Boot unsigned boot timeout" > > + int > > + default 3 > > + > > menu "memory layout" > > > > source "pbl/Kconfig" > > diff --git a/common/Makefile b/common/Makefile > > index 5f58c81d2..e57cc2c15 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_HAS_SECURE_BOOT) += secure_boot.o > > > > quiet_cmd_pwd_h = PWDH $@ > > ifdef CONFIG_PASSWORD > > diff --git a/common/bootm.c b/common/bootm.c > > index 81625d915..0ebf65681 100644 > > --- a/common/bootm.c > > +++ b/common/bootm.c > > @@ -23,6 +23,7 @@ > > #include <environment.h> > > #include <linux/stat.h> > > #include <magicvar.h> > > +#include <secure_boot.h> > > > > static LIST_HEAD(handler_list); > > > > @@ -625,6 +626,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 = secure_boot_start_unsigned(); > > + if (ret) > > + goto err_out; > > + } > > We already have CONFIG_BOOTM_FORCE_SIGNED_IMAGES which admittedly is not > the most generic implementation, but in the end we shouldn't have two > different ways how secure boot is integrated in the bootm code. Did not seen it I check it and this is only for BOOTM as we have go command too that need to call image check too and on efi this is not done at config time but runtime and this is really FIT image oriented :( Can we have one for EFI sperately first and then merge them as this will need a lot of testing and checking to ensure we break nothing Best Regards, J. > > 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] 19+ messages in thread
* Re: [PATCH 4/5] boot: if we are in secure boot mode 2017-03-14 8:07 ` Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-14 9:48 ` Sascha Hauer 0 siblings, 0 replies; 19+ messages in thread From: Sascha Hauer @ 2017-03-14 9:48 UTC (permalink / raw) To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox On Tue, Mar 14, 2017 at 09:07:35AM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote: > On 08:55 Mon 13 Mar , Sascha Hauer wrote: > > On Thu, Mar 09, 2017 at 03:34:09PM +0100, 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 | 7 +++++++ > > > common/Kconfig | 8 ++++++++ > > > common/Makefile | 1 + > > > common/bootm.c | 7 +++++++ > > > common/secure_boot.c | 43 +++++++++++++++++++++++++++++++++++++++++++ > > > include/bootm.h | 1 + > > > include/secure_boot.h | 25 +++++++++++++++++++++++++ > > > 7 files changed, 92 insertions(+) > > > create mode 100644 common/secure_boot.c > > > create mode 100644 include/secure_boot.h > > > > > > diff --git a/commands/go.c b/commands/go.c > > > index fb319b320..61c9ce43f 100644 > > > --- a/commands/go.c > > > +++ b/commands/go.c > > > @@ -26,6 +26,7 @@ > > > #include <fcntl.h> > > > #include <linux/ctype.h> > > > #include <errno.h> > > > +#include <secure_boot.h> > > > > > > static int do_go(int argc, char *argv[]) > > > { > > > @@ -37,6 +38,12 @@ static int do_go(int argc, char *argv[]) > > > if (argc < 2) > > > return COMMAND_ERROR_USAGE; > > > > > > + rcode = secure_boot_start_unsigned(); > > > + if (rcode) > > > + return rcode ? 1 : 0; > > > + > > > + rcode = 1; > > > + > > > if (!isdigit(*argv[1])) { > > > fd = open(argv[1], O_RDONLY); > > > if (fd < 0) { > > > diff --git a/common/Kconfig b/common/Kconfig > > > index f7ff04664..9e7d027a2 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 > > > @@ -184,6 +187,11 @@ config NVVAR > > > while global variables can be changed during runtime without changing the > > > default. > > > > > > +config SECURE_BOOT_TIMEOUT > > > + prompt "Secure Boot unsigned boot timeout" > > > + int > > > + default 3 > > > + > > > menu "memory layout" > > > > > > source "pbl/Kconfig" > > > diff --git a/common/Makefile b/common/Makefile > > > index 5f58c81d2..e57cc2c15 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_HAS_SECURE_BOOT) += secure_boot.o > > > > > > quiet_cmd_pwd_h = PWDH $@ > > > ifdef CONFIG_PASSWORD > > > diff --git a/common/bootm.c b/common/bootm.c > > > index 81625d915..0ebf65681 100644 > > > --- a/common/bootm.c > > > +++ b/common/bootm.c > > > @@ -23,6 +23,7 @@ > > > #include <environment.h> > > > #include <linux/stat.h> > > > #include <magicvar.h> > > > +#include <secure_boot.h> > > > > > > static LIST_HEAD(handler_list); > > > > > > @@ -625,6 +626,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 = secure_boot_start_unsigned(); > > > + if (ret) > > > + goto err_out; > > > + } > > > > We already have CONFIG_BOOTM_FORCE_SIGNED_IMAGES which admittedly is not > > the most generic implementation, but in the end we shouldn't have two > > different ways how secure boot is integrated in the bootm code. > Did not seen it > > I check it and this is only for BOOTM as we have go command too > > that need to call image check too > > and on efi this is not done at config time but runtime > > and this is really FIT image oriented :( You have the handler->is_secure_supported variable. This could be implemented by the FIT image support aswell. Also we have the decision if we want to boot unsigned images. Right now this is done in the data->verify variable. We can force the decision in Kconfig using CONFIG_BOOTM_FORCE_SIGNED_IMAGES or we can fill in data->verify before booting. > > Can we have one for EFI sperately first and then merge them as this will need > a lot of testing and checking to ensure we break nothing No. It's just too confusing to implement two different secure boot ways in the same code and I give nothing to the promise that this will be cleaned up later. 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] 19+ messages in thread
* [PATCH 5/5] efi: enable sercure boot support 2017-03-09 14:34 ` [PATCH 1/5] efi: add more security related guid for the efivars Jean-Christophe PLAGNIOL-VILLARD ` (2 preceding siblings ...) 2017-03-09 14:34 ` [PATCH 4/5] boot: if we are in secure boot mode Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-09 14:34 ` Jean-Christophe PLAGNIOL-VILLARD 3 siblings, 0 replies; 19+ messages in thread From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-09 14:34 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 | 9 +++++++++ 3 files changed, 11 insertions(+) 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 7029bfb31..959878e7f 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 <secure_boot.h> #include <efi.h> #include <efi/efi.h> #include <efi/efi-device.h> @@ -382,6 +383,14 @@ static int efi_is_setup_mode(void) return ret != 0; } +int 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; -- 2.11.0 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2017-03-14 9:48 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-03-09 14:31 [PATCH 0/5] EFI Secure boot support Jean-Christophe PLAGNIOL-VILLARD 2017-03-09 14:34 ` [PATCH 1/5] efi: add more security related guid for the efivars Jean-Christophe PLAGNIOL-VILLARD 2017-03-09 14:34 ` [PATCH 2/5] efi: fix lds for secure boot support Jean-Christophe PLAGNIOL-VILLARD 2017-03-09 17:24 ` Lucas Stach 2017-03-10 10:17 ` Jean-Christophe PLAGNIOL-VILLARD 2017-03-10 11:05 ` Lucas Stach 2017-03-10 13:54 ` Jean-Christophe PLAGNIOL-VILLARD 2017-03-10 13:57 ` Lucas Stach 2017-03-10 14:13 ` Jean-Christophe PLAGNIOL-VILLARD 2017-03-09 14:34 ` [PATCH 3/5] efi: fix secure and setup mode report Jean-Christophe PLAGNIOL-VILLARD 2017-03-13 7:34 ` Sascha Hauer 2017-03-14 8:15 ` Jean-Christophe PLAGNIOL-VILLARD 2017-03-09 14:34 ` [PATCH 4/5] boot: if we are in secure boot mode Jean-Christophe PLAGNIOL-VILLARD 2017-03-13 7:50 ` Sascha Hauer 2017-03-14 8:14 ` Jean-Christophe PLAGNIOL-VILLARD 2017-03-13 7:55 ` Sascha Hauer 2017-03-14 8:07 ` Jean-Christophe PLAGNIOL-VILLARD 2017-03-14 9:48 ` Sascha Hauer 2017-03-09 14:34 ` [PATCH 5/5] efi: enable sercure boot support Jean-Christophe PLAGNIOL-VILLARD
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox