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

C++ TSSLSocket shutdown delay/vulnerability

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.8, 0.9, 0.9.1, 0.9.2
    • Fix Version/s: 0.9.3
    • Component/s: C++ - Library
    • Labels:

      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

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: