From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1lO2w8-0002ts-FM for mharc-grub-devel@gnu.org; Sun, 21 Mar 2021 14:36:28 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:51286) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lO2w6-0002tB-K9 for grub-devel@gnu.org; Sun, 21 Mar 2021 14:36:26 -0400 Received: from mail-ot1-x334.google.com ([2607:f8b0:4864:20::334]:45693) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lO2vz-0005ox-5y for grub-devel@gnu.org; Sun, 21 Mar 2021 14:36:26 -0400 Received: by mail-ot1-x334.google.com with SMTP id f73-20020a9d03cf0000b02901b4d889bce0so13742343otf.12 for ; Sun, 21 Mar 2021 11:36:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=efficientek-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=FS3XkSWqxIHo0n8/LzU89+9yHAl68R9kRfd9FwNwPpc=; b=Un28MvZ4aOH92FEuE9iV1YwZvzEkf/DQ+hDDsymgxBIkhoVaucss3siIE0Mjnto9dk Doydvkjcv8BivOS2eKMbl3VmW2RBUPC7vAkEAjszeatoi7Oi20OEB+uyBrc2H+JECnEH pZqQEhR9aJ08OOdIDSc4CiPgBXZFA9PHyoaLgjYZE9znshhmPz4FmPOaAKD1zzl1FcSK cJYgL/cSn5tmbn8pRvIUu2wMbm5GM77aKZCa/7tE1P5MnZyHRIFoiQKsOnpXqfXz+Pix sM3/GRMBiJupFkTNWhT5aTqYtbI21+uRhxBmIgMeosQxzxAoUJ1JZcRx9+rs3mjNUoRC nZiA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=FS3XkSWqxIHo0n8/LzU89+9yHAl68R9kRfd9FwNwPpc=; b=Ap/I8bq50kANZh1/TOIn6d/o4/lnSFnPq2kCHrJpyv7tsxDPaHeshFY7bVVFOLUn+0 7hzwyIQesunQ3O171KqIer2am6jrWEOE2Oqwb2eqpRk5Cy7QtcuIAiFVNADdsX2bPj89 5UBKRRjT1U6RTM1GrZyrgQ3+nXFiY92SvrToirTTemIfd5B0RacVpAgC3dGkIPd4HZn8 QK0z2rBMc89rCkjmv5nCKTGzQKnSow36qzt9qxlyQYY8GuBPzYgAOBHfVGIb/uU15Kah gMSOLQ9WCtMFrgk/PCbs6ZyEfFmdEfzheVfSYHVKq3OCVvhxIHpi0i3q7tqK6JZqKnzP Jdfg== X-Gm-Message-State: AOAM530+UV8IR1HFhbpfaf/3K+dbUEFYL+vg2dXtfyPuVg4mMZzhEQxs 0KD43b43wOi5EAGLFVLeMPTelFvHggugxQ== X-Google-Smtp-Source: ABdhPJy1CK5G5bP2+PfFbi6KCdVrQ82tWdNAwOZk/9MuprkpSUdY+0d9+1B8Oy3e1VhP0HLzs6bm2w== X-Received: by 2002:a9d:3b0a:: with SMTP id z10mr8594804otb.326.1616351777330; Sun, 21 Mar 2021 11:36:17 -0700 (PDT) Received: from localhost.localdomain ([2605:a601:ab16:db00:23af:6780:efd4:8e25]) by smtp.gmail.com with ESMTPSA id m126sm2676061oig.31.2021.03.21.11.36.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 21 Mar 2021 11:36:16 -0700 (PDT) From: Glenn Washburn To: grub-devel@gnu.org, Daniel Kiper Cc: Glenn Washburn Subject: [PATCH] cryptodisk: Allows UUIDs to be compared in a dash-insensitive manner Date: Sun, 21 Mar 2021 13:36:06 -0500 Message-Id: <20210321183606.1829782-1-development@efficientek.com> X-Mailer: git-send-email 2.27.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Received-SPF: pass client-ip=2607:f8b0:4864:20::334; envelope-from=development@efficientek.com; helo=mail-ot1-x334.google.com X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 21 Mar 2021 18:36:26 -0000 A user can now specify UUID strings with dashes, instead of having to remove dashes. This is backwards-compatability preserving and also fixes a source of user confusion over the inconsistency with how UUIDs are specified between file system UUIDs and cryptomount UUIDs. Since cryptsetup, the reference implementation for LUKS, displays and generates UUIDs with dashes there has been additional confusion when using the UUID strings from cryptsetup as exact input into GRUB does not find the expected cryptodisk. A new function grub_uuidcasecmp is added that is general enough to be used other places where UUIDs are being compared. Signed-off-by: Glenn Washburn --- I have another version of grub_uuidcasecmp which is more efficient by not copying the UUID bytes to another buffer to strip out the dashes. This also implies that it does not require null-terminated strings as inputs because one can use the n parameter to prevent access beyond length of array. However, I'm not sure we're so memory constrained that the added complexity is worth it here, so I've chosen to submit this simpler version. If that assumption isn't valid, I can submit the more complex version. This current version uses two extra buffer copies, one to copy the UUID out of the LUKS header to ensure the string is null-terminated (unnecessary in geli), as this version of grub_uuidcasecmp expects, and another in grub_uuidcasecmp to strip out dashes so that grub_strncasecmp can be called. Another benefit of this is that the complexity of UUID processing is moved into one place as opposed to each cryptodisk module potentially having that complexity (eg. current code duplication in LUKS1 and LUKS2 modules). Glenn --- grub-core/disk/cryptodisk.c | 4 ++-- grub-core/disk/geli.c | 2 +- grub-core/disk/luks.c | 16 +++------------- grub-core/disk/luks2.c | 9 +++------ include/grub/misc.h | 27 +++++++++++++++++++++++++++ 5 files changed, 36 insertions(+), 22 deletions(-) diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c index 9e9c256fc..a8ae2b541 100644 --- a/grub-core/disk/cryptodisk.c +++ b/grub-core/disk/cryptodisk.c @@ -683,7 +683,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, sizeof (dev->uuid)) == 0) break; } else @@ -917,7 +917,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, sizeof (dev->uuid)) == 0) return dev; return NULL; } diff --git a/grub-core/disk/geli.c b/grub-core/disk/geli.c index e0223e4fa..fa1ab849e 100644 --- a/grub-core/disk/geli.c +++ b/grub-core/disk/geli.c @@ -310,7 +310,7 @@ geli_scan (grub_disk_t disk, const char *check_uuid, int boot_only, return NULL; } - if (check_uuid && grub_strcasecmp (check_uuid, uuid) != 0) + if (check_uuid && grub_uuidcasecmp (check_uuid, uuid) != 0) { grub_dprintf ("geli", "%s != %s\n", uuid, check_uuid); return NULL; diff --git a/grub-core/disk/luks.c b/grub-core/disk/luks.c index b69443caa..cf7bee1a7 100644 --- a/grub-core/disk/luks.c +++ b/grub-core/disk/luks.c @@ -70,9 +70,7 @@ luks_scan (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]; @@ -106,17 +104,9 @@ luks_scan (grub_disk_t disk, const char *check_uuid, int check_boot, || grub_be_to_cpu16 (header.version) != 1) return NULL; - grub_memset (uuid, 0, sizeof (uuid)); - 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_memcpy(uuid, header.uuid, sizeof (header.uuid)); + uuid[sizeof(uuid) - 1] = 0; + if (check_uuid && grub_uuidcasecmp (check_uuid, uuid, sizeof (uuid)) != 0) { grub_dprintf ("luks", "%s != %s\n", uuid, check_uuid); return NULL; diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c index 47ff572e0..d7fceecc2 100644 --- a/grub-core/disk/luks2.c +++ b/grub-core/disk/luks2.c @@ -372,7 +372,6 @@ luks2_scan (grub_disk_t disk, const char *check_uuid, int check_boot, grub_cryptodisk_t cryptodisk; grub_luks2_header_t header; char uuid[sizeof (header.uuid) + 1]; - grub_size_t i, j; if (check_boot) return NULL; @@ -383,12 +382,10 @@ luks2_scan (grub_disk_t disk, const char *check_uuid, int check_boot, return NULL; } - for (i = 0, j = 0; i < sizeof (header.uuid); i++) - if (header.uuid[i] != '-') - uuid[j++] = header.uuid[i]; - uuid[j] = '\0'; + grub_memcpy(uuid, header.uuid, sizeof (header.uuid)); + uuid[sizeof(uuid) - 1] = 0; - if (check_uuid && grub_strcasecmp (check_uuid, uuid) != 0) + if (check_uuid && grub_uuidcasecmp (check_uuid, uuid, sizeof (uuid)) != 0) return NULL; cryptodisk = grub_zalloc (sizeof (*cryptodisk)); diff --git a/include/grub/misc.h b/include/grub/misc.h index 6f4624d79..a2a7622cb 100644 --- a/include/grub/misc.h +++ b/include/grub/misc.h @@ -27,6 +27,8 @@ #include #include +#define GRUB_MAX_UUID_LENGTH 71 + #define ALIGN_UP(addr, align) \ (((addr) + (typeof (addr)) (align) - 1) & ~((typeof (addr)) (align) - 1)) #define ALIGN_UP_OVERHEAD(addr, align) ((-(addr)) & ((typeof (addr)) (align) - 1)) @@ -257,6 +259,31 @@ grub_strncasecmp (const char *s1, const char *s2, grub_size_t n) - (int) grub_tolower ((grub_uint8_t) *s2); } +/* + * Do a case insensitive compare of two UUID strings by first stripping out + * all dashes from both UUID arguments and then doing a regular + * grub_strncasecmp. + */ +static inline int +grub_uuidcasecmp (const char *uuid1, const char *uuid2, grub_size_t n) +{ + char *ptr; + char ruuid1[GRUB_MAX_UUID_LENGTH + 1]; + char ruuid2[GRUB_MAX_UUID_LENGTH + 1]; + + for (ptr = ruuid1; *uuid1; uuid1++) + if (*uuid1 != '-') + *(ptr++) = *uuid1; + *ptr = '\0'; + + for (ptr = ruuid2; *uuid2; uuid2++) + if (*uuid2 != '-') + *(ptr++) = *uuid2; + *ptr = '\0'; + + return grub_strncasecmp (ruuid1, ruuid2, n); +} + /* * 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.27.0