From abbb8ed48f709adf51d6998f913915a3aa787277 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Mon, 22 Apr 2024 14:44:31 +0100 Subject: [PATCH] Forcibly disconnect sessions on split packet errors since split packets are usually reliable, we're breaking the reliability promise if we don't fully process these packets. worse, if the packet in question is reliable-ordered (always the case with Minecraft), no further packets will be processed anyway, causing the session to break down. --- src/generic/DisconnectReason.php | 8 ++++++ src/generic/PacketHandlingException.php | 38 +++++++++++++++++++++++++ src/generic/ReceiveReliabilityLayer.php | 29 ++++++++++++------- src/generic/Session.php | 7 +++++ src/server/Server.php | 8 +++++- 5 files changed, 79 insertions(+), 11 deletions(-) create mode 100644 src/generic/PacketHandlingException.php diff --git a/src/generic/DisconnectReason.php b/src/generic/DisconnectReason.php index d03c3b5..2b82fd8 100644 --- a/src/generic/DisconnectReason.php +++ b/src/generic/DisconnectReason.php @@ -22,6 +22,10 @@ final class DisconnectReason{ public const PEER_TIMEOUT = 2; public const CLIENT_RECONNECT = 3; public const SERVER_SHUTDOWN = 4; //TODO: do we really need a separate reason for this in addition to SERVER_DISCONNECT? + public const SPLIT_PACKET_TOO_LARGE = 5; + public const SPLIT_PACKET_TOO_MANY_CONCURRENT = 6; + public const SPLIT_PACKET_INVALID_PART_INDEX = 7; + public const SPLIT_PACKET_INCONSISTENT_HEADER = 8; public static function toString(int $reason) : string{ return match($reason){ @@ -30,6 +34,10 @@ public static function toString(int $reason) : string{ self::PEER_TIMEOUT => "timeout", self::CLIENT_RECONNECT => "new session established on same address and port", self::SERVER_SHUTDOWN => "server shutdown", + self::SPLIT_PACKET_TOO_LARGE => "received packet split into more parts than allowed", + self::SPLIT_PACKET_TOO_MANY_CONCURRENT => "too many received split packets being reassembled at once", + self::SPLIT_PACKET_INVALID_PART_INDEX => "invalid split packet part index", + self::SPLIT_PACKET_INCONSISTENT_HEADER => "received split packet header inconsistent with previous fragments", default => "Unknown reason $reason" }; } diff --git a/src/generic/PacketHandlingException.php b/src/generic/PacketHandlingException.php new file mode 100644 index 0000000..86d3f3c --- /dev/null +++ b/src/generic/PacketHandlingException.php @@ -0,0 +1,38 @@ + + * + * RakLib is not affiliated with Jenkins Software LLC nor RakNet. + * + * RakLib is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + */ + +declare(strict_types=1); + +namespace raklib\generic; + +class PacketHandlingException extends \RuntimeException{ + + /** @phpstan-var DisconnectReason::* */ + private int $disconnectReason; + + /** + * @phpstan-param DisconnectReason::* $disconnectReason + */ + public function __construct(string $message, int $disconnectReason, int $code = 0, ?\Throwable $previous = null){ + $this->disconnectReason = $disconnectReason; + parent::__construct($message, $code, $previous); + } + + /** + * @phpstan-return DisconnectReason::* + */ + public function getDisconnectReason() : int{ + return $this->disconnectReason; + } +} diff --git a/src/generic/ReceiveReliabilityLayer.php b/src/generic/ReceiveReliabilityLayer.php index 9105eb6..b7bcb22 100644 --- a/src/generic/ReceiveReliabilityLayer.php +++ b/src/generic/ReceiveReliabilityLayer.php @@ -84,8 +84,14 @@ private function handleEncapsulatedPacketRoute(EncapsulatedPacket $pk) : void{ /** * Processes a split part of an encapsulated packet. + * If an error occurs (limit exceeded, inconsistent header, etc.) a PacketHandlingException is thrown. + * + * An error processing a split packet means we can't fulfill reliability promises, which at best would lead to some + * packets not arriving, and in the worst case (reliable-ordered) cause no more packets to be processed. + * Therefore, the owning session MUST disconnect the peer if this exception is thrown. * * @return null|EncapsulatedPacket Reassembled packet if we have all the parts, null otherwise. + * @throws PacketHandlingException if there was a problem with processing the split packet. */ private function handleSplit(EncapsulatedPacket $packet) : ?EncapsulatedPacket{ if($packet->splitInfo === null){ @@ -93,24 +99,21 @@ private function handleSplit(EncapsulatedPacket $packet) : ?EncapsulatedPacket{ } $totalParts = $packet->splitInfo->getTotalPartCount(); $partIndex = $packet->splitInfo->getPartIndex(); - if( - $totalParts >= $this->maxSplitPacketPartCount or $totalParts < 0 or - $partIndex >= $totalParts or $partIndex < 0 - ){ - $this->logger->debug("Invalid split packet part, too many parts or invalid split index (part index $partIndex, part count $totalParts)"); - return null; + if($totalParts >= $this->maxSplitPacketPartCount || $totalParts < 0){ + throw new PacketHandlingException("Invalid split packet part count ($totalParts)", DisconnectReason::SPLIT_PACKET_TOO_LARGE); + } + if($partIndex >= $totalParts || $partIndex < 0){ + throw new PacketHandlingException("Invalid split packet part index (part index $partIndex, part count $totalParts)", DisconnectReason::SPLIT_PACKET_INVALID_PART_INDEX); } $splitId = $packet->splitInfo->getId(); if(!isset($this->splitPackets[$splitId])){ if(count($this->splitPackets) >= $this->maxConcurrentSplitPackets){ - $this->logger->debug("Ignored split packet part because reached concurrent split packet limit of $this->maxConcurrentSplitPackets"); - return null; + throw new PacketHandlingException("Exceeded concurrent split packet reassembly limit of $this->maxConcurrentSplitPackets", DisconnectReason::SPLIT_PACKET_TOO_MANY_CONCURRENT); } $this->splitPackets[$splitId] = array_fill(0, $totalParts, null); }elseif(count($this->splitPackets[$splitId]) !== $totalParts){ - $this->logger->debug("Wrong split count $totalParts for split packet $splitId, expected " . count($this->splitPackets[$splitId])); - return null; + throw new PacketHandlingException("Wrong split count $totalParts for split packet $splitId, expected " . count($this->splitPackets[$splitId]), DisconnectReason::SPLIT_PACKET_INCONSISTENT_HEADER); } $this->splitPackets[$splitId][$partIndex] = $packet; @@ -142,6 +145,9 @@ private function handleSplit(EncapsulatedPacket $packet) : ?EncapsulatedPacket{ return $pk; } + /** + * @throws PacketHandlingException + */ private function handleEncapsulatedPacket(EncapsulatedPacket $packet) : void{ if($packet->messageIndex !== null){ //check for duplicates or out of range @@ -209,6 +215,9 @@ private function handleEncapsulatedPacket(EncapsulatedPacket $packet) : void{ } } + /** + * @throws PacketHandlingException + */ public function onDatagram(Datagram $packet) : void{ if($packet->seqNumber < $this->windowStart or $packet->seqNumber > $this->windowEnd or isset($this->ACKQueue[$packet->seqNumber])){ $this->logger->debug("Received duplicate or out-of-window packet (sequence number $packet->seqNumber, window " . $this->windowStart . "-" . $this->windowEnd . ")"); diff --git a/src/generic/Session.php b/src/generic/Session.php index 43ad7d0..2db1c65 100644 --- a/src/generic/Session.php +++ b/src/generic/Session.php @@ -155,6 +155,10 @@ protected function getRakNetTimeMS() : int{ return intdiv(hrtime(true), 1_000_000); } + public function getLogger() : \Logger{ + return $this->logger; + } + public function getAddress() : InternetAddress{ return $this->address; } @@ -275,6 +279,9 @@ private function handlePong(int $sendPingTime, int $sendPongTime) : void{ } } + /** + * @throws PacketHandlingException + */ public function handlePacket(Packet $packet) : void{ $this->isActive = true; $this->lastUpdate = microtime(true); diff --git a/src/server/Server.php b/src/server/Server.php index 7629e23..359ca1d 100644 --- a/src/server/Server.php +++ b/src/server/Server.php @@ -20,6 +20,7 @@ use raklib\generic\DisconnectReason; use raklib\generic\Session; use raklib\generic\SocketException; +use raklib\generic\PacketHandlingException; use raklib\protocol\ACK; use raklib\protocol\Datagram; use raklib\protocol\EncapsulatedPacket; @@ -247,7 +248,12 @@ private function receivePacket() : bool{ $packet = new Datagram(); } $packet->decode(new PacketSerializer($buffer)); - $session->handlePacket($packet); + try{ + $session->handlePacket($packet); + }catch(PacketHandlingException $e){ + $session->getLogger()->error("Error receiving packet: " . $e->getMessage()); + $session->forciblyDisconnect($e->getDisconnectReason()); + } return true; }elseif($session->isConnected()){ //allows unconnected packets if the session is stuck in DISCONNECTING state, useful if the client