From e4089dc578636c15390f0c45f92781f08b50f3ca Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 23 Jun 2022 15:18:57 +0200 Subject: [PATCH 1/2] gh-94172: Remove keyfile, certfile and check_hostname parameters Remove the keyfile, certfile and check_hostname parameters, deprecated since Python 3.6, in modules: ftplib, http.client, imaplib, poplib and smtplib. Use the context parameter (ssl_context in imaplib) instead. Parameters following the removed parameters become keyword-only parameters. ftplib: Remove the FTP_TLS.ssl_version class attribute: use the context parameter instead. --- Doc/whatsnew/3.12.rst | 10 +++++ Lib/ftplib.py | 24 ++--------- Lib/http/client.py | 25 ++--------- Lib/imaplib.py | 25 ++--------- Lib/poplib.py | 26 +++-------- Lib/smtplib.py | 42 +++--------------- Lib/test/test_ftplib.py | 6 --- Lib/test/test_httplib.py | 43 +++++-------------- Lib/test/test_imaplib.py | 21 --------- Lib/test/test_poplib.py | 7 --- ...2-06-23-15-31-49.gh-issue-94172.AXE2IZ.rst | 5 +++ ...2-06-23-15-36-49.gh-issue-94172.DzQk0s.rst | 2 + 12 files changed, 50 insertions(+), 186 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2022-06-23-15-31-49.gh-issue-94172.AXE2IZ.rst create mode 100644 Misc/NEWS.d/next/Library/2022-06-23-15-36-49.gh-issue-94172.DzQk0s.rst diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index a9432561f3fc50..65c2980d54966f 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -524,6 +524,16 @@ Removed `_. (Contributed by Julien Palard in :gh:`98179`.) +* Remove the *keyfile*, *certfile* and *check_hostname* parameters, deprecated + since Python 3.6, in modules: :mod:`ftplib`, :mod:`http.client`, + :mod:`imaplib`, :mod:`poplib` and :mod:`smtplib`. Use the *context* parameter + (*ssl_context* in :mod:`imaplib`) instead. + (Contributed by Victor Stinner in :gh:`94172`.) + +* :mod:`ftplib`: Remove the ``FTP_TLS.ssl_version`` class attribute: use the + *context* parameter instead. + (Contributed by Victor Stinner in :gh:`94172`.) + Porting to Python 3.12 ====================== diff --git a/Lib/ftplib.py b/Lib/ftplib.py index dc9a8afbd8d247..c7ca8f632e1bd4 100644 --- a/Lib/ftplib.py +++ b/Lib/ftplib.py @@ -713,28 +713,12 @@ class FTP_TLS(FTP): '221 Goodbye.' >>> ''' - ssl_version = ssl.PROTOCOL_TLS_CLIENT def __init__(self, host='', user='', passwd='', acct='', - keyfile=None, certfile=None, context=None, - timeout=_GLOBAL_DEFAULT_TIMEOUT, source_address=None, *, - encoding='utf-8'): - if context is not None and keyfile is not None: - raise ValueError("context and keyfile arguments are mutually " - "exclusive") - if context is not None and certfile is not None: - raise ValueError("context and certfile arguments are mutually " - "exclusive") - if keyfile is not None or certfile is not None: - import warnings - warnings.warn("keyfile and certfile are deprecated, use a " - "custom context instead", DeprecationWarning, 2) - self.keyfile = keyfile - self.certfile = certfile + *, context=None, timeout=_GLOBAL_DEFAULT_TIMEOUT, + source_address=None, encoding='utf-8'): if context is None: - context = ssl._create_stdlib_context(self.ssl_version, - certfile=certfile, - keyfile=keyfile) + context = ssl._create_stdlib_context() self.context = context self._prot_p = False super().__init__(host, user, passwd, acct, @@ -749,7 +733,7 @@ def auth(self): '''Set up secure control connection by using TLS/SSL.''' if isinstance(self.sock, ssl.SSLSocket): raise ValueError("Already using TLS") - if self.ssl_version >= ssl.PROTOCOL_TLS: + if self.context.protocol >= ssl.PROTOCOL_TLS: resp = self.voidcmd('AUTH TLS') else: resp = self.voidcmd('AUTH SSL') diff --git a/Lib/http/client.py b/Lib/http/client.py index 0720990f84e7ed..0a3e950c669622 100644 --- a/Lib/http/client.py +++ b/Lib/http/client.py @@ -1414,33 +1414,14 @@ class HTTPSConnection(HTTPConnection): default_port = HTTPS_PORT - # XXX Should key_file and cert_file be deprecated in favour of context? - - def __init__(self, host, port=None, key_file=None, cert_file=None, - timeout=socket._GLOBAL_DEFAULT_TIMEOUT, - source_address=None, *, context=None, - check_hostname=None, blocksize=8192): + def __init__(self, host, port=None, + *, timeout=socket._GLOBAL_DEFAULT_TIMEOUT, + source_address=None, context=None, blocksize=8192): super(HTTPSConnection, self).__init__(host, port, timeout, source_address, blocksize=blocksize) - if (key_file is not None or cert_file is not None or - check_hostname is not None): - import warnings - warnings.warn("key_file, cert_file and check_hostname are " - "deprecated, use a custom context instead.", - DeprecationWarning, 2) - self.key_file = key_file - self.cert_file = cert_file if context is None: context = _create_https_context(self._http_vsn) - if check_hostname is not None: - context.check_hostname = check_hostname - if key_file or cert_file: - context.load_cert_chain(cert_file, key_file) - # cert and key file means the user wants to authenticate. - # enable TLS 1.3 PHA implicitly even for custom contexts. - if context.post_handshake_auth is not None: - context.post_handshake_auth = True self._context = context def connect(self): diff --git a/Lib/imaplib.py b/Lib/imaplib.py index fa4c0f8f62361a..577b4b9b03a88d 100644 --- a/Lib/imaplib.py +++ b/Lib/imaplib.py @@ -1285,16 +1285,12 @@ class IMAP4_SSL(IMAP4): """IMAP4 client class over SSL connection - Instantiate with: IMAP4_SSL([host[, port[, keyfile[, certfile[, ssl_context[, timeout=None]]]]]]) + Instantiate with: IMAP4_SSL([host[, port[, ssl_context[, timeout=None]]]]) host - host's name (default: localhost); port - port number (default: standard IMAP4 SSL port); - keyfile - PEM formatted file that contains your private key (default: None); - certfile - PEM formatted certificate chain file (default: None); ssl_context - a SSLContext object that contains your certificate chain and private key (default: None) - Note: if ssl_context is provided, then parameters keyfile or - certfile should not be set otherwise ValueError is raised. timeout - socket timeout (default: None) If timeout is not given or is None, the global default socket timeout is used @@ -1302,23 +1298,10 @@ class IMAP4_SSL(IMAP4): """ - def __init__(self, host='', port=IMAP4_SSL_PORT, keyfile=None, - certfile=None, ssl_context=None, timeout=None): - if ssl_context is not None and keyfile is not None: - raise ValueError("ssl_context and keyfile arguments are mutually " - "exclusive") - if ssl_context is not None and certfile is not None: - raise ValueError("ssl_context and certfile arguments are mutually " - "exclusive") - if keyfile is not None or certfile is not None: - import warnings - warnings.warn("keyfile and certfile are deprecated, use a " - "custom ssl_context instead", DeprecationWarning, 2) - self.keyfile = keyfile - self.certfile = certfile + def __init__(self, host='', port=IMAP4_SSL_PORT, + *, ssl_context=None, timeout=None): if ssl_context is None: - ssl_context = ssl._create_stdlib_context(certfile=certfile, - keyfile=keyfile) + ssl_context = ssl._create_stdlib_context() self.ssl_context = ssl_context IMAP4.__init__(self, host, port, timeout) diff --git a/Lib/poplib.py b/Lib/poplib.py index 0f8587317c2bbc..bb28b2380ad37b 100644 --- a/Lib/poplib.py +++ b/Lib/poplib.py @@ -419,35 +419,19 @@ def stls(self, context=None): class POP3_SSL(POP3): """POP3 client class over SSL connection - Instantiate with: POP3_SSL(hostname, port=995, keyfile=None, certfile=None, - context=None) + Instantiate with: POP3_SSL(hostname, port=995, context=None) hostname - the hostname of the pop3 over ssl server port - port number - keyfile - PEM formatted file that contains your private key - certfile - PEM formatted certificate chain file context - a ssl.SSLContext See the methods of the parent class POP3 for more documentation. """ - def __init__(self, host, port=POP3_SSL_PORT, keyfile=None, certfile=None, - timeout=socket._GLOBAL_DEFAULT_TIMEOUT, context=None): - if context is not None and keyfile is not None: - raise ValueError("context and keyfile arguments are mutually " - "exclusive") - if context is not None and certfile is not None: - raise ValueError("context and certfile arguments are mutually " - "exclusive") - if keyfile is not None or certfile is not None: - import warnings - warnings.warn("keyfile and certfile are deprecated, use a " - "custom context instead", DeprecationWarning, 2) - self.keyfile = keyfile - self.certfile = certfile + def __init__(self, host, port=POP3_SSL_PORT, + *, timeout=socket._GLOBAL_DEFAULT_TIMEOUT, context=None): if context is None: - context = ssl._create_stdlib_context(certfile=certfile, - keyfile=keyfile) + context = ssl._create_stdlib_context() self.context = context POP3.__init__(self, host, port, timeout) @@ -457,7 +441,7 @@ def _create_socket(self, timeout): server_hostname=self.host) return sock - def stls(self, keyfile=None, certfile=None, context=None): + def stls(self, *, context=None): """The method unconditionally raises an exception since the STLS command doesn't make any sense on an already established SSL/TLS session. diff --git a/Lib/smtplib.py b/Lib/smtplib.py index 324a1c19f12afe..05d2f8ccd73c98 100755 --- a/Lib/smtplib.py +++ b/Lib/smtplib.py @@ -749,14 +749,14 @@ def login(self, user, password, *, initial_response_ok=True): # We could not login successfully. Return result of last attempt. raise last_exception - def starttls(self, keyfile=None, certfile=None, context=None): + def starttls(self, *, context=None): """Puts the connection to the SMTP server into TLS mode. If there has been no previous EHLO or HELO command this session, this method tries ESMTP EHLO first. If the server supports TLS, this will encrypt the rest of the SMTP - session. If you provide the keyfile and certfile parameters, + session. If you provide the context parameter, the identity of the SMTP server and client can be checked. This, however, depends on whether the socket module really checks the certificates. @@ -774,19 +774,8 @@ def starttls(self, keyfile=None, certfile=None, context=None): if resp == 220: if not _have_ssl: raise RuntimeError("No SSL support included in this Python") - if context is not None and keyfile is not None: - raise ValueError("context and keyfile arguments are mutually " - "exclusive") - if context is not None and certfile is not None: - raise ValueError("context and certfile arguments are mutually " - "exclusive") - if keyfile is not None or certfile is not None: - import warnings - warnings.warn("keyfile and certfile are deprecated, use a " - "custom context instead", DeprecationWarning, 2) if context is None: - context = ssl._create_stdlib_context(certfile=certfile, - keyfile=keyfile) + context = ssl._create_stdlib_context() self.sock = context.wrap_socket(self.sock, server_hostname=self._host) self.file = None @@ -1017,35 +1006,18 @@ class SMTP_SSL(SMTP): compiled with SSL support). If host is not specified, '' (the local host) is used. If port is omitted, the standard SMTP-over-SSL port (465) is used. local_hostname and source_address have the same meaning - as they do in the SMTP class. keyfile and certfile are also optional - - they can contain a PEM formatted private key and certificate chain file - for the SSL connection. context also optional, can contain a - SSLContext, and is an alternative to keyfile and certfile; If it is - specified both keyfile and certfile must be None. + as they do in the SMTP class. context also optional, can contain a + SSLContext. """ default_port = SMTP_SSL_PORT def __init__(self, host='', port=0, local_hostname=None, - keyfile=None, certfile=None, - timeout=socket._GLOBAL_DEFAULT_TIMEOUT, + *, timeout=socket._GLOBAL_DEFAULT_TIMEOUT, source_address=None, context=None): - if context is not None and keyfile is not None: - raise ValueError("context and keyfile arguments are mutually " - "exclusive") - if context is not None and certfile is not None: - raise ValueError("context and certfile arguments are mutually " - "exclusive") - if keyfile is not None or certfile is not None: - import warnings - warnings.warn("keyfile and certfile are deprecated, use a " - "custom context instead", DeprecationWarning, 2) - self.keyfile = keyfile - self.certfile = certfile if context is None: - context = ssl._create_stdlib_context(certfile=certfile, - keyfile=keyfile) + context = ssl._create_stdlib_context() self.context = context SMTP.__init__(self, host, port, local_hostname, timeout, source_address) diff --git a/Lib/test/test_ftplib.py b/Lib/test/test_ftplib.py index 082a90d46baedc..cd8e77ecdfa82f 100644 --- a/Lib/test/test_ftplib.py +++ b/Lib/test/test_ftplib.py @@ -984,12 +984,6 @@ def test_context(self): ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) ctx.check_hostname = False ctx.verify_mode = ssl.CERT_NONE - self.assertRaises(ValueError, ftplib.FTP_TLS, keyfile=CERTFILE, - context=ctx) - self.assertRaises(ValueError, ftplib.FTP_TLS, certfile=CERTFILE, - context=ctx) - self.assertRaises(ValueError, ftplib.FTP_TLS, certfile=CERTFILE, - keyfile=CERTFILE, context=ctx) self.client = ftplib.FTP_TLS(context=ctx, timeout=TIMEOUT) self.client.connect(self.server.host, self.server.port) diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py index b3d94e0a21cb6a..f97e3b8af595f6 100644 --- a/Lib/test/test_httplib.py +++ b/Lib/test/test_httplib.py @@ -1978,7 +1978,7 @@ def test_local_unknown_cert(self): self.assertEqual(exc_info.exception.reason, 'CERTIFICATE_VERIFY_FAILED') def test_local_good_hostname(self): - # The (valid) cert validates the HTTP hostname + # The (valid) cert validates the HTTPS hostname import ssl server = self.make_server(CERT_localhost) context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) @@ -1991,7 +1991,7 @@ def test_local_good_hostname(self): self.assertEqual(resp.status, 404) def test_local_bad_hostname(self): - # The (valid) cert doesn't validate the HTTP hostname + # The (valid) cert doesn't validate the HTTPS hostname import ssl server = self.make_server(CERT_fakehostname) context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) @@ -1999,38 +1999,22 @@ def test_local_bad_hostname(self): h = client.HTTPSConnection('localhost', server.port, context=context) with self.assertRaises(ssl.CertificateError): h.request('GET', '/') - # Same with explicit check_hostname=True - with warnings_helper.check_warnings(('', DeprecationWarning)): - h = client.HTTPSConnection('localhost', server.port, - context=context, check_hostname=True) + + # Same with explicit context.check_hostname=True + context.check_hostname = True + h = client.HTTPSConnection('localhost', server.port, + context=context, ) with self.assertRaises(ssl.CertificateError): h.request('GET', '/') - # With check_hostname=False, the mismatching is ignored - context.check_hostname = False - with warnings_helper.check_warnings(('', DeprecationWarning)): - h = client.HTTPSConnection('localhost', server.port, - context=context, check_hostname=False) - h.request('GET', '/nonexistent') - resp = h.getresponse() - resp.close() - h.close() - self.assertEqual(resp.status, 404) - # The context's check_hostname setting is used if one isn't passed to - # HTTPSConnection. + + # With context.check_hostname=False, the mismatching is ignored context.check_hostname = False h = client.HTTPSConnection('localhost', server.port, context=context) h.request('GET', '/nonexistent') resp = h.getresponse() - self.assertEqual(resp.status, 404) resp.close() h.close() - # Passing check_hostname to HTTPSConnection should override the - # context's setting. - with warnings_helper.check_warnings(('', DeprecationWarning)): - h = client.HTTPSConnection('localhost', server.port, - context=context, check_hostname=True) - with self.assertRaises(ssl.CertificateError): - h.request('GET', '/') + self.assertEqual(resp.status, 404) @unittest.skipIf(not hasattr(client, 'HTTPSConnection'), 'http.client.HTTPSConnection not available') @@ -2066,13 +2050,6 @@ def test_tls13_pha(self): self.assertIs(h._context, context) self.assertFalse(h._context.post_handshake_auth) - with warnings.catch_warnings(): - warnings.filterwarnings('ignore', 'key_file, cert_file and check_hostname are deprecated', - DeprecationWarning) - h = client.HTTPSConnection('localhost', 443, context=context, - cert_file=CERT_localhost) - self.assertTrue(h._context.post_handshake_auth) - class RequestBodyTest(TestCase): """Test cases where a request includes a message body.""" diff --git a/Lib/test/test_imaplib.py b/Lib/test/test_imaplib.py index b554bc0c79960d..7626d9572e1e96 100644 --- a/Lib/test/test_imaplib.py +++ b/Lib/test/test_imaplib.py @@ -573,15 +573,6 @@ def test_ssl_verified(self): ssl_context=ssl_context) client.shutdown() - # Mock the private method _connect(), so mark the test as specific - # to CPython stdlib - @cpython_only - def test_certfile_arg_warn(self): - with warnings_helper.check_warnings(('', DeprecationWarning)): - with mock.patch.object(self.imap_class, 'open'): - with mock.patch.object(self.imap_class, '_connect'): - self.imap_class('localhost', 143, certfile=CERTFILE) - class ThreadedNetworkedTests(unittest.TestCase): server_class = socketserver.TCPServer imap_class = imaplib.IMAP4 @@ -1070,18 +1061,6 @@ def test_logout(self): rs = _server.logout() self.assertEqual(rs[0], 'BYE', rs) - def test_ssl_context_certfile_exclusive(self): - with socket_helper.transient_internet(self.host): - self.assertRaises( - ValueError, self.imap_class, self.host, self.port, - certfile=CERTFILE, ssl_context=self.create_ssl_context()) - - def test_ssl_context_keyfile_exclusive(self): - with socket_helper.transient_internet(self.host): - self.assertRaises( - ValueError, self.imap_class, self.host, self.port, - keyfile=CERTFILE, ssl_context=self.create_ssl_context()) - if __name__ == "__main__": unittest.main() diff --git a/Lib/test/test_poplib.py b/Lib/test/test_poplib.py index 5ad9202433dcfb..4917e85f42ec84 100644 --- a/Lib/test/test_poplib.py +++ b/Lib/test/test_poplib.py @@ -425,13 +425,6 @@ def test_context(self): ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) ctx.check_hostname = False ctx.verify_mode = ssl.CERT_NONE - self.assertRaises(ValueError, poplib.POP3_SSL, self.server.host, - self.server.port, keyfile=CERTFILE, context=ctx) - self.assertRaises(ValueError, poplib.POP3_SSL, self.server.host, - self.server.port, certfile=CERTFILE, context=ctx) - self.assertRaises(ValueError, poplib.POP3_SSL, self.server.host, - self.server.port, keyfile=CERTFILE, - certfile=CERTFILE, context=ctx) self.client.quit() self.client = poplib.POP3_SSL(self.server.host, self.server.port, diff --git a/Misc/NEWS.d/next/Library/2022-06-23-15-31-49.gh-issue-94172.AXE2IZ.rst b/Misc/NEWS.d/next/Library/2022-06-23-15-31-49.gh-issue-94172.AXE2IZ.rst new file mode 100644 index 00000000000000..569fec95ffbb54 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-06-23-15-31-49.gh-issue-94172.AXE2IZ.rst @@ -0,0 +1,5 @@ +Remove the *keyfile*, *certfile* and *check_hostname* parameters, deprecated +since Python 3.6, in modules: :mod:`ftplib`, :mod:`http.client`, +:mod:`imaplib`, :mod:`poplib` and :mod:`smtplib`. Use the *context* +parameter (*ssl_context* in :mod:`imaplib`) instead. Patch by Victor +Stinner. diff --git a/Misc/NEWS.d/next/Library/2022-06-23-15-36-49.gh-issue-94172.DzQk0s.rst b/Misc/NEWS.d/next/Library/2022-06-23-15-36-49.gh-issue-94172.DzQk0s.rst new file mode 100644 index 00000000000000..3be4b09b5ec195 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-06-23-15-36-49.gh-issue-94172.DzQk0s.rst @@ -0,0 +1,2 @@ +:mod:`ftplib`: Remove the ``FTP_TLS.ssl_version`` class attribute: use the +*context* parameter instead. Patch by Victor Stinner From 1ba8627d156e3621c4c1df38a572f38203e49efb Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 2 Nov 2022 17:53:34 +0100 Subject: [PATCH 2/2] Address Serhiy's review * poplib: stls() context parameter can be positional again * Keep test_ftplib tests but replace ValueError with TypeError * Reformat test_httplib * Keep but update test_tls13_pha() test --- Lib/poplib.py | 2 +- Lib/test/test_ftplib.py | 6 ++++++ Lib/test/test_httplib.py | 8 ++++++-- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Lib/poplib.py b/Lib/poplib.py index bb28b2380ad37b..9a5ef03c983103 100644 --- a/Lib/poplib.py +++ b/Lib/poplib.py @@ -441,7 +441,7 @@ def _create_socket(self, timeout): server_hostname=self.host) return sock - def stls(self, *, context=None): + def stls(self, context=None): """The method unconditionally raises an exception since the STLS command doesn't make any sense on an already established SSL/TLS session. diff --git a/Lib/test/test_ftplib.py b/Lib/test/test_ftplib.py index cd8e77ecdfa82f..218fecfbb019ae 100644 --- a/Lib/test/test_ftplib.py +++ b/Lib/test/test_ftplib.py @@ -984,6 +984,12 @@ def test_context(self): ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) ctx.check_hostname = False ctx.verify_mode = ssl.CERT_NONE + self.assertRaises(TypeError, ftplib.FTP_TLS, keyfile=CERTFILE, + context=ctx) + self.assertRaises(TypeError, ftplib.FTP_TLS, certfile=CERTFILE, + context=ctx) + self.assertRaises(TypeError, ftplib.FTP_TLS, certfile=CERTFILE, + keyfile=CERTFILE, context=ctx) self.client = ftplib.FTP_TLS(context=ctx, timeout=TIMEOUT) self.client.connect(self.server.host, self.server.port) diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py index f97e3b8af595f6..620a5b19109a8c 100644 --- a/Lib/test/test_httplib.py +++ b/Lib/test/test_httplib.py @@ -2002,8 +2002,7 @@ def test_local_bad_hostname(self): # Same with explicit context.check_hostname=True context.check_hostname = True - h = client.HTTPSConnection('localhost', server.port, - context=context, ) + h = client.HTTPSConnection('localhost', server.port, context=context) with self.assertRaises(ssl.CertificateError): h.request('GET', '/') @@ -2050,6 +2049,11 @@ def test_tls13_pha(self): self.assertIs(h._context, context) self.assertFalse(h._context.post_handshake_auth) + context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT, cert_file=CERT_localhost) + context.post_handshake_auth = True + h = client.HTTPSConnection('localhost', 443, context=context) + self.assertTrue(h._context.post_handshake_auth) + class RequestBodyTest(TestCase): """Test cases where a request includes a message body."""