diff options
| -rw-r--r-- | Makefile | 3 | ||||
| -rw-r--r-- | debian/changelog | 6 | ||||
| -rw-r--r-- | error.cpp | 32 | ||||
| -rw-r--r-- | error.h | 6 | ||||
| -rw-r--r-- | http.cpp | 27 | ||||
| -rw-r--r-- | tests/Makefile | 3 | ||||
| -rw-r--r-- | webserver.conf | 6 | ||||
| -rw-r--r-- | websocket.cpp | 30 | ||||
| -rw-r--r-- | websocket.h | 1 |
9 files changed, 59 insertions, 55 deletions
@@ -39,7 +39,6 @@ LDFLAGS+=-pie PROGSRC=\ auth.cpp \ config.cpp \ - error.cpp \ http.cpp \ plugin.cpp \ privileges.cpp \ @@ -150,8 +149,6 @@ DISTFILES= \ debian/webserver.install \ debian/webserver.manpages \ debian/webserver.service \ - error.cpp \ - error.h \ http.cpp \ http.h \ install-webserver.sh \ diff --git a/debian/changelog b/debian/changelog index fda1b28..85d40eb 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +webserver (1.19) UNRELEASED; urgency=medium + + * Websocket bugfix: close remaining connection, prevent leak + + -- Roland Reichwein <mail@reichwein.it> Thu, 26 Jan 2023 19:11:21 +0100 + webserver (1.18) unstable; urgency=medium * Added websockets support (configurable forwarding proxy) diff --git a/error.cpp b/error.cpp deleted file mode 100644 index d7a26de..0000000 --- a/error.cpp +++ /dev/null @@ -1,32 +0,0 @@ -#include "error.h" - -#include <iostream> - -#include <boost/asio/ssl/error.hpp> - -// Report a failure -void fail(boost::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 == boost::asio::ssl::error::stream_truncated) - return; - - std::cerr << what << ": " << ec.message() << "\n"; -} - diff --git a/error.h b/error.h deleted file mode 100644 index f7a9da3..0000000 --- a/error.h +++ /dev/null @@ -1,6 +0,0 @@ -#pragma once - -#include <boost/beast/core/error.hpp> - -void fail(boost::beast::error_code ec, char const* what); - @@ -1,7 +1,6 @@ #include "http.h" #include "config.h" -#include "error.h" #include "server.h" #include "response.h" #include "websocket.h" @@ -46,6 +45,32 @@ using namespace Reichwein; namespace { + // Report a failure +void fail(boost::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 == boost::asio::ssl::error::stream_truncated) + return; + + std::cerr << what << ": " << ec.message() << "\n"; +} + // Handles an HTTP server connection template<class Derived> class session diff --git a/tests/Makefile b/tests/Makefile index 898afbf..c88a285 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -36,7 +36,6 @@ LDFLAGS+=-pie UNITS=\ auth.cpp \ config.cpp \ - error.cpp \ fastcgiprocess.cpp \ http.cpp \ plugin.cpp \ @@ -91,8 +90,6 @@ auth.o: ../auth.cpp $(CXX) $(CXXFLAGS) $(CXXTESTFLAGS) -c $< -o $@ config.o: ../config.cpp $(CXX) $(CXXFLAGS) $(CXXTESTFLAGS) -c $< -o $@ -error.o: ../error.cpp - $(CXX) $(CXXFLAGS) $(CXXTESTFLAGS) -c $< -o $@ fastcgiprocess.o: ../plugins/fcgi/fastcgiprocess.cpp $(CXX) $(CXXFLAGS) $(CXXTESTFLAGS) -c $< -o $@ http.o: ../http.cpp diff --git a/webserver.conf b/webserver.conf index a00549f..598a57c 100644 --- a/webserver.conf +++ b/webserver.conf @@ -99,9 +99,9 @@ <plugin>static-files</plugin> <target>/home/ernie/code/whiteboard/html</target> </path> - <path requested="/whiteboard/whiteboard.fcgi"> - <plugin>fcgi</plugin> - <target>127.0.0.1:9014</target> + <path requested="/whiteboard/websocket"> + <plugin>websocket</plugin> + <target>::1:9014</target> </path> <path requested="/redirect1"> diff --git a/websocket.cpp b/websocket.cpp index 68ff194..d361ec1 100644 --- a/websocket.cpp +++ b/websocket.cpp @@ -65,6 +65,20 @@ public: relative_target_ = websocket_address.substr(slash_pos); } + void fail(boost::beast::error_code ec, char const* what) + { + if (ec == websocket::error::closed) + return; + + boost::beast::error_code ec2; + if (auto& ws_in{derived().ws_in()}; ws_in.is_open()) + ws_in.close(beast::websocket::close_code::going_away, ec2); + if (auto& ws_app{ws_app_}; ws_app.is_open()) + ws_app.close(beast::websocket::close_code::going_away, ec2); + + std::cerr << what << ": " << ec.message() << "\n"; + } + // // The initial setup path // @@ -181,8 +195,10 @@ private: if (ec == websocket::error::closed) return; - if (ec) - fail(ec, "read in"); + if (ec) { + // not error::closed as above, but no other strategy known + return fail(ec, "read in"); + } ws_app_.text(derived().ws_in().got_text()); @@ -202,7 +218,7 @@ private: boost::ignore_unused(bytes_transferred); if (ec) - fail(ec, "write app"); + return fail(ec, "write app"); buffer_in_.consume(buffer_in_.size()); @@ -231,8 +247,10 @@ private: if (ec == websocket::error::closed) return; - if (ec) - fail(ec, "read app"); + if (ec) { + // not error::closed as above, but no other strategy known + return fail(ec, "read app"); + } do_write_out(); } @@ -251,7 +269,7 @@ private: { boost::ignore_unused(bytes_transferred); - if(ec) + if (ec) return fail(ec, "write out"); // Clear the buffer diff --git a/websocket.h b/websocket.h index 87b5a04..5306b9b 100644 --- a/websocket.h +++ b/websocket.h @@ -3,7 +3,6 @@ // #pragma once -#include "error.h" #include "response.h" #include <boost/asio/buffer.hpp> |