From 82921f8650be0637fcd0985e9238947526c16ffb Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Wed, 17 Jun 2020 11:30:18 -0600 Subject: [PATCH 1/5] fix for x509 store add crl --- src/crl.c | 203 ++++++++++++++++++++++++++++++++++++++++++++++---- src/ssl.c | 18 ++--- wolfssl/ssl.h | 2 +- 3 files changed, 196 insertions(+), 27 deletions(-) diff --git a/src/crl.c b/src/crl.c index a48cf9d83..f3f52c855 100644 --- a/src/crl.c +++ b/src/crl.c @@ -490,27 +490,204 @@ int BufferLoadCRL(WOLFSSL_CRL* crl, const byte* buff, long sz, int type, } #if defined(OPENSSL_EXTRA) && defined(HAVE_CRL) +/* helper function to create a new dynamic WOLFSSL_X509_CRL structure */ +static WOLFSSL_X509_CRL* wolfSSL_X509_crl_new(WOLFSSL_CERT_MANAGER* cm) +{ + WOLFSSL_X509_CRL* ret; + + ret = (WOLFSSL_X509_CRL*)XMALLOC(sizeof(WOLFSSL_X509_CRL), cm->heap, + DYNAMIC_TYPE_CRL); + if (ret != NULL) { + if (InitCRL(ret, cm) < 0) { + WOLFSSL_MSG("Unable to initialize new CRL structure"); + XFREE(ret, cm->heap, DYNAMIC_TYPE_CRL); + ret = NULL; + } + } + return ret; +} + + +/* returns head of copied list that was alloc'd */ +static RevokedCert *DupRevokedCertList(RevokedCert* in, void* heap) +{ + RevokedCert* head = NULL; + RevokedCert* current = in; + RevokedCert* prev = NULL; + while (current) { + RevokedCert* tmp = (RevokedCert*)XMALLOC(sizeof(RevokedCert), heap, + DYNAMIC_TYPE_REVOKED); + if (tmp != NULL) { + XMEMCPY(tmp->serialNumber, current->serialNumber, + EXTERNAL_SERIAL_SIZE); + tmp->serialSz = current->serialSz; + tmp->next = NULL; + if (prev != NULL) + prev->next = tmp; + if (head == NULL) + head = tmp; + } + current = current->next; + } + return head; +} + + +/* returns a deep copy of ent on success and null on fail */ +static CRL_Entry* DupCRL_Entry(CRL_Entry* ent, void* heap) +{ + CRL_Entry *dup; + + dup = (CRL_Entry*)XMALLOC(sizeof(CRL_Entry), heap, DYNAMIC_TYPE_CRL_ENTRY); + if (dup == NULL) { + WOLFSSL_MSG("alloc CRL Entry failed"); + return NULL; + } + + XMEMCPY(dup->issuerHash, ent->issuerHash, CRL_DIGEST_SIZE); + XMEMCPY(dup->lastDate, ent->lastDate, MAX_DATE_SIZE); + XMEMCPY(dup->nextDate, ent->nextDate, MAX_DATE_SIZE); + dup->lastDateFormat = ent->lastDateFormat; + dup->nextDateFormat = ent->nextDateFormat; + dup->certs = DupRevokedCertList(ent->certs, heap); + + dup->totalCerts = ent->totalCerts; + dup->verified = ent->verified; + + if (!ent->verified) { + dup->tbsSz = ent->tbsSz; + dup->signatureSz = ent->signatureSz; + dup->signatureOID = ent->signatureOID; + dup->toBeSigned = (byte*)XMALLOC(dup->tbsSz, heap, + DYNAMIC_TYPE_CRL_ENTRY); + if (dup->toBeSigned == NULL) { + XFREE(dup, heap, DYNAMIC_TYPE_CRL_ENTRY); + return NULL; + } + + dup->signature = (byte*)XMALLOC(dup->signatureSz, heap, + DYNAMIC_TYPE_CRL_ENTRY); + if (dup->signature == NULL) { + XFREE(dup, heap, DYNAMIC_TYPE_CRL_ENTRY); + XFREE(dup->toBeSigned, heap, DYNAMIC_TYPE_CRL_ENTRY); + return NULL; + } + XMEMCPY(dup->toBeSigned, ent->toBeSigned, dup->tbsSz); + XMEMCPY(dup->signature, ent->signature, dup->signatureSz); + #ifndef NO_SKID + dup->extAuthKeyIdSet = ent->extAuthKeyIdSet; + if (dup->extAuthKeyIdSet) + XMEMCPY(dup->extAuthKeyId, ent->extAuthKeyId, KEYID_SIZE); + #endif + } + else { + dup->toBeSigned = NULL; + dup->tbsSz = 0; + dup->signature = NULL; + dup->signatureSz = 0; + } + + return dup; +} + + +/* returns the head of a deep copy of the list on success and null on fail */ +static CRL_Entry* DupCRL_list(CRL_Entry* crl, void* heap) +{ + CRL_Entry* current; + CRL_Entry* head = NULL; + CRL_Entry* prev = NULL; + + current = crl; + while (current != NULL) { + CRL_Entry* tmp = DupCRL_Entry(current, heap); + if (tmp != NULL) { + tmp->next = NULL; + if (head == NULL) + head = tmp; + if (prev != NULL) + prev->next = tmp; + } + current = current->next; + } + return head; +} + + +/* Duplicates everything except the parent cm pointed to. + * Expects that Init has already been done to 'dup' + * return 0 on success */ +static int DupX509_CRL(WOLFSSL_X509_CRL *dup, WOLFSSL_X509_CRL* crl) +{ + if (dup == NULL || crl == NULL) { + return BAD_FUNC_ARG; + } + + dup->crlList = DupCRL_list(crl->crlList, dup->heap); +#ifdef HAVE_CRL_IO + dup->crlIOCb = crl->crlIOCb; +#endif + if (crl->monitors[0].path) { + int pathSz = (int)XSTRLEN(crl->monitors[0].path) + 1; + dup->monitors[0].path = (char*)XMALLOC(pathSz, dup->heap, + DYNAMIC_TYPE_CRL_MONITOR); + if (dup->monitors[0].path != NULL) { + XSTRNCPY(dup->monitors[0].path, crl->monitors[0].path, pathSz); + } + } + + if (crl->monitors[1].path) { + int pathSz = (int)XSTRLEN(crl->monitors[1].path) + 1; + dup->monitors[1].path = (char*)XMALLOC(pathSz, dup->heap, + DYNAMIC_TYPE_CRL_MONITOR); + if (dup->monitors[1].path != NULL) { + XSTRNCPY(dup->monitors[1].path, crl->monitors[1].path, pathSz); + } + } + + return 0; +} + +/* returns WOLFSSL_SUCCESS on success. Does not take ownership of newcrl */ int wolfSSL_X509_STORE_add_crl(WOLFSSL_X509_STORE *store, WOLFSSL_X509_CRL *newcrl) { CRL_Entry *crle; - WOLFSSL_CRL *crl; + WOLFSSL_X509_CRL *crl; WOLFSSL_ENTER("wolfSSL_X509_STORE_add_crl"); - if (store == NULL || newcrl == NULL) + if (store == NULL || newcrl == NULL || store->cm == NULL) return BAD_FUNC_ARG; - crl = store->crl; - crle = newcrl->crlList; - - if (wc_LockMutex(&crl->crlLock) != 0) - { - WOLFSSL_MSG("wc_LockMutex failed"); - return BAD_MUTEX_E; + if (store->cm->crl == NULL) { + crl = wolfSSL_X509_crl_new(store->cm); + DupX509_CRL(crl, newcrl); + store->crl = store->cm->crl = crl; + return WOLFSSL_SUCCESS; + } + + /* find tail of current list and add new list */ + crl = store->cm->crl; + crle = crl->crlList; + if (newcrl->crlList != NULL) { + CRL_Entry *tail = crle; + CRL_Entry *toAdd; + + if (wc_LockMutex(&crl->crlLock) != 0) + { + WOLFSSL_MSG("wc_LockMutex failed"); + return BAD_MUTEX_E; + } + + toAdd = DupCRL_list(newcrl->crlList, crl->heap); + if (tail == NULL) { + crl->crlList = toAdd; + } + else { + while (tail->next != NULL) tail = tail->next; + tail->next = toAdd; + } + wc_UnLockMutex(&crl->crlLock); } - crle->next = crl->crlList; - crl->crlList = crle; - newcrl->crlList = NULL; - wc_UnLockMutex(&crl->crlLock); WOLFSSL_LEAVE("wolfSSL_X509_STORE_add_crl", WOLFSSL_SUCCESS); diff --git a/src/ssl.c b/src/ssl.c index 0bdcbe418..1106300e8 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -22738,12 +22738,7 @@ WOLFSSL_X509_STORE* wolfSSL_X509_STORE_new(void) goto err_exit; #ifdef HAVE_CRL - store->crl = NULL; - if ((store->crl = (WOLFSSL_X509_CRL *)XMALLOC(sizeof(WOLFSSL_X509_CRL), - NULL, DYNAMIC_TYPE_TMP_BUFFER)) == NULL) - goto err_exit; - if (InitCRL(store->crl, NULL) < 0) - goto err_exit; + store->crl = store->cm->crl; #endif #if defined(OPENSSL_EXTRA) || defined(WOLFSSL_WPAS_SMALL) @@ -22771,10 +22766,6 @@ void wolfSSL_X509_STORE_free(WOLFSSL_X509_STORE* store) if (store != NULL && store->isDynamic) { if (store->cm != NULL) wolfSSL_CertManagerFree(store->cm); -#ifdef HAVE_CRL - if (store->crl != NULL) - wolfSSL_X509_CRL_free(store->crl); -#endif #if defined(OPENSSL_EXTRA) || defined(WOLFSSL_WPAS_SMALL) if (store->param != NULL) XFREE(store->param, NULL, DYNAMIC_TYPE_OPENSSL); @@ -22989,7 +22980,8 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx) wolfSSL_X509_STORE_CTX_set_error(ctx, error); wolfSSL_X509_STORE_CTX_set_error_depth(ctx, depth); - ctx->store->verify_cb(0, ctx); + if (ctx->store && ctx->store->verify_cb) + ctx->store->verify_cb(0, ctx); } error = 0; @@ -23184,8 +23176,8 @@ WOLFSSL_X509_CRL* wolfSSL_d2i_X509_CRL(WOLFSSL_X509_CRL** crl, if (in == NULL) { WOLFSSL_MSG("Bad argument value"); } else { - newcrl = (WOLFSSL_X509_CRL*)XMALLOC(sizeof(WOLFSSL_X509_CRL), NULL, - DYNAMIC_TYPE_TMP_BUFFER); + newcrl =(WOLFSSL_X509_CRL*)XMALLOC(sizeof(WOLFSSL_X509_CRL), NULL, + DYNAMIC_TYPE_CRL); if (newcrl == NULL){ WOLFSSL_MSG("New CRL allocation failed"); } else { diff --git a/wolfssl/ssl.h b/wolfssl/ssl.h index 606e2c12d..551b68aa1 100644 --- a/wolfssl/ssl.h +++ b/wolfssl/ssl.h @@ -517,7 +517,7 @@ struct WOLFSSL_X509_STORE { WOLFSSL_CRYPTO_EX_DATA ex_data; #endif #if (defined(OPENSSL_EXTRA) || defined(WOLFSSL_WPAS_SMALL)) && defined(HAVE_CRL) - WOLFSSL_X509_CRL *crl; + WOLFSSL_X509_CRL *crl; /* points to cm->crl */ #endif }; From 1e431e1ade7aa2fe395feafe1f22f2f92220ad06 Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Thu, 18 Jun 2020 10:57:25 -0600 Subject: [PATCH 2/5] add test case and fixes from review --- src/crl.c | 23 +++++++++++++++++++---- src/ssl.c | 11 +++++------ tests/api.c | 36 +++++++++++++++++++++++++++++------- 3 files changed, 53 insertions(+), 17 deletions(-) diff --git a/src/crl.c b/src/crl.c index f3f52c855..628acda78 100644 --- a/src/crl.c +++ b/src/crl.c @@ -527,6 +527,16 @@ static RevokedCert *DupRevokedCertList(RevokedCert* in, void* heap) if (head == NULL) head = tmp; } + else { + WOLFSSL_MSG("Failed to allocate new RevokedCert structure"); + /* free up any existing list */ + while (head != NULL) { + current = head; + head = head->next; + XFREE(current, heap, DYNAMIC_TYPE_REVOKED); + } + return NULL; + } current = current->next; } return head; @@ -534,7 +544,7 @@ static RevokedCert *DupRevokedCertList(RevokedCert* in, void* heap) /* returns a deep copy of ent on success and null on fail */ -static CRL_Entry* DupCRL_Entry(CRL_Entry* ent, void* heap) +static CRL_Entry* DupCRL_Entry(const CRL_Entry* ent, void* heap) { CRL_Entry *dup; @@ -543,6 +553,7 @@ static CRL_Entry* DupCRL_Entry(CRL_Entry* ent, void* heap) WOLFSSL_MSG("alloc CRL Entry failed"); return NULL; } + XMEMSET(dup, 0, sizeof(CRL_Entry)); XMEMCPY(dup->issuerHash, ent->issuerHash, CRL_DIGEST_SIZE); XMEMCPY(dup->lastDate, ent->lastDate, MAX_DATE_SIZE); @@ -561,6 +572,7 @@ static CRL_Entry* DupCRL_Entry(CRL_Entry* ent, void* heap) dup->toBeSigned = (byte*)XMALLOC(dup->tbsSz, heap, DYNAMIC_TYPE_CRL_ENTRY); if (dup->toBeSigned == NULL) { + FreeCRL_Entry(dup, heap); XFREE(dup, heap, DYNAMIC_TYPE_CRL_ENTRY); return NULL; } @@ -568,8 +580,8 @@ static CRL_Entry* DupCRL_Entry(CRL_Entry* ent, void* heap) dup->signature = (byte*)XMALLOC(dup->signatureSz, heap, DYNAMIC_TYPE_CRL_ENTRY); if (dup->signature == NULL) { + FreeCRL_Entry(dup, heap); XFREE(dup, heap, DYNAMIC_TYPE_CRL_ENTRY); - XFREE(dup->toBeSigned, heap, DYNAMIC_TYPE_CRL_ENTRY); return NULL; } XMEMCPY(dup->toBeSigned, ent->toBeSigned, dup->tbsSz); @@ -617,7 +629,7 @@ static CRL_Entry* DupCRL_list(CRL_Entry* crl, void* heap) /* Duplicates everything except the parent cm pointed to. * Expects that Init has already been done to 'dup' * return 0 on success */ -static int DupX509_CRL(WOLFSSL_X509_CRL *dup, WOLFSSL_X509_CRL* crl) +static int DupX509_CRL(WOLFSSL_X509_CRL *dup, const WOLFSSL_X509_CRL* crl) { if (dup == NULL || crl == NULL) { return BAD_FUNC_ARG; @@ -660,7 +672,10 @@ int wolfSSL_X509_STORE_add_crl(WOLFSSL_X509_STORE *store, WOLFSSL_X509_CRL *newc if (store->cm->crl == NULL) { crl = wolfSSL_X509_crl_new(store->cm); - DupX509_CRL(crl, newcrl); + if (DupX509_CRL(crl, newcrl) != 0) { + FreeCRL(crl, 1); + return WOLFSSL_FAILURE; + } store->crl = store->cm->crl = crl; return WOLFSSL_SUCCESS; } diff --git a/src/ssl.c b/src/ssl.c index 1106300e8..ae2da8636 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -22921,7 +22921,6 @@ void wolfSSL_X509_STORE_CTX_cleanup(WOLFSSL_X509_STORE_CTX* ctx) /* Do nothing */ } -#if defined(OPENSSL_ALL) || defined(WOLFSSL_QT) /* Returns corresponding X509 error from internal ASN error */ static int GetX509Error(int e) { @@ -22947,7 +22946,6 @@ static int GetX509Error(int e) return e; } } -#endif /* OPENSSL_ALL || WOLFSSL_QT */ /* Verifies certificate chain using WOLFSSL_X509_STORE_CTX * returns 0 on success or < 0 on failure. @@ -22955,11 +22953,10 @@ static int GetX509Error(int e) int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx) { int ret = 0; -#if defined(OPENSSL_ALL) || defined(WOLFSSL_QT) int depth = 0; int error; byte *afterDate, *beforeDate; -#endif + WOLFSSL_ENTER("wolfSSL_X509_verify_cert"); if (ctx != NULL && ctx->store != NULL && ctx->store->cm != NULL @@ -22969,7 +22966,6 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx) ctx->current_cert->derCert->length, WOLFSSL_FILETYPE_ASN1); -#if defined(OPENSSL_ALL) || defined(WOLFSSL_QT) /* If there was an error, process it and add it to CTX */ if (ret < 0) { /* Get corresponding X509 error */ @@ -22980,8 +22976,10 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx) wolfSSL_X509_STORE_CTX_set_error(ctx, error); wolfSSL_X509_STORE_CTX_set_error_depth(ctx, depth); + #if defined(OPENSSL_ALL) || defined(WOLFSSL_QT) if (ctx->store && ctx->store->verify_cb) ctx->store->verify_cb(0, ctx); + #endif } error = 0; @@ -23004,10 +23002,11 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx) if (error != 0 ) { wolfSSL_X509_STORE_CTX_set_error(ctx, error); wolfSSL_X509_STORE_CTX_set_error_depth(ctx, depth); + #if defined(OPENSSL_ALL) || defined(WOLFSSL_QT) if (ctx->store && ctx->store->verify_cb) ctx->store->verify_cb(0, ctx); + #endif } -#endif /* OPENSSL_ALL || WOLFSSL_QT */ return ret; } return WOLFSSL_FATAL_ERROR; diff --git a/tests/api.c b/tests/api.c index f289dd513..7a24b1662 100644 --- a/tests/api.c +++ b/tests/api.c @@ -22561,26 +22561,48 @@ static void test_wolfSSL_X509_STORE(void) X509_STORE *store; #ifdef HAVE_CRL + X509_STORE_CTX *storeCtx; X509_CRL *crl; X509 *x509; - const char crl_pem[] = "./certs/crl/crl.pem"; - const char svrCert[] = "./certs/server-cert.pem"; + const char crlPem[] = "./certs/crl/crl.revoked"; + const char srvCert[] = "./certs/server-revoked-cert.pem"; + const char caCert[] = "./certs/ca-cert.pem"; XFILE fp; printf(testingFmt, "test_wolfSSL_X509_STORE"); AssertNotNull(store = (X509_STORE *)X509_STORE_new()); - AssertNotNull((x509 = - wolfSSL_X509_load_certificate_file(svrCert, SSL_FILETYPE_PEM))); + AssertNotNull((x509 = wolfSSL_X509_load_certificate_file(caCert, + SSL_FILETYPE_PEM))); AssertIntEQ(X509_STORE_add_cert(store, x509), SSL_SUCCESS); + AssertNotNull((x509 = wolfSSL_X509_load_certificate_file(srvCert, + SSL_FILETYPE_PEM))); + AssertNotNull((storeCtx = X509_STORE_CTX_new())); + AssertIntEQ(X509_STORE_CTX_init(storeCtx, store, x509, NULL), SSL_SUCCESS); + AssertIntEQ(X509_verify_cert(storeCtx), SSL_SUCCESS); + X509_STORE_CTX_free(storeCtx); X509_free(x509); - fp = XFOPEN(crl_pem, "rb"); + /* should fail to verify now after adding in CRL */ + AssertNotNull(store = (X509_STORE *)X509_STORE_new()); + AssertNotNull((x509 = wolfSSL_X509_load_certificate_file(caCert, + SSL_FILETYPE_PEM))); + AssertIntEQ(X509_STORE_add_cert(store, x509), SSL_SUCCESS); + fp = XFOPEN(crlPem, "rb"); AssertTrue((fp != XBADFILE)); - AssertNotNull(crl = (X509_CRL *)PEM_read_X509_CRL(fp, (X509_CRL **)NULL, NULL, NULL)); + AssertNotNull(crl = (X509_CRL *)PEM_read_X509_CRL(fp, (X509_CRL **)NULL, + NULL, NULL)); XFCLOSE(fp); AssertIntEQ(X509_STORE_add_crl(store, crl), SSL_SUCCESS); + AssertIntEQ(X509_STORE_set_flags(store, X509_V_FLAG_CRL_CHECK),SSL_SUCCESS); + AssertNotNull((storeCtx = X509_STORE_CTX_new())); + AssertNotNull((x509 = wolfSSL_X509_load_certificate_file(srvCert, + SSL_FILETYPE_PEM))); + AssertIntEQ(X509_STORE_CTX_init(storeCtx, store, x509, NULL), SSL_SUCCESS); + AssertIntNE(X509_verify_cert(storeCtx), SSL_SUCCESS); + AssertIntEQ(X509_STORE_CTX_get_error(storeCtx), CRL_CERT_REVOKED); + X509_free(x509); + X509_STORE_CTX_free(storeCtx); X509_CRL_free(crl); - X509_STORE_free(store); #endif /* HAVE_CRL */ From b88342eeaf70488ddd352207f744468a56838c7f Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Fri, 19 Jun 2020 10:08:42 -0700 Subject: [PATCH 3/5] memory handling fixes --- src/crl.c | 31 +++++++++++++++++++++++++++---- src/ssl.c | 23 +++++++++++------------ tests/api.c | 42 ++++++++++++++++++------------------------ 3 files changed, 56 insertions(+), 40 deletions(-) diff --git a/src/crl.c b/src/crl.c index 628acda78..9053c79a0 100644 --- a/src/crl.c +++ b/src/crl.c @@ -526,6 +526,7 @@ static RevokedCert *DupRevokedCertList(RevokedCert* in, void* heap) prev->next = tmp; if (head == NULL) head = tmp; + prev = tmp; } else { WOLFSSL_MSG("Failed to allocate new RevokedCert structure"); @@ -619,6 +620,17 @@ static CRL_Entry* DupCRL_list(CRL_Entry* crl, void* heap) head = tmp; if (prev != NULL) prev->next = tmp; + prev = tmp; + } + else { + WOLFSSL_MSG("Failed to allocate new CRL_Entry structure"); + /* free up any existing list */ + while (head != NULL) { + current = head; + head = head->next; + FreeCRL_Entry(current, heap); + } + return NULL; } current = current->next; } @@ -635,10 +647,6 @@ static int DupX509_CRL(WOLFSSL_X509_CRL *dup, const WOLFSSL_X509_CRL* crl) return BAD_FUNC_ARG; } - dup->crlList = DupCRL_list(crl->crlList, dup->heap); -#ifdef HAVE_CRL_IO - dup->crlIOCb = crl->crlIOCb; -#endif if (crl->monitors[0].path) { int pathSz = (int)XSTRLEN(crl->monitors[0].path) + 1; dup->monitors[0].path = (char*)XMALLOC(pathSz, dup->heap, @@ -646,6 +654,9 @@ static int DupX509_CRL(WOLFSSL_X509_CRL *dup, const WOLFSSL_X509_CRL* crl) if (dup->monitors[0].path != NULL) { XSTRNCPY(dup->monitors[0].path, crl->monitors[0].path, pathSz); } + else { + return MEMORY_E; + } } if (crl->monitors[1].path) { @@ -655,8 +666,20 @@ static int DupX509_CRL(WOLFSSL_X509_CRL *dup, const WOLFSSL_X509_CRL* crl) if (dup->monitors[1].path != NULL) { XSTRNCPY(dup->monitors[1].path, crl->monitors[1].path, pathSz); } + else { + if (dup->monitors[0].path != NULL) { + XFREE(dup->monitors[0].path, dup->heap, + DYNAMIC_TYPE_CRL_MONITOR); + } + return MEMORY_E; + } } + dup->crlList = DupCRL_list(crl->crlList, dup->heap); +#ifdef HAVE_CRL_IO + dup->crlIOCb = crl->crlIOCb; +#endif + return 0; } diff --git a/src/ssl.c b/src/ssl.c index ae2da8636..d165a398c 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -22764,11 +22764,15 @@ err_exit: void wolfSSL_X509_STORE_free(WOLFSSL_X509_STORE* store) { if (store != NULL && store->isDynamic) { - if (store->cm != NULL) + if (store->cm != NULL) { wolfSSL_CertManagerFree(store->cm); + store->cm = NULL; + } #if defined(OPENSSL_EXTRA) || defined(WOLFSSL_WPAS_SMALL) - if (store->param != NULL) + if (store->param != NULL) { XFREE(store->param, NULL, DYNAMIC_TYPE_OPENSSL); + store->param = NULL; + } #endif XFREE(store, NULL, DYNAMIC_TYPE_X509_STORE); } @@ -22893,23 +22897,18 @@ int wolfSSL_X509_STORE_CTX_init(WOLFSSL_X509_STORE_CTX* ctx, } +/* free's own cert chain holding and extra data */ void wolfSSL_X509_STORE_CTX_free(WOLFSSL_X509_STORE_CTX* ctx) { WOLFSSL_ENTER("X509_STORE_CTX_free"); if (ctx != NULL) { - #if !defined(OPENSSL_ALL) && !defined(WOLFSSL_QT) - if (ctx->store != NULL) - wolfSSL_X509_STORE_free(ctx->store); - #ifndef WOLFSSL_KEEP_STORE_CERTS - if (ctx->current_cert != NULL) - wolfSSL_FreeX509(ctx->current_cert); - #endif - #endif /* !OPENSSL_ALL && !WOLFSSL_QT */ -#ifdef OPENSSL_EXTRA + #ifdef OPENSSL_EXTRA + wolfSSL_sk_free(ctx->chain); if (ctx->param != NULL){ XFREE(ctx->param,NULL,DYNAMIC_TYPE_OPENSSL); + ctx->param = NULL; } -#endif + #endif XFREE(ctx, NULL, DYNAMIC_TYPE_X509_CTX); } } diff --git a/tests/api.c b/tests/api.c index 7a24b1662..40850d289 100644 --- a/tests/api.c +++ b/tests/api.c @@ -22085,10 +22085,8 @@ static void test_wolfSSL_X509_STORE_CTX_get0_current_issuer(void) X509_free(issuer); X509_STORE_CTX_free(ctx); - #if defined(WOLFSSL_KEEP_STORE_CERTS) || defined(OPENSSL_ALL) || defined(WOLFSSL_QT) - X509_free(x509Svr); - X509_STORE_free(str); - #endif + X509_free(x509Svr); + X509_STORE_free(str); X509_free(x509Ca); printf(resultFmt, passed); @@ -22130,10 +22128,8 @@ static void test_wolfSSL_X509_STORE_CTX(void) #ifdef OPENSSL_ALL sk_X509_free(sk); #endif - #if defined(WOLFSSL_KEEP_STORE_CERTS) || defined(OPENSSL_ALL) || defined(WOLFSSL_QT) X509_STORE_free(str); X509_free(x509); - #endif AssertNotNull(ctx = X509_STORE_CTX_new()); X509_STORE_CTX_set_verify_cb(ctx, verify_cb); @@ -22158,11 +22154,9 @@ static void test_wolfSSL_X509_STORE_CTX(void) AssertIntEQ(sk_num(sk3), 1); /* sanity, make sure chain has 1 cert */ X509_STORE_CTX_free(ctx); sk_X509_free(sk); - #if defined(WOLFSSL_KEEP_STORE_CERTS) || defined(WOLFSSL_QT) X509_STORE_free(str); /* CTX certs not freed yet */ X509_free(x5092); - #endif /* sk2 freed as part of X509_STORE_CTX_free(), sk3 is dup so free here */ sk_X509_free(sk3); #endif @@ -22354,9 +22348,7 @@ static void test_wolfSSL_X509_STORE_CTX_get0_store(void) wolfSSL_X509_STORE_CTX_free(ctx); wolfSSL_X509_STORE_CTX_free(ctx_no_init); -#if defined(OPENSSL_ALL) || defined(WOLFSSL_QT) X509_STORE_free(store); -#endif printf(resultFmt, passed); #endif /* OPENSSL_EXTRA */ @@ -22563,7 +22555,7 @@ static void test_wolfSSL_X509_STORE(void) #ifdef HAVE_CRL X509_STORE_CTX *storeCtx; X509_CRL *crl; - X509 *x509; + X509 *ca, *cert; const char crlPem[] = "./certs/crl/crl.revoked"; const char srvCert[] = "./certs/server-revoked-cert.pem"; const char caCert[] = "./certs/ca-cert.pem"; @@ -22571,22 +22563,24 @@ static void test_wolfSSL_X509_STORE(void) printf(testingFmt, "test_wolfSSL_X509_STORE"); AssertNotNull(store = (X509_STORE *)X509_STORE_new()); - AssertNotNull((x509 = wolfSSL_X509_load_certificate_file(caCert, + AssertNotNull((ca = wolfSSL_X509_load_certificate_file(caCert, SSL_FILETYPE_PEM))); - AssertIntEQ(X509_STORE_add_cert(store, x509), SSL_SUCCESS); - AssertNotNull((x509 = wolfSSL_X509_load_certificate_file(srvCert, + AssertIntEQ(X509_STORE_add_cert(store, ca), SSL_SUCCESS); + AssertNotNull((cert = wolfSSL_X509_load_certificate_file(srvCert, SSL_FILETYPE_PEM))); AssertNotNull((storeCtx = X509_STORE_CTX_new())); - AssertIntEQ(X509_STORE_CTX_init(storeCtx, store, x509, NULL), SSL_SUCCESS); + AssertIntEQ(X509_STORE_CTX_init(storeCtx, store, cert, NULL), SSL_SUCCESS); AssertIntEQ(X509_verify_cert(storeCtx), SSL_SUCCESS); + X509_STORE_free(store); X509_STORE_CTX_free(storeCtx); - X509_free(x509); + X509_free(cert); + X509_free(ca); /* should fail to verify now after adding in CRL */ AssertNotNull(store = (X509_STORE *)X509_STORE_new()); - AssertNotNull((x509 = wolfSSL_X509_load_certificate_file(caCert, + AssertNotNull((ca = wolfSSL_X509_load_certificate_file(caCert, SSL_FILETYPE_PEM))); - AssertIntEQ(X509_STORE_add_cert(store, x509), SSL_SUCCESS); + AssertIntEQ(X509_STORE_add_cert(store, ca), SSL_SUCCESS); fp = XFOPEN(crlPem, "rb"); AssertTrue((fp != XBADFILE)); AssertNotNull(crl = (X509_CRL *)PEM_read_X509_CRL(fp, (X509_CRL **)NULL, @@ -22595,14 +22589,16 @@ static void test_wolfSSL_X509_STORE(void) AssertIntEQ(X509_STORE_add_crl(store, crl), SSL_SUCCESS); AssertIntEQ(X509_STORE_set_flags(store, X509_V_FLAG_CRL_CHECK),SSL_SUCCESS); AssertNotNull((storeCtx = X509_STORE_CTX_new())); - AssertNotNull((x509 = wolfSSL_X509_load_certificate_file(srvCert, + AssertNotNull((cert = wolfSSL_X509_load_certificate_file(srvCert, SSL_FILETYPE_PEM))); - AssertIntEQ(X509_STORE_CTX_init(storeCtx, store, x509, NULL), SSL_SUCCESS); + AssertIntEQ(X509_STORE_CTX_init(storeCtx, store, cert, NULL), SSL_SUCCESS); AssertIntNE(X509_verify_cert(storeCtx), SSL_SUCCESS); AssertIntEQ(X509_STORE_CTX_get_error(storeCtx), CRL_CERT_REVOKED); - X509_free(x509); - X509_STORE_CTX_free(storeCtx); X509_CRL_free(crl); + X509_STORE_free(store); + X509_STORE_CTX_free(storeCtx); + X509_free(cert); + X509_free(ca); #endif /* HAVE_CRL */ @@ -23797,10 +23793,8 @@ static void test_wolfSSL_X509(void) X509_STORE_CTX_free(ctx); - #if defined(WOLFSSL_KEEP_STORE_CERTS) || defined(WOLFSSL_QT) X509_STORE_free(store); X509_free(x509); - #endif BIO_free(bio); /** d2i_X509_fp test **/ From 8511d0769875d7767c42e198c9562016863b3804 Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Tue, 23 Jun 2020 15:42:32 -0600 Subject: [PATCH 4/5] store chain is free'd when store is free'd --- tests/api.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/api.c b/tests/api.c index 40850d289..e6106a417 100644 --- a/tests/api.c +++ b/tests/api.c @@ -22125,9 +22125,6 @@ static void test_wolfSSL_X509_STORE_CTX(void) X509_STORE_CTX_set_error(NULL, -5); X509_STORE_CTX_free(ctx); -#ifdef OPENSSL_ALL - sk_X509_free(sk); -#endif X509_STORE_free(str); X509_free(x509); @@ -22153,7 +22150,6 @@ static void test_wolfSSL_X509_STORE_CTX(void) AssertNotNull((sk3 = X509_STORE_CTX_get1_chain(ctx))); AssertIntEQ(sk_num(sk3), 1); /* sanity, make sure chain has 1 cert */ X509_STORE_CTX_free(ctx); - sk_X509_free(sk); X509_STORE_free(str); /* CTX certs not freed yet */ X509_free(x5092); From b8b2f7ef7d2d56c63749ba26fc61c3f4628a1576 Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Wed, 24 Jun 2020 10:57:31 -0600 Subject: [PATCH 5/5] vs build warning fixes --- src/ssl.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index d165a398c..cdab65701 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -22989,12 +22989,12 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx) afterDate = ctx->current_cert->notAfter.data; beforeDate = ctx->current_cert->notBefore.data; - if (ValidateDate(afterDate, ctx->current_cert->notAfter.type, + if (ValidateDate(afterDate, (byte)ctx->current_cert->notAfter.type, AFTER) < 1) { error = X509_V_ERR_CERT_HAS_EXPIRED; } - else if (ValidateDate(beforeDate, ctx->current_cert->notBefore.type, - BEFORE) < 1) { + else if (ValidateDate(beforeDate, + (byte)ctx->current_cert->notBefore.type, BEFORE) < 1) { error = X509_V_ERR_CERT_NOT_YET_VALID; }