On Tue, 14 Nov 2023 04:05:00 +0000 Dimitri John Ledkov <dimitri.led...@canonical.com> wrote:
> On Tue, 14 Nov 2023, 03:19 Glenn Washburn, <developm...@efficientek.com> > wrote: > > > On Mon, 13 Nov 2023 17:18:50 +0100 > > Daniel Kiper <dki...@net-space.pl> wrote: > > > > > On Sun, Nov 12, 2023 at 08:22:42AM +0100, Heinrich Schuchardt wrote: > > > > On 11/12/23 04:23, Glenn Washburn wrote: > > > > > The canary, __stack_chk_guard, is in the BSS and so will get > > initialized to > > > > > zero if it is not explicitly initialized. If the UEFI firmware does > > not > > > > > support the RNG protocol, then the canary will not be randomized and > > will > > > > > be used as zero. This seems like a possibly easier value to write by > > an > > > > > attacker. Initialize canary to static random bytes, so that it is > > still > > > > > random when there is not RNG protocol. > > > > Ugh. s/not/no/ > > > > > > > > > > > > Signed-off-by: Glenn Washburn <developm...@efficientek.com> > > > > > > > > Reviewed-by: Heinrich Schuchardt <xypron.g...@gmx.de> > > > > > > Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com> > > > > > > > > --- > > > > > grub-core/kern/efi/init.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/grub-core/kern/efi/init.c b/grub-core/kern/efi/init.c > > > > > index 0e28bea17a76..b85d98ca47fd 100644 > > > > > --- a/grub-core/kern/efi/init.c > > > > > +++ b/grub-core/kern/efi/init.c > > > > > @@ -41,7 +41,7 @@ static grub_guid_t rng_protocol_guid = > > GRUB_EFI_RNG_PROTOCOL_GUID; > > > > > > > > > > static grub_efi_uint8_t stack_chk_guard_buf[32]; > > > > > > > > > > -grub_addr_t __stack_chk_guard; > > > > > +grub_addr_t __stack_chk_guard = (grub_addr_t) 0x92f2b7e2f193b25c; > > > > Just now reflecting on this I had the idea that perhaps even better is > > to have this randomly generated at build time and inserted as an > > autoconf variable. Then every build would have a different canary. > > I think I'll send a patch for this soon. > > > > > But please insure it is reproducible (as in reproducible builds initiative). This is a good idea. > But also, one already has time and date macros that are unique and > reproducible in most distros. Can you use TIME macro as either the random > value or the seed to make a suitable random number? > https://reproducible-builds.org/docs/source-date-epoch/ and all the ways it > is already available at cpp time. This link seems to be suggesting to use the SOURCE_DATE_EPOCH environment variable if it exists. Is this what you mean by "TIME macro"? If not, could you explain further? I'm wondering if it might be better to provide a --enable-stack-protector-init= configure option instead. I like the idea of not having this and everything be automated based on the reproducible value, which would create less work for those using this feature. However, I'm concerned that the reproducible value itself may not be suitable as a canary and that other tools/dependencies might be required to make it more suitable (eg. sha1sum). Do you have any thoughts on this? Glenn > > > > > > > > > I would add last sentence from the commit message before this line. > > > I can do it for you before push... > > > > Sure. And please remove the 't' mentioned above in the commit message. > > > > Glenn > > > > > > > > > > > > > > > void __attribute__ ((noreturn)) > > > > > __stack_chk_fail (void) > > > > > > Daniel > > > > _______________________________________________ > > Grub-devel mailing list > > Grub-devel@gnu.org > > https://lists.gnu.org/mailman/listinfo/grub-devel > > _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
