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 SecurityPolicy blocks CORS preflight requests, breaking OIDC + JWT flow #2312

Closed
apjoseph opened this issue Dec 18, 2023 · 16 comments
Closed
Assignees
Labels
kind/bug Something isn't working

Comments

@apjoseph
Copy link

Description:
the JWT configuration in the Security filter does not include the bypass_cors_preflight configuration option. Without this setting, OPTIONS requests are blocked unless the JWT is provided.

If the JWT is being passed through cookies with a standard OIDC frontend / JWT backend setup then the credentials won't be included since the preflight request needs to include the Access-Control-Allow-Credentials: true response header` before credentials will be sent.

I think bypass_cors_preflight should be set to true by default. I can't think of a good reason why someone would want to require a JWT for OPTIONS requests since Envoy doesn't even send OPTIONS requests down to the backend service. As a result the backend application can't customize the response. So blocking by default will just confuse people while providing no real benefit.

@apjoseph apjoseph added kind/bug Something isn't working triage labels Dec 18, 2023
@zirain
Copy link
Member

zirain commented Dec 18, 2023

cc @zhaohuabing

@zhaohuabing
Copy link
Member

zhaohuabing commented Dec 18, 2023

We can either:

  • set bypass_cors_preflight as true by default, or
  • leave the envoy false default, and expose bypass_cors_preflight knob to the API

I prefer to just set bypass_cors_preflight as true by default. But I'm not 100% sure if there are any security concerns on this. Would love more inputs from @arkodg @envoyproxy/gateway-maintainers

@apjoseph
Copy link
Author

@zhaohuabing Another potential solution is to allow different security policies to be applied when a different method is specified with the same path. This was actually my attempted workaround when I first encountered the issue. I created two http routes:

api-route

        matches:
          - path:
              type: PathPrefix
              value: /

api-route-cors

        matches:
          - path:
              type: PathPrefix
              value: /
           method: OPTIONS

and then created corresponding SecurityPolicies for both HttpRoutes. I only included the JWT section in the SecurityPolicy for api-route. I assumed this would prevent OPTIONS requests from being blocked; however when looking at the response from egctl I found that it was being applied to both HttpRoutes BUT only in some instances. Other times it would remove JWT auth from BOTH HttpRoutes -depending on which security policy was applied last.

I am not sure whether this is intended behavior or not, but it was counter intuitive to me as I would have expected all attributes of a match to be taken into account when applying a security policy if they are in separate routes -especially since the security policy is explicitly bound to an HTTPRoute

I like your approach of just turning the bypass_cors_preflight on by default though. It would be highly annoying to need to configure separate HttpRoute objects just for CORS.

@ardikabs
Copy link
Contributor

We can either:

  • set bypass_cors_preflight as true by default, or
  • leave the envoy false default, and expose bypass_cors_preflight knob to the API

I prefer to just set bypass_cors_preflight as true by default. But I'm not 100% sure if there are any security concerns on this. Would love more inputs from @arkodg @envoyproxy/gateway-maintainers

I was once concerned with bypass_cors_preflight #1930, but as per @arkodg #1930 (comment), it seems that it is intended to have it disabled by default, and configured through CORS policy.

@arkodg
Copy link
Contributor

arkodg commented Dec 18, 2023

@apjoseph can you also set the cors config within SecurityPolicy and try ?
we should make a note of this in the docs so others dont hit this issue

@apjoseph
Copy link
Author

@arkodg confirmed that the field is missing when defining the cors block in SecurityPolicy :

            typedPerFilterConfig:
              envoy.filters.http.cors:
                '@type': type.googleapis.com/envoy.extensions.filters.http.cors.v3.CorsPolicy
                allowCredentials: true
                allowHeaders: '*'
                allowMethods: '*'
                allowOriginStringMatch:
                - suffix: web.dev.example.io
                maxAge: "600"

@arkodg
Copy link
Contributor

arkodg commented Dec 20, 2023

@apjoseph not sure I understand.

if you setup cors in the SecurityPolicy https://gateway.envoyproxy.io/latest/user/cors/#configuration with allowCredentials set to true https://gateway.envoyproxy.io/latest/api/extension_types/#cors you should be pass the JWT back from the browser to the backend

@apjoseph
Copy link
Author

@arkodg I would be able to -but without bypass_cors_preflight enabled, the OPTIONS request for the route I'm trying to pass it to returns a 401 response that doesnt't contain the allow credentials CORS header. So the browser will never make the request in the first place.

The same issue is reported in multiple places, for instance in istio/istio/issues/36911

In the pull request adding the flag on EnvoyProxy, the contributors specifically state that it has to be explicitly enabled but frustratingly don't mention what the default setting is in the official documentation.

As you can see from this comment I think most people assume it is enabled by default when CORS is enabled and are deeply confused as to why Envoy's default behavior is to break any oauth2 application that uses an API with a different subdomain.

@zhaohuabing
Copy link
Member

zhaohuabing commented Dec 21, 2023

I reproduced this in my dev environment:

apiVersion: gateway.envoyproxy.io/v1alpha1
kind: SecurityPolicy
metadata:
  name: jwt-example
  namespace: default
spec:
  cors:
    allowHeaders:
    - x-header-1
    - x-header-2
    allowMethods:
    - GET
    - POST
    allowOrigins:
    - type: Exact
      value: www.foo.com
    exposeHeaders:
    - x-header-3
    - x-header-4
  jwt:
    providers:
    - name: example
      remoteJWKS:
        uri: https://raw.githubusercontent.com/envoyproxy/gateway/main/examples/kubernetes/jwt/jwks.json
  targetRef:
    group: gateway.networking.k8s.io
    kind: HTTPRoute
    name: foo
$  curl -I -H "Origin: http://www.foo.com" \
>   -H "Host: www.example.com" \
>   -H "Access-Control-Request-Method: GET" \
>   -X OPTIONS  -s \
>   http://localhost:8888/foo
Handling connection for 8888
HTTP/1.1 401 Unauthorized
www-authenticate: Bearer realm="http://www.example.com/foo"
content-length: 14
content-type: text/plain
date: Thu, 21 Dec 2023 12:14:31 GMT
server: envoy

I believe setting bypass_cors_preflight to true by default is fine because Istio also does the same.

But this is odd: the JWT filter is placed after the CORS filter in the filter chain. So, technically, a preflight request shouldn't even hit the JWT filter.

Envoy docs on CORS:

This filter will be used to respond to preflight OPTIONS requests. Any legal OPTIONS requests will be responded directly by the filter and will not be passed to the next filter in the filter chain.

// Set a rational order for all the filters.
switch {
case filter.Name == wellknown.CORS:
order = 1
case filter.Name == basicAuthFilter:
order = 2
case isOAuth2Filter(filter):
order = 3
case filter.Name == jwtAuthn:
order = 4
case filter.Name == wellknown.HTTPRateLimit:
order = 5
case filter.Name == wellknown.Router:
order = 100

@arkodg
Copy link
Contributor

arkodg commented Dec 21, 2023

@zhaohuabing I think its worth further debugging #2312 (comment) to understand why the OPTIONS request is getting a 401 in your test above instead of returning after hitting the cors filter because it doesnt match with https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_filters/cors_filter#cors

@zhaohuabing
Copy link
Member

zhaohuabing commented Dec 22, 2023

@apjoseph @arkodg

I tested the below SecurityPolicy with cors and jwt, and it worked.

apiVersion: gateway.envoyproxy.io/v1alpha1
kind: SecurityPolicy
metadata:
  name: cors-example
  namespace: default
spec:
  cors:
    allowHeaders:
    - x-header-1
    - x-header-2
    allowMethods:
    - GET
    - POST
    allowOrigins:
    - type: RegularExpression
      value: .*\.foo\.com
    exposeHeaders:
    - x-header-3
    - x-header-4
  jwt:
    providers:
    - name: example
      remoteJWKS:
        uri: https://raw.githubusercontent.com/envoyproxy/gateway/main/examples/kubernetes/jwt/jwks.json
  targetRef:
    group: gateway.networking.k8s.io
    kind: HTTPRoute
    name: backend

Use curl to send a CORS preflight request with the origin http://www.foo.com, and it succeeded.

$ curl -H "Origin: http://www.foo.com" \
>   -H "Host: www.example.com" \
>   -H "Access-Control-Request-Method: GET" \
>   -X OPTIONS -v -s \
>   http://$GATEWAY_HOST/ \
>   1> /dev/null
*   Trying ::1:8888...
* TCP_NODELAY set
* Connected to localhost (::1) port 8888 (#0)
> OPTIONS / HTTP/1.1
> Host: www.example.com
> User-Agent: curl/7.68.0
> Accept: */*
> Origin: http://www.foo.com
> Access-Control-Request-Method: GET
> 
Handling connection for 8888
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< access-control-allow-origin: http://www.foo.com
< access-control-allow-methods: GET, POST
< access-control-allow-headers: x-header-1, x-header-2
< access-control-expose-headers: x-header-3, x-header-4
< date: Fri, 22 Dec 2023 02:56:29 GMT
< server: envoy
< content-length: 0
< 
* Connection #0 to host localhost left intact

Tried to use another origin http://www.bar.com, and it failed with 401 Unauthorized, which is expected because this origin doesn't match the CORS setting in the SecuritPolicy, and it's passed down to the JWT filter.

$ curl -H "Origin: http://www.bar.com" \
>   -H "Host: www.example.com" \
>   -H "Access-Control-Request-Method: GET" \
>   -X OPTIONS -v -s \
>   http://$GATEWAY_HOST \
>   1> /dev/null
*   Trying ::1:8888...
* TCP_NODELAY set
* Connected to localhost (::1) port 8888 (#0)
> OPTIONS / HTTP/1.1
> Host: www.example.com
> User-Agent: curl/7.68.0
> Accept: */*
> Origin: http://www.bar.com
> Access-Control-Request-Method: GET
> 
Handling connection for 8888
* Mark bundle as not supporting multiuse
< HTTP/1.1 401 Unauthorized
< www-authenticate: Bearer realm="http://www.example.com/"
< content-length: 14
< content-type: text/plain
< date: Fri, 22 Dec 2023 03:13:46 GMT
< server: envoy

In this comment, I followed the user doc to test, but the demo SecurityPolicy in the user doc is wrong, the cors setting in the user doc is

  cors:
    allowOrigins:
    - type: Exact
      value: "www.foo.com"

but the test command is

curl -H "Origin: http://www.foo.com" \
  -H "Host: www.example.com" \
  -H "Access-Control-Request-Method: GET" \
  -X OPTIONS -v -s \
  http://$GATEWAY_HOST \
  1> /dev/null

Since the http://www.foo.com origin in the test command doesn't match the www.foo.com in the SecurityPolicy, the CORS filter ignores this OPTION request and passes it down to the JWT filter. That's why it gets a 401 Unauthorized response.

My bad on the confusing doc :-). I'm the one who wrote it. I'll go ahead to raise a PR to clean things up.

@apjoseph I suspect your preflight request failed for a similar reason. Could you please try to verify it with curl and share the SecurityPolicy and the curl command and result?

@apjoseph
Copy link
Author

@zhaohuabing I'll test again in the morning, but I seem to remember that when I applied allow_cors_preflgiht via a patchpolicy I got a 200 OPTIONS response on the same request where I was getting a 401 response before. If I had http:// in allowOrigin I wouldn't have ever received a 200 result. it's possible my application of the patch policy caused the filters to be reordered in some way.

@zhaohuabing
Copy link
Member

your preflight request failed for a similar reason. Could you please try to verify it with curl and share

Can you remove the patch and just use curl to test so we can isolate the problems?

@apjoseph
Copy link
Author

@zhaohuabing @arkodg you guys are right, I was able to confirm this works without bypass_cors_preflight. Not sure how I messed this up, sorry for wasting your time on this -feel free to close!

@arkodg
Copy link
Contributor

arkodg commented Dec 22, 2023

@apjoseph thanks for testing this out, its a brand new feature, and you're helping us close the gaps on user docs

hey @zhaohuabing thanks for testing this out !
it sounds like our oidc user doc MUST have the cors section set, else all users will hit this issue ?

@zhaohuabing
Copy link
Member

zhaohuabing commented Dec 23, 2023

hey @zhaohuabing thanks for testing this out ! it sounds like our oidc user doc MUST have the cors section set, else all users will hit this issue ?

@arkodg if the origin of a cors preflight request matches the cors allowOrigins settings, it will be handled by the cors filter and response directly without being passed down to the jwt filter. If it doesn't match, the request will be passed to the jwt filter and get a 401 response if it doesn't include a jwt token. I believe EG doesn't have to modify the cors or jwt user doc to mention that because it's an expected behavior, and this has nothing to do with oidc.

There was a minor issue in the cors doc though, it's been fixed with #2341

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants