Script 'mail_helper' called by obssrc
Hello community,

here is the log from the commit of package grub2 for openSUSE:Factory checked 
in at 2023-03-22 22:29:15
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Comparing /work/SRC/openSUSE:Factory/grub2 (Old)
 and      /work/SRC/openSUSE:Factory/.grub2.new.31432 (New)
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

Package is "grub2"

Wed Mar 22 22:29:15 2023 rev:289 rq:1073668 version:2.06

Changes:
--------
--- /work/SRC/openSUSE:Factory/grub2/grub2.changes      2023-03-17 
17:02:39.957178526 +0100
+++ /work/SRC/openSUSE:Factory/.grub2.new.31432/grub2.changes   2023-03-22 
22:29:17.617786131 +0100
@@ -1,0 +2,15 @@
+Mon Mar 20 05:02:01 UTC 2023 - Michael Chang <mch...@suse.com>
+
+- Restrict cryptsetup key file permission for better security (bsc#1207499)
+  * 0001-loader-linux-Ensure-the-newc-pathname-is-NULL-termin.patch
+  * 0002-Restrict-cryptsetup-key-file-permission-for-better-s.patch
+
+-------------------------------------------------------------------
+Wed Mar 15 21:46:00 UTC 2023 - Hans-Peter Jansen <h...@urpla.net>
+
+- Meanwhile, memtest86+ gained EFI support, but using the grub
+  command line to run it manually is quite tedious...
+  Adapt 20_memtest86+ to provide a proper menu entry. Executing
+  memtest requires to turn security off in BIOS: (Boot Mode: Other OS).
+
+-------------------------------------------------------------------

New:
----
  0001-loader-linux-Ensure-the-newc-pathname-is-NULL-termin.patch
  0002-Restrict-cryptsetup-key-file-permission-for-better-s.patch

++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

Other differences:
------------------
++++++ grub2.spec ++++++
--- /var/tmp/diff_new_pack.GA5tJj/_old  2023-03-22 22:29:20.697801630 +0100
+++ /var/tmp/diff_new_pack.GA5tJj/_new  2023-03-22 22:29:20.701801650 +0100
@@ -503,6 +503,8 @@
 Patch975:       0002-discard-cached-key-before-entering-grub-shell-and-ed.patch
 # Make grub more robust against storage race condition causing system boot 
failures (bsc#1189036)
 Patch976:       0001-ieee1275-ofdisk-retry-on-open-and-read-failure.patch
+Patch977:       0001-loader-linux-Ensure-the-newc-pathname-is-NULL-termin.patch
+Patch978:       0002-Restrict-cryptsetup-key-file-permission-for-better-s.patch
 
 Requires:       gettext-runtime
 %if 0%{?suse_version} >= 1140
@@ -528,6 +530,10 @@
 %ifarch ppc64 ppc64le
 Requires:       powerpc-utils
 %endif
+%ifarch %{ix86}
+# meanwhile, memtest is available as EFI executable
+Recommends:     memtest86+
+%endif
 
 %if 0%{?only_x86_64:1}
 ExclusiveArch:  x86_64

++++++ 0001-loader-linux-Ensure-the-newc-pathname-is-NULL-termin.patch ++++++
>From 1dcab5bf3843abc997f7e7dba32e5dbcb9bf66b2 Mon Sep 17 00:00:00 2001
From: Gary Lin <g...@suse.com>
Date: Fri, 25 Nov 2022 15:37:35 +0800
Subject: [PATCH 1/2] loader/linux: Ensure the newc pathname is NULL-terminated

Per "man 5 cpio", the namesize in the cpio header includes the trailing
NUL byte of the pathname and the pathname is followed by NUL bytes, but
the current implementation ignores the trailing NUL byte when making
the newc header. Although make_header() tries to pad the pathname string,
the padding won't happen when strlen(name) + sizeof(struct newc_head)
is a multiple of 4, and the non-NULL-terminated pathname may lead to
unexpected results.

Assume that a file is created with 'echo -n aaaa > /boot/test12' and
loaded by grub2:

    linux /boot/vmlinuz
    initrd newc:test12:/boot/test12 /boot/initrd

The initrd command eventually invoked grub_initrd_load() and sent
't''e''s''t''1''2' to make_header() to generate the header:

00000070  30 37 30 37 30 31 33 30  31 43 41 30 44 45 30 30  |070701301CA0DE00|
00000080  30 30 38 31 41 34 30 30  30 30 30 33 45 38 30 30  |0081A4000003E800|
00000090  30 30 30 30 36 34 30 30  30 30 30 30 30 31 36 33  |0000640000000163|
000000a0  37 36 45 34 35 32 30 30  30 30 30 30 30 34 30 30  |76E4520000000400|
000000b0  30 30 30 30 30 38 30 30  30 30 30 30 31 33 30 30  |0000080000001300|
000000c0  30 30 30 30 30 30 30 30  30 30 30 30 30 30 30 30  |0000000000000000|
000000d0  30 30 30 30 30 36 30 30  30 30 30 30 30 30 74 65  |00000600000000te|
                                                                  ^namesize
000000e0  73 74 31 32 61 61 61 61  30 37 30 37 30 31 30 30  |st12aaaa07070100|
                   ^^ end of the pathname

Since strlen("test12") + sizeof(struct newc_head) is 116 = 29 * 4,
make_header() didn't pad the pathname, and the file content followed
"test12" immediately. This violates the cpio format and may trigger such
error during linux boot:

    Initramfs unpacking failed: ZSTD-compressed data is trunc

To avoid the potential problems, this commit counts the trailing NUL byte
in when calling make_header() and adjusts the initrd size accordingly.

Now the header becomes

00000070  30 37 30 37 30 31 33 30  31 43 41 30 44 45 30 30  |070701301CA0DE00|
00000080  30 30 38 31 41 34 30 30  30 30 30 33 45 38 30 30  |0081A4000003E800|
00000090  30 30 30 30 36 34 30 30  30 30 30 30 30 31 36 33  |0000640000000163|
000000a0  37 36 45 34 35 32 30 30  30 30 30 30 30 34 30 30  |76E4520000000400|
000000b0  30 30 30 30 30 38 30 30  30 30 30 30 31 33 30 30  |0000080000001300|
000000c0  30 30 30 30 30 30 30 30  30 30 30 30 30 30 30 30  |0000000000000000|
000000d0  30 30 30 30 30 37 30 30  30 30 30 30 30 30 74 65  |00000700000000te|
                                                                  ^namesize
000000e0  73 74 31 32 00 00 00 00  61 61 61 61 30 37 30 37  |st12....aaaa0707|
                      ^^ end of the pathname

Besides the trailing NUL byte, make_header() pads 3 more NUL bytes, and
the user can safely read the pathname without a further check.

To conform to the cpio format, the headers for "TRAILER!!!" are also
adjusted to include the trailing NUL byte, not ignore it.

Signed-off-by: Gary Lin <g...@suse.com>
Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com>
---
 grub-core/loader/linux.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/grub-core/loader/linux.c b/grub-core/loader/linux.c
index 8f0fad805..e4018e65e 100644
--- a/grub-core/loader/linux.c
+++ b/grub-core/loader/linux.c
@@ -130,12 +130,23 @@ insert_dir (const char *name, struct dir **root,
          n->name = grub_strndup (cb, ce - cb);
          if (ptr)
            {
+             /*
+              * Create the substring with the trailing NUL byte
+              * to be included in the cpio header.
+              */
+             char *tmp_name = grub_strndup (name, ce - name);
+             if (!tmp_name) {
+               grub_free (n->name);
+               grub_free (n);
+               return grub_errno;
+             }
              grub_dprintf ("linux", "Creating directory %s, %s\n", name, ce);
-             ptr = make_header (ptr, name, ce - name,
+             ptr = make_header (ptr, tmp_name, ce - name + 1,
                                 040777, 0);
+             grub_free (tmp_name);
            }
          if (grub_add (*size,
-                       ALIGN_UP ((ce - (char *) name)
+                       ALIGN_UP ((ce - (char *) name + 1)
                                  + sizeof (struct newc_head), 4),
                        size))
            {
@@ -260,7 +271,7 @@ grub_initrd_init (int argc, char *argv[],
                  grub_initrd_close (initrd_ctx);
                  return grub_errno;
                }
-             name_len = grub_strlen (initrd_ctx->components[i].newc_name);
+             name_len = grub_strlen (initrd_ctx->components[i].newc_name) + 1;
              if (grub_add (initrd_ctx->size,
                            ALIGN_UP (sizeof (struct newc_head) + name_len, 4),
                            &initrd_ctx->size) ||
@@ -274,7 +285,7 @@ grub_initrd_init (int argc, char *argv[],
        {
          if (grub_add (initrd_ctx->size,
                        ALIGN_UP (sizeof (struct newc_head)
-                                 + sizeof ("TRAILER!!!") - 1, 4),
+                                 + sizeof ("TRAILER!!!"), 4),
                        &initrd_ctx->size))
            goto overflow;
          free_dir (root);
@@ -302,7 +313,7 @@ grub_initrd_init (int argc, char *argv[],
       initrd_ctx->size = ALIGN_UP (initrd_ctx->size, 4);
       if (grub_add (initrd_ctx->size,
                    ALIGN_UP (sizeof (struct newc_head)
-                             + sizeof ("TRAILER!!!") - 1, 4),
+                             + sizeof ("TRAILER!!!"), 4),
                    &initrd_ctx->size))
        goto overflow;
       free_dir (root);
@@ -378,7 +389,7 @@ grub_initrd_load (struct grub_linux_initrd_context 
*initrd_ctx,
        }
       else if (newc)
        {
-         ptr = make_header (ptr, "TRAILER!!!", sizeof ("TRAILER!!!") - 1,
+         ptr = make_header (ptr, "TRAILER!!!", sizeof ("TRAILER!!!"),
                             0, 0);
          free_dir (root);
          root = 0;
@@ -406,7 +417,7 @@ grub_initrd_load (struct grub_linux_initrd_context 
*initrd_ctx,
     {
       grub_memset (ptr, 0, ALIGN_UP_OVERHEAD (cursize, 4));
       ptr += ALIGN_UP_OVERHEAD (cursize, 4);
-      ptr = make_header (ptr, "TRAILER!!!", sizeof ("TRAILER!!!") - 1, 0, 0);
+      ptr = make_header (ptr, "TRAILER!!!", sizeof ("TRAILER!!!"), 0, 0);
     }
   free_dir (root);
   root = 0;
-- 
2.39.2


++++++ 0002-Restrict-cryptsetup-key-file-permission-for-better-s.patch ++++++
>From 9f18541245858f53fea72d8d60304f9015d88b5f Mon Sep 17 00:00:00 2001
From: Michael Chang <mch...@suse.com>
Date: Fri, 17 Mar 2023 22:00:23 +0800
Subject: [PATCH 2/2] Restrict cryptsetup key file permission for better
 security

GRUB's default permission 777 for concatenated initrd files was too
permissive for the cryptsetup key file, causing a complaint from
systemd-cryptsetup during boot. This commit replaces the 0777 permission
with a more secure 0400 permission for the key file.

Signed-off-by: Michael Chang <mch...@suse.com>
---
 grub-core/loader/linux.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/grub-core/loader/linux.c b/grub-core/loader/linux.c
index e4018e65e..fc71a78d7 100644
--- a/grub-core/loader/linux.c
+++ b/grub-core/loader/linux.c
@@ -32,6 +32,7 @@ struct grub_linux_initrd_component
   char *buf;
   char *newc_name;
   grub_off_t size;
+  grub_uint32_t mode;
 };
 
 struct dir
@@ -202,6 +203,7 @@ grub_initrd_component (const char *buf, int bufsz, const 
char *newc_name,
   grub_memcpy (comp->buf, buf, bufsz);
   initrd_ctx->nfiles++;
   comp->size = bufsz;
+  comp->mode = 0100400;
   if (grub_add (initrd_ctx->size, comp->size,
                &initrd_ctx->size))
     goto overflow;
@@ -271,6 +273,7 @@ grub_initrd_init (int argc, char *argv[],
                  grub_initrd_close (initrd_ctx);
                  return grub_errno;
                }
+             initrd_ctx->components[i].mode = 0100777;
              name_len = grub_strlen (initrd_ctx->components[i].newc_name) + 1;
              if (grub_add (initrd_ctx->size,
                            ALIGN_UP (sizeof (struct newc_head) + name_len, 4),
@@ -372,6 +375,7 @@ grub_initrd_load (struct grub_linux_initrd_context 
*initrd_ctx,
       if (initrd_ctx->components[i].newc_name)
        {
          grub_size_t dir_size;
+         grub_uint32_t mode = initrd_ctx->components[i].mode; 
 
          if (insert_dir (initrd_ctx->components[i].newc_name, &root, ptr,
                          &dir_size))
@@ -383,7 +387,7 @@ grub_initrd_load (struct grub_linux_initrd_context 
*initrd_ctx,
          ptr += dir_size;
          ptr = make_header (ptr, initrd_ctx->components[i].newc_name,
                             grub_strlen (initrd_ctx->components[i].newc_name) 
+ 1,
-                            0100777,
+                            mode,
                             initrd_ctx->components[i].size);
          newc = 1;
        }
-- 
2.39.2


++++++ 20_memtest86+ ++++++
--- /var/tmp/diff_new_pack.GA5tJj/_old  2023-03-22 22:29:21.153803924 +0100
+++ /var/tmp/diff_new_pack.GA5tJj/_new  2023-03-22 22:29:21.153803924 +0100
@@ -4,6 +4,7 @@
 # grub-mkconfig helper script.
 # Copyright (C) 2011  Michal Ambroz <re...@seznam.cz>
 # Adapted for openSUSE by Andrey Borzenkov <arvidj...@gmail.com>
+# Adapted for EFI by Hans-Peter Jansen <h...@urpla.net>
 #
 # you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -18,17 +19,11 @@
 # You should have received a copy of the GNU General Public License
 # along with the script.  If not, see <http://www.gnu.org/licenses/>.
 
-. /usr/share/grub2/grub-mkconfig_lib
+. "$pkgdatadir/grub-mkconfig_lib"
 
 export TEXTDOMAIN=grub2
 export TEXTDOMAINDIR=/usr/share/locale
 
-# memset86+ requires the x86 real mode
-# which is not available with UEFI booting.
-if [ -d /sys/firmware/efi ]; then
-  exit 0
-fi
-
 CLASS="--class memtest86 --class gnu --class tools"
 
 if [ "x${GRUB_DISTRIBUTOR}" = "x" ] ; then
@@ -38,7 +33,19 @@
   CLASS="--class $(echo ${GRUB_DISTRIBUTOR} | tr 'A-Z' 'a-z' | cut -d' ' -f1) 
${CLASS}"
 fi
 
-memtest=/boot/memtest.bin
+# memtest86+ comes in two flavours, one EFI and one suitable for x86 real mode.
+# The EFI module requires security disabled in BIOS (Boot Mode: Other OS)
+if [ -d /sys/firmware/efi -a -f /boot/efi/EFI/memtest86/memtest.efi ]; then
+  memtest=/boot/efi/EFI/memtest86/memtest.efi
+  loader='linux  '
+  message="$(gettext_printf "Loading EFI memtest ...\n" | grub_quote)"
+  # locate the real EFI partition
+  GRUB_DEVICE_BOOT=$(grub2-probe -t device "$memtest")
+else
+  memtest=/boot/memtest.bin
+  loader='linux16'
+  message="$(gettext_printf "Loading x86 memtest ...\n" | grub_quote)"
+fi
 
 if grub_file_is_not_garbage "$memtest" ; then
   gettext_printf "Found memtest image: %s\n" "$memtest" >&2
@@ -50,11 +57,11 @@
   printf "menuentry '%s' %s \$menuentry_id_option '%s' {\n" "${OS}" "${CLASS}" 
"memtest-$boot_device_id"
   prepare_boot_cache="$(prepare_grub_to_access_device ${GRUB_DEVICE_BOOT} | 
sed -e "s/^/\t/")"
   printf '%s\n' "${prepare_boot_cache}"
-  message="$(gettext_printf "Loading memtest ...\n" | grub_quote)"
   cat << EOF
-       echo    '$message'
-       linux16 ${rel_dirname}/${basename}
+        echo    '$message'
+        ${loader} ${rel_dirname}/${basename}
 }
+
 EOF
 fi
 

Reply via email to