From 31e4fc6fcb50a3e0587de18b67a82b7177a16121 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Fri, 15 May 2020 10:55:29 +0100 Subject: [PATCH] fixing incompatible protocol handling, do not explode immediately on bad clientdata, login encode/decode is now symmetrical --- .../mcpe/handler/LoginPacketHandler.php | 83 +++++++++++++++-- src/network/mcpe/protocol/LoginPacket.php | 91 +++++-------------- 2 files changed, 101 insertions(+), 73 deletions(-) diff --git a/src/network/mcpe/handler/LoginPacketHandler.php b/src/network/mcpe/handler/LoginPacketHandler.php index efd3e4d9a..3606152f8 100644 --- a/src/network/mcpe/handler/LoginPacketHandler.php +++ b/src/network/mcpe/handler/LoginPacketHandler.php @@ -28,12 +28,17 @@ use pocketmine\event\player\PlayerPreLoginEvent; use pocketmine\network\BadPacketException; use pocketmine\network\mcpe\auth\ProcessLoginTask; use pocketmine\network\mcpe\convert\SkinAdapterSingleton; +use pocketmine\network\mcpe\JwtException; +use pocketmine\network\mcpe\JwtUtils; use pocketmine\network\mcpe\NetworkSession; use pocketmine\network\mcpe\protocol\LoginPacket; use pocketmine\network\mcpe\protocol\PlayStatusPacket; use pocketmine\network\mcpe\protocol\ProtocolInfo; +use pocketmine\network\mcpe\protocol\types\login\AuthenticationData; +use pocketmine\network\mcpe\protocol\types\login\ClientData; use pocketmine\network\mcpe\protocol\types\login\ClientDataPersonaPieceTintColor; use pocketmine\network\mcpe\protocol\types\login\ClientDataPersonaSkinPiece; +use pocketmine\network\mcpe\protocol\types\login\JwtChain; use pocketmine\network\mcpe\protocol\types\PersonaPieceTintColor; use pocketmine\network\mcpe\protocol\types\PersonaSkinPiece; use pocketmine\network\mcpe\protocol\types\SkinAnimation; @@ -45,6 +50,7 @@ use pocketmine\Server; use pocketmine\uuid\UUID; use function array_map; use function base64_decode; +use function is_array; /** * Handles the initial login phase of the session. This handler is used as the initial state. @@ -94,12 +100,15 @@ class LoginPacketHandler extends PacketHandler{ return true; } - if(!Player::isValidUserName($packet->extraData->displayName)){ + $extraData = $this->fetchAuthData($packet->chainDataJwt); + + if(!Player::isValidUserName($extraData->displayName)){ $this->session->disconnect("disconnectionScreen.invalidName"); return true; } + $clientData = $this->parseClientData($packet->clientDataJwt); $safeB64Decode = static function(string $base64, string $context) : string{ $result = base64_decode($base64, true); if($result === false){ @@ -108,7 +117,6 @@ class LoginPacketHandler extends PacketHandler{ return $result; }; try{ - $clientData = $packet->clientData; //this serves no purpose except readability /** @var SkinAnimation[] $animations */ $animations = []; foreach($clientData->AnimatedImageData as $k => $animation){ @@ -154,17 +162,17 @@ class LoginPacketHandler extends PacketHandler{ } try{ - $uuid = UUID::fromString($packet->extraData->identity); + $uuid = UUID::fromString($extraData->identity); }catch(\InvalidArgumentException $e){ throw BadPacketException::wrap($e, "Failed to parse login UUID"); } ($this->playerInfoConsumer)(new PlayerInfo( - $packet->extraData->displayName, + $extraData->displayName, $uuid, $skin, - $packet->clientData->LanguageCode, - $packet->extraData->XUID, - (array) $packet->clientData + $clientData->LanguageCode, + $extraData->XUID, + (array) $clientData )); $ev = new PlayerPreLoginEvent( @@ -194,6 +202,67 @@ class LoginPacketHandler extends PacketHandler{ return true; } + /** + * @throws BadPacketException + */ + protected function fetchAuthData(JwtChain $chain) : AuthenticationData{ + /** @var AuthenticationData|null $extraData */ + $extraData = null; + foreach($chain->chain as $k => $chain){ + //validate every chain element + try{ + [, $claims, ] = JwtUtils::parse($chain); + }catch(JwtException $e){ + throw BadPacketException::wrap($e); + } + if(isset($claims["extraData"])){ + if($extraData !== null){ + throw new BadPacketException("Found 'extraData' more than once in chainData"); + } + + if(!is_array($claims["extraData"])){ + throw new BadPacketException("'extraData' key should be an array"); + } + $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{ + /** @var AuthenticationData $extraData */ + $extraData = $mapper->map($claims['extraData'], new AuthenticationData); + }catch(\JsonMapper_Exception $e){ + throw BadPacketException::wrap($e); + } + } + } + if($extraData === null){ + throw new BadPacketException("'extraData' not found in chain data"); + } + return $extraData; + } + + /** + * @throws BadPacketException + */ + protected function parseClientData(string $clientDataJwt) : ClientData{ + try{ + [, $clientDataClaims, ] = JwtUtils::parse($clientDataJwt); + }catch(JwtException $e){ + throw BadPacketException::wrap($e); + } + + $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{ + $clientData = $mapper->map($clientDataClaims, new ClientData); + }catch(\JsonMapper_Exception $e){ + throw BadPacketException::wrap($e); + } + return $clientData; + } + /** * TODO: This is separated for the purposes of allowing plugins (like Specter) to hack it and bypass authentication. * In the future this won't be necessary. diff --git a/src/network/mcpe/protocol/LoginPacket.php b/src/network/mcpe/protocol/LoginPacket.php index c018f6582..35e838745 100644 --- a/src/network/mcpe/protocol/LoginPacket.php +++ b/src/network/mcpe/protocol/LoginPacket.php @@ -25,18 +25,14 @@ namespace pocketmine\network\mcpe\protocol; #include -use pocketmine\network\mcpe\JwtException; -use pocketmine\network\mcpe\JwtUtils; use pocketmine\network\mcpe\protocol\serializer\NetworkBinaryStream; -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\utils\BinaryDataException; use pocketmine\utils\BinaryStream; -use function is_array; use function is_object; use function json_decode; +use function json_encode; use function json_last_error_msg; +use function strlen; class LoginPacket extends DataPacket implements ServerboundPacket{ public const NETWORK_ID = ProtocolInfo::LOGIN_PACKET; @@ -48,12 +44,8 @@ class LoginPacket extends DataPacket implements ServerboundPacket{ /** @var JwtChain */ public $chainDataJwt; - /** @var AuthenticationData|null extraData index of whichever JWT has it */ - public $extraData = null; /** @var string */ public $clientDataJwt; - /** @var ClientData decoded payload of the clientData JWT */ - public $clientData; public function canBeSentBeforeLogin() : bool{ return true; @@ -61,17 +53,13 @@ class LoginPacket extends DataPacket implements ServerboundPacket{ protected function decodePayload(NetworkBinaryStream $in) : void{ $this->protocol = $in->getInt(); - $this->decodeConnectionRequest($in); + $this->decodeConnectionRequest($in->getString()); } - /** - * @throws PacketDecodeException - * @throws BinaryDataException - */ - protected function decodeConnectionRequest(NetworkBinaryStream $in) : void{ - $buffer = new BinaryStream($in->getString()); + protected function decodeConnectionRequest(string $binary) : void{ + $connRequestReader = new BinaryStream($binary); - $chainDataJson = json_decode($buffer->get($buffer->getLInt())); + $chainDataJson = json_decode($connRequestReader->get($connRequestReader->getLInt())); if(!is_object($chainDataJson)){ throw new PacketDecodeException("Failed decoding chain data JSON: " . json_last_error_msg()); } @@ -85,57 +73,28 @@ class LoginPacket extends DataPacket implements ServerboundPacket{ } $this->chainDataJwt = $chainData; - - foreach($this->chainDataJwt->chain as $k => $chain){ - //validate every chain element - try{ - [, $claims, ] = JwtUtils::parse($chain); - }catch(JwtException $e){ - throw new PacketDecodeException($e->getMessage(), 0, $e); - } - if(isset($claims["extraData"])){ - if(!is_array($claims["extraData"])){ - throw new PacketDecodeException("'extraData' key should be an array"); - } - if($this->extraData !== null){ - throw new PacketDecodeException("Found 'extraData' more than once in chainData"); - } - - $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 PacketDecodeException::wrap($e); - } - } - } - if($this->extraData === null){ - throw new PacketDecodeException("'extraData' not found in chain data"); - } - - $this->clientDataJwt = $buffer->get($buffer->getLInt()); - try{ - [, $clientData, ] = JwtUtils::parse($this->clientDataJwt); - }catch(JwtException $e){ - throw new PacketDecodeException($e->getMessage(), 0, $e); - } - - $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 PacketDecodeException::wrap($e); - } + $this->clientDataJwt = $connRequestReader->get($connRequestReader->getLInt()); } protected function encodePayload(NetworkBinaryStream $out) : void{ - //TODO + $out->putInt($this->protocol); + $out->putString($this->encodeConnectionRequest()); + } + + protected function encodeConnectionRequest() : string{ + $connRequestWriter = new BinaryStream(); + + $chainDataJson = json_encode($this->chainDataJwt); + if($chainDataJson === false){ + throw new \InvalidStateException("Failed to encode chain data JSON: " . json_last_error_msg()); + } + $connRequestWriter->putLInt(strlen($chainDataJson)); + $connRequestWriter->put($chainDataJson); + + $connRequestWriter->putLInt(strlen($this->clientDataJwt)); + $connRequestWriter->put($this->clientDataJwt); + + return $connRequestWriter->getBuffer(); } public function handle(PacketHandlerInterface $handler) : bool{