From b078e01b65a9a45386ff23633d927362ea9c58a4 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Mon, 24 Jul 2023 13:35:32 +0100 Subject: [PATCH] JwtUtils: handle DER <-> raw signature conversion in-house, drop fgrosse/phpasn1 dependency normally I would hesitate to reinvent the wheel, but we only need a tiny subset of the ASN.1 spec which is trivial to implement by itself. I'd rather this than depend on another library that could introduce security vulnerabilities (I'm looking at you, jsonmapper). closes #5935 --- composer.json | 1 - composer.lock | 78 +------------------- src/network/mcpe/JwtUtils.php | 131 ++++++++++++++++++++++------------ 3 files changed, 85 insertions(+), 125 deletions(-) diff --git a/composer.json b/composer.json index ea312ac14..608c88459 100644 --- a/composer.json +++ b/composer.json @@ -32,7 +32,6 @@ "ext-zlib": ">=1.2.11", "composer-runtime-api": "^2.0", "adhocore/json-comment": "~1.2.0", - "fgrosse/phpasn1": "~2.5.0", "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", diff --git a/composer.lock b/composer.lock index 53807f312..2a4521d26 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": "74166e6c2f09b356c83a951efef349f2", + "content-hash": "fa059375785ed5b842af30c6b99c572f", "packages": [ { "name": "adhocore/json-comment", @@ -120,82 +120,6 @@ ], "time": "2023-01-15T23:15:59+00:00" }, - { - "name": "fgrosse/phpasn1", - "version": "v2.5.0", - "source": { - "type": "git", - "url": "https://github.com/fgrosse/PHPASN1.git", - "reference": "42060ed45344789fb9f21f9f1864fc47b9e3507b" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/fgrosse/PHPASN1/zipball/42060ed45344789fb9f21f9f1864fc47b9e3507b", - "reference": "42060ed45344789fb9f21f9f1864fc47b9e3507b", - "shasum": "" - }, - "require": { - "php": "^7.1 || ^8.0" - }, - "require-dev": { - "php-coveralls/php-coveralls": "~2.0", - "phpunit/phpunit": "^7.0 || ^8.0 || ^9.0" - }, - "suggest": { - "ext-bcmath": "BCmath is the fallback extension for big integer calculations", - "ext-curl": "For loading OID information from the web if they have not bee defined statically", - "ext-gmp": "GMP is the preferred extension for big integer calculations", - "phpseclib/bcmath_compat": "BCmath polyfill for servers where neither GMP nor BCmath is available" - }, - "type": "library", - "extra": { - "branch-alias": { - "dev-master": "2.0.x-dev" - } - }, - "autoload": { - "psr-4": { - "FG\\": "lib/" - } - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "authors": [ - { - "name": "Friedrich Große", - "email": "friedrich.grosse@gmail.com", - "homepage": "https://github.com/FGrosse", - "role": "Author" - }, - { - "name": "All contributors", - "homepage": "https://github.com/FGrosse/PHPASN1/contributors" - } - ], - "description": "A PHP Framework that allows you to encode and decode arbitrary ASN.1 structures using the ITU-T X.690 Encoding Rules.", - "homepage": "https://github.com/FGrosse/PHPASN1", - "keywords": [ - "DER", - "asn.1", - "asn1", - "ber", - "binary", - "decoding", - "encoding", - "x.509", - "x.690", - "x509", - "x690" - ], - "support": { - "issues": "https://github.com/fgrosse/PHPASN1/issues", - "source": "https://github.com/fgrosse/PHPASN1/tree/v2.5.0" - }, - "abandoned": true, - "time": "2022-12-19T11:08:26+00:00" - }, { "name": "pocketmine/bedrock-block-upgrade-schema", "version": "3.1.0", diff --git a/src/network/mcpe/JwtUtils.php b/src/network/mcpe/JwtUtils.php index a3cebbd73..259a602d4 100644 --- a/src/network/mcpe/JwtUtils.php +++ b/src/network/mcpe/JwtUtils.php @@ -23,28 +23,26 @@ declare(strict_types=1); namespace pocketmine\network\mcpe; -use FG\ASN1\Exception\ParserException; -use FG\ASN1\Universal\Integer; -use FG\ASN1\Universal\Sequence; use pocketmine\utils\AssumptionFailedError; +use pocketmine\utils\BinaryStream; use pocketmine\utils\Utils; use function base64_decode; use function base64_encode; +use function bin2hex; +use function chr; use function count; use function explode; -use function gmp_export; -use function gmp_import; -use function gmp_init; -use function gmp_strval; use function is_array; use function json_decode; use function json_encode; use function json_last_error_msg; +use function ltrim; use function openssl_error_string; use function openssl_pkey_get_details; use function openssl_pkey_get_public; use function openssl_sign; use function openssl_verify; +use function ord; use function preg_match; use function rtrim; use function sprintf; @@ -54,8 +52,7 @@ use function str_replace; use function str_split; use function strlen; use function strtr; -use const GMP_BIG_ENDIAN; -use const GMP_MSW_FIRST; +use function substr; use const JSON_THROW_ON_ERROR; use const OPENSSL_ALGO_SHA384; use const STR_PAD_LEFT; @@ -63,6 +60,12 @@ use const STR_PAD_LEFT; final class JwtUtils{ public const BEDROCK_SIGNING_KEY_CURVE_NAME = "secp384r1"; + private const ASN1_INTEGER_TAG = "\x02"; + private const ASN1_SEQUENCE_TAG = "\x30"; + + private const SIGNATURE_PART_LENGTH = 48; + private const SIGNATURE_ALGORITHM = OPENSSL_ALGO_SHA384; + /** * @return string[] * @phpstan-return array{string, string, string} @@ -98,30 +101,84 @@ final class JwtUtils{ return [$header, $body, $signature]; } + private static function signaturePartToAsn1(string $part) : string{ + if(strlen($part) !== self::SIGNATURE_PART_LENGTH){ + throw new JwtException("R and S for a SHA384 signature must each be exactly 48 bytes, but have " . strlen($part) . " bytes"); + } + $part = ltrim($part, "\x00"); + if(ord($part[0]) >= 128){ + //ASN.1 integers with a leading 1 bit are considered negative - add a leading 0 byte to prevent this + //ECDSA signature R and S values are always positive + $part = "\x00" . $part; + } + + //we can assume the length is 1 byte here - if it were larger than 127, more complex logic would be needed + return self::ASN1_INTEGER_TAG . chr(strlen($part)) . $part; + } + + private static function rawSignatureToDer(string $rawSignature) : string{ + if(strlen($rawSignature) !== self::SIGNATURE_PART_LENGTH * 2){ + throw new JwtException("JWT signature has unexpected length, expected 96, got " . strlen($rawSignature)); + } + + [$rString, $sString] = str_split($rawSignature, self::SIGNATURE_PART_LENGTH); + $sequence = self::signaturePartToAsn1($rString) . self::signaturePartToAsn1($sString); + + //we can assume the length is 1 byte here - if it were larger than 127, more complex logic would be needed + return self::ASN1_SEQUENCE_TAG . chr(strlen($sequence)) . $sequence; + } + + private static function signaturePartFromAsn1(BinaryStream $stream) : string{ + $prefix = $stream->get(1); + if($prefix !== self::ASN1_INTEGER_TAG){ + throw new \InvalidArgumentException("Expected an ASN.1 INTEGER tag, got " . bin2hex($prefix)); + } + //we can assume the length is 1 byte here - if it were larger than 127, more complex logic would be needed + $length = $stream->getByte(); + if($length > self::SIGNATURE_PART_LENGTH + 1){ //each part may have an extra leading 0 byte to prevent it being interpreted as a negative number + throw new \InvalidArgumentException("Expected at most 49 bytes for signature R or S, got $length"); + } + $part = $stream->get($length); + return str_pad(ltrim($part, "\x00"), self::SIGNATURE_PART_LENGTH, "\x00", STR_PAD_LEFT); + } + + private static function rawSignatureFromDer(string $derSignature) : string{ + if($derSignature[0] !== self::ASN1_SEQUENCE_TAG){ + throw new \InvalidArgumentException("Invalid DER signature, expected ASN.1 SEQUENCE tag, got " . bin2hex($derSignature[0])); + } + + //we can assume the length is 1 byte here - if it were larger than 127, more complex logic would be needed + $length = ord($derSignature[1]); + $parts = substr($derSignature, 2, $length); + if(strlen($parts) !== $length){ + throw new \InvalidArgumentException("Invalid DER signature, expected $length sequence bytes, got " . strlen($parts)); + } + + $stream = new BinaryStream($parts); + $rRaw = self::signaturePartFromAsn1($stream); + $sRaw = self::signaturePartFromAsn1($stream); + + if(!$stream->feof()){ + throw new \InvalidArgumentException("Invalid DER signature, unexpected trailing sequence data"); + } + + return $rRaw . $sRaw; + } + /** * @throws JwtException */ public static function verify(string $jwt, \OpenSSLAsymmetricKey $signingKey) : bool{ [$header, $body, $signature] = self::split($jwt); - $plainSignature = self::b64UrlDecode($signature); - if(strlen($plainSignature) !== 96){ - throw new JwtException("JWT signature has unexpected length, expected 96, got " . strlen($plainSignature)); - } - - [$rString, $sString] = str_split($plainSignature, 48); - $convert = fn(string $str) => gmp_strval(gmp_import($str, 1, GMP_BIG_ENDIAN | GMP_MSW_FIRST), 10); - - $sequence = new Sequence( - new Integer($convert($rString)), - new Integer($convert($sString)) - ); + $rawSignature = self::b64UrlDecode($signature); + $derSignature = self::rawSignatureToDer($rawSignature); $v = openssl_verify( $header . '.' . $body, - $sequence->getBinary(), + $derSignature, $signingKey, - OPENSSL_ALGO_SHA384 + self::SIGNATURE_ALGORITHM ); switch($v){ case 0: return false; @@ -140,33 +197,13 @@ final class JwtUtils{ openssl_sign( $jwtBody, - $rawDerSig, + $derSignature, $signingKey, - OPENSSL_ALGO_SHA384 + self::SIGNATURE_ALGORITHM ); - try{ - $asnObject = Sequence::fromBinary($rawDerSig); - }catch(ParserException $e){ - throw new AssumptionFailedError("Failed to parse OpenSSL signature: " . $e->getMessage(), 0, $e); - } - if(count($asnObject) !== 2){ - throw new AssumptionFailedError("OpenSSL produced invalid signature, expected exactly 2 parts"); - } - [$r, $s] = [$asnObject[0], $asnObject[1]]; - if(!($r instanceof Integer) || !($s instanceof Integer)){ - throw new AssumptionFailedError("OpenSSL produced invalid signature, expected 2 INTEGER parts"); - } - $rString = $r->getContent(); - $sString = $s->getContent(); - - $toBinary = fn($str) => str_pad( - gmp_export(gmp_init($str, 10), 1, GMP_BIG_ENDIAN | GMP_MSW_FIRST), - 48, - "\x00", - STR_PAD_LEFT - ); - $jwtSig = JwtUtils::b64UrlEncode($toBinary($rString) . $toBinary($sString)); + $rawSignature = self::rawSignatureFromDer($derSignature); + $jwtSig = self::b64UrlEncode($rawSignature); return "$jwtBody.$jwtSig"; }