From 52be7c94b8da1d1b9c19fdd780c6a7d5cb736773 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Fri, 18 Sep 2020 13:54:22 +0200 Subject: [PATCH 1/4] Introduce thread safety to unsafe functions in wolfSSL Add warnings to one shot hash functions --- src/ssl.c | 135 ++++++++++++++++++++++++++------------------- wolfssl/internal.h | 14 ++++- 2 files changed, 90 insertions(+), 59 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index b5978f151..271de43f3 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -4815,6 +4815,14 @@ int wolfSSL_Init(void) WOLFSSL_MSG("Bad Init Mutex count"); return BAD_MUTEX_E; } + +#if defined(OPENSSL_EXTRA) || \ + (defined(OPENSSL_EXTRA_X509_SMALL) && !defined(NO_RSA)) + if (wc_InitMutex(&globalRNGMutex) != 0) { + WOLFSSL_MSG("Bad Init Mutex rng"); + return BAD_MUTEX_E; + } +#endif } if (wc_LockMutex(&count_mutex) != 0) { @@ -17828,43 +17836,40 @@ const byte* wolfSSL_X509_get_der(WOLFSSL_X509* x509, int* outSz) #ifdef OPENSSL_EXTRA /* used by JSSE (not a standard compatibility function) */ -/* this is not thread safe */ WOLFSSL_ABI const byte* wolfSSL_X509_notBefore(WOLFSSL_X509* x509) { - static byte notBeforeData[CTC_DATE_SIZE]; /* temp buffer for date */ WOLFSSL_ENTER("wolfSSL_X509_notBefore"); if (x509 == NULL) return NULL; - XMEMSET(notBeforeData, 0, sizeof(notBeforeData)); - notBeforeData[0] = (byte)x509->notBefore.type; - notBeforeData[1] = (byte)x509->notBefore.length; - XMEMCPY(¬BeforeData[2], x509->notBefore.data, x509->notBefore.length); + XMEMSET(x509->notBeforeData, 0, sizeof(x509->notBeforeData)); + x509->notBeforeData[0] = (byte)x509->notBefore.type; + x509->notBeforeData[1] = (byte)x509->notBefore.length; + XMEMCPY(&x509->notBeforeData[2], x509->notBefore.data, x509->notBefore.length); - return notBeforeData; + return x509->notBeforeData; } /* used by JSSE (not a standard compatibility function) */ -/* this is not thread safe */ WOLFSSL_ABI const byte* wolfSSL_X509_notAfter(WOLFSSL_X509* x509) { - static byte notAfterData[CTC_DATE_SIZE]; /* temp buffer for date */ WOLFSSL_ENTER("wolfSSL_X509_notAfter"); if (x509 == NULL) return NULL; - XMEMSET(notAfterData, 0, sizeof(notAfterData)); - notAfterData[0] = (byte)x509->notAfter.type; - notAfterData[1] = (byte)x509->notAfter.length; - XMEMCPY(¬AfterData[2], x509->notAfter.data, x509->notAfter.length); + XMEMSET(x509->notAfterData, 0, sizeof(x509->notAfterData)); + x509->notAfterData[0] = (byte)x509->notAfter.type; + x509->notAfterData[1] = (byte)x509->notAfter.length; + XMEMCPY(&x509->notAfterData[2], x509->notAfter.data, x509->notAfter.length); - return notAfterData; + return x509->notAfterData; } + #if defined(WOLFSSL_QT) || defined(OPENSSL_ALL) && !defined(NO_WOLFSSL_STUB) WOLFSSL_ASN1_TIME* wolfSSL_X509_gmtime_adj(WOLFSSL_ASN1_TIME *s, long adj) { @@ -28328,8 +28333,9 @@ const size_t wolfssl_object_info_sz = WOLFSSL_OBJECT_INFO_SZ; #endif #if defined(OPENSSL_EXTRA) || \ (defined(OPENSSL_EXTRA_X509_SMALL) && !defined(NO_RSA)) -static WC_RNG globalRNG; -static int initGlobalRNG = 0; +WC_RNG globalRNG; +int initGlobalRNG = 0; +wolfSSL_Mutex globalRNGMutex; #endif #if defined(OPENSSL_EXTRA) && \ !defined(NO_RSA) && !defined(HAVE_USER_RSA) && !defined(HAVE_FAST_RSA) @@ -28382,21 +28388,26 @@ WC_RNG* WOLFSSL_RSA_GetRNG(WOLFSSL_RSA *rsa, WC_RNG **tmpRNG, int *initTmpRng) #ifdef OPENSSL_EXTRA -/* Not thread safe! Can be called multiple times. - * Checks if the global RNG has been created. If not then one is created. +/* Checks if the global RNG has been created. If not then one is created. * * Returns SSL_SUCCESS when no error is encountered. */ static int wolfSSL_RAND_Init(void) { + if (wc_LockMutex(&globalRNGMutex) != 0) { + WOLFSSL_MSG("Bad Lock Mutex rng"); + return 0; + } if (initGlobalRNG == 0) { if (wc_InitRng(&globalRNG) < 0) { WOLFSSL_MSG("wolfSSL Init Global RNG failed"); + wc_UnLockMutex(&globalRNGMutex); return 0; } initGlobalRNG = 1; } + wc_UnLockMutex(&globalRNGMutex); return SSL_SUCCESS; } @@ -28554,7 +28565,6 @@ int wolfSSL_RAND_write_file(const char* fname) #endif /* This collects entropy from the path nm and seeds the global PRNG with it. - * Makes a call to wolfSSL_RAND_Init which is not thread safe. * * nm is the file path to the egd server * @@ -28667,14 +28677,18 @@ int wolfSSL_RAND_egd(const char* nm) } if (bytes > 0 && ret == WOLFSSL_SUCCESS) { - wolfSSL_RAND_Init(); /* call to check global RNG is created */ - if (wc_RNG_DRBG_Reseed(&globalRNG, (const byte*) buf, bytes) + /* call to check global RNG is created */ + if (wolfSSL_RAND_Init() != SSL_SUCCESS) { + WOLFSSL_MSG("Error with initializing global RNG structure"); + ret = WOLFSSL_FATAL_ERROR; + } + else if (wc_RNG_DRBG_Reseed(&globalRNG, (const byte*) buf, bytes) != 0) { WOLFSSL_MSG("Error with reseeding DRBG structure"); ret = WOLFSSL_FATAL_ERROR; } #ifdef SHOW_SECRETS - { /* print out entropy found */ + else { /* print out entropy found only when no error occured */ word32 i; printf("EGD Entropy = "); for (i = 0; i < bytes; i++) { @@ -28712,10 +28726,15 @@ void wolfSSL_RAND_Cleanup(void) { WOLFSSL_ENTER("wolfSSL_RAND_Cleanup()"); + if (wc_LockMutex(&globalRNGMutex) != 0) { + WOLFSSL_MSG("Bad Lock Mutex rng"); + return; + } if (initGlobalRNG != 0) { wc_FreeRng(&globalRNG); initGlobalRNG = 0; } + wc_UnLockMutex(&globalRNGMutex); } @@ -39100,6 +39119,7 @@ err: unsigned char *md) { static byte dig[WC_SHA_DIGEST_SIZE]; + byte* ret = md; wc_Sha sha; WOLFSSL_ENTER("wolfSSL_SHA1"); @@ -39114,20 +39134,19 @@ err: return NULL; } - if (wc_ShaFinal(&sha, dig) != 0) { + if (md == NULL) { + WOLFSSL_MSG("STATIC BUFFER BEING USED. wolfSSL_SHA1 IS NOT " + "THREAD SAFE WHEN md == NULL"); + ret = dig; + } + if (wc_ShaFinal(&sha, ret) != 0) { WOLFSSL_MSG("SHA1 Final failed"); + wc_ShaFree(&sha); return NULL; } - wc_ShaFree(&sha); - if (md != NULL) { - XMEMCPY(md, dig, WC_SHA_DIGEST_SIZE); - return md; - } - else { - return (unsigned char*)dig; - } + return ret; } #endif /* ! NO_SHA */ @@ -39147,6 +39166,7 @@ err: unsigned char *md) { static byte dig[WC_SHA256_DIGEST_SIZE]; + byte* ret = md; wc_Sha256 sha; WOLFSSL_ENTER("wolfSSL_SHA256"); @@ -39161,20 +39181,19 @@ err: return NULL; } - if (wc_Sha256Final(&sha, dig) != 0) { + if (md == NULL) { + WOLFSSL_MSG("STATIC BUFFER BEING USED. wolfSSL_SHA256 IS NOT " + "THREAD SAFE WHEN md == NULL"); + ret = dig; + } + if (wc_Sha256Final(&sha, ret) != 0) { WOLFSSL_MSG("SHA256 Final failed"); + wc_Sha256Free(&sha); return NULL; } - wc_Sha256Free(&sha); - if (md != NULL) { - XMEMCPY(md, dig, WC_SHA256_DIGEST_SIZE); - return md; - } - else { - return (unsigned char*)dig; - } + return ret; } #endif /* ! NO_SHA256 */ @@ -39194,6 +39213,7 @@ err: unsigned char *md) { static byte dig[WC_SHA384_DIGEST_SIZE]; + byte* ret = md; wc_Sha384 sha; WOLFSSL_ENTER("wolfSSL_SHA384"); @@ -39208,20 +39228,19 @@ err: return NULL; } - if (wc_Sha384Final(&sha, dig) != 0) { + if (md == NULL) { + WOLFSSL_MSG("STATIC BUFFER BEING USED. wolfSSL_SHA384 IS NOT " + "THREAD SAFE WHEN md == NULL"); + ret = dig; + } + if (wc_Sha384Final(&sha, ret) != 0) { WOLFSSL_MSG("SHA384 Final failed"); + wc_Sha384Free(&sha); return NULL; } - wc_Sha384Free(&sha); - if (md != NULL) { - XMEMCPY(md, dig, WC_SHA384_DIGEST_SIZE); - return md; - } - else { - return (unsigned char*)dig; - } + return ret; } #endif /* WOLFSSL_SHA384 */ @@ -39242,6 +39261,7 @@ err: unsigned char *md) { static byte dig[WC_SHA512_DIGEST_SIZE]; + byte* ret = md; wc_Sha512 sha; WOLFSSL_ENTER("wolfSSL_SHA512"); @@ -39256,20 +39276,19 @@ err: return NULL; } - if (wc_Sha512Final(&sha, dig) != 0) { + if (md == NULL) { + WOLFSSL_MSG("STATIC BUFFER BEING USED. wolfSSL_SHA512 IS NOT " + "THREAD SAFE WHEN md == NULL"); + ret = dig; + } + if (wc_Sha512Final(&sha, ret) != 0) { WOLFSSL_MSG("SHA512 Final failed"); + wc_Sha512Free(&sha); return NULL; } - wc_Sha512Free(&sha); - if (md != NULL) { - XMEMCPY(md, dig, WC_SHA512_DIGEST_SIZE); - return md; - } - else { - return (unsigned char*)dig; - } + return ret; } #endif /* WOLFSSL_SHA512 */ #endif /* OPENSSL_EXTRA */ diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 14aa5991e..fc47bb408 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -3726,6 +3726,7 @@ struct WOLFSSL_X509 { byte subjAltNameCrit:1; byte authKeyIdSet:1; byte authKeyIdCrit:1; + byte issuerSet:1; #endif /* OPENSSL_EXTRA || OPENSSL_EXTRA_X509_SMALL */ byte serial[EXTERNAL_SERIAL_SIZE]; char subjectCN[ASN_NAME_MAX]; /* common name short cut */ @@ -3738,7 +3739,11 @@ struct WOLFSSL_X509 { WOLFSSL_X509_ALGOR algor; WOLFSSL_X509_PUBKEY key; #endif - byte issuerSet:1; +#if defined(OPENSSL_ALL) || defined(KEEP_OUR_CERT) || defined(KEEP_PEER_CERT) || \ + defined(SESSION_CERTS) + byte notBeforeData[CTC_DATE_SIZE]; + byte notAfterData[CTC_DATE_SIZE]; +#endif }; @@ -4375,6 +4380,13 @@ extern const WOLF_EC_NIST_NAME kNistCurves[]; #define kNistCurves_MAX_NAME_LEN 7 #endif +#if defined(OPENSSL_EXTRA) || \ + (defined(OPENSSL_EXTRA_X509_SMALL) && !defined(NO_RSA)) +extern WC_RNG globalRNG; +extern int initGlobalRNG; +extern wolfSSL_Mutex globalRNGMutex; +#endif + /* internal functions */ WOLFSSL_LOCAL int SendChangeCipher(WOLFSSL*); WOLFSSL_LOCAL int SendTicket(WOLFSSL*); From 2153009efa1dd49380a731327835101228b74f83 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Tue, 29 Sep 2020 19:47:58 +0200 Subject: [PATCH 2/4] Fix access violation in Visual Studio Test --- src/ssl.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 271de43f3..021b43601 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -4798,6 +4798,14 @@ int wolfSSL_Init(void) return WC_INIT_E; } +#if defined(OPENSSL_EXTRA) || \ + (defined(OPENSSL_EXTRA_X509_SMALL) && !defined(NO_RSA)) + if (wc_InitMutex(&globalRNGMutex) != 0) { + WOLFSSL_MSG("Bad Init Mutex rng"); + return BAD_MUTEX_E; + } +#endif + #ifdef OPENSSL_EXTRA if (wolfSSL_RAND_seed(NULL, 0) != WOLFSSL_SUCCESS) { WOLFSSL_MSG("wolfSSL_RAND_Seed failed"); @@ -4815,14 +4823,6 @@ int wolfSSL_Init(void) WOLFSSL_MSG("Bad Init Mutex count"); return BAD_MUTEX_E; } - -#if defined(OPENSSL_EXTRA) || \ - (defined(OPENSSL_EXTRA_X509_SMALL) && !defined(NO_RSA)) - if (wc_InitMutex(&globalRNGMutex) != 0) { - WOLFSSL_MSG("Bad Init Mutex rng"); - return BAD_MUTEX_E; - } -#endif } if (wc_LockMutex(&count_mutex) != 0) { From 24030d5f32315137081cfc85e67ddf9d7e369059 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Fri, 16 Oct 2020 17:33:28 +0200 Subject: [PATCH 3/4] Move globalRNG and co to ssl.c --- src/ssl.c | 13 +++++++------ wolfssl/internal.h | 7 ------- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 021b43601..772bd394a 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -4786,6 +4786,13 @@ int AddCA(WOLFSSL_CERT_MANAGER* cm, DerBuffer** pDer, int type, int verify) #endif /* NO_SESSION_CACHE */ +#if defined(OPENSSL_EXTRA) || \ + (defined(OPENSSL_EXTRA_X509_SMALL) && !defined(NO_RSA)) +static WC_RNG globalRNG; +static int initGlobalRNG = 0; +static wolfSSL_Mutex globalRNGMutex; +#endif + WOLFSSL_ABI int wolfSSL_Init(void) { @@ -28331,12 +28338,6 @@ const WOLFSSL_ObjectInfo wolfssl_object_info[] = { (sizeof(wolfssl_object_info) / sizeof(*wolfssl_object_info)) const size_t wolfssl_object_info_sz = WOLFSSL_OBJECT_INFO_SZ; #endif -#if defined(OPENSSL_EXTRA) || \ - (defined(OPENSSL_EXTRA_X509_SMALL) && !defined(NO_RSA)) -WC_RNG globalRNG; -int initGlobalRNG = 0; -wolfSSL_Mutex globalRNGMutex; -#endif #if defined(OPENSSL_EXTRA) && \ !defined(NO_RSA) && !defined(HAVE_USER_RSA) && !defined(HAVE_FAST_RSA) WC_RNG* WOLFSSL_RSA_GetRNG(WOLFSSL_RSA *rsa, WC_RNG **tmpRNG, int *initTmpRng) diff --git a/wolfssl/internal.h b/wolfssl/internal.h index fc47bb408..db1b5d176 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -4380,13 +4380,6 @@ extern const WOLF_EC_NIST_NAME kNistCurves[]; #define kNistCurves_MAX_NAME_LEN 7 #endif -#if defined(OPENSSL_EXTRA) || \ - (defined(OPENSSL_EXTRA_X509_SMALL) && !defined(NO_RSA)) -extern WC_RNG globalRNG; -extern int initGlobalRNG; -extern wolfSSL_Mutex globalRNGMutex; -#endif - /* internal functions */ WOLFSSL_LOCAL int SendChangeCipher(WOLFSSL*); WOLFSSL_LOCAL int SendTicket(WOLFSSL*); From 147cb8e60ce6cbda1c44f15657ad62e9b4831261 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Mon, 19 Oct 2020 12:46:11 +0200 Subject: [PATCH 4/4] Jenkins scope fixes --- src/ssl.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 772bd394a..86683be78 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -145,13 +145,13 @@ #define WOLFSSL_EVP_INCLUDED #include "wolfcrypt/src/evp.c" +#ifndef WOLFCRYPT_ONLY + #ifdef OPENSSL_EXTRA /* Global pointer to constant BN on */ static WOLFSSL_BIGNUM* bn_one = NULL; #endif -#ifndef WOLFCRYPT_ONLY - #if defined(OPENSSL_EXTRA) && defined(HAVE_ECC) const WOLF_EC_NIST_NAME kNistCurves[] = { {XSTR_SIZEOF("P-192"), "P-192", NID_X9_62_prime192v1}, @@ -28029,7 +28029,6 @@ int wolfSSL_cmp_peer_cert_to_file(WOLFSSL* ssl, const char *fname) } #endif #endif /* OPENSSL_EXTRA */ -#endif /* !WOLFCRYPT_ONLY */ #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) const WOLFSSL_ObjectInfo wolfssl_object_info[] = { #ifndef NO_CERTS @@ -28385,7 +28384,6 @@ WC_RNG* WOLFSSL_RSA_GetRNG(WOLFSSL_RSA *rsa, WC_RNG **tmpRNG, int *initTmpRng) return rng; } #endif -#ifndef WOLFCRYPT_ONLY #ifdef OPENSSL_EXTRA @@ -44645,8 +44643,6 @@ int wolfSSL_set_alpn_protos(WOLFSSL* ssl, #endif /* HAVE_ALPN */ #endif -#endif /* WOLFCRYPT_ONLY */ - #if defined(OPENSSL_EXTRA) #define WOLFSSL_BIO_INCLUDED @@ -48524,3 +48520,5 @@ int wolfSSL_set_ephemeral_key(WOLFSSL* ssl, int keyAlgo, } #endif /* WOLFSSL_STATIC_EPHEMERAL */ + +#endif /* WOLFCRYPT_ONLY */