Hi, The attached patches attempt to make early data behave well when there are multiple streams in one connection, ie with HTTP/2. The first one makes sure we can mix read and write for early data. With a single stream, we usually just read, and then write, with multiple streams, we can read, handle the request, write, and read again data for a different stream. The second patch makes wait_for_handshake work for HTTP/2, by making sure all the stream tasks are woken up once the handshake is done, in case some of the streams were waiting for it.
Regards, Olivier
>From cdb181d78466a1ce2be2b8b621231ba2086f4979 Mon Sep 17 00:00:00 2001 From: Olivier Houchard <[email protected]> Date: Thu, 23 Nov 2017 18:21:29 +0100 Subject: [PATCH 1/2] MINOR: ssl: Handle reading early data after writing better. It can happen that we want to read early data, write some, and then continue reading them. To do so, we can't reuse tmp_early_data to store the amount of data sent, so introduce a new member. If we read early data, then ssl_sock_to_buf() is now the only responsible for getting back to the handshake, to make sure we don't miss any early data. --- include/proto/connection.h | 1 + include/types/connection.h | 1 + src/ssl_sock.c | 23 ++++++----------------- 3 files changed, 8 insertions(+), 17 deletions(-) diff --git a/include/proto/connection.h b/include/proto/connection.h index 05a63fe6a..c68ae20ba 100644 --- a/include/proto/connection.h +++ b/include/proto/connection.h @@ -607,6 +607,7 @@ static inline void conn_init(struct connection *conn) conn->obj_type = OBJ_TYPE_CONN; conn->flags = CO_FL_NONE; conn->tmp_early_data = -1; + conn->sent_early_data = 0; conn->mux = NULL; conn->mux_ctx = NULL; conn->owner = NULL; diff --git a/include/types/connection.h b/include/types/connection.h index 88573b8a7..a9d04474d 100644 --- a/include/types/connection.h +++ b/include/types/connection.h @@ -373,6 +373,7 @@ struct connection { void *owner; /* pointer to the owner session for incoming connections, or NULL */ int xprt_st; /* transport layer state, initialized to zero */ int tmp_early_data; /* 1st byte of early data, if any */ + int sent_early_data; /* Amount of early data we sent so far */ union conn_handle handle; /* connection handle at the socket layer */ enum obj_type *target; /* the target to connect to (server, proxy, applet, ...) */ struct list list; /* attach point to various connection lists (idle, ...) */ diff --git a/src/ssl_sock.c b/src/ssl_sock.c index 24bb36877..28f770664 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -1377,7 +1377,7 @@ void ssl_sock_infocbk(const SSL *ssl, int where, int ret) if (where & SSL_CB_HANDSHAKE_START) { /* Disable renegotiation (CVE-2009-3555) */ - if ((conn->flags & (CO_FL_CONNECTED | CO_FL_EARLY_SSL_HS)) == CO_FL_CONNECTED) { + if ((conn->flags & (CO_FL_CONNECTED | CO_FL_EARLY_SSL_HS | CO_FL_EARLY_DATA)) == CO_FL_CONNECTED) { conn->flags |= CO_FL_ERROR; conn->err_code = CO_ER_SSL_RENEG; } @@ -5318,15 +5318,6 @@ static int ssl_sock_to_buf(struct connection *conn, struct buffer *buf, int coun /* let's realign the buffer to optimize I/O */ if (buffer_empty(buf)) { buf->p = buf->data; -#if (OPENSSL_VERSION_NUMBER >= 0x10101000L) - /* - * If we're done reading the early data, and we're using - * a new buffer, then we know for sure we're not tainted - * with early data anymore - */ - if ((conn->flags & (CO_FL_EARLY_SSL_HS |CO_FL_EARLY_DATA)) == CO_FL_EARLY_DATA) - conn->flags &= ~CO_FL_EARLY_DATA; -#endif } /* read the largest possible block. For this, we perform only one call @@ -5499,9 +5490,6 @@ static int ssl_sock_from_buf(struct connection *conn, struct buffer *buf, int fl if (!SSL_is_init_finished(conn->xprt_ctx)) { unsigned int max_early; - if (conn->tmp_early_data == -1) - conn->tmp_early_data = 0; - if (objt_listener(conn->target)) max_early = SSL_get_max_early_data(conn->xprt_ctx); else { @@ -5511,17 +5499,18 @@ static int ssl_sock_from_buf(struct connection *conn, struct buffer *buf, int fl max_early = 0; } - if (try + conn->tmp_early_data > max_early) { - try -= (try + conn->tmp_early_data) - max_early; + if (try + conn->sent_early_data > max_early) { + try -= (try + conn->sent_early_data) - max_early; if (try <= 0) { - conn->flags |= CO_FL_SSL_WAIT_HS | CO_FL_WAIT_L6_CONN; + if (!(conn->flags & CO_FL_EARLY_SSL_HS)) + conn->flags |= CO_FL_SSL_WAIT_HS | CO_FL_WAIT_L6_CONN; break; } } ret = SSL_write_early_data(conn->xprt_ctx, bo_ptr(buf), try, &written_data); if (ret == 1) { ret = written_data; - conn->tmp_early_data += ret; + conn->sent_early_data += ret; if (objt_server(conn->target)) { conn->flags &= ~CO_FL_EARLY_SSL_HS; conn->flags |= CO_FL_SSL_WAIT_HS | CO_FL_WAIT_L6_CONN | CO_FL_EARLY_DATA; -- 2.14.3
>From 2ee1c6d6bf7c6e89011e2b8a9f54deb21aae0a00 Mon Sep 17 00:00:00 2001 From: Olivier Houchard <[email protected]> Date: Thu, 23 Nov 2017 18:25:47 +0100 Subject: [PATCH 2/2] MINOR: mux: Make sure every string is woken up after the handshake. In case any stream was waiting for the handshake after receiving early data, we have to wake all of them. Do so by making the mux responsible for removing the CO_FL_EARLY_DATA flag after all of them are woken up, instead of doing it in si_cs_wake_cb(), which would then only work for the first one. This makes wait_for_handshake work with HTTP/2. --- src/mux_h2.c | 9 +++++++++ src/mux_pt.c | 6 ++++++ src/stream_interface.c | 1 - 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/mux_h2.c b/src/mux_h2.c index eb8dd0ed4..3fb1bd29d 100644 --- a/src/mux_h2.c +++ b/src/mux_h2.c @@ -2086,6 +2086,15 @@ static int h2_wake(struct connection *conn) { struct h2c *h2c = conn->mux_ctx; + /* + * If we received early data, try to wake any stream, just in case + * at least one of them was waiting for the handshake + */ + if ((conn->flags & (CO_FL_EARLY_SSL_HS | CO_FL_EARLY_DATA | CO_FL_HANDSHAKE)) == + CO_FL_EARLY_DATA) { + h2_wake_some_streams(h2c, 0, 0); + conn->flags &= ~CO_FL_EARLY_DATA; + } if (conn->flags & CO_FL_ERROR || conn_xprt_read0_pending(conn) || h2c->st0 == H2_CS_ERROR2 || h2c->flags & H2_CF_GOAWAY_FAILED || (eb_is_empty(&h2c->streams_by_id) && h2c->last_sid >= 0 && diff --git a/src/mux_pt.c b/src/mux_pt.c index cd3a28b42..71bb47775 100644 --- a/src/mux_pt.c +++ b/src/mux_pt.c @@ -51,6 +51,12 @@ static int mux_pt_wake(struct connection *conn) ret = cs->data_cb->wake ? cs->data_cb->wake(cs) : 0; + /* If we had early data, and we're done with the handshake + * then whe know the data are safe, and we can remove the flag. + */ + if ((conn->flags & (CO_FL_EARLY_DATA | CO_FL_EARLY_SSL_HS | CO_FL_HANDSHAKE)) == + CO_FL_EARLY_DATA) + conn->flags &= ~CO_FL_EARLY_DATA; if (ret >= 0) cs_update_mux_polling(cs); return ret; diff --git a/src/stream_interface.c b/src/stream_interface.c index c2e3fffd4..6f9f27b98 100644 --- a/src/stream_interface.c +++ b/src/stream_interface.c @@ -585,7 +585,6 @@ static int si_cs_wake_cb(struct conn_stream *cs) * the handshake. */ if ((conn->flags & (CO_FL_EARLY_DATA | CO_FL_EARLY_SSL_HS)) == CO_FL_EARLY_DATA) { - conn->flags &= ~CO_FL_EARLY_DATA; task_wakeup(si_task(si), TASK_WOKEN_MSG); } -- 2.14.3
