From 69440d3d88cec126df8d7337971f87d18327f549 Mon Sep 17 00:00:00 2001 From: night1rider Date: Wed, 26 Aug 2026 21:34:05 -0600 Subject: [PATCH 1/3] Regression tests for crypto callback registration from a register command --- .github/configs/os-check-linux.json | 3 + doc/dox_comments/header_files/cryptocb.h | 17 ++ tests/api.c | 236 +++++++++++++++++++++ tests/unit-mcdc/test_cryptocb_whitebox.c | 50 ++++- wolfcrypt/src/cryptocb.c | 3 +- wolfcrypt/test/test.c | 259 +++++++++++++++++++++++ 6 files changed, 566 insertions(+), 2 deletions(-) diff --git a/.github/configs/os-check-linux.json b/.github/configs/os-check-linux.json index d3ec20c7331..e6af488018b 100644 --- a/.github/configs/os-check-linux.json +++ b/.github/configs/os-check-linux.json @@ -149,6 +149,9 @@ {"name": "cryptocb-utils-setkey-export-find", "minutes": 2.2, "configure": ["--enable-cryptocb", "--enable-keygen", "--enable-cryptocbutils=setkey,export", "CPPFLAGS=-DWOLF_CRYPTO_CB_FIND"]}, +{"name": "cryptocb-cmd", "minutes": 2.2, + "comment": "Only WOLF_CRYPTO_CB_CMD builds the register/unregister command dispatch, so without this entry the nested-registration regression tests never compile in CI.", + "configure": ["--enable-cryptocb", "CPPFLAGS=-DWOLF_CRYPTO_CB_CMD"]}, {"name": "cryptocb-shake", "minutes": 2.2, "comment": "Exercises the SHAKE128/SHAKE256 crypto callback wiring (wc_CryptoCb_Shake, the sha3.c Update/Final hooks, and the dedicated offload unit tests). cryptocbutils=copy,free adds the Copy/Free dispatch, which must see WC_HASH_TYPE_SHAKE* (not NONE) from SHAKE contexts. A normal (non-ONLY) cryptocb build keeps the host software SHA3/SHAKE present as the callbacks' offload fallback.", "configure": ["--enable-cryptocb", "--enable-sha3", diff --git a/doc/dox_comments/header_files/cryptocb.h b/doc/dox_comments/header_files/cryptocb.h index b5e716ef6b5..8bbbe41c0f7 100644 --- a/doc/dox_comments/header_files/cryptocb.h +++ b/doc/dox_comments/header_files/cryptocb.h @@ -11,6 +11,20 @@ For TPM based crypto callbacks example see the wolfTPM2_CryptoDevCb function in wolfTPM src/tpm2_wrap.c + With WOLF_CRYPTO_CB_CMD defined, the callback is invoked during this + call with a WC_CRYPTOCB_CMD_TYPE_REGISTER command (and later with + WC_CRYPTOCB_CMD_TYPE_UNREGISTER from wc_CryptoCb_UnRegisterDevice). A + register command handler may itself call wc_CryptoCb_RegisterDevice to + register additional devIds, such as a driver exposing several devices. + It must not dispatch through, or re-register, the devId it is being + registered with: that devId is not published until this call returns, + so a nested registration of it is not caught by the ALREADY_E check and + consumes fresh slots until the table fills and BUFFER_E unwinds them. + If the handler returns an error other than CRYPTOCB_UNAVAILABLE or + NOT_COMPILED_IN, the registration is unwound and the error returned; + devIds the handler already registered stay registered and are the + caller's to clean up. + \return CRYPTOCB_UNAVAILABLE to fallback to using software crypto \return 0 for success \return ALREADY_E if devId is already registered. A devId must be @@ -22,6 +36,9 @@ \param devId any unique value, not -2 (INVALID_DEVID) \param cb a callback function with prototype: typedef int (*CryptoDevCallbackFunc)(int devId, wc_CryptoInfo* info, void* ctx); + \param ctx user context handed back to the callback on every invocation. + With WOLF_CRYPTO_CB_CMD, a register command handler may replace it by + updating info->cmd.ctx on success. _Example_ \code diff --git a/tests/api.c b/tests/api.c index 3618a957733..50471462ad7 100644 --- a/tests/api.c +++ b/tests/api.c @@ -34773,6 +34773,239 @@ static int test_wc_CryptoCb_TLS(int tlsVer, } #endif /* WOLF_CRYPTO_CB && HAVE_IO_TESTS_DEPENDENCIES */ +#if defined(WOLF_CRYPTO_CB) && defined(WOLF_CRYPTO_CB_CMD) + /* The nested scenarios hold up to five devices at once, so skip the + * test on smaller tables. */ + #if MAX_CRYPTO_DEVID_CALLBACKS >= 5 + #define TEST_CRYPTOCB_NESTED_REGISTER + #endif +#endif + +#ifdef TEST_CRYPTOCB_NESTED_REGISTER +#define TEST_CRYPTOCB_NESTED_PARENT_DEVID 0x5A00 +#define TEST_CRYPTOCB_NESTED_CHILD_DEVID 0x5A10 +/* Grandchild devId is its child's devId plus this offset. */ +#define TEST_CRYPTOCB_NESTED_GRANDCHILD_DIFF 0x20 +#define TEST_CRYPTOCB_NESTED_SELF_DEVID 0x5A40 +#define TEST_CRYPTOCB_NESTED_FILLER_DEVID 0x5A50 +#define TEST_CRYPTOCB_NESTED_NUM_CHILDREN 2 + +/* Which callback saw the last unregister command: 1 child, 2 parent. */ +static int test_CryptoCb_lastUnregCb; + +static int test_CryptoCb_NestedGrandchild_Func(int devId, wc_CryptoInfo* info, + void* ctx) +{ + (void)devId; + (void)info; + (void)ctx; + + return WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE); +} + +static int test_CryptoCb_NestedChild_Func(int devId, wc_CryptoInfo* info, + void* ctx) +{ + (void)ctx; + + if (info == NULL || info->algo_type != WC_ALGO_TYPE_NONE) + return WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE); + + if (info->cmd.type == WC_CRYPTOCB_CMD_TYPE_REGISTER) { + /* One level deeper: each child brings up its own sub-device. */ + return wc_CryptoCb_RegisterDevice( + devId + TEST_CRYPTOCB_NESTED_GRANDCHILD_DIFF, + test_CryptoCb_NestedGrandchild_Func, NULL); + } + if (info->cmd.type == WC_CRYPTOCB_CMD_TYPE_UNREGISTER) + test_CryptoCb_lastUnregCb = 1; + return WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE); +} + +static int test_CryptoCb_SelfRegister_Func(int devId, wc_CryptoInfo* info, + void* ctx) +{ + (void)ctx; + + if (info != NULL && info->algo_type == WC_ALGO_TYPE_NONE && + info->cmd.type == WC_CRYPTOCB_CMD_TYPE_REGISTER) { + /* Our own devId is not published yet, so this recurses into a fresh + * slot each time until the table fills and BUFFER_E unwinds it. */ + return wc_CryptoCb_RegisterDevice(devId, + test_CryptoCb_SelfRegister_Func, NULL); + } + return WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE); +} + +static int test_CryptoCb_NestedParent_Func(int devId, wc_CryptoInfo* info, + void* ctx) +{ + int i; + (void)devId; + (void)ctx; + + if (info == NULL || info->algo_type != WC_ALGO_TYPE_NONE) + return WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE); + + if (info->cmd.type == WC_CRYPTOCB_CMD_TYPE_REGISTER) { + /* Register more devices from inside our own register command, like + * ports that expose several devIds from one driver. */ + for (i = 0; i < TEST_CRYPTOCB_NESTED_NUM_CHILDREN; i++) { + int rc = wc_CryptoCb_RegisterDevice( + TEST_CRYPTOCB_NESTED_CHILD_DEVID + i, + test_CryptoCb_NestedChild_Func, NULL); + if (rc != 0) + return rc; + } + return 0; + } + if (info->cmd.type == WC_CRYPTOCB_CMD_TYPE_UNREGISTER) + test_CryptoCb_lastUnregCb = 2; + return WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE); +} +#endif /* TEST_CRYPTOCB_NESTED_REGISTER */ + +static int test_wc_CryptoCb_nested_register(void) +{ + EXPECT_DECLS; +#ifdef TEST_CRYPTOCB_NESTED_REGISTER + int i; + int rc; + int fillers; + + /* Registering devices from inside a register command must not hand a + * child (or grandchild) the half filled slot of a caller up the chain. */ + ExpectIntEQ(wc_CryptoCb_RegisterDevice(TEST_CRYPTOCB_NESTED_PARENT_DEVID, + test_CryptoCb_NestedParent_Func, NULL), 0); + + /* Parent, every child, and every grandchild must all be registered. */ + ExpectIntEQ(wc_CryptoCb_IsDeviceRegistered( + TEST_CRYPTOCB_NESTED_PARENT_DEVID), 1); + for (i = 0; i < TEST_CRYPTOCB_NESTED_NUM_CHILDREN; i++) { + ExpectIntEQ(wc_CryptoCb_IsDeviceRegistered( + TEST_CRYPTOCB_NESTED_CHILD_DEVID + i), 1); + ExpectIntEQ(wc_CryptoCb_IsDeviceRegistered( + TEST_CRYPTOCB_NESTED_CHILD_DEVID + i + + TEST_CRYPTOCB_NESTED_GRANDCHILD_DIFF), 1); + } + + for (i = 0; i < TEST_CRYPTOCB_NESTED_NUM_CHILDREN; i++) { + wc_CryptoCb_UnRegisterDevice(TEST_CRYPTOCB_NESTED_CHILD_DEVID + i); + wc_CryptoCb_UnRegisterDevice(TEST_CRYPTOCB_NESTED_CHILD_DEVID + i + + TEST_CRYPTOCB_NESTED_GRANDCHILD_DIFF); + } + + /* The parent's slot must still hold the parent's callback, so its own + * unregister handler runs, not a child's. */ + test_CryptoCb_lastUnregCb = 0; + wc_CryptoCb_UnRegisterDevice(TEST_CRYPTOCB_NESTED_PARENT_DEVID); + ExpectIntEQ(test_CryptoCb_lastUnregCb, 2); + ExpectIntEQ(wc_CryptoCb_IsDeviceRegistered( + TEST_CRYPTOCB_NESTED_PARENT_DEVID), 0); + + /* A failing nested registration must unwind cleanly: pre-register the + * first child devId so the parent's register command hits ALREADY_E. */ + ExpectIntEQ(wc_CryptoCb_RegisterDevice(TEST_CRYPTOCB_NESTED_CHILD_DEVID, + NULL, NULL), 0); + ExpectIntEQ(wc_CryptoCb_RegisterDevice(TEST_CRYPTOCB_NESTED_PARENT_DEVID, + test_CryptoCb_NestedParent_Func, NULL), WC_NO_ERR_TRACE(ALREADY_E)); + ExpectIntEQ(wc_CryptoCb_IsDeviceRegistered( + TEST_CRYPTOCB_NESTED_PARENT_DEVID), 0); + wc_CryptoCb_UnRegisterDevice(TEST_CRYPTOCB_NESTED_CHILD_DEVID); + + /* Partial success: pre-register the second child so the parent's command + * registers child 0 (and its grandchild), then fails on child 1. Only + * the parent's own slot is unwound; devices the command already + * registered survive and are the caller's to clean up. */ + ExpectIntEQ(wc_CryptoCb_RegisterDevice( + TEST_CRYPTOCB_NESTED_CHILD_DEVID + 1, NULL, NULL), 0); + ExpectIntEQ(wc_CryptoCb_RegisterDevice(TEST_CRYPTOCB_NESTED_PARENT_DEVID, + test_CryptoCb_NestedParent_Func, NULL), WC_NO_ERR_TRACE(ALREADY_E)); + ExpectIntEQ(wc_CryptoCb_IsDeviceRegistered( + TEST_CRYPTOCB_NESTED_PARENT_DEVID), 0); + ExpectIntEQ(wc_CryptoCb_IsDeviceRegistered( + TEST_CRYPTOCB_NESTED_CHILD_DEVID), 1); + ExpectIntEQ(wc_CryptoCb_IsDeviceRegistered( + TEST_CRYPTOCB_NESTED_CHILD_DEVID + + TEST_CRYPTOCB_NESTED_GRANDCHILD_DIFF), 1); + wc_CryptoCb_UnRegisterDevice(TEST_CRYPTOCB_NESTED_CHILD_DEVID); + wc_CryptoCb_UnRegisterDevice(TEST_CRYPTOCB_NESTED_CHILD_DEVID + + TEST_CRYPTOCB_NESTED_GRANDCHILD_DIFF); + wc_CryptoCb_UnRegisterDevice(TEST_CRYPTOCB_NESTED_CHILD_DEVID + 1); + + /* Table-full boundary: leave exactly one free slot, so the parent takes + * it and the nested child finds none (the parent's half filled slot must + * be skipped, not handed out) and BUFFER_E unwinds the registration. */ + rc = 0; + for (fillers = 0; rc == 0 && fillers <= MAX_CRYPTO_DEVID_CALLBACKS; ) { + rc = wc_CryptoCb_RegisterDevice(TEST_CRYPTOCB_NESTED_FILLER_DEVID + + fillers, NULL, NULL); + if (rc == 0) + fillers++; + } + ExpectIntEQ(rc, WC_NO_ERR_TRACE(BUFFER_E)); + ExpectIntGT(fillers, 0); + if (fillers > 0) { + fillers--; + wc_CryptoCb_UnRegisterDevice(TEST_CRYPTOCB_NESTED_FILLER_DEVID + + fillers); + } + + ExpectIntEQ(wc_CryptoCb_RegisterDevice(TEST_CRYPTOCB_NESTED_PARENT_DEVID, + test_CryptoCb_NestedParent_Func, NULL), WC_NO_ERR_TRACE(BUFFER_E)); + ExpectIntEQ(wc_CryptoCb_IsDeviceRegistered( + TEST_CRYPTOCB_NESTED_PARENT_DEVID), 0); + ExpectIntEQ(wc_CryptoCb_IsDeviceRegistered( + TEST_CRYPTOCB_NESTED_CHILD_DEVID), 0); + + /* Two free slots: the parent and first child each hold a half filled + * slot when the grandchild's registration hits BUFFER_E, so the free + * slot search must skip both and the error must cascade through both + * frames, clearing every half filled slot on the way out. */ + if (fillers > 0) { + fillers--; + wc_CryptoCb_UnRegisterDevice(TEST_CRYPTOCB_NESTED_FILLER_DEVID + + fillers); + } + ExpectIntEQ(wc_CryptoCb_RegisterDevice(TEST_CRYPTOCB_NESTED_PARENT_DEVID, + test_CryptoCb_NestedParent_Func, NULL), WC_NO_ERR_TRACE(BUFFER_E)); + ExpectIntEQ(wc_CryptoCb_IsDeviceRegistered( + TEST_CRYPTOCB_NESTED_PARENT_DEVID), 0); + ExpectIntEQ(wc_CryptoCb_IsDeviceRegistered( + TEST_CRYPTOCB_NESTED_CHILD_DEVID), 0); + ExpectIntEQ(wc_CryptoCb_IsDeviceRegistered( + TEST_CRYPTOCB_NESTED_CHILD_DEVID + + TEST_CRYPTOCB_NESTED_GRANDCHILD_DIFF), 0); + + for (i = 0; i < fillers; i++) { + wc_CryptoCb_UnRegisterDevice(TEST_CRYPTOCB_NESTED_FILLER_DEVID + i); + } + + /* The unwound slots must be reusable once the table has room again. */ + ExpectIntEQ(wc_CryptoCb_RegisterDevice(TEST_CRYPTOCB_NESTED_PARENT_DEVID, + test_CryptoCb_NestedParent_Func, NULL), 0); + for (i = 0; i < TEST_CRYPTOCB_NESTED_NUM_CHILDREN; i++) { + wc_CryptoCb_UnRegisterDevice(TEST_CRYPTOCB_NESTED_CHILD_DEVID + i); + wc_CryptoCb_UnRegisterDevice(TEST_CRYPTOCB_NESTED_CHILD_DEVID + i + + TEST_CRYPTOCB_NESTED_GRANDCHILD_DIFF); + } + wc_CryptoCb_UnRegisterDevice(TEST_CRYPTOCB_NESTED_PARENT_DEVID); + + /* Re-registering your own devId from your own register command is + * prohibited: the unpublished devId passes the ALREADY_E check, so it + * recurses into fresh slots until the table fills, then BUFFER_E + * unwinds every one of them. */ + ExpectIntEQ(wc_CryptoCb_RegisterDevice(TEST_CRYPTOCB_NESTED_SELF_DEVID, + test_CryptoCb_SelfRegister_Func, NULL), WC_NO_ERR_TRACE(BUFFER_E)); + ExpectIntEQ(wc_CryptoCb_IsDeviceRegistered( + TEST_CRYPTOCB_NESTED_SELF_DEVID), 0); + ExpectIntEQ(wc_CryptoCb_RegisterDevice(TEST_CRYPTOCB_NESTED_SELF_DEVID, + NULL, NULL), 0); + wc_CryptoCb_UnRegisterDevice(TEST_CRYPTOCB_NESTED_SELF_DEVID); +#endif + return EXPECT_RESULT(); +} + static int test_wc_CryptoCb(void) { EXPECT_DECLS; @@ -42017,6 +42250,9 @@ TEST_CASE testCases[] = { * unconditionally, as on master. */ /* Can't memory test as client/server hangs. */ TEST_DECL(test_wc_CryptoCb), + /* Unconditional shell (body self-guards on WOLF_CRYPTO_CB && + * WOLF_CRYPTO_CB_CMD and a big-enough callback table). */ + TEST_DECL(test_wc_CryptoCb_nested_register), /* Can't memory test as client/server hangs. */ TEST_DECL(test_wolfSSL_CTX_StaticMemory), #if !defined(NO_FILESYSTEM) && \ diff --git a/tests/unit-mcdc/test_cryptocb_whitebox.c b/tests/unit-mcdc/test_cryptocb_whitebox.c index 9750ef5c2f4..69d88d53836 100644 --- a/tests/unit-mcdc/test_cryptocb_whitebox.c +++ b/tests/unit-mcdc/test_cryptocb_whitebox.c @@ -139,6 +139,15 @@ * this file is evaluated with CryptoCb_FindCb == NULL, i.e. with * wc_CryptoCb_FindDevice() behaving exactly as in the shipped * variants. + * + * Fourth pass - wc_CryptoCb_RegisterDevice()'s free-slot scan, + * `devId == INVALID_DEVID && cb == NULL` in wc_CryptoCb_GetFreeDevice(). + * The (T,F) half of the pair - a slot part way through registration, with + * cb already set but devId not yet published - exists only transiently + * inside a WOLF_CRYPTO_CB_CMD register command, so the vector plants it + * directly in gCryptoDev[]; see the section just before the final + * unregisters. (T,T) is every successful registration in this file and + * (F,-) every occupied slot the scan walks past. */ /* See the "third pass" note above: compiled in for this TU only, so the @@ -252,7 +261,7 @@ int main(void) /* gCryptoDev is a plain static array; its BSS zero-init leaves every * slot's devId == 0, not INVALID_DEVID. wc_CryptoCb_RegisterDevice() - * looks for a free slot via wc_CryptoCb_GetDevice(INVALID_DEVID), so + * looks for a free slot via wc_CryptoCb_GetFreeDevice(), so * without this call every registration below fails with BUFFER_E ("out * of devices") - none of the BSS-zeroed slots match INVALID_DEVID. * wc_CryptoCb_Init() marks all slots devId == INVALID_DEVID, matching @@ -1364,6 +1373,45 @@ int main(void) WB_NOTE("HAVE_CURVE448 not defined; Curve448MakePub/Generic skipped"); #endif + /* ---- wc_CryptoCb_GetFreeDevice: `devId == INVALID_DEVID && + * cb == NULL` (cryptocb.c :409) ---- + * See the "fourth pass" note in the file header: the (T,F) half of the + * pair is planted directly, since it only exists transiently inside a + * WOLF_CRYPTO_CB_CMD register command. */ + { + int slot; + + /* (T,F): slot 0 half filled. A registration must skip it, land in + * a later slot, and leave the half filled slot untouched. */ + wc_CryptoCb_Init(); + gCryptoDev[0].cb = wb_cb; + if (wc_CryptoCb_RegisterDevice(WB_DEVID, wb_cb, NULL) != 0) + wb_fail = 1; + if (wc_CryptoCb_GetDevice(WB_DEVID) == &gCryptoDev[0]) + wb_fail = 1; + if (gCryptoDev[0].devId != INVALID_DEVID || gCryptoDev[0].cb != wb_cb) + wb_fail = 1; + + /* With every other slot registered, the scan must reject the half + * filled slot rather than hand it out: no free slot, BUFFER_E. */ + for (slot = 0; slot < MAX_CRYPTO_DEVID_CALLBACKS; slot++) { + (void)wc_CryptoCb_RegisterDevice(WB_DEVID_FILL + slot, wb_cb, + NULL); + } + if (wc_CryptoCb_RegisterDevice(WB_DEVID_NOCB, NULL, NULL) != + WC_NO_ERR_TRACE(BUFFER_E)) + wb_fail = 1; + if (gCryptoDev[0].devId != INVALID_DEVID || gCryptoDev[0].cb != wb_cb) + wb_fail = 1; + WB_NOTE("GetFreeDevice: devId==INVALID_DEVID&&cb==NULL [:409] " + "(T,F) half filled slot skipped, full-table BUFFER_E"); + + /* Leave the table the way the rest of this file expects it. */ + wc_CryptoCb_Init(); + if (wc_CryptoCb_RegisterDevice(WB_DEVID, wb_cb, NULL) != 0) + wb_fail = 1; + } + wc_CryptoCb_UnRegisterDevice(WB_DEVID); wc_CryptoCb_UnRegisterDevice(WB_DEVID_NOCB); wc_CryptoCb_UnRegisterDevice(WB_DEVID_HASH_OK); diff --git a/wolfcrypt/src/cryptocb.c b/wolfcrypt/src/cryptocb.c index 8783a1684fb..3ec1273bbbf 100644 --- a/wolfcrypt/src/cryptocb.c +++ b/wolfcrypt/src/cryptocb.c @@ -614,7 +614,8 @@ int wc_CryptoCb_RegisterDevice(int devId, CryptoDevCallbackFunc cb, void* ctx) /* Publish the entry last, after everything it points at is in place. * The slot is therefore not discoverable from the register command - * itself - a handler must not dispatch through its own devId. */ + * itself - a handler must not dispatch through, or re-register, its + * own devId (a nested register of it would claim a second slot). */ WC_BARRIER(); dev->devId = devId; diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 33a25729dcc..00fee2ac523 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -85562,6 +85562,260 @@ static wc_test_ret_t hkdf_cryptocb_async_test(myCryptoDevCtx* ctx) #endif /* HAVE_HKDF && !NO_HMAC && !NO_SHA256 && !HAVE_SELFTEST && ... */ +#if defined(WOLF_CRYPTO_CB) && defined(WOLF_CRYPTO_CB_CMD) && \ + !defined(WC_TEST_NO_CRYPTOCB_SW_TEST) + /* The nested scenarios hold up to five devices at once on top of the + * devIds the harness pre-registers (the main test devId, plus the RNG + * seed device in no-hashdrbg builds), so skip on smaller tables. */ + #if MAX_CRYPTO_DEVID_CALLBACKS >= 7 + #define NESTED_CB_TEST + #endif +#endif + +#ifdef NESTED_CB_TEST +#define NESTED_CB_PARENT_DEVID 0x5A00 +#define NESTED_CB_CHILD_DEVID 0x5A10 +/* Grandchild devId is its child's devId plus this offset. */ +#define NESTED_CB_GRANDCHILD_DIFF 0x20 +#define NESTED_CB_SELF_DEVID 0x5A40 +#define NESTED_CB_FILLER_DEVID 0x5A50 +#define NESTED_CB_NUM_CHILDREN 2 + +/* Which callback saw the last unregister command: 1 child, 2 parent. */ +static int nestedCbLastUnreg; + +static int myNestedGrandchildCb(int devIdArg, wc_CryptoInfo* info, void* ctx) +{ + (void)devIdArg; + (void)info; + (void)ctx; + + return WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE); +} + +static int myNestedChildCb(int devIdArg, wc_CryptoInfo* info, void* ctx) +{ + (void)ctx; + + if (info == NULL || info->algo_type != WC_ALGO_TYPE_NONE) + return WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE); + + if (info->cmd.type == WC_CRYPTOCB_CMD_TYPE_REGISTER) { + /* One level deeper: each child brings up its own sub-device. */ + return wc_CryptoCb_RegisterDevice( + devIdArg + NESTED_CB_GRANDCHILD_DIFF, myNestedGrandchildCb, NULL); + } + if (info->cmd.type == WC_CRYPTOCB_CMD_TYPE_UNREGISTER) + nestedCbLastUnreg = 1; + return WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE); +} + +static int mySelfRegisterCb(int devIdArg, wc_CryptoInfo* info, void* ctx) +{ + (void)ctx; + + if (info != NULL && info->algo_type == WC_ALGO_TYPE_NONE && + info->cmd.type == WC_CRYPTOCB_CMD_TYPE_REGISTER) { + /* Our own devId is not published yet, so this recurses into a fresh + * slot each time until the table fills and BUFFER_E unwinds it. */ + return wc_CryptoCb_RegisterDevice(devIdArg, mySelfRegisterCb, NULL); + } + return WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE); +} + +static int myNestedParentCb(int devIdArg, wc_CryptoInfo* info, void* ctx) +{ + int i; + (void)devIdArg; + (void)ctx; + + if (info == NULL || info->algo_type != WC_ALGO_TYPE_NONE) + return WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE); + + if (info->cmd.type == WC_CRYPTOCB_CMD_TYPE_REGISTER) { + /* Register more devices from inside our own register command, like + * ports that expose several devIds from one driver. */ + for (i = 0; i < NESTED_CB_NUM_CHILDREN; i++) { + int rc = wc_CryptoCb_RegisterDevice(NESTED_CB_CHILD_DEVID + i, + myNestedChildCb, NULL); + if (rc != 0) + return rc; + } + return 0; + } + if (info->cmd.type == WC_CRYPTOCB_CMD_TYPE_UNREGISTER) + nestedCbLastUnreg = 2; + return WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE); +} + +/* Registering devices from inside a register command must not hand a child + * the parent's half filled slot. */ +static wc_test_ret_t cryptocb_nested_register_test(void) +{ + wc_test_ret_t ret = 0; + int i; + int rc; + int fillers; + + rc = wc_CryptoCb_RegisterDevice(NESTED_CB_PARENT_DEVID, myNestedParentCb, + NULL); + if (rc != 0) + ret = WC_TEST_RET_ENC_EC(rc); + + /* Parent, every child, and every grandchild must all be registered. */ + if (ret == 0 && wc_CryptoCb_IsDeviceRegistered(NESTED_CB_PARENT_DEVID) != 1) + ret = WC_TEST_RET_ENC_NC; + for (i = 0; ret == 0 && i < NESTED_CB_NUM_CHILDREN; i++) { + if (wc_CryptoCb_IsDeviceRegistered(NESTED_CB_CHILD_DEVID + i) != 1 || + wc_CryptoCb_IsDeviceRegistered(NESTED_CB_CHILD_DEVID + i + + NESTED_CB_GRANDCHILD_DIFF) != 1) + ret = WC_TEST_RET_ENC_NC; + } + + for (i = 0; i < NESTED_CB_NUM_CHILDREN; i++) { + wc_CryptoCb_UnRegisterDevice(NESTED_CB_CHILD_DEVID + i); + wc_CryptoCb_UnRegisterDevice(NESTED_CB_CHILD_DEVID + i + + NESTED_CB_GRANDCHILD_DIFF); + } + + /* The parent's slot must still hold the parent's callback, so its own + * unregister handler runs, not a child's. */ + nestedCbLastUnreg = 0; + wc_CryptoCb_UnRegisterDevice(NESTED_CB_PARENT_DEVID); + if (ret == 0 && nestedCbLastUnreg != 2) + ret = WC_TEST_RET_ENC_NC; + if (ret == 0 && wc_CryptoCb_IsDeviceRegistered(NESTED_CB_PARENT_DEVID) != 0) + ret = WC_TEST_RET_ENC_NC; + + /* A failing nested registration must unwind cleanly: pre-register the + * first child devId so the parent's register command hits ALREADY_E. */ + if (ret == 0) { + rc = wc_CryptoCb_RegisterDevice(NESTED_CB_CHILD_DEVID, NULL, NULL); + if (rc != 0) + ret = WC_TEST_RET_ENC_EC(rc); + } + if (ret == 0) { + rc = wc_CryptoCb_RegisterDevice(NESTED_CB_PARENT_DEVID, + myNestedParentCb, NULL); + if (rc != WC_NO_ERR_TRACE(ALREADY_E)) + ret = WC_TEST_RET_ENC_NC; + } + if (ret == 0 && wc_CryptoCb_IsDeviceRegistered(NESTED_CB_PARENT_DEVID) != 0) + ret = WC_TEST_RET_ENC_NC; + wc_CryptoCb_UnRegisterDevice(NESTED_CB_CHILD_DEVID); + + /* Partial success: pre-register the second child so the parent's command + * registers child 0 (and its grandchild), then fails on child 1. Only + * the parent's own slot is unwound; devices the command already + * registered survive and are the caller's to clean up. */ + if (ret == 0) { + rc = wc_CryptoCb_RegisterDevice(NESTED_CB_CHILD_DEVID + 1, NULL, NULL); + if (rc != 0) + ret = WC_TEST_RET_ENC_EC(rc); + } + if (ret == 0) { + rc = wc_CryptoCb_RegisterDevice(NESTED_CB_PARENT_DEVID, + myNestedParentCb, NULL); + if (rc != WC_NO_ERR_TRACE(ALREADY_E)) + ret = WC_TEST_RET_ENC_NC; + } + if (ret == 0 && + (wc_CryptoCb_IsDeviceRegistered(NESTED_CB_PARENT_DEVID) != 0 || + wc_CryptoCb_IsDeviceRegistered(NESTED_CB_CHILD_DEVID) != 1 || + wc_CryptoCb_IsDeviceRegistered(NESTED_CB_CHILD_DEVID + + NESTED_CB_GRANDCHILD_DIFF) != 1)) + ret = WC_TEST_RET_ENC_NC; + wc_CryptoCb_UnRegisterDevice(NESTED_CB_CHILD_DEVID); + wc_CryptoCb_UnRegisterDevice(NESTED_CB_CHILD_DEVID + + NESTED_CB_GRANDCHILD_DIFF); + wc_CryptoCb_UnRegisterDevice(NESTED_CB_CHILD_DEVID + 1); + + /* Table-full boundary: leave exactly one free slot, so the parent takes + * it and the nested child finds none (the parent's half filled slot must + * be skipped, not handed out) and BUFFER_E unwinds the registration. */ + rc = 0; + for (fillers = 0; rc == 0 && fillers <= MAX_CRYPTO_DEVID_CALLBACKS; ) { + rc = wc_CryptoCb_RegisterDevice(NESTED_CB_FILLER_DEVID + fillers, + NULL, NULL); + if (rc == 0) + fillers++; + } + if (ret == 0 && (rc != WC_NO_ERR_TRACE(BUFFER_E) || fillers == 0)) + ret = WC_TEST_RET_ENC_NC; + if (fillers > 0) { + fillers--; + wc_CryptoCb_UnRegisterDevice(NESTED_CB_FILLER_DEVID + fillers); + } + + if (ret == 0) { + rc = wc_CryptoCb_RegisterDevice(NESTED_CB_PARENT_DEVID, + myNestedParentCb, NULL); + if (rc != WC_NO_ERR_TRACE(BUFFER_E)) + ret = WC_TEST_RET_ENC_NC; + } + if (ret == 0 && (wc_CryptoCb_IsDeviceRegistered(NESTED_CB_PARENT_DEVID) || + wc_CryptoCb_IsDeviceRegistered(NESTED_CB_CHILD_DEVID))) + ret = WC_TEST_RET_ENC_NC; + + /* Two free slots: the parent and first child each hold a half filled + * slot when the grandchild's registration hits BUFFER_E, so the free + * slot search must skip both and the error must cascade through both + * frames, clearing every half filled slot on the way out. */ + if (fillers > 0) { + fillers--; + wc_CryptoCb_UnRegisterDevice(NESTED_CB_FILLER_DEVID + fillers); + } + if (ret == 0) { + rc = wc_CryptoCb_RegisterDevice(NESTED_CB_PARENT_DEVID, + myNestedParentCb, NULL); + if (rc != WC_NO_ERR_TRACE(BUFFER_E)) + ret = WC_TEST_RET_ENC_NC; + } + if (ret == 0 && (wc_CryptoCb_IsDeviceRegistered(NESTED_CB_PARENT_DEVID) || + wc_CryptoCb_IsDeviceRegistered(NESTED_CB_CHILD_DEVID) || + wc_CryptoCb_IsDeviceRegistered(NESTED_CB_CHILD_DEVID + + NESTED_CB_GRANDCHILD_DIFF))) + ret = WC_TEST_RET_ENC_NC; + + for (i = 0; i < fillers; i++) + wc_CryptoCb_UnRegisterDevice(NESTED_CB_FILLER_DEVID + i); + + /* The unwound slots must be reusable once the table has room again. */ + if (ret == 0) { + rc = wc_CryptoCb_RegisterDevice(NESTED_CB_PARENT_DEVID, + myNestedParentCb, NULL); + if (rc != 0) + ret = WC_TEST_RET_ENC_EC(rc); + } + for (i = 0; i < NESTED_CB_NUM_CHILDREN; i++) { + wc_CryptoCb_UnRegisterDevice(NESTED_CB_CHILD_DEVID + i); + wc_CryptoCb_UnRegisterDevice(NESTED_CB_CHILD_DEVID + i + + NESTED_CB_GRANDCHILD_DIFF); + } + wc_CryptoCb_UnRegisterDevice(NESTED_CB_PARENT_DEVID); + + /* Re-registering your own devId from your own register command is + * prohibited: the unpublished devId passes the ALREADY_E check, so it + * recurses into fresh slots until the table fills, then BUFFER_E + * unwinds every one of them. */ + if (ret == 0) { + rc = wc_CryptoCb_RegisterDevice(NESTED_CB_SELF_DEVID, mySelfRegisterCb, + NULL); + if (rc != WC_NO_ERR_TRACE(BUFFER_E)) + ret = WC_TEST_RET_ENC_NC; + } + if (ret == 0 && wc_CryptoCb_IsDeviceRegistered(NESTED_CB_SELF_DEVID) != 0) + ret = WC_TEST_RET_ENC_NC; + if (ret == 0) { + rc = wc_CryptoCb_RegisterDevice(NESTED_CB_SELF_DEVID, NULL, NULL); + if (rc != 0) + ret = WC_TEST_RET_ENC_EC(rc); + } + wc_CryptoCb_UnRegisterDevice(NESTED_CB_SELF_DEVID); + + return ret; +} +#endif /* NESTED_CB_TEST */ #if !defined(WC_TEST_NO_CRYPTOCB_SW_TEST) WOLFSSL_TEST_SUBROUTINE wc_test_ret_t cryptocb_test(void) @@ -86311,6 +86565,11 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t cryptocb_test(void) } #endif /* WC_RSA_PSS && WOLF_CRYPTO_CB_RSA_PAD */ +#ifdef NESTED_CB_TEST + if (ret == 0) + ret = cryptocb_nested_register_test(); +#endif + wc_CryptoCb_UnRegisterDevice(devId); /* restore devId */ From 4247a21ca579daf45a993d7c5eb4cd6d67cda881 Mon Sep 17 00:00:00 2001 From: night1rider Date: Tue, 25 Aug 2026 16:31:43 -0600 Subject: [PATCH 2/3] Skip half filled slots when looking for a free crypto callback slot A device that registers more devices from its own register command could be handed the same slot, which left the first device holding the wrong callback. --- wolfcrypt/src/cryptocb.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/wolfcrypt/src/cryptocb.c b/wolfcrypt/src/cryptocb.c index 3ec1273bbbf..316bbbc68af 100644 --- a/wolfcrypt/src/cryptocb.c +++ b/wolfcrypt/src/cryptocb.c @@ -437,6 +437,20 @@ static CryptoCb* wc_CryptoCb_GetDevice(int devId) return NULL; } +/* Find a slot that nothing is using yet. A slot that is part way through + * being registered has a callback but no devId, so check both fields. */ +static CryptoCb* wc_CryptoCb_GetFreeDevice(void) +{ + int i; + for (i = 0; i < MAX_CRYPTO_DEVID_CALLBACKS; i++) { + if ((gCryptoDev[i].devId == INVALID_DEVID) && + (gCryptoDev[i].cb == NULL)) { + return &gCryptoDev[i]; + } + } + return NULL; +} + /* Returns 1 if the given device ID is currently registered, 0 otherwise. * INVALID_DEVID marks free table slots, so it is never reported registered. */ int wc_CryptoCb_IsDeviceRegistered(int devId) @@ -577,7 +591,7 @@ int wc_CryptoCb_RegisterDevice(int devId, CryptoDevCallbackFunc cb, void* ctx) return ALREADY_E; /* find a free slot */ - dev = wc_CryptoCb_GetDevice(INVALID_DEVID); + dev = wc_CryptoCb_GetFreeDevice(); if (dev == NULL) return BUFFER_E; /* out of devices */ From e9666b019691c7bfcff20303123b535b96191a55 Mon Sep 17 00:00:00 2001 From: night1rider Date: Fri, 11 Sep 2026 10:36:16 -0600 Subject: [PATCH 3/3] Make the crypto callback white-box test run under the smoke runner and fail on a bad slot check It now includes the build's options so WOLF_CRYPTO_CB is defined instead of compiling to a stub, and the slot checks set their own flag that main() returns nonzero on. --- tests/unit-mcdc/test_cryptocb_whitebox.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/tests/unit-mcdc/test_cryptocb_whitebox.c b/tests/unit-mcdc/test_cryptocb_whitebox.c index 69d88d53836..51848395285 100644 --- a/tests/unit-mcdc/test_cryptocb_whitebox.c +++ b/tests/unit-mcdc/test_cryptocb_whitebox.c @@ -157,11 +157,19 @@ #define WOLF_CRYPTO_CB_FIND #endif +/* Pick up the build's feature defines, else WOLF_CRYPTO_CB is never seen + * here and this whole file compiles down to its "nothing to exercise" stub. */ +#ifndef WOLFSSL_USER_SETTINGS +#include +#endif + #include #include static int wb_fail = 0; +/* Set by a check of what the code must do, not by a coverage skip. */ +static int wb_bad = 0; #define WB_NOTE(msg) do { printf(" [wb] %s\n", (msg)); } while (0) #ifdef WOLF_CRYPTO_CB @@ -1386,11 +1394,11 @@ int main(void) wc_CryptoCb_Init(); gCryptoDev[0].cb = wb_cb; if (wc_CryptoCb_RegisterDevice(WB_DEVID, wb_cb, NULL) != 0) - wb_fail = 1; + wb_bad = 1; if (wc_CryptoCb_GetDevice(WB_DEVID) == &gCryptoDev[0]) - wb_fail = 1; + wb_bad = 1; if (gCryptoDev[0].devId != INVALID_DEVID || gCryptoDev[0].cb != wb_cb) - wb_fail = 1; + wb_bad = 1; /* With every other slot registered, the scan must reject the half * filled slot rather than hand it out: no free slot, BUFFER_E. */ @@ -1400,9 +1408,9 @@ int main(void) } if (wc_CryptoCb_RegisterDevice(WB_DEVID_NOCB, NULL, NULL) != WC_NO_ERR_TRACE(BUFFER_E)) - wb_fail = 1; + wb_bad = 1; if (gCryptoDev[0].devId != INVALID_DEVID || gCryptoDev[0].cb != wb_cb) - wb_fail = 1; + wb_bad = 1; WB_NOTE("GetFreeDevice: devId==INVALID_DEVID&&cb==NULL [:409] " "(T,F) half filled slot skipped, full-table BUFFER_E"); @@ -1425,10 +1433,10 @@ int main(void) (void)wb_find_cb; #endif - printf("done (%s)\n", wb_fail ? "with skips" : "ok"); + printf("done (%s)\n", wb_bad ? "FAILED" : wb_fail ? "with skips" : "ok"); #else printf(" WOLF_CRYPTO_CB not defined; nothing to exercise\n"); #endif /* WOLF_CRYPTO_CB */ (void)wb_fail; - return 0; + return wb_bad ? 1 : 0; }