Add initial support for crypto callbacks. by roberthdevries · Pull Request #114 · wolfSSL/wolfcrypt-py

roberthdevries · 2026-04-27T13:35:22Z

This patch does not add support for types of callbacks. Adding support for callbacks for cipher functions is complicated by issues with cffi code generation for the wc_CryptoInfo structure with two layers of anonymous unions.
Uncommenting more parts of the cipher struct causes errors regarding conflicting struct sizes of other parts of the wc_CryptoInfo struct.
Cffi cdef and the compiler seem to disagree.

However as it is, callbacks work for random and hash functions.

JeremiahM37

Skoll Code Review

Scan type: review
Overall recommendation: REQUEST_CHANGES
Findings: 14 total — 9 posted, 5 skipped
7 finding(s) posted as inline comments (see file-level comments below)

Posted findings

[High] Importing wolfcrypt fails when CRYPTO_CB is disabled at build time — wolfcrypt/__init__.py:49 and wolfcrypt/cryptocb.py:33-58
[High] Double unregister of device on context-manager exit + GC — wolfcrypt/cryptocb.py:73-77, 117-118
[Medium] Return value of wolfCrypt_Init not checked — wolfcrypt/__init__.py:52
[Medium] Debug print() statements left in production callback dispatcher — wolfcrypt/cryptocb.py:80, 84, 85, 107, 111
[Medium] Hash digest dict KeyError on unmapped hash types — wolfcrypt/cryptocb.py:84, 90
[Medium] Digest output buffer not size-validated — wolfcrypt/cryptocb.py:90
[Medium] RNG output length not validated — wolfcrypt/cryptocb.py:96-97
[Medium] No tests for hash callbacks despite PR claim — tests/test_cryptocb.py
[Low] Free-floating string literal used as 'comment' in build_ffi.py is misleading — scripts/build_ffi.py:1376-1412

Skipped findings

[Medium] ~~Anonymous-union cdef relies on '...' partial layout — may not match reality~~
[Low] ~~ctx==NULL fallback would never be reached — wrong return code~~
[Low] ~~Type annotations on overridable methods declare positional args with leading underscores~~
[Low] ~~rng_callback signature passes raw cffi pointer with no type annotation~~
[Info] ~~Stale exception import in init.py guard~~

Review generated by Skoll

dgarske · 2026-05-05T18:37:54Z

Hi @roberthdevries nice work on this. See peer review feedback and also fix merge conflicts. Thanks

dgarske

Skoll Code Review

Scan type: reviewOverall recommendation: REQUEST_CHANGES
Findings: 7 total — 7 posted, 0 skipped
6 finding(s) posted as inline comments (see file-level comments below)

Posted findings

[High] Callback exceptions are converted to 0 (success) with unwritten output buffer — wolfcrypt/__init__.py:57-63, wolfcrypt/cryptocb.py:103-138
[Medium] cipher_callback path returns success (0) without performing any cipher operation — wolfcrypt/cryptocb.py:125-127,149-150
[Medium] Hash callbacks and error paths are untested; default-device-id test asserts nothing — tests/test_cryptocb.py:33-53
[Medium] del unregister not guarded against interpreter-shutdown errors — wolfcrypt/cryptocb.py:100-101,152-153
[Medium] CRYPTO_CB cdef hash union depends on SHA/SHA256/SHA384/SHA512/SHA3 cdefs being present — scripts/build_ffi.py:1424-1440
[Low] Inconsistent logging style in callback (f-string vs lazy %-formatting) — wolfcrypt/cryptocb.py:104,109
[Low] wolfCrypt_Init() added with no corresponding cleanup — wolfcrypt/__init__.py:53-55

Review generated by Skoll

roberthdevries

Not all issues are addressed. Will continue working on this after typing is merged so that this part can be typed as well.

dgarske · 2026-07-08T19:28:01Z

@roberthdevries now that the other PR's have been merged please take another look at the remaining items. Thanks!

dgarske

Skoll Code Review

Scan type: reviewOverall recommendation: REQUEST_CHANGES
Findings: 8 total — 8 posted, 0 skipped
8 finding(s) posted as inline comments (see file-level comments below)

Posted findings

[High] Import-time reference to _lib.INVALID_DEVID reintroduces issue #2659 regression — wolfcrypt/hashes.py:43
[High] device_id silently ignored for Sha256/Sha384/Sha512 (callback never fires) — wolfcrypt/hashes.py:235-298
[High] Uncaught callback exceptions report success (0) to wolfCrypt with an unfilled buffer — wolfcrypt/__init__.py:64-69
[Medium] Sha3 constructor does not expose device_id despite _init supporting it — wolfcrypt/hashes.py:352-358
[Medium] CryptoCallback.del not guarded for interpreter shutdown; double-unregister and self-reference cycle — wolfcrypt/cryptocb.py:96-103
[Medium] Missing test coverage for callback error paths and SHA-2 device_id — tests/test_cryptocb.py:1-87
[Low] Hash update-vs-finalize discriminator drops input when both in and digest are set — wolfcrypt/cryptocb.py:112-126
[Low] lib.pyi wc_HashInfo union field name does not match C member — wolfcrypt/_ffi/lib.pyi:592-601

Review generated by Skoll

dgarske

#114 (review)

roberthdevries · 2026-07-10T19:43:57Z

Now that the INVALID_DEVID constant is always available, the need for the regression test for the no-pqc job becomes questionable.
Propose to remove it from the pipeline

roberthdevries

Not all review comments addressed. More time needed.

dgarske · 2026-10-01T22:27:48Z

Hi @roberthdevries , is this PR ready for another review or are you planning to push further work?

dgarske · 2026-10-13T18:44:49Z

Hi @roberthdevries , checking on this again to see if you have pushed and finalized all the work planned. This is a really neat feature additional.

* Comment out code in build_ffi.py instead of quoted string. * Check return value of wolfCrypt_init() * Complete algorithm type hash table * Change dicts into defaultdicts with a sensible value when key is missing. * Convert debug print to debug log messages * Add length checks to produce nicer exceptions.

* Add if _lib.CRYPT_CB_ENABLED guards around code that depends on that configuration option being enabled.

* Add parametrized callback test for all Sha classes. * Add callback test for unsupported algo type AES GCM Stream.

…store the no-PQC import smoke step

…nd forward device_id in new()

…vice_id paths

…ument the override contract

Copilot

🟡 Changes recommended

CFFI field names break callback-enabled builds, and hash hooks cannot distinguish independent hash contexts.

2 open issues

What changed in this PR

Adds initial wolfCrypt callback support for random generation and hashing, with software fallback for unsupported operations.

Changes:

Adds callback registration, dispatch, and error handling.
Threads device IDs through cryptographic wrappers.
Extends CFFI bindings and adds callback tests.

File	Description
wolfcrypt/random.py	Uses the native invalid-device constant.
wolfcrypt/hashes.py	Adds device-aware hash and HMAC initialization.
wolfcrypt/exceptions.py	Retains native error codes.
wolfcrypt/cryptocb.py	Adds callback registration and dispatch.
wolfcrypt/ciphers.py	Adds device selection for streaming AES-GCM.
wolfcrypt/_ffi/lib.pyi	Declares callback types and native APIs.
wolfcrypt/__init__.py	Initializes wolfCrypt and registers the callback bridge.
tests/test_cryptocb.py	Tests callbacks, fallback, errors, and registration ownership.
scripts/build_ffi.py	Enables callbacks and extends native bindings.
.github/workflows/python-app.yml	Updates the regression-test explanatory comment.

_{Review effort: Balanced}

… free

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #114

Scan targets checked: wolfcrypt-py-src, wolfcrypt-py-bugs
Not reviewed (no scan target covers them): .github/workflows/python-app.yml, tests/test_cryptocb.py, wolfcrypt/_ffi/lib.pyi
Coverage: 3 of 7 in-scope changed file(s) opened by the reviewer; not opened: wolfcrypt/__init__.py, wolfcrypt/ciphers.py, wolfcrypt/exceptions.py, wolfcrypt/random.py

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

Review tier: Lite

…e wc_HmacCopy for HMAC digest

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #114

Scan targets checked: wolfcrypt-py-src, wolfcrypt-py-bugs
Not reviewed (no scan target covers them): .github/workflows/python-app.yml, tests/test_cryptocb.py, wolfcrypt/_ffi/lib.pyi
Coverage: 3 of 3 in-scope changed file(s) opened by the reviewer

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

Review tier: Lite

…ontexts are freed

JeremiahM37 reviewed Apr 28, 2026

View reviewed changes

Comment thread wolfcrypt/cryptocb.py Outdated

Comment thread wolfcrypt/__init__.py Outdated

Comment thread wolfcrypt/cryptocb.py Outdated

Comment thread wolfcrypt/cryptocb.py

Comment thread wolfcrypt/cryptocb.py

Comment thread wolfcrypt/cryptocb.py

Comment thread scripts/build_ffi.py Outdated

JeremiahM37 assigned roberthdevries Apr 28, 2026

dgarske assigned wolfSSL-Bot Apr 28, 2026

JeremiahM37 reviewed Apr 29, 2026

View reviewed changes

Comment thread wolfcrypt/cryptocb.py Outdated

roberthdevries force-pushed the add-support-for-crypto-callbacks branch from e10a1d3 to 1f974ad Compare May 18, 2026 18:08

dgarske unassigned roberthdevries Jun 8, 2026

dgarske reviewed Jun 8, 2026

View reviewed changes

Comment thread wolfcrypt/cryptocb.py Outdated

Comment thread tests/test_cryptocb.py

Comment thread wolfcrypt/cryptocb.py

Comment thread scripts/build_ffi.py

Comment thread wolfcrypt/cryptocb.py Outdated

Comment thread wolfcrypt/__init__.py

dgarske assigned roberthdevries Jun 8, 2026

dgarske assigned roberthdevries and unassigned roberthdevries Jul 6, 2026

roberthdevries force-pushed the add-support-for-crypto-callbacks branch from 1f974ad to 1ab3833 Compare July 6, 2026 20:48

roberthdevries commented Jul 6, 2026

View reviewed changes

Comment thread wolfcrypt/cryptocb.py Outdated

roberthdevries force-pushed the add-support-for-crypto-callbacks branch from 1ab3833 to d2debf4 Compare July 9, 2026 20:47

dgarske reviewed Jul 10, 2026

View reviewed changes

dgarske requested changes Jul 10, 2026

View reviewed changes

dgarske assigned roberthdevries and unassigned roberthdevries Jul 10, 2026

roberthdevries commented Jul 10, 2026

View reviewed changes

Comment thread wolfcrypt/hashes.py Outdated

Comment thread wolfcrypt/hashes.py

Comment thread wolfcrypt/__init__.py Outdated

Comment thread wolfcrypt/_ffi/lib.pyi

roberthdevries requested a review from dgarske July 15, 2026 17:51

roberthdevries force-pushed the add-support-for-crypto-callbacks branch 2 times, most recently from 9a0ecf7 to 55971eb Compare July 15, 2026 20:41

roberthdevries added 4 commits October 19, 2026 12:17

Add initial support for crypto callbacks.

e57bcfe

Address review comments

7412dd1

* Add if _lib.CRYPT_CB_ENABLED guards around code that depends on that configuration option being enabled.

Address review comments.

97820a5

roberthdevries and others added 6 commits October 19, 2026 12:17

Address review comments

d884b75

Add more tests.

483b532

* Add parametrized callback test for all Sha classes. * Add callback test for unsupported algo type AES GCM Stream.

Declare CRYPTOCB_UNAVAILABLE and WC_HW_E without ERROR_STRINGS and re…

2b042fa

…store the no-PQC import smoke step

Track crypto callback device ownership, require full hash overrides a…

42fcbc8

…nd forward device_id in new()

Add crypto callback tests for error, fallback, re-registration and de…

3da4b6d

…vice_id paths

Log crypto callback tracebacks, fail closed on handler errors and doc…

db57784

…ument the override contract

Copilot AI balanced review requested due to automatic review settings October 19, 2026 20:25

dgarske force-pushed the add-support-for-crypto-callbacks branch from 55971eb to db57784 Compare October 19, 2026 20:25

Copilot started reviewing on behalf of roberthdevries October 19, 2026 20:26

View session

Copilot AI reviewed Oct 19, 2026

View reviewed changes

Comment thread scripts/build_ffi.py

Comment thread wolfcrypt/cryptocb.py Outdated

dgarske self-assigned this Oct 19, 2026

Give each hash object its own crypto callback context across copy and…

cdfeacd

… free

dgarske previously approved these changes Oct 19, 2026

View reviewed changes

dgarske unassigned dgarske and roberthdevries Oct 19, 2026

dgarske requested a review from philljj October 19, 2026 21:28

dgarske dismissed their stale review via cdfeacd October 19, 2026 22:06

philljj self-assigned this Oct 21, 2026

philljj requested changes Oct 21, 2026

View reviewed changes

Comment thread wolfcrypt/cryptocb.py Outdated

philljj requested a review from wolfSSL-Fenrir-bot October 21, 2026 03:50

wolfSSL-Fenrir-bot requested changes Oct 21, 2026

View reviewed changes

Comment thread wolfcrypt/hashes.py

philljj assigned dgarske and unassigned wolfSSL-Bot Oct 21, 2026

Deep copy crypto callback hashes with wolfCrypt copy functions and us…

f0e3918

…e wc_HmacCopy for HMAC digest

dgarske requested a review from philljj October 21, 2026 19:14

dgarske assigned wolfSSL-Bot and unassigned dgarske Oct 22, 2026

philljj requested a review from wolfSSL-Fenrir-bot October 24, 2026 18:40

wolfSSL-Fenrir-bot requested changes Oct 24, 2026

View reviewed changes

Comment thread wolfcrypt/cryptocb.py

Keep a crypto callback device registered until its outstanding hash c…

6e05480

…ontexts are freed

Conversation

roberthdevries commented Apr 27, 2026

Uh oh!

JeremiahM37 left a comment

Choose a reason for hiding this comment

Skoll Code Review

Posted findings

Skipped findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dgarske commented May 5, 2026

Uh oh!

dgarske left a comment

Choose a reason for hiding this comment

Skoll Code Review

Posted findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

roberthdevries left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dgarske commented Jul 8, 2026

Uh oh!

dgarske left a comment

Choose a reason for hiding this comment

Skoll Code Review

Posted findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dgarske left a comment

Choose a reason for hiding this comment

Uh oh!

roberthdevries commented Jul 10, 2026

Uh oh!

roberthdevries left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dgarske commented Oct 1, 2026

Uh oh!

dgarske commented Oct 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #114

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #114