From: Matt Caswell <matt@openssl.org>
Date: Wed, 23 May 2018 11:00:10 +0000 (+0100)
Subject: Auto retry if we ditch records during shutdown
X-Git-Tag: OpenSSL_1_1_1-pre9~222
X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=93f528f36eb9423c31b2d75669cea85da97f9633;hp=c7504aeb640a88949dfe3146f7e0f275f517464c;p=oweals%2Fopenssl.git

Auto retry if we ditch records during shutdown

If we've sent a close_notify and we're waiting for one back we drop
incoming records until we see the close_notify we're looking for. If
SSL_MODE_AUTO_RETRY is on, then we should immediately try and read the
next record.

Fixes #6262

Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/6340)
---

diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index 8d5b53fb39..5e12c53806 100644
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -1457,40 +1457,6 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
         return -1;
     }
 
-    /*
-     * In case of record types for which we have 'fragment' storage, fill
-     * that so that we can process the data at a fixed place.
-     */
-    {
-        size_t dest_maxlen = 0;
-        unsigned char *dest = NULL;
-        size_t *dest_len = NULL;
-
-        if (SSL3_RECORD_get_type(rr) == SSL3_RT_HANDSHAKE) {
-            dest_maxlen = sizeof(s->rlayer.handshake_fragment);
-            dest = s->rlayer.handshake_fragment;
-            dest_len = &s->rlayer.handshake_fragment_len;
-        }
-
-        if (dest_maxlen > 0) {
-            n = dest_maxlen - *dest_len; /* available space in 'dest' */
-            if (SSL3_RECORD_get_length(rr) < n)
-                n = SSL3_RECORD_get_length(rr); /* available bytes */
-
-            /* now move 'n' bytes: */
-            memcpy(dest + *dest_len,
-                   SSL3_RECORD_get_data(rr) + SSL3_RECORD_get_off(rr), n);
-            SSL3_RECORD_add_off(rr, n);
-            SSL3_RECORD_sub_length(rr, n);
-            *dest_len += n;
-            if (SSL3_RECORD_get_length(rr) == 0)
-                SSL3_RECORD_set_read(rr);
-
-            if (*dest_len < dest_maxlen)
-                goto start;     /* fragment was too small */
-        }
-    }
-
     /*-
      * s->rlayer.handshake_fragment_len == 4  iff  rr->type == SSL3_RT_HANDSHAKE;
      * (Possibly rr is 'empty' now, i.e. rr->length may be 0.)
@@ -1583,14 +1549,55 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
         return -1;
     }
 
-    if (s->shutdown & SSL_SENT_SHUTDOWN) { /* but we have not received a
-                                            * shutdown */
+    /*
+     * If we've sent a close_notify but not yet received one back then ditch
+     * anything we read.
+     */
+    if ((s->shutdown & SSL_SENT_SHUTDOWN) != 0 ) {
+        /*
+         * In TLSv1.3 this could get problematic if we receive a KeyUpdate
+         * message after we sent a close_notify because we're about to ditch it,
+         * so we won't be able to read a close_notify sent afterwards! We don't
+         * support that.
+         */
         s->rwstate = SSL_NOTHING;
         SSL3_RECORD_set_length(rr, 0);
         SSL3_RECORD_set_read(rr);
+
+        if (SSL3_RECORD_get_type(rr) == SSL3_RT_HANDSHAKE
+                && (s->mode & SSL_MODE_AUTO_RETRY) != 0)
+            goto start;
         return 0;
     }
 
+    /*
+     * For handshake data we have 'fragment' storage, so fill that so that we
+     * can process the header at a fixed place. This is done after the
+     * "SHUTDOWN" code above to avoid filling the fragment storage with data
+     * that we're just going to discard.
+     */
+    if (SSL3_RECORD_get_type(rr) == SSL3_RT_HANDSHAKE) {
+        size_t dest_maxlen = sizeof(s->rlayer.handshake_fragment);
+        unsigned char *dest = s->rlayer.handshake_fragment;
+        size_t *dest_len = &s->rlayer.handshake_fragment_len;
+
+        n = dest_maxlen - *dest_len; /* available space in 'dest' */
+        if (SSL3_RECORD_get_length(rr) < n)
+            n = SSL3_RECORD_get_length(rr); /* available bytes */
+
+        /* now move 'n' bytes: */
+        memcpy(dest + *dest_len,
+               SSL3_RECORD_get_data(rr) + SSL3_RECORD_get_off(rr), n);
+        SSL3_RECORD_add_off(rr, n);
+        SSL3_RECORD_sub_length(rr, n);
+        *dest_len += n;
+        if (SSL3_RECORD_get_length(rr) == 0)
+            SSL3_RECORD_set_read(rr);
+
+        if (*dest_len < dest_maxlen)
+            goto start;     /* fragment was too small */
+    }
+
     if (SSL3_RECORD_get_type(rr) == SSL3_RT_CHANGE_CIPHER_SPEC) {
         SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE, SSL_F_SSL3_READ_BYTES,
                  SSL_R_CCS_RECEIVED_EARLY);