Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jwt_authn: Support extraction of JWT from Cookies in JWT Extension #17721

Merged
merged 9 commits into from
Sep 3, 2021
15 changes: 14 additions & 1 deletion api/envoy/extensions/filters/http/jwt_authn/v3/config.proto
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
// cache_duration:
// seconds: 300
//
// [#next-free-field: 13]
// [#next-free-field: 14]
message JwtProvider {
option (udpa.annotations.versioning).previous_message_type =
"envoy.config.filter.http.jwt_authn.v2alpha.JwtProvider";
Expand Down Expand Up @@ -224,6 +224,19 @@ message JwtProvider {
// Enables JWT cache, its size is specified by *jwt_cache_size*.
// Only valid JWT tokens are cached.
JwtCacheConfig jwt_cache_config = 12;

// JWT is sent in a cookie. `from_cookies` represents the cookie names to extract from.
//
// For example, if config is:
//
// .. code-block:: yaml
//
// from_cookies:
// - auth-token
//
// Then JWT will be extracted from `auth-token` cookie in the request.
//
repeated string from_cookies = 13;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this block after from_params.

This doesn't have to be ordered by field tag.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks for pointing this out

}

// This message specifies JWT Cache configuration.
Expand Down
15 changes: 14 additions & 1 deletion api/envoy/extensions/filters/http/jwt_authn/v4alpha/config.proto
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO
// cache_duration:
// seconds: 300
//
// [#next-free-field: 13]
// [#next-free-field: 14]
message JwtProvider {
option (udpa.annotations.versioning).previous_message_type =
"envoy.extensions.filters.http.jwt_authn.v3.JwtProvider";
Expand Down Expand Up @@ -224,6 +224,19 @@ message JwtProvider {
// Enables JWT cache, its size is specified by *jwt_cache_size*.
// Only valid JWT tokens are cached.
JwtCacheConfig jwt_cache_config = 12;

// JWT is sent in a cookie. `from_cookies` represents the cookie names to extract from.
//
// For example, if config is:
//
// .. code-block:: yaml
//
// from_cookies:
// - auth-token
//
// Then JWT will be extracted from `auth-token` cookie in the request.
//
repeated string from_cookies = 13;
}

// This message specifies JWT Cache configuration.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ JwtProvider
* *forward*: if true, JWT will be forwarded to the upstream.
* *from_headers*: extract JWT from HTTP headers.
* *from_params*: extract JWT from query parameters.
* *from_cookies*: extract JWT from HTTP request cookies.
* *forward_payload_header*: forward the JWT payload in the specified HTTP header.
* *jwt_cache_config*: Enables JWT cache, its size can be specified by *jwt_cache_size*. Only valid JWT tokens are cached.

Expand Down
1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ New Features
* http: added :ref:`string_match <envoy_v3_api_field_config.route.v3.HeaderMatcher.string_match>` in the header matcher.
* http: added support for :ref:`max_requests_per_connection <envoy_v3_api_field_config.core.v3.HttpProtocolOptions.max_requests_per_connection>` for both upstream and downstream connections.
* jwt_authn: added support for :ref:`Jwt Cache <envoy_v3_api_field_extensions.filters.http.jwt_authn.v3.JwtProvider.jwt_cache_config>` and its size can be specified by :ref:`jwt_cache_size <envoy_v3_api_field_extensions.filters.http.jwt_authn.v3.JwtCacheConfig.jwt_cache_size>`.
* jwt_authn: added support for extracting JWTs from request cookies using :ref:`from_cookies <envoy_v3_api_field_extensions.filters.http.jwt_authn.v3.JwtProvider.from_cookies>`
* listener: new listener metric ``downstream_cx_transport_socket_connect_timeout`` to track transport socket timeouts.
* rbac: added :ref:`destination_port_range <envoy_v3_api_field_config.rbac.v3.Permission.destination_port_range>` for matching range of destination ports.
* thrift_proxy: added support for :ref:`mirroring requests <envoy_v3_api_field_extensions.filters.network.thrift_proxy.v3.RouteAction.request_mirror_policies>`.
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

33 changes: 33 additions & 0 deletions source/common/http/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,39 @@ std::string parseCookie(const HeaderMap& headers, const std::string& key,
return EMPTY_STRING;
}

std::map<std::string, std::string> Utility::parseCookies(const RequestHeaderMap& headers) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very similar to the existing code in source/common/http/utility.cc #L257-L282

Could not use it as-is because that'd scan over cookies multiple times. Any suggestions on merging them together ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps you could introduce something like void forEachCookie(const Http::HeaderMap::GetResult cookie_headers, std::function<bool(...)> fn) where fn is a closure dealing with a cookie and returning false to stop iteration.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion! Was able to get this working locally 👍

I would prefer to do this change in a follow-up PR. I hope that's okay.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, a follow-up would be Ok. Just put a TODO note to the function.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other places using parseCookieValue would be benefit from this logic if you see how oauth2 filter use it.

https://github.com/envoyproxy/envoy/blob/main/source/extensions/filters/http/oauth2/filter.cc#L141-L143

I would suggest you to land the refactoring before this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, I'll do that first

std::map<std::string, std::string> cookies;
const Http::HeaderMap::GetResult cookie_headers = headers.get(Http::Headers::get().Cookie);

for (size_t index = 0; index < cookie_headers.size(); index++) {
auto cookie_header_value = cookie_headers[index]->value().getStringView();

// Split the cookie header into individual cookies.
for (const auto& s : StringUtil::splitToken(cookie_header_value, ";")) {
// Find the key part of the cookie (i.e. the name of the cookie).
size_t first_non_space = s.find_first_not_of(' ');
size_t equals_index = s.find('=');
if (equals_index == absl::string_view::npos) {
// The cookie is malformed if it does not have an `=`. Continue
// checking other cookies in this header.
continue;
}
absl::string_view k = s.substr(first_non_space, equals_index - first_non_space);
absl::string_view v = s.substr(equals_index + 1, s.size() - 1);

// Cookie values may be wrapped in double quotes.
// https://tools.ietf.org/html/rfc6265#section-4.1.1
if (v.size() >= 2 && v.back() == '"' && v[0] == '"') {
v = v.substr(1, v.size() - 2);
}

cookies.emplace(std::string{k}, std::string{v});
}
}

return cookies;
}

bool Utility::Url::initialize(absl::string_view absolute_url, bool is_connect) {
struct http_parser_url u;
http_parser_url_init(&u);
Expand Down
7 changes: 7 additions & 0 deletions source/common/http/utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,13 @@ std::string stripQueryString(const HeaderString& path);
**/
std::string parseCookieValue(const HeaderMap& headers, const std::string& key);

/**
* Parse cookies from header into a map.
* @param headers supplies the headers to get cookies from.
* @return std::map cookie map.
**/
std::map<std::string, std::string> parseCookies(const RequestHeaderMap& headers);

/**
* Parse a particular value out of a set-cookie
* @param headers supplies the headers to get the set-cookie from.
Expand Down
67 changes: 55 additions & 12 deletions source/extensions/filters/http/jwt_authn/extractor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,17 @@ class JwtParamLocation : public JwtLocationBase {
}
};

// The JwtLocation for cookie extraction.
class JwtCookieLocation : public JwtLocationBase {
public:
JwtCookieLocation(const std::string& token, const JwtIssuerChecker& issuer_checker)
: JwtLocationBase(token, issuer_checker) {}

void removeJwt(Http::HeaderMap&) const override {
// TODO(theshubhamp): remove JWT from cookies.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use NOT_IMPLEMENTED_GCOVR_EXCL_LINE

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added below the comment

}
};

/**
* The class implements Extractor interface
*
Expand All @@ -133,6 +144,8 @@ class ExtractorImpl : public Logger::Loggable<Logger::Id::jwt>, public Extractor
const std::string& value_prefix);
// add a query param config
void addQueryParamConfig(const std::string& issuer, const std::string& param);
// add a query param config
void addCookieConfig(const std::string& issuer, const std::string& cookie);
// ctor helper for a jwt provider config
void addProvider(const JwtProvider& provider);

Expand Down Expand Up @@ -164,6 +177,14 @@ class ExtractorImpl : public Logger::Loggable<Logger::Id::jwt>, public Extractor
// The map of a parameter key to set of issuers specified the parameter
std::map<std::string, ParamLocationSpec> param_locations_;

// CookieMap value type to store issuers that specified this cookie.
struct CookieLocationSpec {
// Issuers that specified this param.
JwtIssuerChecker issuer_checker_;
};
// The map of a cookie key to set of issuers specified the cookie.
std::map<std::string, CookieLocationSpec> cookie_locations_;

std::vector<LowerCaseString> forward_payload_headers_;
};

Expand All @@ -183,6 +204,9 @@ void ExtractorImpl::addProvider(const JwtProvider& provider) {
for (const std::string& param : provider.from_params()) {
addQueryParamConfig(provider.issuer(), param);
}
for (const std::string& cookie : provider.from_cookies()) {
addCookieConfig(provider.issuer(), cookie);
}
// If not specified, use default locations.
if (provider.from_headers().empty() && provider.from_params().empty()) {
addHeaderConfig(provider.issuer(), Http::CustomHeaders::get().Authorization,
Expand Down Expand Up @@ -210,6 +234,11 @@ void ExtractorImpl::addQueryParamConfig(const std::string& issuer, const std::st
param_location_spec.issuer_checker_.add(issuer);
}

void ExtractorImpl::addCookieConfig(const std::string& issuer, const std::string& cookie) {
auto& cookie_location_spec = cookie_locations_[cookie];
cookie_location_spec.issuer_checker_.add(issuer);
}

std::vector<JwtLocationConstPtr>
ExtractorImpl::extract(const Http::RequestHeaderMap& headers) const {
std::vector<JwtLocationConstPtr> tokens;
Expand All @@ -235,22 +264,36 @@ ExtractorImpl::extract(const Http::RequestHeaderMap& headers) const {
}
}

// If no query parameter locations specified, or Path() is null, bail out
if (param_locations_.empty() || headers.Path() == nullptr) {
return tokens;
// Check query parameter locations only if query parameter locations specified and Path() is not
// null
if (!param_locations_.empty() && headers.Path() != nullptr) {
const auto& params = Http::Utility::parseAndDecodeQueryString(headers.getPathValue());
for (const auto& location_it : param_locations_) {
const auto& param_key = location_it.first;
const auto& location_spec = location_it.second;
const auto& it = params.find(param_key);
if (it != params.end()) {
tokens.push_back(std::make_unique<const JwtParamLocation>(
it->second, location_spec.issuer_checker_, param_key));
}
}
}

// Check query parameter locations.
const auto& params = Http::Utility::parseAndDecodeQueryString(headers.getPathValue());
for (const auto& location_it : param_locations_) {
const auto& param_key = location_it.first;
const auto& location_spec = location_it.second;
const auto& it = params.find(param_key);
if (it != params.end()) {
tokens.push_back(std::make_unique<const JwtParamLocation>(
it->second, location_spec.issuer_checker_, param_key));
// Check cookie locations.
if (!cookie_locations_.empty()) {
const auto& cookies = Http::Utility::parseCookies(headers);

for (const auto& location_it : cookie_locations_) {
const auto& cookie_key = location_it.first;
const auto& location_spec = location_it.second;
const auto& it = cookies.find(cookie_key);
if (it != cookies.end()) {
tokens.push_back(
std::make_unique<const JwtCookieLocation>(it->second, location_spec.issuer_checker_));
}
}
}

return tokens;
}

Expand Down
33 changes: 33 additions & 0 deletions test/extensions/filters/http/jwt_authn/extractor_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,15 @@ const char ExampleConfig[] = R"(
from_headers:
- name: prefix-header
value_prefix: '"CCCDDD"'
provider9:
issuer: issuer9
from_cookies:
- token-cookie
- token-cookie-2
provider10:
issuer: issuer10
from_cookies:
- token-cookie-3
)";

class ExtractorTest : public testing::Test {
Expand Down Expand Up @@ -265,6 +274,30 @@ TEST_F(ExtractorTest, TestCustomParamToken) {
tokens[0]->removeJwt(headers);
}

// Test extracting token from a cookie
TEST_F(ExtractorTest, TestCookieToken) {
auto headers = TestRequestHeaderMapImpl{
{"cookie", "token-cookie=token-cookie-value; token-cookie-2=token-cookie-value-2"},
{"cookie", "token-cookie-3=token-cookie-value-3"}};
auto tokens = extractor_->extract(headers);
EXPECT_EQ(tokens.size(), 3);

// only issuer9 has specified "token-cookie" cookie location.
EXPECT_EQ(tokens[0]->token(), "token-cookie-value");
EXPECT_TRUE(tokens[0]->isIssuerAllowed("issuer9"));
EXPECT_FALSE(tokens[0]->isIssuerAllowed("issuer10"));

// only issuer9 has specified "token-cookie-2" cookie location.
EXPECT_EQ(tokens[1]->token(), "token-cookie-value-2");
EXPECT_TRUE(tokens[1]->isIssuerAllowed("issuer9"));
EXPECT_FALSE(tokens[1]->isIssuerAllowed("issuer10"));

// only issuer10 has specified "token-cookie-3" cookie location.
EXPECT_EQ(tokens[2]->token(), "token-cookie-value-3");
EXPECT_TRUE(tokens[2]->isIssuerAllowed("issuer10"));
EXPECT_FALSE(tokens[2]->isIssuerAllowed("issuer9"));
}

// Test extracting multiple tokens.
TEST_F(ExtractorTest, TestMultipleTokens) {
auto headers = TestRequestHeaderMapImpl{
Expand Down