Skip to content

Add initial support for crypto callbacks. #114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
roberthdevries wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Add initial support for crypto callbacks. #114

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ec8c0cc
Add initial support for crypto callbacks.
roberthdevries Apr 8, 2026
57901a7
Addressed review comments
roberthdevries Apr 28, 2026
b7a0995
Address review comments
roberthdevries May 18, 2026
67794e4
Address review comments.
roberthdevries Jun 17, 2026
c7306ed
Fix ruff errors and add type info.
roberthdevries Jul 6, 2026
d2debf4
Addressed review comments
roberthdevries Jul 9, 2026
4532f74
INVALID_DEVID is also needed when the CRYPTO_CB feature is enabled.
roberthdevries Jul 9, 2026
edb22ae
INVALID_DEVID is always needed as default argument value.
roberthdevries Jul 10, 2026
90fce4b
Address review comments
roberthdevries Jul 10, 2026
55971eb
Add more tests.
roberthdevries Jul 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions .github/workflows/python-app.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,6 @@ jobs:
uv run --no-sync pytest tests

build-no-pqc:
# Regression coverage for issue #2659: INVALID_DEVID is only declared
# in the CFFI cdef when ML_KEM or ML_DSA is enabled, so a wolfSSL build
# without those features must still import wolfcrypt.random successfully.

runs-on: ubuntu-latest
env:
USE_LOCAL_WOLFSSL: ${{ github.workspace }}/wolfssl-install
Expand Down Expand Up @@ -93,7 +89,5 @@ jobs:
run: |
uv run --no-sync python -c "from wolfcrypt._ffi import lib as _lib; assert _lib.ML_KEM_ENABLED == 0, 'ML-KEM should be disabled'"
uv run --no-sync python -c "from wolfcrypt._ffi import lib as _lib; assert _lib.ML_DSA_ENABLED == 0, 'ML-DSA should be disabled'"
- name: Import smoke (regression for INVALID_DEVID)
run: uv run --no-sync python -c "from wolfcrypt.random import Random; Random()"
- name: Run tests
run: uv run --no-sync pytest tests
147 changes: 141 additions & 6 deletions scripts/build_ffi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,10 @@ def make_flags(prefix, fips):
# ML-DSA (note: to be able to use the legacy option of signing without context, pass `yes,no-ctx` as argument)
flags.append("--enable-mldsa=yes")

# Crypto Callbacks
flags.append("--enable-cryptocb")
# flags.append("EXTRACPPFLAGS=-DDEBUG_CRYPTOCB")

# disabling other configs enabled by default
flags.append("--disable-oldtls")
flags.append("--disable-oldnames")
Expand Down Expand Up @@ -394,6 +398,7 @@ def get_features(local_wolfssl, features):
# Unlike the other fatures, HASHDRBG is enabled by default in random.h, unless WC_NO_HASHDRBG or
# CUSTOM_RAND_GENERATE_BLOCK is defined.
features["HASHDRBG"] = 0 if ("#define WC_NO_HASHDRBG" in defines or "#define CUSTOM_RAND_GENERATE_BLOCK" in defines) else 1
features["CRYPTO_CB"] = 1 if "#define WOLF_CRYPTO_CB" in defines else 0

if '#define HAVE_FIPS' in defines:
if not fips:
Expand Down Expand Up @@ -471,6 +476,7 @@ def build_ffi(local_wolfssl, features):
#include <wolfssl/wolfcrypt/chacha20_poly1305.h>
#include <wolfssl/wolfcrypt/wc_mlkem.h>
#include <wolfssl/wolfcrypt/wc_mldsa.h>
#include <wolfssl/wolfcrypt/cryptocb.h>
"""

init_source_string = f"""
Expand Down Expand Up @@ -515,6 +521,7 @@ def build_ffi(local_wolfssl, features):
int ML_DSA_NO_CTX_ENABLED = {features["ML_DSA_NO_CTX"]};
int HKDF_ENABLED = {features["HKDF"]};
int HASHDRBG_ENABLED = {features["HASHDRBG"]};
int CRYPTO_CB_ENABLED = {features["CRYPTO_CB"]};
"""

ffibuilder.set_source( "wolfcrypt._ffi", init_source_string,
Expand Down Expand Up @@ -558,13 +565,18 @@ def build_ffi(local_wolfssl, features):
extern int ML_DSA_NO_CTX_ENABLED;
extern int HKDF_ENABLED;
extern int HASHDRBG_ENABLED;
extern int CRYPTO_CB_ENABLED;

static const int INVALID_DEVID;

typedef unsigned char byte;
typedef unsigned int word32;

typedef struct { ...; } WC_RNG;
typedef struct { ...; } OS_Seed;

int wolfCrypt_Init(void);

int wc_InitRng(WC_RNG*);
int wc_InitRngNonce(WC_RNG*, byte*, word32);
int wc_InitRngNonce_ex(WC_RNG*, byte*, word32, void*, int);
Expand Down Expand Up @@ -860,7 +872,7 @@ def build_ffi(local_wolfssl, features):
if features["SHA"]:
cdef += """
typedef struct { ...; } wc_Sha;
int wc_InitSha(wc_Sha*);
int wc_InitSha_ex(wc_Sha*, void*, int);
int wc_ShaUpdate(wc_Sha*, const byte*, word32);
int wc_ShaFinal(wc_Sha*, byte*);
void wc_ShaFree(wc_Sha*);
Expand Down Expand Up @@ -1304,11 +1316,6 @@ def build_ffi(local_wolfssl, features):
int wolfCrypt_GetPrivateKeyReadEnable_fips(enum wc_KeyType);
"""

if features["ML_KEM"] or features["ML_DSA"]:
cdef += """
static const int INVALID_DEVID;
"""

if features["ML_KEM"]:
cdef += """
static const int WC_ML_KEM_512;
Expand Down Expand Up @@ -1363,6 +1370,133 @@ def build_ffi(local_wolfssl, features):
int wc_dilithium_verify_msg(const byte* sig, word32 sigLen, const byte* msg, word32 msgLen, int* res, dilithium_key* key);
"""

if features["CRYPTO_CB"]:
cdef += """
static const int WC_ALGO_TYPE_NONE;
static const int WC_ALGO_TYPE_HASH;
static const int WC_ALGO_TYPE_CIPHER;
static const int WC_ALGO_TYPE_PK;
static const int WC_ALGO_TYPE_RNG;
static const int WC_ALGO_TYPE_SEED;
static const int WC_ALGO_TYPE_HMAC;
static const int WC_ALGO_TYPE_CMAC;
static const int WC_ALGO_TYPE_CERT;
static const int WC_ALGO_TYPE_KDF;
static const int WC_ALGO_TYPE_COPY;
static const int WC_ALGO_TYPE_FREE;
static const int WC_ALGO_TYPE_MAX;

static const int WC_HASH_TYPE_SHA; /* SHA-1 (not old SHA-0) */
static const int WC_HASH_TYPE_SHA256;
static const int WC_HASH_TYPE_SHA384;
static const int WC_HASH_TYPE_SHA512;
static const int WC_HASH_TYPE_SHA3_256;
static const int WC_HASH_TYPE_SHA3_384;
static const int WC_HASH_TYPE_SHA3_512;
"""


cdef += """
typedef struct {
int algo_type; /* enum wc_AlgoType */
union {
"""

# The following block is commented out as there are 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.
#
# cdef += """
# struct {
# int type; /* enum wc_CipherType */
# int enc;
# union {
# //wc_CryptoCb_AesAuthEnc aesgcm_enc;
# //wc_CryptoCb_AesAuthDec aesgcm_dec;
# //wc_CryptoCb_AesAuthEnc aesccm_enc;
# //wc_CryptoCb_AesAuthDec aesccm_dec;
# struct {
# Aes* aes;
# byte* out;
# const byte* in;
# word32 sz;
# } aescbc;
# //struct {
# // Aes* aes;
# // byte* out;
# // const byte* in;
# // word32 sz;
# //} aesctr;
# //struct {
# // Aes* aes;
# // byte* out;
# // const byte* in;
# // word32 sz;
# //} aesecb;
# //struct {
# // Des3* des;
# // byte* out;
# // const byte* in;
# // word32 sz;
# //} des3;
# //void* ctx;
# };
# } cipher;
# """

cdef += """
Comment thread
dgarske marked this conversation as resolved.
struct {
int type; /* enum wc_HashType */
const byte* data;
word32 data_size;
byte* digest;
union {
"""
if features["SHA"]:
cdef += """
wc_Sha* sha1;
"""
if features["SHA256"]:
cdef += """
wc_Sha256* sha256;
"""
if features["SHA384"]:
cdef += """
wc_Sha384* sha384;
"""
if features["SHA512"]:
cdef += """
wc_Sha512* sha512;
"""
if features["SHA3"]:
cdef += """
wc_Sha3* sha3;
"""
cdef += """
void* ctx;
} u;
} hash;
"""
cdef += """
struct {
WC_RNG* rng;
byte* out;
word32 sz;
} rng;
};
...;
} wc_CryptoInfo;

typedef int (*CryptoDevCallbackFunc)(int devId, wc_CryptoInfo* info, void* ctx);
extern "Python" int py_wc_crypto_callback(int devId, wc_CryptoInfo* info, void* ctx);
int wc_CryptoCb_RegisterDevice(int devId, CryptoDevCallbackFunc cb, void* ctx);
void wc_CryptoCb_UnRegisterDevice(int devId);
int wc_CryptoCb_DefaultDevID();
// void wc_CryptoCb_InfoString(wc_CryptoInfo* info);
"""

ffibuilder.cdef(cdef)

def main(ffibuilder):
Expand Down Expand Up @@ -1400,6 +1534,7 @@ def main(ffibuilder):
"ML_DSA_NO_CTX": 0,
"HKDF": 1,
"HASHDRBG": 1,
"CRYPTO_CB": 1,
}

# Ed448 requires SHAKE256, which isn't part of the Windows build, yet.
Expand Down
87 changes: 87 additions & 0 deletions tests/test_cryptocb.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
# test_cryptocb.py

@dgarske dgarske Jul 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🟠 [Medium] Missing test coverage for callback error paths and SHA-2 device_id

The new tests cover only the happy path for RNG (SHA-1 hash) and default_device_id. There is no coverage for: (a) a callback returning the wrong number of bytes (the ValueError guard in callback - important given the def_extern success-on-exception issue above); (b) an unsupported algo/hash type returning CRYPTOCB_UNAVAILABLE; (c) Sha256/Sha384/Sha512 with device_id set (which would have surfaced that device_id is silently ignored for those classes); (d) the not-implemented default methods raising NotImplementedError. These gaps let the two SHA-2 and exception-handling bugs above pass CI.

Fix: Extend test_cryptocb.py to cover error paths, unsupported-algo fallthrough, and SHA-256/384/512 device routing.

@roberthdevries roberthdevries Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added a Sha256,Sha384,Sha512 and a Sha3 test case. (good flow).
Added a test with an empty crypto callback instance for AesGcmStream that confirms that no behavior is changed.

#
# Copyright (C) 2026 wolfSSL Inc.
#
# This file is part of wolfSSL. (formerly known as CyaSSL)
#
# wolfSSL is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 2 of the License, or
# (at your option) any later version.
#
# wolfSSL is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
# ty: ignore[possibly-missing-import]

from __future__ import annotations

import struct

import pytest
from typing_extensions import override

from wolfcrypt._ffi import lib as _lib
from wolfcrypt.random import Random

if _lib.SHA_ENABLED:
from wolfcrypt.hashes import Sha

if not _lib.CRYPTO_CB_ENABLED:
pytest.skip("Crypto Callbacks not supported", allow_module_level=True)

from wolfcrypt.cryptocb import CryptoCallback


def test_default_device_id():

@dgarske dgarske Jun 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🟠 [Medium] Hash callbacks and error paths are untested; default-device-id test asserts nothing

Only the RNG callback path is exercised. There is no coverage for hash_update_callback / hash_finalize_callback (the most complex branch in callback(), including the NULL-digest update vs finalize distinction and the digest-length validation), no coverage for the length-mismatch error branches (which is exactly where the success-on-exception bug above manifests), and no coverage for the cipher / unknown-algo fallback to CRYPTOCB_UNAVAILABLE. test_default_device_id only print()s the value and asserts nothing, so it cannot fail meaningfully.

Fix: Add tests for the hash update/finalize paths, the length-mismatch/error paths, and the unknown-algo fallback; make test_default_device_id assert on the return value.

@roberthdevries roberthdevries Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed the test_default_device_id so that it checks that the expected value (according to the configuration used for the Python version).

Added test to check support for crypto callbacks for hash functions. For this some infrastructure had to be added to the _Hash class to support device_ids needed to call the callbacks.

# In the python implementation the default device ID is the invalid device ID.
assert CryptoCallback.default_device_id() == _lib.INVALID_DEVID

class RngCryptoCallback(CryptoCallback):
@override
def rng_callback(self, device_id: int, rng: _lib.RNG, size: int) -> bytes:
# Generate fake random data for testing purposes.
return bytes(range(1, 1 + size))


def test_rng_callback():
with RngCryptoCallback(10):
rng = Random(device_id=10)

random = rng.byte()
assert random == b"\01"

random = rng.bytes(1)
assert random == b"\01"

random = rng.bytes(3)
assert random == b"\01\02\03"

class HashCryptoCallback(CryptoCallback):
def __init__(self, device_id):
super().__init__(device_id)
self.data: list[bytes] = []

@override
def hash_update_callback(self, device_id: int, hash_type: int, data: bytes) -> None:
self.data.append(data)

@override
def hash_finalize_callback(self, device_id: int, hash_type: int) -> bytes:
# quite lame hash function, just returns the length of the data as an integer (padded to match the expected hash length).
return struct.pack("I16x", len(b"".join(self.data)))


if _lib.SHA_ENABLED:
def test_hash_callback():
with HashCryptoCallback(11):
sha = Sha(device_id=11)
sha.update(bytes(10))
sha.update(bytes(5))
digest = sha.digest()
assert digest == struct.pack("I16x", 15)
28 changes: 24 additions & 4 deletions wolfcrypt/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA

import os
import sys
from typing import TYPE_CHECKING

from wolfcrypt._version import __version__, __wolfssl_version__

__title__ = "wolfcrypt"
Expand All @@ -33,21 +37,37 @@
__all__ = [
"__title__", "__summary__", "__uri__", "__version__", "__wolfssl_version__",
"__author__", "__email__", "__license__", "__copyright__",
"ciphers", "hashes", "random", "pwdbased"
"ciphers", "hashes", "random", "pwdbased", "cryptocb"
]

import os
import sys

top_level_py = os.path.basename(sys.argv[0])

# The code below is intended to only be used after the CFFI is built, so we
# don't want it invoked whilst building the CFFI with build_ffi.py or setup.py.
if top_level_py not in ["setup.py", "build_ffi.py"]:
from wolfcrypt._ffi import ffi as _ffi
from wolfcrypt._ffi import lib as _lib

if TYPE_CHECKING:
if _lib.CRYPTO_CB_ENABLED:
from wolfcrypt.cryptocb import CryptoCallback # ty: ignore[possibly-missing-import]
from wolfcrypt.exceptions import WolfCryptApiError

# Only wolfCrypt_Init() is called here.
# Calling wolfCrypt_Cleanup() is not needed as the application exit() will clean up the entire process
# including any wolfcrypt data in any case.
ret = _lib.wolfCrypt_Init()

@dgarske dgarske Jun 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🔵 [Low] wolfCrypt_Init() added with no corresponding cleanup

wolfCrypt_Init() is now called unconditionally on package import (a reasonable change, and required for the cryptocb registration to work), but there is no matching wolfCrypt_Cleanup() registered (e.g. via atexit). For a process-lifetime library this is generally acceptable, but it is worth a deliberate decision/comment so it is clear cleanup was intentionally omitted rather than forgotten.

Fix: Confirm the omission of wolfCrypt_Cleanup() is intentional; add a brief comment or an atexit cleanup if appropriate.

@roberthdevries roberthdevries Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added a comment that it is not needed to call this function. This function initializes a global data structure that lives as long as the application and gets cleaned up together with the process.

if ret < 0:
raise WolfCryptApiError("WolfCrypt_Init failed", ret)

if _lib.CRYPTO_CB_ENABLED:
@_ffi.def_extern()

@dgarske dgarske Jul 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🔴 [High] Uncaught callback exceptions report success (0) to wolfCrypt with an unfilled buffer

CryptoCallback.callback only catches NotImplementedError; any other exception (e.g. the ValueError it deliberately raises when len(out) != info.rng.sz or the digest length is wrong) propagates out through the @_ffi.def_extern() function py_wc_crypto_callback. cffi's def_extern with no error=/onerror= argument prints the traceback to stderr and returns the default value 0 to the C caller. In the wolfCrypt crypto-callback convention 0 means 'handled successfully'. So a buggy user callback that returns the wrong number of bytes causes wolfCrypt to treat the operation as successful while the output buffer (info.rng.out / info.hash.digest) was never written. For RNG this yields uninitialized stack/heap memory being consumed as 'random' data. The length-check raise ValueError(...) is therefore not just ineffective, it is actively harmful because the error is swallowed into a success code.

Fix: Pass a negative error= (and/or onerror=) to @_ffi.def_extern() so an uncaught exception returns a hard failure to wolfCrypt, and/or wrap the body of callback in a broad except Exception: that logs and returns a negative error code instead of letting exceptions escape. Add a test that a wrong-length RNG/digest return does not report success.

@roberthdevries roberthdevries Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added an onerror callback that checks the exception type for a WolfCryptApiError and then returns the error code of that exception, in all other cases it returns a generic -1 error code.
It will log an error message for debugging purposes with the original exception string.

Also added a test to check the correct handling of a ValueError.

def py_wc_crypto_callback(device_id: int, info: _ffi.CData, ctx: _ffi.CData) -> int:
if ctx == _ffi.NULL:
return _lib.CRYPTOCB_UNAVAILABLE
crypto_cb: CryptoCallback = _ffi.from_handle(ctx)
return crypto_cb.callback(device_id, info)

if hasattr(_lib, 'WC_RNG_SEED_CB_ENABLED'):
if _lib.WC_RNG_SEED_CB_ENABLED:
ret = _lib.wc_SetSeed_Cb(_ffi.addressof(_lib, "wc_GenerateSeed")) # ty: ignore[no-matching-overload]
Expand Down
Loading