Patchwork [8,of,8,v5] sslutil: properly detect which TLS versions are supported by the ssl module

login
register
mail settings
Submitter Manuel Jacob
Date June 1, 2020, 3:28 a.m.
Message ID <fbd7fcc81958b551b6dc.1590982099@tmp>
Download mbox | patch
Permalink /patch/46445/
State New
Headers show

Comments

Manuel Jacob - June 1, 2020, 3:28 a.m.
# HG changeset patch
# User Manuel Jacob <me@manueljacob.de>
# Date 1590976314 -7200
#      Mon Jun 01 03:51:54 2020 +0200
# Node ID fbd7fcc81958b551b6dcbc26cc22fbdd0a0cdc33
# Parent  64807e560eedc6c2571d34ffb7bd2f7e356dd606
# EXP-Topic require_modern_ssl
sslutil: properly detect which TLS versions are supported by the ssl module

For the record, I contacted the CPython developers to remark that
unconditionally defining ssl.PROTOCOL_TLSv1_1 / ssl.PROTOCOL_TLSv1_2 is
problematic:
https://github.com/python/cpython/commit/6e8cda91d92da72800d891b2fc2073ecbc134d98#r39569316

Patch

diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -44,19 +44,17 @@  configprotocols = {
 
 hassni = getattr(ssl, 'HAS_SNI', False)
 
-# TLS 1.1 and 1.2 may not be supported if the OpenSSL Python is compiled
-# against doesn't support them.
-# FIXME: Since CPython commit 6e8cda91d92da72800d891b2fc2073ecbc134d98
-# individual TLS versions can be turned on and off, and the
-# ssl.PROTOCOL_TLSv1_* constants are always defined.
-# This means that, on unusual configurations, the following dict may contain
-# too many entries. A proper fix would be to check ssl.HAS_TLSv* where
-# available (Python 3.7+). Before that, this module should be proofed against
-# all possible combinations.
-supportedprotocols = {b'tls1.0'}
-if util.safehasattr(ssl, b'PROTOCOL_TLSv1_1'):
+# ssl.HAS_TLSv1* are preferred to check support but they were added in Python
+# 3.7. Prior to CPython commit 6e8cda91d92da72800d891b2fc2073ecbc134d98
+# (backported to the 3.7 branch), ssl.PROTOCOL_TLSv1_1 / ssl.PROTOCOL_TLSv1_2
+# were defined only if compiled against a OpenSSL version with TLS 1.1 / 1.2
+# support. At the mentioned commit, they were unconditionally defined.
+supportedprotocols = set()
+if getattr(ssl, 'HAS_TLSv1', util.safehasattr(ssl, 'PROTOCOL_TLSv1')):
+    supportedprotocols.add(b'tls1.0')
+if getattr(ssl, 'HAS_TLSv1_1', util.safehasattr(ssl, 'PROTOCOL_TLSv1_1')):
     supportedprotocols.add(b'tls1.1')
-if util.safehasattr(ssl, b'PROTOCOL_TLSv1_2'):
+if getattr(ssl, 'HAS_TLSv1_2', util.safehasattr(ssl, 'PROTOCOL_TLSv1_2')):
     supportedprotocols.add(b'tls1.2')