From f2bcf31dabb8f69261b0b829fc989e9ba5323ee6 Mon Sep 17 00:00:00 2001
From: neil_a_wilson <neil_a_wilson@localhost>
Date: Mon, 25 Sep 2006 23:14:21 +0000
Subject: [PATCH] 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/src/server/org/opends/server/protocols/ldap/LDAPClientConnection.java |  123 +++++++++++++++++++++++++++++++++--------
 1 files changed, 99 insertions(+), 24 deletions(-)

diff --git a/opends/src/server/org/opends/server/protocols/ldap/LDAPClientConnection.java b/opends/src/server/org/opends/server/protocols/ldap/LDAPClientConnection.java
index d3cf3e1..1c03ae9 100644
--- a/opends/src/server/org/opends/server/protocols/ldap/LDAPClientConnection.java
+++ b/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));
+  }
 }
 

--
Gitblit v1.10.0