Re: [PATCH 5/5] Cryptomount support for hyphens in UUID

[Andrei Borzenkov]

I still believe that generally ignoring hyphens for every future crypto
implementation is wrong. In future we simply should avoid mangling
UUID. So this should be restricted to LUKS only, where the problem
exists.

В Mon, 29 Jun 2015 15:31:00 +0100
John Lane <address@hidden> пишет:

> From: John Lane <address@hidden>
> 
> ---
>  grub-core/disk/cryptodisk.c | 20 +++++++++++++++++---
>  grub-core/disk/luks.c       | 26 ++++++++------------------
>  include/grub/cryptodisk.h   |  2 ++
>  3 files changed, 27 insertions(+), 21 deletions(-)
> 
> diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> index cd5cfc9..d36d16b 100644
> --- a/grub-core/disk/cryptodisk.c
> +++ b/grub-core/disk/cryptodisk.c
> @@ -113,6 +113,20 @@ gf_mul_be (grub_uint8_t *o, const grub_uint8_t *a, const 
> grub_uint8_t *b)
>      }
>  }
>  
> +int
> +grub_cryptodisk_uuidcmp(char *uuid_a, char *uuid_b)
> +{
> +  while ((*uuid_a != '\0') && (*uuid_b != '\0'))
> +    {
> +      while (*uuid_a == '-') uuid_a++;
> +      while (*uuid_b == '-') uuid_b++;
> +      if (grub_toupper(*uuid_a) != grub_toupper(*uuid_b)) break;
> +      uuid_a++;
> +      uuid_b++;
> +    }
> +  return (*uuid_a == '\0') && (*uuid_b == '\0');
> +}
> +
>  static gcry_err_code_t
>  grub_crypto_pcbc_decrypt (grub_crypto_cipher_handle_t cipher,
>                        void *out, void *in, grub_size_t size,
> @@ -507,8 +521,8 @@ 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)
> -       break;
> +        if (grub_cryptodisk_uuidcmp(name + sizeof ("cryptouuid/") - 1, 
> dev->uuid))
> +          break;
>      }
>    else
>      {
> @@ -739,7 +753,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_cryptodisk_uuidcmp(dev->uuid, uuid))
>        return dev;
>    return NULL;
>  }
> diff --git a/grub-core/disk/luks.c b/grub-core/disk/luks.c
> index 4ebe21b..80a7606 100644
> --- a/grub-core/disk/luks.c
> +++ b/grub-core/disk/luks.c
> @@ -68,9 +68,7 @@ configure_ciphers (grub_disk_t disk, const char *check_uuid,
>                  int check_boot, grub_file_t hdr)
>  {
>    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];
> @@ -104,22 +102,6 @@ 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 (*iptr != '-')
> -        *optr++ = *iptr;
> -    }
> -  *optr = 0;
> -
> -  if (check_uuid && grub_strcasecmp (check_uuid, uuid) != 0)
> -    {
> -      grub_dprintf ("luks", "%s != %s\n", uuid, check_uuid);
> -      return NULL;
> -    }
> -
> -
>    /* Make sure that strings are null terminated.  */
>    grub_memcpy (ciphername, header.cipherName, sizeof (header.cipherName));
>    ciphername[sizeof (header.cipherName)] = 0;
> @@ -127,6 +109,14 @@ configure_ciphers (grub_disk_t disk, const char 
> *check_uuid,
>    ciphermode[sizeof (header.cipherMode)] = 0;
>    grub_memcpy (hashspec, header.hashSpec, sizeof (header.hashSpec));
>    hashspec[sizeof (header.hashSpec)] = 0;
> +  grub_memcpy (uuid, header.uuid, sizeof (header.uuid));
> +  uuid[sizeof (header.uuid)] = 0;
> +
> +  if ( check_uuid && ! grub_cryptodisk_uuidcmp(check_uuid, uuid))
> +    {
> +      grub_dprintf ("luks", "%s != %s\n", uuid, check_uuid);
> +      return NULL;
> +    }
>  
>    newdev = grub_cryptodisk_create (disk, uuid, ciphername, ciphermode, 
> hashspec);
>  
> diff --git a/include/grub/cryptodisk.h b/include/grub/cryptodisk.h
> index 4076412..a564f2c 100644
> --- a/include/grub/cryptodisk.h
> +++ b/include/grub/cryptodisk.h
> @@ -167,4 +167,6 @@ grub_cryptodisk_t grub_cryptodisk_get_by_source_disk 
> (grub_disk_t disk);
>  grub_cryptodisk_t grub_cryptodisk_create (grub_disk_t disk, char *uuid,
>                                  char *ciphername, char *ciphermode, char 
> *digest);
>  
> +int
> +grub_cryptodisk_uuidcmp(char *uuid_a, char *uuid_b);
>  #endif