inline scripts and Content-Security-Policy

[oupala]

FreshRSS contains some inline javascript on the homepage (in /i/ directory):

<script src="../scripts/bcrypt.min.js?1443537072" defer="defer" async="async"></script>
        <script>//<![CDATA[
...

This is a bad habit as inline scripts and css are a good entrance for XSS attacks. See https://scotthelme.co.uk/content-security-policy-an-introduction/

With a secure configuration such as Content-Security-Policy :"default-src 'self'", FreshRSS do not work any more. The configuration has to be set to Content-Security-Policy :"default-src 'self' 'unsafe-inline'" which is working but less secure.

A good improvement would be to follow this advice:

To be able to accommodate this, you need to externalise the content of any <script> tags. Instead of a block of code for your Google Analytics, you would have to create an external file and reference it like <script src="/assets/js/ga.min.js"></script> instead. That's not such a big problem.

[Alkarex]

Hello @oupala
And thanks for your suggestion. The reason for the inline JavaScript is that it contains dynamically generated values. Putting them in an external file would make another HTTP request that cannot be cached, so would hurt the performances.
I will try to think about other approaches (e.g. through cookie, local cache, URL parameters, ...).

[Alkarex]

https://hacks.mozilla.org/2016/02/implementing-content-security-policy/

[Alkarex]

@oupala See a first draft #1078 using a one-time cookie (edit: now inline JSON) instead of the inline dynamic JavaScript. There are still several places to change (such as in the install script and statistics). There are also a few onclick events here and there. CSS would need some work too.
The main features are working fine with
Content-Security-Policy: default-src 'self'; img-src * data:; media-src *; style-src 'self' 'unsafe-inline'
which I have currently hard-coded https://github.com/Alkarex/FreshRSS/blob/e4a459a6edc40b64cba7845b52f3e90666b2818a/app/FreshRSS.php#L171
Anyway, let me know what you think with this version.

[oupala]

Excellent @Alkarex !

But I see that you set the header in php. I have the habit to set the header in apache virtual host. Do you now what happens if the header is set twice? Is there a conflict? And what happens if headers setting in forbidden in php?

And can you explain me the default header that you set?

Content-Security-Policy: default-src 'self'; img-src * data:; media-src *; style-src 'self' 'unsafe-inline'

I don't understand this notation: img-src * data:; and this one: media-src *;.

[Alkarex]

I am not sure this line will stay in the code, but in any case in Apache, you should be able to replace (set) or edit headers http://httpd.apache.org/docs/trunk/en/mod/mod_headers.html#header

• img-src * data:; we have images coming from multiple domains (all the feeds), hence the *. We also have a dynamic favicon showing that there are some unread articles. This is done with a data-uri.
• media-src *; similar than for images, feeds often provide videos or sounds.
• child-src *; similar than above, but to allow iframes, e.g. for YouTube videos and similar. Alkarex@8cdf44c
• For style-src, we have some inline CSS, but this is something we can work on if desired

[Alkarex]

@oupala I have implemented much more CSP.
There is a remaining style-src 'self' 'unsafe-inline' but only in the statistics page due to the way the CSS styling is done by https://github.com/HumbleSoftware/Flotr2
Otherwise, we have now Content-Security-Policy: default-src 'self'; child-src *; img-src * data:; media-src *
Testing welcome (in particular for the installer, not tested yet)

[Alkarex]

P.S. I now use inline JSON instead of a one-time cookie Alkarex@e3dc7d4

[Alkarex]

There is a bug when manually refreshing all the feeds due to jQuery $.getScript()

[Alkarex]

Last known bug (manual refreshing) fixed in Alkarex@c9d3d78
Testing of #1078 welcome

[Alkarex]

Done. Any remaining comments in #1078

[Alkarex]

Merged in /dev

[oupala]

I've just updated my FreshRSS instance to version 1.6.2 and I start to test CSP. Is CSP available in 1.6.2? I think so as I has been merged in dev in february 2016...

First look, the console tells me frame-src is deprecated, please use child-src instead.

I think you should remove the deprecated CSP attribute.

[oupala]

I'm also having a problem testing CSP with FreshRSS as I'm having a default CSP policy for my whole apache web server, while FreshRSS is setting its own CSP headers via php.

Between apache and php, apache wins (see this php issue):

Header directives (mod_header) are processed just before the response is sent to the network (and after any content generator like PHP).

Setting php to overwrite the header (see the following php documentation) does only allow php to overwrite a previous php header, not an apache header.

What would be the best way to set CSP headers? I think it might be via using an .htaccess file (but this file might differ depending on the http server used (apache, nginx...).

Any thought?

[Alkarex]

Hello,
frame-src is needed for Firefox 44-. It is not a problem for newer versions. I will remove it soon.
#1099 #1173

In Apache, you can just avoid setting CSPs for the FreshRSS directory. FreshRSS will use different CSP rules depending on the pages. If you really want to have additional Apache rules, see the additional Apache rules https://httpd.apache.org/docs/current/en/mod/mod_headers.html#header , in particular setifempty.

[oupala]

Firefox 44 market share is almost null: 0.07%. But that's not my point.

My point is that FreshRSS should be compliant with CSP, and needed CSP should be clearly written in the documentation. But setting CSP should be the server admin responsibility, not the application developer.

In my case, I have a website with a CSP policy on the whole website. I can overwrite this policy on a specific subdirectory, which is what I did for the FreshRSS subdirectory. But this way, I cannot unset CSP setting to let php set this for me. As apache headers is set after php processing, apache headers always overwrite php headers. This is why I think apache should be responsible for setting CSP, not php.

So this lets me to my main point: the recommended CSP policy should be clearly indicated in the documentation.

And this main point leads me to another point: why does FreshRSS needs different CSP depending on the page?

[Alkarex]

Hello @oupala (sorry for the delay - forgot about this one).
If you use setifempty as I suggested above, you can have general Apache rules, which can be overridden by FreshRSS or other apps that already define their CSP rules. For instance, I use the following at the root of my Web site:

Header setifempty Content-Security-Policy "default-src 'self'; frame-ancestors 'self'; report-uri /blabla/csp/"

Regarding the different CSP rules: there are not the same needs on e.g. configuration pages (more protected) than content pages (which need to allow more third-party content). See https://github.com/FreshRSS/FreshRSS/blob/master/app/FreshRSS.php#L112

Let me know whether that addresses your needs.

[oupala]

I have to say once more that I do not think that the primary goal of implementing CSP should be adding a header generator into the php code.

The primary goal of implementing CSP is make the software works with the CSP headers as strict as possible.

The secondary goal of implementing CSP could be to make the software self- generating its own CSP headers (but this is not the way CSP have been thinked).

When I look at FreshRSS, I can see that the secondary goal seems to be already implemented.

But when I look at the link the previous comment, I think that there should be some changes in index and stats pages.

The stricter (and best) CSP header is - according to me - the following:

Content-Security-Policy: default-src 'self'

The good point is that it is the default header for FreshRSS.

But I don't understand the headers for index and stats pages.

• index : Content-Security-Policy: default-src 'self'; child-src *; frame-src *; img-src * data:; media-src *
• stats : Content-Security-Policy: default-src 'self'; style-src 'self' 'unsafe-inline'

Here are my questions:

• I don't understand why does index page needs child-src and frame-src as I don't think FreshRSS is not using frames (or maybe frames are used for embedding content such as videos?)
• I don't understand why stats page needs to allow (unsafe) inline styling, couldn't styles be moved into a separate css file?

Thanks for all these answers.

I wish all this could end with a generic CSP header that could look like:

Content-Security-Policy: default-src 'self'; img-src * data:; media-src *

This would allow content only from self (from FreshRSS resources) except for images and medias that could come from anywhere (for all image and video resources linked into rss feed's articles).

[Alkarex]

FreshRSS comes with some default CSP rules, which have a purpose and have been tested. The advantage of having built-in CSP rules is to have some functional security turned on by default. Furthermore, deciding the appropriate rules requires an understanding of the app's behaviour, which is not always something the Web server admin has.

But as the Web server admin, you are free to remove the CSP headers, edit them, add some default ones, etc. All that easily when using appropriate Header rules in Apache. So having some default rules neither decreases the security nor reduces your options to use whatever CSP rule you find appropriate.
You can therefore easily overwrite FreshRSS's CSP rules to one of your choice, and test whether the results match your desires.

The rule for index is designed to allow external content such as YouTube videos (which are using embed / iframe), as well as third-party img / video / audio.

The rule for stats is designed to allow graphs, which are done with the Flotr2 library, and which currently makes some dynamic inline styling.

[oupala]

Ok, thanks for these explanations.

If I want to set a global CSP rule for the whole FreshRSS subfolder, I would have to use the following header:

Content-Security-Policy: default-src 'self'; style-src 'self' 'unsafe-inline'; child-src *; img-src * data:; media-src *

I omited frame-src as it is now deprecated.

[Alkarex]

Yes indeed, but I do not see any advantage of doing so, since the rule is poorer than the built-in FreshRSS rules.

[oupala]

Sometime, a paranoid sysadmin wants to set security policies from outside the installed piece of software. In this case, apache config is the right place to do that.

But I agree, as the rule is less precise that the built-in CSP set by FreshRSS, the final security level will be lower.

[Alkarex]

You should consider using the Header setifempty approach I was suggesting above. This way, your rule will be set only if FreshRSS fails to provide its own.
Alternatively, you can reproduce FreshRSS rules in Apache by setting the headers depending on URL QUERY_STRING patterns. This can be done for instance by using the expr parameter at the end of the Header line https://httpd.apache.org/docs/current/en/expr.html

[oupala]

I moved my settings to use setifempty and I'm now using the headers from FreshRSS's php.

But I'd like to add a report uri, which is set inside the Content-Security-Policy header.

How can I add this report-uri without overwriting FreshRSS's embedded Content-Security-Policy headers?

[Alkarex]

@oupala A simple Header add should work (remember to put it after any setifempty). Example

Header add Content-Security-Policy "report-uri /report-uri/" "expr=%{CONTENT_TYPE} =~ m#application/xhtml[+]xml|text/html#i"

On my server, it looks like there is a bug somewhere and the result is similar to Header merge (i.e. with a comma instead of a new line). In case of such problems, Header edit is also possible:

Header edit Content-Security-Policy "$" "; report-uri /report-uri/" "expr=%{CONTENT_TYPE} =~ m#application/xhtml[+]xml|text/html#i"

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy#Multiple_content_security_policies

But report-uri is deprecated https://www.w3.org/TR/CSP3/#changes-from-level-2

P.S.: frame-src has been un-deprecated by CSP v3 #1099 (comment)

[oupala]

Thanks for the tip. I'm having the same bug as you. But the Header edit is a working workaround for me.

And directives are changing from a release of CSP to another. According to caniuse, we should stick to CSP v1 for now as CSP v2 is not yet supported widely enough (and CSP v3 is not on caniuse at all...).

But maybe it is possible to mix directives from CSP v1, v2 and v3...

[jaysingh17]

Hello,

I am facing this type of issue when I have testing speed optimization in gtmetrix.

The following inline script blocks were found in https://www.youtube.com/embed/C68xtnQ5Oabcdssereser?rel=0 between an external CSS file and another resource. To allow parallel downloading, move the inline script before the external CSS file, or after the next resource.

Inline script block #3

Can you please help me, how to fix this issue.

[Alkarex]

@jaysingh17 Is there any FreshRSS functionality that is not working?

[oupala]

Hi @Alkarex

I tried many of your suggestion of your previous comment but I can't succeed to modify the header sent by FreshRSS.

I can only succeed to add another Content-Security-Policy header, but this does not work for adding a report-uri to the header. This report-uri should be added to the corresponding header in order to take effect.

Is there a limitation of editing in apache configuration headers that has been sent by php?

It is working with apache 2.4.10 (Raspbian) and FreshRSS 1.16.0 on debian/raspbian jessie with the following module enabled:

$ ls /etc/apache2/mods-enabled/
access_compat.load  authn_file.load  autoindex.load  env.load      mpm_prefork.conf  php5.load        setenvif.load       wsgi.load
alias.conf          authz_core.load  deflate.conf    filter.load   mpm_prefork.load  reqtimeout.conf  socache_shmcb.load
alias.load          authz_host.load  deflate.load    headers.load  negotiation.conf  reqtimeout.load  ssl.conf
auth_basic.load     authz_user.load  dir.conf        mime.conf     negotiation.load  rewrite.load     ssl.load
authn_core.load     autoindex.conf   dir.load        mime.load     php5.conf         setenvif.conf    wsgi.conf

It is not working with apache 2.4.38 (Raspbian) and FresRSS 1.18.0 on debian/raspbian buster with the following module enabled:

$ ls /etc/apache2/mods-enabled/
access_compat.load  authn_file.load  autoindex.load  env.load      http2.load      negotiation.conf  reqtimeout.conf  socache_shmcb.load
alias.conf          authz_core.load  deflate.conf    expires.load  mime.conf       negotiation.load  reqtimeout.load  ssl.conf
alias.load          authz_host.load  deflate.load    filter.load   mime.load       proxy.conf        rewrite.load     ssl.load
auth_basic.load     authz_user.load  dir.conf        headers.load  mpm_event.conf  proxy.load        setenvif.conf
authn_core.load     autoindex.conf   dir.load        http2.conf    mpm_event.load  proxy_fcgi.load   setenvif.load

[Alkarex]

@oupala I have not tried recently, but it might depend on how you run PHP (as module, fcgi...). You are supposed to be allowed to change everything you want. Do you maybe have another proxy in front such as Traefik - in which case it can also be done there - ?

[oupala]

Nope, there is nothing more than an apache server.

In fact, I think that FreshRSS has nothing to do with the problem I'm having (not being able to modify a header using Header edit), but I have no idea on what to look for as I really think that it should work as is. Same server, (almost) same configuration...

Would you have an idea?

[oupala]

I finally found that mod_headers has a specific behavior when used alongside with mod_proxy_fcgi (which I am using in order to enable http/2).

As stated in the documentation of mod_headers:

You're modifying or removing a header generated by a CGI script or by mod_proxy_fcgi, in which case the CGI scripts' headers are in the table corresponding to always and not in the default table.

And:

You're modifying or removing a header generated by a CGI script or by mod_proxy_fcgi, in which case the CGI scripts' headers are in the table corresponding to always and not in the default table.

As a consequence, Header edit Content-Security-Policy "$" "; report-uri /report-uri/" should be replaced by Header always edit Content-Security-Policy "$" "; report-uri /report-uri/".

Hope this can help someone someday!