Skip to content

Commit

Permalink
Don't allow insmod when secure boot is enabled.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
cjwatson authored and vathpela committed Jun 24, 2014
1 parent acc83cf commit fe7b32a
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 0 deletions.
21 changes: 21 additions & 0 deletions grub-core/kern/dl.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@
#define GRUB_MODULES_MACHINE_READONLY
#endif

#ifdef GRUB_MACHINE_EMU
#include <sys/mman.h>
#endif

#ifdef GRUB_MACHINE_EFI
#include <grub/efi/efi.h>
#endif



#pragma GCC diagnostic ignored "-Wcast-align"
Expand Down Expand Up @@ -680,6 +688,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 ())
{
#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

grub_boot_time ("Loading module %s", filename);

file = grub_file_open (filename);
Expand Down
28 changes: 28 additions & 0 deletions grub-core/kern/efi/efi.c
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,34 @@ grub_efi_get_variable (const char *var, const grub_efi_guid_t *guid,
return NULL;
}

grub_efi_boolean_t
grub_efi_secure_boot (void)
{
grub_efi_guid_t efi_var_guid = GRUB_EFI_GLOBAL_VARIABLE_GUID;
grub_size_t datasize;
char *secure_boot = NULL;
char *setup_mode = NULL;
grub_efi_boolean_t ret = 0;

secure_boot = grub_efi_get_variable("SecureBoot", &efi_var_guid, &datasize);

if (datasize != 1 || !secure_boot)
goto out;

setup_mode = grub_efi_get_variable("SetupMode", &efi_var_guid, &datasize);

if (datasize != 1 || !setup_mode)
goto out;

if (*secure_boot && !*setup_mode)
ret = 1;

out:
grub_free (secure_boot);
grub_free (setup_mode);
return ret;
}

#pragma GCC diagnostic ignored "-Wcast-align"

/* Search the mods section from the PE32/PE32+ image. This code uses
Expand Down
1 change: 1 addition & 0 deletions include/grub/efi/efi.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ EXPORT_FUNC (grub_efi_set_variable) (const char *var,
const grub_efi_guid_t *guid,
void *data,
grub_size_t datasize);
grub_efi_boolean_t EXPORT_FUNC (grub_efi_secure_boot) (void);
int
EXPORT_FUNC (grub_efi_compare_device_paths) (const grub_efi_device_path_t *dp1,
const grub_efi_device_path_t *dp2);
Expand Down

0 comments on commit fe7b32a

Please sign in to comment.