Skip to content

Jwt_authn config update: Support per route Jwt requirements #3381

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.

Sign up for GitHub

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

Merged
htuch merged 20 commits into from
Jun 21, 2018
Merged

Jwt_authn config update: Support per route Jwt requirements #3381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
781e9e0
Support per route Jwt requirements
qiwzhang May 14, 2018
811514a
Support backward compatibility
qiwzhang May 15, 2018
0a4f5c5
Update per comment
qiwzhang May 15, 2018
5e43cf1
Update per comment
qiwzhang May 15, 2018
1919dc3
Update per comment
qiwzhang May 15, 2018
bfa4442
change issuer names in the example
qiwzhang May 15, 2018
dda8e27
Fix compiling error after rename
qiwzhang May 17, 2018
cf125e5
Update example yaml format
qiwzhang May 30, 2018
9aa5b59
Update the comment
qiwzhang May 30, 2018
93200ce
Added or and and in the Requirement.
qiwzhang May 31, 2018
408ad55
rename or_jwt to one_of
qiwzhang May 31, 2018
8621c59
Update config to use requires_any
qiwzhang Jun 4, 2018
9172633
Define RequiredProviderList
qiwzhang Jun 4, 2018
24ff45d
Added nesting JwtRequirement message
qiwzhang Jun 4, 2018
1f9a2c8
Add provider id
qiwzhang Jun 5, 2018
1e9c569
rename provide id to name
qiwzhang Jun 5, 2018
be1a847
Change providers to map
qiwzhang Jun 5, 2018
4ffb59e
Modify jwt_authn code to use new config
qiwzhang Jun 15, 2018
e487059
Add ProviderWithAudiences
qiwzhang Jun 17, 2018
d2716de
Add allow_missing_or_failed back
qiwzhang Jun 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
236 changes: 206 additions & 30 deletions api/envoy/config/filter/http/jwt_authn/v2alpha/config.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import "envoy/api/v2/core/base.proto";
import "envoy/api/v2/core/http_uri.proto";
import "envoy/api/v2/route/route.proto";
import "google/protobuf/duration.proto";
import "google/protobuf/wrappers.proto";
import "validate/validate.proto";

// This message specifies how a JSON Web Token (JWT) can be verified. JWT format is defined
Expand All @@ -30,7 +31,7 @@ import "validate/validate.proto";
// - seconds: 300
//
// [#not-implemented-hide:]
message JwtRule {
message JwtProvider {
Copy link
Contributor

@nickrmc83 nickrmc83 Jun 15, 2018

Choose a reason for hiding this comment

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

@qiwzhang From a configuration PoV the only part of this change that doesn't exactly meet my usecase is that the JwtProvider type describes both the verification and forwarding parameters for a JWT. As a result filters that wish to re-use just the verification logic are also required to set the forward rule to true (as false is the default) so that the headers are not mutated. It'd be nice if either forwarding JWT headers was the default behaviour or alternatively and preferentially there was a clear type separation between verification configuration and forwarding configuration. The latter case would allow other filters to expose just the verification configuration as part of their own config. For example:

message VerificationRules {
  string issuer = 1 [(validate.rules).string.min_bytes = 1];
  ...
}

message FrowardingRules {
  bool forward = 1;
  string forward_payload_header = 2;
  ...
}

message JwtProvider {
  VerificationRules verification_rules = 1;
  ForwardingRules forwarding_rules = 2;
}

Copy link
Contributor Author

@qiwzhang qiwzhang Jun 16, 2018

Choose a reason for hiding this comment

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

Not clear on what is requested. Yes, current Provider has combined VerificationRule and ForwardRule. But not clear the benefits of separating them.
If jwt_authn filter is used alone, it needs both VerificationRule and ForwardRule. If jwt_authn filter is used only for verification by another filter (like your or Istio-authn), you still need to set ForwardRule for jwt_authn filer. For example, istio_authn doesn't need origin token, so its "forward" is false, just use its payload so it will use "forward_payload_header". If your filter needs origin token, you just set "forward" to true.

Maybe you will not use jwt_authn filter as a whole filter, you will just link its code as c++ library into your filter? I don't think it is a good way to go.

Copy link
Contributor

@nickrmc83 nickrmc83 Jun 16, 2018

Choose a reason for hiding this comment

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

Maybe you will not use jwt_authn filter as a whole filter, you will just link its code as c++ library
into your filter? I don't think it is a good way to go.

Unfortunately I think the only approach for my filter is to include and link the jwt_authn code directly and invoke it in the same way as http/jwt_authn/http_filter.cc:decode_headers. The reason for this is because the JWT on which I need verification performed is obtained by my filter via a filter-initiated external asynchronous HTTP request. As a result the JWT that is acquired does not pass through the filter chain. So using the code as a filter is not really an option. As such I need to make my filter expose the necessary config to perform verification and ideally not expose parameters that are not relevant whilst not redefining similar structures that already exist. In my case the forward field is not applicable. A solution to that is simply to document that setting this field within my filter will have no effect. And that's fine. So disregard my initial request but I will be wanting to directly invoke the Authenticator code.

// Identifies the principal that issued the JWT. See `here
// <https://tools.ietf.org/html/rfc7519#section-4.1.1>`_. Usually a URL or an email address.
//
Expand Down Expand Up @@ -183,44 +184,219 @@ message JwtHeader {
string value_prefix = 2;
}

// This is the Envoy HTTP filter config for JWT authentication.
// [#not-implemented-hide:]
message JwtAuthentication {
// List of JWT rules to valide.
repeated JwtRule rules = 1;
// Specify a required provider with audiences.
message ProviderWithAudiences {
// Specify a required provider name.
string provider_name = 1;

// This field overrides the one specified in the JwtProvider.
repeated string audiences = 2;
}

// This message specifies a Jwt requirement. An empty message means JWT verification is not
// required. Here are some config examples:
//
// .. code-block:: yaml
//
// # Example 1: not required with an empty message
//
// # Example 2: require A
// provider_name: "provider-A"
//
// # Example 3: require A or B
// requires_any:
// requirements:
// - provider_name: "provider-A"
// - provider_name: "provider-B"
//
// # Example 4: require A and B
// requires_all:
// requirements:
// - provider_name: "provider-A"
// - provider_name: "provider-B"
//
// # Example 5: require A and (B or C)
// requires_all:
// requirements:
// - provider_name: "provider-A"
// - requires_any:
// requirements:
// - provider_name: "provider-B"
// - provider_name: "provider-C"
//
// # Example 6: require A or (B and C)
// requires_any:
// requirements:
// - provider_name: "provider-A"
// - requires_all:
// requirements:
// - provider_name: "provider-B"
// - provider_name: "provider-C"
//
message JwtRequirement {
oneof requires_type {
// Specify a required provider name.
string provider_name = 1;

// Specify a required provider with audiences.
ProviderWithAudiences provider_and_audiences = 2;

// If true, the request is allowed if JWT is missing or JWT verification fails.
// Default is false, a request without JWT or failed JWT verification is not allowed.
bool allow_missing_or_failed = 2;
// Specify list of JwtRequirement. Their results are OR-ed.
// If any one of them passes, the result is passed.
JwtRequirementOrList requires_any = 3;

// This field lists the patterns allowed to bypass JWT verification. This only applies when
// `allow_missing_or_failed_jwt` is false. Under this config, if a request doesn't have JWT, it
// will be rejected. But some requests still needed to be forwarded without JWT, such as OPTIONS
// for CORS and some health checking paths.
// Specify list of JwtRequirement. Their results are AND-ed.
// All of them must pass, if one of them fails or missing, it fails.
JwtRequirementAndList requires_all = 4;

// The requirement is always satisfied even if JWT is missing or the JWT
// verification fails. A typical usage is: this filter is used to only verify
// JWTs and pass the verified JWT payloads to another filter, the other filter
// will make decision. In this mode, all JWT tokens will be verified.
google.protobuf.BoolValue allow_missing_or_failed = 5;
}
}

// This message specifies a list of RequiredProvider.
// Their results are OR-ed; if any one of them passes, the result is passed
message JwtRequirementOrList {
Copy link
Member

@htuch htuch Jun 19, 2018

Choose a reason for hiding this comment

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

Out of curiosity, would this work if we just inline it into the above fields? E.g. if we had repeasted JwtRequirement requires_any = 3 [constraints];? I'm not familiar with the proto rules on self-reference.

Copy link
Contributor Author

@qiwzhang qiwzhang Jun 19, 2018

Choose a reason for hiding this comment

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

Not, it will not work, proto "oneof" doesn't "repeated" field.

// Specify a list of JwtRequirement.
repeated JwtRequirement requirements = 1 [(validate.rules).repeated .min_items = 2];
}

// This message specifies a list of RequiredProvider.
// Their results are AND-ed; all of them must pass, if one of them fails or missing, it fails.
message JwtRequirementAndList {
// Specify a list of JwtRequirement.
repeated JwtRequirement requirements = 1 [(validate.rules).repeated .min_items = 2];
}

// This message specifies a Jwt requirement for a specific Route condition.
// Example 1:
//
// .. code-block:: yaml
//
// - match:
// prefix: "/healthz"
//
// In above example, "requires" field is empty for /healthz prefix match,
// it means that requests matching the path prefix don't require JWT authentication.
//
// Example 2:
//
// .. code-block:: yaml
//
// - match:
// prefix: "/"
// requires: { provider_name: "provider-A" }
//
// In above example, all requests matched the path prefix require jwt authentication
// from "provider-A".
message RequirementRule {
// The route matching parameter. Only when the match is satisfied, the "requires" field will
// apply.
//
// Examples: bypass all CORS options requests
// For example: following match will match all requests.
//
// .. code-block:: yaml
//
// bypass:
// - headers:
// - name: :method
// value: OPTIONS
// - headers:
// - name: :path
// regex_match: /.*
// match:
// prefix: "/"
//
// Examples: bypass /healthz check
envoy.api.v2.route.RouteMatch match = 1 [(validate.rules).message.required = true];

// Specify a Jwt Requirement. Please detail comment in message JwtRequirement.
JwtRequirement requires = 2;
}

// This is the Envoy HTTP filter config for JWT authentication.
//
// For example:
//
// .. code-block:: yaml
//
// providers:
// provider1:
// issuer: issuer1
// audiences:
// - audience1
// - audience2
// remote_jwks:
// http_uri:
// uri: https://example.com/.well-known/jwks.json
// cluster: example_jwks_cluster
// provider2:
// issuer: issuer2
// local_jwks:
// inline_string: jwks_string
//
// rules:
// # Not jwt verification is required for /health path
// - match:
// prefix: "/health"
//
// # Jwt verification for provider1 is required for path prefixed with "prefix"
// - match:
// prefix: "/prefix"
// requires:
// provider_name: "provider1"
//
// # Jwt verification for either provider1 or provider2 is required for all other requests.
// - match:
// prefix: "/"
// requires:
// requires_any:
// requirements:
// - provider_name: "provider1"
// - provider_name: "provider2"
//
//// [#not-implemented-hide:]
message JwtAuthentication {
// Map of provider names to JwtProviders.
//
// .. code-block:: yaml
//
// bypass:
// - headers:
// - name: :method
// value: GET
// - headers:
// - name: :path
// exact_match: /healthz
// providers:
// provider1:
// issuer: issuer1
// audiences:
// - audience1
// - audience2
// remote_jwks:
// http_uri:
// uri: https://example.com/.well-known/jwks.json
// cluster: example_jwks_cluster
// provider2:
// issuer: provider2
// local_jwks:
// inline_string: jwks_string
//
map<string, JwtProvider> providers = 1;

// Specifies requirements based on the route matches. The first matched requirement will be
// applied. If there are overlapped match conditions, please put the most specific match first.
//
// Examples
//
// .. code-block:: yaml
//
repeated envoy.api.v2.route.RouteMatch bypass = 3;
// rules:
// - match: { prefix: "/healthz" }
// - match: { prefix: "/baz" }
// requires:
// provider_name: "provider1"
// - match: { prefix: "/foo" }
// requires:
// requires_any:
// requirements:
// - provider_name: "provider1"
// - provider_name: "provider2"
// - match: { prefix: "/bar" }
// requires:
// requires_all:
// requirements:
// - provider_name: "provider1"
// - provider_name: "provider2"
//
repeated RequirementRule rules = 2;
}
27 changes: 10 additions & 17 deletions source/extensions/filters/http/jwt_authn/authenticator.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,10 @@ class AuthenticatorImpl : public Logger::Loggable<Logger::Id::filter>,
};

void AuthenticatorImpl::sanitizePayloadHeaders(Http::HeaderMap& headers) const {
for (const auto& rule : config_->getProtoConfig().rules()) {
if (!rule.forward_payload_header().empty()) {
headers.remove(Http::LowerCaseString(rule.forward_payload_header()));
for (const auto& it : config_->getProtoConfig().providers()) {
const auto& provider = it.second;
if (!provider.forward_payload_header().empty()) {
headers.remove(Http::LowerCaseString(provider.forward_payload_header()));
}
}
}
Expand Down Expand Up @@ -147,7 +148,7 @@ void AuthenticatorImpl::verify(Http::HeaderMap& headers, Authenticator::Callback
}

void AuthenticatorImpl::fetchRemoteJwks() {
const auto& http_uri = jwks_data_->getJwtRule().remote_jwks().http_uri();
const auto& http_uri = jwks_data_->getJwtProvider().remote_jwks().http_uri();

Http::MessagePtr message = Http::Utility::prepareHeaders(http_uri);
message->headers().insertMethod().value().setReference(Http::Headers::get().MethodValues.Get);
Expand Down Expand Up @@ -218,13 +219,13 @@ void AuthenticatorImpl::verifyKey() {
}

// Forward the payload
const auto& rule = jwks_data_->getJwtRule();
if (!rule.forward_payload_header().empty()) {
headers_->addCopy(Http::LowerCaseString(rule.forward_payload_header()),
const auto& provider = jwks_data_->getJwtProvider();
if (!provider.forward_payload_header().empty()) {
headers_->addCopy(Http::LowerCaseString(provider.forward_payload_header()),
jwt_.payload_str_base64url_);
}

if (!rule.forward()) {
if (!provider.forward()) {
// Remove JWT from headers.
token_->removeJwt(*headers_);
}
Expand All @@ -233,22 +234,14 @@ void AuthenticatorImpl::verifyKey() {
}

bool AuthenticatorImpl::okToBypass() const {
if (config_->getProtoConfig().allow_missing_or_failed()) {
return true;
}

// TODO(qiwzhang): use requirement field
return false;
}

void AuthenticatorImpl::doneWithStatus(const Status& status) {
ENVOY_LOG(debug, "Jwt authentication completed with: {}",
::google::jwt_verify::getStatusString(status));
if (status != Status::Ok && config_->getProtoConfig().allow_missing_or_failed()) {
callback_->onComplete(Status::Ok);
} else {
callback_->onComplete(status);
}
callback_->onComplete(status);
Copy link
Contributor

@nickrmc83 nickrmc83 Jun 15, 2018

Choose a reason for hiding this comment

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

For use by other filters, is it possible to have an onComplete callback that passes a const reference/pointer to the token as well as the status? In my uses case (OpenID Connect) I need to check some additional JWT claims to validate against XSRF attacks. These claims are potentially use case specific so pushing them into the Authenticator would be wrong.

Copy link
Contributor Author

@qiwzhang qiwzhang Jun 16, 2018

Choose a reason for hiding this comment

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

Clearly, you were thinking about linking jwt_authn code into your filter.

I will prefer to explore if you can use jwt_authn code as a filter. Your filter can use its verification result. Istio_authn originally linked jwt_authn code into its filter, it makes the filter very complicated since it needs to deal with aync callbacks. We abandoned that idea. Now it will not touch jwt_authn filter, just config it to behave the way istio_authn wanted and uses its result.

Now the result is passed by HTTP headers. We can change it to use requestinfo.dynamidMetadata to pass the result. https://github.com/envoyproxy/envoy/blob/master/include/envoy/request_info/request_info.h#L282

Copy link
Member

@lizan lizan Jun 16, 2018

Choose a reason for hiding this comment

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

In (near) future we will make the verified JWT into metadata, that will allow JWT works with RBAC filter, see #3638 too. I think that will address your use case? @nickrmc83

Copy link
Contributor

@nickrmc83 nickrmc83 Jun 16, 2018

Choose a reason for hiding this comment

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

@qiwzhang @lizan see my earlier comment about why I can't really re-use the code as a stand-alone filter and why I would like to directly invoke the Authenticator verification code. I suggest pushing forward this PR and discussing my usage in a PR I raise as I think my needs will become clearer.

callback_ = nullptr;
}

Expand Down
17 changes: 9 additions & 8 deletions source/extensions/filters/http/jwt_authn/extractor.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,19 +116,20 @@ class ExtractorImpl : public Extractor {
};

ExtractorImpl::ExtractorImpl(const JwtAuthentication& config) {
for (const auto& rule : config.rules()) {
for (const auto& header : rule.from_headers()) {
addHeaderConfig(rule.issuer(), LowerCaseString(header.name()), header.value_prefix());
for (const auto& it : config.providers()) {
const auto& provider = it.second;
for (const auto& header : provider.from_headers()) {
addHeaderConfig(provider.issuer(), LowerCaseString(header.name()), header.value_prefix());
}
for (const std::string& param : rule.from_params()) {
addQueryParamConfig(rule.issuer(), param);
for (const std::string& param : provider.from_params()) {
addQueryParamConfig(provider.issuer(), param);
}

// If not specified, use default locations.
if (rule.from_headers().empty() && rule.from_params().empty()) {
addHeaderConfig(rule.issuer(), Http::Headers::get().Authorization,
if (provider.from_headers().empty() && provider.from_params().empty()) {
addHeaderConfig(provider.issuer(), Http::Headers::get().Authorization,
JwtConstValues::get().BearerPrefix);
addQueryParamConfig(rule.issuer(), JwtConstValues::get().AccessTokenParam);
addQueryParamConfig(provider.issuer(), JwtConstValues::get().AccessTokenParam);
}
}
}
Expand Down
11 changes: 6 additions & 5 deletions source/extensions/filters/http/jwt_authn/filter_factory.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,15 @@ namespace {
* Validate inline jwks, make sure they are the valid
*/
void validateJwtConfig(const JwtAuthentication& proto_config) {
for (const auto& rule : proto_config.rules()) {
const auto inline_jwks = Config::DataSource::read(rule.local_jwks(), true);
for (const auto& it : proto_config.providers()) {
const auto& provider = it.second;
const auto inline_jwks = Config::DataSource::read(provider.local_jwks(), true);
if (!inline_jwks.empty()) {
auto jwks_obj = Jwks::createFrom(inline_jwks, Jwks::JWKS);
if (jwks_obj->getStatus() != Status::Ok) {
throw EnvoyException(
fmt::format("Issuer '{}' in jwt_authn config has invalid local jwks: {}", rule.issuer(),
::google::jwt_verify::getStatusString(jwks_obj->getStatus())));
throw EnvoyException(fmt::format(
"Issuer '{}' in jwt_authn config has invalid local jwks: {}", provider.issuer(),
::google::jwt_verify::getStatusString(jwks_obj->getStatus())));
}
}
}
Expand Down
Loading