quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.73k stars 2.67k forks source link

CORS Request same origin ignored if no other origin set #30698

Closed yoadey closed 1 year ago

yoadey commented 1 year ago

Describe the bug

If the CORS filter is enabled to prevent CORS attacks, but no other origin is set because we expect all requests from the same origin, at least POST requests are blocked in Chrome.

GET requests do not send the origin header, so the CORS check is skipped, but POST requests include the origin header even, if it's from the same origin.

Expected behavior

If CORS is enabled, same origin requests are always successful, even if no other origins are set.

Actual behavior

If the property quarkus.http.cors=true is set, but quarkus.http.cors.origins is not set, the same origin policy introduced in #29626 should come into place and always allow requests from the same origin.

How to Reproduce?

Steps to reproduce:

  1. create a web app with swagger UI
  2. add a POST method
  3. set quarkus.http.cors=true
  4. invoke it in Chrome via swagger

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.16.0.FINAL

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

In CORSFilter line 191: https://github.com/quarkusio/quarkus/blob/f060bb8ced7147a8f7487d7179cebc8f14847711/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/cors/CORSFilter.java#L191 the AND includes also the sameOrigin.

The closing bracket of the AND should probably be after line 192?

quarkus-bot[bot] commented 1 year ago

/cc @sberyozkin (jwt,security)

sberyozkin commented 1 year ago

@yoadey Hi,

If the CORS filter is enabled to prevent CORS attacks, but no other origin is set because we expect all requests from the same origin

If you expect all requests from the same Origin then you should not enable the CORS filter as it is now strictly requires that a specific origin is configured. Having quarkus.http.cors=true is not enough to provide the protection.

I think this issue is invalid

geoand commented 1 year ago

I think this issue is invalid

If you reach that definitive conclusion, feel free to close the issue and add the invalid label :)

yoadey commented 1 year ago

@sberyozkin Hi, what other option do I have to prevent CORS requests?

sberyozkin commented 1 year ago

@yoadey CORS requests won't be coming from anywhere, you said in your application you have single origin calls only, so CORS protection is not necessary. If you expect Quarkus application be accessed from some 3rd party origins then these origins must be allowed. The reason it worked before for this case was that * origin was enabled by default which was not good at all. so if you can add the http://localhost:port origin then it will work

That said, perhaps we should relax a bit and do allow same host origins without any explicit configuration, so lets keep the issue open for now :-)

sberyozkin commented 1 year ago

@yoadey I can't promise the PR will be approved but I agree now that the same origin check should not be pre-conditioned by the 3rd party origin configuration

sberyozkin commented 1 year ago

@yoadey PR is opened but like I said no guarantee it will be merged, for now, adding the same origin urls to the config will do

mcanzerini commented 1 year ago

Hi, I think this issue is pretty relevant because the migration guide tells us:

Please note that the same-site Origin requests will be supported without quarkus.http.cors.origins having to be configured. Therefore configuring quarkus.http.cors.origins will only be required when the trusted 3rd party Origin requests should be allowed in which case allowing all Origins will be risky and unnecessary.

https://github.com/quarkusio/quarkus/wiki/Migration-Guide-2.16#no-wildcard-origin-support-by-default-for-cors-filter

What do you think ? I think the PR is welcome.

Legion2 commented 1 year ago

The isSameOrigin check does not work if the application is deployed behind a reverse proxy which hands TLS termination. Because the request which reaches the application has the http scheme but the Origin header contains the original scheme which is https.

KingRial commented 2 weeks ago

The isSameOrigin check does not work if the application is deployed behind a reverse proxy which hands TLS termination. Because the request which reaches the application has the http scheme but the Origin header contains the original scheme which is https.

Got your problem and discovered that, by using the ENV variables with regexp, it works...

For example to accept any foo.local subdomain I was using the regexp into application.properties: quarkus.http.cors.origins=/^((http)|(https)):\/\/(\w+)\.foo\.local/ And it didn't work as you described

So I started to change the configuration by using ENV variable QUARKUS_HTTP_CORS_ORIGINS: QUARKUS_HTTP_CORS_ORIGINS=/^((http)|(https)):\/\/(\\w+)\.foo\.local/ And it works

Please note the double slash before "w+"

I hope this can help someone else!