From 41789bc67aaf45f5295941aef952240d69af6269 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Tue, 11 Mar 2025 12:41:59 +0000 Subject: [PATCH 1/4] Report debug stats for cost accounting every 10 seconds --- composer.json | 2 +- composer.lock | 33 +++++++++----- src/network/mcpe/NetworkSession.php | 69 +++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 13 deletions(-) diff --git a/composer.json b/composer.json index 10454c56070..cca1fc5e67f 100644 --- a/composer.json +++ b/composer.json @@ -37,7 +37,7 @@ "pocketmine/bedrock-data": "~4.0.0+bedrock-1.21.60", "pocketmine/bedrock-item-upgrade-schema": "~1.14.0+bedrock-1.21.50", "pocketmine/bedrock-protocol": "~36.0.0+bedrock-1.21.60", - "pocketmine/binaryutils": "^0.2.1", + "pocketmine/binaryutils": "dev-experimental/read-ops-accounting as 0.2.6", "pocketmine/callback-validator": "^1.0.2", "pocketmine/color": "^0.3.0", "pocketmine/errorhandler": "^0.7.0", diff --git a/composer.lock b/composer.lock index e224887d66b..dcbeeb2c8b2 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "bef9decc40d9f5bd82e1de2d151bd99f", + "content-hash": "415cb668882a32039066467bb308421f", "packages": [ { "name": "adhocore/json-comment", @@ -302,16 +302,16 @@ }, { "name": "pocketmine/binaryutils", - "version": "0.2.6", + "version": "dev-experimental/read-ops-accounting", "source": { "type": "git", "url": "https://github.com/pmmp/BinaryUtils.git", - "reference": "ccfc1899b859d45814ea3592e20ebec4cb731c84" + "reference": "8cfa34c9d5aae11886a4142c172cff05f1e87ee2" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/pmmp/BinaryUtils/zipball/ccfc1899b859d45814ea3592e20ebec4cb731c84", - "reference": "ccfc1899b859d45814ea3592e20ebec4cb731c84", + "url": "https://api.github.com/repos/pmmp/BinaryUtils/zipball/8cfa34c9d5aae11886a4142c172cff05f1e87ee2", + "reference": "8cfa34c9d5aae11886a4142c172cff05f1e87ee2", "shasum": "" }, "require": { @@ -320,9 +320,9 @@ }, "require-dev": { "phpstan/extension-installer": "^1.0", - "phpstan/phpstan": "~1.10.3", - "phpstan/phpstan-phpunit": "^1.0", - "phpstan/phpstan-strict-rules": "^1.0.0", + "phpstan/phpstan": "2.1.0", + "phpstan/phpstan-phpunit": "^2.0", + "phpstan/phpstan-strict-rules": "^2.0.0", "phpunit/phpunit": "^9.5 || ^10.0 || ^11.0" }, "type": "library", @@ -338,9 +338,9 @@ "description": "Classes and methods for conveniently handling binary data", "support": { "issues": "https://github.com/pmmp/BinaryUtils/issues", - "source": "https://github.com/pmmp/BinaryUtils/tree/0.2.6" + "source": "https://github.com/pmmp/BinaryUtils/tree/experimental/read-ops-accounting" }, - "time": "2024-03-04T15:04:17+00:00" + "time": "2025-03-11T11:50:46+00:00" }, { "name": "pocketmine/callback-validator", @@ -2930,9 +2930,18 @@ "time": "2024-03-03T12:36:25+00:00" } ], - "aliases": [], + "aliases": [ + { + "package": "pocketmine/binaryutils", + "version": "dev-experimental/read-ops-accounting", + "alias": "0.2.6", + "alias_normalized": "0.2.6.0" + } + ], "minimum-stability": "stable", - "stability-flags": [], + "stability-flags": { + "pocketmine/binaryutils": 20 + }, "prefer-stable": false, "prefer-lowest": false, "platform": { diff --git a/src/network/mcpe/NetworkSession.php b/src/network/mcpe/NetworkSession.php index 8c457ed4054..c08550b8533 100644 --- a/src/network/mcpe/NetworkSession.php +++ b/src/network/mcpe/NetworkSession.php @@ -113,6 +113,7 @@ use pocketmine\utils\BinaryStream; use pocketmine\utils\ObjectSet; use pocketmine\utils\TextFormat; +use pocketmine\utils\Utils; use pocketmine\world\format\io\GlobalItemDataHandlers; use pocketmine\world\Position; use pocketmine\world\World; @@ -122,10 +123,12 @@ use function bin2hex; use function count; use function get_class; +use function hrtime; use function implode; use function in_array; use function is_string; use function json_encode; +use function microtime; use function ord; use function random_bytes; use function str_split; @@ -135,7 +138,9 @@ use function substr; use function time; use function ucfirst; +use const JSON_PRETTY_PRINT; use const JSON_THROW_ON_ERROR; +use const PHP_INT_MAX; class NetworkSession{ private const INCOMING_PACKET_BATCH_PER_TICK = 2; //usually max 1 per tick, but transactions arrive separately @@ -194,6 +199,29 @@ class NetworkSession{ */ private ObjectSet $disposeHooks; + /** + * @var int[] + * @phpstan-var array + */ + private array $packetCostsMin = []; + /** + * @var int[] + * @phpstan-var array + */ + private array $packetCostsMax = []; + /** + * @var int[] + * @phpstan-var array + */ + private array $packetCostsTotal = []; + /** + * @var int[] + * @phpstan-var array + */ + private array $packetCounts = []; + private int $decodeCostTotal = 0; + private int $sessionStartHrtime; + public function __construct( private Server $server, private NetworkSessionManager $manager, @@ -223,6 +251,8 @@ public function __construct( $this->manager->add($this); $this->logger->info($this->server->getLanguage()->translate(KnownTranslationFactory::pocketmine_network_session_open())); + + $this->sessionStartHrtime = hrtime(true); } private function getLogPrefix() : string{ @@ -450,7 +480,17 @@ public function handleDataPacket(Packet $packet, string $buffer) : void{ try{ $stream = PacketSerializer::decoder($buffer, 0); try{ + $stream->setReadOpsLimit(PHP_INT_MAX); //just for stats collection $packet->decode($stream); + + $readOps = $stream->getReadOps(); + + $this->decodeCostTotal += $readOps; + $key = $packet->getName(); + $this->packetCostsTotal[$key] = ($this->packetCostsTotal[$key] ?? 0) + $readOps; + $this->packetCostsMin[$key] = min($this->packetCostsMin[$key] ?? PHP_INT_MAX, $readOps); + $this->packetCostsMax[$key] = max($this->packetCostsMax[$key] ?? 0, $readOps); + $this->packetCounts[$key] = ($this->packetCounts[$key] ?? 0) + 1; }catch(PacketDecodeException $e){ throw PacketHandlingException::wrap($e); } @@ -483,6 +523,27 @@ public function handleDataPacket(Packet $packet, string $buffer) : void{ } } + /** + * @return int[]|float[] + * @phpstan-return array> + */ + public function dumpDecodeCostStats() : array{ + $sessionTime = ((hrtime(true) - $this->sessionStartHrtime) / 1_000_000_000); + $packetDecodeAverages = []; + $packetCostsPerSecondAverages = []; + foreach(Utils::stringifyKeys($this->packetCostsTotal) as $packet => $total){ + $packetDecodeAverages[$packet] = $total / $this->packetCounts[$packet]; + $packetCostsPerSecondAverages[$packet] = $total / $sessionTime; + } + return [ + "decodeCostAvgPerSecond" => $this->decodeCostTotal / $sessionTime, + "decodeCostAvgPerPacketPerSecond" => $packetCostsPerSecondAverages, + "decodeCostAvgPerPacket" => $packetDecodeAverages, + "decodeCostMinPerPacket" => $this->packetCostsMin, + "decodeCostMaxPerPacket" => $this->packetCostsMax + ]; + } + public function handleAckReceipt(int $receiptId) : void{ if(!$this->connected){ return; @@ -1346,5 +1407,13 @@ public function tick() : void{ } $this->flushSendBuffer(); + + $now = microtime(true); + if($now - $this->lastStatsReportTime > 10){ + $this->logger->debug("Decode cost stats: " . json_encode($this->dumpDecodeCostStats(), JSON_PRETTY_PRINT | JSON_THROW_ON_ERROR)); + $this->lastStatsReportTime = $now; + } } + + private float $lastStatsReportTime = 0; } From 8d255a65129791b7ce4626949f1eae9e99dbcf6e Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Tue, 11 Mar 2025 14:28:44 +0000 Subject: [PATCH 2/4] ... --- src/network/mcpe/NetworkSession.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/network/mcpe/NetworkSession.php b/src/network/mcpe/NetworkSession.php index c08550b8533..127bbe71f64 100644 --- a/src/network/mcpe/NetworkSession.php +++ b/src/network/mcpe/NetworkSession.php @@ -128,7 +128,9 @@ use function in_array; use function is_string; use function json_encode; +use function max; use function microtime; +use function min; use function ord; use function random_bytes; use function str_split; From 2589fcb31d2e6ba06285fc28df1a445e3a11958e Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 12 Mar 2025 01:03:34 +0000 Subject: [PATCH 3/4] Implement rate limits by packet read ops for now this is configured to warn only by default, until we get the parameters for it dialled in --- resources/pocketmine.yml | 23 ++++ src/YmlServerProperties.php | 5 + src/network/mcpe/NetworkSession.php | 104 ++++++++++++------- src/network/mcpe/PacketRateLimiter.php | 8 ++ src/network/mcpe/PacketRateLimiterAction.php | 30 ++++++ 5 files changed, 131 insertions(+), 39 deletions(-) create mode 100644 src/network/mcpe/PacketRateLimiterAction.php diff --git a/resources/pocketmine.yml b/resources/pocketmine.yml index 53192497317..c7d3606020a 100644 --- a/resources/pocketmine.yml +++ b/resources/pocketmine.yml @@ -83,6 +83,29 @@ network: #DO NOT DISABLE THIS unless you understand the risks involved. enable-encryption: true + #EXPERIMENTAL! Limit packet read operations per network session. + #This is intended to stop exploitation of packets with arrays in them. + #Note that enabling this system may cause players to be unexpectedly kicked, or it may fail to stop attackers. + #As of March 2025, the system is still in development and subject to change. + packet-read-ops-limit: + #What to do when a session's read ops budget is depleted. + #Supported actions are "none", "warn" and "kick". + deplete-action: warn + + #How many backlog ticks to budget for. 200 allows for a 10-second network lag spike, or a small number of complex + #packets. + session-budget-ticks: 100 + + #How much to top up each session's read operations budget per tick. Recommended to set this to about 2x the + #average number of read operations per tick per session. + #Exceeding this value won't cause any action to be taken. However, consistently exceeding it will cause the + #session's budget to be depleted, resulting in action being taken. + session-budget-per-tick: 250 + + #Whether to collect stats for debugging. This might impact performance. + #See NetworkSession::dumpDecodeCostStats() if you want to visualize the stats yourself. + collect-stats: false + debug: #If > 1, it will show debug messages in the console level: 1 diff --git a/src/YmlServerProperties.php b/src/YmlServerProperties.php index 282b0b3cdfe..81568a393b4 100644 --- a/src/YmlServerProperties.php +++ b/src/YmlServerProperties.php @@ -90,6 +90,11 @@ private function __construct(){ public const NETWORK_COMPRESSION_LEVEL = 'network.compression-level'; public const NETWORK_ENABLE_ENCRYPTION = 'network.enable-encryption'; public const NETWORK_MAX_MTU_SIZE = 'network.max-mtu-size'; + public const NETWORK_PACKET_READ_OPS_LIMIT = 'network.packet-read-ops-limit'; + public const NETWORK_PACKET_READ_OPS_LIMIT_COLLECT_STATS = 'network.packet-read-ops-limit.collect-stats'; + public const NETWORK_PACKET_READ_OPS_LIMIT_DEPLETE_ACTION = 'network.packet-read-ops-limit.deplete-action'; + public const NETWORK_PACKET_READ_OPS_LIMIT_SESSION_BUDGET_PER_TICK = 'network.packet-read-ops-limit.session-budget-per-tick'; + public const NETWORK_PACKET_READ_OPS_LIMIT_SESSION_BUDGET_TICKS = 'network.packet-read-ops-limit.session-budget-ticks'; public const NETWORK_UPNP_FORWARDING = 'network.upnp-forwarding'; public const PLAYER = 'player'; public const PLAYER_SAVE_PLAYER_DATA = 'player.save-player-data'; diff --git a/src/network/mcpe/NetworkSession.php b/src/network/mcpe/NetworkSession.php index 127bbe71f64..779d55d64aa 100644 --- a/src/network/mcpe/NetworkSession.php +++ b/src/network/mcpe/NetworkSession.php @@ -117,13 +117,12 @@ use pocketmine\world\format\io\GlobalItemDataHandlers; use pocketmine\world\Position; use pocketmine\world\World; -use pocketmine\YmlServerProperties; +use pocketmine\YmlServerProperties as Yml; use function array_map; use function base64_encode; use function bin2hex; use function count; use function get_class; -use function hrtime; use function implode; use function in_array; use function is_string; @@ -140,7 +139,6 @@ use function substr; use function time; use function ucfirst; -use const JSON_PRETTY_PRINT; use const JSON_THROW_ON_ERROR; use const PHP_INT_MAX; @@ -151,9 +149,17 @@ class NetworkSession{ private const INCOMING_GAME_PACKETS_PER_TICK = 2; private const INCOMING_GAME_PACKETS_BUFFER_TICKS = 100; + private const READ_OPS_PER_TICK = 100; //subject to change + private const READ_OPS_BUFFER_TICKS = 200; //subject to change + private PacketRateLimiter $packetBatchLimiter; private PacketRateLimiter $gamePacketLimiter; + private PacketRateLimiter $readOpsLimiter; + private PacketRateLimiterAction $readOpsLimiterAction; + + private bool $readOpsStats; + private \PrefixedLogger $logger; private ?Player $player = null; private ?PlayerInfo $info = null; @@ -205,24 +211,23 @@ class NetworkSession{ * @var int[] * @phpstan-var array */ - private array $packetCostsMin = []; + private array $readOpsPerPacketMin = []; /** * @var int[] * @phpstan-var array */ - private array $packetCostsMax = []; + private array $readOpsPerPacketMax = []; /** * @var int[] * @phpstan-var array */ - private array $packetCostsTotal = []; + private array $readOpsPerPacketTotal = []; /** * @var int[] * @phpstan-var array */ - private array $packetCounts = []; - private int $decodeCostTotal = 0; - private int $sessionStartHrtime; + private array $receivedPacketCounts = []; + private int $totalReadOpsUsed = 0; public function __construct( private Server $server, @@ -246,6 +251,15 @@ public function __construct( $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); + $serverConfigGroup = $this->server->getConfigGroup(); + $this->readOpsLimiter = new PacketRateLimiter( + "Packet Read Operations", + $serverConfigGroup->getPropertyInt(Yml::NETWORK_PACKET_READ_OPS_LIMIT_SESSION_BUDGET_PER_TICK, self::READ_OPS_PER_TICK), + $serverConfigGroup->getPropertyInt(Yml::NETWORK_PACKET_READ_OPS_LIMIT_SESSION_BUDGET_TICKS, self::READ_OPS_BUFFER_TICKS), + ); + $this->readOpsLimiterAction = PacketRateLimiterAction::from($serverConfigGroup->getPropertyString(Yml::NETWORK_PACKET_READ_OPS_LIMIT_DEPLETE_ACTION, PacketRateLimiterAction::WARN->value)); + $this->readOpsStats = $serverConfigGroup->getPropertyBool(Yml::NETWORK_PACKET_READ_OPS_LIMIT_COLLECT_STATS, false); + $this->setHandler(new SessionStartPacketHandler( $this, $this->onSessionStartSuccess(...) @@ -253,8 +267,6 @@ public function __construct( $this->manager->add($this); $this->logger->info($this->server->getLanguage()->translate(KnownTranslationFactory::pocketmine_network_session_open())); - - $this->sessionStartHrtime = hrtime(true); } private function getLogPrefix() : string{ @@ -482,17 +494,36 @@ public function handleDataPacket(Packet $packet, string $buffer) : void{ try{ $stream = PacketSerializer::decoder($buffer, 0); try{ - $stream->setReadOpsLimit(PHP_INT_MAX); //just for stats collection - $packet->decode($stream); - - $readOps = $stream->getReadOps(); - - $this->decodeCostTotal += $readOps; - $key = $packet->getName(); - $this->packetCostsTotal[$key] = ($this->packetCostsTotal[$key] ?? 0) + $readOps; - $this->packetCostsMin[$key] = min($this->packetCostsMin[$key] ?? PHP_INT_MAX, $readOps); - $this->packetCostsMax[$key] = max($this->packetCostsMax[$key] ?? 0, $readOps); - $this->packetCounts[$key] = ($this->packetCounts[$key] ?? 0) + 1; + if($this->readOpsLimiterAction === PacketRateLimiterAction::NONE){ + $packet->decode($stream); + }else{ + $this->readOpsLimiter->update(); + $stream->setReadOpsLimit( + $this->readOpsLimiterAction === PacketRateLimiterAction::KICK ? + $this->readOpsLimiter->getBudget() : + PHP_INT_MAX //don't bail out of decoding if we're only warning + ); + + $packet->decode($stream); + $readOps = $stream->getReadOps(); + //If we exceeded the budget, a PacketDecodeException has already been thrown if we're in KICK + //mode, so we can assume we're in WARN mode here + if($this->readOpsLimiter->getBudget() < $readOps){ + $this->getLogger()->warning("Decoding " . $packet->getName() . " exceeded read ops budget! $readOps > " . $this->readOpsLimiter->getBudget()); + $this->readOpsLimiter->reset(); + }else{ + $this->readOpsLimiter->decrement($readOps); + } + + if($this->readOpsStats){ + $this->totalReadOpsUsed += $readOps; + $key = $packet->getName(); + $this->readOpsPerPacketTotal[$key] = ($this->readOpsPerPacketTotal[$key] ?? 0) + $readOps; + $this->readOpsPerPacketMin[$key] = min($this->readOpsPerPacketMin[$key] ?? PHP_INT_MAX, $readOps); + $this->readOpsPerPacketMax[$key] = max($this->readOpsPerPacketMax[$key] ?? 0, $readOps); + $this->receivedPacketCounts[$key] = ($this->receivedPacketCounts[$key] ?? 0) + 1; + } + } }catch(PacketDecodeException $e){ throw PacketHandlingException::wrap($e); } @@ -530,19 +561,22 @@ public function handleDataPacket(Packet $packet, string $buffer) : void{ * @phpstan-return array> */ public function dumpDecodeCostStats() : array{ - $sessionTime = ((hrtime(true) - $this->sessionStartHrtime) / 1_000_000_000); + if(!$this->readOpsStats){ + throw new \LogicException("Not collecting stats for this session"); + } + $sessionTime = microtime(true) - $this->connectTime; $packetDecodeAverages = []; $packetCostsPerSecondAverages = []; - foreach(Utils::stringifyKeys($this->packetCostsTotal) as $packet => $total){ - $packetDecodeAverages[$packet] = $total / $this->packetCounts[$packet]; + foreach(Utils::stringifyKeys($this->readOpsPerPacketTotal) as $packet => $total){ + $packetDecodeAverages[$packet] = $total / $this->receivedPacketCounts[$packet]; $packetCostsPerSecondAverages[$packet] = $total / $sessionTime; } return [ - "decodeCostAvgPerSecond" => $this->decodeCostTotal / $sessionTime, - "decodeCostAvgPerPacketPerSecond" => $packetCostsPerSecondAverages, - "decodeCostAvgPerPacket" => $packetDecodeAverages, - "decodeCostMinPerPacket" => $this->packetCostsMin, - "decodeCostMaxPerPacket" => $this->packetCostsMax + "readOpsAvgPerSecondTotal" => $this->totalReadOpsUsed / $sessionTime, + "readOpsAvgPerPacketPerSecond" => $packetCostsPerSecondAverages, + "readOpsAvgPerPacket" => $packetDecodeAverages, + "readOpsMinPerPacket" => $this->readOpsPerPacketMin, + "readOpsMaxPerPacket" => $this->readOpsPerPacketMax ]; } @@ -920,7 +954,7 @@ private function setAuthenticationStatus(bool $authenticated, bool $authRequired } $this->logger->debug("Xbox Live authenticated: " . ($this->authenticated ? "YES" : "NO")); - $checkXUID = $this->server->getConfigGroup()->getPropertyBool(YmlServerProperties::PLAYER_VERIFY_XUID, true); + $checkXUID = $this->server->getConfigGroup()->getPropertyBool(Yml::PLAYER_VERIFY_XUID, true); $myXUID = $this->info instanceof XboxLivePlayerInfo ? $this->info->getXuid() : ""; $kickForXUIDMismatch = function(string $xuid) use ($checkXUID, $myXUID) : bool{ if($checkXUID && $myXUID !== $xuid){ @@ -1409,13 +1443,5 @@ public function tick() : void{ } $this->flushSendBuffer(); - - $now = microtime(true); - if($now - $this->lastStatsReportTime > 10){ - $this->logger->debug("Decode cost stats: " . json_encode($this->dumpDecodeCostStats(), JSON_PRETTY_PRINT | JSON_THROW_ON_ERROR)); - $this->lastStatsReportTime = $now; - } } - - private float $lastStatsReportTime = 0; } diff --git a/src/network/mcpe/PacketRateLimiter.php b/src/network/mcpe/PacketRateLimiter.php index 3f0fbf768a1..cd1bce96847 100644 --- a/src/network/mcpe/PacketRateLimiter.php +++ b/src/network/mcpe/PacketRateLimiter.php @@ -62,6 +62,14 @@ public function decrement(int $amount = 1) : void{ $this->budget -= $amount; } + public function getBudget() : int{ + return $this->budget; + } + + public function reset() : void{ + $this->budget = $this->maxBudget; + } + public function update() : void{ $nowNs = hrtime(true); $timeSinceLastUpdateNs = $nowNs - $this->lastUpdateTimeNs; diff --git a/src/network/mcpe/PacketRateLimiterAction.php b/src/network/mcpe/PacketRateLimiterAction.php new file mode 100644 index 00000000000..cb773a547ad --- /dev/null +++ b/src/network/mcpe/PacketRateLimiterAction.php @@ -0,0 +1,30 @@ + Date: Wed, 12 Mar 2025 01:06:01 +0000 Subject: [PATCH 4/4] tweak config defaults --- resources/pocketmine.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resources/pocketmine.yml b/resources/pocketmine.yml index c7d3606020a..b9082d36057 100644 --- a/resources/pocketmine.yml +++ b/resources/pocketmine.yml @@ -94,13 +94,13 @@ network: #How many backlog ticks to budget for. 200 allows for a 10-second network lag spike, or a small number of complex #packets. - session-budget-ticks: 100 + session-budget-ticks: 200 #How much to top up each session's read operations budget per tick. Recommended to set this to about 2x the #average number of read operations per tick per session. #Exceeding this value won't cause any action to be taken. However, consistently exceeding it will cause the #session's budget to be depleted, resulting in action being taken. - session-budget-per-tick: 250 + session-budget-per-tick: 100 #Whether to collect stats for debugging. This might impact performance. #See NetworkSession::dumpDecodeCostStats() if you want to visualize the stats yourself.