Skip to content

Setkey/Export callbacks #9851

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
night1rider wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Setkey/Export callbacks #9851

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d2183ee
Add WOLF_CRYPTO_CB_SETKEY and WOLF_CRYPTO_CB_EXPORT_KEY crypto call…
night1rider Mar 2, 2026
8b092de
Add setkey/exportkey/eccgetsize test coverage in api.c
night1rider Mar 15, 2026
19d1e4a
Add export hooks for ecc
night1rider Mar 16, 2026
94b248a
Address feedback from Fenrir
night1rider Mar 23, 2026
d2ce541
Address pr review: add braces, move scope of variables, add X9.63 com…
night1rider Mar 25, 2026
8b6972e
narrow ecc_size/sig_size guards to SETKEY||EXPORT_KEY, update _WC_PK_…
night1rider Apr 9, 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
8 changes: 8 additions & 0 deletions .github/workflows/os-check.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,14 @@ jobs:
CPPFLAGS=-DWOLFSSL_TLS13_IGNORE_PT_ALERT_ON_ENC',
'--enable-all --enable-certgencache',
'--enable-all --enable-dilithium --enable-cryptocb --enable-cryptocbutils --enable-pkcallbacks',
'--enable-cryptocb --enable-aesgcm CPPFLAGS="-DWOLF_CRYPTO_CB_AES_SETKEY"',
'--enable-cryptocb --enable-keygen --enable-cryptocbutils=setkey',
Copy link
Copy Markdown
Contributor

@dgarske dgarske Apr 9, 2026

Choose a reason for hiding this comment

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

LOW-8: New CI test configurations don't cover WOLF_CRYPTO_CB_FIND interaction

  • File: .github/workflows/os-check.yml:87-92
  • Function: N/A (CI)
  • Category: [test]
  • Confidence: Low

Description: Six new CI configurations test various --enable-cryptocbutils combinations but none test the interaction with WOLF_CRYPTO_CB_FIND. Several code paths have #ifndef WOLF_CRYPTO_CB_FIND guards that change behavior (e.g., skipping the devId != INVALID_DEVID check). Testing this combination would increase coverage of the new feature.

Recommendation: Consider adding at least one CI config that combines --enable-cryptocbutils with WOLF_CRYPTO_CB_FIND.

Copy link
Copy Markdown
Contributor Author

@night1rider night1rider Apr 10, 2026

Choose a reason for hiding this comment

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

'--enable-cryptocb --enable-keygen --enable-cryptocbutils=setkey,export CPPFLAGS="-DWOLF_CRYPTO_CB_FIND"'. Also fixed a pre-existing NULL pointer dereference in wc_RsaPrivateKeyDecode (asn.c) where *inOutIdx was accessed in the SETKEY block before validating inOutIdx != NULL. With WOLF_CRYPTO_CB_FIND the SETKEY block runs unconditionally, so the bad-args test case (which passes NULL) crashed. Added early NULL check for input and inOutIdx parameters.

'--enable-cryptocb --enable-keygen --enable-cryptocbutils CPPFLAGS="-DWOLF_CRYPTO_CB_AES_SETKEY"',
'--enable-cryptocb --enable-keygen --enable-aesgcm --enable-cryptocbutils=setkey,free CPPFLAGS="-DWOLF_CRYPTO_CB_AES_SETKEY"',
'--enable-cryptocb --enable-keygen --enable-cryptocbutils=export',
'--enable-cryptocb --enable-keygen CPPFLAGS="-DWOLF_CRYPTO_CB_EXPORT_KEY"',
'--enable-cryptocb --enable-keygen --enable-aesgcm --enable-cryptocbutils=setkey,free,export CPPFLAGS="-DWOLF_CRYPTO_CB_AES_SETKEY"',
'--enable-cryptocb --enable-keygen --enable-cryptocbutils=setkey,export CPPFLAGS="-DWOLF_CRYPTO_CB_FIND"',
'CPPFLAGS=-DWOLFSSL_NO_CLIENT_AUTH',
'CPPFLAGS=''-DNO_WOLFSSL_CLIENT -DWOLFSSL_NO_CLIENT_AUTH''',
'CPPFLAGS=''-DNO_WOLFSSL_SERVER -DWOLFSSL_NO_CLIENT_AUTH''',
Expand Down
14 changes: 9 additions & 5 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9968,7 +9968,7 @@ fi

# Crypto Callbacks Utils (Copy/Free/etc)
AC_ARG_ENABLE([cryptocbutils],
[AS_HELP_STRING([--enable-cryptocbutils@<:@=copy,free,...@:>@],
[AS_HELP_STRING([--enable-cryptocbutils@<:@=copy,free,setkey,export,...@:>@],
[Enable crypto callback utilities (default: all)])],
[ ENABLED_CRYPTOCB_UTILS=$enableval ],
[ ENABLED_CRYPTOCB_UTILS=no ]
Expand All @@ -9981,8 +9981,7 @@ if test "$ENABLED_CRYPTOCB_UTILS" != "no"; then

if test "$ENABLED_CRYPTOCB_UTILS" = "yes"; then
# Enable all utilities
AM_CFLAGS="$AM_CFLAGS -DWOLF_CRYPTO_CB_COPY -DWOLF_CRYPTO_CB_FREE"
# Future utilities go here when added
AM_CFLAGS="$AM_CFLAGS -DWOLF_CRYPTO_CB_COPY -DWOLF_CRYPTO_CB_FREE -DWOLF_CRYPTO_CB_SETKEY -DWOLF_CRYPTO_CB_EXPORT_KEY"
else
# Parse comma-separated list
OIFS="$IFS"
Expand All @@ -9995,9 +9994,14 @@ if test "$ENABLED_CRYPTOCB_UTILS" != "no"; then
free)
AM_CFLAGS="$AM_CFLAGS -DWOLF_CRYPTO_CB_FREE"
;;
# Add future options here (e.g., malloc, realloc, etc)
setkey)
AM_CFLAGS="$AM_CFLAGS -DWOLF_CRYPTO_CB_SETKEY"
;;
export)
AM_CFLAGS="$AM_CFLAGS -DWOLF_CRYPTO_CB_EXPORT_KEY"
;;
*)
AC_MSG_ERROR([Unknown cryptocbutils option: $util. Valid options: copy, free])
AC_MSG_ERROR([Unknown cryptocbutils option: $util. Valid options: copy, free, setkey, export])
;;
esac
done
Expand Down
327 changes: 327 additions & 0 deletions tests/api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27974,6 +27974,35 @@ static int test_CryptoCb_Func(int thisDevId, wc_CryptoInfo* info, void* ctx)
ret, *info->pk.eccsign.outlen);
#endif
}
else if (info->pk.type == WC_PK_TYPE_EC_GET_SIZE) {
WC_DECLARE_VAR(tmpEcc, ecc_key, 1, NULL);
WC_ALLOC_VAR(tmpEcc, ecc_key, 1, NULL);
if (!WC_VAR_OK(tmpEcc)) {
ret = MEMORY_E;
}
else {
XMEMCPY(tmpEcc, info->pk.ecc_get_size.key, sizeof(ecc_key));
tmpEcc->devId = INVALID_DEVID;
*info->pk.ecc_get_size.keySize = wc_ecc_size(tmpEcc);
WC_FREE_VAR(tmpEcc, NULL);
ret = 0;
}
}
else if (info->pk.type == WC_PK_TYPE_EC_GET_SIG_SIZE) {
WC_DECLARE_VAR(tmpEcc, ecc_key, 1, NULL);
WC_ALLOC_VAR(tmpEcc, ecc_key, 1, NULL);
if (!WC_VAR_OK(tmpEcc)) {
ret = MEMORY_E;
}
else {
XMEMCPY(tmpEcc, info->pk.ecc_get_sig_size.key,
sizeof(ecc_key));
tmpEcc->devId = INVALID_DEVID;
*info->pk.ecc_get_sig_size.sigSize = wc_ecc_sig_size(tmpEcc);
WC_FREE_VAR(tmpEcc, NULL);
ret = 0;
}
}
#endif /* HAVE_ECC */
#ifdef HAVE_ED25519
if (info->pk.type == WC_PK_TYPE_ED25519_SIGN) {
Expand Down Expand Up @@ -28306,6 +28335,304 @@ static int test_CryptoCb_Func(int thisDevId, wc_CryptoInfo* info, void* ctx)
}
}
#endif /* WOLF_CRYPTO_CB_FREE */
#ifdef WOLF_CRYPTO_CB_SETKEY
Copy link
Copy Markdown
Contributor

@dgarske dgarske Apr 9, 2026

Choose a reason for hiding this comment

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

MEDIUM-4: Significant code duplication between test callback implementations

  • File: tests/api.c:28322-28625 and wolfcrypt/test/test.c:65838-66177
  • Function: test_CryptoCb_Func, myCryptoDevCb
  • Category: [style]
  • Confidence: High

Description: The SETKEY and EXPORT_KEY callback implementations in tests/api.c and wolfcrypt/test/test.c are nearly identical (~300 lines each). Every key type handler (AES, HMAC, RSA pub/priv, ECC pub/priv) is copy-pasted between the two test files. This makes maintenance error-prone — any bug fix in one copy must be manually replicated in the other.

Recommendation: Consider extracting common callback helper functions into a shared test utility header, or use a common #include file for the shared handler implementations.

Copy link
Copy Markdown
Contributor Author

@night1rider night1rider Apr 10, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

There is currently no sharing code between tests/api.c and wolfcrypt/test/test.c they have separate include structures and their existing crypto callback implementations (test_CryptoCb_Func vs myCryptoDevCb) take fundamentally different approaches even for the pre-existing operations (e.g., api.c loads RSA keys from PEM files while test.c uses pre-initialized keys directly).

The SETKEY/EXPORT_KEY handlers are the first significant duplication between them. Introducing a shared test utility header would be a new pattern for the repo.

I can look at consolidating this in a follow-up if desired.

else if (info->algo_type == WC_ALGO_TYPE_SETKEY) {
#ifdef DEBUG_WOLFSSL
fprintf(stderr, "test_CryptoCb_Func: SetKey Type=%d\n",
info->setkey.type);
#endif
switch (info->setkey.type) {
#ifndef NO_AES
case WC_SETKEY_AES:
{
Aes* aes = (Aes*)info->setkey.obj;
aes->devId = INVALID_DEVID;
ret = wc_AesSetKey(aes,
(const byte*)info->setkey.key, info->setkey.keySz,
(const byte*)info->setkey.aux, info->setkey.flags);
aes->devId = thisDevId;
break;
}
#endif /* !NO_AES */
#ifndef NO_HMAC
case WC_SETKEY_HMAC:
{
Hmac* hmac = (Hmac*)info->setkey.obj;
hmac->devId = INVALID_DEVID;
ret = wc_HmacSetKey(hmac, hmac->macType,
(const byte*)info->setkey.key, info->setkey.keySz);
hmac->devId = thisDevId;
break;
}
#endif /* !NO_HMAC */
#if !defined(NO_RSA) && defined(WOLFSSL_KEY_TO_DER)
case WC_SETKEY_RSA_PUB:
{
RsaKey* rsaObj = (RsaKey*)info->setkey.obj;
RsaKey* rsaTmp = (RsaKey*)info->setkey.key;
int derSz;
word32 idx = 0;
byte* der = NULL;

derSz = wc_RsaPublicKeyDerSize(rsaTmp, 1);
if (derSz <= 0) { ret = derSz; break; }

der = (byte*)XMALLOC(derSz, NULL, DYNAMIC_TYPE_TMP_BUFFER);
if (der == NULL) { ret = MEMORY_E; break; }

derSz = wc_RsaKeyToPublicDer_ex(rsaTmp, der,
(word32)derSz, 1);
if (derSz <= 0) {
XFREE(der, NULL, DYNAMIC_TYPE_TMP_BUFFER);
ret = derSz; break;
}

rsaObj->devId = INVALID_DEVID;
ret = wc_RsaPublicKeyDecode(der, &idx, rsaObj,
(word32)derSz);
rsaObj->devId = thisDevId;
XFREE(der, NULL, DYNAMIC_TYPE_TMP_BUFFER);
break;
}
case WC_SETKEY_RSA_PRIV:
{
RsaKey* rsaObj = (RsaKey*)info->setkey.obj;
RsaKey* rsaTmp = (RsaKey*)info->setkey.key;
int derSz;
word32 idx = 0;
byte* der = NULL;

derSz = wc_RsaKeyToDer(rsaTmp, NULL, 0);
if (derSz <= 0) { ret = derSz; break; }

der = (byte*)XMALLOC(derSz, NULL, DYNAMIC_TYPE_TMP_BUFFER);
if (der == NULL) { ret = MEMORY_E; break; }

derSz = wc_RsaKeyToDer(rsaTmp, der, (word32)derSz);
if (derSz <= 0) {
XFREE(der, NULL, DYNAMIC_TYPE_TMP_BUFFER);
ret = derSz; break;
}

rsaObj->devId = INVALID_DEVID;
ret = wc_RsaPrivateKeyDecode(der, &idx, rsaObj,
(word32)derSz);
rsaObj->devId = thisDevId;
XFREE(der, NULL, DYNAMIC_TYPE_TMP_BUFFER);
break;
}
#endif /* !NO_RSA && WOLFSSL_KEY_TO_DER */
#if defined(HAVE_ECC) && defined(HAVE_ECC_KEY_EXPORT) && \
defined(HAVE_ECC_KEY_IMPORT)
case WC_SETKEY_ECC_PUB:
{
ecc_key* eccObj = (ecc_key*)info->setkey.obj;
ecc_key* eccTmp = (ecc_key*)info->setkey.key;
word32 bufSz = ECC_BUFSIZE;
int curveId;
WC_DECLARE_VAR(buf, byte, ECC_BUFSIZE, NULL);
WC_ALLOC_VAR(buf, byte, ECC_BUFSIZE, NULL);
if (!WC_VAR_OK(buf)) {
ret = MEMORY_E;
break;
}

ret = wc_ecc_export_x963(eccTmp, buf, &bufSz);
if (ret != 0) {
WC_FREE_VAR(buf, NULL);
break;
}

curveId = wc_ecc_get_curve_id(eccTmp->idx);
eccObj->devId = INVALID_DEVID;
ret = wc_ecc_import_x963_ex2(buf, bufSz, eccObj, curveId, 0);
eccObj->devId = thisDevId;

WC_FREE_VAR(buf, NULL);
break;
}
case WC_SETKEY_ECC_PRIV:
{
ecc_key* eccObj = (ecc_key*)info->setkey.obj;
ecc_key* eccTmp = (ecc_key*)info->setkey.key;
word32 pubSz = ECC_BUFSIZE;
word32 privSz = MAX_ECC_BYTES;
byte* pubPtr = NULL;
int curveId;
WC_DECLARE_VAR(pubBuf, byte, ECC_BUFSIZE, NULL);
WC_DECLARE_VAR(privBuf, byte, MAX_ECC_BYTES, NULL);
WC_ALLOC_VAR(pubBuf, byte, ECC_BUFSIZE, NULL);
WC_ALLOC_VAR(privBuf, byte, MAX_ECC_BYTES, NULL);
if (!WC_VAR_OK(pubBuf) || !WC_VAR_OK(privBuf)) {
WC_FREE_VAR(pubBuf, NULL);
WC_FREE_VAR(privBuf, NULL);
ret = MEMORY_E;
break;
}

/* Export public key from temp (if available) */
if (eccTmp->type != ECC_PRIVATEKEY_ONLY) {
ret = wc_ecc_export_x963(eccTmp, pubBuf, &pubSz);
if (ret != 0) {
WC_FREE_VAR(pubBuf, NULL);
WC_FREE_VAR(privBuf, NULL);
break;
}
pubPtr = pubBuf;
}

ret = wc_ecc_export_private_only(eccTmp, privBuf, &privSz);
if (ret != 0) {
WC_FREE_VAR(pubBuf, NULL);
WC_FREE_VAR(privBuf, NULL);
break;
}

curveId = wc_ecc_get_curve_id(eccTmp->idx);
eccObj->devId = INVALID_DEVID;
ret = wc_ecc_import_private_key_ex(privBuf, privSz,
pubPtr, (pubPtr != NULL) ? pubSz : 0,
eccObj, curveId);
eccObj->devId = thisDevId;

WC_FREE_VAR(pubBuf, NULL);
WC_FREE_VAR(privBuf, NULL);
break;
}
#endif /* HAVE_ECC && HAVE_ECC_KEY_EXPORT && HAVE_ECC_KEY_IMPORT */
default:
ret = WC_NO_ERR_TRACE(NOT_COMPILED_IN);
break;
}
}
#endif /* WOLF_CRYPTO_CB_SETKEY */
#ifdef WOLF_CRYPTO_CB_EXPORT_KEY
else if (info->algo_type == WC_ALGO_TYPE_EXPORT_KEY) {
#ifdef DEBUG_WOLFSSL
fprintf(stderr, "test_CryptoCb_Func: ExportKey Type=%d\n",
info->export_key.type);
#endif
switch (info->export_key.type) {
#if !defined(NO_RSA) && defined(WOLFSSL_KEY_TO_DER)
case WC_PK_TYPE_RSA:
{
RsaKey* src = (RsaKey*)info->export_key.obj;
RsaKey* dst = (RsaKey*)info->export_key.out;
int derSz;
word32 idx = 0;
byte* der = NULL;

/* Try private key export first, fall back to public */
derSz = wc_RsaKeyToDer(src, NULL, 0);
if (derSz > 0) {
der = (byte*)XMALLOC(derSz, NULL,
DYNAMIC_TYPE_TMP_BUFFER);
if (der == NULL) { ret = MEMORY_E; break; }
derSz = wc_RsaKeyToDer(src, der, (word32)derSz);
if (derSz > 0) {
ret = wc_RsaPrivateKeyDecode(der, &idx, dst,
(word32)derSz);
}
else {
ret = derSz;
}
XFREE(der, NULL, DYNAMIC_TYPE_TMP_BUFFER);
}
else {
/* Public key only */
derSz = wc_RsaPublicKeyDerSize(src, 1);
if (derSz <= 0) { ret = derSz; break; }
der = (byte*)XMALLOC(derSz, NULL,
DYNAMIC_TYPE_TMP_BUFFER);
if (der == NULL) { ret = MEMORY_E; break; }
derSz = wc_RsaKeyToPublicDer_ex(src, der,
(word32)derSz, 1);
if (derSz > 0) {
ret = wc_RsaPublicKeyDecode(der, &idx, dst,
(word32)derSz);
}
else {
ret = derSz;
}
XFREE(der, NULL, DYNAMIC_TYPE_TMP_BUFFER);
}
break;
}
#endif /* !NO_RSA && WOLFSSL_KEY_TO_DER */
#if defined(HAVE_ECC) && defined(HAVE_ECC_KEY_EXPORT) && \
defined(HAVE_ECC_KEY_IMPORT)
case WC_PK_TYPE_ECDSA_SIGN: /* ECC key */
{
ecc_key* src = (ecc_key*)info->export_key.obj;
ecc_key* dst = (ecc_key*)info->export_key.out;
word32 pubSz = ECC_BUFSIZE;
word32 privSz = MAX_ECC_BYTES;
byte* pubPtr = NULL;
int curveId;
WC_DECLARE_VAR(pubBuf, byte, ECC_BUFSIZE, NULL);
WC_DECLARE_VAR(privBuf, byte, MAX_ECC_BYTES, NULL);
WC_ALLOC_VAR(pubBuf, byte, ECC_BUFSIZE, NULL);
WC_ALLOC_VAR(privBuf, byte, MAX_ECC_BYTES, NULL);
if (!WC_VAR_OK(pubBuf) || !WC_VAR_OK(privBuf)) {
WC_FREE_VAR(pubBuf, NULL);
WC_FREE_VAR(privBuf, NULL);
ret = MEMORY_E;
break;
}

/* Use software to export from src - prevent recursion */
{
int savedDevId = src->devId;
src->devId = INVALID_DEVID;

/* Export public key if available */
if (src->type != ECC_PRIVATEKEY_ONLY) {
ret = wc_ecc_export_x963(src, pubBuf, &pubSz);
if (ret != 0) {
src->devId = savedDevId;
WC_FREE_VAR(pubBuf, NULL);
WC_FREE_VAR(privBuf, NULL);
break;
}
pubPtr = pubBuf;
}

/* Export private key if available */
if (src->type != ECC_PUBLICKEY) {
ret = wc_ecc_export_private_only(src, privBuf,
&privSz);
if (ret != 0) {
src->devId = savedDevId;
WC_FREE_VAR(pubBuf, NULL);
WC_FREE_VAR(privBuf, NULL);
break;
}

curveId = wc_ecc_get_curve_id(src->idx);
ret = wc_ecc_import_private_key_ex(privBuf, privSz,
pubPtr, (pubPtr != NULL) ? pubSz : 0,
dst, curveId);
}
else {
/* Public key only */
curveId = wc_ecc_get_curve_id(src->idx);
ret = wc_ecc_import_x963_ex2(pubBuf, pubSz, dst,
curveId, 0);
}

src->devId = savedDevId;
}
WC_FREE_VAR(pubBuf, NULL);
WC_FREE_VAR(privBuf, NULL);
break;
}
#endif /* HAVE_ECC && HAVE_ECC_KEY_EXPORT && HAVE_ECC_KEY_IMPORT */
default:
ret = WC_NO_ERR_TRACE(NOT_COMPILED_IN);
break;
}
}
#endif /* WOLF_CRYPTO_CB_EXPORT_KEY */
(void)thisDevId;
(void)keyFormat;

Expand Down
Loading
Loading