[PATCH 2 of 5 V3] sslutil: config option to specify TLS protocol version
Pierre-Yves David
pierre-yves.david at ens-lyon.org
Thu Jul 14 21:18:33 EDT 2016
That patch seems fine to me, but I've a test failure on my test machine
(works fine locally)
--- /home/marmoute/mercurial-testing/tests/test-https.t
+++ /home/marmoute/mercurial-testing/tests/test-https.t.err
@@ -394,6 +394,7 @@
$ killdaemons.py hg0.pid
$ killdaemons.py proxy.pid
+ $TESTTMP.sh: line 184: 28996 Terminated tinyproxy.py
$HGPORT1 localhost > proxy.log < /dev/null 2>&1
$ killdaemons.py hg2.pid
#if sslcontext
ERROR: test-https.t output changed
Can you look into it?
Patch 1 is pushed, see comment about Patch 3 in the other email.
On 07/14/2016 06:50 AM, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc at gmail.com>
> # Date 1468468118 25200
> # Wed Jul 13 20:48:38 2016 -0700
> # Node ID 6a6d56e1391ff7e1468ef1b44b7e4c5cbe406f7b
> # Parent 944f37469b94ea956566a037f9b7ae136a99b119
> sslutil: config option to specify TLS protocol version
>
> Currently, Mercurial will use TLS 1.0 or newer when connecting to
> remote servers, selecting the highest TLS version supported by both
> peers. On older Pythons, only TLS 1.0 is available. On newer Pythons,
> TLS 1.1 and 1.2 should be available.
>
> Security-minded people may want to not take any risks running
> TLS 1.0 (or even TLS 1.1). This patch gives those people a config
> option to explicitly control which TLS versions Mercurial should use.
> By providing this option, one can require newer TLS versions
> before they are formally deprecated by Mercurial/Python/OpenSSL/etc
> and lower their security exposure. This option also provides an
> easy mechanism to change protocol policies in Mercurial. If there
> is a 0-day and TLS 1.0 is completely broken, we can act quickly
> without changing much code.
>
> Because setting the minimum TLS protocol is something you'll likely
> want to do globally, this patch introduces a global config option under
> [hostsecurity] for that purpose.
>
> wrapserversocket() has been taught a hidden config option to define
> the explicit protocol to use. This is queried in this function and
> not passed as an argument because I don't want to expose this dangerous
> option as part of the Python API. There is a risk someone could footgun
> themselves. But the config option is a devel option, has a warning
> comment, and I doubt most people are using `hg serve` to run a
> production HTTPS server (I would have something not Mercurial/Python
> handle TLS). If this is problematic, we can go back to using a
> custom extension in tests to coerce the server into bad behavior.
>
> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
> --- a/mercurial/help/config.txt
> +++ b/mercurial/help/config.txt
> @@ -995,20 +995,32 @@ For example::
>
> [hostfingerprints]
> hg.intevation.de = fc:e2:8d:d9:51:cd:cb:c1:4d:18:6b:b7:44:8d:49:72:57:e6:cd:33
> hg.intevation.org = fc:e2:8d:d9:51:cd:cb:c1:4d:18:6b:b7:44:8d:49:72:57:e6:cd:33
>
> ``hostsecurity``
> ----------------
>
> -Used to specify per-host security settings.
> -
> -Options in this section have the form ``hostname``:``setting``. This allows
> -multiple settings to be defined on a per-host basis.
> +Used to specify global and per-host security settings for connecting to
> +other machines.
> +
> +The following options control default behavior for all hosts.
> +
> +``minimumprotocol``
> + Defines the minimum channel encryption protocol to use.
> +
> + By default, the highest version of TLS - 1.0 or greater - supported by
> + both client and server is used.
> +
> + Allowed values are: ``tls1.0`` (the default), ``tls1.1``, ``tls1.2``.
> +
> +Options in the ``[hostsecurity]`` section can have the form
> +``hostname``:``setting``. This allows multiple settings to be defined on a
> +per-host basis.
>
> The following per-host settings can be defined.
>
> ``fingerprints``
> A list of hashes of the DER encoded peer/remote certificate. Values have
> the form ``algorithm``:``fingerprint``. e.g.
> ``sha256:c3ab8ff13720e8ad9047dd39466b3c8974e592c2fa383d4a3960714caef0c4f2``.
>
> @@ -1021,16 +1033,20 @@ The following per-host settings can be d
> host and Mercurial will require the remote certificate to match one
> of the fingerprints specified. This means if the server updates its
> certificate, Mercurial will abort until a new fingerprint is defined.
> This can provide stronger security than traditional CA-based validation
> at the expense of convenience.
>
> This option takes precedence over ``verifycertsfile``.
>
> +``minimumprotocol``
> + This behaves like ``minimumprotocol`` as described above except it
> + only applies to the host on which it is defined.
> +
> ``verifycertsfile``
> Path to file a containing a list of PEM encoded certificates used to
> verify the server certificate. Environment variables and ``~user``
> constructs are expanded in the filename.
>
> The server certificate or the certificate's certificate authority (CA)
> must match a certificate from this file or certificate verification
> will fail and connections to the server will be refused.
> @@ -1053,16 +1069,23 @@ The following per-host settings can be d
>
> For example::
>
> [hostsecurity]
> hg.example.com:fingerprints = sha256:c3ab8ff13720e8ad9047dd39466b3c8974e592c2fa383d4a3960714caef0c4f2
> hg2.example.com:fingerprints = sha1:914f1aff87249c09b6859b88b1906d30756491ca, sha1:fc:e2:8d:d9:51:cd:cb:c1:4d:18:6b:b7:44:8d:49:72:57:e6:cd:33
> foo.example.com:verifycertsfile = /etc/ssl/trusted-ca-certs.pem
>
> +To change the default minimum protocol version to TLS 1.2 but to allow TLS 1.1
> +when connecting to ``hg.example.com``::
> +
> + [hostsecurity]
> + minimumprotocol = tls1.2
> + hg.example.com:minimumprotocol = tls1.1
> +
> ``http_proxy``
> --------------
>
> Used to access web-based Mercurial repositories through a HTTP
> proxy.
>
> ``host``
> Host name and (optional) port of the proxy server, for example
> diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
> --- a/mercurial/sslutil.py
> +++ b/mercurial/sslutil.py
> @@ -24,24 +24,23 @@ from . import (
> # Python 2.7.9+ overhauled the built-in SSL/TLS features of Python. It added
> # support for TLS 1.1, TLS 1.2, SNI, system CA stores, etc. These features are
> # all exposed via the "ssl" module.
> #
> # Depending on the version of Python being used, SSL/TLS support is either
> # modern/secure or legacy/insecure. Many operations in this module have
> # separate code paths depending on support in Python.
>
> -hassni = getattr(ssl, 'HAS_SNI', False)
> +configprotocols = set([
> + 'tls1.0',
> + 'tls1.1',
> + 'tls1.2',
> +])
>
> -try:
> - OP_NO_SSLv2 = ssl.OP_NO_SSLv2
> - OP_NO_SSLv3 = ssl.OP_NO_SSLv3
> -except AttributeError:
> - OP_NO_SSLv2 = 0x1000000
> - OP_NO_SSLv3 = 0x2000000
> +hassni = getattr(ssl, 'HAS_SNI', False)
>
> try:
> # ssl.SSLContext was added in 2.7.9 and presence indicates modern
> # SSL/TLS features are available.
> SSLContext = ssl.SSLContext
> modernssl = True
> _canloaddefaultcerts = util.safehasattr(SSLContext, 'load_default_certs')
> except AttributeError:
> @@ -131,34 +130,45 @@ def _hostsettings(ui, hostname):
> # ssl.CERT_* constant used by SSLContext.verify_mode.
> 'verifymode': None,
> # Defines extra ssl.OP* bitwise options to set.
> 'ctxoptions': None,
> }
>
> # Despite its name, PROTOCOL_SSLv23 selects the highest protocol
> # that both ends support, including TLS protocols. On legacy stacks,
> - # the highest it likely goes in TLS 1.0. On modern stacks, it can
> + # the highest it likely goes is TLS 1.0. On modern stacks, it can
> # support TLS 1.2.
> #
> # The PROTOCOL_TLSv* constants select a specific TLS version
> # only (as opposed to multiple versions). So the method for
> # supporting multiple TLS versions is to use PROTOCOL_SSLv23 and
> # disable protocols via SSLContext.options and OP_NO_* constants.
> # However, SSLContext.options doesn't work unless we have the
> # full/real SSLContext available to us.
> - if modernssl:
> - s['protocol'] = ssl.PROTOCOL_SSLv23
> - else:
> - s['protocol'] = ssl.PROTOCOL_TLSv1
>
> - # SSLv2 and SSLv3 are broken. We ban them outright.
> - # WARNING: ctxoptions doesn't have an effect unless the modern ssl module
> - # is available. Be careful when adding flags!
> - s['ctxoptions'] = OP_NO_SSLv2 | OP_NO_SSLv3
> + # Allow minimum TLS protocol to be specified in the config.
> + def validateprotocol(protocol, key):
> + if protocol not in configprotocols:
> + raise error.Abort(
> + _('unsupported protocol from hostsecurity.%s: %s') %
> + (key, protocol),
> + hint=_('valid protocols: %s') %
> + ' '.join(sorted(configprotocols)))
> +
> + key = 'minimumprotocol'
> + # Default to TLS 1.0+ as that is what browsers are currently doing.
> + protocol = ui.config('hostsecurity', key, 'tls1.0')
> + validateprotocol(protocol, key)
> +
> + key = '%s:minimumprotocol' % hostname
> + protocol = ui.config('hostsecurity', key, protocol)
> + validateprotocol(protocol, key)
> +
> + s['protocol'], s['ctxoptions'] = protocolsettings(protocol)
>
> # Look for fingerprints in [hostsecurity] section. Value is a list
> # of <alg>:<fingerprint> strings.
> fingerprints = ui.configlist('hostsecurity', '%s:fingerprints' % hostname,
> [])
> for fingerprint in fingerprints:
> if not (fingerprint.startswith(('sha1:', 'sha256:', 'sha512:'))):
> raise error.Abort(_('invalid fingerprint for %s: %s') % (
> @@ -241,16 +251,52 @@ def _hostsettings(ui, hostname):
> s['verifymode'] = ssl.CERT_NONE
>
> assert s['protocol'] is not None
> assert s['ctxoptions'] is not None
> assert s['verifymode'] is not None
>
> return s
>
> +def protocolsettings(protocol):
> + """Resolve the protocol and context options for a config value."""
> + if protocol not in configprotocols:
> + raise ValueError('protocol value not supported: %s' % protocol)
> +
> + # Legacy ssl module only supports up to TLS 1.0. Ideally we'd use
> + # PROTOCOL_SSLv23 and options to disable SSLv2 and SSLv3. However,
> + # SSLContext.options doesn't work in our implementation since we use
> + # a fake SSLContext on these Python versions.
> + if not modernssl:
> + if protocol != 'tls1.0':
> + raise error.Abort(_('current Python does not support protocol '
> + 'setting %s') % protocol,
> + hint=_('upgrade Python or disable setting since '
> + 'only TLS 1.0 is supported'))
> +
> + return ssl.PROTOCOL_TLSv1, 0
> +
> + # WARNING: returned options don't work unless the modern ssl module
> + # is available. Be careful when adding options here.
> +
> + # SSLv2 and SSLv3 are broken. We ban them outright.
> + options = ssl.OP_NO_SSLv2 | ssl.OP_NO_SSLv3
> +
> + if protocol == 'tls1.0':
> + # Defaults above are to use TLS 1.0+
> + pass
> + elif protocol == 'tls1.1':
> + options |= ssl.OP_NO_TLSv1
> + elif protocol == 'tls1.2':
> + options |= ssl.OP_NO_TLSv1 | ssl.OP_NO_TLSv1_1
> + else:
> + raise error.Abort(_('this should not happen'))
> +
> + return ssl.PROTOCOL_SSLv23, options
> +
> def wrapsocket(sock, keyfile, certfile, ui, serverhostname=None):
> """Add SSL/TLS to a socket.
>
> This is a glorified wrapper for ``ssl.wrap_socket()``. It makes sane
> choices based on what security options are available.
>
> In addition to the arguments supported by ``ssl.wrap_socket``, we allow
> the following additional arguments:
> @@ -297,28 +343,35 @@ def wrapsocket(sock, keyfile, certfile,
> # This is a no-op on old Python.
> sslcontext.load_default_certs()
> caloaded = True
> else:
> caloaded = False
>
> try:
> sslsocket = sslcontext.wrap_socket(sock, server_hostname=serverhostname)
> - except ssl.SSLError:
> + except ssl.SSLError as e:
> # If we're doing certificate verification and no CA certs are loaded,
> # that is almost certainly the reason why verification failed. Provide
> # a hint to the user.
> # Only modern ssl module exposes SSLContext.get_ca_certs() so we can
> # only show this warning if modern ssl is available.
> if (caloaded and settings['verifymode'] == ssl.CERT_REQUIRED and
> modernssl and not sslcontext.get_ca_certs()):
> ui.warn(_('(an attempt was made to load CA certificates but none '
> 'were loaded; see '
> 'https://mercurial-scm.org/wiki/SecureConnections for '
> 'how to configure Mercurial to avoid this error)\n'))
> + # Try to print more helpful error messages for known failures.
> + if util.safehasattr(e, 'reason'):
> + if e.reason == 'UNSUPPORTED_PROTOCOL':
> + ui.warn(_('(could not negotiate a common protocol; see '
> + 'https://mercurial-scm.org/wiki/SecureConnections '
> + 'for how to configure Mercurial to avoid this '
> + 'error)\n'))
> raise
>
> # check if wrap_socket failed silently because socket had been
> # closed
> # - see http://bugs.python.org/issue13721
> if not sslsocket.cipher():
> raise error.Abort(_('ssl connection failed'))
>
> @@ -341,17 +394,47 @@ def wrapserversocket(sock, ui, certfile=
>
> ``cafile`` defines the path to certificate authorities.
>
> ``requireclientcert`` specifies whether to require client certificates.
>
> Typically ``cafile`` is only defined if ``requireclientcert`` is true.
> """
> if modernssl:
> - sslcontext = ssl.create_default_context(purpose=ssl.Purpose.CLIENT_AUTH)
> + protocol, options = protocolsettings('tls1.0')
> +
> + # This config option is intended for use in tests only. It is a giant
> + # footgun to kill security. Don't define it.
> + exactprotocol = ui.config('devel', 'serverexactprotocol')
> + if exactprotocol == 'tls1.0':
> + protocol = ssl.PROTOCOL_TLSv1
> + elif exactprotocol == 'tls1.1':
> + protocol = ssl.PROTOCOL_TLSv1_1
> + elif exactprotocol == 'tls1.2':
> + protocol = ssl.PROTOCOL_TLSv1_2
> + elif exactprotocol:
> + raise error.Abort(_('invalid value for serverexactprotocol: %s') %
> + exactprotocol)
> +
> + # The protocol is pinned as SSLContext construction time and can't be
> + # changed after the fact (even though the class allows you to assign
> + # the variable it has no effect). Unfortunately,
> + # ``create_default_context`` doesn't allow us to pass a custom
> + # protocol.
> + if exactprotocol:
> + sslcontext = ssl.SSLContext(protocol)
> + sslcontext.options |= options
> + # Emulate create_default_context() behavior.
> + sslcontext.options |= ssl.OP_CIPHER_SERVER_PREFERENCE
> + sslcontext.options |= ssl.OP_SINGLE_DH_USE
> + sslcontext.options |= ssl.OP_SINGLE_ECDH_USE
> + else:
> + sslcontext = ssl.create_default_context(
> + purpose=ssl.Purpose.CLIENT_AUTH)
> +
> else:
> sslcontext = SSLContext(ssl.PROTOCOL_TLSv1)
>
> # We don't need to apply options to the context here because either
> # a) create_default_context() has appropriate defaults and we don't have
> # any custom settings to apply b) legacy ssl module is already using the
> # appropriate protocol.
>
> diff --git a/tests/test-https.t b/tests/test-https.t
> --- a/tests/test-https.t
> +++ b/tests/test-https.t
> @@ -387,18 +387,78 @@ Test https with cert problems through pr
> abort: error: *certificate verify failed* (glob)
> [255]
> $ http_proxy=http://localhost:$HGPORT1/ hg -R copy-pull pull \
> > --config web.cacerts="$CERTSDIR/pub-expired.pem" https://localhost:$HGPORT2/
> pulling from https://localhost:$HGPORT2/
> abort: error: *certificate verify failed* (glob)
> [255]
>
> + $ killdaemons.py hg0.pid
> + $ killdaemons.py proxy.pid
> + $ killdaemons.py hg2.pid
> +
> +#if sslcontext
> +Start servers running supported TLS versions
> +
> + $ cd test
> + $ hg serve -p $HGPORT -d --pid-file=../hg0.pid --certificate=$PRIV \
> + > --config devel.serverexactprotocol=tls1.0
> + $ cat ../hg0.pid >> $DAEMON_PIDS
> + $ hg serve -p $HGPORT1 -d --pid-file=../hg1.pid --certificate=$PRIV \
> + > --config devel.serverexactprotocol=tls1.1
> + $ cat ../hg1.pid >> $DAEMON_PIDS
> + $ hg serve -p $HGPORT2 -d --pid-file=../hg2.pid --certificate=$PRIV \
> + > --config devel.serverexactprotocol=tls1.2
> + $ cat ../hg2.pid >> $DAEMON_PIDS
> + $ cd ..
> +
> +Clients talking same TLS versions work
> +
> + $ P="$CERTSDIR" hg --config hostsecurity.minimumprotocol=tls1.0 id https://localhost:$HGPORT/
> + 5fed3813f7f5
> + $ P="$CERTSDIR" hg --config hostsecurity.minimumprotocol=tls1.1 id https://localhost:$HGPORT1/
> + 5fed3813f7f5
> + $ P="$CERTSDIR" hg --config hostsecurity.minimumprotocol=tls1.2 id https://localhost:$HGPORT2/
> + 5fed3813f7f5
> +
> +Clients requiring newer TLS version than what server supports fail
> +
> + $ P="$CERTSDIR" hg --config hostsecurity.minimumprotocol=tls1.1 id https://localhost:$HGPORT/
> + (could not negotiate a common protocol; see https://mercurial-scm.org/wiki/SecureConnections for how to configure Mercurial to avoid this error)
> + abort: error: *unsupported protocol* (glob)
> + [255]
> + $ P="$CERTSDIR" hg --config hostsecurity.minimumprotocol=tls1.2 id https://localhost:$HGPORT/
> + (could not negotiate a common protocol; see https://mercurial-scm.org/wiki/SecureConnections for how to configure Mercurial to avoid this error)
> + abort: error: *unsupported protocol* (glob)
> + [255]
> + $ P="$CERTSDIR" hg --config hostsecurity.minimumprotocol=tls1.2 id https://localhost:$HGPORT1/
> + (could not negotiate a common protocol; see https://mercurial-scm.org/wiki/SecureConnections for how to configure Mercurial to avoid this error)
> + abort: error: *unsupported protocol* (glob)
> + [255]
> +
> +The per-host config option overrides the default
> +
> + $ P="$CERTSDIR" hg id https://localhost:$HGPORT/ \
> + > --config hostsecurity.minimumprotocol=tls1.2 \
> + > --config hostsecurity.localhost:minimumprotocol=tls1.0
> + 5fed3813f7f5
> +
> +The per-host config option by itself works
> +
> + $ P="$CERTSDIR" hg id https://localhost:$HGPORT/ \
> + > --config hostsecurity.localhost:minimumprotocol=tls1.2
> + (could not negotiate a common protocol; see https://mercurial-scm.org/wiki/SecureConnections for how to configure Mercurial to avoid this error)
> + abort: error: *unsupported protocol* (glob)
> + [255]
>
> $ killdaemons.py hg0.pid
> + $ killdaemons.py hg1.pid
> + $ killdaemons.py hg2.pid
> +#endif
>
> #if sslcontext
>
> Start hgweb that requires client certificates:
>
> $ cd test
> $ hg serve -p $HGPORT -d --pid-file=../hg0.pid --certificate=$PRIV \
> > --config devel.servercafile=$PRIV --config devel.serverrequirecert=true
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
--
Pierre-Yves David
More information about the Mercurial-devel
mailing list