[PATCH 3 of 6 V6] sslutil: config option to specify TLS protocol version

[Gregory Szorc]

On Sun, Jul 17, 2016 at 1:25 AM, Yuya Nishihara <yuya at tcha.org> wrote:

> On Fri, 15 Jul 2016 22:18:46 -0700, Gregory Szorc wrote:
> > # HG changeset patch
> > # User Gregory Szorc <gregory.szorc at gmail.com>
> > # Date 1468554442 25200
> > #      Thu Jul 14 20:47:22 2016 -0700
> > # Node ID 862344cd55472d3d5233abcb829766c947f7df90
> > # Parent  da2cb958a05e738646878e564ad109cfb478ac13
> > sslutil: config option to specify TLS protocol version
>
> The change LGTM per the last discussion. Queued 3, 5, and 6, thanks.
>
> >      # 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.
>
> Perhaps this comment should be moved to protocolsettings() where
> PROTOCOL_SSLv23 is used.
>
> > +    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
>
> Nit: OP_NO_TLSv1_1 requires OpenSSL 1.0.1+. Better to drop 'tls1.2' from
> the
> configprotocols if unsupported?
>
> https://docs.python.org/2.7/library/ssl.html#ssl.OP_NO_TLSv1_1


It looks like a number of constants aren't always defined on the ssl
module: https://hg.python.org/cpython/file/345ec7455b75/Modules/_ssl.c#l4129

I guess I'll send some follow-ups to this series to make the code more
robust.


>
>
> >      if modernssl:
> >          # We /could/ use create_default_context() here since it doesn't
> load
> > -        # CAs when configured for client auth.
> > -        sslcontext = SSLContext(ssl.PROTOCOL_SSLv23)
> > -        # SSLv2 and SSLv3 are broken. Ban them outright.
> > -        sslcontext.options |= OP_NO_SSLv2 | OP_NO_SSLv3
> > -        # Prevent CRIME
> > -        sslcontext.options |= getattr(ssl, 'OP_NO_COMPRESSION', 0)
> > +        # CAs when configured for client auth. However, it is
> hard-coded to
> > +        # use ssl.PROTOCOL_SSLv23m which may not be appropriate here.
>
> Fixed s/PROTOCOL_SSLv23m/PROTOCOL_SSLv23/
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20160717/a57c930e/attachment.html>