Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-3061

C++ TSSLSocket shutdown delay/vulnerability

VotersWatch issueWatchersLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 0.8, 0.9, 0.9.1, 0.9.2
    • 0.9.3
    • C++ - Library

    Description

      In the C++ implementation of TSSLSocket, during close() the following code sequence is found:

      void TSSLSocket::close() {
        if (ssl_ != NULL) {
          int rc = SSL_shutdown(ssl_);
          if (rc == 0) {
            rc = SSL_shutdown(ssl_);
          }
      

      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.

      Attachments

        1. THRIFT-3061.patch
          0.6 kB
          James E. King III

        Issue Links

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            jking3 James E. King III
            jking3 James E. King III
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Issue deployment