From a47bcbb09936813028fb5257805d85c48e08ee52 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Tue, 23 Oct 2012 10:45:04 -0400 Subject: [PATCH] Do a better job of preventing insmod on secure boot systems. Signed-off-by: Peter Jones --- grub-2.00-no-insmod-on-sb.patch | 73 +++++++++++++++++++++------------ 1 file changed, 46 insertions(+), 27 deletions(-) diff --git a/grub-2.00-no-insmod-on-sb.patch b/grub-2.00-no-insmod-on-sb.patch index 828ce81..1bfd6f8 100644 --- a/grub-2.00-no-insmod-on-sb.patch +++ b/grub-2.00-no-insmod-on-sb.patch @@ -1,43 +1,62 @@ -From 7a65d7b558974c89f19afaf0d78b54dc0327f56c Mon Sep 17 00:00:00 2001 -From: Matthew Garrett -Date: Wed, 15 Aug 2012 09:53:05 -0400 -Subject: [PATCH] Don't permit insmod on secure boot +From 8a2a8d6021d926f00c5f85dab2d66f4ed8be86a2 Mon Sep 17 00:00:00 2001 +From: Colin Watson +Date: Tue, 23 Oct 2012 10:40:49 -0400 +Subject: [PATCH] Don't allow insmod when secure boot is enabled. +Hi, + +Fedora's patch to forbid insmod in UEFI Secure Boot environments is fine +as far as it goes. However, the insmod command is not the only way that +modules can be loaded. In particular, the 'normal' command, which +implements the usual GRUB menu and the fully-featured command prompt, +will implicitly load commands not currently loaded into memory. This +permits trivial Secure Boot violations by writing commands implementing +whatever you want to do and pointing $prefix at the malicious code. + +I'm currently test-building this patch (replacing your current +grub-2.00-no-insmod-on-sb.patch), but this should be more correct. It +moves the check into grub_dl_load_file. --- - grub-core/kern/corecmd.c | 9 +++++++++ + grub-core/kern/dl.c | 17 +++++++++++++++++ grub-core/kern/efi/efi.c | 28 ++++++++++++++++++++++++++++ include/grub/efi/efi.h | 1 + - 3 files changed, 38 insertions(+) + 3 files changed, 46 insertions(+) -diff --git a/grub-core/kern/corecmd.c b/grub-core/kern/corecmd.c -index eec575c..3df9dbd 100644 ---- a/grub-core/kern/corecmd.c -+++ b/grub-core/kern/corecmd.c -@@ -28,6 +28,10 @@ - #include - #include +diff --git a/grub-core/kern/dl.c b/grub-core/kern/dl.c +index a498682..2578fce 100644 +--- a/grub-core/kern/dl.c ++++ b/grub-core/kern/dl.c +@@ -43,6 +43,10 @@ + #include + #endif +#ifdef GRUB_MACHINE_EFI +#include +#endif + - /* set ENVVAR=VALUE */ - static grub_err_t - grub_core_cmd_set (struct grub_command *cmd __attribute__ ((unused)), -@@ -81,6 +85,13 @@ grub_core_cmd_insmod (struct grub_command *cmd __attribute__ ((unused)), - { - grub_dl_t mod; + + + #pragma GCC diagnostic ignored "-Wcast-align" +@@ -721,6 +725,19 @@ grub_dl_load_file (const char *filename) + void *core = 0; + grub_dl_t mod = 0; +#ifdef GRUB_MACHINE_EFI -+ if (grub_efi_secure_boot()) { -+ //grub_printf("%s\n", N_("Secure Boot forbids insmod")); -+ return 0; -+ } ++ if (grub_efi_secure_boot ()) ++ { ++#if 0 ++ /* This is an error, but grub2-mkconfig still generates a pile of ++ * insmod commands, so emitting it would be mostly just obnoxious. */ ++ grub_error (GRUB_ERR_ACCESS_DENIED, ++ "Secure Boot forbids loading module from %s", filename); ++#endif ++ return 0; ++ } +#endif + - if (argc == 0) - return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("one argument expected")); - + file = grub_file_open (filename); + if (! file) + return 0; diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c index 820968f..ad7aa8d 100644 --- a/grub-core/kern/efi/efi.c @@ -90,5 +109,5 @@ index 9370fd5..a000c38 100644 EXPORT_FUNC (grub_efi_compare_device_paths) (const grub_efi_device_path_t *dp1, const grub_efi_device_path_t *dp2); -- -1.7.11.2 +1.7.12.1