Add Support for privilege check in handleUpgrade
This commit enables privilege check for user(s) in case of upgraded
connections.
Currently users with no privileges will also be able to access
Websockets connections (Ex: KVM).
The privilege check was already in place for normal connections (i.e.
router->handle()). This commit lifts off the privilege check code and
moves it into a common function (validatePrivilege()), which can be used
both by handle() and handleUpgrade() and register required callback to
be called.
Also, the const qualifier for Request in the handleUpgrade() function's
signature is removed to enable setting "isConfigureSelf" field of
request. The signature of handleUpgrade() is made identical to handle()
Tested:
- websocket_test.py Passed
- Admin and Operator users are able to access KVM on WebUI
- Readonly User was unable to access KVM on WebUI
Signed-off-by: P Dheeraj Srujan Kumar <p.dheeraj.srujan.kumar@intel.com>
Change-Id: I6f743c27e7e6077f1c6c56e6958922027e4404e8
diff --git a/http/app.hpp b/http/app.hpp
index a2892ce..60c2215 100644
--- a/http/app.hpp
+++ b/http/app.hpp
@@ -58,7 +58,7 @@
App& operator=(const App&&) = delete;
template <typename Adaptor>
- void handleUpgrade(const Request& req,
+ void handleUpgrade(Request& req,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
Adaptor&& adaptor)
{
diff --git a/http/routing.hpp b/http/routing.hpp
index 40218fb..e86c774 100644
--- a/http/routing.hpp
+++ b/http/routing.hpp
@@ -57,6 +57,7 @@
virtual void handle(const Request& /*req*/,
const std::shared_ptr<bmcweb::AsyncResp>&,
const RoutingParams&) = 0;
+#ifndef BMCWEB_ENABLE_SSL
virtual void
handleUpgrade(const Request& /*req*/,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
@@ -64,7 +65,7 @@
{
asyncResp->res.result(boost::beast::http::status::not_found);
}
-#ifdef BMCWEB_ENABLE_SSL
+#else
virtual void handleUpgrade(
const Request& /*req*/,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
@@ -348,6 +349,7 @@
asyncResp->res.result(boost::beast::http::status::not_found);
}
+#ifndef BMCWEB_ENABLE_SSL
void handleUpgrade(const Request& req,
const std::shared_ptr<bmcweb::AsyncResp>& /*asyncResp*/,
boost::asio::ip::tcp::socket&& adaptor) override
@@ -361,7 +363,7 @@
closeHandler, errorHandler);
myConnection->start();
}
-#ifdef BMCWEB_ENABLE_SSL
+#else
void handleUpgrade(const Request& req,
const std::shared_ptr<bmcweb::AsyncResp>& /*asyncResp*/,
boost::beast::ssl_stream<boost::asio::ip::tcp::socket>&&
@@ -1243,20 +1245,10 @@
return findRoute;
}
- static void
- isUserPrivileged(const boost::system::error_code& ec, Request& req,
- const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
- BaseRule& rule, const RoutingParams& params,
- const dbus::utility::DBusPropertiesMap& userInfoMap)
+ static bool isUserPrivileged(
+ Request& req, const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
+ BaseRule& rule, const dbus::utility::DBusPropertiesMap& userInfoMap)
{
- if (ec)
- {
- BMCWEB_LOG_ERROR << "GetUserInfo failed...";
- asyncResp->res.result(
- boost::beast::http::status::internal_server_error);
- return;
- }
-
std::string userRole{};
const std::string* userRolePtr = nullptr;
const bool* remoteUser = nullptr;
@@ -1271,7 +1263,7 @@
{
asyncResp->res.result(
boost::beast::http::status::internal_server_error);
- return;
+ return false;
}
if (userRolePtr != nullptr)
@@ -1286,7 +1278,7 @@
BMCWEB_LOG_ERROR << "RemoteUser property missing or wrong type";
asyncResp->res.result(
boost::beast::http::status::internal_server_error);
- return;
+ return false;
}
bool expired = false;
if (passwordExpired == nullptr)
@@ -1298,7 +1290,7 @@
" local user but is missing or wrong type";
asyncResp->res.result(
boost::beast::http::status::internal_server_error);
- return;
+ return false;
}
}
else
@@ -1334,15 +1326,60 @@
"redfish", "v1", "AccountService",
"Accounts", req.session->username));
}
- return;
+ return false;
}
req.userRole = userRole;
- rule.handle(req, asyncResp, params);
+
+ return true;
+ }
+
+ template <typename CallbackFn>
+ void afterGetUserInfo(Request& req,
+ const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
+ BaseRule& rule, CallbackFn&& callback,
+ const boost::system::error_code& ec,
+ const dbus::utility::DBusPropertiesMap& userInfoMap)
+ {
+ if (ec)
+ {
+ BMCWEB_LOG_ERROR << "GetUserInfo failed...";
+ asyncResp->res.result(
+ boost::beast::http::status::internal_server_error);
+ return;
+ }
+
+ if (!Router::isUserPrivileged(req, asyncResp, rule, userInfoMap))
+ {
+ // User is not privileged
+ BMCWEB_LOG_ERROR << "Insufficient Privilege";
+ asyncResp->res.result(boost::beast::http::status::forbidden);
+ return;
+ }
+ callback();
+ }
+
+ template <typename CallbackFn>
+ void validatePrivilege(Request& req,
+ const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
+ BaseRule& rule, CallbackFn&& callback)
+ {
+ crow::connections::systemBus->async_method_call(
+ [this, &req, asyncResp, &rule,
+ callback(std::forward<CallbackFn>(callback))](
+ const boost::system::error_code& ec,
+ const dbus::utility::DBusPropertiesMap& userInfoMap) mutable {
+ afterGetUserInfo(req, asyncResp, rule,
+ std::forward<CallbackFn>(callback), ec,
+ userInfoMap);
+ },
+ "xyz.openbmc_project.User.Manager", "/xyz/openbmc_project/user",
+ "xyz.openbmc_project.User.Manager", "GetUserInfo",
+ req.session->username);
}
template <typename Adaptor>
- void handleUpgrade(const Request& req,
+ void handleUpgrade(Request& req,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
Adaptor&& adaptor)
{
@@ -1372,24 +1409,28 @@
throw std::runtime_error("Trie internal structure corrupted!");
}
- if ((rules[ruleIndex]->getMethods() &
- (1U << static_cast<size_t>(*verb))) == 0)
+ BaseRule& rule = *rules[ruleIndex];
+ size_t methods = rule.getMethods();
+ if ((methods & (1U << static_cast<size_t>(*verb))) == 0)
{
- BMCWEB_LOG_DEBUG << "Rule found but method mismatch: "
- << req.url().encoded_path() << " with "
- << req.methodString() << "("
- << static_cast<uint32_t>(*verb) << ") / "
- << rules[ruleIndex]->getMethods();
+ BMCWEB_LOG_DEBUG
+ << "Rule found but method mismatch: "
+ << req.url().encoded_path() << " with " << req.methodString()
+ << "(" << static_cast<uint32_t>(*verb) << ") / " << methods;
asyncResp->res.result(boost::beast::http::status::not_found);
return;
}
- BMCWEB_LOG_DEBUG << "Matched rule (upgrade) '" << rules[ruleIndex]->rule
- << "' " << static_cast<uint32_t>(*verb) << " / "
- << rules[ruleIndex]->getMethods();
+ BMCWEB_LOG_DEBUG << "Matched rule (upgrade) '" << rule.rule << "' "
+ << static_cast<uint32_t>(*verb) << " / " << methods;
- rules[ruleIndex]->handleUpgrade(req, asyncResp,
- std::forward<Adaptor>(adaptor));
+ // TODO(ed) This should be able to use std::bind_front, but it doesn't
+ // appear to work with the std::move on adaptor.
+ validatePrivilege(req, asyncResp, rule,
+ [&rule, &req, asyncResp,
+ adaptor(std::forward<Adaptor>(adaptor))]() mutable {
+ rule.handleUpgrade(req, asyncResp, std::move(adaptor));
+ });
}
void handle(Request& req,
@@ -1457,19 +1498,11 @@
rule.handle(req, asyncResp, params);
return;
}
- std::string username = req.session->username;
- crow::connections::systemBus->async_method_call(
- [req{std::move(req)}, asyncResp, &rule, params](
-
- const boost::system::error_code& ec,
- const dbus::utility::DBusPropertiesMap& userInfoMap
-
- ) mutable {
- isUserPrivileged(ec, req, asyncResp, rule, params, userInfoMap);
- },
- "xyz.openbmc_project.User.Manager", "/xyz/openbmc_project/user",
- "xyz.openbmc_project.User.Manager", "GetUserInfo", username);
+ validatePrivilege(req, asyncResp, rule,
+ std::bind_front(&BaseRule::handle, std::ref(rule),
+ std::ref(req), asyncResp,
+ std::ref(params)));
}
void debugPrint()