Skip to content

Commit ac8a58f

Browse files
committed
Fix phpGH-9348: FTP & SSL session reuse
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. Closes phpGH-12851.
1 parent 0a39890 commit ac8a58f

File tree

3 files changed

+28
-3
lines changed

3 files changed

+28
-3
lines changed

NEWS

+3
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ PHP NEWS
2424
- FPM:
2525
. Fixed bug GH-12705 (Segmentation fault in fpm_status_export_to_zval).
2626
(Patrick Prasse)
27+
28+
- FTP:
29+
. Fixed bug GH-9348 (FTP & SSL session reuse). (nielsdos)
2730

2831
- Intl:
2932
. Fixed bug GH-12635 (Test bug69398.phpt fails with ICU 74.1). (nielsdos)

ext/ftp/ftp.c

+24-3
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,9 @@ ftp_close(ftpbuf_t *ftp)
167167
if (ftp == NULL) {
168168
return NULL;
169169
}
170+
if (ftp->last_ssl_session) {
171+
SSL_SESSION_free(ftp->last_ssl_session);
172+
}
170173
if (ftp->data) {
171174
data_close(ftp, ftp->data);
172175
}
@@ -229,6 +232,20 @@ ftp_quit(ftpbuf_t *ftp)
229232
}
230233
/* }}} */
231234

235+
static int ftp_ssl_new_session_cb(SSL *ssl, SSL_SESSION *sess)
236+
{
237+
ftpbuf_t *ftp = SSL_get_app_data(ssl);
238+
239+
/* Technically there can be multiple sessions per connection, but we only care about the most recent one. */
240+
if (ftp->last_ssl_session) {
241+
SSL_SESSION_free(ftp->last_ssl_session);
242+
}
243+
ftp->last_ssl_session = SSL_get1_session(ssl);
244+
245+
/* Return 0 as we are not using OpenSSL's session cache. */
246+
return 0;
247+
}
248+
232249
/* {{{ ftp_login */
233250
int
234251
ftp_login(ftpbuf_t *ftp, const char *user, const size_t user_len, const char *pass, const size_t pass_len)
@@ -279,10 +296,13 @@ ftp_login(ftpbuf_t *ftp, const char *user, const size_t user_len, const char *pa
279296
#endif
280297
SSL_CTX_set_options(ctx, ssl_ctx_options);
281298

282-
/* allow SSL to re-use sessions */
283-
SSL_CTX_set_session_cache_mode(ctx, SSL_SESS_CACHE_BOTH);
299+
/* Allow SSL to re-use sessions.
300+
* We're relying on our own session storage as only at most one session will ever be active per FTP connection. */
301+
SSL_CTX_set_session_cache_mode(ctx, SSL_SESS_CACHE_BOTH | SSL_SESS_CACHE_NO_INTERNAL);
302+
SSL_CTX_sess_set_new_cb(ctx, ftp_ssl_new_session_cb);
284303

285304
ftp->ssl_handle = SSL_new(ctx);
305+
SSL_set_app_data(ftp->ssl_handle, ftp); /* Needed for ftp_ssl_new_session_cb */
286306
SSL_CTX_free(ctx);
287307

288308
if (ftp->ssl_handle == NULL) {
@@ -1789,14 +1809,15 @@ data_accept(databuf_t *data, ftpbuf_t *ftp)
17891809
}
17901810

17911811
/* get the session from the control connection so we can re-use it */
1792-
session = SSL_get_session(ftp->ssl_handle);
1812+
session = ftp->last_ssl_session;
17931813
if (session == NULL) {
17941814
php_error_docref(NULL, E_WARNING, "data_accept: failed to retrieve the existing SSL session");
17951815
SSL_free(data->ssl_handle);
17961816
return 0;
17971817
}
17981818

17991819
/* and set it on the data connection */
1820+
SSL_set_app_data(data->ssl_handle, ftp); /* Needed for ftp_ssl_new_session_cb */
18001821
res = SSL_set_session(data->ssl_handle, session);
18011822
if (res == 0) {
18021823
php_error_docref(NULL, E_WARNING, "data_accept: failed to set the existing SSL session");

ext/ftp/ftp.h

+1
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ typedef struct ftpbuf
8282
int old_ssl; /* old mode = forced data encryption */
8383
SSL *ssl_handle; /* handle for control connection */
8484
int ssl_active; /* ssl active on control conn */
85+
SSL_SESSION *last_ssl_session; /* last negotiated session */
8586
#endif
8687

8788
} ftpbuf_t;

0 commit comments

Comments
 (0)