From patchwork Mon Dec 15 14:21:07 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nick Mathewson X-Patchwork-Id: 4254 Received: (qmail 1655 invoked by alias); 15 Dec 2014 14:21:14 -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 1644 invoked by uid 89); 15 Dec 2014 14:21:13 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-lb0-f173.google.com MIME-Version: 1.0 X-Received: by 10.112.170.7 with SMTP id ai7mr31050050lbc.67.1418653267784; Mon, 15 Dec 2014 06:21:07 -0800 (PST) Date: Mon, 15 Dec 2014 09:21:07 -0500 Message-ID: Subject: [RFC] [PATCH] Support explicit_bzero, memset_s, memzero_explicit, or similar. From: Nick Mathewson To: libc-alpha@sourceware.org Hello! (This is my first attempt at a glibc RFC/patch post. I'm hoping I can do more in the future, if this works out.) It's frequently desirable in secure C programming to cause a piece of memory to be erased in a way that the compiler is not allowed to optimize away. For example, if you're writing a program that needs to perform a single operation with a high-value password cryptographic key, you might want to make sure that the key is no longer resident in memory once you're done with it. But this pattern is inadequate to do so: int password_is_okay(void) { int result = -1; char passwordbuf[MAX_PASSWORD + 1]; if (read_password(passwordbuf, MAX_PASSWORD) < 0) goto err; if (password_matches(passwordbuf) < 0) goto err; result = 0; err: memset(passwordbuf, 0, sizeof(passwordbuf)); return result; } The pattern above fails because the memset call is a dead store, and the compiler is allowed to eliminate it. Different operating systems, libraries, and standards have approached this problem differently. Windows provides SecureZeroMemory(void *, size_t); The BSD family is moving towards explicit_bzero(void *, size_t); And the C11 standard defines memset_s(void *, rsize_t, int, size_t); And OpenSSL (along with LibreSSL and BoringSSL) provide: OPENSSL_cleanse(void *, size_t); And in the Linux kernel these says, they're using: memzero_explicit(void *, size_t); These APIs are in wide use where available: http://codesearch.debian.net/results/memset_s/page_0 http://codesearch.debian.net/results/explicit_bzero/page_0 especially in cryptographic and security-related tools. In the absence of these APIs, programs typically try to cobble their own implementations together -- typically using some combination of volatile and explicit barriers. I propose that glibc add such an API: either memset_s (which has a standard backing it), or explicit_bzero (which has been around longer, and is pretty widely used), or memzero_explicit (if we feel strongly that even mentioning bzero is deprecated.) Now, at first glance it would seem that that memset_s() is the obvious choice, since it's the one supported by a standard. But it's a part of a much larger pile of bounds-checking variants of other C functions (C11 Annex K). It's *possible* to implement memset_s() without the rest of Annex K, but it requires a bit of infrastructure to be fully compliant. (Like, we would need an rsize_t, and a set_constraint_handler_s(). I do not believe we would need a full implementation of Annex K.) It's possible to do a non-compliant memset_s() implementation, and not unprecedented. The OSX version, for instance, doesn't include set_constraint_handler_s(), or any other Annex K functions as far as I can tell. On the other hand, if we think that memset_s() is the only useful part of Annex K, maybe memset_s() isn't the variant for us? I'm including an example manpage for a somewhat-compliant memset_s(), and one for an explicit_bzero. I'm also attaching an incomplete explicit_bzero implementation patch -- this is my first glibc patch, so I have probably made many mistakes. I can write an memset_s one instead if needed. I hereby dedicate this email, the patch attached to it, and all later versions of this patch written by me to the public domain. ========== variant 1 NAME memset_s - fill memory with a constant byte, securely. SYNOPSIS #define __STDC_WANT_LIB_EXT1__ 1 #include errno_t memset_s(void *s, rsize_t szmax, int c, rsize_t n); DESCRIPTION The memset() function fills the first n bytes of the memory area pointed to by s with the constant byte c. The szmax value indicates the length of the memory area at s; it must be no smaller than the value of n. If it is, a runtime constraint violation occurs. Unlike memset(), the memset_s() function will not be removed by the compiler, even if it can prove that there are no subsequent legal reads to the memory in s. RETURN VALUE The memset_s() function zero if no constraint violation occurred. Otherwise, it returns an error. CONFORMING TO C11 Annex K specifies a memset_s(). This implementation does not provide set_constraint_handler_s(). SEE ALSO bzero(3), memset(3) ========== ========== variant 2 NAME explicit_bzero - fill memory with zero, securely. SYNOPSIS #include void explicit_bzero(void *s, size_t n); DESCRIPTION The explicit_bzero() function sets the first n bytes of the memory area starting at 's' to zero (bytes containing '\0'). Unlike memset() and the regular bzero() function, the explicit_bzero() function will not be removed by the compiler, even if it can prove that there are no subsequent legal reads to the memory in s. RETURN VALUE None. CONFORMING TO explicit_bzero() first appeared in OpenBSD 5.5. SEE ALSO bzero(3), memset(3) ========== commit 5dd7fa4810de0b5ae150af4d7eb1e6672fafb70d Author: Nick Mathewson Date: Mon Dec 15 09:14:48 2014 -0500 Add new explicit_bzero() function diff --git a/debug/tst-chk1.c b/debug/tst-chk1.c index 3393153..9085e3e 100644 --- a/debug/tst-chk1.c +++ b/debug/tst-chk1.c @@ -283,6 +283,10 @@ do_test (void) CHK_FAIL_END CHK_FAIL_START + explicit_bzero (buf + 9, l0 + 2); + CHK_FAIL_END + + CHK_FAIL_START strcpy (buf + 5, str1 + 5); CHK_FAIL_END diff --git a/include/string.h b/include/string.h index 034e295..64340be 100644 --- a/include/string.h +++ b/include/string.h @@ -100,6 +100,7 @@ libc_hidden_builtin_proto (mempcpy) libc_hidden_builtin_proto (memcmp) libc_hidden_builtin_proto (memmove) libc_hidden_builtin_proto (memset) +libc_hidden_builtin_proto (bzero) libc_hidden_builtin_proto (strcat) libc_hidden_builtin_proto (strchr) libc_hidden_builtin_proto (strcmp) diff --git a/manual/string.texi b/manual/string.texi index ba5a2c7..9fa60d8 100644 --- a/manual/string.texi +++ b/manual/string.texi @@ -1057,6 +1057,17 @@ BSD. Note that it is not as general as @code{memset}, because the only value it can store is zero. @end deftypefun +@comment string.h +@comment BSD +@deftypefun void explicit_bzero (void *@var{block}, size_t @var{size}) +@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}} +This function behaves as @code{bzero} or as @code{memset(block, 0, size)}, +except that the compiler is not permitted to perform dead-assignment +elimination on it. It can be useful in cryptographic code that needs to +clear sensitive data from the stack. +@end deftypefun + + @node String/Array Comparison @section String/Array Comparison @cindex comparing strings and arrays diff --git a/string/Makefile b/string/Makefile index 98c2961..6fd9726 100644 --- a/string/Makefile +++ b/string/Makefile @@ -33,6 +33,7 @@ routines := strcat strchr strcmp strcoll strcpy strcspn \ strrchr strpbrk strsignal strspn strstr strtok \ strtok_r strxfrm memchr memcmp memmove memset \ mempcpy bcopy bzero ffs ffsll stpcpy stpncpy \ + explicit_bzero \ strcasecmp strncase strcasecmp_l strncase_l \ memccpy memcpy wordcopy strsep strcasestr \ swab strfry memfrob memmem rawmemchr strchrnul \ @@ -47,7 +48,8 @@ strop-tests := memchr memcmp memcpy memmove mempcpy memset memccpy \ stpcpy stpncpy strcat strchr strcmp strcpy strcspn \ strlen strncmp strncpy strpbrk strrchr strspn memmem \ strstr strcasestr strnlen strcasecmp strncasecmp \ - strncat rawmemchr strchrnul bcopy bzero memrchr + strncat rawmemchr strchrnul bcopy bzero memrchr \ + explicit_bzero tests := tester inl-tester noinl-tester testcopy test-ffs \ tst-strlen stratcliff tst-svc tst-inlcall \ bug-strncat1 bug-strspn1 bug-strpbrk1 tst-bswap \ diff --git a/string/Versions b/string/Versions index 59bf35a..f9aba06 100644 --- a/string/Versions +++ b/string/Versions @@ -80,4 +80,7 @@ libc { GLIBC_2.6 { strerror_l; } + GLIBC_2.21 { + explicit_bzero; + } } diff --git a/string/bits/string3.h b/string/bits/string3.h index 801e7ac..65d9067 100644 --- a/string/bits/string3.h +++ b/string/bits/string3.h @@ -102,6 +102,13 @@ __NTH (bzero (void *__dest, size_t __len)) { (void) __builtin___memset_chk (__dest, '\0', __len, __bos0 (__dest)); } + +__fortify_function void +__NTH (explicit_bzero (void *__dest, size_t __len)) +{ + (void) __builtin___memset_chk (__dest, '\0', __len, __bos0 (__dest)); + __asm__ __volatile__ ( "" : : "r"(__dest) : "memory" ); +} #endif __fortify_function char * diff --git a/string/bzero.c b/string/bzero.c index 9c220b9..9525de8 100644 --- a/string/bzero.c +++ b/string/bzero.c @@ -20,6 +20,7 @@ #include #undef __bzero +#undef __explicit_bzero /* Set N bytes of S to 0. */ void @@ -80,3 +81,17 @@ __bzero (s, len) } } weak_alias (__bzero, bzero) + + +void +__explicit_bzero(s, len) + void *s; + size_t len; +{ + __bzero (s, len); + + /* Add a barrier to prevent the compiler from optimizing the above bzero + * away. */ + __asm__ __volatile__ ( "" : : "r"(s) : "memory" ); +} +weak_alias (__explicit_bzero, explicit_bzero) diff --git a/string/explicit_bzero.c b/string/explicit_bzero.c new file mode 100644 index 0000000..7ddd190 --- /dev/null +++ b/string/explicit_bzero.c @@ -0,0 +1,32 @@ +/* Copyright (C) 1991-2014 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; if not, see + . */ + +#include +#include + +void +__explicit_bzero(s, len) + void *s; + size_t len; +{ + __bzero (s, len); + + /* Add a barrier to prevent the compiler from optimizing the above bzero + * away. */ + __asm__ __volatile__ ( "" : : "r"(s) : "memory" ); +} +weak_alias (__explicit_bzero, explicit_bzero) diff --git a/string/string.h b/string/string.h index c79debc..c6888c0 100644 --- a/string/string.h +++ b/string/string.h @@ -445,9 +445,10 @@ extern char *strerror_l (int __errnum, __locale_t __l) __THROW; #endif -/* We define this function always since `bzero' is sometimes needed when +/* We define these functions always since `bzero' is sometimes needed when the namespace rules does not allow this. */ extern void __bzero (void *__s, size_t __n) __THROW __nonnull ((1)); +extern void __explicit_bzero (void *__s, size_t __n) __THROW __nonnull ((1)); #ifdef __USE_MISC /* Copy N bytes of SRC to DEST (like memmove, but args reversed). */ @@ -457,6 +458,9 @@ extern void bcopy (const void *__src, void *__dest, size_t __n) /* Set N bytes of S to 0. */ extern void bzero (void *__s, size_t __n) __THROW __nonnull ((1)); +/* Set N bytes of S to 0, and don't let the compiler eliminate this. */ +extern void explicit_bzero (void *__s, size_t __n) __THROW __nonnull ((1)); + /* Compare N bytes of S1 and S2 (same as memcmp). */ extern int bcmp (const void *__s1, const void *__s2, size_t __n) __THROW __attribute_pure__ __nonnull ((1, 2)); diff --git a/string/strings.h b/string/strings.h index 872a0b2..12c4f51 100644 --- a/string/strings.h +++ b/string/strings.h @@ -49,6 +49,9 @@ extern void bcopy (const void *__src, void *__dest, size_t __n) __THROW; /* Set N bytes of S to 0. */ extern void bzero (void *__s, size_t __n) __THROW; +/* Set N bytes of S to 0, and don't let the compiler eliminate this. */ +extern void explicit_bzero (void *__s, size_t __n) __THROW __nonnull ((1)); + /* Find the first occurrence of C in S (same as strchr). */ # ifdef __CORRECT_ISO_CPP_STRINGS_H_PROTO extern "C++" diff --git a/string/test-explicit_bzero.c b/string/test-explicit_bzero.c new file mode 100644 index 0000000..7ed52b3 --- /dev/null +++ b/string/test-explicit_bzero.c @@ -0,0 +1,20 @@ +/* Test and measure bzero functions. + Copyright (C) 2012-2014 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; if not, see + . */ +#define TEST_EXPLICIT_BZERO +#define TEST_BZERO +#include "test-memset.c" diff --git a/string/test-memset.c b/string/test-memset.c index 2171b0d..a4820dc 100644 --- a/string/test-memset.c +++ b/string/test-memset.c @@ -18,7 +18,9 @@ . */ #define TEST_MAIN -#ifdef TEST_BZERO +#ifdef TEST_EXPLICIT_BZERO +# define TEST_NAME "explicit_bzero" +#elif defined (TEST_BZERO) # define TEST_NAME "bzero" #else # define TEST_NAME "memset" @@ -35,7 +37,11 @@ void builtin_bzero (char *, size_t); IMPL (simple_bzero, 0) IMPL (builtin_bzero, 0) +#ifdef TEST_EXPLICIT_BZERO +IMPL (explicit_bzero, 1) +#else IMPL (bzero, 1) +#endif void simple_bzero (char *s, size_t n)