Skip to content

Commit 9320fed

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 58e543e commit 9320fed

File tree

6 files changed

+168
-59
lines changed

6 files changed

+168
-59
lines changed

jose/backends/cryptography_backend.py

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
from ..constants import ALGORITHMS
1818
from ..exceptions import JWEError, JWKError
1919
from ..utils import base64_to_long, base64url_decode, base64url_encode, ensure_binary, long_to_base64
20+
from ..utils import is_pem_format, is_ssh_key
2021
from .base import Key
2122

2223
_binding = None
@@ -555,14 +556,7 @@ def __init__(self, key, algorithm):
555556
if isinstance(key, str):
556557
key = key.encode("utf-8")
557558

558-
invalid_strings = [
559-
b"-----BEGIN PUBLIC KEY-----",
560-
b"-----BEGIN RSA PUBLIC KEY-----",
561-
b"-----BEGIN CERTIFICATE-----",
562-
b"ssh-rsa",
563-
]
564-
565-
if any(string_value in key for string_value in invalid_strings):
559+
if is_pem_format(key) or is_ssh_key(key):
566560
raise JWKError(
567561
"The specified key is an asymmetric key or x509 certificate and"
568562
" should not be used as an HMAC secret."

jose/backends/native.py

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from jose.constants import ALGORITHMS
77
from jose.exceptions import JWKError
88
from jose.utils import base64url_decode, base64url_encode
9+
from jose.utils import is_pem_format, is_ssh_key
910

1011

1112
def get_random_bytes(num_bytes):
@@ -36,14 +37,7 @@ def __init__(self, key, algorithm):
3637
if isinstance(key, str):
3738
key = key.encode("utf-8")
3839

39-
invalid_strings = [
40-
b"-----BEGIN PUBLIC KEY-----",
41-
b"-----BEGIN RSA PUBLIC KEY-----",
42-
b"-----BEGIN CERTIFICATE-----",
43-
b"ssh-rsa",
44-
]
45-
46-
if any(string_value in key for string_value in invalid_strings):
40+
if is_pem_format(key) or is_ssh_key(key):
4741
raise JWKError(
4842
"The specified key is an asymmetric key or x509 certificate and"
4943
" should not be used as an HMAC secret."

jose/jwt.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,14 @@ def decode(token, key, algorithms=None, options=None, audience=None, issuer=None
141141

142142
verify_signature = defaults.get("verify_signature", True)
143143

144+
# Forbid the usage of the jwt.decode without alogrightms parameter
145+
# See https://github.com/mpdavis/python-jose/issues/346 for more
146+
# information CVE-2024-33663
147+
if verify_signature and algorithms is None:
148+
raise JWTError("It is required that you pass in a value for "
149+
'the "algorithms" argument when calling '
150+
"decode().")
151+
144152
try:
145153
payload = jws.verify(token, key, algorithms, verify=verify_signature)
146154
except JWSError as e:

jose/utils.py

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import re
12
import base64
23
import struct
34

@@ -105,3 +106,75 @@ def ensure_binary(s):
105106
if isinstance(s, str):
106107
return s.encode("utf-8", "strict")
107108
raise TypeError(f"not expecting type '{type(s)}'")
109+
110+
111+
# Based on https://github.com/jpadilla/pyjwt/commit/9c528670c455b8d948aff95ed50e22940d1ad3fc
112+
# Based on https://github.com/hynek/pem/blob/7ad94db26b0bc21d10953f5dbad3acfdfacf57aa/src/pem/_core.py#L224-L252
113+
_PEMS = {
114+
b"CERTIFICATE",
115+
b"TRUSTED CERTIFICATE",
116+
b"PRIVATE KEY",
117+
b"PUBLIC KEY",
118+
b"ENCRYPTED PRIVATE KEY",
119+
b"OPENSSH PRIVATE KEY",
120+
b"DSA PRIVATE KEY",
121+
b"RSA PRIVATE KEY",
122+
b"RSA PUBLIC KEY",
123+
b"EC PRIVATE KEY",
124+
b"DH PARAMETERS",
125+
b"NEW CERTIFICATE REQUEST",
126+
b"CERTIFICATE REQUEST",
127+
b"SSH2 PUBLIC KEY",
128+
b"SSH2 ENCRYPTED PRIVATE KEY",
129+
b"X509 CRL",
130+
}
131+
132+
133+
_PEM_RE = re.compile(
134+
b"----[- ]BEGIN ("
135+
+ b"|".join(_PEMS)
136+
+ b""")[- ]----\r?
137+
.+?\r?
138+
----[- ]END \\1[- ]----\r?\n?""",
139+
re.DOTALL,
140+
)
141+
142+
143+
def is_pem_format(key):
144+
"""
145+
Return True if the key is PEM format
146+
This function uses the list of valid PEM headers defined in
147+
_PEMS dict.
148+
"""
149+
return bool(_PEM_RE.search(key))
150+
151+
152+
# Based on https://github.com/pyca/cryptography/blob/bcb70852d577b3f490f015378c75cba74986297b/src/cryptography/hazmat/primitives/serialization/ssh.py#L40-L46
153+
_CERT_SUFFIX = b"[email protected]"
154+
_SSH_PUBKEY_RC = re.compile(br"\A(\S+)[ \t]+(\S+)")
155+
_SSH_KEY_FORMATS = [
156+
b"ssh-ed25519",
157+
b"ssh-rsa",
158+
b"ssh-dss",
159+
b"ecdsa-sha2-nistp256",
160+
b"ecdsa-sha2-nistp384",
161+
b"ecdsa-sha2-nistp521",
162+
]
163+
164+
165+
def is_ssh_key(key):
166+
"""
167+
Return True if the key is a SSH key
168+
This function uses the list of valid SSH key format defined in
169+
_SSH_KEY_FORMATS dict.
170+
"""
171+
if any(string_value in key for string_value in _SSH_KEY_FORMATS):
172+
return True
173+
174+
ssh_pubkey_match = _SSH_PUBKEY_RC.match(key)
175+
if ssh_pubkey_match:
176+
key_type = ssh_pubkey_match.group(1)
177+
if _CERT_SUFFIX == key_type[-len(_CERT_SUFFIX) :]:
178+
return True
179+
180+
return False

tests/algorithms/test_HMAC.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,17 @@ def test_non_string_key(self):
1414

1515
def test_RSA_key(self):
1616
key = "-----BEGIN PUBLIC KEY-----"
17+
key += "\n\n\n-----END PUBLIC KEY-----"
1718
with pytest.raises(JOSEError):
1819
HMACKey(key, ALGORITHMS.HS256)
1920

2021
key = "-----BEGIN RSA PUBLIC KEY-----"
22+
key += "\n\n\n-----END RSA PUBLIC KEY-----"
2123
with pytest.raises(JOSEError):
2224
HMACKey(key, ALGORITHMS.HS256)
2325

2426
key = "-----BEGIN CERTIFICATE-----"
27+
key += "\n\n\n-----END CERTIFICATE-----"
2528
with pytest.raises(JOSEError):
2629
HMACKey(key, ALGORITHMS.HS256)
2730

0 commit comments

Comments
 (0)