diff --git a/include/steam/steamnetworkingtypes.h b/include/steam/steamnetworkingtypes.h index aff2112..0ceb5b7 100644 --- a/include/steam/steamnetworkingtypes.h +++ b/include/steam/steamnetworkingtypes.h @@ -1135,6 +1135,28 @@ enum ESteamNetworkingConfigValue /// Default is 512k (524288 bytes) k_ESteamNetworkingConfig_SendBufferSize = 9, + /// [connection int32] Upper limit on total size (in bytes) of received messages + /// that will be buffered waiting to be processed by the application. If this limit + /// is exceeded, packets will be dropped. This is to protect us from a malicious + /// peer flooding us with messages faster than we can process them. + /// + /// This must be bigger than k_ESteamNetworkingConfig_RecvMaxMessageSize + k_ESteamNetworkingConfig_RecvBufferSize = 47, + + /// [connection int32] Upper limit on the number of received messages that will + /// that will be buffered waiting to be processed by the application. If this limit + /// is exceeded, packets will be dropped. This is to protect us from a malicious + /// peer flooding us with messages faster than we can pull them off the wire. + k_ESteamNetworkingConfig_RecvBufferMessages = 48, + + /// [connection int32] Maximum message size that we are willing to receive. + /// if a client attempts to send us a message larger than this, the connection + /// will be immediately closed. + /// + /// Default is 512k (524288 bytes). Note that the peer needs to be able to + /// send a message this big. (See k_cbMaxSteamNetworkingSocketsMessageSizeSend.) + k_ESteamNetworkingConfig_RecvMaxMessageSize = 49, + /// [connection int64] Get/set userdata as a configuration option. /// The default value is -1. You may want to set the user data as /// a config value, instead of using ISteamNetworkingSockets::SetConnectionUserData diff --git a/src/steamnetworkingsockets/clientlib/csteamnetworkingsockets.cpp b/src/steamnetworkingsockets/clientlib/csteamnetworkingsockets.cpp index 372b73f..cc99004 100644 --- a/src/steamnetworkingsockets/clientlib/csteamnetworkingsockets.cpp +++ b/src/steamnetworkingsockets/clientlib/csteamnetworkingsockets.cpp @@ -68,7 +68,10 @@ DEFINE_GLOBAL_CONFIGVAL( void *, Callback_FakeIPResult, nullptr ); DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int32, TimeoutInitial, 10000, 0, INT32_MAX ); DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int32, TimeoutConnected, 10000, 0, INT32_MAX ); -DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int32, SendBufferSize, 512*1024, 0, 0x10000000 ); +DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int32, SendBufferSize, 512*1024, 4*1024, 0x10000000 ); +DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int32, RecvBufferSize, 1024*1024, 4*1024, 0x10000000 ); +DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int32, RecvBufferMessages, 1000, 2, 0x10000000 ); +DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int32, RecvMaxMessageSize, 512*1024, 64, 0x10000000 ); DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int64, ConnectionUserData, -1 ); // no limits here DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int32, SendRateMin, 128*1024, 1024, 0x10000000 ); DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int32, SendRateMax, 1024*1024, 1024, 0x10000000 ); diff --git a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.cpp b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.cpp index 75dc4e4..e37ae3b 100644 --- a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.cpp +++ b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.cpp @@ -170,7 +170,6 @@ void SteamNetworkingMessageQueue::AssertLockHeld() const void SteamNetworkingMessageQueue::PurgeMessages() { - AssertLockHeld(); while ( !empty() ) { @@ -179,6 +178,7 @@ void SteamNetworkingMessageQueue::PurgeMessages() Assert( m_pFirst != pMsg ); pMsg->Release(); } + Assert( m_nMessageCount == 0 ); } int SteamNetworkingMessageQueue::RemoveMessages( SteamNetworkingMessage_t **ppOutMessages, int nMaxMessages ) @@ -2104,6 +2104,7 @@ int CSteamNetworkConnectionBase::APIReceiveMessages( SteamNetworkingMessage_t ** m_pLock->AssertHeldByCurrentThread(); g_lockAllRecvMessageQueues.lock(); + int result = m_queueRecvMessages.RemoveMessages( ppOutMessages, nMaxMessages ); g_lockAllRecvMessageQueues.unlock(); @@ -2606,7 +2607,27 @@ void CSteamNetworkConnectionBase::ConnectionStateChanged( ESteamNetworkingConnec CSteamNetworkingMessage *CSteamNetworkConnectionBase::AllocateNewRecvMessage( uint32 cbSize, int nFlags, SteamNetworkingMicroseconds usecNow ) { + // + // Check limits + // + + // Max message size + if ( (uint32)m_connectionConfig.m_RecvMaxMessageSize.Get() < cbSize ) + { + SpewMsg( "[%s] recv message of size %u too large for limit of %d.\n", GetDescription(), cbSize, m_connectionConfig.m_RecvMaxMessageSize.Get() ); + ConnectionState_ProblemDetectedLocally( k_ESteamNetConnectionEnd_Misc_InternalError, "Failed to allocate a buffer of size %u (limit is %d).", cbSize, m_connectionConfig.m_RecvMaxMessageSize.Get() ); + return nullptr; + } + + // Max number of messages buffered + if ( m_queueRecvMessages.m_nMessageCount >= m_connectionConfig.m_RecvBufferMessages.Get() ) + { + SpewWarningRateLimited( usecNow, "[%s] recv queue overflow %d messages already queued.\n", GetDescription(), m_queueRecvMessages.m_nMessageCount ); + return nullptr; + } + CSteamNetworkingMessage *pMsg = CSteamNetworkingMessage::New( cbSize ); + if ( !pMsg ) { // Failed! if it's for a reliable message, then we must abort the connection. @@ -2629,8 +2650,7 @@ bool CSteamNetworkConnectionBase::ReceivedMessageData( const void *pData, int cb if ( !pMsg ) { // Hm. this failure really is probably a sign that we are in a pretty bad state, - // and we are unlikely to recover. Should we just abort the connection? - // Right now, we'll try to muddle on. + // and we are unlikely to recover. Let the caller know about failure (this drops the connection). return false; } @@ -2642,12 +2662,10 @@ bool CSteamNetworkConnectionBase::ReceivedMessageData( const void *pData, int cb memcpy( pMsg->m_pData, pData, cbData ); // Receive it - ReceivedMessage( pMsg ); - - return true; + return ReceivedMessage( pMsg ); } -void CSteamNetworkConnectionBase::ReceivedMessage( CSteamNetworkingMessage *pMsg ) +bool CSteamNetworkConnectionBase::ReceivedMessage( CSteamNetworkingMessage *pMsg ) { m_pLock->AssertHeldByCurrentThread(); @@ -2661,7 +2679,7 @@ void CSteamNetworkConnectionBase::ReceivedMessage( CSteamNetworkingMessage *pMsg if ( m_pMessagesEndPointSessionOwner ) { m_pMessagesEndPointSessionOwner->ReceivedMessage( pMsg, this ); - return; + return true; } #endif @@ -2674,14 +2692,30 @@ void CSteamNetworkConnectionBase::ReceivedMessage( CSteamNetworkingMessage *pMsg // which keeps this really simple. g_lockAllRecvMessageQueues.lock(); + Assert( pMsg->m_cbSize >= 0 ); + if ( m_queueRecvMessages.m_nMessageCount >= m_connectionConfig.m_RecvBufferMessages.Get() ) + { + g_lockAllRecvMessageQueues.unlock(); + SpewWarningRateLimited ( SteamNetworkingSockets_GetLocalTimestamp(), "[%s] recv queue overflow %d messages already queued.\n", GetDescription(), m_queueRecvMessages.m_nMessageCount ); + pMsg->Release(); + return false; + } + + if ( m_queueRecvMessages.m_nMessageSize + pMsg->m_cbSize > m_connectionConfig.m_RecvBufferSize.Get() ) + { + g_lockAllRecvMessageQueues.unlock(); + SpewWarningRateLimited ( SteamNetworkingSockets_GetLocalTimestamp(), "[%s] recv queue overflow %d + %d bytes exceeds limit of %d.\n", GetDescription(), m_queueRecvMessages.m_nMessageSize, pMsg->m_cbSize, m_connectionConfig.m_RecvBufferSize.Get() ); + pMsg->Release(); + return false; + } + // Add to end of my queue. pMsg->LinkToQueueTail( &CSteamNetworkingMessage::m_links, &m_queueRecvMessages ); - // Add to the poll group, if we are in one if ( m_pPollGroup ) pMsg->LinkToQueueTail( &CSteamNetworkingMessage::m_linksSecondaryQueue, &m_pPollGroup->m_queueRecvMessages ); - g_lockAllRecvMessageQueues.unlock(); + return true; } void CSteamNetworkConnectionBase::PostConnectionStateChangedCallback( ESteamNetworkingConnectionState eOldAPIState, ESteamNetworkingConnectionState eNewAPIState ) @@ -3823,6 +3857,12 @@ CSteamNetworkConnectionPipe::CSteamNetworkConnectionPipe( CSteamNetworkingSocket m_connectionConfig.m_SendRateMin.Set( nRate ); m_connectionConfig.m_SendRateMax.Set( nRate ); + // Don't limit the recv buffer. (Send buffer doesn't + // matter since we immediately transfer.) + m_connectionConfig.m_RecvBufferSize.Set( 0x10000000 ); + m_connectionConfig.m_RecvBufferMessages.Set( 0x10000000 ); + m_connectionConfig.m_RecvMaxMessageSize.Set( 0x10000000 ); + // Diagnostics usually not useful on these types of connections. // (App can enable it or clear this override if it wants to.) #ifdef STEAMNETWORKINGSOCKETS_ENABLE_DIAGNOSTICSUI diff --git a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.h b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.h index ae0ba0c..96a2797 100644 --- a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.h +++ b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.h @@ -753,7 +753,7 @@ protected: /// Called when we receive a complete message. Should allocate a message object and put it into the proper queues bool ReceivedMessageData( const void *pData, int cbData, int idxLane, int64 nMsgNum, int nFlags, SteamNetworkingMicroseconds usecNow ); - void ReceivedMessage( CSteamNetworkingMessage *pMsg ); + bool ReceivedMessage( CSteamNetworkingMessage *pMsg ); CSteamNetworkingMessage *AllocateNewRecvMessage( uint32 cbSize, int nFlags, SteamNetworkingMicroseconds usecNow ); /// Timestamp when we last sent an end-to-end connection request packet diff --git a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_snp.cpp b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_snp.cpp index 27060c2..710fa18 100644 --- a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_snp.cpp +++ b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_snp.cpp @@ -3369,6 +3369,8 @@ bool CSteamNetworkConnectionBase::SNP_ReceiveReliableSegment( int64 nPktNum, int // What do we expect to receive next? const int64 nExpectNextStreamPos = lane.m_nReliableStreamPos + len( lane.m_bufReliableStream ); + const int32 nMaxRecvBufferSize = m_connectionConfig.m_RecvBufferSize.Get(); + const int32 nMaxMessageSize = m_connectionConfig.m_RecvMaxMessageSize.Get(); // Check if we need to grow the reliable buffer to hold the data if ( nSegEnd > nExpectNextStreamPos ) @@ -3381,7 +3383,7 @@ bool CSteamNetworkConnectionBase::SNP_ReceiveReliableSegment( int64 nPktNum, int // against a malicious sender trying to create big gaps. If they // are legit, they will notice that we go back and fill in the gaps // and we will get caught up. - if ( cbNewSize > k_cbMaxBufferedReceiveReliableData ) + if ( cbNewSize > nMaxRecvBufferSize ) { // Stop processing the packet, and don't ack it. // This indicates the connection is in pretty bad shape, @@ -3650,7 +3652,7 @@ bool CSteamNetworkConnectionBase::SNP_ReceiveReliableSegment( int64 nPktNum, int // Sanity check size. Note that we do this check before we shift, // to protect against overflow. // (Although DeserializeVarInt doesn't detect overflow...) - if ( nMsgSizeUpperBits > (uint64)k_cbMaxMessageSizeRecv<<5 ) + if ( nMsgSizeUpperBits > (((uint64)nMaxRecvBufferSize)<<5) ) { ConnectionState_ProblemDetectedLocally( k_ESteamNetConnectionEnd_Misc_InternalError, "Reliable message size too large. (%llu<<5 + %d)", @@ -3660,7 +3662,14 @@ bool CSteamNetworkConnectionBase::SNP_ReceiveReliableSegment( int64 nPktNum, int // Compute total size, and check it again cbMsgSize += int( nMsgSizeUpperBits<<5 ); - if ( cbMsgSize > k_cbMaxMessageSizeRecv ) + if ( cbMsgSize > nMaxRecvBufferSize ) + { + ConnectionState_ProblemDetectedLocally( k_ESteamNetConnectionEnd_Misc_InternalError, + "Reliable message size %d too large.", cbMsgSize ); + return false; + } + + if ( cbMsgSize > nMaxMessageSize ) { ConnectionState_ProblemDetectedLocally( k_ESteamNetConnectionEnd_Misc_InternalError, "Reliable message size %d too large.", cbMsgSize ); @@ -3677,7 +3686,10 @@ bool CSteamNetworkConnectionBase::SNP_ReceiveReliableSegment( int64 nPktNum, int // We have a full message! Queue it if ( !ReceivedMessageData( pReliableDecode, cbMsgSize, idxLane, nMsgNum, k_nSteamNetworkingSend_Reliable, usecNow ) ) - return false; // Weird failure. Most graceful response is to not ack this packet, and maybe we will work next on retry. + { + ConnectionState_ProblemDetectedLocally( k_ESteamNetConnectionEnd_Misc_InternalError, "Reliable message size %d too large for remaining space in message queue.", cbMsgSize ); + return false; + } pReliableDecode += cbMsgSize; int cbStreamConsumed = pReliableDecode-pReliableStart; diff --git a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_snp.h b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_snp.h index 22c53a2..e0a156a 100644 --- a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_snp.h +++ b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_snp.h @@ -33,13 +33,6 @@ constexpr SteamNetworkingMicroseconds k_usecAckDelayPrecision = (1 << k_nAckDela // balance between false positive and false negative rates. constexpr SteamNetworkingMicroseconds k_usecNackFlush = 3*1000; -// Max size of a message that we are wiling to *receive*. -constexpr int k_cbMaxMessageSizeRecv = k_cbMaxSteamNetworkingSocketsMessageSizeSend*2; - -// The max we will look ahead and allocate data, ahead of the reliable -// messages we have been able to decode. We limit this to make sure that -// a malicious sender cannot exploit us. -constexpr int k_cbMaxBufferedReceiveReliableData = k_cbMaxMessageSizeRecv + 64*1024; constexpr int k_nMaxReliableStreamGaps_Extend = 30; // Discard reliable data past the end of the stream, if it would cause us to get too many gaps constexpr int k_nMaxReliableStreamGaps_Fragment = 20; // Discard reliable data that is filling in the middle of a hole, if it would cause the number of gaps to exceed this number constexpr int k_nMaxPacketGaps = 62; // Don't bother tracking more than N gaps. Instead, we will end up NACKing some packets that we actually did receive. This should not break the protocol, but it protects us from malicious sender @@ -189,15 +182,21 @@ struct SteamNetworkingMessageQueue CSteamNetworkingMessage *m_pFirst = nullptr; CSteamNetworkingMessage *m_pLast = nullptr; LockDebugInfo *m_pRequiredLock = nullptr; // Is there a lock that is required to be held while we access this queue? + int m_nMessageCount = 0; + int m_nMessageSize = 0; + inline bool empty() const { if ( m_pFirst ) { Assert( m_pLast ); + Assert( m_nMessageCount > 0 ); return false; } Assert( !m_pLast ); + Assert( m_nMessageCount == 0 ); + Assert( m_nMessageSize == 0 ); return true; } @@ -615,7 +614,7 @@ struct SSNPReceiverState /// Queue a flush of ALL acks (and NACKs!) by the given time. /// If anything is scheduled to happen earlier, that schedule - /// will still be honered. We will ack up to that packet number, + /// will still be honored. We will ack up to that packet number, /// and then we we may report higher numbered blocks, or we may /// stop and wait to report more acks until later. void QueueFlushAllAcks( SteamNetworkingMicroseconds usecWhen ); @@ -667,6 +666,7 @@ inline void CSteamNetworkingMessage::LinkBefore( CSteamNetworkingMessage *pSucce Assert( pQueue->m_pFirst ); Assert( pQueue->m_pLast ); Assert( (pSuccessor->*pMbrLinks).m_pQueue == pQueue ); + Assert( pQueue->m_nMessageCount > 0 ); CSteamNetworkingMessage *pPrev = (pSuccessor->*pMbrLinks).m_pPrev; if ( pPrev ) @@ -686,6 +686,11 @@ inline void CSteamNetworkingMessage::LinkBefore( CSteamNetworkingMessage *pSucce } // Finish up + Assert( pQueue->m_nMessageCount > 0 ); + ++pQueue->m_nMessageCount; + Assert( m_cbSize >= 0 ); + pQueue->m_nMessageSize += m_cbSize; + (this->*pMbrLinks).m_pQueue = pQueue; (this->*pMbrLinks).m_pNext = pSuccessor; (pSuccessor->*pMbrLinks).m_pPrev = this; @@ -705,11 +710,13 @@ inline void CSteamNetworkingMessage::LinkToQueueTail( CSteamNetworkingMessage::L { Assert( pQueue->m_pFirst ); Assert( !(pQueue->m_pLast->*pMbrLinks).m_pNext ); + Assert( pQueue->m_nMessageCount > 0 ); (pQueue->m_pLast->*pMbrLinks).m_pNext = this; } else { Assert( !pQueue->m_pFirst ); + Assert( pQueue->m_nMessageCount == 0 ); pQueue->m_pFirst = this; } @@ -721,6 +728,9 @@ inline void CSteamNetworkingMessage::LinkToQueueTail( CSteamNetworkingMessage::L pQueue->m_pLast = this; // Remember what queue we're in + ++pQueue->m_nMessageCount; + Assert( m_cbSize >= 0 ); + pQueue->m_nMessageSize += m_cbSize; (this->*pMbrLinks).m_pQueue = pQueue; } @@ -737,6 +747,7 @@ inline void CSteamNetworkingMessage::UnlinkFromQueue( CSteamNetworkingMessage::L } SteamNetworkingMessageQueue &q = *links.m_pQueue; q.AssertLockHeld(); + Assert( q.m_nMessageCount > 0 ); // Unlink from previous if ( links.m_pPrev ) @@ -763,7 +774,10 @@ inline void CSteamNetworkingMessage::UnlinkFromQueue( CSteamNetworkingMessage::L Assert( q.m_pLast == this ); q.m_pLast = links.m_pPrev; } - + --q.m_nMessageCount; + Assert( m_cbSize >= 0 ); + Assert( q.m_nMessageSize >= m_cbSize ); + q.m_nMessageSize -= m_cbSize; // Clear links links.m_pQueue = nullptr; links.m_pPrev = nullptr; diff --git a/src/steamnetworkingsockets/steamnetworkingsockets_internal.h b/src/steamnetworkingsockets/steamnetworkingsockets_internal.h index 456826d..5a0f3e9 100644 --- a/src/steamnetworkingsockets/steamnetworkingsockets_internal.h +++ b/src/steamnetworkingsockets/steamnetworkingsockets_internal.h @@ -279,6 +279,10 @@ const int k_cbSteamNetworkingSocketsTypicalMaxPlaintextPayloadSend = k_cbSteamNe const int k_cbSteamNetworkingSocketsMaxEncryptedPayloadRecv = k_cbSteamNetworkingSocketsMaxUDPMsgLen; const int k_cbSteamNetworkingSocketsMaxPlaintextPayloadRecv = k_cbSteamNetworkingSocketsMaxUDPMsgLen; +/// Max value that RecvMaxMessageSize can be set to. +const int k_cbMaxMessageSizeRecv_Limit = k_cbMaxSteamNetworkingSocketsMessageSizeSend*2; +COMPILE_TIME_ASSERT( k_cbMaxMessageSizeRecv_Limit >= k_cbMaxSteamNetworkingSocketsMessageSizeSend*2 ); + /// If we have a cert that is going to expire in m_TimeoutInitial; ConfigValue m_TimeoutConnected; ConfigValue m_SendBufferSize; + ConfigValue m_RecvBufferSize; + ConfigValue m_RecvBufferMessages; + ConfigValue m_RecvMaxMessageSize; ConfigValue m_SendRateMin; ConfigValue m_SendRateMax; ConfigValue m_MTU_PacketSize;