[PATCH] hgweb: support Content Security Policy

[Gregory Szorc]

On Wed, Jan 11, 2017 at 10:53 AM, Sean Farley <sean at farley.io> wrote:

> Gregory Szorc <gregory.szorc at gmail.com> writes:
>
> > # HG changeset patch
> > # User Gregory Szorc <gregory.szorc at gmail.com>
> > # Date 1484120228 28800
> > #      Tue Jan 10 23:37:08 2017 -0800
> > # Node ID 113293954736e020d29e8e48aa3e01657ec853f3
> > # Parent  79314c9a79b3aa033b6f79d066b97d7157ecac33
> > hgweb: support Content Security Policy
> >
> > Content-Security-Policy (CSP) is a web security feature that allows
> > servers to declare what loaded content is allowed to do. For example,
> > a policy can prevent loading of images, JavaScript, CSS, etc unless
> > the source of that content is whitelisted (by hostname, URI scheme,
> > hashes of content, etc). It's a nifty security feature that provides
> > extra mitigation against some attacks, notably XSS.
> >
> > Mitigation against these attacks is important for Mercurial because
> > hgweb renders repository data, which is commonly untrusted. While we
> > make attempts to escape things, etc, there's the possibility that
> > malicious data could be injected into the site content. If this happens
> > today, the full power of the web browser is available to that
> > malicious content. A restrictive CSP policy (defined by the server
> > operator and sent in an HTTP header which is outside the control of
> > malicious content), could restrict browser capabilities and mitigate
> > security problems posed by malicious data.
> >
> > CSP works by emitting an HTTP header declaring the policy that browsers
> > should apply. Ideally, this header would be emitted by a layer above
> > Mercurial (likely the HTTP server doing the WSGI "proxying"). This
> > works for some CSP policies, but not all.
> >
> > For example, policies to allow inline JavaScript may require setting
> > a "nonce" attribute on <script>. This attribute value must be unique
> > and non-guessable. And, the value must be present in the HTTP header
> > and the HTML body. This means that coordinating the value between
> > Mercurial and another HTTP server could be difficult: it is much
> > easier to generate and emit the nonce in a central location.
> >
> > This commit introduces support for emitting a
> > Content-Security-Policy header from hgweb. A config option defines
> > the header value. If present, the header is emitted. A special
> > "%nonce%" syntax in the value triggers generation of a nonce and
> > inclusion in <script> elements in templates. The inclusion of a
> > nonce does not occur unless "%nonce%" is present. This makes this
> > commit completely backwards compatible and the feature opt-in.
> >
> > The nonce is a type 4 UUID, which is the flavor that is randomly
> > generated. It has 122 random bits, which should be plenty to satisfy
> > the guarantees of a nonce.
>
> Looks pretty good to me. I'd appreciate if someone else with web server
> experience to gave it a look over. One small nit below.
>
> > diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
> > --- a/mercurial/help/config.txt
> > +++ b/mercurial/help/config.txt
> > @@ -2084,6 +2084,20 @@ The full set of options is:
> >      Name or email address of the person in charge of the repository.
> >      (default: ui.username or ``$EMAIL`` or "unknown" if unset or empty)
> >
> > +``csp``
> > +    Send a ``Content-Security-Policy`` HTTP header with this value.
> > +
> > +    The value may contain a special string ``%nonce%``, which will be
> replaced
> > +    by a randomly-generated one-time use value. If the value contains
> > +    ``%nonce%``, ``web.cache`` will be disabled, as caching undermines
> the
> > +    one-time property of the nonce. This nonce will also be inserted
> into
> > +    ``<script>`` elements containing inline JavaScript.
> > +
> > +    Note: lots of HTML content sent by the server is derived from
> repository
> > +    data. Please consider the potential for malicious repository data to
> > +    "inject" itself into generated HTML content as part of your security
> > +    threat model.
> > +
> >  ``deny_push``
> >      Whether to deny pushing to the repository. If empty or not set,
> >      push is not denied. If the special value ``*``, all remote users are
> > diff --git a/mercurial/hgweb/common.py b/mercurial/hgweb/common.py
> > --- a/mercurial/hgweb/common.py
> > +++ b/mercurial/hgweb/common.py
> > @@ -8,9 +8,11 @@
> >
> >  from __future__ import absolute_import
> >
> > +import base64
> >  import errno
> >  import mimetypes
> >  import os
> > +import uuid
> >
> >  from .. import (
> >      encoding,
> > @@ -199,3 +201,22 @@ def caching(web, req):
> >      if req.env.get('HTTP_IF_NONE_MATCH') == tag:
> >          raise ErrorResponse(HTTP_NOT_MODIFIED)
> >      req.headers.append(('ETag', tag))
> > +
> > +def cspvalues(ui):
> > +    """Obtain the Content-Security-Policy header and nonce value.
> > +
> > +    Returns a 2-tuple of the CSP header value and the nonce value.
> > +
> > +    First value is ``None`` if CSP isn't enabled. Second value is
> ``None``
> > +    if CSP isn't enabled or if the CSP header doesn't need a nonce.
> > +    """
> > +    # Don't allow untrusted CSP setting since it be disable protections
> > +    # from a trusted/global source.
> > +    csp = ui.config('web', 'csp', untrusted=False)
> > +    nonce = None
> > +
> > +    if csp and '%nonce%' in csp:
>
> Since we just talked about this recently, should we test 'if csp is not
> None'?
>

I'm not sure exactly which conversation you are referring to, but
ui.config() will return a string or None. A Python string can only be true
if it is non-empty. So an explicit identity check against None should not
be necessary (and is unPythonic).

(Or am I missing a scenario where ui.config() returns something other than
a string or None?)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170111/6fb52334/attachment.html>