[prev in list] [next in list] [prev in thread] [next in thread]
List: grub-devel
Subject: Re: [CRYPTO-LUKS v1 08/19] cryptodisk, luks: Allow special processing for comparing UUIDs.
From: Patrick Steinhardt <ps () pks ! im>
Date: 2020-07-31 15:34:18
Message-ID: 20200731153418.GA11731 () tanuki ! pks ! im
[Download RAW message or body]
[Attachment #2 (multipart/signed)]
On Fri, Jul 31, 2020 at 07:01:49AM -0500, Glenn Washburn wrote:
> 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.
>
> 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 847337072..e7a4b8082 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)
I don't think the header's UUID is NUL-terminated. So if
`strlen(check_uuid) > sizeof(header.uuid)`, we'll overflow the header.
We should probably add a size parameter to `grub_uuidcasecmp` to fix
this.
> {
> - 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 41329f745..7f5deb469 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;
Same over here.
Patrick
["signature.asc" (application/pgp-signature)]
[Attachment #6 (text/plain)]
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic