[PATCH] luks2: Continue trying all keyslots even if there are some failures

[PATCH] luks2: Continue trying all keyslots even if there are some failures

[Glenn Washburn]

luks2_get_keyslot can fail for a variety of reasons that do not neccesarily
mean the next keyslot should not be tried (eg. a new kdf type). So always
try the next slot. This will make GRUB more resilient to non-spec json data
that 3rd party systems may add. We do not care if some of the keyslots are
unusable, only if there is at least one that is.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/disk/luks2.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
index bf741d70f..d8d3180ed 100644
--- a/grub-core/disk/luks2.c
+++ b/grub-core/disk/luks2.c
@@ -610,7 +610,15 @@ luks2_recover_key (grub_disk_t source,
       grub_errno = GRUB_ERR_NONE;
       ret = luks2_get_keyslot (&keyslot, &digest, &segment, json, json_idx);
       if (ret)
-	goto err;
+	{
+	  /*
+	   * luks2_get_keyslot can fail for a variety of reasons that do not
+	   * neccesarily mean the next keyslot should not be tried (eg. a new
+	   * kdf type). So always try the next slot.
+	   */
+	  grub_dprintf ("luks2", "Failed to get keyslot %" PRIuGRUB_UINT64_T "\n", keyslot.idx);
+	  continue;
+	}
       if (grub_errno != GRUB_ERR_NONE)
 	  grub_dprintf ("luks2", "Ignoring unhandled error %d from luks2_get_keyslot\n", grub_errno);
 
-- 
2.34.1

Re: [PATCH] luks2: Continue trying all keyslots even if there are some failures

[Daniel Kiper]

On Fri, Jul 22, 2022 at 03:04:50AM -0500, Glenn Washburn wrote:
> luks2_get_keyslot can fail for a variety of reasons that do not neccesarily
> mean the next keyslot should not be tried (eg. a new kdf type). So always
> try the next slot. This will make GRUB more resilient to non-spec json data
> that 3rd party systems may add. We do not care if some of the keyslots are
> unusable, only if there is at least one that is.
>
> Signed-off-by: Glenn Washburn <development@efficientek.com>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

Re: [PATCH] luks2: Continue trying all keyslots even if there are some failures

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 1543 bytes --]

On Fri, Jul 22, 2022 at 03:04:50AM -0500, Glenn Washburn wrote:
> luks2_get_keyslot can fail for a variety of reasons that do not neccesarily
> mean the next keyslot should not be tried (eg. a new kdf type). So always
> try the next slot. This will make GRUB more resilient to non-spec json data
> that 3rd party systems may add. We do not care if some of the keyslots are
> unusable, only if there is at least one that is.
> 
> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
>  grub-core/disk/luks2.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> index bf741d70f..d8d3180ed 100644
> --- a/grub-core/disk/luks2.c
> +++ b/grub-core/disk/luks2.c
> @@ -610,7 +610,15 @@ luks2_recover_key (grub_disk_t source,
>        grub_errno = GRUB_ERR_NONE;
>        ret = luks2_get_keyslot (&keyslot, &digest, &segment, json, json_idx);
>        if (ret)
> -	goto err;
> +	{
> +	  /*
> +	   * luks2_get_keyslot can fail for a variety of reasons that do not
> +	   * neccesarily mean the next keyslot should not be tried (eg. a new
> +	   * kdf type). So always try the next slot.
> +	   */
> +	  grub_dprintf ("luks2", "Failed to get keyslot %" PRIuGRUB_UINT64_T "\n", keyslot.idx);
> +	  continue;
> +	}
>        if (grub_errno != GRUB_ERR_NONE)
>  	  grub_dprintf ("luks2", "Ignoring unhandled error %d from luks2_get_keyslot\n", grub_errno);
>  
> -- 
> 2.34.1
> 

Reviewed-by: Patrick Steinhardt <ps@pks.im>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] luks2: Continue trying all keyslots even if there are some failures

[Daniel Kiper]

On Mon, Aug 15, 2022 at 05:23:15PM +0200, Patrick Steinhardt wrote:
> On Fri, Jul 22, 2022 at 03:04:50AM -0500, Glenn Washburn wrote:
> > luks2_get_keyslot can fail for a variety of reasons that do not neccesarily
> > mean the next keyslot should not be tried (eg. a new kdf type). So always
> > try the next slot. This will make GRUB more resilient to non-spec json data
> > that 3rd party systems may add. We do not care if some of the keyslots are
> > unusable, only if there is at least one that is.
> >
> > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > ---
> >  grub-core/disk/luks2.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> > index bf741d70f..d8d3180ed 100644
> > --- a/grub-core/disk/luks2.c
> > +++ b/grub-core/disk/luks2.c
> > @@ -610,7 +610,15 @@ luks2_recover_key (grub_disk_t source,
> >        grub_errno = GRUB_ERR_NONE;
> >        ret = luks2_get_keyslot (&keyslot, &digest, &segment, json, json_idx);
> >        if (ret)
> > -	goto err;
> > +	{
> > +	  /*
> > +	   * luks2_get_keyslot can fail for a variety of reasons that do not
> > +	   * neccesarily mean the next keyslot should not be tried (eg. a new
> > +	   * kdf type). So always try the next slot.
> > +	   */
> > +	  grub_dprintf ("luks2", "Failed to get keyslot %" PRIuGRUB_UINT64_T "\n", keyslot.idx);
> > +	  continue;
> > +	}
> >        if (grub_errno != GRUB_ERR_NONE)
> >  	  grub_dprintf ("luks2", "Ignoring unhandled error %d from luks2_get_keyslot\n", grub_errno);
> >
> > --
> > 2.34.1
> >
>
> Reviewed-by: Patrick Steinhardt <ps@pks.im>

Thank you for review but I have merged this patch earlier and cannot add
your RB now... :-(

Daniel

Re: [PATCH] luks2: Continue trying all keyslots even if there are some failures

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 2033 bytes --]

On Fri, Aug 19, 2022 at 04:10:44PM +0200, Daniel Kiper wrote:
> On Mon, Aug 15, 2022 at 05:23:15PM +0200, Patrick Steinhardt wrote:
> > On Fri, Jul 22, 2022 at 03:04:50AM -0500, Glenn Washburn wrote:
> > > luks2_get_keyslot can fail for a variety of reasons that do not neccesarily
> > > mean the next keyslot should not be tried (eg. a new kdf type). So always
> > > try the next slot. This will make GRUB more resilient to non-spec json data
> > > that 3rd party systems may add. We do not care if some of the keyslots are
> > > unusable, only if there is at least one that is.
> > >
> > > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > > ---
> > >  grub-core/disk/luks2.c | 10 +++++++++-
> > >  1 file changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> > > index bf741d70f..d8d3180ed 100644
> > > --- a/grub-core/disk/luks2.c
> > > +++ b/grub-core/disk/luks2.c
> > > @@ -610,7 +610,15 @@ luks2_recover_key (grub_disk_t source,
> > >        grub_errno = GRUB_ERR_NONE;
> > >        ret = luks2_get_keyslot (&keyslot, &digest, &segment, json, json_idx);
> > >        if (ret)
> > > -	goto err;
> > > +	{
> > > +	  /*
> > > +	   * luks2_get_keyslot can fail for a variety of reasons that do not
> > > +	   * neccesarily mean the next keyslot should not be tried (eg. a new
> > > +	   * kdf type). So always try the next slot.
> > > +	   */
> > > +	  grub_dprintf ("luks2", "Failed to get keyslot %" PRIuGRUB_UINT64_T "\n", keyslot.idx);
> > > +	  continue;
> > > +	}
> > >        if (grub_errno != GRUB_ERR_NONE)
> > >  	  grub_dprintf ("luks2", "Ignoring unhandled error %d from luks2_get_keyslot\n", grub_errno);
> > >
> > > --
> > > 2.34.1
> > >
> >
> > Reviewed-by: Patrick Steinhardt <ps@pks.im>
> 
> Thank you for review but I have merged this patch earlier and cannot add
> your RB now... :-(
> 
> Daniel

No worries! I assumed as much when I reviewed this but was too lazy to
check ;)

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]