route config: handle no host/path headers (CVE-2019-18838). (#76)

This can happen during certain early reply cases in the HCM
when an encoder filter tries to lookup the cached route/cluster.

Signed-off-by: Matt Klein <mklein@lyft.com>

source/common/router/config_impl.cc

@@ -388,6 +388,12 @@ bool RouteEntryImplBase::matchRoute(const Http::HeaderMap& headers,
                                     uint64_t random_value) const {
   bool matches = true;
 
+  // TODO(mattklein123): Currently all match types require a path header. When we support CONNECT
+  // we will need to figure out how to safely relax this.
+  if (headers.Path() == nullptr) {
+    return false;
+  }
+
   matches &= evaluateRuntimeMatch(random_value);
   if (!matches) {
     // No need to waste further cycles calculating a route match.
@@ -1070,6 +1076,11 @@ const VirtualHostImpl* RouteMatcher::findVirtualHost(const Http::HeaderMap& head
     return default_virtual_host_.get();
   }
 
+  // There may be no authority in early reply paths in the HTTP connection manager.
+  if (headers.Host() == nullptr) {
+    return nullptr;
+  }
+
   // TODO (@rshriram) Match Origin header in WebSocket
   // request with VHost, using wildcard match
   const std::string host =

test/common/router/config_impl_test.cc

@@ -648,6 +648,17 @@ TEST_F(RouteMatcherTest, TestRoutes) {
   NiceMock<Envoy::StreamInfo::MockStreamInfo> stream_info;
   TestConfigImpl config(parseRouteConfigurationFromV2Yaml(yaml), factory_context_, true);
 
+  // No host header, no x-forwarded-proto and no path header testing.
+  EXPECT_EQ(nullptr, config.route(Http::TestHeaderMapImpl{{":path", "/"}, {":method", "GET"}}, 0));
+  EXPECT_EQ(
+      nullptr,
+      config.route(
+          Http::TestHeaderMapImpl{{":authority", "foo"}, {":path", "/"}, {":method", "GET"}}, 0));
+  EXPECT_EQ(nullptr, config.route(Http::TestHeaderMapImpl{{":authority", "foo"},
+                                                          {":method", "CONNECT"},
+                                                          {"x-forwarded-proto", "http"}},
+                                  0));
+
   // Base routing testing.
   EXPECT_EQ("instant-server",
             config.route(genHeaders("api.lyft.com", "/", "GET"), 0)->routeEntry()->clusterName());

test/common/router/route_fuzz_test.cc

@@ -84,17 +84,6 @@ DEFINE_PROTO_FUZZER(const test::common::router::RouteTestCase& input) {
     ConfigImpl config(cleanRouteConfig(input.config()), factory_context,
                       ProtobufMessage::getNullValidationVisitor(), true);
     Http::TestHeaderMapImpl headers = Fuzz::fromHeaders(input.headers());
-    // It's a precondition of routing that {:authority, :path, x-forwarded-proto} headers exists,
-    // HCM enforces this.
-    if (headers.Host() == nullptr) {
-      headers.setHost("example.com");
-    }
-    if (headers.Path() == nullptr) {
-      headers.setPath("/");
-    }
-    if (headers.ForwardedProto() == nullptr) {
-      headers.setForwardedProto("http");
-    }
     auto route = config.route(headers, stream_info, input.random_value());
     if (route != nullptr && route->routeEntry() != nullptr) {
       route->routeEntry()->finalizeRequestHeaders(headers, stream_info, true);

test/extensions/filters/http/lua/lua_integration_test.cc

@@ -21,7 +21,7 @@ class LuaIntegrationTest : public testing::TestWithParam<Network::Address::IpVer
         new FakeUpstream(0, FakeHttpConnection::Type::HTTP1, version_, timeSystem()));
   }
 
-  void initializeFilter(const std::string& filter_config) {
+  void initializeFilter(const std::string& filter_config, const std::string& domain = "*") {
     config_helper_.addFilter(filter_config);
 
     config_helper_.addConfigModifier([](envoy::config::bootstrap::v2::Bootstrap& bootstrap) {
@@ -35,14 +35,15 @@ class LuaIntegrationTest : public testing::TestWithParam<Network::Address::IpVer
     });
 
     config_helper_.addConfigModifier(
-        [](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager&
-               hcm) {
+        [domain](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager&
+                     hcm) {
           hcm.mutable_route_config()
               ->mutable_virtual_hosts(0)
               ->mutable_routes(0)
               ->mutable_match()
               ->set_prefix("/test/long/url");
 
+          hcm.mutable_route_config()->mutable_virtual_hosts(0)->set_domains(0, domain);
           auto* new_route = hcm.mutable_route_config()->mutable_virtual_hosts(0)->add_routes();
           new_route->mutable_match()->set_prefix("/alt/route");
           new_route->mutable_route()->set_cluster("alt_cluster");
@@ -96,6 +97,32 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, LuaIntegrationTest,
                          testing::ValuesIn(TestEnvironment::getIpVersionsForTest()),
                          TestUtility::ipTestParamsToString);
 
+// Regression test for pulling route info during early local replies using the Lua filter
+// metadata() API. Covers both the upgrade required and no authority cases.
+TEST_P(LuaIntegrationTest, CallMetadataDuringLocalReply) {
+  const std::string FILTER_AND_CODE =
+      R"EOF(
+name: envoy.lua
+typed_config:
+  "@type": type.googleapis.com/envoy.config.filter.http.lua.v2.Lua
+  inline_code: |
+    function envoy_on_response(response_handle)
+      local metadata = response_handle:metadata():get("foo.bar")
+      if metadata == nil then
+      end
+    end
+)EOF";
+
+  initializeFilter(FILTER_AND_CODE, "foo");
+  std::string response;
+  sendRawHttpAndWaitForResponse(lookupPort("http"), "GET / HTTP/1.0\r\n\r\n", &response, true);
+  EXPECT_TRUE(response.find("HTTP/1.1 426 Upgrade Required\r\n") == 0);
+
+  response = "";
+  sendRawHttpAndWaitForResponse(lookupPort("http"), "GET / HTTP/1.1\r\n\r\n", &response, true);
+  EXPECT_TRUE(response.find("HTTP/1.1 400 Bad Request\r\n") == 0);
+}
+
 // Basic request and response.
 TEST_P(LuaIntegrationTest, RequestAndResponse) {
   const std::string FILTER_AND_CODE =