Sanitize OR add Content-Security-Policy for SVGs

mr.baileys → made their first commit to this issue’s fork.

* Converted the patch to a MR, rerolled for 11.x
* Added a test to confirm the presence of the CSP header for file downloads.

I'm not sure we should be adding the CSP header (Content-Security-Policy: "default-src 'none') for none image/svg+xml downloads, as that seems out of scope for this issue and might break existing functionality.

@mr.baileys: It should probably at least work properly with the "csp" module. An easy solution would be to the set the priority of the respond event such that our event handler is guaranteed to run after their event handler. Then we could check whether the header is already set and do nothing in this case and just hope that the user knows what they're doing.

Re-titling, since the scope of this issue has shifted to adding a content security policy header, rather than sanitizing the file.

@rgpublic: makes sense, I added a condition so that the default CSP header is only set if none has been set earlier. I think it's up to CSP and other contrib modules to alter/set handler priorities?

@mr.baileys: Yes, you are probably right. Currently they don't do this (I just checked, I see no explicit priority in their code), but I hope/assume their code gets after ours anyway because this is in core and their code is in contrib. If not, they could easily solve this by adding a priority - so: fine for me.

There's also seckit → which also can set the CSP headers.

The evil thing is: If we set sth. here and anyone keeps overwriting it they can inadvertently create a security hole because people can upload SVGs with a XSS payload again :-(

But I don't want to spoil the party. Glad this is finally moving forward.

In a perfect world, core would provide a CSP service that anyone can *add* their stuff to if they want so nothing gets destroyed except intentionally. But this is probably way out-of-scope for this issue...

It's quite an unfortunate situtation, but OTOH seckit / csp etc. are not modules anyone not savvy with these security issues will probably install lightheartedly. So perhaps we can just assume IF they change sth. they should know that then THEY have full responsibility over what security problems arise. Perhaps these contrib module owners should be informed that this is happening and add an appropriate readme note or sth. to their installation instructions...

Just my 0,02€

There's also seckit which also can set the CSP headers.

Can we set the CSP heasers on both public and private served files? Is Seckit able to add CSP headers on files?

@xmacinfo: for public files, I think the only way to add the header is through via web server directives (MR here includes a change to .htaccess for this reason)

👋 CSP module maintainer here.
CSP module provides APIs for modules to amend a site's policy → . In most cases it's pretty straightforward, but gets more involved for cases where it's not just adding an additional external domain to a directive. The goal of the module is that anyone can install the module for a security improvement, that it's easily configurable for site builders to add exceptions not captured automatically (& ideally harden the default config with basic knowledge), but also flexible for developers to handle any more complex uses (and to do so safely without breaking other site functionality).
Module subscribers with the same priority will execute after core, so it's the module's responsibility to preserve/amend/replace any value previously set by core.

CSP has its own issue for applying a different policy to private files ( ✨ Provide different CSP policy for private files Active ), though there hasn't been any work done on it so far. If a special value is added to core for SVGs, then I'll update the module to match. If there's consensus on what should be added to core, I'll probably have a new release of the module before it's committed to core 🙂.
Alternately, the CSP module would be a good way to evaluate a potential solution (at least for private files, I'm not sure about the module altering htaccess), with users who have opted in to using a policy on their site. It would have to be an optional feature for the current major version, but could be enabled by default for new modules installs (and a site status message could prompt existing sites to try enabling it).

This issue for core would add a static policy that can be configured in a service parameter: 📌 Add a default CSP and clickjacking defence and minimal API for CSP to core Active
Instead of providing a single policy set, maybe it should be keyed policies to allow for additional cases (e.g. default, private-files, svg, etc, each with report-only and enforced values).