From 3f5620089e1ec96cb6053ecf98677de745fa8798 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Sat, 24 Oct 2020 12:41:10 -0700 Subject: [PATCH 1/4] PKCS7: In EncodeEncryptedData, free the attribs and flattenedAttribs if they were allocated, not based on if they should be allocated. --- wolfcrypt/src/pkcs7.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index e9ae9c64e..3d0941f5a 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -11751,10 +11751,10 @@ int wc_PKCS7_EncodeEncryptedData(PKCS7* pkcs7, byte* output, word32 outputSz) if (totalSz > (int)outputSz) { WOLFSSL_MSG("PKCS#7 output buffer too small"); - if (pkcs7->unprotectedAttribsSz != 0) { + if (attribs != NULL) XFREE(attribs, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + if (flatAttribs != NULL) XFREE(flatAttribs, pkcs7->heap, DYNAMIC_TYPE_PKCS7); - } XFREE(encryptedContent, pkcs7->heap, DYNAMIC_TYPE_PKCS7); XFREE(plain, pkcs7->heap, DYNAMIC_TYPE_PKCS7); return BUFFER_E; @@ -11790,10 +11790,12 @@ int wc_PKCS7_EncodeEncryptedData(PKCS7* pkcs7, byte* output, word32 outputSz) idx += attribsSetSz; XMEMCPY(output + idx, flatAttribs, attribsSz); idx += attribsSz; - XFREE(attribs, pkcs7->heap, DYNAMIC_TYPE_PKCS7); - XFREE(flatAttribs, pkcs7->heap, DYNAMIC_TYPE_PKCS7); } + if (attribs != NULL) + XFREE(attribs, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + if (flatAttribs != NULL) + XFREE(flatAttribs, pkcs7->heap, DYNAMIC_TYPE_PKCS7); XFREE(encryptedContent, pkcs7->heap, DYNAMIC_TYPE_PKCS7); XFREE(plain, pkcs7->heap, DYNAMIC_TYPE_PKCS7); From f5f883597e49efeec188be83f34277ee6a3f3a78 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Sat, 24 Oct 2020 13:06:42 -0700 Subject: [PATCH 2/4] RSA PSS Fix 1. Change the utility function in wc_encrypt that returns the size of a hash to initialize the size to HASH_TYPE_E, like the other utility functions. 2. When getting the hash size returns an error, RSA-PSS verify inline should return a BAD_FUNC_ARG error. --- wolfcrypt/src/rsa.c | 2 +- wolfcrypt/src/wc_encrypt.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/wolfcrypt/src/rsa.c b/wolfcrypt/src/rsa.c index faa403173..a77a32099 100644 --- a/wolfcrypt/src/rsa.c +++ b/wolfcrypt/src/rsa.c @@ -3472,7 +3472,7 @@ int wc_RsaPSS_VerifyCheckInline(byte* in, word32 inLen, byte** out, hLen = wc_HashGetDigestSize(hash); if (hLen < 0) - return hLen; + return BAD_FUNC_ARG; if ((word32)hLen != digestLen) return BAD_FUNC_ARG; diff --git a/wolfcrypt/src/wc_encrypt.c b/wolfcrypt/src/wc_encrypt.c index 39dbeec5a..7afc032b5 100644 --- a/wolfcrypt/src/wc_encrypt.c +++ b/wolfcrypt/src/wc_encrypt.c @@ -374,7 +374,7 @@ int wc_CryptKey(const char* password, int passwordSz, byte* salt, int saltSz, int iterations, int id, byte* input, int length, int version, byte* cbcIv, int enc, int shaOid) { - int typeH; + int typeH = WC_HASH_TYPE_NONE; int derivedLen = 0; int ret = 0; #ifdef WOLFSSL_SMALL_STACK From 9c1049f11286cfdd2388680f820023d21dbc48e5 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Sun, 25 Oct 2020 14:38:07 -0700 Subject: [PATCH 3/4] Compatibility Layer 1. Changed the ASN1_OBJECT member of the X509_NAME_ENTRY to be a pointer rather than an object. It could lead to a double free on the name entry. 2. The ASN1_OBJECT allocator should set the dynamic flag, as the deallocator is the one that uses it. 3. General changes to treat the member as a pointer rather than a member. 4. In the api test, we were iterating over the name members in the name checking the NIDs. After the loop we freed the name member object. This led to a double free error. --- src/internal.c | 4 ++-- src/ssl.c | 40 +++++++++++++--------------------------- tests/api.c | 2 -- wolfssl/ssl.h | 2 +- 4 files changed, 16 insertions(+), 32 deletions(-) diff --git a/src/internal.c b/src/internal.c index 47f99f180..a2e856542 100644 --- a/src/internal.c +++ b/src/internal.c @@ -3337,7 +3337,7 @@ void InitX509Name(WOLFSSL_X509_NAME* name, int dynamicFlag, void* heap) name->sz = 0; name->heap = heap; #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) - XMEMSET(&name->entry, 0, sizeof(name->entry)); + XMEMSET(name->entry, 0, sizeof(name->entry)); name->x509 = NULL; name->entrySz = 0; #endif /* OPENSSL_EXTRA */ @@ -3357,7 +3357,7 @@ void FreeX509Name(WOLFSSL_X509_NAME* name) int i; for (i = 0; i < MAX_NAME_ENTRIES; i++) { if (name->entry[i].set) { - wolfSSL_ASN1_OBJECT_free(&name->entry[i].object); + wolfSSL_ASN1_OBJECT_free(name->entry[i].object); wolfSSL_ASN1_STRING_free(name->entry[i].value); } } diff --git a/src/ssl.c b/src/ssl.c index a6b0f90f4..f87108f70 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -9359,8 +9359,6 @@ void* wolfSSL_X509_get_ext_d2i(const WOLFSSL_X509* x509, int nid, int* c, obj->grp = oidCertExtType; obj->obj = x509->CRLInfo; obj->objSz = x509->CRLInfoSz; - obj->dynamic |= WOLFSSL_ASN1_DYNAMIC; - obj->dynamic &= ~WOLFSSL_ASN1_DYNAMIC_DATA ; } else { WOLFSSL_MSG("No CRL dist set"); @@ -9381,8 +9379,6 @@ void* wolfSSL_X509_get_ext_d2i(const WOLFSSL_X509* x509, int nid, int* c, obj->grp = oidCertExtType; obj->obj = x509->authInfo; obj->objSz = x509->authInfoSz; - obj->dynamic |= WOLFSSL_ASN1_DYNAMIC; - obj->dynamic &= ~WOLFSSL_ASN1_DYNAMIC_DATA; } else { WOLFSSL_MSG("No Auth Info set"); @@ -9409,8 +9405,6 @@ void* wolfSSL_X509_get_ext_d2i(const WOLFSSL_X509* x509, int nid, int* c, obj->grp = oidCertExtType; obj->obj = x509->authKeyId; obj->objSz = x509->authKeyIdSz; - obj->dynamic |= WOLFSSL_ASN1_DYNAMIC; - obj->dynamic &= ~WOLFSSL_ASN1_DYNAMIC_DATA; akey->issuer = obj; return akey; } @@ -9433,8 +9427,6 @@ void* wolfSSL_X509_get_ext_d2i(const WOLFSSL_X509* x509, int nid, int* c, obj->grp = oidCertExtType; obj->obj = x509->subjKeyId; obj->objSz = x509->subjKeyIdSz; - obj->dynamic |= WOLFSSL_ASN1_DYNAMIC; - obj->dynamic &= ~WOLFSSL_ASN1_DYNAMIC_DATA; } else { WOLFSSL_MSG("No Subject Key set"); @@ -9472,8 +9464,6 @@ void* wolfSSL_X509_get_ext_d2i(const WOLFSSL_X509* x509, int nid, int* c, obj->grp = oidCertExtType; obj->obj = (byte*)(x509->certPolicies[i]); obj->objSz = MAX_CERTPOL_SZ; - obj->dynamic |= WOLFSSL_ASN1_DYNAMIC; - obj->dynamic &= ~WOLFSSL_ASN1_DYNAMIC_DATA; if (wolfSSL_sk_ASN1_OBJECT_push(sk, obj) != WOLFSSL_SUCCESS) { WOLFSSL_MSG("Error pushing ASN1 object onto stack"); @@ -9492,8 +9482,6 @@ void* wolfSSL_X509_get_ext_d2i(const WOLFSSL_X509* x509, int nid, int* c, obj->grp = oidCertExtType; obj->obj = (byte*)(x509->certPolicies[i]); obj->objSz = MAX_CERTPOL_SZ; - obj->dynamic |= WOLFSSL_ASN1_DYNAMIC; - obj->dynamic &= ~WOLFSSL_ASN1_DYNAMIC_DATA; } else { WOLFSSL_MSG("No Cert Policy set"); @@ -9510,7 +9498,6 @@ void* wolfSSL_X509_get_ext_d2i(const WOLFSSL_X509* x509, int nid, int* c, } obj->type = CERT_POLICY_OID; obj->grp = oidCertExtType; - obj->dynamic |= WOLFSSL_ASN1_DYNAMIC; } else { WOLFSSL_MSG("No Cert Policy set"); @@ -9572,8 +9559,6 @@ void* wolfSSL_X509_get_ext_d2i(const WOLFSSL_X509* x509, int nid, int* c, obj->grp = oidCertExtType; obj->obj = x509->extKeyUsageSrc; obj->objSz = x509->extKeyUsageSz; - obj->dynamic |= WOLFSSL_ASN1_DYNAMIC; - obj->dynamic &= ~WOLFSSL_ASN1_DYNAMIC_DATA; } else { WOLFSSL_MSG("No Extended Key Usage set"); @@ -38692,7 +38677,7 @@ err: { WOLFSSL_ENTER("wolfSSL_X509_NAME_ENTRY_free"); if (ne != NULL) { - wolfSSL_ASN1_OBJECT_free(&ne->object); + wolfSSL_ASN1_OBJECT_free(ne->object); if (ne->value != NULL) { wolfSSL_ASN1_STRING_free(ne->value); } @@ -38747,7 +38732,7 @@ err: } } ne->nid = nid; - wolfSSL_OBJ_nid2obj_ex(nid, &ne->object); + wolfSSL_OBJ_nid2obj_ex(nid, ne->object); ne->value = wolfSSL_ASN1_STRING_type_new(type); if (ne->value != NULL) { wolfSSL_ASN1_STRING_set(ne->value, (const void*)data, dataSz); @@ -38787,7 +38772,7 @@ err: } ne->nid = nid; - wolfSSL_OBJ_nid2obj_ex(nid, &ne->object); + ne->object = wolfSSL_OBJ_nid2obj_ex(nid, ne->object); ne->value = wolfSSL_ASN1_STRING_type_new(type); if (ne->value != NULL) { wolfSSL_ASN1_STRING_set(ne->value, (const void*)data, dataSz); @@ -39064,9 +39049,6 @@ err: WOLFSSL_MSG("Issue creating WOLFSSL_ASN1_OBJECT struct"); return NULL; } - obj->dynamic |= WOLFSSL_ASN1_DYNAMIC; - } else { - obj->dynamic &= ~WOLFSSL_ASN1_DYNAMIC; } obj->type = id; obj->grp = type; @@ -39229,9 +39211,9 @@ err: for (idx++; idx < MAX_NAME_ENTRIES; idx++) { /* Find index of desired name */ if (name->entry[idx].set) { - if (XSTRLEN(obj->sName) == XSTRLEN(name->entry[idx].object.sName) && + if (XSTRLEN(obj->sName) == XSTRLEN(name->entry[idx].object->sName) && XSTRNCMP((const char*) obj->sName, - name->entry[idx].object.sName, obj->objSz - 1) == 0) { + name->entry[idx].object->sName, obj->objSz - 1) == 0) { return idx; } } @@ -39811,12 +39793,16 @@ err: defined(HAVE_LIGHTY) || defined(WOLFSSL_MYSQL_COMPATIBLE) || \ defined(HAVE_STUNNEL) || defined(WOLFSSL_NGINX) || \ defined(HAVE_POCO_LIB) || defined(WOLFSSL_HAPROXY) - WOLFSSL_ASN1_OBJECT * wolfSSL_X509_NAME_ENTRY_get_object(WOLFSSL_X509_NAME_ENTRY *ne) { + WOLFSSL_ASN1_OBJECT * wolfSSL_X509_NAME_ENTRY_get_object(WOLFSSL_X509_NAME_ENTRY *ne) + { + WOLFSSL_ASN1_OBJECT* obj = NULL; + WOLFSSL_ENTER("wolfSSL_X509_NAME_ENTRY_get_object"); if (ne == NULL) return NULL; - if (wolfSSL_OBJ_nid2obj_ex(ne->nid, &ne->object) != NULL) { - ne->object.nid = ne->nid; - return &ne->object; + obj = wolfSSL_OBJ_nid2obj_ex(ne->nid, ne->object); + if (obj != NULL) { + obj->nid = ne->nid; + return obj; } return NULL; } diff --git a/tests/api.c b/tests/api.c index d51d17e3c..48e71caec 100644 --- a/tests/api.c +++ b/tests/api.c @@ -29373,7 +29373,6 @@ static void test_wolfSSL_OBJ(void) AssertTrue((nid = OBJ_obj2nid(asn1Name)) > 0); } BIO_free(bio); - ASN1_OBJECT_free(asn1Name); X509_free(x509); } @@ -29397,7 +29396,6 @@ static void test_wolfSSL_OBJ(void) AssertTrue((nid = OBJ_obj2nid(asn1Name)) > 0); } BIO_free(bio); - ASN1_OBJECT_free(asn1Name); X509_free(x509); } diff --git a/wolfssl/ssl.h b/wolfssl/ssl.h index 8af089fba..68f026888 100644 --- a/wolfssl/ssl.h +++ b/wolfssl/ssl.h @@ -3239,7 +3239,7 @@ WOLFSSL_API int wolfSSL_accept_ex(WOLFSSL*, HandShakeCallBack, TimeoutCallBack, #include struct WOLFSSL_X509_NAME_ENTRY { - WOLFSSL_ASN1_OBJECT object; /* static object just for keeping grp, type */ + WOLFSSL_ASN1_OBJECT* object; /* static object just for keeping grp, type */ WOLFSSL_ASN1_STRING* value; /* points to data, for lighttpd port */ int nid; /* i.e. ASN_COMMON_NAME */ int set; From 7dbd6102d23c0df0ba2b6638d8068149da2cb711 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 26 Oct 2020 16:10:44 -0700 Subject: [PATCH 4/4] Compatibility Layer When wolfSSL_X509_NAME_ENTRY_create_by_txt() needs to make a new ASN.1 object ID, actually store it in the name entry. --- src/ssl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ssl.c b/src/ssl.c index f87108f70..3c545874b 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -38732,7 +38732,7 @@ err: } } ne->nid = nid; - wolfSSL_OBJ_nid2obj_ex(nid, ne->object); + ne->object = wolfSSL_OBJ_nid2obj_ex(nid, ne->object); ne->value = wolfSSL_ASN1_STRING_type_new(type); if (ne->value != NULL) { wolfSSL_ASN1_STRING_set(ne->value, (const void*)data, dataSz);