Skip to content

Conversation

toadjaune
Copy link

Hi,

There's a bit of confusion regarding FTP over TLS implementation in django-storages, and I'd like to propose a way to fix it here.

Context

So, there are 2 different ways of adding TLS to the FTP protocol (wikipedia ref) :

  1. Implicit TLS :

This is similar to what is done for HTTPS ; the client connects to a different port (990 by default), establishes a TLS connection, then establishes a normal FTP session inside the TLS tunnel.
This is what the ftps URL scheme represents.
This way of implementing TLS in FTP was never really standardized, and is nowadays considered deprecated

  1. Explicit TLS :

This method of implementation was specified in RFC 4217, and is the preferred method for securing FTP.
Here, the client initiates a plaintext FTP connexion to the normal port (21), and issues a command to upgrade the connection to TLS. Well-behaved clients would do this before sending anything else (especially sending the password), and secure servers would refuse any command before upgrading the connection.

As this method is part of the FTP protocol, and on the same port as plaintext FTP, no URL scheme is standardized.
It's however common for FTP clients and servers to accept the non-standard ftpes scheme to convey the idea that despite the connection being originally established in plaintext, upgrading to a secure connection is mandatory.

Problem

Django-storages implements the second method, and yet uses the ftps scheme associated with the first one for enabling this behavior.

This is confusing.

Proposed solution

  • Add support for ftpes scheme, with the same behavior as already existing (done in this PR)
  • Add a logging warning if initializing the connector with the ftps scheme (I can add it in this PR if it's fine with you)
  • Removing the support for the ftps scheme at the next major version update

I'm of course open to discussion regarding the points 2 and 3, as one could argue that it's not worth making a breaking change, warning just might be enough ?

Also, documenting this is probably a good idea as well.

@toadjaune
Copy link
Author

NB : This PR includes commits from #1508, to avoid git conflits (that's why it's in draft state)
Ideally, I'd rebase it cleanly once #1508 is merged, but I can also rewrite it to target the current master code if you prefer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant