Re: [boost] [asio] Modified SSL client to reproduce error
OK, this is almost certainly not going to work :)
My apologies Chris. You are absolutely right and only proves I need to work on example programs in the morning after a good night's sleep! :) Our software does have appropriate locking and works fine (outside of SSL). The thread code I wrote in the example client was terrible. I took a new direction with the client/server example. I removed the thread and rewrote the client and server to pass ever increasing message sizes. In the beginning, everything works fine. But once messages start getting over 8k in size, the server eventually just locks up. If I keep the messages small, the client/server seems to run continually without problems. BTW, I am using a version of the boost asio implementation you are working on. I believe you send me a link to it back in May or June for downloading. This version is actually in the "boost::asio" namespace and not just "asio". If there's a newer version I should use, please let me know. Thanks, Scott
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
participants (2)
-
Christopher Kohlhoff -
Scott