This is an automated email from the ASF dual-hosted git repository.
cliffjansen pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/qpid-proton.git
The following commit(s) were added to refs/heads/main by this push:
new 2eab5c0f3 PROTON-2658: fix Proton-C TLS buffer leak
2eab5c0f3 is described below
commit 2eab5c0f3be03412600d3682803cca19bf658159
Author: Clifford Jansen <[email protected]>
AuthorDate: Mon Dec 5 10:06:56 2022 -0800
PROTON-2658: fix Proton-C TLS buffer leak
---
c/src/tls/openssl.c | 58 +++++++++++++++++++++++++++++++++++++++++++-
c/tests/tls_test.cpp | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 125 insertions(+), 1 deletion(-)
diff --git a/c/src/tls/openssl.c b/c/src/tls/openssl.c
index ab125cb82..8c96a35d6 100644
--- a/c/src/tls/openssl.c
+++ b/c/src/tls/openssl.c
@@ -222,7 +222,8 @@ struct pn_tls_t {
static void encrypt(pn_tls_t *);
static void decrypt(pn_tls_t *);
static int pn_tls_alpn_cb(SSL *ssn, const unsigned char **out, unsigned char
*outlen, const unsigned char *in, unsigned int inlen, void *arg);
-
+static void release_buffers(pn_tls_t *
+);
static void tls_initialize_buffers(pn_tls_t *tls) {
// Link together free lists
@@ -1143,6 +1144,8 @@ int pn_tls_stop(pn_tls_t *tls) {
if (tls->stopped)
return PN_ARG_ERR;
tls->stopped = true;
+ release_buffers(tls);
+ if (tls->validating) validate_strict(tls);
return 0;
}
@@ -1927,6 +1930,59 @@ static buff_ptr current_decrypted_result(pn_tls_t *tls) {
return tls->dresult_first_blank;
}
+static void release_buffers(pn_tls_t *tls) {
+ // encrypt input
+ for(;tls->encrypt_first_pending;) {
+ buff_ptr p = tls->encrypt_first_pending;
+ assert(tls->encrypt_buffers[p-1].type == buff_encrypt_pending);
+ if (!tls->encrypt_first_done) {
+ tls->encrypt_first_done = p;
+ }
+ if (tls->encrypt_last_done) {
+ tls->encrypt_buffers[tls->encrypt_last_done-1].next = p;
+ }
+ tls->encrypt_last_done = p;
+ tls->encrypt_first_pending = tls->encrypt_buffers[p-1].next;
+
+ tls->encrypt_buffers[p-1].next = 0;
+ tls->encrypt_buffers[p-1].type = buff_encrypt_done;
+ }
+
+ // decrypt input
+ for(;tls->decrypt_first_pending;) {
+ buff_ptr p = tls->decrypt_first_pending;
+ assert(tls->decrypt_buffers[p-1].type == buff_decrypt_pending);
+ if (!tls->decrypt_first_done) {
+ tls->decrypt_first_done = p;
+ }
+ if (tls->decrypt_last_done) {
+ tls->decrypt_buffers[tls->decrypt_last_done-1].next = p;
+ }
+ tls->decrypt_last_done = p;
+ tls->decrypt_first_pending = tls->decrypt_buffers[p-1].next;
+
+ tls->decrypt_buffers[p-1].next = 0;
+ tls->decrypt_buffers[p-1].type = buff_decrypt_done;
+ }
+
+ // encrypt output
+ for(;tls->eresult_first_blank;) {
+ buff_ptr p = tls->eresult_first_blank;
+ assert(tls->eresult_buffers[p-1].type == buff_eresult_blank);
+ blank_eresult_pop(tls, p);
+ encrypted_result_add(tls, p);
+ }
+
+ // decrypt output
+ for(;tls->dresult_first_blank;) {
+ buff_ptr p = tls->dresult_first_blank;
+ assert(tls->dresult_buffers[p-1].type == buff_dresult_blank);
+ blank_dresult_pop(tls, p);
+ decrypted_result_add(tls, p);
+ }
+
+}
+
static bool try_shutdown(pn_tls_t *tls) {
assert(tls->enc_closed && !tls->encrypt_first_pending && !tls->ssl_shutdown);
bool success = false;
diff --git a/c/tests/tls_test.cpp b/c/tests/tls_test.cpp
index 44b4b972f..c735abe44 100644
--- a/c/tests/tls_test.cpp
+++ b/c/tests/tls_test.cpp
@@ -471,3 +471,71 @@ TEST_CASE("missing client cert") {
set_rbuf(&early_data_buf, NULL, 0, 0);
reset_rbuf(&early_data_buf);
}
+
+TEST_CASE("buffer release on pn_tls_stop()") {
+ pn_raw_buffer_t bufs[4];
+ size_t give_count = 0;
+ size_t take_count = 0;
+ TestPeer client(false);
+ client.init(); // ... up to pn_tls_start()
+ pn_tls_t *tls = client.tls;
+
+ // Confirm we can add them but not take out empty buffers until after
pn_tls_top()
+ size_t count = 1;
+ for (size_t i = 0; i < count; i++)
+ bufs[i] = new_rbuf(512);
+ give_count += count;
+ REQUIRE(pn_tls_give_encrypt_input_buffers(tls, bufs, count) == count);
+ REQUIRE(pn_tls_take_encrypt_input_buffers(tls, bufs, count) == 0);
+
+ count = 2;
+ for (size_t i = 0; i < count; i++)
+ bufs[i] = new_rbuf(512);
+ give_count += count;
+ REQUIRE(pn_tls_give_decrypt_input_buffers(tls, bufs, count) == count);
+ REQUIRE(pn_tls_take_decrypt_input_buffers(tls, bufs, count) == 0);
+
+ count = 3;
+ for (size_t i = 0; i < count; i++)
+ bufs[i] = new_rbuf(512);
+ give_count += count;
+ REQUIRE(pn_tls_give_encrypt_output_buffers(tls, bufs, count) == count);
+ REQUIRE(pn_tls_get_encrypt_output_buffer_count(tls) == 0);
+ REQUIRE(pn_tls_take_encrypt_output_buffers(tls, bufs, count) == 0);
+
+ count = 4;
+ for (size_t i = 0; i < count; i++)
+ bufs[i] = new_rbuf(512);
+ give_count += count;
+ REQUIRE(pn_tls_give_decrypt_output_buffers(tls, bufs, count) == count);
+ REQUIRE(pn_tls_get_decrypt_output_buffer_count(tls) == 0);
+ REQUIRE(pn_tls_take_decrypt_output_buffers(tls, bufs, count) == 0);
+
+ pn_tls_stop(tls);
+
+ count = pn_tls_take_encrypt_input_buffers(tls, bufs, 4);
+ for (size_t i = 0; i < count; i++)
+ free_rbuf(bufs[i]);
+ take_count += count;
+
+ count = pn_tls_take_decrypt_input_buffers(tls, bufs, 4);
+ for (size_t i = 0; i < count; i++)
+ free_rbuf(bufs[i]);
+ take_count += count;
+
+ size_t avail = pn_tls_get_encrypt_output_buffer_count(tls);
+ count = pn_tls_take_encrypt_output_buffers(tls, bufs, 4);
+ REQUIRE(avail == count);
+ for (size_t i = 0; i < count; i++)
+ free_rbuf(bufs[i]);
+ take_count += count;
+
+ avail = pn_tls_get_decrypt_output_buffer_count(tls);
+ count = pn_tls_take_decrypt_output_buffers(tls, bufs, 4);
+ REQUIRE(avail == count);
+ for (size_t i = 0; i < count; i++)
+ free_rbuf(bufs[i]);
+ take_count += count;
+
+ REQUIRE(take_count == give_count);
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
