From 24374297e7548e8f957c0eb4e470eecd6cc51eba Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Mon, 10 Apr 2023 13:53:15 +0100 Subject: [PATCH] NetworkSession: extract rate limiting functionality into its own unit, and apply a separate rate limit to game packets --- src/network/mcpe/NetworkSession.php | 48 ++++------------ src/network/mcpe/PacketRateLimiter.php | 80 ++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 37 deletions(-) create mode 100644 src/network/mcpe/PacketRateLimiter.php diff --git a/src/network/mcpe/NetworkSession.php b/src/network/mcpe/NetworkSession.php index 92b916326..a59d0ccc8 100644 --- a/src/network/mcpe/NetworkSession.php +++ b/src/network/mcpe/NetworkSession.php @@ -128,19 +128,14 @@ use function ucfirst; use const JSON_THROW_ON_ERROR; class NetworkSession{ - private const INCOMING_PACKET_BATCH_PER_TICK = 2; //usually max 1 per tick, but transactions may arrive separately - private const INCOMING_PACKET_BATCH_MAX_BUDGET = 100 * self::INCOMING_PACKET_BATCH_PER_TICK; //enough to account for a 5-second lag spike + private const INCOMING_PACKET_BATCH_PER_TICK = 2; //usually max 1 per tick, but transactions arrive separately + private const INCOMING_PACKET_BATCH_BUFFER_TICKS = 100; //enough to account for a 5-second lag spike - /** - * At most this many more packets can be received. If this reaches zero, any additional packets received will cause - * the player to be kicked from the server. - * This number is increased every tick up to a maximum limit. - * - * @see self::INCOMING_PACKET_BATCH_PER_TICK - * @see self::INCOMING_PACKET_BATCH_MAX_BUDGET - */ - private int $incomingPacketBatchBudget = self::INCOMING_PACKET_BATCH_MAX_BUDGET; - private int $lastPacketBudgetUpdateTimeNs; + private const INCOMING_GAME_PACKETS_PER_TICK = 2; + private const INCOMING_GAME_PACKETS_BUFFER_TICKS = 100; + + private PacketRateLimiter $packetBatchLimiter; + private PacketRateLimiter $gamePacketLimiter; private \PrefixedLogger $logger; private ?Player $player = null; @@ -196,7 +191,8 @@ class NetworkSession{ $this->disposeHooks = new ObjectSet(); $this->connectTime = time(); - $this->lastPacketBudgetUpdateTimeNs = hrtime(true); + $this->packetBatchLimiter = new PacketRateLimiter("Packet Batches", self::INCOMING_PACKET_BATCH_PER_TICK, self::INCOMING_PACKET_BATCH_BUFFER_TICKS); + $this->gamePacketLimiter = new PacketRateLimiter("Game Packets", self::INCOMING_GAME_PACKETS_PER_TICK, self::INCOMING_GAME_PACKETS_BUFFER_TICKS); $this->setHandler(new SessionStartPacketHandler( $this->server, @@ -339,13 +335,7 @@ class NetworkSession{ Timings::$playerNetworkReceive->startTiming(); try{ - if($this->incomingPacketBatchBudget <= 0){ - $this->updatePacketBudget(); - if($this->incomingPacketBatchBudget <= 0){ - throw new PacketHandlingException("Receiving packets too fast"); - } - } - $this->incomingPacketBatchBudget--; + $this->packetBatchLimiter->decrement(); if($this->cipher !== null){ Timings::$playerNetworkReceiveDecrypt->startTiming(); @@ -377,6 +367,7 @@ class NetworkSession{ $stream = new BinaryStream($decompressed); $count = 0; foreach(PacketBatch::decodeRaw($stream) as $buffer){ + $this->gamePacketLimiter->decrement(); if(++$count > 100){ throw new PacketHandlingException("Too many packets in batch"); } @@ -1105,23 +1096,6 @@ class NetworkSession{ $this->sendDataPacket(ToastRequestPacket::create($title, $body)); } - private function updatePacketBudget() : void{ - $nowNs = hrtime(true); - $timeSinceLastUpdateNs = $nowNs - $this->lastPacketBudgetUpdateTimeNs; - if($timeSinceLastUpdateNs > 50_000_000){ - $ticksSinceLastUpdate = intdiv($timeSinceLastUpdateNs, 50_000_000); - /* - * If the server takes an abnormally long time to process a tick, add the budget for time difference to - * compensate. This extra budget may be very large, but it will disappear the next time a normal update - * occurs. This ensures that backlogs during a large lag spike don't cause everyone to get kicked. - * As long as all the backlogged packets are processed before the next tick, everything should be OK for - * clients behaving normally. - */ - $this->incomingPacketBatchBudget = min($this->incomingPacketBatchBudget, self::INCOMING_PACKET_BATCH_MAX_BUDGET) + (self::INCOMING_PACKET_BATCH_PER_TICK * 2 * $ticksSinceLastUpdate); - $this->lastPacketBudgetUpdateTimeNs = $nowNs; - } - } - public function tick() : void{ if(!$this->isConnected()){ $this->dispose(); diff --git a/src/network/mcpe/PacketRateLimiter.php b/src/network/mcpe/PacketRateLimiter.php new file mode 100644 index 000000000..3a9dffa01 --- /dev/null +++ b/src/network/mcpe/PacketRateLimiter.php @@ -0,0 +1,80 @@ +maxBudget = $this->averagePerTick * $maxBufferTicks; + $this->budget = $this->maxBudget; + $this->lastUpdateTimeNs = hrtime(true); + } + + /** + * @throws PacketHandlingException if the rate limit has been exceeded + */ + public function decrement(int $amount = 1) : void{ + if($this->budget <= 0){ + $this->update(); + if($this->budget <= 0){ + throw new PacketHandlingException("Exceeded rate limit for \"$this->name\""); + } + } + $this->budget -= $amount; + } + + public function update() : void{ + $nowNs = hrtime(true); + $timeSinceLastUpdateNs = $nowNs - $this->lastUpdateTimeNs; + if($timeSinceLastUpdateNs > $this->updateFrequencyNs){ + $ticksSinceLastUpdate = intdiv($timeSinceLastUpdateNs, $this->updateFrequencyNs); + /* + * If the server takes an abnormally long time to process a tick, add the budget for time difference to + * compensate. This extra budget may be very large, but it will disappear the next time a normal update + * occurs. This ensures that backlogs during a large lag spike don't cause everyone to get kicked. + * As long as all the backlogged packets are processed before the next tick, everything should be OK for + * clients behaving normally. + */ + $this->budget = min($this->budget, $this->maxBudget) + ($this->averagePerTick * 2 * $ticksSinceLastUpdate); + $this->lastUpdateTimeNs = $nowNs; + } + } +} \ No newline at end of file