From e82e600d6edb04247d1fd15f1d4b340ef544736e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 19 Mar 2015 15:28:22 -0400 Subject: [PATCH 1/2] Here is a test for memwipe. It invokes undefined behavior, I'm afraid, since there's no other c-legal way to test whether memwipe() works when we're not allowed to look at it. Closes ticket 15377. --- changes/test-memwipe | 3 + src/test/include.am | 11 ++- src/test/test-memwipe.c | 204 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 217 insertions(+), 1 deletion(-) create mode 100644 changes/test-memwipe create mode 100644 src/test/test-memwipe.c diff --git a/changes/test-memwipe b/changes/test-memwipe new file mode 100644 index 000000000..04a21f75f --- /dev/null +++ b/changes/test-memwipe @@ -0,0 +1,3 @@ + o Testing: + - Add a test to verify that the compiler does not eliminate our + memwipe() implementation. Closes ticket 15377. diff --git a/src/test/include.am b/src/test/include.am index c56e887ca..46fb99fc3 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -1,10 +1,11 @@ -TESTS += src/test/test src/test/test-slow +TESTS += src/test/test src/test/test-slow src/test/test-memwipe noinst_PROGRAMS+= src/test/bench if UNITTESTS_ENABLED noinst_PROGRAMS+= \ src/test/test \ src/test/test-slow \ + src/test/test-memwipe \ src/test/test-child \ src/test/test_workqueue endif @@ -73,6 +74,9 @@ src_test_test_slow_SOURCES = \ src/test/testing_common.c \ src/ext/tinytest.c +src_test_test_memwipe_SOURCES = \ + src/test/test-memwipe.c + src_test_test_CFLAGS = $(AM_CFLAGS) $(TEST_CFLAGS) @@ -100,6 +104,11 @@ src_test_test_slow_CFLAGS = $(src_test_test_CFLAGS) src_test_test_slow_LDADD = $(src_test_test_LDADD) src_test_test_slow_LDFLAGS = $(src_test_test_LDFLAGS) +src_test_test_memwipe_CPPFLAGS = $(src_test_test_CPPFLAGS) +src_test_test_memwipe_CFLAGS = $(src_test_test_CFLAGS) +src_test_test_memwipe_LDADD = $(src_test_test_LDADD) +src_test_test_memwipe_LDFLAGS = $(src_test_test_LDFLAGS) + src_test_bench_LDFLAGS = @TOR_LDFLAGS_zlib@ @TOR_LDFLAGS_openssl@ \ @TOR_LDFLAGS_libevent@ src_test_bench_LDADD = src/or/libtor.a src/common/libor.a \ diff --git a/src/test/test-memwipe.c b/src/test/test-memwipe.c new file mode 100644 index 000000000..a721a8e72 --- /dev/null +++ b/src/test/test-memwipe.c @@ -0,0 +1,204 @@ +#include +#include +#include +#include + +#include "crypto.h" +#include "compat.h" + +#undef MIN +#define MIN(a,b) ( ((a)<(b)) ? (a) : (b) ) + +static unsigned fill_a_buffer_memset(void) __attribute__((noinline)); +static unsigned fill_a_buffer_memwipe(void) __attribute__((noinline)); +static unsigned fill_a_buffer_nothing(void) __attribute__((noinline)); +static unsigned fill_heap_buffer_memset(void) __attribute__((noinline)); +static unsigned fill_heap_buffer_memwipe(void) __attribute__((noinline)); +static unsigned fill_heap_buffer_nothing(void) __attribute__((noinline)); +static unsigned check_a_buffer(void) __attribute__((noinline)); + +const char *s = NULL; + +#define FILL_BUFFER_IMPL() \ + unsigned int i; \ + unsigned sum = 0; \ + \ + /* Fill up a 1k buffer with a recognizable pattern. */ \ + for (i = 0; i < 2048; i += strlen(s)) { \ + memcpy(buf+i, s, MIN(strlen(s), 2048-i)); \ + } \ + \ + /* Use the buffer as input to a computation so the above can't get */ \ + /* optimized away. */ \ + for (i = 0; i < 2048; ++i) { \ + sum += (unsigned char)buf[i]; \ + } + +static unsigned +fill_a_buffer_memset(void) +{ + char buf[2048]; + FILL_BUFFER_IMPL() + memset(buf, 0, sizeof(buf)); + return sum; +} + +static unsigned +fill_a_buffer_memwipe(void) +{ + char buf[2048]; + FILL_BUFFER_IMPL() + memwipe(buf, 0, sizeof(buf)); + return sum; +} + +static unsigned +fill_a_buffer_nothing(void) +{ + char buf[2048]; + FILL_BUFFER_IMPL() + return sum; +} + +static INLINE int +vmemeq(volatile char *a, const char *b, size_t n) +{ + while (n--) { + if (*a++ != *b++) + return 0; + } + return 1; +} + +static unsigned +check_a_buffer(void) +{ + unsigned int i; + volatile char buf[1024]; + unsigned sum = 0; + + /* See if this buffer has the string in it. + + YES, THIS DOES INVOKE UNDEFINED BEHAVIOR BY READING FROM AN UNINITIALIZED + BUFFER. + + If you know a better way to figure out whether the compiler eliminated + the memset/memwipe calls or not, please let me know. + */ + for (i = 0; i < sizeof(buf); ++i) { + if (vmemeq(buf+i, s, strlen(s))) + ++sum; + } + + return sum; +} + +static char *heap_buf = NULL; + +static unsigned +fill_heap_buffer_memset(void) +{ + char *buf = heap_buf = malloc(2048); + FILL_BUFFER_IMPL() + memset(buf, 0, 2048); + free(buf); + return sum; +} + +static unsigned +fill_heap_buffer_memwipe(void) +{ + char *buf = heap_buf = malloc(2048); + FILL_BUFFER_IMPL() + memwipe(buf, 0, 2048); + free(buf); + return sum; +} + +static unsigned +fill_heap_buffer_nothing(void) +{ + char *buf = heap_buf = malloc(2048); + FILL_BUFFER_IMPL() + free(buf); + return sum; +} + +static unsigned +check_heap_buffer(void) +{ + unsigned int i; + unsigned sum = 0; + volatile char *buf = heap_buf; + + /* See if this buffer has the string in it. + + YES, THIS DOES INVOKE UNDEFINED BEHAVIOR BY READING FROM A FREED BUFFER. + + If you know a better way to figure out whether the compiler eliminated + the memset/memwipe calls or not, please let me know. + */ + for (i = 0; i < sizeof(buf); ++i) { + if (vmemeq(buf+i, s, strlen(s))) + ++sum; + } + + return sum; +} + +static struct testcase { + const char *name; + unsigned (*fill_fn)(void); + unsigned (*check_fn)(void); +} testcases[] = { + { "nil", fill_a_buffer_nothing, check_a_buffer }, + { "nil-heap", fill_heap_buffer_nothing, check_heap_buffer }, + { "memset", fill_a_buffer_memset, check_a_buffer }, + { "memset-heap", fill_heap_buffer_memset, check_heap_buffer }, + { "memwipe", fill_a_buffer_memwipe, check_a_buffer }, + { "memwipe-heap", fill_heap_buffer_memwipe, check_heap_buffer }, + { NULL, NULL, NULL } +}; + +int +main(int argc, char **argv) +{ + unsigned x, x2; + int i; + int working = 1; + unsigned found[6]; + (void) argc; (void) argv; + + s = "squamous haberdasher gallimaufry"; + + memset(found, 0, sizeof(found)); + + for (i = 0; testcases[i].name; ++i) { + x = testcases[i].fill_fn(); + found[i] = testcases[i].check_fn(); + + x2 = fill_a_buffer_nothing(); + + if (x != x2) { + working = 0; + } + } + + if (!working || !found[0] || !found[1]) { + printf("It appears that this test case may not give you reliable " + "information. Sorry.\n"); + } + + if (!found[2] && !found[3]) { + printf("It appears that memset is good enough on this platform. Good.\n"); + } + + if (found[4] || found[5]) { + printf("ERROR: memwipe does not wipe data!\n"); + return 1; + } else { + printf("OKAY: memwipe seems to work."); + return 0; + } +} + From 2dc5d790d314beb202dfa1659161113f1f6dabf1 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Tue, 24 Mar 2015 16:04:28 -0400 Subject: [PATCH 2/2] Test: add missing libor.a in include.am Also add test-memwipe to .gitignore file. Signed-off-by: David Goulet --- .gitignore | 1 + src/test/include.am | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index b31a7132d..d1bdc68de 100644 --- a/.gitignore +++ b/.gitignore @@ -167,6 +167,7 @@ cscope.* /src/test/test-slow /src/test/test-bt-cl /src/test/test-child +/src/test/test-memwipe /src/test/test-ntor-cl /src/test/test_workqueue /src/test/test.exe diff --git a/src/test/include.am b/src/test/include.am index 46fb99fc3..c0a1b374f 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -93,7 +93,7 @@ src_test_test_workqueue_CFLAGS = $(AM_CFLAGS) $(TEST_CFLAGS) src_test_test_LDFLAGS = @TOR_LDFLAGS_zlib@ @TOR_LDFLAGS_openssl@ \ @TOR_LDFLAGS_libevent@ src_test_test_LDADD = src/or/libtor-testing.a src/common/libor-testing.a \ - src/common/libor-crypto-testing.a $(LIBDONNA) \ + src/common/libor-crypto-testing.a $(LIBDONNA) src/common/libor.a \ src/common/libor-event-testing.a src/trunnel/libor-trunnel-testing.a \ @TOR_ZLIB_LIBS@ @TOR_LIB_MATH@ @TOR_LIBEVENT_LIBS@ \ @TOR_OPENSSL_LIBS@ @TOR_LIB_WS32@ @TOR_LIB_GDI@ @CURVE25519_LIBS@ \