From patchwork Fri Oct 28 15:39:18 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 16917 Received: (qmail 72587 invoked by alias); 28 Oct 2016 15:39:32 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 72568 invoked by uid 89); 28 Oct 2016 15:39:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=0xff, paid, secret X-HELO: mx1.redhat.com Subject: Re: [PATCH] Generate additional secret keys for the heap protector To: "Carlos O'Donell" , libc-alpha@sourceware.org References: <20161028130509.A74DC439942E0@oldenburg.str.redhat.com> From: Florian Weimer Message-ID: Date: Fri, 28 Oct 2016 17:39:18 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: On 10/28/2016 04:46 PM, Carlos O'Donell wrote: > 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 New version attached. I'm waiting until the libcrypt fix is checked in (I'll try to test on SPARC and commit if successful). And it only makes sense to have this change if we end up with the heap protector, so I'll wait for that, too. > 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. Before: -rwxrwxr-x. 1 fweimer fweimer 1231576 Oct 28 17:17 elf/ld.so text data bss dec hex filename 141373 5092 424 146889 23dc9 elf/ld.so After: -rwxrwxr-x. 1 fweimer fweimer 1245552 Oct 28 17:15 elf/ld.so text data bss dec hex filename 143629 5092 424 149145 24699 elf/ld.so So it's a 2256 bytes increase in text size. The disk size increase is mostly debugging information (it's down to 4096 bytes without debugging information). I have not tried to measure the cycle impact. Thanks, Florian Generate additional secret keys for the heap protector 2016-10-28 Florian Weimer * 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 * sysdeps/generic/dl-osinfo.h (_dl_setup_stack_chk_guard) (_dl_setup_pointer_guard): Remove. * sysdeps/unix/sysv/linux/dl-osinfo.h (_dl_setup_stack_chk_guard) (_dl_setup_pointer_guard): Likewise. 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 +#include "sha256-block.c" 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 #include #include +#include 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); + /* 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 #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) 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 . */ + +#include +#include + +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); +#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" + +#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..6197b1d --- /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 . */ + +#ifndef KEY_SETUP_H +#define KEY_SETUP_H + +#include + +/* 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, protecting the malloc chunk header. */ + 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 #include #include +#include #include @@ -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); + /* 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 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; /* We do not need the _dl_random value anymore. The less information we leave behind, the better, so clear the diff --git a/sysdeps/generic/dl-osinfo.h b/sysdeps/generic/dl-osinfo.h index 2489e7b..47464c2 100644 --- a/sysdeps/generic/dl-osinfo.h +++ b/sysdeps/generic/dl-osinfo.h @@ -16,44 +16,4 @@ License along with the GNU C Library; if not, see . */ -#include -#include - -static inline uintptr_t __attribute__ ((always_inline)) -_dl_setup_stack_chk_guard (void *dl_random) -{ - union - { - uintptr_t num; - unsigned char bytes[sizeof (uintptr_t)]; - } ret = { 0 }; - - if (dl_random == NULL) - { - ret.bytes[sizeof (ret) - 1] = 255; - ret.bytes[sizeof (ret) - 2] = '\n'; - } - else - { - memcpy (ret.bytes, dl_random, sizeof (ret)); -#if BYTE_ORDER == LITTLE_ENDIAN - ret.num &= ~(uintptr_t) 0xff; -#elif BYTE_ORDER == BIG_ENDIAN - ret.num &= ~((uintptr_t) 0xff << (8 * (sizeof (ret) - 1))); -#else -# error "BYTE_ORDER unknown" -#endif - } - return ret.num; -} - -static inline uintptr_t __attribute__ ((always_inline)) -_dl_setup_pointer_guard (void *dl_random, uintptr_t stack_chk_guard) -{ - uintptr_t ret; - if (dl_random == NULL) - ret = stack_chk_guard; - else - memcpy (&ret, (char *) dl_random + sizeof (ret), sizeof (ret)); - return ret; -} +/* No operating-system specific code in the default version. */ diff --git a/sysdeps/unix/sysv/linux/dl-osinfo.h b/sysdeps/unix/sysv/linux/dl-osinfo.h index ac72c92..408cf5a 100644 --- a/sysdeps/unix/sysv/linux/dl-osinfo.h +++ b/sysdeps/unix/sysv/linux/dl-osinfo.h @@ -46,34 +46,3 @@ else if (__LINUX_KERNEL_VERSION > 0) \ FATAL ("FATAL: cannot determine kernel version\n"); \ } while (0) - -static inline uintptr_t __attribute__ ((always_inline)) -_dl_setup_stack_chk_guard (void *dl_random) -{ - union - { - uintptr_t num; - unsigned char bytes[sizeof (uintptr_t)]; - } ret; - - /* We need in the moment only 8 bytes on 32-bit platforms and 16 - bytes on 64-bit platforms. Therefore we can use the data - directly and not use the kernel-provided data to seed a PRNG. */ - memcpy (ret.bytes, dl_random, sizeof (ret)); -#if BYTE_ORDER == LITTLE_ENDIAN - ret.num &= ~(uintptr_t) 0xff; -#elif BYTE_ORDER == BIG_ENDIAN - ret.num &= ~((uintptr_t) 0xff << (8 * (sizeof (ret) - 1))); -#else -# error "BYTE_ORDER unknown" -#endif - return ret.num; -} - -static inline uintptr_t __attribute__ ((always_inline)) -_dl_setup_pointer_guard (void *dl_random, uintptr_t stack_chk_guard) -{ - uintptr_t ret; - memcpy (&ret, (char *) dl_random + sizeof (ret), sizeof (ret)); - return ret; -}