Skip to content

Fenrir fixes (2026-06-23)#128

Open
julek-wolfssl wants to merge 4 commits into
wolfSSL:masterfrom
julek-wolfssl:fenrir/20260623
Open

Fenrir fixes (2026-06-23)#128
julek-wolfssl wants to merge 4 commits into
wolfSSL:masterfrom
julek-wolfssl:fenrir/20260623

Conversation

@julek-wolfssl

Copy link
Copy Markdown
Member

This branch collects a set of Fenrir-tracked fixes for the wolfCrypt Python bindings.

Changes

  • Reject unsafe HMAC copy() (F-5428).
  • Reject ChaCha encrypt/decrypt before set_iv (F-4463).
  • Remove contradictory cipher mode validation (F-4015).

Each fix ships with accompanying tests.

_FEEDBACK_MODES advertised MODE_ECB/MODE_CFB/MODE_OFB as supported, but
_Cipher.__init__ then rejected every mode other than CBC/CTR with a
contradictory 'not supported by this cipher' error after they had
already passed the 'is supported' check. Prune _FEEDBACK_MODES to the
modes the cipher actually implements (CBC, CTR) so unsupported modes
get a single, accurate rejection, and drop the now-dead else branch.
ChaCha.__init__ leaves _IV_nonce empty and requires set_iv() before
use, but encrypt()/decrypt() (inherited from _Cipher) did not check
this. The first call ran _set_key(), which passed the empty nonce to
wc_Chacha_SetIV() - a function that unconditionally reads 12 bytes -
reading past the buffer and silently producing output with an
undefined IV. Track an _iv_set flag and override encrypt()/decrypt()
to raise WolfCryptError until set_iv() has been called.
_Hmac inherited _Hash.copy(), which - lacking a wolfCrypt copy function
for Hmac - fell back to a byte-level memmove and returned an object
marked _shallow_copy that aliases the original's internal C state. In
async or hardware-accelerated builds those internal pointers are shared,
so freeing the original leaves the copy with stale state
(use-after-free, wrong MACs, or corruption). wolfCrypt exposes no safe
public Hmac copy, so override copy() to raise NotImplementedError.
digest()/hexdigest() are unaffected. Update the shared hash tests to
expect this for HMAC.
Copilot AI review requested due to automatic review settings June 23, 2026 13:19
@julek-wolfssl julek-wolfssl self-assigned this Jun 23, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR applies three Fenrir-tracked safety/consistency fixes to the wolfCrypt Python bindings: disabling unsafe HMAC state copying, tightening cipher-mode validation to avoid contradictory behavior, and preventing ChaCha encrypt/decrypt usage before an IV is set. The changes primarily harden API behavior against unsafe states and align error paths with what is actually supported.

Changes:

  • Make HMAC copy() unsupported (raises NotImplementedError) to avoid aliasing underlying C resources.
  • Restrict _Cipher mode gating to the actually supported modes (CBC, CTR) and simplify IV validation.
  • Require ChaCha.set_iv() before encrypt()/decrypt(), with new regression tests.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
wolfcrypt/hashes.py Disables unsafe HMAC copying by overriding copy() to raise.
wolfcrypt/ciphers.py Tightens supported mode list/validation and adds ChaCha “IV must be set” enforcement.
tests/test_hmac_copy.py Adds regression coverage ensuring HMAC copy() is rejected and digest behavior remains stable.
tests/test_hashes.py Updates shared hash tests to expect/skip HMAC copy behavior appropriately.
tests/test_cipher_modes.py Adds coverage that supported/unsupported mode gating is consistent and non-contradictory.
tests/test_chacha_iv.py Adds coverage requiring set_iv() before ChaCha encrypt/decrypt and ensuring roundtrip works after.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread wolfcrypt/ciphers.py
Comment thread wolfcrypt/ciphers.py

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

- ChaCha.set_iv(): only mark _iv_set after _set_key() succeeds, and clear
  it first, so a failed re-key cannot leave encrypt()/decrypt() unblocked
  with a stale or partially-applied IV. Add a regression test.
- Update _Cipher.new()/encrypt()/decrypt() docstrings that still referred
  to CFB/segment-size behavior to match the actually supported modes
  (MODE_CBC, MODE_CTR) and their IV requirements.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants