From 1018e47ed7cfeb18f0d5b4733dcd39725a0c36b3 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Sat, 2 Dec 2023 16:20:15 +0100
Subject: [PATCH] Fix GH-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.
---
 ext/ftp/ftp.c | 27 ++++++++++++++++++++++++---
 ext/ftp/ftp.h |  1 +
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/ext/ftp/ftp.c b/ext/ftp/ftp.c
index 85cfd0a90cdf..54326889042f 100644
--- a/ext/ftp/ftp.c
+++ b/ext/ftp/ftp.c
@@ -167,6 +167,9 @@ ftp_close(ftpbuf_t *ftp)
 	if (ftp == NULL) {
 		return NULL;
 	}
+	if (ftp->last_ssl_session) {
+		SSL_SESSION_free(ftp->last_ssl_session);
+	}
 	if (ftp->data) {
 		data_close(ftp, ftp->data);
 	}
@@ -229,6 +232,20 @@ ftp_quit(ftpbuf_t *ftp)
 }
 /* }}} */
 
+static int ftp_ssl_new_session_cb(SSL *ssl, SSL_SESSION *sess)
+{
+	ftpbuf_t *ftp = SSL_get_app_data(ssl);
+
+	/* Technically there can be multiple sessions per connection, but we only care about the most recent one. */
+	if (ftp->last_ssl_session) {
+		SSL_SESSION_free(ftp->last_ssl_session);
+	}
+	ftp->last_ssl_session = SSL_get1_session(ssl);
+
+	/* Return 0 as we are not using OpenSSL's session cache. */
+	return 0;
+}
+
 /* {{{ ftp_login */
 int
 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
 #endif
 		SSL_CTX_set_options(ctx, ssl_ctx_options);
 
-		/* allow SSL to re-use sessions */
-		SSL_CTX_set_session_cache_mode(ctx, SSL_SESS_CACHE_BOTH);
+		/* Allow SSL to re-use sessions.
+		 * We're relying on our own session storage as only at most one session will ever be active per FTP connection. */
+		SSL_CTX_set_session_cache_mode(ctx, SSL_SESS_CACHE_BOTH | SSL_SESS_CACHE_NO_INTERNAL);
+		SSL_CTX_sess_set_new_cb(ctx, ftp_ssl_new_session_cb);
 
 		ftp->ssl_handle = SSL_new(ctx);
+		SSL_set_app_data(ftp->ssl_handle, ftp); /* Needed for ftp_ssl_new_session_cb */
 		SSL_CTX_free(ctx);
 
 		if (ftp->ssl_handle == NULL) {
@@ -1789,7 +1809,7 @@ data_accept(databuf_t *data, ftpbuf_t *ftp)
 		}
 
 		/* get the session from the control connection so we can re-use it */
-		session = SSL_get_session(ftp->ssl_handle);
+		session = ftp->last_ssl_session;
 		if (session == NULL) {
 			php_error_docref(NULL, E_WARNING, "data_accept: failed to retrieve the existing SSL session");
 			SSL_free(data->ssl_handle);
@@ -1797,6 +1817,7 @@ data_accept(databuf_t *data, ftpbuf_t *ftp)
 		}
 
 		/* and set it on the data connection */
+		SSL_set_app_data(data->ssl_handle, ftp); /* Needed for ftp_ssl_new_session_cb */
 		res = SSL_set_session(data->ssl_handle, session);
 		if (res == 0) {
 			php_error_docref(NULL, E_WARNING, "data_accept: failed to set the existing SSL session");
diff --git a/ext/ftp/ftp.h b/ext/ftp/ftp.h
index 9b50ea71ff5d..5e96351ebdae 100644
--- a/ext/ftp/ftp.h
+++ b/ext/ftp/ftp.h
@@ -82,6 +82,7 @@ typedef struct ftpbuf
 	int				old_ssl;	/* old mode = forced data encryption */
 	SSL				*ssl_handle;      /* handle for control connection */
 	int				ssl_active;		  /* ssl active on control conn */
+	SSL_SESSION     *last_ssl_session; /* last negotiated session */
 #endif
 
 } ftpbuf_t;