Generate additional secret keys for the heap protector

  2016-10-27  Florian Weimer  <fweimer@redhat.com>

	* elf/dl-keysetup.h: New file.
	* elf/dl-keysetup.c: Likewise.
	* elf/Makefile (dl-routines): Add dl-keysetup.
	* csu/libc-start.c (LIBC_START_MAIN): Call __compute_keys to
	obtain key material.
	* elf/rtld.c (security_init): Likewise.
	* crypt/sha256.c: Use relative #include for sha256-block.c
  

Florian,

Awesome patch. This is looking really good and the expansion of entropy
into the keys using sha256 looks great.

OK to checkin if you:

* Fix spelling.
* Remove _dl_setup_pointer_guard linux+generic implementation as unused.
* Remove _dl_setup_stack_chk_guard linux+generic implementation as unused

Non-blocking questions:

Does the sha256 code add about 10kb to the loader's size? A fair trade-off
for being able to correctly expand the randomness we have into the size we
need. I also think the startup cost has to be paid, particularly for the
benefit of hardening the malloc headers against any many of the previously
used overflow attacks.

Does the leading zero placed into the stack canary reduce its usefullness?
Is it worth it compared to the protection against memory copy functions
which terminate on null characters? This is not code you added, but it's
a question about code you're touching and I wonder if you have given it
any thought. A follow on could be to discuss this with oss-security list.

On 10/28/2016 09:05 AM, Florian Weimer wrote:
> 2016-10-27  Florian Weimer  <fweimer@redhat.com>
> 
> 	* elf/dl-keysetup.h: New file.
> 	* elf/dl-keysetup.c: Likewise.
> 	* elf/Makefile (dl-routines): Add dl-keysetup.
> 	* csu/libc-start.c (LIBC_START_MAIN): Call __compute_keys to
> 	obtain key material.
> 	* elf/rtld.c (security_init): Likewise.
> 	* crypt/sha256.c: Use relative #include for sha256-block.c
> 
> diff --git a/crypt/sha256.c b/crypt/sha256.c
> index b5497d9..a6ab2e1 100644
> --- a/crypt/sha256.c
> +++ b/crypt/sha256.c
> @@ -211,4 +211,4 @@ __sha256_process_bytes (const void *buffer, size_t len, struct sha256_ctx *ctx)
>      }
>  }
>  
> -#include <sha256-block.c>
> +#include "sha256-block.c"

OK.

> diff --git a/csu/libc-start.c b/csu/libc-start.c
> index 99c040a..333a4cc 100644
> --- a/csu/libc-start.c
> +++ b/csu/libc-start.c
> @@ -21,6 +21,7 @@
>  #include <unistd.h>
>  #include <ldsodefs.h>
>  #include <exit-thread.h>
> +#include <elf/dl-keysetup.h>

OK.

>  
>  extern void __libc_init_first (int argc, char **argv, char **envp);
>  
> @@ -192,21 +193,21 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
>       we need to setup errno.  */
>    __pthread_initialize_minimal ();
>  
> +  struct key_setup keys;
> +  __compute_keys (_dl_random, &keys);
> +

OK.

>    /* Set up the stack checker's canary.  */
> -  uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random);
>  # ifdef THREAD_SET_STACK_GUARD
> -  THREAD_SET_STACK_GUARD (stack_chk_guard);
> +  THREAD_SET_STACK_GUARD (keys.stack);
>  # else
> -  __stack_chk_guard = stack_chk_guard;
> +  __stack_chk_guard = keys.stack;
>  # endif
>  
>    /* Set up the pointer guard value.  */
> -  uintptr_t pointer_chk_guard = _dl_setup_pointer_guard (_dl_random,
> -							 stack_chk_guard);
>  # ifdef THREAD_SET_POINTER_GUARD
> -  THREAD_SET_POINTER_GUARD (pointer_chk_guard);
> +  THREAD_SET_POINTER_GUARD (keys.pointer);
>  # else
> -  __pointer_chk_guard_local = pointer_chk_guard;
> +  __pointer_chk_guard_local = keys.pointer;
>  # endif

OK.

>  
>  #endif
> diff --git a/elf/Makefile b/elf/Makefile
> index 82c7e05..c0a8e2e 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -31,7 +31,8 @@ routines	= $(all-dl-routines) dl-support dl-iteratephdr \
>  dl-routines	= $(addprefix dl-,load lookup object reloc deps hwcaps \
>  				  runtime error init fini debug misc \
>  				  version profile conflict tls origin scope \
> -				  execstack caller open close trampoline)
> +				  execstack caller open close trampoline \
> +				  keysetup)

OK.

>  ifeq (yes,$(use-ldconfig))
>  dl-routines += dl-cache
>  endif
> diff --git a/elf/dl-keysetup.c b/elf/dl-keysetup.c
> new file mode 100644
> index 0000000..45bd4de
> --- /dev/null
> +++ b/elf/dl-keysetup.c
> @@ -0,0 +1,77 @@
> +/* Compute secret keys used for protection heuristics.
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public License as
> +   published by the Free Software Foundation; either version 2.1 of the
> +   License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; see the file COPYING.LIB.  If
> +   not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include <dl-keysetup.h>
> +#include <string.h>
> +
> +enum { at_random_size = 16 };
> +
> +#if __WORDSIZE == 64
> +enum { sha256_output_size = 32 };
> +static void compute_sha256_of_random (const void *random, void *result);
> +#endif
> +
> +void
> +__compute_keys (const void *random, struct key_setup *result)
> +{
> +#if __WORDSIZE == 32
> +  _Static_assert (sizeof (*result) == at_random_size,
> +                  "no key expansion required");
> +  memcpy (random, result, sizeof (result));
> +#else
> +  /* We use SHA-256 to expand the 16 bytes of randomness into 32
> +     bytes, so that it is hard to guess the remaining keys once a
> +     subset of them is known.  */
> +  _Static_assert (sizeof (*result) == sha256_output_size,
> +                  "SHA-256 provides required size");
> +  compute_sha256_of_random (random, result);

OK.

> +#endif
> +
> +  /* Prevent leakage of the stack canary through a read buffer
> +     overflow of a NUL-terminated string.  */
> +  *(char *) &result->stack = '\0';
> +
> +  /* Clear the lowest three bits in the heap header guard value, so
> +     that the flag bits remain unchanged.  */
> +  result->heap_header <<= 3;
> +}
> +
> +#if __WORDSIZE == 64
> +
> +#pragma GCC visibility push (hidden)
> +
> +/* Avoid symbol collisions with libcrypt.  */
> +#define __sha256_process_block __dl_sha256_process_block
> +#define __sha256_init_ctx __dl_sha256_init_ctx
> +#define __sha256_process_bytes __dl_sha256_process_bytes
> +#define __sha256_finish_ctx __dl_sha256_finish_ctx
> +
> +#include "../crypt/sha256.h"
> +#include "../crypt/sha256.c"

OK.

> +
> +#pragma GCC visibility pop
> +
> +static void
> +compute_sha256_of_random (const void *random, void *result)
> +{
> +  struct sha256_ctx ctx;
> +  __sha256_init_ctx (&ctx);
> +  __sha256_process_bytes (random, at_random_size, &ctx);
> +  __sha256_finish_ctx (&ctx, result);
> +}
> +#endif
> diff --git a/elf/dl-keysetup.h b/elf/dl-keysetup.h
> new file mode 100644
> index 0000000..3c7e9bb
> --- /dev/null
> +++ b/elf/dl-keysetup.h
> @@ -0,0 +1,45 @@
> +/* Compute secret keys used for protection heuristics.
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public License as
> +   published by the Free Software Foundation; either version 2.1 of the
> +   License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; see the file COPYING.LIB.  If
> +   not, see <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef KEY_SETUP_H
> +#define KEY_SETUP_H
> +
> +#include <stdint.h>
> +
> +/* The set of protection keys used by glibc.  */
> +struct key_setup
> +{
> +  /* Canary for the stack-smashing protector.  */
> +  uintptr_t stack;
> +
> +  /* Pointer guard, protecting selected function pointers.  */
> +  uintptr_t pointer;
> +
> +  /* Heap guard, proctecting the malloc chunk header.  */

s/proctecting/protecting/g

> +  uintptr_t heap_header;
> +
> +  /* Heap guard part two, protecting the previous chunk size field.  */
> +  uintptr_t heap_footer;
> +};
> +
> +/* Derive the keys in *RESULT from RANDOM, which comes from the
> +   auxiliary vector and points to 16 bytes of randomness.  */
> +void __compute_keys (const void *random, struct key_setup *result)
> +  attribute_hidden;
> +
> +#endif /* KEY_SETUP_H */
> diff --git a/elf/rtld.c b/elf/rtld.c
> index 647661c..de965da 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -41,6 +41,7 @@
>  #include <tls.h>
>  #include <stap-probe.h>
>  #include <stackinfo.h>
> +#include <dl-keysetup.h>

OK.

>  
>  #include <assert.h>
>  
> @@ -699,21 +700,21 @@ rtld_lock_default_unlock_recursive (void *lock)
>  static void
>  security_init (void)
>  {
> +  struct key_setup keys;
> +  __compute_keys (_dl_random, &keys);

OK.

> +
>    /* Set up the stack checker's canary.  */
> -  uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random);
>  #ifdef THREAD_SET_STACK_GUARD
> -  THREAD_SET_STACK_GUARD (stack_chk_guard);
> +  THREAD_SET_STACK_GUARD (keys.stack);
>  #else
> -  __stack_chk_guard = stack_chk_guard;
> +  __stack_chk_guard = keys.stack;
>  #endif

OK.

>  
>    /* Set up the pointer guard as well, if necessary.  */
> -  uintptr_t pointer_chk_guard
> -    = _dl_setup_pointer_guard (_dl_random, stack_chk_guard);
>  #ifdef THREAD_SET_POINTER_GUARD
> -  THREAD_SET_POINTER_GUARD (pointer_chk_guard);
> +  THREAD_SET_POINTER_GUARD (keys.pointer);
>  #endif
> -  __pointer_chk_guard_local = pointer_chk_guard;
> +  __pointer_chk_guard_local = keys.pointer;
>  

OK.

>    /* We do not need the _dl_random value anymore.  The less
>       information we leave behind, the better, so clear the
>