io.c: Close outer read file descriptor at cleanup in nested transformations.

In the case of a nested transformation, like compression on top of TLS
encryption (a common paradigm in IMAP), when an inner transformation
(e.g. compression) cleans up, we need to clean up its outer read file
descriptor, i.e. close its read file descriptor towards the network.
This is because this was a pipe file descriptor created by the next
outer transformation (e.g. TLS). If the transformation is not nested
(e.g. just compression), this isn't needed since that would be the
node's actual file descriptor, and node.c will clean it up. But if the
transformation is nested, then we need to clean it up.

We now close this file descriptor if the transformation was nested
inside another one. A test (test_imap_compress_tls) has been added
to test this behavior. The actual I/O doesn't work properly, but it
does trigger a case (first observed in the while) that previously
caused a file descriptor leak.

Author: InterLinked1

bbs/io.c

@@ -367,6 +367,26 @@ static void *io_thread(void *varg)
 	 * We need to do this before we exit, since this thread isn't joined immediately. */
 	bbs_debug(4, "%s I/O thread exiting\n", t->name);
 	t->funcs->io_finalize(tran->data);
+
+	if (tran->outer_rfd != tran->outer_wfd) {
+		/* There is an added complication for layered I/O connections, where we also need to close the outer read file descriptor, since we don't need to read from the network side anymore.
+		 *
+		 * For example, if we are running compression on top of TLS, then we leak a file descriptor if we don't close z->orig_fd in io_compress (this is what test_imap_compress_tls tests)
+		 *
+		 * However, if we always do that, that will mess up sessions that only use compression without TLS (test_imap_compress tests that),
+		 * since in that case, z->orig_rfd is the node's actual file descriptor, which gets closed in node_cleanup.
+		 *
+		 * We need to handle both cases, because if there are no I/O transformations, node.c needs to close its own file descriptor,
+		 * but if we are running on top of another I/O module, then we are responsible for closing it.
+		 *
+		 * We can detect that if, when we set up a transformation, rfd != wfd. This indicates this is a nested transformation,
+		 * since if we were the first transformation on top of the node socket, rfd == wfd.
+		 * Just below, in bbs_io_transform_setup, before calling t->funcs->setup, we save the original rfd and set tran->outer_rfd to it,
+		 * so we have access to it via that variable now and can close it. */
+		bbs_debug(5, "Closing file descriptor %d since this is a nested transformation\n", tran->outer_rfd);
+		close(tran->outer_rfd); /* Nothing more that we are going to read from a lower I/O layer since we already closed write to the application side */
+	}
+
 	return NULL;
 }
 

tests/compress.c

@@ -78,9 +78,14 @@ void z_client_free(struct z_data *z)
 	free(z);
 }
 
+ssize_t zlib_write(struct z_data *z, int line, const char *buf, size_t len)
+{
+	return zlib_write_cb(z, line, buf, len, NULL, NULL);
+}
+
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wcast-qual"
-ssize_t zlib_write(struct z_data *z, int line, const char *buf, size_t len)
+ssize_t zlib_write_cb(struct z_data *z, int line, const char *buf, size_t len, ssize_t (*write_cb)(void *cbdata, const char *buf, size_t len), void *cbdata)
 {
 	char output[BUFSIZ];
 
@@ -102,7 +107,11 @@ ssize_t zlib_write(struct z_data *z, int line, const char *buf, size_t len)
 		}
 		comp_len = sizeof(output) - z->compressor->avail_out;
 		bbs_debug(10, "Deflated to %lu bytes at line %d\n", comp_len, line);
-		wres = write(z->fd, output, comp_len);
+		if (write_cb) {
+			wres = write_cb(cbdata, buf, len);
+		} else {
+			wres = write(z->fd, output, comp_len);
+		}
 		if (wres <= 0) {
 			bbs_error("write returned %ld at line %d\n", wres, line);
 			return -1;
@@ -114,14 +123,23 @@ ssize_t zlib_write(struct z_data *z, int line, const char *buf, size_t len)
 #pragma GCC diagnostic pop
 
 ssize_t zlib_read(struct z_data *z, int line, char *buf, size_t len)
+{
+	return zlib_read_cb(z, line, buf, len, NULL, NULL);
+}
+
+ssize_t zlib_read_cb(struct z_data *z, int line, char *buf, size_t len, ssize_t (*read_cb)(void *cbdata, char *buf, size_t len), void *cbdata)
 {
 	char input[BUFSIZ / 10]; /* Hopefully the compression doesn't reduce the size by more than 90%... */
 	char output[BUFSIZ];
 	int zres;
 	ssize_t rres;
 	size_t bytes = 0;
 
-	rres = read(z->fd, input, sizeof(input));
+	if (read_cb) {
+		rres = read_cb(cbdata, input, sizeof(input));
+	} else {
+		rres = read(z->fd, input, sizeof(input));
+	}
 	if (rres <= 0) {
 		bbs_error("read returned %ld at line %d\n", rres, line);
 		return -1;
@@ -204,7 +222,18 @@ int test_z_client_expect_eventually(struct z_data *z, int ms, const char *restri
 	return test_z_client_expect_eventually_buf(z, ms, s, line, buf, sizeof(buf));
 }
 
+int test_z_client_expect_eventually_readcb(struct z_data *z, int ms, const char *restrict s, int line, ssize_t (*read_cb)(void *cbdata, char *buf, size_t len), void *cbdata)
+{
+	char buf[4096];
+	return test_z_client_expect_eventually_buf_readcb(z, ms, s, line, buf, sizeof(buf), read_cb, cbdata);
+}
+
 int test_z_client_expect_eventually_buf(struct z_data *z, int ms, const char *restrict s, int line, char *restrict buf, size_t len)
+{
+	return test_z_client_expect_eventually_buf_readcb(z, ms, s, line, buf, len, NULL, NULL);
+}
+
+int test_z_client_expect_eventually_buf_readcb(struct z_data *z, int ms, const char *restrict s, int line, char *restrict buf, size_t len, ssize_t (*read_cb)(void *cbdata, char *buf, size_t len), void *cbdata)
 {
 	struct pollfd pfd;
 
@@ -223,7 +252,7 @@ int test_z_client_expect_eventually_buf(struct z_data *z, int ms, const char *re
 		}
 		if (res > 0 && pfd.revents) {
 			ssize_t bytes;
-			bytes = zlib_read(z, line, buf, len - 1);
+			bytes = zlib_read_cb(z, line, buf, len - 1, read_cb, cbdata);
 			if (bytes <= 0) {
 				return -1;
 			}

tests/compress.h

@@ -22,16 +22,24 @@ void z_client_free(struct z_data *z);
 #define ZLIB_CLIENT_SHUTDOWN(z) if (z) { z_client_free(z); z = NULL; }
 
 ssize_t zlib_write(struct z_data *z, int line, const char *buf, size_t len);
+ssize_t zlib_write_cb(struct z_data *z, int line, const char *buf, size_t len, ssize_t (*write_cb)(void *cbdata, const char *buf, size_t len), void *cbdata);
 
 ssize_t zlib_read(struct z_data *z, int line, char *buf, size_t len);
+ssize_t zlib_read_cb(struct z_data *z, int line, char *buf, size_t len, ssize_t (*read_cb)(void *cbdata, char *buf, size_t len), void *cbdata);
 
 int test_z_client_expect(struct z_data *z, int ms, const char *s, int line);
 int test_z_client_expect_buf(struct z_data *z, int ms, const char *s, int line, char *buf, size_t len);
+
 int test_z_client_expect_eventually(struct z_data *z, int ms, const char *s, int line);
+int test_z_client_expect_eventually_readcb(struct z_data *z, int ms, const char *restrict s, int line, ssize_t (*read_cb)(void *cbdata, char *buf, size_t len), void *cbdata);
+
 int test_z_client_expect_eventually_buf(struct z_data *z, int ms, const char *s, int line, char *buf, size_t len);
+int test_z_client_expect_eventually_buf_readcb(struct z_data *z, int ms, const char *restrict s, int line, char *restrict buf, size_t len, ssize_t (*read_cb)(void *cbdata, char *buf, size_t len), void *cbdata);
 
 #define ZLIB_CLIENT_EXPECT(z, s) if (test_z_client_expect(z, SEC_MS(5), s, __LINE__)) { goto cleanup; }
 #define ZLIB_CLIENT_EXPECT_BUF(z, s, buf) if (test_z_client_expect_buf(z, SEC_MS(5), s, __LINE__, buf, sizeof(buf))) { goto cleanup; }
 #define ZLIB_CLIENT_EXPECT_EVENTUALLY(z, s) if (test_z_client_expect_eventually(z, SEC_MS(5), s, __LINE__)) { goto cleanup; }
+#define ZLIB_CLIENT_EXPECT_EVENTUALLY_READCB(cb, cbdata, z, s) if (test_z_client_expect_eventually_readcb(z, SEC_MS(5), s, __LINE__, cb, cbdata)) { goto cleanup; }
 
 #define ZLIB_SWRITE(z, s) if (zlib_write(z, __LINE__, s, STRLEN(s)) < 0) { goto cleanup; }
+#define ZLIB_SWRITE_CB(cb, cbdata, z, s) if (zlib_write_cb(z, __LINE__, s, STRLEN(s), cb, cbdata) < 0) { goto cleanup; }

tests/test_imap_compress_tls.c

@@ -0,0 +1,103 @@
+/*
+ * LBBS -- The Lightweight Bulletin Board System
+ *
+ * Copyright (C) 2026, Naveen Albert
+ *
+ * Naveen Albert <bbs@phreaknet.org>
+ *
+ * This program is free software, distributed under the terms of
+ * the GNU General Public License Version 2. See the LICENSE file
+ * at the top of the source tree.
+ */
+
+/*! \file
+ *
+ * \brief IMAP COMPRESS/TLS Tests
+ *
+ * \author Naveen Albert <bbs@phreaknet.org>
+ */
+
+#include "test.h"
+#include "tls.h"
+#include "compress.h"
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <string.h>
+#include <sys/stat.h>
+
+static int pre(void)
+{
+	test_preload_module("io_tls.so");
+	test_preload_module("io_compress.so");
+	test_preload_module("mod_mail.so");
+	test_preload_module("mod_mimeparse.so");
+	test_load_module("net_imap.so");
+
+	TEST_ADD_CONFIG("tls.conf");
+	TEST_ADD_CONFIG("mod_mail.conf");
+	TEST_ADD_SUBCONFIG("tls", "net_imap.conf");
+
+	TEST_RESET_MKDIR(TEST_MAIL_DIR);
+	return 0;
+}
+
+static ssize_t zlib_tls_write(void *cbdata, const char *buf, size_t len)
+{
+	SSL *ssl = cbdata;
+	return tls_write(ssl, __LINE__, buf, len);
+}
+
+static ssize_t zlib_tls_read(void *cbdata, char *buf, size_t len)
+{
+	SSL *ssl = cbdata;
+	return tls_read(ssl, __LINE__, buf, len);
+}
+
+static int run(void)
+{
+	SSL *ssl = NULL;
+	struct z_data *z = NULL;
+	int clientfd = -1;
+	int res = -1;
+
+	clientfd = test_make_socket(993);
+	REQUIRE_FD(clientfd);
+
+	/* Connect and immediately set up TLS */
+	ssl = tls_client_new(clientfd);
+	REQUIRE_SSL(ssl);
+
+	/* Log in */
+	TLS_CLIENT_EXPECT(ssl, "* OK [CAPABILITY");
+	TLS_SWRITE(ssl, "a1 LOGIN \"" TEST_USER "\" \"" TEST_PASS "\"" ENDL);
+	TLS_CLIENT_EXPECT_EVENTUALLY(ssl, "a1 OK");
+
+	TLS_SWRITE(ssl, "a2 ID (\"name\" \"lbbs.test.client\" \"version\" \"" BBS_VERSION "\" NIL)" ENDL);
+	TLS_CLIENT_EXPECT_EVENTUALLY(ssl, "a2 OK");
+
+	/* Now, set up compression on top of the TLS session */
+	TLS_SWRITE(ssl, "a3 COMPRESS DEFLATE" ENDL);
+	TLS_CLIENT_EXPECT_EVENTUALLY(ssl, "a3 OK"); /* The tagged response is without compress. After this, we use compression for the remainder of the session. */
+
+	z = z_client_new(clientfd); /* We pass in the socket file descriptor, but since we enabled compression on top of encryption, we need to use zlib on top of OpenSSL */
+	REQUIRE_ZLIB_CLIENT(z);
+
+	ZLIB_SWRITE_CB(zlib_tls_write, ssl, z, "a4 NAMESPACE" ENDL);
+	ZLIB_CLIENT_EXPECT_EVENTUALLY_READCB(zlib_tls_read, ssl, z, "a4 OK");
+
+	res = 0;
+
+cleanup:
+	/* This test doesn't actually work, and it doesn't really matter. The point is to ensure there are no file descriptor leaks regardless of what the client does.
+	 * Therefore, we always return 0, and when the test is run under valgrind, it should pass as long as we don't leak any file descriptors
+	 * due to the nested transformation. */
+	res = 0;
+	ZLIB_CLIENT_SHUTDOWN(z);
+	SSL_SHUTDOWN(ssl);
+	close_if(clientfd);
+	return res;
+}
+
+TEST_MODULE_INFO_STANDARD("IMAP COMPRESS/TLS Tests");