Filters for OIDC authentication mechanisms

[sadovnikov]

Description:

Currently, when OIDC and JWT authentication mechanisms are configured in the same SecurityPolicy, the OIDC is applied first. It ensures the presence of the bearer and refresh tokens in cookies, and adds the Authorisation header to the request. Then JWT is applied, validating the added header.

This setup works perfectly for browser requests. However, it prevents monitoring systems and regression tests, which provide a valid Authorisation header in the requests, from accessing the services on the same URLs. The OIDC mechanism kicks in first and redirects the requests to the login pages because of the missing cookies.

We suggest extending the configuration of, at least, the OIDC mechanism with the possibility of adding a header-based filter.
When implemented, the OIDC can be skipped if the Authorization header is present.

An example of the configuration:

  oidc:
    provider: {}
    clientID: ""
    scopes: []
    clientSecret: {}
    matches:
    - ifRequestHeader:
        name: Authorization
        valueRegex: Bearer.*
      onMatch: skip

Envoy Proxy supports Composite Filters, which support such decisions and therefore can be used to implement this functionality.

Relevant Links:

• The Combining OIDC and JWT authentication discussion
• Envoy Composite Filters

[zetaab]

@arkodg any comments to this one? As I see this is quite important feature to have possibility to use service with human users and machines.

[arkodg]

@zetaab @sadovnikov my hesitance is with including another field like matches within oidc . Today the request is matching header attributes defined within the HTTPRoute and the SecurityPolicy is applied to a HTTPRoute. Adding another matches within oidc is not required and may confuse the user.

You could implement the same logic (#2425 (reply in thread)) with 2 HTTPRoutes today

• Security Policy 1 with a oidc config that applies to a HTTPRoute1 which has your routing rules
• Security Policy 2 with a jwt config that applies to a HTTPRoute2 which has your routing rules along with extra matches such as BearerToken is set && OauthHMAC is empty && OauthExpires is empty (will take precedence to the extra matches)

This should unblock you for now, but will end up causing some duplicate configuration for you.

You could also use a EnvoyPatchPolicy with a pass_through_matcher defined in https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/http/oauth2/v3/oauth.proto#extensions-filters-http-oauth2-v3-oauth2config

A more long term would be to to figure out the right UX / field for this use case.

[sadovnikov]

@arkodg From my discussion with @zhaohuabing I understood that two HTTP Routes cannot be applied to the same host/path. It seems I read it wrong, it's just necessary to ensure the uniqueness of the "matches" in the routes. We'll give it a try

[github-actions]

This issue has been automatically marked as stale because it has not had activity in the last 30 days.

[zetaab]

here is the example of how it can be used currently https://gist.github.com/zetaab/31b835bfed5bdc7b2a3e29bc6557e3c1

What that does?

• it can have public paths which is allowed to everyone
• it have /login path which will make the OIDC flow for humans and create cookies
• /api path which will work for both: humans (with cookie) and machines (with bearer token) (with two different identity providers)

However, if its used like this it means that application should have the logic to redirect to /login when its needed to use refresh token / create new token.

tl;dr it works but its not pretty

[github-actions]

This issue has been automatically marked as stale because it has not had activity in the last 30 days.

[arkodg]

rethinking this one, if we add introduce a field - skipIfJWTExists, we can solve this, we may also need to then add another field myJWTLivesInThisHeader

[zhaohuabing]

Yeah, this sounds reasonable to me. I'll raise an issue in Envoy and try to support this in v1.3.0.

[arkodg]

@zhaohuabing envoy already supports this as highlighted in #2496 (comment)

An API field is needed in EG that solves the use case

[github-actions]

This issue has been automatically marked as stale because it has not had activity in the last 30 days.

[StephenRobin]

It looks like the pass_through_matcher option in Envoy's oauth2 filter can be used to make this work.

https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/http/oauth2/v3/oauth.proto#envoy-v3-api-msg-extensions-filters-http-oauth2-v3-oauth2

I've tried the following patch and it appears to be working.

apiVersion: gateway.envoyproxy.io/v1alpha1
kind: EnvoyPatchPolicy
metadata:
  name: oidc-and-jwt-patch
  namespace: example
spec:
  jsonPatches:
    - name: example/eg/https
      operation:
        op: add
        path: >-
          /filter_chains/0/filters/0/typed_config/http_filters/0/typed_config/config/pass_through_matcher
        value:
          - name: Authorization
            prefix_match: Bearer
      type: type.googleapis.com/envoy.config.listener.v3.Listener
  targetRef:
    group: gateway.networking.k8s.io
    kind: Gateway
    name: eg
  type: JSONPatch

I guess the fix for this issue is to change the xds translator so that if jwt auth is also specified in the gateway CRD then pass_through_matcher is added to the xDS.

It would be nice if oidc and basic auth also worked well together. In this case the logic would need extending to pass through when "Authorization: Basic...." is in the headers, but the default filter order would also need changing to put the basic auth filter after oidc.

Edit: scratch that last bit about basic auth... clearly not going to work

[arkodg]

hey @StephenRobin thanks for driving this work
we'll need an API field to opt into this functionality (ref to #2496 (comment) )

[StephenRobin]

Hi @arkodg, I'm not so sure that a new field makes sense.

This was my thinking:

1. Would there ever be a situation where you would have OIDC enabled and JWT enabled and want newField=false? I can't think of one, JWT is broken without it.
2. Would there ever be a situation where you have only OIDC enabled and want newField=true? I can't think of one, OIDC is insecure in this case.
3. Why put the new field on OIDC? It feels rather arbitrary, it could equally go on JWT. Its purpose is to make the two work well together.

Therefore it made more sense to just automatically allow oidc bypass when jwt is also enabled.

Thinking about it again, this might be the wrong behaviour if they had either changed the order of the filters, or if they were using the extractFrom feature in JWT.

[arkodg]

Thanks for thinking this through @StephenRobin

Would there ever be a situation where you would have OIDC enabled and JWT enabled and want newField=false? I can't think of one, JWT is broken without it.

There may be a case, when a user may want to force OIDC Auth.

@missBerg @zhaohuabing @denniskniep probably have a better understanding of the spec, but I think there's a chance we may want to implicitly add JWT validation in the future if the spec clearly mentions it, this would mean that a separate JWT instantiation is not required.

Would there ever be a situation where you have only OIDC enabled and want newField=true? I can't think of one, OIDC is insecure in this case.

yes, this is something that can be flagged with a bad status Accepted=False for the SecurityPolicy which should also block the route (EG adds a 500 direct response for this case) .

Why put the new field on OIDC? It feels rather arbitrary, it could equally go on JWT. Its purpose is to make the two work well together.

It pertains to skipping OIDC Auth, so imo should live in OIDC.

Thinking about it again, this might be the wrong behaviour if they had either changed the order of the filters, or if they were using the extractFrom feature in JWT.

We can make this intelligent and peek into extractFrom and use that to define the skip condition

[denniskniep]

My thoughts on that:
I would appreciate a default behavior which makes sense in 90% of cases, but still having the ability to adjust to requirement i.e. able to specify a custom name for the authorization header.

ExtractFrom defines different ways to extract the JWT token from HTTP request.
If empty, it defaults to extract JWT token from the Authorization HTTP request header using Bearer schema
or access_token from query parameters.

https://gateway.envoyproxy.io/docs/api/extension_types/#jwtprovider

That means, that ExtractFrom is not by default extracting jwt from the OIDC Filter issued Cookie, right?
Does it make sense to add this cookie extractions also by default

There may be a case, when a user may want to force OIDC Auth

We do have a case where there is an application specific Authorization Header set which has nothing to do with the OIDC/JWT Filter.
That would break the functionality, if OIDC is skipped ALWAYS if that Authorization Header is present. Because the jwt filter then will validate against the jwt filter authority, but the token is issued by the application specific authority.

Therefore the two proposed fields (skipIfJWTExists and myJWTLivesInThisHeader) are able to tackle those case as far as I can see.

Would make sense to define good conditional defaults if it´s not explicitly specified i.e.

• skipIfJWTExists = true, when jwt filter exists && ExtractFrom is not specified
• myJWTLivesInThisHeader = Authorization, when jwt filter exists && ExtractFrom is not specified

[arkodg]

thanks @denniskniep
imo we can make the boolean skipIfJWTExists smart and have it peek into jwt.extractFrom, if unset it will skip based on the existance of Authorization , else will find the header, cookies, params from jwt.extractFrom and skip if those exist

[StephenRobin]

How about keeping this simple for now, just satisfy the known problems people have actually asked us to resolve:

• Add a property to the oidc config, passThroughAuthHeaders, default to false.
• If passThroughAuthHeaders is true then add a PassThroughMatcher for header "Authorization: Bearer..." to the underlying xDS
• If passThroughAuthHeaders is true and jwt is absent, or jwt.extractFrom is incompatible, then reject the SecurityPolicy with Accepted=false

These criteria will, I believe, ensure we're not breaking or changing the behaviour of existing SecurityPolicy's that people may be running. I'd be particularly concerned about defaulting passThroughAuthHeaders to true if jwt is present, potentially leaving user's configurations insecure if they have some set-up we've not thought of.

These criteria should also make it hard for someone to misconfigure the SecurityPolicy in an insecure way.

I chose the naming to be consistent with the naming of the underlying passThroughMatcher in xDS.

I think this leaves us open to future changes, including those suggested above:

• Adding a second property for the authorization header would be a non breaking change
• Obsoleting passThroughAuthHeaders and exposing the full passThroughMatcher of xDS would be straightforward and non-breaking.
• Relaxing the constraints that reject the SecurityPolicy would be fairly safe (highly unlikely that someone is relying on a policy being blocked).
• Adding additional constraints would be a breaking but safe change.

I hope this strikes a reasonable balance between the concerns.

Thoughts?

[missBerg]

What about making it so that the OIDC filter only does anything if there is no token in the Authorization header?

If a request comes in with a token in the Authorization header it should indicate no OIDC should happen.

[StephenRobin]

Hi @missBerg

What about making it so that the OIDC filter only does anything if there is no token in the Authorization header?

If a request comes in with a token in the Authorization header it should indicate no OIDC should happen.

This is exactly what we're proposing. Envoy itself has a passthroughMatcher option, this can be configured so that if a request comes in with the header "Authorization: Bearer ...." then OIDC does nothing and passes to the next filter for JWT checks.

We propose adding a new option that switches this on (as users may not always want it) and also making it an error to specify this new option when JWT is switched off or not configured to read that header.

[arkodg]

hey @StephenRobin your proposal looks good

[denniskniep]

If a request comes in with a token in the Authorization header it should indicate no OIDC should happen.

That would break our setup. We do have an application which does some app specific stuff with the Authorization-Header. But regardless what the app is doing with the Authorization-Header we want to authenticate with OIDC Filter and use the JWT Filter to extract the idtoken from the cookie and do authorization decisions.

[StephenRobin]

That would break our setup

Hi @denniskniep, as I said above, the behaviour will be unchanged unless the new passThroughAuthHeaders option is specified.

[denniskniep]

@StephenRobin you are right, it will not break if I do not use the new passThroughAuthHeaders option.

But if I want to use the new option, then it breaks, because the auth header is already used differently by the upstream.
Therefore it would be good if one could specify in which header the jwt lives (agree that it makes sense to set it by default to Authorization).

[arkodg]

So @denniskniep you're referring to a case where multiple auth tokens exist ?

[denniskniep]

exactly, one which is controlled by the upstream app (I have no control over this) and the upstream app is using the Authorization header for this.

And another one which is then used for envoy auth inside jwt filter. Because Authorization header is already used, it should be possible to specify an alternative header

[arkodg]

I think thats fine @denniskniep , this is my assumption of the API, please keep me honest here @StephenRobin

• by default passThroughAuthHeaders is false, and oidc is not skipped
• if passThroughAuthHeaders:true and jwt.providers[*].extractFrom: nil then skip if Authorization exists
• if passThroughAuthHeaders:true and jwt.providers[*].extractFrom: <some header/cookie/param> then skip if that header/cookie/param exists
  since providers is based off OR logic, if any provider auth is successful, then the request is accepted, similarly all extractFrom will need to be ORd here as well

[denniskniep]

We probably don't want to skip oidc filter if the cookie, which was issued by the oidc filter, is in the extractFrom of the jwt filter, right?

[StephenRobin]

Ok. That makes sense.

One alternative to consider is just exposing the full pass_through_matcher of Envoy, giving the user complete control.

I think Envoy doesn't support pass through based on params, and I agree with @denniskniep that cookies could be problematic, so I think it should just OR the set of headers specified in extractFrom.

I'm going to code this now, seems like a good Friday afternoon task. I can always change it later.

[StephenRobin]

Implemented in #5142