From 0a76a9b115fbf1a6e985d56e8ab6e3a833c34790 Mon Sep 17 00:00:00 2001 From: dvermd <315743+dvermd@users.noreply.github.com> Date: Wed, 27 Sep 2023 05:58:09 +0200 Subject: [PATCH] Update ftplib to CPython 3.11.5 part_of: #4564 --- Lib/ftplib.py | 55 ++++++++------ Lib/test/test_ftplib.py | 164 ++++++++++++++++++++++++++++------------ 2 files changed, 149 insertions(+), 70 deletions(-) diff --git a/Lib/ftplib.py b/Lib/ftplib.py index 58a46bca4a..7c5a50715f 100644 --- a/Lib/ftplib.py +++ b/Lib/ftplib.py @@ -72,17 +72,17 @@ class error_proto(Error): pass # response does not begin with [1-5] # The class itself class FTP: - '''An FTP client class. To create a connection, call the class using these arguments: - host, user, passwd, acct, timeout + host, user, passwd, acct, timeout, source_address, encoding The first four arguments are all strings, and have default value ''. - timeout must be numeric and defaults to None if not passed, - meaning that no timeout will be set on any ftp socket(s) + The parameter ´timeout´ must be numeric and defaults to None if not + passed, meaning that no timeout will be set on any ftp socket(s). If a timeout is passed, then this is now the default timeout for all ftp socket operations for this instance. + The last parameter is the encoding of filenames, which defaults to utf-8. Then use self.connect() with optional host and port argument. @@ -102,15 +102,19 @@ class FTP: sock = None file = None welcome = None - passiveserver = 1 - encoding = "latin-1" + passiveserver = True + # Disables https://bugs.python.org/issue43285 security if set to True. + trust_server_pasv_ipv4_address = False - # Initialization method (called by class instantiation). - # Initialize host to localhost, port to standard ftp port - # Optional arguments are host (for connect()), - # and user, passwd, acct (for login()) def __init__(self, host='', user='', passwd='', acct='', - timeout=_GLOBAL_DEFAULT_TIMEOUT, source_address=None): + timeout=_GLOBAL_DEFAULT_TIMEOUT, source_address=None, *, + encoding='utf-8'): + """Initialization method (called by class instantiation). + Initialize host to localhost, port to standard ftp port. + Optional arguments are host (for connect()), + and user, passwd, acct (for login()). + """ + self.encoding = encoding self.source_address = source_address self.timeout = timeout if host: @@ -146,6 +150,8 @@ def connect(self, host='', port=0, timeout=-999, source_address=None): self.port = port if timeout != -999: self.timeout = timeout + if self.timeout is not None and not self.timeout: + raise ValueError('Non-blocking socket (timeout=0) is not supported') if source_address is not None: self.source_address = source_address sys.audit("ftplib.connect", self, self.host, self.port) @@ -316,8 +322,13 @@ def makeport(self): return sock def makepasv(self): + """Internal: Does the PASV or EPSV handshake -> (address, port)""" if self.af == socket.AF_INET: - host, port = parse227(self.sendcmd('PASV')) + untrusted_host, port = parse227(self.sendcmd('PASV')) + if self.trust_server_pasv_ipv4_address: + host = untrusted_host + else: + host = self.sock.getpeername()[0] else: host, port = parse229(self.sendcmd('EPSV'), self.sock.getpeername()) return host, port @@ -704,9 +715,10 @@ class FTP_TLS(FTP): ''' 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): + 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") @@ -725,12 +737,13 @@ def __init__(self, host='', user='', passwd='', acct='', keyfile=None, keyfile=keyfile) self.context = context self._prot_p = False - FTP.__init__(self, host, user, passwd, acct, timeout, source_address) + super().__init__(host, user, passwd, acct, + timeout, source_address, encoding=encoding) def login(self, user='', passwd='', acct='', secure=True): if secure and not isinstance(self.sock, ssl.SSLSocket): self.auth() - return FTP.login(self, user, passwd, acct) + return super().login(user, passwd, acct) def auth(self): '''Set up secure control connection by using TLS/SSL.''' @@ -740,8 +753,7 @@ def auth(self): resp = self.voidcmd('AUTH TLS') else: resp = self.voidcmd('AUTH SSL') - self.sock = self.context.wrap_socket(self.sock, - server_hostname=self.host) + self.sock = self.context.wrap_socket(self.sock, server_hostname=self.host) self.file = self.sock.makefile(mode='r', encoding=self.encoding) return resp @@ -778,7 +790,7 @@ def prot_c(self): # --- Overridden FTP methods def ntransfercmd(self, cmd, rest=None): - conn, size = FTP.ntransfercmd(self, cmd, rest) + conn, size = super().ntransfercmd(cmd, rest) if self._prot_p: conn = self.context.wrap_socket(conn, server_hostname=self.host) @@ -823,7 +835,6 @@ def parse227(resp): '''Parse the '227' response for a PASV request. Raises error_proto if it does not contain '(h1,h2,h3,h4,p1,p2)' Return ('host.addr.as.numbers', port#) tuple.''' - if resp[:3] != '227': raise error_reply(resp) global _227_re @@ -843,7 +854,6 @@ def parse229(resp, peer): '''Parse the '229' response for an EPSV request. Raises error_proto if it does not contain '(|||port|)' Return ('host.addr.as.numbers', port#) tuple.''' - if resp[:3] != '229': raise error_reply(resp) left = resp.find('(') @@ -865,7 +875,6 @@ def parse257(resp): '''Parse the '257' response for a MKD or PWD request. This is a response to a MKD or PWD request: a directory name. Returns the directoryname in the 257 reply.''' - if resp[:3] != '257': raise error_reply(resp) if resp[3:5] != ' "': diff --git a/Lib/test/test_ftplib.py b/Lib/test/test_ftplib.py index 1c61697e93..e8c126ddc4 100644 --- a/Lib/test/test_ftplib.py +++ b/Lib/test/test_ftplib.py @@ -3,16 +3,14 @@ # Modified by Giampaolo Rodola' to test FTP class, IPv6 and TLS # environment -import unittest import ftplib -import asyncore -import asynchat import socket import io import errno import os import threading import time +import unittest try: import ssl except ImportError: @@ -31,13 +29,19 @@ if getattr(sys, '_rustpython_debugbuild', False): raise unittest.SkipTest("something's weird on debug builds") +asynchat = warnings_helper.import_deprecated('asynchat') +asyncore = warnings_helper.import_deprecated('asyncore') + + +support.requires_working_socket(module=True) TIMEOUT = support.LOOPBACK_TIMEOUT +DEFAULT_ENCODING = 'utf-8' # the dummy data returned by server over the data channel when # RETR, LIST, NLST, MLSD commands are issued -RETR_DATA = 'abcde12345\r\n' * 1000 -LIST_DATA = 'foo\r\nbar\r\n' -NLST_DATA = 'foo\r\nbar\r\n' +RETR_DATA = 'abcde12345\r\n' * 1000 + 'non-ascii char \xAE\r\n' +LIST_DATA = 'foo\r\nbar\r\n non-ascii char \xAE\r\n' +NLST_DATA = 'foo\r\nbar\r\n non-ascii char \xAE\r\n' MLSD_DATA = ("type=cdir;perm=el;unique==keVO1+ZF4; test\r\n" "type=pdir;perm=e;unique==keVO1+d?3; ..\r\n" "type=OS.unix=slink:/foobar;perm=;unique==keVO1+4G4; foobar\r\n" @@ -52,7 +56,16 @@ "type=dir;perm=cpmel;unique==keVO1+7G4; incoming\r\n" "type=file;perm=r;unique==keVO1+1G4; file2\r\n" "type=file;perm=r;unique==keVO1+1G4; file3\r\n" - "type=file;perm=r;unique==keVO1+1G4; file4\r\n") + "type=file;perm=r;unique==keVO1+1G4; file4\r\n" + "type=dir;perm=cpmel;unique==SGP1; dir \xAE non-ascii char\r\n" + "type=file;perm=r;unique==SGP2; file \xAE non-ascii char\r\n") + + +def default_error_handler(): + # bpo-44359: Silently ignore socket errors. Such errors occur when a client + # socket is closed, in TestFTPClass.tearDown() and makepasv() tests, and + # the server gets an error on its side. + pass class DummyDTPHandler(asynchat.async_chat): @@ -62,9 +75,11 @@ def __init__(self, conn, baseclass): asynchat.async_chat.__init__(self, conn) self.baseclass = baseclass self.baseclass.last_received_data = '' + self.encoding = baseclass.encoding def handle_read(self): - self.baseclass.last_received_data += self.recv(1024).decode('ascii') + new_data = self.recv(1024).decode(self.encoding, 'replace') + self.baseclass.last_received_data += new_data def handle_close(self): # XXX: this method can be called many times in a row for a single @@ -81,17 +96,17 @@ def push(self, what): self.baseclass.next_data = None if not what: return self.close_when_done() - super(DummyDTPHandler, self).push(what.encode('ascii')) + super(DummyDTPHandler, self).push(what.encode(self.encoding)) def handle_error(self): - raise Exception + default_error_handler() class DummyFTPHandler(asynchat.async_chat): dtp_handler = DummyDTPHandler - def __init__(self, conn): + def __init__(self, conn, encoding=DEFAULT_ENCODING): asynchat.async_chat.__init__(self, conn) # tells the socket to handle urgent data inline (ABOR command) self.socket.setsockopt(socket.SOL_SOCKET, socket.SO_OOBINLINE, 1) @@ -105,12 +120,17 @@ def __init__(self, conn): self.rest = None self.next_retr_data = RETR_DATA self.push('220 welcome') + self.encoding = encoding + # We use this as the string IPv4 address to direct the client + # to in response to a PASV command. To test security behavior. + # https://bugs.python.org/issue43285/. + self.fake_pasv_server_ip = '252.253.254.255' def collect_incoming_data(self, data): self.in_buffer.append(data) def found_terminator(self): - line = b''.join(self.in_buffer).decode('ascii') + line = b''.join(self.in_buffer).decode(self.encoding) self.in_buffer = [] if self.next_response: self.push(self.next_response) @@ -129,10 +149,10 @@ def found_terminator(self): self.push('550 command "%s" not understood.' %cmd) def handle_error(self): - raise Exception + default_error_handler() def push(self, data): - asynchat.async_chat.push(self, data.encode('ascii') + b'\r\n') + asynchat.async_chat.push(self, data.encode(self.encoding) + b'\r\n') def cmd_port(self, arg): addr = list(map(int, arg.split(','))) @@ -145,7 +165,8 @@ def cmd_port(self, arg): def cmd_pasv(self, arg): with socket.create_server((self.socket.getsockname()[0], 0)) as sock: sock.settimeout(TIMEOUT) - ip, port = sock.getsockname()[:2] + port = sock.getsockname()[1] + ip = self.fake_pasv_server_ip ip = ip.replace('.', ','); p1 = port / 256; p2 = port % 256 self.push('227 entering passive mode (%s,%d,%d)' %(ip, p1, p2)) conn, addr = sock.accept() @@ -262,7 +283,7 @@ class DummyFTPServer(asyncore.dispatcher, threading.Thread): handler = DummyFTPHandler - def __init__(self, address, af=socket.AF_INET): + def __init__(self, address, af=socket.AF_INET, encoding=DEFAULT_ENCODING): threading.Thread.__init__(self) asyncore.dispatcher.__init__(self) self.daemon = True @@ -273,6 +294,7 @@ def __init__(self, address, af=socket.AF_INET): self.active_lock = threading.Lock() self.host, self.port = self.socket.getsockname()[:2] self.handler_instance = None + self.encoding = encoding def start(self): assert not self.active @@ -295,7 +317,7 @@ def stop(self): self.join() def handle_accepted(self, conn, addr): - self.handler_instance = self.handler(conn) + self.handler_instance = self.handler(conn, encoding=self.encoding) def handle_connect(self): self.close() @@ -305,7 +327,7 @@ def writable(self): return 0 def handle_error(self): - raise Exception + default_error_handler() if ssl is not None: @@ -320,7 +342,7 @@ class SSLConnection(asyncore.dispatcher): _ssl_closing = False def secure_connection(self): - context = ssl.SSLContext() + context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) context.load_cert_chain(CERTFILE) socket = context.wrap_socket(self.socket, suppress_ragged_eofs=False, @@ -357,7 +379,7 @@ def _do_ssl_shutdown(self): if err.args[0] in (ssl.SSL_ERROR_WANT_READ, ssl.SSL_ERROR_WANT_WRITE): return - except OSError as err: + except OSError: # Any "socket error" corresponds to a SSL_ERROR_SYSCALL return # from OpenSSL's SSL_shutdown(), corresponding to a # closed socket condition. See also: @@ -408,7 +430,7 @@ def recv(self, buffer_size): raise def handle_error(self): - raise Exception + default_error_handler() def close(self): if (isinstance(self.socket, ssl.SSLSocket) and @@ -432,8 +454,8 @@ class DummyTLS_FTPHandler(SSLConnection, DummyFTPHandler): dtp_handler = DummyTLS_DTPHandler - def __init__(self, conn): - DummyFTPHandler.__init__(self, conn) + def __init__(self, conn, encoding=DEFAULT_ENCODING): + DummyFTPHandler.__init__(self, conn, encoding=encoding) self.secure_data_channel = False self._ccc = False @@ -473,10 +495,10 @@ class DummyTLS_FTPServer(DummyFTPServer): class TestFTPClass(TestCase): - def setUp(self): - self.server = DummyFTPServer((HOST, 0)) + def setUp(self, encoding=DEFAULT_ENCODING): + self.server = DummyFTPServer((HOST, 0), encoding=encoding) self.server.start() - self.client = ftplib.FTP(timeout=TIMEOUT) + self.client = ftplib.FTP(timeout=TIMEOUT, encoding=encoding) self.client.connect(self.server.host, self.server.port) def tearDown(self): @@ -576,14 +598,14 @@ def test_abort(self): def test_retrbinary(self): def callback(data): - received.append(data.decode('ascii')) + received.append(data.decode(self.client.encoding)) received = [] self.client.retrbinary('retr', callback) self.check_data(''.join(received), RETR_DATA) def test_retrbinary_rest(self): def callback(data): - received.append(data.decode('ascii')) + received.append(data.decode(self.client.encoding)) for rest in (0, 10, 20): received = [] self.client.retrbinary('retr', callback, rest=rest) @@ -596,7 +618,7 @@ def test_retrlines(self): @unittest.skip("TODO: RUSTPYTHON; weird limiting to 8192, something w/ buffering?") def test_storbinary(self): - f = io.BytesIO(RETR_DATA.encode('ascii')) + f = io.BytesIO(RETR_DATA.encode(self.client.encoding)) self.client.storbinary('stor', f) self.check_data(self.server.handler_instance.last_received_data, RETR_DATA) # test new callback arg @@ -606,14 +628,16 @@ def test_storbinary(self): self.assertTrue(flag) def test_storbinary_rest(self): - f = io.BytesIO(RETR_DATA.replace('\r\n', '\n').encode('ascii')) + data = RETR_DATA.replace('\r\n', '\n').encode(self.client.encoding) + f = io.BytesIO(data) for r in (30, '30'): f.seek(0) self.client.storbinary('stor', f, rest=r) self.assertEqual(self.server.handler_instance.rest, str(r)) def test_storlines(self): - f = io.BytesIO(RETR_DATA.replace('\r\n', '\n').encode('ascii')) + data = RETR_DATA.replace('\r\n', '\n').encode(self.client.encoding) + f = io.BytesIO(data) self.client.storlines('stor', f) self.check_data(self.server.handler_instance.last_received_data, RETR_DATA) # test new callback arg @@ -623,7 +647,7 @@ def test_storlines(self): self.assertTrue(flag) f = io.StringIO(RETR_DATA.replace('\r\n', '\n')) - # stowarnings_helper.check_warningsary file, not a text file + # storlines() expects a binary file, not a text file with warnings_helper.check_warnings(('', BytesWarning), quiet=True): self.assertRaises(TypeError, self.client.storlines, 'stor foo', f) @@ -707,6 +731,26 @@ def test_makepasv(self): # IPv4 is in use, just make sure send_epsv has not been used self.assertEqual(self.server.handler_instance.last_received_cmd, 'pasv') + def test_makepasv_issue43285_security_disabled(self): + """Test the opt-in to the old vulnerable behavior.""" + self.client.trust_server_pasv_ipv4_address = True + bad_host, port = self.client.makepasv() + self.assertEqual( + bad_host, self.server.handler_instance.fake_pasv_server_ip) + # Opening and closing a connection keeps the dummy server happy + # instead of timing out on accept. + socket.create_connection((self.client.sock.getpeername()[0], port), + timeout=TIMEOUT).close() + + def test_makepasv_issue43285_security_enabled_default(self): + self.assertFalse(self.client.trust_server_pasv_ipv4_address) + trusted_host, port = self.client.makepasv() + self.assertNotEqual( + trusted_host, self.server.handler_instance.fake_pasv_server_ip) + # Opening and closing a connection keeps the dummy server happy + # instead of timing out on accept. + socket.create_connection((trusted_host, port), timeout=TIMEOUT).close() + def test_with_statement(self): self.client.quit() @@ -802,14 +846,32 @@ def test_storlines_too_long(self): f = io.BytesIO(b'x' * self.client.maxline * 2) self.assertRaises(ftplib.Error, self.client.storlines, 'stor', f) + def test_encoding_param(self): + encodings = ['latin-1', 'utf-8'] + for encoding in encodings: + with self.subTest(encoding=encoding): + self.tearDown() + self.setUp(encoding=encoding) + self.assertEqual(encoding, self.client.encoding) + self.test_retrbinary() + self.test_storbinary() + self.test_retrlines() + new_dir = self.client.mkd('/non-ascii dir \xAE') + self.check_data(new_dir, '/non-ascii dir \xAE') + # Check default encoding + client = ftplib.FTP(timeout=TIMEOUT) + self.assertEqual(DEFAULT_ENCODING, client.encoding) + @skipUnless(socket_helper.IPV6_ENABLED, "IPv6 not enabled") class TestIPv6Environment(TestCase): def setUp(self): - self.server = DummyFTPServer((HOSTv6, 0), af=socket.AF_INET6) + self.server = DummyFTPServer((HOSTv6, 0), + af=socket.AF_INET6, + encoding=DEFAULT_ENCODING) self.server.start() - self.client = ftplib.FTP(timeout=TIMEOUT) + self.client = ftplib.FTP(timeout=TIMEOUT, encoding=DEFAULT_ENCODING) self.client.connect(self.server.host, self.server.port) def tearDown(self): @@ -836,7 +898,7 @@ def test_makepasv(self): def test_transfer(self): def retr(): def callback(data): - received.append(data.decode('ascii')) + received.append(data.decode(self.client.encoding)) received = [] self.client.retrbinary('retr', callback) self.assertEqual(len(''.join(received)), len(RETR_DATA)) @@ -854,10 +916,10 @@ class TestTLS_FTPClassMixin(TestFTPClass): and data connections first. """ - def setUp(self): - self.server = DummyTLS_FTPServer((HOST, 0)) + def setUp(self, encoding=DEFAULT_ENCODING): + self.server = DummyTLS_FTPServer((HOST, 0), encoding=encoding) self.server.start() - self.client = ftplib.FTP_TLS(timeout=TIMEOUT) + self.client = ftplib.FTP_TLS(timeout=TIMEOUT, encoding=encoding) self.client.connect(self.server.host, self.server.port) # enable TLS self.client.auth() @@ -869,8 +931,8 @@ def setUp(self): class TestTLS_FTPClass(TestCase): """Specific TLS_FTP class tests.""" - def setUp(self): - self.server = DummyTLS_FTPServer((HOST, 0)) + def setUp(self, encoding=DEFAULT_ENCODING): + self.server = DummyTLS_FTPServer((HOST, 0), encoding=encoding) self.server.start() self.client = ftplib.FTP_TLS(timeout=TIMEOUT) self.client.connect(self.server.host, self.server.port) @@ -891,7 +953,8 @@ def test_data_connection(self): # clear text with self.client.transfercmd('list') as sock: self.assertNotIsInstance(sock, ssl.SSLSocket) - self.assertEqual(sock.recv(1024), LIST_DATA.encode('ascii')) + self.assertEqual(sock.recv(1024), + LIST_DATA.encode(self.client.encoding)) self.assertEqual(self.client.voidresp(), "226 transfer complete") # secured, after PROT P @@ -900,14 +963,16 @@ def test_data_connection(self): self.assertIsInstance(sock, ssl.SSLSocket) # consume from SSL socket to finalize handshake and avoid # "SSLError [SSL] shutdown while in init" - self.assertEqual(sock.recv(1024), LIST_DATA.encode('ascii')) + self.assertEqual(sock.recv(1024), + LIST_DATA.encode(self.client.encoding)) self.assertEqual(self.client.voidresp(), "226 transfer complete") # PROT C is issued, the connection must be in cleartext again self.client.prot_c() with self.client.transfercmd('list') as sock: self.assertNotIsInstance(sock, ssl.SSLSocket) - self.assertEqual(sock.recv(1024), LIST_DATA.encode('ascii')) + self.assertEqual(sock.recv(1024), + LIST_DATA.encode(self.client.encoding)) self.assertEqual(self.client.voidresp(), "226 transfer complete") def test_login(self): @@ -1017,7 +1082,7 @@ def server(self): self.evt.set() try: conn, addr = self.sock.accept() - except socket.timeout: + except TimeoutError: pass else: conn.sendall(b"1 Hola mundo\n") @@ -1059,6 +1124,10 @@ def testTimeoutValue(self): self.evt.wait() ftp.close() + # bpo-39259 + with self.assertRaises(ValueError): + ftplib.FTP(HOST, timeout=0) + def testTimeoutConnect(self): ftp = ftplib.FTP() ftp.connect(HOST, timeout=30) @@ -1084,9 +1153,10 @@ def testTimeoutDirectAccess(self): class MiscTestCase(TestCase): def test__all__(self): - not_exported = {'MSG_OOB', 'FTP_PORT', 'MAXLINE', 'CRLF', 'B_CRLF', - 'Error', 'parse150', 'parse227', 'parse229', 'parse257', - 'print_line', 'ftpcp', 'test'} + not_exported = { + 'MSG_OOB', 'FTP_PORT', 'MAXLINE', 'CRLF', 'B_CRLF', 'Error', + 'parse150', 'parse227', 'parse229', 'parse257', 'print_line', + 'ftpcp', 'test'} support.check__all__(self, ftplib, not_exported=not_exported)