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

Log on startup with "Default CORS properties will be used, please use 'quarkus.http.cors' properties instead" without more information #28377

Closed
yrodiere opened this issue Oct 4, 2022 · 11 comments · Fixed by #28384
Labels
area/openapi kind/bug Something isn't working
Milestone

Comments

@yrodiere
Copy link
Member

yrodiere commented Oct 4, 2022

Describe the bug

If I start a Quarkus application in production mode, with the smallrye-openapi extension enabled, I get this message on startup:

2022-10-04 13:50:54,394 INFO  [io.qua.sma.ope.run.OpenApiRecorder] (main) Default CORS properties will be used, please use 'quarkus.http.cors' properties instead

I find this confusing, as the message is not telling me what I did wrong (maybe forgot to set some properties, which ones?) nor exactly what is needed to solve it (which properties should I set, simply quarkus.http.cors=true? something more complex?). For the record, my application.properties is completely empty.

Expected behavior

Maybe the message should expand a bit on what's wrong and what should be done to solve the problem?

If there is no problem (as the "INFO" level seems to suggest), maybe it should be rephrased from "please use [...]" to "if you want X, do Y"?

Actual behavior

2022-10-04 13:50:54,394 INFO  [io.qua.sma.ope.run.OpenApiRecorder] (main) Default CORS properties will be used, please use 'quarkus.http.cors' properties instead

How to Reproduce?

quarkus create --extension resteasy,hibernate-orm-panache,jdbc-postgresql,smallrye-openapi
cd code-with-quarkus
./mvnw clean package
java -jar ./target/quarkus-app/quarkus-run.jar

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

No response

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

No response

Additional information

The log was apparently introduced in this commit by @sberyozkin: 597440d

@yrodiere yrodiere added the kind/bug Something isn't working label Oct 4, 2022
@yrodiere yrodiere changed the title "Default CORS properties will be used" without more information Log on startup with "Default CORS properties will be used, please use 'quarkus.http.cors' properties instead" without more information Oct 4, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Oct 4, 2022

@phillip-kruger
Copy link
Member

cc @sberyozkin

@sberyozkin
Copy link
Member

sberyozkin commented Oct 4, 2022

@yrodiere Hi, I'm not sure there is anything that we should address here. Perhaps, in production, we should even do WARN instead since the default CORS properties allowing everything is not a good idea in production.
This message advises users to take control of setting the CORS properties when running in production.

The log was apparently introduced in this commit by @sberyozkin: 597440d

I remember now, there was a SAST Snyk failure related to these default wildcard properties, I can link to the report if you'd like.

@sberyozkin
Copy link
Member

Perhaps the message can be improved, do you have some proposals in mind ?

@yrodiere
Copy link
Member Author

yrodiere commented Oct 4, 2022

the default CORS properties allowing everything is not a good idea in production

I think that's the essence of what you meant, then, and that should be in the message.

That being said, if it's such a big security risk, why aren't we setting quarkus.http.cors to true by default? You know, "secure by default"?

Also... why is this warning in the OpenAPI extension, while quarkus.http.cors is a property of the vert.x extension? Wouldn't CORS being disabled also be a problem when not using the OpenAPI extension?

Also, the documentation (597440d#diff-2df6cfd5c9a3b893d4bfc6e0d9c56c5cf1f9d8dc24727b22fc0206f4dff460b6R527-R530) seems to suggest that one needs to configure the CORS filter, otherwise a single-page application coming from another domain will not work... but that seems to be the opposite, it will work fine without a filter, but one should enable the filter (and correctly configure the filter) to make sure it can only be used from that single-page application?

So many questions... :)

Perhaps the message can be improved, do you have some proposals in mind ?

Maybe this?

                log.info("The CORS filter is not configured, allowing this REST API to be used from resources served by different domains. This is considered bad security practice. Please configure the CORS filter through 'quarkus.http.cors.*' properties. For more information, see https://quarkus.io/guides/http-reference#cors-filter");

Though I'm still not sure this log should exist if we consider that quarkus.http.cors=false is a good default...

@sberyozkin
Copy link
Member

sberyozkin commented Oct 4, 2022

We can't give more information as we don't know how CORS should be configured for a particular production case. But making it more informative can help, perhaps:

Default wildcard CORS properties will be used which is not recommended in production, please use 'quarkus.http.cors' properties instead.

Users can enable nearly the same wildcards with quarkus.http.cors=true only but at least it is the users who make the decision and enable it not Quarkus.

But recommending quarkus.http.cors=true in the same message can be contradictory :-)

@sberyozkin
Copy link
Member

@yrodiere Sure, I'd only propose replace bad security practice with a less scary not recommended :-), and would like to avoid expanding on what CORS properties are for as I guess it is known to most users. But I like the second part of your message. So may be:

Default wildcard CORS properties will be used which is not recommended in production. Please configure the CORS filter through 'quarkus.http.cors.*' properties. For more information, see https://quarkus.io/guides/http-reference#cors-filter.

Re quarkus.http.cors=true - it is also a wildcard if only this property is set...

@sberyozkin
Copy link
Member

The only concern is that https://quarkus.io/guides/http-reference#cors-filter link can be changed during the ongoing doc refactorings...

So probably

Default wildcard CORS properties will be used which is not recommended in production. Please configure the CORS filter through 'quarkus.http.cors.*' properties. For more information, see Quarkus HTTP CORS documentation. ?

@yrodiere
Copy link
Member Author

yrodiere commented Oct 4, 2022

Ok, I'm understanding a bit more.

As someone not working with HTML/HTTP on a daily basis, I must admit it's all quite blurry to me. So maybe it's clearer to the intended audience of Quarkus.

That being said, even for the intended audience, I think it can be confusing that we mention "Default CORS properties" being used as a problem. The problem is not the defaults (which are what they are, from what I understand we cannot do better). The problem is that those defaults are not to filter out anything, and that may not be secure.

So maybe you'd want to replace "Default wildcard CORS properties will be used" with "CORS filtering is disabled and cross-origin resource sharing is allowed without restriction"?

I would also suggest including a link, we've done it elsewhere already, and when the problem is specific enough, that's probably worth it.

CORS filtering is disabled and cross-origin resource sharing is allowed without restriction, which is not recommended in production. Please configure the CORS filter through 'quarkus.http.cors.*' properties. For more information, see Quarkus HTTP CORS documentation: https://quarkus.io/guides/http-reference#cors-filter

Still... why does this warning appear only when using quarkus-smallrye openapi? I'd expect the problem to affect just about any application?

@yrodiere
Copy link
Member Author

yrodiere commented Oct 4, 2022

The only concern is that https://quarkus.io/guides/http-reference#cors-filter link can be changed during the ongoing doc refactorings...

So probably

Default wildcard CORS properties will be used which is not recommended in production. Please configure the CORS filter through 'quarkus.http.cors.*' properties. For more information, see Quarkus HTTP CORS documentation. ?

Wait, if we're concerned that the documentation refactoring will lead to dead links... it will not affect just this message. Are we really intending to break links all over the place?

If it's just about anchors, we can definitely insert additional anchors in the documentation to make sure links don't break.

@sberyozkin
Copy link
Member

sberyozkin commented Oct 4, 2022

@yrodiere Your last message seems the best.

and

Wait, if we're concerned that the documentation refactoring will lead to dead links... it will not affect just this message. Are we really intending to break links all over the place?

I'm just not sure how the doc team will end up presenting it, it might end up in https://quarkus.io/guides/cors.

Still... why does this warning appear only when using quarkus-smallrye openapi? I'd expect the problem to affect just about any application? If it's just about anchors, we can definitely insert additional anchors in the documentation to make sure links don't break.

Good question. For example, with quarkus.oidc we do not set any CORS properties by default and recommend in the docs only to use CORS filter if CORS is required.
It just happens smallrye-open-api sets it by default - which was probably unnecessary as indeed in simple cases this is just the same origin browser to host interaction. But it is there so the message is an attempt to indirectly address the problem.

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

Successfully merging a pull request may close this issue.

4 participants