Fix for CVE-2024-33663 (forbid public key for HMAC) (mpdavis#369)

Author: alistairwatts

jose/backends/cryptography_backend.py

@@ -16,7 +16,15 @@
 
 from ..constants import ALGORITHMS
 from ..exceptions import JWEError, JWKError
-from ..utils import base64_to_long, base64url_decode, base64url_encode, ensure_binary, long_to_base64
+from ..utils import (
+    base64_to_long,
+    base64url_decode,
+    base64url_encode,
+    ensure_binary,
+    is_pem_format,
+    is_ssh_key,
+    long_to_base64,
+)
 from .base import Key
 
 _binding = None
@@ -555,14 +563,7 @@ def __init__(self, key, algorithm):
         if isinstance(key, str):
             key = key.encode("utf-8")
 
-        invalid_strings = [
-            b"-----BEGIN PUBLIC KEY-----",
-            b"-----BEGIN RSA PUBLIC KEY-----",
-            b"-----BEGIN CERTIFICATE-----",
-            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 JWKError(
                 "The specified key is an asymmetric key or x509 certificate and"
                 " should not be used as an HMAC secret."

jose/backends/native.py

@@ -5,7 +5,7 @@
 from jose.backends.base import Key
 from jose.constants import ALGORITHMS
 from jose.exceptions import JWKError
-from jose.utils import base64url_decode, base64url_encode
+from jose.utils import base64url_decode, base64url_encode, is_pem_format, is_ssh_key
 
 
 def get_random_bytes(num_bytes):
@@ -36,14 +36,7 @@ def __init__(self, key, algorithm):
         if isinstance(key, str):
             key = key.encode("utf-8")
 
-        invalid_strings = [
-            b"-----BEGIN PUBLIC KEY-----",
-            b"-----BEGIN RSA PUBLIC KEY-----",
-            b"-----BEGIN CERTIFICATE-----",
-            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 JWKError(
                 "The specified key is an asymmetric key or x509 certificate and"
                 " should not be used as an HMAC secret."

jose/utils.py

@@ -1,4 +1,5 @@
 import base64
+import re
 import struct
 
 # Piggyback of the backends implementation of the function that converts a long
@@ -105,3 +106,60 @@ def ensure_binary(s):
     if isinstance(s, str):
         return s.encode("utf-8", "strict")
     raise TypeError(f"not expecting type '{type(s)}'")
+
+
+# The following was copied from PyJWT:
+#   https://github.com/jpadilla/pyjwt/commit/9c528670c455b8d948aff95ed50e22940d1ad3fc
+# 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(re.escape(pem) for pem in _PEMS) + b")[- ]----",
+)
+
+
+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"[email protected]"
+_SSH_PUBKEY_RC = re.compile(rb"\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/algorithms/test_EC.py

@@ -1,6 +1,8 @@
+import base64
 import json
 import re
 
+from jose import jwt
 from jose.backends import ECKey
 from jose.constants import ALGORITHMS
 from jose.exceptions import JOSEError, JWKError
@@ -14,9 +16,11 @@
 
 try:
     from cryptography.hazmat.backends import default_backend as CryptographyBackend
+    from cryptography.hazmat.primitives import hashes, hmac, serialization
     from cryptography.hazmat.primitives.asymmetric import ec as CryptographyEc
 
     from jose.backends.cryptography_backend import CryptographyECKey
+
 except ImportError:
     CryptographyECKey = CryptographyEc = CryptographyBackend = None
 
@@ -223,3 +227,29 @@ def test_to_dict(self):
         key = ECKey(private_key, ALGORITHMS.ES256)
         self.assert_parameters(key.to_dict(), private=True)
         self.assert_parameters(key.public_key().to_dict(), private=False)
+
+
+@pytest.mark.cryptography
+@pytest.mark.skipif(CryptographyECKey is None, reason="pyca/cryptography backend not available")
+def test_incorrect_public_key_hmac_signing():
+    def b64(x):
+        return base64.urlsafe_b64encode(x).replace(b"=", b"")
+
+    KEY = CryptographyEc.generate_private_key(CryptographyEc.SECP256R1)
+    PUBKEY = KEY.public_key().public_bytes(
+        encoding=serialization.Encoding.OpenSSH,
+        format=serialization.PublicFormat.OpenSSH,
+    )
+
+    # Create and sign the payload using a public key, but specify the "alg" in
+    # the claims that a symmetric key was used.
+    payload = b64(b'{"alg":"HS256"}') + b"." + b64(b'{"pwned":true}')
+    hasher = hmac.HMAC(PUBKEY, hashes.SHA256())
+    hasher.update(payload)
+    evil_token = payload + b"." + b64(hasher.finalize())
+
+    # Verify and decode the token using the public key. The custom algorithm
+    # field is left unspecified. Decoding using a public key should be
+    # rejected raising a JWKError.
+    with pytest.raises(JWKError):
+        jwt.decode(evil_token, PUBKEY)