-
Notifications
You must be signed in to change notification settings - Fork 40
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
Cookie data #229
Cookie data #229
Changes from 27 commits
3e2e59e
963c533
c22a7d1
7425ec5
80e89f0
c58e264
967555e
78f5cf5
3cdf46e
1c94a8c
440196c
1287f4c
3764e4b
312e11e
9078818
9c11bb4
ba4c754
9555243
ec8532b
884bbff
b6c85e7
15d1b13
75e8526
ff2a37e
8e6c677
f8250af
df75ce4
64cb140
b9bb269
65e2da8
0e8b732
bd80d49
cb35ab3
41ac282
7d1586f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,18 +22,29 @@ class OpenConnectionRequest2 extends OfflineMessage{ | |
public static $ID = MessageIdentifiers::ID_OPEN_CONNECTION_REQUEST_2; | ||
|
||
public int $clientID; | ||
public ?int $cookie; | ||
public bool $clientSupportsSecurity = false; // Always false for the vanilla client. | ||
public InternetAddress $serverAddress; | ||
public int $mtuSize; | ||
|
||
protected function encodePayload(PacketSerializer $out) : void{ | ||
$this->writeMagic($out); | ||
if ($this->cookie !== null) { | ||
$out->putInt($this->cookie); | ||
$out->putBool($this->clientSupportsSecurity); | ||
} | ||
$out->putAddress($this->serverAddress); | ||
$out->putShort($this->mtuSize); | ||
$out->putLong($this->clientID); | ||
} | ||
|
||
protected function decodePayload(PacketSerializer $in) : void{ | ||
//$length = strlen($in->getRemaining()); // magic(16) + cookie(4) + clientSupportsSecurity(1) + serverAddress(??) + mtuSize(2) + clientID(8) | ||
$this->readMagic($in); | ||
if ($this->cookie !== null) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The cookie is always going to be null when you haven't decoded it yet There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
what should i do? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This depends on the protocol. How do we know if the client is going to send a cookie in this request? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We can understand something from the length of the incoming buffer, but I don't know if it's the right thing to do. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we just need a context-aware parser that can read whether cookie is expected. If we always send cookies for all clients, we should always read it. If we never send cookies, we should never read it. It doesn't seem that we require any guessing. |
||
$this->cookie = $in->getInt(); | ||
$this->clientSupportsSecurity = $in->getBool(); | ||
} | ||
$this->serverAddress = $in->getAddress(); | ||
$this->mtuSize = $in->getShort(); | ||
$this->clientID = $in->getLong(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,7 @@ | |
use raklib\protocol\NACK; | ||
use raklib\protocol\Packet; | ||
use raklib\protocol\PacketSerializer; | ||
use raklib\utils\Cookie; | ||
use raklib\utils\ExceptionTraceCleaner; | ||
use raklib\utils\InternetAddress; | ||
use function asort; | ||
|
@@ -77,6 +78,8 @@ class Server implements ServerInterface{ | |
|
||
protected int $nextSessionId = 0; | ||
|
||
public ?Cookie $cookie = null; | ||
|
||
/** | ||
* @phpstan-param positive-int $recvMaxSplitParts | ||
* @phpstan-param positive-int $recvMaxConcurrentSplits | ||
|
@@ -99,6 +102,9 @@ public function __construct( | |
$this->socket->setBlocking(false); | ||
|
||
$this->unconnectedMessageHandler = new UnconnectedMessageHandler($this, $protocolAcceptor); | ||
|
||
// If you don't want to use security on the server, just delete this line. | ||
$this->cookie = new Cookie(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Users of the library don't want to change the library itself to change a behavior, a switch in the constructor need to be created |
||
} | ||
|
||
public function getPort() : int{ | ||
|
@@ -113,6 +119,10 @@ public function getLogger() : \Logger{ | |
return $this->logger; | ||
} | ||
|
||
public function getCookie() : ?Cookie { | ||
return $this->cookie; | ||
} | ||
|
||
public function tickProcessor() : void{ | ||
$start = microtime(true); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of RakLib. | ||
* Copyright (C) 2014-2022 PocketMine Team <https://github.com/pmmp/RakLib> | ||
* | ||
* 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\utils; | ||
|
||
use raklib\utils\InternetAddress; | ||
use pocketmine\utils\Binary; | ||
use function random_int; | ||
use function crc32; | ||
|
||
final class Cookie{ | ||
ismaileke marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
/** | ||
* @var array<string, int> $cookies | ||
*/ | ||
private array $cookies = []; | ||
ismaileke marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
public function get(InternetAddress $address) : ?int{ | ||
if (isset($this->cookies[$address->toString()])) { | ||
return $this->cookies[$address->toString()]; | ||
} | ||
return null; | ||
} | ||
|
||
public function check(InternetAddress $address, int $cookie) : bool{ | ||
$addressStr = $address->toString(); | ||
|
||
if (isset($this->cookies[$addressStr])) { | ||
// If it checks the Cookie, it means that it is in the OpenConnectionRequest2 phase, and we can delete it from memory. | ||
if ($this->cookies[$addressStr] === $cookie) { | ||
unset($this->cookies[$addressStr]); | ||
return true; | ||
} | ||
} // Is there any chance that this is something else? | ||
return false; | ||
} | ||
|
||
public function add(InternetAddress $address) : void{ | ||
ismaileke marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (!isset($this->cookies[$address->toString()])) { | ||
$this->cookies[$address->toString()] = $this->generate($address); | ||
} | ||
} | ||
|
||
private function generate(InternetAddress $address) : int{ | ||
return crc32(Binary::writeLInt(random_int(0, 0xffffffff)) . Binary::writeLShort($address->getPort()) . $address->getIp()); | ||
ismaileke marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fwiw, by linux and git conventions, text files should have a trailing newline.