Fix GH-9348: FTP & SSL session reuse #12851
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The issue referenced here doesn't contain a reproducer, but I recently received an email of a user with the exact same problem. I was able to recreate the scenario locally using vsftpd and setting `require_ssl_reuse=YES` in the vsftpd configuration. It turns out that our session resumption code is broken. It only works a single time: the first time a data connection opens. Subsequent data connections fail to reuse the session. This is because on every data connection a new session is negotiated, but the current code always tries to reuse the (stale) session of the control connection. To fix this, we use SSL_CTX_sess_set_new_cb() to setup a callback that gets called every time a new session is negotiated. We take a strong reference using SSL_get1_session() and store it in the ftpbuf_t struct. Every time we open a data connection we'll take that session. This works because every control connection has at most a single associated data connection. Also disable internal session caching storage to not fill the cache up with useless sessions. There is no phpt for this because PHP does not support enforcing SSL session reuse. It is however testable manually by setting up vsftpd and setting the `require_ssl_reuse=YES` function from before.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The issue referenced here doesn't contain a reproducer, but I recently received an email of a user with the exact same problem. I was able to recreate the scenario locally using vsftpd and setting
require_ssl_reuse=YESin the vsftpd configuration.It turns out that our session resumption code is broken. It only works a single time: the first time a data connection opens. Subsequent data connections fail to reuse the session. This is because on every data connection a new session is negotiated, but the current code always tries to reuse the (stale) session of the control connection.
To fix this, we use SSL_CTX_sess_set_new_cb() to setup a callback that gets called every time a new session is negotiated. We take a strong reference using SSL_get1_session() and store it in the ftpbuf_t struct. Every time we open a data connection we'll take that session. This works because every control connection has at most a single associated data connection.
Also disable internal session caching storage to not fill the cache up with useless sessions.
There is no phpt for this because PHP does not support enforcing SSL session reuse.
It is however testable manually by setting up vsftpd and setting the
require_ssl_reuse=YESfunction from before.