Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 17 additions & 8 deletions Doc/library/ftplib.rst
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,14 @@ FTP objects
.. class:: FTP(host='', user='', passwd='', acct='', timeout=None, \
source_address=None, *, encoding='utf-8')

.. warning::

Use of this class may create a vulnerability to
`man-in-the-middle attack <https://en.wikipedia.org/wiki/Man-in-the-middle_attack>`_,
please consider using the :class:`FTP_TLS` class, and reflect
on your `threat model <https://en.wikipedia.org/wiki/Threat_model>`_
before using an unprotected FTP connection.

Return a new instance of the :class:`FTP` class.

:param str host:
Expand Down Expand Up @@ -454,10 +462,6 @@ FTP_TLS objects
Connect to port 21 implicitly securing the FTP control connection
before authenticating.

.. note::
The user must explicitly secure the data connection
by calling the :meth:`prot_p` method.

:param str host:
The hostname to connect to.
If given, :code:`connect(host)` is implicitly called by the constructor.
Expand Down Expand Up @@ -516,8 +520,6 @@ FTP_TLS objects
>>> ftps = FTP_TLS('ftp.pureftpd.org')
>>> ftps.login()
'230 Anonymous user logged in'
>>> ftps.prot_p()
'200 Data protection level set to "private"'
>>> ftps.nlst()
['6jack', 'OpenBSD', 'antilink', 'blogbench', 'bsdcam', 'clockspeed', 'djbdns-jedi', 'docs', 'eaccelerator-jedi', 'favicon.ico', 'francotone', 'fugu', 'ignore', 'libpuzzle', 'metalog', 'minidentd', 'misc', 'mysql-udf-global-user-variables', 'php-jenkins-hash', 'php-skein-hash', 'php-webdav', 'phpaudit', 'phpbench', 'pincaster', 'ping', 'posto', 'pub', 'public', 'public_keys', 'pure-ftpd', 'qscan', 'qtc', 'sharedance', 'skycache', 'sound', 'tmp', 'ucarp']

Expand Down Expand Up @@ -548,11 +550,18 @@ FTP_TLS objects

.. method:: FTP_TLS.prot_p()

Set up secure data connection.
Set up secure data connection (with TLS).

.. method:: FTP_TLS.prot_c()

Set up clear text data connection.
Set up clear text data connection (without TLS).

.. warning::

Calling this method may create a vulnerability to
`man-in-the-middle attack <https://en.wikipedia.org/wiki/Man-in-the-middle_attack>`_.
Please reflect on your `threat model <https://en.wikipedia.org/wiki/Threat_model>`_
before requesting clear text data connection without TLS.


Module variables
Expand Down
4 changes: 1 addition & 3 deletions Lib/ftplib.py
Original file line number Diff line number Diff line change
Expand Up @@ -681,9 +681,6 @@ class FTP_TLS(FTP):
Connect as usual to port 21 implicitly securing the FTP control
connection before authenticating.

Securing the data connection requires user to explicitly ask
for it by calling prot_p() method.

Usage example:
>>> from ftplib import FTP_TLS
>>> ftps = FTP_TLS('ftp.python.org')
Expand Down Expand Up @@ -733,6 +730,7 @@ def auth(self):
resp = self.voidcmd('AUTH SSL')
self.sock = self.context.wrap_socket(self.sock, server_hostname=self.host)
self.file = self.sock.makefile(mode='r', encoding=self.encoding)
self.prot_p()
return resp

def ccc(self):
Expand Down
16 changes: 3 additions & 13 deletions Lib/test/test_ftplib.py
Original file line number Diff line number Diff line change
Expand Up @@ -917,7 +917,6 @@ def setUp(self, encoding=DEFAULT_ENCODING):
self.client.connect(self.server.host, self.server.port)
# enable TLS
self.client.auth()
self.client.prot_p()


@skipUnless(ssl, "SSL not available")
Expand All @@ -944,15 +943,9 @@ def test_control_connection(self):
self.assertIsInstance(self.client.sock, ssl.SSLSocket)

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(self.client.encoding))
self.assertEqual(self.client.voidresp(), "226 transfer complete")
self.client.login()

# secured, after PROT P
self.client.prot_p()
# secured
with self.client.transfercmd('list') as sock:
self.assertIsInstance(sock, ssl.SSLSocket)
# consume from SSL socket to finalize handshake and avoid
Expand All @@ -961,7 +954,7 @@ def test_data_connection(self):
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
# PROT C is issued, the connection must be in cleartext
self.client.prot_c()
with self.client.transfercmd('list') as sock:
self.assertNotIsInstance(sock, ssl.SSLSocket)
Expand Down Expand Up @@ -1000,7 +993,6 @@ def test_context(self):
self.assertIs(self.client.sock.context, ctx)
self.assertIsInstance(self.client.sock, ssl.SSLSocket)

self.client.prot_p()
with self.client.transfercmd('list') as sock:
self.assertIs(sock.context, ctx)
self.assertIsInstance(sock, ssl.SSLSocket)
Expand Down Expand Up @@ -1028,7 +1020,6 @@ def test_check_hostname(self):
# exception quits connection

self.client.connect(self.server.host, self.server.port)
self.client.prot_p()
with self.assertRaises(ssl.CertificateError):
with self.client.transfercmd("list") as sock:
pass
Expand All @@ -1039,7 +1030,6 @@ def test_check_hostname(self):
self.client.quit()

self.client.connect("localhost", self.server.port)
self.client.prot_p()
with self.client.transfercmd("list") as sock:
pass

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Fix defaults of :class:`ftplib.FTP_TLS` to not leave the data connection
without TLS and vulnerable to `man-in-the-middle attack
<https://en.wikipedia.org/wiki/Man-in-the-middle_attack>`_. Also extend the
documenation of :class:`ftplib.FTP_TLS` and :meth:`ftplib.FTP_TLS.prot_c`
to warn about the security implications. +Patch by Sebastian Pipping.
Loading