Skip to content

Commit 34bd82c

Browse files
committed
Improve asymmetric key check in CryptographyHMACKey
This change should fix #346 security issue. The code is based on pyjwt change: jpadilla/pyjwt@9c52867
1 parent 4b0701b commit 34bd82c

File tree

2 files changed

+98
-9
lines changed

2 files changed

+98
-9
lines changed

jose/backends/cryptography_backend.py

+64-8
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import re
12
import math
23
import warnings
34

@@ -22,6 +23,68 @@
2223
_binding = None
2324

2425

26+
# Based on https://github.com/jpadilla/pyjwt/commit/9c528670c455b8d948aff95ed50e22940d1ad3fc
27+
# Based on https://github.com/hynek/pem/blob/7ad94db26b0bc21d10953f5dbad3acfdfacf57aa/src/pem/_core.py#L224-L252
28+
_PEMS = {
29+
b"CERTIFICATE",
30+
b"TRUSTED CERTIFICATE",
31+
b"PRIVATE KEY",
32+
b"PUBLIC KEY",
33+
b"ENCRYPTED PRIVATE KEY",
34+
b"OPENSSH PRIVATE KEY",
35+
b"DSA PRIVATE KEY",
36+
b"RSA PRIVATE KEY",
37+
b"RSA PUBLIC KEY",
38+
b"EC PRIVATE KEY",
39+
b"DH PARAMETERS",
40+
b"NEW CERTIFICATE REQUEST",
41+
b"CERTIFICATE REQUEST",
42+
b"SSH2 PUBLIC KEY",
43+
b"SSH2 ENCRYPTED PRIVATE KEY",
44+
b"X509 CRL",
45+
}
46+
47+
48+
_PEM_RE = re.compile(
49+
b"----[- ]BEGIN ("
50+
+ b"|".join(_PEMS)
51+
+ b""")[- ]----\r?
52+
.+?\r?
53+
----[- ]END \\1[- ]----\r?\n?""",
54+
re.DOTALL,
55+
)
56+
57+
58+
def is_pem_format(key):
59+
return bool(_PEM_RE.search(key))
60+
61+
62+
# Based on https://github.com/pyca/cryptography/blob/bcb70852d577b3f490f015378c75cba74986297b/src/cryptography/hazmat/primitives/serialization/ssh.py#L40-L46
63+
_CERT_SUFFIX = b"[email protected]"
64+
_SSH_PUBKEY_RC = re.compile(br"\A(\S+)[ \t]+(\S+)")
65+
_SSH_KEY_FORMATS = [
66+
b"ssh-ed25519",
67+
b"ssh-rsa",
68+
b"ssh-dss",
69+
b"ecdsa-sha2-nistp256",
70+
b"ecdsa-sha2-nistp384",
71+
b"ecdsa-sha2-nistp521",
72+
]
73+
74+
75+
def is_ssh_key(key):
76+
if any(string_value in key for string_value in _SSH_KEY_FORMATS):
77+
return True
78+
79+
ssh_pubkey_match = _SSH_PUBKEY_RC.match(key)
80+
if ssh_pubkey_match:
81+
key_type = ssh_pubkey_match.group(1)
82+
if _CERT_SUFFIX == key_type[-len(_CERT_SUFFIX) :]:
83+
return True
84+
85+
return False
86+
87+
2588
def get_random_bytes(num_bytes):
2689
"""
2790
Get random bytes
@@ -552,14 +615,7 @@ def __init__(self, key, algorithm):
552615
if isinstance(key, str):
553616
key = key.encode("utf-8")
554617

555-
invalid_strings = [
556-
b"-----BEGIN PUBLIC KEY-----",
557-
b"-----BEGIN RSA PUBLIC KEY-----",
558-
b"-----BEGIN CERTIFICATE-----",
559-
b"ssh-rsa",
560-
]
561-
562-
if any(string_value in key for string_value in invalid_strings):
618+
if is_pem_format(key) or is_ssh_key(key):
563619
raise JWKError(
564620
"The specified key is an asymmetric key or x509 certificate and"
565621
" should not be used as an HMAC secret."

tests/test_jwt.py

+34-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import pytest
66

77
from jose import jws, jwt
8-
from jose.exceptions import JWTError
8+
from jose.exceptions import JWTError, JWKError
99

1010

1111
@pytest.fixture
@@ -522,3 +522,36 @@ def test_require(self, claims, key, claim, value):
522522
new_claims[claim] = value
523523
token = jwt.encode(new_claims, key)
524524
jwt.decode(token, key, options=options, audience=str(value))
525+
526+
def test_CVE_2024_33663(self):
527+
"""Test based on https://github.com/mpdavis/python-jose/issues/346"""
528+
from Crypto.PublicKey import ECC
529+
from Crypto.Hash import HMAC, SHA256
530+
531+
# ----- SETUP -----
532+
# generate an asymmetric ECC keypair
533+
# !! signing should only be possible with the private key !!
534+
KEY = ECC.generate(curve='P-256')
535+
536+
# PUBLIC KEY, AVAILABLE TO USER
537+
# CAN BE RECOVERED THROUGH E.G. PUBKEY RECOVERY WITH TWO SIGNATURES:
538+
# https://en.wikipedia.org/wiki/Elliptic_Curve_Digital_Signature_Algorithm#Public_key_recovery
539+
# https://github.com/FlorianPicca/JWT-Key-Recovery
540+
PUBKEY = KEY.public_key().export_key(format='OpenSSH').encode()
541+
542+
# ---- CLIENT SIDE -----
543+
# without knowing the private key, a valid token can be constructed
544+
# YIKES!!
545+
546+
b64 = lambda x:base64.urlsafe_b64encode(x).replace(b'=',b'')
547+
payload = b64(b'{"alg":"HS256"}') + b'.' + b64(b'{"pwned":true}')
548+
hasher = HMAC.new(PUBKEY, digestmod=SHA256)
549+
hasher.update(payload)
550+
evil_token = payload + b'.' + b64(hasher.digest())
551+
552+
# ---- SERVER SIDE -----
553+
# verify and decode the token using the public key, as is custom
554+
# algorithm field is left unspecified
555+
# but the library will happily still verify without warning, trusting the user-controlled alg field of the token header
556+
with pytest.raises(JWKError):
557+
data = jwt.decode(evil_token, PUBKEY)

0 commit comments

Comments
 (0)