From 1f623d8ccac1cabcb223b6a5d907a90d05918403 Mon Sep 17 00:00:00 2001 From: Hannes Rantzsch Date: Wed, 10 Apr 2019 15:00:55 +0200 Subject: documentation and minor fixes for async ops --- src/lib/tls/asio/asio_async_base.h | 47 ++++++++++++++- src/lib/tls/asio/asio_async_handshake_op.h | 29 +++++---- src/lib/tls/asio/asio_async_read_op.h | 15 ++++- src/lib/tls/asio/asio_async_write_op.h | 95 +++++++++++++++++------------- src/lib/tls/asio/asio_stream.h | 32 +++++----- src/lib/tls/asio/asio_stream_base.h | 3 +- src/lib/tls/asio/asio_stream_core.h | 8 +-- 7 files changed, 149 insertions(+), 80 deletions(-) (limited to 'src/lib') diff --git a/src/lib/tls/asio/asio_async_base.h b/src/lib/tls/asio/asio_async_base.h index 8e74c5647..23707af5b 100644 --- a/src/lib/tls/asio/asio_async_base.h +++ b/src/lib/tls/asio/asio_async_base.h @@ -21,6 +21,40 @@ namespace Botan { namespace TLS { +/** + * Base class for asynchronous stream operations. + * + * Asynchronous operations, used for example to implement an interface for boost::asio::async_read_some and + * boost::asio::async_write_some, are based on boost::asio::coroutines. + * Derived operations should implement a call operator and invoke it with the correct parameters upon construction. The + * call operator needs to make sure that the user-provided handler is not called directly. Typically, yield / reenter is + * used for this in the following fashion: + * + * ``` + * void operator()(boost::system::error_code ec, std::size_t bytes_transferred, bool isContinuation = true) + * { + * reenter(this) + * { + * // operation specific logic, repeatedly interacting with the stream_core and the next_layer (socket) + * + * // make sure intermediate initiating function is called + * if(!isContinuation) + * { + * yield next_layer.async_operation(empty_buffer, this); + * } + * + * // call the completion handler + * complete_now(error_code, bytes_transferred); + * } + * } + * ``` + * + * Once the operation is completed and ready to call the completion handler it checks if an intermediate initiating + * function has been called using the `isContinuation` parameter. If not, it will call an asynchronous operation, such + * as `async_read_some`, with and empty buffer, set the object itself as the handler, and `yield`. As a result, the call + * operator will be invoked again, this time as a continuation, and will jump to the location where it yielded before + * using `reenter`. It is now safe to call the handler function via `complete_now`. + */ template struct AsyncBase : boost::asio::coroutine { @@ -45,6 +79,13 @@ struct AsyncBase : boost::asio::coroutine { } + /** + * Call the completion handler. + * + * This function should only be called after an intermediate initiating function has been called. + * + * @param args Arguments forwarded to the completion handler function. + */ template void complete_now(Args&& ... args) { @@ -55,8 +96,10 @@ struct AsyncBase : boost::asio::coroutine Handler m_handler; boost::asio::executor_work_guard m_work_guard_1; }; -} -} + +} // namespace TLS + +} // namespace Botan #endif // BOOST_VERSION #endif // BOTAN_HAS_TLS && BOTAN_HAS_BOOST_ASIO diff --git a/src/lib/tls/asio/asio_async_handshake_op.h b/src/lib/tls/asio/asio_async_handshake_op.h index d633fd127..a392fd649 100644 --- a/src/lib/tls/asio/asio_async_handshake_op.h +++ b/src/lib/tls/asio/asio_async_handshake_op.h @@ -28,6 +28,14 @@ namespace TLS { template > struct AsyncHandshakeOperation : public AsyncBase { + /** + * Construct and invoke an AsyncHandshakeOperation. + * + * @param handler Handler function to be called upon completion. + * @param stream The stream from which the data will be read + * @param core The stream's core; used to extract decrypted data. + * @param ec Optional error code; used to report an error to the handler function. + */ template AsyncHandshakeOperation( HandlerT&& handler, @@ -49,8 +57,8 @@ struct AsyncHandshakeOperation : public AsyncBase 0) + // Provide TLS data from the core to the TLS::Channel + if(bytesTransferred > 0 && !ec) { boost::asio::const_buffer read_buffer {m_core.input_buffer.data(), bytesTransferred}; try @@ -63,13 +71,13 @@ struct AsyncHandshakeOperation : public AsyncBase 0` assumes that - // `bytesTransferred` bytes were just read and are in the cores input_buffer for further processing. + // Note: we construct `AsyncWriteOperation` with 0 as its last parameter (`plainBytesTransferred`). + // This operation will eventually call `*this` as its own handler, passing the 0 back to this call + // operator. This is necessary because the check of `bytesTransferred > 0` assumes that + // `bytesTransferred` bytes were just read and are in the core's input_buffer for further processing. AsyncWriteOperation< AsyncHandshakeOperation::type, Stream, Allocator>, Stream, @@ -78,7 +86,7 @@ struct AsyncHandshakeOperation : public AsyncBaseis_active() && !ec) { m_stream.next_layer().async_read_some(m_core.input_buffer, std::move(*this)); @@ -87,9 +95,8 @@ struct AsyncHandshakeOperation : public AsyncBase> struct AsyncReadOperation : public AsyncBase { + /** + * Construct and invoke an AsyncWriteOperation. + * + * @param handler Handler function to be called upon completion. + * @param stream The stream from which the data will be read + * @param core The stream's core; used to extract decrypted data. + * @param buffers The buffers into which the data will be read. + * @param ec Optional error code; used to report an error to the handler function. + */ template AsyncReadOperation(HandlerT&& handler, Stream& stream, @@ -42,7 +51,7 @@ struct AsyncReadOperation : public AsyncBaseoperator()(ec, m_decodedBytes, false); + this->operator()(ec, std::size_t(0), false); } AsyncReadOperation(AsyncReadOperation&&) = default; @@ -82,7 +91,7 @@ struct AsyncReadOperation : public AsyncBase> struct AsyncWriteOperation : public AsyncBase { - template - AsyncWriteOperation(HandlerT&& handler, - Stream& stream, - StreamCore& core, - std::size_t plainBytesTransferred, - const boost::system::error_code& ec = {}) - : AsyncBase( - std::forward(handler), - stream.get_executor()) - , m_stream(stream) - , m_core(core) - , m_plainBytesTransferred(plainBytesTransferred) - { - this->operator()(ec, std::size_t(0), false); - } - - AsyncWriteOperation(AsyncWriteOperation&&) = default; - - void operator()(boost::system::error_code ec, std::size_t bytes_transferred, bool isContinuation = true) - { - reenter(this) + /** + * Construct and invoke an AsyncWriteOperation. + * + * @param handler Handler function to be called upon completion. + * @param stream The stream from which the data will be read + * @param core The stream's core; used to extract decrypted data. + * @param plainBytesTransferred Number of bytes to be reported to the user-provided handler function as + * bytes_transferred. This needs to be provided since the amount of plaintext data + * consumed from the input buffer can differ from the amount of encrypted data written + * to the next layer. + * @param ec Optional error code; used to report an error to the handler function. + */ + template + AsyncWriteOperation(HandlerT&& handler, + Stream& stream, + StreamCore& core, + std::size_t plainBytesTransferred, + const boost::system::error_code& ec = {}) + : AsyncBase( + std::forward(handler), + stream.get_executor()) + , m_stream(stream) + , m_core(core) + , m_plainBytesTransferred(plainBytesTransferred) { - m_core.consumeSendBuffer(bytes_transferred); + this->operator()(ec, std::size_t(0), false); + } - if(m_core.hasDataToSend() && !ec) - { - m_stream.next_layer().async_write_some(m_core.sendBuffer(), std::move(*this)); - return; - } + AsyncWriteOperation(AsyncWriteOperation&&) = default; - if(!isContinuation) + void operator()(boost::system::error_code ec, std::size_t bytes_transferred, bool isContinuation = true) + { + reenter(this) { - m_ec = ec; - yield m_stream.next_layer().async_write_some(boost::asio::const_buffer(), std::move(*this)); - ec = m_ec; + m_core.consumeSendBuffer(bytes_transferred); + + if(m_core.hasDataToSend() && !ec) + { + m_stream.next_layer().async_write_some(m_core.sendBuffer(), std::move(*this)); + return; + } + + if(!isContinuation) + { + // Make sure the handler is not called without an intermediate initiating function. + // "Writing" to a zero-byte buffer will complete immediately. + m_ec = ec; + yield m_stream.next_layer().async_write_some(boost::asio::const_buffer(), std::move(*this)); + ec = m_ec; + } + + // The size of the sent TLS record can differ from the size of the payload due to TLS encryption. We need to + // tell the handler how many bytes of the original data we already processed. + this->complete_now(ec, m_plainBytesTransferred); } - - // the size of the sent TLS record can differ from the size of the payload due to TLS encryption. We need to tell - // the handler how many bytes of the original data we already processed. - this->complete_now(ec, m_plainBytesTransferred); } - } - Stream& m_stream; - StreamCore& m_core; + private: + Stream& m_stream; + StreamCore& m_core; - std::size_t m_plainBytesTransferred; - boost::system::error_code m_ec; + std::size_t m_plainBytesTransferred; + boost::system::error_code m_ec; }; } // namespace TLS diff --git a/src/lib/tls/asio/asio_stream.h b/src/lib/tls/asio/asio_stream.h index c3ed68489..3d9e46ed3 100644 --- a/src/lib/tls/asio/asio_stream.h +++ b/src/lib/tls/asio/asio_stream.h @@ -423,8 +423,7 @@ class Stream : public StreamBase std::size_t write_some(const ConstBufferSequence& buffers, boost::system::error_code& ec) { - std::size_t sent; - sent = tls_encrypt_some(buffers, ec); + std::size_t sent = tls_encrypt_some(buffers, ec); if(ec) { return 0; } @@ -438,6 +437,7 @@ class Stream : public StreamBase /** * Write some data to the stream. The function call will block until one or more bytes of data has been written * successfully, or until an error occurs. + * * @param buffers The data to be written. * @return The number of bytes written. * @throws boost::system::system_error if error occured @@ -453,6 +453,7 @@ class Stream : public StreamBase /** * Start an asynchronous write. The function call always returns immediately. + * * @param buffers The data to be written. * @param handler The handler to be called when the write operation completes. Copies will be made of the handler * as required. The equivalent function signature of the handler must be: @@ -467,12 +468,12 @@ class Stream : public StreamBase boost::asio::async_completion init(handler); - std::size_t sent; boost::system::error_code ec; - sent = tls_encrypt_some(buffers, ec); + std::size_t sent = tls_encrypt_some(buffers, ec); if(ec) { - // we can't be sure how many bytes were commited here, so clear the send_buffer and try again + // we cannot be sure how many bytes were committed here + // so clear the send_buffer and let the AsyncWriteOperation call the handler with the error_code set this->m_core.clearSendBuffer(); Botan::TLS::AsyncWriteOperation::type, Stream> op{std::move(init.completion_handler), *this, this->m_core, std::size_t(0), ec}; @@ -487,11 +488,12 @@ class Stream : public StreamBase /** * Start an asynchronous read. The function call always returns immediately. + * * @param buffers The buffers into which the data will be read. Although the buffers object may be copied as - * necessary, ownership of the underlying buffers is retained by the caller, which must guarantee - * that they remain valid until the handler is called. - * @param handler The handler to be called when the read operation completes. - * The equivalent function signature of the handler must be: + * necessary, ownership of the underlying buffers is retained by the caller, which must guarantee + * that they remain valid until the handler is called. + * @param handler The handler to be called when the read operation completes. The equivalent function signature of + * the handler must be: * void(boost::system::error_code, std::size_t) */ template @@ -504,11 +506,7 @@ class Stream : public StreamBase boost::asio::async_completion init(handler); AsyncReadOperation::type, Stream, MutableBufferSequence> - op{std::move(init.completion_handler), - *this, - this->m_core, - buffers}; - + op{std::move(init.completion_handler), *this, this->m_core, buffers}; return init.result.get(); } @@ -549,7 +547,7 @@ class Stream : public StreamBase { std::size_t sent = 0; // NOTE: This is not asynchronous: it encrypts the data synchronously. - // Only writing on the socket is asynchronous. + // Only writing to the socket is asynchronous. for(auto it = boost::asio::buffer_sequence_begin(buffers); it != boost::asio::buffer_sequence_end(buffers); it++) @@ -578,9 +576,9 @@ class Stream : public StreamBase StreamLayer m_nextLayer; }; -} // TLS +} // namespace TLS -} // namespace Botan +} // namespace Botan #endif // BOOST_VERSION #endif // BOTAN_HAS_TLS && BOTAN_HAS_BOOST_ASIO diff --git a/src/lib/tls/asio/asio_stream_base.h b/src/lib/tls/asio/asio_stream_base.h index adf3b8386..a4da63b26 100644 --- a/src/lib/tls/asio/asio_stream_base.h +++ b/src/lib/tls/asio/asio_stream_base.h @@ -29,8 +29,7 @@ enum handshake_type server }; - -/** \brief Base class for all Botan::TLS::Stream implementations. +/** Base class for all Botan::TLS::Stream implementations. * * This template must be specialized for all the Botan::TLS::Channel to be used. * Currently it only supports the Botan::TLS::Client channel that impersonates diff --git a/src/lib/tls/asio/asio_stream_core.h b/src/lib/tls/asio/asio_stream_core.h index 2c4e44717..b5cba9bce 100644 --- a/src/lib/tls/asio/asio_stream_core.h +++ b/src/lib/tls/asio/asio_stream_core.h @@ -35,14 +35,13 @@ struct StreamCore : public Botan::TLS::Callbacks virtual ~StreamCore() = default; - void tls_emit_data(const uint8_t data[], size_t size) override + void tls_emit_data(const uint8_t data[], std::size_t size) override { m_send_buffer.commit( boost::asio::buffer_copy(m_send_buffer.prepare(size), boost::asio::buffer(data, size))); } - void tls_record_received(uint64_t, const uint8_t data[], - size_t size) override + void tls_record_received(uint64_t, const uint8_t data[], std::size_t size) override { // TODO: It would be nice to avoid this buffer copy. However, we need to deal with the case that the receive // buffer provided by the caller is smaller than the decrypted record. @@ -60,8 +59,7 @@ struct StreamCore : public Botan::TLS::Callbacks } } - std::chrono::milliseconds - tls_verify_cert_chain_ocsp_timeout() const override + std::chrono::milliseconds tls_verify_cert_chain_ocsp_timeout() const override { return std::chrono::milliseconds(1000); } -- cgit v1.2.3