Be more proactive at removing stale sessions
The maximum number of sessions is set to limit total resources that
netipmid is allowed to use. But it also opens a door to DoS attacks that
would use up all the sessions and then never close them. This new
mechanism will allow extra sessions, especially if they are short and
active. As the number of sessions grows beyond the desired maximum, the
reaping time becomes shorter and shorter to ensure that only actual
active sessions are kept.
This introduces a variable max idle time that starts at 60s, according
to the IPMI spec, for up to the desired maximum number of sessions per
channel (currently 15). Beyond 15 sessions, The idle time is reduced
proportionally to the inverse^3 of the number of sessions beyond the
desired maximum.
Some sample maximum idle times for active sessions this new scheme:
Idle time for up to 15 sessions stays at 60s
Idle time for 16 sessions is reduced to 7.5s
Idle time for 20 sessions is reduced to 277ms
Idle time for 24 sessions is reduced to 60ms
For sessions in setup, the idle times are calculated the same as for
active sessions, but use the full session count (active and setup) and
are limited to a maximum idle time of 3 seconds.
One other feature added is to schedule session cleaning when a Close
Session command is received. Without this, sessions that are in the
shutDownPending state would live on for much longer than needed. Really,
the session only needs to live long enough to prepare the response
message, but curretly there is no mechanism to remove just that one
session from that context.
Tested: Open lots of sessions and wait for them to get reaped
$ for ((i=0; i<16; i++)); do \
ipmitool -C 17 -I lanplus -H $HOST -U $USR -P $PW sensor list & \
done
$ for ((i=0; i<10; i++)); do \
ipmitool -C 17 -I lanplus -H $HOST -U $USR -P $PW mc info & \
done
In this case, the first 16 sessions will open just fine, but with
a slightly shorted idle time (no problems). The next ten sessions
may or may not all get to open, because the number of setup
sessions open simultaneously will severely limit the idle time of
the setup sessions, causing some of them to fail to fully open.
Change-Id: Iae2e68c7192f3f5a2cafa8e825aa025454405c84
Signed-off-by: Vernon Mauery <vernon.mauery@linux.intel.com>
diff --git a/command/session_cmds.cpp b/command/session_cmds.cpp
index 945d8d9..98016c9 100644
--- a/command/session_cmds.cpp
+++ b/command/session_cmds.cpp
@@ -5,10 +5,13 @@
#include <ipmid/api.h>
+#include <chrono>
#include <ipmid/sessionhelper.hpp>
#include <ipmid/utils.hpp>
#include <phosphor-logging/log.hpp>
+using namespace std::chrono_literals;
+
namespace command
{
using namespace phosphor::logging;
@@ -282,6 +285,8 @@
{
response->completionCode = closeMyNetInstanceSession(
reqSessionId, reqSessionHandle, currentSessionPriv);
+ std::get<session::Manager&>(singletonPool)
+ .scheduleSessionCleaner(100us);
}
else
{
diff --git a/main.cpp b/main.cpp
index 1d84c3c..4092211 100644
--- a/main.cpp
+++ b/main.cpp
@@ -26,7 +26,7 @@
// Tuple of Global Singletons
static auto io = std::make_shared<boost::asio::io_context>();
-session::Manager manager;
+session::Manager manager(io);
command::Table table;
eventloop::EventLoop loop(io);
sol::Manager solManager(io);
diff --git a/session.hpp b/session.hpp
index 4d4bf19..a01d478 100644
--- a/session.hpp
+++ b/session.hpp
@@ -38,11 +38,6 @@
OEM,
};
-// Seconds of inactivity allowed during session setup stage
-constexpr auto SESSION_SETUP_TIMEOUT = 5s;
-// Seconds of inactivity allowed when session is active
-constexpr auto SESSION_INACTIVITY_TIMEOUT = 60s;
-
// Mask to get only the privilege from requested maximum privlege (RAKP message
// 1)
constexpr uint8_t reqMaxPrivMask = 0xF;
@@ -243,31 +238,38 @@
* Session Active status is decided upon the Session State and the last
* transaction time is compared against the session inactivity timeout.
*
+ * @param[in] activeGrace - microseconds of idle time for active sessions
+ * @param[in] setupGrace - microseconds of idle time for sessions in setup
+ *
*/
- bool isSessionActive(uint8_t sessionState)
+ bool isSessionActive(const std::chrono::microseconds& activeGrace,
+ const std::chrono::microseconds& setupGrace)
{
auto currentTime = std::chrono::steady_clock::now();
- auto elapsedSeconds = std::chrono::duration_cast<std::chrono::seconds>(
- currentTime - lastTime);
+ auto elapsedMicros =
+ std::chrono::duration_cast<std::chrono::microseconds>(currentTime -
+ lastTime);
- State state = static_cast<session::State>(sessionState);
+ State state = static_cast<session::State>(this->state());
switch (state)
{
- case State::setupInProgress:
- if (elapsedSeconds < SESSION_SETUP_TIMEOUT)
+ case State::active:
+ if (elapsedMicros < activeGrace)
{
return true;
}
break;
- case State::active:
- if (elapsedSeconds < SESSION_INACTIVITY_TIMEOUT)
+ case State::setupInProgress:
+ if (elapsedMicros < setupGrace)
{
return true;
}
break;
+ case State::tearDownInProgress:
+ break;
default:
- return false;
+ break;
}
return false;
}
diff --git a/sessions_manager.cpp b/sessions_manager.cpp
index a8e2361..ac20621 100644
--- a/sessions_manager.cpp
+++ b/sessions_manager.cpp
@@ -53,10 +53,6 @@
return ipmiNetworkInstance;
}
-Manager::Manager()
-{
-}
-
void Manager::managerInit(const std::string& channel)
{
@@ -77,6 +73,9 @@
setNetworkInstance();
sessionsMap.emplace(
0, std::make_shared<Session>(*getSdBus(), objPath.c_str(), 0, 0, 0));
+
+ // set up the timer for clearing out stale sessions
+ scheduleSessionCleaner(std::chrono::microseconds(3 * 1000 * 1000));
}
std::shared_ptr<Session>
@@ -92,7 +91,7 @@
auto activeSessions = sessionsMap.size() - session::maxSessionlessCount;
- if (activeSessions < session::maxSessionCountPerChannel)
+ if (activeSessions < maxSessionHandles)
{
do
{
@@ -230,28 +229,60 @@
void Manager::cleanStaleEntries()
{
+ // with overflow = min(1, max - active sessions)
+ // active idle time in seconds = 60 / overflow^3
+ constexpr int baseIdleMicros = 60 * 1000 * 1000;
+ // no +1 for the zero session here because this is just active sessions
+ int sessionDivisor =
+ getActiveSessionCount() - session::maxSessionCountPerChannel;
+ sessionDivisor = std::max(0, sessionDivisor) + 1;
+ sessionDivisor = sessionDivisor * sessionDivisor * sessionDivisor;
+ int activeMicros = baseIdleMicros / sessionDivisor;
+
+ // with overflow = min(1, max - total sessions)
+ // setup idle time in seconds = max(3, 60 / overflow^3)
+
+ // +1 for the zero session here because size() counts that too
+ int setupDivisor =
+ sessionsMap.size() - (session::maxSessionCountPerChannel + 1);
+ setupDivisor = std::max(0, setupDivisor) + 1;
+ setupDivisor = setupDivisor * setupDivisor * setupDivisor;
+ constexpr int maxSetupMicros = 3 * 1000 * 1000;
+ int setupMicros = std::min(maxSetupMicros, baseIdleMicros / setupDivisor);
+
+ std::chrono::microseconds activeGrace(activeMicros);
+ std::chrono::microseconds setupGrace(setupMicros);
+
for (auto iter = sessionsMap.begin(); iter != sessionsMap.end();)
{
-
auto session = iter->second;
- if ((session->getBMCSessionID() != session::sessionZero) &&
- !(session->isSessionActive(session->state())))
+ // special handling for sessionZero
+ if (session->getBMCSessionID() == session::sessionZero)
+ {
+ iter++;
+ continue;
+ }
+ if (!(session->isSessionActive(activeGrace, setupGrace)))
{
sessionHandleMap[getSessionHandle(session->getBMCSessionID())] = 0;
iter = sessionsMap.erase(iter);
}
else
{
- ++iter;
+ iter++;
}
}
+ if (sessionsMap.size() > 1)
+ {
+ scheduleSessionCleaner(setupGrace);
+ }
}
uint8_t Manager::storeSessionHandle(SessionID bmcSessionID)
{
// Handler index 0 is reserved for invalid session.
// index starts with 1, for direct usage. Index 0 reserved
- for (uint8_t i = 1; i <= session::maxSessionCountPerChannel; i++)
+ for (size_t i = 1; i < session::maxSessionHandles; i++)
{
if (sessionHandleMap[i] == 0)
{
@@ -264,7 +295,7 @@
uint32_t Manager::getSessionIDbyHandle(uint8_t sessionHandle) const
{
- if (sessionHandle <= session::maxSessionCountPerChannel)
+ if (sessionHandle < session::maxSessionHandles)
{
return sessionHandleMap[sessionHandle];
}
@@ -276,8 +307,7 @@
// Handler index 0 is reserved for invalid session.
// index starts with 1, for direct usage. Index 0 reserved
-
- for (uint8_t i = 1; i <= session::maxSessionCountPerChannel; i++)
+ for (size_t i = 1; i < session::maxSessionHandles; i++)
{
if (sessionHandleMap[i] == bmcSessionID)
{
@@ -297,4 +327,16 @@
static_cast<uint8_t>(session::State::active);
}));
}
+
+void Manager::scheduleSessionCleaner(const std::chrono::microseconds& when)
+{
+ timer.expires_from_now(when);
+ timer.async_wait([this](const boost::system::error_code& ec) {
+ if (!ec)
+ {
+ cleanStaleEntries();
+ }
+ });
+}
+
} // namespace session
diff --git a/sessions_manager.hpp b/sessions_manager.hpp
index ed3db49..f1d060e 100644
--- a/sessions_manager.hpp
+++ b/sessions_manager.hpp
@@ -2,6 +2,8 @@
#include "session.hpp"
+#include <boost/asio/steady_timer.hpp>
+#include <chrono>
#include <ipmid/api.hpp>
#include <ipmid/sessiondef.hpp>
#include <map>
@@ -18,6 +20,8 @@
RC_SESSION_ID,
};
+static constexpr size_t maxSessionHandles = multiIntfaceSessionHandleMask;
+
/**
* @class Manager
*
@@ -32,7 +36,9 @@
// BMC Session ID is the key for the map
using SessionMap = std::map<SessionID, std::shared_ptr<Session>>;
- Manager();
+ Manager() = delete;
+ explicit Manager(std::shared_ptr<boost::asio::io_context>& io) :
+ io(io), timer(*io){};
~Manager() = default;
Manager(const Manager&) = delete;
Manager& operator=(const Manager&) = delete;
@@ -90,10 +96,36 @@
uint8_t getNetworkInstance(void);
+ /**
+ * @brief Clean Session Stale Entries
+ *
+ * Schedules cleaning the inactive sessions entries from the Session Map
+ */
+ void scheduleSessionCleaner(const std::chrono::microseconds& grace);
+
private:
- //+1 for session, as 0 is reserved for sessionless command
- std::array<uint32_t, session::maxSessionCountPerChannel + 1>
- sessionHandleMap = {0};
+ /**
+ * @brief reclaim system resources by limiting idle sessions
+ *
+ * Limits on active, authenticated sessions are calculated independently
+ * from in-setup sessions, which are not required to be authenticated. This
+ * will prevent would-be DoS attacks by calling a bunch of Open Session
+ * requests to fill up all available sessions. Too many active sessions will
+ * trigger a shorter timeout, but is unaffected by setup session counts.
+ *
+ * For active sessions, grace time is inversely proportional to (the number
+ * of active sessions beyond max sessions per channel)^3
+ *
+ * For sessions in setup, grace time is inversely proportional to (the
+ * number of total sessions beyond max sessions per channel)^3, with a max
+ * of 3 seconds
+ */
+ void cleanStaleEntries();
+
+ std::shared_ptr<boost::asio::io_context> io;
+ boost::asio::steady_timer timer;
+
+ std::array<uint32_t, session::maxSessionHandles> sessionHandleMap = {0};
/**
* @brief Session Manager keeps the session objects as a sorted
@@ -103,12 +135,6 @@
std::unique_ptr<sdbusplus::server::manager::manager> objManager = nullptr;
std::string chName{}; // Channel Name
uint8_t ipmiNetworkInstance;
- /**
- * @brief Clean Session Stale Entries
- *
- * Removes the inactive sessions entries from the Session Map
- */
- void cleanStaleEntries();
void setNetworkInstance(void);
};