Engine: minor remote console (RCON) improvements

- Improve ConnectedNetConsoleData_s member variable naming consistency.
- Use exactly the number of bytes needed for storing the Base64 AES-128 netkey for RCON encryption, and use the size to copy the results from the stack faster.
- Use the correct integral size types.
This commit is contained in:
Kawe Mazidjatari committed 2025-06-19 12:12:43 +02:00
1 parent a445ad2ad7
commit 8c55ee9e32
8 files changed
+94 -93

No files matched your search

+20 -20
View File
@@ -30,29 +30,29 @@ enum class ServerDataResponseType_e : int
struct ConnectedNetConsoleData_s
{
SocketHandle_t m_hSocket;
u32 m_nPayloadLen; // Num bytes for this message.
u32 m_nPayloadRead; // Num read bytes from input buffer.
s32 m_nFailedAttempts; // Num failed authentication attempts.
s32 m_nIgnoredMessage; // Count how many times client ignored the no-auth message.
bool m_bValidated; // Revalidates netconsole if false.
bool m_bAuthorized; // Set to true after successful netconsole auth.
bool m_bInputOnly; // If set, don't send spew to this netconsole.
NetConFrameHeader_s m_FrameHeader; // Current frame header.
vector<byte> m_RecvBuffer;
SocketHandle_t socket;
u32 payloadLen; // Num bytes for this message.
u32 payloadRead; // Num read bytes from input buffer.
s32 numFailedAttempts; // Num failed authentication attempts.
s32 numIgnoredMessage; // Count how many times client ignored the no-auth message.
bool validated; // Revalidates netconsole if false.
bool authorized; // Set to true after successful netconsole auth.
bool inputOnly; // If set, don't send spew to this netconsole.
NetConFrameHeader_s frameHeader; // Current frame header.
vector<byte> recvBuffer;
ConnectedNetConsoleData_s(SocketHandle_t hSocket = -1)
{
m_hSocket = hSocket;
m_nPayloadLen = 0;
m_nPayloadRead = 0;
m_nFailedAttempts = 0;
m_nIgnoredMessage = 0;
m_bValidated = false;
m_bAuthorized = false;
m_bInputOnly = true;
m_FrameHeader.magic = 0;
m_FrameHeader.length = 0;
socket = hSocket;
payloadLen = 0;
payloadRead = 0;
numFailedAttempts = 0;
numIgnoredMessage = 0;
validated = false;
authorized = false;
inputOnly = true;
frameHeader.magic = 0;
frameHeader.length = 0;
}
};
+1 -1
View File
@@ -73,7 +73,7 @@ void CRConClient::RunFrame(void)
{
if (IsInitialized() && IsConnected())
{
ConnectedNetConsoleData_s* pData = GetData();
ConnectedNetConsoleData_s* const pData = GetData();
Assert(pData != nullptr);
if (pData)
+1 -1
View File
@@ -2,7 +2,7 @@
constexpr unsigned int AES_128_KEY_SIZE = 16;
constexpr unsigned int AES_128_B64_ENCODED_SIZE = 24;
constexpr const char* DEFAULT_NET_ENCRYPTION_KEY = "WDNWLmJYQ2ZlM0VoTid3Yg==";
constexpr const char DEFAULT_NET_ENCRYPTION_KEY[AES_128_B64_ENCODED_SIZE+1] = "WDNWLmJYQ2ZlM0VoTid3Yg==";
#ifndef _TOOLS
#include "engine/net_chan.h"
+45 -42
View File
@@ -123,7 +123,7 @@ void CRConServer::Shutdown(void)
m_Socket.CloseListenSocket();
}
Msg(eDLL_T::SERVER, "Remote server access deinitialized ('%i' accepted sockets closed)\n", nConnCount);
Msg(eDLL_T::SERVER, "Remote server access deinitialized (%i accepted sockets closed)\n", nConnCount);
}
//-----------------------------------------------------------------------------
@@ -155,7 +155,7 @@ void CRConServer::Think(void)
if (!m_WhiteListAddress.CompareAdr(netAdr))
{
const ConnectedNetConsoleData_s& data = m_Socket.GetAcceptedSocketData(m_nConnIndex);
if (!data.m_bAuthorized)
if (!data.authorized)
{
Disconnect("redundant");
}
@@ -251,7 +251,7 @@ void CRConServer::RunFrame(void)
if (CheckForBan(data))
{
SendEncoded(data.m_hSocket, s_BannedMessage, sizeof(s_BannedMessage)-1, "", 0,
SendEncoded(data.socket, s_BannedMessage, sizeof(s_BannedMessage)-1, "", 0,
netcon::response_e::SERVERDATA_RESPONSE_AUTH, int(eDLL_T::NETCON));
Disconnect("banned");
@@ -278,9 +278,9 @@ bool CRConServer::SendToAll(const byte* pMsgBuf, const u32 nMsgLen) const
{
const ConnectedNetConsoleData_s& data = m_Socket.GetAcceptedSocketData(i);
if (data.m_bAuthorized && !data.m_bInputOnly)
if (data.authorized && !data.inputOnly)
{
const int ret = ::send(data.m_hSocket, (const char*)pMsgBuf, (i32)nMsgLen, MSG_NOSIGNAL);
const int ret = ::send(data.socket, (const char*)pMsgBuf, (i32)nMsgLen, MSG_NOSIGNAL);
if (ret == SOCKET_ERROR)
{
@@ -315,7 +315,7 @@ bool CRConServer::SendEncoded(const char* pResponseMsg, const size_t nResponseMs
{
return false;
}
if (!SendToAll(vecMsg.data(), int(vecMsg.size())))
if (!SendToAll(vecMsg.data(), u32(vecMsg.size())))
{
Error(eDLL_T::SERVER, NO_ERROR, "Failed to send RCON message: (%s)\n", "SOCKET_ERROR");
return false;
@@ -346,7 +346,7 @@ bool CRConServer::SendEncoded(const SocketHandle_t hSocket,
{
return false;
}
if (!Send(hSocket, vecMsg.data(), int(vecMsg.size())))
if (!Send(hSocket, vecMsg.data(), u32(vecMsg.size())))
{
Error(eDLL_T::SERVER, NO_ERROR, "Failed to send RCON message: (%s)\n", "SOCKET_ERROR");
return false;
@@ -382,7 +382,7 @@ bool CRConServer::Serialize(vector<byte>& vecBuf,
//-----------------------------------------------------------------------------
void CRConServer::Authenticate(const netcon::request& request, ConnectedNetConsoleData_s& data)
{
if (data.m_bAuthorized)
if (data.authorized)
{
return;
}
@@ -390,16 +390,16 @@ void CRConServer::Authenticate(const netcon::request& request, ConnectedNetConso
// Authorize.
if (Comparator(request.requestmsg()))
{
data.m_bAuthorized = true;
data.authorized = true;
if (++m_nAuthConnections >= sv_rcon_maxconnections.GetInt())
{
m_Socket.CloseListenSocket();
CloseNonAuthConnection();
}
const char* pSendLogs = (!sv_rcon_sendlogs.GetBool() || data.m_bInputOnly) ? "0" : "1";
const char* const pSendLogs = (!sv_rcon_sendlogs.GetBool() || data.inputOnly) ? "0" : "1";
SendEncoded(data.m_hSocket, s_AuthMessage, sizeof(s_AuthMessage)-1, pSendLogs, 1,
SendEncoded(data.socket, s_AuthMessage, sizeof(s_AuthMessage)-1, pSendLogs, 1,
netcon::response_e::SERVERDATA_RESPONSE_AUTH, static_cast<int>(eDLL_T::NETCON));
}
else // Bad password.
@@ -410,12 +410,12 @@ void CRConServer::Authenticate(const netcon::request& request, ConnectedNetConso
Msg(eDLL_T::SERVER, "Bad RCON password attempt from '%s'\n", netAdr.ToString());
}
SendEncoded(data.m_hSocket, s_WrongPwMessage, sizeof(s_WrongPwMessage)-1, "", 0,
SendEncoded(data.socket, s_WrongPwMessage, sizeof(s_WrongPwMessage)-1, "", 0,
netcon::response_e::SERVERDATA_RESPONSE_AUTH, static_cast<int>(eDLL_T::NETCON));
data.m_bAuthorized = false;
data.m_bValidated = false;
data.m_nFailedAttempts++;
data.authorized = false;
data.validated = false;
data.numFailedAttempts++;
}
}
@@ -457,15 +457,15 @@ bool CRConServer::ProcessMessage(const byte* pMsgBuf, const u32 nMsgLen, const u
ConnectedNetConsoleData_s& data = m_Socket.GetAcceptedSocketData(m_nConnIndex);
if (!data.m_bAuthorized &&
if (!data.authorized &&
request.requesttype() != netcon::request_e::SERVERDATA_REQUEST_AUTH)
{
// Notify netconsole that authentication is required.
SendEncoded(data.m_hSocket, s_NoAuthMessage, sizeof(s_NoAuthMessage)-1, "", 0,
SendEncoded(data.socket, s_NoAuthMessage, sizeof(s_NoAuthMessage)-1, "", 0,
netcon::response_e::SERVERDATA_RESPONSE_AUTH, static_cast<int>(eDLL_T::NETCON));
data.m_bValidated = false;
data.m_nIgnoredMessage++;
data.validated = false;
data.numIgnoredMessage++;
return true;
}
switch (request.requesttype())
@@ -477,7 +477,7 @@ bool CRConServer::ProcessMessage(const byte* pMsgBuf, const u32 nMsgLen, const u
}
case netcon::request_e::SERVERDATA_REQUEST_EXECCOMMAND:
{
if (data.m_bAuthorized) // Only execute if auth was successful.
if (data.authorized) // Only execute if auth was successful.
{
Execute(request);
}
@@ -485,12 +485,12 @@ bool CRConServer::ProcessMessage(const byte* pMsgBuf, const u32 nMsgLen, const u
}
case netcon::request_e::SERVERDATA_REQUEST_SEND_CONSOLE_LOG:
{
if (data.m_bAuthorized)
if (data.authorized)
{
// request value "0" means the netconsole is input only.
const bool bWantLog = atoi(request.requestval().c_str()) != NULL;
data.m_bInputOnly = !bWantLog;
data.inputOnly = !bWantLog;
if (bWantLog && !sv_rcon_sendlogs.GetBool())
{
// Toggle it on since there's at least 1 netconsole that
@@ -524,7 +524,7 @@ void CRConServer::Execute(const netcon::request& request) const
//-----------------------------------------------------------------------------
bool CRConServer::CheckForBan(ConnectedNetConsoleData_s& data)
{
if (data.m_bValidated)
if (data.validated)
{
return false;
}
@@ -555,7 +555,7 @@ bool CRConServer::CheckForBan(ConnectedNetConsoleData_s& data)
}
}
data.m_bValidated = true;
data.validated = true;
// Check if IP is in the banned list.
if (m_BannedList.find(szNetAdr) != m_BannedList.end())
@@ -564,14 +564,14 @@ bool CRConServer::CheckForBan(ConnectedNetConsoleData_s& data)
}
// Check if netconsole has reached maximum number of attempts > add to banned list.
if (data.m_nFailedAttempts >= sv_rcon_maxfailures.GetInt()
|| data.m_nIgnoredMessage >= sv_rcon_maxignores.GetInt())
if (data.numFailedAttempts >= sv_rcon_maxfailures.GetInt()
|| data.numIgnoredMessage >= sv_rcon_maxignores.GetInt())
{
// Don't add white listed address to banned list.
if (m_WhiteListAddress.CompareAdr(netAdr))
{
data.m_nFailedAttempts = 0;
data.m_nIgnoredMessage = 0;
data.numFailedAttempts = 0;
data.numIgnoredMessage = 0;
return false;
}
@@ -598,7 +598,7 @@ void CRConServer::Disconnect(const char* szReason) // NETMGR
void CRConServer::Disconnect(const int nIndex, const char* szReason) // NETMGR
{
ConnectedNetConsoleData_s& data = m_Socket.GetAcceptedSocketData(nIndex);
if (data.m_bAuthorized)
if (data.authorized)
{
// Inform server owner when authenticated connection has been closed.
const netadr_t& netAdr = m_Socket.GetAcceptedSocketAddress(nIndex);
@@ -619,12 +619,13 @@ void CRConServer::Disconnect(const int nIndex, const char* szReason) // NETMGR
//-----------------------------------------------------------------------------
void CRConServer::CloseNonAuthConnection(void)
{
int nCount = m_Socket.GetAcceptedSocketCount();
const int nCount = m_Socket.GetAcceptedSocketCount();
for (int i = nCount - 1; i >= 0; i--)
{
ConnectedNetConsoleData_s& data = m_Socket.GetAcceptedSocketData(i);
const ConnectedNetConsoleData_s& data = m_Socket.GetAcceptedSocketData(i);
if (!data.m_bAuthorized)
if (!data.authorized)
{
m_Socket.CloseAcceptedSocket(i);
}
@@ -677,9 +678,9 @@ int CRConServer::GetAuthenticatedCount(void) const
//-----------------------------------------------------------------------------
static void RCON_PasswordChanged_f(IConVar* pConVar, const char* pOldString, float flOldValue, ChangeUserData_t pUserData)
{
if (ConVar* pConVarRef = g_pCVar->FindVar(pConVar->GetName()))
if (ConVar* const pConVarRef = g_pCVar->FindVar(pConVar->GetName()))
{
const char* pNewString = pConVarRef->GetString();
const char* const pNewString = pConVarRef->GetString();
if (strcmp(pOldString, pNewString) == NULL)
return; // Same password.
@@ -700,7 +701,7 @@ static void RCON_PasswordChanged_f(IConVar* pConVar, const char* pOldString, flo
//-----------------------------------------------------------------------------
static void RCON_WhiteListAddresChanged_f(IConVar* pConVar, const char* pOldString, float flOldValue, ChangeUserData_t pUserData)
{
if (ConVar* pConVarRef = g_pCVar->FindVar(pConVar->GetName()))
if (ConVar* const pConVarRef = g_pCVar->FindVar(pConVar->GetName()))
{
if (strcmp(pOldString, pConVarRef->GetString()) == NULL)
return; // Same address.
@@ -720,15 +721,15 @@ static void RCON_ConnectionCountChanged_f(IConVar* pConVar, const char* pOldStri
if (!RCONServer()->IsInitialized())
return; // Not initialized; no sockets at this point.
if (ConVar* pConVarRef = g_pCVar->FindVar(pConVar->GetName()))
if (ConVar* const pConVarRef = g_pCVar->FindVar(pConVar->GetName()))
{
if (strcmp(pOldString, pConVarRef->GetString()) == NULL)
return; // Same count.
const int maxCount = pConVarRef->GetInt();
const int count = RCONServer()->GetAuthenticatedCount();
int count = RCONServer()->GetAuthenticatedCount();
CSocketCreator* pCreator = RCONServer()->GetSocketCreator();
CSocketCreator* const pCreator = RCONServer()->GetSocketCreator();
if (count < maxCount)
{
@@ -739,10 +740,12 @@ static void RCON_ConnectionCountChanged_f(IConVar* pConVar, const char* pOldStri
}
else
{
while (count > maxCount)
int currCount = count;
while (currCount > maxCount)
{
RCONServer()->Disconnect(count - 1, "too many authenticated sockets");
count = RCONServer()->GetAuthenticatedCount();
RCONServer()->Disconnect(currCount - 1, "too many authenticated sockets");
currCount = RCONServer()->GetAuthenticatedCount();
}
pCreator->CloseListenSocket();
@@ -756,7 +759,7 @@ static void RCON_ConnectionCountChanged_f(IConVar* pConVar, const char* pOldStri
//-----------------------------------------------------------------------------
static void RCON_UseLoopbackSocketChanged_f(IConVar* pConVar, const char* pOldString, float flOldValue, ChangeUserData_t pUserData)
{
if (ConVar* pConVarRef = g_pCVar->FindVar(pConVar->GetName()))
if (ConVar* const pConVarRef = g_pCVar->FindVar(pConVar->GetName()))
{
if (strcmp(pOldString, pConVarRef->GetString()) == NULL)
return; // Same value.
+24 -26
View File
@@ -57,7 +57,7 @@ void CNetConBase::SetKey(const char* pBase64NetKey, const bool bUseDefaultOnFail
}
else
{
m_Base64NetKey = tokenizedKey.c_str();
m_Base64NetKey.SetDirect(tokenizedKey.c_str(), ssize_t(tokenizedKey.length()));
}
}
}
@@ -81,9 +81,7 @@ void CNetConBase::SetKey(const char* pBase64NetKey, const bool bUseDefaultOnFail
}
else // Try to encode it
{
char encodedKey[45];
memset(encodedKey, 0, sizeof(encodedKey));
char encodedKey[AES_128_B64_ENCODED_SIZE+1]; // +1 for null terminator.
size_t numBytesEncoded = 0;
const int encodeRet = mbedtls_base64_encode(reinterpret_cast<unsigned char*>(&encodedKey),
@@ -101,7 +99,7 @@ void CNetConBase::SetKey(const char* pBase64NetKey, const bool bUseDefaultOnFail
}
else
{
m_Base64NetKey = encodedKey;
m_Base64NetKey.SetDirect(encodedKey, numBytesEncoded);
}
}
}
@@ -113,7 +111,7 @@ void CNetConBase::SetKey(const char* pBase64NetKey, const bool bUseDefaultOnFail
mbedtls_base64_decode(m_NetKey, sizeof(m_NetKey), &numBytesDecoded,
reinterpret_cast<const unsigned char*>(DEFAULT_NET_ENCRYPTION_KEY), AES_128_B64_ENCODED_SIZE);
m_Base64NetKey = DEFAULT_NET_ENCRYPTION_KEY;
m_Base64NetKey.SetDirect(DEFAULT_NET_ENCRYPTION_KEY, sizeof(DEFAULT_NET_ENCRYPTION_KEY));
}
}
@@ -149,39 +147,39 @@ bool CNetConBase::ProcessBuffer(ConnectedNetConsoleData_s& data, const byte* pRe
while (nRecvLen > 0)
{
// Read payload if it's already in progress.
if (data.m_nPayloadLen)
if (data.payloadLen)
{
const u32 bytesToCopy = Min(nRecvLen, data.m_nPayloadLen - data.m_nPayloadRead);
memcpy(&data.m_RecvBuffer[data.m_nPayloadRead], pRecvBuf, bytesToCopy);
const u32 bytesToCopy = Min(nRecvLen, data.payloadLen - data.payloadRead);
memcpy(&data.recvBuffer[data.payloadRead], pRecvBuf, bytesToCopy);
data.m_nPayloadRead += bytesToCopy;
data.payloadRead += bytesToCopy;
pRecvBuf += bytesToCopy;
nRecvLen -= bytesToCopy;
if (data.m_nPayloadRead == data.m_nPayloadLen)
if (data.payloadRead == data.payloadLen)
{
if (!ProcessMessage(data.m_RecvBuffer.data(), data.m_nPayloadLen, nMaxLen))
if (!ProcessMessage(data.recvBuffer.data(), data.payloadLen, nMaxLen))
return false;
// Reset state.
data.m_nPayloadLen = 0;
data.m_nPayloadRead = 0;
data.payloadLen = 0;
data.payloadRead = 0;
}
}
else if (data.m_nPayloadRead < sizeof(NetConFrameHeader_s)) // Read the header if we haven't fully recv'd it.
else if (data.payloadRead < sizeof(NetConFrameHeader_s)) // Read the header if we haven't fully recv'd it.
{
const u32 bytesToCopy = Min(nRecvLen, int(sizeof(NetConFrameHeader_s)) - data.m_nPayloadRead);
memcpy(reinterpret_cast<char*>(&data.m_FrameHeader) + data.m_nPayloadRead, pRecvBuf, bytesToCopy);
const u32 bytesToCopy = Min(nRecvLen, int(sizeof(NetConFrameHeader_s)) - data.payloadRead);
memcpy(reinterpret_cast<char*>(&data.frameHeader) + data.payloadRead, pRecvBuf, bytesToCopy);
data.m_nPayloadRead += bytesToCopy;
data.payloadRead += bytesToCopy;
pRecvBuf += bytesToCopy;
nRecvLen -= bytesToCopy;
if (data.m_nPayloadRead == sizeof(NetConFrameHeader_s))
if (data.payloadRead == sizeof(NetConFrameHeader_s))
{
NetConFrameHeader_s& header = data.m_FrameHeader;
NetConFrameHeader_s& header = data.frameHeader;
// Convert byte order and check for desync.
header.magic = ntohl(header.magic);
@@ -216,10 +214,10 @@ bool CNetConBase::ProcessBuffer(ConnectedNetConsoleData_s& data, const byte* pRe
return false;
}
data.m_nPayloadLen = header.length;
data.m_nPayloadRead = 0;
data.payloadLen = header.length;
data.payloadRead = 0;
data.m_RecvBuffer.resize(header.length);
data.recvBuffer.resize(header.length);
}
}
}
@@ -305,7 +303,7 @@ void CNetConBase::Recv(ConnectedNetConsoleData_s& data, const u32 nMaxLen)
static char szRecvBuf[1024];
{//////////////////////////////////////////////
const int nPendingLen = ::recv(data.m_hSocket, szRecvBuf, sizeof(szRecvBuf), MSG_PEEK);
const int nPendingLen = ::recv(data.socket, szRecvBuf, sizeof(szRecvBuf), MSG_PEEK);
if (nPendingLen == SOCKET_ERROR && m_Socket.IsSocketBlocking())
{
return;
@@ -323,7 +321,7 @@ void CNetConBase::Recv(ConnectedNetConsoleData_s& data, const u32 nMaxLen)
}//////////////////////////////////////////////
u_long nReadLen = 0; // Find out how much we have to read.
const int iResult = ::ioctlsocket(data.m_hSocket, FIONREAD, &nReadLen);
const int iResult = ::ioctlsocket(data.socket, FIONREAD, &nReadLen);
if (iResult == SOCKET_ERROR)
{
@@ -333,7 +331,7 @@ void CNetConBase::Recv(ConnectedNetConsoleData_s& data, const u32 nMaxLen)
while (nReadLen > 0)
{
const int nRecvLen = ::recv(data.m_hSocket, szRecvBuf, MIN(sizeof(szRecvBuf), nReadLen), MSG_NOSIGNAL);
const int nRecvLen = ::recv(data.socket, szRecvBuf, MIN(sizeof(szRecvBuf), nReadLen), MSG_NOSIGNAL);
if (nRecvLen == 0) // Socket was closed.
{
Disconnect("socket closed");
+1 -1
View File
@@ -320,7 +320,7 @@ SocketHandle_t NetconShared_GetSocketHandle(CNetConBase* pBase, const int iSocke
return SOCKET_ERROR;
}
return pData->m_hSocket;
return pData->socket;
}
#ifndef _TOOLS
+1 -1
View File
@@ -268,7 +268,7 @@ void CNetCon::RunInput(const string& lineInput)
if (bSend) // Only send if serialization process was successful.
{
if (!Send(hSocket, vecMsg.data(), int(vecMsg.size())))
if (!Send(hSocket, vecMsg.data(), u32(vecMsg.size())))
{
Error(eDLL_T::CLIENT, NO_ERROR, "Failed to send RCON message: (%s)\n", "SOCKET_ERROR");
}
+1 -1
View File
@@ -334,7 +334,7 @@ int CSocketCreator::GetAuthorizedSocketCount(void) const
for (int i = 0; i < m_AcceptedSockets.Count(); ++i)
{
if (m_AcceptedSockets[i].m_Data.m_bAuthorized)
if (m_AcceptedSockets[i].m_Data.authorized)
{
ret++;
}