[MESOS-6621] SSL downgrade path will CHECK-fail when using both temporary and persistent sockets

• Type: Bug
• Status: Resolved
• Priority: Blocker
• Resolution: Fixed
• Affects Version/s: 0.28.2, 1.0.2, 1.1.0
• Fix Version/s: 0.28.3, 1.0.3, 1.1.1, 1.2.0
• Component/s: libprocess
• Labels: mesosphere

Environment

SSL with downgrade enabled

• Target Version/s: 0.28.3, 1.0.3, 1.1.1, 1.2.0
• Epic Link: SSL Tech Debt
• Sprint: Mesosphere Sprint 47
• Story Points: 3
• Assignee: Joseph Wu
• Reporter: Joseph Wu
• Shepherd: Benjamin Mahler
• Created: 22/Nov/16 02:30
• Updated: 01/Sep/17 23:34
• Resolved: 23/Nov/16 00:33

Description

The code path for downgrading sockets from SSL to non-SSL includes this code:

    // If this address is a temporary link.
    if (temps.count(addresses[to_fd]) > 0) {
      temps[addresses[to_fd]] = to_fd;
      // No need to erase as we're changing the value, not the key.
    }

    // If this address is a persistent link.
    if (persists.count(addresses[to_fd]) > 0) {
      persists[addresses[to_fd]] = to_fd;
      // No need to erase as we're changing the value, not the key.
    }

https://github.com/apache/mesos/blob/1.1.x/3rdparty/libprocess/src/process.cpp#L2311-L2321

It is possible for libprocess to hold both temporary and persistent sockets to the same address. This can happen when a message is first sent (ProcessBase::send), and then a link is established (ProcessBase::link). When the target of the message/link is a non-SSL socket, both temporary and persistent sockets go through the downgrade path.

If a temporary socket is present while a persistent socket is being created, the above code will remap both temporary and persistent sockets to the same address (it should only remap the persistent socket). This leads to some CHECK failures if those sockets are used or closed later:

•     bool persist = persists.count(address) > 0;
      bool temp = temps.count(address) > 0;
      if (persist || temp) {
        int s = persist ? persists[address] : temps[address];
        CHECK(sockets.count(s) > 0);
  socket = sockets.at(s);
  

  https://github.com/apache/mesos/blob/1.1.x/3rdparty/libprocess/src/process.cpp#L1942

•         if (dispose.count(s) > 0) {
            // This is either a temporary socket we created or it's a
            // socket that we were receiving data from and possibly
            // sending HTTP responses back on. Clean up either way.
            if (addresses.count(s) > 0) {
              const Address& address = addresses[s];
              CHECK(temps.count(address) > 0 && temps[address] == s);
  temps.erase(address);
  

  https://github.com/apache/mesos/blob/1.1.x/3rdparty/libprocess/src/process.cpp#L2044