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

[4.0] Add Cross-Origin-Resource-Policy header to the core htaccess #28068

Merged
merged 8 commits into from
Mar 19, 2020

Conversation

zero-24
Copy link
Contributor

@zero-24 zero-24 commented Feb 24, 2020

Summary of Changes

Add Cross-Origin-Resource-Policy header to the core htaccess it protect against certain cross-origin requests. More information can be found here: https://developer.mozilla.org/en-US/docs/Web/HTTP/Cross-Origin_Resource_Policy_(CORP)

cc @SniperSister @mikewest

Testing Instructions

  • Apply the header to your htaccess
  • notice the header is set

Expected result

The header is set

Actual result

The header is not set

@zero-24 zero-24 added this to the Joomla 4.0 milestone Feb 24, 2020
@brianteeman
Copy link
Contributor

Last time we updated the htaccess we added a postinstall message

@zero-24
Copy link
Contributor Author

zero-24 commented Feb 24, 2020

Last time we updated the htaccess we added a postinstall message

Yes I know this time we do this in a major release not a patch release, but I'm happy to add a postinstall when requested.

@brianteeman
Copy link
Contributor

well dont forget people will be upgrading from 3 so wont know about the htaccess change if you dont tell them

@mikewest
Copy link

I'm not familiar at all with Joomla's internals, so it's not clear to me what set of resources this change would affect. It looks fairly sweeping! As y'all know, this will prevent cross-origin usage of all the resources that are tagged with this header. That's probably a great thing to do, and I'm excited to see y'all following up on it!

Still, it does seem reasonable to tell your users about that change, as some of them may rely upon static resources being available for same-site usage, or they might intentionally be making some set of resources available for cross-site embedding.

@zero-24
Copy link
Contributor Author

zero-24 commented Feb 25, 2020

Thanks @mikewest i have included the header in the main htaccess as usually our js, css, ... files should not be used by other origins.

I get the point of reporting to the users and will take a look into a postinstall message as suggested by @brainteeman.

@brianteeman
Copy link
Contributor

brianteeman commented Feb 25, 2020

Wont setting a cross-origin policy with these values prevent the loading of anything from a cdn loading and the same with any web fonts or am I completely misreading this

@zero-24
Copy link
Contributor Author

zero-24 commented Feb 25, 2020

You can still load anything from an cdn but your site can by default not be used as CDN. When you need this capitibily you have to set the cross-origin setting in the header.

@brianteeman
Copy link
Contributor

ah - I had it the wrong way around. ;)

So this means that a page like this one would cause problems https://jandbeyond.org/banners.html if someone was to try and use the suggested code to embed the banner on their own site??

@zero-24
Copy link
Contributor Author

zero-24 commented Feb 25, 2020

Yes. In that case you have to set the cross origin header value. I think we have to make this clear in the postinstall.

We might also just set that header in a dedicated htaccess for the banners folder of the JAB site.

@joeforjoomla
Copy link
Contributor

Pay attention to include such a thing by default... This would have the side effect to prevent all cross origin requests such as JSONP towards a Joomla website from an external website

@zero-24
Copy link
Contributor Author

zero-24 commented Feb 25, 2020

We can set the cross-origin setting for the API folder. Right? Or what do you think about?

@joeforjoomla
Copy link
Contributor

That would be a B/C break for people that upgrades from J3 to J4
Personally i would not include a similar directive in the standard htaccess, i think that it's always better to leave this optional

@joeforjoomla
Copy link
Contributor

joeforjoomla commented Feb 25, 2020

In J4 there is the CSP component... why not just using it for similar thing instead? What's the sense of hardcoding this header in the default htaccess? I don't understand why to complicate life

@zero-24
Copy link
Contributor Author

zero-24 commented Feb 25, 2020

That would be a B/C break for people that upgrades from J3 to J4

Well, first things first we would be are allowed to do B/C breaks in majors. But the good thing is that this is not an B/C break at all as affected are only new installs as this PR changes (as usual) only the htaccess.txt file and not the active file used within the running installation. So any update will still have the old htaccess and therefor works like before this PR.

Personally i would not include a similar directive in the standard htaccess, i think that it's always better to leave this optional

It would be optional but recommended like the nosniff stuff nothing more nothing less.

In J4 there is the CSP component... why not just using it for similar thing instead?
What's the sense of hardcoding this header in the default htaccess?

The problem is when this header comes into account joomla is not booted up and can not add any headers. When you load an image from a website joomla does not get triggerd to do anything only the webserver so we need to directly tell it the webserver. :)

@joeforjoomla
Copy link
Contributor

The problem is when this header comes into account joomla is not booted up and can not add any headers. When you load an image from a website joomla does not get triggerd to do anything only the webserver so we need to directly tell it the webserver. :)

Should not the CSP have an option to write to the htaccess as well?

@zero-24
Copy link
Contributor Author

zero-24 commented Feb 25, 2020

Should not the CSP have an option to write to the htaccess as well?

Yes we had but we decided to remove that option. Details can be found here: #24674 and #25754

@joeforjoomla
Copy link
Contributor

Yes i read about that... but was not sure about the final decision...i agree to remove that option

@joeforjoomla
Copy link
Contributor

Maybe just leave an option in the CSP to opt-in/opt-out the CORP directive writing/deleting it from the htaccess?

@zero-24
Copy link
Contributor Author

zero-24 commented Feb 25, 2020

Maybe just leave an option in the CSP to opt-in/opt-out the CORP directive writing/deleting it from the htaccess?

hmm I'm not sure whether it would make sense but i'm happy to try when we have found a solution for the mention issues. But please open a new issue for that as this would be bigger PR than just a few lines in the htaccess.txt file ;)

@brianteeman
Copy link
Contributor

I am really reluctant to see this going in the core htaccess


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28068.

@joeforjoomla
Copy link
Contributor

Me too, very reluctant

@brianteeman
Copy link
Contributor

To be clear the biggest problem with this rule being applied is that its not obvious. You build a web site with resources to share (perhaps banners in the example above). It works perfectly in development but users report that the banners just dont load. You check the embed code, you check the images exist, you check the dns, you shout at your host for having a firewall. You never would think to check the htaccess (thats just for sef isnt it) and even if you were to check it you would not guess that this rule with an obscure name is responsible. Applying this rule by default is creating problems not solving them. IF I know what I am doing then I can apply the rule myself and it will add questionable extra security.

@mbabker
Copy link
Contributor

mbabker commented Feb 26, 2020

Agree with the concerns above. This is one of those things that are too opinionated to be a default configuration for a general purpose web platform.

@zero-24
Copy link
Contributor Author

zero-24 commented Feb 26, 2020

Ok got your point. Do we have besides the image folder any resources that should be allowed to be loaded cross-site?

On solution for that problem could be a default / suggested htaccess for the images folder than. ;)

Another way around would be that we ship the rule but by default it is disabled and we add a postinstall step / docs which mention that to protect the system.

@mbabker
Copy link
Contributor

mbabker commented Feb 26, 2020

Ok got your point. Do we have besides the image folder any resources that should be allowed to be loaded cross-site?

Why is it Joomla's responsibility to decide what resources are hot-linkable across domains? This seems like a site owner's responsibility to define this policy, not the software's. The software's stance here should be providing documentation on why to use this rule and how to implement it appropriately for the end user's different use cases, not try to assume that 3% of the internet will arbitrarily use a default configuration because reasons.

@ot2sen
Copy link
Contributor

ot2sen commented Feb 26, 2020

Agree with the others here. It is a huge step to take on that kind of decision making from the users of the software.

How about making it a 'project recommended' #comment in the htaccess for those who would like to use it? With a link to how to like it was done for the htaccess changes for 3.9.3
https://docs.joomla.org/J3.x:Joomla_3.9.3_Security_Notes

Could that be a useable compromise? :)

@zero-24
Copy link
Contributor Author

zero-24 commented Feb 29, 2020

Could that be a useable compromise? :)

For me for sure. Just done that in that PR. We have to come up with a docs a page when this here has been accepted.

htaccess.txt Outdated Show resolved Hide resolved
htaccess.txt Outdated Show resolved Hide resolved
@carcam
Copy link

carcam commented Mar 5, 2020

I have tested this item ✅ successfully on 73157c7

I have successfully tested this:

  1. Apply test
  2. copy htaccess.txt to .htaccess
  3. Uncomment new content in the file
  4. CORS is now set
    This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28068.

@chmst
Copy link
Contributor

chmst commented Mar 18, 2020

I have tested this item ✅ successfully on 73157c7


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28068.

@joomla-cms-bot joomla-cms-bot removed this from the Joomla 4.0 milestone Mar 18, 2020
@zero-24
Copy link
Contributor Author

zero-24 commented Mar 18, 2020

RTC Thanks!


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28068.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 18, 2020
@wilsonge wilsonge merged commit 2ab4c5b into joomla:4.0-dev Mar 19, 2020
@wilsonge
Copy link
Contributor

Thankyou!

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 19, 2020
@wilsonge wilsonge added this to the Joomla 4.0 milestone Mar 19, 2020
@zero-24
Copy link
Contributor Author

zero-24 commented Mar 19, 2020

Thanks 👍

@zero-24 zero-24 deleted the corp branch March 19, 2020 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants