apache / incubator-pagespeed-mod

Apache module for rewriting web pages to reduce latency and bandwidth.
http://modpagespeed.com
Apache License 2.0
696 stars 158 forks source link

ModPagespeedHonorCsp ignores that unsafe-eval is not an allowed script source #1758

Open helgeklein opened 6 years ago

helgeklein commented 6 years ago

I just upgraded to mod_pagespeed 1.13.35.2. I hoped that I could finally get rid of script-src 'unsafe-eval' in my content security policy. So I added ModPagespeedHonorCsp on to my configuration. After removing unsafe-eval from the CSP, restarting Apache, and, for good measure, deleting pagespeed's disk cache, Chrome's dev tools console shows errors like the following:

Uncaught EvalError: Refused to evaluate a string as JavaScript because 'unsafe-eval' is not an allowed source of script in the following Content Security Policy directive: "script-src 'unsafe-inline' https: data:".

Comparing the count of evals before and after the change, it turns out that not a single eval has been removed my mod_pagespeed. I am referring to constructs like the following:

eval(mod_pagespeed_9iFvSnQ84y)

One of the sites running on the server is uberagent.com in case you want to take a look. I reverted the change, of course.

The configuration shown at /pagespeed_admin/config is as shown below.

Version: 14: on

Filters ah Add Head cw Collapse Whitespace cc Combine Css jc Combine Javascript gp Convert Gif to Png jp Convert Jpeg to Progressive jw Convert Jpeg To Webp mc Convert Meta Tags pj Convert Png to Jpeg ws When converting images to WebP, prefer lossless conversions ec Cache Extend Css ei Cache Extend Images es Cache Extend Scripts fc Fallback Rewrite Css if Flatten CSS Imports hw Flushes html ci Inline Css ii Inline Images il Inline @import to Link ji Inline Javascript js Jpeg Subsampling ll Lazyload Images rj Recompress Jpeg rp Recompress Png rw Recompress Webp ri Resize Images cf Rewrite Css jm Rewrite External Javascript jj Rewrite Inline Javascript cu Rewrite Style Attributes With Url cp Strip Image Color Profiles md Strip Image Meta Data

Options DisableRewriteOnNoTransform (drnt) False EnableRewriting (e) 1 FetchHttps (fhs) enable FileCacheInodeLimit (afcl) 500000 FileCachePath (afcp) /var/cache/mod_pagespeed/ FileCacheSizeKb (afc) 10240000 HonorCsp (hcsp) True LogDir (ald) /var/log/pagespeed SslCertDirectory (assld) /etc/ssl/certs StatisticsLogging (asle) True

Domain Lawyer

Invalidation Timestamp: Sun, 04 Mar 2018 01:09:18 GMT (1520125758000)

oschaaf commented 6 years ago

How is the CSP header added to the response? (E.g. via php?)

helgeklein commented 6 years ago

The CSP header is added via Apache's security.conf. I can post the file; I am also happy to answer any other questions you might have on the configuration.

oschaaf commented 6 years ago

I have a hunch.. One workaround you could try is to add any CSP directives as a html meta tag as well, to see if that improves CSP-awareness of the module.

At some point, while working on something else, I discovered by accident that the module may miss CSP headers depending on how they are associated to the internal apache request. This could explain your issue. (Also #1696 should fix this)

helgeklein commented 6 years ago

Adding the CSP via a meta tag to the HTML head - in addition to the Apache configuration - worked.

Nevertheless, I am going to revert and wait for the integration of a fix in mod_pagespeed because I have multiple sites on the server and do not want to modify each site's config.

Thanks for your help!

helgeklein commented 6 years ago

Not sure if I should close this or not. I'll leave it open until an updated version of mod_pagespeed is available. Hope that is OK.

jmarantz commented 6 years ago

Leaving it open makes sense, although it seems like this might be complex to fix if the filter that adds in these headers runs after mod_pagespeed, preventing mod_pagespeed from seeing the headers at all.

If that's the case, filter-order tweaking can add a fair amount of complexity and risk.

helgeklein commented 6 years ago

If it is possible to change the filter order, maybe some guidance/documentation would help?

jmarantz commented 6 years ago

Changing filter order in Apache generally, and certainly in mod_pagespeed, is done in C, around the vicinity of this code: https://github.com/apache/incubator-pagespeed-mod/blob/8196fc74847e85085fa6f310d658842261acb329/pagespeed/apache/mod_instaweb.cc#L1152

Moving these around may cause other issues, though it's possible we could contrive to change only the order with respect to mod_security.

oschaaf commented 6 years ago

Another thought -- would it make sense to make header reading more configurable per header-type? e.g.:

ModPagespeedReadResponseHeaderPhase HeaderName Phase [optional RestrictiveUrlWildCard]

Where Phase would be one of default or final. Not sure I like the proposed option name and phase names here, but I wonder what the thoughts are about the general idea of delegating responsibility to configuration here.

(Internally, we could still consider defaulting the phase to final for Content-Security-Policy when respecting it is enabled, and no explicit directives are set).

Implementing the new phase for header capturing (final) would be pretty generic and require a similar hook in all ports, I think, though I feel some more thought may be needed to get consistent behavior across html and resource responses when doing this.

RajaHassan commented 6 years ago

can anybody tell that is this issue being fixed ?

oschaaf commented 6 years ago

@RajaHassan As far as I know, no-one is working to fix this right now. Not sure if this helps, but there may be a workaround by adding meta tags in html: https://github.com/apache/incubator-pagespeed-mod/issues/1758#issuecomment-371285918

blue-eyed-devil commented 5 years ago

I had same issue. Once moved CPS header to the very top it stopped throwing error about eval. I assume it has something to do with order in which is CPS header sent