From 9c528670c455b8d948aff95ed50e22940d1ad3fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Padilla?= Date: Thu, 12 May 2022 14:31:00 -0400 Subject: [PATCH] Merge pull request from GHSA-ffqj-6fqr-9h24 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: José Padilla --- .gitignore | 1 + jwt/__init__.py | 2 +- jwt/algorithms.py | 39 +++++++------- jwt/utils.py | 61 ++++++++++++++++++++++ tests/test_advisory.py | 109 +++++++++++++++++++++++++++++++++++++++ tests/test_algorithms.py | 2 +- 6 files changed, 191 insertions(+), 23 deletions(-) create mode 100644 tests/test_advisory.py diff --git a/.gitignore b/.gitignore index 63406706..8f78dc38 100644 --- a/.gitignore +++ b/.gitignore @@ -60,3 +60,4 @@ target/ .pytest_cache .mypy_cache +pip-wheel-metadata/ diff --git a/jwt/__init__.py b/jwt/__init__.py index e692a842..6b3f8ab1 100644 --- a/jwt/__init__.py +++ b/jwt/__init__.py @@ -25,7 +25,7 @@ ) from .jwks_client import PyJWKClient -__version__ = "2.3.0" +__version__ = "2.4.0" __title__ = "PyJWT" __description__ = "JSON Web Token implementation in Python" diff --git a/jwt/algorithms.py b/jwt/algorithms.py index 739df808..46a1a532 100644 --- a/jwt/algorithms.py +++ b/jwt/algorithms.py @@ -9,6 +9,8 @@ der_to_raw_signature, force_bytes, from_base64url_uint, + is_pem_format, + is_ssh_key, raw_to_der_signature, to_base64url_uint, ) @@ -183,14 +185,7 @@ def __init__(self, hash_alg): def prepare_key(self, key): key = force_bytes(key) - invalid_strings = [ - b"-----BEGIN PUBLIC KEY-----", - b"-----BEGIN CERTIFICATE-----", - b"-----BEGIN RSA PUBLIC KEY-----", - b"ssh-rsa", - ] - - if any(string_value in key for string_value in invalid_strings): + if is_pem_format(key) or is_ssh_key(key): raise InvalidKeyError( "The specified key is an asymmetric key or x509 certificate and" " should not be used as an HMAC secret." @@ -551,26 +546,28 @@ def __init__(self, **kwargs): pass def prepare_key(self, key): - - if isinstance( - key, - (Ed25519PrivateKey, Ed25519PublicKey, Ed448PrivateKey, Ed448PublicKey), - ): - return key - if isinstance(key, (bytes, str)): if isinstance(key, str): key = key.encode("utf-8") str_key = key.decode("utf-8") if "-----BEGIN PUBLIC" in str_key: - return load_pem_public_key(key) - if "-----BEGIN PRIVATE" in str_key: - return load_pem_private_key(key, password=None) - if str_key[0:4] == "ssh-": - return load_ssh_public_key(key) + key = load_pem_public_key(key) + elif "-----BEGIN PRIVATE" in str_key: + key = load_pem_private_key(key, password=None) + elif str_key[0:4] == "ssh-": + key = load_ssh_public_key(key) - raise TypeError("Expecting a PEM-formatted or OpenSSH key.") + # Explicit check the key to prevent confusing errors from cryptography + if not isinstance( + key, + (Ed25519PrivateKey, Ed25519PublicKey, Ed448PrivateKey, Ed448PublicKey), + ): + raise InvalidKeyError( + "Expecting a EllipticCurvePrivateKey/EllipticCurvePublicKey. Wrong key provided for EdDSA algorithms" + ) + + return key def sign(self, msg, key): """ diff --git a/jwt/utils.py b/jwt/utils.py index 9dde10cf..8ab73b42 100644 --- a/jwt/utils.py +++ b/jwt/utils.py @@ -1,5 +1,6 @@ import base64 import binascii +import re from typing import Any, Union try: @@ -97,3 +98,63 @@ def raw_to_der_signature(raw_sig: bytes, curve: EllipticCurve) -> bytes: s = bytes_to_number(raw_sig[num_bytes:]) return encode_dss_signature(r, s) + + +# Based on https://github.com/hynek/pem/blob/7ad94db26b0bc21d10953f5dbad3acfdfacf57aa/src/pem/_core.py#L224-L252 +_PEMS = { + b"CERTIFICATE", + b"TRUSTED CERTIFICATE", + b"PRIVATE KEY", + b"PUBLIC KEY", + b"ENCRYPTED PRIVATE KEY", + b"OPENSSH PRIVATE KEY", + b"DSA PRIVATE KEY", + b"RSA PRIVATE KEY", + b"RSA PUBLIC KEY", + b"EC PRIVATE KEY", + b"DH PARAMETERS", + b"NEW CERTIFICATE REQUEST", + b"CERTIFICATE REQUEST", + b"SSH2 PUBLIC KEY", + b"SSH2 ENCRYPTED PRIVATE KEY", + b"X509 CRL", +} + +_PEM_RE = re.compile( + b"----[- ]BEGIN (" + + b"|".join(_PEMS) + + b""")[- ]----\r? +.+?\r? +----[- ]END \\1[- ]----\r?\n?""", + re.DOTALL, +) + + +def is_pem_format(key: bytes) -> bool: + return bool(_PEM_RE.search(key)) + + +# Based on https://github.com/pyca/cryptography/blob/bcb70852d577b3f490f015378c75cba74986297b/src/cryptography/hazmat/primitives/serialization/ssh.py#L40-L46 +_CERT_SUFFIX = b"-cert-v01@openssh.com" +_SSH_PUBKEY_RC = re.compile(br"\A(\S+)[ \t]+(\S+)") +_SSH_KEY_FORMATS = [ + b"ssh-ed25519", + b"ssh-rsa", + b"ssh-dss", + b"ecdsa-sha2-nistp256", + b"ecdsa-sha2-nistp384", + b"ecdsa-sha2-nistp521", +] + + +def is_ssh_key(key: bytes) -> bool: + if any(string_value in key for string_value in _SSH_KEY_FORMATS): + return True + + ssh_pubkey_match = _SSH_PUBKEY_RC.match(key) + if ssh_pubkey_match: + key_type = ssh_pubkey_match.group(1) + if _CERT_SUFFIX == key_type[-len(_CERT_SUFFIX) :]: + return True + + return False diff --git a/tests/test_advisory.py b/tests/test_advisory.py new file mode 100644 index 00000000..f70f54ba --- /dev/null +++ b/tests/test_advisory.py @@ -0,0 +1,109 @@ +import jwt +import pytest +from jwt.exceptions import InvalidKeyError + +priv_key_bytes = b'''-----BEGIN PRIVATE KEY----- +MC4CAQAwBQYDK2VwBCIEIIbBhdo2ah7X32i50GOzrCr4acZTe6BezUdRIixjTAdL +-----END PRIVATE KEY-----''' + +pub_key_bytes = b'ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIPL1I9oiq+B8crkmuV4YViiUnhdLjCp3hvy1bNGuGfNL' + +ssh_priv_key_bytes = b"""-----BEGIN EC PRIVATE KEY----- +MHcCAQEEIOWc7RbaNswMtNtc+n6WZDlUblMr2FBPo79fcGXsJlGQoAoGCCqGSM49 +AwEHoUQDQgAElcy2RSSSgn2RA/xCGko79N+7FwoLZr3Z0ij/ENjow2XpUDwwKEKk +Ak3TDXC9U8nipMlGcY7sDpXp2XyhHEM+Rw== +-----END EC PRIVATE KEY-----""" + +ssh_key_bytes = b"""ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBJXMtkUkkoJ9kQP8QhpKO/TfuxcKC2a92dIo/xDY6MNl6VA8MChCpAJN0w1wvVPJ4qTJRnGO7A6V6dl8oRxDPkc=""" + + +class TestAdvisory: + def test_ghsa_ffqj_6fqr_9h24(self): + # Generate ed25519 private key + # private_key = ed25519.Ed25519PrivateKey.generate() + + # Get private key bytes as they would be stored in a file + # priv_key_bytes = private_key.private_bytes( + # encoding=serialization.Encoding.PEM, + # format=serialization.PrivateFormat.PKCS8, + # encryption_algorithm=serialization.NoEncryption(), + # ) + + # Get public key bytes as they would be stored in a file + # pub_key_bytes = private_key.public_key().public_bytes( + # encoding=serialization.Encoding.OpenSSH, + # format=serialization.PublicFormat.OpenSSH, + # ) + + # Making a good jwt token that should work by signing it + # with the private key + # encoded_good = jwt.encode({"test": 1234}, priv_key_bytes, algorithm="EdDSA") + encoded_good = 'eyJ0eXAiOiJKV1QiLCJhbGciOiJFZERTQSJ9.eyJ0ZXN0IjoxMjM0fQ.M5y1EEavZkHSlj9i8yi9nXKKyPBSAUhDRTOYZi3zZY11tZItDaR3qwAye8pc74_lZY3Ogt9KPNFbVOSGnUBHDg' + + # Using HMAC with the public key to trick the receiver to think that the + # public key is a HMAC secret + encoded_bad = 'eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ0ZXN0IjoxMjM0fQ.6ulDpqSlbHmQ8bZXhZRLFko9SwcHrghCwh8d-exJEE4' + + # Both of the jwt tokens are validated as valid + jwt.decode( + encoded_good, + pub_key_bytes, + algorithms=jwt.algorithms.get_default_algorithms(), + ) + + with pytest.raises(InvalidKeyError): + jwt.decode( + encoded_bad, + pub_key_bytes, + algorithms=jwt.algorithms.get_default_algorithms(), + ) + + # Of course the receiver should specify ed25519 algorithm to be used if + # they specify ed25519 public key. However, if other algorithms are used, + # the POC does not work + # HMAC specifies illegal strings for the HMAC secret in jwt/algorithms.py + # + # invalid_str ings = [ + # b"-----BEGIN PUBLIC KEY-----", + # b"-----BEGIN CERTIFICATE-----", + # b"-----BEGIN RSA PUBLIC KEY-----", + # b"ssh-rsa", + # ] + # + # However, OKPAlgorithm (ed25519) accepts the following in jwt/algorithms.py: + # + # if "-----BEGIN PUBLIC" in str_key: + # return load_pem_public_key(key) + # if "-----BEGIN PRIVATE" in str_key: + # return load_pem_private_key(key, password=None) + # if str_key[0:4] == "ssh-": + # return load_ssh_public_key(key) + # + # These should most likely made to match each other to prevent this behavior + + # POC for the ecdsa-sha2-nistp256 format. + # openssl ecparam -genkey -name prime256v1 -noout -out ec256-key-priv.pem + # openssl ec -in ec256-key-priv.pem -pubout > ec256-key-pub.pem + # ssh-keygen -y -f ec256-key-priv.pem > ec256-key-ssh.pub + + # Making a good jwt token that should work by signing it with the private key + # encoded_good = jwt.encode({"test": 1234}, ssh_priv_key_bytes, algorithm="ES256") + encoded_good = "eyJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCJ9.eyJ0ZXN0IjoxMjM0fQ.NX42mS8cNqYoL3FOW9ZcKw8Nfq2mb6GqJVADeMA1-kyHAclilYo_edhdM_5eav9tBRQTlL0XMeu_WFE_mz3OXg" + + # Using HMAC with the ssh public key to trick the receiver to think that the public key is a HMAC secret + # encoded_bad = jwt.encode({"test": 1234}, ssh_key_bytes, algorithm="HS256") + encoded_bad = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ0ZXN0IjoxMjM0fQ.5eYfbrbeGYmWfypQ6rMWXNZ8bdHcqKng5GPr9MJZITU" + + # Both of the jwt tokens are validated as valid + jwt.decode( + encoded_good, + ssh_key_bytes, + algorithms=jwt.algorithms.get_default_algorithms() + ) + + with pytest.raises(InvalidKeyError): + jwt.decode( + encoded_bad, + ssh_key_bytes, + algorithms=jwt.algorithms.get_default_algorithms() + ) diff --git a/tests/test_algorithms.py b/tests/test_algorithms.py index fca930cf..ac26600d 100644 --- a/tests/test_algorithms.py +++ b/tests/test_algorithms.py @@ -679,7 +679,7 @@ class TestOKPAlgorithms: def test_okp_ed25519_should_reject_non_string_key(self): algo = OKPAlgorithm() - with pytest.raises(TypeError): + with pytest.raises(InvalidKeyError): algo.prepare_key(None) with open(key_path("testkey_ed25519")) as keyfile: