Repository navigation
Check the JWE auth tag before decrypting CBC-HMAC ciphertext - #427
neeraj1235476 wants to merge 2 commits into
Conversation
TheSingulant
left a comment
There was a problem hiding this comment.
The ordering change looks right to me. It also makes the substituted random CEK path (RFC 7516 section 11.5) fail uniformly with the tag error for CBC-HMAC, instead of usually tripping the unpadder first. One note on the test, and a heads-up that #428 adds the same base64url_encode import and a test at the same spot in tests/test_jwe.py, so whichever lands second will need a small rebase.
| encrypted = jwe.encrypt(b"Live long and prosper.", key, enc, ALGORITHMS.DIR) | ||
| header, encrypted_key, iv, cipher_text, auth_tag = encrypted.split(b".") | ||
| tampered = bytearray(base64url_decode(cipher_text)) | ||
| tampered[-1] ^= 0x01 |
There was a problem hiding this comment.
Flipping a bit in the last ciphertext block turns that whole plaintext block into noise, and with a fresh random IV each run it occasionally still ends in valid PKCS7 padding. I ran this tamper 3,000 times against master with A128CBC-HS256 and 10 runs came back as Invalid JWE Auth Tag, so the test passes on master about 0.3% of the time. Flipping the last byte of the previous block instead (tampered[-17] ^= 0x01, fine here since the 22-byte plaintext gives two blocks) flips the last padding byte deterministically: 3000/3000 Invalid padding bytes. on master and 3000/3000 Invalid JWE Auth Tag on this branch.
There was a problem hiding this comment.
Good catch, I only reasoned about the padding check and missed that the last block gets scrambled wholesale. Switched it to tampered[-17] so only the final padding byte flips. I ran that a thousand times per enc against master and got Invalid padding bytes. every time, and the branch still returns Invalid JWE Auth Tag.
|
@asherf and maintainers: there have been no merges or releases since 3.5.0 in May 2025, and this PR and #428 both harden JWE decryption. If you have time to come back to it, a review and a release would help the people still relying on python-jose. If not, would you consider adding a co-maintainer, so fixes like these can land? |
|
Noted on the overlap with #428, the import and the test placement are the same in both. Happy to rebase whichever of the two lands second. |
_decrypt_and_authinjose/jwe.pyhandles the A128CBC-HS256, A192CBC-HS384 and A256CBC-HS512 encryptions by running the AES-CBC decrypt and PKCS7 unpad first, and only then comparing the tag from the token against the computed HMAC, with a plain!=. A JWE whose ciphertext has been modified therefore comes back asInvalid padding bytes.from the unpadder rather than as a tag rejection, and the two failure modes are distinguishable. RFC 7516 section 5.2 step 15 requires the tag to be validated and the input rejected without emitting any decrypted output, and section 11.5 asks for the comparison to be constant time.Compute the HMAC tag and check it with
hmac.compare_digestbefore callingdecrypt, so forged input never reaches the cipher or the unpadder. GCM is left as is because the AEAD backend already authenticates during decryption. The added test tampers with one ciphertext byte for each CBC-HMAC enc and expectsInvalid JWE Auth Tag; it fails on master and passes with this change.