diff options
-rw-r--r-- | http/parsing.hpp | 40 | ||||
-rw-r--r-- | include/openbmc_dbus_rest.hpp | 49 | ||||
-rw-r--r-- | meson.build | 1 | ||||
-rw-r--r-- | meson_options.txt | 11 | ||||
-rw-r--r-- | redfish-core/lib/certificate_service.hpp | 7 | ||||
-rw-r--r-- | redfish-core/lib/task.hpp | 9 | ||||
-rw-r--r-- | redfish-core/src/utils/json_utils.cpp | 16 | ||||
-rw-r--r-- | test/redfish-core/include/utils/json_utils_test.cpp | 7 |
8 files changed, 117 insertions, 23 deletions
diff --git a/http/parsing.hpp b/http/parsing.hpp new file mode 100644 index 0000000000..df63fcaf28 --- /dev/null +++ b/http/parsing.hpp @@ -0,0 +1,40 @@ +#pragma once + +#include "http/http_request.hpp" +#include "logging.hpp" + +#include <boost/algorithm/string/predicate.hpp> +#include <nlohmann/json.hpp> + +#include <string_view> + +enum class JsonParseResult +{ + BadContentType, + BadJsonData, + Success, +}; + +inline JsonParseResult parseRequestAsJson(const crow::Request& req, + nlohmann::json& jsonOut) +{ + std::string_view contentType = + req.getHeaderValue(boost::beast::http::field::content_type); + + if (!boost::iequals(contentType, "application/json") && + !boost::iequals(contentType, "application/json; charset=utf-8")) + { + BMCWEB_LOG_WARNING << "Failed to parse content type on request"; +#ifndef BMCWEB_INSECURE_IGNORE_CONTENT_TYPE + return JsonParseResult::BadContentType; +#endif + } + jsonOut = nlohmann::json::parse(req.body, nullptr, false); + if (jsonOut.is_discarded()) + { + BMCWEB_LOG_WARNING << "Failed to parse json in request"; + return JsonParseResult::BadJsonData; + } + + return JsonParseResult::Success; +} diff --git a/include/openbmc_dbus_rest.hpp b/include/openbmc_dbus_rest.hpp index 2665b2e6d1..9bf968a40d 100644 --- a/include/openbmc_dbus_rest.hpp +++ b/include/openbmc_dbus_rest.hpp @@ -20,6 +20,7 @@ #include "http_request.hpp" #include "http_response.hpp" #include "logging.hpp" +#include "parsing.hpp" #include "routing.hpp" #include "str_utility.hpp" @@ -80,6 +81,7 @@ const constexpr char* notFoundMsg = "404 Not Found"; const constexpr char* badReqMsg = "400 Bad Request"; const constexpr char* methodNotAllowedMsg = "405 Method Not Allowed"; const constexpr char* forbiddenMsg = "403 Forbidden"; +const constexpr char* unsupportedMediaMsg = "415 Unsupported Media Type"; const constexpr char* methodFailedMsg = "500 Method Call Failed"; const constexpr char* methodOutputFailedMsg = "500 Method Output Error"; const constexpr char* notFoundDesc = @@ -87,6 +89,8 @@ const constexpr char* notFoundDesc = const constexpr char* propNotFoundDesc = "The specified property cannot be found"; const constexpr char* noJsonDesc = "No JSON object could be decoded"; +const constexpr char* invalidContentType = + "Content-type header is missing or invalid"; const constexpr char* methodNotFoundDesc = "The specified method cannot be found"; const constexpr char* methodNotAllowedDesc = "Method not allowed"; @@ -1538,10 +1542,17 @@ inline void handleAction(const crow::Request& req, { BMCWEB_LOG_DEBUG << "handleAction on path: " << objectPath << " and method " << methodName; - nlohmann::json requestDbusData = - nlohmann::json::parse(req.body, nullptr, false); + nlohmann::json requestDbusData; - if (requestDbusData.is_discarded()) + JsonParseResult ret = parseRequestAsJson(req, requestDbusData); + if (ret == JsonParseResult::BadContentType) + { + setErrorResponse(asyncResp->res, + boost::beast::http::status::unsupported_media_type, + invalidContentType, unsupportedMediaMsg); + return; + } + if (ret != JsonParseResult::Success) { setErrorResponse(asyncResp->res, boost::beast::http::status::bad_request, noJsonDesc, @@ -1838,11 +1849,18 @@ inline void handlePut(const crow::Request& req, forbiddenResDesc, forbiddenMsg); return; } + nlohmann::json requestDbusData; - nlohmann::json requestDbusData = - nlohmann::json::parse(req.body, nullptr, false); + JsonParseResult ret = parseRequestAsJson(req, requestDbusData); + if (ret == JsonParseResult::BadContentType) + { + setErrorResponse(asyncResp->res, + boost::beast::http::status::unsupported_media_type, + invalidContentType, unsupportedMediaMsg); + return; + } - if (requestDbusData.is_discarded()) + if (ret != JsonParseResult::Success) { setErrorResponse(asyncResp->res, boost::beast::http::status::bad_request, noJsonDesc, @@ -2376,14 +2394,23 @@ inline void return; } - nlohmann::json requestDbusData = - nlohmann::json::parse(req.body, nullptr, false); - - if (requestDbusData.is_discarded()) + nlohmann::json requestDbusData; + JsonParseResult ret = parseRequestAsJson(req, requestDbusData); + if (ret == JsonParseResult::BadContentType) { - asyncResp->res.result(boost::beast::http::status::bad_request); + setErrorResponse(asyncResp->res, + boost::beast::http::status::unsupported_media_type, + invalidContentType, unsupportedMediaMsg); + return; + } + if (ret != JsonParseResult::Success) + { + setErrorResponse(asyncResp->res, + boost::beast::http::status::bad_request, + noJsonDesc, badReqMsg); return; } + if (!requestDbusData.is_array()) { asyncResp->res.result(boost::beast::http::status::bad_request); diff --git a/meson.build b/meson.build index be3c6b103e..ae789a961b 100644 --- a/meson.build +++ b/meson.build @@ -70,6 +70,7 @@ feature_map = { 'insecure-disable-ssl' : '-DBMCWEB_INSECURE_DISABLE_SSL', 'insecure-push-style-notification' : '-DBMCWEB_INSECURE_ENABLE_HTTP_PUSH_STYLE_EVENTING', 'insecure-tftp-update' : '-DBMCWEB_INSECURE_ENABLE_REDFISH_FW_TFTP_UPDATE', + 'insecure-ignore-content-type' : '-DBMCWEB_INSECURE_IGNORE_CONTENT_TYPE', 'kvm' : '-DBMCWEB_ENABLE_KVM' , 'mutual-tls-auth' : '-DBMCWEB_ENABLE_MUTUAL_TLS_AUTHENTICATION', 'redfish-aggregation' : '-DBMCWEB_ENABLE_REDFISH_AGGREGATION', diff --git a/meson_options.txt b/meson_options.txt index ecc7aebc22..52c3e6cefb 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -293,6 +293,17 @@ option( ) option( + 'insecure-ignore-content-type', + type: 'feature', + value: 'enabled', + description: '''Allows parsing PUT/POST/PATCH content as JSON regardless + of the presence of the content-type header. Enabling this + conflicts with the input parsing guidelines, but may be + required to support old clients that may not set the + Content-Type header on payloads.''' +) + +option( 'insecure-push-style-notification', type: 'feature', value: 'disabled', diff --git a/redfish-core/lib/certificate_service.hpp b/redfish-core/lib/certificate_service.hpp index 65f5262077..16f26e37b2 100644 --- a/redfish-core/lib/certificate_service.hpp +++ b/redfish-core/lib/certificate_service.hpp @@ -3,6 +3,7 @@ #include "app.hpp" #include "async_resp.hpp" #include "dbus_utility.hpp" +#include "http/parsing.hpp" #include "http_response.hpp" #include "query.hpp" #include "registries/privilege_registry.hpp" @@ -53,9 +54,9 @@ inline std::string getCertificateFromReqBody( const std::shared_ptr<bmcweb::AsyncResp>& asyncResp, const crow::Request& req) { - nlohmann::json reqJson = nlohmann::json::parse(req.body, nullptr, false); - - if (reqJson.is_discarded()) + nlohmann::json reqJson; + JsonParseResult ret = parseRequestAsJson(req, reqJson); + if (ret != JsonParseResult::Success) { // We did not receive JSON request, proceed as it is RAW data return req.body; diff --git a/redfish-core/lib/task.hpp b/redfish-core/lib/task.hpp index cfe6c3ee1a..950eac3d03 100644 --- a/redfish-core/lib/task.hpp +++ b/redfish-core/lib/task.hpp @@ -19,6 +19,7 @@ #include "dbus_utility.hpp" #include "event_service_manager.hpp" #include "health.hpp" +#include "http/parsing.hpp" #include "query.hpp" #include "registries/privilege_registry.hpp" #include "task_messages.hpp" @@ -47,8 +48,7 @@ struct Payload { explicit Payload(const crow::Request& req) : targetUri(req.url), httpOperation(req.methodString()), - httpHeaders(nlohmann::json::array()), - jsonBody(nlohmann::json::parse(req.body, nullptr, false)) + httpHeaders(nlohmann::json::array()) { using field_ns = boost::beast::http::field; constexpr const std::array<boost::beast::http::field, 7> @@ -57,9 +57,10 @@ struct Payload field_ns::connection, field_ns::content_length, field_ns::upgrade}; - if (jsonBody.is_discarded()) + JsonParseResult ret = parseRequestAsJson(req, jsonBody); + if (ret != JsonParseResult::Success) { - jsonBody = nullptr; + return; } for (const auto& field : req.fields) diff --git a/redfish-core/src/utils/json_utils.cpp b/redfish-core/src/utils/json_utils.cpp index a47b4d8217..89723ccd12 100644 --- a/redfish-core/src/utils/json_utils.cpp +++ b/redfish-core/src/utils/json_utils.cpp @@ -15,6 +15,13 @@ */ #include "utils/json_utils.hpp" +#include "error_messages.hpp" +#include "http/http_request.hpp" +#include "http/http_response.hpp" +#include "http/parsing.hpp" + +#include <nlohmann/json.hpp> + namespace redfish { @@ -24,14 +31,17 @@ namespace json_util bool processJsonFromRequest(crow::Response& res, const crow::Request& req, nlohmann::json& reqJson) { + JsonParseResult ret = parseRequestAsJson(req, reqJson); + if (ret == JsonParseResult::BadContentType) + { + messages::unrecognizedRequestBody(res); + return false; + } reqJson = nlohmann::json::parse(req.body, nullptr, false); if (reqJson.is_discarded()) { messages::malformedJSON(res); - - res.end(); - return false; } diff --git a/test/redfish-core/include/utils/json_utils_test.cpp b/test/redfish-core/include/utils/json_utils_test.cpp index 826d4375a6..b4d1062b24 100644 --- a/test/redfish-core/include/utils/json_utils_test.cpp +++ b/test/redfish-core/include/utils/json_utils_test.cpp @@ -274,7 +274,7 @@ TEST(ReadJsonPatch, ValidElementsReturnsTrueResponseOkValuesUnpackedCorrectly) crow::Request req({}, ec); // Ignore errors intentionally req.body = "{\"integer\": 1}"; - + req.req.set(boost::beast::http::field::content_type, "application/json"); int64_t integer = 0; ASSERT_TRUE(readJsonPatch(req, res, "integer", integer)); EXPECT_EQ(res.result(), boost::beast::http::status::ok); @@ -301,6 +301,7 @@ TEST(ReadJsonPatch, OdataIgnored) crow::Response res; std::error_code ec; crow::Request req({}, ec); + req.req.set(boost::beast::http::field::content_type, "application/json"); // Ignore errors intentionally req.body = R"({"@odata.etag": "etag", "integer": 1})"; @@ -330,6 +331,7 @@ TEST(ReadJsonAction, ValidElementsReturnsTrueResponseOkValuesUnpackedCorrectly) crow::Response res; std::error_code ec; crow::Request req({}, ec); + req.req.set(boost::beast::http::field::content_type, "application/json"); // Ignore errors intentionally req.body = "{\"integer\": 1}"; @@ -345,6 +347,7 @@ TEST(ReadJsonAction, EmptyObjectReturnsTrueResponseOk) crow::Response res; std::error_code ec; crow::Request req({}, ec); + req.req.set(boost::beast::http::field::content_type, "application/json"); // Ignore errors intentionally req.body = "{}"; @@ -355,4 +358,4 @@ TEST(ReadJsonAction, EmptyObjectReturnsTrueResponseOk) } } // namespace -} // namespace redfish::json_util
\ No newline at end of file +} // namespace redfish::json_util |