wolfSSL / wolfssl

The wolfSSL library is a small, fast, portable implementation of TLS/SSL for embedded devices to the cloud. wolfSSL supports up to TLS 1.3 and DTLS 1.3!
https://www.wolfssl.com
GNU General Public License v2.0
2.3k stars 822 forks source link

[Bug]: PKCS11 issue with wc_ecc_init_ex() #7482

Closed mrdeep1 closed 4 months ago

mrdeep1 commented 4 months ago

Contact Details

No response

Version

Latest master

Description

$ ./configure --enable-all --enable-dtls13 --enable-pkcs11

Code in wc_ecc_init_ex() is testing for isPkcs11 in passed in key- which in just about every case is uninitialized.

/* Setup dynamic pointers if using normal math for proper freeing */
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

This (randomly set) isPkcs11is tested later in this function


#if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_ECC)
    #if defined(HAVE_PKCS11)
        if (!isPkcs11)
    #endif
        {
            /* handle as async */
            ret = wolfAsync_DevCtxInit(&key->asyncDev, WOLFSSL_ASYNC_MARKER_ECC,
                                                            key->heap, devId);
        }
#elif defined(HAVE_PKCS11)
    (void)isPkcs11;
#endif

Not sure how this should be fixed.

Reproduction steps

$ ./configure --enable-all --enable-dtls13 --enable-pkcs11

Relevant log output

From libcoap test environment when valgrind is being used to test for issues

+==PID== Conditional jump or move depends on uninitialised value(s)
+==PID==    func wc_ecc_init_ex (ecc.c:6106)
+==PID==    func TLSX_KeyShare_GenEccKey (tls.c:7459)
+==PID==    func TLSX_KeyShare_GenKey (tls.c:7783)
+==PID==    func TLSX_KeyShare_Use (tls.c:9120)
+==PID==    func TLSX_PopulateExtensions (tls.c:13185)
+==PID==    func SendTls13ClientHello (tls13.c:4377)
+==PID==    func SendClientHello (internal.c:28897)
+==PID==    func wolfSSL_connect (ssl.c:9267)

+==PID==  Uninitialised value was created by a heap allocation
+==PID==    func malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
+==PID==    func wolfSSL_Malloc (memory.c:352)
+==PID==    func TLSX_KeyShare_GenEccKey (tls.c:7450)
+==PID==    func TLSX_KeyShare_GenKey (tls.c:7783)
+==PID==    func TLSX_KeyShare_Use (tls.c:9120)
+==PID==    func TLSX_PopulateExtensions (tls.c:13185)
+==PID==    func SendTls13ClientHello (tls13.c:4377)
+==PID==    func SendClientHello (internal.c:28897)
+==PID==    func wolfSSL_connect (ssl.c:9267)
mrdeep1 commented 4 months ago

Same is true for int wc_InitRsaKey_ex(RsaKey* key, void* heap, int devId)

dgarske commented 4 months ago

Thanks @mrdeep1 for the report. We will take a look.

dgarske commented 4 months ago

See https://github.com/wolfSSL/wolfssl/pull/7485 for fix. Thank you for the report.

mrdeep1 commented 4 months ago

@dgarske Changes in #7485 make sense to me. I am not yet in a position to test out the wolfSSL PKCS11 code, but will regress check the rest of the libcoap usage of wolfSSL with this change.