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

diff --git a/apps/speed.c b/apps/speed.c
index 8c350ee83d06c10ee169f8d1b0dc6b3663aded9d..3697b71ec18b457a4023c6393d48629f9fa68012 100644 (file)
--- a/apps/speed.c
+++ b/apps/speed.c
@@ -2791,7 +2791,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;
@@ -2826,7 +2826,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 8161b26325745e78ddf775666fc3a476b81a939e..af4aa1801e58f4b3a7a5d12bfce7d1255edb8c2d 100644 (file)
--- a/crypto/evp/e_aes.c
+++ b/crypto/evp/e_aes.c
@@ -1227,7 +1227,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 e0127a9bb2e7ace51f18e99910b7e0b468db980f..a277d0f86615071bef59dc8b6c47f4139457cdda 100644 (file)
--- 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 30398c7ca43bd72936d421be02e55f4ea06a5299..b74bd80a05af1bb58790dd00910e8b6af9da4cb0 100644 (file)
--- a/crypto/evp/e_aes_cbc_hmac_sha256.c
+++ b/crypto/evp/e_aes_cbc_hmac_sha256.c
@@ -813,6 +813,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 =
@@ -828,8 +833,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 80735d345adfd072d65d73c30e488adb43ab4d8e..e6b0cdff436a7434ea6db50ae70c6f213e34f974 100644 (file)
--- 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 47abbac4a24cfe24ef60ab88510087a4d3dfcc09..4891133daeda99c2686d9f5281bd796e8ada6a91 100644 (file)
--- a/crypto/evp/evp.h
+++ b/crypto/evp/evp.h
@@ -424,6 +424,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/t1_enc.c b/ssl/t1_enc.c
index 0563191af2453bc481d551e965d9ee59d2d6d8c7..e2a8f86919789e4a7ba117857fb174405a413bcc 100644 (file)
--- a/ssl/t1_enc.c
+++ b/ssl/t1_enc.c
@@ -803,7 +803,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;
 
@@ -827,7 +827,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;