From 26fc986d8ea64b29d9f8f34c0bc4cec08cc53032 Mon Sep 17 00:00:00 2001 From: Jens Diewald Date: Tue, 4 Apr 2023 21:08:28 +0200 Subject: [PATCH] Implement full shutdown WinTLS will now produce asio::error::eof when the underlying stream gets closed without the peer sending its close notify. This should be changed to a separate error code analogously to Asio.SSL. --- include/boost/wintls/detail/sspi_handshake.hpp | 16 +++++++++++++++- test/shutdown_test.cpp | 2 +- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/include/boost/wintls/detail/sspi_handshake.hpp b/include/boost/wintls/detail/sspi_handshake.hpp index ddebb8d3..92fb96bf 100644 --- a/include/boost/wintls/detail/sspi_handshake.hpp +++ b/include/boost/wintls/detail/sspi_handshake.hpp @@ -168,6 +168,12 @@ class sspi_handshake { 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()() { @@ -245,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}; } @@ -270,6 +276,14 @@ class sspi_handshake { 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; + } + case SEC_I_INCOMPLETE_CREDENTIALS: BOOST_ASSERT_MSG(false, "client authentication not implemented"); diff --git a/test/shutdown_test.cpp b/test/shutdown_test.cpp index d5ca6fe1..1290623b 100644 --- a/test/shutdown_test.cpp +++ b/test/shutdown_test.cpp @@ -24,7 +24,7 @@ auto stream_truncated_error(const asio_ssl::stream&) { template auto stream_truncated_error(const boost::wintls::stream&) { - return boost::system::errc::success; // #TODO: This should be an error + return net::error::eof; // #TODO: This should be a separate error code. } using TestTypes = std::tuple,