diff --git a/.github/workflows/unittest.yml b/.github/workflows/unittest.yml index 382c3105..11cec1a9 100644 --- a/.github/workflows/unittest.yml +++ b/.github/workflows/unittest.yml @@ -12,7 +12,7 @@ jobs: strategy: fail-fast: false matrix: - boost_version: ["1.75.0", "1.76.0", "1.77.0", "1.78.0", "1.79.0"] + boost_version: ["1.75.0", "1.76.0", "1.77.0", "1.78.0", "1.79.0", "1.80.0", "1.81.0"] os: [windows-2019, windows-2022] toolset: [v141, v142, v143, ClangCL] build_type: [Debug, Release] @@ -32,6 +32,8 @@ jobs: - { boost_version: "1.77.0", toolset: v143 } - { boost_version: "1.78.0", toolset: v141 } - { boost_version: "1.79.0", toolset: v141 } + - { boost_version: "1.80.0", toolset: v141 } + - { boost_version: "1.81.0", toolset: v141 } include: - boost_version: "1.79.0" os: windows-2022 @@ -62,8 +64,9 @@ jobs: with: fetch-depth: 0 + # For Boost Versions >= 1.78, the toolset parameter has to be specified to install-boost. - name: Add boost toolset to environment - if: contains(fromJson('["1.78.0", "1.79.0"]'), matrix.boost_version) + if: contains(fromJson('["1.78.0", "1.79.0", "1.80.0", "1.81.0"]'), matrix.boost_version) run: echo BOOST_TOOLSET=$([[ "${{matrix.generator}}" == "MinGW Makefiles" ]] && echo "mingw" || echo "msvc") >> $GITHUB_ENV # The platform_version passed to boost-install determines the msvc toolset version for which static libs are installed. diff --git a/include/boost/wintls/detail/async_handshake.hpp b/include/boost/wintls/detail/async_handshake.hpp index 0a353ae3..8acb2a39 100644 --- a/include/boost/wintls/detail/async_handshake.hpp +++ b/include/boost/wintls/detail/async_handshake.hpp @@ -10,6 +10,7 @@ #include +#include #include #include @@ -18,17 +19,24 @@ namespace boost { namespace wintls { namespace detail { -template +template struct async_handshake : boost::asio::coroutine { - async_handshake(NextLayer& next_layer, detail::sspi_handshake& handshake, handshake_type type) - : next_layer_(next_layer) - , handshake_(handshake) - , entry_count_(0) - , state_(state::idle) { - handshake_(type); + async_handshake(NextLayer& next_layer, detail::sspi_handshake& handshake, handshake_mode mode) + : next_layer_(next_layer) + , handshake_(handshake) + , mode_(mode) + , entry_count_(0) { + switch (mode) { + case handshake_mode::init: + handshake_.init(); + break; + case handshake_mode::shutdown: + handshake_.shutdown(); + break; + } } - template + template void operator()(Self& self, boost::system::error_code ec = {}, std::size_t length = 0) { if (ec) { self.complete(ec); @@ -40,94 +48,57 @@ struct async_handshake : boost::asio::coroutine { return entry_count_ > 1; }; - switch(state_) { - case state::reading: - handshake_.size_read(length); - state_ = state::idle; - break; - case state::writing: - handshake_.size_written(length); - state_ = state::idle; - break; - case state::idle: - break; - } - - detail::sspi_handshake::state handshake_state; + sspi_handshake::state handshake_state; BOOST_ASIO_CORO_REENTER(*this) { - while((handshake_state = handshake_()) != detail::sspi_handshake::state::done) { - if (handshake_state == detail::sspi_handshake::state::data_needed) { + while (true) { + handshake_state = handshake_(); + if (handshake_state == sspi_handshake::state::data_needed) { BOOST_ASIO_CORO_YIELD { - state_ = state::reading; next_layer_.async_read_some(handshake_.in_buffer(), std::move(self)); } + handshake_.size_read(length); continue; } - if (handshake_state == detail::sspi_handshake::state::data_available) { + if (handshake_state == sspi_handshake::state::data_available) { BOOST_ASIO_CORO_YIELD { - state_ = state::writing; net::async_write(next_layer_, handshake_.out_buffer(), std::move(self)); } + handshake_.size_written(length); continue; } - if (handshake_state == detail::sspi_handshake::state::error) { - if (!is_continuation()) { - BOOST_ASIO_CORO_YIELD { - auto e = self.get_executor(); - net::post(e, [self = std::move(self), ec, length]() mutable { self(ec, length); }); - } - } - self.complete(handshake_.last_error()); - return; - } - - if (handshake_state == detail::sspi_handshake::state::done_with_data) { - BOOST_ASIO_CORO_YIELD { - state_ = state::writing; - net::async_write(next_layer_, handshake_.out_buffer(), std::move(self)); - } + if (handshake_state == sspi_handshake::state::error) { + ec = handshake_.last_error(); break; } - if (handshake_state == detail::sspi_handshake::state::error_with_data) { - BOOST_ASIO_CORO_YIELD { - state_ = state::writing; - net::async_write(next_layer_, handshake_.out_buffer(), std::move(self)); - } - if (!is_continuation()) { - BOOST_ASIO_CORO_YIELD { - auto e = self.get_executor(); - net::post(e, [self = std::move(self), ec, length]() mutable { self(ec, length); }); - } + if (handshake_state == sspi_handshake::state::done) { + BOOST_ASSERT(!handshake_.last_error()); + if (mode_ == handshake_mode::init) { + ec = error::make_error_code(handshake_.manual_auth()); } - self.complete(handshake_.last_error()); - return; + break; } } + if (ec == net::error::eof) { + ec = wintls::error::stream_truncated; + } if (!is_continuation()) { BOOST_ASIO_CORO_YIELD { - auto e = self.get_executor(); - net::post(e, [self = std::move(self), ec, length]() mutable { self(ec, length); }); + post_self(self, next_layer_, ec, length); } } - BOOST_ASSERT(!handshake_.last_error()); - self.complete(handshake_.last_error()); + self.complete(ec); } } private: NextLayer& next_layer_; - detail::sspi_handshake& handshake_; + sspi_handshake& handshake_; + handshake_mode mode_; int entry_count_; - std::vector input_; - enum class state { - idle, - reading, - writing - } state_; }; } // namespace detail diff --git a/include/boost/wintls/detail/async_read.hpp b/include/boost/wintls/detail/async_read.hpp index fda02a35..a7500391 100644 --- a/include/boost/wintls/detail/async_read.hpp +++ b/include/boost/wintls/detail/async_read.hpp @@ -8,6 +8,7 @@ #ifndef BOOST_WINTLS_DETAIL_ASYNC_READ_HPP #define BOOST_WINTLS_DETAIL_ASYNC_READ_HPP +#include #include #include @@ -50,8 +51,7 @@ struct async_read : boost::asio::coroutine { if (state == detail::sspi_decrypt::state::error) { if (!is_continuation()) { BOOST_ASIO_CORO_YIELD { - auto e = self.get_executor(); - net::post(e, [self = std::move(self), ec, size_read]() mutable { self(ec, size_read); }); + post_self(self, next_layer_, ec, size_read); } } ec = decrypt_.last_error(); diff --git a/include/boost/wintls/detail/async_shutdown.hpp b/include/boost/wintls/detail/async_shutdown.hpp deleted file mode 100644 index 8d613349..00000000 --- a/include/boost/wintls/detail/async_shutdown.hpp +++ /dev/null @@ -1,72 +0,0 @@ -// -// Copyright (c) 2020 Kasper Laudrup (laudrup at stacktrace dot dk) -// -// Distributed under the Boost Software License, Version 1.0. (See accompanying -// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) -// - -#ifndef BOOST_WINTLS_DETAIL_ASYNC_SHUTDOWN_HPP -#define BOOST_WINTLS_DETAIL_ASYNC_SHUTDOWN_HPP - -#include - -#include - -namespace boost { -namespace wintls { -namespace detail { - -template -struct async_shutdown : boost::asio::coroutine { - async_shutdown(NextLayer& next_layer, detail::sspi_shutdown& shutdown) - : next_layer_(next_layer) - , shutdown_(shutdown) - , entry_count_(0) { - } - - template - void operator()(Self& self, boost::system::error_code ec = {}, std::size_t size_written = 0) { - if (ec) { - self.complete(ec); - return; - } - - ++entry_count_; - auto is_continuation = [this] { - return entry_count_ > 1; - }; - - ec = shutdown_(); - - BOOST_ASIO_CORO_REENTER(*this) { - if (!ec) { - BOOST_ASIO_CORO_YIELD { - net::async_write(next_layer_, shutdown_.buffer(), std::move(self)); - } - shutdown_.size_written(size_written); - self.complete({}); - return; - } else { - if (!is_continuation()) { - BOOST_ASIO_CORO_YIELD { - auto e = self.get_executor(); - net::post(e, [self = std::move(self), ec, size_written]() mutable { self(ec, size_written); }); - } - } - self.complete(ec); - return; - } - } - } - -private: - NextLayer& next_layer_; - detail::sspi_shutdown& shutdown_; - int entry_count_; -}; - -} // namespace detail -} // namespace wintls -} // namespace boost - -#endif // BOOST_WINTLS_DETAIL_ASYNC_SHUTDOWN_HPP diff --git a/include/boost/wintls/detail/post_self.hpp b/include/boost/wintls/detail/post_self.hpp new file mode 100644 index 00000000..b37abd72 --- /dev/null +++ b/include/boost/wintls/detail/post_self.hpp @@ -0,0 +1,48 @@ +// +// Copyright (c) 2020 Kasper Laudrup (laudrup at stacktrace dot dk) +// +// Distributed under the Boost Software License, Version 1.0. (See accompanying +// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) +// + +#ifndef BOOST_WINTLS_DETAIL_POST_SELF_HPP +#define BOOST_WINTLS_DETAIL_POST_SELF_HPP + +#if BOOST_VERSION >= 108000 +#include +#endif +#include +#include +#include + +namespace boost { +namespace wintls { +namespace detail { + +// If a composed asynchronous operation completes immediately (due to an error) +// we do not want to call self.complete() directly as this may produce an infinite recursion in some cases. +// Instead, we post the intermediate completion handler (self) once. +// To achieve consistent behavior to non-erroneous cases, we post to the executor of the I/O object. +// Note that this only got accessible through self by get_io_executor since boost 1.81. +template +auto post_self(Self& self, IoObject& io_object, boost::system::error_code ec, std::size_t length) { +#if BOOST_VERSION >= 108100 + boost::ignore_unused(io_object); + auto ex = self.get_io_executor(); + return boost::asio::post(ex, boost::asio::append(std::move(self), ec, length)); +#elif BOOST_VERSION >= 108000 + return boost::asio::post(io_object.get_executor(), boost::asio::append(std::move(self), ec, length)); +#else + auto ex = io_object.get_executor(); + // If the completion token associated with self had an associated executor, + // allocator or cancellation slot, we loose these here. + // Therefore, above solutions are better! + return boost::asio::post(ex, [self = std::move(self), ec, length]() mutable { self(ec, length); }); +#endif +} + +} // namespace detail +} // namespace wintls +} // namespace boost + +#endif //BOOST_WINTLS_DETAIL_POST_SELF_HPP \ No newline at end of file diff --git a/include/boost/wintls/detail/sspi_handshake.hpp b/include/boost/wintls/detail/sspi_handshake.hpp index 6315513e..87ca90ee 100644 --- a/include/boost/wintls/detail/sspi_handshake.hpp +++ b/include/boost/wintls/detail/sspi_handshake.hpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -26,17 +27,18 @@ namespace boost { namespace wintls { namespace detail { +enum class handshake_mode { + init, // initial handshake to establish the protocol + shutdown // shutdown the protocol +}; + class sspi_handshake { public: - // TODO: enhancement: done_with_data and error_with_data can be removed if - // we move the manual validate logic out of the handshake loop. enum class state { - data_needed, // data needs to be read from peer - data_available, // data needs to be write to peer - done_with_data, // handshake success, but there is leftover data to be written to peer - error_with_data, // handshake error, but there is leftover data to be written to peer - done, // handshake success - error // handshake error + data_needed, // data needs to be read from peer + data_available, // data needs to be write to peer + done, // handshake success + error // handshake error }; sspi_handshake(context& context, ctxt_handle& ctxt_handle, cred_handle& cred_handle) @@ -48,9 +50,13 @@ class sspi_handshake { input_buffers_[0].pvBuffer = reinterpret_cast(input_data_.data()); } - void operator()(handshake_type type) { + // Specify whether the handshake is performed as a client or as a server. + // This is used for the initial handshake and for the shutdown. + void set_type(handshake_type type) { handshake_type_ = type; + } + void init() { SCHANNEL_CRED creds{}; creds.dwVersion = SCHANNEL_CRED_VERSION; creds.grbitEnabledProtocols = static_cast(context_.method_); @@ -73,16 +79,11 @@ class sspi_handshake { BOOST_UNREACHABLE_RETURN(0); }(); - auto server_cert = context_.server_cert(); - if (handshake_type_ == handshake_type::server && server_cert != nullptr) { - creds.cCreds = 1; - creds.paCred = &server_cert; - } - // TODO: rename server_cert field since it is also used for client cert. // Note: if client cert is set, sspi will auto validate server cert with it. // Even though verify_server_certificate_ in context is set to false. - if (handshake_type_ == handshake_type::client && server_cert != nullptr) { + auto server_cert = context_.server_cert(); + if (server_cert != nullptr) { creds.cCreds = 1; creds.paCred = &server_cert; } @@ -128,13 +129,63 @@ class sspi_handshake { } } + void shutdown() { + shutdown_buffers buffers; + last_error_ = detail::sspi_functions::ApplyControlToken(ctxt_handle_.get(), buffers.desc()); + if (last_error_ != SEC_E_OK) { + return; + } + DWORD out_flags = 0; + switch (handshake_type_) { + case handshake_type::client: + last_error_ = detail::sspi_functions::InitializeSecurityContext(cred_handle_.get(), + ctxt_handle_.get(), + nullptr, + client_context_flags, + 0, + SECURITY_NATIVE_DREP, + nullptr, + 0, + ctxt_handle_.get(), + buffers.desc(), + &out_flags, + nullptr); + break; + case handshake_type::server: { + TimeStamp expiry; + last_error_ = detail::sspi_functions::AcceptSecurityContext(cred_handle_.get(), + ctxt_handle_.get(), + nullptr, + server_context_flags, + SECURITY_NATIVE_DREP, + ctxt_handle_.get(), + buffers.desc(), + &out_flags, + &expiry); + } + } + if (last_error_ != SEC_E_OK) { + return; + } + out_buffer_ = sspi_context_buffer{buffers[0].pvBuffer, buffers[0].cbBuffer}; + // Setting SEC_I_CONTINUE_NEEDED here means that we will wait for the peer to send its close notify. + // Not doing so may open up the possibility of a truncation attack. + // If the underlying protocol is self-terminated (as is http) it would be okay not to wait for the close notify. + // We could implement such a shutdown as a "fast/unsafe_shutdown" and not change last_error_ here for that case. + // Cmp. https://security.stackexchange.com/questions/82028/ssl-tls-is-a-server-always-required-to-respond-to-a-close-notify + last_error_ = SEC_I_CONTINUE_NEEDED; + } + state operator()() { - if (last_error_ != SEC_I_CONTINUE_NEEDED && last_error_ != SEC_E_INCOMPLETE_MESSAGE) { + if (last_error_ != SEC_I_CONTINUE_NEEDED && last_error_ != SEC_E_INCOMPLETE_MESSAGE && last_error_ != SEC_E_OK) { return state::error; } if (!out_buffer_.empty()) { return state::data_available; } + if (last_error_ == SEC_E_OK) { + return state::done; + } if (input_buffers_[0].cbBuffer == 0) { return state::data_needed; } @@ -200,7 +251,7 @@ class sspi_handshake { } bool has_buffer_output = out_buffers[0].cbBuffer != 0 && out_buffers[0].pvBuffer != nullptr; - if(has_buffer_output){ + if (has_buffer_output) { out_buffer_ = sspi_context_buffer{out_buffers[0].pvBuffer, out_buffers[0].cbBuffer}; } @@ -209,27 +260,27 @@ class sspi_handshake { return has_buffer_output ? state::data_available : state::data_needed; } case SEC_E_OK: { - // sspi handshake ok. perform manual auth here. - manual_auth(); - if (handshake_type_ == handshake_type::client) { - if (last_error_ != SEC_E_OK) { - return state::error; - } - } else { - // Note: we are not checking (out_flags & ASC_RET_MUTUAL_AUTH) is true, - // but instead rely on our manual cert validation to establish trust. - // "The AcceptSecurityContext function will return ASC_RET_MUTUAL_AUTH if a - // client certificate was received from the client and schannel was - // successfully able to map the certificate to a user account in AD" - // As observed in tests, this check would wrongly reject openssl client with valid certificate. - - // AcceptSecurityContext documentation: - // "If function generated an output token, the token must be sent to the client process." - // This happens when client cert is requested. - if (has_buffer_output) { - return last_error_ == SEC_E_OK ? state::done_with_data : state::error_with_data; - } - } + // sspi handshake ok. Manual authentication will be done after the handshake loop. + + // Note: When we requested client auth as a server, + // we are not checking (out_flags & ASC_RET_MUTUAL_AUTH) is true, + // but instead rely on our manual cert validation to establish trust. + // "The AcceptSecurityContext function will return ASC_RET_MUTUAL_AUTH if a + // client certificate was received from the client and schannel was + // successfully able to map the certificate to a user account in AD" + // As observed in tests, this check would wrongly reject openssl client with valid certificate. + + // AcceptSecurityContext/InitializeSecurityContext documentation for return value SEC_E_OK: + // "If function generated an output token, the token must be sent to the client/server." + // This happens when client cert is requested. + return has_buffer_output ? state::data_available : state::done; + } + + // If we get here during shutdown, this means we initiated the shutdown and now received the + // close notify from the peer. That means that we are done. + // #TODO: could this happen during the initial handshake where it should be an error? + case SEC_I_CONTEXT_EXPIRED: { + last_error_ = SEC_E_OK; // #TODO: better solve this in the handshake loop? return state::done; } @@ -274,8 +325,7 @@ class sspi_handshake { check_revocation_ = check; } -private: - SECURITY_STATUS manual_auth(){ + SECURITY_STATUS manual_auth() { if (!context_.verify_server_certificate_) { return SEC_E_OK; } @@ -284,16 +334,12 @@ class sspi_handshake { if (last_error_ != SEC_E_OK) { return last_error_; } - cert_context_ptr remote_cert{ctx_ptr}; - last_error_ = static_cast(context_.verify_certificate(remote_cert.get(), server_hostname_, check_revocation_)); - if (last_error_ != SEC_E_OK) { - return last_error_; - } return last_error_; } +private: context& context_; ctxt_handle& ctxt_handle_; cred_handle& cred_handle_; @@ -308,6 +354,53 @@ class sspi_handshake { bool check_revocation_ = false; }; +template +void handshake(NextLayer& next_layer, + sspi_handshake& handshake, + handshake_mode mode, + boost::system::error_code& ec) { + switch (mode) { + case handshake_mode::init: + handshake.init(); + break; + case handshake_mode::shutdown: + handshake.shutdown(); + break; + } + while (true) { + auto handshake_state = handshake(); + if (handshake_state == sspi_handshake::state::data_needed) { + std::size_t size_read = next_layer.read_some(handshake.in_buffer(), ec); + if (ec) { + break; + } + handshake.size_read(size_read); + continue; + } + if (handshake_state == sspi_handshake::state::data_available) { + std::size_t size_written = net::write(next_layer, handshake.out_buffer(), ec); + if (ec) { + break; + } + handshake.size_written(size_written); + continue; + } + if (handshake_state == sspi_handshake::state::error) { + ec = handshake.last_error(); + break; + } + if (handshake_state == sspi_handshake::state::done) { + if (mode == handshake_mode::init && handshake.manual_auth() != SEC_E_OK) { + ec = handshake.last_error(); + } + break; + } + } + if (ec == net::error::eof) { + ec = wintls::error::stream_truncated; + } +} + } // namespace detail } // namespace wintls } // namespace boost diff --git a/include/boost/wintls/detail/sspi_shutdown.hpp b/include/boost/wintls/detail/sspi_shutdown.hpp deleted file mode 100644 index 8983493e..00000000 --- a/include/boost/wintls/detail/sspi_shutdown.hpp +++ /dev/null @@ -1,79 +0,0 @@ -// -// Copyright (c) 2021 Kasper Laudrup (laudrup at stacktrace dot dk) -// -// Distributed under the Boost Software License, Version 1.0. (See accompanying -// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) -// - -#ifndef BOOST_WINTLS_DETAIL_SSPI_SHUTDOWN_HPP -#define BOOST_WINTLS_DETAIL_SSPI_SHUTDOWN_HPP - -#include -#include -#include -#include -#include -#include - -#include - -namespace boost { -namespace wintls { -namespace detail { - -class sspi_shutdown { -public: - sspi_shutdown(ctxt_handle& ctxt_handle, cred_handle& cred_handle) - : ctxt_handle_(ctxt_handle) - , cred_handle_(cred_handle) { - } - - boost::system::error_code operator()() { - shutdown_buffers buffers; - - SECURITY_STATUS sc = detail::sspi_functions::ApplyControlToken(ctxt_handle_.get(), buffers.desc()); - if (sc != SEC_E_OK) { - return error::make_error_code(sc); - } - - DWORD out_flags = 0; - sc = detail::sspi_functions::InitializeSecurityContext(cred_handle_.get(), - ctxt_handle_.get(), - nullptr, - client_context_flags, - 0, - SECURITY_NATIVE_DREP, - nullptr, - 0, - ctxt_handle_.get(), - buffers.desc(), - &out_flags, - nullptr); - if (sc != SEC_E_OK) { - return error::make_error_code(sc); - } - - buffer_ = sspi_context_buffer{buffers[0].pvBuffer, buffers[0].cbBuffer}; - return {}; - } - - net::const_buffer buffer() { - return buffer_.asio_buffer(); - } - - void size_written(std::size_t size) { - BOOST_VERIFY(size == buffer_.size()); - buffer_ = sspi_context_buffer{}; - } - -private: - ctxt_handle& ctxt_handle_; - cred_handle& cred_handle_; - sspi_context_buffer buffer_; -}; - -} // namespace detail -} // namespace wintls -} // namespace boost - -#endif // BOOST_WINTLS_DETAIL_SSPI_SHUTDOWN_HPP diff --git a/include/boost/wintls/detail/sspi_stream.hpp b/include/boost/wintls/detail/sspi_stream.hpp index 22c211ee..dc5f73a9 100644 --- a/include/boost/wintls/detail/sspi_stream.hpp +++ b/include/boost/wintls/detail/sspi_stream.hpp @@ -11,7 +11,6 @@ #include #include #include -#include #include namespace boost { @@ -21,10 +20,9 @@ namespace detail { class sspi_stream { public: sspi_stream(context& ctx) - : handshake(ctx, ctxt_handle_, cred_handle_) - , encrypt(ctxt_handle_) - , decrypt(ctxt_handle_) - , shutdown(ctxt_handle_, cred_handle_) { + : handshake(ctx, ctxt_handle_, cred_handle_) + , encrypt(ctxt_handle_) + , decrypt(ctxt_handle_) { } sspi_stream(sspi_stream&&) = delete; @@ -38,7 +36,6 @@ class sspi_stream { sspi_handshake handshake; sspi_encrypt encrypt; sspi_decrypt decrypt; - sspi_shutdown shutdown; }; } // namespace detail diff --git a/include/boost/wintls/error.hpp b/include/boost/wintls/error.hpp index 11df0148..fde7a713 100644 --- a/include/boost/wintls/error.hpp +++ b/include/boost/wintls/error.hpp @@ -15,6 +15,36 @@ namespace boost { namespace wintls { namespace error { +enum stream_errors { + /// The underlying stream closed before the ssl stream gracefully shut down. + stream_truncated = 1 +}; + +class stream_category : public boost::system::error_category { +public: + const char* name() const BOOST_ASIO_ERROR_CATEGORY_NOEXCEPT { + return "wintls.stream"; + } + + std::string message(int value) const { + switch (value) { + case stream_truncated: + return "stream truncated"; + default: + return "wintls.stream error"; + } + } +}; + +inline const boost::system::error_category& get_stream_category() { + static stream_category instance; + return instance; +} + +inline boost::system::error_code make_error_code(stream_errors e) { + return boost::system::error_code(static_cast(e), get_stream_category()); +} + inline boost::system::error_code make_error_code(SECURITY_STATUS sc) { return boost::system::error_code(static_cast(sc), boost::system::system_category()); } @@ -23,4 +53,15 @@ inline boost::system::error_code make_error_code(SECURITY_STATUS sc) { } // namespace wintls } // namespace boost +namespace boost { +namespace system { + +template<> +struct is_error_code_enum { + static const bool value = true; +}; + +} // namespace system +} // namespace boost + #endif // BOOST_WINTLS_ERROR_HPP diff --git a/include/boost/wintls/stream.hpp b/include/boost/wintls/stream.hpp index 0e1e0747..311c24da 100644 --- a/include/boost/wintls/stream.hpp +++ b/include/boost/wintls/stream.hpp @@ -13,7 +13,6 @@ #include #include -#include #include #include @@ -135,50 +134,8 @@ class stream { * @param ec Set to indicate what error occurred, if any. */ void handshake(handshake_type type, boost::system::error_code& ec) { - sspi_stream_->handshake(type); - - detail::sspi_handshake::state state; - while((state = sspi_stream_->handshake()) != detail::sspi_handshake::state::done) { - switch (state) { - case detail::sspi_handshake::state::data_needed: { - std::size_t size_read = next_layer_.read_some(sspi_stream_->handshake.in_buffer(), ec); - if (ec) { - return; - } - sspi_stream_->handshake.size_read(size_read); - continue; - } - case detail::sspi_handshake::state::data_available: { - std::size_t size_written = net::write(next_layer_, sspi_stream_->handshake.out_buffer(), ec); - if (ec) { - return; - } - sspi_stream_->handshake.size_written(size_written); - continue; - } - case detail::sspi_handshake::state::error: - ec = sspi_stream_->handshake.last_error(); - return; - case detail::sspi_handshake::state::done_with_data:{ - std::size_t size_written = net::write(next_layer_, sspi_stream_->handshake.out_buffer(), ec); - if (ec) { - return; - } - sspi_stream_->handshake.size_written(size_written); - return; - } - case detail::sspi_handshake::state::error_with_data:{ - std::size_t size_written = net::write(next_layer_, sspi_stream_->handshake.out_buffer(), ec); - if (ec) { - return; - } - sspi_stream_->handshake.size_written(size_written); - return; - } - case detail::sspi_handshake::state::done: - BOOST_UNREACHABLE_RETURN(0); - } - } + sspi_stream_->handshake.set_type(type); + detail::handshake(next_layer_, sspi_stream_->handshake, detail::handshake_mode::init, ec); } /** Perform TLS handshaking. @@ -223,10 +180,13 @@ class stream { * this function. Invocation of the handler will be performed in a * manner equivalent to using `net::post`. */ - template + template auto async_handshake(handshake_type type, CompletionToken&& handler) { + sspi_stream_->handshake.set_type(type); return boost::asio::async_compose( - detail::async_handshake{next_layer_, sspi_stream_->handshake, type}, handler); + detail::async_handshake{next_layer_, sspi_stream_->handshake, detail::handshake_mode::init}, + handler, + *this); } /** Read some data from the stream. @@ -320,7 +280,9 @@ class stream { template auto async_read_some(const MutableBufferSequence& buffers, CompletionToken&& handler) { return boost::asio::async_compose( - detail::async_read{next_layer_, buffers, sspi_stream_->decrypt}, handler); + detail::async_read{next_layer_, buffers, sspi_stream_->decrypt}, + handler, + *this); } /** Write some data to the stream. @@ -410,7 +372,9 @@ class stream { template auto async_write_some(const ConstBufferSequence& buffers, CompletionToken&& handler) { return boost::asio::async_compose( - detail::async_write{next_layer_, buffers, sspi_stream_->encrypt}, handler); + detail::async_write{next_layer_, buffers, sspi_stream_->encrypt}, + handler, + *this); } /** Shut down TLS on the stream. @@ -422,14 +386,7 @@ class stream { * @param ec Set to indicate what error occurred, if any. */ void shutdown(boost::system::error_code& ec) { - ec = sspi_stream_->shutdown(); - if (ec) { - return; - } - std::size_t size_written = net::write(next_layer_, sspi_stream_->shutdown.buffer(), ec); - if (!ec) { - sspi_stream_->shutdown.size_written(size_written); - } + detail::handshake(next_layer_, sspi_stream_->handshake, detail::handshake_mode::shutdown, ec); } /** Shut down TLS on the stream. @@ -462,10 +419,14 @@ class stream { *); * @endcode */ - template + template auto async_shutdown(CompletionToken&& handler) { return boost::asio::async_compose( - detail::async_shutdown{next_layer_, sspi_stream_->shutdown}, handler); + detail::async_handshake{next_layer_, + sspi_stream_->handshake, + detail::handshake_mode::shutdown}, + handler, + *this); } private: diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 81bbc241..d74db1ff 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -33,6 +33,7 @@ set(test_sources sspi_buffer_sequence_test.cpp stream_test.cpp decrypted_data_buffer_test.cpp + shutdown_test.cpp ) add_executable(unittest diff --git a/test/certificate_test.cpp b/test/certificate_test.cpp index 7370b9c0..24419506 100644 --- a/test/certificate_test.cpp +++ b/test/certificate_test.cpp @@ -245,13 +245,13 @@ TEST_CASE("check certificate revocation (integration test)", "[.integration]") { // 'running()' does not mean that it is responding yet, but that we did start the process correctly CHECK(ocsp_responder.running()); // It takes a few seconds for the OCSP responder to become available. - // Therefore, we try to verify the certificate in a loop, but for at most 30 seconds. + // Therefore, we try to verify the certificate in a loop, but for at most 60 seconds. auto res_with_responder = CRYPT_E_REVOCATION_OFFLINE; const auto start = std::chrono::system_clock::now(); while (res_with_responder == CRYPT_E_REVOCATION_OFFLINE) { res_with_responder = ctx_certs.verify_certificate(cert_ocsp.get(), "", true); - if (std::chrono::system_clock::now() - start > std::chrono::seconds(30)) { - WARN("OCSP responder did not provide a response within 30 seconds."); + if (std::chrono::system_clock::now() - start > std::chrono::seconds(60)) { + WARN("OCSP responder did not provide a response within 60 seconds."); break; } } diff --git a/test/echo_client.hpp b/test/echo_client.hpp index fc31a2d6..aee8f77f 100644 --- a/test/echo_client.hpp +++ b/test/echo_client.hpp @@ -27,6 +27,14 @@ class echo_client : public Stream { stream.shutdown(); } + auto launch_shutdown() { + return std::async(std::launch::async, [&]() { + boost::system::error_code ec; + stream.shutdown(ec); + return ec; + }); + } + void read() { net::read_until(stream, buffer_, '\0'); } diff --git a/test/echo_test.cpp b/test/echo_test.cpp index f7265622..9c04375f 100644 --- a/test/echo_test.cpp +++ b/test/echo_test.cpp @@ -75,8 +75,10 @@ TEMPLATE_LIST_TEST_CASE("echo test", "", TestTypes) { async_echo_server server(io_context); async_echo_client client(io_context, test_data); client.stream.next_layer().connect(server.stream.next_layer()); - server.run(); + // #TODO: calling server.run() before client.run() causes io_context.run() to block indefinitely + // in the shutdown handshake loop while the server waits for the client close notify. client.run(); + server.run(); io_context.run(); CHECK(client.received_message() == test_data); } diff --git a/test/shutdown_test.cpp b/test/shutdown_test.cpp new file mode 100644 index 00000000..eeb210df --- /dev/null +++ b/test/shutdown_test.cpp @@ -0,0 +1,119 @@ +// +// Copyright (c) 2020 Kasper Laudrup (laudrup at stacktrace dot dk) +// +// Distributed under the Boost Software License, Version 1.0. (See accompanying +// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) +// + +#include "echo_server.hpp" +#include "echo_client.hpp" +#include "async_echo_server.hpp" +#include "async_echo_client.hpp" +#include "asio_ssl_client_stream.hpp" +#include "asio_ssl_server_stream.hpp" +#include "wintls_client_stream.hpp" +#include "wintls_server_stream.hpp" +#include "unittest.hpp" + +#include + +template +auto stream_truncated_error(const asio_ssl::stream&) { + return asio_ssl::error::stream_truncated; +} + +template +auto stream_truncated_error(const boost::wintls::stream&) { + return boost::wintls::error::stream_truncated; +} + +using TestTypes = std::tuple, + std::tuple, + std::tuple, + std::tuple>; + +TEMPLATE_LIST_TEST_CASE("shutdown test", "", TestTypes) { + using ClientStream = typename std::tuple_element<0, TestType>::type; + using ServerStream = typename std::tuple_element<1, TestType>::type; + + SECTION("stream truncated - client closes after server shutdown") { + net::io_context io_context; + echo_client client(io_context); + echo_server server(io_context); + + client.stream.next_layer().connect(server.stream.next_layer()); + + auto handshake_result = server.handshake(); + client.handshake(); + REQUIRE_FALSE(handshake_result.get()); + + auto shutdown_result = server.shutdown(); + client.stream.next_layer().close(); + CHECK(shutdown_result.get() == stream_truncated_error(server.stream)); + } + + SECTION("stream truncated - client closes before server shutdown") { + net::io_context io_context; + echo_client client(io_context); + echo_server server(io_context); + + client.stream.next_layer().connect(server.stream.next_layer()); + + auto handshake_result = server.handshake(); + client.handshake(); + REQUIRE_FALSE(handshake_result.get()); + + client.stream.next_layer().close(); + auto shutdown_result = server.shutdown(); + CHECK(shutdown_result.get() == stream_truncated_error(server.stream)); + } + + SECTION("stream truncated - server closes after client shutdown") { + net::io_context io_context; + echo_client client(io_context); + echo_server server(io_context); + + client.stream.next_layer().connect(server.stream.next_layer()); + + auto handshake_result = server.handshake(); + client.handshake(); + REQUIRE_FALSE(handshake_result.get()); + + auto client_shutdown_result = client.launch_shutdown(); + server.stream.next_layer().close(); + CHECK(client_shutdown_result.get() == stream_truncated_error(client.stream)); + } + + SECTION("stream truncated - server closes before client shutdown") { + net::io_context io_context; + echo_client client(io_context); + echo_server server(io_context); + + client.stream.next_layer().connect(server.stream.next_layer()); + + auto handshake_result = server.handshake(); + client.handshake(); + REQUIRE_FALSE(handshake_result.get()); + + server.stream.next_layer().close(); + CHECK(client.launch_shutdown().get() == stream_truncated_error(client.stream)); + } + + SECTION("okay") { + net::io_context io_context; + echo_client client(io_context); + echo_server server(io_context); + + client.stream.next_layer().connect(server.stream.next_layer()); + + auto handshake_result = server.handshake(); + client.handshake(); + REQUIRE_FALSE(handshake_result.get()); + + auto shutdown_result = server.shutdown(); + client.shutdown(); + // according to https://stackoverflow.com/a/25703699/16814536 this should produce asio::error::eof + // is that information outdated? + CHECK_FALSE(shutdown_result.get()); + } +}