Address Fenrir findings#119
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses several Fenrir findings by tightening AES-GCM tag verification semantics, improving CFFI const-correctness for pointer arguments, and adding version bounds to requirements files to reduce supply-chain risk.
Changes:
- Enforce exact
authTaglength inAesGcmStream.final()decryption and pass the configured tag length towc_AesGcmDecryptFinal. - Update/extend AES-GCM stream tests to cover wrong tag length rejection and fix the short-tag decrypt test setup.
- Update CFFI cdefs to use
const byte*for RSA OAEP label and RNG nonce parameters; add upper/lower bounds to requirements files.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| wolfcrypt/ciphers.py | Rejects truncated/incorrect tag lengths during AES-GCM stream finalization on decrypt. |
| tests/test_aesgcmstream.py | Adjusts short-tag decrypt test and adds coverage for rejecting wrong tag lengths. |
| scripts/build_ffi.py | Marks RSA OAEP label and RNG nonce parameters as const byte* in CFFI cdefs. |
| requirements/test.txt | Adds bounded versions for tox/pytest/types-cffi. |
| requirements/prod.txt | Adds bounded version for cffi. |
| requirements/docs.txt | Adds bounded versions for Sphinx and sphinx_rtd_theme. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
F-3340: AesGcmStream.final decrypt path passed len(authTag) straight to
wc_AesGcmDecryptFinal, letting a caller truncate the verification window
(forgery probability ~2^-32 instead of 2^-128 for a 4-byte tag against a
16-byte configuration). Reject len(authTag) != self._tag_bytes and pass
self._tag_bytes to wolfSSL, mirroring ChaCha20Poly1305.decrypt. Added
test_decrypt_rejects_wrong_tag_length. Also fixed test_encrypt_short_tag
which was relying on the bug (decrypt side defaulted to tag_bytes=16
against a 12-byte tag).
F-3089: Declare label as const byte* in the wc_RsaPublicEncrypt_ex and
wc_RsaPrivateDecrypt_ex cdefs so CFFI can accept Python bytes without
exposing a writable pointer into immutable memory. wolfSSL does not
modify label.
F-3090: Declare nonce as const byte* in the wc_InitRngNonce and
wc_InitRngNonce_ex cdefs for the same reason.
F-1983, F-1984: Add minimum + upper bounds to requirements/{prod,test,
docs}.txt so a hijacked release of cffi, tox, pytest, types-cffi,
Sphinx, or sphinx_rtd_theme does not get pulled silently on the next
pip install. setup.txt resolves transitively via prod.txt.
dgarske
approved these changes
May 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
F-3340: AesGcmStream.final decrypt path passed len(authTag) straight to wc_AesGcmDecryptFinal, letting a caller truncate the verification window (forgery probability ~2^-32 instead of 2^-128 for a 4-byte tag against a 16-byte configuration). Reject len(authTag) != self._tag_bytes and pass self._tag_bytes to wolfSSL, mirroring ChaCha20Poly1305.decrypt. Added test_decrypt_rejects_wrong_tag_length. Also fixed test_encrypt_short_tag which was relying on the bug (decrypt side defaulted to tag_bytes=16 against a 12-byte tag).
F-3089: Declare label as const byte* in the wc_RsaPublicEncrypt_ex and wc_RsaPrivateDecrypt_ex cdefs so CFFI can accept Python bytes without exposing a writable pointer into immutable memory. wolfSSL does not modify label.
F-3090: Declare nonce as const byte* in the wc_InitRngNonce and wc_InitRngNonce_ex cdefs for the same reason.
F-1983, F-1984: Add minimum + upper bounds to requirements/{prod,test, docs}.txt so a hijacked release of cffi, tox, pytest, types-cffi, Sphinx, or sphinx_rtd_theme does not get pulled silently on the next pip install. setup.txt resolves transitively via prod.txt.