Skip to content

Commit

Permalink
Avoid unions in CCM
Browse files Browse the repository at this point in the history 
I believe this one was well-defined in C, because we never take the
address of the uint64_t arm, but this is fragile.

Bug: 574
Change-Id: I439801a3e0564b731f119c520096311f731523a5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65607
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
(cherry picked from commit f42be90d665b6a376177648ccbb76fbbd6497c13)
  • Loading branch information
@davidben @nebeid
davidben authored and nebeid committed Jul 9, 2024
1 parent ffe55ea commit 53249fd
Showing 1 changed file with 42 additions and 49 deletions.
91 changes: 42 additions & 49 deletions crypto/fipsmodule/cipher/e_aesccm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
#include <openssl/mem.h>

#include "../delocate.h"
#include "../modes/internal.h"
#include "../service_indicator/internal.h"
#include "internal.h"

Expand All @@ -69,10 +70,8 @@ typedef struct ccm128_context {
} CCM128_CTX;

typedef struct ccm128_state {
union {
uint64_t u[2];
uint8_t c[16];
} nonce, cmac;
alignas(16) uint8_t nonce[16];
alignas(16) uint8_t cmac[16];
} CCM128_STATE;

typedef struct cipher_aes_ccm_ctx {
Expand Down Expand Up @@ -149,58 +148,58 @@ static int ccm128_init_state(const struct ccm128_context *ctx,

// Assemble the first block for computing the MAC.
OPENSSL_memset(state, 0, sizeof(*state));
state->nonce.c[0] = (uint8_t)((L - 1) | ((M - 2) / 2) << 3);
state->nonce[0] = (uint8_t)((L - 1) | ((M - 2) / 2) << 3);
if (aad_len != 0) {
state->nonce.c[0] |= 0x40; // Set AAD Flag
state->nonce[0] |= 0x40; // Set AAD Flag
}
OPENSSL_memcpy(&state->nonce.c[1], nonce, nonce_len);
OPENSSL_memcpy(&state->nonce[1], nonce, nonce_len);
// Explicitly cast plaintext_len up to 64-bits so that we don't shift out of
// bounds on 32-bit machines when encoding the message length.
uint64_t plaintext_len_64 = plaintext_len;
for (uint32_t i = 0; i < L; i++) {
state->nonce.c[15 - i] = (uint8_t)(plaintext_len_64 >> (8 * i));
state->nonce[15 - i] = (uint8_t)(plaintext_len_64 >> (8 * i));
}

(*block)(state->nonce.c, state->cmac.c, key);
(*block)(state->nonce, state->cmac, key);
size_t blocks = 1;

if (aad_len != 0) {
unsigned i;
// Cast to u64 to avoid the compiler complaining about invalid shifts.
uint64_t aad_len_u64 = aad_len;
if (aad_len_u64 < 0x10000 - 0x100) {
state->cmac.c[0] ^= (uint8_t)(aad_len_u64 >> 8);
state->cmac.c[1] ^= (uint8_t)aad_len_u64;
state->cmac[0] ^= (uint8_t)(aad_len_u64 >> 8);
state->cmac[1] ^= (uint8_t)aad_len_u64;
i = 2;
} else if (aad_len_u64 <= 0xffffffff) {
state->cmac.c[0] ^= 0xff;
state->cmac.c[1] ^= 0xfe;
state->cmac.c[2] ^= (uint8_t)(aad_len_u64 >> 24);
state->cmac.c[3] ^= (uint8_t)(aad_len_u64 >> 16);
state->cmac.c[4] ^= (uint8_t)(aad_len_u64 >> 8);
state->cmac.c[5] ^= (uint8_t)aad_len_u64;
state->cmac[0] ^= 0xff;
state->cmac[1] ^= 0xfe;
state->cmac[2] ^= (uint8_t)(aad_len_u64 >> 24);
state->cmac[3] ^= (uint8_t)(aad_len_u64 >> 16);
state->cmac[4] ^= (uint8_t)(aad_len_u64 >> 8);
state->cmac[5] ^= (uint8_t)aad_len_u64;
i = 6;
} else {
state->cmac.c[0] ^= 0xff;
state->cmac.c[1] ^= 0xff;
state->cmac.c[2] ^= (uint8_t)(aad_len_u64 >> 56);
state->cmac.c[3] ^= (uint8_t)(aad_len_u64 >> 48);
state->cmac.c[4] ^= (uint8_t)(aad_len_u64 >> 40);
state->cmac.c[5] ^= (uint8_t)(aad_len_u64 >> 32);
state->cmac.c[6] ^= (uint8_t)(aad_len_u64 >> 24);
state->cmac.c[7] ^= (uint8_t)(aad_len_u64 >> 16);
state->cmac.c[8] ^= (uint8_t)(aad_len_u64 >> 8);
state->cmac.c[9] ^= (uint8_t)aad_len_u64;
state->cmac[0] ^= 0xff;
state->cmac[1] ^= 0xff;
state->cmac[2] ^= (uint8_t)(aad_len_u64 >> 56);
state->cmac[3] ^= (uint8_t)(aad_len_u64 >> 48);
state->cmac[4] ^= (uint8_t)(aad_len_u64 >> 40);
state->cmac[5] ^= (uint8_t)(aad_len_u64 >> 32);
state->cmac[6] ^= (uint8_t)(aad_len_u64 >> 24);
state->cmac[7] ^= (uint8_t)(aad_len_u64 >> 16);
state->cmac[8] ^= (uint8_t)(aad_len_u64 >> 8);
state->cmac[9] ^= (uint8_t)aad_len_u64;
i = 10;
}

do {
for (; i < 16 && aad_len != 0; i++) {
state->cmac.c[i] ^= *aad;
state->cmac[i] ^= *aad;
aad++;
aad_len--;
}
(*block)(state->cmac.c, state->cmac.c, key);
(*block)(state->cmac, state->cmac, key);
blocks++;
i = 0;
} while (aad_len != 0);
Expand All @@ -219,7 +218,7 @@ static int ccm128_init_state(const struct ccm128_context *ctx,
// Assemble the first block for encrypting and decrypting. The bottom |L|
// bytes are replaced with a counter and all bit the encoding of |L| is
// cleared in the first byte.
state->nonce.c[0] &= 7;
state->nonce[0] &= 7;
return 1;
}

Expand All @@ -228,17 +227,17 @@ static int ccm128_encrypt(const struct ccm128_context *ctx,
uint8_t *out, const uint8_t *in, size_t len) {
// The counter for encryption begins at one.
for (unsigned i = 0; i < ctx->L; i++) {
state->nonce.c[15 - i] = 0;
state->nonce[15 - i] = 0;
}
state->nonce.c[15] = 1;
state->nonce[15] = 1;

uint8_t partial_buf[16];
unsigned num = 0;
if (ctx->ctr != NULL) {
CRYPTO_ctr128_encrypt_ctr32(in, out, len, key, state->nonce.c, partial_buf,
CRYPTO_ctr128_encrypt_ctr32(in, out, len, key, state->nonce, partial_buf,
&num, ctx->ctr);
} else {
CRYPTO_ctr128_encrypt(in, out, len, key, state->nonce.c, partial_buf, &num,
CRYPTO_ctr128_encrypt(in, out, len, key, state->nonce, partial_buf, &num,
ctx->block);
}
return 1;
Expand All @@ -254,34 +253,28 @@ static int ccm128_compute_mac(const struct ccm128_context *ctx,
}

// Incorporate |in| into the MAC.
union {
uint64_t u[2];
uint8_t c[16];
} tmp;
while (len >= 16) {
OPENSSL_memcpy(tmp.c, in, 16);
state->cmac.u[0] ^= tmp.u[0];
state->cmac.u[1] ^= tmp.u[1];
(*block)(state->cmac.c, state->cmac.c, key);
CRYPTO_xor16(state->cmac, state->cmac, in);
(*block)(state->cmac, state->cmac, key);
in += 16;
len -= 16;
}
if (len > 0) {
for (size_t i = 0; i < len; i++) {
state->cmac.c[i] ^= in[i];
state->cmac[i] ^= in[i];
}
(*block)(state->cmac.c, state->cmac.c, key);
(*block)(state->cmac, state->cmac, key);
}

// Encrypt the MAC with counter zero.
for (unsigned i = 0; i < ctx->L; i++) {
state->nonce.c[15 - i] = 0;
state->nonce[15 - i] = 0;
}
(*block)(state->nonce.c, tmp.c, key);
state->cmac.u[0] ^= tmp.u[0];
state->cmac.u[1] ^= tmp.u[1];
alignas(16) uint8_t tmp[16];
(*block)(state->nonce, tmp, key);
CRYPTO_xor16(state->cmac, state->cmac, tmp);

OPENSSL_memcpy(out_tag, state->cmac.c, tag_len);
OPENSSL_memcpy(out_tag, state->cmac, tag_len);
return 1;
}

Expand Down

0 comments on commit 53249fd

Please sign in to comment.