[PATCH 3 of 3] [RFC] sslutil: config option to specify TLS protocol version
Gregory Szorc
gregory.szorc at gmail.com
Tue Jul 12 17:44:12 EDT 2016
On Tue, Jul 12, 2016 at 9:42 AM, Augie Fackler <raf at durin42.com> wrote:
> 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.
>
OK. Will change this.
>
> > * 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.
>
I just asked Eric Rescorla "should Mercurial drop TLS 1.0 support" and his
response was "if you can get away with it." Basically a large portion of
the Internet still doesn't run TLS 1.1+ and dropping TLS 1.0 would make
that Internet unavailable. Just how many Mercurial servers don't run TLS
1.1+, I have no idea and there is no easy way to find out.
I'll throw out an idea. We could make TLS 1.1+ the default (on modern
Python versions since legacy Python only supports TLS 1.0) and provide an
option to allow TLS 1.0+. When connecting to a server that doesn't support
TLS 1.1+, we can suggest users try the legacy config. When TLS 1.0 is
insecure, we can drop the config option to allow it. There is also a
related discussion about whether we should print warnings on legacy Pythons
that don't support TLS 1.1+.
>
> > * 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20160712/96b25347/attachment.html>
More information about the Mercurial-devel
mailing list