From 2c958f40b3ed1ee9c1f6870f90ddd853f4bb98b5 Mon Sep 17 00:00:00 2001 From: Hannes Rantzsch Date: Thu, 25 Apr 2019 11:31:12 +0200 Subject: tidy up StreamCore as an implementation detail of Stream --- src/lib/tls/asio/asio_async_handshake_op.h | 27 ++-- src/lib/tls/asio/asio_async_read_op.h | 24 ++-- src/lib/tls/asio/asio_async_write_op.h | 13 +- src/lib/tls/asio/asio_stream.h | 199 +++++++++++++++-------------- 4 files changed, 131 insertions(+), 132 deletions(-) (limited to 'src/lib/tls/asio') diff --git a/src/lib/tls/asio/asio_async_handshake_op.h b/src/lib/tls/asio/asio_async_handshake_op.h index f3eea7d16..e209c91da 100644 --- a/src/lib/tls/asio/asio_async_handshake_op.h +++ b/src/lib/tls/asio/asio_async_handshake_op.h @@ -33,20 +33,17 @@ class AsyncHandshakeOperation : public AsyncBase AsyncHandshakeOperation( HandlerT&& handler, Stream& stream, - typename Stream::StreamCore& core, const boost::system::error_code& ec = {}) : AsyncBase( std::forward(handler), stream.get_executor()) , m_stream(stream) - , m_core(core) { this->operator()(ec, std::size_t(0), false); } @@ -60,10 +57,12 @@ class AsyncHandshakeOperation : public AsyncBase 0 && !ec) { // Provide encrypted TLS data received from the network to TLS::Channel for decryption - boost::asio::const_buffer read_buffer {m_core.input_buffer.data(), bytesTransferred}; + boost::asio::const_buffer read_buffer {m_stream.input_buffer().data(), bytesTransferred}; try { - m_stream.native_handle()->received_data(static_cast(read_buffer.data()), read_buffer.size()); + m_stream.native_handle()->received_data( + static_cast(read_buffer.data()), read_buffer.size() + ); } catch(const TLS_Exception& e) { @@ -79,26 +78,26 @@ class AsyncHandshakeOperation : public AsyncBase 0` assumes that - // `bytesTransferred` bytes were just read and are in the core's 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 available in input_buffer for further processing. AsyncWriteOperation< AsyncHandshakeOperation::type, Stream, Allocator>, Stream, Allocator> - op{std::move(*this), m_stream, m_core, 0}; + op{std::move(*this), m_stream, 0}; return; } if(!m_stream.native_handle()->is_active() && !ec) { // Read more encrypted TLS data from the network - m_stream.next_layer().async_read_some(m_core.input_buffer, std::move(*this)); + m_stream.next_layer().async_read_some(m_stream.input_buffer(), std::move(*this)); return; } @@ -116,9 +115,7 @@ class AsyncHandshakeOperation : public AsyncBase AsyncReadOperation(HandlerT&& handler, Stream& stream, - typename Stream::StreamCore& core, const MutableBufferSequence& buffers, const boost::system::error_code& ec = {}) : AsyncBase( std::forward(handler), stream.get_executor()) , m_stream(stream) - , m_core(core) , m_buffers(buffers) , m_decodedBytes(0) { @@ -63,11 +60,12 @@ class AsyncReadOperation : public AsyncBase 0 && !ec) { // We have received encrypted data from the network, now hand it to TLS::Channel for decryption. - boost::asio::const_buffer read_buffer{m_core.input_buffer.data(), bytes_transferred}; + boost::asio::const_buffer read_buffer{m_stream.input_buffer().data(), bytes_transferred}; try { - m_stream.native_handle()->received_data(static_cast(read_buffer.data()), - read_buffer.size()); + m_stream.native_handle()->received_data( + static_cast(read_buffer.data()), read_buffer.size() + ); } catch(const TLS_Exception& e) { @@ -83,17 +81,17 @@ class AsyncReadOperation : public AsyncBase 0) + if(!m_stream.hasReceivedData() && !ec && boost::asio::buffer_size(m_buffers) > 0) { // The channel did not decrypt a complete record yet, we need more data from the socket. - m_stream.next_layer().async_read_some(m_core.input_buffer, std::move(*this)); + m_stream.next_layer().async_read_some(m_stream.input_buffer(), std::move(*this)); return; } - if(m_core.hasReceivedData() && !ec) + if(m_stream.hasReceivedData() && !ec) { // The channel has decrypted a TLS record, now copy it to the output buffers. - m_decodedBytes = m_core.copyReceivedData(m_buffers); + m_decodedBytes = m_stream.copyReceivedData(m_buffers); } if(!isContinuation) @@ -110,10 +108,8 @@ class AsyncReadOperation : public AsyncBase AsyncWriteOperation(HandlerT&& handler, Stream& stream, - typename 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); @@ -63,11 +60,11 @@ class AsyncWriteOperation : public AsyncBase class Stream { public: - using next_layer_type = typename std::remove_reference::type; - using lowest_layer_type = typename next_layer_type::lowest_layer_type; - using executor_type = typename next_layer_type::executor_type; - using native_handle_type = typename std::add_pointer::type; + // + // -- -- construction + // - public: template explicit Stream(Context& context, Args&& ... args) - : m_context(context), m_nextLayer(std::forward(args)...) {} + : m_context(context) + , m_nextLayer(std::forward(args)...) + , m_core(m_receive_buffer, m_send_buffer) + , m_input_buffer_space(MAX_CIPHERTEXT_SIZE, '\0') + , m_input_buffer(m_input_buffer_space.data(), m_input_buffer_space.size()) + {} // overload for boost::asio::ssl::stream compatibility template explicit Stream(Arg&& arg, Context& context) - : m_context(context), m_nextLayer(std::forward(arg)) {} + : m_context(context) + , m_nextLayer(std::forward(arg)) + , m_core(m_receive_buffer, m_send_buffer) + , m_input_buffer_space(MAX_CIPHERTEXT_SIZE, '\0') + , m_input_buffer(m_input_buffer_space.data(), m_input_buffer_space.size()) + {} virtual ~Stream() = default; @@ -75,9 +83,14 @@ class Stream Stream& operator=(const Stream& other) = delete; // - // -- -- accessor methods + // -- -- boost::asio compatible accessor methods // + using next_layer_type = typename std::remove_reference::type; + using lowest_layer_type = typename next_layer_type::lowest_layer_type; + using executor_type = typename next_layer_type::executor_type; + using native_handle_type = typename std::add_pointer::type; + executor_type get_executor() noexcept { return m_nextLayer.get_executor(); } const next_layer_type& next_layer() const { return m_nextLayer; } @@ -156,6 +169,43 @@ class Stream ec = Botan::ErrorType::NotImplemented; } + // + // -- -- accessor methods for send and receive buffers + // + + const boost::asio::mutable_buffer& input_buffer() { return m_input_buffer; } + boost::asio::const_buffer sendBuffer() const { return m_send_buffer.data(); } // TODO: really .data() ? + + /** + * Check if decrypted data is available in the receive buffer + */ + bool hasReceivedData() const { return m_receive_buffer.size() > 0; } + + /** + * Copy decrypted data into the user-provided buffer + */ + template + std::size_t copyReceivedData(MutableBufferSequence buffers) + { + // Note: It would be nice to avoid this buffer copy. This could be achieved by equipping the StreamCore with + // the user's desired target buffer once a read is started, and reading directly into that buffer in tls_record + // received. However, we need to deal with the case that the receive buffer provided by the caller is smaller + // than the decrypted record, so this optimization might not be worth the additional complexity. + const auto copiedBytes = boost::asio::buffer_copy(buffers, m_receive_buffer.data()); + m_receive_buffer.consume(copiedBytes); + return copiedBytes; + } + + /** + * Check if encrypted data is available in the send buffer + */ + bool hasDataToSend() const { return m_send_buffer.size() > 0; } + + /** + * Mark bytes in the send buffer as consumed, removing them from the buffer + */ + void consumeSendBuffer(std::size_t bytesConsumed) { m_send_buffer.consume(bytesConsumed); } + // // -- -- handshake methods // @@ -190,11 +240,7 @@ class Stream if(ec) { return; } - boost::asio::const_buffer read_buffer - { - this->m_core.input_buffer.data(), - m_nextLayer.read_some(this->m_core.input_buffer, ec) - }; + boost::asio::const_buffer read_buffer{input_buffer().data(), m_nextLayer.read_some(input_buffer(), ec)}; if(ec) { return; } @@ -244,7 +290,7 @@ class Stream boost::asio::async_completion init(handler); AsyncHandshakeOperation::type, Stream> - op{std::move(init.completion_handler), *this, this->m_core}; + op{std::move(init.completion_handler), *this}; return init.result.get(); } @@ -341,14 +387,14 @@ class Stream std::size_t read_some(const MutableBufferSequence& buffers, boost::system::error_code& ec) { - if(this->m_core.hasReceivedData()) - { return this->m_core.copyReceivedData(buffers); } + if(hasReceivedData()) + { return copyReceivedData(buffers); } tls_receive_some(ec); if(ec) { return 0; } - return this->m_core.copyReceivedData(buffers); + return copyReceivedData(buffers); } /** @@ -429,16 +475,16 @@ class Stream std::size_t sent = tls_encrypt_some(buffers, ec); if(ec) { - // 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(); + // 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 + consumeSendBuffer(m_send_buffer.size()); Botan::TLS::AsyncWriteOperation::type, Stream> - op{std::move(init.completion_handler), *this, this->m_core, std::size_t(0), ec}; + op{std::move(init.completion_handler), *this, std::size_t(0), ec}; return init.result.get(); } Botan::TLS::AsyncWriteOperation::type, Stream> - op{std::move(init.completion_handler), *this, this->m_core, sent}; + op{std::move(init.completion_handler), *this, sent}; return init.result.get(); } @@ -463,39 +509,40 @@ class Stream 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, buffers}; return init.result.get(); } - // TODO: remove StreamCore from public interface + protected: /** - * Contains the buffers for reading/sending, and the needed botan callbacks + * Helper class that implements Botan::TLS::Callbacks + * + * This class is provided to the stream's native_handle (Botan::TLS::Channel) and implements the callback + * functions triggered by the native_handle. + * + * @param receive_buffer reference to the buffer where decrypted data should be placed + * @param send_buffer reference to the buffer where encrypted data should be placed */ class StreamCore : public Botan::TLS::Callbacks { public: - StreamCore() - : m_input_buffer_space(MAX_CIPHERTEXT_SIZE, '\0'), - input_buffer(m_input_buffer_space.data(), m_input_buffer_space.size()) {} + StreamCore(boost::beast::flat_buffer& receive_buffer, boost::beast::flat_buffer& send_buffer) + : m_receive_buffer(receive_buffer), m_send_buffer(send_buffer) {} virtual ~StreamCore() = default; void tls_emit_data(const uint8_t data[], std::size_t size) override { - // Provide the encrypted TLS data in the sendBuffer. Actually sending the data is done - // using (async_)write_some either in the stream or in an async operation. m_send_buffer.commit( - boost::asio::buffer_copy(m_send_buffer.prepare(size), boost::asio::buffer(data, size))); + boost::asio::buffer_copy(m_send_buffer.prepare(size), boost::asio::buffer(data, size)) + ); } 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. - auto buffer = m_receive_buffer.prepare(size); - auto copySize = - boost::asio::buffer_copy(buffer, boost::asio::const_buffer(data, size)); - m_receive_buffer.commit(copySize); + m_receive_buffer.commit( + boost::asio::buffer_copy(m_receive_buffer.prepare(size), boost::asio::const_buffer(data, size)) + ); } void tls_alert(Botan::TLS::Alert alert) override @@ -517,49 +564,10 @@ class Stream return true; } - bool hasReceivedData() const - { - return m_receive_buffer.size() > 0; - } - - template - std::size_t copyReceivedData(MutableBufferSequence buffers) - { - const auto copiedBytes = - boost::asio::buffer_copy(buffers, m_receive_buffer.data()); - m_receive_buffer.consume(copiedBytes); - return copiedBytes; - } - - bool hasDataToSend() const { return m_send_buffer.size() > 0; } - - boost::asio::const_buffer sendBuffer() const - { - return m_send_buffer.data(); - } - - void consumeSendBuffer(std::size_t bytesConsumed) - { - m_send_buffer.consume(bytesConsumed); - } - - void clearSendBuffer() - { - consumeSendBuffer(m_send_buffer.size()); - } - - private: - // Buffer space used to read input intended for the engine. - std::vector m_input_buffer_space; - boost::beast::flat_buffer m_receive_buffer; - boost::beast::flat_buffer m_send_buffer; - - public: - // A buffer that may be used to read input intended for the engine. - const boost::asio::mutable_buffer input_buffer; + boost::beast::flat_buffer& m_receive_buffer; + boost::beast::flat_buffer& m_send_buffer; }; - protected: // TODO: explain, note: c++17 makes this much better with constexpr if template typename std::enable_if::value>::type @@ -571,11 +579,11 @@ class Stream { assert(side == CLIENT); m_channel = std::unique_ptr(new Client(m_core, - *m_context.sessionManager, - *m_context.credentialsManager, - *m_context.policy, - *m_context.randomNumberGenerator, - m_context.serverInfo)); + *m_context.sessionManager, + *m_context.credentialsManager, + *m_context.policy, + *m_context.randomNumberGenerator, + m_context.serverInfo)); } //! \brief validate the connection side (OpenSSL compatibility) @@ -601,27 +609,22 @@ class Stream size_t sendPendingEncryptedData(boost::system::error_code& ec) { - auto writtenBytes = boost::asio::write(m_nextLayer, this->m_core.sendBuffer(), ec); + auto writtenBytes = boost::asio::write(m_nextLayer, sendBuffer(), ec); - this->m_core.consumeSendBuffer(writtenBytes); + consumeSendBuffer(writtenBytes); return writtenBytes; } void tls_receive_some(boost::system::error_code& ec) { - boost::asio::const_buffer read_buffer = - { - this->m_core.input_buffer.data(), - m_nextLayer.read_some(this->m_core.input_buffer, ec) - }; + boost::asio::const_buffer read_buffer{input_buffer().data(), m_nextLayer.read_some(input_buffer(), ec)}; if(ec) { return; } try { - native_handle()->received_data(static_cast(read_buffer.data()), - read_buffer.size()); + native_handle()->received_data(static_cast(read_buffer.data()), read_buffer.size()); } catch(const TLS_Exception& e) { @@ -685,8 +688,16 @@ class Stream Context m_context; StreamLayer m_nextLayer; + + boost::beast::flat_buffer m_receive_buffer; + boost::beast::flat_buffer m_send_buffer; + StreamCore m_core; std::unique_ptr m_channel; + + // Buffer space used to read input intended for the core + std::vector m_input_buffer_space; + const boost::asio::mutable_buffer m_input_buffer; }; } // namespace TLS -- cgit v1.2.3