From 4e646d19a4a1e0d082bd4d1f5a58ae0182a268d9 Mon Sep 17 00:00:00 2001
From: "Dylan K. Taylor" <dktapps@pmmp.io>
Date: Fri, 14 Jul 2023 11:55:47 +0100
Subject: [PATCH] Harden login EC key validation

---
 src/network/mcpe/JwtUtils.php                   | 12 ++++++++++++
 src/network/mcpe/auth/ProcessLoginTask.php      | 10 ++++++++--
 src/network/mcpe/encryption/EncryptionUtils.php | 14 ++++++++++++++
 3 files changed, 34 insertions(+), 2 deletions(-)

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 2da3b5fae..dc7a40224 100644
--- a/src/network/mcpe/auth/ProcessLoginTask.php
+++ b/src/network/mcpe/auth/ProcessLoginTask.php
@@ -32,7 +32,6 @@ use pocketmine\scheduler\AsyncTask;
 use function base64_decode;
 use function igbinary_serialize;
 use function igbinary_unserialize;
-use function openssl_error_string;
 use function time;
 
 class ProcessLoginTask extends AsyncTask{
@@ -156,7 +155,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)){
@@ -196,6 +196,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());