Repository navigation
SSL releated deprecation for 3.6 #72209
Description
Activity
I like to deprecate some SSL related parts of Python:
- ssl.wrap_socket() is a horrible abomination. People should use SSLContext.wrap_socket() instead
- all certfile/cert_file, keyfile/key_file and check_hostname arguments. Use context / ssl_context instead.
- make ftplib, imaplib, nntplib, pop3lib, smtplib etc. validate certs by default.
Lukasa commented
on Sep 8, 2016 LukasamannequinMannequin10/10, yes. I will happily provide code review for this.
- make ftplib, imaplib, nntplib, pop3lib, smtplib etc. validate certs by default.
I'm not sure about this one:
http://legacy.python.org/dev/peps/pep-0476/#other-protocolsAnother deprecation: I like to deprecate all arguments from SSLSocket.__init__() and require users to go through SSLContext.wrap_socket(). It's going to make the implementation much simpler. The argument list is just crazy:
class SSLSocket(socket): def __init__(self, sock=None, keyfile=None, certfile=None, server_side=False, cert_reqs=CERT_NONE, ssl_version=PROTOCOL_TLS, ca_certs=None, do_handshake_on_connect=True, family=AF_INET, type=SOCK_STREAM, proto=0, fileno=None, suppress_ragged_eofs=True, npn_protocols=None, ciphers=None, server_hostname=None, _context=None):
I like the idea of using SSLContext as the obvious and only choice to
"configure" SSL.memo to me: check if SSLContext.wrap_socket() can deal with a fileno as sock argument.
First draft of a patch: https://github.com/tiran/cpython/commits/feature/ssl_deprecation
urllib.request.urlopen() should be affected too right?
Yes, urllib.request.urlopen needs an update too. It takes those certfile and keyfile and usage of those could be deprecated in favor of context.
I have deprecated cafile, capath and cadefault for urlopen(). The function didn't pop up on my radar because I was looking for certfile and cert_file, not cafile. I also added deprecations to the documentation of SSLSocket.read and write.
10 remaining items
The performance benefit is not worth the risk. For 10 httplib requests to pypi.python.org, a shared SSLContext is about 5% faster than a new context for each request. Session resumption improves the simple test case by another 20%.
Leaving the option of context caching entirely to the caller would definitely make things simpler - my main interest is just in avoiding a hard compatibility break for folks that aren't doing anything particularly wrong, by which I mean specifically cases where a wrap_socket() implementation like this one would continue to work for them:
def wrap_socket(sock, *args, *kwds): return ssl.get_default_context().wrap_socket(sock, *args, **kwds)
New test failure when using -Werror:
======================================================================
ERROR: test_local_bad_hostname (test.test_httplib.HTTPSTest)
----------------------------------------------------------------------Traceback (most recent call last): File "/media/disk/home/proj/python/cpython/Lib/test/test_httplib.py", line 1646, in test_local_bad_hostname check_hostname=True) File "/media/disk/home/proj/python/cpython/Lib/http/client.py", line 1373, in __init__ DeprecationWarning, 2) DeprecationWarning: key_file, cert_file and check_hostname are deprecated, use a custom context instead.
python-dev commented
on Sep 11, 2016 python-devmannequinMannequinNew changeset 2e541e994927 by Christian Heimes in branch 'default':
bpo-28022: Catch deprecation warning in test_httplib, reported by Martin Panter
https://hg.python.org/cpython/rev/2e541e994927Thanks for the report. "./python -Werror -m test -uall test_httplib" is now passing for me.
test_imaplib is failed too.
======================================================================
ERROR: test_logincapa_with_client_certfile (test.test_imaplib.RemoteIMAP_SSLTest)
----------------------------------------------------------------------Traceback (most recent call last): File "/home/serhiy/py/cpython-debug/Lib/test/test_imaplib.py", line 641, in test_logincapa_with_client_certfile _server = self.imap_class(self.host, self.port, certfile=CERTFILE) File "/home/serhiy/py/cpython-debug/Lib/imaplib.py", line 1273, in __init__ "custom ssl_context instead", DeprecationWarning, 2) DeprecationWarning: keyfile and certfile are deprecated, use acustom ssl_context instead
python-dev commented
on Sep 11, 2016 python-devmannequinMannequinNew changeset 57e88d1159fc by Christian Heimes in branch 'default':
Issue bpo-28022: Catch another deprecation warning in imaplib
https://hg.python.org/cpython/rev/57e88d1159fcShouldn't this issue be closed?
Since SSL context arguments are not supported in 2.7, the deprecated arguments can't be removed until EOL of 2.7.
kousu commented
on Aug 8, 2021 kousumannequinMannequinHello everyone, and thank you as usual for all your hard work keeping the python ecosystem going.
I saw that the start of this thread said it was going to
- make ftplib, imaplib, nntplib, pop3lib, smtplib etc. validate certs by default.
but this hasn't been done, at least not for imaplib. imaplib is still calling the undocumented "ssl._create_stdlib_context":
Line 1320 in 2b496e7
which is actually "ssl._create_unverified_context":
Line 842 in 2b496e7
which is indeed unverified: despite defaulting to PROTOCOL_TLS_CLIENT, which "enables CERT_REQUIRED and check_hostname by default.", it overrides that by setting check_hostname=False:
Line 811 in 2b496e7
To demonstrate, check out this tester script:
$ cat a.py import os, imaplib
with imaplib.IMAP4_SSL(os.environ.get('HOSTNAME')) as S: print(S.login(os.environ.get('USERNAME'), os.environ.get('PASSWORD'))) $ HOSTNAME=46.23.90.174 USERNAME=test1 PASSWORD=test1test1 python3 a.py ('OK', [b'Logged in'])
I don't have a cert for 46.23.90.174 (no one will give out certs for IPs!), so this is wrong!
In order to actually enable verification you need to know the incantation. It's not that long but it is subtle and frighteningly easy to get wrong. Here it is:
$ cat a.py import os, ssl, imaplib
ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) ctx.load_default_certs() with imaplib.IMAP4_SSL(os.environ.get('HOSTNAME'), ssl_context=ctx) as S: print(S.login(os.environ.get('USERNAME'), os.environ.get('PASSWORD'))) $ HOSTNAME=46.23.90.174 USERNAME=test1 PASSWORD=test1test1 python3 a.py Traceback (most recent call last): File "a.py", line 6, in <module> with imaplib.IMAP4_SSL(os.environ.get('HOSTNAME'), ssl_context=ctx) as S: File "/usr/lib/python3.6/imaplib.py", line 1288, in __init__ IMAP4.__init__(self, host, port) File "/usr/lib/python3.6/imaplib.py", line 198, in __init__ self.open(host, port) File "/usr/lib/python3.6/imaplib.py", line 1301, in open IMAP4.open(self, host, port) File "/usr/lib/python3.6/imaplib.py", line 299, in open self.sock = self._create_socket() File "/usr/lib/python3.6/imaplib.py", line 1293, in _create_socket server_hostname=self.host) File "/usr/lib/python3.6/ssl.py", line 407, in wrap_socket _context=self, _session=session) File "/usr/lib/python3.6/ssl.py", line 817, in __init__ self.do_handshake() File "/usr/lib/python3.6/ssl.py", line 1077, in do_handshake self._sslobj.do_handshake() File "/usr/lib/python3.6/ssl.py", line 694, in do_handshake match_hostname(self.getpeercert(), self.server_hostname) File "/usr/lib/python3.6/ssl.py", line 327, in match_hostname % (hostname, ', '.join(map(repr, dnsnames)))) ssl.CertificateError: hostname '46.23.90.174' doesn't match either of 'comms.kousu.ca', 'comms3.kousu.ca'
I can see from this thread there were some concerns about breaking people's self-signed certs back in 2016. But it's five years later now and letsencrypt is super common now, and most servers and clients are enforcing TLS, especially when credentials are involved.
Could this be revisited?
Thanks for any attention you have gifted to this :)
The part with "make ftplib, imaplib, nntplib, pop3lib, smtplib etc. validate certs by default" was not implemented. These modules still default to unverified connections.
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields: