From e5c4e497828fd541c3060a745c1a24aced2e3307 Mon Sep 17 00:00:00 2001 From: Alibek Omarov Date: Mon, 25 May 2026 23:29:04 +0500 Subject: [PATCH] engine: client: introduce cookie protocol extension, when client connects to the server, it provides randomized 64-bit value to be prefixed into netchan messages * Prevent clientuseragent from leaking data. * Process qport in netchan, cleaning up code --- engine/client/cl_main.c | 47 +++++++++++++++++++++++++++++++- engine/client/client.h | 1 + engine/common/net_chan.c | 57 ++++++++++++++++++++++++++++++++++++--- engine/common/netchan.h | 12 ++++++--- engine/common/protocol.h | 3 ++- engine/server/sv_client.c | 31 ++++++++++++++++++++- engine/server/sv_main.c | 31 ++++++++------------- 7 files changed, 152 insertions(+), 30 deletions(-) diff --git a/engine/client/cl_main.c b/engine/client/cl_main.c index 8a73951d..47469e23 100644 --- a/engine/client/cl_main.c +++ b/engine/client/cl_main.c @@ -13,6 +13,7 @@ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. */ +#include #include "common.h" #include "client.h" #include "net_encode.h" @@ -1156,7 +1157,7 @@ static void CL_SendConnectPacket( connprotocol_t proto, int challenge ) else { const char *qport = Cvar_VariableString( "net_qport" ); - int extensions = adrtype == NA_LOOPBACK ? 0 : NET_EXT_SPLITSIZE; + int extensions = adrtype == NA_LOOPBACK ? 0 : ( NET_EXT_SPLITSIZE | NET_EXT_NETCHAN_COOKIE ); string key; ID_GetMD5ForAddress( key, adr, sizeof( key )); @@ -1171,6 +1172,17 @@ static void CL_SendConnectPacket( connprotocol_t proto, int challenge ) Info_SetValueForKey( protinfo, "qport", qport, sizeof( protinfo )); Info_SetValueForKeyf( protinfo, "ext", sizeof( protinfo ), "%d", extensions ); + if( FBitSet( extensions, NET_EXT_NETCHAN_COOKIE )) + { + uint64_t a = COM_RandomLong( 0, 0xFFFF ); + uint64_t b = COM_RandomLong( 0, 0xFFFF ); + uint64_t c = COM_RandomLong( 0, 0xFFFF ); + uint64_t d = COM_RandomLong( 0, 0xFFFF ); + cls.netchan_pending_cookie = ( a << 48 ) | ( b << 32 ) | ( c << 16 ) | d; + Info_SetValueForKeyf( protinfo, "cookie", sizeof( protinfo ), "%016"PRIx64, cls.netchan_pending_cookie ); + } + else cls.netchan_pending_cookie = 0; + Netchan_OutOfBandPrint( NS_CLIENT, adr, C2S_CONNECT" %i %i \"%s\" \"%s\"\n", PROTOCOL_VERSION, challenge, protinfo, cls.userinfo ); Con_Printf( "Trying to connect with modern protocol\n" ); } @@ -1664,10 +1676,19 @@ void CL_SetupNetchanForProtocol( connprotocol_t proto ) if( FBitSet( cls.extensions, NET_EXT_SPLITSIZE )) Con_Reportf( "^2NET_EXT_SPLITSIZE enabled^7 (packet size is %d)\n", (int)cl_dlmax.value ); + + if( FBitSet( cls.extensions, NET_EXT_NETCHAN_COOKIE )) + { + Con_Reportf( "^2NET_EXT_NETCHAN_COOKIE enabled^7\n" ); + SetBits( flags, NETCHAN_USE_COOKIE ); + } break; } Netchan_Setup( NS_CLIENT, &cls.netchan, net_from, Cvar_VariableInteger( "net_qport" ), NULL, pfnBlockSize, flags ); + + if( FBitSet( flags, NETCHAN_USE_COOKIE )) + Netchan_SetCookie( &cls.netchan, cls.netchan_pending_cookie ); } /* @@ -2385,6 +2406,30 @@ static void CL_ClientConnect( connprotocol_t proto, const char *c, netadr_t from return; } + if( cls.netchan_pending_cookie != 0 ) + { + const char *cookie_str = Info_ValueForKey( Cmd_Argv( 1 ), "cookie" ); + + if( Q_strlen( cookie_str ) != 16 ) + { + Con_Reportf( S_WARN "%s: missing cookie echo from %s, ignoring (possible spoof)\n", __func__, NET_AdrToString( from )); + return; + } + + byte buf[8]; + COM_HexConvert( cookie_str, 16, buf ); + + uint64_t echoed = 0; + for( int i = 0; i < 8; i++ ) + echoed = ( echoed << 8 ) | buf[i]; + + if( echoed != cls.netchan_pending_cookie ) + { + Con_Reportf( S_WARN "%s: invalid cookie echo from %s, ignoring (possible spoof)\n", __func__, NET_AdrToString( from )); + return; + } + } + cls.build_num = 0; // not used in Xash3D protocols cls.allow_cheats = Q_atoi( Info_ValueForKey( Cmd_Argv( 1 ), "cheats" )); } diff --git a/engine/client/client.h b/engine/client/client.h index 243c45f3..432d1796 100644 --- a/engine/client/client.h +++ b/engine/client/client.h @@ -579,6 +579,7 @@ typedef struct sizebuf_t datagram; // unreliable stuff. gets sent in CL_Move about cl_cmdrate times per second. byte datagram_buf[MAX_DATAGRAM]; + uint64_t netchan_pending_cookie; // random NET_EXT_NETCHAN_COOKIE netchan_t netchan; float packet_loss; diff --git a/engine/common/net_chan.c b/engine/common/net_chan.c index f8aeca2c..772e6d36 100644 --- a/engine/common/net_chan.c +++ b/engine/common/net_chan.c @@ -259,10 +259,24 @@ void Netchan_Setup( netsrc_t sock, netchan_t *chan, netadr_t adr, int qport, voi chan->use_bz2 = FBitSet( flags, NETCHAN_USE_BZIP2 ) ? true : false; chan->use_lzss = FBitSet( flags, NETCHAN_USE_LZSS ) ? true : false; chan->gs_netchan = FBitSet( flags, NETCHAN_GOLDSRC ) ? true : false; + chan->use_cookie = FBitSet( flags, NETCHAN_USE_COOKIE ) ? true : false; + chan->cookie = 0; MSG_Init( &chan->message, "NetData", chan->message_buf, sizeof( chan->message_buf )); } +/* +============== +Netchan_SetCookie + +called on the client after parsing NET_EXT_NETCHAN_COOKIE in the connect reply +============== +*/ +void Netchan_SetCookie( netchan_t *chan, uint64_t cookie ) +{ + chan->cookie = cookie; +} + /* ============================== Netchan_IncomingReady @@ -1673,6 +1687,14 @@ void Netchan_TransmitBits( netchan_t *chan, int length, const byte *data ) chan->outgoing_sequence++; + // prefix the cookie so the peer can authenticate this packet as ours + // before doing anything else with it + if( chan->use_cookie ) + { + MSG_WriteLong( &send, (uint)( chan->cookie & 0xFFFFFFFF )); + MSG_WriteLong( &send, (uint)( chan->cookie >> 32 )); + } + MSG_WriteLong( &send, w1 ); MSG_WriteLong( &send, w2 ); @@ -1797,15 +1819,42 @@ qboolean Netchan_Process( netchan_t *chan, sizebuf_t *msg ) // get sequence numbers MSG_Clear( msg ); + + // authenticate via the per-connection cookie before parsing anything else; + // a spoofed packet from a remote attacker won't know the 64-bit cookie and + // will be rejected here without touching sequence/ack state + if( chan->use_cookie ) + { + if( MSG_GetMaxBytes( msg ) < 16 ) + { + Con_Reportf( S_WARN "%s: %s: truncated packet (%d bytes) with cookie expected, dropping\n", __func__, NET_AdrToString( chan->remote_address ), MSG_GetMaxBytes( msg )); + return false; + } + + uint32_t cookie_lo = MSG_ReadDword( msg ); + uint32_t cookie_hi = MSG_ReadDword( msg ); + uint64_t cookie = ((uint64_t)cookie_hi << 32 ) | (uint64_t)cookie_lo; + + if( cookie != chan->cookie ) + { + Con_Reportf( S_WARN "%s: %s: cookie mismatch, dropping (possible spoof attempt)\n", __func__, NET_AdrToString( chan->remote_address )); + return false; + } + } + uint sequence = MSG_ReadLong( msg ); uint sequence_ack = MSG_ReadLong( msg ); if( chan->use_munge && MSG_GetMaxBytes( msg ) >= 8 ) COM_UnMunge2( msg->pData + 8, MSG_GetMaxBytes( msg ) - 8, sequence & 0xFF ); - // read the qport if we are a server + // read the qport if we are a server; serves as a NAT-stable + // connection demultiplexer and rejects packets for the wrong client if( chan->sock == NS_SERVER ) - MSG_ReadShort( msg ); + { + if(( MSG_ReadShort( msg ) & 0xffff ) != chan->qport ) + return false; + } uint reliable_message = sequence >> 31; uint reliable_ack = sequence_ack >> 31; @@ -1870,7 +1919,9 @@ qboolean Netchan_Process( netchan_t *chan, sizebuf_t *msg ) // reject packets that leap too far ahead of the expected sequence // skip on the very first packet — the server starts with a random // outgoing_sequence, so the first one legitimately jumps far ahead of 0 - if( chan->incoming_sequence != 0 && net_sequence_window.value > 0 && sequence > chan->incoming_sequence + (uint)net_sequence_window.value ) + // NOTE: disable sequence window with cookie extension, if cookie ext proves + // to be inefficient, we can safely enable sequence window back + if( !chan->use_cookie && chan->incoming_sequence != 0 && net_sequence_window.value > 0 && sequence > chan->incoming_sequence + (uint)net_sequence_window.value ) { Con_Printf( S_WARN "%s: %s: sequence %u jumps %u ahead of expected %i (window %i), dropping\n", __func__, NET_AdrToString( chan->remote_address ), diff --git a/engine/common/netchan.h b/engine/common/netchan.h index 88c8ccd8..1c55fd52 100644 --- a/engine/common/netchan.h +++ b/engine/common/netchan.h @@ -167,10 +167,11 @@ typedef enum fragsize_e typedef enum netchan_flags_e { - NETCHAN_USE_MUNGE = BIT( 0 ), - NETCHAN_USE_BZIP2 = BIT( 1 ), - NETCHAN_GOLDSRC = BIT( 2 ), - NETCHAN_USE_LZSS = BIT( 3 ), // mutually exclusive with bzip2 + NETCHAN_USE_MUNGE = BIT( 0 ), + NETCHAN_USE_BZIP2 = BIT( 1 ), + NETCHAN_GOLDSRC = BIT( 2 ), + NETCHAN_USE_LZSS = BIT( 3 ), // mutually exclusive with bzip2 + NETCHAN_USE_COOKIE = BIT( 4 ), // per-connection 64-bit cookie prefixed to every sequenced packet (NET_EXT_NETCHAN_COOKIE) } netchan_flags_t; // Network Connection Channel @@ -240,6 +241,8 @@ typedef struct netchan_s qboolean use_bz2; qboolean use_lzss; qboolean gs_netchan; + qboolean use_cookie; + uint64_t cookie; } netchan_t; extern netadr_t net_from; @@ -252,6 +255,7 @@ extern int net_drop; void Netchan_Init( void ); void Netchan_Shutdown( void ); void Netchan_Setup( netsrc_t sock, netchan_t *chan, netadr_t adr, int qport, void *client, int (*pfnBlockSize)(void *, fragsize_t mode ), uint flags ); +void Netchan_SetCookie( netchan_t *chan, uint64_t cookie ); void Netchan_CreateFileFragmentsFromBuffer( netchan_t *chan, const char *filename, byte *pbuf, int size ); qboolean Netchan_CopyNormalFragments( netchan_t *chan, sizebuf_t *msg, size_t *length ); qboolean Netchan_CopyFileFragments( netchan_t *chan, sizebuf_t *msg ); diff --git a/engine/common/protocol.h b/engine/common/protocol.h index f93492ca..f5931eb7 100644 --- a/engine/common/protocol.h +++ b/engine/common/protocol.h @@ -283,7 +283,8 @@ extern const char *const svc_quake_strings[svc_lastmsg+1]; extern const char *const svc_goldsrc_strings[svc_lastmsg+1]; // FWGS extensions -#define NET_EXT_SPLITSIZE (1U<<0) // set splitsize by cl_dlmax +#define NET_EXT_SPLITSIZE (1U<<0) // set splitsize by cl_dlmax +#define NET_EXT_NETCHAN_COOKIE (1U<<1) // per-connection 64-bit netchan cookie validated on every sequenced packet // GoldSrc protocol definitions #define PROTOCOL_GOLDSRC_VERSION 48 diff --git a/engine/server/sv_client.c b/engine/server/sv_client.c index 8e6243d5..f9e5cacc 100644 --- a/engine/server/sv_client.c +++ b/engine/server/sv_client.c @@ -13,6 +13,7 @@ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. */ +#include #include "common.h" #include "const.h" #include "server.h" @@ -301,6 +302,7 @@ static void SV_ConnectClient( netadr_t from ) const char *s; int extensions; uint netchan_flags = 0; + uint64_t netchan_cookie = 0; if( Cmd_Argc() < 5 ) { @@ -345,6 +347,27 @@ static void SV_ConnectClient( netadr_t from ) qport = Q_atoi( Info_ValueForKey( protinfo, "qport" )); extensions = Q_atoi( Info_ValueForKey( protinfo, "ext" )); + if( FBitSet( extensions, NET_EXT_NETCHAN_COOKIE )) + { + const char *cookie_str = Info_ValueForKey( protinfo, "cookie" ); + + if( Q_strlen( cookie_str ) != 16 ) + { + SV_RejectConnection( from, "client advertised NET_EXT_NETCHAN_COOKIE but did not supply a cookie\n" ); + return; + } + + byte buf[8]; + COM_HexConvert( cookie_str, 16, buf ); + for( int i = 0; i < 8; i++ ) + netchan_cookie = ( netchan_cookie << 8 ) | buf[i]; + } + + // these keys aren't useragent + Info_RemoveKey( protinfo, "cookie" ); + Info_RemoveKey( protinfo, "ext" ); + Info_RemoveKey( protinfo, "qport" ); + s = Cmd_Argv( 4 ); // user info if( Q_strlen( s ) > sizeof( userinfo ) || !Info_IsValid( s )) @@ -419,7 +442,7 @@ static void SV_ConnectClient( netadr_t from ) newcl->frames = frames; newcl->userid = g_userid++; // create unique userid newcl->state = cs_connected; // now expect "spawn" command - newcl->extensions = FBitSet( extensions, NET_EXT_SPLITSIZE ); + newcl->extensions = FBitSet( extensions, NET_EXT_SPLITSIZE | NET_EXT_NETCHAN_COOKIE ); Q_strncpy( newcl->useragent, protinfo, sizeof( newcl->useragent )); // HACKHACK: can hear all players by default to avoid issues @@ -429,7 +452,11 @@ static void SV_ConnectClient( netadr_t from ) // initailize netchan if( !Host_IsLocalClient( )) SetBits( netchan_flags, NETCHAN_USE_LZSS ); + if( FBitSet( newcl->extensions, NET_EXT_NETCHAN_COOKIE )) + SetBits( netchan_flags, NETCHAN_USE_COOKIE ); Netchan_Setup( NS_SERVER, &newcl->netchan, from, qport, newcl, SV_GetFragmentSize, netchan_flags ); + if( FBitSet( newcl->extensions, NET_EXT_NETCHAN_COOKIE )) + Netchan_SetCookie( &newcl->netchan, netchan_cookie ); MSG_Init( &newcl->datagram, "Datagram", newcl->datagram_buf, sizeof( newcl->datagram_buf )); // datagram buf Q_strncpy( newcl->hashedcdkey, Info_ValueForKey( protinfo, "uuid" ), 32 ); @@ -439,6 +466,8 @@ static void SV_ConnectClient( netadr_t from ) protinfo[0] = '\0'; Info_SetValueForKeyf( protinfo, "ext", sizeof( protinfo ), "%d", newcl->extensions ); Info_SetValueForKey( protinfo, "cheats", sv_cheats.value ? "1" : "0", sizeof( protinfo )); + if( FBitSet( newcl->extensions, NET_EXT_NETCHAN_COOKIE )) + Info_SetValueForKeyf( protinfo, "cookie", sizeof( protinfo ), "%016"PRIx64, netchan_cookie ); // send the connect packet to the client Netchan_OutOfBandPrint( NS_SERVER, from, S2C_CONNECTION" %s", protinfo ); diff --git a/engine/server/sv_main.c b/engine/server/sv_main.c index cb556b0a..36930c4c 100644 --- a/engine/server/sv_main.c +++ b/engine/server/sv_main.c @@ -377,7 +377,7 @@ SV_ReadPackets static void SV_ReadPackets( void ) { sv_client_t *cl; - int i, qport; + int i; size_t curSize; while( NET_GetPacket( NS_SERVER, &net_from, net_message_buffer, &curSize )) @@ -391,13 +391,6 @@ static void SV_ReadPackets( void ) continue; } - // read the qport out of the message so we can fix up - // stupid address translating routers - MSG_Clear( &net_message ); - MSG_ReadLong( &net_message ); // sequence number - MSG_ReadLong( &net_message ); // sequence number - qport = (int)MSG_ReadShort( &net_message ) & 0xffff; - // check for packets from connected clients for( i = 0, sv.current_client = svs.clients; i < svs.maxclients; i++, sv.current_client++ ) { @@ -409,24 +402,22 @@ static void SV_ReadPackets( void ) if( !NET_CompareBaseAdr( net_from, cl->netchan.remote_address )) continue; - if( cl->netchan.qport != qport ) + if( !Netchan_Process( &cl->netchan, &net_message )) continue; + // authenticated; safe to adopt the (possibly NAT-rewritten) source port if( cl->netchan.remote_address.port != net_from.port ) cl->netchan.remote_address.port = net_from.port; - if( Netchan_Process( &cl->netchan, &net_message )) - { - if(( svs.maxclients == 1 && !host_limitlocal.value ) || ( cl->state != cs_spawned )) - SetBits( cl->flags, FCL_SEND_NET_MESSAGE ); // reply at end of frame + if(( svs.maxclients == 1 && !host_limitlocal.value ) || ( cl->state != cs_spawned )) + SetBits( cl->flags, FCL_SEND_NET_MESSAGE ); // reply at end of frame - // this is a valid, sequenced packet, so process it - if( cl->frames != NULL && cl->state != cs_zombie ) - { - SV_ExecuteClientMessage( cl, &net_message ); - svgame.globals->frametime = sv.frametime; - svgame.globals->time = sv.time; - } + // this is a valid, sequenced packet, so process it + if( cl->frames != NULL && cl->state != cs_zombie ) + { + SV_ExecuteClientMessage( cl, &net_message ); + svgame.globals->frametime = sv.frametime; + svgame.globals->time = sv.time; } // fragmentation/reassembly sending takes priority over all game messages, want this in the future?