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

Should be able to attach OIDC SecurityPolicy to Gateways #2913

Closed
mt-inside opened this issue Mar 13, 2024 · 13 comments · Fixed by #3275
Closed

Should be able to attach OIDC SecurityPolicy to Gateways #2913

mt-inside opened this issue Mar 13, 2024 · 13 comments · Fixed by #3275
Assignees

Comments

@mt-inside
Copy link
Contributor

mt-inside commented Mar 13, 2024

Description:
Currently, a SecurityPolicy that configures OIDC will only work if attached to an HTTPRoute. This is because the callbackURL and logoutPath have to match the path prefix of the HTTPRoute. (Although SecurityPolicies attached to Gateways aren't rejected, which is confusing)

I have a use-case which I believe is common: one Gateway for all my "internal apps" (Grafana, etc). Rather than bind this to an internal network, I'd like to do "zero trust" - expose it to the internet, and enable OIDC SSO. So I have many HTTPRoutes attached to that Gateway, all of which want OIDC.

Currently I have to make a SecurityPolicy for each one, and attach each one to the relevant HTTPRoute. This is kinda messy from an admin point-of-view.
What's worse is that because the SecurityPolicy's callbackURL has to match not only the HTTPRoute's path but also the OAuth2 client's callback path, I have to make an IdP OAuth2 Client for each HTTPRoute. At my company we use gsuite, which has no API for this, so every time I deploy a new app I have to click-ops a new OAuth2 client, deal with its secret key, etc.

I would be much better if I could make one OAuth2 client, one SecurityPolicy, and attach it to an entire Gateway.

@zhaohuabing zhaohuabing self-assigned this Mar 13, 2024
@arkodg
Copy link
Contributor

arkodg commented Mar 13, 2024

Id vote for the user creating the fallback route for this case i.e. creating a prefix match for / with a non existent backend.

@zhaohuabing
Copy link
Member

EG needs to report an error when a SecurityPolicy targeting a Gateway, and we need to add this workaround to the user docs.

@arkodg
Copy link
Contributor

arkodg commented Mar 14, 2024

@zhaohuabing can you highlight the current limitation here wrt routing, redirect and oidc filter ?

@zhaohuabing
Copy link
Member

zhaohuabing commented Mar 14, 2024

The current per-route filter implementation adds a filter for each OIDC/Basic Auth/Ext Auth, etc. config in the HCM http filter chain, and disable them by default. These filter are enable at route-level base on the targeted route/gateway of the SecurityPolicy.

This approach works well for Basic Auth/Ext Auth, but OIDC is a different: the redirectURL and logoutURL must match the HTTPRoute in order to make the IDP redirection works. So we can't apply the same OIDC config for the HTTRoutes of a Gateway because their matching paths are different.

As @arkodg suggested, a fallback HTTPRoute on / should solve this:

 Gateway1
     HTTPRoute1 /myapp1
     HTTPRoute2 /myapp2
     FallBackRoute /

A SecurityPolicy targeting Gateway1
SecurityPolicy
  target:  FallBackRoute
  OIDC:
     issuer ..
     redirctURL  https://www.example.com/oauth2/callback

@zhaohuabing zhaohuabing mentioned this issue Mar 14, 2024
@zhaohuabing
Copy link
Member

zhaohuabing commented Mar 14, 2024

@arkodg Unfortunately, I tested and found out that the HTTPRoutes for apps skipped the oauth2 filter because that filter is disabled by default for them (it's only enabled for the fallback HTTPRoute). This one is different than the JWT claim route issue, it can't be solved with a fallback httproute.

I think we may end up making OIDC "special" than other filters. For other per-route filters, EG disable them in the HCM filter chain, and enables them in the route config. If the SecurityPolicy targets a Gateway, we just spread the config to all the HTTPRoutes and enable them in each route. This approach doesn't work for OIDC. For OIDC, if a SecutiryPolicy targets a Gateway+Listener, we need to enable it in the HCM filter chain, and not allow other SecurityPolicy with OIDC targeting the Gateway's child HTTPRoutes to be created(because the HCM level OAuth2 filter is enabled and there's no way to disable them in the route config).

Copy link

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

@github-actions github-actions bot added the stale label Apr 13, 2024
@sadovnikov
Copy link
Contributor

@zhaohuabing, @arkodg, thank you very much for looking into this limitation 👍

The biggest inconvenience for us is the need to register in IDP additional allowed redirect URIs for every new service deployment, which can happen even from a fix/feature branch. And, of course, the cleaning up IDP upon removal of the temporary deployments.

Attaching OIDC security policies to Gateways (all HTTP Routes of the gateway) will lead to finding a way to detach them from HTTP Routes, which do not require oauth2 authentication. These are "welcome" pages with invitations to log in. Other examples are health and metrics probes, covered or not by JWT security policies.

I tried a workaround with a fallBack HTTP route on / and a fallback security policy with the identical configuration of the OIDC issuer. It failed with "too many redirects" because, I think, the two filters used different cookies and hmac secrets.

Gateway
  HTTPRoute /myapp
  FallBackRoute /. (using service without any endpoints)

  SecurityPolicy 
    target:  HTTPRoute
    OIDC:
      issuer ...
      redirectURL  https://www.example.com/oauth2/callback     # without /myapp in the path
	  
  SecurityPolicy (fallback)
    target:  FallBackRoute
    OIDC:
      issuer ...
      redirectURL  https://www.example.com/oauth2/callback     # without /myapp in the path

Should OIDC configuration be extracted into a separate resource? Then, security policies can reference both HTTP Route and the filter

@github-actions github-actions bot removed the stale label Apr 17, 2024
@zhaohuabing
Copy link
Member

zhaohuabing commented Apr 17, 2024

@zhaohuabing, @arkodg, thank you very much for looking into this limitation 👍

The biggest inconvenience for us is the need to register in IDP additional allowed redirect URIs for every new service deployment, which can happen even from a fix/feature branch. And, of course, the cleaning up IDP upon removal of the temporary deployments.

Yes, this is really annoying. We definitely need to support targeting OIDC on Gateway.

Attaching OIDC security policies to Gateways (all HTTP Routes of the gateway) will lead to finding a way to detach them from HTTP Routes, which do not require oauth2 authentication. These are "welcome" pages with invitations to log in. Other examples are health and metrics probes, covered or not by JWT security policies.

This can be supported since Envoy supports disabling a filter for specific routes.

I tried a workaround with a fallBack HTTP route on / and a fallback security policy with the identical configuration of the OIDC issuer. It failed with "too many redirects" because, I think, the two filters used different cookies and hmac secrets.

Gateway
  HTTPRoute /myapp
  FallBackRoute /. (using service without any endpoints)

  SecurityPolicy 
    target:  HTTPRoute
    OIDC:
      issuer ...
      redirectURL  https://www.example.com/oauth2/callback     # without /myapp in the path
	  
  SecurityPolicy (fallback)
    target:  FallBackRoute
    OIDC:
      issuer ...
      redirectURL  https://www.example.com/oauth2/callback     # without /myapp in the path

This doesn't work since the redirectURL URL "https://www.example.com/oauth2/callback" isn't prefixed the target HTTPRoute "/myapp", and the redirect goes to the oauth2 filter at the HTTPRoute FallBackRoute. As you said, these two filters use different cookies for session storage, so the second filter doesn't recognize that redirected request.

Should OIDC configuration be extracted into a separate resource? Then, security policies can reference both HTTP Route and the filter

To a certain extent, yes. OIDC requires targeting a specific HTTP listener or HTTP Route rather than the entire gateway, distinguishing it from other configurations in SecurityPolicy like basic auth, JWT, and CORS. Making it a separate resource is also easier to implement. However, I'm not entirely certain about this since it's no different from the user's perspective.

@sadovnikov
Copy link
Contributor

To a certain extent, yes. OIDC requires targeting a specific HTTP listener or HTTP Route rather than the entire gateway, distinguishing it from other configurations in SecurityPolicy like basic auth, JWT, and CORS. Making it a separate resource is also easier to implement. However, I'm not entirely certain about this since it's no different from the user's perspective.

I see two possible directions for resolving the problem of multiplying the redirectURL

  • Attaching security policies to the entire Gateway and disabling it on routes where it's not necessary
  • Configuring the OIDC filter separately and referring it from security policies, which remain attached to HTTP Routes. With this option, users can define multiple OIDC filters and use them in the same Gateway

@zhaohuabing
Copy link
Member

zhaohuabing commented Apr 18, 2024

@sadovnikov The challenge with the OIDC config is that it needs listener-specific targeting within a Gateway. This is because redirectURL must be unique for different hostnames or ports used by each listener. To accommodate this, SecurityPolicy needs SectionName support. This effectively adds a third layer(Listener) to the existing two-layer(Gateway - xRoute) policy hierarchy, which is already quite complex.

@sadovnikov
Copy link
Contributor

sadovnikov commented Apr 19, 2024

@zhaohuabing, we continued our experiment with a small change in the EG code, which makes OIDC filters on the actual service and the fallback routes use the same cookie. And it worked. The code changes in the image below are partial

image

The security policy CRs on the /myapp1, /myapp2 and / routes specify the same redirectURL (we use Helm template for this). And it's only known by the IDP redirect URL. Users can log in to both /myapp1 and /myapp2

Will you accept a PR to allow specifying the cookie suffix through securitypolicies.spec.oidc?

@zhaohuabing
Copy link
Member

zhaohuabing commented Apr 23, 2024

@sadovnikov This is genius! Let's do this!

@zhaohuabing
Copy link
Member

@mt-inside Actually, targeting OIDC SecurityPolicy to Gateways has already been supported. I'll update the doc to add an example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants