ChangeLogTag:Fri Sep 28 12:14:29 2001 Ossama Othman <ossama@uci.edu>

8 files changed, 207 insertions, 24 deletions

diff --git a/ChangeLog b/ChangeLog
index 64283716268..0cc0a1ebd47 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,30 @@
+Fri Sep 28 12:14:29 2001  Ossama Othman  <ossama@uci.edu>
+
+	* ace/SSL/SSL_SOCK_Acceptor.cpp (ssl_accept):
+
+	  Transfer ownership of the Reactor to the previous owner for the
+	  SSL_ERROR_ZERO_RETURN case.  Thanks to Vladimir Chovanec
+	  <Vladimir.CHOVANEC@asset.sk> for pointing out this problem.
+
+	  Remove the SSL event handler from the reactor for the
+	  SSL_ERROR_ZERO_RETURN case.  Previously, this method returned
+	  without removing it, resulting in a seg fault.
+
+	* ace/SSL/SSL_SOCK_Connector.cpp (ssl_connect): 
+
+	  Ported the updates made to the ssl_accept() counterpart to this
+	  method.
+
+	* ace/SSL/SSL_Accept_Handler.cpp (ssl_accept):
+	* ace/SSL/SSL_Connect_Handler.cpp (ssl_connect):
+	* ace/SSL/SSL_SOCK_Stream.i (send_i):
+
+	  In the event a SSL_ERROR_SYSCALL error condition occurs, call
+	  ACE_OS::set_errno_to_last_error() to make sure errno is updated
+	  appropriately since OpenSSL does not do that.  Fixes some
+	  "misbehaving" Reactor interactions.  Thanks to Andrew Finnell
+	  <Andrew.Finnell@esecurityinc.com> for suggesting these fixes.
+
 Fri Sep 28 09:37:28 2001  Ossama Othman  <ossama@uci.edu>
 
 	* ace/SSL/SSL_Context.h (report_error):
diff --git a/ChangeLogs/ChangeLog-02a b/ChangeLogs/ChangeLog-02a
index 64283716268..0cc0a1ebd47 100644
--- a/ChangeLogs/ChangeLog-02a
+++ b/ChangeLogs/ChangeLog-02a
@@ -1,3 +1,30 @@
+Fri Sep 28 12:14:29 2001  Ossama Othman  <ossama@uci.edu>
+
+	* ace/SSL/SSL_SOCK_Acceptor.cpp (ssl_accept):
+
+	  Transfer ownership of the Reactor to the previous owner for the
+	  SSL_ERROR_ZERO_RETURN case.  Thanks to Vladimir Chovanec
+	  <Vladimir.CHOVANEC@asset.sk> for pointing out this problem.
+
+	  Remove the SSL event handler from the reactor for the
+	  SSL_ERROR_ZERO_RETURN case.  Previously, this method returned
+	  without removing it, resulting in a seg fault.
+
+	* ace/SSL/SSL_SOCK_Connector.cpp (ssl_connect): 
+
+	  Ported the updates made to the ssl_accept() counterpart to this
+	  method.
+
+	* ace/SSL/SSL_Accept_Handler.cpp (ssl_accept):
+	* ace/SSL/SSL_Connect_Handler.cpp (ssl_connect):
+	* ace/SSL/SSL_SOCK_Stream.i (send_i):
+
+	  In the event a SSL_ERROR_SYSCALL error condition occurs, call
+	  ACE_OS::set_errno_to_last_error() to make sure errno is updated
+	  appropriately since OpenSSL does not do that.  Fixes some
+	  "misbehaving" Reactor interactions.  Thanks to Andrew Finnell
+	  <Andrew.Finnell@esecurityinc.com> for suggesting these fixes.
+
 Fri Sep 28 09:37:28 2001  Ossama Othman  <ossama@uci.edu>
 
 	* ace/SSL/SSL_Context.h (report_error):
diff --git a/ChangeLogs/ChangeLog-03a b/ChangeLogs/ChangeLog-03a
index 64283716268..0cc0a1ebd47 100644
--- a/ChangeLogs/ChangeLog-03a
+++ b/ChangeLogs/ChangeLog-03a
@@ -1,3 +1,30 @@
+Fri Sep 28 12:14:29 2001  Ossama Othman  <ossama@uci.edu>
+
+	* ace/SSL/SSL_SOCK_Acceptor.cpp (ssl_accept):
+
+	  Transfer ownership of the Reactor to the previous owner for the
+	  SSL_ERROR_ZERO_RETURN case.  Thanks to Vladimir Chovanec
+	  <Vladimir.CHOVANEC@asset.sk> for pointing out this problem.
+
+	  Remove the SSL event handler from the reactor for the
+	  SSL_ERROR_ZERO_RETURN case.  Previously, this method returned
+	  without removing it, resulting in a seg fault.
+
+	* ace/SSL/SSL_SOCK_Connector.cpp (ssl_connect): 
+
+	  Ported the updates made to the ssl_accept() counterpart to this
+	  method.
+
+	* ace/SSL/SSL_Accept_Handler.cpp (ssl_accept):
+	* ace/SSL/SSL_Connect_Handler.cpp (ssl_connect):
+	* ace/SSL/SSL_SOCK_Stream.i (send_i):
+
+	  In the event a SSL_ERROR_SYSCALL error condition occurs, call
+	  ACE_OS::set_errno_to_last_error() to make sure errno is updated
+	  appropriately since OpenSSL does not do that.  Fixes some
+	  "misbehaving" Reactor interactions.  Thanks to Andrew Finnell
+	  <Andrew.Finnell@esecurityinc.com> for suggesting these fixes.
+
 Fri Sep 28 09:37:28 2001  Ossama Othman  <ossama@uci.edu>
 
 	* ace/SSL/SSL_Context.h (report_error):
diff --git a/ace/SSL/SSL_Accept_Handler.cpp b/ace/SSL/SSL_Accept_Handler.cpp
index d05971d6659..558abdd9df7 100644
--- a/ace/SSL/SSL_Accept_Handler.cpp
+++ b/ace/SSL/SSL_Accept_Handler.cpp
@@ -95,6 +95,20 @@ ACE_SSL_Accept_Handler::ssl_accept (void)
         case SSL_ERROR_WANT_READ:
           break;
 
+        case SSL_ERROR_ZERO_RETURN:
+          // The peer has notified us that it is shutting down via
+          // the SSL "close_notify" message so we need to
+          // shutdown, too.
+          //
+          // Removing this event handler causes the SSL stream to be
+          // shutdown.
+          return -1;
+
+        case SSL_ERROR_SYSCALL:
+          // On some platforms (e.g. MS Windows) OpenSSL does not
+          // store the last error in errno so explicitly do so.
+          ACE_OS::set_errno_to_last_error ();
+
         default:
           ACE_SSL_Context::report_error ();
 
diff --git a/ace/SSL/SSL_Connect_Handler.cpp b/ace/SSL/SSL_Connect_Handler.cpp
index c64d6a48c71..6bc31382a1f 100644
--- a/ace/SSL/SSL_Connect_Handler.cpp
+++ b/ace/SSL/SSL_Connect_Handler.cpp
@@ -80,6 +80,20 @@ ACE_SSL_Connect_Handler::ssl_connect (void)
         case SSL_ERROR_WANT_READ:
           break;
 
+        case SSL_ERROR_ZERO_RETURN:
+          // The peer has notified us that it is shutting down via
+          // the SSL "close_notify" message so we need to
+          // shutdown, too.
+          //
+          // Removing this event handler causes the SSL stream to be
+          // shutdown.
+          return -1;
+
+        case SSL_ERROR_SYSCALL:
+          // On some platforms (e.g. MS Windows) OpenSSL does not
+          // store the last error in errno so explicitly do so.
+          ACE_OS::set_errno_to_last_error ();
+
         default:
           ACE_SSL_Context::report_error ();
 
diff --git a/ace/SSL/SSL_SOCK_Acceptor.cpp b/ace/SSL/SSL_SOCK_Acceptor.cpp
index 8ffb8c5deca..2c52ce3666a 100644
--- a/ace/SSL/SSL_SOCK_Acceptor.cpp
+++ b/ace/SSL/SSL_SOCK_Acceptor.cpp
@@ -146,8 +146,7 @@ ACE_SSL_SOCK_Acceptor::ssl_accept (ACE_SSL_SOCK_Stream &new_stream,
     ACE_Event_Handler::WRITE_MASK;
 
   // In case a thread other than the one running the Reactor event
-  // loop
-  // performs the passive SSL connection establishment, transfer
+  // loop performs the passive SSL connection establishment, transfer
   // ownership of the Reactor to the current thread.  Control will be
   // passed back to the previous owner when accepting or rejecting the
   // passive SSL connection.
@@ -199,7 +198,11 @@ ACE_SSL_SOCK_Acceptor::ssl_accept (ACE_SSL_SOCK_Stream &new_stream,
           // The peer has notified us that it is shutting down via
           // the SSL "close_notify" message so we need to
           // shutdown, too.
-          (void) new_stream.close ();
+          //
+          // Removing the event handler from the Reactor causes the
+          // SSL stream to be shutdown.
+          (void) this->reactor_->remove_handler (&eh, reactor_mask);
+          (void) this->reactor_->owner (old_owner);
 
           return -1;
 
diff --git a/ace/SSL/SSL_SOCK_Connector.cpp b/ace/SSL/SSL_SOCK_Connector.cpp
index 12cc72d0d66..4c7f95e888d 100644
--- a/ace/SSL/SSL_SOCK_Connector.cpp
+++ b/ace/SSL/SSL_SOCK_Connector.cpp
@@ -53,25 +53,29 @@ int
 ACE_SSL_SOCK_Connector::ssl_connect (ACE_SSL_SOCK_Stream &new_stream,
                                      const ACE_Time_Value *max_wait_time)
 {
-  if (SSL_is_init_finished (new_stream.ssl ()))
+  SSL *ssl = new_stream.ssl ();
+
+  if (SSL_is_init_finished (ssl))
     return 0;
 
   // Check if a connection is already pending for the given SSL
   // structure.
-  if (!SSL_in_connect_init (new_stream.ssl ()))
-    ::SSL_set_connect_state (new_stream.ssl ());
+  if (!SSL_in_connect_init (ssl))
+    ::SSL_set_connect_state (ssl);
 
   // Register an event handler to complete the non-blocking SSL
   // connect.  A specialized event handler is necessary since since
   // the ACE Connector strategies are not designed for protocols
   // that require additional handshakes after the initial connect.
   ACE_SSL_Connect_Handler eh (new_stream);
+  ACE_Reactor_Mask reactor_mask =
+    ACE_Event_Handler::READ_MASK |
+    ACE_Event_Handler::WRITE_MASK;
 
   if (this->reactor_->register_handler (
         new_stream.get_handle (),
         &eh,
-        ACE_Event_Handler::READ_MASK |
-        ACE_Event_Handler::WRITE_MASK) == -1)
+        reactor_mask) == -1)
     return -1;
 
   ACE_Time_Value tv;
@@ -80,28 +84,82 @@ ACE_SSL_SOCK_Connector::ssl_connect (ACE_SSL_SOCK_Stream &new_stream,
 
   ACE_Time_Value *timeout = (max_wait_time == 0 ? 0 : &tv);
 
-  // Make the current thread take ownership of the Reactor.
-  this->reactor_->owner (ACE_Thread::self ());
+  // In case a thread other than the one running the Reactor event
+  // loop performs the passive SSL connection establishment, transfer
+  // ownership of the Reactor to the current thread.  Control will be
+  // passed back to the previous owner when accepting or rejecting the
+  // passive SSL connection.
+  ACE_thread_t old_owner;
+
+  if (this->reactor_->owner (ACE_Thread::self (),
+                             &old_owner) != 0)
+    return -1;  // Failed to transfer ownership!  Should never happen!
 
   // Have the Reactor complete the SSL active connection.  Run the
   // event loop until the active connection is completed.  Since
   // the Reactor is used, this isn't a busy wait.
-  while (SSL_in_connect_init (new_stream.ssl ()))
-    if (this->reactor_->handle_events (timeout) == -1)
-      {
-        reactor_->remove_handler (&eh,
-                                  ACE_Event_Handler::READ_MASK |
-                                  ACE_Event_Handler::WRITE_MASK);
-        return -1;
-      }
+  while (SSL_in_connect_init (ssl))
+    {
+      // Before blocking in the Reactor, do an SSL_connect() in case
+      // OpenSSL buffered additional data sent within an SSL record
+      // during session negotiation.  The buffered data must be
+      // handled prior to entering the Reactor event loop since the
+      // Reactor may end up waiting indefinitely for data that has
+      // already arrived.
+      int status = ::SSL_connect (ssl);
+
+      switch (::SSL_get_error (ssl, status))
+        {
+        case SSL_ERROR_NONE:
+          break;
+
+        case SSL_ERROR_WANT_WRITE:
+        case SSL_ERROR_WANT_READ:
+          // No data buffered by OpenSSL, so wait for data in the
+          // Reactor.
+          if (this->reactor_->handle_events (timeout) == -1
+              || new_stream.get_handle () == ACE_INVALID_HANDLE)
+            {
+              (void) this->reactor_->remove_handler (&eh, reactor_mask);
+              (void) this->reactor_->owner (old_owner);
+              return -1;
+            }
+
+          break;
+
+        case SSL_ERROR_ZERO_RETURN:
+          // The peer has notified us that it is shutting down via
+          // the SSL "close_notify" message so we need to
+          // shutdown, too.
+          //
+          // Removing the event handler from the Reactor causes the
+          // SSL stream to be shutdown.
+          (void) this->reactor_->remove_handler (&eh, reactor_mask);
+          (void) this->reactor_->owner (old_owner);
+
+          return -1;
+
+        default:
+
+#ifndef ACE_NDEBUG
+          //ERR_print_errors_fp (stderr);
+#endif  /* ACE_NDEBUG */
+
+          (void) this->reactor_->remove_handler (&eh, reactor_mask);
+          (void) this->reactor_->owner (old_owner);
+
+          return -1;
+        }
+    }
 
   // SSL active connection was completed.  Deregister the event
-  // handler from the Reactor.
-  return
-    this->reactor_->remove_handler (&eh,
-                                    ACE_Event_Handler::READ_MASK |
-                                    ACE_Event_Handler::WRITE_MASK |
-                                    ACE_Event_Handler::DONT_CALL);
+  // handler from the Reactor, but don't close it.
+  (void) this->reactor_->remove_handler (&eh,
+                                         reactor_mask |
+                                         ACE_Event_Handler::DONT_CALL);
+
+  // Transfer control of the Reactor to the previous owner.
+  return this->reactor_->owner (old_owner);
 }
 
 int
diff --git a/ace/SSL/SSL_SOCK_Stream.i b/ace/SSL/SSL_SOCK_Stream.i
index 8f07eb26e6f..9553e326e3b 100644
--- a/ace/SSL/SSL_SOCK_Stream.i
+++ b/ace/SSL/SSL_SOCK_Stream.i
@@ -81,6 +81,19 @@ ACE_SSL_SOCK_Stream::send_i (const void *buf,
           (void) ::SSL_shutdown (this->ssl_);
           return bytes_sent;
 
+        case SSL_ERROR_SYSCALL:
+          if (bytes_sent == 0)
+            // An EOF occured but the SSL "close_notify" message was
+            // not sent.  This is a protocol error, but we ignore it.
+            return 0;
+
+          // If not an EOF, then fall through to "default" case.
+
+          // On some platforms (e.g. MS Windows) OpenSSL does not
+          // store the last error in errno so explicitly do so.
+
+          ACE_OS::set_errno_to_last_error ();
+
         default:
           ACE_SSL_Context::report_error ();