summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJoseph Reynolds <joseph-reynolds@charter.net>2019-11-25 15:37:29 -0600
committerJoseph Reynolds <joseph-reynolds@charter.net>2019-12-06 18:20:13 +0000
commit900f949773795141266271107219ea019f2839cd (patch)
treec5f94af49249e824c3a979856a39f05020954084
parentd7e080295f1f3c2517a440e3911600cec0c190fa (diff)
downloadbmcweb-900f949773795141266271107219ea019f2839cd.tar.gz
bmcweb-900f949773795141266271107219ea019f2839cd.zip
bmcweb: Handle ConfigureSelf privilege
Enhances BMCWeb to correctly handle the Redfish ConfigureSelf privilege. Redfish document DSP2046 defines the ConfigureSelf privilege as "Can change the password for the current user account and log out of their own sessions." This notion is formalized in the Redfish DSP8011 PrivilegeRegistry where ConfigureSelf appears in three operations: - ManagerAccount (/redfish/v1/AccountService/Accounts/{account}) GET operation. - ManagerAccount (/redfish/v1/AccountService/Accounts/{account}) PATCH Password property override. - Session (/redfish/v1/SessionService/Sessions/{sessionid}) DELETE operation. Tested: Yes, tested the above operations using users with various Roles to determine which operations are allowed. ReadOnly users (privileges: Login, ConfigureSelf): - Can GET their own account. - Can change their password. - Can log out. - Cannot change any other properties of their own account. - Cannot change anyone else's password. - Cannot GET someone else's account. - Cannot log out anyone else. Operator users (privileges: Login, ConfigureComponents, ConfigureSelf): - Same access as a ReadOnly user. Administrator users (all privileges): - Can do everything Operator can do. - Can change one or more properties of their account - Can GET and change properties of someone else's account. - Can logoff any session. Signed-off-by: Joseph Reynolds <joseph-reynolds@charter.net> Change-Id: If8efd71cb9743a59b7c5fe1565804d21e788ea29
-rw-r--r--redfish-core/include/node.hpp21
-rw-r--r--redfish-core/include/privileges.hpp80
-rw-r--r--redfish-core/lib/account_service.hpp38
-rw-r--r--redfish-core/lib/redfish_sessions.hpp20
4 files changed, 141 insertions, 18 deletions
diff --git a/redfish-core/include/node.hpp b/redfish-core/include/node.hpp
index 936e19f..fddeaa0 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 423f95b..1ca57fa 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 637be86..9f066e3 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 d4085af..88f250b 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);
OpenPOWER on IntegriCloud