Authenticate expand query parameter
Addressed the below requirements for security GAP
1) PrivilegeRegistry is now enforced for all users and all expanded
resources. If a user lacks access, the node will not be expanded.
2) If a user with Operator privilege calls '/redfish/v1?$expand',
the response includes only resources the user has access to.
3) 'ReadOnly' and 'Operator' roles can expand, but not into nodes
they lack privileges for.
4) Expand is completely disallowed without authentication. Requests
without valid credentials receive 401 Unauthorized.
Testing:
Tested and verified that users can access only authorized resources.
Unauthorized access and unauthenticated expand requests are blocked.
Change-Id: Ic1c2761f30db73a2884cdecdb8eb08168446523b
Signed-off-by: Chandramohan Harkude <chandramohan.harkude@gmail.com>
diff --git a/redfish-core/include/query.hpp b/redfish-core/include/query.hpp
index 9629f03..d38da62 100644
--- a/redfish-core/include/query.hpp
+++ b/redfish-core/include/query.hpp
@@ -144,6 +144,19 @@
return false;
}
+ if constexpr (!BMCWEB_INSECURE_DISABLE_AUTH)
+ {
+ // Handle unauthorized expand query parameters for service root example
+ // /redfish/v1/?$expand=< >
+ if (req.session == nullptr &&
+ queryOpt->expandType != query_param::ExpandType::None)
+ {
+ messages::resourceAtUriUnauthorized(asyncResp->res, req.url(),
+ "Invalid username or password");
+ return false;
+ }
+ }
+
if (!handleIfMatch(app, req, asyncResp))
{
return false;
@@ -168,14 +181,27 @@
return needToCallHandlers;
}
+ // make a copy of the request because older request goes out of scope
+ // trying to access it after it goes out of scope will cause a crash
+ // Create a copy of the request using shared_ptr
+
+ // The lambda capture of newReq by value in the lambda is causing problems
+ // because the lambda needs to be copy-constructible, but crow::Request
+ // isn't copy-constructible. So we use a shared_ptr to the request to avoid
+ // copying the request.
+ // TODO: making a copy for every GET request is expensive,
+ // Cleanup is required here.
+ auto newReq = std::make_shared<crow::Request>(req.copy());
+
delegated = query_param::delegate(queryCapabilities, *queryOpt);
std::function<void(crow::Response&)> handler =
asyncResp->res.releaseCompleteRequestHandler();
asyncResp->res.setCompleteRequestHandler(
[&app, handler(std::move(handler)), query{std::move(*queryOpt)},
- delegated{delegated}](crow::Response& resIn) mutable {
- processAllParams(app, query, delegated, handler, resIn);
+ delegated{delegated},
+ newReq{std::move(newReq)}](crow::Response& resIn) mutable {
+ processAllParams(app, query, delegated, handler, resIn, *newReq);
});
return needToCallHandlers;
diff --git a/redfish-core/include/utils/query_param.hpp b/redfish-core/include/utils/query_param.hpp
index 57b5d06..53b8bd7 100644
--- a/redfish-core/include/utils/query_param.hpp
+++ b/redfish-core/include/utils/query_param.hpp
@@ -783,7 +783,8 @@
// Handles the very first level of Expand, and starts a chain of sub-queries
// for deeper levels.
- void startQuery(const Query& query, const Query& delegated)
+ void startQuery(const Query& query, const Query& delegated,
+ const crow::Request& req)
{
std::vector<ExpandNode> nodes = findNavigationReferences(
query.expandType, query.expandLevel, delegated.expandLevel,
@@ -810,6 +811,15 @@
return;
}
+ if (req.session == nullptr)
+ {
+ BMCWEB_LOG_ERROR("Session is null");
+ messages::internalError(finalRes->res);
+ return;
+ }
+ // Share the session from the original request
+ newReq->session = req.session;
+
auto asyncResp = std::make_shared<bmcweb::AsyncResp>();
BMCWEB_LOG_DEBUG("setting completion handler on {}",
logPtr(&asyncResp->res));
@@ -987,7 +997,7 @@
inline void processAllParams(
crow::App& app, const Query& query, const Query& delegated,
std::function<void(crow::Response&)>& completionHandler,
- crow::Response& intermediateResponse)
+ crow::Response& intermediateResponse, const crow::Request& req)
{
if (!completionHandler)
{
@@ -1023,7 +1033,7 @@
asyncResp->res.setCompleteRequestHandler(std::move(completionHandler));
auto multi = std::make_shared<MultiAsyncResp>(app, asyncResp);
- multi->startQuery(query, delegated);
+ multi->startQuery(query, delegated, req);
return;
}