On 27.03.19 14:10, Heinrich Schuchardt wrote:
On 3/27/19 5:40 AM, AKASHI Takahiro wrote:
Add CONFIG_CMD_STANDALONE_EFIBOOTMGR. With this patch, EFI boot manager can be kicked in by a standalone command, efibootmgr.
I miss your comment form 0/11 here:
- When we will support "secure boot" in the future, EFI Boot Manager is expected to be invoked as a standalone command without any arguments to mitigate security surfaces.
Simply requiring that environment variable $fdtaddr, which is under user control, is used for loading the device tree does not offer any security at all.
For secure boot I would expect that the device tree has to be part of a signed binary and that that signature is checked.
For secure boot, the device tree must not come from the user, correct. However, for secure boot, the user must never get access to the U-Boot shell either, as that would allow access to arbitrary memory modification commands. So in a way, we can assume that $fdtaddr is trusted I guess?
At the same time, it means that the boot command (and thus environment) is trusted too, so there is no need to prohibit passing a DT location, no?
Sorry for mentioning this so late :)
If the user passes in a device tree that should be okay if it has the required signature.
How would you do a signature check on the DT?
Alex
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
cmd/Kconfig | 8 ++++++++ cmd/bootefi.c | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+)
diff --git a/cmd/Kconfig b/cmd/Kconfig index 4bcc5c45579d..aa27b21956b3 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -224,6 +224,14 @@ config CMD_BOOTEFI help Boot an EFI image from memory.
+config CMD_STANDALONE_EFIBOOTMGR
- bool "Enable EFI Boot Manager as a standalone command"
- depends on CMD_BOOTEFI
- default n
- help
With this option enabled, EFI Boot Manager can be invokedas a standalone command.config CMD_BOOTEFI_HELLO_COMPILE bool "Compile a standard EFI hello world binary for testing" depends on CMD_BOOTEFI && !CPU_V7M && !SANDBOX diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 17dccd718008..2a2ff4034831 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -635,3 +635,38 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path) bootefi_image_path = NULL; } }
+#ifdef CONFIG_CMD_STANDALONE_EFIBOOTMGR +static int do_efibootmgr_cmd(cmd_tbl_t *cmdtp, int flag, int argc,
char * const argv[])+{
- char *fdt_opt;
- int ret;
- if (argc != 1)
return CMD_RET_USAGE;It is unclear why you do not allow the user to pass the location of the device tree as a parameter.
- fdt_opt = env_get("fdtaddr");
You are looking at some environment variable $fdtaddr while most boards use $fdt_addr_r as preferred address to load the device tree.
Best regards
Heinrich
- if (!fdt_opt)> + fdt_opt = env_get("fdtcontroladdr");
- ret = do_efibootmgr(fdt_opt);
- free(fdt_opt);
- return ret;
+}
+#ifdef CONFIG_SYS_LONGHELP +static char efibootmgr_help_text[] =
- "(no arguments)\n"
- " - Launch EFI boot manager and execute EFI application\n"
- " based on BootNext and BootOrder variables\n";
+#endif
+U_BOOT_CMD(
- efibootmgr, 1, 0, do_efibootmgr_cmd,
- "Launch EFI boot manager",
- efibootmgr_help_text
+); +#endif /* CONFIG_CMD_STANDALONE_EFIBOOTMGR */