[openssl-users] Strange problem with 1.0.2f SSL_shutdown in multithreaded server

[Jakob Bohm]

I have not yet found the cause of this issue, however I have
found that a minimal version of your patch which just adds
back the SSL_in_init() condition seems to at least make the
diagnostic test case (using s_client) work again.

I have not kept the test for s being NULL, as that case would
have crashed a few lines earlier in SSL_shutdown(), so can't
reach the if statement anyway.

I have attached the reduced patch, but I still think the real
cause must be elsewhere.

On 02/02/2016 12:34, Matt Caswell wrote:
> On 02/02/16 11:24, Jakob Bohm wrote:
>> On 02/02/2016 11:40, Matt Caswell wrote:
>>> On 02/02/16 07:52, Jakob Bohm wrote:
>>>> I am trying to upgrade an existing 3rd party multithreaded server
>>>> from OpenSSL 1.0.2c to 1.0.2f .  However when I do so, it starts
>>>> mishandling the close_notify "alert".
>>>>
>>>> 1.0.2f seems to send the close_notify alert unencrypted followed
>>>> by an encrypted decrypt_failed alert, where 1.0.2c correctly
>>>> sends just an encrypted close_notify alert.
>>>>
>>>> I am unsure if this exposed a bug in the daemon or in OpenSSL
>>>> itself.
>>> I have a theory.
>>>
>>> Previous versions of 1.0.2 handled an SSL_shutdown() call while in the
>>> middle of a handshake by ignoring it and returning 1 immediately
>>> (meaning everything shutdown successfully). Clearly everything did not
>>> shutdown successfully so the return value is not correct.
>> No, actual application data (just a few bytes) was sent in each
>> direction.
>>
>> Specifically, some bytes were sent client to server, then a reply
>> was sent server to client and the connection was closed.
>>
>> Also, the s_client output showed a completed handshake, with
>> ChangeCipherSpec in both directions and dump of certificate
>> chain before the first application data was sent (client to
>> server, then server to client).
>>
>> The s_client command line was
>>
>> cat data | openssl s_client -connect xx.xx.xx.xx:xxxx -msg -tls1
>> -ign_eof -debug
>>
>> However I cannot rule out that the changes to either SSL_shutdown()
>> or the rearranged error checking code triggered the issue.
> Hmmm. Perhaps try reverting the SSL_shutdown() change to rule that out
> as related in some way? Patch attached to revert that change back to the
> previous implementation.


Enjoy

Jakob
-- 
Jakob Bohm, CIO, Partner, WiseMo A/S.  https://www.wisemo.com
Transformervej 29, 2860 Søborg, Denmark.  Direct +45 31 13 16 10
This public discussion message is non-binding and may contain errors.
WiseMo - Remote Service Management for PCs, Phones and Embedded

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mta.openssl.org/pipermail/openssl-users/attachments/20160205/c88bb508/attachment.html>
-------------- next part --------------
This is sufficient to stop the race condition in one multithreaded server
application that causes it to send ordinary after init shutdown unencrypted,
the underlying race condition has not been found.
---
 ssl/ssl_lib.c |  5 ++++-
 4 files changed, 4 insertions(+), 20 deletions(-)

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 2744be8..f2071db 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -1060,7 +1060,10 @@ int SSL_shutdown(SSL *s)
         return -1;
     }
 
-    return s->method->ssl_shutdown(s);
+    if (!SSL_in_init(s))
+        return (s->method->ssl_shutdown(s));
+    else
+        return (1);
 }
 
 int SSL_renegotiate(SSL *s)
-- 
2.5.0