[RFC PATCH v4 1/1] kern/dl: Add module vermagic check

2022-12-25 Thread Zhang Boyang
This commit add vermagic string to GRUB modules. Specifically, a unique
but shared vermagic string is embedded in .modver section of each
module, and it is checked at module loading time. The vermagic string is
uniquely generated by configure script in each run (formatted like
"{version} {target}-{platform} {random}", may be changed in future), so
it is different for each individual build.

If GRUB is locked down, modules with mismatched vermagic will be
rejected. This is a prerequisite for implementing external signed
modules securely.

If GRUB isn't locked down, modules can still be loaded even if they have
mismatched vermagic. A warning message will be generated if mismatched
vermagic is detected.

For reproducible builds, the vermagic string can be overridden by
"--with-vermagic=foobar". The value should be chosen carefully to
achieve uniqueness for each individual build. It's recommended to
include vendor, product, version of product, version of GRUB package in
product repository, target of GRUB (e.g. i386, x86_64), platform of GRUB
(e.g. pc, efi, emu) in that string.

For those who want to avoid warnings about mismatched modules, it can be
disabled by "--without-vermagic". However, this option has no effect if
GRUB is locked down (mismatched modules will be always rejected with an
error message in this case).

Signed-off-by: Zhang Boyang 
---
 config.h.in   |  3 +++
 configure.ac  | 24 +
 grub-core/genmod.sh.in| 28 +++--
 grub-core/kern/dl.c   | 39 +++
 util/grub-module-verifierXX.c | 10 +
 5 files changed, 93 insertions(+), 11 deletions(-)

diff --git a/config.h.in b/config.h.in
index 4d1e50eba..dc3b6dc89 100644
--- a/config.h.in
+++ b/config.h.in
@@ -13,6 +13,9 @@
 #define MM_DEBUG @MM_DEBUG@
 #endif
 
+#define GRUB_VERMAGIC_WARNING @vermagic_warning@
+#define GRUB_VERMAGIC_STRING "@vermagic_string@"
+
 /* Define to 1 to enable disk cache statistics.  */
 #define DISK_CACHE_STATS @DISK_CACHE_STATS@
 #define BOOT_TIME_STATS @BOOT_TIME_STATS@
diff --git a/configure.ac b/configure.ac
index 93626b798..87cc654ad 100644
--- a/configure.ac
+++ b/configure.ac
@@ -316,6 +316,25 @@ AC_SUBST(grubdirname)
 AC_DEFINE_UNQUOTED(GRUB_DIR_NAME, "$grubdirname",
 [Default grub directory name])
 
+AC_ARG_WITH([vermagic],
+AS_HELP_STRING([--with-vermagic=STRING],
+   [set vermagic string [[auto-generated]]]),
+  [vermagic_option="$with_vermagic"],
+  [vermagic_option="yes"])
+if test x"$vermagic_option" = xno; then
+  vermagic_warning=0
+  vermagic_string="${PACKAGE_VERSION} ${target_cpu}-${platform}"
+elif test x"$vermagic_option" = xyes; then
+  vermagic_warning=1
+  vermagic_string="${PACKAGE_VERSION} ${target_cpu}-${platform} $(LC_ALL=C tr 
-d -c 0-9A-Za-z < /dev/urandom | head -c 22)"
+else
+  vermagic_warning=1
+  vermagic_string="$vermagic_option"
+fi
+
+AC_SUBST(vermagic_warning)
+AC_SUBST(vermagic_string)
+
 #
 # Checks for build programs.
 #
@@ -2107,6 +2126,11 @@ AC_OUTPUT
 echo "***"
 echo GRUB2 will be compiled with following components:
 echo Platform: "$target_cpu"-"$platform"
+if [ x"$vermagic_warning" = x1 ]; then
+echo Vermagic: "'$vermagic_string'"
+else
+echo Vermagic: "'$vermagic_string'" "(warning disabled)"
+fi
 if [ x"$platform" = xemu ]; then
 if [ x"$grub_emu_sdl_excuse" = x ]; then
 echo SDL support for grub-emu: Yes
diff --git a/grub-core/genmod.sh.in b/grub-core/genmod.sh.in
index e57c4d920..32679bbf0 100644
--- a/grub-core/genmod.sh.in
+++ b/grub-core/genmod.sh.in
@@ -36,22 +36,25 @@ deps=`grep ^$modname: $moddep | sed s@^.*:@@`
 rm -f $tmpfile $outfile
 
 if test x@TARGET_APPLE_LINKER@ != x1; then
-# stripout .modname and .moddeps sections from input module
-@TARGET_OBJCOPY@ -R .modname -R .moddeps $infile $tmpfile
+# stripout .modname and .moddeps and .modver sections from input module
+@TARGET_OBJCOPY@ -R .modname -R .moddeps -R .modver $infile $tmpfile
 
-# Attach .modname and .moddeps sections
+# Attach .modname and .moddeps and .modver sections
 t1=`mktemp "${TMPDIR:-/tmp}/tmp.XX"` || exit 1
 printf "$modname\0" >$t1
 
 t2=`mktemp "${TMPDIR:-/tmp}/tmp.XX"` || exit 1
 for dep in $deps; do printf "$dep\0" >> $t2; done
 
+t3=`mktemp "${TMPDIR:-/tmp}/tmp.XX"` || exit 1
+printf "@vermagic_string@\0" >$t3
+
 if test -n "$deps"; then
-   @TARGET_OBJCOPY@ --add-section .modname=$t1 --add-section .moddeps=$t2 
$tmpfile
+   @TARGET_OBJCOPY@ --add-section .modname=$t1 --add-section .moddeps=$t2 
--add-section .modver=$t3 $tmpfile
 else
-   @TARGET_OBJCOPY@ --add-section .modname=$t1 $tmpfile
+   @TARGET_OBJCOPY@ --add-section .modname=$t1 --add-section .modver=$t3 
$tmpfile
 fi
-rm -f $t1 $t2
+rm -f $t1 $t2 $t3
 
if t

[RFC PATCH v4 0/1] kern/dl: Add module vermagic check

2022-12-25 Thread Zhang Boyang
Hi,

This is the V4 of my patch.

V3 is at:
https://lists.gnu.org/archive/html/grub-devel/2022-12/msg00280.html

V2 is at:
https://lists.gnu.org/archive/html/grub-devel/2022-12/msg00234.html

V1 is at:
https://lists.gnu.org/archive/html/grub-devel/2022-12/msg00213.html


[ TL;DR ]

1) V4 use a string like "2.11 x86_64-efi Zgmcauq40DxP78CHS2fvUf" as "vermagic", 
unlike V3 which only has PACKAGE_VERSION in version string.

2) Modules with mismatched vermagic will be always rejected if GRUB is locked 
down.

3) If GRUB is not locked down, unlike V3, the check is never enforced.

4) If GRUB is not locked down, and GRUB is built with "--without-vermagic", the 
warning about mismatchd modules is disabled. This option has no effect if GRUB 
is locked down.

5) For reproducible builds, the vermagic string can be overridden by 
"--with-vermagic=foobar".


[ Why use vermagic instead of PACKAGE_VERSION ]

PACKAGE_VERSION is not sufficient to make each individual build unique. For 
example, i386-efi and i386-pc shares same PACKAGE_VERSION but they should be 
considered different.

The vermagic approach adopted the idea of ephemeral key which can also be found 
in Linux kernel (suggested by Robbie), but it is much simpler. The vermagic 
string is uniquely generated by configure script in each run, formatted like 
"{version} {target}-{platform} {random}" (may be changed in future). The random 
part of vermagic is a string taken from a space lager than 2**128, so it can be 
considerd unique.

For reproducible builds, the vermagic string can be overridden by 
"--with-vermagic=foobar". It's recommended to include vendor, product, version 
of product, version of GRUB package in product repository, target of GRUB (e.g. 
i386, x86_64), platform of GRUB (e.g. pc, efi, emu) in that string.


[ Why this patch is useful, even for BIOS boot ]

Because it helps people diagnose broken (or improper) GRUB installations.

For example, if you google "452: out of range pointer", you will got a lot of 
results in 2022. I think the most of them are related to mismatched modules. 
However, these problem are often not properly diagnosed because they disappear 
magically, e.g. update whole system (which triggers grub reinstall). There are 
several people even suspect there are problems with their hard disk / BIOS. 
However, the root cause is 052e6068be62 ("mm: When adding a region, merge with 
region after as well as before") changed the layout of `struct grub_mm_region`, 
which is both used in main program and "relocator.mod", so the module reads the 
wrong field and crashes GRUB. Please note the commit did nothing wrong because 
there is no API/ABI compatibility guarantees in GRUB.

If there are warning messages about mismatched modules, user will easily notice 
there are problems with their GRUB installation.


[ Why not enforce this check to prevent crashes ]

As Glenn & Pete said, most mismatched modules isn't harmful. At most times, 
GRUB with mismatched modules can boot Linux happily, even if these modules come 
from another Linux distribution. This also enables user to fix his/her GRUB 
installation without using a boot/rescue disk, because the user can boot the 
existing Linux using the existing (but improperly installed) GRUB.


[ Why warning can be disabled ]

Some tools like Rufus relies on mismatched modules. Some advanced users also 
doesn't like redundant warnings for their existing known-to-work configurations.

However, it's highly unrecommended to disable this warning.


[ Why this patch is a prerequisite for external signed module support ]

Consider this scenario:

1) GRUB 2.XX is free of vulnerabilities

2) GRUB 2.YY is also free of vulnerabilities

3) So GRUB 2.XX shares same SBAT numbers with GRUB 2.YY, therefore SBAT can't 
help in vermagic check

4) If there is no vermagic check, it's possible to load GRUB 2.YY modules into 
GRUB 2.XX (and vice versa)

5) However, due to some changes in API or ABI, although unlikely, there is 
possibility that there are vulnerabilities when using GRUB 2.YY modules with 
GRUB 2.XX (and vice versa)

6) So we must enforce vermagic check to prevent this from happening


Best Regards,
Zhang Boyang


Zhang Boyang (1):
  kern/dl: Add module vermagic check

 config.h.in   |  3 +++
 configure.ac  | 24 +
 grub-core/genmod.sh.in| 28 +++--
 grub-core/kern/dl.c   | 39 +++
 util/grub-module-verifierXX.c | 10 +
 5 files changed, 93 insertions(+), 11 deletions(-)

-- 
2.30.2


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