From af518f8c86c85c52c3aeae89697add9ba1f0367c Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Mon, 17 Oct 2022 08:55:41 -0700 Subject: [PATCH 1/6] adjust saving new OCSP cert --- src/ocsp.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ocsp.c b/src/ocsp.c index d18e5739d..5d021bc13 100644 --- a/src/ocsp.c +++ b/src/ocsp.c @@ -327,6 +327,8 @@ int CheckOcspResponse(WOLFSSL_OCSP *ocsp, byte *response, int responseSz, goto end; } if (ocspRequest != NULL) { + /* Has the chance to bubble up response changing ocspResponse->single to + no longer be pointing at newSingle */ ret = CompareOcspReqResp(ocspRequest, ocspResponse); if (ret != 0) { goto end; @@ -368,7 +370,7 @@ int CheckOcspResponse(WOLFSSL_OCSP *ocsp, byte *response, int responseSz, status = (CertStatus*)XMALLOC(sizeof(CertStatus), ocsp->cm->heap, DYNAMIC_TYPE_OCSP_STATUS); if (status != NULL) { - XMEMCPY(status, newSingle->status, sizeof(CertStatus)); + XMEMCPY(status, ocspResponse->single->status, sizeof(CertStatus)); status->next = entry->status; entry->status = status; entry->ownStatus = 1; From 7381846edb59acfd9df43c5b1b73daa4a9d3fd3a Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Tue, 25 Oct 2022 15:33:17 -0700 Subject: [PATCH 2/6] fix case of copying over status to existing struct --- src/ocsp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ocsp.c b/src/ocsp.c index 5d021bc13..5d2650a32 100644 --- a/src/ocsp.c +++ b/src/ocsp.c @@ -363,7 +363,7 @@ int CheckOcspResponse(WOLFSSL_OCSP *ocsp, byte *response, int responseSz, /* Replace existing certificate entry with updated */ newSingle->status->next = status->next; - XMEMCPY(status, newSingle->status, sizeof(CertStatus)); + XMEMCPY(status, ocspResponse->single->status, sizeof(CertStatus)); } else { /* Save new certificate entry */ From 29a5c04c2e514b4ed79451f6322f943fb186dd70 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Tue, 25 Oct 2022 15:35:37 -0700 Subject: [PATCH 3/6] add test case --- certs/ocsp/include.am | 3 +- certs/ocsp/renewcerts.sh | 1 + certs/ocsp/test-multi-response.der | Bin 0 -> 1961 bytes tests/api.c | 60 +++++++++++++++++++++++++++++ 4 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 certs/ocsp/test-multi-response.der diff --git a/certs/ocsp/include.am b/certs/ocsp/include.am index 92a72b81e..1b663075f 100644 --- a/certs/ocsp/include.am +++ b/certs/ocsp/include.am @@ -35,4 +35,5 @@ EXTRA_DIST += \ certs/ocsp/root-ca-cert.pem \ certs/ocsp/test-response.der \ certs/ocsp/test-response-rsapss.der \ - certs/ocsp/test-response-nointern.der + certs/ocsp/test-response-nointern.der \ + certs/ocsp/test-multi-response.der diff --git a/certs/ocsp/renewcerts.sh b/certs/ocsp/renewcerts.sh index d5d411953..22103c4d0 100755 --- a/certs/ocsp/renewcerts.sh +++ b/certs/ocsp/renewcerts.sh @@ -87,6 +87,7 @@ PID=$! openssl ocsp -issuer ./root-ca-cert.pem -cert ./intermediate1-ca-cert.pem -url http://localhost:22221/ -respout test-response.der -noverify openssl ocsp -issuer ./root-ca-cert.pem -cert ./intermediate1-ca-cert.pem -url http://localhost:22221/ -respout test-response-nointern.der -no_intern -noverify +openssl ocsp -issuer ./root-ca-cert.pem -cert ./intermediate1-ca-cert.pem -cert ./intermediate2-ca-cert.pem -url http://localhost:22221/ -respout test-multi-response.der -noverify kill $PID wait $PID diff --git a/certs/ocsp/test-multi-response.der b/certs/ocsp/test-multi-response.der new file mode 100644 index 0000000000000000000000000000000000000000..09ea5d1bea2e1bf1232283e7312bd64d5bf67442 GIT binary patch literal 1961 zcmXqLVqeO|$grS^eV#!R`z$t2Z8k<$R(1nMMwTY_Zi6QFc7rCyr3)Ju8Z^!``IHC^L^3RGosqb8vt{P-<~OeqKsy5m*7I zR+~rLcV0$LMlpfRytI4=klNzn9KGcHTnT<710y3t10z!-LlZMYqbP&M69!2JRtB7G ztlE6cOj4{2EFvx|ZeQFpovYb8+HjW7j!Qx2qFo&M??-2ix7*ztF#Sfq1wGGlmGYz zy8XHI(}0%^7CwyJtPIRejEoE(-~Ox5HD?ch_vCMD>$4l4OV4WlOEJhr(_Xz7KX$W;M+H|lS{(JP=G9ZHG1c7h z_2u`>Y2e7BsOOGiYKtWYEO2&!CBU*#c%JMkXdk z7PPQ3Xq-;d#4TYU21(rfpadHn?4wYWpI@Tj>_{+G8_0?CqNHkZUL%y$H`ueKK@+1A za%eMxQzk!yK@%ev6DW!9*rmMX+u>@F!%M!(CNBIcm%Ou8*(4zMN6n%xmhP*iF}mNA zJ1(+Uz2bH?)qX9%QEAH*M~g>uORjYrCq%1|-b~%@d_>}&Si^*KDyE7@Lf1qEKG4(M%oXMDRcsjW&P!~sW%pWEkM@l-9|H1b5tU_B_9#VB?3Lb+=U7#jS6!q~f!jj z#2xJ|zZ#^tx@UZT{NK+-CbgudFeLS$IupI_w28~D9m_uciSsM2kH12G`T81rboV%oP*5FOt zO^nF~!XRI$vhWyiv2j4^9%fGD^a3l%x$;{>#aNCslt4KR3Gj zqBnY0{Vh)26aDvV%ytMdceDKTDKuT3&$g&&;u3c$TkC~;=I&j(xkqlZ{EBD6s{QHH z7X0JPv}txoYho(>TWOSj;1Z*O&cwG3|?b-<7QSD5NlndynGZggMV2E((5B zbIIv<-nT0@j*?CJD(5G4e)(zeYhL2o|Nr>D&W}~EOPc+{_2`d)ef~9jL~~g77U?aX z>sl7}`xBd2+=pcipWO>|uX*ck=4(DRC8v$$lAq`#(~#(uANFnd@hCiZ;s}9XlX^p-)4JO<0^NhM-c%3lcf#- literal 0 HcmV?d00001 diff --git a/tests/api.c b/tests/api.c index 9a0fc101d..bcdadc575 100644 --- a/tests/api.c +++ b/tests/api.c @@ -1672,6 +1672,7 @@ static int test_wolfSSL_CheckOCSPResponse(void) { #if defined(HAVE_OCSP) && !defined(NO_RSA) && defined(OPENSSL_ALL) const char* responseFile = "./certs/ocsp/test-response.der"; + const char* responseMultiFile = "./certs/ocsp/test-multi-response.der"; const char* responseNoInternFile = "./certs/ocsp/test-response-nointern.der"; const char* caFile = "./certs/ocsp/root-ca-cert.pem"; OcspResponse* res = NULL; @@ -1720,6 +1721,65 @@ static int test_wolfSSL_CheckOCSPResponse(void) AssertNotNull(res); wolfSSL_OCSP_RESPONSE_free(res); + /* check loading a response with multiple certs */ + { + WOLFSSL_CERT_MANAGER* cm = NULL; + OcspEntry entry[1]; + CertStatus status[1]; + OcspRequest* request; + + byte serial[] = {0x02}; + + byte issuerHash[] = { + 0x44, 0xA8, 0xDB, 0xD1, 0xBC, 0x97, 0x0A, 0x83, + 0x3B, 0x5B, 0x31, 0x9A, 0x4C, 0xB8, 0xD2, 0x52, + 0x37, 0x15, 0x8A, 0x88 + }; + byte issuerKeyHash[] = { + 0x73, 0xB0, 0x1C, 0xA4, 0x2F, 0x82, 0xCB, 0xCF, + 0x47, 0xA5, 0x38, 0xD7, 0xB0, 0x04, 0x82, 0x3A, + 0x7E, 0x72, 0x15, 0x21 + }; + + XMEMSET(entry, 0, sizeof(OcspEntry)); + XMEMSET(status, 0, sizeof(CertStatus)); + + AssertNotNull(request = wolfSSL_OCSP_REQUEST_new()); + request->serial = (byte*)XMALLOC(sizeof(serial), NULL, + DYNAMIC_TYPE_OCSP_REQUEST); + AssertNotNull(request->serial); + + request->serialSz = sizeof(serial); + XMEMCPY(request->serial, serial, sizeof(serial)); + XMEMCPY(request->issuerHash, issuerHash, sizeof(issuerHash)); + XMEMCPY(request->issuerKeyHash, issuerKeyHash, sizeof(issuerKeyHash)); + + AssertNotNull(cm = wolfSSL_CertManagerNew_ex(NULL)); + AssertIntEQ(wolfSSL_CertManagerEnableOCSP(cm, 0), WOLFSSL_SUCCESS); + AssertIntEQ(wolfSSL_CertManagerLoadCA(cm, caFile, NULL), + WOLFSSL_SUCCESS); + + f = XFOPEN(responseMultiFile, "rb"); + AssertTrue(f != XBADFILE); + dataSz = (word32)XFREAD(data, 1, sizeof(data), f); + AssertIntGT(dataSz, 0); + XFCLOSE(f); + + AssertIntEQ(wolfSSL_CertManagerCheckOCSPResponse(cm, data, + dataSz, NULL, status, entry, request), WOLFSSL_SUCCESS); + AssertIntEQ(wolfSSL_CertManagerCheckOCSPResponse(cm, data, + dataSz, NULL, entry->status, entry, request), WOLFSSL_SUCCESS); + + /* compare the status found */ + AssertNotNull(entry->status); + AssertIntEQ(status->serialSz, entry->status->serialSz); + AssertIntEQ(XMEMCMP(status->serial, entry->status->serial, + status->serialSz), 0); + + wolfSSL_OCSP_REQUEST_free(request); + wolfSSL_CertManagerFree(cm); + } + #if defined(WC_RSA_PSS) { const char* responsePssFile = "./certs/ocsp/test-response-rsapss.der"; From a26b89f66b0f8cc4307ff94ea7ab1dd5fd0983f7 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Wed, 26 Oct 2022 09:29:06 -0700 Subject: [PATCH 4/6] fix leak with multiple entries --- src/ocsp.c | 2 ++ tests/api.c | 7 ++++++- wolfcrypt/src/asn.c | 13 ++++++++----- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/ocsp.c b/src/ocsp.c index 5d2650a32..44972419c 100644 --- a/src/ocsp.c +++ b/src/ocsp.c @@ -312,6 +312,7 @@ int CheckOcspResponse(WOLFSSL_OCSP *ocsp, byte *response, int responseSz, return MEMORY_E; } #endif + XMEMSET(ocspResponse, 0, sizeof(OcspResponse)); InitOcspResponse(ocspResponse, newSingle, newStatus, response, responseSz, ocsp->cm->heap); @@ -399,6 +400,7 @@ end: ret = OCSP_LOOKUP_FAIL; } + FreeOcspResponse(ocspResponse); #ifdef WOLFSSL_SMALL_STACK XFREE(newStatus, NULL, DYNAMIC_TYPE_OCSP_STATUS); XFREE(newSingle, NULL, DYNAMIC_TYPE_OCSP_ENTRY); diff --git a/tests/api.c b/tests/api.c index bcdadc575..e3872e4eb 100644 --- a/tests/api.c +++ b/tests/api.c @@ -1724,7 +1724,7 @@ static int test_wolfSSL_CheckOCSPResponse(void) /* check loading a response with multiple certs */ { WOLFSSL_CERT_MANAGER* cm = NULL; - OcspEntry entry[1]; + OcspEntry *entry; CertStatus status[1]; OcspRequest* request; @@ -1741,6 +1741,10 @@ static int test_wolfSSL_CheckOCSPResponse(void) 0x7E, 0x72, 0x15, 0x21 }; + entry = (OcspEntry*)XMALLOC(sizeof(OcspEntry), NULL, + DYNAMIC_TYPE_OPENSSL); + AssertNotNull(entry); + XMEMSET(entry, 0, sizeof(OcspEntry)); XMEMSET(status, 0, sizeof(CertStatus)); @@ -1776,6 +1780,7 @@ static int test_wolfSSL_CheckOCSPResponse(void) AssertIntEQ(XMEMCMP(status->serial, entry->status->serial, status->serialSz), 0); + wolfSSL_OCSP_CERTID_free(entry); wolfSSL_OCSP_REQUEST_free(request); wolfSSL_CertManagerFree(cm); } diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index e8018ea0e..2c962ffb3 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -34675,11 +34675,14 @@ void InitOcspResponse(OcspResponse* resp, OcspEntry* single, CertStatus* status, void FreeOcspResponse(OcspResponse* resp) { OcspEntry *single, *next; - for (single = resp->single; single; single = next) { - next = single->next; - if (single->isDynamic) { - XFREE(single->status, resp->heap, DYNAMIC_TYPE_OCSP_STATUS); - XFREE(single, resp->heap, DYNAMIC_TYPE_OCSP_ENTRY); + + if (resp != NULL) { + for (single = resp->single; single; single = next) { + next = single->next; + if (single->isDynamic) { + XFREE(single->status, resp->heap, DYNAMIC_TYPE_OCSP_STATUS); + XFREE(single, resp->heap, DYNAMIC_TYPE_OCSP_ENTRY); + } } } } From 33617588fced38e65530f90bc45c569600bf6824 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Wed, 26 Oct 2022 10:31:50 -0700 Subject: [PATCH 5/6] fix setting dynamic flag with ocsp and asn template --- wolfcrypt/src/asn.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 2c962ffb3..fbc1f6483 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -34226,7 +34226,7 @@ static int DecodeResponseData(byte* source, word32* ioIndex, XMEMSET(single->next->status, 0, sizeof(CertStatus)); /* Entry to be freed. */ - single->isDynamic = 1; + single->next->isDynamic = 1; /* used will be 0 (false) */ single = single->next; From d08c204466dd2e541f2bf7530bda0102dac0a0f7 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Wed, 26 Oct 2022 12:54:17 -0700 Subject: [PATCH 6/6] remove extra memset --- src/ocsp.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ocsp.c b/src/ocsp.c index 44972419c..60be257a7 100644 --- a/src/ocsp.c +++ b/src/ocsp.c @@ -312,7 +312,6 @@ int CheckOcspResponse(WOLFSSL_OCSP *ocsp, byte *response, int responseSz, return MEMORY_E; } #endif - XMEMSET(ocspResponse, 0, sizeof(OcspResponse)); InitOcspResponse(ocspResponse, newSingle, newStatus, response, responseSz, ocsp->cm->heap);