From 83a3adecffef6a854682e9a956a7271f7bf6dd7b Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Mon, 23 Mar 2020 21:58:12 +0000 Subject: [PATCH] LoginPacket: use netresearch/jsonmapper for login data decoding this makes retrieval static analysis friendly without extra steps. --- composer.json | 3 +- composer.lock | 48 +++- phpstan.neon.dist | 1 + src/network/mcpe/auth/ProcessLoginTask.php | 2 +- .../mcpe/handler/LoginPacketHandler.php | 45 ++-- src/network/mcpe/protocol/LoginPacket.php | 154 +++--------- .../types/login/AuthenticationData.php | 51 ++++ .../mcpe/protocol/types/login/ClientData.php | 219 ++++++++++++++++++ .../types/login/ClientDataAnimationFrame.php | 60 +++++ .../mcpe/protocol/types/login/JwtChain.php | 36 +++ tests/phpstan/stubs/JsonMapper.stub | 19 ++ 11 files changed, 495 insertions(+), 143 deletions(-) create mode 100644 src/network/mcpe/protocol/types/login/AuthenticationData.php create mode 100644 src/network/mcpe/protocol/types/login/ClientData.php create mode 100644 src/network/mcpe/protocol/types/login/ClientDataAnimationFrame.php create mode 100644 src/network/mcpe/protocol/types/login/JwtChain.php create mode 100644 tests/phpstan/stubs/JsonMapper.stub diff --git a/composer.json b/composer.json index 01fa85081..f6727a6b1 100644 --- a/composer.json +++ b/composer.json @@ -41,7 +41,8 @@ "pocketmine/classloader": "dev-master", "pocketmine/callback-validator": "^1.0.1", "adhocore/json-comment": "^0.1.0", - "particle/validator": "^2.3" + "particle/validator": "^2.3", + "netresearch/jsonmapper": "^2.0" }, "require-dev": { "phpstan/phpstan": "^0.12.14", diff --git a/composer.lock b/composer.lock index bb7a5cc77..5eb5a2591 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": "b59b043a71525b45752770b4fd1ce2cb", + "content-hash": "8a94ad4a1822c04cb884a6dee81923df", "packages": [ { "name": "adhocore/json-comment", @@ -191,6 +191,52 @@ ], "time": "2018-12-03T18:17:01+00:00" }, + { + "name": "netresearch/jsonmapper", + "version": "v2.0.0", + "source": { + "type": "git", + "url": "https://github.com/cweiske/jsonmapper.git", + "reference": "e245890383c3ed38b6d202ee373c23ccfebc0f54" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/cweiske/jsonmapper/zipball/e245890383c3ed38b6d202ee373c23ccfebc0f54", + "reference": "e245890383c3ed38b6d202ee373c23ccfebc0f54", + "shasum": "" + }, + "require": { + "ext-json": "*", + "ext-pcre": "*", + "ext-reflection": "*", + "ext-spl": "*", + "php": ">=5.6" + }, + "require-dev": { + "phpunit/phpunit": "~4.8.35 || ~5.7 || ~6.4 || ~7.0", + "squizlabs/php_codesniffer": "~3.5" + }, + "type": "library", + "autoload": { + "psr-0": { + "JsonMapper": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "OSL-3.0" + ], + "authors": [ + { + "name": "Christian Weiske", + "email": "cweiske@cweiske.de", + "homepage": "http://github.com/cweiske/jsonmapper/", + "role": "Developer" + } + ], + "description": "Map nested JSON structures onto PHP classes", + "time": "2020-03-04T17:23:33+00:00" + }, { "name": "particle/validator", "version": "v2.3.4", diff --git a/phpstan.neon.dist b/phpstan.neon.dist index c756ebabe..c19818e44 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -33,6 +33,7 @@ parameters: - pocketmine\IS_DEVELOPMENT_BUILD - pocketmine\DEBUG stubFiles: + - tests/phpstan/stubs/JsonMapper.stub - tests/phpstan/stubs/pthreads.stub reportUnmatchedIgnoredErrors: false #no other way to silence platform-specific non-warnings ignoreErrors: diff --git a/src/network/mcpe/auth/ProcessLoginTask.php b/src/network/mcpe/auth/ProcessLoginTask.php index dcc90d5b2..cadd54c04 100644 --- a/src/network/mcpe/auth/ProcessLoginTask.php +++ b/src/network/mcpe/auth/ProcessLoginTask.php @@ -95,7 +95,7 @@ class ProcessLoginTask extends AsyncTask{ $currentKey = null; $first = true; - foreach($packet->chainDataJwt as $jwt){ + foreach($packet->chainDataJwt->chain as $jwt){ $this->validateToken($jwt, $currentKey, $first); if($first){ $first = false; diff --git a/src/network/mcpe/handler/LoginPacketHandler.php b/src/network/mcpe/handler/LoginPacketHandler.php index c76935dbd..c4f215e57 100644 --- a/src/network/mcpe/handler/LoginPacketHandler.php +++ b/src/network/mcpe/handler/LoginPacketHandler.php @@ -68,38 +68,39 @@ class LoginPacketHandler extends PacketHandler{ return true; } - if(!Player::isValidUserName($packet->extraData[LoginPacket::I_USERNAME])){ + if(!Player::isValidUserName($packet->extraData->displayName)){ $this->session->disconnect("disconnectionScreen.invalidName"); return true; } try{ + $clientData = $packet->clientData; //this serves no purpose except readability /** @var SkinAnimation[] $animations */ $animations = []; - foreach($packet->clientData[LoginPacket::I_ANIMATION_IMAGES] as $animation){ + foreach($clientData->AnimatedImageData as $animation){ $animations[] = new SkinAnimation( new SkinImage( - $animation[LoginPacket::I_ANIMATION_IMAGE_HEIGHT], - $animation[LoginPacket::I_ANIMATION_IMAGE_WIDTH], - base64_decode($animation[LoginPacket::I_ANIMATION_IMAGE_DATA], true) + $animation->ImageHeight, + $animation->ImageWidth, + base64_decode($animation->Image, true) ), - $animation[LoginPacket::I_ANIMATION_IMAGE_TYPE], - $animation[LoginPacket::I_ANIMATION_IMAGE_FRAMES] + $animation->Type, + $animation->Frames ); } $skinData = new SkinData( - $packet->clientData[LoginPacket::I_SKIN_ID], - base64_decode($packet->clientData[LoginPacket::I_SKIN_RESOURCE_PATCH], true), - new SkinImage($packet->clientData[LoginPacket::I_SKIN_HEIGHT], $packet->clientData[LoginPacket::I_SKIN_WIDTH], base64_decode($packet->clientData[LoginPacket::I_SKIN_DATA], true)), + $clientData->SkinId, + base64_decode($clientData->SkinResourcePatch, true), + new SkinImage($clientData->SkinImageHeight, $clientData->SkinImageWidth, base64_decode($clientData->SkinData, true)), $animations, - new SkinImage($packet->clientData[LoginPacket::I_CAPE_HEIGHT], $packet->clientData[LoginPacket::I_CAPE_WIDTH], base64_decode($packet->clientData[LoginPacket::I_CAPE_DATA], true)), - base64_decode($packet->clientData[LoginPacket::I_GEOMETRY_DATA], true), - base64_decode($packet->clientData[LoginPacket::I_ANIMATION_DATA], true), - $packet->clientData[LoginPacket::I_PREMIUM_SKIN], - $packet->clientData[LoginPacket::I_PERSONA_SKIN], - $packet->clientData[LoginPacket::I_PERSONA_CAPE_ON_CLASSIC_SKIN], - $packet->clientData[LoginPacket::I_CAPE_ID] + new SkinImage($clientData->CapeImageHeight, $clientData->CapeImageWidth, base64_decode($clientData->CapeData, true)), + base64_decode($clientData->SkinGeometryData, true), + base64_decode($clientData->SkinAnimationData, true), + $clientData->PremiumSkin, + $clientData->PersonaSkin, + $clientData->CapeOnClassicSkin, + $clientData->CapeId ); $skin = SkinAdapterSingleton::get()->fromSkinData($skinData); @@ -111,12 +112,12 @@ class LoginPacketHandler extends PacketHandler{ } $this->session->setPlayerInfo(new PlayerInfo( - $packet->extraData[LoginPacket::I_USERNAME], - UUID::fromString($packet->extraData[LoginPacket::I_UUID]), + $packet->extraData->displayName, + UUID::fromString($packet->extraData->identity), $skin, - $packet->clientData[LoginPacket::I_LANGUAGE_CODE], - $packet->extraData[LoginPacket::I_XUID], - $packet->clientData + $packet->clientData->LanguageCode, + $packet->extraData->XUID, + (array) $packet->clientData )); $ev = new PlayerPreLoginEvent( diff --git a/src/network/mcpe/protocol/LoginPacket.php b/src/network/mcpe/protocol/LoginPacket.php index af33b46f6..f2bc7c23e 100644 --- a/src/network/mcpe/protocol/LoginPacket.php +++ b/src/network/mcpe/protocol/LoginPacket.php @@ -25,77 +25,34 @@ namespace pocketmine\network\mcpe\protocol; #include -use Particle\Validator\Validator; use pocketmine\network\BadPacketException; use pocketmine\network\mcpe\handler\PacketHandler; +use pocketmine\network\mcpe\protocol\types\login\AuthenticationData; +use pocketmine\network\mcpe\protocol\types\login\ClientData; +use pocketmine\network\mcpe\protocol\types\login\JwtChain; use pocketmine\network\mcpe\serializer\NetworkBinaryStream; use pocketmine\utils\BinaryDataException; use pocketmine\utils\BinaryStream; use pocketmine\utils\Utils; -use function array_filter; -use function count; -use function implode; use function is_array; use function json_decode; -use function json_last_error_msg; class LoginPacket extends DataPacket implements ServerboundPacket{ public const NETWORK_ID = ProtocolInfo::LOGIN_PACKET; public const EDITION_POCKET = 0; - public const I_USERNAME = 'displayName'; - public const I_UUID = 'identity'; - public const I_XUID = 'XUID'; - - public const I_CLIENT_RANDOM_ID = 'ClientRandomId'; - public const I_SERVER_ADDRESS = 'ServerAddress'; - public const I_LANGUAGE_CODE = 'LanguageCode'; - - public const I_SKIN_RESOURCE_PATCH = 'SkinResourcePatch'; - - public const I_SKIN_ID = 'SkinId'; - public const I_SKIN_HEIGHT = 'SkinImageHeight'; - public const I_SKIN_WIDTH = 'SkinImageWidth'; - public const I_SKIN_DATA = 'SkinData'; - - public const I_CAPE_ID = 'CapeId'; - public const I_CAPE_HEIGHT = 'CapeImageHeight'; - public const I_CAPE_WIDTH = 'CapeImageWidth'; - public const I_CAPE_DATA = 'CapeData'; - - public const I_GEOMETRY_DATA = 'SkinGeometryData'; - - public const I_ANIMATION_DATA = 'SkinAnimationData'; - public const I_ANIMATION_IMAGES = 'AnimatedImageData'; - - public const I_ANIMATION_IMAGE_HEIGHT = 'ImageHeight'; - public const I_ANIMATION_IMAGE_WIDTH = 'ImageWidth'; - public const I_ANIMATION_IMAGE_FRAMES = 'Frames'; - public const I_ANIMATION_IMAGE_TYPE = 'Type'; - public const I_ANIMATION_IMAGE_DATA = 'Image'; - - public const I_PREMIUM_SKIN = 'PremiumSkin'; - public const I_PERSONA_SKIN = 'PersonaSkin'; - public const I_PERSONA_CAPE_ON_CLASSIC_SKIN = 'CapeOnClassicSkin'; - /** @var int */ public $protocol; - /** @var string[] array of encoded JWT */ - public $chainDataJwt = []; - /** - * @var mixed[]|null extraData index of whichever JWT has it - * @phpstan-var array - */ + /** @var JwtChain */ + public $chainDataJwt; + /** @var AuthenticationData|null extraData index of whichever JWT has it */ public $extraData = null; /** @var string */ public $clientDataJwt; - /** - * @var mixed[] decoded payload of the clientData JWT - * @phpstan-var array - */ - public $clientData = []; + /** @var ClientData decoded payload of the clientData JWT */ + public $clientData; /** * This field may be used by plugins to bypass keychain verification. It should only be used for plugins such as @@ -114,22 +71,6 @@ class LoginPacket extends DataPacket implements ServerboundPacket{ $this->decodeConnectionRequest($in); } - /** - * @param mixed $data - * - * @throws BadPacketException - */ - private static function validate(Validator $v, string $name, $data) : void{ - $result = $v->validate($data); - if($result->isNotValid()){ - $messages = []; - foreach($result->getFailures() as $f){ - $messages[] = $f->format(); - } - throw new BadPacketException("Failed to validate '$name': " . implode(", ", $messages)); - } - } - /** * @throws BadPacketException * @throws BinaryDataException @@ -137,19 +78,19 @@ class LoginPacket extends DataPacket implements ServerboundPacket{ protected function decodeConnectionRequest(NetworkBinaryStream $in) : void{ $buffer = new BinaryStream($in->getString()); - $chainData = json_decode($buffer->get($buffer->getLInt()), true); - if(!is_array($chainData)){ - throw new BadPacketException("Failed to decode chainData JSON: " . json_last_error_msg()); + $chainDataJson = json_decode($buffer->get($buffer->getLInt())); + $mapper = new \JsonMapper; + $mapper->bExceptionOnMissingData = true; + $mapper->bExceptionOnUndefinedProperty = true; + try{ + $chainData = $mapper->map($chainDataJson, new JwtChain); + }catch(\JsonMapper_Exception $e){ + throw BadPacketException::wrap($e); } - $vd = new Validator(); - $vd->required('chain')->isArray()->callback(function(array $data) : bool{ - return count($data) <= 3 and count(array_filter($data, '\is_string')) === count($data); - }); - self::validate($vd, "chainData", $chainData); + $this->chainDataJwt = $chainData; - $this->chainDataJwt = $chainData['chain']; - foreach($this->chainDataJwt as $k => $chain){ + foreach($this->chainDataJwt->chain as $k => $chain){ //validate every chain element try{ $claims = Utils::getJwtClaims($chain); @@ -164,13 +105,15 @@ class LoginPacket extends DataPacket implements ServerboundPacket{ throw new BadPacketException("Found 'extraData' more than once in chainData"); } - $extraV = new Validator(); - $extraV->required(self::I_USERNAME)->string(); - $extraV->required(self::I_UUID)->uuid(); - $extraV->required(self::I_XUID)->string()->digits()->allowEmpty(true); - self::validate($extraV, "chain.$k.extraData", $claims['extraData']); - - $this->extraData = $claims['extraData']; + $mapper = new \JsonMapper; + $mapper->bEnforceMapType = false; //TODO: we don't really need this as an array, but right now we don't have enough models + $mapper->bExceptionOnMissingData = true; + $mapper->bExceptionOnUndefinedProperty = true; + try{ + $this->extraData = $mapper->map($claims['extraData'], new AuthenticationData); + }catch(\JsonMapper_Exception $e){ + throw BadPacketException::wrap($e); + } } } if($this->extraData === null){ @@ -184,40 +127,15 @@ class LoginPacket extends DataPacket implements ServerboundPacket{ throw new BadPacketException($e->getMessage(), 0, $e); } - $v = new Validator(); - $v->required(self::I_CLIENT_RANDOM_ID)->integer(); - $v->required(self::I_SERVER_ADDRESS)->string(); - $v->required(self::I_LANGUAGE_CODE)->string(); - - $v->required(self::I_SKIN_RESOURCE_PATCH)->string(); - - $v->required(self::I_SKIN_ID)->string(); - $v->required(self::I_SKIN_DATA)->string(); - $v->required(self::I_SKIN_HEIGHT)->integer(true); - $v->required(self::I_SKIN_WIDTH)->integer(true); - - $v->required(self::I_CAPE_ID, null, true)->string(); - $v->required(self::I_CAPE_DATA, null, true)->string(); - $v->required(self::I_CAPE_HEIGHT)->integer(true); - $v->required(self::I_CAPE_WIDTH)->integer(true); - - $v->required(self::I_GEOMETRY_DATA, null, true)->string(); - - $v->required(self::I_ANIMATION_DATA, null, true)->string(); - $v->required(self::I_ANIMATION_IMAGES, null, true)->isArray()->each(function(Validator $vSub) : void{ - $vSub->required(self::I_ANIMATION_IMAGE_HEIGHT)->integer(true); - $vSub->required(self::I_ANIMATION_IMAGE_WIDTH)->integer(true); - $vSub->required(self::I_ANIMATION_IMAGE_FRAMES)->numeric(); //float() doesn't accept ints ??? - $vSub->required(self::I_ANIMATION_IMAGE_TYPE)->integer(true); - $vSub->required(self::I_ANIMATION_IMAGE_DATA)->string(); - }); - $v->required(self::I_PREMIUM_SKIN)->bool(); - $v->required(self::I_PERSONA_SKIN)->bool(); - $v->required(self::I_PERSONA_CAPE_ON_CLASSIC_SKIN)->bool(); - - self::validate($v, 'clientData', $clientData); - - $this->clientData = $clientData; + $mapper = new \JsonMapper; + $mapper->bEnforceMapType = false; //TODO: we don't really need this as an array, but right now we don't have enough models + $mapper->bExceptionOnMissingData = true; + $mapper->bExceptionOnUndefinedProperty = true; + try{ + $this->clientData = $mapper->map($clientData, new ClientData); + }catch(\JsonMapper_Exception $e){ + throw BadPacketException::wrap($e); + } } protected function encodePayload(NetworkBinaryStream $out) : void{ diff --git a/src/network/mcpe/protocol/types/login/AuthenticationData.php b/src/network/mcpe/protocol/types/login/AuthenticationData.php new file mode 100644 index 000000000..6e48143d2 --- /dev/null +++ b/src/network/mcpe/protocol/types/login/AuthenticationData.php @@ -0,0 +1,51 @@ +