OpenSSL and Early data

[Sergey Kandaurov]

> On 6 Dec 2018, at 19:20, Jan Prachař <jan.prachar at gmail.com> wrote:
> 
> On Thu, 2018-12-06 at 18:13 +0300, Sergey Kandaurov wrote:
>>> On 6 Dec 2018, at 02:39, Honza Prachař <jan.prachar at gmail.com>
>>> wrote:
>>> 
>>> Hello! FYI there is an issue with TLS 1.3 Early data in OpenSSL – 
>>> https://github.com/openssl/openssl/issues/7757
>>> 
>>> So maybe you would want to consider ignoring Early data with HTTP/2
>>> and OpenSSL. Or try to fix the problem on the nginx side, i.e. do
>>> not call SSL_read_early_data() until all pending data is written
>>> with SSL_write_early_data().
>> 
>> Hello.
>> 
>> This is not strictly related to HTTP/2.
>> I could reproduce it with s_client -early_data over HTTP/1.1,
>> where 1st request is sent in 0-RTT, and 2nd - after handshake.
>> 
>> This quick workaround helped me.  The idea is that we block reading
>> if SSL_write_early_data returned SSL_ERROR_WANT_WRITE, until one of
>> the next SSL_write_early_data will succeed.  In practice, we won't
>> read until there's also no more data to send.  For static content,
>> that means that we will continue to read only after the whole file
>> was sent.  This doesn't look perfect but seems to work.
> 
> This patch works for me too. SSL_read_early_data waits until all
> requested files are sent. Then the handshake is finished.

Thanks.
It would be nice if you could also try this patch instead.
Unlike previous, this one is closer to what would be committed.

# HG changeset patch
# User Sergey Kandaurov <pluknet at nginx.com>
# Date 1544744260 -10800
#      Fri Dec 14 02:37:40 2018 +0300
# Node ID bdd5189649f5d788a0e21d0dcd6bc6f4593af401
# Parent  81d49f85afedac2ec7237ee3ce4e6ae927c94706
SSL: avoid reading on pending SSL_write_early_data().

If SSL_write_early_data() returned SSL_ERROR_WANT_WRITE, stop further reading
using a newly introduced c->ssl->write_blocked flag, as otherwise this would
result in SSL error "ssl3_write_bytes:bad length".  Eventually, normal reading
will be restored by read event posted from successful SSL_write_early_data().

While here, place "SSL_write_early_data: want write" debug on the path.

diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
--- a/src/event/ngx_event_openssl.c
+++ b/src/event/ngx_event_openssl.c
@@ -1839,6 +1839,10 @@ ngx_ssl_recv_early(ngx_connection_t *c, 
         buf += 1;
     }
 
+    if (c->ssl->write_blocked) {
+        return NGX_AGAIN;
+    }
+
     /*
      * SSL_read_early_data() may return data in parts, so try to read
      * until SSL_read_early_data() would return no data
@@ -2339,6 +2343,12 @@ ngx_ssl_write_early(ngx_connection_t *c,
             ngx_post_event(c->read, &ngx_posted_events);
         }
 
+        if (c->ssl->write_blocked) {
+            c->ssl->write_blocked = 0;
+
+            ngx_post_event(c->read, &ngx_posted_events);
+        }
+
         c->sent += written;
 
         return written;
@@ -2352,6 +2362,9 @@ ngx_ssl_write_early(ngx_connection_t *c,
 
     if (sslerr == SSL_ERROR_WANT_WRITE) {
 
+        ngx_log_debug0(NGX_LOG_DEBUG_EVENT, c->log, 0,
+                       "SSL_write_early_data: want write");
+
         if (c->ssl->saved_read_handler) {
 
             c->read->handler = c->ssl->saved_read_handler;
@@ -2366,6 +2379,15 @@ ngx_ssl_write_early(ngx_connection_t *c,
         }
 
         c->write->ready = 0;
+
+        /*
+         * OpenSSL 1.1.1a fails to handle SSL_read_early_data()
+         * if an SSL_write_early_data() call blocked on writing,
+         * see https://github.com/openssl/openssl/issues/7757
+         */
+
+        c->ssl->write_blocked = 1;
+
         return NGX_AGAIN;
     }
 
diff --git a/src/event/ngx_event_openssl.h b/src/event/ngx_event_openssl.h
--- a/src/event/ngx_event_openssl.h
+++ b/src/event/ngx_event_openssl.h
@@ -98,6 +98,7 @@ struct ngx_ssl_connection_s {
     unsigned                    try_early_data:1;
     unsigned                    in_early:1;
     unsigned                    early_preread:1;
+    unsigned                    write_blocked:1;
 };
 
 

-- 
Sergey Kandaurov