diff --git a/changelogs/4.23.md b/changelogs/4.23.md index 6a5c5d49a..c87364d67 100644 --- a/changelogs/4.23.md +++ b/changelogs/4.23.md @@ -17,3 +17,10 @@ Consider using the `mcpe-protocol` directive in `plugin.yml` as a constraint if ## Fixes - Fixed Docker image build failure due to outdated `build/php` submodule. + +# 4.23.1 +Released 14th July 2023. + +## Fixes +- Hardened validation of JWT signing keys in `LoginPacket`. +- Fixed server crash due to a bug in upstream dependency [`netresearch/jsonmapper`](https://github.com/cweiske/JsonMapper). diff --git a/changelogs/5.3.md b/changelogs/5.3.md index c575cf01b..0a776a11e 100644 --- a/changelogs/5.3.md +++ b/changelogs/5.3.md @@ -21,3 +21,13 @@ If you're upgrading directly from 5.1.x to 5.3.x, please also read the following ## Internals - `BlockTypeNames`, `BlockStateNames`, `BlockStateStringValues` and `ItemTypeNames` in the `pocketmine\data\bedrock` package have BC-breaking changes to accommodate Bedrock 1.20.10. + +# 5.3.1 +Released 14th July 2023. + +## Included releases +**This release includes changes from the following releases:** +- [4.23.1](https://github.com/pmmp/PocketMine-MP/blob/4.23.1/changelogs/4.23.md#4231) - Security fixes + +## General +- Updated `build/php` submodule to pmmp/PHP-Binaries@e0c918d1379465964acefd562d9e48f87cfc2c9e. diff --git a/composer.json b/composer.json index 03b8c5177..4589d73a1 100644 --- a/composer.json +++ b/composer.json @@ -33,7 +33,7 @@ "composer-runtime-api": "^2.0", "adhocore/json-comment": "~1.2.0", "fgrosse/phpasn1": "~2.5.0", - "pocketmine/netresearch-jsonmapper": "~v4.2.999", + "pocketmine/netresearch-jsonmapper": "~v4.2.1000", "pocketmine/bedrock-block-upgrade-schema": "~3.1.0+bedrock-1.20.10", "pocketmine/bedrock-data": "~2.4.0+bedrock-1.20.10", "pocketmine/bedrock-item-upgrade-schema": "~1.4.0+bedrock-1.20.10", diff --git a/composer.lock b/composer.lock index b8a2ac39a..9b9c02c92 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": "e0c0208b3fc3d1b20fef20d2fc43fc90", + "content-hash": "ee46ec27f8dfc8c767527b7776fe9992", "packages": [ { "name": "adhocore/json-comment", @@ -639,16 +639,16 @@ }, { "name": "pocketmine/netresearch-jsonmapper", - "version": "v4.2.999", + "version": "v4.2.1000", "source": { "type": "git", "url": "https://github.com/pmmp/netresearch-jsonmapper.git", - "reference": "f700806dec756ed825a8200dc2950ead98265956" + "reference": "078764e869e9b732f97206ec9363480a77c35532" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/pmmp/netresearch-jsonmapper/zipball/f700806dec756ed825a8200dc2950ead98265956", - "reference": "f700806dec756ed825a8200dc2950ead98265956", + "url": "https://api.github.com/repos/pmmp/netresearch-jsonmapper/zipball/078764e869e9b732f97206ec9363480a77c35532", + "reference": "078764e869e9b732f97206ec9363480a77c35532", "shasum": "" }, "require": { @@ -687,9 +687,9 @@ "support": { "email": "cweiske@cweiske.de", "issues": "https://github.com/cweiske/jsonmapper/issues", - "source": "https://github.com/pmmp/netresearch-jsonmapper/tree/v4.2.999" + "source": "https://github.com/pmmp/netresearch-jsonmapper/tree/v4.2.1000" }, - "time": "2023-06-01T13:43:01+00:00" + "time": "2023-07-14T10:44:14+00:00" }, { "name": "pocketmine/raklib", diff --git a/src/VersionInfo.php b/src/VersionInfo.php index 40e83b5d2..4c6553e27 100644 --- a/src/VersionInfo.php +++ b/src/VersionInfo.php @@ -31,7 +31,7 @@ use function str_repeat; final class VersionInfo{ public const NAME = "PocketMine-MP"; - public const BASE_VERSION = "5.3.1"; + public const BASE_VERSION = "5.3.2"; public const IS_DEVELOPMENT_BUILD = true; public const BUILD_CHANNEL = "stable"; diff --git a/src/network/mcpe/JwtUtils.php b/src/network/mcpe/JwtUtils.php index 12d21974d..a3cebbd73 100644 --- a/src/network/mcpe/JwtUtils.php +++ b/src/network/mcpe/JwtUtils.php @@ -61,6 +61,7 @@ use const OPENSSL_ALGO_SHA384; use const STR_PAD_LEFT; final class JwtUtils{ + public const BEDROCK_SIGNING_KEY_CURVE_NAME = "secp384r1"; /** * @return string[] @@ -203,6 +204,17 @@ final class JwtUtils{ if($signingKeyOpenSSL === false){ throw new JwtException("OpenSSL failed to parse key: " . openssl_error_string()); } + $details = openssl_pkey_get_details($signingKeyOpenSSL); + if($details === false){ + throw new JwtException("OpenSSL failed to get details from key: " . openssl_error_string()); + } + if(!isset($details['ec']['curve_name'])){ + throw new JwtException("Expected an EC key"); + } + $curve = $details['ec']['curve_name']; + if($curve !== self::BEDROCK_SIGNING_KEY_CURVE_NAME){ + throw new JwtException("Key must belong to curve " . self::BEDROCK_SIGNING_KEY_CURVE_NAME . ", got $curve"); + } return $signingKeyOpenSSL; } } diff --git a/src/network/mcpe/auth/ProcessLoginTask.php b/src/network/mcpe/auth/ProcessLoginTask.php index 6102efbcd..607b75c89 100644 --- a/src/network/mcpe/auth/ProcessLoginTask.php +++ b/src/network/mcpe/auth/ProcessLoginTask.php @@ -34,7 +34,6 @@ use pocketmine\thread\NonThreadSafeValue; use function base64_decode; use function igbinary_serialize; use function igbinary_unserialize; -use function openssl_error_string; use function time; class ProcessLoginTask extends AsyncTask{ @@ -164,7 +163,8 @@ class ProcessLoginTask extends AsyncTask{ try{ $signingKeyOpenSSL = JwtUtils::parseDerPublicKey($headerDerKey); }catch(JwtException $e){ - throw new VerifyLoginException("Invalid JWT public key: " . openssl_error_string()); + //TODO: we shouldn't be showing this internal information to the client + throw new VerifyLoginException("Invalid JWT public key: " . $e->getMessage(), null, 0, $e); } try{ if(!JwtUtils::verify($jwt, $signingKeyOpenSSL)){ @@ -204,6 +204,12 @@ class ProcessLoginTask extends AsyncTask{ if($identityPublicKey === false){ throw new VerifyLoginException("Invalid identityPublicKey: base64 error decoding"); } + try{ + //verify key format and parameters + JwtUtils::parseDerPublicKey($identityPublicKey); + }catch(JwtException $e){ + throw new VerifyLoginException("Invalid identityPublicKey: " . $e->getMessage(), null, 0, $e); + } $currentPublicKey = $identityPublicKey; //if there are further links, the next link should be signed with this } } diff --git a/src/network/mcpe/encryption/EncryptionUtils.php b/src/network/mcpe/encryption/EncryptionUtils.php index 920c54a7e..4ad9ceb31 100644 --- a/src/network/mcpe/encryption/EncryptionUtils.php +++ b/src/network/mcpe/encryption/EncryptionUtils.php @@ -33,6 +33,7 @@ use function hex2bin; use function openssl_digest; use function openssl_error_string; use function openssl_pkey_derive; +use function openssl_pkey_get_details; use function str_pad; use const STR_PAD_LEFT; @@ -42,7 +43,20 @@ final class EncryptionUtils{ //NOOP } + private static function validateKey(\OpenSSLAsymmetricKey $key) : void{ + $keyDetails = Utils::assumeNotFalse(openssl_pkey_get_details($key)); + if(!isset($keyDetails["ec"]["curve_name"])){ + throw new \InvalidArgumentException("Key must be an EC key"); + } + $curveName = $keyDetails["ec"]["curve_name"]; + if($curveName !== JwtUtils::BEDROCK_SIGNING_KEY_CURVE_NAME){ + throw new \InvalidArgumentException("Key must belong to the " . JwtUtils::BEDROCK_SIGNING_KEY_CURVE_NAME . " elliptic curve, got $curveName"); + } + } + public static function generateSharedSecret(\OpenSSLAsymmetricKey $localPriv, \OpenSSLAsymmetricKey $remotePub) : \GMP{ + self::validateKey($localPriv); + self::validateKey($remotePub); $hexSecret = openssl_pkey_derive($remotePub, $localPriv, 48); if($hexSecret === false){ throw new \InvalidArgumentException("Failed to derive shared secret: " . openssl_error_string());