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 <cliffjan...@apache.org> 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: commits-unsubscr...@qpid.apache.org For additional commands, e-mail: commits-h...@qpid.apache.org