Add option for validating content-type header
For systems implementing to the OWASP security guidelines[1] (of which all
should ideally) we should be checking the content-type header all times
that we parse a request as JSON.
This commit adds an option for parsing content-type, and sets a default
of "must get content-type". Ideally this would not be a breaking
change, but given the number of guides and scripts that omit the content
type, it seems worthwhile to add a trapdoor, such that people can opt
into their own model on how they would like to see this checking work.
Tested:
```
curl --insecure -H "Content-Type: application/json" -X POST -D headers.txt https://${bmc}/redfish/v1/SessionService/Sessions -d '{"UserName":"root", "Password":"0penBmc"}'
```
Succeeds.
Removing Content-Type argument causes bmc to return
Base.1.13.0.UnrecognizedRequestBody.
[1] cheatsheetseries.owasp.org/cheatsheets/REST_Security_Cheat_Sheet.html
Change-Id: Iaa47dd563b40036ff2fc2cacb70d941fd8853038
Signed-off-by: Ed Tanous <edtanous@google.com>
diff --git a/http/parsing.hpp b/http/parsing.hpp
new file mode 100644
index 0000000..df63fca
--- /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 2665b2e..9bf968a 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* 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* 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 @@
{
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 @@
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 @@
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 be3c6b1..ae789a9 100644
--- a/meson.build
+++ b/meson.build
@@ -70,6 +70,7 @@
'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 ecc7aeb..52c3e6c 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -293,6 +293,17 @@
)
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 65f5262..16f26e3 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 @@
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 cfe6c3e..950eac3 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 @@
{
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 @@
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 a47b4d8..89723cc 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 @@
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 826d437..b4d1062 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 @@
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 @@
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 @@
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 @@
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 @@
}
} // namespace
-} // namespace redfish::json_util
\ No newline at end of file
+} // namespace redfish::json_util