nghttpd, nghttpx: Check that pseudo headers come before normal headers

src/HttpServer.cc

@@ -1168,6 +1168,18 @@ int on_header_callback(nghttp2_session *session,
   if(!http2::check_nv(name, namelen, value, valuelen)) {
     return 0;
   }
+
+  if(namelen > 0 && name[0] == ':') {
+    if((!stream->headers.empty() &&
+        stream->headers.back().name.c_str()[0] != ':') ||
+     !http2::check_http2_request_pseudo_header(name, namelen)) {
+
+      nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, frame->hd.stream_id,
+                                NGHTTP2_PROTOCOL_ERROR);
+      return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
+    }
+  }
+
   http2::add_header(stream->headers, name, namelen, value, valuelen,
                     flags & NGHTTP2_NV_FLAG_NO_INDEX);
   return 0;

src/http2.cc

@@ -249,59 +249,42 @@ bool check_http2_headers(const Headers& nva)
   return true;
 }
 
-namespace {
-template<typename InputIterator>
-bool check_pseudo_headers(const Headers& nva,
-                          InputIterator allowed_first,
-                          InputIterator allowed_last)
-{
-  // strict checking for pseudo headers.
-  for(auto& hd : nva) {
-    auto c = hd.name.c_str()[0];
-
-    if(c < ':') {
-      continue;
-    }
-
-    if(c > ':') {
-      break;
-    }
-
-    auto i = allowed_first;
-
-    for(; i != allowed_last; ++i) {
-      if(hd.name == *i) {
-        break;
-      }
-    }
-
-    if(i == allowed_last) {
-      return false;
-    }
-  }
-
-  return true;
-}
-} // namespace
-
 bool check_http2_request_headers(const Headers& nva)
 {
-  if(!check_http2_headers(nva)) {
+  return 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)
 {
-  if(!check_http2_headers(nva)) {
+  return check_http2_headers(nva);
-    return false;
+}
+
+namespace {
+template<typename InputIterator>
+bool check_pseudo_header(const uint8_t *name, size_t namelen,
+                         InputIterator allowed_first,
+                         InputIterator allowed_last)
+{
+  for(auto i = allowed_first; i != allowed_last; ++i) {
+    if(util::streq(*i, name, namelen)) {
+      return true;
+    }
   }
 
-  return check_pseudo_headers(nva, RESPONSE_PSEUDO_HD,
+  return false;
-                              RESPONSE_PSEUDO_HD + RESPONSE_PSEUDO_HDLEN);
+}
+} // namespace
+
+bool check_http2_request_pseudo_header(const uint8_t *name, size_t namelen)
+{
+  return check_pseudo_header(name, namelen, REQUEST_PSEUDO_HD,
+                             REQUEST_PSEUDO_HD + REQUEST_PSEUDO_HDLEN);
+}
+
+bool check_http2_response_pseudo_header(const uint8_t *name, size_t namelen)
+{
+  return check_pseudo_header(name, namelen, RESPONSE_PSEUDO_HD,
+                             RESPONSE_PSEUDO_HD + RESPONSE_PSEUDO_HDLEN);
 }
 
 void normalize_headers(Headers& nva)

src/http2.h

@@ -96,16 +96,18 @@ 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
+// Calls check_http2_headers()
-// contains pseudo headers allowed in request.  Returns true if all
-// checks passed.
 bool check_http2_request_headers(const Headers& nva);
 
-// Calls check_http2_headers() and also checks that |nva| only
+// Calls check_http2_headers()
-// contains pseudo headers allowed in response.  Returns true if all
-// checks passed.
 bool check_http2_response_headers(const Headers& nva);
 
+// Returns true if |name| is allowed pusedo header for request.
+bool check_http2_request_pseudo_header(const uint8_t *name, size_t namelen);
+
+// Returns true if |name| is allowed pusedo header for response.
+bool check_http2_response_pseudo_header(const uint8_t *name, size_t namelen);
+
 bool name_less(const Headers::value_type& lhs, const Headers::value_type& rhs);
 
 void normalize_headers(Headers& nva);

src/http2_test.cc

@@ -93,20 +93,17 @@ void test_http2_check_http2_headers(void)
   };
   CU_ASSERT(http2::check_http2_headers(nva3));
 
-  auto nva4 = Headers{
+  auto n1 = ":authority";
-    { ":authority", "1" },
+  auto n1u8 = reinterpret_cast<const uint8_t*>(n1);
-    { ":method", "2" },
-    { ":path", "3" },
-    { ":scheme", "4" }
-  };
-  CU_ASSERT(http2::check_http2_request_headers(nva4));
-  CU_ASSERT(!http2::check_http2_response_headers(nva4));
 
-  auto nva5 = Headers{
+  CU_ASSERT(http2::check_http2_request_pseudo_header(n1u8, strlen(n1)));
-    { ":status", "1" }
+  CU_ASSERT(!http2::check_http2_response_pseudo_header(n1u8, strlen(n1)));
-  };
+
-  CU_ASSERT(!http2::check_http2_request_headers(nva5));
+  auto n2 = ":status";
-  CU_ASSERT(http2::check_http2_response_headers(nva5));
+  auto n2u8 = reinterpret_cast<const uint8_t*>(n2);
+
+  CU_ASSERT(!http2::check_http2_request_pseudo_header(n2u8, strlen(n2)));
+  CU_ASSERT(http2::check_http2_response_pseudo_header(n2u8, strlen(n2)));
 }
 
 void test_http2_get_unique_header(void)

src/shrpx_downstream.cc

@@ -859,4 +859,25 @@ bool Downstream::expect_response_body() const
     response_http_status_ != 204;
 }
 
+namespace {
+bool pseudo_header_allowed(const Headers& headers)
+{
+  if(headers.empty()) {
+    return true;
+  }
+
+  return headers.back().name.c_str()[0] == ':';
+}
+} // namespace
+
+bool Downstream::request_pseudo_header_allowed() const
+{
+  return pseudo_header_allowed(request_headers_);
+}
+
+bool Downstream::response_pseudo_header_allowed() const
+{
+  return pseudo_header_allowed(response_headers_);
+}
+
 } // namespace shrpx

src/shrpx_downstream.h

@@ -144,6 +144,7 @@ public:
   int end_upload_data();
   size_t get_request_datalen() const;
   void reset_request_datalen();
+  bool request_pseudo_header_allowed() const;
   bool expect_response_body() const;
   enum {
     INITIAL,
@@ -217,6 +218,7 @@ public:
   void add_response_datalen(size_t len);
   size_t get_response_datalen() const;
   void reset_response_datalen();
+  bool response_pseudo_header_allowed() const;
 
   // Call this method when there is incoming data in downstream
   // connection.

src/shrpx_http2_session.cc

@@ -820,6 +820,7 @@ int on_header_callback(nghttp2_session *session,
                        uint8_t flags,
                        void *user_data)
 {
+  auto http2session = static_cast<Http2Session*>(user_data);
   auto sd = static_cast<StreamData*>
     (nghttp2_session_get_stream_user_data(session, frame->hd.stream_id));
   if(!sd || !sd->dconn) {
@@ -846,6 +847,17 @@ int on_header_callback(nghttp2_session *session,
   if(!http2::check_nv(name, namelen, value, valuelen)) {
     return 0;
   }
+
+  if(namelen > 0 && name[0] == ':') {
+    if(!downstream->response_pseudo_header_allowed() ||
+       !http2::check_http2_response_pseudo_header(name, namelen)) {
+
+      http2session->submit_rst_stream(frame->hd.stream_id,
+                                      NGHTTP2_PROTOCOL_ERROR);
+      return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
+    }
+  }
+
   downstream->split_add_response_header(name, namelen, value, valuelen,
                                         flags & NGHTTP2_NV_FLAG_NO_INDEX);
   return 0;

src/shrpx_http2_upstream.cc

@@ -233,8 +233,19 @@ int on_header_callback(nghttp2_session *session,
     return 0;
   }
   if(!http2::check_nv(name, namelen, value, valuelen)) {
+    // Simply discard name/value, as if it never happen.
     return 0;
   }
+
+  if(namelen > 0 && name[0] == ':') {
+    if(!downstream->request_pseudo_header_allowed() ||
+       !http2::check_http2_request_pseudo_header(name, namelen)) {
+
+      upstream->rst_stream(downstream, NGHTTP2_PROTOCOL_ERROR);
+      return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
+    }
+  }
+
   downstream->split_add_request_header(name, namelen, value, valuelen,
                                        flags & NGHTTP2_NV_FLAG_NO_INDEX);
   return 0;