Skip to content

Commit

Permalink
Be more proactive at removing stale sessions
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@vmauery
vmauery committed Jun 18, 2021
1 parent cfb34ca commit ecc8efa
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 38 deletions.
5 changes: 5 additions & 0 deletions command/session_cmds.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -282,6 +285,8 @@ std::vector<uint8_t> closeSession(const std::vector<uint8_t>& inPayload,
{
response->completionCode = closeMyNetInstanceSession(
reqSessionId, reqSessionHandle, currentSessionPriv);
std::get<session::Manager&>(singletonPool)
.scheduleSessionCleaner(100us);
}
else
{
Expand Down
2 changes: 1 addition & 1 deletion main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ using namespace phosphor::logging;

// 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);
Expand Down
30 changes: 16 additions & 14 deletions session.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,6 @@ enum class Privilege : uint8_t
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;
Expand Down Expand Up @@ -243,31 +238,38 @@ class Session : public SessionIface
* 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;
}
Expand Down
68 changes: 55 additions & 13 deletions sessions_manager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,6 @@ uint8_t Manager::getNetworkInstance(void)
return ipmiNetworkInstance;
}

Manager::Manager()
{
}

void Manager::managerInit(const std::string& channel)
{

Expand All @@ -77,6 +73,9 @@ void Manager::managerInit(const std::string& channel)
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>
Expand All @@ -92,7 +91,7 @@ std::shared_ptr<Session>

auto activeSessions = sessionsMap.size() - session::maxSessionlessCount;

if (activeSessions < session::maxSessionCountPerChannel)
if (activeSessions < maxSessionHandles)
{
do
{
Expand Down Expand Up @@ -230,28 +229,60 @@ std::shared_ptr<Session> Manager::getSession(SessionID sessionID,

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)
{
Expand All @@ -264,7 +295,7 @@ uint8_t Manager::storeSessionHandle(SessionID bmcSessionID)

uint32_t Manager::getSessionIDbyHandle(uint8_t sessionHandle) const
{
if (sessionHandle <= session::maxSessionCountPerChannel)
if (sessionHandle < session::maxSessionHandles)
{
return sessionHandleMap[sessionHandle];
}
Expand All @@ -276,8 +307,7 @@ uint8_t Manager::getSessionHandle(SessionID bmcSessionID) const

// 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)
{
Expand All @@ -297,4 +327,16 @@ uint8_t Manager::getActiveSessionCount() const
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
46 changes: 36 additions & 10 deletions sessions_manager.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -18,6 +20,8 @@ enum class RetrieveOption
RC_SESSION_ID,
};

static constexpr size_t maxSessionHandles = multiIntfaceSessionHandleMask;

/**
* @class Manager
*
Expand All @@ -32,7 +36,9 @@ class Manager
// 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;
Expand Down Expand Up @@ -90,10 +96,36 @@ class Manager

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
Expand All @@ -103,12 +135,6 @@ class Manager
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);
};

Expand Down

0 comments on commit ecc8efa

Please sign in to comment.