<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Jul 12, 2016 at 9:42 AM, Augie Fackler <span dir="ltr"><<a href="mailto:raf@durin42.com" target="_blank">raf@durin42.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="">On Wed, Jul 06, 2016 at 11:31:37PM -0700, Gregory Szorc wrote:<br>
> # HG changeset patch<br>
> # User Gregory Szorc <<a href="mailto:gregory.szorc@gmail.com">gregory.szorc@gmail.com</a>><br>
> # Date 1467873039 25200<br>
> # Wed Jul 06 23:30:39 2016 -0700<br>
> # Node ID 961c91677f9b2394b3b8140c28059d5fc5eee00f<br>
> # Parent 66a5adf42ac7bc0031e772b407403e809e67fb6e<br>
> [RFC] sslutil: config option to specify TLS protocol version<br>
<br>
</span>queued patches 1 and 2, thanks. Comments inline below for some questions.<br>
<div><div class="h5"><br>
><br>
> Currently, Mercurial will use TLS 1.0 or newer when connecting to<br>
> remote servers, selecting the highest TLS version supported by both<br>
> peers. On older Pythons, only TLS 1.0 is available. On newer Pythons,<br>
> TLS 1.1 and 1.2 should be available.<br>
><br>
> Security professionals recommend avoiding TLS 1.0 if possible.<br>
> PCI DSS 3.1 "strongly encourages" the use of TLS 1.2.<br>
><br>
> Known attacks like BEAST and POODLE exist against TLS 1.0 (although<br>
> mitigations are available and properly configured servers aren't<br>
> vulnerable).<br>
><br>
> Web browsers and many other applications continue to support TLS<br>
> 1.0 for compatibility reasons.<br>
><br>
> Security-minded people may want to not take any risks running<br>
> TLS 1.0 (or even TLS 1.1). This patch gives those people a config<br>
> option to explicitly control which TLS version Mercurial should use.<br>
> By providing this option, people can require newer TLS versions<br>
> before they are formally deprecated by Mercurial/Python/OpenSSL/etc<br>
> and lower their security exposure. This option also provides an<br>
> easy mechanism to change protocol policies in Mercurial. If there<br>
> is a 0-day and TLS 1.0 is completely broken, we can act quickly<br>
> without changing much code.<br>
><br>
> Because setting the minimum TLS protocol is something you'll likely<br>
> want to do globally, this patch introduces the special<br>
> "hostsecurity._default_" config option to hold settings that apply<br>
> to all hosts. Only the "protocol" sub-option currently works.<br>
><br>
> Open Issues:<br>
><br>
> * Need to write tests<br>
> * Is "hostsecurity._default_" really the best way to do that?<br>
> * Do we want to support both the explicit and "+" values. I could be<br>
> convinced that only the e.g. "tls1.1+" values are needed. Why would<br>
> you only choose TLS 1.1 when TLS 1.2 is available? That feels like<br>
> a footgun that could accidentally lock you into old security when<br>
> TLS 1.3 is available.<br>
<br>
</div></div>I think that it should really just be a "minimum version" of TLS, so<br>
tls1.1 means 1.1 or newer. The footgun aspect is important.<br></blockquote><div><br></div><div>OK. Will change this.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class=""><br>
> * Support "hostsecurity._default_:verifycertsfile"? (would effectively<br>
> be web.cacerts for clients)<br>
> * Should we issue a warning if TLS 1.0 is used? What if TLS 1.0 is all<br>
> that is available?<br>
<br>
</span>I'm not sure. Browsers are still allowing TLS1, so we probably should<br>
just silently allow it by default too. <br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class=""><br>
> * Should we stop using TLS 1.0 if TLS 1.1+ is available by default?<br>
<br>
</span>I don't know that I'd go this far yet.<br></blockquote><div><br><div>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.<br><br></div>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+.<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class=""><br>
> * Should we also provide a config option that sets value for<br>
> SSLContext.set_ciphers()?<br>
<br>
</span>Probably.<br>
<div><div class="h5"><br>
><br>
> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt<br>
> --- a/mercurial/help/config.txt<br>
> +++ b/mercurial/help/config.txt<br>
> @@ -1021,16 +1021,24 @@ The following per-host settings can be d<br>
> host and Mercurial will require the remote certificate to match one<br>
> of the fingerprints specified. This means if the server updates its<br>
> certificate, Mercurial will abort until a new fingerprint is defined.<br>
> This can provide stronger security than traditional CA-based validation<br>
> at the expense of convenience.<br>
><br>
> This option takes precedence over ``verifycertsfile``.<br>
><br>
> +``protocol``<br>
> + Defines the channel encryption protocol to use.<br>
> +<br>
> + By default, the highest TLS version 1.0 or greater protocol supported by<br>
> + both client and server is used. This option can be used to explicitly use<br>
> + a specific protocol version or to bump the minimum version that would<br>
> + otherwise be used.<br>
> +<br>
> ``verifycertsfile``<br>
> Path to file a containing a list of PEM encoded certificates used to<br>
> verify the server certificate. Environment variables and ``~user``<br>
> constructs are expanded in the filename.<br>
><br>
> The server certificate or the certificate's certificate authority (CA)<br>
> must match a certificate from this file or certificate verification<br>
> will fail and connections to the server will be refused.<br>
> diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py<br>
> --- a/mercurial/sslutil.py<br>
> +++ b/mercurial/sslutil.py<br>
> @@ -24,24 +24,26 @@ from . import (<br>
> # Python 2.7.9+ overhauled the built-in SSL/TLS features of Python. It added<br>
> # support for TLS 1.1, TLS 1.2, SNI, system CA stores, etc. These features are<br>
> # all exposed via the "ssl" module.<br>
> #<br>
> # Depending on the version of Python being used, SSL/TLS support is either<br>
> # modern/secure or legacy/insecure. Many operations in this module have<br>
> # separate code paths depending on support in Python.<br>
><br>
> -hassni = getattr(ssl, 'HAS_SNI', False)<br>
> +configprotocols = set([<br>
> + 'tls1.0',<br>
> + 'tls1.0+',<br>
> + 'tls1.1',<br>
> + 'tls1.1+',<br>
> + 'tls1.2',<br>
> + 'tls1.2+',<br>
> +])<br>
><br>
> -try:<br>
> - OP_NO_SSLv2 = ssl.OP_NO_SSLv2<br>
> - OP_NO_SSLv3 = ssl.OP_NO_SSLv3<br>
> -except AttributeError:<br>
> - OP_NO_SSLv2 = 0x1000000<br>
> - OP_NO_SSLv3 = 0x2000000<br>
> +hassni = getattr(ssl, 'HAS_SNI', False)<br>
><br>
> try:<br>
> # ssl.SSLContext was added in 2.7.9 and presence indicates modern<br>
> # SSL/TLS features are available.<br>
> SSLContext = ssl.SSLContext<br>
> modernssl = True<br>
> _canloaddefaultcerts = util.safehasattr(SSLContext, 'load_default_certs')<br>
> except AttributeError:<br>
> @@ -140,25 +142,65 @@ def _hostsettings(ui, hostname):<br>
> # support TLS 1.2.<br>
> #<br>
> # The PROTOCOL_TLSv* constants select a specific TLS version<br>
> # only (as opposed to multiple versions). So the method for<br>
> # supporting multiple TLS versions is to use PROTOCOL_SSLv23 and<br>
> # disable protocols via SSLContext.options and OP_NO_* constants.<br>
> # However, SSLContext.options doesn't work unless we have the<br>
> # full/real SSLContext available to us.<br>
> - if modernssl:<br>
> - s['protocol'] = ssl.PROTOCOL_SSLv23<br>
> +<br>
> + # Allow TLS protocol to be specified in the config.<br>
> + def validateprotocol(protocol, key):<br>
> + if protocol not in configprotocols:<br>
> + raise error.Abort(<br>
> + _('unsupported protocol from hostsecurity.%s: %s') %<br>
> + (key, protocol),<br>
> + hint=_('valid protocols: %s') %<br>
> + ' '.join(sorted(configprotocols)))<br>
> +<br>
> + protocol = ui.config('hostsecurity', '_default_:protocol', 'tls1.0+')<br>
> + validateprotocol(protocol, '_default_:protocol')<br>
> +<br>
> + protocol = ui.config('hostsecurity', '%s:protocol' % hostname, protocol)<br>
> + validateprotocol(protocol, '%s.protocol' % hostname)<br>
> +<br>
> + # Legacy ssl module only supports TLS 1.0.<br>
> + if not modernssl:<br>
> + if protocol in ('tls1.0', 'tls1.0+'):<br>
> + s['protocol'] = ssl.PROTOCOL_TLSv1<br>
> + s['ctxoptions'] = 0<br>
> + else:<br>
> + raise error.Abort(_('current Python does not support protocol '<br>
> + 'setting %s') % protocol,<br>
> + hint=_('upgrade Python or disable setting since '<br>
> + 'only TLS 1.0 is supported'))<br>
> else:<br>
> - s['protocol'] = ssl.PROTOCOL_TLSv1<br>
> -<br>
> - # SSLv2 and SSLv3 are broken. We ban them outright.<br>
> - # WARNING: ctxoptions doesn't have an effect unless the modern ssl module<br>
> - # is available. Be careful when adding flags!<br>
> - s['ctxoptions'] = OP_NO_SSLv2 | OP_NO_SSLv3<br>
> + if protocol == 'tls1.0':<br>
> + s['protocol'] = ssl.PROTOCOL_TLSv1<br>
> + s['ctxoptions'] = 0<br>
> + elif protocol == 'tls1.0+':<br>
> + s['protocol'] = ssl.PROTOCOL_SSLv23<br>
> + s['ctxoptions'] = ssl.OP_NO_SSLv2 | ssl.OP_NO_SSLv3<br>
> + elif protocol == 'tls1.1':<br>
> + s['protocol'] = ssl.PROTOCOL_TLSv1_1<br>
> + s['ctxoptions'] = 0<br>
> + elif protocol == 'tls1.1+':<br>
> + s['protocol'] = ssl.PROTOCOL_SSLv23<br>
> + s['ctxoptions'] = (ssl.OP_NO_SSLv2 | ssl.OP_NO_SSLv3 |<br>
> + ssl.OP_NO_TLSv1_1)<br>
> + elif protocol == 'tls1.2':<br>
> + s['protocol'] = ssl.PROTOCOL_TLSv1_2<br>
> + s['ctxoptions'] = 0<br>
> + elif protocol == 'tls1.2+':<br>
> + s['protocol'] = ssl.PROTOCOL_SSLv23<br>
> + s['ctxoptions'] = (ssl.OP_NO_SSLv2 | ssl.OP_NO_SSLv3 |<br>
> + ssl.OP_NO_TLSv1_1 | ssl.OP_NO_TLSv1_2)<br>
> + else:<br>
> + raise error.Abort(_('this should not happen'))<br>
><br>
> # Look for fingerprints in [hostsecurity] section. Value is a list<br>
> # of <alg>:<fingerprint> strings.<br>
> fingerprints = ui.configlist('hostsecurity', '%s:fingerprints' % hostname,<br>
> [])<br>
> for fingerprint in fingerprints:<br>
> if not (fingerprint.startswith(('sha1:', 'sha256:', 'sha512:'))):<br>
> raise error.Abort(_('invalid fingerprint for %s: %s') % (<br>
</div></div>> _______________________________________________<br>
> Mercurial-devel mailing list<br>
> <a href="mailto:Mercurial-devel@mercurial-scm.org">Mercurial-devel@mercurial-scm.org</a><br>
> <a href="https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel" rel="noreferrer" target="_blank">https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel</a><br>
</blockquote></div><br></div></div>