Better treatment of SSL short reads:

fix #38

This improves the behavior when encountering a short read:

* Any stream error encountered during a read is converting into
  `http::error::partial_message` if some data was received but
  the message is incomplete.

* Examples squelch SSL short read errors from the logs.
This commit is contained in:
Vinnie Falco
2019-02-23 06:34:52 -08:00
parent d4dddec1c0
commit 094f5ec5cb
11 changed files with 124 additions and 5 deletions

View File

@@ -2,6 +2,7 @@ Version 219:
* More split definitions in test::stream
* Visual Studio 2017 minimum requirement for Windows
* Better treatment of SSL short reads
--------------------------------------------------------------------------------

View File

@@ -127,7 +127,6 @@
<simplelist type="vert" columns="1">
<member><link linkend="beast.ref.boost__beast__basic_flat_buffer">basic_flat_buffer</link></member>
<member><link linkend="beast.ref.boost__beast__basic_multi_buffer">basic_multi_buffer</link></member>
<member><link linkend="beast.ref.boost__beast__buffer_size">buffer_size</link>&nbsp;<emphasis role="green">&#128946;</emphasis></member>
<member><link linkend="beast.ref.boost__beast__buffered_read_stream">buffered_read_stream</link></member>
<member><link linkend="beast.ref.boost__beast__buffers_adaptor">buffers_adaptor</link></member>
<member><link linkend="beast.ref.boost__beast__buffers_cat_view">buffers_cat_view</link></member>
@@ -141,6 +140,7 @@
</entry><entry valign="top">
<bridgehead renderas="sect3">Functions</bridgehead>
<simplelist type="vert" columns="1">
<member><link linkend="beast.ref.boost__beast__buffer_size">buffer_size</link>&nbsp;<emphasis role="green">&#128946;</emphasis></member>
<member><link linkend="beast.ref.boost__beast__buffers_cat">buffers_cat</link></member>
<member><link linkend="beast.ref.boost__beast__buffers_front">buffers_front</link></member>
<member><link linkend="beast.ref.boost__beast__buffers_prefix">buffers_prefix</link></member>

View File

@@ -218,6 +218,26 @@ handle_request(
void
fail(beast::error_code ec, char const* what)
{
// ssl::error::stream_truncated, also known as an SSL "short read",
// indicates the peer closed the connection without performing the
// required closing handshake (for example, Google does this to
// improve performance). Generally this can be a security issue,
// but if your communication protocol is self-terminated (as
// it is with both HTTP and WebSocket) then you may simply
// ignore the lack of close_notify.
//
// https://github.com/boostorg/beast/issues/38
//
// https://security.stackexchange.com/questions/91435/how-to-handle-a-malicious-ssl-tls-shutdown
//
// When a short read would cut off the end of an HTTP message,
// Beast returns the error beast::http::error::partial_message.
// Therefore, if we see a short read here, it has occurred
// after the message has been completed, so it is safe to ignore it.
if(ec == net::ssl::error::stream_truncated)
return;
std::cerr << what << ": " << ec.message() << "\n";
}

View File

@@ -212,6 +212,26 @@ handle_request(
void
fail(beast::error_code ec, char const* what)
{
// ssl::error::stream_truncated, also known as an SSL "short read",
// indicates the peer closed the connection without performing the
// required closing handshake (for example, Google does this to
// improve performance). Generally this can be a security issue,
// but if your communication protocol is self-terminated (as
// it is with both HTTP and WebSocket) then you may simply
// ignore the lack of close_notify.
//
// https://github.com/boostorg/beast/issues/38
//
// https://security.stackexchange.com/questions/91435/how-to-handle-a-malicious-ssl-tls-shutdown
//
// When a short read would cut off the end of an HTTP message,
// Beast returns the error beast::http::error::partial_message.
// Therefore, if we see a short read here, it has occurred
// after the message has been completed, so it is safe to ignore it.
if(ec == net::ssl::error::stream_truncated)
return;
std::cerr << what << ": " << ec.message() << "\n";
}

View File

@@ -211,6 +211,26 @@ handle_request(
void
fail(beast::error_code ec, char const* what)
{
// ssl::error::stream_truncated, also known as an SSL "short read",
// indicates the peer closed the connection without performing the
// required closing handshake (for example, Google does this to
// improve performance). Generally this can be a security issue,
// but if your communication protocol is self-terminated (as
// it is with both HTTP and WebSocket) then you may simply
// ignore the lack of close_notify.
//
// https://github.com/boostorg/beast/issues/38
//
// https://security.stackexchange.com/questions/91435/how-to-handle-a-malicious-ssl-tls-shutdown
//
// When a short read would cut off the end of an HTTP message,
// Beast returns the error beast::http::error::partial_message.
// Therefore, if we see a short read here, it has occurred
// after the message has been completed, so it is safe to ignore it.
if(ec == net::ssl::error::stream_truncated)
return;
std::cerr << what << ": " << ec.message() << "\n";
}

View File

@@ -211,6 +211,26 @@ handle_request(
void
fail(beast::error_code ec, char const* what)
{
// ssl::error::stream_truncated, also known as an SSL "short read",
// indicates the peer closed the connection without performing the
// required closing handshake (for example, Google does this to
// improve performance). Generally this can be a security issue,
// but if your communication protocol is self-terminated (as
// it is with both HTTP and WebSocket) then you may simply
// ignore the lack of close_notify.
//
// https://github.com/boostorg/beast/issues/38
//
// https://security.stackexchange.com/questions/91435/how-to-handle-a-malicious-ssl-tls-shutdown
//
// When a short read would cut off the end of an HTTP message,
// Beast returns the error beast::http::error::partial_message.
// Therefore, if we see a short read here, it has occurred
// after the message has been completed, so it is safe to ignore it.
if(ec == net::ssl::error::stream_truncated)
return;
std::cerr << what << ": " << ec.message() << "\n";
}

View File

@@ -213,6 +213,26 @@ handle_request(
void
fail(beast::error_code ec, char const* what)
{
// ssl::error::stream_truncated, also known as an SSL "short read",
// indicates the peer closed the connection without performing the
// required closing handshake (for example, Google does this to
// improve performance). Generally this can be a security issue,
// but if your communication protocol is self-terminated (as
// it is with both HTTP and WebSocket) then you may simply
// ignore the lack of close_notify.
//
// https://github.com/boostorg/beast/issues/38
//
// https://security.stackexchange.com/questions/91435/how-to-handle-a-malicious-ssl-tls-shutdown
//
// When a short read would cut off the end of an HTTP message,
// Beast returns the error beast::http::error::partial_message.
// Therefore, if we see a short read here, it has occurred
// after the message has been completed, so it is safe to ignore it.
if(ec == net::ssl::error::stream_truncated)
return;
std::cerr << what << ": " << ec.message() << "\n";
}

View File

@@ -43,7 +43,7 @@ parse_until(
{
if(parser.got_some())
{
// caller sees EOF on next read
// Caller sees EOF on next read
ec = {};
parser.put_eof(ec);
BOOST_ASSERT(ec || parser.is_done());
@@ -55,7 +55,17 @@ parse_until(
return 0;
}
if(ec)
{
// Upgrade the error if we have a partial message.
// This causes SSL short reads (and every other error)
// to be converted into something else, allowing the
// caller to distinguish an SSL short read which
// represents a safe connection closure, versus
// a closure with data loss.
if(parser.got_some() && ! parser.is_done())
ec = error::partial_message;
return 0;
}
if(parser.is_done())
return 0;
if(buffer.size() > 0)

View File

@@ -105,6 +105,7 @@ public:
// What else could it be?
BOOST_ASSERT(sr_.is_done());
}
return net::post(
s_.get_executor(),
beast::bind_front_handler(

View File

@@ -18,10 +18,10 @@ namespace websocket {
/*
See
http://stackoverflow.com/questions/32046034/what-is-the-proper-way-to-securely-disconnect-an-asio-ssl-socket/32054476#32054476
See
http://stackoverflow.com/questions/32046034/what-is-the-proper-way-to-securely-disconnect-an-asio-ssl-socket/32054476#32054476
Behavior of ssl::stream regarding close_
Behavior of ssl::stream regarding close_notify
If the remote host calls async_shutdown then the
local host's async_read will complete with eof.

View File

@@ -73,10 +73,17 @@ public:
}
catch(system_error const& se)
{
// VFALCO Commented this out after the short
// read change, because it converts test_failure
// into http::partial_message
//
boost::ignore_unused(se);
#if 0
if(! BEAST_EXPECTS(
se.code() == test::error::test_failure,
se.code().message()))
throw;
#endif
if(! BEAST_EXPECTS(
clock_type::now() < expires_at,
"a test timeout occurred"))