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] [plg_system_httpheader] Implement security http headers per default and create the posibility to add custom http headers #18301

Merged
merged 48 commits into from
Jan 22, 2018

Conversation

zero-24
Copy link
Contributor

@zero-24 zero-24 commented Oct 10, 2017

Summary of Changes

Implement security http headers per default and create the possibility to add custom http headers.

This plugin bases on https://github.com/zero-24/plg_system_httpheader which does the same for 3.x

Testing Instructions

You can also check the http header using the Network Tab of your Browser:
image

Per default this plugin set the following headers + values:

X-Frame-Options: SAMEORIGIN

More Infos: https://scotthelme.co.uk/hardening-your-http-response-headers/#x-frame-options

X-XSS-Protection: 1; mode=block

More Infos: https://scotthelme.co.uk/hardening-your-http-response-headers/#x-xss-protection

X-Content-Type-Options: nosniff

More Infos: https://scotthelme.co.uk/hardening-your-http-response-headers/#x-content-type-options

Referrer-Policy: no-referrer-when-downgrade

More Infos: https://scotthelme.co.uk/a-new-security-header-referrer-policy/

Expected result

After enabeling the default options the above mention headers are set.

Actual result

Joomla does not set any security headers.

Additional Infos

Give of the nature of the other headers we can not give a save default as they are very site specific. But atleast this plugin offers a easy to implement way to add the other headers using subforms.

Thanks for @SniperSister @wilsonge and @yvesh for the feedback on the first implementation ;)

Next todo's

  • Please test the plugin
  • We should review the language strings
  • We need to decide if the descriptions can stay or should be dropped.
  • Give your feedback.

Thanks.

@zero-24 zero-24 requested a review from wilsonge October 10, 2017 22:14
@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-4.0-dev labels Oct 10, 2017
; License GNU General Public License version 2 or later; see LICENSE.txt, see LICENSE.php
; Note : All ini files need to be saved as UTF-8

PLG_SYSTEM_HTTPHEADER="System - HttpHeader"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HTTP

@brianteeman
Copy link
Contributor

If we are going to do this then we should be complete and also offer CSP and STS options.

protected $app;

/**
* The list of the suported HTTP headers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change to supported

}

/**
* The `setHttpHeaders` methode handle the setting of the configured HTTP Headers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix methode

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear this should be method without an e at the end

@zero-24
Copy link
Contributor Author

zero-24 commented Oct 10, 2017

If we are going to do this then we should be complete and also offer CSP and STS options.

hmm the problem with that both things is that you can not set just one default option. But anleast for the csp there are planings beyond this PR / Plugin by the Joomla 4 team. So the system can generate a suggestion. But this should not stop this plugin as this prepare anything we can do from now.

Thanks for your other feedback. Commit is incoming 😄

@brianteeman
Copy link
Contributor

Agree about the issue of the default. Although it is possible to have a default value that will pass the test (I've done it) it is a crazy value

@brianteeman brianteeman changed the title [plg_system_httpheader] Implement security http headers per default and create the posibility to add custom http headers [4.0] [plg_system_httpheader] Implement security http headers per default and create the posibility to add custom http headers Oct 10, 2017
@wilsonge
Copy link
Contributor

@zero-24 do you want to add STS header into this PR - i think that one is pretty easy

@brianteeman i'm going to leave CSP for a later addition to this plugin - as you know from my experience at the sprint with https://github.com/wilsonge/joomla-cms/tree/csp it was lots of pain to have a half decent CSP - i'm going to keep plugging away at it. But in the mean time I think this gives us some quick wins whilst we think about things

@brianteeman
Copy link
Contributor

Re the descriptions. All the descriptions that are just long winded ways of saying the same thing as the label can be removed.

@zero-24
Copy link
Contributor Author

zero-24 commented Oct 10, 2017

@zero-24 do you want to add STS header into this PR - i think that one is pretty easy

Sure but what default value do you suggest? And as we don't force https on new sites it need to be per default disabled anyway.

@zero-24
Copy link
Contributor Author

zero-24 commented Oct 10, 2017

All the descriptions that are just long winded ways of saying the same thing as the label can be removed.

Sure. The question is what are our target users. Does a "normal" enduser understand what the setting means or not?

@wilsonge
Copy link
Contributor

Sure but what default value do you suggest? And as we don't force https on new sites it need to be per default disabled anyway.

I think we don't need on/off - we can use the main application values for force https? All we need to do is have the time till expires in the plugin which is configurable (by default probably a month - 2592000 seconds)

…ys overwrite the default values if they have the same name
@zero-24
Copy link
Contributor Author

zero-24 commented Oct 10, 2017

I think we don't need on/off - we can use the main application values for force https? All we need to do is have the time till expires in the plugin which is configurable (by default probably a month - 2592000 seconds)

Please check https://github.com/joomla/joomla-cms/pull/18301/files#diff-660b387da9d45a7ecca4cd27e25b3280R126 is it that what you had in mind?

Should we include the includeSubDomains setting too?

I have skipped the option part and allow now to overwrite the values using the additional headers i think this keeps the options low.

@wilsonge
Copy link
Contributor

Please check https://github.com/joomla/joomla-cms/pull/18301/files#diff-660b387da9d45a7ecca4cd27e25b3280R126 is it that what you had in mind?

Seems good to me!

Should we include the includeSubDomains setting too?

Umm I wouldn't do includeSubDomains - we're only concerned about the current Joomla site - no more

@SniperSister
Copy link
Contributor

If i read this correctly, the default headers will be enabled even for updated 3.x installs, correct? If so, we should all have in mind that this might be experienced as a backwards breaking change for users that are running their site in an iframe (I know a number of hosts who implement "redirects where the original domain stays the same" using an iframe) and make sure that this is documented properly.

@brianteeman
Copy link
Contributor

PLG_SYSTEM_HTTPHEADER_XFRAMEOPTIONS_DESC and the following descriptions are great examples of !eaningless strings. They don't say anything. And if your argument is that they are useful for less technical users then they are worse than useless as they are without any useful information (which should be in docs not tooltips anyway)

@brianteeman
Copy link
Contributor

Default on will be a problem. No question about that.

@brianteeman
Copy link
Contributor

#18301 (comment) no we can't use the force HTTPS value in global config. On a good server you do not (and should not) use that

@wilsonge wilsonge merged commit c71c11b into joomla:4.0-dev Jan 22, 2018
@wilsonge
Copy link
Contributor

Let's see how this goes for now :)

@zero-24 zero-24 deleted the security-headers branch January 23, 2018 17:53
@zero-24
Copy link
Contributor Author

zero-24 commented Jan 23, 2018

❤️

@brianteeman
Copy link
Contributor

Is there a reason why all the language strings are in the .sys.ini language file and not spread across two language files as normal

@zero-24
Copy link
Contributor Author

zero-24 commented Jan 24, 2018

see: #19449 @brianteeman

@Timeforsmilin
Copy link

Timeforsmilin commented Aug 30, 2018

Hello, I am not a coder, but you guys have hidden the csp and x-frame policies, which are creating duplicate entries, when someone uses alternate security software to protect a Joomla domain. Your csp entries are considered insecure, and may be creating a conflict that disables the administrator save/save&close/close button functions.

I haven't had my shared hosting website taken offline since installing this software https://securitycheck.protegetuordenador.com about 4 years ago. That is more than I can say for any Joomla development! I don't know if my sites were hacked from the shared hosting platform or not, but they have been fine since I have added that security.

Whatever you have created, please make it visible and adjustable, or remove it, as there are better options.

Regards,
Louis

@ghost
Copy link

ghost commented Aug 30, 2018

@Timeforsmilin can you please open a new Issue? Comments on closed Issues didn't get much Notice.

@ghost ghost mentioned this pull request Aug 30, 2018
@dialanerd
Copy link

What would cause this plugin to stop working?
Everything is enabled and it's the latest version.
None of the code is showing up on my website
I've uninstalled and reinstalled the plugin, but still no luck

Using Joomla 3 (to be upgraded), PHP8


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

@zero-24
Copy link
Contributor Author

zero-24 commented Apr 12, 2023

This here is the J4 version of the plugin this can not be installed into 3.x please use https://github.com/zero-24/plg_system_httpheader within J3.

After activation the plugin configuration has to be adjusted to meet your requirements feel free to follow: https://github.com/zero-24/plg_system_httpheader/blob/master/README.md

@dialanerd
Copy link

dialanerd commented Apr 12, 2023 via email

@dialanerd
Copy link

dialanerd commented Apr 12, 2023 via email

@zero-24
Copy link
Contributor Author

zero-24 commented Apr 12, 2023

Ok please create an issue on that repo and include some more details about what you have done, how the plugin configuration looks right now, what is the URL and what you expect. This is the repo for the J4 implementation. Thanks.

@dialanerd
Copy link

dialanerd commented Apr 12, 2023 via email

@zero-24
Copy link
Contributor Author

zero-24 commented Apr 12, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Language Change This is for Translators
Projects
None yet
Development

Successfully merging this pull request may close these issues.