summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPrzemyslaw Czarnowski <przemyslaw.hawrylewicz.czarnowski@intel.com>2020-02-24 12:23:56 +0300
committerPrzemyslaw Czarnowski <przemyslaw.hawrylewicz.czarnowski@intel.com>2020-03-05 11:42:23 +0300
commit250b0ebb0e8d55882fa8e6b156f88828a7ba185d (patch)
tree146596763a6cc809899e0d931fe0f6bd6012760b
parent363c23022eb3fb0cde577405e8a084a2e819b642 (diff)
downloadbmcweb-250b0ebb0e8d55882fa8e6b156f88828a7ba185d.tar.xz
Permission check for virtual media proxy mode
This patch enables checking of user permission for proxy mode, as start of this kind service is not triggered by redfish (which has permission check by default). Permission check is done in .onopen handler of websocket. For this reason another dbus call for user privileges is added to verify if user has "ConfigureManager" privilege. I have chosen this approach, as generic privilege check for all websockets introduces significant changes in connection upgrade flow which makes implementaion vague and caused some memory issues difficult to track down. It is worth noting that other websockets (eg. kvm) uses .required() function to set privilege but this information is lost during connection upgrade and is not checked anywhere in upgrade flow. Tested: Manual tests with opening websockets via web browser and dedicated nbd proxy utility. For users with/without appropriate permissions. Single request and burst of requests has been tested as well. Change-Id: I2a56bec606fa0e5f3d4232e48794c9055bf6095e Signed-off-by: Przemyslaw Czarnowski <przemyslaw.hawrylewicz.czarnowski@intel.com>
-rw-r--r--http/websocket.h11
-rw-r--r--include/nbd_proxy.hpp196
2 files changed, 143 insertions, 64 deletions
diff --git a/http/websocket.h b/http/websocket.h
index f7c818e6e1..c467d25594 100644
--- a/http/websocket.h
+++ b/http/websocket.h
@@ -23,6 +23,9 @@ struct Connection : std::enable_shared_from_this<Connection>
explicit Connection(const crow::Request& reqIn) :
req(reqIn.req), userdataPtr(nullptr){};
+ explicit Connection(const crow::Request& reqIn, std::string user) :
+ req(reqIn.req), userName{std::move(user)}, userdataPtr(nullptr){};
+
virtual void sendBinary(const std::string_view msg) = 0;
virtual void sendBinary(std::string&& msg) = 0;
virtual void sendText(const std::string_view msg) = 0;
@@ -40,10 +43,16 @@ struct Connection : std::enable_shared_from_this<Connection>
return userdataPtr;
}
+ const std::string& getUserName() const
+ {
+ return userName;
+ }
+
boost::beast::http::request<boost::beast::http::string_body> req;
crow::Response res;
private:
+ std::string userName{};
void* userdataPtr;
};
@@ -58,7 +67,7 @@ template <typename Adaptor> class ConnectionImpl : public Connection
message_handler,
std::function<void(Connection&, const std::string&)> close_handler,
std::function<void(Connection&)> error_handler) :
- Connection(reqIn),
+ Connection(reqIn, reqIn.session->username),
ws(std::move(adaptorIn)), inString(), inBuffer(inString, 131088),
openHandler(std::move(open_handler)),
messageHandler(std::move(message_handler)),
diff --git a/include/nbd_proxy.hpp b/include/nbd_proxy.hpp
index 6922ae2765..12fbb8ed10 100644
--- a/include/nbd_proxy.hpp
+++ b/include/nbd_proxy.hpp
@@ -23,6 +23,7 @@
#include <boost/container/flat_map.hpp>
#include <dbus_utility.hpp>
#include <experimental/filesystem>
+#include <privileges.hpp>
#include <variant>
#include <webserver_common.hpp>
@@ -35,6 +36,7 @@ namespace nbd_proxy
using boost::asio::local::stream_protocol;
static constexpr auto nbdBufferSize = 131088;
+static const char* requiredPrivilegeString = "ConfigureManager";
struct NbdProxyServer : std::enable_shared_from_this<NbdProxyServer>
{
@@ -250,105 +252,173 @@ void requestRoutes(CrowApp& app)
{
BMCWEB_ROUTE(app, "/nbd/<str>")
.websocket()
- .onopen([&app](crow::websocket::Connection& conn,
- std::shared_ptr<bmcweb::AsyncResp> asyncResp) {
+ .onopen([](crow::websocket::Connection& conn,
+ std::shared_ptr<bmcweb::AsyncResp> asyncResp) {
BMCWEB_LOG_DEBUG << "nbd-proxy.onopen(" << &conn << ")";
- for (const auto session : sessions)
- {
- if (session.second->getEndpointId() == conn.req.target())
+ auto getUserInfoHandler = [&conn, asyncResp{std::move(asyncResp)}](
+ const boost::system::error_code ec,
+ boost::container::flat_map<
+ std::string,
+ std::variant<
+ bool, std::string,
+ std::vector<std::string>>>
+ userInfo) {
+ if (ec)
{
- BMCWEB_LOG_ERROR
- << "Cannot open new connection - socket is in use";
+ BMCWEB_LOG_ERROR << "GetUserInfo failed...";
+ asyncResp->res.result(
+ boost::beast::http::status::internal_server_error);
return;
}
- }
- auto openHandler = [asyncResp, &conn](
- const boost::system::error_code ec,
- dbus::utility::ManagedObjectType& objects) {
- const std::string* socketValue = nullptr;
- const std::string* endpointValue = nullptr;
- const std::string* endpointObjectPath = nullptr;
-
- if (ec)
+ const std::string* userRolePtr = nullptr;
+ auto userInfoIter = userInfo.find("UserPrivilege");
+ if (userInfoIter != userInfo.end())
{
- BMCWEB_LOG_ERROR << "DBus error: " << ec.message();
- return;
+ userRolePtr =
+ std::get_if<std::string>(&userInfoIter->second);
}
- for (const auto& objectPath : objects)
+ std::string userRole{};
+ if (userRolePtr != nullptr)
{
- const auto interfaceMap = objectPath.second.find(
- "xyz.openbmc_project.VirtualMedia.MountPoint");
+ userRole = *userRolePtr;
+ BMCWEB_LOG_DEBUG << "userName = " << conn.getUserName()
+ << " userRole = " << *userRolePtr;
+ }
- if (interfaceMap == objectPath.second.end())
- {
- BMCWEB_LOG_DEBUG << "Cannot find MountPoint object";
- continue;
- }
+ // Get the user privileges from the role
+ ::redfish::Privileges userPrivileges =
+ ::redfish::getUserPrivileges(userRole);
+
+ const ::redfish::Privileges requiredPrivileges{
+ requiredPrivilegeString};
- const auto endpoint =
- interfaceMap->second.find("EndpointId");
- if (endpoint == interfaceMap->second.end())
+ if (!userPrivileges.isSupersetOf(requiredPrivileges))
+ {
+ BMCWEB_LOG_DEBUG << "User " << conn.getUserName()
+ << " not authorized for nbd connection";
+ asyncResp->res.result(
+ boost::beast::http::status::unauthorized);
+ return;
+ }
+
+ for (const auto session : sessions)
+ {
+ if (session.second->getEndpointId() == conn.req.target())
{
- BMCWEB_LOG_DEBUG << "Cannot find EndpointId property";
- continue;
+ BMCWEB_LOG_ERROR
+ << "Cannot open new connection - socket is in use";
+ asyncResp->res.result(
+ boost::beast::http::status::bad_request);
+ return;
}
+ }
- endpointValue = std::get_if<std::string>(&endpoint->second);
+ auto openHandler = [asyncResp,
+ &conn](const boost::system::error_code ec,
+ dbus::utility::ManagedObjectType&
+ objects) {
+ const std::string* socketValue = nullptr;
+ const std::string* endpointValue = nullptr;
+ const std::string* endpointObjectPath = nullptr;
- if (endpointValue == nullptr)
+ if (ec)
{
- BMCWEB_LOG_ERROR << "EndpointId property value is null";
- continue;
+ BMCWEB_LOG_ERROR << "DBus error: " << ec.message();
+ asyncResp->res.result(
+ boost::beast::http::status::internal_server_error);
+ return;
}
- if (*endpointValue == conn.req.target())
+ for (const auto& objectPath : objects)
{
- const auto socket = interfaceMap->second.find("Socket");
- if (socket == interfaceMap->second.end())
+ const auto interfaceMap = objectPath.second.find(
+ "xyz.openbmc_project.VirtualMedia.MountPoint");
+
+ if (interfaceMap == objectPath.second.end())
{
- BMCWEB_LOG_DEBUG << "Cannot find Socket property";
+ BMCWEB_LOG_DEBUG << "Cannot find MountPoint object";
continue;
}
- socketValue = std::get_if<std::string>(&socket->second);
- if (socketValue == nullptr)
+ const auto endpoint =
+ interfaceMap->second.find("EndpointId");
+ if (endpoint == interfaceMap->second.end())
{
- BMCWEB_LOG_ERROR << "Socket property value is null";
+ BMCWEB_LOG_DEBUG
+ << "Cannot find EndpointId property";
continue;
}
- endpointObjectPath = &objectPath.first.str;
- break;
+ endpointValue =
+ std::get_if<std::string>(&endpoint->second);
+
+ if (endpointValue == nullptr)
+ {
+ BMCWEB_LOG_ERROR
+ << "EndpointId property value is null";
+ continue;
+ }
+
+ if (*endpointValue == conn.req.target())
+ {
+ const auto socket =
+ interfaceMap->second.find("Socket");
+ if (socket == interfaceMap->second.end())
+ {
+ BMCWEB_LOG_DEBUG
+ << "Cannot find Socket property";
+ continue;
+ }
+
+ socketValue =
+ std::get_if<std::string>(&socket->second);
+ if (socketValue == nullptr)
+ {
+ BMCWEB_LOG_ERROR
+ << "Socket property value is null";
+ continue;
+ }
+
+ endpointObjectPath = &objectPath.first.str;
+ break;
+ }
}
- }
- if (endpointObjectPath == nullptr)
- {
- BMCWEB_LOG_ERROR << "Cannot find requested EndpointId";
- asyncResp->res.result(
- boost::beast::http::status::not_found);
- return;
- }
+ if (endpointObjectPath == nullptr)
+ {
+ BMCWEB_LOG_ERROR << "Cannot find requested EndpointId";
+ asyncResp->res.result(
+ boost::beast::http::status::not_found);
+ return;
+ }
- // If the socket file exists (i.e. after bmcweb crash), we
- // cannot reuse it.
- std::remove((*socketValue).c_str());
+ // If the socket file exists (i.e. after bmcweb crash),
+ // we cannot reuse it.
+ std::remove((*socketValue).c_str());
- sessions[&conn] = std::make_shared<NbdProxyServer>(
- conn, std::move(*socketValue), std::move(*endpointValue),
- std::move(*endpointObjectPath));
+ sessions[&conn] = std::make_shared<NbdProxyServer>(
+ conn, std::move(*socketValue),
+ std::move(*endpointValue),
+ std::move(*endpointObjectPath));
- sessions[&conn]->run();
+ sessions[&conn]->run();
- asyncResp->res.result(boost::beast::http::status::ok);
+ asyncResp->res.result(boost::beast::http::status::ok);
+ };
+ crow::connections::systemBus->async_method_call(
+ std::move(openHandler), "xyz.openbmc_project.VirtualMedia",
+ "/xyz/openbmc_project/VirtualMedia",
+ "org.freedesktop.DBus.ObjectManager", "GetManagedObjects");
};
+
crow::connections::systemBus->async_method_call(
- std::move(openHandler), "xyz.openbmc_project.VirtualMedia",
- "/xyz/openbmc_project/VirtualMedia",
- "org.freedesktop.DBus.ObjectManager", "GetManagedObjects");
+ std::move(getUserInfoHandler),
+ "xyz.openbmc_project.User.Manager", "/xyz/openbmc_project/user",
+ "xyz.openbmc_project.User.Manager", "GetUserInfo",
+ conn.getUserName());
})
.onclose(
[](crow::websocket::Connection& conn, const std::string& reason) {