Files
asterisk/third-party/pjproject/patches/0302-Locking-fix-so-that-SSL_shutdown-and-SSL_write-are-n.patch
George Joseph 452e0d7258 bundled_pjproject: Backport security fixes from pjproject 2.13.1
Merge-pull-request-from-GHSA-9pfh-r8x4-w26w.patch
Merge-pull-request-from-GHSA-cxwq-5g9x-x7fr.patch
Locking-fix-so-that-SSL_shutdown-and-SSL_write-are-n.patch
Don-t-call-SSL_shutdown-when-receiving-SSL_ERROR_SYS.patch

Resolves: #188
2023-07-05 10:05:50 -06:00

167 lines
5.7 KiB
Diff

From 67e62627240b3f925c765cbda63ccbec328fed29 Mon Sep 17 00:00:00 2001
From: Matthew Fredrickson <mfredrickson@fluentstream.com>
Date: Tue, 30 May 2023 04:33:05 -0500
Subject: [PATCH 302/303] Locking fix so that SSL_shutdown and SSL_write are
not called at same time (#3583)
---
pjlib/src/pj/ssl_sock_ossl.c | 82 ++++++++++++++++++++++--------------
1 file changed, 51 insertions(+), 31 deletions(-)
diff --git a/pjlib/src/pj/ssl_sock_ossl.c b/pjlib/src/pj/ssl_sock_ossl.c
index 56841f80a..5c68edb2b 100644
--- a/pjlib/src/pj/ssl_sock_ossl.c
+++ b/pjlib/src/pj/ssl_sock_ossl.c
@@ -1230,44 +1230,58 @@ static void ssl_destroy(pj_ssl_sock_t *ssock)
/* Potentially shutdown OpenSSL library if this is the last
* context exists.
*/
shutdown_openssl();
}
/* Reset SSL socket state */
static void ssl_reset_sock_state(pj_ssl_sock_t *ssock)
{
+ int post_unlock_flush_circ_buf = 0;
+
ossl_sock_t *ossock = (ossl_sock_t *)ssock;
+ /* Must lock around SSL calls, particularly SSL_shutdown
+ * as it can modify the write BIOs and destructively
+ * interfere with any ssl_write() calls in progress
+ * above in a multithreaded environment */
+ pj_lock_acquire(ssock->write_mutex);
+
/* Detach from SSL instance */
if (ossock->ossl_ssl) {
SSL_set_ex_data(ossock->ossl_ssl, sslsock_idx, NULL);
}
/**
* Avoid calling SSL_shutdown() if handshake wasn't completed.
* OpenSSL 1.0.2f complains if SSL_shutdown() is called during an
* SSL handshake, while previous versions always return 0.
*/
if (ossock->ossl_ssl && SSL_in_init(ossock->ossl_ssl) == 0) {
- int ret = SSL_shutdown(ossock->ossl_ssl);
- if (ret == 0) {
- /* Flush data to send close notify. */
- flush_circ_buf_output(ssock, &ssock->shutdown_op_key, 0, 0);
- }
+ int ret = SSL_shutdown(ossock->ossl_ssl);
+ if (ret == 0) {
+ /* SSL_shutdown will potentially trigger a bunch of
+ * data to dump to the socket */
+ post_unlock_flush_circ_buf = 1;
+ }
}
- pj_lock_acquire(ssock->write_mutex);
ssock->ssl_state = SSL_STATE_NULL;
+
pj_lock_release(ssock->write_mutex);
+ if (post_unlock_flush_circ_buf) {
+ /* Flush data to send close notify. */
+ flush_circ_buf_output(ssock, &ssock->shutdown_op_key, 0, 0);
+ }
+
ssl_close_sockets(ssock);
/* Upon error, OpenSSL may leave any error description in the thread
* error queue, which sometime may cause next call to SSL API returning
* false error alarm, e.g: in Linux, SSL_CTX_use_certificate_chain_file()
* returning false error after a handshake error (in different SSL_CTX!).
* For now, just clear thread error queue here.
*/
ERR_clear_error();
}
@@ -1855,52 +1869,58 @@ static pj_status_t ssl_read(pj_ssl_sock_t *ssock, void *data, int *size)
{
ossl_sock_t *ossock = (ossl_sock_t *)ssock;
int size_ = *size;
int len = size_;
/* SSL_read() may write some data to write buffer when re-negotiation
* is on progress, so let's protect it with write mutex.
*/
pj_lock_acquire(ssock->write_mutex);
*size = size_ = SSL_read(ossock->ossl_ssl, data, size_);
- pj_lock_release(ssock->write_mutex);
if (size_ <= 0) {
pj_status_t status;
int err = SSL_get_error(ossock->ossl_ssl, size_);
- /* SSL might just return SSL_ERROR_WANT_READ in
- * re-negotiation.
- */
- if (err != SSL_ERROR_NONE && err != SSL_ERROR_WANT_READ) {
- if (err == SSL_ERROR_SYSCALL && size_ == -1 &&
- ERR_peek_error() == 0 && errno == 0)
- {
- status = STATUS_FROM_SSL_ERR2("Read", ssock, size_,
- err, len);
- PJ_LOG(4,("SSL", "SSL_read() = -1, with "
- "SSL_ERROR_SYSCALL, no SSL error, "
- "and errno = 0 - skip BIO error"));
- /* Ignore these errors */
- } else {
- /* Reset SSL socket state, then return PJ_FALSE */
- status = STATUS_FROM_SSL_ERR2("Read", ssock, size_,
- err, len);
- ssl_reset_sock_state(ssock);
- return status;
- }
- }
-
- /* Need renegotiation */
- return PJ_EEOF;
+ /* SSL might just return SSL_ERROR_WANT_READ in
+ * re-negotiation.
+ */
+ if (err != SSL_ERROR_NONE && err != SSL_ERROR_WANT_READ) {
+ if (err == SSL_ERROR_SYSCALL && size_ == -1 &&
+ ERR_peek_error() == 0 && errno == 0)
+ {
+ status = STATUS_FROM_SSL_ERR2("Read", ssock, size_,
+ err, len);
+ PJ_LOG(4,("SSL", "SSL_read() = -1, with "
+ "SSL_ERROR_SYSCALL, no SSL error, "
+ "and errno = 0 - skip BIO error"));
+ /* Ignore these errors */
+ } else {
+ /* Reset SSL socket state, then return PJ_FALSE */
+ status = STATUS_FROM_SSL_ERR2("Read", ssock, size_,
+ err, len);
+ pj_lock_release(ssock->write_mutex);
+ /* Unfortunately we can't hold the lock here to reset all the state.
+ * We probably should though.
+ */
+ ssl_reset_sock_state(ssock);
+ return status;
+ }
+ }
+
+ pj_lock_release(ssock->write_mutex);
+ /* Need renegotiation */
+ return PJ_EEOF;
}
+ pj_lock_release(ssock->write_mutex);
+
return PJ_SUCCESS;
}
/* Write plain data to SSL and flush write BIO. */
static pj_status_t ssl_write(pj_ssl_sock_t *ssock, const void *data,
pj_ssize_t size, int *nwritten)
{
ossl_sock_t *ossock = (ossl_sock_t *)ssock;
pj_status_t status = PJ_SUCCESS;
--
2.41.0