[PATCH v3 0/3] efi: Initialize canary to non-zero value

[Glenn Washburn]

Updates from v2:
 * Change NULL to NUL
 * Describe more why it is desirable to have a NUL byte in the canary

Glenn

Glenn Washburn (3):
  efi: Initialize canary to non-zero value
  efi: Generate stack protector canary at build time if urandom is
    available
  efi: Add support for reproducible builds

 config.h.in               |  2 ++
 configure.ac              | 22 ++++++++++++++++++++++
 grub-core/kern/efi/init.c |  3 ++-
 3 files changed, 26 insertions(+), 1 deletion(-)

Range-diff against v2:
1:  a993f050ce89 ! 1:  c4d3769d2c26 efi: Initialize canary to non-zero value
    @@ Commit message
         support the RNG protocol, then the canary will not be randomized and 
will
         be 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 no RNG protocol. Set at least one byte to NULL to protect 
against
    -    string buffer overflow attacks.
    +    there is no RNG protocol. Set at least one byte to NUL to protect 
against
    +    string buffer overflow attacks. Code that writes NUL terminated strings
    +    will terminate when a NUL is encountered in the input byte stream. So 
the
    +    attacker will not be able to forge the canary by including it in the 
input
    +    stream without terminating the string operation and thus limiting the
    +    stack corruption.
    +
    +    [1] 
https://www.sans.org/blog/stack-canaries-gingerly-sidestepping-the-cage/
     
      ## grub-core/kern/efi/init.c ##
     @@ grub-core/kern/efi/init.c: static grub_guid_t rng_protocol_guid = 
GRUB_EFI_RNG_PROTOCOL_GUID;
2:  177ae1cf1015 ! 2:  01a56aed2f26 efi: Generate stack protector canary at 
build time if urandom is available
    @@ Commit message
         Fallback to the statically generated random bytes if /dev/urandom is 
not
         readable (eg. Windows).
     
    -    Reduce the canary to 3 bytes with a NULL upper byte on 32-bit 
architectures,
    -    which use a 32-bit canary, to filter out string buffer overflow 
attacks.
    +    On 32-bit architectures, which use a 32-bit canary, reduce the canary 
to 4
    +    bytes with one byte being NUL to filter out string buffer overflow 
attacks.
     
      ## config.h.in ##
     @@
    @@ configure.ac: else
     +
     +  if test -r /dev/urandom; then
     +     # Generate the 8 byte stack protector canary at build time if 
/dev/urandom
    -+     # is able to be read. The first byte should be NULL to filter out 
string
    ++     # is able to be read. The first byte should be NUL to filter out 
string
     +     # buffer overflow attacks.
     +     GRUB_STACK_PROTECTOR_INIT="$($PYTHON -c 'import codecs; 
rf=open("/dev/urandom", "rb"); print("0x00"+codecs.encode(rf.read(7), 
"hex").decode("ascii"))')"
     +  else
    @@ configure.ac: else
     +
     +  if test x"$target_m32" = x1 ; then
     +    # Make sure that the canary default value is 24-bits by only using the
    -+    # lower 3 bytes on 32 bit systems. This allows the upper byte to be 
NULL
    ++    # lower 3 bytes on 32 bit systems. This allows the upper byte to be 
NUL
     +    # to filter out string buffer overflow attacks.
     +    GRUB_STACK_PROTECTOR_INIT="0x00$(echo "$GRUB_STACK_PROTECTOR_INIT" | 
sed 's/.*\(......\)$/\1/')"
     +  fi
3:  c38fa7791697 ! 3:  5989c0102154 efi: Add support for reproducible builds
    @@ configure.ac: else
     +     GRUB_STACK_PROTECTOR_INIT="0x00f2b7e2$(printf "%x" 
"$SOURCE_DATE_EPOCH" | sed 's/.*\(........\)$/\1/')"
     +  elif test -r /dev/urandom; then
           # Generate the 8 byte stack protector canary at build time if 
/dev/urandom
    -      # is able to be read. The first byte should be NULL to filter out 
string
    +      # is able to be read. The first byte should be NUL to filter out 
string
           # buffer overflow attacks.
-- 
2.34.1