[tor-commits] [tor/master] Add a data-independent variant of memcmp and a d-i memeq function.
nickm at torproject.org
nickm at torproject.org
Thu May 12 23:28:05 UTC 2011
commit 4b19730c8234de3587bd50256fcb11660fb07388
Author: Nick Mathewson <nickm at torproject.org>
Date: Mon May 9 18:39:23 2011 -0400
Add a data-independent variant of memcmp and a d-i memeq function.
The tor_memcmp code is by Robert Ransom, and the tor_memeq code is
by me. Both incorporate some ideas from DJB's stuff.
---
changes/bug3122_memcmp | 7 +++
configure.in | 18 +++++++
src/common/Makefile.am | 4 +-
src/common/di_ops.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++
src/common/di_ops.h | 28 ++++++++++
src/or/test.c | 55 ++++++++++++++++++++
6 files changed, 243 insertions(+), 2 deletions(-)
diff --git a/changes/bug3122_memcmp b/changes/bug3122_memcmp
new file mode 100644
index 0000000..a049476
--- /dev/null
+++ b/changes/bug3122_memcmp
@@ -0,0 +1,7 @@
+ o Security fixes
+ - Replace all potentially sensitive memory comparison operations
+ with versions whose runtime does not depend on the data being
+ compared. This will help resist a class of attacks where an
+ adversary can use variations in timing information to learn
+ sensitive data. Fix for one case of bug 3122. (Safe memcmp
+ implementation by Robert Ransom based partially on code by DJB.)
diff --git a/configure.in b/configure.in
index 1fddaca..eead014 100644
--- a/configure.in
+++ b/configure.in
@@ -626,6 +626,24 @@ if test "$tor_cv_twos_complement" != no ; then
[Define to 1 iff we represent negative integers with two's complement])
fi
+# What does shifting a negative value do?
+AC_CACHE_CHECK([whether right-shift on negative values does sign-extension], tor_cv_sign_extend,
+[AC_RUN_IFELSE([AC_LANG_SOURCE(
+[[int main () { int okay = (-60 >> 8) == -1; return okay ? 0 : 1; }]])],
+ [tor_cv_sign_extend=yes],
+ [tor_cv_sign_extend=no],
+ [tor_cv_sign_extend=cross])])
+
+if test "$tor_cv_sign_extend" = cross ; then
+ # Cross-compiling; let's hope that the target isn't raving mad.
+ AC_MSG_NOTICE([Cross-compiling: we'll assume that right-shifting negative integers causes sign-extension])
+fi
+
+if test "$tor_cv_sign_extend" != no ; then
+ AC_DEFINE([RSHIFT_DOES_SIGN_EXTEND], 1,
+ [Define to 1 iff right-shifting a negative value performs sign-extension])
+fi
+
# Whether we should use the dmalloc memory allocation debugging library.
AC_MSG_CHECKING(whether to use dmalloc (debug memory allocation library))
AC_ARG_WITH(dmalloc,
diff --git a/src/common/Makefile.am b/src/common/Makefile.am
index 105c413..db94a98 100644
--- a/src/common/Makefile.am
+++ b/src/common/Makefile.am
@@ -10,7 +10,7 @@ libor_extra_source=
endif
libor_a_SOURCES = address.c log.c util.c compat.c container.c mempool.c \
- memarea.c $(libor_extra_source)
+ memarea.c di_ops.c $(libor_extra_source)
libor_crypto_a_SOURCES = crypto.c aes.c tortls.c torgzip.c
-noinst_HEADERS = address.h log.h crypto.h test.h util.h compat.h aes.h torint.h tortls.h strlcpy.c strlcat.c torgzip.h container.h ht.h mempool.h memarea.h ciphers.inc
+noinst_HEADERS = address.h log.h crypto.h test.h util.h compat.h aes.h torint.h tortls.h strlcpy.c strlcat.c torgzip.h container.h ht.h mempool.h memarea.h di_ops.h ciphers.inc
diff --git a/src/common/di_ops.c b/src/common/di_ops.c
new file mode 100644
index 0000000..c1e292f
--- /dev/null
+++ b/src/common/di_ops.c
@@ -0,0 +1,133 @@
+/* Copyright (c) 2011, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+/**
+ * \file di_ops.c
+ * \brief Functions for data-independent operations
+ **/
+
+#include "orconfig.h"
+#include "di_ops.h"
+
+/**
+ * Timing-safe version of memcmp. As memcmp, compare the <b>sz</b> bytes
+ * at <b>a</b> with the <b>sz</b> bytes at <b>, and returns less than 0 if the
+ * bytes at <b>a</b> lexically precede those at <b>b</b>, 0 if the byte ranges
+ * are equal, and greater than zero if the bytes at <b>a</b> lexically follow
+ * those at <b>.
+ *
+ * This implementation differs from memcmp in that its timing behavior is not
+ * data-dependent: it should return in the same amount of time regardless of
+ * the contents of <b>a</b> and <b>b</b>.
+ */
+int
+tor_memcmp(const void *a, const void *b, size_t len)
+{
+ const uint8_t *x = a;
+ const uint8_t *y = b;
+ size_t i = len;
+ int retval = 0;
+
+ /* This loop goes from the end of the arrays to the start. At the
+ * start of every iteration, before we decrement i, we have set
+ * "retval" equal to the result of memcmp(a+i,b+i,len-i). During the
+ * loop, we update retval by leaving it unchanged if x[i]==y[i] and
+ * setting it to x[i]-y[i] if x[i]!= y[i].
+ *
+ * The following assumes we are on a system with two's-complement
+ * arithmetic. We check for this at configure-time with the check
+ * that sets USING_TWOS_COMPLEMENT. If we aren't two's complement, then
+ * torint.h will stop compilation with an error.
+ */
+ while (i--) {
+ int v1 = x[i];
+ int v2 = y[i];
+ int equal_p = v1 ^ v2;
+
+ /* The following sets bits 8 and above of equal_p to 'equal_p ==
+ * 0', and thus to v1 == v2. (To see this, note that if v1 ==
+ * v2, then v1^v2 == equal_p == 0, so equal_p-1 == -1, which is the
+ * same as ~0 on a two's-complement machine. Then note that if
+ * v1 != v2, then 0 < v1 ^ v2 < 256, so 0 <= equal_p - 1 < 255.)
+ */
+ --equal_p;
+
+ equal_p >>= 8;
+ /* Thanks to (sign-preserving) arithmetic shift, equal_p is now
+ * equal to -(v1 == v2), which is exactly what we need below.
+ * (Since we're assuming two's-complement arithmetic, -1 is the
+ * same as ~0 (all bits set).)
+ *
+ * (The result of an arithmetic shift on a negative value is
+ * actually implementation-defined in standard C. So how do we
+ * get away with assuming it? Easy. We check.) */
+#if ((-60 >> 8) != -1)
+#error "According to cpp, right-shift doesn't perform sign-extension."
+#endif
+#ifndef RSHIFT_DOES_SIGN_EXTEND
+#error "According to configure, right-shift doesn't perform sign-extension."
+#endif
+
+ /* If v1 == v2, equal_p is ~0, so this will leave retval
+ * unchanged; otherwise, equal_p is 0, so this will zero it. */
+ retval &= equal_p;
+
+ /* If v1 == v2, then this adds 0, and leaves retval unchanged.
+ * Otherwise, we just zeroed retval, so this sets it to v1 - v2. */
+ retval += (v1 - v2);
+
+ /* There. Now retval is equal to its previous value if v1 == v2, and
+ * equal to v1 - v2 if v1 != v2. */
+ }
+
+ return retval;
+}
+
+/**
+ * Timing-safe memory comparison. Return true if the <b>sz</b> bytes at
+ * <b>a</b> are the same as the <b>sz</b> bytes at <b>, and 0 otherwise.
+ *
+ * This implementation differs from !memcmp(a,b,sz) in that its timing
+ * behavior is not data-dependent: it should return in the same amount of time
+ * regardless of the contents of <b>a</b> and <b>b</b>. It differs from
+ * !tor_memcmp(a,b,sz) by being faster.
+ */
+int
+tor_memeq(const void *a, const void *b, size_t sz)
+{
+ /* Treat a and b as byte ranges. */
+ const uint8_t *ba = a, *bb = b;
+ uint32_t any_difference = 0;
+ while (sz--) {
+ /* Set byte_diff to all of those bits that are different in *ba and *bb,
+ * and advance both ba and bb. */
+ const uint8_t byte_diff = *ba++ ^ *bb++;
+
+ /* Set bits in any_difference if they are set in byte_diff. */
+ any_difference |= byte_diff;
+ }
+
+ /* Now any_difference is 0 if there are no bits different between
+ * a and b, and is nonzero if there are bits different between a
+ * and b. Now for paranoia's sake, let's convert it to 0 or 1.
+ *
+ * (If we say "!any_difference", the compiler might get smart enough
+ * to optimize-out our data-independence stuff above.)
+ *
+ * To unpack:
+ *
+ * If any_difference == 0:
+ * any_difference - 1 == ~0
+ * (any_difference - 1) >> 8 == 0x00ffffff
+ * 1 & ((any_difference - 1) >> 8) == 1
+ *
+ * If any_difference != 0:
+ * 0 < any_difference < 256, so
+ * 0 < any_difference - 1 < 255
+ * (any_difference - 1) >> 8 == 0
+ * 1 & ((any_difference - 1) >> 8) == 0
+ */
+
+ return 1 & ((any_difference - 1) >> 8);
+}
+
diff --git a/src/common/di_ops.h b/src/common/di_ops.h
new file mode 100644
index 0000000..1b223d9
--- /dev/null
+++ b/src/common/di_ops.h
@@ -0,0 +1,28 @@
+/* Copyright (c) 2003-2004, Roger Dingledine
+ * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson.
+ * Copyright (c) 2007-2011, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+/**
+ * \file di_ops.h
+ * \brief Headers for di_ops.c
+ **/
+
+#ifndef TOR_DI_OPS_H
+#define TOR_DI_OPS_H
+
+#include "orconfig.h"
+#include "torint.h"
+
+int tor_memcmp(const void *a, const void *b, size_t sz);
+int tor_memeq(const void *a, const void *b, size_t sz);
+#define tor_memneq(a,b,sz) (!tor_memeq((a),(b),(sz)))
+
+/** Alias for the platform's memcmp() function. This function is
+ * <em>not</em> data-independent: we define this alias so that we can
+ * mark cases where we are deliberately using a data-dependent memcmp()
+ * implementation.
+ */
+#define fast_memcmp(a,b,c) (memcmp((a),(b),(c)))
+
+#endif
diff --git a/src/or/test.c b/src/or/test.c
index b08f202..dc71db4 100644
--- a/src/or/test.c
+++ b/src/or/test.c
@@ -43,6 +43,7 @@ const char tor_svn_revision[] = "";
#include "torgzip.h"
#include "mempool.h"
#include "memarea.h"
+#include "di_ops.h"
#ifdef USE_DMALLOC
#include <dmalloc.h>
@@ -1331,6 +1332,59 @@ test_util(void)
;
}
+static void
+test_util_di_ops(void)
+{
+#define LT -1
+#define GT 1
+#define EQ 0
+ const struct {
+ const char *a; int want_sign; const char *b;
+ } examples[] = {
+ { "Foo", EQ, "Foo" },
+ { "foo", GT, "bar", },
+ { "foobar", EQ ,"foobar" },
+ { "foobar", LT, "foobaw" },
+ { "foobar", GT, "f00bar" },
+ { "foobar", GT, "boobar" },
+ { "", EQ, "" },
+ { NULL, 0, NULL },
+ };
+
+ int i;
+
+ for (i = 0; examples[i].a; ++i) {
+ size_t len = strlen(examples[i].a);
+ int eq1, eq2, neq1, neq2, cmp1, cmp2;
+ test_eq(len, strlen(examples[i].b));
+ /* We do all of the operations, with operands in both orders. */
+ eq1 = tor_memeq(examples[i].a, examples[i].b, len);
+ eq2 = tor_memeq(examples[i].b, examples[i].a, len);
+ neq1 = tor_memneq(examples[i].a, examples[i].b, len);
+ neq2 = tor_memneq(examples[i].b, examples[i].a, len);
+ cmp1 = tor_memcmp(examples[i].a, examples[i].b, len);
+ cmp2 = tor_memcmp(examples[i].b, examples[i].a, len);
+
+ /* Check for correctness of cmp1 */
+ if (cmp1 < 0 && examples[i].want_sign != LT)
+ test_fail();
+ else if (cmp1 > 0 && examples[i].want_sign != GT)
+ test_fail();
+ else if (cmp1 == 0 && examples[i].want_sign != EQ)
+ test_fail();
+
+ /* Check for consistency of everything else with cmp1 */
+ test_eq(eq1, eq2);
+ test_eq(neq1, neq2);
+ test_eq(cmp1, -cmp2);
+ test_eq(eq1, cmp1 == 0);
+ test_eq(neq1, !eq1);
+ }
+
+ done:
+ ;
+}
+
/** Helper: assert that IPv6 addresses <b>a</b> and <b>b</b> are the same. On
* failure, reports an error, describing the addresses as <b>e1</b> and
* <b>e2</b>, and reporting the line number as <b>line</b>. */
@@ -4753,6 +4807,7 @@ static struct {
SUBENT(crypto, aes_iv),
SUBENT(crypto, base32_decode),
ENT(util),
+ SUBENT(util, di_ops),
SUBENT(util, ip6_helpers),
SUBENT(util, gzip),
SUBENT(util, datadir),
More information about the tor-commits
mailing list