From 5af0b1e83b1f07e8221d20ed3d6a6eaf964a536e Mon Sep 17 00:00:00 2001 From: David Garske Date: Mon, 29 Apr 2024 10:34:01 -0700 Subject: [PATCH] Improved the prioritization of crypto callback vs async crypt in ECC and RSA. Resolves possible use of uninitialized value on ECC/RSA key when PKCS11 is enabled. See #7482 --- wolfcrypt/src/ecc.c | 42 ++++++++++--------------------------- wolfcrypt/src/rsa.c | 46 +++++++++-------------------------------- wolfssl/wolfcrypt/ecc.h | 3 --- wolfssl/wolfcrypt/rsa.h | 3 --- 4 files changed, 21 insertions(+), 73 deletions(-) diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index 47b68a229..501631490 100644 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -6094,20 +6094,11 @@ WOLFSSL_ABI int wc_ecc_init_ex(ecc_key* key, void* heap, int devId) { int ret = 0; -#if defined(HAVE_PKCS11) - int isPkcs11 = 0; -#endif if (key == NULL) { return BAD_FUNC_ARG; } -#if defined(HAVE_PKCS11) - if (key->isPkcs11) { - isPkcs11 = 1; - } -#endif - #ifdef ECC_DUMP_OID wc_ecc_dump_oids(); #endif @@ -6161,16 +6152,17 @@ int wc_ecc_init_ex(ecc_key* key, void* heap, int devId) #endif #if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_ECC) - #if defined(HAVE_PKCS11) - if (!isPkcs11) + #ifdef WOLF_CRYPTO_CB + /* prefer crypto callback */ + if (key->devId != INVALID_DEVID) #endif - { - /* handle as async */ - ret = wolfAsync_DevCtxInit(&key->asyncDev, WOLFSSL_ASYNC_MARKER_ECC, - key->heap, devId); - } -#elif defined(HAVE_PKCS11) - (void)isPkcs11; + { + /* handle as async */ + ret = wolfAsync_DevCtxInit(&key->asyncDev, WOLFSSL_ASYNC_MARKER_ECC, + key->heap, devId); + } + if (ret != 0) + return ret; #endif #if defined(WOLFSSL_DSP) @@ -6222,12 +6214,6 @@ int wc_ecc_init_id(ecc_key* key, unsigned char* id, int len, void* heap, ret = BAD_FUNC_ARG; if (ret == 0 && (len < 0 || len > ECC_MAX_ID_LEN)) ret = BUFFER_E; - -#if defined(HAVE_PKCS11) - XMEMSET(key, 0, sizeof(ecc_key)); - key->isPkcs11 = 1; -#endif - if (ret == 0) ret = wc_ecc_init_ex(key, heap, devId); if (ret == 0 && id != NULL && len != 0) { @@ -6257,12 +6243,6 @@ int wc_ecc_init_label(ecc_key* key, const char* label, void* heap, int devId) if (labelLen == 0 || labelLen > ECC_MAX_LABEL_LEN) ret = BUFFER_E; } - -#if defined(HAVE_PKCS11) - XMEMSET(key, 0, sizeof(ecc_key)); - key->isPkcs11 = 1; -#endif - if (ret == 0) ret = wc_ecc_init_ex(key, heap, devId); if (ret == 0) { @@ -7177,7 +7157,7 @@ int wc_ecc_sign_hash_ex(const byte* in, word32 inlen, WC_RNG* rng, #if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_ECC) && \ - defined(WOLFSSL_ASYNC_CRYPT_SW) + defined(WOLFSSL_ASYNC_CRYPT_SW) if (key->asyncDev.marker == WOLFSSL_ASYNC_MARKER_ECC) { if (wc_AsyncSwInit(&key->asyncDev, ASYNC_SW_ECC_SIGN)) { WC_ASYNC_SW* sw = &key->asyncDev.sw; diff --git a/wolfcrypt/src/rsa.c b/wolfcrypt/src/rsa.c index b11740f4e..3c4071c91 100644 --- a/wolfcrypt/src/rsa.c +++ b/wolfcrypt/src/rsa.c @@ -153,20 +153,11 @@ static void wc_RsaCleanup(RsaKey* key) int wc_InitRsaKey_ex(RsaKey* key, void* heap, int devId) { int ret = 0; -#if defined(HAVE_PKCS11) - int isPkcs11 = 0; -#endif if (key == NULL) { return BAD_FUNC_ARG; } -#if defined(HAVE_PKCS11) - if (key->isPkcs11) { - isPkcs11 = 1; - } -#endif - XMEMSET(key, 0, sizeof(RsaKey)); key->type = RSA_TYPE_UNKNOWN; @@ -193,19 +184,18 @@ int wc_InitRsaKey_ex(RsaKey* key, void* heap, int devId) #endif #ifdef WC_ASYNC_ENABLE_RSA - #if defined(HAVE_PKCS11) - if (!isPkcs11) + #ifdef WOLF_CRYPTO_CB + /* prefer crypto callback */ + if (key->devId != INVALID_DEVID) #endif - { - /* handle as async */ - ret = wolfAsync_DevCtxInit(&key->asyncDev, - WOLFSSL_ASYNC_MARKER_RSA, key->heap, devId); - if (ret != 0) - return ret; - } + { + /* handle as async */ + ret = wolfAsync_DevCtxInit(&key->asyncDev, + WOLFSSL_ASYNC_MARKER_RSA, key->heap, devId); + if (ret != 0) + return ret; + } #endif /* WC_ASYNC_ENABLE_RSA */ -#elif defined(HAVE_PKCS11) - (void)isPkcs11; #endif /* WOLFSSL_ASYNC_CRYPT */ #ifndef WOLFSSL_RSA_PUBLIC_ONLY @@ -278,14 +268,6 @@ int wc_InitRsaKey_Id(RsaKey* key, unsigned char* id, int len, void* heap, ret = BAD_FUNC_ARG; if (ret == 0 && (len < 0 || len > RSA_MAX_ID_LEN)) ret = BUFFER_E; - -#if defined(HAVE_PKCS11) - if (ret == 0) { - XMEMSET(key, 0, sizeof(RsaKey)); - key->isPkcs11 = 1; - } -#endif - if (ret == 0) ret = wc_InitRsaKey_ex(key, heap, devId); if (ret == 0 && id != NULL && len != 0) { @@ -315,14 +297,6 @@ int wc_InitRsaKey_Label(RsaKey* key, const char* label, void* heap, int devId) if (labelLen == 0 || labelLen > RSA_MAX_LABEL_LEN) ret = BUFFER_E; } - -#if defined(HAVE_PKCS11) - if (ret == 0) { - XMEMSET(key, 0, sizeof(RsaKey)); - key->isPkcs11 = 1; - } -#endif - if (ret == 0) ret = wc_InitRsaKey_ex(key, heap, devId); if (ret == 0) { diff --git a/wolfssl/wolfcrypt/ecc.h b/wolfssl/wolfcrypt/ecc.h index 21e25fcbd..73f2fa560 100644 --- a/wolfssl/wolfcrypt/ecc.h +++ b/wolfssl/wolfcrypt/ecc.h @@ -519,9 +519,6 @@ struct ecc_key { void* devCtx; int devId; #endif -#if defined(HAVE_PKCS11) - byte isPkcs11 : 1; /* indicate if PKCS11 is preferred */ -#endif #ifdef WOLFSSL_SILABS_SE_ACCEL sl_se_command_context_t cmd_ctx; sl_se_key_descriptor_t key; diff --git a/wolfssl/wolfcrypt/rsa.h b/wolfssl/wolfcrypt/rsa.h index 9296bf894..0f31c8605 100644 --- a/wolfssl/wolfcrypt/rsa.h +++ b/wolfssl/wolfcrypt/rsa.h @@ -217,9 +217,6 @@ struct RsaKey { void* devCtx; int devId; #endif -#if defined(HAVE_PKCS11) - byte isPkcs11 : 1; /* indicate if PKCS11 is preferred */ -#endif #ifdef WOLFSSL_ASYNC_CRYPT WC_ASYNC_DEV asyncDev; #ifdef WOLFSSL_CERT_GEN