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 5e12c53806288af02931257f72b5adb93d01b8c7..e12a2178a417d646099d891e004f3c68890c9e74 100644 (file)
--- 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 d998e104d4e1a86a7834ccb51648cc5fd9bfec23..ec449560f487a36df5b48f39d2c77d56705a8032 100644 (file)
--- 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;