extensions: make jwt_authn filter token-parsing more flexible (#6355)

api/envoy/config/filter/http/jwt_authn/v2alpha/README.md

@@ -29,3 +29,38 @@ If a custom location is desired, `from_headers` or `from_params` can be used to
 ## HTTP header to pass successfully verified JWT
 
 If a JWT is valid, its payload will be passed to the backend in a new HTTP header specified in `forward_payload_header` field. Its value is base64 encoded JWT payload in JSON.
+
+
+## Further header options
+
+In addition to the `name` field, which specifies the HTTP header name,
+the `from_headers` section can specify an optional `value_prefix` value, as in:
+
+```yaml
+    from_headers:
+      - name: bespoke
+        value_prefix: jwt_value
+```
+
+The above will cause the jwt_authn filter to look for the JWT in the `bespoke` header, following the tag `jwt_value`.
+
+Any non-JWT characters (i.e., anything _other than_ alphanumerics, `_`, `-`, and `.`) will be skipped,
+and all following, contiguous, JWT-legal chars will be taken as the JWT.
+
+This means all of the following will return a JWT of `eyJFbnZveSI6ICJyb2NrcyJ9.e30.c2lnbmVk`:
+
+```text
+bespoke: jwt_value=eyJFbnZveSI6ICJyb2NrcyJ9.e30.c2lnbmVk
+
+bespoke: {"jwt_value": "eyJFbnZveSI6ICJyb2NrcyJ9.e30.c2lnbmVk"}
+
+bespoke: beta:true,jwt_value:"eyJFbnZveSI6ICJyb2NrcyJ9.e30.c2lnbmVk",trace=1234
+```
+
+The header `name` may be `Authorization`.
+
+The `value_prefix` must match exactly, i.e., case-sensitively.
+If the `value_prefix` is not found, the header is skipped: not considered as a source for a JWT token.
+
+If there are no JWT-legal characters after the `value_prefix`, the entire string after it
+is taken to be the JWT token. This is unlikely to succeed; the error will reported by the JWT parser.

docs/root/intro/version_history.rst

@@ -50,6 +50,7 @@ Version history
 * http: added :ref:`max request headers size <envoy_api_field_config.filter.network.http_connection_manager.v2.HttpConnectionManager.max_request_headers_kb>`. The default behaviour is unchanged.
 * http: added modifyDecodingBuffer/modifyEncodingBuffer to allow modifying the buffered request/response data.
 * http: added encodeComplete/decodeComplete. These are invoked at the end of the stream, after all data has been encoded/decoded respectively. Default implementation is a no-op.
+* jwt_authn: make filter's parsing of JWT more flexible, allowing syntax like ``jwt=eyJhbGciOiJS...ZFnFIw,extra=7,realm=123``
 * outlier_detection: added support for :ref:`outlier detection event protobuf-based logging <arch_overview_outlier_detection_logging>`.
 * mysql: added a MySQL proxy filter that is capable of parsing SQL queries over MySQL wire protocol. Refer to :ref:`MySQL proxy<config_network_filters_mysql_proxy>` for more details.
 * performance: new buffer implementation (disabled by default; to test it, add "--use-libevent-buffers 0" to the command-line arguments when starting Envoy).

source/extensions/filters/http/jwt_authn/extractor.cc

@@ -102,6 +102,10 @@ class ExtractorImpl : public Extractor {
   // ctor helper for a jwt provider config
   void addProvider(const JwtProvider& provider);
 
+  // @return what should be the 3-part base64url-encoded substring; see RFC-7519
+  absl::string_view extractJWT(absl::string_view value_str,
+                               absl::string_view::size_type after) const;
+
   // HeaderMap value type to store prefix and issuers that specified this
   // header.
   struct HeaderLocationSpec {
@@ -181,11 +185,12 @@ std::vector<JwtLocationConstPtr> ExtractorImpl::extract(const Http::HeaderMap& h
     if (entry) {
       auto value_str = entry->value().getStringView();
       if (!location_spec->value_prefix_.empty()) {
-        if (!absl::StartsWith(value_str, location_spec->value_prefix_)) {
-          // prefix doesn't match, skip it.
+        const auto pos = value_str.find(location_spec->value_prefix_);
+        if (pos == absl::string_view::npos) {
+          // value_prefix not found anywhere in value_str, so skip
           continue;
         }
-        value_str = value_str.substr(location_spec->value_prefix_.size());
+        value_str = extractJWT(value_str, pos + location_spec->value_prefix_.length());
       }
       tokens.push_back(std::make_unique<const JwtHeaderLocation>(
           std::string(value_str), location_spec->specified_issuers_, location_spec->header_));
@@ -211,6 +216,36 @@ std::vector<JwtLocationConstPtr> ExtractorImpl::extract(const Http::HeaderMap& h
   return tokens;
 }
 
+// as specified in RFC-4648 § 5, plus dot (period, 0x2e), of which two are required in the JWT
+constexpr absl::string_view ConstantBase64UrlEncodingCharsPlusDot =
+    "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_.";
+
+// Returns a token, not a URL: skips non-Base64Url-legal (or dot) characters, collects following
+// Base64Url+dot string until first non-Base64Url char.
+//
+// The input parameters:
+//    "value_str" - the header value string, perhaps "Bearer string....", and
+//    "after" - the offset into that string after which to begin looking for JWT-legal characters
+//
+// For backwards compatibility, if it finds no suitable string, it returns value_str as-is.
+//
+// It is forgiving w.r.t. dots/periods, as the exact syntax will be verified after extraction.
+//
+// See RFC-7519 § 2, RFC-7515 § 2, and RFC-4648 "Base-N Encodings" § 5.
+absl::string_view ExtractorImpl::extractJWT(absl::string_view value_str,
+                                            absl::string_view::size_type after) const {
+  const auto starting = value_str.find_first_of(ConstantBase64UrlEncodingCharsPlusDot, after);
+  if (starting == value_str.npos) {
+    return value_str;
+  }
+  // There should be two dots (periods; 0x2e) inside the string, but we don't verify that here
+  auto ending = value_str.find_first_not_of(ConstantBase64UrlEncodingCharsPlusDot, starting);
+  if (ending == value_str.npos) { // Base64Url-encoded string occupies the rest of the line
+    return value_str.substr(starting);
+  }
+  return value_str.substr(starting, ending - starting);
+}
+
 void ExtractorImpl::sanitizePayloadHeaders(Http::HeaderMap& headers) const {
   for (const auto& header : forward_payload_headers_) {
     headers.remove(header);

test/extensions/filters/http/jwt_authn/BUILD

@@ -34,6 +34,7 @@ envoy_extension_cc_test(
     extension_name = "envoy.filters.http.jwt_authn",
     deps = [
         "//source/extensions/filters/http/jwt_authn:extractor_lib",
+        "//test/extensions/filters/http/jwt_authn:test_common_lib",
         "//test/test_common:utility_lib",
     ],
 )

test/extensions/filters/http/jwt_authn/extractor_test.cc

@@ -2,6 +2,7 @@
 
 #include "extensions/filters/http/jwt_authn/extractor.h"
 
+#include "test/extensions/filters/http/jwt_authn/test_common.h"
 #include "test/test_common/utility.h"
 
 using ::envoy::config::filter::http::jwt_authn::v2alpha::JwtAuthentication;
@@ -46,6 +47,16 @@ const char ExampleConfig[] = R"(
     from_headers:
       - name: prefix-header
         value_prefix: AAABBB
+  provider7:
+    issuer: issuer7
+    from_headers:
+      - name: prefix-header
+        value_prefix: CCCDDD
+  provider8:
+    issuer: issuer8
+    from_headers:
+      - name: prefix-header
+        value_prefix: '"CCCDDD"'
 )";
 
 class ExtractorTest : public testing::Test {
@@ -102,6 +113,19 @@ TEST_F(ExtractorTest, TestDefaultHeaderLocation) {
   EXPECT_FALSE(headers.Authorization());
 }
 
+// Test extracting JWT as Bearer token from the default header location: "Authorization" -
+// using an actual (correctly-formatted) JWT:
+TEST_F(ExtractorTest, TestDefaultHeaderLocationWithValidJWT) {
+  auto headers =
+      TestHeaderMapImpl{{absl::StrCat("Authorization"), absl::StrCat("Bearer ", GoodToken)}};
+  auto tokens = extractor_->extract(headers);
+  EXPECT_EQ(tokens.size(), 1);
+
+  // Only the issue1 is using default header location.
+  EXPECT_EQ(tokens[0]->token(), GoodToken);
+  EXPECT_TRUE(tokens[0]->isIssuerSpecified("issuer1"));
+}
+
 // Test extracting token from the default query parameter: "access_token"
 TEST_F(ExtractorTest, TestDefaultParamLocation) {
   auto headers = TestHeaderMapImpl{{":path", "/path?access_token=jwt_token"}};
@@ -172,6 +196,44 @@ TEST_F(ExtractorTest, TestPrefixHeaderMatch) {
   EXPECT_FALSE(headers.get(Http::LowerCaseString("prefix-header")));
 }
 
+// Test extracting token from the custom header: "prefix-header"
+// The value is found after the "CCCDDD", then between the '=' and the ','.
+TEST_F(ExtractorTest, TestPrefixHeaderFlexibleMatch1) {
+  auto headers = TestHeaderMapImpl{{"prefix-header", "preamble CCCDDD=jwt_token,extra=more"}};
+  auto tokens = extractor_->extract(headers);
+  EXPECT_EQ(tokens.size(), 1);
+
+  // Match issuer 7 with map key as: prefix-header + 'CCCDDD'
+  EXPECT_TRUE(tokens[0]->isIssuerSpecified("issuer7"));
+  EXPECT_EQ(tokens[0]->token(), "jwt_token");
+}
+
+TEST_F(ExtractorTest, TestPrefixHeaderFlexibleMatch2) {
+  auto headers =
+      TestHeaderMapImpl{{"prefix-header", "CCCDDD=\"and0X3Rva2Vu\",comment=\"fish tag\""}};
+  auto tokens = extractor_->extract(headers);
+  EXPECT_EQ(tokens.size(), 1);
+
+  // Match issuer 7 with map key as: prefix-header + AAA
+  EXPECT_TRUE(tokens[0]->isIssuerSpecified("issuer7"));
+  EXPECT_EQ(tokens[0]->token(), "and0X3Rva2Vu");
+}
+
+TEST_F(ExtractorTest, TestPrefixHeaderFlexibleMatch3) {
+  auto headers = TestHeaderMapImpl{
+      {"prefix-header", "creds={\"authLevel\": \"20\", \"CCCDDD\": \"and0X3Rva2Vu\"}"}};
+  auto tokens = extractor_->extract(headers);
+  EXPECT_EQ(tokens.size(), 2);
+
+  // Match issuer 8 with map key as: prefix-header + '"CCCDDD"'
+  EXPECT_TRUE(tokens[0]->isIssuerSpecified("issuer8"));
+  EXPECT_EQ(tokens[0]->token(), "and0X3Rva2Vu");
+
+  // Match issuer 7 with map key as: prefix-header + 'CCCDDD'
+  EXPECT_TRUE(tokens[1]->isIssuerSpecified("issuer7"));
+  EXPECT_EQ(tokens[1]->token(), "and0X3Rva2Vu");
+}
+
 // Test extracting token from the custom query parameter: "token_param"
 TEST_F(ExtractorTest, TestCustomParamToken) {
   auto headers = TestHeaderMapImpl{{":path", "/path?token_param=jwt_token"}};