From baaec8473ad693dc0093924929e4b6d33795d2e0 Mon Sep 17 00:00:00 2001 From: aivong-openhands Date: Fri, 27 Feb 2026 16:15:23 -0600 Subject: [PATCH] Fix CVE-2024-23342: Replace python-jose with jwcrypto (#13012) Co-authored-by: OpenHands CVE Fix Bot --- openhands/app_server/services/jwt_service.py | 74 +++++++++++------- poetry.lock | 61 ++++----------- pyproject.toml | 4 +- tests/unit/app_server/test_jwt_service.py | 82 ++++++++++++++++++-- 4 files changed, 142 insertions(+), 79 deletions(-) diff --git a/openhands/app_server/services/jwt_service.py b/openhands/app_server/services/jwt_service.py index d380ded77d..42ef783c0d 100644 --- a/openhands/app_server/services/jwt_service.py +++ b/openhands/app_server/services/jwt_service.py @@ -6,8 +6,8 @@ from typing import Any, AsyncGenerator import jwt from fastapi import Request -from jose import jwe -from jose.constants import ALGORITHMS +from jwcrypto import jwe as jwcrypto_jwe +from jwcrypto import jwk from pydantic import BaseModel, PrivateAttr from openhands.agent_server.utils import utc_now @@ -168,21 +168,21 @@ class JwtService: # Derive a 256-bit key using SHA256 key_256 = hashlib.sha256(key_bytes).digest() - # Encrypt the payload (convert to JSON string first) - payload_json = json.dumps(jwt_payload) - encrypted_token = jwe.encrypt( - payload_json, - key_256, - algorithm=ALGORITHMS.DIR, - encryption=ALGORITHMS.A256GCM, - kid=key_id, - ) - # Ensure we return a string - return ( - encrypted_token.decode('utf-8') - if isinstance(encrypted_token, bytes) - else encrypted_token + # Create JWK from symmetric key for jwcrypto + symmetric_key = jwk.JWK(kty='oct', k=jwk.base64url_encode(key_256)) + + # Create JWE token with jwcrypto + protected_header = { + 'alg': 'dir', + 'enc': 'A256GCM', + 'kid': key_id, + } + jwe_token = jwcrypto_jwe.JWE( + json.dumps(jwt_payload).encode('utf-8'), + recipient=symmetric_key, + protected=protected_header, ) + return jwe_token.serialize(compact=True) def decrypt_jwe_token( self, token: str, key_id: str | None = None @@ -201,15 +201,31 @@ class JwtService: ValueError: If token is invalid or key_id is not found Exception: If token decryption fails """ + # Deserialize once and reuse for both header extraction and decryption + try: + jwe_obj = jwcrypto_jwe.JWE() + jwe_obj.deserialize(token) + except Exception: + raise ValueError('Invalid JWE token format') + + # Extract and validate the protected header + try: + protected_header = json.loads(jwe_obj.objects['protected']) + except (KeyError, json.JSONDecodeError) as e: + raise ValueError(f'Invalid JWE token format: {type(e).__name__}') + + # Verify algorithms to prevent cryptographic agility attacks + if ( + protected_header.get('alg') != 'dir' + or protected_header.get('enc') != 'A256GCM' + ): + raise ValueError('Unsupported or unexpected JWE algorithm') + if key_id is None: - # Try to extract key_id from the token's header - try: - header = jwe.get_unverified_header(token) - key_id = header.get('kid') - if not key_id: - raise ValueError("Token does not contain 'kid' header with key ID") - except Exception: - raise ValueError('Invalid JWE token format') + # Extract key_id from the token's header + key_id = protected_header.get('kid') + if not key_id: + raise ValueError("Token does not contain 'kid' header with key ID") if key_id not in self._keys: raise ValueError(f"Key ID '{key_id}' not found") @@ -221,10 +237,14 @@ class JwtService: key_256 = hashlib.sha256(key_bytes).digest() try: - payload_json = jwe.decrypt(token, key_256) - assert payload_json is not None + # Create JWK from symmetric key for jwcrypto + symmetric_key = jwk.JWK(kty='oct', k=jwk.base64url_encode(key_256)) + + # Decrypt the JWE token (reusing already deserialized jwe_obj) + jwe_obj.decrypt(symmetric_key) + # Parse the JSON string back to dictionary - payload = json.loads(payload_json) + payload = json.loads(jwe_obj.payload.decode('utf-8')) return payload except Exception as e: raise Exception(f'Token decryption failed: {str(e)}') diff --git a/poetry.lock b/poetry.lock index 91007e9924..f52815963f 100644 --- a/poetry.lock +++ b/poetry.lock @@ -2030,25 +2030,6 @@ attrs = ">=21.3.0" e2b = ">=2.7.0,<3.0.0" httpx = ">=0.20.0,<1.0.0" -[[package]] -name = "ecdsa" -version = "0.19.1" -description = "ECDSA cryptographic signature library (pure python)" -optional = false -python-versions = "!=3.0.*,!=3.1.*,!=3.2.*,!=3.3.*,!=3.4.*,!=3.5.*,>=2.6" -groups = ["main"] -files = [ - {file = "ecdsa-0.19.1-py2.py3-none-any.whl", hash = "sha256:30638e27cf77b7e15c4c4cc1973720149e1033827cfd00661ca5c8cc0cdb24c3"}, - {file = "ecdsa-0.19.1.tar.gz", hash = "sha256:478cba7b62555866fcb3bb3fe985e06decbdb68ef55713c4e5ab98c57d508e61"}, -] - -[package.dependencies] -six = ">=1.9.0" - -[package.extras] -gmpy = ["gmpy"] -gmpy2 = ["gmpy2"] - [[package]] name = "email-validator" version = "2.3.0" @@ -4439,6 +4420,22 @@ files = [ {file = "jupyterlab_widgets-3.0.16.tar.gz", hash = "sha256:423da05071d55cf27a9e602216d35a3a65a3e41cdf9c5d3b643b814ce38c19e0"}, ] +[[package]] +name = "jwcrypto" +version = "1.5.6" +description = "Implementation of JOSE Web standards" +optional = false +python-versions = ">= 3.8" +groups = ["main"] +files = [ + {file = "jwcrypto-1.5.6-py3-none-any.whl", hash = "sha256:150d2b0ebbdb8f40b77f543fb44ffd2baeff48788be71f67f03566692fd55789"}, + {file = "jwcrypto-1.5.6.tar.gz", hash = "sha256:771a87762a0c081ae6166958a954f80848820b2ab066937dc8b8379d65b1b039"}, +] + +[package.dependencies] +cryptography = ">=3.4" +typing-extensions = ">=4.5.0" + [[package]] name = "keyring" version = "25.7.0" @@ -11692,30 +11689,6 @@ PyYAML = "*" docs = ["sphinx"] test = ["mypy", "pyaml", "pytest", "toml", "types-PyYAML", "types-toml"] -[[package]] -name = "python-jose" -version = "3.5.0" -description = "JOSE implementation in Python" -optional = false -python-versions = ">=3.9" -groups = ["main"] -files = [ - {file = "python_jose-3.5.0-py2.py3-none-any.whl", hash = "sha256:abd1202f23d34dfad2c3d28cb8617b90acf34132c7afd60abd0b0b7d3cb55771"}, - {file = "python_jose-3.5.0.tar.gz", hash = "sha256:fb4eaa44dbeb1c26dcc69e4bd7ec54a1cb8dd64d3b4d81ef08d90ff453f2b01b"}, -] - -[package.dependencies] -cryptography = {version = ">=3.4.0", optional = true, markers = "extra == \"cryptography\""} -ecdsa = "!=0.15" -pyasn1 = ">=0.5.0" -rsa = ">=4.0,<4.1.1 || >4.1.1,<4.4 || >4.4,<5.0" - -[package.extras] -cryptography = ["cryptography (>=3.4.0)"] -pycrypto = ["pycrypto (>=2.6.0,<2.7.0)"] -pycryptodome = ["pycryptodome (>=3.3.1,<4.0.0)"] -test = ["pytest", "pytest-cov"] - [[package]] name = "python-json-logger" version = "3.3.0" @@ -14703,4 +14676,4 @@ third-party-runtimes = ["daytona", "e2b-code-interpreter", "modal", "runloop-api [metadata] lock-version = "2.1" python-versions = "^3.12,<3.14" -content-hash = "4c79ced8690123a266bda76964619015580432721c4b03043621fb35f6b1f29b" +content-hash = "1353c2f30d46d205c369736ead67515e81041ec5e0af4534c52a57d4b2307da2" diff --git a/pyproject.toml b/pyproject.toml index 8f7a263ce5..f8d1a65f2d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -46,6 +46,7 @@ dependencies = [ "joblib", "json-repair", "jupyter-kernel-gateway", + "jwcrypto>=1.5.6", "kubernetes>=33.1", "libtmux>=0.46.2", "litellm!=1.64.4,!=1.67.*,>=1.74.3", @@ -76,7 +77,6 @@ dependencies = [ "python-docx", "python-dotenv", "python-frontmatter>=1.1", - "python-jose[cryptography]>=3.3", "python-json-logger>=3.2.1", "python-multipart>=0.0.22", "python-pptx", @@ -249,7 +249,7 @@ pybase62 = "^1.0.0" openhands-sdk = "1.11.5" openhands-agent-server = "1.11.5" openhands-tools = "1.11.5" -python-jose = { version = ">=3.3", extras = [ "cryptography" ] } +jwcrypto = ">=1.5.6" sqlalchemy = { extras = [ "asyncio" ], version = "^2.0.40" } pg8000 = "^1.31.5" asyncpg = "^0.30.0" diff --git a/tests/unit/app_server/test_jwt_service.py b/tests/unit/app_server/test_jwt_service.py index abbeff092f..2c4409760f 100644 --- a/tests/unit/app_server/test_jwt_service.py +++ b/tests/unit/app_server/test_jwt_service.py @@ -13,7 +13,8 @@ from unittest.mock import patch import jwt import pytest -from jose import jwe +from jwcrypto import jwe as jwcrypto_jwe +from jwcrypto import jwk from pydantic import SecretStr from openhands.app_server.services.jwt_service import JwtService @@ -257,16 +258,25 @@ class TestJwtService: def test_jwe_token_decryption_no_kid_header(self, jwt_service): """Test JWE token decryption fails when token has no kid header.""" - # Create a JWE token without kid header using python-jose directly + # Create a JWE token without kid header using jwcrypto directly payload = {'user_id': '123'} # Create a proper 32-byte key for A256GCM - key = b'12345678901234567890123456789012' # Exactly 32 bytes + key_bytes = b'12345678901234567890123456789012' # Exactly 32 bytes + symmetric_key = jwk.JWK(kty='oct', k=jwk.base64url_encode(key_bytes)) - token = jwe.encrypt( - json.dumps(payload), key, algorithm='dir', encryption='A256GCM' + # Create JWE token without kid in protected header + protected_header = { + 'alg': 'dir', + 'enc': 'A256GCM', + } + jwe_token = jwcrypto_jwe.JWE( + json.dumps(payload).encode('utf-8'), + recipient=symmetric_key, + protected=protected_header, ) + token = jwe_token.serialize(compact=True) - with pytest.raises(ValueError, match='Invalid JWE token format'): + with pytest.raises(ValueError, match="Token does not contain 'kid' header"): jwt_service.decrypt_jwe_token(token) def test_jwe_token_decryption_wrong_key(self, jwt_service): @@ -444,3 +454,63 @@ class TestJwtService: for key, value in unicode_payload.items(): assert jwe_decrypted[key] == value + + def test_jwe_backwards_compatibility_with_python_jose_tokens(self, jwt_service): + """Test that JWE tokens created with python-jose can be decrypted with jwcrypto. + + This test ensures backwards compatibility during the migration from python-jose + to jwcrypto. These tokens were generated using python-jose with the same key + derivation used by jwt_service (SHA256 of the secret key). + + The tokens use: + - Algorithm: dir (direct encryption) + - Encryption: A256GCM + - Key: SHA256 hash of 'test_secret_key_1' (matches key1 fixture) + """ + # Token with simple payload: {"user_id": "123", "role": "admin", "iat": 1704067200} + simple_token = ( + 'eyJhbGciOiJkaXIiLCJlbmMiOiJBMjU2R0NNIiwia2lkIjoia2V5MSJ9' + '..NJs9xezbNx3va7Q1.I7FvCODEX_cnrB7qmAVkxFaNET89ZoEVo9Enp33plE7jeBJObPGPzWLjEg-khlzeggyUa_7u' + '.TmGNAVzMIIl4dbMB5NfyGg' + ) + + # Token with complex nested payload + complex_token = ( + 'eyJhbGciOiJkaXIiLCJlbmMiOiJBMjU2R0NNIiwia2lkIjoia2V5MSJ9' + '..trjTZoGEg_mBSE3r.-o5RbtSnF_cacNnWQ_z4LGzA1FKfo5OFetjJzvgEAOe0z7DOvzAURIkUwhgKGWM55HEqRGrH' + 'KrIvdNi8-VeWA0p0-bbX0rHHSK4qN1pRfMAbm7ftQ5tl-UMnG52z5D8aFZM6JRGz5loynqo__lx2onSb87t84tcpvK' + 'yteyu7vnoqKxDUw0iK-TwQGg12jz0a1PgHneCqdE8.wzWMxLkkPv7O3dbkrrNraw' + ) + + # Token with unicode characters in payload + unicode_token = ( + 'eyJhbGciOiJkaXIiLCJlbmMiOiJBMjU2R0NNIiwia2lkIjoia2V5MSJ9' + '..y5Ez0HSrowxdufK5.Egv1ApEVRg-O5RN8GKj1K-1jLA9DZVQrx2vc7a0lkZkW4FQ3PtEMym3UXClIpbIiO4zLrd1U' + 'cq3sBaBqAhand4hYXte1GvANBqtn59mAoyEZz_w1dFQJQfUYvXrphf2ZjrRC6GuVILsUncK1Kyttc_E0hfnaet6vOU' + '3MCrGueR1LQNhg7SZo8eXyEDoPfqgXBEpM9OInMg.AiGz8aLdIPUZ__OkezpkmA' + ) + + # Test simple token decryption + simple_decrypted = jwt_service.decrypt_jwe_token(simple_token) + assert simple_decrypted['user_id'] == '123' + assert simple_decrypted['role'] == 'admin' + assert simple_decrypted['iat'] == 1704067200 + + # Test complex token decryption with nested structures + complex_decrypted = jwt_service.decrypt_jwe_token(complex_token) + assert complex_decrypted['user_id'] == 'user123' + assert complex_decrypted['metadata']['permissions'] == [ + 'read', + 'write', + 'admin', + ] + assert complex_decrypted['metadata']['settings']['theme'] == 'dark' + assert complex_decrypted['metadata']['settings']['notifications'] is True + assert complex_decrypted['iat'] == 1704067200 + + # Test unicode token decryption + unicode_decrypted = jwt_service.decrypt_jwe_token(unicode_token) + assert unicode_decrypted['user_name'] == 'José María' + assert unicode_decrypted['description'] == 'Testing with émojis 🚀' + assert unicode_decrypted['chinese'] == '你好世界' + assert unicode_decrypted['iat'] == 1704067200