Various -Wshorten-64-to-32 fixes.

This is far from all of it, but finishes a good chunk of bcm.c.

Bug: 516
Change-Id: If764e5af1c6b62e8342554502ecc4d563e44bc50
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54207
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

crypto/fipsmodule/aes/aes_nohw.c

@@ -1192,7 +1192,7 @@ void aes_nohw_ctr32_encrypt_blocks(const uint8_t *in, uint8_t *out,
   for (;;) {
     // Update counters.
     for (size_t i = 0; i < AES_NOHW_BATCH_SIZE; i++) {
-      CRYPTO_store_u32_be(ivs + 16 * i + 12, ctr + i);
+      CRYPTO_store_u32_be(ivs + 16 * i + 12, ctr + (uint32_t)i);
     }
 
     size_t todo = blocks >= AES_NOHW_BATCH_SIZE ? AES_NOHW_BATCH_SIZE : blocks;

crypto/fipsmodule/bn/bn.c

@@ -67,6 +67,11 @@
 #include "../delocate.h"
 
 
+// BN_MAX_WORDS is the maximum number of words allowed in a |BIGNUM|. It is
+// sized so byte and bit counts of a |BIGNUM| always fit in |int|, with room to
+// spare.
+#define BN_MAX_WORDS (INT_MAX / (4 * BN_BITS2))
+
 BIGNUM *BN_new(void) {
   BIGNUM *bn = OPENSSL_malloc(sizeof(BIGNUM));
 
@@ -292,8 +297,9 @@ void bn_set_static_words(BIGNUM *bn, const BN_ULONG *words, size_t num) {
   }
   bn->d = (BN_ULONG *)words;
 
-  bn->width = num;
+  assert(num <= BN_MAX_WORDS);
-  bn->dmax = num;
+  bn->width = (int)num;
+  bn->dmax = (int)num;
   bn->neg = 0;
   bn->flags |= BN_FLG_STATIC_DATA;
 }
@@ -346,7 +352,7 @@ int bn_wexpand(BIGNUM *bn, size_t words) {
     return 1;
   }
 
-  if (words > (INT_MAX / (4 * BN_BITS2))) {
+  if (words > BN_MAX_WORDS) {
     OPENSSL_PUT_ERROR(BN, BN_R_BIGNUM_TOO_LONG);
     return 0;
   }
@@ -403,7 +409,7 @@ int bn_resize_words(BIGNUM *bn, size_t words) {
     }
     OPENSSL_memset(bn->d + bn->width, 0,
                    (words - bn->width) * sizeof(BN_ULONG));
-    bn->width = words;
+    bn->width = (int)words;
     return 1;
   }
 
@@ -412,7 +418,7 @@ int bn_resize_words(BIGNUM *bn, size_t words) {
     OPENSSL_PUT_ERROR(BN, BN_R_BIGNUM_TOO_LONG);
     return 0;
   }
-  bn->width = words;
+  bn->width = (int)words;
   return 1;
 }
 

crypto/fipsmodule/bn/bytes.c

@@ -138,7 +138,7 @@ BIGNUM *BN_le2bn(const uint8_t *in, size_t len, BIGNUM *ret) {
     BN_free(bn);
     return NULL;
   }
-  ret->width = num_words;
+  ret->width = (int)num_words;
 
   // Make sure the top bytes will be zeroed.
   ret->d[num_words - 1] = 0;

crypto/fipsmodule/bn/div.c

@@ -552,7 +552,7 @@ static BIGNUM *bn_scratch_space_from_ctx(size_t width, BN_CTX *ctx) {
     return NULL;
   }
   ret->neg = 0;
-  ret->width = width;
+  ret->width = (int)width;
   return ret;
 }
 

crypto/fipsmodule/bn/div_extra.c

@@ -70,7 +70,7 @@ uint16_t bn_mod_u16_consttime(const BIGNUM *bn, uint16_t d) {
   // This operation is not constant-time, but |p| and |d| are public values.
   // Note that |p| is at most 16, so the computation fits in |uint64_t|.
   assert(p <= 16);
-  uint32_t m = ((UINT64_C(1) << (32 + p)) + d - 1) / d;
+  uint32_t m = (uint32_t)(((UINT64_C(1) << (32 + p)) + d - 1) / d);
 
   uint16_t ret = 0;
   for (int i = bn->width - 1; i >= 0; i--) {

crypto/fipsmodule/bn/gcd_extra.c

@@ -240,8 +240,10 @@ int bn_mod_inverse_consttime(BIGNUM *r, int *out_no_inverse, const BIGNUM *a,
 
   // Each loop iteration halves at least one of |u| and |v|. Thus we need at
   // most the combined bit width of inputs for at least one value to be zero.
-  unsigned a_bits = a_width * BN_BITS2, n_bits = n_width * BN_BITS2;
+  // |a_bits| and |n_bits| cannot overflow because |bn_wexpand| ensures bit
-  unsigned num_iters = a_bits + n_bits;
+  // counts fit in even |int|.
+  size_t a_bits = a_width * BN_BITS2, n_bits = n_width * BN_BITS2;
+  size_t num_iters = a_bits + n_bits;
   if (num_iters < a_bits) {
     OPENSSL_PUT_ERROR(BN, BN_R_BIGNUM_TOO_LONG);
     goto err;
@@ -260,7 +262,7 @@ int bn_mod_inverse_consttime(BIGNUM *r, int *out_no_inverse, const BIGNUM *a,
   //
   // After each loop iteration, u and v only get smaller, and at least one of
   // them shrinks by at least a factor of two.
-  for (unsigned i = 0; i < num_iters; i++) {
+  for (size_t i = 0; i < num_iters; i++) {
     BN_ULONG both_odd = word_is_odd_mask(u->d[0]) & word_is_odd_mask(v->d[0]);
 
     // If both |u| and |v| are odd, subtract the smaller from the larger.

crypto/fipsmodule/bn/internal.h

@@ -439,7 +439,7 @@ int bn_jacobi(const BIGNUM *a, const BIGNUM *b, BN_CTX *ctx);
 
 // bn_is_bit_set_words returns one if bit |bit| is set in |a| and zero
 // otherwise.
-int bn_is_bit_set_words(const BN_ULONG *a, size_t num, unsigned bit);
+int bn_is_bit_set_words(const BN_ULONG *a, size_t num, size_t bit);
 
 // bn_one_to_montgomery sets |r| to one in Montgomery form. It returns one on
 // success and zero on error. This function treats the bit width of the modulus

crypto/fipsmodule/bn/random.c

@@ -336,7 +336,7 @@ int bn_rand_secret_range(BIGNUM *r, int *out_is_uniform, BN_ULONG min_inclusive,
   assert(bn_in_range_words(r->d, min_inclusive, max_exclusive->d, words));
 
   r->neg = 0;
-  r->width = words;
+  r->width = (int)words;
   return 1;
 }
 

crypto/fipsmodule/bn/shift.c

@@ -258,9 +258,9 @@ int BN_clear_bit(BIGNUM *a, int n) {
   return 1;
 }
 
-int bn_is_bit_set_words(const BN_ULONG *a, size_t num, unsigned bit) {
+int bn_is_bit_set_words(const BN_ULONG *a, size_t num, size_t bit) {
-  unsigned i = bit / BN_BITS2;
+  size_t i = bit / BN_BITS2;
-  unsigned j = bit % BN_BITS2;
+  size_t j = bit % BN_BITS2;
   if (i >= num) {
     return 0;
   }

crypto/fipsmodule/cipher/e_aes.c

@@ -47,6 +47,7 @@
  * ==================================================================== */
 
 #include <assert.h>
+#include <limits.h>
 #include <string.h>
 
 #include <openssl/aead.h>
@@ -289,8 +290,10 @@ static int aes_ofb_cipher(EVP_CIPHER_CTX *ctx, uint8_t *out, const uint8_t *in,
 ctr128_f aes_ctr_set_key(AES_KEY *aes_key, GCM128_KEY *gcm_key,
                          block128_f *out_block, const uint8_t *key,
                          size_t key_bytes) {
+  // This function assumes the key length was previously validated.
+  assert(key_bytes == 128 / 8 || key_bytes == 192 / 8 || key_bytes == 256 / 8);
   if (hwaes_capable()) {
-    aes_hw_set_encrypt_key(key, key_bytes * 8, aes_key);
+    aes_hw_set_encrypt_key(key, (int)key_bytes * 8, aes_key);
     if (gcm_key != NULL) {
       CRYPTO_gcm128_init_key(gcm_key, aes_key, aes_hw_encrypt, 1);
     }
@@ -301,7 +304,7 @@ ctr128_f aes_ctr_set_key(AES_KEY *aes_key, GCM128_KEY *gcm_key,
   }
 
   if (vpaes_capable()) {
-    vpaes_set_encrypt_key(key, key_bytes * 8, aes_key);
+    vpaes_set_encrypt_key(key, (int)key_bytes * 8, aes_key);
     if (out_block) {
       *out_block = vpaes_encrypt;
     }
@@ -318,7 +321,7 @@ ctr128_f aes_ctr_set_key(AES_KEY *aes_key, GCM128_KEY *gcm_key,
 #endif
   }
 
-  aes_nohw_set_encrypt_key(key, key_bytes * 8, aes_key);
+  aes_nohw_set_encrypt_key(key, (int)key_bytes * 8, aes_key);
   if (gcm_key != NULL) {
     CRYPTO_gcm128_init_key(gcm_key, aes_key, aes_nohw_encrypt, 0);
   }
@@ -552,6 +555,14 @@ static int aes_gcm_cipher(EVP_CIPHER_CTX *ctx, uint8_t *out, const uint8_t *in,
     return -1;
   }
 
+  if (len > INT_MAX) {
+    // This function signature can only express up to |INT_MAX| bytes encrypted.
+    //
+    // TODO(https://crbug.com/boringssl/494): Make the internal |EVP_CIPHER|
+    // calling convention |size_t|-clean.
+    return -1;
+  }
+
   if (in) {
     if (out == NULL) {
       if (!CRYPTO_gcm128_aad(&gctx->gcm, in, len)) {
@@ -580,7 +591,7 @@ static int aes_gcm_cipher(EVP_CIPHER_CTX *ctx, uint8_t *out, const uint8_t *in,
         }
       }
     }
-    return len;
+    return (int)len;
   } else {
     if (!ctx->encrypt) {
       if (gctx->taglen < 0 ||

crypto/fipsmodule/cmac/cmac.c

@@ -49,6 +49,7 @@
 #include <openssl/cmac.h>
 
 #include <assert.h>
+#include <limits.h>
 #include <string.h>
 
 #include <openssl/aes.h>
@@ -269,7 +270,10 @@ int CMAC_Update(CMAC_CTX *ctx, const uint8_t *in, size_t in_len) {
   }
 
   OPENSSL_memcpy(ctx->block, in, in_len);
-  ctx->block_used = in_len;
+  // |in_len| is bounded by |block_size|, which fits in |unsigned|.
+  static_assert(EVP_MAX_BLOCK_LENGTH < UINT_MAX,
+                "EVP_MAX_BLOCK_LENGTH is too large");
+  ctx->block_used = (unsigned)in_len;
   ret = 1;
 
 out:

crypto/fipsmodule/dh/dh.c

@@ -368,7 +368,8 @@ int DH_compute_key(unsigned char *out, const BIGNUM *peers_key, DH *dh) {
   int ret = -1;
   BIGNUM *shared_key = BN_CTX_get(ctx);
   if (shared_key && dh_compute_key(dh, shared_key, peers_key, ctx)) {
-    ret = BN_bn2bin(shared_key, out);
+    // A |BIGNUM|'s byte count fits in |int|.
+    ret = (int)BN_bn2bin(shared_key, out);
   }
 
   BN_CTX_end(ctx);

crypto/fipsmodule/rand/ctrdrbg.c

@@ -184,7 +184,7 @@ int CTR_DRBG_generate(CTR_DRBG_STATE *drbg, uint8_t *out, size_t out_len,
       OPENSSL_memset(out, 0, todo);
       ctr32_add(drbg, 1);
       drbg->ctr(out, out, num_blocks, &drbg->ks, drbg->counter);
-      ctr32_add(drbg, num_blocks - 1);
+      ctr32_add(drbg, (uint32_t)(num_blocks - 1));
     } else {
       for (size_t i = 0; i < todo; i += AES_BLOCK_SIZE) {
         ctr32_add(drbg, 1);

crypto/fipsmodule/rsa/rsa.c

@@ -300,7 +300,7 @@ int RSA_public_encrypt(size_t flen, const uint8_t *from, uint8_t *to, RSA *rsa,
     OPENSSL_PUT_ERROR(RSA, ERR_R_OVERFLOW);
     return -1;
   }
-  return out_len;
+  return (int)out_len;
 }
 
 static int rsa_sign_raw_no_self_test(RSA *rsa, size_t *out_len, uint8_t *out,
@@ -332,7 +332,7 @@ int RSA_private_encrypt(size_t flen, const uint8_t *from, uint8_t *to, RSA *rsa,
     OPENSSL_PUT_ERROR(RSA, ERR_R_OVERFLOW);
     return -1;
   }
-  return out_len;
+  return (int)out_len;
 }
 
 int RSA_decrypt(RSA *rsa, size_t *out_len, uint8_t *out, size_t max_out,
@@ -347,7 +347,6 @@ int RSA_decrypt(RSA *rsa, size_t *out_len, uint8_t *out, size_t max_out,
 int RSA_private_decrypt(size_t flen, const uint8_t *from, uint8_t *to, RSA *rsa,
                         int padding) {
   size_t out_len;
-
   if (!RSA_decrypt(rsa, &out_len, to, RSA_size(rsa), from, flen, padding)) {
     return -1;
   }
@@ -356,13 +355,12 @@ int RSA_private_decrypt(size_t flen, const uint8_t *from, uint8_t *to, RSA *rsa,
     OPENSSL_PUT_ERROR(RSA, ERR_R_OVERFLOW);
     return -1;
   }
-  return out_len;
+  return (int)out_len;
 }
 
 int RSA_public_decrypt(size_t flen, const uint8_t *from, uint8_t *to, RSA *rsa,
                        int padding) {
   size_t out_len;
-
   if (!RSA_verify_raw(rsa, &out_len, to, RSA_size(rsa), from, flen, padding)) {
     return -1;
   }
@@ -371,15 +369,16 @@ int RSA_public_decrypt(size_t flen, const uint8_t *from, uint8_t *to, RSA *rsa,
     OPENSSL_PUT_ERROR(RSA, ERR_R_OVERFLOW);
     return -1;
   }
-  return out_len;
+  return (int)out_len;
 }
 
 unsigned RSA_size(const RSA *rsa) {
-  if (rsa->meth->size) {
+  size_t ret = rsa->meth->size ? rsa->meth->size(rsa) : rsa_default_size(rsa);
-    return rsa->meth->size(rsa);
+  // RSA modulus sizes are bounded by |BIGNUM|, which must fit in |unsigned|.
-  }
+  //
-
+  // TODO(https://crbug.com/boringssl/516): Should we make this return |size_t|?
-  return rsa_default_size(rsa);
+  assert(ret < UINT_MAX);
+  return (unsigned)ret;
 }
 
 int RSA_is_opaque(const RSA *rsa) {
@@ -474,8 +473,6 @@ static const struct pkcs1_sig_prefix kPKCS1SigPrefixes[] = {
 int RSA_add_pkcs1_prefix(uint8_t **out_msg, size_t *out_msg_len,
                          int *is_alloced, int hash_nid, const uint8_t *digest,
                          size_t digest_len) {
-  unsigned i;
-
   if (hash_nid == NID_md5_sha1) {
     // Special case: SSL signature, just check the length.
     if (digest_len != SSL_SIG_LENGTH) {
@@ -489,7 +486,7 @@ int RSA_add_pkcs1_prefix(uint8_t **out_msg, size_t *out_msg_len,
     return 1;
   }
 
-  for (i = 0; kPKCS1SigPrefixes[i].nid != NID_undef; i++) {
+  for (size_t i = 0; kPKCS1SigPrefixes[i].nid != NID_undef; i++) {
     const struct pkcs1_sig_prefix *sig_prefix = &kPKCS1SigPrefixes[i];
     if (sig_prefix->nid != hash_nid) {
       continue;
@@ -501,17 +498,14 @@ int RSA_add_pkcs1_prefix(uint8_t **out_msg, size_t *out_msg_len,
     }
 
     const uint8_t* prefix = sig_prefix->bytes;
-    unsigned prefix_len = sig_prefix->len;
+    size_t prefix_len = sig_prefix->len;
-    unsigned signed_msg_len;
+    size_t signed_msg_len = prefix_len + digest_len;
-    uint8_t *signed_msg;
-
-    signed_msg_len = prefix_len + digest_len;
     if (signed_msg_len < prefix_len) {
       OPENSSL_PUT_ERROR(RSA, RSA_R_TOO_LONG);
       return 0;
     }
 
-    signed_msg = OPENSSL_malloc(signed_msg_len);
+    uint8_t *signed_msg = OPENSSL_malloc(signed_msg_len);
     if (!signed_msg) {
       OPENSSL_PUT_ERROR(RSA, ERR_R_MALLOC_FAILURE);
       return 0;
@@ -554,7 +548,12 @@ int rsa_sign_no_self_test(int hash_nid, const uint8_t *digest,
     goto err;
   }
 
-  *out_len = size_t_out_len;
+  if (size_t_out_len > UINT_MAX) {
+    OPENSSL_PUT_ERROR(RSA, ERR_R_OVERFLOW);
+    goto err;
+  }
+
+  *out_len = (unsigned)size_t_out_len;
   ret = 1;
 
 err:

crypto/fipsmodule/tls/kdf.c

@@ -91,7 +91,7 @@ static int tls1_P_hash(uint8_t *out, size_t out_len,
   }
 
   for (;;) {
-    unsigned len;
+    unsigned len_u;
     uint8_t hmac[EVP_MAX_MD_SIZE];
     if (!HMAC_CTX_copy_ex(&ctx, &ctx_init) ||
         !HMAC_Update(&ctx, A1, A1_len) ||
@@ -100,16 +100,17 @@ static int tls1_P_hash(uint8_t *out, size_t out_len,
         !HMAC_Update(&ctx, (const uint8_t *) label, label_len) ||
         !HMAC_Update(&ctx, seed1, seed1_len) ||
         !HMAC_Update(&ctx, seed2, seed2_len) ||
-        !HMAC_Final(&ctx, hmac, &len)) {
+        !HMAC_Final(&ctx, hmac, &len_u)) {
       goto err;
     }
+    size_t len = len_u;
     assert(len == chunk);
 
     // XOR the result into |out|.
     if (len > out_len) {
       len = out_len;
     }
-    for (unsigned i = 0; i < len; i++) {
+    for (size_t i = 0; i < len; i++) {
       out[i] ^= hmac[i];
     }
     out += len;

crypto/test/file_test.cc

@@ -20,6 +20,7 @@
 #include <assert.h>
 #include <ctype.h>
 #include <errno.h>
+#include <limits.h>
 #include <stdarg.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -377,7 +378,8 @@ class FileLineReader : public FileTest::LineReader {
       return FileTest::kReadError;
     }
 
-    if (fgets(out, len, file_) == nullptr) {
+    len = std::min(len, size_t{INT_MAX});
+    if (fgets(out, static_cast<int>(len), file_) == nullptr) {
       return feof(file_) ? FileTest::kReadEOF : FileTest::kReadError;
     }
 

crypto/test/wycheproof_util.cc

@@ -14,6 +14,7 @@
 
 #include "./wycheproof_util.h"
 
+#include <limits.h>
 #include <stdlib.h>
 
 #include <algorithm>
@@ -138,7 +139,8 @@ bssl::UniquePtr<BIGNUM> GetWycheproofBIGNUM(FileTest *t, const char *key,
     return nullptr;
   }
   BIGNUM *bn = nullptr;
-  if (BN_hex2bn(&bn, value.c_str()) != static_cast<int>(value.size())) {
+  if (value.size() > INT_MAX ||
+      BN_hex2bn(&bn, value.c_str()) != static_cast<int>(value.size())) {
     BN_free(bn);
     t->PrintLine("Could not decode value '%s'", value.c_str());
     return nullptr;
@@ -151,8 +153,9 @@ bssl::UniquePtr<BIGNUM> GetWycheproofBIGNUM(FileTest *t, const char *key,
     // https://github.com/google/wycheproof/blob/0329f5b751ef102bd6b7b7181b6e049522a887f5/java/com/google/security/wycheproof/JsonUtil.java#L62.
     if ('0' > value[0] || value[0] > '7') {
       bssl::UniquePtr<BIGNUM> tmp(BN_new());
-      if (!tmp ||
+      if (!tmp ||  //
-          !BN_set_bit(tmp.get(), value.size() * 4) ||
+          value.size() > INT_MAX / 4 ||
+          !BN_set_bit(tmp.get(), static_cast<int>(value.size() * 4)) ||
           !BN_sub(ret.get(), ret.get(), tmp.get())) {
         return nullptr;
       }

include/openssl/bn.h

@@ -205,6 +205,10 @@ OPENSSL_EXPORT unsigned BN_num_bits(const BIGNUM *bn);
 
 // BN_num_bytes returns the minimum number of bytes needed to represent the
 // absolute value of |bn|.
+//
+// While |size_t| is the preferred type for byte counts, callers can assume that
+// |BIGNUM|s are bounded such that this value, and its corresponding bit count,
+// will always fit in |int|.
 OPENSSL_EXPORT unsigned BN_num_bytes(const BIGNUM *bn);
 
 // BN_zero sets |bn| to zero.