drop wincertstore and certifi dep by dropping setuptools.ssl_support #2711

graingert · 2021-06-28T18:37:40Z

Summary of changes

Closes #86

Pull Request Checklist

Changes have tests
News fragment added in changelog.d/.
(See documentation for details)

graingert · 2021-06-28T18:38:05Z

wincertstore only supports python < 3.4

Cc @tiran

tiran · 2021-06-28T19:58:38Z

Urks, this code is bad and should be completely rewritten using SSLContext.

tiran · 2021-06-28T20:03:54Z

setuptools/ssl_support.py

-    try:
-        import wincertstore
-    except ImportError:
-        return None
-
-    class CertFile(wincertstore.CertFile):
-        def __init__(self):
-            super(CertFile, self).__init__()
-            atexit.register(self.close)
-
-        def close(self):
-            try:
-                super(CertFile, self).close()
-            except OSError:
-                pass
-
-    _wincerts = CertFile()
-    _wincerts.addstore('CA')
-    _wincerts.addstore('ROOT')
-    return _wincerts.name


Don't drop this feature. Either replace it with https://github.com/python/cpython/blob/db3ff76da19004f266b62e98a81bdfd322861436/Lib/ssl.py#L555-L575, replace it with SSLContext.load_default_certs or even better: use ssl.create_default_context().

so it turns out the code already does use ssl.create_default_context( where available. But now it's always available so the whole module can go!

tiran · 2021-07-02T11:38:31Z

setup.cfg

-	wincertstore; sys_platform=='win32'
-
-certs =
-	certifi


That is probably a really bad idea for certain kinds of Windows users, most macOS users, and Debian/Ubuntu users.

Consider using system trust stores by default in 3.0.0. psf/requests#2966

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=960869

previously setuptools only fell back on certifi if every system cert file was missing:

return ( get_win_certfile() or next(extant_cert_paths, None) or _certifi_where() )

so I think this is correct?

@tiran as far as I can tell I can't install certifi without installing ca-certificates

https://gist.github.com/tiran/2dec9e03c6f901814f6d1e8dad09528e#ssl-module-cannot-verify-connections

Debian-based distros don't come with ca-certificates in their minimal package list because Debian-based distros use plain HTTP + GPG signatures instead of HTTPS to download and verify packages. Starting with Ubuntu >= 21.04 and current Debian Testing, the Python package has a weak dependency on ca-certificates.

The same applies for Windows. ssl.enum_certificates no longer works in some cases because Windows retrieves trust anchors on demand.

It seems windows urllib.request.urlopen(..., context=ssl.create_default_context(capath=None)) works correctly too:

>>> import ssl >>> urllib.request.urlopen("https://google.com", context=ssl.create_default_context(capath=None)) <http.client.HTTPResponse object at 0x0000011183D54130> >>> urllib.request.urlopen("https://wrong.host.badssl.com/", context=ssl.create_default_context(capath=None)) Traceback (most recent call last): File "C:\Python39\lib\urllib\request.py", line 1346, in do_open h.request(req.get_method(), req.selector, req.data, headers, File "C:\Python39\lib\http\client.py", line 1253, in request self._send_request(method, url, body, headers, encode_chunked) File "C:\Python39\lib\http\client.py", line 1299, in _send_request self.endheaders(body, encode_chunked=encode_chunked) File "C:\Python39\lib\http\client.py", line 1248, in endheaders self._send_output(message_body, encode_chunked=encode_chunked) File "C:\Python39\lib\http\client.py", line 1008, in _send_output self.send(msg) File "C:\Python39\lib\http\client.py", line 948, in send self.connect() File "C:\Python39\lib\http\client.py", line 1422, in connect self.sock = self._context.wrap_socket(self.sock, File "C:\Python39\lib\ssl.py", line 500, in wrap_socket return self.sslsocket_class._create( File "C:\Python39\lib\ssl.py", line 1040, in _create self.do_handshake() File "C:\Python39\lib\ssl.py", line 1309, in do_handshake self._sslobj.do_handshake() ssl.SSLCertVerificationError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: Hostname mismatch, certificate is not valid for 'wrong.host.badssl.com'. (_ssl.c:1129) During handling of the above exception, another exception occurred: Traceback (most recent call last): File "<stdin>", line 1, in <module> File "C:\Python39\lib\urllib\request.py", line 214, in urlopen return opener.open(url, data, timeout) File "C:\Python39\lib\urllib\request.py", line 517, in open response = self._open(req, data) File "C:\Python39\lib\urllib\request.py", line 534, in _open result = self._call_chain(self.handle_open, protocol, protocol + File "C:\Python39\lib\urllib\request.py", line 494, in _call_chain result = func(*args) File "C:\Python39\lib\urllib\request.py", line 1389, in https_open return self.do_open(http.client.HTTPSConnection, req, File "C:\Python39\lib\urllib\request.py", line 1349, in do_open raise URLError(err) urllib.error.URLError: <urlopen error [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: Hostname mismatch, certificate is not valid for 'wrong.host.badssl.com'. (_ssl.c:1129)> >>>

My windows installation of setuptools doesn't have certifi or wincertstore installed

@tiran I'm confident certifi is never used here, barring intentional user intervention.

The setuptools.package_index module is only used when pip is not available and setup.py is invoked with a deprecated setup_requires kwarg. On Windows and macos ensurepip works correctly so this isn't an issue.

further certifi is only used when ca-certificates is not installed. because on debian python3-certifi depends on ca-certificates this code is unreachable.

installing certifi manually with pip (eg via get-pip.py sideloaded via USB stick) would also render this code unreachable because installing pip means this code won't run either

installing certifi with easyinstall isn't possible because neither certifi nor ca-certificates is installed and so certificate verification will fail

To reach this code a user would have to manually copy over the certifi files into their site-packages or add the whl to their sys.path

It should also be noted that setup.py upload (also deprecated) uses vanilla urllib.request.urlopen without any certificate shenanigans

setuptools/setuptools/_distutils/command/upload.py

Lines 194 to 195 in 582465b

result = urlopen(request)

status = result.getcode()

graingert · 2021-07-02T12:16:55Z

setuptools/package_index.py

+        if verify_ssl or ca_bundle:
+            import ssl  # deferred import because ssl technically isn't mandatory
+
+            self.opener = partial(
+                urllib.request.urlopen,
+                context=ssl.create_default_context(cafile=ca_bundle)
+            )
        else:
            self.opener = urllib.request.urlopen


it seems verify_ssl=False doesn't behave as expected. The default urllib.request.urlopen now verifies TLS correctly, and so this code is equivalent:

Suggested change

if verify_ssl or ca_bundle:

import ssl # deferred import because ssl technically isn't mandatory

self.opener = partial(

urllib.request.urlopen,

context=ssl.create_default_context(cafile=ca_bundle)

)

else:

self.opener = urllib.request.urlopen

if ca_bundle is not None:

import ssl # deferred import because ssl technically isn't mandatory

self.opener = partial(

urllib.request.urlopen,

context=ssl.create_default_context(cafile=ca_bundle)

)

else:

self.opener = urllib.request.urlopen

Here I'm running on a fresh ubuntu docker image after apt install python3-setuptools ca-certificates

>>> import setuptools >>> import setuptools.package_index >>> setuptools.package_index.PackageIndex(verify_ssl=False).opener("https://google.com") <http.client.HTTPResponse object at 0x7f0754c604c0> >>> setuptools.package_index.PackageIndex(verify_ssl=False).opener("https://wrong.host.badssl.com/") Traceback (most recent call last): File "/usr/lib/python3.8/urllib/request.py", line 1354, in do_open h.request(req.get_method(), req.selector, req.data, headers, File "/usr/lib/python3.8/http/client.py", line 1252, in request self._send_request(method, url, body, headers, encode_chunked) File "/usr/lib/python3.8/http/client.py", line 1298, in _send_request self.endheaders(body, encode_chunked=encode_chunked) File "/usr/lib/python3.8/http/client.py", line 1247, in endheaders self._send_output(message_body, encode_chunked=encode_chunked) File "/usr/lib/python3.8/http/client.py", line 1007, in _send_output self.send(msg) File "/usr/lib/python3.8/http/client.py", line 947, in send self.connect() File "/usr/lib/python3.8/http/client.py", line 1421, in connect self.sock = self._context.wrap_socket(self.sock, File "/usr/lib/python3.8/ssl.py", line 500, in wrap_socket return self.sslsocket_class._create( File "/usr/lib/python3.8/ssl.py", line 1040, in _create self.do_handshake() File "/usr/lib/python3.8/ssl.py", line 1309, in do_handshake self._sslobj.do_handshake() ssl.SSLCertVerificationError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: Hostname mismatch, certificate is not valid for 'wrong.host.badssl.com'. (_ssl.c:1131)

jaraco · 2021-07-05T02:03:51Z

See #2716 where I removed ssl support based on findings similar to what graingert reported above. I'll remove those dependencies as well. If custom bundle support is needed, it can be brought back, but it will need to be done using new, non-deprecated functionality.

jaraco · 2021-07-05T02:07:03Z

Removing the extras by name might break backward compatibility, so I've only removed the dependencies for now. I'll remove the extra names in a future, backward-incompatible release.

graingert · 2021-07-05T07:09:06Z

See #2716 where I removed ssl support based on findings similar to what graingert reported above. I'll remove those dependencies as well. If custom bundle support is needed, it can be brought back, but it will need to be done using new, non-deprecated functionality.

As far as I can tell custom CA bundle support works via the pip based setup_requires codepath already. I'll double check

drop wincertstore dep

97c5b87

graingert added 2 commits June 28, 2021 19:41

remove wincertstore import

08da0b8

remove unused atexit

29e1736

tiran reviewed Jun 28, 2021

View reviewed changes

graingert force-pushed the patch-1 branch from 3ee99dc to dc45a73 Compare July 2, 2021 11:04

graingert changed the title ~~drop wincertstore dep~~ drop wincertstore dep by dropping setuptools.ssl_support Jul 2, 2021

graingert force-pushed the patch-1 branch from ad63413 to 7c67cdb Compare July 2, 2021 11:08

graingert changed the title ~~drop wincertstore dep by dropping setuptools.ssl_support~~ drop wincertstore and certifi dep by dropping setuptools.ssl_support Jul 2, 2021

graingert added 2 commits July 2, 2021 12:13

drop setuptools.ssl_support

44e3afa

no need for certifi either

1efe6fe

graingert force-pushed the patch-1 branch from 7c67cdb to 1efe6fe Compare July 2, 2021 11:14

ignore SelectableGroups warning

582465b

tiran reviewed Jul 2, 2021

View reviewed changes

graingert commented Jul 2, 2021

View reviewed changes

jaraco closed this Jul 5, 2021

jaraco added a commit that referenced this pull request Jul 5, 2021

Remove wincertstore and certifi. Ref #2711 and #2716.

0aa3576

graingert deleted the patch-1 branch July 5, 2021 07:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drop wincertstore and certifi dep by dropping setuptools.ssl_support #2711

drop wincertstore and certifi dep by dropping setuptools.ssl_support #2711

graingert commented Jun 28, 2021 •

edited

graingert commented Jun 28, 2021

tiran commented Jun 28, 2021

tiran Jun 28, 2021

graingert Jul 2, 2021

tiran Jul 2, 2021

graingert Jul 2, 2021

graingert Jul 2, 2021

tiran Jul 2, 2021

tiran Jul 2, 2021

tiran Jul 2, 2021

graingert Jul 2, 2021

graingert Jul 2, 2021

graingert Jul 2, 2021 •

edited

graingert Jul 2, 2021

graingert Jul 2, 2021 •

edited

graingert Jul 2, 2021

jaraco commented Jul 5, 2021

jaraco commented Jul 5, 2021

graingert commented Jul 5, 2021

drop wincertstore and certifi dep by dropping setuptools.ssl_support #2711

drop wincertstore and certifi dep by dropping setuptools.ssl_support #2711

Conversation

graingert commented Jun 28, 2021 • edited

Summary of changes

Pull Request Checklist

graingert commented Jun 28, 2021

tiran commented Jun 28, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

graingert Jul 2, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

graingert Jul 2, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaraco commented Jul 5, 2021

jaraco commented Jul 5, 2021

graingert commented Jul 5, 2021

graingert commented Jun 28, 2021 •

edited

graingert Jul 2, 2021 •

edited

graingert Jul 2, 2021 •

edited