Store Request Fields that are needed later
Because of recent changes to how dbus authentication is done, Requests
might be moved out before they can be used. This commit is an attempt
to mitigate the problem without needing to revert that patch.
This commit does two relatively distinct things.
First, it moves basic auth types to a model where they're timed out
instead of removed on destruction. This removes the need for a Request
object to track that state, and arguably gives better behavior, as
basic auth sessions will survive through the timeout.
To prevent lots of basic auth sessions getting created, a basic auth
session is reused if it was:
1. Created by basic auth previously.
2. Created by the same user.
3. Created from the same source IP address.
Second, both connection classes now store the accept, and origin headers
from the request in the connection class itself, removing the need for
them.
Tested: HTML page now loads when pointing at a redfish URL with a
browser.
Change-Id: I623b43cbcbb43d9e65b408853660be09a5edb2b3
Signed-off-by: Ed Tanous <ed@tanous.net>
diff --git a/include/authentication.hpp b/include/authentication.hpp
index 2c3a08a..5c7ec19 100644
--- a/include/authentication.hpp
+++ b/include/authentication.hpp
@@ -19,20 +19,6 @@
namespace authentication
{
-inline void cleanupTempSession(const 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 ==
- persistent_data::PersistenceType::SINGLE_REQUEST)
- {
- persistent_data::SessionStore::getInstance().removeSession(req.session);
- }
-}
-
inline std::shared_ptr<persistent_data::UserSession>
performBasicAuth(const boost::asio::ip::address& clientIp,
std::string_view authHeader)
@@ -76,15 +62,29 @@
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
+ // Attempt to locate an existing Basic Auth session from the same ip address
+ // and user
+ for (auto& session :
+ persistent_data::SessionStore::getInstance().getSessions())
+ {
+ if (session->sessionType != persistent_data::SessionType::Basic)
+ {
+ continue;
+ }
+ if (session->clientIp != redfish::ip_util::toString(clientIp))
+ {
+ continue;
+ }
+ if (session->username != user)
+ {
+ continue;
+ }
+ return session;
+ }
+
return persistent_data::SessionStore::getInstance().generateUserSession(
- user, clientIp, std::nullopt,
- persistent_data::PersistenceType::SINGLE_REQUEST, isConfigureSelfOnly);
+ user, clientIp, std::nullopt, persistent_data::SessionType::Basic,
+ isConfigureSelfOnly);
}
inline std::shared_ptr<persistent_data::UserSession>
diff --git a/include/login_routes.hpp b/include/login_routes.hpp
index 0adb9d4..988bf45 100644
--- a/include/login_routes.hpp
+++ b/include/login_routes.hpp
@@ -161,11 +161,11 @@
}
else
{
- auto session = persistent_data::SessionStore::getInstance()
- .generateUserSession(
- username, req.ipAddress, std::nullopt,
- persistent_data::PersistenceType::TIMEOUT,
- isConfigureSelfOnly);
+ auto session =
+ persistent_data::SessionStore::getInstance()
+ .generateUserSession(username, req.ipAddress, std::nullopt,
+ persistent_data::SessionType::Session,
+ isConfigureSelfOnly);
bmcweb::setSessionCookies(asyncResp->res, *session);
diff --git a/include/persistent_data.hpp b/include/persistent_data.hpp
index fff08d3..3b98e1a 100644
--- a/include/persistent_data.hpp
+++ b/include/persistent_data.hpp
@@ -244,8 +244,9 @@
sessions = nlohmann::json::array();
for (const auto& p : SessionStore::getInstance().authTokens)
{
- if (p.second->persistence !=
- persistent_data::PersistenceType::SINGLE_REQUEST)
+ if (p.second->sessionType != persistent_data::SessionType::Basic &&
+ p.second->sessionType !=
+ persistent_data::SessionType::MutualTLS)
{
nlohmann::json::object_t session;
session["unique_id"] = p.second->uniqueId;
diff --git a/include/security_headers.hpp b/include/security_headers.hpp
index 2a2eb40..3206acc16 100644
--- a/include/security_headers.hpp
+++ b/include/security_headers.hpp
@@ -5,8 +5,7 @@
#include "http_request.hpp"
#include "http_response.hpp"
-inline void addSecurityHeaders(const crow::Request& req [[maybe_unused]],
- crow::Response& res)
+inline void addSecurityHeaders(crow::Response& res)
{
using bf = boost::beast::http::field;
diff --git a/include/sessions.hpp b/include/sessions.hpp
index 9c064ee..e6e4a68 100644
--- a/include/sessions.hpp
+++ b/include/sessions.hpp
@@ -9,9 +9,11 @@
#include <algorithm>
#include <csignal>
+#include <memory>
#include <optional>
#include <random>
#include <string>
+#include <vector>
namespace persistent_data
{
@@ -21,10 +23,13 @@
// https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html#session-id-entropy
constexpr std::size_t sessionTokenSize = 20;
-enum class PersistenceType
+enum class SessionType
{
- TIMEOUT, // User session times out after a predetermined amount of time
- SINGLE_REQUEST // User times out once this request is completed.
+ None,
+ Basic,
+ Session,
+ Cookie,
+ MutualTLS
};
struct UserSession
@@ -36,7 +41,7 @@
std::optional<std::string> clientId;
std::string clientIp;
std::chrono::time_point<std::chrono::steady_clock> lastUpdated;
- PersistenceType persistence{PersistenceType::TIMEOUT};
+ SessionType sessionType{SessionType::None};
bool cookieAuth = false;
bool isConfigureSelfOnly = false;
std::string userRole;
@@ -128,7 +133,7 @@
// the tradeoffs of all the corner cases involved are non-trivial, so
// this is done temporarily
userSession->lastUpdated = std::chrono::steady_clock::now();
- userSession->persistence = PersistenceType::TIMEOUT;
+ userSession->sessionType = SessionType::Session;
return userSession;
}
@@ -245,8 +250,7 @@
public:
std::shared_ptr<UserSession> generateUserSession(
std::string_view username, const boost::asio::ip::address& clientIp,
- const std::optional<std::string>& clientId,
- PersistenceType persistence = PersistenceType::TIMEOUT,
+ const std::optional<std::string>& clientId, SessionType sessionType,
bool isConfigureSelfOnly = false)
{
// Only need csrf tokens for cookie based auth, token doesn't matter
@@ -270,14 +274,15 @@
clientId,
redfish::ip_util::toString(clientIp),
std::chrono::steady_clock::now(),
- persistence,
+ sessionType,
false,
isConfigureSelfOnly,
"",
{}});
auto it = authTokens.emplace(sessionToken, session);
// Only need to write to disk if session isn't about to be destroyed.
- needWrite = persistence == PersistenceType::TIMEOUT;
+ needWrite = sessionType != SessionType::Basic &&
+ sessionType != SessionType::MutualTLS;
return it.first->second;
}
@@ -320,24 +325,45 @@
needWrite = true;
}
- std::vector<const std::string*> getUniqueIds(
- bool getAll = true,
- const PersistenceType& type = PersistenceType::SINGLE_REQUEST)
+ std::vector<std::string> getAllUniqueIds()
{
applySessionTimeouts();
-
- std::vector<const std::string*> ret;
+ std::vector<std::string> ret;
ret.reserve(authTokens.size());
for (auto& session : authTokens)
{
- if (getAll || type == session.second->persistence)
+ ret.push_back(session.second->uniqueId);
+ }
+ return ret;
+ }
+
+ std::vector<std::string> getUniqueIdsBySessionType(SessionType type)
+ {
+ applySessionTimeouts();
+
+ std::vector<std::string> ret;
+ ret.reserve(authTokens.size());
+ for (auto& session : authTokens)
+ {
+ if (type == session.second->sessionType)
{
- ret.push_back(&session.second->uniqueId);
+ ret.push_back(session.second->uniqueId);
}
}
return ret;
}
+ std::vector<std::shared_ptr<UserSession>> getSessions()
+ {
+ std::vector<std::shared_ptr<UserSession>> sessions;
+ sessions.reserve(authTokens.size());
+ for (auto& session : authTokens)
+ {
+ sessions.push_back(session.second);
+ }
+ return sessions;
+ }
+
void removeSessionsByUsername(std::string_view username)
{
std::erase_if(authTokens, [username](const auto& value) {