Skip to content

Commit 9480542

Browse files
dktappsdktapps
committed
Merge branch 'stable' into major-next
2 parents 575ed2f + 669eb4d commit 9480542

12 files changed

Lines changed: 109 additions & 64 deletions

File tree

‎.github/dependabot.yml‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,13 @@ updates:
33
- package-ecosystem: composer
44
directory: "/"
55
schedule:
6-
interval: daily
6+
interval: monthly
77
time: "10:00"
88
open-pull-requests-limit: 10
99
- package-ecosystem: github-actions
1010
directory: "/"
1111
schedule:
1212
interval: monthly
13+
groups:
14+
github-actions:
15+
patterns: ["*"]

‎.github/workflows/main.yml‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,20 +10,20 @@ jobs:
1010
if: "!contains(github.event.head_commit.message, '[ci skip]')"
1111
strategy:
1212
matrix:
13-
php: ["8.1", "8.2", "8.3"]
13+
php: ["8.1", "8.2", "8.3", "8.4", "8.5"]
1414
name: PHP ${{ matrix.php }}
15-
runs-on: ubuntu-20.04
15+
runs-on: ubuntu-22.04
1616
steps:
17-
- uses: actions/checkout@v3
17+
- uses: actions/checkout@v4
1818

1919
- name: Setup PHP
20-
uses: shivammathur/setup-php@2.24.0
20+
uses: shivammathur/setup-php@2.32.0
2121
with:
2222
php-version: ${{ matrix.php }}
2323

2424
- name: Cache Composer packages
2525
id: composer-cache
26-
uses: actions/cache@v3
26+
uses: actions/cache@v4
2727
with:
2828
path: "~/.cache/composer"
2929
key: "php-${{ matrix.php }}-composer-${{ hashFiles('**/composer.json') }}"

‎composer.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@
1212
"pocketmine/binaryutils": "^0.2.0"
1313
},
1414
"require-dev": {
15-
"phpstan/phpstan": "1.10.1",
16-
"phpstan/phpstan-strict-rules": "^1.0"
15+
"phpstan/phpstan": "2.1.33",
16+
"phpstan/phpstan-strict-rules": "^2.0"
1717
},
1818
"autoload": {
1919
"psr-4": {

‎phpstan.neon.dist‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
includes:
2-
- tests/phpstan/configs/encapsulated-packet-sucks.neon
32
- tests/phpstan/configs/phpstan-bugs.neon
43
- vendor/phpstan/phpstan-strict-rules/rules.neon
54

‎src/generic/ReceiveReliabilityLayer.php‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@ private function handleEncapsulatedPacket(EncapsulatedPacket $packet) : void{
176176
}
177177

178178
if($packet->reliability->isSequenced()){
179+
assert($packet->orderChannel !== null && $packet->sequenceIndex !== null, 'These should have been set during decode');
179180
if($packet->sequenceIndex < $this->receiveSequencedHighestIndex[$packet->orderChannel] or $packet->orderIndex < $this->receiveOrderedIndex[$packet->orderChannel]){
180181
//too old sequenced packet, discard it
181182
return;
@@ -184,28 +185,30 @@ private function handleEncapsulatedPacket(EncapsulatedPacket $packet) : void{
184185
$this->receiveSequencedHighestIndex[$packet->orderChannel] = $packet->sequenceIndex + 1;
185186
$this->handleEncapsulatedPacketRoute($packet);
186187
}elseif($packet->reliability->isOrdered()){
187-
if($packet->orderIndex === $this->receiveOrderedIndex[$packet->orderChannel]){
188+
$orderChannel = $packet->orderChannel;
189+
assert($orderChannel !== null, 'This should have been set during decode');
190+
if($packet->orderIndex === $this->receiveOrderedIndex[$orderChannel]){
188191
//this is the packet we expected to get next
189192
//Any ordered packet resets the sequence index to zero, so that sequenced packets older than this ordered
190193
//one get discarded. Sequenced packets also include (but don't increment) the order index, so a sequenced
191194
//packet with an order index less than this will get discarded
192-
$this->receiveSequencedHighestIndex[$packet->orderChannel] = 0;
193-
$this->receiveOrderedIndex[$packet->orderChannel] = $packet->orderIndex + 1;
195+
$this->receiveSequencedHighestIndex[$orderChannel] = 0;
196+
$this->receiveOrderedIndex[$orderChannel] = $packet->orderIndex + 1;
194197

195198
$this->handleEncapsulatedPacketRoute($packet);
196-
$i = $this->receiveOrderedIndex[$packet->orderChannel];
197-
for(; isset($this->receiveOrderedPackets[$packet->orderChannel][$i]); ++$i){
198-
$this->handleEncapsulatedPacketRoute($this->receiveOrderedPackets[$packet->orderChannel][$i]);
199-
unset($this->receiveOrderedPackets[$packet->orderChannel][$i]);
199+
$i = $this->receiveOrderedIndex[$orderChannel];
200+
for(; isset($this->receiveOrderedPackets[$orderChannel][$i]); ++$i){
201+
$this->handleEncapsulatedPacketRoute($this->receiveOrderedPackets[$orderChannel][$i]);
202+
unset($this->receiveOrderedPackets[$orderChannel][$i]);
200203
}
201204

202-
$this->receiveOrderedIndex[$packet->orderChannel] = $i;
203-
}elseif($packet->orderIndex > $this->receiveOrderedIndex[$packet->orderChannel]){
204-
if(count($this->receiveOrderedPackets[$packet->orderChannel]) >= self::$WINDOW_SIZE){
205+
$this->receiveOrderedIndex[$orderChannel] = $i;
206+
}elseif($packet->orderIndex > $this->receiveOrderedIndex[$orderChannel]){
207+
if(count($this->receiveOrderedPackets[$orderChannel]) >= self::$WINDOW_SIZE){
205208
//queue overflow for this channel - we should probably disconnect the peer at this point
206209
return;
207210
}
208-
$this->receiveOrderedPackets[$packet->orderChannel][$packet->orderIndex] = $packet;
211+
$this->receiveOrderedPackets[$orderChannel][$packet->orderIndex] = $packet;
209212
}else{
210213
//duplicate/already received packet
211214
}

‎src/generic/SendReliabilityLayer.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,14 @@ public function addEncapsulatedToQueue(EncapsulatedPacket $packet, bool $immedia
169169
}
170170

171171
if($packet->reliability->isOrdered()){
172+
if($packet->orderChannel === null){
173+
throw new \InvalidArgumentException("Order channel must be set reliability {$packet->reliability->name}");
174+
}
172175
$packet->orderIndex = $this->sendOrderedIndex[$packet->orderChannel]++;
173176
}elseif($packet->reliability->isSequenced()){
177+
if($packet->orderChannel === null){
178+
throw new \InvalidArgumentException("Order channel must be set for reliability {$packet->reliability->name}");
179+
}
174180
$packet->orderIndex = $this->sendOrderedIndex[$packet->orderChannel]; //sequenced packets don't increment the ordered channel index
175181
$packet->sequenceIndex = $this->sendSequencedIndex[$packet->orderChannel]++;
176182
}

‎src/generic/Session.php‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -270,12 +270,16 @@ private function handleEncapsulatedPacketRoute(EncapsulatedPacket $packet) : voi
270270
* @param int $sendPongTime TODO: clock differential stuff
271271
*/
272272
private function handlePong(int $sendPingTime, int $sendPongTime) : void{
273-
$currentTime = $this->getRakNetTimeMS();
274-
if($currentTime < $sendPingTime){
275-
$this->logger->debug("Received invalid pong: timestamp is in the future by " . ($sendPingTime - $currentTime) . " ms");
273+
if($sendPingTime < 0){
274+
$this->logger->debug("Received invalid pong: timestamp overflow");
276275
}else{
277-
$this->lastPingMeasure = $currentTime - $sendPingTime;
278-
$this->onPingMeasure($this->lastPingMeasure);
276+
$currentTime = $this->getRakNetTimeMS();
277+
if($currentTime < $sendPingTime){
278+
$this->logger->debug("Received invalid pong: timestamp is in the future by " . ($sendPingTime - $currentTime) . " ms");
279+
}else{
280+
$this->lastPingMeasure = $currentTime - $sendPingTime;
281+
$this->onPingMeasure($this->lastPingMeasure);
282+
}
279283
}
280284
}
281285

‎src/protocol/EncapsulatedPacket.php‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,19 @@ public function toBinary() : string{
8888
return
8989
chr(($this->reliability->value << self::RELIABILITY_SHIFT) | ($this->splitInfo !== null ? self::SPLIT_FLAG : 0)) .
9090
Binary::writeShort(strlen($this->buffer) << 3) .
91-
($this->reliability->isReliable() ? Binary::writeLTriad($this->messageIndex) : "") .
92-
($this->reliability->isSequenced() ? Binary::writeLTriad($this->sequenceIndex) : "") .
93-
($this->reliability->isSequencedOrOrdered() ? Binary::writeLTriad($this->orderIndex) . chr($this->orderChannel) : "") .
91+
($this->reliability->isReliable() ?
92+
Binary::writeLTriad($this->messageIndex ?? throw new \LogicException("Message index must be set for reliability {$this->reliability->name}")) :
93+
""
94+
) .
95+
($this->reliability->isSequenced() ?
96+
Binary::writeLTriad($this->sequenceIndex ?? throw new \LogicException("Sequence index must be set for reliability {$this->reliability->name}")) :
97+
""
98+
) .
99+
($this->reliability->isSequencedOrOrdered() ?
100+
Binary::writeLTriad($this->orderIndex ?? throw new \LogicException("Order index must be set for reliability {$this->reliability->name}")) .
101+
chr($this->orderChannel ?? throw new \LogicException("Order channel must be set for reliability {$this->reliability->name}")) :
102+
""
103+
) .
94104
($this->splitInfo !== null ? Binary::writeInt($this->splitInfo->getTotalPartCount()) . Binary::writeShort($this->splitInfo->getId()) . Binary::writeInt($this->splitInfo->getPartIndex()) : "")
95105
. $this->buffer;
96106
}

‎src/server/Server.php‎

Lines changed: 50 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@
1818

1919
use pocketmine\utils\BinaryDataException;
2020
use raklib\generic\DisconnectReason;
21+
use raklib\generic\PacketHandlingException;
2122
use raklib\generic\Session;
2223
use raklib\generic\SocketException;
23-
use raklib\generic\PacketHandlingException;
2424
use raklib\protocol\ACK;
2525
use raklib\protocol\Datagram;
2626
use raklib\protocol\EncapsulatedPacket;
@@ -30,6 +30,7 @@
3030
use raklib\utils\ExceptionTraceCleaner;
3131
use raklib\utils\InternetAddress;
3232
use function asort;
33+
use function assert;
3334
use function bin2hex;
3435
use function count;
3536
use function get_class;
@@ -46,6 +47,8 @@ class Server implements ServerInterface{
4647

4748
private const RAKLIB_TPS = 100;
4849
private const RAKLIB_TIME_PER_TICK = 1 / self::RAKLIB_TPS;
50+
private const BLOCK_MESSAGE_SUPPRESSION_THRESHOLD = 2;
51+
private const PACKET_ERROR_SUPPRESSION_THRESHOLD = 2;
4952

5053
protected int $receiveBytes = 0;
5154
protected int $sendBytes = 0;
@@ -70,6 +73,10 @@ class Server implements ServerInterface{
7073
/** @var int[] string (address) => int (number of packets) */
7174
protected array $ipSec = [];
7275

76+
private int $blockedSinceLastUpdate = 0;
77+
78+
private int $packetErrorsSinceLastUpdate = 0;
79+
7380
/** @var string[] regex filters used to block out unwanted raw packets */
7481
protected array $rawPacketFilters = [];
7582

@@ -91,7 +98,10 @@ public function __construct(
9198
private ServerEventListener $eventListener,
9299
private ExceptionTraceCleaner $traceCleaner,
93100
private int $recvMaxSplitParts = ServerSession::DEFAULT_MAX_SPLIT_PART_COUNT,
94-
private int $recvMaxConcurrentSplits = ServerSession::DEFAULT_MAX_CONCURRENT_SPLIT_COUNT
101+
private int $recvMaxConcurrentSplits = ServerSession::DEFAULT_MAX_CONCURRENT_SPLIT_COUNT,
102+
private int $blockMessageSuppressionThreshold = self::BLOCK_MESSAGE_SUPPRESSION_THRESHOLD,
103+
private int $packetErrorSuppressionThreshold = self::PACKET_ERROR_SUPPRESSION_THRESHOLD,
104+
private bool $blockIpOnPacketErrors = true
95105
){
96106
if($maxMtuSize < Session::MIN_MTU_SIZE){
97107
throw new \InvalidArgumentException("MTU size must be at least " . Session::MIN_MTU_SIZE . ", got $maxMtuSize");
@@ -182,6 +192,18 @@ private function tick() : void{
182192
$this->receiveBytes = 0;
183193
}
184194

195+
$packetErrorsWithoutMessage = $this->packetErrorsSinceLastUpdate - $this->packetErrorSuppressionThreshold;
196+
if($packetErrorsWithoutMessage > 0){
197+
$this->logger->warning("$packetErrorsWithoutMessage suppressed packet errors - RakLib may be under attack");
198+
}
199+
$this->packetErrorsSinceLastUpdate = 0;
200+
201+
$ipsBlockedWithoutMessage = $this->blockedSinceLastUpdate - $this->blockMessageSuppressionThreshold;
202+
if($ipsBlockedWithoutMessage > 0){
203+
$this->logger->warning("$ipsBlockedWithoutMessage more IP addresses were blocked - RakLib may be under attack");
204+
}
205+
$this->blockedSinceLastUpdate = 0;
206+
185207
if(count($this->block) > 0){
186208
asort($this->block);
187209
$now = time();
@@ -214,6 +236,9 @@ private function receivePacket() : bool{
214236
if($buffer === null){
215237
return false; //no data
216238
}
239+
assert($addressIp !== null, "Can't be null if we got a buffer");
240+
assert($addressPort !== null, "Can't be null if we got a buffer");
241+
217242
$len = strlen($buffer);
218243

219244
$this->receiveBytes += $len;
@@ -279,20 +304,25 @@ private function receivePacket() : bool{
279304
}
280305
}
281306
}catch(BinaryDataException $e){
282-
$logFn = function() use ($address, $e, $buffer) : void{
283-
$this->logger->debug("Packet from $address (" . strlen($buffer) . " bytes): 0x" . bin2hex($buffer));
284-
$this->logger->debug(get_class($e) . ": " . $e->getMessage() . " in " . $e->getFile() . " on line " . $e->getLine());
285-
foreach($this->traceCleaner->getTrace(0, $e->getTrace()) as $line){
286-
$this->logger->debug($line);
307+
if($this->packetErrorsSinceLastUpdate < $this->packetErrorSuppressionThreshold){
308+
$logFn = function() use ($address, $e, $buffer) : void{
309+
$this->logger->debug("Packet from $address (" . strlen($buffer) . " bytes): 0x" . bin2hex($buffer));
310+
$this->logger->debug(get_class($e) . ": " . $e->getMessage() . " in " . $e->getFile() . " on line " . $e->getLine());
311+
foreach($this->traceCleaner->getTrace(0, $e->getTrace()) as $line){
312+
$this->logger->debug($line);
313+
}
314+
$this->logger->error("Bad packet from $address: " . $e->getMessage());
315+
};
316+
if($this->logger instanceof \BufferedLogger){
317+
$this->logger->buffer($logFn);
318+
}else{
319+
$logFn();
287320
}
288-
$this->logger->error("Bad packet from $address: " . $e->getMessage());
289-
};
290-
if($this->logger instanceof \BufferedLogger){
291-
$this->logger->buffer($logFn);
292-
}else{
293-
$logFn();
294321
}
295-
$this->blockAddress($address->getIp(), 5);
322+
$this->packetErrorsSinceLastUpdate++;
323+
if($this->blockIpOnPacketErrors){
324+
$this->blockAddress($address->getIp(), 5);
325+
}
296326
}
297327

298328
return true;
@@ -350,10 +380,14 @@ public function blockAddress(string $address, int $timeout = 300) : void{
350380
if(!isset($this->block[$address]) or $timeout === -1){
351381
if($timeout === -1){
352382
$final = PHP_INT_MAX;
353-
}else{
354-
$this->logger->notice("Blocked $address for $timeout seconds");
383+
}
384+
if($this->blockedSinceLastUpdate < $this->blockMessageSuppressionThreshold){
385+
//Suppress additional log messages if multiple IPs have been banned in quick succession
386+
//In the case of IP spoofing attacks we don't want log spam to slow down the server
387+
$this->logger->notice("Blocked $address" . ($timeout === -1 ? " forever" : " for $timeout seconds"));
355388
}
356389
$this->block[$address] = $final;
390+
$this->blockedSinceLastUpdate++;
357391
}elseif($this->block[$address] < $final){
358392
$this->block[$address] = $final;
359393
}

‎tests/phpstan/configs/encapsulated-packet-sucks.neon‎

Lines changed: 0 additions & 17 deletions
This file was deleted.

0 commit comments

Comments
 (0)