nghttpx, nghttpd: Check pseudo header fields come before normal header fields

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;
       }

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);
 }

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);
 

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)

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);

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);
     }