#23 Skip test_ssl and test_asyncio tests failing with OpenSSL 1.1.1
Merged 5 months ago by cstratak. Opened 6 months ago by vstinner.
rpms/ vstinner/python35 skip_test_ssl  into  master

@@ -0,0 +1,90 @@ 

+ bpo-36576: Skip test_ssl and test_asyncio tests failing with OpenSSL 1.1.1

+ 

+ Some test_ssl and test_asyncio are written for OpenSSL 1.0 and TLS

+ 1.0, but fail with OpenSSL 1.1.1 and TLS 1.3.

+ 

+ Fixing these needs require to backport new ssl flags like

+ ssl.OP_NO_TLSv1_3 or ssl.OP_NO_COMPRESSION which cannot be done in a

+ minor 3.5.x release. Moreover, it is not really worth it: the code

+ works fine, issues are in the tests.

+ 

+ Backport of: https://github.com/python/cpython/pull/12694

+ 

+ Resolves: rhbz#1685609

+ 

+ diff --git a/Lib/test/test_asyncio/test_events.py b/Lib/test/test_asyncio/test_events.py

+ index 492a84a2313b..b23889b20dea 100644

+ --- a/Lib/test/test_asyncio/test_events.py

+ +++ b/Lib/test/test_asyncio/test_events.py

+ @@ -38,6 +38,12 @@

+      from asyncio import test_support as support

+  

+  

+ +if ssl is not None:

+ +    IS_OPENSSL_1_1_1 = ssl.OPENSSL_VERSION_INFO >= (1, 1, 1)

+ +else:

+ +    IS_OPENSSL_1_1_1 = False

+ +

+ +

+  def data_file(filename):

+      if hasattr(support, 'TEST_HOME_DIR'):

+          fullname = os.path.join(support.TEST_HOME_DIR, filename)

+ @@ -1145,6 +1151,7 @@ def test_legacy_create_unix_server_ssl_verify_failed(self):

+              self.test_create_unix_server_ssl_verify_failed()

+  

+      @unittest.skipIf(ssl is None, 'No ssl module')

+ +    @unittest.skipIf(IS_OPENSSL_1_1_1, "bpo-36576: fail on OpenSSL 1.1.1")

+      def test_create_server_ssl_match_failed(self):

+          proto = MyProto(loop=self.loop)

+          server, host, port = self._make_ssl_server(

+ diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py

+ index 6c78601b4589..30a64ee0a4c7 100644

+ --- a/Lib/test/test_ssl.py

+ +++ b/Lib/test/test_ssl.py

+ @@ -25,6 +25,7 @@

+  HOST = support.HOST

+  IS_LIBRESSL = ssl.OPENSSL_VERSION.startswith('LibreSSL')

+  IS_OPENSSL_1_1 = not IS_LIBRESSL and ssl.OPENSSL_VERSION_INFO >= (1, 1, 0)

+ +IS_OPENSSL_1_1_1 = not IS_LIBRESSL and ssl.OPENSSL_VERSION_INFO >= (1, 1, 1)

+  

+  

+  def data_file(*name):

+ @@ -857,6 +858,7 @@ def test_ciphers(self):

+              ctx.set_ciphers("^$:,;?*'dorothyx")

+  

+      @skip_if_broken_ubuntu_ssl

+ +    @unittest.skipIf(IS_OPENSSL_1_1_1, "bpo-36576: fail on OpenSSL 1.1.1")

+      def test_options(self):

+          ctx = ssl.SSLContext(ssl.PROTOCOL_TLSv1)

+          # OP_ALL | OP_NO_SSLv2 | OP_NO_SSLv3 is the default value

+ @@ -3047,6 +3049,7 @@ def test_version_basic(self):

+                  self.assertIs(s.version(), None)

+  

+          @unittest.skipUnless(ssl.HAS_ECDH, "test requires ECDH-enabled OpenSSL")

+ +        @unittest.skipIf(IS_OPENSSL_1_1_1, "bpo-36576: fail on OpenSSL 1.1.1")

+          def test_default_ecdh_curve(self):

+              # Issue #21015: elliptic curve-based Diffie Hellman key exchange

+              # should be enabled by default on SSL contexts.

+ @@ -3176,6 +3179,7 @@ def test_selected_alpn_protocol_if_server_uses_alpn(self):

+              self.assertIs(stats['client_alpn_protocol'], None)

+  

+          @unittest.skipUnless(ssl.HAS_ALPN, "ALPN support needed for this test")

+ +        @unittest.skipIf(IS_OPENSSL_1_1_1, "bpo-36576: fail on OpenSSL 1.1.1")

+          def test_alpn_protocols(self):

+              server_protocols = ['foo', 'bar', 'milkshake']

+              protocol_tests = [

+ @@ -3356,6 +3360,7 @@ def cb_wrong_return_type(ssl_sock, server_name, initial_context):

+              self.assertEqual(cm.exception.reason, 'TLSV1_ALERT_INTERNAL_ERROR')

+              self.assertIn("TypeError", stderr.getvalue())

+  

+ +        @unittest.skipIf(IS_OPENSSL_1_1_1, "bpo-36576: fail on OpenSSL 1.1.1")

+          def test_shared_ciphers(self):

+              server_context = ssl.SSLContext(ssl.PROTOCOL_TLSv1)

+              server_context.load_cert_chain(SIGNED_CERTFILE)

+ diff --git a/Misc/NEWS.d/next/Tests/2019-04-05-10-34-29.bpo-36576.7Cp2kK.rst b/Misc/NEWS.d/next/Tests/2019-04-05-10-34-29.bpo-36576.7Cp2kK.rst

+ new file mode 100644

+ index 000000000000..4d15bdf42796

+ --- /dev/null

+ +++ b/Misc/NEWS.d/next/Tests/2019-04-05-10-34-29.bpo-36576.7Cp2kK.rst

+ @@ -0,0 +1 @@

+ +Skip test_ssl and test_asyncio tests failing with OpenSSL 1.1.1.

file modified
+10 -1

@@ -85,7 +85,7 @@ 

  #global prerel ...

  %global upstream_version %{general_version}%{?prerel}

  Version: %{general_version}%{?prerel:~%{prerel}}

- Release: 1%{?dist}

+ Release: 2%{?dist}

  License: Python

  

  # Whether to use RPM build wheels from the python-{pip,setuptools}-wheel package

@@ -401,6 +401,11 @@ 

  # https://bugzilla.redhat.com/show_bug.cgi?id=1652843

  Patch315: 00315-test_email-mktime.patch

  

+ # 00322 #

+ # Skip test_ssl and test_asyncio tests failing with OpenSSL 1.1.1

+ # https://bugzilla.redhat.com/show_bug.cgi?id=1685609

+ Patch322: 00322-test_ssl-skip-openssl111.patch

+ 

  # (New patches go here ^^^)

  #

  # When adding new patches to "python" and "python3" in Fedora, EL, etc.,

@@ -527,6 +532,7 @@ 

  %patch273 -p1

  %patch290 -p1

  %patch315 -p1

+ %patch322 -p1

  

  # Currently (2010-01-15), http://docs.python.org/library is for 2.6, and there

  # are many differences between 2.6 and the Python 3 library.

@@ -1043,6 +1049,9 @@ 

  # ======================================================

  

  %changelog

+ * Tue Apr 02 2019 Victor Stinner <vstinner@redhat.com> - 3.5.7-2

+ - Skip test_ssl and test_asyncio tests failing with OpenSSL 1.1.1 (rhbz#1685609)

+ 

  * Tue Mar 19 2019 Miro Hrončok <mhroncok@redhat.com> - 3.5.7-1

  - Update to 3.5.7

  

rebased onto 9000839

6 months ago

Shall we skip if openssl version is >= 1.1.1? Is that checkable?

Shall we skip if openssl version is >= 1.1.1? Is that checkable?

Sure, done. I skipped the tests on OpenSSL >= 1.1.0.

rebased onto 49ec440

6 months ago

I proposed this change upstream:
https://github.com/python/cpython/pull/12694

I am not sure that it will be accepted. The Python 3.5 release manager usually only shows up just before a 3.5.x release and I don't expect the PR to be reviewed nor merged soon.

I would prefer to not wait for the upstream PR to be merged. Fedora makes different practical compromises than upstream.

@churchyard: Would you mind to review the updated PR?

Metadata Update from @churchyard:
- Request assigned

6 months ago

On Monday.

A scratchbuild with openssl 1.1.1 would be appreciated, but if you don't have time, I'll get my own later.

A scratchbuild with openssl 1.1.1 would be appreciated, but if you don't have time, I'll get my own later.

Here you have: https://koji.fedoraproject.org/koji/taskinfo?taskID=33970861

diff --git a/python35.spec b/python35.spec
index d6a11e8..d34ccf6 100644
--- a/python35.spec
+++ b/python35.spec
@@ -126,7 +126,7 @@ BuildRequires: readline-devel
 BuildRequires: sqlite-devel

 # https://bugzilla.redhat.com/show_bug.cgi?id=1609291
-BuildRequires: compat-openssl10-devel
+BuildRequires: openssl-devel

 %if 0%{?with_systemtap}
 BuildRequires: systemtap-sdt-devel

Here you have: https://koji.fedoraproject.org/koji/taskinfo?taskID=33970861

Green on all archs but s390x: "test_multiprocessing_spawn test_socket test_threading" failed on s390x, but it's unrelated to my PR. The build confirms that test_hashlib, test_ssl and test_asyncio passed on all archs with OpenSSL 1.1.1 on Fedora Rawhide, good :-)

Here we say 1.1.1. but upstream we say 1.1.0. Can we have the same patch everywhere?

https://github.com/python/cpython/pull/12694

There is no ruch in merging this yet, so i guess we can wait for the upstream PR to be merged (or rejected) before we merge this one?

Thank you folks for dealing with this and sorry for the late answer. The upstream PR looks good to me and as I see it has already been reviewed by Miro. @vstinner could you sync the downstream patch with the upstream one. One final build to verify and it should be good to go.

Also please change the dependency from the compat ssl package to the openssl-devel

I rewrote this PR using the change that I proposed upstream:
https://github.com/python/cpython/pull/12694

Also please change the dependency from the compat ssl package to the openssl-devel

Miro asked me to not do that. We will keep compat-openssl10-devel until it gets removed from Fedora. This PR only prepares the package so it will be trivial to replace compat-openssl10-devel with openssl-devel.

Scratch build with the patch:

diff --git a/python35.spec b/python35.spec
index d6a11e8..d34ccf6 100644
--- a/python35.spec
+++ b/python35.spec
@@ -126,7 +126,7 @@ BuildRequires: readline-devel
BuildRequires: sqlite-devel

# https://bugzilla.redhat.com/show_bug.cgi?id=1609291
-BuildRequires: compat-openssl10-devel
+BuildRequires: openssl-devel

%if 0%{?with_systemtap}
BuildRequires: systemtap-sdt-devel

=> https://koji.fedoraproject.org/koji/taskinfo?taskID=34466349

rebased onto 35c70e3

6 months ago

Pull-Request has been merged by cstratak

5 months ago

And of course thanks @vstinner :confetti_ball: