diff options
author | Przemyslaw Czarnowski <przemyslaw.hawrylewicz.czarnowski@intel.com> | 2020-02-24 12:23:56 +0300 |
---|---|---|
committer | Przemyslaw Czarnowski <przemyslaw.hawrylewicz.czarnowski@intel.com> | 2020-03-05 11:42:23 +0300 |
commit | 250b0ebb0e8d55882fa8e6b156f88828a7ba185d (patch) | |
tree | 146596763a6cc809899e0d931fe0f6bd6012760b | |
parent | 363c23022eb3fb0cde577405e8a084a2e819b642 (diff) | |
download | bmcweb-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.h | 11 | ||||
-rw-r--r-- | include/nbd_proxy.hpp | 196 |
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) { |