Authentication support for Legacy mode
This change introduces new 'Mount' API argument - UNIX_FD for unnamed pipe.
This unnamed pipe is utilized to securely send secret data over D-Bus.
Currently data consists of null-terminated char buffers with username and
password, that are passed as InsertMedia action parameters.
Data on receiving side is encapsulated into classes whose role is to:
- keep secret as short-lived as possible
- erase secret from memory when it's not needed
- pass secrets (and format them) to another secure container with above
capabilities
New classes:
- Credentials: is a class encapsulating login and password. It zeroes them
at destruction.
- CredentialProvider: contains Credentials, specifies SecureBuffer, allows
to store credentials in SecureBuffer
New behavior:
- When credentials are provided they are encapsulated as char array of two
null-terminated strings
- Pipe is opened as a medium to send this buffer
- UNIX_FD of the pipe source is passed in ‘Mount’ call. Virtual-Media
service reads from credentials over the pipe
Tested:
Manual and automated tests:
- positive and negative tests for authentication on both CIFS and HTTPS
resources
- error injection (ill-formed data transfered over pipe, pipe broken etc.)
Signed-off-by: Agata Olender <agata.olender@intel.com>
Change-Id: I5b330b18c4bff222eab3062abfe27b5adaebf877
diff --git a/redfish-core/lib/virtual_media.hpp b/redfish-core/lib/virtual_media.hpp
index a76ff50..88fd765 100644
--- a/redfish-core/lib/virtual_media.hpp
+++ b/redfish-core/lib/virtual_media.hpp
@@ -16,6 +16,8 @@
#pragma once
#include <boost/container/flat_map.hpp>
+#include <boost/process/async_pipe.hpp>
+#include <boost/type_traits/has_dereference.hpp>
#include <node.hpp>
#include <utils/json_utils.hpp>
// for GetObjectType and ManagedObjectType
@@ -347,12 +349,16 @@
{
// Legacy mode
std::string imageUrl;
+ std::string userName;
+ std::string password;
bool writeProtected;
// Read obligatory paramters (url of image)
if (!json_util::readJson(
req, aResp->res, "Image", imageUrl,
- "WriteProtected", writeProtected))
+ "WriteProtected", writeProtected,
+ "UserName", userName, "Password",
+ password))
{
BMCWEB_LOG_DEBUG
@@ -376,7 +382,9 @@
// dbus calls
doMountVmLegacy(std::move(aResp), service,
resName, imageUrl,
- !writeProtected);
+ !writeProtected,
+ std::move(userName),
+ std::move(password));
return;
}
@@ -395,6 +403,148 @@
"/xyz/openbmc_project/VirtualMedia", std::array<const char *, 0>());
}
+ template <typename T> static void secureCleanup(T &value)
+ {
+ auto raw = const_cast<typename T::value_type *>(value.data());
+ explicit_bzero(raw, value.size() * sizeof(*raw));
+ }
+
+ class Credentials
+ {
+ public:
+ Credentials(std::string &&user, std::string &&password) :
+ userBuf(std::move(user)), passBuf(std::move(password))
+ {
+ }
+
+ ~Credentials()
+ {
+ secureCleanup(userBuf);
+ secureCleanup(passBuf);
+ }
+
+ const std::string &user()
+ {
+ return userBuf;
+ }
+
+ const std::string &password()
+ {
+ return passBuf;
+ }
+
+ private:
+ Credentials() = delete;
+ Credentials(const Credentials &) = delete;
+ Credentials &operator=(const Credentials &) = delete;
+
+ std::string userBuf;
+ std::string passBuf;
+ };
+
+ class CredentialsProvider
+ {
+ public:
+ template <typename T> struct Deleter
+ {
+ void operator()(T *buff) const
+ {
+ if (buff)
+ {
+ secureCleanup(*buff);
+ delete buff;
+ }
+ }
+ };
+
+ using Buffer = std::vector<char>;
+ using SecureBuffer = std::unique_ptr<Buffer, Deleter<Buffer>>;
+ // Using explicit definition instead of std::function to avoid implicit
+ // conversions eg. stack copy instead of reference
+ using FormatterFunc = void(const std::string &username,
+ const std::string &password, Buffer &dest);
+
+ CredentialsProvider(std::string &&user, std::string &&password) :
+ credentials(std::move(user), std::move(password))
+ {
+ }
+
+ const std::string &user()
+ {
+ return credentials.user();
+ }
+
+ const std::string &password()
+ {
+ return credentials.password();
+ }
+
+ SecureBuffer pack(const FormatterFunc formatter)
+ {
+ SecureBuffer packed{new Buffer{}};
+ if (formatter)
+ {
+ formatter(credentials.user(), credentials.password(), *packed);
+ }
+
+ return packed;
+ }
+
+ private:
+ Credentials credentials;
+ };
+
+ // Wrapper for boost::async_pipe ensuring proper pipe cleanup
+ template <typename Buffer> class Pipe
+ {
+ public:
+ using unix_fd = sdbusplus::message::unix_fd;
+
+ Pipe(boost::asio::io_context &io, Buffer &&buffer) :
+ impl(io), buffer{std::move(buffer)}
+ {
+ }
+
+ ~Pipe()
+ {
+ // Named pipe needs to be explicitly removed
+ impl.close();
+ }
+
+ unix_fd fd()
+ {
+ return unix_fd{impl.native_source()};
+ }
+
+ template <typename WriteHandler>
+ void async_write(WriteHandler &&handler)
+ {
+ impl.async_write_some(data(), std::forward<WriteHandler>(handler));
+ }
+
+ private:
+ // Specialization for pointer types
+ template <typename B = Buffer>
+ typename std::enable_if<boost::has_dereference<B>::value,
+ boost::asio::const_buffer>::type
+ data()
+ {
+ return boost::asio::buffer(*buffer);
+ }
+
+ template <typename B = Buffer>
+ typename std::enable_if<!boost::has_dereference<B>::value,
+ boost::asio::const_buffer>::type
+ data()
+ {
+ return boost::asio::buffer(buffer);
+ }
+
+ const std::string name;
+ boost::process::async_pipe impl;
+ Buffer buffer;
+ };
+
/**
* @brief Function transceives data with dbus directly.
*
@@ -402,10 +552,60 @@
*/
void doMountVmLegacy(std::shared_ptr<AsyncResp> asyncResp,
const std::string &service, const std::string &name,
- const std::string &imageUrl, const bool rw)
+ const std::string &imageUrl, const bool rw,
+ std::string &&userName, std::string &&password)
{
+ using SecurePipe = Pipe<CredentialsProvider::SecureBuffer>;
+ constexpr const size_t secretLimit = 1024;
+
+ std::shared_ptr<SecurePipe> secretPipe;
+ std::variant<int, SecurePipe::unix_fd> unixFd = -1;
+
+ if (!userName.empty() || !password.empty())
+ {
+ // Encapsulate in safe buffer
+ CredentialsProvider credentials(std::move(userName),
+ std::move(password));
+
+ // Payload must contain data + NULL delimiters
+ if (credentials.user().size() + credentials.password().size() + 2 >
+ secretLimit)
+ {
+ BMCWEB_LOG_ERROR << "Credentials too long to handle";
+ messages::unrecognizedRequestBody(asyncResp->res);
+ return;
+ }
+
+ // Pack secret
+ auto secret = credentials.pack([](const auto &user,
+ const auto &pass, auto &buff) {
+ std::copy(user.begin(), user.end(), std::back_inserter(buff));
+ buff.push_back('\0');
+ std::copy(pass.begin(), pass.end(), std::back_inserter(buff));
+ buff.push_back('\0');
+ });
+
+ // Open pipe
+ secretPipe = std::make_shared<SecurePipe>(
+ crow::connections::systemBus->get_io_context(),
+ std::move(secret));
+ unixFd = secretPipe->fd();
+
+ // Pass secret over pipe
+ secretPipe->async_write(
+ [asyncResp](const boost::system::error_code &ec,
+ std::size_t size) {
+ if (ec)
+ {
+ BMCWEB_LOG_ERROR << "Failed to pass secret: " << ec;
+ messages::internalError(asyncResp->res);
+ }
+ });
+ }
+
crow::connections::systemBus->async_method_call(
- [asyncResp](const boost::system::error_code ec, bool success) {
+ [asyncResp, secretPipe](const boost::system::error_code ec,
+ bool success) {
if (ec)
{
BMCWEB_LOG_ERROR << "Bad D-Bus request error: " << ec;
@@ -418,7 +618,8 @@
}
},
service, "/xyz/openbmc_project/VirtualMedia/Legacy/" + name,
- "xyz.openbmc_project.VirtualMedia.Legacy", "Mount", imageUrl, rw);
+ "xyz.openbmc_project.VirtualMedia.Legacy", "Mount", imageUrl, rw,
+ unixFd);
}
};