Merge pull request from GHSA-ffqj-6fqr-9h24

Co-authored-by: José Padilla <jpadilla@users.noreply.github.com>

.gitignore

@@ -60,3 +60,4 @@ target/
 
 .pytest_cache
 .mypy_cache
+pip-wheel-metadata/

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"

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):
             """

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

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()
+            )

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: