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

Yuya Nishihara yuya at tcha.org
Sun Jul 17 04:25:19 EDT 2016


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

>      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/


More information about the Mercurial-devel mailing list