diff --git a/src/HttpServer.cc b/src/HttpServer.cc index d2e43d57..579d4adf 100644 --- a/src/HttpServer.cc +++ b/src/HttpServer.cc @@ -1255,9 +1255,14 @@ int hd_on_frame_recv_callback } if(frame->headers.cat == NGHTTP2_HCAT_REQUEST) { + if(!http2::check_http2_request_pseudo_headers_without_sort + (stream->headers)) { + hd->submit_rst_stream(stream, NGHTTP2_PROTOCOL_ERROR); + return 0; + } http2::normalize_headers(stream->headers); - if(!http2::check_http2_request_headers(stream->headers)) { + if(!http2::check_http2_headers(stream->headers)) { hd->submit_rst_stream(stream, NGHTTP2_PROTOCOL_ERROR); return 0; } diff --git a/src/http2.cc b/src/http2.cc index 0d7cc591..901698fc 100644 --- a/src/http2.cc +++ b/src/http2.cc @@ -247,16 +247,19 @@ bool check_pseudo_headers(const Headers& nva, InputIterator allowed_first, InputIterator allowed_last) { + bool expect_no_pseudo_header = false; // strict checking for pseudo headers. for(auto& hd : nva) { auto c = hd.name.c_str()[0]; - if(c < ':') { + if(c != ':') { + expect_no_pseudo_header = true; continue; } - if(c > ':') { - break; + // Pseudo headers must come before normal headers + if(expect_no_pseudo_header) { + return false; } auto i = allowed_first; @@ -276,22 +279,14 @@ bool check_pseudo_headers(const Headers& nva, } } // namespace -bool check_http2_request_headers(const Headers& nva) +bool check_http2_request_pseudo_headers_without_sort(const Headers& nva) { - if(!check_http2_headers(nva)) { - return false; - } - return check_pseudo_headers(nva, REQUEST_PSEUDO_HD, REQUEST_PSEUDO_HD + REQUEST_PSEUDO_HDLEN); } -bool check_http2_response_headers(const Headers& nva) +bool check_http2_response_pseudo_headers_without_sort(const Headers& nva) { - if(!check_http2_headers(nva)) { - return false; - } - return check_pseudo_headers(nva, RESPONSE_PSEUDO_HD, RESPONSE_PSEUDO_HD + RESPONSE_PSEUDO_HDLEN); } diff --git a/src/http2.h b/src/http2.h index 7e1a4ff4..e88ef5ed 100644 --- a/src/http2.h +++ b/src/http2.h @@ -96,15 +96,15 @@ bool check_http2_allowed_header(const char *name); // contains such headers. bool check_http2_headers(const Headers& nva); -// Calls check_http2_headers() and also checks that |nva| only -// contains pseudo headers allowed in request. Returns true if all +// Checks that |nva| only contains pseudo headers allowed in request +// and pseudo headers come before normal headers. Returns true if all // checks passed. -bool check_http2_request_headers(const Headers& nva); +bool check_http2_request_pseudo_headers_without_sort(const Headers& nva); -// Calls check_http2_headers() and also checks that |nva| only -// contains pseudo headers allowed in response. Returns true if all +// Checks that |nva| only contains pseudo headers allowed in response +// and pseudo headers come before normal headers. Returns true if all // checks passed. -bool check_http2_response_headers(const Headers& nva); +bool check_http2_response_pseudo_headers_without_sort(const Headers& nva); bool name_less(const Headers::value_type& lhs, const Headers::value_type& rhs); diff --git a/src/http2_test.cc b/src/http2_test.cc index 63d7074a..b2953b09 100644 --- a/src/http2_test.cc +++ b/src/http2_test.cc @@ -99,14 +99,20 @@ void test_http2_check_http2_headers(void) { ":path", "3" }, { ":scheme", "4" } }; - CU_ASSERT(http2::check_http2_request_headers(nva4)); - CU_ASSERT(!http2::check_http2_response_headers(nva4)); + CU_ASSERT(http2::check_http2_request_pseudo_headers_without_sort(nva4)); + CU_ASSERT(!http2::check_http2_response_pseudo_headers_without_sort(nva4)); auto nva5 = Headers{ { ":status", "1" } }; - CU_ASSERT(!http2::check_http2_request_headers(nva5)); - CU_ASSERT(http2::check_http2_response_headers(nva5)); + CU_ASSERT(!http2::check_http2_request_pseudo_headers_without_sort(nva5)); + CU_ASSERT(http2::check_http2_response_pseudo_headers_without_sort(nva5)); + + auto nva6 = Headers{ + { "content-length", "1"}, + { ":authority", "2" }, + }; + CU_ASSERT(!http2::check_http2_request_pseudo_headers_without_sort(nva6)); } void test_http2_get_unique_header(void) diff --git a/src/shrpx_http2_session.cc b/src/shrpx_http2_session.cc index 9b983d7f..804fcc26 100644 --- a/src/shrpx_http2_session.cc +++ b/src/shrpx_http2_session.cc @@ -895,12 +895,22 @@ int on_response_headers(Http2Session *http2session, auto upstream = downstream->get_upstream(); + if(!http2::check_http2_response_pseudo_headers_without_sort + (downstream->get_response_headers())) { + + http2session->submit_rst_stream(frame->hd.stream_id, + NGHTTP2_PROTOCOL_ERROR); + downstream->set_response_state(Downstream::MSG_RESET); + call_downstream_readcb(http2session, downstream); + return 0; + } + downstream->normalize_response_headers(); auto& nva = downstream->get_response_headers(); downstream->set_expect_final_response(false); - if(!http2::check_http2_response_headers(nva)) { + if(!http2::check_http2_headers(nva)) { http2session->submit_rst_stream(frame->hd.stream_id, NGHTTP2_PROTOCOL_ERROR); downstream->set_response_state(Downstream::MSG_RESET); diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index b043b7ce..76526082 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -285,6 +285,13 @@ int on_request_headers(Http2Upstream *upstream, return 0; } + if(!http2::check_http2_request_pseudo_headers_without_sort + (downstream->get_request_headers())) { + + upstream->rst_stream(downstream, NGHTTP2_PROTOCOL_ERROR); + return 0; + } + downstream->normalize_request_headers(); auto& nva = downstream->get_request_headers(); @@ -302,7 +309,7 @@ int on_request_headers(Http2Upstream *upstream, http2::dump_nv(get_config()->http2_upstream_dump_request_header, nva); } - if(!http2::check_http2_request_headers(nva)) { + if(!http2::check_http2_headers(nva)) { if(upstream->error_reply(downstream, 400) != 0) { upstream->rst_stream(downstream, NGHTTP2_PROTOCOL_ERROR); }