Improve ticket validity checking when tlsext_ticket_key_cb() callback

chooses a different HMAC algorithm.
Avert memory leaks if the callback preps the HMAC in some way.

Based on openssl commit 1bbe48ab149893a78bf99c8eb8895c928900a16f
but retaining a pre-callback length check to guarantee the callback
is provided the buffer that the API claims.

ok bcook@ jsing@

1 files changed, 25 insertions, 4 deletions

diff --git a/src/lib/libssl/t1_lib.c b/src/lib/libssl/t1_lib.c
index 3022469ea9..6853bc210e 100644
--- a/src/lib/libssl/t1_lib.c
+++ b/src/lib/libssl/t1_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: t1_lib.c,v 1.88 2016/08/27 15:58:06 jsing Exp $ */
+/* $OpenBSD: t1_lib.c,v 1.89 2016/09/22 06:57:40 guenther Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -2163,9 +2163,16 @@ tls_decrypt_ticket(SSL *s, const unsigned char *etick, int eticklen,
         HMAC_CTX hctx;
         EVP_CIPHER_CTX ctx;
         SSL_CTX *tctx = s->initial_ctx;
-        /* Need at least keyname + iv + some encrypted data */
-        if (eticklen < 48)
+
+        /*
+         * The API guarantees EVP_MAX_IV_LENGTH bytes of space for
+         * the iv to tlsext_ticket_key_cb().  Since the total space
+         * required for a session cookie is never less than this,
+         * this check isn't too strict.  The exact check comes later.
+         */
+        if (eticklen < 16 + EVP_MAX_IV_LENGTH)
                 return 2;
+
         /* Initialize session ticket encryption and HMAC contexts */
         HMAC_CTX_init(&hctx);
         EVP_CIPHER_CTX_init(&ctx);
@@ -2174,10 +2181,12 @@ tls_decrypt_ticket(SSL *s, const unsigned char *etick, int eticklen,
                 int rv = tctx->tlsext_ticket_key_cb(s, nctick, nctick + 16,
                     &ctx, &hctx, 0);
                 if (rv < 0) {
+                        HMAC_CTX_cleanup(&hctx);
                         EVP_CIPHER_CTX_cleanup(&ctx);
                         return -1;
                 }
                 if (rv == 0) {
+                        HMAC_CTX_cleanup(&hctx);
                         EVP_CIPHER_CTX_cleanup(&ctx);
                         return 2;
                 }
@@ -2192,15 +2201,26 @@ tls_decrypt_ticket(SSL *s, const unsigned char *etick, int eticklen,
                 EVP_DecryptInit_ex(&ctx, EVP_aes_128_cbc(), NULL,
                     tctx->tlsext_tick_aes_key, etick + 16);
         }
-        /* Attempt to process session ticket, first conduct sanity and
+
+        /*
+         * Attempt to process session ticket, first conduct sanity and
          * integrity checks on ticket.
          */
         mlen = HMAC_size(&hctx);
         if (mlen < 0) {
+                HMAC_CTX_cleanup(&hctx);
                 EVP_CIPHER_CTX_cleanup(&ctx);
                 return -1;
         }
+
+        /* Sanity check ticket length: must exceed keyname + IV + HMAC */
+        if (eticklen < 16 + EVP_CIPHER_CTX_iv_length(&ctx) + mlen) {
+                HMAC_CTX_cleanup(&hctx);
+                EVP_CIPHER_CTX_cleanup(&ctx);
+                return 2;
+        }
         eticklen -= mlen;
+
         /* Check HMAC of encrypted ticket */
         HMAC_Update(&hctx, etick, eticklen);
         HMAC_Final(&hctx, tick_hmac, NULL);
@@ -2209,6 +2229,7 @@ tls_decrypt_ticket(SSL *s, const unsigned char *etick, int eticklen,
                 EVP_CIPHER_CTX_cleanup(&ctx);
                 return 2;
         }
+
         /* Attempt to decrypt session data */
         /* Move p after IV to start of encrypted ticket, update length */
         p = etick + 16 + EVP_CIPHER_CTX_iv_length(&ctx);