[PATCH v13 18/20] diskfilter: look up cryptodisk devices first - Gary Lin via Grub-devel

From: Gary Lin via Grub-devel <grub-devel@gnu.org>
To: The development of GNU GRUB <grub-devel@gnu.org>
Cc: Gary Lin <glin@suse.com>,
	Hernan Gatta <hegatta@linux.microsoft.com>,
	Daniel Axtens <dja@axtens.net>,
	Daniel Kiper <daniel.kiper@oracle.com>,
	shkhisti@microsoft.com, jaskaran.khurana@microsoft.com,
	christopher.co@microsoft.com, daniel.mihai@microsoft.com,
	jaredz@redhat.com, development@efficientek.com,
	jejb@linux.ibm.com, mchang@suse.com, patrick.colp@oracle.com,
	Stefan Berger <stefanb@linux.ibm.com>,
	Fabian Vogt <fvogt@suse.com>
Subject: [PATCH v13 18/20] diskfilter: look up cryptodisk devices first
Date: Thu, 25 Apr 2024 16:02:04 +0800	[thread overview]
Message-ID: <20240425080206.23902-19-glin@suse.com> (raw)
In-Reply-To: <20240425080206.23902-1-glin@suse.com>

When using disk auto-unlocking with TPM 2.0, the typical grub.cfg may
look like this:

  tpm2_key_protector_init --tpm2key=(hd0,gpt1)/boot/grub2/sealed.tpm
  cryptomount -u <PART-UUID> -P tpm2
  search --fs-uuid --set=root <FS-UUID>

Since the disk search order is based on the order of module loading, the
attacker could insert a malicious disk with the same FS-UUID root to
trick grub2 to boot into the malicious root and further dump memory to
steal the unsealed key.

Do defend against such an attack, we can specify the hint provided by
'grub-probe' to search the encrypted partition first:

search --fs-uuid --set=root --hint='cryptouuid/<PART-UUID>' <FS-UUID>

However, for LVM on an encrypted partition, the search hint provided by
'grub-probe' is:

  --hint='lvmid/<VG-UUID>/<LV-UUID>'

It doesn't guarantee to look up the logical volume from the encrypted
partition, so the attacker may have the chance to fool grub2 to boot
into the malicious disk.

To minimize the attack surface, this commit tweaks the disk device search
in diskfilter to look up cryptodisk devices first and then others, so
that the auto-unlocked disk will be found first, not the attacker's disk.

Cc: Fabian Vogt <fvogt@suse.com>
Signed-off-by: Gary Lin <glin@suse.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
---
 grub-core/disk/diskfilter.c | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/grub-core/disk/diskfilter.c b/grub-core/disk/diskfilter.c
index 21e239511..df1992305 100644
--- a/grub-core/disk/diskfilter.c
+++ b/grub-core/disk/diskfilter.c
@@ -226,15 +226,32 @@ scan_devices (const char *arname)
   int need_rescan;
 
   for (pull = 0; pull < GRUB_DISK_PULL_MAX; pull++)
-    for (p = grub_disk_dev_list; p; p = p->next)
-      if (p->id != GRUB_DISK_DEVICE_DISKFILTER_ID
-	  && p->disk_iterate)
-	{
-	  if ((p->disk_iterate) (scan_disk_hook, NULL, pull))
-	    return;
-	  if (arname && is_lv_readable (find_lv (arname), 1))
-	    return;
-	}
+    {
+      /* look up the crytodisk devices first */
+      for (p = grub_disk_dev_list; p; p = p->next)
+	if (p->id == GRUB_DISK_DEVICE_CRYPTODISK_ID
+	    && p->disk_iterate)
+	  {
+	    if ((p->disk_iterate) (scan_disk_hook, NULL, pull))
+	      return;
+	    if (arname && is_lv_readable (find_lv (arname), 1))
+	      return;
+	    break;
+	  }
+
+      /* check the devices other than crytodisk */
+      for (p = grub_disk_dev_list; p; p = p->next)
+	if (p->id == GRUB_DISK_DEVICE_CRYPTODISK_ID)
+	  continue;
+	else if (p->id != GRUB_DISK_DEVICE_DISKFILTER_ID
+	    && p->disk_iterate)
+	  {
+	    if ((p->disk_iterate) (scan_disk_hook, NULL, pull))
+	      return;
+	    if (arname && is_lv_readable (find_lv (arname), 1))
+	      return;
+	  }
+    }
 
   scan_depth = 0;
   need_rescan = 1;
-- 
2.35.3


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
