From 2789d1a8592b910292d1eb54ffd62b34ac7091ed Mon Sep 17 00:00:00 2001 From: Gunnar Beutner Date: Wed, 31 Jan 2018 07:59:49 +0100 Subject: [PATCH 1/9] Add validation for HTTP connection sizes --- lib/remote/httpchunkedencoding.cpp | 2 ++ lib/remote/httprequest.cpp | 7 ++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/remote/httpchunkedencoding.cpp b/lib/remote/httpchunkedencoding.cpp index e79d3483a..9981749c2 100644 --- a/lib/remote/httpchunkedencoding.cpp +++ b/lib/remote/httpchunkedencoding.cpp @@ -37,6 +37,8 @@ StreamReadStatus HttpChunkedEncoding::ReadChunkFromStream(const Stream::Ptr& str msgbuf << std::hex << line; msgbuf >> context.LengthIndicator; + if (context.LengthIndicator < 0) + BOOST_THROW_EXCEPTION(std::invalid_argument("HTTP chunk length must not be negative.")); } StreamReadContext& scontext = context.StreamContext; diff --git a/lib/remote/httprequest.cpp b/lib/remote/httprequest.cpp index 546728d66..b85a3d0ec 100644 --- a/lib/remote/httprequest.cpp +++ b/lib/remote/httprequest.cpp @@ -126,7 +126,12 @@ bool HttpRequest::Parse(StreamReadContext& src, bool may_wait) src.MustRead = false; } - size_t length_indicator = Convert::ToLong(Headers->Get("content-length")); + long length_indicator_signed = Convert::ToLong(Headers->Get("content-length")); + + if (length_indicator_signed < 0) + BOOST_THROW_EXCEPTION(std::invalid_argument("Content-Length must not be negative.")); + + size_t length_indicator = length_indicator_signed; if (src.Size < length_indicator) { src.MustRead = true; From a9f2a8de19a0740ccc5c82ca9ccadf4e1b819890 Mon Sep 17 00:00:00 2001 From: Gunnar Beutner Date: Wed, 31 Jan 2018 09:05:06 +0100 Subject: [PATCH 2/9] Add HTTP Header size limits --- lib/remote/httprequest.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/remote/httprequest.cpp b/lib/remote/httprequest.cpp index b85a3d0ec..11480d86a 100644 --- a/lib/remote/httprequest.cpp +++ b/lib/remote/httprequest.cpp @@ -41,8 +41,15 @@ bool HttpRequest::Parse(StreamReadContext& src, bool may_wait) String line; StreamReadStatus srs = m_Stream->ReadLine(&line, src, may_wait); - if (srs != StatusNewItem) + if (srs != StatusNewItem) { + if (src.Size > 512) + BOOST_THROW_EXCEPTION(std::invalid_argument("Line length for HTTP header exceeded")); + return false; + } + + if (line.GetLength() > 512) + BOOST_THROW_EXCEPTION(std::invalid_argument("Line length for HTTP header exceeded")); if (m_State == HttpRequestStart) { /* ignore trailing new-lines */ @@ -79,6 +86,9 @@ bool HttpRequest::Parse(StreamReadContext& src, bool may_wait) return true; } else { + if (Headers->GetLength() > 128) + BOOST_THROW_EXCEPTION(std::invalid_argument("Maximum number of HTTP request headers exceeded")); + String::SizeType pos = line.FindFirstOf(":"); if (pos == String::NPos) BOOST_THROW_EXCEPTION(std::invalid_argument("Invalid HTTP request")); From 81c4004894f19d6ad39c5b7bc48c0d6ac97af515 Mon Sep 17 00:00:00 2001 From: Jean Flach Date: Tue, 30 Jan 2018 13:34:26 +0100 Subject: [PATCH 3/9] Fix nullptr deref --- lib/remote/httpserverconnection.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/remote/httpserverconnection.cpp b/lib/remote/httpserverconnection.cpp index 1b19635a4..39cb397a6 100644 --- a/lib/remote/httpserverconnection.cpp +++ b/lib/remote/httpserverconnection.cpp @@ -165,9 +165,12 @@ void HttpServerConnection::ProcessMessageAsync(HttpRequest& request) String requestUrl = request.RequestUrl->Format(); + Socket::Ptr socket = m_Stream->GetSocket(); + Log(LogInformation, "HttpServerConnection") << "Request: " << request.RequestMethod << " " << requestUrl - << " (from " << m_Stream->GetSocket()->GetPeerAddress() << ", user: " << (user ? user->GetName() : "") << ")"; + << " (from " << (socket ? socket->GetPeerAddress() : "") + << ", user: " << (user ? user->GetName() : "") << ")"; HttpResponse response(m_Stream, request); From 73b85bcccbb2c7239c4f43d9d1157a82d6786b98 Mon Sep 17 00:00:00 2001 From: Jean Flach Date: Thu, 1 Feb 2018 15:10:28 +0100 Subject: [PATCH 4/9] Only read body from authenticated connections This means we are not allowing unauthenticated requests anymore --- lib/remote/httprequest.cpp | 239 +++++++++++++++------------- lib/remote/httprequest.hpp | 6 +- lib/remote/httpserverconnection.cpp | 48 +++--- 3 files changed, 160 insertions(+), 133 deletions(-) diff --git a/lib/remote/httprequest.cpp b/lib/remote/httprequest.cpp index 11480d86a..0d6c4a618 100644 --- a/lib/remote/httprequest.cpp +++ b/lib/remote/httprequest.cpp @@ -25,134 +25,147 @@ using namespace icinga; HttpRequest::HttpRequest(Stream::Ptr stream) - : Complete(false), + : CompleteHeaders(false), + CompleteBody(false), ProtocolVersion(HttpVersion11), Headers(new Dictionary()), m_Stream(std::move(stream)), m_State(HttpRequestStart) { } -bool HttpRequest::Parse(StreamReadContext& src, bool may_wait) +bool HttpRequest::ParseHeader(StreamReadContext& src, bool may_wait) { if (!m_Stream) return false; - if (m_State != HttpRequestBody) { - String line; - StreamReadStatus srs = m_Stream->ReadLine(&line, src, may_wait); + if (m_State != HttpRequestStart && m_State != HttpRequestHeaders) + return false; - if (srs != StatusNewItem) { - if (src.Size > 512) - BOOST_THROW_EXCEPTION(std::invalid_argument("Line length for HTTP header exceeded")); + String line; + StreamReadStatus srs = m_Stream->ReadLine(&line, src, may_wait); + if (srs != StatusNewItem) { + if (src.Size > 512) + BOOST_THROW_EXCEPTION(std::invalid_argument("Line length for HTTP header exceeded")); + + return false; + } + + if (line.GetLength() > 512) + BOOST_THROW_EXCEPTION(std::invalid_argument("Line length for HTTP header exceeded")); + + if (m_State == HttpRequestStart) { + /* ignore trailing new-lines */ + if (line == "") + return true; + + std::vector tokens = line.Split(" "); + Log(LogDebug, "HttpRequest") + << "line: " << line << ", tokens: " << tokens.size(); + if (tokens.size() != 3) + BOOST_THROW_EXCEPTION(std::invalid_argument("Invalid HTTP request")); + + RequestMethod = tokens[0]; + RequestUrl = new class Url(tokens[1]); + + if (tokens[2] == "HTTP/1.0") + ProtocolVersion = HttpVersion10; + else if (tokens[2] == "HTTP/1.1") { + ProtocolVersion = HttpVersion11; + } else + BOOST_THROW_EXCEPTION(std::invalid_argument("Unsupported HTTP version")); + + m_State = HttpRequestHeaders; + return true; + } else { // m_State = HttpRequestHeaders + if (line == "") { + m_State = HttpRequestBody; + CompleteHeaders = true; + return true; + + } else { + if (Headers->GetLength() > 128) + BOOST_THROW_EXCEPTION(std::invalid_argument("Maximum number of HTTP request headers exceeded")); + + String::SizeType pos = line.FindFirstOf(":"); + if (pos == String::NPos) + BOOST_THROW_EXCEPTION(std::invalid_argument("Invalid HTTP request")); + + String key = line.SubStr(0, pos).ToLower().Trim(); + String value = line.SubStr(pos + 1).Trim(); + Headers->Set(key, value); + + if (key == "x-http-method-override") + RequestMethod = value; + + return true; + } + } +} + +bool HttpRequest::ParseBody(StreamReadContext& src, bool may_wait) +{ + if (!m_Stream || m_State != HttpRequestBody) + return false; + + /* we're done if the request doesn't contain a message body */ + if (!Headers->Contains("content-length") && !Headers->Contains("transfer-encoding")) { + CompleteBody = true; + return true; + } else if (!m_Body) + m_Body = new FIFO(); + + if (CompleteBody) + return true; + + if (Headers->Get("transfer-encoding") == "chunked") { + if (!m_ChunkContext) + m_ChunkContext = std::make_shared(std::ref(src)); + + char *data; + size_t size; + StreamReadStatus srs = HttpChunkedEncoding::ReadChunkFromStream(m_Stream, &data, &size, *m_ChunkContext.get(), may_wait); + + if (srs != StatusNewItem) + return false; + + m_Body->Write(data, size); + + delete [] data; + + if (size == 0) { + CompleteBody = true; + return true; + } + } else { + if (src.Eof) + BOOST_THROW_EXCEPTION(std::invalid_argument("Unexpected EOF in HTTP body")); + + if (src.MustRead) { + if (!src.FillFromStream(m_Stream, false)) { + src.Eof = true; + BOOST_THROW_EXCEPTION(std::invalid_argument("Unexpected EOF in HTTP body")); + } + + src.MustRead = false; + } + + long length_indicator_signed = Convert::ToLong(Headers->Get("content-length")); + + if (length_indicator_signed < 0) + BOOST_THROW_EXCEPTION(std::invalid_argument("Content-Length must not be negative.")); + + size_t length_indicator = length_indicator_signed; + + if (src.Size < length_indicator) { + src.MustRead = true; return false; } - if (line.GetLength() > 512) - BOOST_THROW_EXCEPTION(std::invalid_argument("Line length for HTTP header exceeded")); - - if (m_State == HttpRequestStart) { - /* ignore trailing new-lines */ - if (line == "") - return true; - - std::vector tokens = line.Split(" "); - Log(LogDebug, "HttpRequest") - << "line: " << line << ", tokens: " << tokens.size(); - if (tokens.size() != 3) - BOOST_THROW_EXCEPTION(std::invalid_argument("Invalid HTTP request")); - - RequestMethod = tokens[0]; - RequestUrl = new class Url(tokens[1]); - - if (tokens[2] == "HTTP/1.0") - ProtocolVersion = HttpVersion10; - else if (tokens[2] == "HTTP/1.1") { - ProtocolVersion = HttpVersion11; - } else - BOOST_THROW_EXCEPTION(std::invalid_argument("Unsupported HTTP version")); - - m_State = HttpRequestHeaders; - } else if (m_State == HttpRequestHeaders) { - if (line == "") { - m_State = HttpRequestBody; - - /* we're done if the request doesn't contain a message body */ - if (!Headers->Contains("content-length") && !Headers->Contains("transfer-encoding")) - Complete = true; - else - m_Body = new FIFO(); - - return true; - - } else { - if (Headers->GetLength() > 128) - BOOST_THROW_EXCEPTION(std::invalid_argument("Maximum number of HTTP request headers exceeded")); - - String::SizeType pos = line.FindFirstOf(":"); - if (pos == String::NPos) - BOOST_THROW_EXCEPTION(std::invalid_argument("Invalid HTTP request")); - - String key = line.SubStr(0, pos).ToLower().Trim(); - String value = line.SubStr(pos + 1).Trim(); - Headers->Set(key, value); - - if (key == "x-http-method-override") - RequestMethod = value; - } - } else { - VERIFY(!"Invalid HTTP request state."); - } - } else if (m_State == HttpRequestBody) { - if (Headers->Get("transfer-encoding") == "chunked") { - if (!m_ChunkContext) - m_ChunkContext = std::make_shared(std::ref(src)); - - char *data; - size_t size; - StreamReadStatus srs = HttpChunkedEncoding::ReadChunkFromStream(m_Stream, &data, &size, *m_ChunkContext.get(), may_wait); - - if (srs != StatusNewItem) - return false; - - m_Body->Write(data, size); - - delete [] data; - - if (size == 0) { - Complete = true; - return true; - } - } else { - if (src.Eof) - BOOST_THROW_EXCEPTION(std::invalid_argument("Unexpected EOF in HTTP body")); - - if (src.MustRead) { - if (!src.FillFromStream(m_Stream, false)) { - src.Eof = true; - BOOST_THROW_EXCEPTION(std::invalid_argument("Unexpected EOF in HTTP body")); - } - - src.MustRead = false; - } - - long length_indicator_signed = Convert::ToLong(Headers->Get("content-length")); - - if (length_indicator_signed < 0) - BOOST_THROW_EXCEPTION(std::invalid_argument("Content-Length must not be negative.")); - - size_t length_indicator = length_indicator_signed; - - if (src.Size < length_indicator) { - src.MustRead = true; - return false; - } - - m_Body->Write(src.Buffer, length_indicator); - src.DropData(length_indicator); - Complete = true; - return true; - } + m_Body->Write(src.Buffer, length_indicator); + src.DropData(length_indicator); + CompleteBody = true; + return true; } return true; diff --git a/lib/remote/httprequest.hpp b/lib/remote/httprequest.hpp index b59917283..e8591474c 100644 --- a/lib/remote/httprequest.hpp +++ b/lib/remote/httprequest.hpp @@ -52,7 +52,8 @@ enum HttpRequestState struct HttpRequest { public: - bool Complete; + bool CompleteHeaders; + bool CompleteBody; String RequestMethod; Url::Ptr RequestUrl; @@ -62,7 +63,8 @@ public: HttpRequest(Stream::Ptr stream); - bool Parse(StreamReadContext& src, bool may_wait); + bool ParseHeader(StreamReadContext& src, bool may_wait); + bool ParseBody(StreamReadContext& src, bool may_wait); size_t ReadBody(char *data, size_t count); void AddHeader(const String& key, const String& value); diff --git a/lib/remote/httpserverconnection.cpp b/lib/remote/httpserverconnection.cpp index 39cb397a6..9e400da16 100644 --- a/lib/remote/httpserverconnection.cpp +++ b/lib/remote/httpserverconnection.cpp @@ -92,7 +92,7 @@ bool HttpServerConnection::ProcessMessage() bool res; try { - res = m_CurrentRequest.Parse(m_Context, false); + res = m_CurrentRequest.ParseHeader(m_Context, false); } catch (const std::invalid_argument& ex) { HttpResponse response(m_Stream, m_CurrentRequest); response.SetStatus(400, "Bad request"); @@ -113,7 +113,7 @@ bool HttpServerConnection::ProcessMessage() return false; } - if (m_CurrentRequest.Complete) { + if (m_CurrentRequest.CompleteHeaders) { m_RequestQueue.Enqueue(std::bind(&HttpServerConnection::ProcessMessageAsync, HttpServerConnection::Ptr(this), m_CurrentRequest)); @@ -241,25 +241,37 @@ void HttpServerConnection::ProcessMessageAsync(HttpRequest& request) response.WriteBody(msg.CStr(), msg.GetLength()); } } else { - try { - HttpHandler::ProcessRequest(user, request, response); - } catch (const std::exception& ex) { - Log(LogCritical, "HttpServerConnection") - << "Unhandled exception while processing Http request: " << DiagnosticInformation(ex); - response.SetStatus(503, "Unhandled exception"); + bool res = true; + while (!request.CompleteBody) + res = request.ParseBody(m_Context, false); + if (!res) { + Log(LogCritical, "HttpServerConnection", "Failed to read body"); + Dictionary::Ptr result = new Dictionary({ + { "error", 400 }, + { "status", "Bad Request: Malformed body." } + }); + HttpUtility::SendJsonBody(response, nullptr, result); + } else { + try { + HttpHandler::ProcessRequest(user, request, response); + } catch (const std::exception& ex) { + Log(LogCritical, "HttpServerConnection") + << "Unhandled exception while processing Http request: " << DiagnosticInformation(ex); + response.SetStatus(503, "Unhandled exception"); - String errorInfo = DiagnosticInformation(ex); + String errorInfo = DiagnosticInformation(ex); - if (request.Headers->Get("accept") == "application/json") { - Dictionary::Ptr result = new Dictionary({ - { "error", 503 }, - { "status", errorInfo } - }); + if (request.Headers->Get("accept") == "application/json") { + Dictionary::Ptr result = new Dictionary({ + { "error", 503 }, + { "status", errorInfo } + }); - HttpUtility::SendJsonBody(response, nullptr, result); - } else { - response.AddHeader("Content-Type", "text/plain"); - response.WriteBody(errorInfo.CStr(), errorInfo.GetLength()); + HttpUtility::SendJsonBody(response, nullptr, result); + } else { + response.AddHeader("Content-Type", "text/plain"); + response.WriteBody(errorInfo.CStr(), errorInfo.GetLength()); + } } } } From ee5954726dc1c054ac952b3a7e45e2ac972cfb0b Mon Sep 17 00:00:00 2001 From: Jean Flach Date: Thu, 8 Feb 2018 14:54:52 +0100 Subject: [PATCH 5/9] Authenticate API user before parsing body --- lib/remote/apiuser.cpp | 28 ++++ lib/remote/apiuser.hpp | 1 + lib/remote/httprequest.cpp | 72 +++++----- lib/remote/httprequest.hpp | 1 + lib/remote/httpserverconnection.cpp | 210 +++++++++++++--------------- lib/remote/httpserverconnection.hpp | 6 +- 6 files changed, 169 insertions(+), 149 deletions(-) diff --git a/lib/remote/apiuser.cpp b/lib/remote/apiuser.cpp index 183291af3..416eedb34 100644 --- a/lib/remote/apiuser.cpp +++ b/lib/remote/apiuser.cpp @@ -20,6 +20,7 @@ #include "remote/apiuser.hpp" #include "remote/apiuser-ti.cpp" #include "base/configtype.hpp" +#include "base/base64.hpp" using namespace icinga; @@ -34,3 +35,30 @@ ApiUser::Ptr ApiUser::GetByClientCN(const String& cn) return nullptr; } + +ApiUser::Ptr ApiUser::GetByAuthHeader(const String& auth_header) { + String::SizeType pos = auth_header.FindFirstOf(" "); + String username, password; + + if (pos != String::NPos && auth_header.SubStr(0, pos) == "Basic") { + String credentials_base64 = auth_header.SubStr(pos + 1); + String credentials = Base64::Decode(credentials_base64); + + String::SizeType cpos = credentials.FindFirstOf(":"); + + if (cpos != String::NPos) { + username = credentials.SubStr(0, cpos); + password = credentials.SubStr(cpos + 1); + } + } + + const ApiUser::Ptr& user = ApiUser::GetByName(username); + + /* Deny authentication if 1) given password is empty 2) configured password does not match. */ + if (password.IsEmpty()) + return nullptr; + else if (user && user->GetPassword() != password) + return nullptr; + + return user; +} diff --git a/lib/remote/apiuser.hpp b/lib/remote/apiuser.hpp index 755273bf4..15b1c41e3 100644 --- a/lib/remote/apiuser.hpp +++ b/lib/remote/apiuser.hpp @@ -36,6 +36,7 @@ public: DECLARE_OBJECTNAME(ApiUser); static ApiUser::Ptr GetByClientCN(const String& cn); + static ApiUser::Ptr GetByAuthHeader(const String& auth_header); }; } diff --git a/lib/remote/httprequest.cpp b/lib/remote/httprequest.cpp index 0d6c4a618..c5382d254 100644 --- a/lib/remote/httprequest.cpp +++ b/lib/remote/httprequest.cpp @@ -26,6 +26,7 @@ using namespace icinga; HttpRequest::HttpRequest(Stream::Ptr stream) : CompleteHeaders(false), + CompleteHeaderCheck(false), CompleteBody(false), ProtocolVersion(HttpVersion11), Headers(new Dictionary()), @@ -39,7 +40,7 @@ bool HttpRequest::ParseHeader(StreamReadContext& src, bool may_wait) return false; if (m_State != HttpRequestStart && m_State != HttpRequestHeaders) - return false; + BOOST_THROW_EXCEPTION(std::runtime_error("Invalid HTTP state")); String line; StreamReadStatus srs = m_Stream->ReadLine(&line, src, may_wait); @@ -105,19 +106,19 @@ bool HttpRequest::ParseHeader(StreamReadContext& src, bool may_wait) bool HttpRequest::ParseBody(StreamReadContext& src, bool may_wait) { - if (!m_Stream || m_State != HttpRequestBody) + if (!m_Stream) return false; + if (m_State != HttpRequestBody) + BOOST_THROW_EXCEPTION(std::runtime_error("Invalid HTTP state")); + /* we're done if the request doesn't contain a message body */ if (!Headers->Contains("content-length") && !Headers->Contains("transfer-encoding")) { CompleteBody = true; - return true; + return false; } else if (!m_Body) m_Body = new FIFO(); - if (CompleteBody) - return true; - if (Headers->Get("transfer-encoding") == "chunked") { if (!m_ChunkContext) m_ChunkContext = std::make_shared(std::ref(src)); @@ -135,39 +136,38 @@ bool HttpRequest::ParseBody(StreamReadContext& src, bool may_wait) if (size == 0) { CompleteBody = true; - return true; - } - } else { - if (src.Eof) - BOOST_THROW_EXCEPTION(std::invalid_argument("Unexpected EOF in HTTP body")); - - if (src.MustRead) { - if (!src.FillFromStream(m_Stream, false)) { - src.Eof = true; - BOOST_THROW_EXCEPTION(std::invalid_argument("Unexpected EOF in HTTP body")); - } - - src.MustRead = false; - } - - long length_indicator_signed = Convert::ToLong(Headers->Get("content-length")); - - if (length_indicator_signed < 0) - BOOST_THROW_EXCEPTION(std::invalid_argument("Content-Length must not be negative.")); - - size_t length_indicator = length_indicator_signed; - - if (src.Size < length_indicator) { - src.MustRead = true; return false; - } - - m_Body->Write(src.Buffer, length_indicator); - src.DropData(length_indicator); - CompleteBody = true; - return true; + } else + return true; } + if (src.Eof) + BOOST_THROW_EXCEPTION(std::invalid_argument("Unexpected EOF in HTTP body")); + + if (src.MustRead) { + if (!src.FillFromStream(m_Stream, false)) { + src.Eof = true; + BOOST_THROW_EXCEPTION(std::invalid_argument("Unexpected EOF in HTTP body")); + } + + src.MustRead = false; + } + + long length_indicator_signed = Convert::ToLong(Headers->Get("content-length")); + + if (length_indicator_signed < 0) + BOOST_THROW_EXCEPTION(std::invalid_argument("Content-Length must not be negative.")); + + size_t length_indicator = length_indicator_signed; + + if (src.Size < length_indicator) { + src.MustRead = true; + return false; + } + + m_Body->Write(src.Buffer, length_indicator); + src.DropData(length_indicator); + CompleteBody = true; return true; } diff --git a/lib/remote/httprequest.hpp b/lib/remote/httprequest.hpp index e8591474c..b456d7229 100644 --- a/lib/remote/httprequest.hpp +++ b/lib/remote/httprequest.hpp @@ -53,6 +53,7 @@ struct HttpRequest { public: bool CompleteHeaders; + bool CompleteHeaderCheck; bool CompleteBody; String RequestMethod; diff --git a/lib/remote/httpserverconnection.cpp b/lib/remote/httpserverconnection.cpp index 9e400da16..c22f0ee15 100644 --- a/lib/remote/httpserverconnection.cpp +++ b/lib/remote/httpserverconnection.cpp @@ -90,100 +90,105 @@ void HttpServerConnection::Disconnect() bool HttpServerConnection::ProcessMessage() { bool res; + HttpResponse response(m_Stream, m_CurrentRequest); - try { - res = m_CurrentRequest.ParseHeader(m_Context, false); - } catch (const std::invalid_argument& ex) { - HttpResponse response(m_Stream, m_CurrentRequest); - response.SetStatus(400, "Bad request"); - String msg = String("

Bad request

") + ex.what() + "

"; - response.WriteBody(msg.CStr(), msg.GetLength()); - response.Finish(); + if (!m_CurrentRequest.CompleteHeaders) { + try { + res = m_CurrentRequest.ParseHeader(m_Context, false); + } catch (const std::invalid_argument& ex) { + response.SetStatus(400, "Bad Request"); + String msg = String("

Bad Request

") + ex.what() + "

"; + response.WriteBody(msg.CStr(), msg.GetLength()); + response.Finish(); - m_Stream->Shutdown(); - return false; - } catch (const std::exception& ex) { - HttpResponse response(m_Stream, m_CurrentRequest); - response.SetStatus(400, "Bad request"); - String msg = "

Bad request

" + DiagnosticInformation(ex) + "

"; - response.WriteBody(msg.CStr(), msg.GetLength()); - response.Finish(); + m_Stream->Shutdown(); + return false; + } catch (const std::exception& ex) { + response.SetStatus(500, "Internal Server Error"); + String msg = "

Internal Server Error

" + DiagnosticInformation(ex) + "

"; + response.WriteBody(msg.CStr(), msg.GetLength()); + response.Finish(); - m_Stream->Shutdown(); - return false; + m_Stream->Shutdown(); + return false; + } + return res; } - if (m_CurrentRequest.CompleteHeaders) { - m_RequestQueue.Enqueue(std::bind(&HttpServerConnection::ProcessMessageAsync, - HttpServerConnection::Ptr(this), m_CurrentRequest)); - - m_Seen = Utility::GetTime(); - m_PendingRequests++; - - m_CurrentRequest.~HttpRequest(); - new (&m_CurrentRequest) HttpRequest(m_Stream); - - return true; - } - - return res; -} - -void HttpServerConnection::ProcessMessageAsync(HttpRequest& request) -{ - String auth_header = request.Headers->Get("authorization"); - - String::SizeType pos = auth_header.FindFirstOf(" "); - String username, password; - - if (pos != String::NPos && auth_header.SubStr(0, pos) == "Basic") { - String credentials_base64 = auth_header.SubStr(pos + 1); - String credentials = Base64::Decode(credentials_base64); - - String::SizeType cpos = credentials.FindFirstOf(":"); - - if (cpos != String::NPos) { - username = credentials.SubStr(0, cpos); - password = credentials.SubStr(cpos + 1); + if (!m_CurrentRequest.CompleteHeaderCheck) { + m_CurrentRequest.CompleteHeaderCheck = true; + if (!ManageHeaders(response)) { + m_Stream->Shutdown(); + return false; } } - ApiUser::Ptr user; + if (!m_CurrentRequest.CompleteBody) { + try { + res = m_CurrentRequest.ParseBody(m_Context, false); + } catch (const std::invalid_argument& ex) { + response.SetStatus(400, "Bad Request"); + String msg = String("

Bad Request

") + ex.what() + "

"; + response.WriteBody(msg.CStr(), msg.GetLength()); + response.Finish(); + + m_Stream->Shutdown(); + return false; + } catch (const std::exception& ex) { + response.SetStatus(500, "Internal Server Error"); + String msg = "

Internal Server Error

" + DiagnosticInformation(ex) + "

"; + response.WriteBody(msg.CStr(), msg.GetLength()); + response.Finish(); + + m_Stream->Shutdown(); + return false; + } + return res; + } + + m_RequestQueue.Enqueue(std::bind(&HttpServerConnection::ProcessMessageAsync, + HttpServerConnection::Ptr(this), m_CurrentRequest, response, m_AuthenticatedUser)); + + m_Seen = Utility::GetTime(); + m_PendingRequests++; + + m_CurrentRequest.~HttpRequest(); + new (&m_CurrentRequest) HttpRequest(m_Stream); + + return false; +} + +bool HttpServerConnection::ManageHeaders(HttpResponse& response) +{ + if (m_CurrentRequest.Headers->Get("expect") == "100-continue") { + String continueResponse = "HTTP/1.1 100 Continue\r\n\r\n"; + m_Stream->Write(continueResponse.CStr(), continueResponse.GetLength()); + } /* client_cn matched. */ if (m_ApiUser) - user = m_ApiUser; - else { - user = ApiUser::GetByName(username); + m_AuthenticatedUser = m_ApiUser; + else + m_AuthenticatedUser = ApiUser::GetByAuthHeader(m_CurrentRequest.Headers->Get("authorization")); - /* Deny authentication if 1) given password is empty 2) configured password does not match. */ - if (password.IsEmpty()) - user.reset(); - else if (user && user->GetPassword() != password) - user.reset(); - } - - String requestUrl = request.RequestUrl->Format(); + String requestUrl = m_CurrentRequest.RequestUrl->Format(); Socket::Ptr socket = m_Stream->GetSocket(); Log(LogInformation, "HttpServerConnection") - << "Request: " << request.RequestMethod << " " << requestUrl + << "Request: " << m_CurrentRequest.RequestMethod << " " << requestUrl << " (from " << (socket ? socket->GetPeerAddress() : "") - << ", user: " << (user ? user->GetName() : "") << ")"; - - HttpResponse response(m_Stream, request); + << ", user: " << (m_AuthenticatedUser ? m_AuthenticatedUser->GetName() : "") << ")"; ApiListener::Ptr listener = ApiListener::GetInstance(); if (!listener) - return; + return false; Array::Ptr headerAllowOrigin = listener->GetAccessControlAllowOrigin(); if (headerAllowOrigin->GetLength() != 0) { - String origin = request.Headers->Get("origin"); - + String origin = m_CurrentRequest.Headers->Get("origin"); { ObjectLock olock(headerAllowOrigin); @@ -196,9 +201,9 @@ void HttpServerConnection::ProcessMessageAsync(HttpRequest& request) if (listener->GetAccessControlAllowCredentials()) response.AddHeader("Access-Control-Allow-Credentials", "true"); - String accessControlRequestMethodHeader = request.Headers->Get("access-control-request-method"); + String accessControlRequestMethodHeader = m_CurrentRequest.Headers->Get("access-control-request-method"); - if (!accessControlRequestMethodHeader.IsEmpty()) { + if (m_CurrentRequest.RequestMethod == "OPTIONS" && !accessControlRequestMethodHeader.IsEmpty()) { response.SetStatus(200, "OK"); response.AddHeader("Access-Control-Allow-Methods", listener->GetAccessControlAllowMethods()); @@ -208,27 +213,27 @@ void HttpServerConnection::ProcessMessageAsync(HttpRequest& request) response.WriteBody(msg.CStr(), msg.GetLength()); response.Finish(); - m_PendingRequests--; - - return; + return false; } } - String accept_header = request.Headers->Get("accept"); - - if (request.RequestMethod != "GET" && accept_header != "application/json") { + if (m_CurrentRequest.RequestMethod != "GET" && m_CurrentRequest.Headers->Get("accept") != "application/json") { response.SetStatus(400, "Wrong Accept header"); response.AddHeader("Content-Type", "text/html"); String msg = "

Accept header is missing or not set to 'application/json'.

"; response.WriteBody(msg.CStr(), msg.GetLength()); - } else if (!user) { + response.Finish(); + return false; + } + + if (!m_AuthenticatedUser) { Log(LogWarning, "HttpServerConnection") - << "Unauthorized request: " << request.RequestMethod << " " << requestUrl; + << "Unauthorized request: " << m_CurrentRequest.RequestMethod << " " << requestUrl; response.SetStatus(401, "Unauthorized"); response.AddHeader("WWW-Authenticate", "Basic realm=\"Icinga 2\""); - if (request.Headers->Get("accept") == "application/json") { + if (m_CurrentRequest.Headers->Get("accept") == "application/json") { Dictionary::Ptr result = new Dictionary({ { "error", 401 }, { "status", "Unauthorized. Please check your user credentials." } @@ -240,44 +245,25 @@ void HttpServerConnection::ProcessMessageAsync(HttpRequest& request) String msg = "

Unauthorized. Please check your user credentials.

"; response.WriteBody(msg.CStr(), msg.GetLength()); } - } else { - bool res = true; - while (!request.CompleteBody) - res = request.ParseBody(m_Context, false); - if (!res) { - Log(LogCritical, "HttpServerConnection", "Failed to read body"); - Dictionary::Ptr result = new Dictionary({ - { "error", 400 }, - { "status", "Bad Request: Malformed body." } - }); - HttpUtility::SendJsonBody(response, nullptr, result); - } else { - try { - HttpHandler::ProcessRequest(user, request, response); - } catch (const std::exception& ex) { - Log(LogCritical, "HttpServerConnection") - << "Unhandled exception while processing Http request: " << DiagnosticInformation(ex); - response.SetStatus(503, "Unhandled exception"); - String errorInfo = DiagnosticInformation(ex); + response.Finish(); + return false; + } - if (request.Headers->Get("accept") == "application/json") { - Dictionary::Ptr result = new Dictionary({ - { "error", 503 }, - { "status", errorInfo } - }); + return true; +} - HttpUtility::SendJsonBody(response, nullptr, result); - } else { - response.AddHeader("Content-Type", "text/plain"); - response.WriteBody(errorInfo.CStr(), errorInfo.GetLength()); - } - } - } +void HttpServerConnection::ProcessMessageAsync(HttpRequest& request, HttpResponse& response, const ApiUser::Ptr& user) +{ + try { + HttpHandler::ProcessRequest(user, request, response); + } catch (const std::exception& ex) { + Log(LogCritical, "HttpServerConnection") + << "Unhandled exception while processing Http request: " << DiagnosticInformation(ex); + HttpUtility::SendJsonError(response, nullptr, 503, "Unhandled exception" , DiagnosticInformation(ex)); } response.Finish(); - m_PendingRequests--; } diff --git a/lib/remote/httpserverconnection.hpp b/lib/remote/httpserverconnection.hpp index f52110013..104df7509 100644 --- a/lib/remote/httpserverconnection.hpp +++ b/lib/remote/httpserverconnection.hpp @@ -21,6 +21,7 @@ #define HTTPSERVERCONNECTION_H #include "remote/httprequest.hpp" +#include "remote/httpresponse.hpp" #include "remote/apiuser.hpp" #include "base/tlsstream.hpp" #include "base/timer.hpp" @@ -51,6 +52,7 @@ public: private: ApiUser::Ptr m_ApiUser; + ApiUser::Ptr m_AuthenticatedUser; TlsStream::Ptr m_Stream; double m_Seen; HttpRequest m_CurrentRequest; @@ -67,7 +69,9 @@ private: static void TimeoutTimerHandler(); void CheckLiveness(); - void ProcessMessageAsync(HttpRequest& request); + bool ManageHeaders(HttpResponse& response); + + void ProcessMessageAsync(HttpRequest& request, HttpResponse& response, const ApiUser::Ptr&); }; } From 8ffa4f04a7d519a80115b663caf92ed7ae9c7f98 Mon Sep 17 00:00:00 2001 From: Jean Flach Date: Tue, 13 Feb 2018 17:29:48 +0100 Subject: [PATCH 6/9] Add timeout for TLS handshakes --- lib/base/stream.cpp | 25 ++++++++++++++++++++----- lib/base/stream.hpp | 4 +++- lib/base/tlsstream.cpp | 13 +++++++++++-- lib/remote/apilistener.cpp | 12 +++++++++--- lib/remote/httpserverconnection.cpp | 8 +++++++- lib/remote/pkiutility.cpp | 6 ++++-- 6 files changed, 54 insertions(+), 14 deletions(-) diff --git a/lib/base/stream.cpp b/lib/base/stream.cpp index 1e6b9baf1..72ca82c1a 100644 --- a/lib/base/stream.cpp +++ b/lib/base/stream.cpp @@ -60,7 +60,7 @@ void Stream::SignalDataAvailable() } } -bool Stream::WaitForData(int timeout) +bool Stream::WaitForData() { if (!SupportsWaiting()) BOOST_THROW_EXCEPTION(std::runtime_error("Stream does not support waiting.")); @@ -68,10 +68,25 @@ bool Stream::WaitForData(int timeout) boost::mutex::scoped_lock lock(m_Mutex); while (!IsDataAvailable() && !IsEof()) - if (timeout < 0) - m_CV.wait(lock); - else - m_CV.timed_wait(lock, boost::posix_time::milliseconds(timeout * 1000)); + m_CV.wait(lock); + + return IsDataAvailable() || IsEof(); +} + +bool Stream::WaitForData(int timeout) +{ + if (!SupportsWaiting()) + BOOST_THROW_EXCEPTION(std::runtime_error("Stream does not support waiting.")); + + if (timeout < 0) + BOOST_THROW_EXCEPTION(std::runtime_error("Timeout can't be negative")); + + boost::system_time const point_of_timeout = boost::get_system_time() + boost::posix_time::seconds(timeout); + + boost::mutex::scoped_lock lock(m_Mutex); + + while (!IsDataAvailable() && !IsEof() && point_of_timeout > boost::get_system_time()) + m_CV.timed_wait(lock, point_of_timeout); return IsDataAvailable() || IsEof(); } diff --git a/lib/base/stream.hpp b/lib/base/stream.hpp index 58569246c..8a140a586 100644 --- a/lib/base/stream.hpp +++ b/lib/base/stream.hpp @@ -122,8 +122,10 @@ public: /** * Waits until data can be read from the stream. + * Optionally with a timeout. */ - bool WaitForData(int timeout = -1); + bool WaitForData(); + bool WaitForData(int timeout); virtual bool SupportsWaiting() const; diff --git a/lib/base/tlsstream.cpp b/lib/base/tlsstream.cpp index 029c9b446..e97f68b60 100644 --- a/lib/base/tlsstream.cpp +++ b/lib/base/tlsstream.cpp @@ -27,6 +27,9 @@ # include #endif /* _WIN32 */ +#define TLS_TIMEOUT_SECONDS 10 +#define TLS_TIMEOUT_STEP_SECONDS 1 + using namespace icinga; int TlsStream::m_SSLIndex; @@ -285,8 +288,14 @@ void TlsStream::Handshake() m_CurrentAction = TlsActionHandshake; ChangeEvents(POLLOUT); - while (!m_HandshakeOK && !m_ErrorOccurred && !m_Eof) - m_CV.wait(lock); + boost::system_time const timeout = boost::get_system_time() + boost::posix_time::seconds(TLS_TIMEOUT_SECONDS); + + while (!m_HandshakeOK && !m_ErrorOccurred && !m_Eof && timeout > boost::get_system_time()) + m_CV.timed_wait(lock, timeout); + + // We should _NOT_ (underline, bold, itallic and wordart) throw an exception for a timeout. + if (timeout < boost::get_system_time()) + BOOST_THROW_EXCEPTION(std::runtime_error("Timeout during handshake.")); if (m_Eof) BOOST_THROW_EXCEPTION(std::runtime_error("Socket was closed during TLS handshake.")); diff --git a/lib/remote/apilistener.cpp b/lib/remote/apilistener.cpp index 251aff791..bc0df4449 100644 --- a/lib/remote/apilistener.cpp +++ b/lib/remote/apilistener.cpp @@ -512,11 +512,17 @@ void ApiListener::NewClientHandlerInternal(const Socket::Ptr& client, const Stri JsonRpc::SendMessage(tlsStream, message); ctype = ClientJsonRpc; } else { - tlsStream->WaitForData(5); + tlsStream->WaitForData(10); if (!tlsStream->IsDataAvailable()) { - Log(LogWarning, "ApiListener") - << "No data received on new API connection for identity '" << identity << "'. Ensure that the remote endpoints are properly configured in a cluster setup."; + if (identity.IsEmpty()) + Log(LogInformation, "ApiListener") + << "No data received on new API connection. " + << "Ensure that the remote endpoints are properly configured in a cluster setup."; + else + Log(LogWarning, "ApiListener") + << "No data received on new API connection for identity '" << identity << "'. " + << "Ensure that the remote endpoints are properly configured in a cluster setup."; return; } diff --git a/lib/remote/httpserverconnection.cpp b/lib/remote/httpserverconnection.cpp index c22f0ee15..7c86f9be9 100644 --- a/lib/remote/httpserverconnection.cpp +++ b/lib/remote/httpserverconnection.cpp @@ -52,7 +52,7 @@ void HttpServerConnection::StaticInitialize() { l_HttpServerConnectionTimeoutTimer = new Timer(); l_HttpServerConnectionTimeoutTimer->OnTimerExpired.connect(std::bind(&HttpServerConnection::TimeoutTimerHandler)); - l_HttpServerConnectionTimeoutTimer->SetInterval(15); + l_HttpServerConnectionTimeoutTimer->SetInterval(5); l_HttpServerConnectionTimeoutTimer->Start(); } @@ -76,6 +76,12 @@ TlsStream::Ptr HttpServerConnection::GetStream() const void HttpServerConnection::Disconnect() { + boost::mutex::scoped_try_lock lock(m_DataHandlerMutex); + if (!lock.owns_lock()) { + Log(LogInformation, "HttpServerConnection", "Unable to disconnect Http client, I/O thread busy"); + return; + } + Log(LogDebug, "HttpServerConnection", "Http client disconnected"); ApiListener::Ptr listener = ApiListener::GetInstance(); diff --git a/lib/remote/pkiutility.cpp b/lib/remote/pkiutility.cpp index 6b896dab1..20d9ca6c2 100644 --- a/lib/remote/pkiutility.cpp +++ b/lib/remote/pkiutility.cpp @@ -121,8 +121,10 @@ std::shared_ptr PkiUtility::FetchCert(const String& host, const String& po try { stream->Handshake(); - } catch (...) { - + } catch (const std::exception& ex) { + Log(LogCritical, "pki") + << "Client TLS handshake failed. (" << ex.what() << ")"; + return std::shared_ptr(); } return stream->GetPeerCertificate(); From 817415f6a54fb0fb498d241d9f931c703defb5ce Mon Sep 17 00:00:00 2001 From: Noah Hilverling Date: Mon, 19 Feb 2018 10:47:14 +0100 Subject: [PATCH 7/9] Fix requests not being closed correctly --- lib/remote/httpserverconnection.cpp | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/lib/remote/httpserverconnection.cpp b/lib/remote/httpserverconnection.cpp index 7c86f9be9..4fba15cb6 100644 --- a/lib/remote/httpserverconnection.cpp +++ b/lib/remote/httpserverconnection.cpp @@ -107,7 +107,11 @@ bool HttpServerConnection::ProcessMessage() response.WriteBody(msg.CStr(), msg.GetLength()); response.Finish(); + m_CurrentRequest.~HttpRequest(); + new (&m_CurrentRequest) HttpRequest(m_Stream); + m_Stream->Shutdown(); + return false; } catch (const std::exception& ex) { response.SetStatus(500, "Internal Server Error"); @@ -115,7 +119,11 @@ bool HttpServerConnection::ProcessMessage() response.WriteBody(msg.CStr(), msg.GetLength()); response.Finish(); + m_CurrentRequest.~HttpRequest(); + new (&m_CurrentRequest) HttpRequest(m_Stream); + m_Stream->Shutdown(); + return false; } return res; @@ -124,7 +132,11 @@ bool HttpServerConnection::ProcessMessage() if (!m_CurrentRequest.CompleteHeaderCheck) { m_CurrentRequest.CompleteHeaderCheck = true; if (!ManageHeaders(response)) { + m_CurrentRequest.~HttpRequest(); + new (&m_CurrentRequest) HttpRequest(m_Stream); + m_Stream->Shutdown(); + return false; } } @@ -138,7 +150,11 @@ bool HttpServerConnection::ProcessMessage() response.WriteBody(msg.CStr(), msg.GetLength()); response.Finish(); + m_CurrentRequest.~HttpRequest(); + new (&m_CurrentRequest) HttpRequest(m_Stream); + m_Stream->Shutdown(); + return false; } catch (const std::exception& ex) { response.SetStatus(500, "Internal Server Error"); @@ -146,7 +162,11 @@ bool HttpServerConnection::ProcessMessage() response.WriteBody(msg.CStr(), msg.GetLength()); response.Finish(); + m_CurrentRequest.~HttpRequest(); + new (&m_CurrentRequest) HttpRequest(m_Stream); + m_Stream->Shutdown(); + return false; } return res; From 2823ebb831c7f60e2b9339cfdd7116e3dfe12caf Mon Sep 17 00:00:00 2001 From: Noah Hilverling Date: Mon, 19 Feb 2018 13:33:58 +0100 Subject: [PATCH 8/9] Limit HTTP body size --- doc/12-icinga2-api.md | 34 ++++++++++++++++------------- lib/remote/apiuser.cpp | 3 ++- lib/remote/httpserverconnection.cpp | 34 +++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 16 deletions(-) diff --git a/doc/12-icinga2-api.md b/doc/12-icinga2-api.md index 58aaf6873..676bc9784 100644 --- a/doc/12-icinga2-api.md +++ b/doc/12-icinga2-api.md @@ -208,23 +208,27 @@ The [regex function](18-library-reference.md#global-functions-regex) is availabl More information about filters can be found in the [filters](12-icinga2-api.md#icinga2-api-filters) chapter. +Note that the permissions a API user has also specify the max body size of their requests. +A API user with `*` permissions is allowed to send 512 MB. + + Available permissions for specific URL endpoints: - Permissions | URL Endpoint | Supports Filters - ------------------------------|---------------|----------------- - actions/<action> | /v1/actions | Yes - config/query | /v1/config | No - config/modify | /v1/config | No - console | /v1/console | No - events/<type> | /v1/events | No - objects/query/<type> | /v1/objects | Yes - objects/create/<type> | /v1/objects | No - objects/modify/<type> | /v1/objects | Yes - objects/delete/<type> | /v1/objects | Yes - status/query | /v1/status | Yes - templates/<type> | /v1/templates | Yes - types | /v1/types | Yes - variables | /v1/variables | Yes + Permissions | URL Endpoint | Supports Filters | Max Body Size in MB + ------------------------------|---------------|-------------------|--------------------- + actions/<action> | /v1/actions | Yes | 1 + config/query | /v1/config | No | 1 + config/modify | /v1/config | No | 512 + console | /v1/console | No | 512 + events/<type> | /v1/events | No | 1 + objects/query/<type> | /v1/objects | Yes | 1 + objects/create/<type> | /v1/objects | No | 512 + objects/modify/<type> | /v1/objects | Yes | 512 + objects/delete/<type> | /v1/objects | Yes | 512 + status/query | /v1/status | Yes | 1 + templates/<type> | /v1/templates | Yes | 1 + types | /v1/types | Yes | 1 + variables | /v1/variables | Yes | 1 The required actions or types can be replaced by using a wildcard match ("\*"). diff --git a/lib/remote/apiuser.cpp b/lib/remote/apiuser.cpp index 416eedb34..7bd953880 100644 --- a/lib/remote/apiuser.cpp +++ b/lib/remote/apiuser.cpp @@ -36,7 +36,8 @@ ApiUser::Ptr ApiUser::GetByClientCN(const String& cn) return nullptr; } -ApiUser::Ptr ApiUser::GetByAuthHeader(const String& auth_header) { +ApiUser::Ptr ApiUser::GetByAuthHeader(const String& auth_header) +{ String::SizeType pos = auth_header.FindFirstOf(" "); String username, password; diff --git a/lib/remote/httpserverconnection.cpp b/lib/remote/httpserverconnection.cpp index 4fba15cb6..c9cdf5a3c 100644 --- a/lib/remote/httpserverconnection.cpp +++ b/lib/remote/httpserverconnection.cpp @@ -95,6 +95,7 @@ void HttpServerConnection::Disconnect() bool HttpServerConnection::ProcessMessage() { + bool res; HttpResponse response(m_Stream, m_CurrentRequest); @@ -186,6 +187,16 @@ bool HttpServerConnection::ProcessMessage() bool HttpServerConnection::ManageHeaders(HttpResponse& response) { + static const size_t defaultContentLengthLimit = 1 * 1028 * 1028; + static const Dictionary::Ptr specialContentLengthLimits = new Dictionary({ + {"*", 512 * 1028 * 1028}, + {"config/modify", 512 * 1028 * 1028}, + {"console", 512 * 1028 * 1028}, + {"objects/create", 512 * 1028 * 1028}, + {"objects/modify", 512 * 1028 * 1028}, + {"objects/delete", 512 * 1028 * 1028} + }); + if (m_CurrentRequest.Headers->Get("expect") == "100-continue") { String continueResponse = "HTTP/1.1 100 Continue\r\n\r\n"; m_Stream->Write(continueResponse.CStr(), continueResponse.GetLength()); @@ -276,6 +287,29 @@ bool HttpServerConnection::ManageHeaders(HttpResponse& response) return false; } + size_t maxSize = defaultContentLengthLimit; + + Array::Ptr permissions = m_AuthenticatedUser->GetPermissions(); + ObjectLock olock(permissions); + + for (const Value& permission : permissions) { + std::vector permissionParts = String(permission).Split("/"); + String permissionPath = permissionParts[0] + (permissionParts.size() > 1 ? "/" + permissionParts[1] : ""); + int size = specialContentLengthLimits->Get(permissionPath); + maxSize = size > maxSize ? size : maxSize; + } + + size_t contentLength = m_CurrentRequest.Headers->Get("content-length"); + + if (contentLength > maxSize) { + response.SetStatus(400, "Bad Request"); + String msg = String("

Content length exceeded maximum

"); + response.WriteBody(msg.CStr(), msg.GetLength()); + response.Finish(); + + return false; + } + return true; } From 85f45d9b948eeeb702e3e5893dcdfccad4aff322 Mon Sep 17 00:00:00 2001 From: Jean Flach Date: Wed, 21 Feb 2018 13:22:17 +0100 Subject: [PATCH 9/9] Minor codestyle and doc changes --- doc/12-icinga2-api.md | 10 ++++------ lib/base/tlsstream.cpp | 1 - lib/remote/httprequest.cpp | 2 +- lib/remote/httprequest.hpp | 2 +- lib/remote/httpserverconnection.cpp | 13 +++++++------ lib/remote/httpserverconnection.hpp | 1 - 6 files changed, 13 insertions(+), 16 deletions(-) diff --git a/doc/12-icinga2-api.md b/doc/12-icinga2-api.md index 676bc9784..0f44bbfda 100644 --- a/doc/12-icinga2-api.md +++ b/doc/12-icinga2-api.md @@ -208,13 +208,12 @@ The [regex function](18-library-reference.md#global-functions-regex) is availabl More information about filters can be found in the [filters](12-icinga2-api.md#icinga2-api-filters) chapter. -Note that the permissions a API user has also specify the max body size of their requests. -A API user with `*` permissions is allowed to send 512 MB. - +Permissions are tied to a maximum HTTP request size to prevent abuse, responses sent by Icinga are not limited. +An API user with all permissions ("\*") may send up to 512 MB regardless of the endpoint. Available permissions for specific URL endpoints: - Permissions | URL Endpoint | Supports Filters | Max Body Size in MB + Permissions | URL Endpoint | Supports filters | Max body size in MB ------------------------------|---------------|-------------------|--------------------- actions/<action> | /v1/actions | Yes | 1 config/query | /v1/config | No | 1 @@ -234,8 +233,7 @@ The required actions or types can be replaced by using a wildcard match ("\*"). ### Parameters -Depending on the request method there are two ways of -passing parameters to the request: +Depending on the request method there are two ways of passing parameters to the request: * JSON object as request body (all request methods other than `GET`) * Query string as URL parameter (all request methods) diff --git a/lib/base/tlsstream.cpp b/lib/base/tlsstream.cpp index e97f68b60..2846e8962 100644 --- a/lib/base/tlsstream.cpp +++ b/lib/base/tlsstream.cpp @@ -28,7 +28,6 @@ #endif /* _WIN32 */ #define TLS_TIMEOUT_SECONDS 10 -#define TLS_TIMEOUT_STEP_SECONDS 1 using namespace icinga; diff --git a/lib/remote/httprequest.cpp b/lib/remote/httprequest.cpp index c5382d254..a50cd3783 100644 --- a/lib/remote/httprequest.cpp +++ b/lib/remote/httprequest.cpp @@ -34,7 +34,7 @@ HttpRequest::HttpRequest(Stream::Ptr stream) m_State(HttpRequestStart) { } -bool HttpRequest::ParseHeader(StreamReadContext& src, bool may_wait) +bool HttpRequest::ParseHeaders(StreamReadContext& src, bool may_wait) { if (!m_Stream) return false; diff --git a/lib/remote/httprequest.hpp b/lib/remote/httprequest.hpp index b456d7229..14e0677cd 100644 --- a/lib/remote/httprequest.hpp +++ b/lib/remote/httprequest.hpp @@ -64,7 +64,7 @@ public: HttpRequest(Stream::Ptr stream); - bool ParseHeader(StreamReadContext& src, bool may_wait); + bool ParseHeaders(StreamReadContext& src, bool may_wait); bool ParseBody(StreamReadContext& src, bool may_wait); size_t ReadBody(char *data, size_t count); diff --git a/lib/remote/httpserverconnection.cpp b/lib/remote/httpserverconnection.cpp index c9cdf5a3c..e54653f22 100644 --- a/lib/remote/httpserverconnection.cpp +++ b/lib/remote/httpserverconnection.cpp @@ -24,12 +24,13 @@ #include "remote/apifunction.hpp" #include "remote/jsonrpc.hpp" #include "base/base64.hpp" -#include "base/configtype.hpp" -#include "base/objectlock.hpp" -#include "base/utility.hpp" -#include "base/logger.hpp" -#include "base/exception.hpp" #include "base/convert.hpp" +#include "base/configtype.hpp" +#include "base/exception.hpp" +#include "base/logger.hpp" +#include "base/objectlock.hpp" +#include "base/timer.hpp" +#include "base/utility.hpp" #include using namespace icinga; @@ -101,7 +102,7 @@ bool HttpServerConnection::ProcessMessage() if (!m_CurrentRequest.CompleteHeaders) { try { - res = m_CurrentRequest.ParseHeader(m_Context, false); + res = m_CurrentRequest.ParseHeaders(m_Context, false); } catch (const std::invalid_argument& ex) { response.SetStatus(400, "Bad Request"); String msg = String("

Bad Request

") + ex.what() + "

"; diff --git a/lib/remote/httpserverconnection.hpp b/lib/remote/httpserverconnection.hpp index 104df7509..5f959b8a0 100644 --- a/lib/remote/httpserverconnection.hpp +++ b/lib/remote/httpserverconnection.hpp @@ -24,7 +24,6 @@ #include "remote/httpresponse.hpp" #include "remote/apiuser.hpp" #include "base/tlsstream.hpp" -#include "base/timer.hpp" #include "base/workqueue.hpp" namespace icinga