Re: [boost] [asio] Modified SSL client to reproduce error

From: Christopher Kohlhoff (chris_at_[hidden])
Date: 2006-07-21 02:10:30

Hi Scott and Viktor,

After some time spent debugging and trying to figure out the
ssl::stream code, I think have found the problem (actually there
were two):

- A read operation should check the read buffer first to see if
  there's any data present and use that first, before issuing a
  new read from the socket.

- The read buffer is a member of the openssl_operation class,
  and a new openssl_operation object is created for each read or
  write initiated by the application. This means that any
  surplus data from a previous read is lost.

Can you please try the diffs included below and let me know how
it goes. Thanks!

Cheers,
Chris

Index: asio/ssl/detail/openssl_operation.hpp
===================================================================
RCS file: /cvsroot/asio/asio/include/asio/ssl/detail/openssl_operation.hpp,v
retrieving revision 1.9
diff -u -r1.9 openssl_operation.hpp
--- asio/ssl/detail/openssl_operation.hpp    28 May 2006 06:54:42 -0000    1.9
+++ asio/ssl/detail/openssl_operation.hpp    21 Jul 2006 05:31:07 -0000
@@ -82,12 +82,14 @@
   // Constructor for asynchronous operations
   openssl_operation(ssl_primitive_func primitive,
                     Stream& socket,
+                    net_buffer& recv_buf,
                     SSL* session,
                     BIO* ssl_bio,
                     user_handler_func  handler
                     )
     : primitive_(primitive)
     , user_handler_(handler)
+    , recv_buf_(recv_buf)
     , socket_(socket)
     , ssl_bio_(ssl_bio)
     , session_(session)
@@ -105,9 +107,11 @@
   // Constructor for synchronous operations
   openssl_operation(ssl_primitive_func primitive,
                     Stream& socket,
+                    net_buffer& recv_buf,
                     SSL* session,
                     BIO* ssl_bio)
     : primitive_(primitive)
+    , recv_buf_(recv_buf)
     , socket_(socket)
     , ssl_bio_(ssl_bio)
     , session_(session)
@@ -165,6 +169,36 @@
       // not want network communication nor does want to send shutdown out...
       return handler_(asio::error(error_code), rc); 
 
+    if (!is_operation_done && !is_write_needed)
+    {
+      // We may have left over data that we can pass to SSL immediately
+      if (recv_buf_.get_data_len() > 0)
+      {
+        // Pass the buffered data to SSL
+        int written = ::BIO_write
+        ( 
+          ssl_bio_, 
+          recv_buf_.get_data_start(), 
+          recv_buf_.get_data_len() 
+        );
+
+        if (written > 0)
+        {
+          recv_buf_.data_removed(written);
+        }
+        else if (written < 0)
+        {
+          if (!BIO_should_retry(ssl_bio_))
+          {
+            // Some serios error with BIO....
+            return handler_(asio::error(asio::error::no_recovery), 0);
+          }
+        }
+
+        return start();
+      }
+    }
+
     // Continue with operation, flush any SSL data out to network...
     return write_(is_operation_done, rc); 
   }
@@ -181,7 +215,12 @@
   int_handler_func handler_;
     
   net_buffer send_buf_; // buffers for network IO
-  net_buffer recv_buf_; 
+
+  // The recv buffer is owned by the stream, not the operation, since there can
+  // be left over bytes after passing the data up to the application, and these
+  // bytes need to be kept around for the next read operation issued by the
+  // application.
+  net_buffer& recv_buf_;
 
   Stream& socket_;
   BIO*    ssl_bio_;
Index: asio/ssl/detail/openssl_stream_service.hpp
===================================================================
RCS file: /cvsroot/asio/asio/include/asio/ssl/detail/openssl_stream_service.hpp,v
retrieving revision 1.11
diff -u -r1.11 openssl_stream_service.hpp
--- asio/ssl/detail/openssl_stream_service.hpp    16 Jun 2006 11:52:28 -0000    1.11
+++ asio/ssl/detail/openssl_stream_service.hpp    21 Jul 2006 05:41:34 -0000
@@ -152,6 +152,7 @@
   {
     ::SSL* ssl;
     ::BIO* ext_bio;
+    net_buffer recv_buf;
   } * impl_type;
 
   // Construct a new stream socket service for the specified io_service.
@@ -179,6 +180,7 @@
     impl = new impl_struct;
     impl->ssl = ::SSL_new(context.impl());
     ::SSL_set_mode(impl->ssl, SSL_MODE_ENABLE_PARTIAL_WRITE);
+    ::SSL_set_mode(impl->ssl, SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER);
     ::BIO* int_bio = 0;
     impl->ext_bio = 0;
     ::BIO_new_bio_pair(&int_bio, 8192, &impl->ext_bio, 8192);
@@ -240,6 +242,7 @@
         &ssl_wrap<mutex_type>::SSL_connect:
         &ssl_wrap<mutex_type>::SSL_accept,
       next_layer,
+      impl->recv_buf,
       impl->ssl,
       impl->ext_bio,
       boost::bind
@@ -265,6 +268,7 @@
       openssl_operation<Stream> op(
         &ssl_wrap<mutex_type>::SSL_shutdown,
         next_layer,
+        impl->recv_buf,
         impl->ssl,
         impl->ext_bio);
       op.start();
@@ -292,6 +296,7 @@
     (
       &ssl_wrap<mutex_type>::SSL_shutdown,
       next_layer,
+      impl->recv_buf,
       impl->ssl,
       impl->ext_bio,
       boost::bind
@@ -322,6 +327,7 @@
       openssl_operation<Stream> op(
         send_func,
         next_layer,
+        impl->recv_buf,
         impl->ssl,
         impl->ext_bio
       );
@@ -356,6 +362,7 @@
     (
       send_func,
       next_layer,
+      impl->recv_buf,
       impl->ssl,
       impl->ext_bio,
       boost::bind
@@ -385,6 +392,7 @@
             asio::buffer_size(*buffers.begin()));
       openssl_operation<Stream> op(recv_func,
         next_layer,
+        impl->recv_buf,
         impl->ssl,
         impl->ext_bio
       );
@@ -420,6 +428,7 @@
     (
       recv_func,
       next_layer,
+      impl->recv_buf,
       impl->ssl,
       impl->ext_bio,
       boost::bind