[PATCH 3 of 6 V6] sslutil: config option to specify TLS protocol version
gregory.szorc at gmail.com
Sun Jul 17 13:31:40 EDT 2016
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
> configprotocols if unsupported?
It looks like a number of constants aren't always defined on the ssl
I guess I'll send some follow-ups to this series to make the code more
> > if modernssl:
> > # We /could/ use create_default_context() here since it doesn't
> > - # 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...
More information about the Mercurial-devel