From 864789ca6578c98849ce2c491e56bcba5e6618b0 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Tue, 28 Jan 2014 01:17:54 +0900 Subject: [PATCH] nghttpx: Limit the maximum header block size (again) --- src/http2.cc | 15 -------------- src/http2.h | 4 ---- src/shrpx_downstream.cc | 20 ++++++++++++++++++ src/shrpx_downstream.h | 12 ++++++++--- src/shrpx_http2_session.cc | 11 +++++----- src/shrpx_http2_upstream.cc | 11 +++++----- src/shrpx_http_downstream_connection.cc | 14 +++++++++++++ src/shrpx_https_upstream.cc | 27 ++++++++++++++----------- 8 files changed, 68 insertions(+), 46 deletions(-) diff --git a/src/http2.cc b/src/http2.cc index 6ea829a9..f94f17b8 100644 --- a/src/http2.cc +++ b/src/http2.cc @@ -730,21 +730,6 @@ int check_nv(const uint8_t *name, size_t namelen, return 1; } -int handle_too_many_headers(nghttp2_session *session, int32_t stream_id) -{ - int rv; - rv = nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, stream_id, - NGHTTP2_INTERNAL_ERROR); - if(nghttp2_is_fatal(rv)) { - return rv; - } - rv = nghttp2_session_terminate_session(session, NGHTTP2_INTERNAL_ERROR); - if(nghttp2_is_fatal(rv)) { - return rv; - } - return 0; -} - } // namespace http2 } // namespace nghttp2 diff --git a/src/http2.h b/src/http2.h index f2cb4c53..3a177522 100644 --- a/src/http2.h +++ b/src/http2.h @@ -221,10 +221,6 @@ int check_header_value(const uint8_t* value, size_t len); int check_nv(const uint8_t *name, size_t namelen, const uint8_t *value, size_t valuelen); -// Handles the situation that incoming headers are too many. It is -// dealt with by issuing RST_STREAM and GOAWAY. -int handle_too_many_headers(nghttp2_session *session, int32_t stream_id); - } // namespace http2 } // namespace nghttp2 diff --git a/src/shrpx_downstream.cc b/src/shrpx_downstream.cc index 6d4c587e..bd48581c 100644 --- a/src/shrpx_downstream.cc +++ b/src/shrpx_downstream.cc @@ -258,12 +258,14 @@ void Downstream::concat_norm_request_headers() void Downstream::add_request_header(std::string name, std::string value) { request_header_key_prev_ = true; + request_headers_sum_ += name.size() + value.size(); request_headers_.emplace_back(std::move(name), std::move(value)); } void Downstream::set_last_request_header_value(std::string value) { request_header_key_prev_ = false; + request_headers_sum_ += value.size(); Headers::value_type &item = request_headers_.back(); item.second = std::move(value); check_transfer_encoding_chunked(&chunked_request_, item); @@ -274,6 +276,7 @@ void Downstream::split_add_request_header (const uint8_t *name, size_t namelen, const uint8_t *value, size_t valuelen) { + request_headers_sum_ += namelen + valuelen; http2::split_add_header(request_headers_, name, namelen, value, valuelen); } @@ -285,6 +288,7 @@ bool Downstream::get_request_header_key_prev() const void Downstream::append_last_request_header_key(const char *data, size_t len) { assert(request_header_key_prev_); + request_headers_sum_ += len; auto& item = request_headers_.back(); item.first.append(data, len); } @@ -292,10 +296,16 @@ void Downstream::append_last_request_header_key(const char *data, size_t len) void Downstream::append_last_request_header_value(const char *data, size_t len) { assert(!request_header_key_prev_); + request_headers_sum_ += len; auto& item = request_headers_.back(); item.second.append(data, len); } +size_t Downstream::get_request_headers_sum() const +{ + return request_headers_sum_; +} + void Downstream::set_request_method(std::string method) { request_method_ = std::move(method); @@ -512,6 +522,7 @@ void Downstream::rewrite_norm_location_response_header void Downstream::add_response_header(std::string name, std::string value) { response_header_key_prev_ = true; + response_headers_sum_ += name.size() + value.size(); response_headers_.emplace_back(std::move(name), std::move(value)); check_transfer_encoding_chunked(&chunked_response_, response_headers_.back()); @@ -520,6 +531,7 @@ void Downstream::add_response_header(std::string name, std::string value) void Downstream::set_last_response_header_value(std::string value) { response_header_key_prev_ = false; + response_headers_sum_ += value.size(); auto& item = response_headers_.back(); item.second = std::move(value); check_transfer_encoding_chunked(&chunked_response_, item); @@ -529,6 +541,7 @@ void Downstream::split_add_response_header (const uint8_t *name, size_t namelen, const uint8_t *value, size_t valuelen) { + response_headers_sum_ += namelen + valuelen; http2::split_add_header(response_headers_, name, namelen, value, valuelen); } @@ -540,6 +553,7 @@ bool Downstream::get_response_header_key_prev() const void Downstream::append_last_response_header_key(const char *data, size_t len) { assert(response_header_key_prev_); + response_headers_sum_ += len; auto& item = response_headers_.back(); item.first.append(data, len); } @@ -548,10 +562,16 @@ void Downstream::append_last_response_header_value(const char *data, size_t len) { assert(!response_header_key_prev_); + response_headers_sum_ += len; auto& item = response_headers_.back(); item.second.append(data, len); } +size_t Downstream::get_response_headers_sum() const +{ + return response_headers_sum_; +} + unsigned int Downstream::get_response_http_status() const { return response_http_status_; diff --git a/src/shrpx_downstream.h b/src/shrpx_downstream.h index fa6ca494..cbf4300d 100644 --- a/src/shrpx_downstream.h +++ b/src/shrpx_downstream.h @@ -109,6 +109,8 @@ public: void append_last_request_header_key(const char *data, size_t len); void append_last_request_header_value(const char *data, size_t len); + size_t get_request_headers_sum() const; + void set_request_method(std::string method); const std::string& get_request_method() const; void set_request_path(std::string path); @@ -175,6 +177,8 @@ public: void append_last_response_header_key(const char *data, size_t len); void append_last_response_header_value(const char *data, size_t len); + size_t get_response_headers_sum() const; + unsigned int get_response_http_status() const; void set_response_http_status(unsigned int status); void set_response_major(int major); @@ -200,9 +204,8 @@ public: // Change the priority of downstream int change_priority(int32_t pri); - // Maximum number of headers per HEADERS frame, including its - // following CONTINUATIONs. - static const size_t MAX_HEADERS = 128; + // Maximum buffer size for header name/value pairs. + static const size_t MAX_HEADERS_SUM = 32768; private: Headers request_headers_; Headers response_headers_; @@ -221,6 +224,9 @@ private: // body. nghttp2 library reads data from this in the callback. evbuffer *response_body_buf_; + size_t request_headers_sum_; + size_t response_headers_sum_; + int32_t stream_id_; int32_t priority_; // stream ID in backend connection diff --git a/src/shrpx_http2_session.cc b/src/shrpx_http2_session.cc index a97548fe..4b4f3a65 100644 --- a/src/shrpx_http2_session.cc +++ b/src/shrpx_http2_session.cc @@ -804,7 +804,6 @@ int on_header_callback(nghttp2_session *session, const uint8_t *value, size_t valuelen, void *user_data) { - int rv; if(frame->hd.type != NGHTTP2_HEADERS || frame->headers.cat != NGHTTP2_HCAT_RESPONSE) { return 0; @@ -818,12 +817,12 @@ int on_header_callback(nghttp2_session *session, if(!downstream) { return 0; } - if(downstream->get_request_headers().size() >= Downstream::MAX_HEADERS) { - rv = http2::handle_too_many_headers(session, frame->hd.stream_id); - if(nghttp2_is_fatal(rv)) { - return rv; + if(downstream->get_response_headers_sum() > Downstream::MAX_HEADERS_SUM) { + if(LOG_ENABLED(INFO)) { + DLOG(INFO, downstream) << "Too large header block size=" + << downstream->get_response_headers_sum(); } - return 0; + return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; } if(!http2::check_nv(name, namelen, value, valuelen)) { return 0; diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index 674718ed..42f4acf5 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -205,7 +205,6 @@ int on_header_callback(nghttp2_session *session, const uint8_t *value, size_t valuelen, void *user_data) { - int rv; if(frame->hd.type != NGHTTP2_HEADERS || frame->headers.cat != NGHTTP2_HCAT_REQUEST) { return 0; @@ -215,12 +214,12 @@ int on_header_callback(nghttp2_session *session, if(!downstream) { return 0; } - if(downstream->get_request_headers().size() >= Downstream::MAX_HEADERS) { - rv = http2::handle_too_many_headers(session, frame->hd.stream_id); - if(nghttp2_is_fatal(rv)) { - return rv; + if(downstream->get_request_headers_sum() > Downstream::MAX_HEADERS_SUM) { + if(LOG_ENABLED(INFO)) { + ULOG(INFO, upstream) << "Too large header block size=" + << downstream->get_request_headers_sum(); } - return 0; + return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; } if(!http2::check_nv(name, namelen, value, valuelen)) { return 0; diff --git a/src/shrpx_http_downstream_connection.cc b/src/shrpx_http_downstream_connection.cc index b4fb33b3..4479004f 100644 --- a/src/shrpx_http_downstream_connection.cc +++ b/src/shrpx_http_downstream_connection.cc @@ -431,6 +431,13 @@ int htp_hdr_keycb(http_parser *htp, const char *data, size_t len) } else { downstream->add_response_header(std::string(data, len), ""); } + if(downstream->get_response_headers_sum() > Downstream::MAX_HEADERS_SUM) { + if(LOG_ENABLED(INFO)) { + DLOG(INFO, downstream) << "Too large header block size=" + << downstream->get_response_headers_sum(); + } + return -1; + } return 0; } } // namespace @@ -444,6 +451,13 @@ int htp_hdr_valcb(http_parser *htp, const char *data, size_t len) } else { downstream->append_last_response_header_value(data, len); } + if(downstream->get_response_headers_sum() > Downstream::MAX_HEADERS_SUM) { + if(LOG_ENABLED(INFO)) { + DLOG(INFO, downstream) << "Too large header block size=" + << downstream->get_response_headers_sum(); + } + return -1; + } return 0; } } // namespace diff --git a/src/shrpx_https_upstream.cc b/src/shrpx_https_upstream.cc index 6cdcbacb..ddfcf122 100644 --- a/src/shrpx_https_upstream.cc +++ b/src/shrpx_https_upstream.cc @@ -45,7 +45,6 @@ namespace shrpx { namespace { const size_t OUTBUF_MAX_THRES = 64*1024; -const size_t SHRPX_HTTPS_MAX_HEADER_LENGTH = 64*1024; } // namespace HttpsUpstream::HttpsUpstream(ClientHandler *handler) @@ -102,6 +101,13 @@ int htp_hdr_keycb(http_parser *htp, const char *data, size_t len) } else { downstream->add_request_header(std::string(data, len), ""); } + if(downstream->get_request_headers_sum() > Downstream::MAX_HEADERS_SUM) { + if(LOG_ENABLED(INFO)) { + ULOG(INFO, upstream) << "Too large header block size=" + << downstream->get_request_headers_sum(); + } + return -1; + } return 0; } } // namespace @@ -116,6 +122,13 @@ int htp_hdr_valcb(http_parser *htp, const char *data, size_t len) } else { downstream->append_last_request_header_value(data, len); } + if(downstream->get_request_headers_sum() > Downstream::MAX_HEADERS_SUM) { + if(LOG_ENABLED(INFO)) { + ULOG(INFO, upstream) << "Too large header block size=" + << downstream->get_request_headers_sum(); + } + return -1; + } return 0; } } // namespace @@ -321,17 +334,7 @@ int HttpsUpstream::on_read() } else if(htperr == HPE_OK) { // downstream can be NULL here. if(downstream) { - if(downstream->get_request_state() == Downstream::INITIAL && - current_header_length_ > SHRPX_HTTPS_MAX_HEADER_LENGTH) { - ULOG(WARNING, this) << "Request Header too long:" - << current_header_length_ - << " bytes"; - handler->set_should_close_after_write(true); - pause_read(SHRPX_MSG_BLOCK); - if(error_reply(400) != 0) { - return -1; - } - } else if(downstream->get_output_buffer_full()) { + if(downstream->get_output_buffer_full()) { if(LOG_ENABLED(INFO)) { ULOG(INFO, this) << "Downstream output buffer is full"; }