Skip to content

[3.6] bpo-35925: Skip SSL tests that fail due to weak external certs or old TLS (GH-13124) #13252

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 30 additions & 8 deletions Lib/test/test_nntplib.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import functools
import contextlib
import os.path
import re

from test import support
from nntplib import NNTP, GroupInfo
import nntplib
Expand All @@ -22,6 +24,13 @@
TIMEOUT = 30
certfile = os.path.join(os.path.dirname(__file__), 'keycert3.pem')

if ssl is not None:
SSLError = ssl.SSLError
else:
class SSLError(Exception):
"""Non-existent exception class when we lack SSL support."""
reason = "This will never be raised."

# TODO:
# - test the `file` arg to more commands
# - test error conditions
Expand Down Expand Up @@ -262,14 +271,21 @@ def is_connected():
return False
return True

with self.NNTP_CLASS(self.NNTP_HOST, timeout=TIMEOUT, usenetrc=False) as server:
self.assertTrue(is_connected())
self.assertTrue(server.help())
self.assertFalse(is_connected())

with self.NNTP_CLASS(self.NNTP_HOST, timeout=TIMEOUT, usenetrc=False) as server:
server.quit()
self.assertFalse(is_connected())
try:
with self.NNTP_CLASS(self.NNTP_HOST, timeout=TIMEOUT, usenetrc=False) as server:
self.assertTrue(is_connected())
self.assertTrue(server.help())
self.assertFalse(is_connected())

with self.NNTP_CLASS(self.NNTP_HOST, timeout=TIMEOUT, usenetrc=False) as server:
server.quit()
self.assertFalse(is_connected())
except SSLError as ssl_err:
# matches "[SSL: DH_KEY_TOO_SMALL] dh key too small"
if re.search(r'(?i)KEY.TOO.SMALL', ssl_err.reason):
raise unittest.SkipTest(f"Got {ssl_err} connecting "
f"to {self.NNTP_HOST!r}")
raise


NetworkedNNTPTestsMixin.wrap_methods()
Expand All @@ -290,6 +306,12 @@ def setUpClass(cls):
try:
cls.server = cls.NNTP_CLASS(cls.NNTP_HOST, timeout=TIMEOUT,
usenetrc=False)
except SSLError as ssl_err:
# matches "[SSL: DH_KEY_TOO_SMALL] dh key too small"
if re.search(r'(?i)KEY.TOO.SMALL', ssl_err.reason):
raise unittest.SkipTest(f"{cls} got {ssl_err} connecting "
f"to {cls.NNTP_HOST!r}")
raise
except EOFError:
raise unittest.SkipTest(f"{cls} got EOF error on connecting "
f"to {cls.NNTP_HOST!r}")
Expand Down
35 changes: 35 additions & 0 deletions Lib/test/test_ssl.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import asyncore
import weakref
import platform
import re
import functools
try:
import ctypes
Expand Down Expand Up @@ -145,6 +146,38 @@ def f(*args, **kwargs):
else:
return func

def skip_if_openssl_cnf_minprotocol_gt_tls1(func):
"""Skip a test if the OpenSSL config MinProtocol is > TLSv1.

OS distros with an /etc/ssl/openssl.cnf and MinProtocol set often do so to
require TLSv1.2 or higher (Debian Buster). Some of our tests for older
protocol versions will fail under such a config.

Alternative workaround: Run this test in a process with
OPENSSL_CONF=/dev/null in the environment.
"""
@functools.wraps(func)
def f(*args, **kwargs):
openssl_cnf = os.environ.get("OPENSSL_CONF", "/etc/ssl/openssl.cnf")
try:
with open(openssl_cnf, "r") as config:
for line in config:
match = re.match(r"MinProtocol\s*=\s*(TLSv\d+\S*)", line)
if match:
tls_ver = match.group(1)
if tls_ver > "TLSv1":
raise unittest.SkipTest(
"%s has MinProtocol = %s which is > TLSv1." %
(openssl_cnf, tls_ver))
except (EnvironmentError, UnicodeDecodeError) as err:
# no config file found, etc.
if support.verbose:
sys.stdout.write("\n Could not scan %s for MinProtocol: %s\n"
% (openssl_cnf, err))
return func(*args, **kwargs)
return f


needs_sni = unittest.skipUnless(ssl.HAS_SNI, "SNI support needed for this test")


Expand Down Expand Up @@ -2614,6 +2647,7 @@ def test_protocol_sslv2(self):
client_options=ssl.OP_NO_TLSv1)

@skip_if_broken_ubuntu_ssl
@skip_if_openssl_cnf_minprotocol_gt_tls1
def test_protocol_sslv23(self):
"""Connecting to an SSLv23 server with various client options"""
if support.verbose:
Expand Down Expand Up @@ -2691,6 +2725,7 @@ def test_protocol_tlsv1(self):
@skip_if_broken_ubuntu_ssl
@unittest.skipUnless(hasattr(ssl, "PROTOCOL_TLSv1_1"),
"TLS version 1.1 not supported.")
@skip_if_openssl_cnf_minprotocol_gt_tls1
def test_protocol_tlsv1_1(self):
"""Connecting to a TLSv1.1 server with various client options.
Testing against older TLS versions."""
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Skip specific nntplib and ssl networking tests when they would otherwise fail due to a modern OS or distro with a default OpenSSL policy of rejecting connections to servers with weak certificates or disabling TLS below TLSv1.2.