Add size_t variants of constant-time functions.
These will be used in follow-up commits. The _s names are taken from
upstream, to ease importing code. I've also promoted the CONSTTIME_*
macros from the test. None of them are really necessary except
~0u cannot substitute for CONSTTIME_TRUE_S on 64-bit platforms, so
having the macros seems safer.
Once everything is converted, I expect the unsigned versions can be
removed, so I've made the _8 and _int functions act on size_t rather
than unsigned. The users of these functions basically only believe that
array indices and bytes exist.
BUG=22
Change-Id: I987bfb0c708dc726a6f2afcb05b6619bbd600564
Reviewed-on: https://boringssl-review.googlesource.com/14306
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/cipher/e_tls.c b/crypto/cipher/e_tls.c
index 7d9bbee..7af44f3 100644
--- a/crypto/cipher/e_tls.c
+++ b/crypto/cipher/e_tls.c
@@ -273,7 +273,7 @@
return 0;
}
} else {
- padding_ok = ~0u;
+ padding_ok = CONSTTIME_TRUE;
data_plus_mac_len = total;
/* |data_plus_mac_len| = |total| = |in_len| at this point. |in_len| has
* already been checked against the MAC size at the top of the function. */
diff --git a/crypto/constant_time_test.cc b/crypto/constant_time_test.cc
index adfe272..628681f 100644
--- a/crypto/constant_time_test.cc
+++ b/crypto/constant_time_test.cc
@@ -49,14 +49,11 @@
#include <stdio.h>
#include <stdlib.h>
+#include <limits>
+
#include <gtest/gtest.h>
-static const unsigned CONSTTIME_TRUE = (unsigned)(~0);
-static const unsigned CONSTTIME_FALSE = 0;
-static const uint8_t CONSTTIME_TRUE_8 = 0xff;
-static const uint8_t CONSTTIME_FALSE_8 = 0;
-
static unsigned FromBool(bool b) {
return b ? CONSTTIME_TRUE : CONSTTIME_FALSE;
}
@@ -65,6 +62,10 @@
return b ? CONSTTIME_TRUE_8 : CONSTTIME_FALSE_8;
}
+static size_t FromBoolS(bool b) {
+ return b ? CONSTTIME_TRUE_S : CONSTTIME_FALSE_S;
+}
+
static unsigned test_values[] = {
0,
1,
@@ -80,28 +81,63 @@
static uint8_t test_values_8[] = {0, 1, 2, 20, 32, 127, 128, 129, 255};
+static size_t test_values_s[] = {
+ 0,
+ 1,
+ 1024,
+ 12345,
+ 32000,
+#if defined(OPENSSL_64_BIT)
+ 0xffffffff / 2 - 1,
+ 0xffffffff / 2,
+ 0xffffffff / 2 + 1,
+ 0xffffffff - 1,
+ 0xffffffff,
+#endif
+ std::numeric_limits<size_t>::max() / 2 - 1,
+ std::numeric_limits<size_t>::max() / 2,
+ std::numeric_limits<size_t>::max() / 2 + 1,
+ std::numeric_limits<size_t>::max() - 1,
+ std::numeric_limits<size_t>::max(),
+};
+
static int signed_test_values[] = {
0, 1, -1, 1024, -1024, 12345, -12345,
32000, -32000, INT_MAX, INT_MIN, INT_MAX - 1, INT_MIN + 1};
TEST(ConstantTimeTest, Test) {
- for (unsigned a : test_values) {
+ for (size_t a : test_values_s) {
SCOPED_TRACE(a);
- EXPECT_EQ(FromBool(a == 0), constant_time_is_zero(a));
+ EXPECT_EQ(FromBoolS(a == 0), constant_time_is_zero_s(a));
EXPECT_EQ(FromBool8(a == 0), constant_time_is_zero_8(a));
+ for (size_t b : test_values_s) {
+ SCOPED_TRACE(b);
+
+ EXPECT_EQ(FromBoolS(a < b), constant_time_lt_s(a, b));
+ EXPECT_EQ(FromBool8(a < b), constant_time_lt_8(a, b));
+
+ EXPECT_EQ(FromBoolS(a >= b), constant_time_ge_s(a, b));
+ EXPECT_EQ(FromBool8(a >= b), constant_time_ge_8(a, b));
+
+ EXPECT_EQ(FromBoolS(a == b), constant_time_eq_s(a, b));
+ EXPECT_EQ(FromBool8(a == b), constant_time_eq_8(a, b));
+
+ EXPECT_EQ(a, constant_time_select_s(CONSTTIME_TRUE_S, a, b));
+ EXPECT_EQ(b, constant_time_select_s(CONSTTIME_FALSE_S, a, b));
+ }
+ }
+
+ for (unsigned a : test_values) {
+ SCOPED_TRACE(a);
+ EXPECT_EQ(FromBool(a == 0), constant_time_is_zero(a));
for (unsigned b : test_values) {
SCOPED_TRACE(b);
EXPECT_EQ(FromBool(a < b), constant_time_lt(a, b));
- EXPECT_EQ(FromBool8(a < b), constant_time_lt_8(a, b));
-
EXPECT_EQ(FromBool(a >= b), constant_time_ge(a, b));
- EXPECT_EQ(FromBool8(a >= b), constant_time_ge_8(a, b));
-
EXPECT_EQ(FromBool(a == b), constant_time_eq(a, b));
- EXPECT_EQ(FromBool8(a == b), constant_time_eq_8(a, b));
EXPECT_EQ(a, constant_time_select(CONSTTIME_TRUE, a, b));
EXPECT_EQ(b, constant_time_select(CONSTTIME_FALSE, a, b));
@@ -113,10 +149,14 @@
for (int b : signed_test_values) {
SCOPED_TRACE(b);
+ // constant_time_select_int accepts both size_t and unsigned masks.
EXPECT_EQ(a, constant_time_select_int(CONSTTIME_TRUE, a, b));
EXPECT_EQ(b, constant_time_select_int(CONSTTIME_FALSE, a, b));
- EXPECT_EQ(FromBool(a == b), constant_time_eq_int(a, b));
+ EXPECT_EQ(a, constant_time_select_int(CONSTTIME_TRUE_S, a, b));
+ EXPECT_EQ(b, constant_time_select_int(CONSTTIME_FALSE_S, a, b));
+
+ EXPECT_EQ(FromBoolS(a == b), constant_time_eq_int(a, b));
EXPECT_EQ(FromBool8(a == b), constant_time_eq_int_8(a, b));
}
}
diff --git a/crypto/internal.h b/crypto/internal.h
index 2724956..4e6026f 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -186,14 +186,27 @@
* unsigned int lt = constant_time_lt(a, b);
* c = constant_time_select(lt, a, b); */
+#define CONSTTIME_TRUE_S ~((size_t)0)
+#define CONSTTIME_FALSE_S ((size_t)0)
+#define CONSTTIME_TRUE ~0u
+#define CONSTTIME_FALSE 0u
+#define CONSTTIME_TRUE_8 ((uint8_t)0xff)
+#define CONSTTIME_FALSE_8 ((uint8_t)0)
+
+/* constant_time_msb_s returns the given value with the MSB copied to all the
+ * other bits. */
+static inline size_t constant_time_msb_s(size_t a) {
+ return 0u - (a >> (sizeof(a) * 8 - 1));
+}
+
/* constant_time_msb returns the given value with the MSB copied to all the
* other bits. */
static inline unsigned int constant_time_msb(unsigned int a) {
- return (unsigned int)((int)(a) >> (sizeof(int) * 8 - 1));
+ return 0u - (a >> (sizeof(a) * 8 - 1));
}
-/* constant_time_lt returns 0xff..f if a < b and 0 otherwise. */
-static inline unsigned int constant_time_lt(unsigned int a, unsigned int b) {
+/* constant_time_lt_s returns 0xff..f if a < b and 0 otherwise. */
+static inline size_t constant_time_lt_s(size_t a, size_t b) {
/* Consider the two cases of the problem:
* msb(a) == msb(b): a < b iff the MSB of a - b is set.
* msb(a) != msb(b): a < b iff the MSB of b is set.
@@ -225,26 +238,38 @@
* (check-sat)
* (get-model)
*/
- return constant_time_msb(a^((a^b)|((a-b)^a)));
+ return constant_time_msb_s(a^((a^b)|((a-b)^a)));
}
-/* constant_time_lt_8 acts like |constant_time_lt| but returns an 8-bit mask. */
-static inline uint8_t constant_time_lt_8(unsigned int a, unsigned int b) {
- return (uint8_t)(constant_time_lt(a, b));
+/* constant_time_lt acts like |constant_time_lt_s| but acts on |unsigned|. */
+static inline unsigned constant_time_lt(unsigned a, unsigned b) {
+ return (unsigned)constant_time_lt_s(a, b);
}
-/* constant_time_gt returns 0xff..f if a >= b and 0 otherwise. */
-static inline unsigned int constant_time_ge(unsigned int a, unsigned int b) {
- return ~constant_time_lt(a, b);
+/* constant_time_lt_8 acts like |constant_time_lt_s| but returns an 8-bit
+ * mask. */
+static inline uint8_t constant_time_lt_8(size_t a, size_t b) {
+ return (uint8_t)(constant_time_lt_s(a, b));
}
-/* constant_time_ge_8 acts like |constant_time_ge| but returns an 8-bit mask. */
-static inline uint8_t constant_time_ge_8(unsigned int a, unsigned int b) {
- return (uint8_t)(constant_time_ge(a, b));
+/* constant_time_ge_s returns 0xff..f if a >= b and 0 otherwise. */
+static inline size_t constant_time_ge_s(size_t a, size_t b) {
+ return ~constant_time_lt_s(a, b);
+}
+
+/* constant_time_ge acts like |constant_time_ge_s| but acts on |unsigned|. */
+static inline unsigned constant_time_ge(unsigned a, unsigned b) {
+ return (unsigned)(constant_time_ge_s(a, b));
+}
+
+/* constant_time_ge_8 acts like |constant_time_ge_s| but returns an 8-bit
+ * mask. */
+static inline uint8_t constant_time_ge_8(size_t a, size_t b) {
+ return (uint8_t)(constant_time_ge_s(a, b));
}
/* constant_time_is_zero returns 0xff..f if a == 0 and 0 otherwise. */
-static inline unsigned int constant_time_is_zero(unsigned int a) {
+static inline size_t constant_time_is_zero_s(size_t a) {
/* Here is an SMT-LIB verification of this formula:
*
* (define-fun is_zero ((a (_ BitVec 32))) (_ BitVec 32)
@@ -257,55 +282,72 @@
* (check-sat)
* (get-model)
*/
- return constant_time_msb(~a & (a - 1));
+ return constant_time_msb_s(~a & (a - 1));
}
-/* constant_time_is_zero_8 acts like constant_time_is_zero but returns an 8-bit
+/* constant_time_is_zero acts like |constant_time_is_zero_s| but acts on
+ * |unsigned|. */
+static inline unsigned constant_time_is_zero(unsigned a) {
+ return (unsigned)constant_time_is_zero_s(a);
+}
+
+/* constant_time_is_zero_8 acts like |constant_time_is_zero_s| but returns an
+ * 8-bit mask. */
+static inline uint8_t constant_time_is_zero_8(size_t a) {
+ return (uint8_t)(constant_time_is_zero_s(a));
+}
+
+/* constant_time_eq_s returns 0xff..f if a == b and 0 otherwise. */
+static inline size_t constant_time_eq_s(size_t a, size_t b) {
+ return constant_time_is_zero_s(a ^ b);
+}
+
+/* constant_time_eq acts like |constant_time_eq_s| but acts on |unsigned|. */
+static inline unsigned constant_time_eq(unsigned a, unsigned b) {
+ return (unsigned)constant_time_eq_s(a, b);
+}
+
+/* constant_time_eq_8 acts like |constant_time_eq_s| but returns an 8-bit
* mask. */
-static inline uint8_t constant_time_is_zero_8(unsigned int a) {
- return (uint8_t)(constant_time_is_zero(a));
+static inline uint8_t constant_time_eq_8(size_t a, size_t b) {
+ return (uint8_t)(constant_time_eq_s(a, b));
}
-/* constant_time_eq returns 0xff..f if a == b and 0 otherwise. */
-static inline unsigned int constant_time_eq(unsigned int a, unsigned int b) {
- return constant_time_is_zero(a ^ b);
-}
-
-/* constant_time_eq_8 acts like |constant_time_eq| but returns an 8-bit mask. */
-static inline uint8_t constant_time_eq_8(unsigned int a, unsigned int b) {
- return (uint8_t)(constant_time_eq(a, b));
-}
-
-/* constant_time_eq_int acts like |constant_time_eq| but works on int values. */
-static inline unsigned int constant_time_eq_int(int a, int b) {
- return constant_time_eq((unsigned)(a), (unsigned)(b));
+/* constant_time_eq_int acts like |constant_time_eq_s| but works on int
+ * values. */
+static inline size_t constant_time_eq_int(int a, int b) {
+ return constant_time_eq_s((size_t)(a), (size_t)(b));
}
/* constant_time_eq_int_8 acts like |constant_time_eq_int| but returns an 8-bit
* mask. */
static inline uint8_t constant_time_eq_int_8(int a, int b) {
- return constant_time_eq_8((unsigned)(a), (unsigned)(b));
+ return constant_time_eq_8((size_t)(a), (size_t)(b));
}
-/* constant_time_select returns (mask & a) | (~mask & b). When |mask| is all 1s
- * or all 0s (as returned by the methods above), the select methods return
+/* constant_time_select_s returns (mask & a) | (~mask & b). When |mask| is all
+ * 1s or all 0s (as returned by the methods above), the select methods return
* either |a| (if |mask| is nonzero) or |b| (if |mask| is zero). */
-static inline unsigned int constant_time_select(unsigned int mask,
- unsigned int a, unsigned int b) {
+static inline size_t constant_time_select_s(size_t mask, size_t a, size_t b) {
return (mask & a) | (~mask & b);
}
+static inline unsigned constant_time_select(unsigned mask, unsigned a,
+ unsigned b) {
+ return (unsigned)(constant_time_select_s(mask, a, b));
+}
+
/* constant_time_select_8 acts like |constant_time_select| but operates on
* 8-bit values. */
static inline uint8_t constant_time_select_8(uint8_t mask, uint8_t a,
uint8_t b) {
- return (uint8_t)(constant_time_select(mask, a, b));
+ return (uint8_t)(constant_time_select_s(mask, a, b));
}
/* constant_time_select_int acts like |constant_time_select| but operates on
* ints. */
-static inline int constant_time_select_int(unsigned int mask, int a, int b) {
- return (int)(constant_time_select(mask, (unsigned)(a), (unsigned)(b)));
+static inline int constant_time_select_int(size_t mask, int a, int b) {
+ return (int)(constant_time_select_s(mask, (size_t)(a), (size_t)(b)));
}
diff --git a/crypto/rsa/padding.c b/crypto/rsa/padding.c
index ac583c4..c6751af 100644
--- a/crypto/rsa/padding.c
+++ b/crypto/rsa/padding.c
@@ -205,7 +205,7 @@
unsigned first_byte_is_zero = constant_time_eq(from[0], 0);
unsigned second_byte_is_two = constant_time_eq(from[1], 2);
- unsigned i, zero_index = 0, looking_for_index = ~0u;
+ unsigned i, zero_index = 0, looking_for_index = CONSTTIME_TRUE;
for (i = 2; i < from_len; i++) {
unsigned equals0 = constant_time_is_zero(from[i]);
zero_index = constant_time_select(looking_for_index & equals0, (unsigned)i,
@@ -439,7 +439,7 @@
bad = ~constant_time_is_zero(CRYPTO_memcmp(db, phash, mdlen));
bad |= ~constant_time_is_zero(from[0]);
- looking_for_one_byte = ~0u;
+ looking_for_one_byte = CONSTTIME_TRUE;
for (i = mdlen; i < dblen; i++) {
unsigned equals1 = constant_time_eq(db[i], 1);
unsigned equals0 = constant_time_eq(db[i], 0);
