From 912384e63c1e3b6aa9d90effb71cd535a17da1e2 Mon Sep 17 00:00:00 2001
From: Michael Chang <mchang@suse.com>
Date: Sat, 18 Nov 2023 19:02:31 +0800
Subject: [PATCH 2/4] Restrict file access on cryptodisk print

When the encrypted partition is automatically unlocked by TPM, granting
access to the system upon validation of its known good state, there's a
potential vulnerability. Grub gains access to file systems that were
previously inaccessible to the public, enabling certain commands from
the grub console to print content. This arises due to grub lacking
restrictions similar to those imposed by password authentication, which
typically occurs before privileged access is granted.

Although the automatic unlocking process ensures system integrity and a
secure environment for grub to operate in, it doesn't directly address
the issue of authentication for viewing encrypted partition content.

This commit addresses this security loophole by implementing a file
filter upon adding a TPM key. The newly added file filter will
specifically verify if the disk is encrypted, denying access and
returning an "Access Denied: prohibited to view encrypted data" error
message to alert the user.

Since the policy to filter out unwanted commands from leaking encrypted
content is irreversible, it is advisable to make the loaded module
persistent to prevent its removal.

This enhancement aims to bolster security measures and prevent
unauthorized access to encrypted data.

Signed-Off-by Michael Chang <mchang@suse.com>
---
 grub-core/commands/crypttab.c | 35 ++++++++++++++++++++++++++++++++++-
 grub-core/disk/diskfilter.c   | 35 +++++++++++++++++++++++++++++++++++
 include/grub/disk.h           | 10 ++++++++++
 include/grub/file.h           |  1 +
 4 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/grub-core/commands/crypttab.c b/grub-core/commands/crypttab.c
index 9397bede9..d3acc4b59 100644
--- a/grub-core/commands/crypttab.c
+++ b/grub-core/commands/crypttab.c
@@ -6,11 +6,39 @@
 #include <grub/mm.h>
 #include <grub/list.h>
 #include <grub/crypttab.h>
+#include <grub/file.h>
 
 GRUB_MOD_LICENSE ("GPLv3+");
 
 grub_crypto_key_list_t *cryptokey_lst;
 
+static grub_file_t
+grub_nocat_open (grub_file_t io, enum grub_file_type type)
+{
+  grub_disk_t disk;
+
+  /* Network device */
+  if (!io->device->disk)
+    return io;
+
+  disk = io->device->disk; 
+
+  if (grub_disk_is_crypto (disk))
+    {
+      switch (type & GRUB_FILE_TYPE_MASK)
+	{
+	  case GRUB_FILE_TYPE_CAT:
+	  case GRUB_FILE_TYPE_HEXCAT:
+	    grub_error (GRUB_ERR_ACCESS_DENIED, N_("prohibited to view encrypted data"));
+	    return NULL;
+	  default:
+	    break;
+	}
+    }
+
+  return io;
+}
+
 grub_err_t
 grub_cryptokey_add_or_update (const char *uuid, const char *key, grub_size_t key_len, const char *path, int is_tpmkey)
 {
@@ -48,7 +76,11 @@ grub_cryptokey_add_or_update (const char *uuid, const char *key, grub_size_t key
     }
 
   if (is_tpmkey >= 0)
-    cur->is_tpmkey = is_tpmkey;
+    {
+      cur->is_tpmkey = is_tpmkey;
+      if (is_tpmkey)
+	grub_file_filter_register (GRUB_FILE_FILTER_NOCAT, grub_nocat_open);
+    }
 
   if (!cur->name)
     {
@@ -121,6 +153,7 @@ GRUB_MOD_INIT(crypttab)
 {
   cmd = grub_register_command ("crypttab_entry", grub_cmd_crypttab_entry,
 			       N_("VOLUME-NAME ENCRYPTED-DEVICE KEY-FILE") , N_("No description"));
+  grub_dl_set_persistent (mod);
 }
 
 GRUB_MOD_FINI(crypttab)
diff --git a/grub-core/disk/diskfilter.c b/grub-core/disk/diskfilter.c
index 5c5fabe1a..b0c1c880d 100644
--- a/grub-core/disk/diskfilter.c
+++ b/grub-core/disk/diskfilter.c
@@ -558,6 +558,39 @@ find_lv (const char *name)
   return NULL;
 }
 
+static int
+grub_diskfilter_has_cryptodisk (const struct grub_diskfilter_lv *lv)
+{
+  struct grub_diskfilter_pv *pv;
+
+  if (!lv)
+    return 0;
+
+  if (lv->vg->pvs)
+    for (pv = lv->vg->pvs; pv; pv = pv->next)
+      {
+	if (!pv->disk)
+	{
+	  grub_dprintf ("diskfilter", _("Couldn't find physical volume `%s'."
+			" Some modules may be missing from core image."),
+			pv->name);
+	  continue;
+	}
+
+	switch (pv->disk->dev->id)
+	  {
+	    case GRUB_DISK_DEVICE_CRYPTODISK_ID:
+	      return 1;
+	    case GRUB_DISK_DEVICE_DISKFILTER_ID:
+	      return grub_diskfilter_has_cryptodisk (pv->disk->data);
+	    default:
+	      break;
+	  }
+      }
+
+  return 0;
+}
+
 static grub_err_t
 grub_diskfilter_open (const char *name, grub_disk_t disk)
 {
@@ -589,6 +622,8 @@ grub_diskfilter_open (const char *name, grub_disk_t disk)
 
   disk->total_sectors = lv->size;
   disk->max_agglomerate = GRUB_DISK_MAX_MAX_AGGLOMERATE;
+  disk->is_crypto_diskfilter = grub_diskfilter_has_cryptodisk (lv);
+
   return 0;
 }
 
diff --git a/include/grub/disk.h b/include/grub/disk.h
index 3b3db6222..63982f16c 100644
--- a/include/grub/disk.h
+++ b/include/grub/disk.h
@@ -147,6 +147,8 @@ struct grub_disk
 
   /* Device-specific data.  */
   void *data;
+
+  int is_crypto_diskfilter;
 };
 typedef struct grub_disk *grub_disk_t;
 
@@ -314,4 +316,12 @@ void grub_mdraid1x_fini (void);
 void grub_diskfilter_fini (void);
 #endif
 
+static inline int
+grub_disk_is_crypto (grub_disk_t disk)
+{
+  return ((disk->is_crypto_diskfilter ||
+	   disk->dev->id == GRUB_DISK_DEVICE_CRYPTODISK_ID) ?
+	  1 : 0);
+}
+
 #endif /* ! GRUB_DISK_HEADER */
diff --git a/include/grub/file.h b/include/grub/file.h
index fde58f0fa..fcfd32ce2 100644
--- a/include/grub/file.h
+++ b/include/grub/file.h
@@ -185,6 +185,7 @@ extern grub_disk_read_hook_t EXPORT_VAR(grub_file_progress_hook);
 /* Filters with lower ID are executed first.  */
 typedef enum grub_file_filter_id
   {
+    GRUB_FILE_FILTER_NOCAT,
     GRUB_FILE_FILTER_VERIFY,
     GRUB_FILE_FILTER_GZIO,
     GRUB_FILE_FILTER_XZIO,
-- 
2.42.1