summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMatt Caswell <matt@openssl.org>2015-04-27 11:07:06 +0100
committerMatt Caswell <matt@openssl.org>2015-04-30 23:26:06 +0100
commit974d4d675cc6f3e1aa50b294ae12a5ba2acebd62 (patch)
tree3681670d8d71ba644bca79c3884ef964baf52754
parent3be5df227259628cea91faffbea5054b872f793a (diff)
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 <appro@openssl.org> (cherry picked from commit c8269881093324b881b81472be037055571f73f3) Conflicts: ssl/record/ssl3_record.c Conflicts: apps/speed.c crypto/evp/e_aes_cbc_hmac_sha256.c crypto/evp/evp.h
-rw-r--r--crypto/evp/e_aes.c2
-rw-r--r--crypto/evp/e_aes_cbc_hmac_sha1.c9
-rw-r--r--crypto/evp/e_rc4_hmac_md5.c7
-rw-r--r--crypto/evp/evp.h3
-rw-r--r--ssl/t1_enc.c7
5 files changed, 21 insertions, 7 deletions
diff --git a/crypto/evp/e_aes.c b/crypto/evp/e_aes.c
index 245c18a693..bde480481d 100644
--- a/crypto/evp/e_aes.c
+++ b/crypto/evp/e_aes.c
@@ -753,7 +753,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 3f8a5ae391..d1f5928f62 100644
--- a/crypto/evp/e_aes_cbc_hmac_sha1.c
+++ b/crypto/evp/e_aes_cbc_hmac_sha1.c
@@ -503,7 +503,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;
@@ -520,8 +525,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_rc4_hmac_md5.c b/crypto/evp/e_rc4_hmac_md5.c
index 80735d345a..e6b0cdff43 100644
--- a/crypto/evp/e_rc4_hmac_md5.c
+++ b/crypto/evp/e_rc4_hmac_md5.c
@@ -258,7 +258,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/crypto/evp/evp.h b/crypto/evp/evp.h
index b00997b149..01bdeebfbc 100644
--- a/crypto/evp/evp.h
+++ b/crypto/evp/evp.h
@@ -409,6 +409,9 @@ struct evp_cipher_st {
/* Set the GCM invocation field, decrypt only */
# define EVP_CTRL_GCM_SET_IV_INV 0x18
+/* RFC 5246 defines additional data to be 13 bytes in length */
+# define EVP_AEAD_TLS1_AAD_LEN 13
+
/* GCM TLS constants */
/* Length of fixed part of IV derived from PRF */
# define EVP_GCM_TLS_FIXED_IV_LEN 4
diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c
index 2736238f8e..8f45294e55 100644
--- a/ssl/t1_enc.c
+++ b/ssl/t1_enc.c
@@ -785,7 +785,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 ? s->s3->write_sequence : s->s3->read_sequence;
@@ -809,7 +809,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;