Refactor server's client address functions, handle debug dummies

Generate unique local addresses (IPv6 addresses) for debug dummies so address-based functions work with debug dummies. Every debug dummy's ULA has a randomly generated global ID such that debug dummy addresses are most likely unique. The ULA's subnet ID is set to a constant value. The ULA's interface ID is set to the dummy's client ID. See https://en.wikipedia.org/wiki/Unique_local_address for details about the format. The port is generated randomly in the same way that normal clients select their port.

Change the `const char (*::CNetServer::ClientAddrString(int ClientID) const)[NETADDR_MAXSTRSIZE]` function signature to `const std::array<char, NETADDR_MAXSTRSIZE> &ClientAddrString(int ClientId, bool IncludePort) const` and change the `const char (*CNetConnection::PeerAddressString() const)[NETADDR_MAXSTRSIZE]` function signature to `const std::array<char, NETADDR_MAXSTRSIZE> &PeerAddressString(bool IncludePort) const` to optionally include the port and because a reference to an `std::array` is more readable then a pointer/reference to a C-array. The address string is now cached separately with and without port by `CNetConnection`.

Add the `CNetConnection::SetPeerAddr` and `CNetConnection::ClearPeerAddr` functions to avoid duplicate code. Fix the peer address string not being initialized for 0.7 connections, though this is currently unused from the client-side so it had no effect.

Change the `void IServer::GetClientAddr(int ClientId, NETADDR *pAddr) const` function signature to `const NETADDR *ClientAddr(int ClientId) const`. This simplifies the usage as most address-based functions take pointers to `NETADDR` as arguments. Use this function consistently instead of the `CNetServer::ClientAddr` function, as the `IServer::ClientAddr` function has assertions and returns the generated ULA for debug dummies.

Change the `void IServer::GetClientAddr(int ClientId, char *pAddrStr, int Size) const` function signature to `const std::array<char, NETADDR_MAXSTRSIZE> &ClientAddrStringImpl(int ClientId, bool IncludePort) const` and add the `inline const char *ClientAddrString(int ClientId, bool IncludePort) const` wrapper function. This also simplifies the usage in most cases while retaining the static size information for the address strings. These functions can return the string with and without port, whereas before the string was always returned without port. Use this function consistently instead of the `CNetServer::ClientAddrString` function and instead of the `net_addr_str` function together with the `IServer::GetClientAddr` function, as the `IServer::ClientAddrString` function has assertions and returns the generated ULA strings for debug dummies. This should also slightly improves performance as the cached address strings are thereby used in all cases. Note that the `CNetServer::ClientAddrString` function previously returned the address string with port, whereas the `IServer::GetClientAddr` function returned the address string without port.

Compare `NETADDR`s directly without ports instead of converting to strings without ports when counting votes.

Closes ddnet#9554.

Author: Robyt3

src/engine/server.h

@@ -3,6 +3,7 @@
 #ifndef ENGINE_SERVER_H
 #define ENGINE_SERVER_H
 
+#include <array>
 #include <optional>
 #include <type_traits>
 
@@ -60,7 +61,9 @@ class IServer : public IInterface
 	virtual bool ClientIngame(int ClientId) const = 0;
 	virtual bool GetClientInfo(int ClientId, CClientInfo *pInfo) const = 0;
 	virtual void SetClientDDNetVersion(int ClientId, int DDNetVersion) = 0;
-	virtual void GetClientAddr(int ClientId, char *pAddrStr, int Size) const = 0;
+	virtual const NETADDR *ClientAddr(int ClientId) const = 0;
+	virtual const std::array<char, NETADDR_MAXSTRSIZE> &ClientAddrStringImpl(int ClientId, bool IncludePort) const = 0;
+	inline const char *ClientAddrString(int ClientId, bool IncludePort) const { return ClientAddrStringImpl(ClientId, IncludePort).data(); }
 
 	/**
 	 * Returns the version of the client with the given client ID.
@@ -265,8 +268,6 @@ class IServer : public IInterface
 	virtual bool IsRecording(int ClientId) = 0;
 	virtual void StopDemos() = 0;
 
-	virtual void GetClientAddr(int ClientId, NETADDR *pAddr) const = 0;
-
 	virtual int *GetIdMap(int ClientId) = 0;
 
 	virtual bool DnsblWhite(int ClientId) = 0;

src/engine/server/server.cpp

@@ -162,6 +162,9 @@ class CServer : public IServer
 		int m_Flags;
 		bool m_ShowIps;
 		bool m_DebugDummy;
+		NETADDR m_DebugDummyAddr;
+		std::array<char, NETADDR_MAXSTRSIZE> m_aDebugDummyAddrString;
+		std::array<char, NETADDR_MAXSTRSIZE> m_aDebugDummyAddrStringNoPort;
 
 		const IConsole::CCommandInfo *m_pRconCmdToSend;
 
@@ -300,7 +303,8 @@ class CServer : public IServer
 	void GetMapInfo(char *pMapName, int MapNameSize, int *pMapSize, SHA256_DIGEST *pMapSha256, int *pMapCrc) override;
 	bool GetClientInfo(int ClientId, CClientInfo *pInfo) const override;
 	void SetClientDDNetVersion(int ClientId, int DDNetVersion) override;
-	void GetClientAddr(int ClientId, char *pAddrStr, int Size) const override;
+	const NETADDR *ClientAddr(int ClientId) const override;
+	const std::array<char, NETADDR_MAXSTRSIZE> &ClientAddrStringImpl(int ClientId, bool IncludePort) const override;
 	const char *ClientName(int ClientId) const override;
 	const char *ClientClan(int ClientId) const override;
 	int ClientCountry(int ClientId) const override;
@@ -445,7 +449,6 @@ class CServer : public IServer
 
 	// DDRace
 
-	void GetClientAddr(int ClientId, NETADDR *pAddr) const override;
 	int m_aPrevStates[MAX_CLIENTS];
 	const char *GetAnnouncementLine() override;
 	void ReadAnnouncementsFile();

src/engine/server/server.h

@@ -9,6 +9,8 @@
 #include <base/math.h>
 #include <base/system.h>
 
+#include <array>
+
 class CHuffman;
 class CNetBan;
 class CPacker;
@@ -265,13 +267,16 @@ class CNetConnection
 	NETSOCKET m_Socket;
 	NETSTATS m_Stats;
 
-	char m_aPeerAddrStr[NETADDR_MAXSTRSIZE];
+	std::array<char, NETADDR_MAXSTRSIZE> m_aPeerAddrStr;
+	std::array<char, NETADDR_MAXSTRSIZE> m_aPeerAddrStrNoPort;
 	// client 0.7
 	static TOKEN GenerateToken7(const NETADDR *pPeerAddr);
 	class CNetBase *m_pNetBase;
 	bool IsSixup() { return m_Sixup; }
 
 	//
+	void SetPeerAddr(const NETADDR *pAddr);
+	void ClearPeerAddr();
 	void ResetStats();
 	void SetError(const char *pString);
 	void AckChunks(int Ack);
@@ -305,7 +310,10 @@ class CNetConnection
 	void SignalResend();
 	int State() const { return m_State; }
 	const NETADDR *PeerAddress() const { return &m_PeerAddr; }
-	const char (*PeerAddressString() const)[NETADDR_MAXSTRSIZE] { return &m_aPeerAddrStr; }
+	const std::array<char, NETADDR_MAXSTRSIZE> &PeerAddressString(bool IncludePort) const
+	{
+		return IncludePort ? m_aPeerAddrStr : m_aPeerAddrStrNoPort;
+	}
 	void ConnectAddresses(const NETADDR **ppAddrs, int *pNumAddrs) const
 	{
 		*ppAddrs = m_aConnectAddrs;
@@ -455,7 +463,7 @@ class CNetServer
 
 	// status requests
 	const NETADDR *ClientAddr(int ClientId) const { return m_aSlots[ClientId].m_Connection.PeerAddress(); }
-	const char (*ClientAddrString(int ClientID) const)[NETADDR_MAXSTRSIZE] { return m_aSlots[ClientID].m_Connection.PeerAddressString(); }
+	const std::array<char, NETADDR_MAXSTRSIZE> &ClientAddrString(int ClientId, bool IncludePort) const { return m_aSlots[ClientId].m_Connection.PeerAddressString(IncludePort); }
 	bool HasSecurityToken(int ClientId) const { return m_aSlots[ClientId].m_Connection.SecurityToken() != NET_SECURITY_TOKEN_UNSUPPORTED; }
 	NETADDR Address() const { return m_Address; }
 	NETSOCKET Socket() const { return m_Socket; }

src/engine/shared/network.h

@@ -4,11 +4,24 @@
 #include "network.h"
 #include <base/system.h>
 
-void CNetConnection::ResetStats()
+void CNetConnection::SetPeerAddr(const NETADDR *pAddr)
+{
+	m_PeerAddr = *pAddr;
+	net_addr_str(pAddr, m_aPeerAddrStr.data(), m_aPeerAddrStr.size(), true);
+	net_addr_str(pAddr, m_aPeerAddrStrNoPort.data(), m_aPeerAddrStrNoPort.size(), false);
+}
+
+void CNetConnection::ClearPeerAddr()
 {
-	mem_zero(&m_Stats, sizeof(m_Stats));
 	mem_zero(&m_PeerAddr, sizeof(m_PeerAddr));
 	m_aPeerAddrStr[0] = '\0';
+	m_aPeerAddrStrNoPort[0] = '\0';
+}
+
+void CNetConnection::ResetStats()
+{
+	mem_zero(&m_Stats, sizeof(m_Stats));
+	ClearPeerAddr();
 	m_LastUpdateTime = 0;
 }
 
@@ -32,11 +45,9 @@ void CNetConnection::Reset(bool Rejoin)
 
 	m_LastSendTime = 0;
 	m_LastRecvTime = 0;
-	//m_LastUpdateTime = 0;
 
 	mem_zero(&m_aConnectAddrs, sizeof(m_aConnectAddrs));
 	m_NumConnectAddrs = 0;
-	//mem_zero(&m_PeerAddr, sizeof(m_PeerAddr));
 	m_UnknownSeq = false;
 
 	m_Buffer.Init();
@@ -196,8 +207,7 @@ int CNetConnection::Connect(const NETADDR *pAddr, int NumAddrs)
 
 	// init connection
 	Reset();
-	mem_zero(&m_PeerAddr, sizeof(m_PeerAddr));
-	m_aPeerAddrStr[0] = '\0';
+	ClearPeerAddr();
 
 	for(int i = 0; i < NumAddrs; i++)
 	{
@@ -224,14 +234,13 @@ int CNetConnection::Connect7(const NETADDR *pAddr, int NumAddrs)
 
 	// init connection
 	Reset();
-	mem_zero(&m_PeerAddr, sizeof(m_PeerAddr));
 	for(int i = 0; i < NumAddrs; i++)
 	{
 		m_aConnectAddrs[i] = pAddr[i];
 	}
 	m_LastRecvTime = time_get();
 	m_NumConnectAddrs = NumAddrs;
-	m_PeerAddr = *pAddr;
+	SetPeerAddr(pAddr);
 	SetToken7(GenerateToken7(pAddr));
 	mem_zero(m_aErrorString, sizeof(m_aErrorString));
 	m_State = NET_CONNSTATE_TOKEN;
@@ -287,8 +296,7 @@ void CNetConnection::DirectInit(const NETADDR &Addr, SECURITY_TOKEN SecurityToke
 
 	m_State = NET_CONNSTATE_ONLINE;
 
-	m_PeerAddr = Addr;
-	net_addr_str(&Addr, m_aPeerAddrStr, sizeof(m_aPeerAddrStr), true);
+	SetPeerAddr(&Addr);
 	mem_zero(m_aErrorString, sizeof(m_aErrorString));
 
 	int64_t Now = time_get();
@@ -420,8 +428,7 @@ int CNetConnection::Feed(CNetPacketConstruct *pPacket, NETADDR *pAddr, SECURITY_
 						// send response and init connection
 						Reset();
 						m_State = NET_CONNSTATE_PENDING;
-						m_PeerAddr = *pAddr;
-						net_addr_str(pAddr, m_aPeerAddrStr, sizeof(m_aPeerAddrStr), true);
+						SetPeerAddr(pAddr);
 						mem_zero(m_aErrorString, sizeof(m_aErrorString));
 						m_LastSendTime = Now;
 						m_LastRecvTime = Now;
@@ -448,8 +455,7 @@ int CNetConnection::Feed(CNetPacketConstruct *pPacket, NETADDR *pAddr, SECURITY_
 					// connection made
 					if(CtrlMsg == NET_CTRLMSG_CONNECTACCEPT)
 					{
-						m_PeerAddr = *pAddr;
-						net_addr_str(pAddr, m_aPeerAddrStr, sizeof(m_aPeerAddrStr), true);
+						SetPeerAddr(pAddr);
 						if(m_SecurityToken == NET_SECURITY_TOKEN_UNKNOWN && pPacket->m_DataSize >= (int)(1 + sizeof(SECURITY_TOKEN_MAGIC) + sizeof(m_SecurityToken)) && !mem_comp(&pPacket->m_aChunkData[1], SECURITY_TOKEN_MAGIC, sizeof(SECURITY_TOKEN_MAGIC)))
 						{
 							m_SecurityToken = ToSecurityToken(&pPacket->m_aChunkData[1 + sizeof(SECURITY_TOKEN_MAGIC)]);
@@ -576,8 +582,7 @@ void CNetConnection::SetTimedOut(const NETADDR *pAddr, int Sequence, int Ack, SE
 	m_RemoteClosed = 0;
 
 	m_State = NET_CONNSTATE_ONLINE;
-	m_PeerAddr = *pAddr;
-	net_addr_str(pAddr, m_aPeerAddrStr, sizeof(m_aPeerAddrStr), true);
+	SetPeerAddr(pAddr);
 	mem_zero(m_aErrorString, sizeof(m_aErrorString));
 	m_LastSendTime = Now;
 	m_LastRecvTime = Now;

src/engine/shared/network_conn.cpp

@@ -254,9 +254,7 @@ int CNetServer::TryAcceptClient(NETADDR &Addr, SECURITY_TOKEN SecurityToken, boo
 
 	if(g_Config.m_Debug)
 	{
-		char aAddrStr[NETADDR_MAXSTRSIZE];
-		net_addr_str(&Addr, aAddrStr, sizeof(aAddrStr), true);
-		dbg_msg("security", "client accepted %s", aAddrStr);
+		dbg_msg("security", "client accepted %s", m_aSlots[Slot].m_Connection.PeerAddressString(true).data());
 	}
 
 	if(VanillaAuth)

src/engine/shared/network_server.cpp

@@ -610,12 +610,11 @@ void CGameContext::ConVoteMute(IConsole::IResult *pResult, void *pUserData)
 		return;
 	}
 
-	NETADDR Addr;
-	pSelf->Server()->GetClientAddr(Victim, &Addr);
+	const NETADDR *pAddr = pSelf->Server()->ClientAddr(Victim);
 
 	int Seconds = clamp(pResult->GetInteger(1), 1, 86400);
 	const char *pReason = pResult->NumArguments() > 2 ? pResult->GetString(2) : "";
-	pSelf->VoteMute(&Addr, Seconds, pReason, pSelf->Server()->ClientName(Victim), pResult->m_ClientId);
+	pSelf->VoteMute(pAddr, Seconds, pReason, pSelf->Server()->ClientName(Victim), pResult->m_ClientId);
 }
 
 void CGameContext::ConVoteUnmute(IConsole::IResult *pResult, void *pUserData)
@@ -629,10 +628,9 @@ void CGameContext::ConVoteUnmute(IConsole::IResult *pResult, void *pUserData)
 		return;
 	}
 
-	NETADDR Addr;
-	pSelf->Server()->GetClientAddr(Victim, &Addr);
+	const NETADDR *pAddr = pSelf->Server()->ClientAddr(Victim);
 
-	bool Found = pSelf->VoteUnmute(&Addr, pSelf->Server()->ClientName(Victim), pResult->m_ClientId);
+	bool Found = pSelf->VoteUnmute(pAddr, pSelf->Server()->ClientName(Victim), pResult->m_ClientId);
 	if(Found)
 	{
 		char aBuf[128];
@@ -655,7 +653,7 @@ void CGameContext::ConVoteMutes(IConsole::IResult *pResult, void *pUserData)
 		return;
 	}
 
-	char aIpBuf[64];
+	char aIpBuf[NETADDR_MAXSTRSIZE];
 	char aBuf[128];
 	pSelf->Console()->Print(IConsole::OUTPUT_LEVEL_STANDARD, "votemutes",
 		"Active vote mutes:");
@@ -689,12 +687,11 @@ void CGameContext::ConMuteId(IConsole::IResult *pResult, void *pUserData)
 		return;
 	}
 
-	NETADDR Addr;
-	pSelf->Server()->GetClientAddr(Victim, &Addr);
+	const NETADDR *pAddr = pSelf->Server()->ClientAddr(Victim);
 
 	const char *pReason = pResult->NumArguments() > 2 ? pResult->GetString(2) : "";
 
-	pSelf->Mute(&Addr, clamp(pResult->GetInteger(1), 1, 86400),
+	pSelf->Mute(pAddr, clamp(pResult->GetInteger(1), 1, 86400),
 		pSelf->Server()->ClientName(Victim), pReason);
 }
 
@@ -721,7 +718,7 @@ void CGameContext::ConUnmute(IConsole::IResult *pResult, void *pUserData)
 	if(Index < 0 || Index >= pSelf->m_NumMutes)
 		return;
 
-	char aIpBuf[64];
+	char aIpBuf[NETADDR_MAXSTRSIZE];
 	char aBuf[64];
 	net_addr_str(&pSelf->m_aMutes[Index].m_Addr, aIpBuf, sizeof(aIpBuf), false);
 	str_format(aBuf, sizeof(aBuf), "Unmuted %s", aIpBuf);
@@ -740,14 +737,13 @@ void CGameContext::ConUnmuteId(IConsole::IResult *pResult, void *pUserData)
 	if(Victim < 0 || Victim > MAX_CLIENTS || !pSelf->m_apPlayers[Victim])
 		return;
 
-	NETADDR Addr;
-	pSelf->Server()->GetClientAddr(Victim, &Addr);
+	const NETADDR *pAddr = pSelf->Server()->ClientAddr(Victim);
 
 	for(int i = 0; i < pSelf->m_NumMutes; i++)
 	{
-		if(net_addr_comp_noport(&pSelf->m_aMutes[i].m_Addr, &Addr) == 0)
+		if(net_addr_comp_noport(&pSelf->m_aMutes[i].m_Addr, pAddr) == 0)
 		{
-			char aIpBuf[64];
+			char aIpBuf[NETADDR_MAXSTRSIZE];
 			char aBuf[64];
 			net_addr_str(&pSelf->m_aMutes[i].m_Addr, aIpBuf, sizeof(aIpBuf), false);
 			str_format(aBuf, sizeof(aBuf), "Unmuted %s", aIpBuf);
@@ -975,7 +971,7 @@ void CGameContext::LogEvent(const char *Description, int ClientId)
 	if(!pNewEntry->m_FromServer)
 	{
 		pNewEntry->m_ClientVersion = Server()->GetClientVersion(ClientId);
-		Server()->GetClientAddr(ClientId, pNewEntry->m_aClientAddrStr, sizeof(pNewEntry->m_aClientAddrStr));
+		str_copy(pNewEntry->m_aClientAddrStr, Server()->ClientAddrString(ClientId, false));
 		str_copy(pNewEntry->m_aClientName, Server()->ClientName(ClientId));
 	}
 }