[PATCH v4 10/16] cmd: env: use appropriate guid for authenticated UEFI variable

[AKASHI Takahiro]

On Wed, Jan 22, 2020 at 07:38:06AM +0100, Heinrich Schuchardt wrote:
> On 1/22/20 2:01 AM, AKASHI Takahiro wrote:
> >On Tue, Jan 21, 2020 at 08:13:06AM +0100, Heinrich Schuchardt wrote:
> >>On 12/18/19 1:45 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.
> >>>
> >>>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"))
> >>
> >>Why is "dbr" missing?
> >
> >Because it is not yet supported and I have no plan to support it
> >in short term.
> 
> This should not stop us from adding "dbr" here just to keep the setenv
> command consistent even if the value of dbr will not yet be considered
> in our secure boot implementation.

Your comments are inconsistent from time to time.
Please remember when I submitted the patch "cmd: env: extend "env
[set|print] -e" to manage UEFI variables".
In my early versions, "-at" (TIME_BASED_AUTHENTICATED_WRITE_ACCESS)
was also supported. But you rejected it just because secure boot
was not merged yet at the moment.

So nak to your suggestion above.

-Takahiro Akashi


> Best regards
> 
> Heinrich
> 
> >
> >>I guess dbDefault, dbrDefault, dbxDefault, dbtDefault use
> >>EFI_GLOBAL_VARIABLE?
> >
> >Yes.
> >I have a patch for supporting those *Default now, but will submit it
> >once my core secure boot patch is accepted.
> >
> >Thanks,
> >-Takahiro Akashi
> >
> >>Best regards
> >>
> >>Heinrich
> >>
> >>>+			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 *)
> >>>
> >>
> >
>