[IMPALA-2747] Thrift-client cleans openSSL state before using it in the case of the catalog

• Type: Sub-task
• Status: Resolved
• Priority: Blocker
• Resolution: Fixed
• Affects Version/s: Impala 2.3.0
• Fix Version/s: Impala 2.5.0, Impala 2.3.2
• Component/s: Security
• Labels: catalog-server impala security
• Target Version: Impala 2.5.0, Impala 2.3.2
• Assignee: Sailesh Mukil
• Reporter: Sailesh Mukil
• Created: 06/Dec/15 01:57
• Updated: 20/Jan/16 19:29
• Resolved: 19/Jan/16 19:14

Description

The ThriftClientImpl::CreateSocket() function creates a local TSSLSocketFactory instance which gets cleaned up when the function exits in the catalogd.

Status ThriftClientImpl::CreateSocket() {
  if (!ssl_) {
    socket_.reset(new TSocket(address_.hostname, address_.port));
  } else {
    try {
      TSSLSocketFactory factory;
      // TODO: No need to do this every time we create a socket, the factory can be
      // shared. But since there may be many certificates, this needs some slightly more
      // complex infrastructure to do right.
      factory.loadTrustedCertificates(FLAGS_ssl_client_ca_certificate.c_str());
      socket_ = factory.createSocket(address_.hostname, address_.port);
    } catch (const TException& e) {
      return Status(Substitute("Failed to create socket: $0", e.what()));
    }
  }

  return Status::OK();
}

This code is shared by the statestored, impalad and the catalogd. The reason it happens only in the catalogd is because in the statestored and impalad, we create a ThriftServer socket (see ThriftServer::CreateSocket()) before creating a ThriftClientImpl socket. However, in the catalogd, we create the server after we create the client.

The ThriftServer::CreateSocket() code creates a TSSLSocketFactory shared pointer which is not destroyed when the function completes.

Why this makes a difference is because of how TSSLSocketFactory is setup and destroyed:

// TSSLSocketFactory implementation
bool     TSSLSocketFactory::initialized = false;
uint64_t TSSLSocketFactory::count_ = 0;
Mutex    TSSLSocketFactory::mutex_;

TSSLSocketFactory::TSSLSocketFactory(): server_(false) {
  Guard guard(mutex_);
  if (count_ == 0) {
    initializeOpenSSL();
    randomize();
  }
  count_++;
  ctx_ = boost::shared_ptr<SSLContext>(new SSLContext);
}

TSSLSocketFactory::~TSSLSocketFactory() {
  Guard guard(mutex_);
  count_--;
  if (count_ == 0) {
    cleanupOpenSSL();
  }
}

The interesting variable here is count_. In case of the catalogd, since we run the ThriftClientImpl::CreateSocket() code first, the count_ is decremented to 0 when the CreateSocket() function exits, causing it to call cleanupOpenSSL(), which loses all the state set up by the client.