diff options
| -rw-r--r-- | redfish-core/include/node.hpp | 21 | ||||
| -rw-r--r-- | redfish-core/include/privileges.hpp | 80 | ||||
| -rw-r--r-- | redfish-core/lib/account_service.hpp | 38 | ||||
| -rw-r--r-- | redfish-core/lib/redfish_sessions.hpp | 20 |
4 files changed, 141 insertions, 18 deletions
diff --git a/redfish-core/include/node.hpp b/redfish-core/include/node.hpp index 936e19f7e3..fddeaa01e5 100644 --- a/redfish-core/include/node.hpp +++ b/redfish-core/include/node.hpp @@ -168,6 +168,27 @@ class Node res.result(boost::beast::http::status::method_not_allowed); res.end(); } + + /* @brief Would the operation be allowed if the user did not have + * the ConfigureSelf Privilege? + * + * @param req the request + * + * @returns True if allowed, false otherwise + */ + inline bool isAllowedWithoutConfigureSelf(const crow::Request& req) + { + const std::string& userRole = + crow::persistent_data::UserRoleMap::getInstance().getUserRole( + req.session->username); + Privileges effectiveUserPrivileges = + redfish::getUserPrivileges(userRole); + effectiveUserPrivileges.resetSinglePrivilege("ConfigureSelf"); + const auto& requiredPrivilegesIt = entityPrivileges.find(req.method()); + return (requiredPrivilegesIt != entityPrivileges.end()) && + isOperationAllowedWithPrivileges(requiredPrivilegesIt->second, + effectiveUserPrivileges); + } }; } // namespace redfish diff --git a/redfish-core/include/privileges.hpp b/redfish-core/include/privileges.hpp index 423f95be60..1ca57fad36 100644 --- a/redfish-core/include/privileges.hpp +++ b/redfish-core/include/privileges.hpp @@ -50,7 +50,8 @@ static const std::vector<std::string> privilegeNames{basePrivileges.begin(), /** * @brief Redfish privileges * - * Entity privileges and user privileges are represented by this class. + * This implements a set of Redfish privileges. These directly represent + * user privileges and help represent entity privileges. * * Each incoming Connection requires a comparison between privileges held * by the user issuing a request and the target entity's privileges. @@ -127,6 +128,28 @@ class Privileges } /** + * @brief Resets the given privilege in the bitset + * + * @param[in] privilege Privilege to be reset + * + * @return None + * + */ + bool resetSinglePrivilege(const char* privilege) + { + for (size_t searchIndex = 0; searchIndex < privilegeNames.size(); + searchIndex++) + { + if (privilege == privilegeNames[searchIndex]) + { + privilegeBitset.reset(searchIndex); + return true; + } + } + return false; + } + + /** * @brief Retrieves names of all active privileges for a given type * * @param[in] type Base or OEM @@ -206,9 +229,49 @@ inline const Privileges& getUserPrivileges(const std::string& userRole) } } +/** + * @brief The OperationMap represents the privileges required for a + * single entity (URI). It maps from the allowable verbs to the + * privileges required to use that operation. + * + * This represents the Redfish "Privilege AND and OR syntax" as given + * in the spec and shown in the Privilege Registry. This does not + * implement any Redfish property overrides, subordinate overrides, or + * resource URI overrides. This does not implement the limitation of + * the ConfigureSelf privilege to operate only on your own account or + * session. + **/ using OperationMap = boost::container::flat_map<boost::beast::http::verb, std::vector<Privileges>>; +/* @brief Checks if user is allowed to call an operation + * + * @param[in] operationPrivilegesRequired Privileges required + * @param[in] userPrivileges Privileges the user has + * + * @return True if operation is allowed, false otherwise + */ +inline bool isOperationAllowedWithPrivileges( + const std::vector<Privileges>& operationPrivilegesRequired, + const Privileges& userPrivileges) +{ + // If there are no privileges assigned, there are no privileges required + if (operationPrivilegesRequired.empty()) + { + return true; + } + for (auto& requiredPrivileges : operationPrivilegesRequired) + { + BMCWEB_LOG_ERROR << "Checking operation privileges..."; + if (userPrivileges.isSupersetOf(requiredPrivileges)) + { + BMCWEB_LOG_ERROR << "...success"; + return true; + } + } + return false; +} + /** * @brief Checks if given privileges allow to call an HTTP method * @@ -228,20 +291,7 @@ inline bool isMethodAllowedWithPrivileges(const boost::beast::http::verb method, return false; } - // If there are no privileges assigned, assume no privileges required - if (it->second.empty()) - { - return true; - } - - for (auto& requiredPrivileges : it->second) - { - if (userPrivileges.isSupersetOf(requiredPrivileges)) - { - return true; - } - } - return false; + return isOperationAllowedWithPrivileges(it->second, userPrivileges); } /** diff --git a/redfish-core/lib/account_service.hpp b/redfish-core/lib/account_service.hpp index 637be86c49..9f066e3c42 100644 --- a/redfish-core/lib/account_service.hpp +++ b/redfish-core/lib/account_service.hpp @@ -1499,7 +1499,8 @@ class ManagerAccount : public Node {boost::beast::http::verb::get, {{"ConfigureUsers"}, {"ConfigureManager"}, {"ConfigureSelf"}}}, {boost::beast::http::verb::head, {{"Login"}}}, - {boost::beast::http::verb::patch, {{"ConfigureUsers"}}}, + {boost::beast::http::verb::patch, + {{"ConfigureUsers"}, {"ConfigureSelf"}}}, {boost::beast::http::verb::put, {{"ConfigureUsers"}}}, {boost::beast::http::verb::delete_, {{"ConfigureUsers"}}}, {boost::beast::http::verb::post, {{"ConfigureUsers"}}}}; @@ -1509,7 +1510,6 @@ class ManagerAccount : public Node void doGet(crow::Response& res, const crow::Request& req, const std::vector<std::string>& params) override { - auto asyncResp = std::make_shared<AsyncResp>(res); if (params.size() != 1) @@ -1518,6 +1518,21 @@ class ManagerAccount : public Node return; } + // Perform a proper ConfigureSelf authority check. If the + // user is operating on an account not their own, then their + // ConfigureSelf privilege does not apply. In this case, + // perform the authority check again without the user's + // ConfigureSelf privilege. + if (req.session->username != params[0]) + { + if (!isAllowedWithoutConfigureSelf(req)) + { + BMCWEB_LOG_DEBUG << "GET Account denied access"; + messages::insufficientPrivilege(asyncResp->res); + return; + } + } + crow::connections::systemBus->async_method_call( [asyncResp, accountName{std::string(params[0])}]( const boost::system::error_code ec, @@ -1655,6 +1670,25 @@ class ManagerAccount : public Node const std::string& username = params[0]; + // Perform a proper ConfigureSelf authority check. If the + // session is being used to PATCH a property other than + // Password, then the ConfigureSelf privilege does not apply. + // If the user is operating on an account not their own, then + // their ConfigureSelf privilege does not apply. In either + // case, perform the authority check again without the user's + // ConfigureSelf privilege. + if ((username != req.session->username) || + (newUserName || enabled || roleId || locked)) + { + if (!isAllowedWithoutConfigureSelf(req)) + { + BMCWEB_LOG_WARNING << "PATCH Password denied access"; + asyncResp->res.clear(); + messages::insufficientPrivilege(asyncResp->res); + return; + } + } + // if user name is not provided in the patch method or if it // matches the user name in the URI, then we are treating it as updating // user properties other then username. If username provided doesn't diff --git a/redfish-core/lib/redfish_sessions.hpp b/redfish-core/lib/redfish_sessions.hpp index d4085af9a7..88f250bc1d 100644 --- a/redfish-core/lib/redfish_sessions.hpp +++ b/redfish-core/lib/redfish_sessions.hpp @@ -35,7 +35,8 @@ class Sessions : public Node {boost::beast::http::verb::head, {{"Login"}}}, {boost::beast::http::verb::patch, {{"ConfigureManager"}}}, {boost::beast::http::verb::put, {{"ConfigureManager"}}}, - {boost::beast::http::verb::delete_, {{"ConfigureManager"}}}, + {boost::beast::http::verb::delete_, + {{"ConfigureManager"}, {"ConfigureSelf"}}}, {boost::beast::http::verb::post, {{"ConfigureManager"}}}}; } @@ -43,6 +44,7 @@ class Sessions : public Node void doGet(crow::Response& res, const crow::Request& req, const std::vector<std::string>& params) override { + // Note that control also reaches here via doPost and doDelete. auto session = crow::persistent_data::SessionStore::getInstance().getSessionByUid( params[0]); @@ -93,6 +95,22 @@ class Sessions : public Node return; } + // Perform a proper ConfigureSelf authority check. If a + // session is being used to DELETE some other user's session, + // then the ConfigureSelf privilege does not apply. In that + // case, perform the authority check again without the user's + // ConfigureSelf privilege. + if (session->username != req.session->username) + { + if (!isAllowedWithoutConfigureSelf(req)) + { + BMCWEB_LOG_WARNING << "DELETE Session denied access"; + messages::insufficientPrivilege(res); + res.end(); + return; + } + } + // DELETE should return representation of object that will be removed doGet(res, req, params); |