envoy icon indicating copy to clipboard operation
envoy copied to clipboard

CORS: The preflight request is forwarded to the upstream if the origin is not allowed.

Open VirajSalaka opened this issue 4 years ago • 10 comments

Description:

When the CORS filter is engaged and the preflight request reaches the filter, that request should be handled via the CORS filter despite the request should be rejected or not.

While testing, I have had the following observations.

  • When I am sending the preflight request, with one of the allowed origins as the origin header It works perfectly fine. Cors filter responds for the request as expected. No issues found.
  • When the preflight request is sent with some other origin, instead of its being failed it is passed to the next filter (and eventually to the upstream). In my opinion, this also needs to be handled via the cors filter.

I think this is something better to be fixed. And your thoughts are highly appreciated.

Repro steps:

Provide a route with CORS configuration applied within envoy.yaml and perform a preflight request with some arbitrary Origin header (which is not the allowed one ) using curl. ex. curl -X OPTIONS "https://localhost:9095/v2/pet" -H "Origin:https://test.com" -H "Access-Control-Request-Method:POST" -k -v

Config: This is a sample CORS configuration I used for the route.

"cors": {
            "allow_methods": "GET, PUT",
            "allow_headers": "Authorization, Content-Type",
            "allow_credentials": true,
            "allow_origin_string_match": [
             {
              "safe_regex": {
               "google_re2": {},
               "regex": "https://test\\.com"
              }
             }
            ]
           }
          },

VirajSalaka avatar Dec 01 '20 19:12 VirajSalaka

As per w3c, I think we can separate the preflight request from other Options calls via Access-Control-Request-Method and Access-Control-Request-Headers headers. Rather than allowing that request to proceed to the next filter, we could simply stop from there and reply back with 2XX response code. (204 most likely)

https://github.com/envoyproxy/envoy/blob/39bb9a1bcb6e010ce6e32daf0e4318a2fcc376c9/source/extensions/filters/http/cors/cors_filter.cc#L68-L71

What do you think about introducing this as a fix ?

VirajSalaka avatar Dec 06 '20 13:12 VirajSalaka

cc @dschaller

mattklein123 avatar Dec 07 '20 05:12 mattklein123

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

github-actions[bot] avatar Jan 06 '21 08:01 github-actions[bot]

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.

github-actions[bot] avatar Jan 13 '21 08:01 github-actions[bot]

Can we re-open this? We also see similar issues. This results in the request error with HTTP 503, which our retry policy then retries.

nareddyt avatar Jan 26 '21 23:01 nareddyt

I also think that this should be reopened - the filter is hardly usable this way.

Tarick avatar Jan 20 '22 13:01 Tarick

@jmarantz I see you closed this as completed. Was this issue fixed somewhere else?

jwineinger avatar Jun 30 '22 20:06 jwineinger

can we reopen this issue @jmarantz ? If the behavior of forwarding the request to the upstream if the Origin doesnt not match is acceptable and not a bug, can we introduce a new API field called respond_to_all_preflights that always responds to a preflight request w/o forwarding it to upstream ?

arkodg avatar Feb 23 '24 18:02 arkodg

I am not sure what happened; this may have been an accident. Re-opening.

jmarantz avatar Feb 23 '24 18:02 jmarantz

peeking into https://github.com/envoyproxy/envoy/blob/5c056957f8742dc1cd056bc5f20a02d28ab480b2/CODEOWNERS#L290 @wbpcode @daixiang0 can you weight in on https://github.com/envoyproxy/envoy/issues/14233#issuecomment-1961827100 ? tia

arkodg avatar Feb 23 '24 19:02 arkodg

If the behavior of forwarding the request to the upstream if the Origin doesnt not match is acceptable and not a bug, can we introduce a new API field called respond_to_all_preflights that always responds to a preflight request w/o forwarding it to upstream ?

New api SGTM.

wbpcode avatar Mar 05 '24 05:03 wbpcode

I am working on the fix. Should have PR ready for review within few days.

cpakulski avatar Mar 06 '24 21:03 cpakulski

@wbpcode I think you added the following comment to CorsPolicy proto message: https://github.com/envoyproxy/envoy/blob/release/v1.29/api/envoy/config/route/v3/route_components.proto#L672-L674

I added a new field to the message https://github.com/envoyproxy/envoy/blob/release/v1.29/api/envoy/extensions/filters/http/cors/v3/cors.proto#L37, but the filter implemented in https://github.com/envoyproxy/envoy/tree/release/v1.29/source/extensions/filters/http/cors does not "see" the new field and compilation fails. It seems to use the "old" proto, which you marked as deprecated.

Is it by design? Maybe deprecation has not been finished. I can fix the problem if you explain the current situation with two almost identical proto messages. Thanks.

cpakulski avatar Mar 12 '24 00:03 cpakulski