ftplib: Add client-side SSL session resumption

[YeWang]

BPO	19500
Nosy	@pitrou, @giampaolo, @tiran, @alex, @dstufft, @zhangyangyu, @rreilink, @larsschellhas
Files	ftplib-FTPS-bug.txt: Python source and command prompt input/output reuse_session.diff: proof-of-concept session reuse implement_ssl_session_reuse.patch: Patch which implements SSL session reuse and fixes corresponding FTP_TLS issue implement_ssl_session_reuse_3.6.patch: Patch which implements SSL session reuse and fixes corresponding FTP_TLS issue targeted at 3.6 SSLSession-support.patch sesstime.py SSLSession-support-2.patch SSL-client-side-SSL-session-resumption.patch ftplib_session.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 = None
created_at = <Date 2013-11-05.04:02:10.597>
labels = ['type-feature', 'library', '3.10']
title = 'ftplib: Add client-side SSL session resumption'
updated_at = <Date 2020-09-23.19:34:39.319>
user = 'https://bugs.python.org/YeWang'

bugs.python.org fields:

activity = <Date 2020-09-23.19:34:39.319>
actor = 'larsschellhas'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2013-11-05.04:02:10.597>
creator = 'Ye.Wang'
dependencies = []
files = ['32505', '40666', '40708', '40716', '44362', '44368', '44376', '44480', '46409']
hgrepos = []
issue_num = 19500
keywords = ['patch']
message_count = 27.0
messages = ['202193', '216674', '216679', '216680', '225078', '252205', '252467', '252468', '252527', '252530', '252543', '252635', '274367', '274391', '274394', '274396', '274417', '274893', '275161', '275703', '275704', '286190', '312885', '370001', '377412', '377413', '377419']
nosy_count = 13.0
nosy_names = ['janssen', 'pitrou', 'giampaolo.rodola', 'christian.heimes', 'alex', 'python-dev', 'dstufft', 'Ye.Wang', 'Mark.Ribau', 'xiang.zhang', 'robr', 'Florian Wickert', 'larsschellhas']
pr_nums = []
priority = 'low'
resolution = None
stage = 'needs patch'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue19500'
versions = ['Python 3.10']

[YeWang]

According to RFC4217 (Securing FTP with TLS, aka the FTPS spec),

http://tools.ietf.org/html/rfc4217.html#section-10.2

" It is reasonable for the server to insist that the data connection
uses a TLS cached session. This might be a cache of a previous data
connection or of a cleared control connection. If this is the reason
for the refusal to allow the data transfer, then the '522' reply
should indicate this.

Note: This has an important impact on client design, but allows
servers to minimize the cycles used during TLS negotiation by
refusing to perform a full negotiation with a previously
authenticated client."

It appears that vsftpd server implemented exactly that by enforcing the "SSL session reuse between the control and data connection".

http://scarybeastsecurity.blogspot.com/2009/02/vsftpd-210-released.html

Looking at the source of Python core library ftplib.py, there isn't any regard to the idea of SSL session reuse between data connection vs. control connection (correct me if I am wrong here. I've tried FTP_TLS.transfercmd(cmd[, rest])¶, didn't work).

This issue is well documented on other FTP clients that supports FTPS, I.E. WinSCP: http://winscp.net/tracker/show_bug.cgi?id=668

See test log file attached. A vsftpd server with "require_ssl_reuse" set to true in vsftpd.conf would do the trick and can be reproduced.

[giampaolo]

Interesting, I wasn't aware of this FTP(S) feature.
Unfortunately RFC-4217 really doesn't say much about how this should be done but it definitively looks like something worth having.
AFAIU this looks like something which should be implemented by servers though, not clients.

[pitrou]

Yuck. Is there a public FTP server available somewhere with this "feature"?

[pitrou]

The RFC is unhelpfully lousy. It's not enough to process a "522" error, since that can be triggered for different reasons. You also somehow have to interpret the error text to detect that session reuse is indeed mandated by the server.

Regardless, to progress with this we would first need to implement client-side SSL session reuse, which necessitates a bunch of additional APIs (since which session is to be reused is a decision made by user code), and a new opaque type to carry SSL_SESSION objects...

(see issue bpo-8106)

[MarkRibau]

Adding Python v2.7 as also exhibiting this behavior.

Some people over on Stack Overflow have done some things to work around the issue via subclassing, but I'm not sure their solutions are "correct", so much as have useful side effects. (For example, when only the server has a key/cert and the client does not, how is that handled for reuse?)

http://stackoverflow.com/questions/12164470/python-ftp-tls-connection-issue

[AlexWarhawk]

I encountered this problem recently and could not find a fix, so i tried fixing it myself.

Note that the patch attached is my first contribution to cpython as well as the first time I used the C extension mechanism. Therefore I do not consider the patch polished enough to be just merged upstream.

Maybe it helps in solving this issue.

The attached patch is based on:
changeset: 79113:ec373d762213
branch: 2.7

[AlexWarhawk]

Based on the proof-of-concept patch I submitted a few days ago I have built a more sophisticated patch. Please review it and let me know about necessary changes.

[giampaolo]

This is supposed to be a new feature hence the patch should be targeted against Python 3.6, definitively not 2.7.

[AlexWarhawk]

I have re-targeted the patch for 3.6. It is not a 1 to 1 port of the prior one, but quite similar.

[tiran]

Thanks for your patch. There might be a simpler way. By default a SSLContext only caches server sessions. You can enable client session caching with:

  SSL_CTX_set_session_cache_mode(ctx, SSL_SESS_CACHE_CLIENT)

This may be sufficient for FTP over TLS since both sockets are created from the same context.

The new patch has a flaw. With the new SSLSession object a user could attempt to reuse a SSLSession with a different SSLContext. That's going to break OpenSSL.

From SSL_set_session(3)

NOTES
SSL_SESSION objects keep internal link information about the session cache list, when being inserted into one SSL_CTX object's session cache. One SSL_SESSION object, regardless of its reference count, must therefore only be used with one SSL_CTX object (and the SSL objects created from this SSL_CTX object).

[AlexWarhawk]

Thanks for the heads up Christian I'll try enabling client session caching. If this does not work I'll try to adapt the patch to only allow session reusing within the same context.

[AlexWarhawk]

Even after enabling client cache one still has to call SSL_set_session. See documentation of SSL_CTX_set_session_cache_mode point SSL_SESS_CACHE_CLIENT.

I started thinking about not exposing a SSL_SESSION object to the user but rather extending wrap_socket to take an already established socket as argument and use that socket's session object. This way I can ensure that both sockets share the same SSL context

I am not really convinced by this idea myself, what do you think about this? Any better ideas?

[tiran]

Here is my take on the SSLSession feature. The patch provides a SSLSession type, SSLSocket.session getter/setter and SSLSocket.session_reused getter. The setter makes sure that the session can only set for client sockets from the same SSLContext and before handshake. Tests and documentation need some improvements.

https://github.com/tiran/cpython/commits/feature/openssl_session