1333615 - Switch to Mercurial's built-in CSP support

Status: RESOLVED FIXED

(Developer Services :: Mercurial: hg.mozilla.org, defect)

Description

[Gregory Szorc [:gps]]

+++ This bug was initially created as a clone of Bug #1319172 +++

Mercurial 4.1 adds built-in CSP support via the web.csp config option. We should switch from defining CSP in httpd to this.

The main benefit of switching is that we'll no longer need unsafe-inline for scripts, as Mercurial knows how to emit nonces.

However, some hosted content on hg.mozilla.org (such as the reftest analyzer) may also rely on inline content. And it doesn't go through Mercurial's templating layer, so it won't know what nonce to use. We may have to have httpd override the default/Mercurial provided CSP header with a custom one allowing unsafe-inline for specific paths.

Comment 1

[Gregory Szorc [:gps]]

Bug 1333929 will make this a bit harder since we have a special case for reftest analyzer. But, we can continue to overwrite whatever CSP header value hg sets in httpd for reftest analyzer and any other special snowflakes until the snow is melted.

Comment 2

[Gregory Szorc [:gps]]

This is now unblocked.

Comment 3

[Gregory Szorc [:gps]]

Lemme see if I can crank this out real quick...

Comment 4

[Gregory Szorc [:gps]]

Attachment: ansible/hg-web: emit CSP header from Mercurial (bug 1333615);

Mercurial 4.1 (which we just upgraded to) has built-in support for
emitting a Content-Security-Policy header. The big benefit over having
the parent HTTP server add the header is that Mercurial can generate
and use a nonce in inline <script> so the policy doesn't have to use
"unsafe-inline."

This commit moves the generation of the CSP header from httpd to hg.

As part of this, the CSP header was added to HTTP responses that
it previously wasn't. Despite me implementing the CSP feature
upstream, I forgot to special case protocol requests from emitting
the CSP policy like we did at Mozilla (to minimize bytes). Derp.
I may fix this in 4.2.

For now, let's pave over it by unsetting the header for identified
protocol requests. This required a clean-up block of sorts in the
vhost config. Apache httpd does have some wonky behavior when it
comes to order of execution of directives (especially when <If>
and <Location> are involved. But our tests running an actual
Apache httpd process using the same version used in production
show the desired behavior. So I'll take it.

Review commit: https://reviewboard.mozilla.org/r/127016/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/127016/

Comment 5

[Gregory Szorc [:gps]]

Attachment: hgserver: explicitly test for <script> content (bug 1333615);

In preparation for eventually adding a nonce, let's tweak the test so
we can demonstrate change in behavior.

Review commit: https://reviewboard.mozilla.org/r/127018/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/127018/

Comment 6

[Gregory Szorc [:gps]]

Good news: we can switch to Mercurial's native CSP easily and nonces work.

Bad news: Mercurial's default templates use inline event handlers on various HTML elements, which CSP disallows unless using unsafe-inline. So, we can't move away from unsafe-inline just yet.

I may hack on upstream patches to remove the inline event handlers. We should be able to backport them to hg.mozilla.org easily (since we maintain a shadow copy of Mercurial's templates and can update them without upgrading Mercurial).

I'll get another bug on file to track unsafe-inline removal explicitly. I think moving the CSP generation to Mercurial is a step in the right direction. So let's get that reviewed and deployed.

Comment 7

[Gregory Szorc [:gps]]

Wait - I may have been misled by Firefox's devtools :|

When I test a local HTTP (not HTTPS) server as such:

  $ hg serve --hgmo --config "web.csp=default-src 'none'; connect-src 'self' https://bugzilla.mozilla.org/; img-src 'self'; script-src 'self' 'nonce-%nonce%'; style-src 'self' 'unsafe-inline'"

Then load http://10.251.30.244:8000/graph the developer console says the following:

18:38:15.729 Content Security Policy: The page’s settings blocked the loading of a resource at self (“script-src http://10.251.30.244:8000 'nonce-8xyvd8egSN6xqxAWRyZ5qA'”). Source: onsubmit attribute on DIV element. 1 graph

The thing is, there is no "onsubmit" in Mercurial's templates. And the inline JS on that page seems to work fine. Huh. Chrome doesn't report any CSP warnings. I wonder if this is some Firefox add-on injecting content in such a way that it violates the CSP policy. I really wish Firefox's devtools told me which element that JS handler was attached to. Is there a way to search for event handlers in devtools?

Comment 8

[Gregory Szorc [:gps]]

Attachment: ansible/hg-web: use nonce for script-src in CSP (bug 1333615);

We previously swapped in Mercurial's native CSP support while preserving
existing behavior. This commit "upgrades" our CSP policy to replace
"unsafe-inline" for script-src to use a nonce (which Mercurial generates
when it sees the special "%nonce%" string in its CSP policy string).

The test changes demonstrate a nonce being added to an inline <script>.
Sadly, we don't verify the nonce in the header exactly matches what is
in the body. Doing this in a .t test is a bit hard. In this case, I
think we can trust that upstream Mercurial is doing the right thing. So
I'm fine not explicitly testing this. FWIW, I did verify it manually.

Review commit: https://reviewboard.mozilla.org/r/127020/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/127020/

Comment 9

[Gregory Szorc [:gps]]

I've uploaded the patch to switch to nonces in case glob or someone else wants to experiment.

  $ hg clone https://hg.mozilla.org/hgcustom/version-control-tools
  $ hg serve --config extensions.hgmo=version-control-tools/hgext/hgmo --config "web.csp=..." serve --hgmo

Comment 10

[Gregory Szorc [:gps]]

False positive on CSP errors. I blame a Firefox add-on. We should be clear to move forward with this.

Comment 11

[Gregory Szorc [:gps]]

Comment on attachment 8855137 [details]
ansible/hg-web: emit CSP header from Mercurial (bug 1333615);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/127016/diff/1-2/

Comment 12

[Gregory Szorc [:gps]]

Comment on attachment 8855138 [details]
hgserver: explicitly test for <script> content (bug 1333615);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/127018/diff/1-2/

Comment 13

[Gregory Szorc [:gps]]

Comment on attachment 8855139 [details]
ansible/hg-web: use nonce for script-src in CSP (bug 1333615);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/127020/diff/1-2/

Comment 14

[:glob ✱]

Comment on attachment 8855137 [details]
ansible/hg-web: emit CSP header from Mercurial (bug 1333615);

https://reviewboard.mozilla.org/r/127016/#review132014

Comment 15

[:glob ✱]

Comment on attachment 8855138 [details]
hgserver: explicitly test for <script> content (bug 1333615);

https://reviewboard.mozilla.org/r/127018/#review132348

Comment 16

[:glob ✱]

Comment on attachment 8855139 [details]
ansible/hg-web: use nonce for script-src in CSP (bug 1333615);

https://reviewboard.mozilla.org/r/127020/#review132350

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/ffb95353c3f1
ansible/hg-web: emit CSP header from Mercurial ; r=glob
https://hg.mozilla.org/hgcustom/version-control-tools/rev/f836f41d4e78
hgserver: explicitly test for <script> content ; r=glob
https://hg.mozilla.org/hgcustom/version-control-tools/rev/fb6202dbcb4a
ansible/hg-web: use nonce for script-src in CSP ; r=glob

Comment 18

[Gregory Szorc [:gps]]

And deployed.

https://mozilla.github.io/http-observatory-website/analyze.html?host=hg.mozilla.org now gives us a B rating.