Fix a problem in the StartTLS extended operation processing that could caus...

neil_a_wilson

26.14.2006 f2bcf31dabb8f69261b0b829fc989e9ba5323ee6

Fix a problem in the StartTLS extended operation processing that could cause
problems with clients due to a race condition.  Previously, the success
response was sent to the client before TLS negotiation was started (because the
StartTLS response must be sent in the clear), and it was possible that if a
client was able to receive that response and send a subsequent TLS-protected
request before the was able to begin the TLS negotiation, then the server would
try to handle the client request as if it were in the clear and would not be
able to decode it.  The server now prepares to perform the TLS negotiation
before sending the response to the client to eliminate that race condition.

OpenDS Issue Number:  725

 opends/src/server/org/opends/server/core/ExtendedOperation.java

@@ -702,6 +702,23 @@


  /**
   * Indicates whether the response for this extended operation has been sent
   * from somewhere outside of this class.  This should only be used by the
   * StartTLS extended operation for the case in which it needs to send a
   * response in the clear after TLS negotiation has already started on the
   * connection.
   */
  public void setResponseSent()
  {
    assert debugEnter(CLASS_NAME, "setResponseSent",
                      String.valueOf(responseSent));

    this.responseSent = true;
  }



  /**
   * Attempts to cancel this operation before processing has completed.
   *
   * @param  cancelRequest  Information about the way in which the operation

 opends/src/server/org/opends/server/extensions/StartTLSExtendedOperation.java

@@ -36,6 +36,7 @@
import org.opends.server.core.DirectoryServer;
import org.opends.server.core.ExtendedOperation;
import org.opends.server.core.InitializationException;
import org.opends.server.types.DisconnectReason;
import org.opends.server.types.ErrorLogCategory;
import org.opends.server.types.ErrorLogSeverity;
import org.opends.server.types.ResultCode;
@@ -176,15 +177,6 @@
    }


    // If we've gotten here, then we are going to enable TLS protection or
    // close the client connection if an error occurs.  But we have to send the
    // response to the client now before enabling TLS.  Note that by doing this,
    // we forfeit the ability to send and error response if a failure occurs
    // later (e.g., in a post-operation plugin).
    operation.setResultCode(ResultCode.SUCCESS);
    operation.sendExtendedResponse();


    // Actually enable TLS protection on the client connection.  This may fail,
    // but if it does then the connection will be closed so we'll just need to
    // log it.
@@ -200,6 +192,29 @@
               MSGID_STARTTLS_ERROR_ON_ENABLE,
               stackTraceToSingleLineString(de));
    }


    // TLS was successfully enabled on the client connection, but we need to
    // send the response in the clear.
    operation.setResultCode(ResultCode.SUCCESS);

    try
    {
      tlsCapableConnection.sendClearResponse(operation);
      operation.setResponseSent();
    }
    catch (Exception e)
    {
      assert debugException(CLASS_NAME, "processExtendedOperation", e);

      logError(ErrorLogCategory.CORE_SERVER, ErrorLogSeverity.MILD_ERROR,
               MSGID_STARTTLS_ERROR_SENDING_CLEAR_RESPONSE,
               stackTraceToSingleLineString(e));

      clientConnection.disconnect(DisconnectReason.SECURITY_PROBLEM, false,
                                  MSGID_STARTTLS_ERROR_SENDING_CLEAR_RESPONSE,
                                  stackTraceToSingleLineString(e));
    }
  }
}


 opends/src/server/org/opends/server/extensions/TLSCapableConnection.java

@@ -29,6 +29,7 @@


import org.opends.server.core.DirectoryException;
import org.opends.server.core.Operation;



@@ -86,5 +87,22 @@
   */
  public void disableTLSConnectionSecurityProvider()
         throws DirectoryException;



  /**
   * Sends a response to the client in the clear rather than through the
   * encrypted channel.  This should only be used when processing the StartTLS
   * extended operation to send the response in the clear after the SSL
   * negotiation has already been initiated.
   *
   * @param  operation  The operation for which to send the response in the
   *                    clear.
   *
   * @throws  DirectoryException  If a problem occurs while sending the response
   *                              in the clear.
   */
  public void sendClearResponse(Operation operation)
         throws DirectoryException;
}


 opends/src/server/org/opends/server/messages/ExtensionsMessages.java

@@ -4003,6 +4003,17 @@


  /**
   * The message ID for the message that will be used if an error occurs while
   * attempting to send the clear-text StartTLS response after initiating TLS
   * negotiation.  This takes a single argument, which is a string
   * representation of the exception that was caught.
   */
  public static final int MSGID_STARTTLS_ERROR_SENDING_CLEAR_RESPONSE =
       CATEGORY_MASK_EXTENSIONS | SEVERITY_MASK_MILD_ERROR | 379;



  /**
   * Associates a set of generic messages with the message IDs defined in this
   * class.
   */
@@ -4818,6 +4829,10 @@
                    "An unexpected error occurred while attempting to enable " +
                    "the TLS connection security manager on the client " +
                    "connection for the purpose of StartTLS:  %s.");
    registerMessage(MSGID_STARTTLS_ERROR_SENDING_CLEAR_RESPONSE,
                    "An unexpected error occurred while attempting to " +
                    "send the clear-text response to the client after " +
                    "starting TLS negotiation:  %s.");




 opends/src/server/org/opends/server/messages/ProtocolMessages.java

@@ -4155,6 +4155,19 @@
  public static final int MSGID_ADDRESSMASK_FORMAT_DECODE_ERROR =
       CATEGORY_MASK_PROTOCOL | SEVERITY_MASK_SEVERE_ERROR | 382;



  /**
   * The message ID for the message that will be used if an attempt is made to
   * send a clear-text response over a client connection that doesn't have a
   * handle to the clear-text security provider.  This takes a single argument,
   * which is a string representation of the client connection.
   */
  public static final int MSGID_LDAP_NO_CLEAR_SECURITY_PROVIDER =
       CATEGORY_MASK_PROTOCOL | SEVERITY_MASK_MILD_ERROR | 383;



  /**
   * Associates a set of generic messages with the message IDs defined in this
   * class.
@@ -5504,6 +5517,10 @@
                    "close a StartTLS session on a client connection while " +
                    "leaving the underlying TCP connection active.  The " +
                    "TCP connection will be closed.");
    registerMessage(MSGID_LDAP_NO_CLEAR_SECURITY_PROVIDER,
                    "LDAP connection handler %s could not send a clear-text " +
                    "response to the client because it does not have a " +
                    "reference to a clear connection security provider.");


    registerMessage(MSGID_LDAP_PAGED_RESULTS_DECODE_NULL,

 opends/src/server/org/opends/server/protocols/ldap/LDAPClientConnection.java

@@ -126,6 +126,10 @@
  // The set of all operations currently in progress on this connection.
  private ConcurrentHashMap<Integer,Operation> operationsInProgress;

  // The connection security provider that was in use for the client connection
  // before switching to a TLS-based provider.
  private ConnectionSecurityProvider clearSecurityProvider;

  // The connection security provider for this client connection.
  private ConnectionSecurityProvider securityProvider;

@@ -209,9 +213,10 @@
    assert debugConstructor(CLASS_NAME, String.valueOf(connectionHandler),
                            String.valueOf(clientChannel));

    this.connectionHandler = connectionHandler;
    this.clientChannel     = clientChannel;
    this.securityProvider  = null;
    this.connectionHandler     = connectionHandler;
    this.clientChannel         = clientChannel;
    this.securityProvider      = null;
    this.clearSecurityProvider = null;

    opsInProgressLock = new ReentrantLock();
    transmitLock      = new ReentrantLock();
@@ -561,6 +566,30 @@
  {
    assert debugEnter(CLASS_NAME, "sendResponse", String.valueOf(operation));

    LDAPMessage message = operationToResponseLDAPMessage(operation);
    if (message != null)
    {
      sendLDAPMessage(securityProvider, message);
    }
  }



  /**
   * Retrieves an LDAPMessage containing a response generated from the provided
   * operation.
   *
   * @param  operation  The operation to use to generate the response
   *                    LDAPMessage.
   *
   * @return  An LDAPMessage containing a response generated from the provided
   *          operation.
   */
  private LDAPMessage operationToResponseLDAPMessage(Operation operation)
  {
    assert debugEnter(CLASS_NAME, "operationToResponseLDAPMessage",
                      String.valueOf(operation));

    ResultCode resultCode = operation.getResultCode();
    if (resultCode == null)
    {
@@ -646,7 +675,7 @@
                   ErrorLogSeverity.MILD_WARNING,
                   MSGID_LDAPV2_SKIPPING_EXTENDED_RESPONSE,
                   getConnectionID(), String.valueOf(operation));
          return;
          return null;
        }

        ExtendedOperation extOp = (ExtendedOperation) operation;
@@ -675,7 +704,7 @@
        logError(ErrorLogCategory.REQUEST_HANDLING, ErrorLogSeverity.MILD_ERROR,
                 MSGID_LDAP_CLIENT_SEND_RESPONSE_INVALID_OP,
                 String.valueOf(operation));
        return;
        return null;
    }


@@ -702,9 +731,7 @@
      }
    }


    sendLDAPMessage(new LDAPMessage(operation.getMessageID(), protocolOp,
                                    controls));
    return new LDAPMessage(operation.getMessageID(), protocolOp, controls);
  }


@@ -741,7 +768,8 @@
      }
    }

    sendLDAPMessage(new LDAPMessage(searchOperation.getMessageID(), protocolOp,
    sendLDAPMessage(securityProvider,
                    new LDAPMessage(searchOperation.getMessageID(), protocolOp,
                                    controls));
  }

@@ -801,7 +829,8 @@
      }
    }

    sendLDAPMessage(new LDAPMessage(searchOperation.getMessageID(), protocolOp,
    sendLDAPMessage(securityProvider,
                    new LDAPMessage(searchOperation.getMessageID(), protocolOp,
                                    controls));
    return true;
  }
@@ -840,7 +869,7 @@

    LDAPMessage message = new LDAPMessage(operation.getMessageID(), protocolOp,
                                          ldapControls);
    sendLDAPMessage(message);
    sendLDAPMessage(securityProvider, message);


    // The only reason we shouldn't continue processing is if the connection is
@@ -853,9 +882,12 @@
  /**
   * Sends the provided LDAP message to the client.
   *
   * @param  message  The LDAP message to send to the client.
   * @param  securityProvider  The connection security provider to use to
   *                           handle any necessary security translation.
   * @param  message           The LDAP message to send to the client.
   */
  private void sendLDAPMessage(LDAPMessage message)
  private void sendLDAPMessage(ConnectionSecurityProvider secProvider,
                               LDAPMessage message)
  {
    assert debugEnter(CLASS_NAME, "sendLDAPMessage", String.valueOf(message));

@@ -873,7 +905,7 @@
      try
      {
        int bytesWritten = messageBuffer.limit() - messageBuffer.position();
        if (! securityProvider.writeData(messageBuffer))
        if (! secProvider.writeData(messageBuffer))
        {
          return;
        }
@@ -1837,7 +1869,8 @@
           new AddResponseProtocolOp(de.getResultCode().getIntValue(),
                                     de.getErrorMessage(), de.getMatchedDN(),
                                     de.getReferralURLs());
      sendLDAPMessage(new LDAPMessage(message.getMessageID(), responseOp));
      sendLDAPMessage(securityProvider,
                      new LDAPMessage(message.getMessageID(), responseOp));
    }


@@ -1876,7 +1909,8 @@
           new BindResponseProtocolOp(
                    LDAPResultCode.INAPPROPRIATE_AUTHENTICATION,
                    getMessage(MSGID_LDAPV2_CLIENTS_NOT_ALLOWED));
      sendLDAPMessage(new LDAPMessage(message.getMessageID(), responseOp));
      sendLDAPMessage(securityProvider,
                      new LDAPMessage(message.getMessageID(), responseOp));
      disconnect(DisconnectReason.PROTOCOL_ERROR, false, null, -1);
      return false;
    }
@@ -1923,7 +1957,8 @@
           new BindResponseProtocolOp(de.getResultCode().getIntValue(),
                                      de.getErrorMessage(), de.getMatchedDN(),
                                      de.getReferralURLs());
      sendLDAPMessage(new LDAPMessage(message.getMessageID(), responseOp));
      sendLDAPMessage(securityProvider,
                      new LDAPMessage(message.getMessageID(), responseOp));

      // If it was a protocol error, then terminate the connection.
      if (de.getResultCode() == ResultCode.PROTOCOL_ERROR)
@@ -1983,7 +2018,8 @@
                                         de.getErrorMessage(),
                                         de.getMatchedDN(),
                                         de.getReferralURLs());
      sendLDAPMessage(new LDAPMessage(message.getMessageID(), responseOp));
      sendLDAPMessage(securityProvider,
                      new LDAPMessage(message.getMessageID(), responseOp));
    }


@@ -2032,7 +2068,8 @@
           new DeleteResponseProtocolOp(de.getResultCode().getIntValue(),
                                        de.getErrorMessage(), de.getMatchedDN(),
                                        de.getReferralURLs());
      sendLDAPMessage(new LDAPMessage(message.getMessageID(), responseOp));
      sendLDAPMessage(securityProvider,
                      new LDAPMessage(message.getMessageID(), responseOp));
    }


@@ -2103,7 +2140,8 @@
                                          de.getErrorMessage(),
                                          de.getMatchedDN(),
                                          de.getReferralURLs());
      sendLDAPMessage(new LDAPMessage(message.getMessageID(), responseOp));
      sendLDAPMessage(securityProvider,
                      new LDAPMessage(message.getMessageID(), responseOp));
    }


@@ -2152,7 +2190,8 @@
           new ModifyResponseProtocolOp(de.getResultCode().getIntValue(),
                                        de.getErrorMessage(), de.getMatchedDN(),
                                        de.getReferralURLs());
      sendLDAPMessage(new LDAPMessage(message.getMessageID(), responseOp));
      sendLDAPMessage(securityProvider,
                      new LDAPMessage(message.getMessageID(), responseOp));
    }


@@ -2205,7 +2244,8 @@
                                          de.getErrorMessage(),
                                          de.getMatchedDN(),
                                          de.getReferralURLs());
      sendLDAPMessage(new LDAPMessage(message.getMessageID(), responseOp));
      sendLDAPMessage(securityProvider,
                      new LDAPMessage(message.getMessageID(), responseOp));
    }


@@ -2260,7 +2300,8 @@
                                          de.getErrorMessage(),
                                          de.getMatchedDN(),
                                          de.getReferralURLs());
      sendLDAPMessage(new LDAPMessage(message.getMessageID(), responseOp));
      sendLDAPMessage(securityProvider,
                      new LDAPMessage(message.getMessageID(), responseOp));
    }


@@ -2427,7 +2468,8 @@
                                   message, msgID);
    }

    securityProvider = tlsSecurityProvider;
    clearSecurityProvider = securityProvider;
    securityProvider      = tlsSecurityProvider;
  }


@@ -2455,5 +2497,38 @@
    throw new DirectoryException(DirectoryServer.getServerErrorResultCode(),
                                 message, msgID);
  }



  /**
   * Sends a response to the client in the clear rather than through the
   * encrypted channel.  This should only be used when processing the StartTLS
   * extended operation to send the response in the clear after the TLS
   * negotiation has already been initiated.
   *
   * @param  operation  The operation for which to send the response in the
   *                    clear.
   *
   *
   * @throws  DirectoryException  If a problem occurs while sending the response
   *                              in the clear.
   */
  public void sendClearResponse(Operation operation)
         throws DirectoryException
  {
    assert debugEnter(CLASS_NAME, "sendClearResponse",
                      String.valueOf(operation));

    if (clearSecurityProvider == null)
    {
      int    msgID   = MSGID_LDAP_NO_CLEAR_SECURITY_PROVIDER;
      String message = getMessage(msgID, toString());
      throw new DirectoryException(DirectoryServer.getServerErrorResultCode(),
                                   message, msgID);
    }

    sendLDAPMessage(clearSecurityProvider,
                    operationToResponseLDAPMessage(operation));
  }
}