Skip to content

Commit

Permalink
Fix undefined block128_f, etc., casts.
Browse files Browse the repository at this point in the history 
This one is a little thorny. All the various block cipher modes
functions and callbacks take a void *key. This allows them to be used
with multiple kinds of block ciphers.

However, the implementations of those callbacks are the normal typed
functions, like AES_encrypt. Those take AES_KEY *key. While, at the ABI
level, this is perfectly fine, C considers this undefined behavior.

If we wish to preserve this genericness, we could either instantiate
multiple versions of these mode functions or create wrappers of
AES_encrypt, etc., that take void *key.

The former means more code and is tedious without C++ templates (maybe
someday...). The latter would not be difficult for a compiler to
optimize out. C mistakenly allowed comparing function pointers for
equality, which means a compiler cannot replace pointers to wrapper
functions with the real thing. (That said, the performance-sensitive
bits already act in chunks, e.g. ctr128_f, so the function call overhead
shouldn't matter.)

But our only 128-bit block cipher is AES anyway, so I just switched
things to use AES_KEY throughout. AES is doing fine, and hopefully we
would have the sense not to pair a hypothetical future block cipher with
so many modes!

Change-Id: Ied3e843f0e3042a439f09e655b29847ade9d4c7d
Reviewed-on: https://boringssl-review.googlesource.com/32107
Reviewed-by: Adam Langley <[email protected]>
  • Loading branch information
@davidben @agl
davidben authored and agl committed Oct 1, 2018
1 parent 419144a commit 73535ab
Show file tree
Hide file tree
Showing 11 changed files with 123 additions and 120 deletions.
13 changes: 5 additions & 8 deletions crypto/fipsmodule/aes/mode_wrappers.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@
void AES_ctr128_encrypt(const uint8_t *in, uint8_t *out, size_t len,
const AES_KEY *key, uint8_t ivec[AES_BLOCK_SIZE],
uint8_t ecount_buf[AES_BLOCK_SIZE], unsigned int *num) {
CRYPTO_ctr128_encrypt(in, out, len, key, ivec, ecount_buf, num,
(block128_f)AES_encrypt);
CRYPTO_ctr128_encrypt(in, out, len, key, ivec, ecount_buf, num, AES_encrypt);
}

void AES_ecb_encrypt(const uint8_t *in, uint8_t *out, const AES_KEY *key,
Expand Down Expand Up @@ -90,26 +89,24 @@ void AES_cbc_encrypt(const uint8_t *in, uint8_t *out, size_t len,
aes_nohw_cbc_encrypt(in, out, len, key, ivec, enc);
#else
if (enc) {
CRYPTO_cbc128_encrypt(in, out, len, key, ivec, (block128_f)AES_encrypt);
CRYPTO_cbc128_encrypt(in, out, len, key, ivec, AES_encrypt);
} else {
CRYPTO_cbc128_decrypt(in, out, len, key, ivec, (block128_f)AES_decrypt);
CRYPTO_cbc128_decrypt(in, out, len, key, ivec, AES_decrypt);
}
#endif
}

void AES_ofb128_encrypt(const uint8_t *in, uint8_t *out, size_t length,
const AES_KEY *key, uint8_t *ivec, int *num) {
unsigned num_u = (unsigned)(*num);
CRYPTO_ofb128_encrypt(in, out, length, key, ivec, &num_u,
(block128_f)AES_encrypt);
CRYPTO_ofb128_encrypt(in, out, length, key, ivec, &num_u, AES_encrypt);
*num = (int)num_u;
}

void AES_cfb128_encrypt(const uint8_t *in, uint8_t *out, size_t length,
const AES_KEY *key, uint8_t *ivec, int *num,
int enc) {
unsigned num_u = (unsigned)(*num);
CRYPTO_cfb128_encrypt(in, out, length, key, ivec, &num_u, enc,
(block128_f)AES_encrypt);
CRYPTO_cfb128_encrypt(in, out, length, key, ivec, &num_u, enc, AES_encrypt);
*num = (int)num_u;
}
75 changes: 36 additions & 39 deletions crypto/fipsmodule/cipher/e_aes.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,49 +198,45 @@ static int aes_init_key(EVP_CIPHER_CTX *ctx, const uint8_t *key,
if ((mode == EVP_CIPH_ECB_MODE || mode == EVP_CIPH_CBC_MODE) && !enc) {
if (hwaes_capable()) {
ret = aes_hw_set_decrypt_key(key, ctx->key_len * 8, &dat->ks.ks);
dat->block = (block128_f)aes_hw_decrypt;
dat->block = aes_hw_decrypt;
dat->stream.cbc = NULL;
if (mode == EVP_CIPH_CBC_MODE) {
dat->stream.cbc = (cbc128_f)aes_hw_cbc_encrypt;
dat->stream.cbc = aes_hw_cbc_encrypt;
}
} else if (bsaes_capable() && mode == EVP_CIPH_CBC_MODE) {
ret = AES_set_decrypt_key(key, ctx->key_len * 8, &dat->ks.ks);
dat->block = (block128_f)AES_decrypt;
dat->stream.cbc = (cbc128_f)bsaes_cbc_encrypt;
dat->block = AES_decrypt;
dat->stream.cbc = bsaes_cbc_encrypt;
} else if (vpaes_capable()) {
ret = vpaes_set_decrypt_key(key, ctx->key_len * 8, &dat->ks.ks);
dat->block = (block128_f)vpaes_decrypt;
dat->stream.cbc =
mode == EVP_CIPH_CBC_MODE ? (cbc128_f)vpaes_cbc_encrypt : NULL;
dat->block = vpaes_decrypt;
dat->stream.cbc = mode == EVP_CIPH_CBC_MODE ? vpaes_cbc_encrypt : NULL;
} else {
ret = AES_set_decrypt_key(key, ctx->key_len * 8, &dat->ks.ks);
dat->block = (block128_f)AES_decrypt;
dat->stream.cbc =
mode == EVP_CIPH_CBC_MODE ? (cbc128_f)AES_cbc_encrypt : NULL;
dat->block = AES_decrypt;
dat->stream.cbc = mode == EVP_CIPH_CBC_MODE ? AES_cbc_encrypt : NULL;
}
} else if (hwaes_capable()) {
ret = aes_hw_set_encrypt_key(key, ctx->key_len * 8, &dat->ks.ks);
dat->block = (block128_f)aes_hw_encrypt;
dat->block = aes_hw_encrypt;
dat->stream.cbc = NULL;
if (mode == EVP_CIPH_CBC_MODE) {
dat->stream.cbc = (cbc128_f)aes_hw_cbc_encrypt;
dat->stream.cbc = aes_hw_cbc_encrypt;
} else if (mode == EVP_CIPH_CTR_MODE) {
dat->stream.ctr = (ctr128_f)aes_hw_ctr32_encrypt_blocks;
dat->stream.ctr = aes_hw_ctr32_encrypt_blocks;
}
} else if (bsaes_capable() && mode == EVP_CIPH_CTR_MODE) {
ret = AES_set_encrypt_key(key, ctx->key_len * 8, &dat->ks.ks);
dat->block = (block128_f)AES_encrypt;
dat->stream.ctr = (ctr128_f)bsaes_ctr32_encrypt_blocks;
dat->block = AES_encrypt;
dat->stream.ctr = bsaes_ctr32_encrypt_blocks;
} else if (vpaes_capable()) {
ret = vpaes_set_encrypt_key(key, ctx->key_len * 8, &dat->ks.ks);
dat->block = (block128_f)vpaes_encrypt;
dat->stream.cbc =
mode == EVP_CIPH_CBC_MODE ? (cbc128_f)vpaes_cbc_encrypt : NULL;
dat->block = vpaes_encrypt;
dat->stream.cbc = mode == EVP_CIPH_CBC_MODE ? vpaes_cbc_encrypt : NULL;
} else {
ret = AES_set_encrypt_key(key, ctx->key_len * 8, &dat->ks.ks);
dat->block = (block128_f)AES_encrypt;
dat->stream.cbc =
mode == EVP_CIPH_CBC_MODE ? (cbc128_f)AES_cbc_encrypt : NULL;
dat->block = AES_encrypt;
dat->stream.cbc = mode == EVP_CIPH_CBC_MODE ? AES_cbc_encrypt : NULL;
}

if (ret < 0) {
Expand All @@ -256,11 +252,11 @@ static int aes_cbc_cipher(EVP_CIPHER_CTX *ctx, uint8_t *out, const uint8_t *in,
EVP_AES_KEY *dat = (EVP_AES_KEY *)ctx->cipher_data;

if (dat->stream.cbc) {
(*dat->stream.cbc)(in, out, len, &dat->ks, ctx->iv, ctx->encrypt);
(*dat->stream.cbc)(in, out, len, &dat->ks.ks, ctx->iv, ctx->encrypt);
} else if (ctx->encrypt) {
CRYPTO_cbc128_encrypt(in, out, len, &dat->ks, ctx->iv, dat->block);
CRYPTO_cbc128_encrypt(in, out, len, &dat->ks.ks, ctx->iv, dat->block);
} else {
CRYPTO_cbc128_decrypt(in, out, len, &dat->ks, ctx->iv, dat->block);
CRYPTO_cbc128_decrypt(in, out, len, &dat->ks.ks, ctx->iv, dat->block);
}

return 1;
Expand All @@ -277,7 +273,7 @@ static int aes_ecb_cipher(EVP_CIPHER_CTX *ctx, uint8_t *out, const uint8_t *in,

len -= bl;
for (size_t i = 0; i <= len; i += bl) {
(*dat->block)(in + i, out + i, &dat->ks);
(*dat->block)(in + i, out + i, &dat->ks.ks);
}

return 1;
Expand All @@ -288,11 +284,11 @@ static int aes_ctr_cipher(EVP_CIPHER_CTX *ctx, uint8_t *out, const uint8_t *in,
EVP_AES_KEY *dat = (EVP_AES_KEY *)ctx->cipher_data;

if (dat->stream.ctr) {
CRYPTO_ctr128_encrypt_ctr32(in, out, len, &dat->ks, ctx->iv, ctx->buf,
CRYPTO_ctr128_encrypt_ctr32(in, out, len, &dat->ks.ks, ctx->iv, ctx->buf,
&ctx->num, dat->stream.ctr);
} else {
CRYPTO_ctr128_encrypt(in, out, len, &dat->ks, ctx->iv, ctx->buf, &ctx->num,
dat->block);
CRYPTO_ctr128_encrypt(in, out, len, &dat->ks.ks, ctx->iv, ctx->buf,
&ctx->num, dat->block);
}
return 1;
}
Expand All @@ -301,7 +297,8 @@ static int aes_ofb_cipher(EVP_CIPHER_CTX *ctx, uint8_t *out, const uint8_t *in,
size_t len) {
EVP_AES_KEY *dat = (EVP_AES_KEY *)ctx->cipher_data;

CRYPTO_ofb128_encrypt(in, out, len, &dat->ks, ctx->iv, &ctx->num, dat->block);
CRYPTO_ofb128_encrypt(in, out, len, &dat->ks.ks, ctx->iv, &ctx->num,
dat->block);
return 1;
}

Expand All @@ -311,42 +308,42 @@ ctr128_f aes_ctr_set_key(AES_KEY *aes_key, GCM128_KEY *gcm_key,
if (hwaes_capable()) {
aes_hw_set_encrypt_key(key, key_bytes * 8, aes_key);
if (gcm_key != NULL) {
CRYPTO_gcm128_init_key(gcm_key, aes_key, (block128_f)aes_hw_encrypt, 1);
CRYPTO_gcm128_init_key(gcm_key, aes_key, aes_hw_encrypt, 1);
}
if (out_block) {
*out_block = (block128_f) aes_hw_encrypt;
*out_block = aes_hw_encrypt;
}
return (ctr128_f)aes_hw_ctr32_encrypt_blocks;
return aes_hw_ctr32_encrypt_blocks;
}

if (bsaes_capable()) {
AES_set_encrypt_key(key, key_bytes * 8, aes_key);
if (gcm_key != NULL) {
CRYPTO_gcm128_init_key(gcm_key, aes_key, (block128_f)AES_encrypt, 0);
CRYPTO_gcm128_init_key(gcm_key, aes_key, AES_encrypt, 0);
}
if (out_block) {
*out_block = (block128_f) AES_encrypt;
*out_block = AES_encrypt;
}
return (ctr128_f)bsaes_ctr32_encrypt_blocks;
return bsaes_ctr32_encrypt_blocks;
}

if (vpaes_capable()) {
vpaes_set_encrypt_key(key, key_bytes * 8, aes_key);
if (out_block) {
*out_block = (block128_f) vpaes_encrypt;
*out_block = vpaes_encrypt;
}
if (gcm_key != NULL) {
CRYPTO_gcm128_init_key(gcm_key, aes_key, (block128_f)vpaes_encrypt, 0);
CRYPTO_gcm128_init_key(gcm_key, aes_key, vpaes_encrypt, 0);
}
return NULL;
}

AES_set_encrypt_key(key, key_bytes * 8, aes_key);
if (gcm_key != NULL) {
CRYPTO_gcm128_init_key(gcm_key, aes_key, (block128_f)AES_encrypt, 0);
CRYPTO_gcm128_init_key(gcm_key, aes_key, AES_encrypt, 0);
}
if (out_block) {
*out_block = (block128_f) AES_encrypt;
*out_block = AES_encrypt;
}
return NULL;
}
Expand Down
4 changes: 2 additions & 2 deletions crypto/fipsmodule/modes/cbc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@


void CRYPTO_cbc128_encrypt(const uint8_t *in, uint8_t *out, size_t len,
const void *key, uint8_t ivec[16],
const AES_KEY *key, uint8_t ivec[16],
block128_f block) {
size_t n;
const uint8_t *iv = ivec;
Expand Down Expand Up @@ -108,7 +108,7 @@ void CRYPTO_cbc128_encrypt(const uint8_t *in, uint8_t *out, size_t len,
}

void CRYPTO_cbc128_decrypt(const uint8_t *in, uint8_t *out, size_t len,
const void *key, uint8_t ivec[16],
const AES_KEY *key, uint8_t ivec[16],
block128_f block) {
size_t n;
union {
Expand Down
14 changes: 7 additions & 7 deletions crypto/fipsmodule/modes/ccm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ struct ccm128_state {
} nonce, cmac;
};

int CRYPTO_ccm128_init(CCM128_CONTEXT *ctx, const void *key, block128_f block,
ctr128_f ctr, unsigned M, unsigned L) {
int CRYPTO_ccm128_init(CCM128_CONTEXT *ctx, const AES_KEY *key,
block128_f block, ctr128_f ctr, unsigned M, unsigned L) {
if (M < 4 || M > 16 || (M & 1) != 0 || L < 2 || L > 8) {
return 0;
}
Expand All @@ -82,7 +82,7 @@ size_t CRYPTO_ccm128_max_input(const CCM128_CONTEXT *ctx) {
}

static int ccm128_init_state(const CCM128_CONTEXT *ctx,
struct ccm128_state *state, const void *key,
struct ccm128_state *state, const AES_KEY *key,
const uint8_t *nonce, size_t nonce_len,
const uint8_t *aad, size_t aad_len,
size_t plaintext_len) {
Expand Down Expand Up @@ -170,7 +170,7 @@ static int ccm128_init_state(const CCM128_CONTEXT *ctx,
}

static int ccm128_encrypt(const CCM128_CONTEXT *ctx, struct ccm128_state *state,
const void *key, uint8_t *out, const uint8_t *in,
const AES_KEY *key, 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++) {
Expand All @@ -191,7 +191,7 @@ static int ccm128_encrypt(const CCM128_CONTEXT *ctx, struct ccm128_state *state,
}

static int ccm128_compute_mac(const CCM128_CONTEXT *ctx,
struct ccm128_state *state, const void *key,
struct ccm128_state *state, const AES_KEY *key,
uint8_t *out_tag, size_t tag_len,
const uint8_t *in, size_t len) {
block128_f block = ctx->block;
Expand Down Expand Up @@ -231,7 +231,7 @@ static int ccm128_compute_mac(const CCM128_CONTEXT *ctx,
return 1;
}

int CRYPTO_ccm128_encrypt(const CCM128_CONTEXT *ctx, const void *key,
int CRYPTO_ccm128_encrypt(const CCM128_CONTEXT *ctx, const AES_KEY *key,
uint8_t *out, uint8_t *out_tag, size_t tag_len,
const uint8_t *nonce, size_t nonce_len,
const uint8_t *in, size_t len, const uint8_t *aad,
Expand All @@ -243,7 +243,7 @@ int CRYPTO_ccm128_encrypt(const CCM128_CONTEXT *ctx, const void *key,
ccm128_encrypt(ctx, &state, key, out, in, len);
}

int CRYPTO_ccm128_decrypt(const CCM128_CONTEXT *ctx, const void *key,
int CRYPTO_ccm128_decrypt(const CCM128_CONTEXT *ctx, const AES_KEY *key,
uint8_t *out, uint8_t *out_tag, size_t tag_len,
const uint8_t *nonce, size_t nonce_len,
const uint8_t *in, size_t len, const uint8_t *aad,
Expand Down
10 changes: 5 additions & 5 deletions crypto/fipsmodule/modes/cfb.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
OPENSSL_COMPILE_ASSERT((16 % sizeof(size_t)) == 0, bad_size_t_size_cfb);

void CRYPTO_cfb128_encrypt(const uint8_t *in, uint8_t *out, size_t len,
const void *key, uint8_t ivec[16], unsigned *num,
const AES_KEY *key, uint8_t ivec[16], unsigned *num,
int enc, block128_f block) {
size_t l = 0;

Expand Down Expand Up @@ -161,7 +161,7 @@ void CRYPTO_cfb128_encrypt(const uint8_t *in, uint8_t *out, size_t len,
/* This expects a single block of size nbits for both in and out. Note that
it corrupts any extra bits in the last byte of out */
static void cfbr_encrypt_block(const uint8_t *in, uint8_t *out, unsigned nbits,
const void *key, uint8_t ivec[16], int enc,
const AES_KEY *key, uint8_t ivec[16], int enc,
block128_f block) {
int n, rem, num;
uint8_t ovec[16 * 2 + 1]; /* +1 because we dererefence (but don't use) one
Expand Down Expand Up @@ -203,8 +203,8 @@ static void cfbr_encrypt_block(const uint8_t *in, uint8_t *out, unsigned nbits,

// N.B. This expects the input to be packed, MS bit first
void CRYPTO_cfb128_1_encrypt(const uint8_t *in, uint8_t *out, size_t bits,
const void *key, uint8_t ivec[16], unsigned *num,
int enc, block128_f block) {
const AES_KEY *key, uint8_t ivec[16],
unsigned *num, int enc, block128_f block) {
size_t n;
uint8_t c[1], d[1];

Expand All @@ -220,7 +220,7 @@ void CRYPTO_cfb128_1_encrypt(const uint8_t *in, uint8_t *out, size_t bits,
}

void CRYPTO_cfb128_8_encrypt(const unsigned char *in, unsigned char *out,
size_t length, const void *key,
size_t length, const AES_KEY *key,
unsigned char ivec[16], unsigned *num, int enc,
block128_f block) {
size_t n;
Expand Down
11 changes: 5 additions & 6 deletions crypto/fipsmodule/modes/ctr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ OPENSSL_COMPILE_ASSERT((16 % sizeof(size_t)) == 0, bad_size_t_size_ctr);
// of the IV. This implementation takes NO responsibility for checking that
// the counter doesn't overflow into the rest of the IV when incremented.
void CRYPTO_ctr128_encrypt(const uint8_t *in, uint8_t *out, size_t len,
const void *key, uint8_t ivec[16],
const AES_KEY *key, uint8_t ivec[16],
uint8_t ecount_buf[16], unsigned int *num,
block128_f block) {
unsigned int n;
Expand Down Expand Up @@ -153,11 +153,10 @@ static void ctr96_inc(uint8_t *counter) {
} while (n);
}

void CRYPTO_ctr128_encrypt_ctr32(const uint8_t *in, uint8_t *out,
size_t len, const void *key,
uint8_t ivec[16],
uint8_t ecount_buf[16],
unsigned int *num, ctr128_f func) {
void CRYPTO_ctr128_encrypt_ctr32(const uint8_t *in, uint8_t *out, size_t len,
const AES_KEY *key, uint8_t ivec[16],
uint8_t ecount_buf[16], unsigned int *num,
ctr128_f func) {
unsigned int n, ctr32;

assert(key && ecount_buf && num);
Expand Down
Loading

0 comments on commit 73535ab

Please sign in to comment.