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

Set CORS to be an optional feature #12827

Merged
merged 2 commits into from Aug 5, 2021

Conversation

ascillato
Copy link
Contributor

@ascillato ascillato commented Aug 5, 2021

Description:

As reported in #6767, CORS is not safely implemented in Tasmota, so until a better solution is submitted, with this PR we disable by default all code related to CORS. If any user want to enable it, just add #define USE_CORS in user_config_override.h file and compile. Just remember to set a webpassword!

Note: In a following PR (separated from this just for better organization) will be added a message telling the user when they don't have a password set.

Related issue (if applicable): #6767

Checklist:

  • The pull request is done against the latest development branch
  • Only relevant files were touched
  • Only one feature/fix was added per PR and the code change compiles without warnings
  • The code change is tested and works with Tasmota core ESP8266 V.2.7.4.9
  • The code change is tested and works with Tasmota core ESP32 V.1.0.7.3
  • I accept the CLA.

NOTE: The code change must pass CI tests. Your PR cannot be merged unless tests pass

@ascillato2 ascillato2 linked an issue Aug 5, 2021 that may be closed by this pull request
14 tasks
@arendst arendst merged commit 1adced7 into arendst:development Aug 5, 2021
@arendst
Copy link
Owner

arendst commented Aug 5, 2021

Thx!!

@ascillato ascillato deleted the CORS_to_Optional branch August 5, 2021 17:55
@fmarzocca
Copy link

I think this is not a practical solution. I have over 10 devices at home, and I am using CORS with my local AJAX client. Now all the devices are all getting errors, due to CORS.
A solution could have been to add a SetOption specific for CORS.

@ascillato
Copy link
Contributor Author

I agree but as the CORS implementation in Tasmota has security issues, it is disabled by default. If you want to use it, you can compile by yourself enabling CORS.

This patch is temporary until the CORS implementation in Tasmota got fixed. Please, see comments in the linked issue. Thanks.

@fmarzocca
Copy link

Thank for answering. I agree to disable CORS by default, but a user setting should have been enough, not to re-compile alle flavors I am using (tasmota, tasmota-lite, tasmota-it, tasmota-sensors). Or I have to fully re-code my AJAX client.
I am not exposing the devices on internet, so it was only a matter of letting my app (which is running in the LAN) to talk with the devices.

@ascillato
Copy link
Contributor Author

ascillato commented Oct 19, 2021

Yes, I totally agree with you, but as explained in the bug report #6767, the security concerns about it are higher enough so as to not include it in any official firmware. It had already an user setting to enable or disable it, that was the command CORS. But also, with that command, the CORS implementation was a security risk. In the report you can see examples on how to enable CORS from a malicious website and taking control of a device. So, an user setting wasn't enough. Sorry.

So, because of that, CORS support is not included in any official precompiled firmware until a proper fix is implemented.

Anyway, all the code related to CORS has not been deleted. It is just not included. You can compile your own version adding CORS support by adding #define USE_CORS in your user_config_override.h file.

@pgorod
Copy link

pgorod commented Nov 26, 2021

A workaround for some cases:
#13818

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Local access security issue due to HTTP CORS header
4 participants