envoy
envoy copied to clipboard
CORS: The preflight request is forwarded to the upstream if the origin is not allowed.
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"
}
}
]
}
},
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 ?
cc @dschaller
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.
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.
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.
I also think that this should be reopened - the filter is hardly usable this way.
@jmarantz I see you closed this as completed. Was this issue fixed somewhere else?
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 ?
I am not sure what happened; this may have been an accident. Re-opening.
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
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.
I am working on the fix. Should have PR ready for review within few days.
@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.