[U-Boot] [PATCH 12/16] cmd: env: use appropriate guid for authenticated UEFI variable

[AKASHI Takahiro]

Heinrich,

On Sat, Nov 16, 2019 at 09:10:35PM +0100, Heinrich Schuchardt wrote:
> On 11/13/19 1:53 AM, AKASHI Takahiro wrote:
> >A signature database variable is associated with a specific guid.
> >For convenience, if user doesn't supply any guid info, "env set|print -e"
> >should complement it.
> 
> If secure boot is enforced, users should not be able to change any
> security relevant variables.

I disagree. In fact, UEFI specification allows users to modify
security database variables if their signatures are verified.
For example, "db" must be signed by one of certificates in PK or KEK,
and updating its value will should be authenticated in SetVariable API.
That is what my patch#7 exactly does.

Thanks,
-Takahiro Akashi

> Instead we need a way to compile the
> security relevant data into the U-Boot binary and add a signature to the
> U-Boot binary which can be checked by the primary boot loader.
> 
> Best regards
> 
> Heinrich
> 
> >
> >Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> >---
> >  cmd/nvedit_efi.c | 18 ++++++++++++++----
> >  1 file changed, 14 insertions(+), 4 deletions(-)
> >
> >diff --git a/cmd/nvedit_efi.c b/cmd/nvedit_efi.c
> >index 8ea0da01283f..579cf430593c 100644
> >--- a/cmd/nvedit_efi.c
> >+++ b/cmd/nvedit_efi.c
> >@@ -41,6 +41,11 @@ static const struct {
> >  } efi_guid_text[] = {
> >  	/* signature database */
> >  	{EFI_GLOBAL_VARIABLE_GUID, "EFI_GLOBAL_VARIABLE_GUID"},
> >+	{EFI_IMAGE_SECURITY_DATABASE_GUID, "EFI_IMAGE_SECURITY_DATABASE_GUID"},
> >+	/* certificate type */
> >+	{EFI_CERT_SHA256_GUID, "EFI_CERT_SHA256_GUID"},
> >+	{EFI_CERT_X509_GUID, "EFI_CERT_X509_GUID"},
> >+	{EFI_CERT_TYPE_PKCS7_GUID, "EFI_CERT_TYPE_PKCS7_GUID"},
> >  };
> >
> >  /* "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" */
> >@@ -525,9 +530,9 @@ int do_env_set_efi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> >  			if (*ep != ',')
> >  				return CMD_RET_USAGE;
> >
> >+			/* 0 should be allowed for delete */
> >  			size = simple_strtoul(++ep, NULL, 16);
> >-			if (!size)
> >-				return CMD_RET_FAILURE;
> >+
> >  			value_on_memory = true;
> >  		} else if (!strcmp(argv[0], "-v")) {
> >  			verbose = true;
> >@@ -539,8 +544,13 @@ int do_env_set_efi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> >  		return CMD_RET_USAGE;
> >
> >  	var_name = argv[0];
> >-	if (default_guid)
> >-		guid = efi_global_variable_guid;
> >+	if (default_guid) {
> >+		if (!strcmp(var_name, "db") || !strcmp(var_name, "dbx") ||
> >+		    !strcmp(var_name, "dbt"))
> >+			guid = efi_guid_image_security_database;
> >+		else
> >+			guid = efi_global_variable_guid;
> >+	}
> >
> >  	if (verbose) {
> >  		printf("GUID: %s\n", efi_guid_to_str((const efi_guid_t *)
> >
>