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

OIDC SecurityPolicy: original authorization header removed #3396

Closed
denniskniep opened this issue May 15, 2024 · 14 comments · Fixed by #3567
Closed

OIDC SecurityPolicy: original authorization header removed #3396

denniskniep opened this issue May 15, 2024 · 14 comments · Fixed by #3567
Assignees
Labels
area/policy help wanted Extra attention is needed kind/feature new feature

Comments

@denniskniep
Copy link
Contributor

Description:
I use the OIDC SecurityPolicy to secure a route to an upstream application.
But the upstream application needs a different jwt token than issued by the envoy oauth2 filter. This jwt is added by the application frontend to the authorization header on every request.

Envoy oauth2 filter overwrites this authorization header.

Therefore I created an EnvoyPatchPolicy to set forward_bearer_token to false (and also a Ticket to make it configurable in the SecurityPolicy CRD: #3395)

The issue is that after setting forward_bearer_token to false no authorization header is forwarded to the upstream application.

When setting loglevel of envoy to debug then you can see the following logs:

Incoming Request with authorization header from application frontend:

[2024-05-15 10:27:02.943][14][debug][http] [source/common/http/conn_manager_impl.cc:393] [Tags: "ConnectionId":"8"] new stream                                                                                                                                                
[2024-05-15 10:27:02.943][14][debug][http] [source/common/http/conn_manager_impl.cc:1192] [Tags: "ConnectionId":"8","StreamId":"12035930501644059014"] request headers complete (end_stream=true):
':method', 'GET'
':authority', 'example.com'
':scheme', 'https'
':path', '/my/path'
'accept', 'application/json, text/plain, */*'                                                                                                                                                                                                                                 
'content-type', 'application/json'                                                                                                                                                                                                          
'authorization', 'Bearer ey....'
'cookie', 'OauthHMAC-4105b4aa=Fr...; OauthExpires-4105b4aa=1715771641'

And after skipping oauth flow due to valid hmac cookie the authorization header is gone!

[2024-05-15 10:27:02.943][14][debug][http] [source/common/http/conn_manager_impl.cc:1175] [Tags: "ConnectionId":"8","StreamId":"12035930501644059014"] request end stream
[2024-05-15 10:27:02.943][14][debug][connection] [./source/common/network/connection_impl.h:98] [Tags: "ConnectionId":"8"] current connecting state: false
[2024-05-15 10:27:02.943][14][debug][oauth2] [source/extensions/filters/http/oauth2/filter.cc:438] skipping oauth flow due to valid hmac cookie
[2024-05-15 10:27:02.943][14][debug][router] [source/common/router/router.cc:514] [Tags: "ConnectionId":"8","StreamId":"12035930501644059014"] cluster 'xyz' match for URL '/my/path'
[2024-05-15 10:27:02.943][14][debug][router] [source/common/router/router.cc:731] [Tags: "ConnectionId":"8","StreamId":"12035930501644059014"] router decoding headers:
':method', 'GET'
':authority', 'example.com'
':scheme', 'https'
':path', '/my/path'
'accept', 'application/json, text/plain, */*'                                                                                                                                                                                                                                 
'content-type', 'application/json'                                                                                                                                                                                                                   
'cookie', 'OauthHMAC-4105b4aa=Fr...; OauthExpires-4105b4aa=1715771641'      

Any Ideas?

Environment:
K8s Cluster
envoyproxy/gateway-dev:7863f81
envoyproxy/envoy:distroless-v1.29.2

@denniskniep denniskniep changed the title Authori OIDC SecurityPolicy: original authorization header removed May 15, 2024
@arkodg
Copy link
Contributor

arkodg commented May 15, 2024

cc @zhaohuabing

@zhaohuabing
Copy link
Member

zhaohuabing commented May 15, 2024

EG OIDC enables forward_bearer_token by default and overrides the authorization header if it exists. We can add an option to the API to opt it out. However, it's weird that the header is gone after setting forward_bearer_token to false with an EnvoyPatchPolicy.

@zhaohuabing zhaohuabing self-assigned this May 15, 2024
@arkodg
Copy link
Contributor

arkodg commented May 15, 2024

@zhaohuabing any idea why the EnvoyPatchPolicy did not work

@zhaohuabing
Copy link
Member

zhaohuabing commented May 15, 2024

@arkodg EnvoyPatchPolicy worked. It's just the Authorization header has been sanitized by the OAuth2 filter when the oauth2 filter is in the request path, and Envoy believes this is the right behavior :-)


  // Sanitize the Authorization header, since we have no way to validate its content. Also,
  // if token forwarding is enabled, this header will be set based on what is on the HMAC cookie
  // before forwarding the request upstream.
  headers.removeInline(authorization_handle.handle());

https://github.com/envoyproxy/envoy/blob/416cd425d50d3bc54c18ea134835a07ec21370d7/source/extensions/filters/http/oauth2/filter.cc#L292-L295

@arkodg
Copy link
Contributor

arkodg commented May 15, 2024

Ah nice find ! So filter ordering may be useful here ?

@zhaohuabing
Copy link
Member

Filter ordering won't help here since the OAuth2 filter will remove this header no matter where it's in the chain.

@denniskniep I'm curious why you use both a JWT added by your "frontend application" and a SecurityPolicy with OIDC at the same time?

@denniskniep
Copy link
Contributor Author

denniskniep commented May 16, 2024

Thanks a lot for looking into this.

There are at least two reasons why I want to do this:

  1. Upstream Application is not exposed/touched by external requests without being an legitimate authenticated user. That is especially interesting if there are unauthenticated RCE or other vulnerabilities for that application.
  2. Security of upstream applications AuthenticationLayer is not fulfilling required level. i.e. no MFA possible

In my case its really a jwt which is added to the request by the frontend application.
Therefore I want to mention a scenario, which maybe makes more sense. Upstream application is using basic auth and one want to increase security, by adding authentication with an IDP. UX is not great due to double authentication (First OIDC, then Basic Auth). But technically the basic auth can not be executed, because the header is removed.

@zhaohuabing
Copy link
Member

zhaohuabing commented May 17, 2024

@denniskniep thanks for detailing your use case! I agree that MFA (two or more layers of authentication) is a valid scenario here. Let's open an issue in Envoy upstream to see if a knob can be added to opt this out.

@zetaab
Copy link
Contributor

zetaab commented Jun 7, 2024

@zhaohuabing the feature for envoy is merged. So basically this can be added now to gateway?

@zhaohuabing
Copy link
Member

@zetaab Yes :-)

@zhaohuabing zhaohuabing added the help wanted Extra attention is needed label Jun 7, 2024
@zhaohuabing zhaohuabing removed their assignment Jun 7, 2024
@denniskniep
Copy link
Contributor Author

How are we handling backwards compatibility with current envoy proxy releases? Is there any common way, how this is done in envoy gateway?

Or is xDS ignoring the new property when it is sent to older envoy proxy versions where it is not yet existing?

@arkodg
Copy link
Contributor

arkodg commented Jun 7, 2024

How are we handling backwards compatibility with current envoy proxy releases? Is there any common way, how this is done in envoy gateway?

Or is xDS ignoring the new property when it is sent to older envoy proxy versions where it is not yet existing?

@denniskniep Envoy Gateway can only guarantee a 1:1 support with Envoy Gateway version and Envoy Proxy version, which is highlighted in https://gateway.envoyproxy.io/v1.0.1/install/matrix/, others may work but we cannot guarantee it .

@zhaohuabing
Copy link
Member

There are two ways to add support to this in EG:

  • add a preserveAuthorizationHeader knob to EG OIDC API, defaulting to false. This would maintain the current behavior.
  • set the Envoy OAuth2 filter preserve_authorization_header field to true by default. This flag can be flipped and added to the EG API if needed in the future.

I prefer the second option because, it prevents users from being surprised by the OAuth2 filter deleting an existing authorization header. Envoy's default behavior seems not reasonable.

@arkodg
Copy link
Contributor

arkodg commented Jun 7, 2024

+1 to 2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/policy help wanted Extra attention is needed kind/feature new feature
Projects
None yet
4 participants