From 6c2af68fe8540863a59fa877db207678b74cc293 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Mon, 20 Jun 2022 15:12:48 -0400 Subject: [PATCH] Remove a few more unions. Bug: 301 Change-Id: Idb558cd2a925e9c762369ec7cead08f7d1cec2eb Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53093 Reviewed-by: Adam Langley --- crypto/fipsmodule/aes/aes_nohw.c | 19 +++++++++---------- crypto/fipsmodule/aes/key_wrap.c | 8 ++------ crypto/fipsmodule/cipher/e_aes.c | 15 +++++---------- crypto/fipsmodule/modes/cbc.c | 14 +++++--------- crypto/fipsmodule/rand/ctrdrbg.c | 16 ++++++++-------- crypto/fipsmodule/rand/internal.h | 5 +---- 6 files changed, 30 insertions(+), 47 deletions(-) diff --git a/crypto/fipsmodule/aes/aes_nohw.c b/crypto/fipsmodule/aes/aes_nohw.c index 12cabcb86b..5bb24bc259 100644 --- a/crypto/fipsmodule/aes/aes_nohw.c +++ b/crypto/fipsmodule/aes/aes_nohw.c @@ -18,6 +18,7 @@ #include #include "../../internal.h" +#include "internal.h" #if defined(OPENSSL_SSE2) #include @@ -1181,29 +1182,27 @@ void aes_nohw_ctr32_encrypt_blocks(const uint8_t *in, uint8_t *out, aes_nohw_expand_round_keys(&sched, key); // Make |AES_NOHW_BATCH_SIZE| copies of |ivec|. - alignas(AES_NOHW_WORD_SIZE) union { - uint32_t u32[AES_NOHW_BATCH_SIZE * 4]; - uint8_t u8[AES_NOHW_BATCH_SIZE * 16]; - } ivs, enc_ivs; + alignas(AES_NOHW_WORD_SIZE) uint8_t ivs[AES_NOHW_BATCH_SIZE * 16]; + alignas(AES_NOHW_WORD_SIZE) uint8_t enc_ivs[AES_NOHW_BATCH_SIZE * 16]; for (size_t i = 0; i < AES_NOHW_BATCH_SIZE; i++) { - memcpy(ivs.u8 + 16 * i, ivec, 16); + memcpy(ivs + 16 * i, ivec, 16); } - uint32_t ctr = CRYPTO_bswap4(ivs.u32[3]); + uint32_t ctr = CRYPTO_load_u32_be(ivs + 12); for (;;) { // Update counters. for (size_t i = 0; i < AES_NOHW_BATCH_SIZE; i++) { - ivs.u32[4 * i + 3] = CRYPTO_bswap4(ctr + i); + CRYPTO_store_u32_be(ivs + 16 * i + 12, ctr + i); } size_t todo = blocks >= AES_NOHW_BATCH_SIZE ? AES_NOHW_BATCH_SIZE : blocks; AES_NOHW_BATCH batch; - aes_nohw_to_batch(&batch, ivs.u8, todo); + aes_nohw_to_batch(&batch, ivs, todo); aes_nohw_encrypt_batch(&sched, key->rounds, &batch); - aes_nohw_from_batch(enc_ivs.u8, todo, &batch); + aes_nohw_from_batch(enc_ivs, todo, &batch); for (size_t i = 0; i < todo; i++) { - aes_nohw_xor_block(out + 16 * i, in + 16 * i, enc_ivs.u8 + 16 * i); + aes_nohw_xor_block(out + 16 * i, in + 16 * i, enc_ivs + 16 * i); } blocks -= todo; diff --git a/crypto/fipsmodule/aes/key_wrap.c b/crypto/fipsmodule/aes/key_wrap.c index 0ec5f26276..95b12d28e5 100644 --- a/crypto/fipsmodule/aes/key_wrap.c +++ b/crypto/fipsmodule/aes/key_wrap.c @@ -164,10 +164,8 @@ static const uint8_t kPaddingConstant[4] = {0xa6, 0x59, 0x59, 0xa6}; int AES_wrap_key_padded(const AES_KEY *key, uint8_t *out, size_t *out_len, size_t max_out, const uint8_t *in, size_t in_len) { // See https://tools.ietf.org/html/rfc5649#section-4.1 - const uint32_t in_len32_be = CRYPTO_bswap4(in_len); const uint64_t in_len64 = in_len; const size_t padded_len = (in_len + 7) & ~7; - *out_len = 0; if (in_len == 0 || in_len64 > 0xffffffffu || in_len + 7 < in_len || padded_len + 8 < padded_len || max_out < padded_len + 8) { @@ -176,7 +174,7 @@ int AES_wrap_key_padded(const AES_KEY *key, uint8_t *out, size_t *out_len, uint8_t block[AES_BLOCK_SIZE]; memcpy(block, kPaddingConstant, sizeof(kPaddingConstant)); - memcpy(block + 4, &in_len32_be, sizeof(in_len32_be)); + CRYPTO_store_u32_be(block + 4, (uint32_t)in_len); if (in_len <= 8) { memset(block + 8, 0, 8); @@ -226,9 +224,7 @@ int AES_unwrap_key_padded(const AES_KEY *key, uint8_t *out, size_t *out_len, crypto_word_t ok = constant_time_eq_int( CRYPTO_memcmp(iv, kPaddingConstant, sizeof(kPaddingConstant)), 0); - uint32_t claimed_len32; - memcpy(&claimed_len32, iv + 4, sizeof(claimed_len32)); - const size_t claimed_len = CRYPTO_bswap4(claimed_len32); + const size_t claimed_len = CRYPTO_load_u32_be(iv + 4); ok &= ~constant_time_is_zero_w(claimed_len); ok &= constant_time_eq_w((claimed_len - 1) >> 3, (in_len - 9) >> 3); diff --git a/crypto/fipsmodule/cipher/e_aes.c b/crypto/fipsmodule/cipher/e_aes.c index ad64a39c3f..e94d59a144 100644 --- a/crypto/fipsmodule/cipher/e_aes.c +++ b/crypto/fipsmodule/cipher/e_aes.c @@ -1297,12 +1297,9 @@ static int aead_aes_gcm_tls12_seal_scatter( } // The given nonces must be strictly monotonically increasing. - uint64_t given_counter; - OPENSSL_memcpy(&given_counter, nonce + nonce_len - sizeof(given_counter), - sizeof(given_counter)); - given_counter = CRYPTO_bswap8(given_counter); - if (given_counter == UINT64_MAX || - given_counter < gcm_ctx->min_next_nonce) { + uint64_t given_counter = + CRYPTO_load_u64_be(nonce + nonce_len - sizeof(uint64_t)); + if (given_counter == UINT64_MAX || given_counter < gcm_ctx->min_next_nonce) { OPENSSL_PUT_ERROR(CIPHER, CIPHER_R_INVALID_NONCE); return 0; } @@ -1397,10 +1394,8 @@ static int aead_aes_gcm_tls13_seal_scatter( // The given nonces must be strictly monotonically increasing. See // https://tools.ietf.org/html/rfc8446#section-5.3 for details of the TLS 1.3 // nonce construction. - uint64_t given_counter; - OPENSSL_memcpy(&given_counter, nonce + nonce_len - sizeof(given_counter), - sizeof(given_counter)); - given_counter = CRYPTO_bswap8(given_counter); + uint64_t given_counter = + CRYPTO_load_u64_be(nonce + nonce_len - sizeof(uint64_t)); if (gcm_ctx->first) { // In the first call the sequence number will be zero and therefore the diff --git a/crypto/fipsmodule/modes/cbc.c b/crypto/fipsmodule/modes/cbc.c index 192580ebd5..ac0cd426a1 100644 --- a/crypto/fipsmodule/modes/cbc.c +++ b/crypto/fipsmodule/modes/cbc.c @@ -116,11 +116,7 @@ void CRYPTO_cbc128_decrypt(const uint8_t *in, uint8_t *out, size_t len, assert(inptr >= outptr || inptr + len <= outptr); size_t n; - union { - crypto_word_t t[16 / sizeof(crypto_word_t)]; - uint8_t c[16]; - } tmp; - + alignas(16) uint8_t tmp[16]; if ((inptr >= 32 && outptr <= inptr - 32) || inptr < outptr) { // If |out| is at least two blocks behind |in| or completely disjoint, there // is no need to decrypt to a temporary block. @@ -144,10 +140,10 @@ void CRYPTO_cbc128_decrypt(const uint8_t *in, uint8_t *out, size_t len, "block cannot be evenly divided into words"); while (len >= 16) { - (*block)(in, tmp.c, key); + (*block)(in, tmp, key); for (n = 0; n < 16; n += sizeof(crypto_word_t)) { crypto_word_t c = CRYPTO_load_word_le(in + n); - CRYPTO_store_word_le(out + n, tmp.t[n / sizeof(crypto_word_t)] ^ + CRYPTO_store_word_le(out + n, CRYPTO_load_word_le(tmp + n) ^ CRYPTO_load_word_le(ivec + n)); CRYPTO_store_word_le(ivec + n, c); } @@ -159,10 +155,10 @@ void CRYPTO_cbc128_decrypt(const uint8_t *in, uint8_t *out, size_t len, while (len) { uint8_t c; - (*block)(in, tmp.c, key); + (*block)(in, tmp, key); for (n = 0; n < 16 && n < len; ++n) { c = in[n]; - out[n] = tmp.c[n] ^ ivec[n]; + out[n] = tmp[n] ^ ivec[n]; ivec[n] = c; } if (len <= 16) { diff --git a/crypto/fipsmodule/rand/ctrdrbg.c b/crypto/fipsmodule/rand/ctrdrbg.c index 83e7f5b614..3b21d76300 100644 --- a/crypto/fipsmodule/rand/ctrdrbg.c +++ b/crypto/fipsmodule/rand/ctrdrbg.c @@ -59,7 +59,7 @@ int CTR_DRBG_init(CTR_DRBG_STATE *drbg, } drbg->ctr = aes_ctr_set_key(&drbg->ks, NULL, &drbg->block, seed_material, 32); - OPENSSL_memcpy(drbg->counter.bytes, seed_material + 32, 16); + OPENSSL_memcpy(drbg->counter, seed_material + 32, 16); drbg->reseed_counter = 1; return 1; @@ -71,8 +71,8 @@ OPENSSL_STATIC_ASSERT(CTR_DRBG_ENTROPY_LEN % AES_BLOCK_SIZE == 0, // ctr_inc adds |n| to the last four bytes of |drbg->counter|, treated as a // big-endian number. static void ctr32_add(CTR_DRBG_STATE *drbg, uint32_t n) { - drbg->counter.words[3] = - CRYPTO_bswap4(CRYPTO_bswap4(drbg->counter.words[3]) + n); + uint32_t ctr = CRYPTO_load_u32_be(drbg->counter + 12); + CRYPTO_store_u32_be(drbg->counter + 12, ctr + n); } static int ctr_drbg_update(CTR_DRBG_STATE *drbg, const uint8_t *data, @@ -87,7 +87,7 @@ static int ctr_drbg_update(CTR_DRBG_STATE *drbg, const uint8_t *data, uint8_t temp[CTR_DRBG_ENTROPY_LEN]; for (size_t i = 0; i < CTR_DRBG_ENTROPY_LEN; i += AES_BLOCK_SIZE) { ctr32_add(drbg, 1); - drbg->block(drbg->counter.bytes, temp + i, &drbg->ks); + drbg->block(drbg->counter, temp + i, &drbg->ks); } for (size_t i = 0; i < data_len; i++) { @@ -95,7 +95,7 @@ static int ctr_drbg_update(CTR_DRBG_STATE *drbg, const uint8_t *data, } drbg->ctr = aes_ctr_set_key(&drbg->ks, NULL, &drbg->block, temp, 32); - OPENSSL_memcpy(drbg->counter.bytes, temp + 32, 16); + OPENSSL_memcpy(drbg->counter, temp + 32, 16); return 1; } @@ -167,12 +167,12 @@ int CTR_DRBG_generate(CTR_DRBG_STATE *drbg, uint8_t *out, size_t out_len, if (drbg->ctr) { OPENSSL_memset(out, 0, todo); ctr32_add(drbg, 1); - drbg->ctr(out, out, num_blocks, &drbg->ks, drbg->counter.bytes); + drbg->ctr(out, out, num_blocks, &drbg->ks, drbg->counter); ctr32_add(drbg, num_blocks - 1); } else { for (size_t i = 0; i < todo; i += AES_BLOCK_SIZE) { ctr32_add(drbg, 1); - drbg->block(drbg->counter.bytes, out + i, &drbg->ks); + drbg->block(drbg->counter, out + i, &drbg->ks); } } @@ -183,7 +183,7 @@ int CTR_DRBG_generate(CTR_DRBG_STATE *drbg, uint8_t *out, size_t out_len, if (out_len > 0) { uint8_t block[AES_BLOCK_SIZE]; ctr32_add(drbg, 1); - drbg->block(drbg->counter.bytes, block, &drbg->ks); + drbg->block(drbg->counter, block, &drbg->ks); OPENSSL_memcpy(out, block, out_len); } diff --git a/crypto/fipsmodule/rand/internal.h b/crypto/fipsmodule/rand/internal.h index eccf047f61..b0133a8c78 100644 --- a/crypto/fipsmodule/rand/internal.h +++ b/crypto/fipsmodule/rand/internal.h @@ -99,10 +99,7 @@ typedef struct { AES_KEY ks; block128_f block; ctr128_f ctr; - union { - uint8_t bytes[16]; - uint32_t words[4]; - } counter; + uint8_t counter[16]; uint64_t reseed_counter; } CTR_DRBG_STATE;