Switch constant-time functions to using |crypto_word_t|.

Using |size_t| was correct, except for NaCl, which is a 64-bit build
with 32-bit pointers. In that configuration, |size_t| is smaller than
the native word size.

This change adds |crypto_word_t|, an unsigned type with native size and
switches constant-time functions to using it.

Change-Id: Ib275127063d5edbb7c55d413132711b7c74206b0
Reviewed-on: https://boringssl-review.googlesource.com/15325
Reviewed-by: Adam Langley <agl@google.com>

crypto/cipher/e_tls.c

@@ -262,7 +262,8 @@ static int aead_tls_open(const EVP_AEAD_CTX *ctx, uint8_t *out,
 
   /* Remove CBC padding. Code from here on is timing-sensitive with respect to
    * |padding_ok| and |data_plus_mac_len| for CBC ciphers. */
-  size_t padding_ok, data_plus_mac_len;
+  size_t data_plus_mac_len;
+  crypto_word_t padding_ok;
   if (EVP_CIPHER_CTX_mode(&tls_ctx->cipher_ctx) == EVP_CIPH_CBC_MODE) {
     if (!EVP_tls_cbc_remove_padding(
             &padding_ok, &data_plus_mac_len, out, total,
@@ -273,7 +274,7 @@ static int aead_tls_open(const EVP_AEAD_CTX *ctx, uint8_t *out,
       return 0;
     }
   } else {
-    padding_ok = CONSTTIME_TRUE_S;
+    padding_ok = CONSTTIME_TRUE_W;
     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. */
@@ -332,7 +333,7 @@ static int aead_tls_open(const EVP_AEAD_CTX *ctx, uint8_t *out,
    * safe to simply perform the padding check first, but it would not be under a
    * different choice of MAC location on padding failure. See
    * EVP_tls_cbc_remove_padding. */
-  size_t good =
+  crypto_word_t good =
       constant_time_eq_int(CRYPTO_memcmp(record_mac, mac, mac_len), 0);
   good &= padding_ok;
   if (!good) {

crypto/cipher/internal.h

@@ -62,6 +62,7 @@
 #include <openssl/aead.h>
 #include <openssl/aes.h>
 
+#include "../internal.h"
 #include "../fipsmodule/modes/internal.h"
 
 #if defined(__cplusplus)
@@ -113,7 +114,7 @@ struct evp_aead_st {
  * If the function returns one, it runs in time independent of the contents of
  * |in|. It is also guaranteed that |*out_len| >= |mac_size|, satisfying
  * |EVP_tls_cbc_copy_mac|'s precondition. */
-int EVP_tls_cbc_remove_padding(size_t *out_padding_ok, size_t *out_len,
+int EVP_tls_cbc_remove_padding(crypto_word_t *out_padding_ok, size_t *out_len,
                                const uint8_t *in, size_t in_len,
                                size_t block_size, size_t mac_size);
 

crypto/cipher/tls_cbc.c

@@ -61,9 +61,6 @@
 #include "internal.h"
 
 
-/* TODO(davidben): unsigned should be size_t. The various constant_time
- * functions need to be switched to size_t. */
-
 /* MAX_HASH_BIT_COUNT_BYTES is the maximum number of bytes in the hash's length
  * field. (SHA-384/512 have 128-bit length.) */
 #define MAX_HASH_BIT_COUNT_BYTES 16
@@ -73,7 +70,7 @@
  * supported by TLS.) */
 #define MAX_HASH_BLOCK_SIZE 128
 
-int EVP_tls_cbc_remove_padding(size_t *out_padding_ok, size_t *out_len,
+int EVP_tls_cbc_remove_padding(crypto_word_t *out_padding_ok, size_t *out_len,
                                const uint8_t *in, size_t in_len,
                                size_t block_size, size_t mac_size) {
   const size_t overhead = 1 /* padding length byte */ + mac_size;
@@ -85,7 +82,7 @@ int EVP_tls_cbc_remove_padding(size_t *out_padding_ok, size_t *out_len,
 
   size_t padding_length = in[in_len - 1];
 
-  size_t good = constant_time_ge_s(in_len, overhead + padding_length);
+  crypto_word_t good = constant_time_ge_w(in_len, overhead + padding_length);
   /* The padding consists of a length byte at the end of the record and
    * then that many bytes of padding, all with the same value as the
    * length byte. Thus, with the length byte included, there are i+1
@@ -110,7 +107,7 @@ int EVP_tls_cbc_remove_padding(size_t *out_padding_ok, size_t *out_len,
 
   /* If any of the final |padding_length+1| bytes had the wrong value,
    * one or more of the lower eight bits of |good| will be cleared. */
-  good = constant_time_eq_s(0xff, good & 0xff);
+  good = constant_time_eq_w(0xff, good & 0xff);
 
   /* Always treat |padding_length| as zero on error. If, assuming block size of
    * 16, a padding of [<15 arbitrary bytes> 15] treated |padding_length| as 16
@@ -151,7 +148,7 @@ void EVP_tls_cbc_copy_mac(uint8_t *out, size_t md_size, const uint8_t *in,
     if (j >= md_size) {
       j -= md_size;
     }
-    size_t is_mac_start = constant_time_eq_s(i, mac_start);
+    crypto_word_t is_mac_start = constant_time_eq_w(i, mac_start);
     mac_started |= is_mac_start;
     uint8_t mac_ended = constant_time_ge_8(i, mac_end);
     rotated_mac[j] |= in[i] & mac_started & ~mac_ended;

crypto/constant_time_test.cc

@@ -58,13 +58,13 @@ static uint8_t FromBool8(bool b) {
   return b ? CONSTTIME_TRUE_8 : CONSTTIME_FALSE_8;
 }
 
-static size_t FromBoolS(bool b) {
-  return b ? CONSTTIME_TRUE_S : CONSTTIME_FALSE_S;
+static crypto_word_t FromBoolW(bool b) {
+  return b ? CONSTTIME_TRUE_W : CONSTTIME_FALSE_W;
 }
 
 static const uint8_t test_values_8[] = {0, 1, 2, 20, 32, 127, 128, 129, 255};
 
-static size_t test_values_s[] = {
+static crypto_word_t test_values_w[] = {
     0,
     1,
     1024,
@@ -77,11 +77,11 @@ static size_t test_values_s[] = {
     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(),
+    std::numeric_limits<crypto_word_t>::max() / 2 - 1,
+    std::numeric_limits<crypto_word_t>::max() / 2,
+    std::numeric_limits<crypto_word_t>::max() / 2 + 1,
+    std::numeric_limits<crypto_word_t>::max() - 1,
+    std::numeric_limits<crypto_word_t>::max(),
 };
 
 static int signed_test_values[] = {
@@ -89,26 +89,26 @@ static int signed_test_values[] = {
     32000, -32000, INT_MAX, INT_MIN, INT_MAX - 1, INT_MIN + 1};
 
 TEST(ConstantTimeTest, Test) {
-  for (size_t a : test_values_s) {
+  for (crypto_word_t a : test_values_w) {
     SCOPED_TRACE(a);
 
-    EXPECT_EQ(FromBoolS(a == 0), constant_time_is_zero_s(a));
+    EXPECT_EQ(FromBoolW(a == 0), constant_time_is_zero_w(a));
     EXPECT_EQ(FromBool8(a == 0), constant_time_is_zero_8(a));
 
-    for (size_t b : test_values_s) {
+    for (crypto_word_t b : test_values_w) {
       SCOPED_TRACE(b);
 
-      EXPECT_EQ(FromBoolS(a < b), constant_time_lt_s(a, b));
+      EXPECT_EQ(FromBoolW(a < b), constant_time_lt_w(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(FromBoolW(a >= b), constant_time_ge_w(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(FromBoolW(a == b), constant_time_eq_w(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));
+      EXPECT_EQ(a, constant_time_select_w(CONSTTIME_TRUE_W, a, b));
+      EXPECT_EQ(b, constant_time_select_w(CONSTTIME_FALSE_W, a, b));
     }
   }
 
@@ -117,10 +117,10 @@ TEST(ConstantTimeTest, Test) {
     for (int b : signed_test_values) {
       SCOPED_TRACE(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(a, constant_time_select_int(CONSTTIME_TRUE_W, a, b));
+      EXPECT_EQ(b, constant_time_select_int(CONSTTIME_FALSE_W, a, b));
 
-      EXPECT_EQ(FromBoolS(a == b), constant_time_eq_int(a, b));
+      EXPECT_EQ(FromBoolW(a == b), constant_time_eq_int(a, b));
       EXPECT_EQ(FromBool8(a == b), constant_time_eq_int_8(a, b));
     }
   }

crypto/fipsmodule/modes/polyval.c

@@ -36,11 +36,11 @@ static void byte_reverse(polyval_block *b) {
 static void reverse_and_mulX_ghash(polyval_block *b) {
   uint64_t hi = b->u[0];
   uint64_t lo = b->u[1];
-  const size_t carry = constant_time_eq_s(hi & 1, 1);
+  const crypto_word_t carry = constant_time_eq_w(hi & 1, 1);
   hi >>= 1;
   hi |= lo << 63;
   lo >>= 1;
-  lo ^= ((uint64_t) constant_time_select_s(carry, 0xe1, 0)) << 56;
+  lo ^= ((uint64_t) constant_time_select_w(carry, 0xe1, 0)) << 56;
 
   b->u[0] = CRYPTO_bswap8(lo);
   b->u[1] = CRYPTO_bswap8(hi);

crypto/internal.h

@@ -183,22 +183,41 @@ static inline int buffers_alias(const uint8_t *a, size_t a_len,
  *
  * can be written as
  *
- * size_t lt = constant_time_lt_s(a, b);
- * c = constant_time_select_s(lt, a, b); */
+ * crypto_word_t lt = constant_time_lt_w(a, b);
+ * c = constant_time_select_w(lt, a, b); */
+
+/* crypto_word_t is the type that most constant-time functions use. Ideally we
+ * would like it to be |size_t|, but NaCl builds in 64-bit mode with 32-bit
+ * pointers, which means that |size_t| can be 32 bits when |BN_ULONG| is 64
+ * bits. Since we want to be able to do constant-time operations on a
+ * |BN_ULONG|, |crypto_word_t| is defined as an unsigned value with the native
+ * word length. */
+#if defined(OPENSSL_64_BIT)
+typedef uint64_t crypto_word_t;
+#elif defined(OPENSSL_32_BIT)
+typedef uint32_t crypto_word_t;
+#else
+#error "Must define either OPENSSL_32_BIT or OPENSSL_64_BIT"
+#endif
+
+#define CONSTTIME_TRUE_W ~((crypto_word_t)0)
+#define CONSTTIME_FALSE_W ((crypto_word_t)0)
+#define CONSTTIME_TRUE_8 ((uint8_t)0xff)
 
-#define CONSTTIME_TRUE_S ~((size_t)0)
-#define CONSTTIME_FALSE_S ((size_t)0)
+#define CONSTTIME_TRUE_W ~((crypto_word_t)0)
+#define CONSTTIME_FALSE_W ((crypto_word_t)0)
 #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
+/* constant_time_msb_w returns the given value with the MSB copied to all the
  * other bits. */
-static inline size_t constant_time_msb_s(size_t a) {
+static inline crypto_word_t constant_time_msb_w(crypto_word_t a) {
   return 0u - (a >> (sizeof(a) * 8 - 1));
 }
 
-/* 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) {
+/* constant_time_lt_w returns 0xff..f if a < b and 0 otherwise. */
+static inline crypto_word_t constant_time_lt_w(crypto_word_t a,
+                                               crypto_word_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.
@@ -230,28 +249,29 @@ static inline size_t constant_time_lt_s(size_t a, size_t b) {
    * (check-sat)
    * (get-model)
    */
-  return constant_time_msb_s(a^((a^b)|((a-b)^a)));
+  return constant_time_msb_w(a^((a^b)|((a-b)^a)));
 }
 
-/* constant_time_lt_8 acts like |constant_time_lt_s| but returns an 8-bit
+/* constant_time_lt_8 acts like |constant_time_lt_w| 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));
+static inline uint8_t constant_time_lt_8(crypto_word_t a, crypto_word_t b) {
+  return (uint8_t)(constant_time_lt_w(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_w returns 0xff..f if a >= b and 0 otherwise. */
+static inline crypto_word_t constant_time_ge_w(crypto_word_t a,
+                                               crypto_word_t b) {
+  return ~constant_time_lt_w(a, b);
 }
 
-/* constant_time_ge_8 acts like |constant_time_ge_s| but returns an 8-bit
+/* constant_time_ge_8 acts like |constant_time_ge_w| 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));
+static inline uint8_t constant_time_ge_8(crypto_word_t a, crypto_word_t b) {
+  return (uint8_t)(constant_time_ge_w(a, b));
 }
 
 /* constant_time_is_zero returns 0xff..f if a == 0 and 0 otherwise. */
-static inline size_t constant_time_is_zero_s(size_t a) {
+static inline crypto_word_t constant_time_is_zero_w(crypto_word_t a) {
   /* Here is an SMT-LIB verification of this formula:
    *
    * (define-fun is_zero ((a (_ BitVec 32))) (_ BitVec 32)
@@ -264,42 +284,45 @@ static inline size_t constant_time_is_zero_s(size_t a) {
    * (check-sat)
    * (get-model)
    */
-  return constant_time_msb_s(~a & (a - 1));
+  return constant_time_msb_w(~a & (a - 1));
 }
 
-/* constant_time_is_zero_8 acts like |constant_time_is_zero_s| but returns an
+/* constant_time_is_zero_8 acts like |constant_time_is_zero_w| 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));
+static inline uint8_t constant_time_is_zero_8(crypto_word_t a) {
+  return (uint8_t)(constant_time_is_zero_w(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_w returns 0xff..f if a == b and 0 otherwise. */
+static inline crypto_word_t constant_time_eq_w(crypto_word_t a,
+                                               crypto_word_t b) {
+  return constant_time_is_zero_w(a ^ b);
 }
 
-/* constant_time_eq_8 acts like |constant_time_eq_s| but returns an 8-bit
+/* constant_time_eq_8 acts like |constant_time_eq_w| but returns an 8-bit
  * mask. */
-static inline uint8_t constant_time_eq_8(size_t a, size_t b) {
-  return (uint8_t)(constant_time_eq_s(a, b));
+static inline uint8_t constant_time_eq_8(crypto_word_t a, crypto_word_t b) {
+  return (uint8_t)(constant_time_eq_w(a, b));
 }
 
-/* constant_time_eq_int acts like |constant_time_eq_s| but works on int
+/* constant_time_eq_int acts like |constant_time_eq_w| 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));
+static inline crypto_word_t constant_time_eq_int(int a, int b) {
+  return constant_time_eq_w((crypto_word_t)(a), (crypto_word_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((size_t)(a), (size_t)(b));
+  return constant_time_eq_8((crypto_word_t)(a), (crypto_word_t)(b));
 }
 
-/* constant_time_select_s returns (mask & a) | (~mask & b). When |mask| is all
+/* constant_time_select_w 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 size_t constant_time_select_s(size_t mask, size_t a, size_t b) {
+static inline crypto_word_t constant_time_select_w(crypto_word_t mask,
+                                                   crypto_word_t a,
+                                                   crypto_word_t b) {
   return (mask & a) | (~mask & b);
 }
 
@@ -307,13 +330,14 @@ static inline size_t constant_time_select_s(size_t mask, size_t a, size_t b) {
  * 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_s(mask, a, b));
+  return (uint8_t)(constant_time_select_w(mask, a, b));
 }
 
 /* constant_time_select_int acts like |constant_time_select| but operates on
  * ints. */
-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)));
+static inline int constant_time_select_int(crypto_word_t mask, int a, int b) {
+  return (int)(constant_time_select_w(mask, (crypto_word_t)(a),
+                                      (crypto_word_t)(b)));
 }
 
 

crypto/rsa/padding.c

@@ -204,26 +204,26 @@ int RSA_padding_check_PKCS1_type_2(uint8_t *out, size_t *out_len,
     return 0;
   }
 
-  size_t first_byte_is_zero = constant_time_eq_s(from[0], 0);
-  size_t second_byte_is_two = constant_time_eq_s(from[1], 2);
+  crypto_word_t first_byte_is_zero = constant_time_eq_w(from[0], 0);
+  crypto_word_t second_byte_is_two = constant_time_eq_w(from[1], 2);
 
-  size_t zero_index = 0, looking_for_index = CONSTTIME_TRUE_S;
+  crypto_word_t zero_index = 0, looking_for_index = CONSTTIME_TRUE_W;
   for (size_t i = 2; i < from_len; i++) {
-    size_t equals0 = constant_time_is_zero_s(from[i]);
+    crypto_word_t equals0 = constant_time_is_zero_w(from[i]);
     zero_index =
-        constant_time_select_s(looking_for_index & equals0, i, zero_index);
-    looking_for_index = constant_time_select_s(equals0, 0, looking_for_index);
+        constant_time_select_w(looking_for_index & equals0, i, zero_index);
+    looking_for_index = constant_time_select_w(equals0, 0, looking_for_index);
   }
 
   /* The input must begin with 00 02. */
-  size_t valid_index = first_byte_is_zero;
+  crypto_word_t valid_index = first_byte_is_zero;
   valid_index &= second_byte_is_two;
 
   /* We must have found the end of PS. */
   valid_index &= ~looking_for_index;
 
   /* PS must be at least 8 bytes long, and it starts two bytes into |from|. */
-  valid_index &= constant_time_ge_s(zero_index, 2 + 8);
+  valid_index &= constant_time_ge_w(zero_index, 2 + 8);
 
   /* Skip the zero byte. */
   zero_index++;
@@ -436,17 +436,18 @@ int RSA_padding_check_PKCS1_OAEP_mgf1(uint8_t *out, size_t *out_len,
     goto err;
   }
 
-  size_t bad = ~constant_time_is_zero_s(CRYPTO_memcmp(db, phash, mdlen));
-  bad |= ~constant_time_is_zero_s(from[0]);
+  crypto_word_t bad = ~constant_time_is_zero_w(CRYPTO_memcmp(db, phash, mdlen));
+  bad |= ~constant_time_is_zero_w(from[0]);
 
-  size_t looking_for_one_byte = CONSTTIME_TRUE_S, one_index = 0;
+  crypto_word_t looking_for_one_byte = CONSTTIME_TRUE_W;
+  size_t one_index = 0;
   for (size_t i = mdlen; i < dblen; i++) {
-    size_t equals1 = constant_time_eq_s(db[i], 1);
-    size_t equals0 = constant_time_eq_s(db[i], 0);
+    crypto_word_t equals1 = constant_time_eq_w(db[i], 1);
+    crypto_word_t equals0 = constant_time_eq_w(db[i], 0);
     one_index =
-        constant_time_select_s(looking_for_one_byte & equals1, i, one_index);
+        constant_time_select_w(looking_for_one_byte & equals1, i, one_index);
     looking_for_one_byte =
-        constant_time_select_s(equals1, 0, looking_for_one_byte);
+        constant_time_select_w(equals1, 0, looking_for_one_byte);
     bad |= looking_for_one_byte & ~equals0;
   }
 

crypto/rsa/rsa_impl.c

@@ -773,13 +773,13 @@ const BN_ULONG kBoringSSLRSASqrtTwo[] = {
 const size_t kBoringSSLRSASqrtTwoLen = OPENSSL_ARRAY_SIZE(kBoringSSLRSASqrtTwo);
 
 int rsa_less_than_words(const BN_ULONG *a, const BN_ULONG *b, size_t len) {
-  OPENSSL_COMPILE_ASSERT(sizeof(BN_ULONG) <= sizeof(size_t),
-                         size_t_constant_time_functions_too_small);
+  OPENSSL_COMPILE_ASSERT(sizeof(BN_ULONG) <= sizeof(crypto_word_t),
+                         crypto_word_t_too_small);
   int ret = 0;
   /* Process the words in little-endian order. */
   for (size_t i = 0; i < len; i++) {
-    size_t eq = constant_time_eq_s(a[i], b[i]);
-    size_t lt = constant_time_lt_s(a[i], b[i]);
+    crypto_word_t eq = constant_time_eq_w(a[i], b[i]);
+    crypto_word_t lt = constant_time_lt_w(a[i], b[i]);
     ret = constant_time_select_int(eq, ret, constant_time_select_int(lt, 1, 0));
   }
   return ret;