[jira] [Commented] (THRIFT-3061) C++ TSSLSocket shutdown delay/vulnerability

["ASF GitHub Bot (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/THRIFT-3061?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14394925#comment-14394925 ] 

ASF GitHub Bot commented on THRIFT-3061:
----------------------------------------

GitHub user jeking3 opened a pull request:

    https://github.com/apache/thrift/pull/423

    [THRIFT-3061] eliminate potential for client to hang SSLSocket close

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/jeking3/thrift bugfix/THRIFT-3061-ssl-shutdown-vulnerability

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/thrift/pull/423.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #423
    
----
commit 5cb13f25ee6d292f2fde0051a2b7ee56b7884055
Author: Jim King <ji...@simplivity.com>
Date:   2015-04-03T18:56:27Z

    [THRIFT-3061] eliminate potential for client to hang SSLSocket close

----


> C++ TSSLSocket shutdown delay/vulnerability
> -------------------------------------------
>
>                 Key: THRIFT-3061
>                 URL: https://issues.apache.org/jira/browse/THRIFT-3061
>             Project: Thrift
>          Issue Type: Bug
>          Components: C++ - Library
>    Affects Versions: 0.8, 0.9, 0.9.1, 0.9.2
>            Reporter: James E. King, III
>              Labels: OpenSSL, SSL
>         Attachments: THRIFT-3061.patch
>
>
> In the C++ implementation of TSSLSocket, during close() the following code sequence is found:
> {noformat}
> void TSSLSocket::close() {
>   if (ssl_ != NULL) {
>     int rc = SSL_shutdown(ssl_);
>     if (rc == 0) {
>       rc = SSL_shutdown(ssl_);
>     }
> {noformat}
> According to the OpenSSL documentation, while it is "nice" to attempt to shutdown SSL in this way, it is not required when the code is performing a unilateral shutdown (meaning the underlying connection will be closed).  It is only necessary to call SSL_Shutdown twice like this if the socket (and configured SSL therein) is going to be reused.
> It is possible to have a misbehaving client that does not handle this part of the shutdown process properly and fail to reply, and also fail to close.  The most likely embodiment would be a program designed specifically to fail to reply to the "close notify" and hold the socket open in order to produce a denial-of-service attack.  Another possibility is a poorly written client.
> Given the OpenSSL documentation states that calling SSL_Shutdown once is sufficient when the socket is going to be closed, I recommend that we remove the second SSL_Shutdown call and prevent something that can block the close() call.  We have seen this happen, and it interacts with THRIFT-2441.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)