Skip to content

Commit

Permalink
Implement full shutdown
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jens-diewald committed Apr 4, 2023
1 parent 0d765c4 commit 26fc986
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 2 deletions.
16 changes: 15 additions & 1 deletion include/boost/wintls/detail/sspi_handshake.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()() {
Expand Down Expand Up @@ -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};
}

Expand All @@ -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");

Expand Down
2 changes: 1 addition & 1 deletion test/shutdown_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ auto stream_truncated_error(const asio_ssl::stream<NextLayer>&) {

template<typename NextLayer>
auto stream_truncated_error(const boost::wintls::stream<NextLayer>&) {
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<std::tuple<asio_ssl_client_stream, asio_ssl_server_stream>,
Expand Down

0 comments on commit 26fc986

Please sign in to comment.