From c5cdb78a952162aec9d0f2c9341742bb0b208631 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 1 Apr 2017 15:24:12 +0900 Subject: [PATCH 1/9] nghttpx: Add TLSv1.3 0-RTT early data support --- src/shrpx_connection.cc | 129 +++++++++++++++++++++++++++++++++++++--- src/shrpx_connection.h | 8 +++ src/shrpx_rate_limit.cc | 5 +- src/shrpx_tls.cc | 7 +++ 4 files changed, 140 insertions(+), 9 deletions(-) diff --git a/src/shrpx_connection.cc b/src/shrpx_connection.cc index 2269c63f..0b3c0302 100644 --- a/src/shrpx_connection.cc +++ b/src/shrpx_connection.cc @@ -60,7 +60,8 @@ Connection::Connection(struct ev_loop *loop, int fd, SSL *ssl, IOCb readcb, TimerCb timeoutcb, void *data, size_t tls_dyn_rec_warmup_threshold, ev_tstamp tls_dyn_rec_idle_timeout, shrpx_proto proto) - : tls{DefaultMemchunks(mcpool), DefaultPeekMemchunks(mcpool)}, + : tls{DefaultMemchunks(mcpool), DefaultPeekMemchunks(mcpool), + DefaultMemchunks(mcpool)}, wlimit(loop, &wev, write_limit.rate, write_limit.burst), rlimit(loop, &rev, read_limit.rate, read_limit.burst, this), loop(loop), @@ -120,10 +121,11 @@ void Connection::disconnect() { tls.warmup_writelen = 0; tls.last_writelen = 0; tls.last_readlen = 0; - tls.handshake_state = 0; + tls.handshake_state = TLS_CONN_NORMAL; tls.initial_handshake_done = false; tls.reneg_started = false; tls.sct_requested = false; + tls.early_data_finish = false; } if (fd != -1) { @@ -141,7 +143,11 @@ void Connection::disconnect() { wlimit.stopw(); } -void Connection::prepare_client_handshake() { SSL_set_connect_state(tls.ssl); } +void Connection::prepare_client_handshake() { + SSL_set_connect_state(tls.ssl); + // This prevents SSL_read_early_data from being called. + tls.early_data_finish = true; +} void Connection::prepare_server_handshake() { SSL_set_accept_state(tls.ssl); @@ -327,8 +333,9 @@ int Connection::tls_handshake() { wlimit.stopw(); ev_timer_stop(loop, &wt); + std::array buf; + if (ev_is_active(&rev)) { - std::array buf; auto nread = read_clear(buf.data(), buf.size()); if (nread < 0) { if (LOG_ENABLED(INFO)) { @@ -381,9 +388,59 @@ int Connection::tls_handshake() { break; } + int rv; + ERR_clear_error(); - auto rv = SSL_do_handshake(tls.ssl); +#if OPENSSL_1_1_1_API + if (!tls.server_handshake || tls.early_data_finish) { + rv = SSL_do_handshake(tls.ssl); + } else { + for (;;) { + size_t nread; + + rv = SSL_read_early_data(tls.ssl, buf.data(), buf.size(), &nread); + if (rv == SSL_READ_EARLY_DATA_ERROR) { + // If we have early data, and server sends ServerHello, assume + // that handshake is completed in server side, and start + // processing request. If we don't exit handshake code here, + // server waits for EndOfEarlyData and Finished message from + // client, which voids the purpose of 0-RTT data. The left + // over of handshake is done through write_tls or read_tls. + rv = (tls.handshake_state == TLS_CONN_WRITE_STARTED || + tls.wbuf.rleft()) && + tls.earlybuf.rleft(); + break; + } + + if (LOG_ENABLED(INFO)) { + LOG(INFO) << "tls: read early data " << nread << " bytes"; + } + + tls.earlybuf.append(buf.data(), nread); + + if (rv == SSL_READ_EARLY_DATA_FINISH) { + if (LOG_ENABLED(INFO)) { + LOG(INFO) << "tls: read all early data; total " + << tls.earlybuf.rleft() << " bytes"; + } + tls.early_data_finish = true; + // The same reason stated above. + if ((tls.handshake_state == TLS_CONN_WRITE_STARTED || + tls.wbuf.rleft()) && + tls.earlybuf.rleft()) { + rv = 1; + } else { + ERR_clear_error(); + rv = SSL_do_handshake(tls.ssl); + } + break; + } + } + } +#else // !OPENSSL_1_1_1_API + rv = SSL_do_handshake(tls.ssl); +#endif // !OPENSSL_1_1_1_API if (rv <= 0) { auto err = SSL_get_error(tls.ssl, rv); @@ -621,7 +678,21 @@ ssize_t Connection::write_tls(const void *data, size_t len) { ERR_clear_error(); +#if OPENSSL_1_1_1_API + int rv; + if (SSL_is_init_finished(tls.ssl)) { + rv = SSL_write(tls.ssl, data, len); + } else { + size_t nwrite; + rv = SSL_write_early_data(tls.ssl, data, len, &nwrite); + // Use the same semantics with SSL_write. + if (rv == 1) { + rv = nwrite; + } + } +#else // !OPENSSL_1_1_1_API auto rv = SSL_write(tls.ssl, data, len); +#endif // !OPENSSL_1_1_1_API if (rv <= 0) { auto err = SSL_get_error(tls.ssl, rv); @@ -656,6 +727,52 @@ ssize_t Connection::write_tls(const void *data, size_t len) { } ssize_t Connection::read_tls(void *data, size_t len) { + ERR_clear_error(); + +#if OPENSSL_1_1_1_API + if (tls.earlybuf.rleft()) { + return tls.earlybuf.remove(data, len); + } + if (!tls.early_data_finish) { + // TLSv1.3 handshake is still going on. + size_t nread; + auto rv = SSL_read_early_data(tls.ssl, data, len, &nread); + if (rv == SSL_READ_EARLY_DATA_ERROR) { + auto err = SSL_get_error(tls.ssl, rv); + switch (err) { + case SSL_ERROR_WANT_READ: + case SSL_ERROR_WANT_WRITE: // TODO Probably not required. + return 0; + case SSL_ERROR_SSL: + if (LOG_ENABLED(INFO)) { + LOG(INFO) << "SSL_read: " + << ERR_error_string(ERR_get_error(), nullptr); + } + return SHRPX_ERR_NETWORK; + default: + if (LOG_ENABLED(INFO)) { + LOG(INFO) << "SSL_read: SSL_get_error returned " << err; + } + return SHRPX_ERR_NETWORK; + } + } + + if (LOG_ENABLED(INFO)) { + LOG(INFO) << "tls: read early data " << nread << " bytes"; + } + + if (rv == SSL_READ_EARLY_DATA_FINISH) { + if (LOG_ENABLED(INFO)) { + LOG(INFO) << "tls: read all early data"; + } + tls.early_data_finish = true; + // We may have stopped write watcher in write_tls. + wlimit.startw(); + } + return nread; + } +#endif // OPENSSL_1_1_1_API + // SSL_read requires the same arguments (buf pointer and its // length) on SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE. // rlimit_.avail() or rlimit_.avail() may return different length @@ -673,8 +790,6 @@ ssize_t Connection::read_tls(void *data, size_t len) { tls.last_readlen = 0; } - ERR_clear_error(); - auto rv = SSL_read(tls.ssl, data, len); if (rv <= 0) { diff --git a/src/shrpx_connection.h b/src/shrpx_connection.h index 441ff513..d9ca5c0f 100644 --- a/src/shrpx_connection.h +++ b/src/shrpx_connection.h @@ -56,6 +56,8 @@ enum { struct TLSConnection { DefaultMemchunks wbuf; DefaultPeekMemchunks rbuf; + // Stores TLSv1.3 early data. + DefaultMemchunks earlybuf; SSL *ssl; SSL_SESSION *cached_session; MemcachedRequest *cached_session_lookup_req; @@ -74,6 +76,12 @@ struct TLSConnection { // true if ssl is initialized as server, and client requested // signed_certificate_timestamp extension. bool sct_requested; + // true if TLSv1.3 early data has been completely received. Since + // SSL_read_early_data acts like SSL_do_handshake, this field may be + // true even if the negotiated TLS version is TLSv1.2 or earlier. + // This value is also true if this is client side connection for + // convenience. + bool early_data_finish; }; struct TCPHint { diff --git a/src/shrpx_rate_limit.cc b/src/shrpx_rate_limit.cc index 77a1fe22..f05ae64c 100644 --- a/src/shrpx_rate_limit.cc +++ b/src/shrpx_rate_limit.cc @@ -108,8 +108,9 @@ void RateLimit::stopw() { } void RateLimit::handle_tls_pending_read() { - if (!conn_ || !conn_->tls.ssl || - (SSL_pending(conn_->tls.ssl) == 0 && conn_->tls.rbuf.rleft() == 0)) { + if (!conn_ || !conn_->tls.ssl || !conn_->tls.initial_handshake_done || + (SSL_pending(conn_->tls.ssl) == 0 && conn_->tls.rbuf.rleft() == 0 && + conn_->tls.earlybuf.rleft() == 0)) { return; } diff --git a/src/shrpx_tls.cc b/src/shrpx_tls.cc index 74ca9e16..bbfd6cfc 100644 --- a/src/shrpx_tls.cc +++ b/src/shrpx_tls.cc @@ -517,6 +517,13 @@ int ticket_key_cb(SSL *ssl, unsigned char *key_name, unsigned char *iv, namespace { void info_callback(const SSL *ssl, int where, int ret) { +#ifdef TLS1_3_VERSION + // TLSv1.3 has no renegotiation. + if (SSL_version(ssl) == TLS1_3_VERSION) { + return; + } +#endif // TLS1_3_VERSION + // To mitigate possible DOS attack using lots of renegotiations, we // disable renegotiation. Since OpenSSL does not provide an easy way // to disable it, we check that renegotiation is started in this From b30f312a70971a3fc570a86f963be0bfe0412232 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 1 Apr 2017 22:15:26 +0900 Subject: [PATCH 2/9] Honor SSL_read semantics --- src/shrpx_connection.cc | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/src/shrpx_connection.cc b/src/shrpx_connection.cc index 0b3c0302..21d8b4fd 100644 --- a/src/shrpx_connection.cc +++ b/src/shrpx_connection.cc @@ -733,6 +733,26 @@ ssize_t Connection::read_tls(void *data, size_t len) { if (tls.earlybuf.rleft()) { return tls.earlybuf.remove(data, len); } +#endif // OPENSSL_1_1_1_API + + // SSL_read requires the same arguments (buf pointer and its + // length) on SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE. + // rlimit_.avail() or rlimit_.avail() may return different length + // than the length previously passed to SSL_read, which violates + // OpenSSL assumption. To avoid this, we keep last legnth passed + // to SSL_read to tls_last_readlen_ if SSL_read indicated I/O + // blocking. + if (tls.last_readlen == 0) { + len = std::min(len, rlimit.avail()); + if (len == 0) { + return 0; + } + } else { + len = tls.last_readlen; + tls.last_readlen = 0; + } + +#if OPENSSL_1_1_1_API if (!tls.early_data_finish) { // TLSv1.3 handshake is still going on. size_t nread; @@ -742,6 +762,7 @@ ssize_t Connection::read_tls(void *data, size_t len) { switch (err) { case SSL_ERROR_WANT_READ: case SSL_ERROR_WANT_WRITE: // TODO Probably not required. + tls.last_readlen = len; return 0; case SSL_ERROR_SSL: if (LOG_ENABLED(INFO)) { @@ -773,23 +794,6 @@ ssize_t Connection::read_tls(void *data, size_t len) { } #endif // OPENSSL_1_1_1_API - // SSL_read requires the same arguments (buf pointer and its - // length) on SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE. - // rlimit_.avail() or rlimit_.avail() may return different length - // than the length previously passed to SSL_read, which violates - // OpenSSL assumption. To avoid this, we keep last legnth passed - // to SSL_read to tls_last_readlen_ if SSL_read indicated I/O - // blocking. - if (tls.last_readlen == 0) { - len = std::min(len, rlimit.avail()); - if (len == 0) { - return 0; - } - } else { - len = tls.last_readlen; - tls.last_readlen = 0; - } - auto rv = SSL_read(tls.ssl, data, len); if (rv <= 0) { From 3992302432acfb4f300bb2497cdd4355080a824c Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 29 Apr 2017 22:29:41 +0900 Subject: [PATCH 3/9] Remove SSL_ERROR_WANT_WRITE handling --- src/shrpx_connection.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/shrpx_connection.cc b/src/shrpx_connection.cc index 21d8b4fd..53404848 100644 --- a/src/shrpx_connection.cc +++ b/src/shrpx_connection.cc @@ -761,7 +761,6 @@ ssize_t Connection::read_tls(void *data, size_t len) { auto err = SSL_get_error(tls.ssl, rv); switch (err) { case SSL_ERROR_WANT_READ: - case SSL_ERROR_WANT_WRITE: // TODO Probably not required. tls.last_readlen = len; return 0; case SSL_ERROR_SSL: From 2ab319c1375bd3a8a7d07faffa28fc4e9da00041 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 26 Nov 2017 10:32:46 +0900 Subject: [PATCH 4/9] Don't hide error code from openssl --- src/shrpx_connection.cc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/shrpx_connection.cc b/src/shrpx_connection.cc index 53404848..101435d4 100644 --- a/src/shrpx_connection.cc +++ b/src/shrpx_connection.cc @@ -407,9 +407,12 @@ int Connection::tls_handshake() { // server waits for EndOfEarlyData and Finished message from // client, which voids the purpose of 0-RTT data. The left // over of handshake is done through write_tls or read_tls. - rv = (tls.handshake_state == TLS_CONN_WRITE_STARTED || - tls.wbuf.rleft()) && - tls.earlybuf.rleft(); + if ((tls.handshake_state == TLS_CONN_WRITE_STARTED || + tls.wbuf.rleft()) && + tls.earlybuf.rleft()) { + rv = 1; + } + break; } From 770e44de4d31bfa3c3989306d087f14ce2f765ad Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 26 Nov 2017 10:56:39 +0900 Subject: [PATCH 5/9] Implement draft-ietf-httpbis-replay-02 nghttpx sends early-data header field when forwarding requests which are received in TLSv1.3 early data, and the TLS handshake is still in progress. --- src/http2.cc | 3 +++ src/shrpx_http2_downstream_connection.cc | 14 ++++++++++++-- src/shrpx_http_downstream_connection.cc | 9 +++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/http2.cc b/src/http2.cc index 6db59744..0402d9c0 100644 --- a/src/http2.cc +++ b/src/http2.cc @@ -107,6 +107,9 @@ StringRef get_reason_phrase(unsigned int status_code) { return StringRef::from_lit("Expectation Failed"); case 421: return StringRef::from_lit("Misdirected Request"); + case 425: + // https://tools.ietf.org/html/draft-ietf-httpbis-replay-02 + return StringRef::from_lit("Too Early"); case 426: return StringRef::from_lit("Upgrade Required"); case 428: diff --git a/src/shrpx_http2_downstream_connection.cc b/src/shrpx_http2_downstream_connection.cc index 7a2d5598..5e4e8952 100644 --- a/src/shrpx_http2_downstream_connection.cc +++ b/src/shrpx_http2_downstream_connection.cc @@ -41,6 +41,7 @@ #include "shrpx_log.h" #include "http2.h" #include "util.h" +#include "ssl_compat.h" using namespace nghttp2; @@ -271,7 +272,7 @@ int Http2DownstreamConnection::push_request_headers() { num_cookies = downstream_->count_crumble_request_cookie(); } - // 9 means: + // 10 means: // 1. :method // 2. :scheme // 3. :path @@ -281,8 +282,9 @@ int Http2DownstreamConnection::push_request_headers() { // 7. x-forwarded-proto (optional) // 8. te (optional) // 9. forwarded (optional) + // 10. early-data (optional) auto nva = std::vector(); - nva.reserve(req.fs.headers().size() + 9 + num_cookies + + nva.reserve(req.fs.headers().size() + 10 + num_cookies + httpconf.add_request_headers.size()); nva.push_back( @@ -333,6 +335,14 @@ int Http2DownstreamConnection::push_request_headers() { auto upstream = downstream_->get_upstream(); auto handler = upstream->get_client_handler(); +#if OPENSSL_1_1_1_API + auto conn = handler->get_connection(); + + if (!SSL_is_init_finished(conn->tls.ssl)) { + nva.push_back(http2::make_nv_ll("early-data", "1")); + } +#endif // OPENSSL_1_1_1_API + auto fwd = fwdconf.strip_incoming ? nullptr : req.fs.header(http2::HD_FORWARDED); diff --git a/src/shrpx_http_downstream_connection.cc b/src/shrpx_http_downstream_connection.cc index 29f01791..e78b868f 100644 --- a/src/shrpx_http_downstream_connection.cc +++ b/src/shrpx_http_downstream_connection.cc @@ -39,6 +39,7 @@ #include "shrpx_log.h" #include "http2.h" #include "util.h" +#include "ssl_compat.h" using namespace nghttp2; @@ -584,6 +585,14 @@ int HttpDownstreamConnection::push_request_headers() { auto upstream = downstream_->get_upstream(); auto handler = upstream->get_client_handler(); +#if OPENSSL_1_1_1_API + auto conn = handler->get_connection(); + + if (!SSL_is_init_finished(conn->tls.ssl)) { + buf->append("Early-Data: 1\r\n"); + } +#endif // OPENSSL_1_1_1_API + auto fwd = fwdconf.strip_incoming ? nullptr : req.fs.header(http2::HD_FORWARDED); From 47f6012407fda0a49e467344bfe35b73279c9654 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 26 Nov 2017 17:47:19 +0900 Subject: [PATCH 6/9] nghttpx: Add an option to postpone early data processing --- gennghttpxfun.py | 1 + src/shrpx.cc | 12 ++++++++++++ src/shrpx_config.cc | 9 +++++++++ src/shrpx_config.h | 6 ++++++ src/shrpx_connection.cc | 7 +++++-- 5 files changed, 33 insertions(+), 2 deletions(-) diff --git a/gennghttpxfun.py b/gennghttpxfun.py index 425ed74a..fecfafd5 100755 --- a/gennghttpxfun.py +++ b/gennghttpxfun.py @@ -170,6 +170,7 @@ OPTIONS = [ "no-verify-ocsp", "verify-client-tolerate-expired", "ignore-per-pattern-mruby-error", + "tls-postpone-early-data", ] LOGVARS = [ diff --git a/src/shrpx.cc b/src/shrpx.cc index be867a10..671adb65 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -2370,6 +2370,12 @@ SSL/TLS: HTTP/2. To use those cipher suites with HTTP/2, consider to use --client-no-http2-cipher-black-list option. But be aware its implications. + --tls-postpone-early-data + Postpone forwarding HTTP requests sent in early data, + including those sent in partially in it, until TLS + handshake finishes. This option must be used to + mitigate possible replay attack unless all backend + servers recognize "Early-Data" header field. HTTP/2: -c, --frontend-http2-max-concurrent-streams= @@ -3436,6 +3442,7 @@ int main(int argc, char **argv) { 160}, {SHRPX_OPT_IGNORE_PER_PATTERN_MRUBY_ERROR.c_str(), no_argument, &flag, 161}, + {SHRPX_OPT_TLS_POSTPONE_EARLY_DATA.c_str(), no_argument, &flag, 162}, {nullptr, 0, nullptr, 0}}; int option_index = 0; @@ -4207,6 +4214,11 @@ int main(int argc, char **argv) { cmdcfgs.emplace_back(SHRPX_OPT_IGNORE_PER_PATTERN_MRUBY_ERROR, StringRef::from_lit("yes")); break; + case 162: + // --tls-postpone-early-data + cmdcfgs.emplace_back(SHRPX_OPT_TLS_POSTPONE_EARLY_DATA, + StringRef::from_lit("yes")); + break; default: break; } diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index 503007c8..e3675dc9 100644 --- a/src/shrpx_config.cc +++ b/src/shrpx_config.cc @@ -2040,6 +2040,11 @@ int option_lookup_token(const char *name, size_t namelen) { break; case 23: switch (name[22]) { + case 'a': + if (util::strieq_l("tls-postpone-early-dat", name, 22)) { + return SHRPX_OPTID_TLS_POSTPONE_EARLY_DATA; + } + break; case 'e': if (util::strieq_l("client-private-key-fil", name, 22)) { return SHRPX_OPTID_CLIENT_PRIVATE_KEY_FILE; @@ -3590,6 +3595,10 @@ int parse_config(Config *config, int optid, const StringRef &opt, case SHRPX_OPTID_IGNORE_PER_PATTERN_MRUBY_ERROR: config->ignore_per_pattern_mruby_error = util::strieq_l("yes", optarg); + return 0; + case SHRPX_OPTID_TLS_POSTPONE_EARLY_DATA: + config->tls.postpone_early_data = util::strieq_l("yes", optarg); + return 0; case SHRPX_OPTID_CONF: LOG(WARN) << "conf: ignored"; diff --git a/src/shrpx_config.h b/src/shrpx_config.h index d4cb4d9d..f2fc4511 100644 --- a/src/shrpx_config.h +++ b/src/shrpx_config.h @@ -347,6 +347,8 @@ constexpr auto SHRPX_OPT_VERIFY_CLIENT_TOLERATE_EXPIRED = StringRef::from_lit("verify-client-tolerate-expired"); constexpr auto SHRPX_OPT_IGNORE_PER_PATTERN_MRUBY_ERROR = StringRef::from_lit("ignore-per-pattern-mruby-error"); +constexpr auto SHRPX_OPT_TLS_POSTPONE_EARLY_DATA = + StringRef::from_lit("tls-postpone-early-data"); constexpr size_t SHRPX_OBFUSCATED_NODE_LENGTH = 8; @@ -656,6 +658,9 @@ struct TLSConfig { int max_proto_version; bool insecure; bool no_http2_cipher_black_list; + // true if forwarding requests included in TLS early data should be + // postponed until TLS handshake finishes. + bool postpone_early_data; }; // custom error page @@ -1116,6 +1121,7 @@ enum { SHRPX_OPTID_TLS_DYN_REC_WARMUP_THRESHOLD, SHRPX_OPTID_TLS_MAX_PROTO_VERSION, SHRPX_OPTID_TLS_MIN_PROTO_VERSION, + SHRPX_OPTID_TLS_POSTPONE_EARLY_DATA, SHRPX_OPTID_TLS_PROTO_LIST, SHRPX_OPTID_TLS_SCT_DIR, SHRPX_OPTID_TLS_SESSION_CACHE_MEMCACHED, diff --git a/src/shrpx_connection.cc b/src/shrpx_connection.cc index 101435d4..3e5c67c3 100644 --- a/src/shrpx_connection.cc +++ b/src/shrpx_connection.cc @@ -396,6 +396,7 @@ int Connection::tls_handshake() { if (!tls.server_handshake || tls.early_data_finish) { rv = SSL_do_handshake(tls.ssl); } else { + auto &tlsconf = get_config()->tls; for (;;) { size_t nread; @@ -407,7 +408,8 @@ int Connection::tls_handshake() { // server waits for EndOfEarlyData and Finished message from // client, which voids the purpose of 0-RTT data. The left // over of handshake is done through write_tls or read_tls. - if ((tls.handshake_state == TLS_CONN_WRITE_STARTED || + if (!tlsconf.postpone_early_data && + (tls.handshake_state == TLS_CONN_WRITE_STARTED || tls.wbuf.rleft()) && tls.earlybuf.rleft()) { rv = 1; @@ -429,7 +431,8 @@ int Connection::tls_handshake() { } tls.early_data_finish = true; // The same reason stated above. - if ((tls.handshake_state == TLS_CONN_WRITE_STARTED || + if (!tlsconf.postpone_early_data && + (tls.handshake_state == TLS_CONN_WRITE_STARTED || tls.wbuf.rleft()) && tls.earlybuf.rleft()) { rv = 1; From 9f212587205aebd7dbaabbe3472527140d377d20 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 20 May 2018 22:54:59 +0900 Subject: [PATCH 7/9] Specify SSL_CTX_set_max_early_data and add an option to change max value --- gennghttpxfun.py | 1 + src/shrpx.cc | 11 +++++++++++ src/shrpx_config.cc | 8 ++++++++ src/shrpx_config.h | 5 +++++ src/shrpx_tls.cc | 8 ++++++++ 5 files changed, 33 insertions(+) diff --git a/gennghttpxfun.py b/gennghttpxfun.py index fecfafd5..e0d36391 100755 --- a/gennghttpxfun.py +++ b/gennghttpxfun.py @@ -171,6 +171,7 @@ OPTIONS = [ "verify-client-tolerate-expired", "ignore-per-pattern-mruby-error", "tls-postpone-early-data", + "tls-max-early-data", ] LOGVARS = [ diff --git a/src/shrpx.cc b/src/shrpx.cc index 671adb65..52a7b2af 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -1465,6 +1465,7 @@ void fill_default_config(Config *config) { tls::proto_version_from_string(DEFAULT_TLS_MIN_PROTO_VERSION); tlsconf.max_proto_version = tls::proto_version_from_string(DEFAULT_TLS_MAX_PROTO_VERSION); + tlsconf.max_early_data = 16_k; #if OPENSSL_1_1_API || defined(OPENSSL_IS_BORINGSSL) tlsconf.ecdh_curves = StringRef::from_lit("X25519:P-256:P-384:P-521"); #else // !OPENSSL_1_1_API && !defined(OPENSSL_IS_BORINGSSL) @@ -2376,6 +2377,11 @@ SSL/TLS: handshake finishes. This option must be used to mitigate possible replay attack unless all backend servers recognize "Early-Data" header field. + --tls-max-early-data= + Sets the maximum amount of 0-RTT data that server + accepts. + Default: )" + << util::utos_unit(config->tls.max_early_data) << R"( HTTP/2: -c, --frontend-http2-max-concurrent-streams= @@ -3443,6 +3449,7 @@ int main(int argc, char **argv) { {SHRPX_OPT_IGNORE_PER_PATTERN_MRUBY_ERROR.c_str(), no_argument, &flag, 161}, {SHRPX_OPT_TLS_POSTPONE_EARLY_DATA.c_str(), no_argument, &flag, 162}, + {SHRPX_OPT_TLS_MAX_EARLY_DATA.c_str(), required_argument, &flag, 163}, {nullptr, 0, nullptr, 0}}; int option_index = 0; @@ -4219,6 +4226,10 @@ int main(int argc, char **argv) { cmdcfgs.emplace_back(SHRPX_OPT_TLS_POSTPONE_EARLY_DATA, StringRef::from_lit("yes")); break; + case 163: + // --tls-max-early-data + cmdcfgs.emplace_back(SHRPX_OPT_TLS_MAX_EARLY_DATA, StringRef{optarg}); + break; default: break; } diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index e3675dc9..ddf2abdc 100644 --- a/src/shrpx_config.cc +++ b/src/shrpx_config.cc @@ -1883,6 +1883,11 @@ int option_lookup_token(const char *name, size_t namelen) { break; case 18: switch (name[17]) { + case 'a': + if (util::strieq_l("tls-max-early-dat", name, 17)) { + return SHRPX_OPTID_TLS_MAX_EARLY_DATA; + } + break; case 'r': if (util::strieq_l("add-request-heade", name, 17)) { return SHRPX_OPTID_ADD_REQUEST_HEADER; @@ -3600,6 +3605,9 @@ int parse_config(Config *config, int optid, const StringRef &opt, config->tls.postpone_early_data = util::strieq_l("yes", optarg); return 0; + case SHRPX_OPTID_TLS_MAX_EARLY_DATA: { + return parse_uint_with_unit(&config->tls.max_early_data, opt, optarg); + } case SHRPX_OPTID_CONF: LOG(WARN) << "conf: ignored"; diff --git a/src/shrpx_config.h b/src/shrpx_config.h index f2fc4511..c8987974 100644 --- a/src/shrpx_config.h +++ b/src/shrpx_config.h @@ -349,6 +349,8 @@ constexpr auto SHRPX_OPT_IGNORE_PER_PATTERN_MRUBY_ERROR = StringRef::from_lit("ignore-per-pattern-mruby-error"); constexpr auto SHRPX_OPT_TLS_POSTPONE_EARLY_DATA = StringRef::from_lit("tls-postpone-early-data"); +constexpr auto SHRPX_OPT_TLS_MAX_EARLY_DATA = + StringRef::from_lit("tls-max-early-data"); constexpr size_t SHRPX_OBFUSCATED_NODE_LENGTH = 8; @@ -652,6 +654,8 @@ struct TLSConfig { StringRef ciphers; StringRef ecdh_curves; StringRef cacert; + // The maximum amount of 0-RTT data that server accepts. + uint32_t max_early_data; // The minimum and maximum TLS version. These values are defined in // OpenSSL header file. int min_proto_version; @@ -1119,6 +1123,7 @@ enum { SHRPX_OPTID_SYSLOG_FACILITY, SHRPX_OPTID_TLS_DYN_REC_IDLE_TIMEOUT, SHRPX_OPTID_TLS_DYN_REC_WARMUP_THRESHOLD, + SHRPX_OPTID_TLS_MAX_EARLY_DATA, SHRPX_OPTID_TLS_MAX_PROTO_VERSION, SHRPX_OPTID_TLS_MIN_PROTO_VERSION, SHRPX_OPTID_TLS_POSTPONE_EARLY_DATA, diff --git a/src/shrpx_tls.cc b/src/shrpx_tls.cc index bbfd6cfc..1b701fdf 100644 --- a/src/shrpx_tls.cc +++ b/src/shrpx_tls.cc @@ -973,6 +973,14 @@ SSL_CTX *create_ssl_context(const char *private_key_file, const char *cert_file, } #endif // !LIBRESSL_IN_USE && OPENSSL_VERSION_NUMBER >= 0x10002000L +#if OPENSSL_1_1_1_API + if (SSL_CTX_set_max_early_data(ssl_ctx, tlsconf.max_early_data) != 1) { + LOG(FATAL) << "SSL_CTX_set_max_early_data failed: " + << ERR_error_string(ERR_get_error(), nullptr); + DIE(); + } +#endif // OPENSSL_1_1_1_API + #ifndef OPENSSL_NO_PSK SSL_CTX_set_psk_server_callback(ssl_ctx, psk_server_cb); #endif // !LIBRESSL_NO_PSK From b8eccec62dc7662d2c69aa129cfe5423c36ade5b Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 8 Sep 2018 19:10:59 +0900 Subject: [PATCH 8/9] nghttpx: Disable OpenSSL anti-replay --- src/shrpx_tls.cc | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/shrpx_tls.cc b/src/shrpx_tls.cc index 1b701fdf..c9ea5fc5 100644 --- a/src/shrpx_tls.cc +++ b/src/shrpx_tls.cc @@ -770,7 +770,17 @@ SSL_CTX *create_ssl_context(const char *private_key_file, const char *cert_file, (SSL_OP_ALL & ~SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS) | SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3 | SSL_OP_NO_COMPRESSION | SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION | SSL_OP_SINGLE_ECDH_USE | - SSL_OP_SINGLE_DH_USE | SSL_OP_CIPHER_SERVER_PREFERENCE; + SSL_OP_SINGLE_DH_USE | + SSL_OP_CIPHER_SERVER_PREFERENCE +#if OPENSSL_1_1_1_API + // The reason for disabling built-in anti-replay in OpenSSL is + // that it only works if client gets back to the same server. + // The freshness check described in + // https://tools.ietf.org/html/rfc8446#section-8.3 is still + // performed. + | SSL_OP_NO_ANTI_REPLAY +#endif // OPENSSL_1_1_1_API + ; auto config = mod_config(); auto &tlsconf = config->tls; From 9b03c64f68077fb54a68b4cae9fe35ca1d0a00ed Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 8 Sep 2018 19:22:30 +0900 Subject: [PATCH 9/9] nghttpx: Should postpone early data by default --- gennghttpxfun.py | 2 +- src/shrpx.cc | 19 ++++++++++--------- src/shrpx_config.cc | 14 +++++++------- src/shrpx_config.h | 12 ++++++------ src/shrpx_connection.cc | 4 ++-- 5 files changed, 26 insertions(+), 25 deletions(-) diff --git a/gennghttpxfun.py b/gennghttpxfun.py index e0d36391..344b4308 100755 --- a/gennghttpxfun.py +++ b/gennghttpxfun.py @@ -170,7 +170,7 @@ OPTIONS = [ "no-verify-ocsp", "verify-client-tolerate-expired", "ignore-per-pattern-mruby-error", - "tls-postpone-early-data", + "tls-no-postpone-early-data", "tls-max-early-data", ] diff --git a/src/shrpx.cc b/src/shrpx.cc index 52a7b2af..56819af3 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -2371,12 +2371,13 @@ SSL/TLS: HTTP/2. To use those cipher suites with HTTP/2, consider to use --client-no-http2-cipher-black-list option. But be aware its implications. - --tls-postpone-early-data - Postpone forwarding HTTP requests sent in early data, - including those sent in partially in it, until TLS - handshake finishes. This option must be used to - mitigate possible replay attack unless all backend - servers recognize "Early-Data" header field. + --tls-no-postpone-early-data + By default, nghttpx postpones forwarding HTTP requests + sent in early data, including those sent in partially in + it, until TLS handshake finishes. If all backend server + recognizes "Early-Data" header field, using this option + makes nghttpx not postpone forwarding request and get + full potential of 0-RTT data. --tls-max-early-data= Sets the maximum amount of 0-RTT data that server accepts. @@ -3448,7 +3449,7 @@ int main(int argc, char **argv) { 160}, {SHRPX_OPT_IGNORE_PER_PATTERN_MRUBY_ERROR.c_str(), no_argument, &flag, 161}, - {SHRPX_OPT_TLS_POSTPONE_EARLY_DATA.c_str(), no_argument, &flag, 162}, + {SHRPX_OPT_TLS_NO_POSTPONE_EARLY_DATA.c_str(), no_argument, &flag, 162}, {SHRPX_OPT_TLS_MAX_EARLY_DATA.c_str(), required_argument, &flag, 163}, {nullptr, 0, nullptr, 0}}; @@ -4222,8 +4223,8 @@ int main(int argc, char **argv) { StringRef::from_lit("yes")); break; case 162: - // --tls-postpone-early-data - cmdcfgs.emplace_back(SHRPX_OPT_TLS_POSTPONE_EARLY_DATA, + // --tls-no-postpone-early-data + cmdcfgs.emplace_back(SHRPX_OPT_TLS_NO_POSTPONE_EARLY_DATA, StringRef::from_lit("yes")); break; case 163: diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index ddf2abdc..336df42f 100644 --- a/src/shrpx_config.cc +++ b/src/shrpx_config.cc @@ -2045,11 +2045,6 @@ int option_lookup_token(const char *name, size_t namelen) { break; case 23: switch (name[22]) { - case 'a': - if (util::strieq_l("tls-postpone-early-dat", name, 22)) { - return SHRPX_OPTID_TLS_POSTPONE_EARLY_DATA; - } - break; case 'e': if (util::strieq_l("client-private-key-fil", name, 22)) { return SHRPX_OPTID_CLIENT_PRIVATE_KEY_FILE; @@ -2124,6 +2119,11 @@ int option_lookup_token(const char *name, size_t namelen) { break; case 26: switch (name[25]) { + case 'a': + if (util::strieq_l("tls-no-postpone-early-dat", name, 25)) { + return SHRPX_OPTID_TLS_NO_POSTPONE_EARLY_DATA; + } + break; case 'e': if (util::strieq_l("frontend-http2-window-siz", name, 25)) { return SHRPX_OPTID_FRONTEND_HTTP2_WINDOW_SIZE; @@ -3601,8 +3601,8 @@ int parse_config(Config *config, int optid, const StringRef &opt, config->ignore_per_pattern_mruby_error = util::strieq_l("yes", optarg); return 0; - case SHRPX_OPTID_TLS_POSTPONE_EARLY_DATA: - config->tls.postpone_early_data = util::strieq_l("yes", optarg); + case SHRPX_OPTID_TLS_NO_POSTPONE_EARLY_DATA: + config->tls.no_postpone_early_data = util::strieq_l("yes", optarg); return 0; case SHRPX_OPTID_TLS_MAX_EARLY_DATA: { diff --git a/src/shrpx_config.h b/src/shrpx_config.h index c8987974..2dd35b0b 100644 --- a/src/shrpx_config.h +++ b/src/shrpx_config.h @@ -347,8 +347,8 @@ constexpr auto SHRPX_OPT_VERIFY_CLIENT_TOLERATE_EXPIRED = StringRef::from_lit("verify-client-tolerate-expired"); constexpr auto SHRPX_OPT_IGNORE_PER_PATTERN_MRUBY_ERROR = StringRef::from_lit("ignore-per-pattern-mruby-error"); -constexpr auto SHRPX_OPT_TLS_POSTPONE_EARLY_DATA = - StringRef::from_lit("tls-postpone-early-data"); +constexpr auto SHRPX_OPT_TLS_NO_POSTPONE_EARLY_DATA = + StringRef::from_lit("tls-no-postpone-early-data"); constexpr auto SHRPX_OPT_TLS_MAX_EARLY_DATA = StringRef::from_lit("tls-max-early-data"); @@ -662,9 +662,9 @@ struct TLSConfig { int max_proto_version; bool insecure; bool no_http2_cipher_black_list; - // true if forwarding requests included in TLS early data should be - // postponed until TLS handshake finishes. - bool postpone_early_data; + // true if forwarding requests included in TLS early data should not + // be postponed until TLS handshake finishes. + bool no_postpone_early_data; }; // custom error page @@ -1126,7 +1126,7 @@ enum { SHRPX_OPTID_TLS_MAX_EARLY_DATA, SHRPX_OPTID_TLS_MAX_PROTO_VERSION, SHRPX_OPTID_TLS_MIN_PROTO_VERSION, - SHRPX_OPTID_TLS_POSTPONE_EARLY_DATA, + SHRPX_OPTID_TLS_NO_POSTPONE_EARLY_DATA, SHRPX_OPTID_TLS_PROTO_LIST, SHRPX_OPTID_TLS_SCT_DIR, SHRPX_OPTID_TLS_SESSION_CACHE_MEMCACHED, diff --git a/src/shrpx_connection.cc b/src/shrpx_connection.cc index 3e5c67c3..05572698 100644 --- a/src/shrpx_connection.cc +++ b/src/shrpx_connection.cc @@ -408,7 +408,7 @@ int Connection::tls_handshake() { // server waits for EndOfEarlyData and Finished message from // client, which voids the purpose of 0-RTT data. The left // over of handshake is done through write_tls or read_tls. - if (!tlsconf.postpone_early_data && + if (tlsconf.no_postpone_early_data && (tls.handshake_state == TLS_CONN_WRITE_STARTED || tls.wbuf.rleft()) && tls.earlybuf.rleft()) { @@ -431,7 +431,7 @@ int Connection::tls_handshake() { } tls.early_data_finish = true; // The same reason stated above. - if (!tlsconf.postpone_early_data && + if (tlsconf.no_postpone_early_data && (tls.handshake_state == TLS_CONN_WRITE_STARTED || tls.wbuf.rleft()) && tls.earlybuf.rleft()) {