Skip to content

EXPERIMENTAL! Packet read ops accounting and limiting #6651

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: minor-next
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
33 changes: 21 additions & 12 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 23 additions & 0 deletions resources/pocketmine.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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: 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: 100

#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
Expand Down
5 changes: 5 additions & 0 deletions src/YmlServerProperties.php
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
103 changes: 100 additions & 3 deletions src/network/mcpe/NetworkSession.php
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,11 @@
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;
use pocketmine\YmlServerProperties;
use pocketmine\YmlServerProperties as Yml;
use function array_map;
use function base64_encode;
use function bin2hex;
Expand All @@ -126,6 +127,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;
Expand All @@ -136,6 +140,7 @@
use function time;
use function ucfirst;
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
Expand All @@ -144,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;
Expand Down Expand Up @@ -194,6 +207,28 @@ class NetworkSession{
*/
private ObjectSet $disposeHooks;

/**
* @var int[]
* @phpstan-var array<string, int>
*/
private array $readOpsPerPacketMin = [];
/**
* @var int[]
* @phpstan-var array<string, int>
*/
private array $readOpsPerPacketMax = [];
/**
* @var int[]
* @phpstan-var array<string, int>
*/
private array $readOpsPerPacketTotal = [];
/**
* @var int[]
* @phpstan-var array<string, int>
*/
private array $receivedPacketCounts = [];
private int $totalReadOpsUsed = 0;

public function __construct(
private Server $server,
private NetworkSessionManager $manager,
Expand All @@ -216,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(...)
Expand Down Expand Up @@ -450,7 +494,36 @@ public function handleDataPacket(Packet $packet, string $buffer) : void{
try{
$stream = PacketSerializer::decoder($buffer, 0);
try{
$packet->decode($stream);
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);
}
Expand Down Expand Up @@ -483,6 +556,30 @@ public function handleDataPacket(Packet $packet, string $buffer) : void{
}
}

/**
* @return int[]|float[]
* @phpstan-return array<string, int|float|array<string, int|float>>
*/
public function dumpDecodeCostStats() : array{
if(!$this->readOpsStats){
throw new \LogicException("Not collecting stats for this session");
}
$sessionTime = microtime(true) - $this->connectTime;
$packetDecodeAverages = [];
$packetCostsPerSecondAverages = [];
foreach(Utils::stringifyKeys($this->readOpsPerPacketTotal) as $packet => $total){
$packetDecodeAverages[$packet] = $total / $this->receivedPacketCounts[$packet];
$packetCostsPerSecondAverages[$packet] = $total / $sessionTime;
}
return [
"readOpsAvgPerSecondTotal" => $this->totalReadOpsUsed / $sessionTime,
"readOpsAvgPerPacketPerSecond" => $packetCostsPerSecondAverages,
"readOpsAvgPerPacket" => $packetDecodeAverages,
"readOpsMinPerPacket" => $this->readOpsPerPacketMin,
"readOpsMaxPerPacket" => $this->readOpsPerPacketMax
];
}

public function handleAckReceipt(int $receiptId) : void{
if(!$this->connected){
return;
Expand Down Expand Up @@ -857,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){
Expand Down
8 changes: 8 additions & 0 deletions src/network/mcpe/PacketRateLimiter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
30 changes: 30 additions & 0 deletions src/network/mcpe/PacketRateLimiterAction.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php

/*
*
* ____ _ _ __ __ _ __ __ ____
* | _ \ ___ ___| | _____| |_| \/ (_)_ __ ___ | \/ | _ \
* | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
* | __/ (_) | (__| < __/ |_| | | | | | | | __/_____| | | | __/
* |_| \___/ \___|_|\_\___|\__|_| |_|_|_| |_|\___| |_| |_|_|
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* @author PocketMine Team
* @link http://www.pocketmine.net/
*
*
*/

declare(strict_types=1);

namespace pocketmine\network\mcpe;

enum PacketRateLimiterAction : string{
case NONE = "none";
case WARN = "warn";
case KICK = "kick";
}
Loading