[openssl] OpenSSL_1_1_1-stable update

[Matt Caswell]

The branch OpenSSL_1_1_1-stable has been updated
       via  f6d64b5142ab59be47c1f10512ce6d58fb399131 (commit)
      from  4a81b8b6e8b908ff70d675c7173ad4923f3dc659 (commit)


- Log -----------------------------------------------------------------
commit f6d64b5142ab59be47c1f10512ce6d58fb399131
Author: Matt Caswell <matt at openssl.org>
Date:   Thu Feb 21 16:02:24 2019 +0000

    Don't restrict the number of KeyUpdate messages we can process
    
    Prior to this commit we were keeping a count of how many KeyUpdates we
    have processed and failing if we had had too many. This simplistic approach
    is not sufficient for long running connections. Since many KeyUpdates
    would not be a particular good DoS route anyway, the simplest solution is
    to simply remove the key update count.
    
    Fixes #8068
    
    Reviewed-by: Kurt Roeckx <kurt at roeckx.be>
    (Merged from https://github.com/openssl/openssl/pull/8299)
    
    (cherry picked from commit 3409a5ff8a44ddaf043d83ed22e657ae871be289)

-----------------------------------------------------------------------

Summary of changes:
 ssl/ssl_locl.h           |  2 --
 ssl/statem/statem_lib.c  |  7 -------
 ssl/statem/statem_locl.h |  3 ---
 test/sslapitest.c        | 53 ++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 53 insertions(+), 12 deletions(-)

diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 307131d..6559012 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -1170,8 +1170,6 @@ struct ssl_st {
     EVP_CIPHER_CTX *enc_write_ctx; /* cryptographic state */
     unsigned char write_iv[EVP_MAX_IV_LENGTH]; /* TLSv1.3 static write IV */
     EVP_MD_CTX *write_hash;     /* used for mac generation */
-    /* Count of how many KeyUpdate messages we have received */
-    unsigned int key_update_count;
     /* session info */
     /* client cert? */
     /* This is used to hold the server certificate used */
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index 15d0148..7e32e75 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -614,13 +614,6 @@ MSG_PROCESS_RETURN tls_process_key_update(SSL *s, PACKET *pkt)
 {
     unsigned int updatetype;
 
-    s->key_update_count++;
-    if (s->key_update_count > MAX_KEY_UPDATE_MESSAGES) {
-        SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_TLS_PROCESS_KEY_UPDATE,
-                 SSL_R_TOO_MANY_KEY_UPDATES);
-        return MSG_PROCESS_ERROR;
-    }
-
     /*
      * A KeyUpdate message signals a key change so the end of the message must
      * be on a record boundary.
diff --git a/ssl/statem/statem_locl.h b/ssl/statem/statem_locl.h
index 6b8cf37..f936c61 100644
--- a/ssl/statem/statem_locl.h
+++ b/ssl/statem/statem_locl.h
@@ -29,9 +29,6 @@
 /* Max should actually be 36 but we are generous */
 #define FINISHED_MAX_LENGTH             64
 
-/* The maximum number of incoming KeyUpdate messages we will accept */
-#define MAX_KEY_UPDATE_MESSAGES     32
-
 /* Dummy message type */
 #define SSL3_MT_DUMMY   -1
 
diff --git a/test/sslapitest.c b/test/sslapitest.c
index b3aa563..f51da0c 100644
--- a/test/sslapitest.c
+++ b/test/sslapitest.c
@@ -4250,6 +4250,58 @@ static int test_export_key_mat_early(int idx)
 
     return testresult;
 }
+
+#define NUM_KEY_UPDATE_MESSAGES 40
+/*
+ * Test KeyUpdate.
+ */
+static int test_key_update(void)
+{
+    SSL_CTX *cctx = NULL, *sctx = NULL;
+    SSL *clientssl = NULL, *serverssl = NULL;
+    int testresult = 0, i, j;
+    char buf[20];
+    static char *mess = "A test message";
+
+    if (!TEST_true(create_ssl_ctx_pair(TLS_server_method(),
+                                       TLS_client_method(),
+                                       TLS1_3_VERSION,
+                                       0,
+                                       &sctx, &cctx, cert, privkey))
+            || !TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl,
+                                             NULL, NULL))
+            || !TEST_true(create_ssl_connection(serverssl, clientssl,
+                                                SSL_ERROR_NONE)))
+        goto end;
+
+    for (j = 0; j < 2; j++) {
+        /* Send lots of KeyUpdate messages */
+        for (i = 0; i < NUM_KEY_UPDATE_MESSAGES; i++) {
+            if (!TEST_true(SSL_key_update(clientssl,
+                                          (j == 0)
+                                          ? SSL_KEY_UPDATE_NOT_REQUESTED
+                                          : SSL_KEY_UPDATE_REQUESTED))
+                    || !TEST_true(SSL_do_handshake(clientssl)))
+                goto end;
+        }
+
+        /* Check that sending and receiving app data is ok */
+        if (!TEST_int_eq(SSL_write(clientssl, mess, strlen(mess)), strlen(mess))
+                || !TEST_int_eq(SSL_read(serverssl, buf, sizeof(buf)),
+                                         strlen(mess)))
+            goto end;
+    }
+
+    testresult = 1;
+
+ end:
+    SSL_free(serverssl);
+    SSL_free(clientssl);
+    SSL_CTX_free(sctx);
+    SSL_CTX_free(cctx);
+
+    return testresult;
+}
 #endif /* OPENSSL_NO_TLS1_3 */
 
 static int test_ssl_clear(int idx)
@@ -5929,6 +5981,7 @@ int setup_tests(void)
     ADD_ALL_TESTS(test_export_key_mat, 6);
 #ifndef OPENSSL_NO_TLS1_3
     ADD_ALL_TESTS(test_export_key_mat_early, 3);
+    ADD_TEST(test_key_update);
 #endif
     ADD_ALL_TESTS(test_ssl_clear, 2);
     ADD_ALL_TESTS(test_max_fragment_len_ext, OSSL_NELEM(max_fragment_len_test));