<p>Joshua Colp <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/6109">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Richard Mudgett: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, approved; Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">bundled_pjproject: Improve SSL/TLS error handling<br><br>OpenSSL has 2 levels or error processing. It's possible for the<br>top layer to return SSL_ERROR_SYSCALL but the lower layer return<br>no error, in which case processing should continue. Only the top<br>layer was being examined though so connections were being torn<br>down when they didn't need to be. This patch adds the examination<br>of the lower level codes, and if they return no errors, allows<br>processing to continue.<br><br>ASTERISK-27001<br>Reported-by: Ian Gilmore<br>Patch-by: Ian Gilmore (pjproject-2.6.patch License 6889)<br> Updated-by: George Joseph and Sauw Ming (Teluu)<br><br>Merged to upstream pjproject on 7/27/2017 (commit 5631)<br><br>Change-Id: I23844ca0c68ef1ee550f14d46f6dae57d33b7bd2<br>---<br>A third-party/pjproject/patches/0075-Fixed-2030-Improve-error-handling-in-OpenSSL-socket.patch<br>1 file changed, 247 insertions(+), 0 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/third-party/pjproject/patches/0075-Fixed-2030-Improve-error-handling-in-OpenSSL-socket.patch b/third-party/pjproject/patches/0075-Fixed-2030-Improve-error-handling-in-OpenSSL-socket.patch<br>new file mode 100644<br>index 0000000..1e7035d<br>--- /dev/null<br>+++ b/third-party/pjproject/patches/0075-Fixed-2030-Improve-error-handling-in-OpenSSL-socket.patch<br>@@ -0,0 +1,247 @@<br>+From 96c06899d95eaf01d05561554b21e8c63baa7129 Mon Sep 17 00:00:00 2001<br>+From: ming <ming@localhost><br>+Date: Thu, 27 Jul 2017 06:07:54 +0000<br>+Subject: [PATCH 75/76] Fixed #2030: Improve error handling in OpenSSL socket<br>+<br>+---<br>+ pjlib/src/pj/ssl_sock_ossl.c | 173 ++++++++++++++++++++++++++++++++++++++-----<br>+ 1 file changed, 156 insertions(+), 17 deletions(-)<br>+<br>+diff --git a/pjlib/src/pj/ssl_sock_ossl.c b/pjlib/src/pj/ssl_sock_ossl.c<br>+index c466b3c..b8175e1 100644<br>+--- a/pjlib/src/pj/ssl_sock_ossl.c<br>++++ b/pjlib/src/pj/ssl_sock_ossl.c<br>+@@ -298,14 +298,104 @@ static pj_status_t flush_delayed_send(pj_ssl_sock_t *ssock);<br>+ /* Expected maximum value of reason component in OpenSSL error code */<br>+ #define MAX_OSSL_ERR_REASON 1200<br>+ <br>+-static pj_status_t STATUS_FROM_SSL_ERR(pj_ssl_sock_t *ssock,<br>+- unsigned long err)<br>++<br>++static char *SSLErrorString (int err)<br>+ {<br>+- pj_status_t status;<br>++ switch (err) {<br>++ case SSL_ERROR_NONE:<br>++ return "SSL_ERROR_NONE";<br>++ case SSL_ERROR_ZERO_RETURN:<br>++ return "SSL_ERROR_ZERO_RETURN";<br>++ case SSL_ERROR_WANT_READ:<br>++ return "SSL_ERROR_WANT_READ";<br>++ case SSL_ERROR_WANT_WRITE:<br>++ return "SSL_ERROR_WANT_WRITE";<br>++ case SSL_ERROR_WANT_CONNECT:<br>++ return "SSL_ERROR_WANT_CONNECT";<br>++ case SSL_ERROR_WANT_ACCEPT:<br>++ return "SSL_ERROR_WANT_ACCEPT";<br>++ case SSL_ERROR_WANT_X509_LOOKUP:<br>++ return "SSL_ERROR_WANT_X509_LOOKUP";<br>++ case SSL_ERROR_SYSCALL:<br>++ return "SSL_ERROR_SYSCALL";<br>++ case SSL_ERROR_SSL:<br>++ return "SSL_ERROR_SSL";<br>++ default:<br>++ return "SSL_ERROR_UNKNOWN";<br>++ }<br>++}<br>+ <br>+- /* General SSL error, dig more from OpenSSL error queue */<br>+- if (err == SSL_ERROR_SSL)<br>+- err = ERR_get_error();<br>++#define ERROR_LOG(msg, err) \<br>++ PJ_LOG(2,("SSL", "%s (%s): Level: %d err: <%lu> <%s-%s-%s> len: %d", \<br>++ msg, action, level, err, \<br>++ (ERR_lib_error_string(err)? ERR_lib_error_string(err): "???"), \<br>++ (ERR_func_error_string(err)? ERR_func_error_string(err):"???"),\<br>++ (ERR_reason_error_string(err)? \<br>++ ERR_reason_error_string(err): "???"), len));<br>++<br>++static void SSLLogErrors(char * action, int ret, int ssl_err, int len)<br>++{<br>++ char *ssl_err_str = SSLErrorString(ssl_err);<br>++<br>++ if (!action) {<br>++ action = "UNKNOWN";<br>++ }<br>++<br>++ switch (ssl_err) {<br>++ case SSL_ERROR_SYSCALL:<br>++ {<br>++ unsigned long err2 = ERR_get_error();<br>++ if (err2) {<br>++ int level = 0;<br>++ while (err2) {<br>++ ERROR_LOG("SSL_ERROR_SYSCALL", err2);<br>++ level++;<br>++ err2 = ERR_get_error();<br>++ }<br>++ } else if (ret == 0) {<br>++ /* An EOF was observed that violates the protocol */<br>++<br>++ /* The TLS/SSL handshake was not successful but was shut down<br>++ * controlled and by the specifications of the TLS/SSL protocol.<br>++ */<br>++ } else if (ret == -1) {<br>++ /* BIO error - look for more info in errno... */<br>++ char errStr[250] = "";<br>++ strerror_r(errno, errStr, sizeof(errStr));<br>++ /* for now - continue logging these if they occur.... */<br>++ PJ_LOG(4,("SSL", "BIO error, SSL_ERROR_SYSCALL (%s): "<br>++ "errno: <%d> <%s> len: %d",<br>++ action, errno, errStr, len));<br>++ } else {<br>++ /* ret!=0 & ret!=-1 & nothing on error stack - is this valid??? */<br>++ PJ_LOG(2,("SSL", "SSL_ERROR_SYSCALL (%s) ret: %d len: %d",<br>++ action, ret, len));<br>++ }<br>++ break;<br>++ }<br>++ case SSL_ERROR_SSL:<br>++ {<br>++ unsigned long err2 = ERR_get_error();<br>++ int level = 0;<br>++<br>++ while (err2) {<br>++ ERROR_LOG("SSL_ERROR_SSL", err2);<br>++ level++;<br>++ err2 = ERR_get_error();<br>++ }<br>++ break;<br>++ }<br>++ default:<br>++ PJ_LOG(2,("SSL", "%lu [%s] (%s) ret: %d len: %d",<br>++ ssl_err, ssl_err_str, action, ret, len));<br>++ break;<br>++ }<br>++}<br>++<br>++<br>++static pj_status_t GET_STATUS_FROM_SSL_ERR(unsigned long err)<br>++{<br>++ pj_status_t status;<br>+ <br>+ /* OpenSSL error range is much wider than PJLIB errno space, so<br>+ * if it exceeds the space, only the error reason will be kept.<br>+@@ -317,13 +407,49 @@ static pj_status_t STATUS_FROM_SSL_ERR(pj_ssl_sock_t *ssock,<br>+ status = ERR_GET_REASON(err);<br>+ <br>+ status += PJ_SSL_ERRNO_START;<br>+- ssock->last_err = err;<br>+ return status;<br>+ }<br>+ <br>++/* err contains ERR_get_error() status */<br>++static pj_status_t STATUS_FROM_SSL_ERR(char *action, pj_ssl_sock_t *ssock,<br>++ unsigned long err)<br>++{<br>++ int level = 0;<br>++ int len = 0; //dummy<br>++<br>++ ERROR_LOG("STATUS_FROM_SSL_ERR", err);<br>++ level++;<br>++<br>++ /* General SSL error, dig more from OpenSSL error queue */<br>++ if (err == SSL_ERROR_SSL) {<br>++ err = ERR_get_error();<br>++ ERROR_LOG("STATUS_FROM_SSL_ERR", err);<br>++ }<br>++<br>++ ssock->last_err = err;<br>++ return GET_STATUS_FROM_SSL_ERR(err);<br>++}<br>++<br>++/* err contains SSL_get_error() status */<br>++static pj_status_t STATUS_FROM_SSL_ERR2(char *action, pj_ssl_sock_t *ssock,<br>++ int ret, int err, int len)<br>++{<br>++ unsigned long ssl_err = err;<br>++<br>++ if (err == SSL_ERROR_SSL) {<br>++ ssl_err = ERR_peek_error();<br>++ }<br>++<br>++ /* Dig for more from OpenSSL error queue */<br>++ SSLLogErrors(action, ret, err, len);<br>++<br>++ ssock->last_err = ssl_err;<br>++ return GET_STATUS_FROM_SSL_ERR(ssl_err);<br>++}<br>++<br>+ static pj_status_t GET_SSL_STATUS(pj_ssl_sock_t *ssock)<br>+ {<br>+- return STATUS_FROM_SSL_ERR(ssock, ERR_get_error());<br>++ return STATUS_FROM_SSL_ERR("status", ssock, ERR_get_error());<br>+ }<br>+ <br>+ <br>+@@ -1514,7 +1640,7 @@ static pj_bool_t on_handshake_complete(pj_ssl_sock_t *ssock,<br>+ unsigned long err;<br>+ err = ERR_get_error();<br>+ if (err != SSL_ERROR_NONE)<br>+- status = STATUS_FROM_SSL_ERR(ssock, err);<br>++ status = STATUS_FROM_SSL_ERR("connecting", ssock, err);<br>+ }<br>+ reset_ssl_sock_state(ssock);<br>+ }<br>+@@ -1833,11 +1959,11 @@ static pj_status_t do_handshake(pj_ssl_sock_t *ssock)<br>+ }<br>+ <br>+ if (err < 0) {<br>+- err = SSL_get_error(ssock->ossl_ssl, err);<br>+- if (err != SSL_ERROR_NONE && err != SSL_ERROR_WANT_READ) <br>++ int err2 = SSL_get_error(ssock->ossl_ssl, err);<br>++ if (err2 != SSL_ERROR_NONE && err2 != SSL_ERROR_WANT_READ)<br>+ {<br>+ /* Handshake fails */<br>+- status = STATUS_FROM_SSL_ERR(ssock, err);<br>++ status = STATUS_FROM_SSL_ERR2("Handshake", ssock, err, err2, 0);<br>+ return status;<br>+ }<br>+ }<br>+@@ -1913,6 +2039,7 @@ static pj_bool_t asock_on_data_read (pj_activesock_t *asock,<br>+ read_data_t *buf = *(OFFSET_OF_READ_DATA_PTR(ssock, data));<br>+ void *data_ = (pj_int8_t*)buf->data + buf->len;<br>+ int size_ = (int)(ssock->read_size - buf->len);<br>++ int len = size_;<br>+ <br>+ /* SSL_read() may write some data to BIO write when re-negotiation<br>+ * is on progress, so let's protect it with write mutex.<br>+@@ -1965,10 +2092,22 @@ static pj_bool_t asock_on_data_read (pj_activesock_t *asock,<br>+ */<br>+ if (err != SSL_ERROR_NONE && err != SSL_ERROR_WANT_READ)<br>+ {<br>+- /* Reset SSL socket state, then return PJ_FALSE */<br>+- status = STATUS_FROM_SSL_ERR(ssock, err);<br>+- reset_ssl_sock_state(ssock);<br>+- goto on_error;<br>++ if (err == SSL_ERROR_SYSCALL && size_ == -1 &&<br>++ ERR_peek_error() == 0 && errno == 0)<br>++ {<br>++ status = STATUS_FROM_SSL_ERR2("Read", ssock, size_,<br>++ err, len);<br>++ PJ_LOG(4,("SSL", "SSL_read() = -1, with "<br>++ "SSL_ERROR_SYSCALL, no SSL error, "<br>++ "and errno = 0 - skip BIO error"));<br>++ /* Ignore these errors */<br>++ } else {<br>++ /* Reset SSL socket state, then return PJ_FALSE */<br>++ status = STATUS_FROM_SSL_ERR2("Read", ssock, size_,<br>++ err, len);<br>++ reset_ssl_sock_state(ssock);<br>++ goto on_error;<br>++ }<br>+ }<br>+ <br>+ status = do_handshake(ssock);<br>+@@ -2856,7 +2995,7 @@ static pj_status_t ssl_write(pj_ssl_sock_t *ssock,<br>+ status = PJ_EBUSY;<br>+ } else {<br>+ /* Some problem occured */<br>+- status = STATUS_FROM_SSL_ERR(ssock, err);<br>++ status = STATUS_FROM_SSL_ERR2("Write", ssock, nwritten, err, size);<br>+ }<br>+ } else {<br>+ /* nwritten < *size, shouldn't happen, unless write BIO cannot hold <br>+-- <br>+2.9.4<br>+<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/6109">change 6109</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/6109"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 14 </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I23844ca0c68ef1ee550f14d46f6dae57d33b7bd2 </div>
<div style="display:none"> Gerrit-Change-Number: 6109 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Richard Mudgett <rmudgett@digium.com> </div>