All of lore.kernel.org
 help / color / mirror / Atom feed
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>, Daniel Kiper <daniel.kiper@oracle.com>,
	Hernan Gatta <hegatta@linux.microsoft.com>,
	Daniel Axtens <dja@axtens.net>,
	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>,
	Vladimir Serbinenko <phcoder@gmail.com>,
	Fabian Vogt <fvogt@suse.com>
Subject: [PATCH v23 30/33] diskfilter: look up cryptodisk devices first
Date: Fri, 15 Nov 2024 15:34:58 +0800	[thread overview]
Message-ID: <20241115073506.2784-31-glin@suse.com> (raw)
In-Reply-To: <20241115073506.2784-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/grub/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 GRUB 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 GRUB 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>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
 grub-core/disk/diskfilter.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/grub-core/disk/diskfilter.c b/grub-core/disk/diskfilter.c
index 21e239511..606195c26 100644
--- a/grub-core/disk/diskfilter.c
+++ b/grub-core/disk/diskfilter.c
@@ -226,15 +226,28 @@ 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_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.43.0


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

  parent reply	other threads:[~2024-11-15  7:38 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-15  7:34 [PATCH v23 00/33] Automatic Disk Unlock with TPM2 Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 01/33] posix_wrap: tweaks in preparation for libtasn1 Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 02/33] libtasn1: import libtasn1-4.19.0 Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 03/33] libtasn1: disable code not needed in grub Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 04/33] libtasn1: replace strcat() with strcpy() in _asn1_str_cat() Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 05/33] libtasn1: replace strcat() with _asn1_str_cat() Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 06/33] libtasn1: adjust the header paths in libtasn1.h Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 07/33] libtasn1: Use grub_divmod64() for division Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 08/33] libtasn1: fix the potential buffer overrun Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 09/33] asn1_test: include asn1_test.h only Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 10/33] asn1_test: rename the main functions to the test names Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 11/33] asn1_test: return either 0 or 1 to reflect the results Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 12/33] asn1_test: remove 'verbose' and the unnecessary printf() Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 13/33] asn1_test: print the error messages with grub_printf() Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 14/33] asn1_test: use the grub-specific functions and types Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 15/33] asn1_test: enable the testcase only when GRUB_LONG_MAX is larger than GRUB_INT_MAX Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 16/33] libtasn1: compile into asn1 module Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 17/33] asn1_test: test module for libtasn1 Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 18/33] libtasn1: Add the documentation Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 19/33] key_protector: Add key protectors framework Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 20/33] tss2: Add TPM2 buffer handling functions Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 21/33] tss2: Add TPM2 types and Marshal/Unmarshal functions Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 22/33] tss2: Add TPM2 Software Stack (TSS2) support Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 23/33] key_protector: Add TPM2 Key Protector Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 24/33] cryptodisk: Support key protectors Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 25/33] util/grub-protect: Add new tool Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 26/33] tpm2_key_protector: Support authorized policy Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 27/33] tpm2_key_protector: Implement NV index Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 28/33] cryptodisk: Fallback to passphrase Gary Lin via Grub-devel
2024-11-15  7:34 ` [PATCH v23 29/33] cryptodisk: wipe out the cached keys from protectors Gary Lin via Grub-devel
2024-11-15  7:34 ` Gary Lin via Grub-devel [this message]
2024-11-15  7:34 ` [PATCH v23 31/33] tpm2_key_protector: Add grub-emu support Gary Lin via Grub-devel
2024-11-15  7:35 ` [PATCH v23 32/33] tests: Add tpm2_key_protector_test Gary Lin via Grub-devel
2024-11-15  7:35 ` [PATCH v23 33/33] docs: Document TPM2 key protector Gary Lin via Grub-devel
2024-11-19 15:15 ` [PATCH v23 00/33] Automatic Disk Unlock with TPM2 Stefan Berger
2024-11-25 17:21   ` Daniel Kiper via Grub-devel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241115073506.2784-31-glin@suse.com \
    --to=grub-devel@gnu.org \
    --cc=christopher.co@microsoft.com \
    --cc=daniel.kiper@oracle.com \
    --cc=daniel.mihai@microsoft.com \
    --cc=development@efficientek.com \
    --cc=dja@axtens.net \
    --cc=fvogt@suse.com \
    --cc=glin@suse.com \
    --cc=hegatta@linux.microsoft.com \
    --cc=jaredz@redhat.com \
    --cc=jaskaran.khurana@microsoft.com \
    --cc=jejb@linux.ibm.com \
    --cc=mchang@suse.com \
    --cc=patrick.colp@oracle.com \
    --cc=phcoder@gmail.com \
    --cc=shkhisti@microsoft.com \
    --cc=stefanb@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.