classification
Title: add ftp-tls support to ftplib - RFC 4217
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.2
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: eric.araujo, giampaolo.rodola, gregory.p.smith, iElectric, janssen, jeffo, josiah.carlson, josiahcarlson, lgedgar, lszyba1, orsenthil, pitrou, qwavel, roberte, twhitema
Priority: normal Keywords: patch

Created on 2008-02-09 02:16 by gregory.p.smith, last changed 2010-05-20 20:37 by giampaolo.rodola. This issue is now closed.

Files
File name Uploaded Description Edit
ftplib.diff giampaolo.rodola, 2008-02-21 00:19 adds FTP over TLS support (RFC-4217)
ftplib.patch giampaolo.rodola, 2009-03-04 16:49 New patch which shutdown the SSL layer before closing FTP data channel
ftplib.patch giampaolo.rodola, 2009-10-16 18:48 includes changes described in message 94103
ftplib.patch giampaolo.rodola, 2009-10-17 20:50 includes tests and doc
ftplib.patch giampaolo.rodola, 2009-10-19 12:55 implements changes described in msg 94225
ftplib.patch giampaolo.rodola, 2009-11-15 16:55 adds SSL support as described in comment 95005
ftptls-py3k.patch pitrou, 2009-11-17 09:36
ftptls-py3k-2.patch pitrou, 2009-11-17 09:45
Messages (64)
msg62217 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2008-02-09 02:16
ftplib does not support ftp over SSL / TLS as described in RFC 4217. 
This would be a nice thing for someone wanting to contribute something
to add.
msg62608 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2008-02-21 00:19
I've tried to work on this in the last 2 days and here is my
implementation attempt.
The patch in attachment provides a new FTP subclass which connects to
port 21 as usual leaving control and data channels implicitly unprotected.
Securing control and data channels requires user to explicitly ask for
it by using the new auth_tls() and prot_p() methods as recommended by
the standard RFC.

Usage example:

>>> from ftplib import FTP_TLS
>>> ftps = FTP_TLS('ftp.python.org')
>>> ftps.auth_tls()  # switch to secure control connection
'234 Using authentication type TLS'
>>> ftps.login()  # login anonimously
'230 Guest login ok, access restrictions apply.'
>>> ftps.prot_p()  # switch to secure data connection
'200 Protection level set to P'
>>> ftps.retrlines('LIST')  # list directory content securely
total 9
drwxr-xr-x   8 root     wheel        1024 Jan  3  1994 .
drwxr-xr-x   8 root     wheel        1024 Jan  3  1994 ..
drwxr-xr-x   2 root     wheel        1024 Jan  3  1994 bin
drwxr-xr-x   2 root     wheel        1024 Jan  3  1994 etc
d-wxrwxr-x   2 ftp      wheel        1024 Sep  5 13:43 incoming
drwxr-xr-x   2 root     wheel        1024 Nov 17  1993 lib
drwxr-xr-x   6 1094     wheel        1024 Sep 13 19:07 pub
drwxr-xr-x   3 root     wheel        1024 Jan  3  1994 usr
-rw-r--r--   1 root     root          312 Aug  1  1994 welcome.msg
'226 Transfer complete.'
>>> ftps.quit()
'221 Goodbye.'
>>>

It also provides a prot_c() method to switch back to a plain text data
channel.
Sorry in advance for any grammatical error I could have made in comments
and docstrings.

Comments are greatly appreciated.
msg62699 - (view) Author: Bill Janssen (janssen) * (Python committer) Date: 2008-02-22 20:54
This is a straightforward implementation of client-side use of SSL, but
it's missing a test case for evaluation.  It should include a patch to
test_ftplib to test it.

Another thing to look at is what the useful arguments are to pass in for
TLS usage over FTP.  If, for example, the client needs to validate the
server's certificate or identity, provision should be made for a file of
cacerts to be passed to the FTP_TLS instance.  Passing in a keyfile and
certfile is usually only necessary when the client uses them to identify
itself to the server.
msg63462 - (view) Author: Robert E. (roberte) Date: 2008-03-11 16:12
Damn I should have looked here earlier - so I implemented FTPS based on
ftplib myself yesterday (looked quite similar). I had a look at you
patch and got one question.
In FTP_TLS.__init__ you call FTP.__init__. The latter in turn calls
FTP.login if a username is supplied. Thus you end up trying to login
before issuing the AUTH TLS command. The result is, that username and
passwords are send unencrypted. Or do I miss a subtle trick here? I
solved that problem by wrapping FTP.connect in my subclass, essentially
calling an FTP.connect and then it would be in this case an auth_tls and
prot_p afterwards.
msg63707 - (view) Author: Domen Kožar (iElectric) Date: 2008-03-17 18:23
The lib should give programmer choice wether to send login through TLS
or not. (as it is described in RFC 4217).

Also, there should be an optional parameter to specify port for ftp
connection.
msg64088 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2008-03-19 19:22
> This is a straightforward implementation of client-side use of SSL,
> but it's missing a test case for evaluation.  It should include a 
> patch to test_ftplib to test it.

I'm not sure how it could be tested, since we don't have an FTPS server
to test against. The current test suite itself only tests the new
timeout feature added in ftplib.FTP class in Python 2.6 and nothing else.

> Another thing to look at is what the useful arguments are to pass in
> for TLS usage over FTP.  If, for example, the client needs to validate
> the server's certificate or identity, provision should be made for a 
> file of cacerts to be passed to the FTP_TLS instance.  Passing in a 
> keyfile and certfile is usually only necessary when the client uses 
> them to identify itself to the server.

I drew from the SSL classes defined in httplib, imaplib, poplib, smtplib
and urllib modules which accept a keyfile and a certfile in the class
constructor so I thought it was the "right way". Is there a reason why
the FTP protocol should behave differently as you have described?

> In FTP_TLS.__init__ you call FTP.__init__. The latter in turn calls
> FTP.login if a username is supplied. Thus you end up trying to login
> before issuing the AUTH TLS command. The result is, that username and
> passwords are send unencrypted. Or do I miss a subtle trick here? 

You're right, I avoided doing that since the TLS encryption should be
requested specifically by the user. We could implicitly secure the
control connection if the "user" argument is provided when invoking the
class constructor and eventually add a "secure" kwarg to login method
that defaults to True.

> The lib should give programmer choice wether to send login through TLS
> or not. (as it is described in RFC 4217).

This is what it does if you use auth_tls() before login().

> Also, there should be an optional parameter to specify port for ftp
> connection.

This is already possible by using the original (inherited) connect() method.
msg64093 - (view) Author: Bill Janssen (janssen) * (Python committer) Date: 2008-03-19 20:19
As you point out, the other classes should be fixed.  The old client-side
protocol was never very well thought out, IMHO.  Continuing to propagate it
would be a mistake.
Bill

On Wed, Mar 19, 2008 at 12:22 PM, Giampaolo Rodola' <report@bugs.python.org>
wrote:

>
> Giampaolo Rodola' <billiejoex@users.sourceforge.net> added the comment:
> > Another thing to look at is what the useful arguments are to pass in
> > for TLS usage over FTP.  If, for example, the client needs to validate
> > the server's certificate or identity, provision should be made for a
> > file of cacerts to be passed to the FTP_TLS instance.  Passing in a
> > keyfile and certfile is usually only necessary when the client uses
> > them to identify itself to the server.
>
> I drew from the SSL classes defined in httplib, imaplib, poplib, smtplib
> and urllib modules which accept a keyfile and a certfile in the class
> constructor so I thought it was the "right way". Is there a reason why
> the FTP protocol should behave differently as you have described?
>
msg64097 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2008-03-19 20:46
> As you point out, the other classes should be fixed.  The old 
> client-side protocol was never very well thought out, IMHO.  
> Continuing to propagate it would be a mistake.

Ok, how do you think it would have be modified?
Could you provide an example or a new patch?
msg64146 - (view) Author: Bill Janssen (janssen) * (Python committer) Date: 2008-03-20 03:56
Probably what I should do is fix httplib, that would provide an example we
could extend to the rest of the modules.
Bill

On Wed, Mar 19, 2008 at 1:46 PM, Giampaolo Rodola' <report@bugs.python.org>
wrote:

>
> Giampaolo Rodola' <billiejoex@users.sourceforge.net> added the comment:
>
> > As you point out, the other classes should be fixed.  The old
> > client-side protocol was never very well thought out, IMHO.
> > Continuing to propagate it would be a mistake.
>
> Ok, how do you think it would have be modified?
> Could you provide an example or a new patch?
>
> __________________________________
> Tracker <report@bugs.python.org>
> <http://bugs.python.org/issue2054>
> __________________________________
>
msg64147 - (view) Author: Bill Janssen (janssen) * (Python committer) Date: 2008-03-20 03:56
Once I've got JCC working, and finished the SSL work for 2.6.

On Wed, Mar 19, 2008 at 1:46 PM, Giampaolo Rodola' <report@bugs.python.org>
wrote:

>
> Giampaolo Rodola' <billiejoex@users.sourceforge.net> added the comment:
>
> > As you point out, the other classes should be fixed.  The old
> > client-side protocol was never very well thought out, IMHO.
> > Continuing to propagate it would be a mistake.
>
> Ok, how do you think it would have be modified?
> Could you provide an example or a new patch?
>
> __________________________________
> Tracker <report@bugs.python.org>
> <http://bugs.python.org/issue2054>
> __________________________________
>
msg64229 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-03-21 01:59
FWIW, m2crypto already provides an FTP-TLS facility with an
ftplib-compatible API. See http://chandlerproject.org/Projects/MeTooCrypto
msg64239 - (view) Author: Robert E. (roberte) Date: 2008-03-21 12:43
>> FWIW, m2crypto already provides an FTP-TLS facility with an
>> ftplib-compatible API. See
http://chandlerproject.org/Projects/MeTooCrypto

Right but m2crypto is not part of the standard library (and is not going
to be anytime soon). SSL is scheduled to be in 2.6. So it would be the
natural choice to use that SSL binding to extend the ftp library. Of
course we could have a look how ftps is implemented in m2crypto.

Concerning the plain-text login. I think a FTPS class should default to
encrypted login (you could use the ftp class if you dont want). In no
way should the login credentials be sent unencrypted on default. Using
another parameter might be a soulution to that, though I would prefer
the library to raise an error if establishing an FTPS connection did not
succeed. The main program could then catch it and decide how to proceed
(using plain ftp or aborting according to a given policy).
msg64296 - (view) Author: Bill Janssen (janssen) * (Python committer) Date: 2008-03-21 23:28
On Fri, Mar 21, 2008 at 5:43 AM, Robert E. <report@bugs.python.org> wrote:

>
> Robert E. <robert@re-factory.de> added the comment:
>
> Concerning the plain-text login. I think a FTPS class should default to
> encrypted login (you could use the ftp class if you dont want). In no
> way should the login credentials be sent unencrypted on default. Using
> another parameter might be a soulution to that, though I would prefer
> the library to raise an error if establishing an FTPS connection did not
> succeed. The main program could then catch it and decide how to proceed
> (using plain ftp or aborting according to a given policy).

Sounds reasonable to me.

Note that FTP is an old and somewhat gnarly protocol, and
doesn't work the way more recent application protocols do.  The SSL
module is designed for TCP-based single-connection call-response
protocols, more or less.  Doing FTPS right might mean we'd have to
extend it.
msg66752 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2008-05-12 18:59
Bill, are there news about the fix to httplib?
I'd like to see this feature included in 2.6 if possible.
msg68978 - (view) Author: Bill Janssen (janssen) * (Python committer) Date: 2008-06-30 01:03
The 2.6/3.0 changes are now up-to-date.  We could reconsider this
problem.  My guess is that we still don't quite know what to do.

I think the issue is that we need a way to "unwrap" the SSL-secured
TCP stream, after it's been used.  So we need to expose the SSL
shutdown mechanism (already in the _ssl.c module) in the Python
code.  Something like

   socket = self.unwrap()

which would return a plain socket.socket instance.
msg69002 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2008-06-30 11:12
Yes, I think that providing an "unwrap" method for the ssl module would
be good, independently from this issue.
With that implemented and httplib fixed in the way you were mentioning
in this same report I can go on with modifying the ftplib patch.
msg69008 - (view) Author: Bill Janssen (janssen) * (Python committer) Date: 2008-06-30 16:09
But httplib is far from fixed.  It's a nasty tarball of interdependencies...

Bill

On Mon, Jun 30, 2008 at 4:12 AM, Giampaolo Rodola' <report@bugs.python.org>
wrote:

>
> Giampaolo Rodola' <billiejoex@users.sourceforge.net> added the comment:
>
> Yes, I think that providing an "unwrap" method for the ssl module would
> be good, independently from this issue.
> With that implemented and httplib fixed in the way you were mentioning
> in this same report I can go on with modifying the ftplib patch.
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue2054>
> _______________________________________
>
msg69010 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2008-06-30 16:45
Ok, so let's leave httplib alone. Let's just add the unwrap() method to
ssl.SSLSocket so that it could be be possible to go ahead with the
current patch.
Modifying it consists in just adding a new prot_c method (I could do that).
As for the cacerts needed to be passed to FTP_TLS constructor you could
do that afterwards.
msg69011 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2008-06-30 16:55
Could what I've just said be an idea?
msg69553 - (view) Author: Lukasz Szybalski (lszyba1) Date: 2008-07-11 15:43
Is the ftp-tls able to use certificate to connect to ftps server?
I currently need to connect to state's ftps server which requires
certificate to be present when authenticating. 

Is that option available? What is the current status of this issue?

Thanks,
Lucas
msg71058 - (view) Author: Bill Janssen (janssen) * (Python committer) Date: 2008-08-12 16:05
I think I'm just going to bring the unwrap already in the _ssl.c code
out to the ssl.py module, that seems to be the simplest fix.  Still not
sure you can do a proper fix to ftplib here, but that seems to be a good
thing to do anyway, rather than having people call directly into the
_ssl module to get at it.
msg71061 - (view) Author: Bill Janssen (janssen) * (Python committer) Date: 2008-08-12 17:13
OK, I think I've done the minimal fix necessary to the SSL module to
allow this work to proceed.
msg82612 - (view) Author: Jeff Oyama (jeffo) Date: 2009-02-22 21:40
Just wondering, has anyone done a patch since Bill made the necessary
changes to ssl.py in order to implement FTP TLS? If so, where can I find
it? I would love to test it out.
msg82625 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2009-02-23 12:57
After Bill added SSL's unwrap() method I modified my previous patch so
that it shutdown the SSL layer before closing the data connection.
I successfully tested it against proftpd, vsftpd and pyftpdlib TLS
server [1].

If some python developer could give me an ok on the patch I could start
writing tests and documentation.

[1] http://code.google.com/p/pyftpdlib/source/browse/trunk/demo/tls_ftpd.py
msg82662 - (view) Author: Jeff Oyama (jeffo) Date: 2009-02-24 07:56
Thank you Giampaolo, it works just as I was hoping, =] I tested it on glftpd
using python 2.6.1.
msg82697 - (view) Author: Jeff Oyama (jeffo) Date: 2009-02-25 06:49
Actually I have encountered a possible bug. the close() method doesn't seem
to actually close the connection...

On Mon, Feb 23, 2009 at 11:56 PM, Jeff Oyama <report@bugs.python.org> wrote:

>
> Jeff Oyama <jeff@oyama.org> added the comment:
>
> Thank you Giampaolo, it works just as I was hoping, =] I tested it on
> glftpd
> using python 2.6.1.
>
> Added file: http://bugs.python.org/file13161/unnamed
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue2054>
> _______________________________________
>
msg82704 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2009-02-25 12:41
> Actually I have encountered a possible bug. the close() 
> method doesn't seem to actually close the connection...

Why? What happens exactly?
msg82876 - (view) Author: Jeff Oyama (jeffo) Date: 2009-02-27 21:47
Ok after examining it more closely, it appears to be a false alarm, my
apologies
msg83128 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2009-03-04 16:50
I realized that the attached patch had some indentation issues.
The one in attachment fixes them.
msg94098 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2009-10-15 17:26
Are there chances to see this reviewed?
msg94099 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-10-15 17:56
Well, first some tests should be added.
As for reviewing, it needs to be done by someone competent with FTP and
SSL/TLS. If no such person is available and you are confident that the
patch is ok (and ready to do necessary maintenance), then perhaps you
can commit it. But please add tests first. It is really too easy to
accidentally break a functionality which is not exercised by the test suite.

Oh, and you need to update the documentation too :)
msg94101 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2009-10-15 18:08
I can surely write tests altough I think that issue 3890 might cause 
some problems since the test server I included some months ago is 
asyncore-based and hence asynchronous.

I have a good knowledge of the FTP protocol but someone with a good 
knowledge of SSL willing to review the patch would surely be useful.


In particular I would ask Bill an update about this comment:

> Probably what I should do is fix httplib, that would  provide an 
> example we could extend to the rest of the modules.
> Bill

I don't follow the development of other stdlib modules that use ssl so 
I'm not sure how to proceed to conform with them.
msg94103 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-10-15 18:27
From a quick look at the patch, if you call login() twice, the socket
will be wrapped twice as well? Perhaps auth_tls() should have a
protection against this.

In prot_p() and prot_c(), it seems that self._prot_p is updated
unconditionally, regardless of the FTP response.

In retrbinary(), retrlines(), storbinary() and storlines(), you
certainly want some try/finally blocks so that the data connection
always gets closed at the end.
msg94104 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-10-15 18:34
One more question, why is "ssl_version=ssl.PROTOCOL_TLSv1" hardwired?
msg94142 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2009-10-16 18:43
> From a quick look at the patch, if you call login() twice, the socket
> will be wrapped twice as well? Perhaps auth_tls() should have a
> protection against this.

You're right. Done.

> In prot_p() and prot_c(), it seems that self._prot_p is updated
> unconditionally, regardless of the FTP response.

Both PROT P and PROT C commands expect a 2xx response.
That's why I used voidcmd(). If a response different than 2xx is 
received voidcmd() automatically raises an error_reply exception already 
and self._prot_p doesn't get set.

> One more question, why is "ssl_version=ssl.PROTOCOL_TLSv1" hardwired?

You're right, it shouldn't be.
This is now provided as a class attribute.
The reason I used PROTOCOL_TLSv1 instead of PROTOCOL_SSLv23 (the ssl.py 
default) is because RFC-4217 recommends it.

> In retrbinary(), retrlines(), storbinary() and storlines(), you
> certainly want some try/finally blocks so that the data connection
> always gets closed at the end.

I agree, it's a good practice.
I avoided to do that because the original FTP class is coded like that.
Anyway, this is done too now.

Modified patch is in attachment.

I hope I'll be able to work on tests and documentation during this week-
end.
msg94188 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2009-10-17 20:50
A patch including tests and documentation is now in attachment.
The test TLS server is very similar to pyftpdlib's I draw on:
http://code.google.com/p/pyftpdlib/source/browse/trunk/demo/tls_ftpd.py
I wasn't able to "compile" the documentation so I'm not 100% sure it looks 
perfectly.
Test suite run successfully on Windows XP, Ubuntu 9.04 and FreeBSD 7.1.
msg94225 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-10-18 20:47
You can build the docs by going to the Doc directory and typing "make
html" there. It isn't critical anyway.

The tests failed to run, I had to replace the KEYCERT declaration with:

    KEYCERT = os.path.join(os.path.dirname(__file__), "keycert.pem") 

(and add "import os" at the top)

Another remark: login() doesn't return the response, while it does on
the normal FTP class.

Apart from that, I'm trying to test on a TLS-enabled FTP server, but
even the regular FTP class doesn't seem to work with it (I get "500
Illegal PORT command" when calling retrlines('LIST')).
msg94234 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2009-10-19 12:55
> You can build the docs by going to the Doc directory and typing "make
> html" there. It isn't critical anyway.

Done. It's well formatted now.

> The tests failed to run, I had to replace the KEYCERT declaration with:
>    KEYCERT = os.path.join(os.path.dirname(__file__), "keycert.pem") 
> (and add "import os" at the top)

Done.


> Another remark: login() doesn't return the response, while it does on
> the normal FTP class.

Good catch, thanks. 
Done.

> Apart from that, I'm trying to test on a TLS-enabled FTP server, but
> even the regular FTP class doesn't seem to work with it (I get "500
> Illegal PORT command" when calling retrlines('LIST')).

Strange. I doubt this issue is related with my changes.
I tried FTP_TLS agains proftpd, filezilla server and pyftpdlib and it
works fine.

New patch in attachment btw.
msg94241 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-10-19 15:33
The patch is ok to me. Perhaps Bill wants to take a look, otherwise I
think you can commit.
msg94244 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-10-19 16:59
A last problem:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: attribute name must be string, not 'classobj'
msg94249 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2009-10-19 18:38
> A last problem:
> 
> Traceback (most recent call last):
>   File "<stdin>", line 1, in <module>
> TypeError: attribute name must be string, not 'classobj'

Mmmm this doesn't say much.
When does it happen?
Is that the complete traceback message?

> The patch is ok to me. Perhaps Bill wants to take a look.

I'd like the opinion of Bill too, specifically about what he was talking  
about in comments 62699 and 64093.

> otherwise I think you can commit.

I don't have commit privileges. Someone else should do it.
msg94251 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-10-19 18:47
Ah, sorry, roundup's e-mail interface ate part of the message.
The error happens when doing "from ftplib import *". Apparently __all__
contains a non-string value.

> I don't have commit privileges. Someone else should do it.

Ok, I'll do it if Bill doesn't give a sign of life.
msg94252 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-10-19 18:54
Regarding msg64093, the only API change Bill's suggestion would entail
is an additional optional parameter to the constructor, so adding it
later would be backwards-compatible.
msg94253 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2009-10-19 19:02
> Ah, sorry, roundup's e-mail interface ate part of the message.
> The error happens when doing "from ftplib import *". Apparently 
__all__
> contains a non-string value.

Oh, shame on me! You're right.
Thanks for the great review you're doing.

> Regarding msg64093, the only API change Bill's suggestion would entail
> is an additional optional parameter to the constructor, so adding it
> later would be backwards-compatible.

Good.
Possibly there might be another change we might want to do in future, 
which is adding support for CCC command, useful for reverting the 
control connection back to clear-text.
I remember I tried to do that at the time I submitted the first patch 
but there was a problem with ssl's unwrap() method (I really don't 
remember what exactly).

Anyway, I'll try to put hands on that soon and let you know what 
happens.
The change should be backward compatible as it just consists in adding a 
ccc() method.
msg94452 - (view) Author: Domen Kožar (iElectric) Date: 2009-10-25 14:34
What about AUTH SSL? Or is it too-deprecated?
msg94902 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-11-04 21:53
I noticed you were using ftp.python.org in the example strings, but that
service doesn't seem to be alive. I don't know if there's another public
FTP-TLS server you could rely on...?
msg94996 - (view) Author: Domen Kožar (iElectric) Date: 2009-11-06 20:10
I've tested TLS with several private servers today, seems to work. I 
cannot test patch against FTP SSL encryption, although it is obviously 
not implemented
msg95005 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2009-11-06 23:51
Sorry for delay in the response. The latest messages slipped under my 
radar.

> What about AUTH SSL? Or is it too-deprecated?

I'm not sure about this.
TLS is certainly preferred over SSL and RFC-4217 only refers to TLS 
protocol, altough SSL is mentioned in some chapters.

RFC-4217 states:

> As the SSL/TLS protocols self-negotiate their levels, there is no
> need to distinguish between SSL and TLS in the application layer. 
> The mechanism name for negotiating TLS is the character string 
> identified in {TLS-PARM}.
>
> [...]
>
> {TLS-PARM} - The parameter for the AUTH command to indicate that TLS
> is required.  To request the TLS protocol in accordance with this
> document, the client MUST use 'TLS'


If we want to support SSL we could change the current implementation by 
renaming "auth_tls()" method to just "auth" and play with the 
ssl_version attribute, like this:


class FTP_TLS(FTP):
    ssl_version = ssl.PROTOCOL_TLSv1

    def auth(self):
        if self.ssl_version == ssl.PROTOCOL_TLSv1:
            resp = self.voidcmd('AUTH TLS')
        else:
            resp = self.voidcmd('AUTH SSL')
        ...

The user willing to use SSL instead of TLS will have to change 
ssl_version class attribute with "FTP_TLS.ssl_version = 
ssl.PROTOCOL_TLSv1" and then call auth().

Deciding whether rejecting or accepting it will be up to the server 
depending on how it has been configured (almost all recent FTP servers 
reject SSLv2).

> I noticed you were using ftp.python.org in the example strings, but 
> that service doesn't seem to be alive. I don't know if there's another 
> public FTP-TLS server you could rely on...?

Yeah, I know. I just copied from original FTP class docstring.
As of now I'm not aware of any public FTPS server we could use.
msg95006 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2009-11-06 23:53
> The user willing to use SSL instead of TLS will have to change 
> ssl_version class attribute with "FTP_TLS.ssl_version = 
> ssl.PROTOCOL_TLSv1" and then call auth().

Sorry but here I obviously meant "ssl.PROTOCOL_SSLv2/3"
msg95295 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-11-15 14:28
Giampaolo, do you plan to add something or is the patch ok to commit?
msg95301 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2009-11-15 16:55
If we want to add SSL support then the patch in attachment modifies the 
last one as I described in my previous comment.
I re-run the tests and they are ok so I guess you can go on with the 
commit.
msg95304 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-11-15 17:50
The tests don't work under py3k, for some reason I can't figure out.
There's the following error and then the tests hang:

test_acct (test.test_ftplib.TestTLS_FTPClassMixin) ... Exception in
thread Thread-31:
Traceback (most recent call last):
  File "/home/antoine/py3k/__svn__/Lib/threading.py", line 521, in
_bootstrap_inner
    self.run()
  File "/home/antoine/py3k/__svn__/Lib/test/test_ftplib.py", line 214,
in run
    asyncore.loop(timeout=0.1, count=1)
  File "/home/antoine/py3k/__svn__/Lib/asyncore.py", line 210, in loop
    poll_fun(timeout, map)
  File "/home/antoine/py3k/__svn__/Lib/asyncore.py", line 136, in poll
    r, w, e = select.select(r, w, e, timeout)
select.error: (9, 'Bad file descriptor')
msg95357 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2009-11-16 18:52
Can you attach the 3.x patch so that I can test it myself?
I tried to apply the current 2.x patch against the 3.x trunk but I get 
conflicts.
msg95369 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-11-17 09:36
Here is the current py3k patch I have, after resolving conflicts and
cleaning up the obvious problems.
After tracing a bit, it seems that ssl.wrap_socket() changes the socket
fileno under py3k, while it doesn't under trunk.
msg95370 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-11-17 09:43
Ok, I now have a working patch. The main fix was to change
SSLConnection.secure_connection() to:

        def secure_connection(self):
            socket = ssl.wrap_socket([ ##etc. ])
            self.del_channel()
            self.set_socket(socket)
            self._ssl_accepting = True

Can you take a look?
msg95396 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2009-11-17 20:01
Ok, I took a look and it seems ok to me but I still get some occasional 
failures on Windows from time to time.
Because of the threading nature of our server I suspect that moving 
del_channel() before ssl.wrap_socket() call, like this:

-            socket = ssl.wrap_socket([ ##etc. ])
-            self.del_channel()
-            self.set_socket(socket)

+            self.del_channel()
+            self.socket = ssl.wrap_socket(...)
+            self.set_socket(self.socket)

...makes more sense (ps: pay attention, it's "self.socket", not 
"socket").
After I did that I stopped seeing the occasional failures (I'm not 100% 
sure it's actually related, but...).

This is quite strange, anyway.
I suspect it has something to do with this:
http://entitycrisis.blogspot.com/2009/11/python-3-is-it-doomed.html
msg95398 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-11-17 20:31
> Ok, I took a look and it seems ok to me but I still get some occasional 
> failures on Windows from time to time.
> Because of the threading nature of our server I suspect that moving 
> del_channel() before ssl.wrap_socket() call, like this:

Ok, thanks!

> ...makes more sense (ps: pay attention, it's "self.socket", not 
> "socket").

set_socket() sets self.socket, so it should be the same.

I'm going to commit on py3k and watch the buildbots a bit.
msg95408 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-11-17 23:16
Buildbots are ok. Thank you!
msg96046 - (view) Author: Domen Kožar (iElectric) Date: 2009-12-07 11:01
Nice! Any chance of merging with 2.7? Python3.2 is waaay too far in
future for such useful change to be actually useful.
msg96047 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-12-07 11:06
It's already in 2.7.
msg102875 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-04-11 19:37
Thinking back about this, I wonder whether "FTPS" could be a better name to use instead of "FTP_TLS".
It's shorter, easier to remember, and also makes more sense since also SSL can be used, not only TLS.
msg102877 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-04-11 19:43
It doesn’t look like a constant, too.

httplib.Client, ftplib.Client, ftplib.SecureClient would be much more descriptive than httplib.HTTP and ftplib.FTP. Any interest about adding aliases?

Regards
msg102878 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-04-11 19:45
> Thinking back about this, I wonder whether "FTPS" could be a better name to use instead of "FTP_TLS".
> It's shorter, easier to remember, and also makes more sense since also SSL can be used, not only TLS.

What do you mean by "also SSL can be used"?
Wikipedia has an interesting article about the subject:
http://en.wikipedia.org/wiki/FTPS

Secured FTP with explicit negotiation (what we are doing) is sometimes
called "FTPES" (that's how it's named in FileZilla indeed).
"FTPS" is more often used to describe secured FTP with implicit
negotiation, i.e. the SSL session is established before the FTP protocol
even kicks in.

I think FTP_TLS is a fine name. Perhaps we can simply make the above
distinction clearer in the docs.
msg102941 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2010-04-12 12:43
On Sun, Apr 11, 2010 at 07:43:56PM +0000, Éric Araujo wrote:
> httplib.Client, ftplib.Client, ftplib.SecureClient would be much more descriptive than httplib.HTTP and ftplib.FTP. Any interest about adding aliases?

Aliases would be a bad idea. -1. It is fine the way the issue is
moving forth now, FTP_TLS.
History
Date User Action Args
2010-05-20 20:37:26giampaolo.rodolasetmessages: - msg83126
2010-04-12 12:43:58orsenthilsetnosy: + orsenthil
messages: + msg102941
2010-04-11 19:45:37pitrousetmessages: + msg102878
2010-04-11 19:43:54eric.araujosetnosy: + eric.araujo
messages: + msg102877
2010-04-11 19:37:23giampaolo.rodolasetmessages: + msg102875
2009-12-07 11:06:46pitrousetmessages: + msg96047
2009-12-07 11:01:10iElectricsetmessages: + msg96046
2009-11-17 23:16:48pitrousetstatus: open -> closed
resolution: accepted -> fixed
messages: + msg95408

stage: commit review -> resolved
2009-11-17 20:31:21pitrousetmessages: + msg95398
2009-11-17 20:01:20giampaolo.rodolasetmessages: + msg95396
2009-11-17 09:45:46pitrousetfiles: + ftptls-py3k-2.patch
2009-11-17 09:45:35pitrousetfiles: - ftptls-py3k-2.patch
2009-11-17 09:43:04pitrousetfiles: + ftptls-py3k-2.patch

messages: + msg95370
2009-11-17 09:36:47pitrousetfiles: + ftptls-py3k.patch

messages: + msg95369
2009-11-16 18:52:07giampaolo.rodolasetmessages: + msg95357
2009-11-15 17:50:42pitrousetassignee: pitrou ->
messages: + msg95304
versions: - Python 2.7
2009-11-15 16:55:28giampaolo.rodolasetfiles: + ftplib.patch

messages: + msg95301
2009-11-15 14:28:42pitrousetmessages: + msg95295
2009-11-06 23:53:27giampaolo.rodolasetmessages: + msg95006
2009-11-06 23:51:40giampaolo.rodolasetmessages: + msg95005
2009-11-06 20:10:10iElectricsetmessages: + msg94996
2009-11-04 21:53:54pitrousetmessages: + msg94902
2009-10-25 14:34:40iElectricsetmessages: + msg94452
2009-10-19 19:02:29giampaolo.rodolasetmessages: + msg94253
2009-10-19 18:54:25pitrousetmessages: + msg94252
2009-10-19 18:47:04pitrousetassignee: pitrou
messages: + msg94251
2009-10-19 18:38:37giampaolo.rodolasetmessages: + msg94249
2009-10-19 16:59:58pitrousetmessages: + msg94244
2009-10-19 15:33:23pitrousetresolution: accepted
messages: + msg94241
stage: patch review -> commit review
2009-10-19 12:55:18giampaolo.rodolasetfiles: + ftplib.patch

messages: + msg94234
2009-10-18 20:47:39pitrousetkeywords: - easy

messages: + msg94225
2009-10-17 20:50:40giampaolo.rodolasetfiles: + ftplib.patch
nosy: + josiahcarlson, josiah.carlson
messages: + msg94188

2009-10-16 18:48:02giampaolo.rodolasetfiles: + ftplib.patch
2009-10-16 18:47:47giampaolo.rodolasetfiles: - ftplib.patch
2009-10-16 18:43:18giampaolo.rodolasetfiles: + ftplib.patch

messages: + msg94142
2009-10-15 18:34:30pitrousetmessages: + msg94104
2009-10-15 18:27:36pitrousetmessages: + msg94103
2009-10-15 18:08:51giampaolo.rodolasetmessages: + msg94101
2009-10-15 17:56:36pitrousetstage: patch review
messages: + msg94099
versions: + Python 2.7, Python 3.2, - Python 2.6, Python 3.0
2009-10-15 17:53:19pitrousetfiles: - unnamed
2009-10-15 17:53:16pitrousetfiles: - unnamed
2009-10-15 17:53:13pitrousetfiles: - unnamed
2009-10-15 17:53:10pitrousetfiles: - unnamed
2009-10-15 17:53:06pitrousetfiles: - unnamed
2009-10-15 17:53:03pitrousetfiles: - unnamed
2009-10-15 17:53:00pitrousetfiles: - unnamed
2009-10-15 17:26:32giampaolo.rodolasetmessages: + msg94098
2009-10-15 17:06:28lgedgarsetnosy: + lgedgar
2009-09-12 20:12:54qwavelsetnosy: + qwavel
2009-03-04 16:50:41giampaolo.rodolasetfiles: - ftplib.patch
2009-03-04 16:50:28giampaolo.rodolasetmessages: + msg83128
2009-03-04 16:49:31giampaolo.rodolasetfiles: + ftplib.patch
messages: + msg83126
2009-02-27 21:47:57jeffosetmessages: + msg82876
2009-02-25 12:41:32giampaolo.rodolasetmessages: + msg82704
2009-02-25 06:49:57jeffosetfiles: + unnamed
messages: + msg82697
2009-02-24 07:56:29jeffosetfiles: + unnamed
messages: + msg82662
2009-02-23 12:57:08giampaolo.rodolasetfiles: + ftplib.patch
keywords: + patch
messages: + msg82625
2009-02-22 21:40:32jeffosetnosy: + jeffo
messages: + msg82612
2008-11-13 01:30:12twhitemasetnosy: + twhitema
2008-08-12 17:13:51janssensetmessages: + msg71061
2008-08-12 16:05:04janssensetmessages: + msg71058
2008-07-11 15:43:21lszyba1setnosy: + lszyba1
messages: + msg69553
2008-06-30 16:55:34giampaolo.rodolasetmessages: + msg69011
2008-06-30 16:45:25giampaolo.rodolasetmessages: + msg69010
2008-06-30 16:09:54janssensetfiles: + unnamed
messages: + msg69008
2008-06-30 11:12:13giampaolo.rodolasetmessages: + msg69002
2008-06-30 01:03:18janssensetmessages: + msg68978
2008-05-12 18:59:24giampaolo.rodolasetmessages: + msg66752
2008-03-21 23:28:36janssensetfiles: + unnamed
messages: + msg64296
2008-03-21 12:43:06robertesetmessages: + msg64239
2008-03-21 01:59:29pitrousetnosy: + pitrou
messages: + msg64229
2008-03-20 03:56:46janssensetfiles: + unnamed
messages: + msg64147
2008-03-20 03:56:25janssensetfiles: + unnamed
messages: + msg64146
2008-03-19 20:46:52giampaolo.rodolasetmessages: + msg64097
2008-03-19 20:19:51janssensetfiles: + unnamed
messages: + msg64093
2008-03-19 19:22:46giampaolo.rodolasetmessages: + msg64088
2008-03-17 18:23:27iElectricsetnosy: + iElectric
messages: + msg63707
2008-03-11 16:12:14robertesetnosy: + roberte
messages: + msg63462
2008-02-22 20:54:04janssensetmessages: + msg62699
2008-02-21 00:19:09giampaolo.rodolasetfiles: + ftplib.diff
nosy: + giampaolo.rodola, janssen
messages: + msg62608
2008-02-09 03:57:17christian.heimessetpriority: normal
keywords: + easy
2008-02-09 02:16:05gregory.p.smithcreate