On Fri, Aug 30, 2024 at 06:10:28PM +0200, Daniel Kiper wrote:
> On Fri, Jun 28, 2024 at 04:19:04PM +0800, Gary Lin via Grub-devel wrote:
> > From: Patrick Colp <patrick.c...@oracle.com>
> >
> > If a protector is specified, but it fails to unlock the disk, fall back
> > to asking for the passphrase. However, an error was set indicating that
> > the protector(s) failed. Later code (e.g., LUKS code) fails as
> > `grub_errno` is now set. Print the existing errors out first, before
> > proceeding with the passphrase.
>
> This behavior has to be documented in the GRUB docs.
>
I'll address the behavior in the section for TPM2 key protector.
> > Signed-off-by: Patrick Colp <patrick.c...@oracle.com>
> > Signed-off-by: Gary Lin <g...@suse.com>
> > Reviewed-by: Stefan Berger <stef...@linux.ibm.com>
> > ---
> > grub-core/disk/cryptodisk.c | 17 ++++++++++++++++-
> > 1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> > index 6f7394942..1a994d935 100644
> > --- a/grub-core/disk/cryptodisk.c
> > +++ b/grub-core/disk/cryptodisk.c
> > @@ -1167,6 +1167,10 @@ grub_cryptodisk_scan_device_real (const char *name,
> > ret = cr->recover_key (source, dev, cargs);
> > if (ret != GRUB_ERR_NONE)
> > {
> > + /* Reset key data to trigger the passphrase prompt later */
> > + cargs->key_data = NULL;
> > + cargs->key_len = 0;
> > +
> > part = grub_partition_get_name (source->partition);
> > grub_dprintf ("cryptodisk",
> > "recovered a key from key protector %s but it "
> > @@ -1192,7 +1196,6 @@ grub_cryptodisk_scan_device_real (const char *name,
> > source->name, source->partition != NULL ? "," : "",
> > part != NULL ? part : N_("UNKNOWN"), dev->uuid);
> > grub_free (part);
> > - goto error;
> > }
> >
> > if (cargs->key_len)
> > @@ -1207,6 +1210,18 @@ grub_cryptodisk_scan_device_real (const char *name,
> > unsigned long tries = 3;
> > const char *tries_env;
> >
> > + /*
> > + * Print the error from key protectors and clear grub_errno.
>
> I think you should explain why you have to do it here. Something similar
> to the commit message...
>
It's mainly to print the error from the key protector. I'll add more
comments for that.
> > + * Since '--protector' doesn't not coexist with '--password' and
>
> s/doesn't not/cannot/?
>
Will fix it in the next versino.
> > + * '--key-file', only "cargs->key_len == 0" is expected if all
> > + * key protectors fail.
> > + */
> > + if (grub_errno)
>
> if (grub_errno != GRUB_ERR_NONE)
>
Ok. I'll fix it.
Gary Lin
> > + {
> > + grub_print_error ();
> > + grub_errno = GRUB_ERR_NONE;
> > + }
>
> Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
