From: Matt Caswell <matt@openssl.org>
Date: Thu, 21 Jun 2018 12:30:38 +0000 (+0100)
Subject: Return SSL_ERROR_WANT_READ if SSL_shutdown() encounters handshake data
X-Git-Tag: OpenSSL_1_1_1-pre9~220
X-Git-Url: https://git.openssl.org/gitweb/?a=commitdiff_plain;h=ba70904949d2f9eec160043bf9a97182b33a2b82;hp=c748834ff7af7949519d2820a79ec35e809b5a71;p=openssl.git

Return SSL_ERROR_WANT_READ if SSL_shutdown() encounters handshake data

In the case where we are shutdown for writing and awaiting a close_notify
back from a subsequent SSL_shutdown() call we skip over handshake data
that is received. This should not be treated as an error - instead it
should be signalled with SSL_ERROR_WANT_READ.

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 5e12c53806..e12a2178a4 100644
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -1553,20 +1553,30 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
      * 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 ) {
+    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;
+        if (SSL3_RECORD_get_type(rr) == SSL3_RT_HANDSHAKE) {
+            BIO *rbio;
+
+            if ((s->mode & SSL_MODE_AUTO_RETRY) != 0)
+                goto start;
+
+            s->rwstate = SSL_READING;
+            rbio = SSL_get_rbio(s);
+            BIO_clear_retry_flags(rbio);
+            BIO_set_retry_read(rbio);
+            return -1;
+        }
+
+        s->rwstate = SSL_NOTHING;
         return 0;
     }
 
diff --git a/test/sslapitest.c b/test/sslapitest.c
index d998e104d4..ec449560f4 100644
--- a/test/sslapitest.c
+++ b/test/sslapitest.c
@@ -5051,12 +5051,7 @@ static int test_shutdown(int tst)
     }
 
     /* Writing on the client after sending close_notify shouldn't be possible */
-    if (!TEST_false(SSL_write_ex(clientssl, msg, sizeof(msg), &written))
-               /*
-                * Writing on the server after sending close_notify shouldn't be
-                * possible.
-                */
-            || !TEST_false(SSL_write_ex(clientssl, msg, sizeof(msg), &written)))
+    if (!TEST_false(SSL_write_ex(clientssl, msg, sizeof(msg), &written)))
         goto end;
 
     if (tst < 4) {
@@ -5066,6 +5061,11 @@ static int test_shutdown(int tst)
          * yet.
          */
         if (!TEST_int_eq(SSL_shutdown(serverssl), 0)
+                   /*
+                    * Writing on the server after sending close_notify shouldn't
+                    * be possible.
+                    */
+                || !TEST_false(SSL_write_ex(serverssl, msg, sizeof(msg), &written))
                 || !TEST_int_eq(SSL_shutdown(clientssl), 1)
                 || !TEST_int_eq(SSL_shutdown(serverssl), 1))
             goto end;