Re: [PATCH 7 of 8 v5] sslutil: propagate return value ssl.PROTOCOL_SSLv23 from protocolsettings()

2020-06-01 Thread Yuya Nishihara 
On Mon, 01 Jun 2020 05:28:18 +0200, Manuel Jacob wrote:
> # HG changeset patch
> # User Manuel Jacob 
> # Date 1590970587 -7200
> #  Mon Jun 01 02:16:27 2020 +0200
> # Node ID 64807e560eedc6c2571d34ffb7bd2f7e356dd606
> # Parent  7576507bfe5ea28ab6d496d532bb9b453998ca35
> # EXP-Topic require_modern_ssl
> sslutil: propagate return value ssl.PROTOCOL_SSLv23 from protocolsettings()

> -def protocolsettings(minimumprotocol):
> +def ssloptions(minimumprotocol):
>  """Resolve the protocol for a config value.
>  
>  Returns a tuple of (protocol, options) which are values used by 
> SSLContext.
> @@ -268,7 +265,7 @@ def protocolsettings(minimumprotocol):
>  # There is no guarantee this attribute is defined on the module.
>  options |= getattr(ssl, 'OP_NO_COMPRESSION', 0)
>  
> -return ssl.PROTOCOL_SSLv23, options
> +return options

Need to move/rephrase the following comment:

# Despite its name, PROTOCOL_SSLv23 selects the highest protocol
# that both ends support, including TLS protocols.
#
# 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.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 7 of 8 v5] sslutil: propagate return value ssl.PROTOCOL_SSLv23 from protocolsettings()

2020-05-31 Thread Manuel Jacob 
# HG changeset patch
# User Manuel Jacob 
# Date 1590970587 -7200
#  Mon Jun 01 02:16:27 2020 +0200
# Node ID 64807e560eedc6c2571d34ffb7bd2f7e356dd606
# Parent  7576507bfe5ea28ab6d496d532bb9b453998ca35
# EXP-Topic require_modern_ssl
sslutil: propagate return value ssl.PROTOCOL_SSLv23 from protocolsettings()

Also, protocolsettings() was renamed to ssloptions() to reflect that only the
options are returned.

Also, the now unused 'protocol' entry of the config dict was removed.

diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -79,8 +79,6 @@ def _hostsettings(ui, hostname):
 b'disablecertverification': False,
 # Whether the legacy [hostfingerprints] section has data for this host.
 b'legacyfingerprint': False,
-# PROTOCOL_* constant to use for SSLContext.__init__.
-b'protocol': None,
 # String representation of minimum protocol to be used for UI
 # presentation.
 b'minimumprotocolui': None,
@@ -126,7 +124,7 @@ def _hostsettings(ui, hostname):
 minimumprotocol = b'tls1.0'
 
 s[b'minimumprotocolui'] = minimumprotocol
-s[b'protocol'], s[b'ctxoptions'] = protocolsettings(minimumprotocol)
+s[b'ctxoptions'] = ssloptions(minimumprotocol)
 
 ciphers = ui.config(b'hostsecurity', b'ciphers')
 ciphers = ui.config(b'hostsecurity', b'%s:ciphers' % bhostname, ciphers)
@@ -228,14 +226,13 @@ def _hostsettings(ui, hostname):
 # user).
 s[b'verifymode'] = ssl.CERT_NONE
 
-assert s[b'protocol'] is not None
 assert s[b'ctxoptions'] is not None
 assert s[b'verifymode'] is not None
 
 return s
 
 
-def protocolsettings(minimumprotocol):
+def ssloptions(minimumprotocol):
 """Resolve the protocol for a config value.
 
 Returns a tuple of (protocol, options) which are values used by SSLContext.
@@ -268,7 +265,7 @@ def protocolsettings(minimumprotocol):
 # There is no guarantee this attribute is defined on the module.
 options |= getattr(ssl, 'OP_NO_COMPRESSION', 0)
 
-return ssl.PROTOCOL_SSLv23, options
+return options
 
 
 def wrapsocket(sock, keyfile, certfile, ui, serverhostname=None):
@@ -323,7 +320,7 @@ def wrapsocket(sock, keyfile, certfile, 
 # bundle with a specific CA cert removed. If the system/default CA bundle
 # is loaded and contains that removed CA, you've just undone the user's
 # choice.
-sslcontext = ssl.SSLContext(settings[b'protocol'])
+sslcontext = ssl.SSLContext(ssl.PROTOCOL_SSLv23)
 sslcontext.options |= settings[b'ctxoptions']
 sslcontext.verify_mode = settings[b'verifymode']
 
@@ -520,7 +517,8 @@ def wrapserversocket(
 _(b'referenced certificate file (%s) does not exist') % f
 )
 
-protocol, options = protocolsettings(b'tls1.0')
+protocol = ssl.PROTOCOL_SSLv23
+options = ssloptions(b'tls1.0')
 
 # This config option is intended for use in tests only. It is a giant
 # footgun to kill security. Don't define it.

___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel