From 441f1cc2823bfd58b016cdcbe6a10a97f8ce84b0 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 17 Jan 2015 19:33:30 +0900 Subject: [PATCH] nghttpx: Validate received response body length against content-length --- src/shrpx_downstream.cc | 33 ++++++++++++++++++++++++++++++--- src/shrpx_downstream.h | 12 ++++++++++-- src/shrpx_http2_session.cc | 25 +++++++++++++------------ src/shrpx_http2_upstream.cc | 11 ++++++++++- src/shrpx_https_upstream.cc | 4 ++++ src/shrpx_spdy_upstream.cc | 9 ++++++++- src/util.cc | 4 ++++ src/util.h | 1 + 8 files changed, 80 insertions(+), 19 deletions(-) diff --git a/src/shrpx_downstream.cc b/src/shrpx_downstream.cc index 06d4826a..b00512a5 100644 --- a/src/shrpx_downstream.cc +++ b/src/shrpx_downstream.cc @@ -109,9 +109,10 @@ Downstream::Downstream(Upstream *upstream, int32_t stream_id, int32_t priority) : request_buf_(upstream ? upstream->get_mcpool() : nullptr), response_buf_(upstream ? upstream->get_mcpool() : nullptr), request_bodylen_(0), response_bodylen_(0), response_sent_bodylen_(0), - upstream_(upstream), request_headers_sum_(0), response_headers_sum_(0), - request_datalen_(0), response_datalen_(0), stream_id_(stream_id), - priority_(priority), downstream_stream_id_(-1), + response_content_length_(-1), upstream_(upstream), + request_headers_sum_(0), response_headers_sum_(0), request_datalen_(0), + response_datalen_(0), stream_id_(stream_id), priority_(priority), + downstream_stream_id_(-1), response_rst_stream_error_code_(NGHTTP2_NO_ERROR), request_state_(INITIAL), request_major_(1), request_minor_(1), response_state_(INITIAL), response_http_status_(0), response_major_(1), @@ -663,6 +664,24 @@ int64_t Downstream::get_response_sent_bodylen() const { return response_sent_bodylen_; } +void Downstream::set_response_content_length(int64_t len) { + response_content_length_ = len; +} + +bool Downstream::validate_response_bodylen() const { + if (!expect_response_body() || response_content_length_ == -1) { + return true; + } + + if (LOG_ENABLED(INFO)) { + DLOG(INFO, this) << "response invalid bodylen: content-length=" + << response_content_length_ + << ", received=" << response_bodylen_; + } + + return response_content_length_ == response_bodylen_; +} + void Downstream::set_priority(int32_t pri) { priority_ = pri; } int32_t Downstream::get_priority() const { return priority_; } @@ -719,6 +738,14 @@ void Downstream::inspect_http1_response() { util::strifind(response_headers_[idx].value.c_str(), "chunked")) { chunked_response_ = true; } + + idx = response_hdidx_[http2::HD_CONTENT_LENGTH]; + if (idx != -1) { + auto len = util::parse_uint(response_headers_[idx].value); + if (len != -1) { + response_content_length_ = len; + } + } } void Downstream::reset_response() { diff --git a/src/shrpx_downstream.h b/src/shrpx_downstream.h index 49a37cfd..a976a6eb 100644 --- a/src/shrpx_downstream.h +++ b/src/shrpx_downstream.h @@ -218,6 +218,10 @@ public: int64_t get_response_bodylen() const; void add_response_sent_bodylen(size_t amount); int64_t get_response_sent_bodylen() const; + void set_response_content_length(int64_t len); + // Validates that received response body length and content-length + // matches. + bool validate_response_bodylen() const; uint32_t get_response_rst_stream_error_code() const; void set_response_rst_stream_error_code(uint32_t error_code); // Inspects HTTP/1 response. This checks tranfer-encoding etc. @@ -297,13 +301,17 @@ private: ev_timer downstream_rtimer_; ev_timer downstream_wtimer_; - // the length of request body + // the length of request body received so far int64_t request_bodylen_; - // the length of response body + // the length of response body received so far int64_t response_bodylen_; + // the length of response body sent to upstream client int64_t response_sent_bodylen_; + // content-length of response body, -1 if it is unknown. + int64_t response_content_length_; + Upstream *upstream_; std::unique_ptr dconn_; diff --git a/src/shrpx_http2_session.cc b/src/shrpx_http2_session.cc index 13ee960f..4adf09f5 100644 --- a/src/shrpx_http2_session.cc +++ b/src/shrpx_http2_session.cc @@ -817,23 +817,24 @@ int on_response_headers(Http2Session *http2session, Downstream *downstream, return 0; } - auto content_length = + auto content_length_hd = downstream->get_response_header(http2::HD_CONTENT_LENGTH); - if (!content_length && downstream->get_request_method() != "HEAD" && - downstream->get_request_method() != "CONNECT") { - unsigned int status; - status = downstream->get_response_http_status(); - if (!((100 <= status && status <= 199) || status == 204 || status == 304)) { - // Here we have response body but Content-Length is not known - // in advance. + + if (content_length_hd) { + auto content_length = util::parse_uint(content_length_hd->value); + if (content_length != -1) { + downstream->set_response_content_length(content_length); + } else if (downstream->expect_response_body()) { + // Here we have response body but Content-Length is not known in + // advance. if (downstream->get_request_major() <= 0 || downstream->get_request_minor() <= 0) { // We simply close connection for pre-HTTP/1.1 in this case. downstream->set_response_connection_close(true); - } else { - // Otherwise, use chunked encoding to keep upstream - // connection open. In HTTP2, we are supporsed not to - // receive transfer-encoding. + } else if (downstream->get_request_method() != "CONNECT") { + // Otherwise, use chunked encoding to keep upstream connection + // open. In HTTP2, we are supporsed not to receive + // transfer-encoding. downstream->add_response_header("transfer-encoding", "chunked"); downstream->set_chunked_response(true); } diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index c20c4d08..2c6ee311 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -526,7 +526,9 @@ int on_frame_not_send_callback(nghttp2_session *session, << ", lib_error_code=" << lib_error_code << ":" << nghttp2_strerror(lib_error_code); if (frame->hd.type == NGHTTP2_HEADERS && - frame->headers.cat == NGHTTP2_HCAT_RESPONSE) { + frame->headers.cat == NGHTTP2_HCAT_RESPONSE && + lib_error_code != NGHTTP2_ERR_STREAM_CLOSED && + lib_error_code != NGHTTP2_ERR_STREAM_CLOSING) { // To avoid stream hanging around, issue RST_STREAM. auto downstream = upstream->find_downstream(frame->hd.stream_id); if (downstream) { @@ -1207,6 +1209,13 @@ int Http2Upstream::on_downstream_body_complete(Downstream *downstream) { if (LOG_ENABLED(INFO)) { DLOG(INFO, downstream) << "HTTP response completed"; } + + if (!downstream->validate_response_bodylen()) { + rst_stream(downstream, NGHTTP2_PROTOCOL_ERROR); + downstream->set_response_connection_close(true); + return 0; + } + nghttp2_session_resume_data(session_, downstream->get_stream_id()); downstream->ensure_upstream_wtimer(); diff --git a/src/shrpx_https_upstream.cc b/src/shrpx_https_upstream.cc index ee3c1367..f1489379 100644 --- a/src/shrpx_https_upstream.cc +++ b/src/shrpx_https_upstream.cc @@ -768,6 +768,10 @@ int HttpsUpstream::on_downstream_body_complete(Downstream *downstream) { DLOG(INFO, downstream) << "HTTP response completed"; } + if (!downstream->validate_response_bodylen()) { + downstream->set_response_connection_close(true); + } + if (downstream->get_request_connection_close() || downstream->get_response_connection_close()) { auto handler = get_client_handler(); diff --git a/src/shrpx_spdy_upstream.cc b/src/shrpx_spdy_upstream.cc index af206e05..313d6a52 100644 --- a/src/shrpx_spdy_upstream.cc +++ b/src/shrpx_spdy_upstream.cc @@ -336,7 +336,8 @@ void on_ctrl_not_send_callback(spdylay_session *session, ULOG(WARN, upstream) << "Failed to send control frame type=" << type << ", error_code=" << error_code << ":" << spdylay_strerror(error_code); - if (type == SPDYLAY_SYN_REPLY) { + if (type == SPDYLAY_SYN_REPLY && error_code != SPDYLAY_ERR_STREAM_CLOSED && + error_code != SPDYLAY_ERR_STREAM_CLOSING) { // To avoid stream hanging around, issue RST_STREAM. auto stream_id = frame->syn_reply.stream_id; auto downstream = upstream->find_downstream(stream_id); @@ -917,6 +918,12 @@ int SpdyUpstream::on_downstream_body_complete(Downstream *downstream) { DLOG(INFO, downstream) << "HTTP response completed"; } + if (!downstream->validate_response_bodylen()) { + rst_stream(downstream, SPDYLAY_PROTOCOL_ERROR); + downstream->set_response_connection_close(true); + return 0; + } + spdylay_session_resume_data(session_, downstream->get_stream_id()); downstream->ensure_upstream_wtimer(); diff --git a/src/util.cc b/src/util.cc index 8e1c558d..eafe13b9 100644 --- a/src/util.cc +++ b/src/util.cc @@ -954,6 +954,10 @@ int64_t parse_uint(const char *s) { return parse_uint(reinterpret_cast(s), strlen(s)); } +int64_t parse_uint(const std::string &s) { + return parse_uint(reinterpret_cast(s.c_str()), s.size()); +} + int64_t parse_uint(const uint8_t *s, size_t len) { if (len == 0) { return -1; diff --git a/src/util.h b/src/util.h index 89d0f42b..8f63fe37 100644 --- a/src/util.h +++ b/src/util.h @@ -506,6 +506,7 @@ int64_t parse_uint_with_unit(const char *s); // the parsed integer. If there is an error, returns -1. int64_t parse_uint(const char *s); int64_t parse_uint(const uint8_t *s, size_t len); +int64_t parse_uint(const std::string &s); } // namespace util