Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add ftp-tls support to ftplib - RFC 4217 #46330

Closed
gpshead opened this issue Feb 9, 2008 · 64 comments
Closed

add ftp-tls support to ftplib - RFC 4217 #46330

gpshead opened this issue Feb 9, 2008 · 64 comments
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@gpshead
Copy link
Member

gpshead commented Feb 9, 2008

BPO 2054
Nosy @gpshead, @josiahcarlson, @orsenthil, @pitrou, @giampaolo, @merwok
Files
  • ftplib.diff: adds FTP over TLS support (RFC-4217)
  • ftplib.patch: New patch which shutdown the SSL layer before closing FTP data channel
  • ftplib.patch: includes changes described in message 94103
  • ftplib.patch: includes tests and doc
  • ftplib.patch: implements changes described in msg 94225
  • ftplib.patch: adds SSL support as described in comment 95005
  • ftptls-py3k.patch
  • ftptls-py3k-2.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2009-11-17.23:16:48.978>
    created_at = <Date 2008-02-09.02:16:05.480>
    labels = ['type-feature', 'library']
    title = 'add ftp-tls support to ftplib - RFC 4217'
    updated_at = <Date 2010-05-20.20:37:26.173>
    user = 'https://github.com/gpshead'

    bugs.python.org fields:

    activity = <Date 2010-05-20.20:37:26.173>
    actor = 'giampaolo.rodola'
    assignee = 'none'
    closed = True
    closed_date = <Date 2009-11-17.23:16:48.978>
    closer = 'pitrou'
    components = ['Library (Lib)']
    creation = <Date 2008-02-09.02:16:05.480>
    creator = 'gregory.p.smith'
    dependencies = []
    files = ['9471', '13242', '15147', '15159', '15163', '15343', '15349', '15351']
    hgrepos = []
    issue_num = 2054
    keywords = ['patch']
    message_count = 64.0
    messages = ['62217', '62608', '62699', '63462', '63707', '64088', '64093', '64097', '64146', '64147', '64229', '64239', '64296', '66752', '68978', '69002', '69008', '69010', '69011', '69553', '71058', '71061', '82612', '82625', '82662', '82697', '82704', '82876', '83128', '94098', '94099', '94101', '94103', '94104', '94142', '94188', '94225', '94234', '94241', '94244', '94249', '94251', '94252', '94253', '94452', '94902', '94996', '95005', '95006', '95295', '95301', '95304', '95357', '95369', '95370', '95396', '95398', '95408', '96046', '96047', '102875', '102877', '102878', '102941']
    nosy_count = 15.0
    nosy_names = ['gregory.p.smith', 'josiahcarlson', 'janssen', 'orsenthil', 'pitrou', 'giampaolo.rodola', 'josiah.carlson', 'roberte', 'iElectric', 'eric.araujo', 'lszyba1', 'twhitema', 'jeffo', 'qwavel', 'lgedgar']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue2054'
    versions = ['Python 3.2']

    @gpshead
    Copy link
    Member Author

    gpshead commented Feb 9, 2008

    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.

    @gpshead gpshead added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Feb 9, 2008
    @tiran tiran added the easy label Feb 9, 2008
    @giampaolo
    Copy link
    Contributor

    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.

    @janssen
    Copy link
    Mannequin

    janssen mannequin commented Feb 22, 2008

    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.

    @roberte
    Copy link
    Mannequin

    roberte mannequin commented Mar 11, 2008

    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.

    @iElectric
    Copy link
    Mannequin

    iElectric mannequin commented Mar 17, 2008

    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.

    @giampaolo
    Copy link
    Contributor

    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.

    @janssen
    Copy link
    Mannequin

    janssen mannequin commented Mar 19, 2008

    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?

    @giampaolo
    Copy link
    Contributor

    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?

    @janssen
    Copy link
    Mannequin

    janssen mannequin commented Mar 20, 2008

    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\>


    @janssen
    Copy link
    Mannequin

    janssen mannequin commented Mar 20, 2008

    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\>


    @pitrou
    Copy link
    Member

    pitrou commented Mar 21, 2008

    FWIW, m2crypto already provides an FTP-TLS facility with an
    ftplib-compatible API. See http://chandlerproject.org/Projects/MeTooCrypto

    @roberte
    Copy link
    Mannequin

    roberte mannequin commented Mar 21, 2008

    > 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).

    @janssen
    Copy link
    Mannequin

    janssen mannequin commented Mar 21, 2008

    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.

    @giampaolo
    Copy link
    Contributor

    Bill, are there news about the fix to httplib?
    I'd like to see this feature included in 2.6 if possible.

    @janssen
    Copy link
    Mannequin

    janssen mannequin commented Jun 30, 2008

    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.

    @giampaolo
    Copy link
    Contributor

    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.

    @janssen
    Copy link
    Mannequin

    janssen mannequin commented Jun 30, 2008

    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\>


    @giampaolo
    Copy link
    Contributor

    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.

    @giampaolo
    Copy link
    Contributor

    Could what I've just said be an idea?

    @lszyba1
    Copy link
    Mannequin

    lszyba1 mannequin commented Jul 11, 2008

    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

    @janssen
    Copy link
    Mannequin

    janssen mannequin commented Aug 12, 2008

    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.

    @janssen
    Copy link
    Mannequin

    janssen mannequin commented Aug 12, 2008

    OK, I think I've done the minimal fix necessary to the SSL module to
    allow this work to proceed.

    @jeffo
    Copy link
    Mannequin

    jeffo mannequin commented Feb 22, 2009

    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.

    @giampaolo
    Copy link
    Contributor

    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

    @jeffo
    Copy link
    Mannequin

    jeffo mannequin commented Feb 24, 2009

    Thank you Giampaolo, it works just as I was hoping, =] I tested it on glftpd
    using python 2.6.1.

    @jeffo
    Copy link
    Mannequin

    jeffo mannequin commented Feb 25, 2009

    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\>


    @giampaolo
    Copy link
    Contributor

    Actually I have encountered a possible bug. the close()
    method doesn't seem to actually close the connection...

    Why? What happens exactly?

    @pitrou
    Copy link
    Member

    pitrou commented Oct 19, 2009

    The patch is ok to me. Perhaps Bill wants to take a look, otherwise I
    think you can commit.

    @pitrou
    Copy link
    Member

    pitrou commented Oct 19, 2009

    A last problem:

    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: attribute name must be string, not 'classobj'

    @giampaolo
    Copy link
    Contributor

    > 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.

    @pitrou
    Copy link
    Member

    pitrou commented Oct 19, 2009

    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.

    @pitrou pitrou self-assigned this Oct 19, 2009
    @pitrou
    Copy link
    Member

    pitrou commented Oct 19, 2009

    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.

    @giampaolo
    Copy link
    Contributor

    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.

    @iElectric
    Copy link
    Mannequin

    iElectric mannequin commented Oct 25, 2009

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

    @pitrou
    Copy link
    Member

    pitrou commented Nov 4, 2009

    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...?

    @iElectric
    Copy link
    Mannequin

    iElectric mannequin commented Nov 6, 2009

    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

    @giampaolo
    Copy link
    Contributor

    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.

    @giampaolo
    Copy link
    Contributor

    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"

    @pitrou
    Copy link
    Member

    pitrou commented Nov 15, 2009

    Giampaolo, do you plan to add something or is the patch ok to commit?

    @giampaolo
    Copy link
    Contributor

    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.

    @pitrou
    Copy link
    Member

    pitrou commented Nov 15, 2009

    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')

    @pitrou pitrou removed their assignment Nov 15, 2009
    @giampaolo
    Copy link
    Contributor

    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.

    @pitrou
    Copy link
    Member

    pitrou commented Nov 17, 2009

    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.

    @pitrou
    Copy link
    Member

    pitrou commented Nov 17, 2009

    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?

    @giampaolo
    Copy link
    Contributor

    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

    @pitrou
    Copy link
    Member

    pitrou commented Nov 17, 2009

    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.

    @pitrou
    Copy link
    Member

    pitrou commented Nov 17, 2009

    Buildbots are ok. Thank you!

    @pitrou pitrou closed this as completed Nov 17, 2009
    @iElectric
    Copy link
    Mannequin

    iElectric mannequin commented Dec 7, 2009

    Nice! Any chance of merging with 2.7? Python3.2 is waaay too far in
    future for such useful change to be actually useful.

    @pitrou
    Copy link
    Member

    pitrou commented Dec 7, 2009

    It's already in 2.7.

    @giampaolo
    Copy link
    Contributor

    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.

    @merwok
    Copy link
    Member

    merwok commented Apr 11, 2010

    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

    @pitrou
    Copy link
    Member

    pitrou commented Apr 11, 2010

    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.

    @orsenthil
    Copy link
    Member

    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.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants