Re: [PATCH 07/17] cryptodisk,luks: Allow special processing for comparing UUIDs.

[Patrick Steinhardt]

On Wed, Jul 29, 2020 at 04:50:12PM -0500, development@efficientek.com wrote:
> From: Glenn Washburn <development@efficientek.com>
> 
> Create grub_uuidcasecmp to compare UUIDs in a case-insensitive manner and
> that ignores '-' characters. This is backwards compatible with the old LUKS1
> code that stored and compared against UUIDs without dashes. However, the new
> LUKS2 code stores and compares UUIDs that contain dashes. Really, the UUID
> comparison shouldn't care about the dashes, as this change implements. Now
> your old scripts will continue to work with UUIDs without dashes, but you
> may choose to use UUIDs with dashes now too for both LUKS1 and LUKS2.

FYI, this is crossing with my own patchset to implement probing support
for LUKS2 [1]. I do like that we start becoming agnostic of those dashes
now, though, as it really is kind of annoying to handle as a user.

1: https://lists.gnu.org/archive/html/grub-devel/2020-05/msg00235.html

> 
> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
>  grub-core/disk/cryptodisk.c |  4 ++--
>  grub-core/disk/luks.c       | 20 ++++----------------
>  grub-core/disk/luks2.c      |  2 +-
>  include/grub/misc.h         | 21 +++++++++++++++++++++
>  4 files changed, 28 insertions(+), 19 deletions(-)
> 
> diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> index f6b6302e1..f460ab838 100644
> --- a/grub-core/disk/cryptodisk.c
> +++ b/grub-core/disk/cryptodisk.c
> @@ -660,7 +660,7 @@ grub_cryptodisk_open (const char *name, grub_disk_t disk)
>    if (grub_memcmp (name, "cryptouuid/", sizeof ("cryptouuid/") - 1) == 0)
>      {
>        for (dev = cryptodisk_list; dev != NULL; dev = dev->next)
> -     if (grub_strcasecmp (name + sizeof ("cryptouuid/") - 1, dev->uuid) == 0)
> +     if (grub_uuidcasecmp (name + sizeof ("cryptouuid/") - 1, dev->uuid) == 
> 0)
>         break;
>      }
>    else
> @@ -897,7 +897,7 @@ grub_cryptodisk_get_by_uuid (const char *uuid)
>  {
>    grub_cryptodisk_t dev;
>    for (dev = cryptodisk_list; dev != NULL; dev = dev->next)
> -    if (grub_strcasecmp (dev->uuid, uuid) == 0)
> +    if (grub_uuidcasecmp (dev->uuid, uuid) == 0)
>        return dev;
>    return NULL;
>  }
> diff --git a/grub-core/disk/luks.c b/grub-core/disk/luks.c
> index 6ae162601..ea54a9d10 100644
> --- a/grub-core/disk/luks.c
> +++ b/grub-core/disk/luks.c
> @@ -69,10 +69,7 @@ configure_ciphers (grub_disk_t disk, const char 
> *check_uuid,
>                  int check_boot)
>  {
>    grub_cryptodisk_t newdev;
> -  const char *iptr;
>    struct grub_luks_phdr header;
> -  char *optr;
> -  char uuid[sizeof (header.uuid) + 1];
>    char ciphername[sizeof (header.cipherName) + 1];
>    char ciphermode[sizeof (header.cipherMode) + 1];
>    char hashspec[sizeof (header.hashSpec) + 1];
> @@ -95,18 +92,9 @@ configure_ciphers (grub_disk_t disk, const char 
> *check_uuid,
>        || grub_be_to_cpu16 (header.version) != 1)
>      return NULL;
>  
> -  optr = uuid;
> -  for (iptr = header.uuid; iptr < &header.uuid[ARRAY_SIZE (header.uuid)];
> -       iptr++)
> +  if (check_uuid && grub_uuidcasecmp (check_uuid, header.uuid) != 0)
>      {
> -      if (*iptr != '-')
> -     *optr++ = *iptr;
> -    }
> -  *optr = 0;
> -
> -  if (check_uuid && grub_strcasecmp (check_uuid, uuid) != 0)
> -    {
> -      grub_dprintf ("luks", "%s != %s\n", uuid, check_uuid);
> +      grub_dprintf ("luks", "%s != %s\n", header.uuid, check_uuid);
>        return NULL;
>      }
>  
> @@ -125,7 +113,7 @@ configure_ciphers (grub_disk_t disk, const char 
> *check_uuid,
>    newdev->source_disk = NULL;
>    newdev->log_sector_size = 9;
>    newdev->total_length = grub_disk_get_size (disk) - newdev->offset;
> -  grub_memcpy (newdev->uuid, uuid, sizeof (newdev->uuid));
> +  grub_memcpy (newdev->uuid, header.uuid, sizeof (newdev->uuid));
>    newdev->modname = "luks";
>  
>    /* Configure the hash used for the AF splitter and HMAC.  */
> @@ -145,7 +133,7 @@ configure_ciphers (grub_disk_t disk, const char 
> *check_uuid,
>        return NULL;
>      }
>  
> -  COMPILE_TIME_ASSERT (sizeof (newdev->uuid) >= sizeof (uuid));
> +  COMPILE_TIME_ASSERT (sizeof (newdev->uuid) >= sizeof (header.uuid));
>    return newdev;
>  }
>  
> diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> index 632309e3c..3c571b7fd 100644
> --- a/grub-core/disk/luks2.c
> +++ b/grub-core/disk/luks2.c
> @@ -356,7 +356,7 @@ luks2_scan (grub_disk_t disk, const char *check_uuid, int 
> check_boot)
>        return NULL;
>      }
>  
> -  if (check_uuid && grub_strcasecmp (check_uuid, header.uuid) != 0)
> +  if (check_uuid && grub_uuidcasecmp (check_uuid, header.uuid) != 0)
>      return NULL;
>  
>    cryptodisk = grub_zalloc (sizeof (*cryptodisk));
> diff --git a/include/grub/misc.h b/include/grub/misc.h
> index b7ca6dd58..3f0f42c22 100644
> --- a/include/grub/misc.h
> +++ b/include/grub/misc.h
> @@ -243,6 +243,27 @@ grub_strncasecmp (const char *s1, const char *s2, 
> grub_size_t n)
>      - (int) grub_tolower ((grub_uint8_t) *s2);
>  }
>  
> +static inline int
> +grub_uuidcasecmp (const char *uuid1, const char *uuid2)
> +{
> +  while (*uuid1 && *uuid2)
> +    {
> +      while (*uuid1 == '-')
> +        uuid1++;
> +      while (*uuid2 == '-')
> +        uuid2++;
> +
> +      if (grub_tolower (*uuid1) != grub_tolower (*uuid2))
> +     break;
> +
> +      uuid1++;
> +      uuid2++;
> +    }
> +
> +  return (int) grub_tolower ((grub_uint8_t) *uuid1)
> +    - (int) grub_tolower ((grub_uint8_t) *uuid2);
> +}
> +
>  /*
>   * Note that these differ from the C standard's definitions of strtol,
>   * strtoul(), and strtoull() by the addition of two const qualifiers on the 
> end
> -- 
> 2.25.1
>

signature.asc
Description: PGP signature