From eeeca9c900350ef75724781ab8765fc1c65c88cf Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Tue, 26 Nov 2019 11:37:19 +0100 Subject: [PATCH] grub-set-bootflag: Write new env to tmpfile and then rename Resolves: CVE-2019-14865 Resolves: rhbz#1776580 Signed-off-by: Javier Martinez Canillas --- ...g-Update-comment-about-running-as-ro.patch | 27 ++++ ...g-Write-new-env-to-tmpfile-and-then-.patch | 152 ++++++++++++++++++ grub.patches | 2 + grub2.spec | 7 +- 4 files changed, 187 insertions(+), 1 deletion(-) create mode 100644 0186-grub-set-bootflag-Update-comment-about-running-as-ro.patch create mode 100644 0187-grub-set-bootflag-Write-new-env-to-tmpfile-and-then-.patch diff --git a/0186-grub-set-bootflag-Update-comment-about-running-as-ro.patch b/0186-grub-set-bootflag-Update-comment-about-running-as-ro.patch new file mode 100644 index 0000000..cd4ef77 --- /dev/null +++ b/0186-grub-set-bootflag-Update-comment-about-running-as-ro.patch @@ -0,0 +1,27 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Hans de Goede +Date: Wed, 13 Nov 2019 12:15:43 +0100 +Subject: [PATCH] grub-set-bootflag: Update comment about running as root + through pkexec + +We have stopped using pkexec for grub-set-bootflag, instead it is now +installed suid root, update the comment accordingly. + +Signed-off-by: Hans de Goede +--- + util/grub-set-bootflag.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/util/grub-set-bootflag.c b/util/grub-set-bootflag.c +index 6a79ee67444..65d74ce010f 100644 +--- a/util/grub-set-bootflag.c ++++ b/util/grub-set-bootflag.c +@@ -18,7 +18,7 @@ + */ + + /* +- * NOTE this gets run by users as root (through pkexec), so this does not ++ * NOTE this gets run by users as root (its suid root), so this does not + * use any grub library / util functions to allow for easy auditing. + * The grub headers are only included to get certain defines. + */ diff --git a/0187-grub-set-bootflag-Write-new-env-to-tmpfile-and-then-.patch b/0187-grub-set-bootflag-Write-new-env-to-tmpfile-and-then-.patch new file mode 100644 index 0000000..122bf68 --- /dev/null +++ b/0187-grub-set-bootflag-Write-new-env-to-tmpfile-and-then-.patch @@ -0,0 +1,152 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Hans de Goede +Date: Wed, 13 Nov 2019 13:02:01 +0100 +Subject: [PATCH] grub-set-bootflag: Write new env to tmpfile and then rename + +Make the grubenv writing code in grub-set-bootflag more robust by +writing the modified grubenv to a tmpfile first and then renaming the +tmpfile over the old grubenv (following symlinks). + +Signed-off-by: Hans de Goede +--- + util/grub-set-bootflag.c | 87 +++++++++++++++++++++++++++++++++++++++++++----- + 1 file changed, 78 insertions(+), 9 deletions(-) + +diff --git a/util/grub-set-bootflag.c b/util/grub-set-bootflag.c +index 65d74ce010f..d1c5e28862b 100644 +--- a/util/grub-set-bootflag.c ++++ b/util/grub-set-bootflag.c +@@ -28,7 +28,9 @@ + #include + #include /* For GRUB_ENVBLK_DEFCFG define */ + #include ++#include + #include ++#include + #include + #include + +@@ -54,8 +56,10 @@ int main(int argc, char *argv[]) + { + /* NOTE buf must be at least the longest bootflag length + 4 bytes */ + char env[GRUBENV_SIZE + 1], buf[64], *s; ++ /* +1 for 0 termination, +6 for "XXXXXX" in tmp filename */ ++ char env_filename[PATH_MAX + 1], tmp_filename[PATH_MAX + 6 + 1]; + const char *bootflag; +- int i, len, ret; ++ int i, fd, len, ret; + FILE *f; + + if (argc != 2) +@@ -77,7 +81,32 @@ int main(int argc, char *argv[]) + bootflag = bootflags[i]; + len = strlen (bootflag); + +- f = fopen (GRUBENV, "r"); ++ /* ++ * Really become root. setuid avoids an user killing us, possibly leaking ++ * the tmpfile. setgid avoids the new grubenv's gid being that of the user. ++ */ ++ ret = setuid(0); ++ if (ret) ++ { ++ perror ("Error setuid(0) failed"); ++ return 1; ++ } ++ ++ ret = setgid(0); ++ if (ret) ++ { ++ perror ("Error setgid(0) failed"); ++ return 1; ++ } ++ ++ /* Canonicalize GRUBENV filename, resolving symlinks, etc. */ ++ if (!realpath(GRUBENV, env_filename)) ++ { ++ perror ("Error canonicalizing " GRUBENV " filename"); ++ return 1; ++ } ++ ++ f = fopen (env_filename, "r"); + if (!f) + { + perror ("Error opening " GRUBENV " for reading"); +@@ -132,30 +161,70 @@ int main(int argc, char *argv[]) + snprintf(buf, sizeof(buf), "%s=1\n", bootflag); + memcpy(s, buf, len + 3); + +- /* "r+", don't truncate so that the diskspace stays reserved */ +- f = fopen (GRUBENV, "r+"); ++ ++ /* ++ * Create a tempfile for writing the new env. Use the canonicalized filename ++ * for the template so that the tmpfile is in the same dir / on same fs. ++ */ ++ snprintf(tmp_filename, sizeof(tmp_filename), "%sXXXXXX", env_filename); ++ fd = mkstemp(tmp_filename); ++ if (fd == -1) ++ { ++ perror ("Creating tmpfile failed"); ++ return 1; ++ } ++ ++ f = fdopen (fd, "w"); + if (!f) + { +- perror ("Error opening " GRUBENV " for writing"); ++ perror ("Error fdopen of tmpfile failed"); ++ unlink(tmp_filename); + return 1; + } + + ret = fwrite (env, 1, GRUBENV_SIZE, f); + if (ret != GRUBENV_SIZE) + { +- perror ("Error writing to " GRUBENV); ++ perror ("Error writing tmpfile"); ++ unlink(tmp_filename); + return 1; + } + + ret = fflush (f); + if (ret) + { +- perror ("Error flushing " GRUBENV); ++ perror ("Error flushing tmpfile"); ++ unlink(tmp_filename); + return 1; + } + +- fsync (fileno (f)); +- fclose (f); ++ ret = fsync (fileno (f)); ++ if (ret) ++ { ++ perror ("Error syncing tmpfile"); ++ unlink(tmp_filename); ++ return 1; ++ } ++ ++ ret = fclose (f); ++ if (ret) ++ { ++ perror ("Error closing tmpfile"); ++ unlink(tmp_filename); ++ return 1; ++ } ++ ++ /* ++ * And finally rename the tmpfile with the new env over the old env, the ++ * linux kernel guarantees that this is atomic (from a syscall pov). ++ */ ++ ret = rename(tmp_filename, env_filename); ++ if (ret) ++ { ++ perror ("Error renaming tmpfile to " GRUBENV " failed"); ++ unlink(tmp_filename); ++ return 1; ++ } + + return 0; + } diff --git a/grub.patches b/grub.patches index 7c802fe..ca20509 100644 --- a/grub.patches +++ b/grub.patches @@ -183,3 +183,5 @@ Patch0182: 0182-Set-a-devicetree-var-in-a-BLS-config-if-GRUB_DEFAULT.patch Patch0183: 0183-Don-t-add-a-class-option-to-menu-entries-generated-f.patch Patch0184: 0184-10_linux.in-Also-use-GRUB_CMDLINE_LINUX_DEFAULT-to-s.patch Patch0185: 0185-blscfg-Don-t-hardcode-an-env-var-as-fallback-for-the.patch +Patch0186: 0186-grub-set-bootflag-Update-comment-about-running-as-ro.patch +Patch0187: 0187-grub-set-bootflag-Write-new-env-to-tmpfile-and-then-.patch diff --git a/grub2.spec b/grub2.spec index c474bb2..27d050c 100644 --- a/grub2.spec +++ b/grub2.spec @@ -9,7 +9,7 @@ Name: grub2 Epoch: 1 Version: 2.04 -Release: 3%{?dist} +Release: 4%{?dist} Summary: Bootloader with support for Linux, Multiboot and more License: GPLv3+ URL: http://www.gnu.org/software/grub/ @@ -515,6 +515,11 @@ rm -r /boot/grub2.tmp/ || : %endif %changelog +* Tue Nov 26 2019 Javier Martinez Canillas - 2.04-4 +- grub-set-bootflag: Write new env to tmpfile and then rename (hdegoede) + Resolves: CVE-2019-14865 + Resolves: rhbz#1776580 + * Thu Oct 17 2019 Javier Martinez Canillas - 2.04-3 - 20-grub-install: Don't add an id field to generated BLS snippets - 99-grub-mkconfig: Disable BLS usage for Xen machines