Skip to content

Commit

Permalink
Implement JWT signature verification and Xbox Live checks, close #315
Browse files Browse the repository at this point in the history
This can be enabled or disabled using the "online-mode" directive in
server.properties.

NOTE: For safety reasons it is enabled by default, since many naive server owners currently believe that authentication is not needed because "the client is forced to sign-in".
Newsflash for readers: the forced authentication is easily bypassed using a LAN proxy.

Un-authenticated LAN connections will still work fine if the online mode is disabled.

Added the following API methods:
- Server->getOnlineMode() : bool
- Server->requiresAuthentication() : bool
- Player->isAuthenticated() : bool

JWT verification is rather expensive, so it is done in an AsyncTask. Make sure you don't hog your worker threads.
  • Loading branch information
dktapps committed Sep 25, 2017
1 parent 8ca59d1 commit 03d3e59
Show file tree
Hide file tree
Showing 4 changed files with 222 additions and 12 deletions.
53 changes: 43 additions & 10 deletions src/pocketmine/Player.php
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@
use pocketmine\network\mcpe\protocol\types\PlayerPermissions;
use pocketmine\network\mcpe\protocol\UpdateAttributesPacket;
use pocketmine\network\mcpe\protocol\UpdateBlockPacket;
use pocketmine\network\mcpe\VerifyLoginTask;
use pocketmine\network\SourceInterface;
use pocketmine\permission\PermissibleBase;
use pocketmine\permission\PermissionAttachment;
Expand All @@ -157,6 +158,7 @@
use pocketmine\tile\Spawnable;
use pocketmine\tile\Tile;
use pocketmine\utils\TextFormat;
use pocketmine\utils\Utils;
use pocketmine\utils\UUID;


Expand Down Expand Up @@ -209,6 +211,8 @@ public static function isValidSkin(string $skin) : bool{
public $joined = false;
public $gamemode;
public $lastBreak;
/** @var bool */
protected $authenticated = false;

protected $windowCnt = 2;
/** @var \SplObjectStorage<Inventory> */
Expand Down Expand Up @@ -352,6 +356,10 @@ public function setWhitelisted(bool $value){
}
}

public function isAuthenticated() : bool{
return $this->authenticated;
}

public function getPlayer(){
return $this;
}
Expand Down Expand Up @@ -1788,18 +1796,34 @@ protected function initEntity(){
$this->addDefaultWindows();
}


protected function processLogin(){
if(!$this->server->isWhitelisted($this->iusername)){
$this->close($this->getLeaveMessage(), "Server is white-listed");

public function onVerifyCompleted(LoginPacket $packet, bool $isValid, bool $isAuthenticated) : void{
if($this->closed){
return;
}elseif($this->server->getNameBans()->isBanned($this->iusername) or $this->server->getIPBans()->isBanned($this->getAddress())){
$this->close($this->getLeaveMessage(), "You are banned");
}

if(!$isValid){
$this->close("", "disconnect.loginFailedInfo.invalidSession");
return;
}

$this->authenticated = $isAuthenticated;

if(!$isAuthenticated){
if($this->server->requiresAuthentication() and $this->kick("disconnectionScreen.notAuthenticated", false)){ //use kick to allow plugins to cancel this
return;
}else{
$this->server->getLogger()->debug($this->getName() . " is NOT logged into to Xbox Live");
}
}else{
$this->server->getLogger()->debug($this->getName() . " is logged into Xbox Live");
}

//TODO: get data from loginpacket (xbox user ID and stuff), add events

$this->processLogin();
}

protected function processLogin(){
foreach($this->server->getLoggedInPlayers() as $p){
if($p !== $this and $p->iusername === $this->iusername){
if($p->kick("logged in from another location") === false){
Expand Down Expand Up @@ -1985,16 +2009,25 @@ public function handleLogin(LoginPacket $packet) : bool{

$this->setSkin($packet->skin, $packet->skinId);

if(!$this->server->isWhitelisted($this->iusername) and $this->kick("Server is white-listed", false)){
return true;
}

if(
($this->server->getNameBans()->isBanned($this->iusername) or $this->server->getIPBans()->isBanned($this->getAddress())) and
$this->kick("You are banned", false)
){
return true;
}

$this->server->getPluginManager()->callEvent($ev = new PlayerPreLoginEvent($this, "Plugin reason"));
if($ev->isCancelled()){
$this->close("", $ev->getKickMessage());

return true;
}

//TODO: add JWT verification, add encryption

$this->processLogin();
$this->server->getScheduler()->scheduleAsyncTask(new VerifyLoginTask($this, $packet));

return true;
}
Expand Down
35 changes: 34 additions & 1 deletion src/pocketmine/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,9 @@ class Server{
/** @var int */
private $maxPlayers;

/** @var bool */
private $onlineMode = true;

/** @var bool */
private $autoSave;

Expand Down Expand Up @@ -339,6 +342,24 @@ public function getMaxPlayers() : int{
return $this->maxPlayers;
}

/**
* Returns whether the server requires that players be authenticated to Xbox Live. If true, connecting players who
* are not logged into Xbox Live will be disconnected.
*
* @return bool
*/
public function getOnlineMode() : bool{
return $this->onlineMode;
}

/**
* Alias of {@link #getOnlineMode()}.
* @return bool
*/
public function requiresAuthentication() : bool{
return $this->getOnlineMode();
}

/**
* @return int
*/
Expand Down Expand Up @@ -1484,7 +1505,8 @@ public function __construct(\ClassLoader $autoloader, \ThreadedLogger $logger, s
"enable-rcon" => false,
"rcon.password" => substr(base64_encode(random_bytes(20)), 3, 10),
"auto-save" => true,
"view-distance" => 8
"view-distance" => 8,
"online-mode" => true
]);

$this->forceLanguage = $this->getProperty("settings.force-language", false);
Expand Down Expand Up @@ -1557,6 +1579,16 @@ public function __construct(\ClassLoader $autoloader, \ThreadedLogger $logger, s
$this->maxPlayers = $this->getConfigInt("max-players", 20);
$this->setAutoSave($this->getConfigBoolean("auto-save", true));

$this->onlineMode = $this->getConfigBoolean("online-mode", true);
if($this->onlineMode){
$this->logger->notice($this->getLanguage()->translateString("pocketmine.server.auth", ["enabled", "will"]));

This comment has been minimized.

Copy link
@Johnmacrocraft

Johnmacrocraft Sep 26, 2017

Contributor

Wait, if the server manually returns words like enabled, how they can be translated?

This comment has been minimized.

Copy link
@dktapps

dktapps Sep 26, 2017

Author Member

welp

$this->logger->notice($this->getLanguage()->translateString("pocketmine.server.authProperty", ["disable", "false"]));
}else{
$this->logger->warning($this->getLanguage()->translateString("pocketmine.server.auth", ["disabled", "will not"]));
$this->logger->warning($this->getLanguage()->translateString("pocketmine.server.authWarning"));
$this->logger->warning($this->getLanguage()->translateString("pocketmine.server.authProperty", ["enable", "true"]));
}

if($this->getConfigBoolean("hardcore", false) === true and $this->getDifficulty() < 3){
$this->setConfigInt("difficulty", 3);
}
Expand Down Expand Up @@ -1584,6 +1616,7 @@ public function __construct(\ClassLoader $autoloader, \ThreadedLogger $logger, s
]));
$this->logger->info($this->getLanguage()->translateString("pocketmine.server.license", [$this->getName()]));


Timings::init();

$this->consoleSender = new ConsoleCommandSender();
Expand Down
2 changes: 1 addition & 1 deletion src/pocketmine/lang/locale
144 changes: 144 additions & 0 deletions src/pocketmine/network/mcpe/VerifyLoginTask.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
<?php

/*
*
* ____ _ _ __ __ _ __ __ ____
* | _ \ ___ ___| | _____| |_| \/ (_)_ __ ___ | \/ | _ \
* | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
* | __/ (_) | (__| < __/ |_| | | | | | | | __/_____| | | | __/
* |_| \___/ \___|_|\_\___|\__|_| |_|_|_| |_|\___| |_| |_|_|
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* @author PocketMine Team
* @link http://www.pocketmine.net/
*
*
*/

declare(strict_types=1);

namespace pocketmine\network\mcpe;

use pocketmine\network\mcpe\protocol\LoginPacket;
use pocketmine\Player;
use pocketmine\scheduler\AsyncTask;
use pocketmine\Server;
use pocketmine\utils\MainLogger;

class VerifyLoginTask extends AsyncTask{

const MOJANG_ROOT_PUBLIC_KEY = "MHYwEAYHKoZIzj0CAQYFK4EEACIDYgAE8ELkixyLcwlZryUQcu1TvPOmI2B7vX83ndnWRUaXm74wFfa5f/lwQNTfrLVHa2PmenpGI6JhIMUJaWZrjmMj90NoKNFSNBuKdm8rYiXsfaz3K36x/1U26HpG0ZxK/V1V";

/** @var LoginPacket */
private $packet;

/**
* @var bool
* Whether the keychain signatures were validated correctly. This will be set to false if any link in the keychain
* has an invalid signature. If false, the keychain might have been tampered with.
* The player will always be disconnected if this is false.
*/
private $valid = true;
/**
* @var bool
* Whether the player is logged into Xbox Live. This is true if any link in the keychain is signed with the Mojang
* root public key.
*/
private $authenticated = false;


public function __construct(Player $player, LoginPacket $packet){
$this->storeLocal($player);
$this->packet = $packet;
}

public function onRun(){
$packet = $this->packet; //Get it in a local variable to make sure it stays unserialized

$currentKey = null;

foreach($packet->chainData["chain"] as $jwt){
if(!$this->validateToken($jwt, $currentKey)){
$this->valid = false;

return;
}
}

if(!$this->validateToken($packet->clientDataJwt, $currentKey)){
$this->valid = false;
}
}

private function validateToken(string $jwt, ?string &$currentPublicKey) : bool{
[$headB64, $payloadB64, $sigB64] = explode('.', $jwt);

$headers = json_decode(base64_decode(strtr($headB64, '-_', '+/'), true), true);

if($currentPublicKey === null){ //First link, check that it is self-signed
$currentPublicKey = $headers["x5u"];
}

$plainSignature = base64_decode(strtr($sigB64, '-_', '+/'), true);

//OpenSSL wants a DER-encoded signature, so we extract R and S from the plain signature and crudely serialize it.

assert(strlen($plainSignature) === 96);

[$rString, $sString] = str_split($plainSignature, 48);

$rString = ltrim($rString, "\x00");
if(ord($rString{0}) >= 128){ //Would be considered signed, pad it with an extra zero
$rString = "\x00" . $rString;
}

$sString = ltrim($sString, "\x00");
if(ord($sString{0}) >= 128){ //Would be considered signed, pad it with an extra zero
$sString = "\x00" . $sString;
}

//0x02 = Integer ASN.1 tag
$sequence = "\x02" . chr(strlen($rString)) . $rString . "\x02" . chr(strlen($sString)) . $sString;
//0x30 = Sequence ASN.1 tag
$derSignature = "\x30" . chr(strlen($sequence)) . $sequence;

$v = openssl_verify("$headB64.$payloadB64", $derSignature, "-----BEGIN PUBLIC KEY-----\n" . wordwrap($currentPublicKey, 64, "\n", true) . "\n-----END PUBLIC KEY-----\n", OPENSSL_ALGO_SHA384);
if($v !== 1){
return false; //bad signature, it might have been tampered with
}

if($currentPublicKey === self::MOJANG_ROOT_PUBLIC_KEY){
$this->authenticated = true; //we're signed into xbox live
}

$claims = json_decode(base64_decode(strtr($payloadB64, '-_', '+/'), true), true);

This comment has been minimized.

Copy link
@Muqsit

Muqsit Sep 25, 2017

Member

Why not Utils::decodeJWT() (for the $headers maybe)?
Thanks btw :D

This comment has been minimized.

Copy link
@dktapps

dktapps Sep 25, 2017

Author Member

that method takes the whole token, doesn't make sense to do that when we already have parts here.

This implementation isn't ideal because it will currently only handle ES384 and most stuff is hardcoded. At a later date I'll write a separate library for it.

For those wondering why I didn't use a preexisting library, the only lightweight implementations were too slow due to depending on pure PHP crypto (PHPECC). This version using OpenSSL is over 20 times faster than using, let's say, lcobucci/jwt.

This comment has been minimized.

Copy link
@geNAZt

geNAZt Sep 26, 2017

Contributor

Just as a side note: Pinning the needed algo is not a bad thing: https://auth0.com/blog/critical-vulnerabilities-in-json-web-token-libraries/

That way you can avoid that the client sends a less secure token to the server.


$time = time();
if(isset($claims["nbf"]) and $claims["nbf"] > $time){
return false; //token can't be used yet
}

if(isset($claims["exp"]) and $claims["exp"] < $time){
return false; //token has expired
}

$currentPublicKey = $claims["identityPublicKey"]; //the next link should be signed with this

return true;
}

public function onCompletion(Server $server){
/** @var Player $player */
$player = $this->fetchLocal($server);
if($player->isClosed()){
$server->getLogger()->error("Player " . $player->getName() . " was disconnected before their login could be verified");
}else{
$player->onVerifyCompleted($this->packet, $this->valid, $this->authenticated);
}
}

}

0 comments on commit 03d3e59

Please sign in to comment.