commit 693cf80c6ff54ae276a44d305d4ad07168ec6895
author Kurt Roeckx <kurt@roeckx.be> Tue May 15 19:01:41 2018 +0200
committer Kurt Roeckx <kurt@roeckx.be> Tue May 22 22:45:28 2018 +0200
tree bed0784d42fe6d1544cbe3c1ebf1abe56eb65cf1
parent 1aac20f5095fca8691ef4495c3e7438c935a33dc

Enable SSL_MODE_AUTO_RETRY by default

Because TLS 1.3 sends more non-application data records some clients run
into problems because they don't expect SSL_read() to return and set
SSL_ERROR_WANT_READ after processing it.

This can cause problems for clients that use blocking I/O and use
select() to see if data is available. It can be cleared using
SSL_CTX_clear_mode().

Reviewed-by: Matt Caswell <matt@openssl.org>
GH: #6260

CHANGES

diff --git a/CHANGES b/CHANGES
index c67a9c6..612da59 100644
--- a/CHANGES
+++ b/CHANGES
@@ -9,6 +9,17 @@
 
  Changes between 1.1.0h and 1.1.1 [xx XXX xxxx]
 
+  *) SSL_MODE_AUTO_RETRY is enabled by default. Applications that use blocking
+     I/O in combination with something like select() or poll() will hang. This
+     can be turned off again using SSL_CTX_clear_mode().
+     Many applications do not properly handle non-application data records, and
+     TLS 1.3 sends more of such records. Setting SSL_MODE_AUTO_RETRY works
+     around the problems in those applications, but can also break some.
+     It's recommended to read the manpages about SSL_read(), SSL_write(),
+     SSL_get_error(), SSL_shutdown(), SSL_CTX_set_mode() and
+     SSL_CTX_set_read_ahead() again.
+     [Kurt Roeckx]
+
   *) When unlocking a pass phrase protected PEM file or PKCS#8 container, we
      now allow empty (zero character) pass phrases.
      [Richard Levitte]

apps/s_client.c

diff --git a/apps/s_client.c b/apps/s_client.c
index 5934236..9122d48 100644
--- a/apps/s_client.c
+++ b/apps/s_client.c
@@ -1675,6 +1675,8 @@
         goto end;
     }
 
+    SSL_CTX_clear_mode(ctx, SSL_MODE_AUTO_RETRY);
+
     if (sdebug)
         ssl_ctx_security_debug(ctx, sdebug);
 

apps/s_server.c

diff --git a/apps/s_server.c b/apps/s_server.c
index 6180617..b0d38e4 100644
--- a/apps/s_server.c
+++ b/apps/s_server.c
@@ -1753,6 +1753,9 @@
         ERR_print_errors(bio_err);
         goto end;
     }
+
+    SSL_CTX_clear_mode(ctx, SSL_MODE_AUTO_RETRY);
+
     if (sdebug)
         ssl_ctx_security_debug(ctx, sdebug);
 

ssl/ssl_lib.c

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 1dd355d..22f729c 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -2896,6 +2896,7 @@
     ret->method = meth;
     ret->min_proto_version = 0;
     ret->max_proto_version = 0;
+    ret->mode = SSL_MODE_AUTO_RETRY;
     ret->session_cache_mode = SSL_SESS_CACHE_SERVER;
     ret->session_cache_size = SSL_SESSION_CACHE_MAX_SIZE_DEFAULT;
     /* We take the system default. */

test/sslapitest.c

diff --git a/test/sslapitest.c b/test/sslapitest.c
index f2978aa..10bfc8a 100644
--- a/test/sslapitest.c
+++ b/test/sslapitest.c
@@ -2351,15 +2351,6 @@
             || !TEST_size_t_eq(written, strlen(MSG2)))
         goto end;
 
-    /*
-     * Should block due to the NewSessionTicket arrival unless we're using
-     * read_ahead, or PSKs
-     */
-    if (idx != 1 && idx != 2) {
-        if (!TEST_false(SSL_read_ex(clientssl, buf, sizeof(buf), &readbytes)))
-            goto end;
-    }
-
     if (!TEST_true(SSL_read_ex(clientssl, buf, sizeof(buf), &readbytes))
             || !TEST_mem_eq(buf, readbytes, MSG2, strlen(MSG2)))
         goto end;