[PATCH 3 of 3] [RFC] sslutil: config option to specify TLS protocol version
Augie Fackler
raf at durin42.com
Tue Jul 12 12:42:26 EDT 2016
On Wed, Jul 06, 2016 at 11:31:37PM -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc at gmail.com>
> # Date 1467873039 25200
> # Wed Jul 06 23:30:39 2016 -0700
> # Node ID 961c91677f9b2394b3b8140c28059d5fc5eee00f
> # Parent 66a5adf42ac7bc0031e772b407403e809e67fb6e
> [RFC] sslutil: config option to specify TLS protocol version
queued patches 1 and 2, thanks. Comments inline below for some questions.
>
> 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 professionals recommend avoiding TLS 1.0 if possible.
> PCI DSS 3.1 "strongly encourages" the use of TLS 1.2.
>
> Known attacks like BEAST and POODLE exist against TLS 1.0 (although
> mitigations are available and properly configured servers aren't
> vulnerable).
>
> Web browsers and many other applications continue to support TLS
> 1.0 for compatibility reasons.
>
> 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 version Mercurial should use.
> By providing this option, people 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 the special
> "hostsecurity._default_" config option to hold settings that apply
> to all hosts. Only the "protocol" sub-option currently works.
>
> Open Issues:
>
> * Need to write tests
> * Is "hostsecurity._default_" really the best way to do that?
> * Do we want to support both the explicit and "+" values. I could be
> convinced that only the e.g. "tls1.1+" values are needed. Why would
> you only choose TLS 1.1 when TLS 1.2 is available? That feels like
> a footgun that could accidentally lock you into old security when
> TLS 1.3 is available.
I think that it should really just be a "minimum version" of TLS, so
tls1.1 means 1.1 or newer. The footgun aspect is important.
> * Support "hostsecurity._default_:verifycertsfile"? (would effectively
> be web.cacerts for clients)
> * Should we issue a warning if TLS 1.0 is used? What if TLS 1.0 is all
> that is available?
I'm not sure. Browsers are still allowing TLS1, so we probably should
just silently allow it by default too.
> * Should we stop using TLS 1.0 if TLS 1.1+ is available by default?
I don't know that I'd go this far yet.
> * Should we also provide a config option that sets value for
> SSLContext.set_ciphers()?
Probably.
>
> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
> --- a/mercurial/help/config.txt
> +++ b/mercurial/help/config.txt
> @@ -1021,16 +1021,24 @@ 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``.
>
> +``protocol``
> + Defines the channel encryption protocol to use.
> +
> + By default, the highest TLS version 1.0 or greater protocol supported by
> + both client and server is used. This option can be used to explicitly use
> + a specific protocol version or to bump the minimum version that would
> + otherwise be used.
> +
> ``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.
> diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
> --- a/mercurial/sslutil.py
> +++ b/mercurial/sslutil.py
> @@ -24,24 +24,26 @@ 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.0+',
> + 'tls1.1',
> + 'tls1.1+',
> + 'tls1.2',
> + '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:
> @@ -140,25 +142,65 @@ def _hostsettings(ui, hostname):
> # 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
> +
> + # Allow 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)))
> +
> + protocol = ui.config('hostsecurity', '_default_:protocol', 'tls1.0+')
> + validateprotocol(protocol, '_default_:protocol')
> +
> + protocol = ui.config('hostsecurity', '%s:protocol' % hostname, protocol)
> + validateprotocol(protocol, '%s.protocol' % hostname)
> +
> + # Legacy ssl module only supports TLS 1.0.
> + if not modernssl:
> + if protocol in ('tls1.0', 'tls1.0+'):
> + s['protocol'] = ssl.PROTOCOL_TLSv1
> + s['ctxoptions'] = 0
> + else:
> + 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'))
> 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
> + if protocol == 'tls1.0':
> + s['protocol'] = ssl.PROTOCOL_TLSv1
> + s['ctxoptions'] = 0
> + elif protocol == 'tls1.0+':
> + s['protocol'] = ssl.PROTOCOL_SSLv23
> + s['ctxoptions'] = ssl.OP_NO_SSLv2 | ssl.OP_NO_SSLv3
> + elif protocol == 'tls1.1':
> + s['protocol'] = ssl.PROTOCOL_TLSv1_1
> + s['ctxoptions'] = 0
> + elif protocol == 'tls1.1+':
> + s['protocol'] = ssl.PROTOCOL_SSLv23
> + s['ctxoptions'] = (ssl.OP_NO_SSLv2 | ssl.OP_NO_SSLv3 |
> + ssl.OP_NO_TLSv1_1)
> + elif protocol == 'tls1.2':
> + s['protocol'] = ssl.PROTOCOL_TLSv1_2
> + s['ctxoptions'] = 0
> + elif protocol == 'tls1.2+':
> + s['protocol'] = ssl.PROTOCOL_SSLv23
> + s['ctxoptions'] = (ssl.OP_NO_SSLv2 | ssl.OP_NO_SSLv3 |
> + ssl.OP_NO_TLSv1_1 | ssl.OP_NO_TLSv1_2)
> + else:
> + raise error.Abort(_('this should not happen'))
>
> # 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') % (
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
More information about the Mercurial-devel
mailing list