From 2878d37def076e2bd619448fdefa0689612f7085 Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Tue, 26 Nov 2019 10:57:00 +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 0296-grub-set-bootflag-Update-comment-about-running-as-ro.patch create mode 100644 0297-grub-set-bootflag-Write-new-env-to-tmpfile-and-then-.patch diff --git a/0296-grub-set-bootflag-Update-comment-about-running-as-ro.patch b/0296-grub-set-bootflag-Update-comment-about-running-as-ro.patch new file mode 100644 index 0000000..f28733f --- /dev/null +++ b/0296-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 20062fe802b..0ce565611fc 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/0297-grub-set-bootflag-Write-new-env-to-tmpfile-and-then-.patch b/0297-grub-set-bootflag-Write-new-env-to-tmpfile-and-then-.patch new file mode 100644 index 0000000..8608e48 --- /dev/null +++ b/0297-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: Fri, 22 Nov 2019 11:54:27 +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 0ce565611fc..2999db27930 100644 +--- a/util/grub-set-bootflag.c ++++ b/util/grub-set-bootflag.c +@@ -27,7 +27,9 @@ + #include + #include + #include /* For GRUB_ENVBLK_DEFCFG define */ ++#include + #include ++#include + #include + #include + +@@ -53,8 +55,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) +@@ -76,7 +80,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"); +@@ -130,30 +159,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 3260752..eca9def 100644 --- a/grub.patches +++ b/grub.patches @@ -293,3 +293,5 @@ Patch0292: 0292-blscfg-remove-BLS-file-size-check.patch Patch0293: 0293-Remove-bogus-load_env-after-blscfg-command-in-10_lin.patch Patch0294: 0294-blscfg-Don-t-leave-grub_errno-set-to-an-error-if-the.patch Patch0295: 0295-blscfg-Fix-typo-for-gfxpayload-variable-name.patch +Patch0296: 0296-grub-set-bootflag-Update-comment-about-running-as-ro.patch +Patch0297: 0297-grub-set-bootflag-Write-new-env-to-tmpfile-and-then-.patch diff --git a/grub2.spec b/grub2.spec index 16cc1eb..66329b4 100644 --- a/grub2.spec +++ b/grub2.spec @@ -7,7 +7,7 @@ Name: grub2 Epoch: 1 Version: 2.02 -Release: 84%{?dist} +Release: 85%{?dist} Summary: Bootloader with support for Linux, Multiboot and more License: GPLv3+ URL: http://www.gnu.org/software/grub/ @@ -476,6 +476,11 @@ rm -r /boot/grub2.tmp/ || : %endif %changelog +* Tue Nov 26 2019 Javier Martinez Canillas - 2.02-85 +- grub-set-bootflag: Write new env to tmpfile and then rename (hdegoede) + Resolves: CVE-2019-14865 + Resolves: rhbz#1776580 + * Wed Oct 16 2019 Javier Martinez Canillas - 2.02-84 - 99-grub-mkconfig: Also disable BLS usage for Xen Dom0 hosts Resolves: rhbz#1761799