NetworkSession: Send less information to clients on error disconnects

in particular, the information from VerifyLoginTask shouldn't be sent to clients, as it could contain sensitive information.
This change only affects disconnection screens. The server log shows the same amount of information as before (though formatted differently in some cases).
This commit is contained in:
Dylan K. Taylor 2023-11-29 16:31:59 +00:00
parent d596dc571d
commit b2df405cc0
No known key found for this signature in database
GPG Key ID: 8927471A91CAFD3D
4 changed files with 28 additions and 11 deletions

View File

@ -236,8 +236,10 @@ class NetworkSession{
$this->onPlayerCreated(...), $this->onPlayerCreated(...),
function() : void{ function() : void{
//TODO: this should never actually occur... right? //TODO: this should never actually occur... right?
$this->logger->error("Failed to create player"); $this->disconnectWithError(
$this->disconnectWithError(KnownTranslationFactory::pocketmine_disconnect_error_internal()); reason: "Failed to create player",
disconnectScreenMessage: KnownTranslationFactory::pocketmine_disconnect_error_internal()
);
} }
); );
} }
@ -675,8 +677,13 @@ class NetworkSession{
}, $reason); }, $reason);
} }
public function disconnectWithError(Translatable|string $reason) : void{ public function disconnectWithError(Translatable|string $reason, Translatable|string|null $disconnectScreenMessage = null) : void{
$this->disconnect(KnownTranslationFactory::pocketmine_disconnect_error($reason, implode("-", str_split(bin2hex(random_bytes(6)), 4)))); $errorId = implode("-", str_split(bin2hex(random_bytes(6)), 4));
$this->disconnect(
reason: KnownTranslationFactory::pocketmine_disconnect_error($reason, $errorId)->prefix(TextFormat::RED),
disconnectScreenMessage: KnownTranslationFactory::pocketmine_disconnect_error($disconnectScreenMessage ?? $reason, $errorId),
);
} }
public function disconnectIncompatibleProtocol(int $protocolVersion) : void{ public function disconnectIncompatibleProtocol(int $protocolVersion) : void{
@ -735,7 +742,10 @@ class NetworkSession{
} }
if($error !== null){ if($error !== null){
$this->disconnectWithError(KnownTranslationFactory::pocketmine_disconnect_invalidSession($error)); $this->disconnectWithError(
reason: KnownTranslationFactory::pocketmine_disconnect_invalidSession($error),
disconnectScreenMessage: KnownTranslationFactory::pocketmine_disconnect_error_authentication()
);
return; return;
} }

View File

@ -73,8 +73,10 @@ class LoginPacketHandler extends PacketHandler{
try{ try{
$skin = $this->session->getTypeConverter()->getSkinAdapter()->fromSkinData(ClientDataToSkinDataHelper::fromClientData($clientData)); $skin = $this->session->getTypeConverter()->getSkinAdapter()->fromSkinData(ClientDataToSkinDataHelper::fromClientData($clientData));
}catch(\InvalidArgumentException | InvalidSkinException $e){ }catch(\InvalidArgumentException | InvalidSkinException $e){
$this->session->getLogger()->debug("Invalid skin: " . $e->getMessage()); $this->session->disconnectWithError(
$this->session->disconnectWithError(KnownTranslationFactory::disconnectionScreen_invalidSkin()); reason: "Invalid skin: " . $e->getMessage(),
disconnectScreenMessage: KnownTranslationFactory::disconnectionScreen_invalidSkin()
);
return true; return true;
} }

View File

@ -85,8 +85,10 @@ class ResourcePacksPacketHandler extends PacketHandler{
} }
private function disconnectWithError(string $error) : void{ private function disconnectWithError(string $error) : void{
$this->session->getLogger()->error("Error downloading resource packs: " . $error); $this->session->disconnectWithError(
$this->session->disconnectWithError(KnownTranslationFactory::disconnectionScreen_resourcePack()); reason: "Error downloading resource packs: " . $error,
disconnectScreenMessage: KnownTranslationFactory::disconnectionScreen_resourcePack()
);
} }
public function handleResourcePackClientResponse(ResourcePackClientResponsePacket $packet) : bool{ public function handleResourcePackClientResponse(ResourcePackClientResponsePacket $packet) : bool{

View File

@ -219,11 +219,14 @@ class RakLibInterface implements ServerEventListener, AdvancedNetworkInterface{
$session->handleEncoded($buf); $session->handleEncoded($buf);
}catch(PacketHandlingException $e){ }catch(PacketHandlingException $e){
$logger = $session->getLogger(); $logger = $session->getLogger();
$logger->error("Bad packet: " . $e->getMessage());
$session->disconnectWithError(
reason: "Bad packet: " . $e->getMessage(),
disconnectScreenMessage: KnownTranslationFactory::pocketmine_disconnect_error_badPacket()
);
//intentionally doesn't use logException, we don't want spammy packet error traces to appear in release mode //intentionally doesn't use logException, we don't want spammy packet error traces to appear in release mode
$logger->debug(implode("\n", Utils::printableExceptionInfo($e))); $logger->debug(implode("\n", Utils::printableExceptionInfo($e)));
$session->disconnectWithError(KnownTranslationFactory::pocketmine_disconnect_error_badPacket());
$this->interface->blockAddress($address, 5); $this->interface->blockAddress($address, 5);
}catch(\Throwable $e){ }catch(\Throwable $e){
//record the name of the player who caused the crash, to make it easier to find the reproducing steps //record the name of the player who caused the crash, to make it easier to find the reproducing steps