From: Matt Caswell Date: Mon, 27 Apr 2015 10:07:06 +0000 (+0100) Subject: Sanity check EVP_CTRL_AEAD_TLS_AAD X-Git-Tag: OpenSSL_1_1_0-pre1~1239 X-Git-Url: https://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff_plain;h=c8269881093324b881b81472be037055571f73f3 Sanity check EVP_CTRL_AEAD_TLS_AAD The various implementations of EVP_CTRL_AEAD_TLS_AAD expect a buffer of at least 13 bytes long. Add sanity checks to ensure that the length is at least that. Also add a new constant (EVP_AEAD_TLS1_AAD_LEN) to evp.h to represent this length. Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3 Solutions) for reporting this issue. Reviewed-by: Andy Polyakov --- diff --git a/apps/speed.c b/apps/speed.c index 720ab1cc21..08ab9c5fa6 100644 --- a/apps/speed.c +++ b/apps/speed.c @@ -2456,7 +2456,7 @@ static void multiblock_speed(const EVP_CIPHER *evp_cipher) print_message(alg_name, 0, mblengths[j]); Time_F(START); for (count = 0, run = 1; run && count < 0x7fffffff; count++) { - unsigned char aad[13]; + unsigned char aad[EVP_AEAD_TLS1_AAD_LEN]; EVP_CTRL_TLS1_1_MULTIBLOCK_PARAM mb_param; size_t len = mblengths[j]; int packlen; @@ -2491,7 +2491,8 @@ static void multiblock_speed(const EVP_CIPHER *evp_cipher) aad[11] = len >> 8; aad[12] = len; pad = EVP_CIPHER_CTX_ctrl(&ctx, - EVP_CTRL_AEAD_TLS1_AAD, 13, aad); + EVP_CTRL_AEAD_TLS1_AAD, + EVP_AEAD_TLS1_AAD_LEN, aad); EVP_Cipher(&ctx, out, inp, len + pad); } } diff --git a/crypto/evp/e_aes.c b/crypto/evp/e_aes.c index 7b4d84f58d..0b7838e455 100644 --- a/crypto/evp/e_aes.c +++ b/crypto/evp/e_aes.c @@ -1327,7 +1327,7 @@ static int aes_gcm_ctrl(EVP_CIPHER_CTX *c, int type, int arg, void *ptr) case EVP_CTRL_AEAD_TLS1_AAD: /* Save the AAD for later use */ - if (arg != 13) + if (arg != EVP_AEAD_TLS1_AAD_LEN) return 0; memcpy(c->buf, ptr, arg); gctx->tls_aad_len = arg; diff --git a/crypto/evp/e_aes_cbc_hmac_sha1.c b/crypto/evp/e_aes_cbc_hmac_sha1.c index 960be3cdbc..7f2848e14b 100644 --- a/crypto/evp/e_aes_cbc_hmac_sha1.c +++ b/crypto/evp/e_aes_cbc_hmac_sha1.c @@ -845,7 +845,12 @@ static int aesni_cbc_hmac_sha1_ctrl(EVP_CIPHER_CTX *ctx, int type, int arg, case EVP_CTRL_AEAD_TLS1_AAD: { unsigned char *p = ptr; - unsigned int len = p[arg - 2] << 8 | p[arg - 1]; + unsigned int len; + + if (arg != EVP_AEAD_TLS1_AAD_LEN) + return -1; + + len = p[arg - 2] << 8 | p[arg - 1]; if (ctx->encrypt) { key->payload_length = len; @@ -862,8 +867,6 @@ static int aesni_cbc_hmac_sha1_ctrl(EVP_CIPHER_CTX *ctx, int type, int arg, AES_BLOCK_SIZE) & -AES_BLOCK_SIZE) - len); } else { - if (arg > 13) - arg = 13; memcpy(key->aux.tls_aad, ptr, arg); key->payload_length = arg; diff --git a/crypto/evp/e_aes_cbc_hmac_sha256.c b/crypto/evp/e_aes_cbc_hmac_sha256.c index bea8f6dd50..3b6827a9fd 100644 --- a/crypto/evp/e_aes_cbc_hmac_sha256.c +++ b/crypto/evp/e_aes_cbc_hmac_sha256.c @@ -817,6 +817,11 @@ static int aesni_cbc_hmac_sha256_ctrl(EVP_CIPHER_CTX *ctx, int type, int arg, unsigned char *p = ptr; unsigned int len = p[arg - 2] << 8 | p[arg - 1]; + if (arg != EVP_AEAD_TLS1_AAD_LEN) + return -1; + + len = p[arg - 2] << 8 | p[arg - 1]; + if (ctx->encrypt) { key->payload_length = len; if ((key->aux.tls_ver = @@ -832,8 +837,6 @@ static int aesni_cbc_hmac_sha256_ctrl(EVP_CIPHER_CTX *ctx, int type, int arg, AES_BLOCK_SIZE) & -AES_BLOCK_SIZE) - len); } else { - if (arg > 13) - arg = 13; memcpy(key->aux.tls_aad, ptr, arg); key->payload_length = arg; diff --git a/crypto/evp/e_rc4_hmac_md5.c b/crypto/evp/e_rc4_hmac_md5.c index 7c4bd34d9b..1ba690da11 100644 --- a/crypto/evp/e_rc4_hmac_md5.c +++ b/crypto/evp/e_rc4_hmac_md5.c @@ -257,7 +257,12 @@ static int rc4_hmac_md5_ctrl(EVP_CIPHER_CTX *ctx, int type, int arg, case EVP_CTRL_AEAD_TLS1_AAD: { unsigned char *p = ptr; - unsigned int len = p[arg - 2] << 8 | p[arg - 1]; + unsigned int len; + + if (arg != EVP_AEAD_TLS1_AAD_LEN) + return -1; + + len = p[arg - 2] << 8 | p[arg - 1]; if (!ctx->encrypt) { len -= MD5_DIGEST_LENGTH; diff --git a/include/openssl/evp.h b/include/openssl/evp.h index 0d26fd3129..4df3ce72c1 100644 --- a/include/openssl/evp.h +++ b/include/openssl/evp.h @@ -426,6 +426,9 @@ struct evp_cipher_st { # define EVP_CTRL_TLS1_1_MULTIBLOCK_DECRYPT 0x1b # define EVP_CTRL_TLS1_1_MULTIBLOCK_MAX_BUFSIZE 0x1c +/* RFC 5246 defines additional data to be 13 bytes in length */ +# define EVP_AEAD_TLS1_AAD_LEN 13 + typedef struct { unsigned char *out; const unsigned char *inp; diff --git a/ssl/record/ssl3_record.c b/ssl/record/ssl3_record.c index cfd8290d33..33d0b302f0 100644 --- a/ssl/record/ssl3_record.c +++ b/ssl/record/ssl3_record.c @@ -658,7 +658,7 @@ int tls1_enc(SSL *s, int send) bs = EVP_CIPHER_block_size(ds->cipher); if (EVP_CIPHER_flags(ds->cipher) & EVP_CIPH_FLAG_AEAD_CIPHER) { - unsigned char buf[13], *seq; + unsigned char buf[EVP_AEAD_TLS1_AAD_LEN], *seq; seq = send ? RECORD_LAYER_get_write_sequence(&s->rlayer) : RECORD_LAYER_get_read_sequence(&s->rlayer); @@ -684,7 +684,10 @@ int tls1_enc(SSL *s, int send) buf[10] = (unsigned char)(s->version); buf[11] = rec->length >> 8; buf[12] = rec->length & 0xff; - pad = EVP_CIPHER_CTX_ctrl(ds, EVP_CTRL_AEAD_TLS1_AAD, 13, buf); + pad = EVP_CIPHER_CTX_ctrl(ds, EVP_CTRL_AEAD_TLS1_AAD, + EVP_AEAD_TLS1_AAD_LEN, buf); + if (pad <= 0) + return -1; if (send) { l += pad; rec->length += pad;