Rework Authorization flow
Currently we parse the whole message before authenticating,
allowing an attacker the ability to upload a large image,
or keep a connection open for the max amount of time easier
than it should be. This moves the authentication to the
earliest point possible, and restricts unauthenticated users
timeouts and max upload sizes. It also makes it so that
unauthenticated users cannot keep the connection alive
forever by refusing to close the connection.
Tested:
- login/logout
- firmware update
- large POST when unauthenticated
- timeouts when unauthenticated
- slowhttptest
Change-Id: Ifa02d8db04eac1821e8950eb85e71634a9e6d265
Signed-off-by: James Feist <james.feist@linux.intel.com>
diff --git a/http/http_connection.h b/http/http_connection.h
index 5f4681f..c413990 100644
--- a/http/http_connection.h
+++ b/http/http_connection.h
@@ -7,6 +7,7 @@
#include "timer_queue.h"
#include "utility.h"
+#include "authorization.hpp"
#include "http_utility.hpp"
#include <boost/algorithm/string.hpp>
@@ -248,6 +249,18 @@
constexpr unsigned int httpReqBodyLimit =
1024 * 1024 * BMCWEB_HTTP_REQ_BODY_LIMIT_MB;
+constexpr uint64_t loggedOutPostBodyLimit = 4096;
+
+constexpr uint32_t httpHeaderLimit = 8192;
+
+// drop all connections after 1 minute, this time limit was chosen
+// arbitrarily and can be adjusted later if needed
+static constexpr const size_t loggedInAttempts =
+ (60 / timerQueueTimeoutSeconds);
+
+static constexpr const size_t loggedOutAttempts =
+ (15 / timerQueueTimeoutSeconds);
+
template <typename Adaptor, typename Handler, typename... Middlewares>
class Connection :
public std::enable_shared_from_this<
@@ -265,10 +278,8 @@
timerQueue(timerQueueIn)
{
parser.emplace(std::piecewise_construct, std::make_tuple());
- // Temporarily set by the BMCWEB_HTTP_REQ_BODY_LIMIT_MB variable; Need
- // to modify uploading/authentication mechanism to a better method that
- // disallows a DOS attack based on a large file size.
parser->body_limit(httpReqBodyLimit);
+ parser->header_limit(httpHeaderLimit);
req.emplace(parser->get());
#ifdef BMCWEB_ENABLE_MUTUAL_TLS_AUTHENTICATION
@@ -472,7 +483,7 @@
void start()
{
- startDeadline();
+ startDeadline(0);
// TODO(ed) Abstract this to a more clever class with the idea of an
// asynchronous "start"
if constexpr (std::is_same_v<Adaptor,
@@ -498,6 +509,7 @@
void handle()
{
cancelDeadlineTimer();
+
bool isInvalidRequest = false;
// Check for HTTP version 1.1.
@@ -647,6 +659,8 @@
decltype(ctx),
decltype(*middlewares)>(
*middlewares, ctx, *req, res);
+
+ crow::authorization::cleanupTempSession(*req);
}
if (!isAlive())
@@ -731,14 +745,21 @@
}
}
+ cancelDeadlineTimer();
+
if (errorWhileReading)
{
- cancelDeadlineTimer();
close();
BMCWEB_LOG_DEBUG << this << " from read(1)";
return;
}
+ if (!req)
+ {
+ close();
+ return;
+ }
+
// Compute the url parameters for the request
req->url = req->target();
std::size_t index = req->url.find("?");
@@ -746,7 +767,32 @@
{
req->url = req->url.substr(0, index);
}
- req->urlParams = QueryString(std::string(req->target()));
+ crow::authorization::authenticate(*req, res);
+
+ bool loggedIn = req && req->session;
+ if (loggedIn)
+ {
+ startDeadline(loggedInAttempts);
+ BMCWEB_LOG_DEBUG << "Starting slow deadline";
+
+ req->urlParams = QueryString(std::string(req->target()));
+ }
+ else
+ {
+ const boost::optional<uint64_t> contentLength =
+ parser->content_length();
+ if (contentLength &&
+ *contentLength > loggedOutPostBodyLimit)
+ {
+ BMCWEB_LOG_DEBUG << "Content length greater than limit "
+ << *contentLength;
+ close();
+ return;
+ }
+
+ startDeadline(loggedOutAttempts);
+ BMCWEB_LOG_DEBUG << "Starting quick deadline";
+ }
doRead();
});
}
@@ -772,7 +818,20 @@
}
else
{
- if (!isAlive())
+ if (isAlive())
+ {
+ cancelDeadlineTimer();
+ bool loggedIn = req && req->session;
+ if (loggedIn)
+ {
+ startDeadline(loggedInAttempts);
+ }
+ else
+ {
+ startDeadline(loggedOutAttempts);
+ }
+ }
+ else
{
errorWhileReading = true;
}
@@ -790,6 +849,15 @@
void doWrite()
{
+ bool loggedIn = req && req->session;
+ if (loggedIn)
+ {
+ startDeadline(loggedInAttempts);
+ }
+ else
+ {
+ startDeadline(loggedOutAttempts);
+ }
BMCWEB_LOG_DEBUG << this << " doWrite";
res.preparePayload();
serializer.emplace(*res.stringResponse);
@@ -817,8 +885,6 @@
BMCWEB_LOG_DEBUG << this << " Clearing response";
res.clear();
parser.emplace(std::piecewise_construct, std::make_tuple());
- parser->body_limit(httpReqBodyLimit); // reset body limit for
- // newly created parser
buffer.consume(buffer.size());
req.emplace(parser->get());
@@ -837,39 +903,37 @@
}
}
- void startDeadline(size_t timerIterations = 0)
+ void startDeadline(size_t timerIterations)
{
- // drop all connections after 1 minute, this time limit was chosen
- // arbitrarily and can be adjusted later if needed
- constexpr const size_t maxReadAttempts =
- (60 / detail::timerQueueTimeoutSeconds);
-
cancelDeadlineTimer();
- timerCancelKey = timerQueue.add([this, self(shared_from_this()),
- readCount{parser->get().body().size()},
- timerIterations{timerIterations + 1}] {
- // Mark timer as not active to avoid canceling it during
- // Connection destructor which leads to double free issue
- timerCancelKey.reset();
- if (!isAlive())
- {
- return;
- }
+ if (timerIterations)
+ {
+ timerIterations--;
+ }
- // Restart timer if read is in progress.
- // With threshold can be used to drop slow connections
- // to protect against slow-rate DoS attack
- if ((parser->get().body().size() > readCount) &&
- (timerIterations < maxReadAttempts))
- {
- BMCWEB_LOG_DEBUG << this << " restart timer - read in progress";
- startDeadline(timerIterations);
- return;
- }
+ timerCancelKey =
+ timerQueue.add([self(shared_from_this()), timerIterations] {
+ // Mark timer as not active to avoid canceling it during
+ // Connection destructor which leads to double free issue
+ self->timerCancelKey.reset();
+ if (!self->isAlive())
+ {
+ return;
+ }
- close();
- });
+ // Threshold can be used to drop slow connections
+ // to protect against slow-rate DoS attack
+ if (timerIterations)
+ {
+ BMCWEB_LOG_DEBUG << self.get()
+ << " restart timer - read in progress";
+ self->startDeadline(timerIterations);
+ return;
+ }
+
+ self->close();
+ });
if (!timerCancelKey)
{
diff --git a/http/timer_queue.h b/http/timer_queue.h
index d83cd0e..85791ea 100644
--- a/http/timer_queue.h
+++ b/http/timer_queue.h
@@ -10,10 +10,11 @@
namespace crow
{
+
+constexpr const size_t timerQueueTimeoutSeconds = 5;
namespace detail
{
-constexpr const size_t timerQueueTimeoutSeconds = 5;
constexpr const size_t maxSize = 100;
// fast timer queue for fixed tick value.
class TimerQueue
diff --git a/include/authorization.hpp b/include/authorization.hpp
new file mode 100644
index 0000000..9074377
--- /dev/null
+++ b/include/authorization.hpp
@@ -0,0 +1,267 @@
+#pragma once
+
+#include "webroutes.hpp"
+
+#include <app.h>
+#include <common.h>
+#include <http_request.h>
+#include <http_response.h>
+
+#include <boost/algorithm/string/predicate.hpp>
+#include <boost/container/flat_set.hpp>
+#include <http_utility.hpp>
+#include <pam_authenticate.hpp>
+#include <persistent_data_middleware.hpp>
+
+#include <random>
+
+namespace crow
+{
+
+namespace authorization
+{
+
+static void cleanupTempSession(Request& req)
+{
+ // TODO(ed) THis should really be handled by the persistent data
+ // middleware, but because it is upstream, it doesn't have access to the
+ // session information. Should the data middleware persist the current
+ // user session?
+ if (req.session != nullptr &&
+ req.session->persistence ==
+ crow::persistent_data::PersistenceType::SINGLE_REQUEST)
+ {
+ persistent_data::SessionStore::getInstance().removeSession(req.session);
+ }
+}
+
+static const std::shared_ptr<crow::persistent_data::UserSession>
+ performBasicAuth(std::string_view auth_header)
+{
+ BMCWEB_LOG_DEBUG << "[AuthMiddleware] Basic authentication";
+
+ std::string authData;
+ std::string_view param = auth_header.substr(strlen("Basic "));
+ if (!crow::utility::base64Decode(param, authData))
+ {
+ return nullptr;
+ }
+ std::size_t separator = authData.find(':');
+ if (separator == std::string::npos)
+ {
+ return nullptr;
+ }
+
+ std::string user = authData.substr(0, separator);
+ separator += 1;
+ if (separator > authData.size())
+ {
+ return nullptr;
+ }
+ std::string pass = authData.substr(separator);
+
+ BMCWEB_LOG_DEBUG << "[AuthMiddleware] Authenticating user: " << user;
+
+ int pamrc = pamAuthenticateUser(user, pass);
+ bool isConfigureSelfOnly = pamrc == PAM_NEW_AUTHTOK_REQD;
+ if ((pamrc != PAM_SUCCESS) && !isConfigureSelfOnly)
+ {
+ return nullptr;
+ }
+
+ // TODO(ed) generateUserSession is a little expensive for basic
+ // auth, as it generates some random identifiers that will never be
+ // used. This should have a "fast" path for when user tokens aren't
+ // needed.
+ // This whole flow needs to be revisited anyway, as we can't be
+ // calling directly into pam for every request
+ return persistent_data::SessionStore::getInstance().generateUserSession(
+ user, crow::persistent_data::PersistenceType::SINGLE_REQUEST,
+ isConfigureSelfOnly);
+}
+
+static const std::shared_ptr<crow::persistent_data::UserSession>
+ performTokenAuth(std::string_view auth_header)
+{
+ BMCWEB_LOG_DEBUG << "[AuthMiddleware] Token authentication";
+
+ std::string_view token = auth_header.substr(strlen("Token "));
+ auto session =
+ persistent_data::SessionStore::getInstance().loginSessionByToken(token);
+ return session;
+}
+
+static const std::shared_ptr<crow::persistent_data::UserSession>
+ performXtokenAuth(const crow::Request& req)
+{
+ BMCWEB_LOG_DEBUG << "[AuthMiddleware] X-Auth-Token authentication";
+
+ std::string_view token = req.getHeaderValue("X-Auth-Token");
+ if (token.empty())
+ {
+ return nullptr;
+ }
+ auto session =
+ persistent_data::SessionStore::getInstance().loginSessionByToken(token);
+ return session;
+}
+
+static const std::shared_ptr<crow::persistent_data::UserSession>
+ performCookieAuth(const crow::Request& req)
+{
+ BMCWEB_LOG_DEBUG << "[AuthMiddleware] Cookie authentication";
+
+ std::string_view cookieValue = req.getHeaderValue("Cookie");
+ if (cookieValue.empty())
+ {
+ return nullptr;
+ }
+
+ auto startIndex = cookieValue.find("SESSION=");
+ if (startIndex == std::string::npos)
+ {
+ return nullptr;
+ }
+ startIndex += sizeof("SESSION=") - 1;
+ auto endIndex = cookieValue.find(";", startIndex);
+ if (endIndex == std::string::npos)
+ {
+ endIndex = cookieValue.size();
+ }
+ std::string_view authKey =
+ cookieValue.substr(startIndex, endIndex - startIndex);
+
+ const std::shared_ptr<crow::persistent_data::UserSession> session =
+ persistent_data::SessionStore::getInstance().loginSessionByToken(
+ authKey);
+ if (session == nullptr)
+ {
+ return nullptr;
+ }
+#ifndef BMCWEB_INSECURE_DISABLE_CSRF_PREVENTION
+ // RFC7231 defines methods that need csrf protection
+ if (req.method() != "GET"_method)
+ {
+ std::string_view csrf = req.getHeaderValue("X-XSRF-TOKEN");
+ // Make sure both tokens are filled
+ if (csrf.empty() || session->csrfToken.empty())
+ {
+ return nullptr;
+ }
+
+ if (csrf.size() != crow::persistent_data::sessionTokenSize)
+ {
+ return nullptr;
+ }
+ // Reject if csrf token not available
+ if (!crow::utility::constantTimeStringCompare(csrf, session->csrfToken))
+ {
+ return nullptr;
+ }
+ }
+#endif
+ return session;
+}
+
+// checks if request can be forwarded without authentication
+static bool isOnWhitelist(const crow::Request& req)
+{
+ // it's allowed to GET root node without authentication
+ if ("GET"_method == req.method())
+ {
+ if (req.url == "/redfish/v1" || req.url == "/redfish/v1/" ||
+ req.url == "/redfish" || req.url == "/redfish/" ||
+ req.url == "/redfish/v1/odata" || req.url == "/redfish/v1/odata/")
+ {
+ return true;
+ }
+ else if (crow::webroutes::routes.find(std::string(req.url)) !=
+ crow::webroutes::routes.end())
+ {
+ return true;
+ }
+ }
+
+ // it's allowed to POST on session collection & login without
+ // authentication
+ if ("POST"_method == req.method())
+ {
+ if ((req.url == "/redfish/v1/SessionService/Sessions") ||
+ (req.url == "/redfish/v1/SessionService/Sessions/") ||
+ (req.url == "/login"))
+ {
+ return true;
+ }
+ }
+
+ return false;
+}
+
+static void authenticate(crow::Request& req, Response& res)
+{
+ if (isOnWhitelist(req))
+ {
+ return;
+ }
+
+ const crow::persistent_data::AuthConfigMethods& authMethodsConfig =
+ crow::persistent_data::SessionStore::getInstance()
+ .getAuthMethodsConfig();
+
+ if (req.session == nullptr && authMethodsConfig.xtoken)
+ {
+ req.session = performXtokenAuth(req);
+ }
+ if (req.session == nullptr && authMethodsConfig.cookie)
+ {
+ req.session = performCookieAuth(req);
+ }
+ if (req.session == nullptr)
+ {
+ std::string_view authHeader = req.getHeaderValue("Authorization");
+ if (!authHeader.empty())
+ {
+ // Reject any kind of auth other than basic or token
+ if (boost::starts_with(authHeader, "Token ") &&
+ authMethodsConfig.sessionToken)
+ {
+ req.session = performTokenAuth(authHeader);
+ }
+ else if (boost::starts_with(authHeader, "Basic ") &&
+ authMethodsConfig.basic)
+ {
+ req.session = performBasicAuth(authHeader);
+ }
+ }
+ }
+
+ if (req.session == nullptr)
+ {
+ BMCWEB_LOG_WARNING << "[AuthMiddleware] authorization failed";
+
+ // If it's a browser connecting, don't send the HTTP authenticate
+ // header, to avoid possible CSRF attacks with basic auth
+ if (http_helpers::requestPrefersHtml(req))
+ {
+ res.result(boost::beast::http::status::temporary_redirect);
+ res.addHeader("Location",
+ "/#/login?next=" + http_helpers::urlEncode(req.url));
+ }
+ else
+ {
+ res.result(boost::beast::http::status::unauthorized);
+ // only send the WWW-authenticate header if this isn't a xhr
+ // from the browser. most scripts,
+ if (req.getHeaderValue("User-Agent").empty())
+ {
+ res.addHeader("WWW-Authenticate", "Basic");
+ }
+ }
+
+ res.end();
+ return;
+ }
+}
+
+} // namespace authorization
+} // namespace crow
diff --git a/include/login_routes.hpp b/include/login_routes.hpp
new file mode 100644
index 0000000..346ab89
--- /dev/null
+++ b/include/login_routes.hpp
@@ -0,0 +1,217 @@
+#pragma once
+
+#include <app.h>
+#include <common.h>
+#include <http_request.h>
+#include <http_response.h>
+
+#include <boost/container/flat_set.hpp>
+#include <pam_authenticate.hpp>
+#include <persistent_data_middleware.hpp>
+#include <webassets.hpp>
+
+#include <random>
+
+namespace crow
+{
+
+namespace login_routes
+{
+
+template <typename... Middlewares>
+void requestRoutes(Crow<Middlewares...>& app)
+{
+ static_assert(
+ black_magic::Contains<persistent_data::Middleware,
+ Middlewares...>::value,
+ "token_authorization middleware must be enabled in app to use "
+ "auth routes");
+
+ BMCWEB_ROUTE(app, "/login")
+ .methods(
+ "POST"_method)([](const crow::Request& req, crow::Response& res) {
+ std::string_view contentType = req.getHeaderValue("content-type");
+ std::string_view username;
+ std::string_view password;
+
+ bool looksLikePhosphorRest = false;
+
+ // This object needs to be declared at this scope so the strings
+ // within it are not destroyed before we can use them
+ nlohmann::json loginCredentials;
+ // Check if auth was provided by a payload
+ if (boost::starts_with(contentType, "application/json"))
+ {
+ loginCredentials =
+ nlohmann::json::parse(req.body, nullptr, false);
+ if (loginCredentials.is_discarded())
+ {
+ BMCWEB_LOG_DEBUG << "Bad json in request";
+ res.result(boost::beast::http::status::bad_request);
+ res.end();
+ return;
+ }
+
+ // check for username/password in the root object
+ // THis method is how intel APIs authenticate
+ nlohmann::json::iterator userIt =
+ loginCredentials.find("username");
+ nlohmann::json::iterator passIt =
+ loginCredentials.find("password");
+ if (userIt != loginCredentials.end() &&
+ passIt != loginCredentials.end())
+ {
+ const std::string* userStr =
+ userIt->get_ptr<const std::string*>();
+ const std::string* passStr =
+ passIt->get_ptr<const std::string*>();
+ if (userStr != nullptr && passStr != nullptr)
+ {
+ username = *userStr;
+ password = *passStr;
+ }
+ }
+ else
+ {
+ // Openbmc appears to push a data object that contains the
+ // same keys (username and password), attempt to use that
+ auto dataIt = loginCredentials.find("data");
+ if (dataIt != loginCredentials.end())
+ {
+ // Some apis produce an array of value ["username",
+ // "password"]
+ if (dataIt->is_array())
+ {
+ if (dataIt->size() == 2)
+ {
+ nlohmann::json::iterator userIt2 =
+ dataIt->begin();
+ nlohmann::json::iterator passIt2 =
+ dataIt->begin() + 1;
+ looksLikePhosphorRest = true;
+ if (userIt2 != dataIt->end() &&
+ passIt2 != dataIt->end())
+ {
+ const std::string* userStr =
+ userIt2->get_ptr<const std::string*>();
+ const std::string* passStr =
+ passIt2->get_ptr<const std::string*>();
+ if (userStr != nullptr &&
+ passStr != nullptr)
+ {
+ username = *userStr;
+ password = *passStr;
+ }
+ }
+ }
+ }
+ else if (dataIt->is_object())
+ {
+ nlohmann::json::iterator userIt2 =
+ dataIt->find("username");
+ nlohmann::json::iterator passIt2 =
+ dataIt->find("password");
+ if (userIt2 != dataIt->end() &&
+ passIt2 != dataIt->end())
+ {
+ const std::string* userStr =
+ userIt2->get_ptr<const std::string*>();
+ const std::string* passStr =
+ passIt2->get_ptr<const std::string*>();
+ if (userStr != nullptr && passStr != nullptr)
+ {
+ username = *userStr;
+ password = *passStr;
+ }
+ }
+ }
+ }
+ }
+ }
+ else
+ {
+ // check if auth was provided as a headers
+ username = req.getHeaderValue("username");
+ password = req.getHeaderValue("password");
+ }
+
+ if (!username.empty() && !password.empty())
+ {
+ int pamrc = pamAuthenticateUser(username, password);
+ bool isConfigureSelfOnly = pamrc == PAM_NEW_AUTHTOK_REQD;
+ if ((pamrc != PAM_SUCCESS) && !isConfigureSelfOnly)
+ {
+ res.result(boost::beast::http::status::unauthorized);
+ }
+ else
+ {
+ auto session =
+ persistent_data::SessionStore::getInstance()
+ .generateUserSession(
+ username,
+ crow::persistent_data::PersistenceType::TIMEOUT,
+ isConfigureSelfOnly);
+
+ if (looksLikePhosphorRest)
+ {
+ // Phosphor-Rest requires a very specific login
+ // structure, and doesn't actually look at the status
+ // code.
+ // TODO(ed).... Fix that upstream
+ res.jsonValue = {
+ {"data",
+ "User '" + std::string(username) + "' logged in"},
+ {"message", "200 OK"},
+ {"status", "ok"}};
+
+ // Hack alert. Boost beast by default doesn't let you
+ // declare multiple headers of the same name, and in
+ // most cases this is fine. Unfortunately here we need
+ // to set the Session cookie, which requires the
+ // httpOnly attribute, as well as the XSRF cookie, which
+ // requires it to not have an httpOnly attribute. To get
+ // the behavior we want, we simply inject the second
+ // "set-cookie" string into the value header, and get
+ // the result we want, even though we are technicaly
+ // declaring two headers here.
+ res.addHeader("Set-Cookie",
+ "XSRF-TOKEN=" + session->csrfToken +
+ "; Secure\r\nSet-Cookie: SESSION=" +
+ session->sessionToken +
+ "; Secure; HttpOnly");
+ }
+ else
+ {
+ // if content type is json, assume json token
+ res.jsonValue = {{"token", session->sessionToken}};
+ }
+ }
+ }
+ else
+ {
+ BMCWEB_LOG_DEBUG << "Couldn't interpret password";
+ res.result(boost::beast::http::status::bad_request);
+ }
+ res.end();
+ });
+
+ BMCWEB_ROUTE(app, "/logout")
+ .methods("POST"_method)(
+ [](const crow::Request& req, crow::Response& res) {
+ auto& session = req.session;
+ if (session != nullptr)
+ {
+ res.jsonValue = {
+ {"data", "User '" + session->username + "' logged out"},
+ {"message", "200 OK"},
+ {"status", "ok"}};
+
+ persistent_data::SessionStore::getInstance().removeSession(
+ session);
+ }
+ res.end();
+ return;
+ });
+}
+} // namespace login_routes
+} // namespace crow
diff --git a/include/persistent_data_middleware.hpp b/include/persistent_data_middleware.hpp
index de3a6ba..819d69d 100644
--- a/include/persistent_data_middleware.hpp
+++ b/include/persistent_data_middleware.hpp
@@ -11,9 +11,9 @@
#include <nlohmann/json.hpp>
#include <pam_authenticate.hpp>
#include <sessions.hpp>
-#include <webassets.hpp>
#include <filesystem>
+#include <fstream>
#include <random>
namespace crow
diff --git a/include/redfish_v1.hpp b/include/redfish_v1.hpp
index d77d893..3a4b9f5 100644
--- a/include/redfish_v1.hpp
+++ b/include/redfish_v1.hpp
@@ -5,7 +5,6 @@
#include <boost/algorithm/string.hpp>
#include <dbus_singleton.hpp>
#include <persistent_data_middleware.hpp>
-#include <token_authorization_middleware.hpp>
#include <fstream>
#include <streambuf>
diff --git a/include/token_authorization_middleware.hpp b/include/token_authorization_middleware.hpp
deleted file mode 100644
index a455926..0000000
--- a/include/token_authorization_middleware.hpp
+++ /dev/null
@@ -1,481 +0,0 @@
-#pragma once
-
-#include <app.h>
-#include <common.h>
-#include <http_request.h>
-#include <http_response.h>
-
-#include <boost/container/flat_set.hpp>
-#include <pam_authenticate.hpp>
-#include <persistent_data_middleware.hpp>
-#include <webassets.hpp>
-
-#include <random>
-
-namespace crow
-{
-
-namespace token_authorization
-{
-
-class Middleware
-{
- public:
- struct Context
- {};
-
- void beforeHandle(crow::Request& req, Response& res, Context& ctx)
- {
- if (isOnWhitelist(req))
- {
- return;
- }
-
- const crow::persistent_data::AuthConfigMethods& authMethodsConfig =
- crow::persistent_data::SessionStore::getInstance()
- .getAuthMethodsConfig();
-
- if (req.session == nullptr && authMethodsConfig.xtoken)
- {
- req.session = performXtokenAuth(req);
- }
- if (req.session == nullptr && authMethodsConfig.cookie)
- {
- req.session = performCookieAuth(req);
- }
- if (req.session == nullptr)
- {
- std::string_view authHeader = req.getHeaderValue("Authorization");
- if (!authHeader.empty())
- {
- // Reject any kind of auth other than basic or token
- if (boost::starts_with(authHeader, "Token ") &&
- authMethodsConfig.sessionToken)
- {
- req.session = performTokenAuth(authHeader);
- }
- else if (boost::starts_with(authHeader, "Basic ") &&
- authMethodsConfig.basic)
- {
- req.session = performBasicAuth(authHeader);
- }
- }
- }
-
- if (req.session == nullptr)
- {
- BMCWEB_LOG_WARNING << "[AuthMiddleware] authorization failed";
-
- // If it's a browser connecting, don't send the HTTP authenticate
- // header, to avoid possible CSRF attacks with basic auth
- if (http_helpers::requestPrefersHtml(req))
- {
- res.result(boost::beast::http::status::temporary_redirect);
- res.addHeader("Location", "/#/login?next=" +
- http_helpers::urlEncode(req.url));
- }
- else
- {
- res.result(boost::beast::http::status::unauthorized);
- // only send the WWW-authenticate header if this isn't a xhr
- // from the browser. most scripts,
- if (req.getHeaderValue("User-Agent").empty())
- {
- res.addHeader("WWW-Authenticate", "Basic");
- }
- }
-
- res.end();
- return;
- }
-
- // TODO get user privileges here and propagate it via MW Context
- // else let the request continue unharmed
- }
-
- template <typename AllContext>
- void afterHandle(Request& req, Response& res, Context& ctx,
- AllContext& allctx)
- {
- // TODO(ed) THis should really be handled by the persistent data
- // middleware, but because it is upstream, it doesn't have access to the
- // session information. Should the data middleware persist the current
- // user session?
- if (req.session != nullptr &&
- req.session->persistence ==
- crow::persistent_data::PersistenceType::SINGLE_REQUEST)
- {
- persistent_data::SessionStore::getInstance().removeSession(
- req.session);
- }
- }
-
- private:
- const std::shared_ptr<crow::persistent_data::UserSession>
- performBasicAuth(std::string_view auth_header) const
- {
- BMCWEB_LOG_DEBUG << "[AuthMiddleware] Basic authentication";
-
- std::string authData;
- std::string_view param = auth_header.substr(strlen("Basic "));
- if (!crow::utility::base64Decode(param, authData))
- {
- return nullptr;
- }
- std::size_t separator = authData.find(':');
- if (separator == std::string::npos)
- {
- return nullptr;
- }
-
- std::string user = authData.substr(0, separator);
- separator += 1;
- if (separator > authData.size())
- {
- return nullptr;
- }
- std::string pass = authData.substr(separator);
-
- BMCWEB_LOG_DEBUG << "[AuthMiddleware] Authenticating user: " << user;
-
- int pamrc = pamAuthenticateUser(user, pass);
- bool isConfigureSelfOnly = pamrc == PAM_NEW_AUTHTOK_REQD;
- if ((pamrc != PAM_SUCCESS) && !isConfigureSelfOnly)
- {
- return nullptr;
- }
-
- // TODO(ed) generateUserSession is a little expensive for basic
- // auth, as it generates some random identifiers that will never be
- // used. This should have a "fast" path for when user tokens aren't
- // needed.
- // This whole flow needs to be revisited anyway, as we can't be
- // calling directly into pam for every request
- return persistent_data::SessionStore::getInstance().generateUserSession(
- user, crow::persistent_data::PersistenceType::SINGLE_REQUEST,
- isConfigureSelfOnly);
- }
-
- const std::shared_ptr<crow::persistent_data::UserSession>
- performTokenAuth(std::string_view auth_header) const
- {
- BMCWEB_LOG_DEBUG << "[AuthMiddleware] Token authentication";
-
- std::string_view token = auth_header.substr(strlen("Token "));
- auto session =
- persistent_data::SessionStore::getInstance().loginSessionByToken(
- token);
- return session;
- }
-
- const std::shared_ptr<crow::persistent_data::UserSession>
- performXtokenAuth(const crow::Request& req) const
- {
- BMCWEB_LOG_DEBUG << "[AuthMiddleware] X-Auth-Token authentication";
-
- std::string_view token = req.getHeaderValue("X-Auth-Token");
- if (token.empty())
- {
- return nullptr;
- }
- auto session =
- persistent_data::SessionStore::getInstance().loginSessionByToken(
- token);
- return session;
- }
-
- const std::shared_ptr<crow::persistent_data::UserSession>
- performCookieAuth(const crow::Request& req) const
- {
- BMCWEB_LOG_DEBUG << "[AuthMiddleware] Cookie authentication";
-
- std::string_view cookieValue = req.getHeaderValue("Cookie");
- if (cookieValue.empty())
- {
- return nullptr;
- }
-
- auto startIndex = cookieValue.find("SESSION=");
- if (startIndex == std::string::npos)
- {
- return nullptr;
- }
- startIndex += sizeof("SESSION=") - 1;
- auto endIndex = cookieValue.find(";", startIndex);
- if (endIndex == std::string::npos)
- {
- endIndex = cookieValue.size();
- }
- std::string_view authKey =
- cookieValue.substr(startIndex, endIndex - startIndex);
-
- const std::shared_ptr<crow::persistent_data::UserSession> session =
- persistent_data::SessionStore::getInstance().loginSessionByToken(
- authKey);
- if (session == nullptr)
- {
- return nullptr;
- }
-#ifndef BMCWEB_INSECURE_DISABLE_CSRF_PREVENTION
- // RFC7231 defines methods that need csrf protection
- if (req.method() != "GET"_method)
- {
- std::string_view csrf = req.getHeaderValue("X-XSRF-TOKEN");
- // Make sure both tokens are filled
- if (csrf.empty() || session->csrfToken.empty())
- {
- return nullptr;
- }
-
- if (csrf.size() != crow::persistent_data::sessionTokenSize)
- {
- return nullptr;
- }
- // Reject if csrf token not available
- if (!crow::utility::constantTimeStringCompare(csrf,
- session->csrfToken))
- {
- return nullptr;
- }
- }
-#endif
- session->cookieAuth = true;
- return session;
- }
-
- // checks if request can be forwarded without authentication
- bool isOnWhitelist(const crow::Request& req) const
- {
- // it's allowed to GET root node without authentica tion
- if ("GET"_method == req.method())
- {
- if (req.url == "/redfish/v1" || req.url == "/redfish/v1/" ||
- req.url == "/redfish" || req.url == "/redfish/" ||
- req.url == "/redfish/v1/odata" ||
- req.url == "/redfish/v1/odata/")
- {
- return true;
- }
- else if (crow::webassets::routes.find(std::string(req.url)) !=
- crow::webassets::routes.end())
- {
- return true;
- }
- }
-
- // it's allowed to POST on session collection & login without
- // authentication
- if ("POST"_method == req.method())
- {
- if ((req.url == "/redfish/v1/SessionService/Sessions") ||
- (req.url == "/redfish/v1/SessionService/Sessions/") ||
- (req.url == "/login"))
- {
- return true;
- }
- }
-
- return false;
- }
-};
-
-// TODO(ed) see if there is a better way to allow middlewares to request
-// routes.
-// Possibly an init function on first construction?
-template <typename... Middlewares>
-void requestRoutes(Crow<Middlewares...>& app)
-{
- static_assert(
- black_magic::Contains<persistent_data::Middleware,
- Middlewares...>::value,
- "token_authorization middleware must be enabled in app to use "
- "auth routes");
-
- BMCWEB_ROUTE(app, "/login")
- .methods(
- "POST"_method)([](const crow::Request& req, crow::Response& res) {
- std::string_view contentType = req.getHeaderValue("content-type");
- std::string_view username;
- std::string_view password;
-
- bool looksLikePhosphorRest = false;
-
- // This object needs to be declared at this scope so the strings
- // within it are not destroyed before we can use them
- nlohmann::json loginCredentials;
- // Check if auth was provided by a payload
- if (boost::starts_with(contentType, "application/json"))
- {
- loginCredentials =
- nlohmann::json::parse(req.body, nullptr, false);
- if (loginCredentials.is_discarded())
- {
- BMCWEB_LOG_DEBUG << "Bad json in request";
- res.result(boost::beast::http::status::bad_request);
- res.end();
- return;
- }
-
- // check for username/password in the root object
- // THis method is how intel APIs authenticate
- nlohmann::json::iterator userIt =
- loginCredentials.find("username");
- nlohmann::json::iterator passIt =
- loginCredentials.find("password");
- if (userIt != loginCredentials.end() &&
- passIt != loginCredentials.end())
- {
- const std::string* userStr =
- userIt->get_ptr<const std::string*>();
- const std::string* passStr =
- passIt->get_ptr<const std::string*>();
- if (userStr != nullptr && passStr != nullptr)
- {
- username = *userStr;
- password = *passStr;
- }
- }
- else
- {
- // Openbmc appears to push a data object that contains the
- // same keys (username and password), attempt to use that
- auto dataIt = loginCredentials.find("data");
- if (dataIt != loginCredentials.end())
- {
- // Some apis produce an array of value ["username",
- // "password"]
- if (dataIt->is_array())
- {
- if (dataIt->size() == 2)
- {
- nlohmann::json::iterator userIt2 =
- dataIt->begin();
- nlohmann::json::iterator passIt2 =
- dataIt->begin() + 1;
- looksLikePhosphorRest = true;
- if (userIt2 != dataIt->end() &&
- passIt2 != dataIt->end())
- {
- const std::string* userStr =
- userIt2->get_ptr<const std::string*>();
- const std::string* passStr =
- passIt2->get_ptr<const std::string*>();
- if (userStr != nullptr &&
- passStr != nullptr)
- {
- username = *userStr;
- password = *passStr;
- }
- }
- }
- }
- else if (dataIt->is_object())
- {
- nlohmann::json::iterator userIt2 =
- dataIt->find("username");
- nlohmann::json::iterator passIt2 =
- dataIt->find("password");
- if (userIt2 != dataIt->end() &&
- passIt2 != dataIt->end())
- {
- const std::string* userStr =
- userIt2->get_ptr<const std::string*>();
- const std::string* passStr =
- passIt2->get_ptr<const std::string*>();
- if (userStr != nullptr && passStr != nullptr)
- {
- username = *userStr;
- password = *passStr;
- }
- }
- }
- }
- }
- }
- else
- {
- // check if auth was provided as a headers
- username = req.getHeaderValue("username");
- password = req.getHeaderValue("password");
- }
-
- if (!username.empty() && !password.empty())
- {
- int pamrc = pamAuthenticateUser(username, password);
- bool isConfigureSelfOnly = pamrc == PAM_NEW_AUTHTOK_REQD;
- if ((pamrc != PAM_SUCCESS) && !isConfigureSelfOnly)
- {
- res.result(boost::beast::http::status::unauthorized);
- }
- else
- {
- auto session =
- persistent_data::SessionStore::getInstance()
- .generateUserSession(
- username,
- crow::persistent_data::PersistenceType::TIMEOUT,
- isConfigureSelfOnly);
-
- if (looksLikePhosphorRest)
- {
- // Phosphor-Rest requires a very specific login
- // structure, and doesn't actually look at the status
- // code.
- // TODO(ed).... Fix that upstream
- res.jsonValue = {
- {"data",
- "User '" + std::string(username) + "' logged in"},
- {"message", "200 OK"},
- {"status", "ok"}};
-
- // Hack alert. Boost beast by default doesn't let you
- // declare multiple headers of the same name, and in
- // most cases this is fine. Unfortunately here we need
- // to set the Session cookie, which requires the
- // httpOnly attribute, as well as the XSRF cookie, which
- // requires it to not have an httpOnly attribute. To get
- // the behavior we want, we simply inject the second
- // "set-cookie" string into the value header, and get
- // the result we want, even though we are technicaly
- // declaring two headers here.
- res.addHeader("Set-Cookie",
- "XSRF-TOKEN=" + session->csrfToken +
- "; Secure\r\nSet-Cookie: SESSION=" +
- session->sessionToken +
- "; Secure; HttpOnly");
- }
- else
- {
- // if content type is json, assume json token
- res.jsonValue = {{"token", session->sessionToken}};
- }
- }
- }
- else
- {
- BMCWEB_LOG_DEBUG << "Couldn't interpret password";
- res.result(boost::beast::http::status::bad_request);
- }
- res.end();
- });
-
- BMCWEB_ROUTE(app, "/logout")
- .methods("POST"_method)(
- [](const crow::Request& req, crow::Response& res) {
- auto& session = req.session;
- if (session != nullptr)
- {
- res.jsonValue = {
- {"data", "User '" + session->username + "' logged out"},
- {"message", "200 OK"},
- {"status", "ok"}};
-
- persistent_data::SessionStore::getInstance().removeSession(
- session);
- }
- res.end();
- return;
- });
-}
-} // namespace token_authorization
-} // namespace crow
diff --git a/include/webassets.hpp b/include/webassets.hpp
index fc58d37..92cf4aa 100644
--- a/include/webassets.hpp
+++ b/include/webassets.hpp
@@ -1,5 +1,7 @@
#pragma once
+#include "webroutes.hpp"
+
#include <app.h>
#include <http_request.h>
#include <http_response.h>
@@ -27,8 +29,6 @@
}
};
-static boost::container::flat_set<std::string> routes;
-
template <typename... Middlewares>
void requestRoutes(Crow<Middlewares...>& app)
{
@@ -100,13 +100,13 @@
webpath.string().back() != '/')
{
// insert the non-directory version of this path
- routes.insert(webpath);
+ webroutes::routes.insert(webpath);
webpath += "/";
}
}
std::pair<boost::container::flat_set<std::string>::iterator, bool>
- inserted = routes.insert(webpath);
+ inserted = webroutes::routes.insert(webpath);
if (!inserted.second)
{
diff --git a/include/webroutes.hpp b/include/webroutes.hpp
new file mode 100644
index 0000000..757a272
--- /dev/null
+++ b/include/webroutes.hpp
@@ -0,0 +1,13 @@
+#pragma once
+
+#include <boost/container/flat_set.hpp>
+namespace crow
+{
+namespace webroutes
+{
+
+static boost::container::flat_set<std::string> routes;
+
+} // namespace webroutes
+
+} // namespace crow
diff --git a/include/webserver_common.hpp b/include/webserver_common.hpp
index 079b17a..d8876d4 100644
--- a/include/webserver_common.hpp
+++ b/include/webserver_common.hpp
@@ -15,9 +15,8 @@
*/
#pragma once
+#include "persistent_data_middleware.hpp"
#include "security_headers_middleware.hpp"
-#include "token_authorization_middleware.hpp"
using CrowApp = crow::App<crow::SecurityHeadersMiddleware,
- crow::persistent_data::Middleware,
- crow::token_authorization::Middleware>;
+ crow::persistent_data::Middleware>;
diff --git a/redfish-core/include/node.hpp b/redfish-core/include/node.hpp
index c2c10d5..2f4cb6b 100644
--- a/redfish-core/include/node.hpp
+++ b/redfish-core/include/node.hpp
@@ -19,7 +19,6 @@
#include "http_response.h"
#include "privileges.hpp"
-#include "token_authorization_middleware.hpp"
#include "webserver_common.hpp"
#include <error_messages.hpp>
diff --git a/src/webserver_main.cpp b/src/webserver_main.cpp
index 11e8e92..036db54 100644
--- a/src/webserver_main.cpp
+++ b/src/webserver_main.cpp
@@ -6,6 +6,7 @@
#include <dbus_singleton.hpp>
#include <image_upload.hpp>
#include <kvm_websocket.hpp>
+#include <login_routes.hpp>
#include <obmc_console.hpp>
#include <openbmc_dbus_rest.hpp>
@@ -21,7 +22,6 @@
#include <sdbusplus/server.hpp>
#include <security_headers_middleware.hpp>
#include <ssl_key_handler.hpp>
-#include <token_authorization_middleware.hpp>
#include <vm_websocket.hpp>
#include <webassets.hpp>
#include <webserver_common.hpp>
@@ -104,7 +104,7 @@
crow::ibm_mc_lock::Lock::getInstance();
#endif
- crow::token_authorization::requestRoutes(app);
+ crow::login_routes::requestRoutes(app);
BMCWEB_LOG_INFO << "bmcweb (" << __DATE__ << ": " << __TIME__ << ')';
setupSocket(app);