From 244aefd208e5578f79b7a8d10fa3267c79902473 Mon Sep 17 00:00:00 2001
From: Nick Travers <n.e.travers@gmail.com>
Date: Thu, 10 Sep 2020 17:10:27 -0700
Subject: [PATCH] filter: http: jwt: implement matching for HTTP CONNECT

Remove an existing TODO and implement the matcher, which mirrors the
behavior of the HTTP router, using the `RouteMatch` configuration to
determine whether the matcher should run for a given request.

This change allows JWT-based authentication for HTTP CONNECT requests.

Clean up a TODO in `uber_per_filter.cc` and remove some obsolete cleanup
code.

Signed-off-by: Nick Travers <n.e.travers@gmail.com>
---
 .../filters/http/jwt_authn/matcher.cc         | 21 ++++++++++---
 .../http/common/fuzz/uber_per_filter.cc       | 14 ---------
 .../filters/http/jwt_authn/matcher_test.cc    | 30 +++++++++++++++++++
 3 files changed, 47 insertions(+), 18 deletions(-)

diff --git a/source/extensions/filters/http/jwt_authn/matcher.cc b/source/extensions/filters/http/jwt_authn/matcher.cc
index 16061fb90196b..a1599dac53943 100644
--- a/source/extensions/filters/http/jwt_authn/matcher.cc
+++ b/source/extensions/filters/http/jwt_authn/matcher.cc
@@ -143,6 +143,22 @@ class RegexMatcherImpl : public BaseMatcherImpl {
   std::string regex_str_;
 };
 
+/**
+ * Perform a match against an HTTP CONNECT request.
+ */
+class ConnectMatcherImpl : public BaseMatcherImpl {
+public:
+  ConnectMatcherImpl(const RequirementRule& rule) : BaseMatcherImpl(rule) {}
+
+  bool matches(const Http::RequestHeaderMap& headers) const override {
+    if (Http::HeaderUtility::isConnect(headers) && BaseMatcherImpl::matchRoute(headers)) {
+      ENVOY_LOG(debug, "CONNECT requirement matched.");
+      return true;
+    }
+
+    return false;
+  }
+};
 } // namespace
 
 MatcherConstPtr Matcher::create(const RequirementRule& rule) {
@@ -155,10 +171,7 @@ MatcherConstPtr Matcher::create(const RequirementRule& rule) {
   case RouteMatch::PathSpecifierCase::kSafeRegex:
     return std::make_unique<RegexMatcherImpl>(rule);
   case RouteMatch::PathSpecifierCase::kConnectMatcher:
-    // TODO: When CONNECT match support is implemented, remove the manual clean-up of CONNECT
-    // matching in the filter fuzzer implementation:
-    // //test/extensions/filters/http/common/fuzz/uber_per_filter.cc
-    NOT_IMPLEMENTED_GCOVR_EXCL_LINE;
+    return std::make_unique<ConnectMatcherImpl>(rule);
   default:
     NOT_REACHED_GCOVR_EXCL_LINE;
   }
diff --git a/test/extensions/filters/http/common/fuzz/uber_per_filter.cc b/test/extensions/filters/http/common/fuzz/uber_per_filter.cc
index fde523d813ab5..c873b7bdd2da5 100644
--- a/test/extensions/filters/http/common/fuzz/uber_per_filter.cc
+++ b/test/extensions/filters/http/common/fuzz/uber_per_filter.cc
@@ -76,16 +76,6 @@ void UberFilterFuzzer::guideAnyProtoType(test::fuzz::HttpData* mutable_data, uin
   mutable_any->set_type_url(type_url);
 }
 
-void removeConnectMatcher(Protobuf::Message* message) {
-  envoy::extensions::filters::http::jwt_authn::v3::JwtAuthentication& config =
-      dynamic_cast<envoy::extensions::filters::http::jwt_authn::v3::JwtAuthentication&>(*message);
-  for (auto& rules : *config.mutable_rules()) {
-    if (rules.match().has_connect_matcher()) {
-      rules.mutable_match()->set_path("/");
-    }
-  }
-}
-
 void cleanAttachmentTemplate(Protobuf::Message* message) {
   envoy::extensions::filters::http::squash::v3::Squash& config =
       dynamic_cast<envoy::extensions::filters::http::squash::v3::Squash&>(*message);
@@ -138,10 +128,6 @@ void UberFilterFuzzer::cleanFuzzedConfig(absl::string_view filter_name,
     // TapDS oneof field and OutputSinkType StreamingGrpc not implemented
     cleanTapConfig(message);
   }
-  if (filter_name == HttpFilterNames::get().JwtAuthn) {
-    // Remove when connect matcher is implemented for Jwt Authentication filter.
-    removeConnectMatcher(message);
-  }
 }
 
 void UberFilterFuzzer::perFilterSetup() {
diff --git a/test/extensions/filters/http/jwt_authn/matcher_test.cc b/test/extensions/filters/http/jwt_authn/matcher_test.cc
index a7559685d8b51..e64245b6cfdba 100644
--- a/test/extensions/filters/http/jwt_authn/matcher_test.cc
+++ b/test/extensions/filters/http/jwt_authn/matcher_test.cc
@@ -162,6 +162,36 @@ TEST_F(MatcherTest, TestMatchPathAndHeader) {
   EXPECT_FALSE(matcher->matches(headers));
 }
 
+TEST_F(MatcherTest, TestMatchConnect) {
+  const char config[] = R"(match:
+  connect_matcher: {})";
+  RequirementRule rule;
+  TestUtility::loadFromYaml(config, rule);
+  MatcherConstPtr matcher = Matcher::create(rule);
+  auto headers = TestRequestHeaderMapImpl{{":method", "CONNECT"}};
+  EXPECT_TRUE(matcher->matches(headers));
+  headers = TestRequestHeaderMapImpl{{":method", "GET"}};
+  EXPECT_FALSE(matcher->matches(headers));
+}
+
+TEST_F(MatcherTest, TestMatchConnectQuery) {
+  const char config[] = R"(match:
+  connect_matcher: {}
+  query_parameters:
+  - name: foo
+    string_match:
+      exact: "bar")";
+  RequirementRule rule;
+  TestUtility::loadFromYaml(config, rule);
+  MatcherConstPtr matcher = Matcher::create(rule);
+  auto headers = TestRequestHeaderMapImpl{{":method", "CONNECT"}, {":path", "/boo?foo=bar"}};
+  EXPECT_TRUE(matcher->matches(headers));
+  headers = TestRequestHeaderMapImpl{{":method", "GET"}, {":path", "/boo?foo=bar"}};
+  EXPECT_FALSE(matcher->matches(headers));
+  headers = TestRequestHeaderMapImpl{{":method", "CONNECT"}, {":path", "/boo?ok=bye"}};
+  EXPECT_FALSE(matcher->matches(headers));
+}
+
 } // namespace
 } // namespace JwtAuthn
 } // namespace HttpFilters