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>

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