Merge pull request #10115 from julek-wolfssl/zd/21469

Fix multiple bugs in OCSP implementation

src/ocsp.c

@@ -89,7 +89,7 @@ int wc_CheckCertOcspResponse(WOLFSSL_OCSP *ocsp, DecodedCert *cert,
     ocspRequest = (OcspRequest*)XMALLOC(sizeof(OcspRequest), NULL,
                                                        DYNAMIC_TYPE_TMP_BUFFER);
     if (ocspRequest == NULL) {
-        WOLFSSL_LEAVE("CheckCertOCSP", MEMORY_ERROR);
+        WOLFSSL_LEAVE("wc_CheckCertOcspResponse", MEMORY_ERROR);
         return MEMORY_E;
     }
 #endif
@@ -105,7 +105,7 @@ int wc_CheckCertOcspResponse(WOLFSSL_OCSP *ocsp, DecodedCert *cert,
     XFREE(ocspRequest, NULL, DYNAMIC_TYPE_TMP_BUFFER);
 #endif
 
-    WOLFSSL_LEAVE("CheckCertOCSP", ret);
+    WOLFSSL_LEAVE("wc_CheckCertOcspResponse", ret);
     return ret;
 }
 
@@ -171,19 +171,20 @@ static void FreeOcspEntry(OcspEntry* entry, void* heap)
 void FreeOCSP(WOLFSSL_OCSP* ocsp, int dynamic)
 {
     OcspEntry *entry, *next;
+    void* heap = (ocsp->cm != NULL) ? ocsp->cm->heap : NULL;
 
     WOLFSSL_ENTER("FreeOCSP");
 
     for (entry = ocsp->ocspList; entry; entry = next) {
         next = entry->next;
-        FreeOcspEntry(entry, ocsp->cm->heap);
-        XFREE(entry, ocsp->cm->heap, DYNAMIC_TYPE_OCSP_ENTRY);
+        FreeOcspEntry(entry, heap);
+        XFREE(entry, heap, DYNAMIC_TYPE_OCSP_ENTRY);
     }
 
     wc_FreeMutex(&ocsp->ocspLock);
 
     if (dynamic)
-        XFREE(ocsp, ocsp->cm->heap, DYNAMIC_TYPE_OCSP);
+        XFREE(ocsp, heap, DYNAMIC_TYPE_OCSP);
 
 }
 
@@ -244,7 +245,7 @@ static int GetOcspEntry(WOLFSSL_OCSP* ocsp, OcspRequest* request,
     *entry = NULL;
 
     if (wc_LockMutex(&ocsp->ocspLock) != 0) {
-        WOLFSSL_LEAVE("CheckCertOCSP", BAD_MUTEX_E);
+        WOLFSSL_LEAVE("GetOcspEntry", BAD_MUTEX_E);
         return BAD_MUTEX_E;
     }
 
@@ -287,7 +288,7 @@ static int GetOcspStatus(WOLFSSL_OCSP* ocsp, OcspRequest* request,
     *status = NULL;
 
     if (wc_LockMutex(&ocsp->ocspLock) != 0) {
-        WOLFSSL_LEAVE("CheckCertOCSP", BAD_MUTEX_E);
+        WOLFSSL_LEAVE("GetOcspStatus", BAD_MUTEX_E);
         return BAD_MUTEX_E;
     }
 
@@ -374,7 +375,7 @@ int CheckOcspResponse(WOLFSSL_OCSP *ocsp, byte *response, int responseSz,
         XFREE(newSingle, NULL, DYNAMIC_TYPE_OCSP_ENTRY);
         XFREE(ocspResponse, NULL, DYNAMIC_TYPE_OCSP_REQUEST);
 
-        WOLFSSL_LEAVE("CheckCertOCSP", MEMORY_ERROR);
+        WOLFSSL_LEAVE("CheckOcspResponse", MEMORY_ERROR);
         return MEMORY_E;
     }
 #endif
@@ -550,7 +551,7 @@ int CheckOcspRequest(WOLFSSL_OCSP* ocsp, OcspRequest* ocspRequest,
 
     request = (byte*)XMALLOC((size_t)requestSz, ocsp->cm->heap, DYNAMIC_TYPE_OCSP);
     if (request == NULL) {
-        WOLFSSL_LEAVE("CheckCertOCSP", MEMORY_ERROR);
+        WOLFSSL_LEAVE("CheckOcspRequest", MEMORY_ERROR);
         return MEMORY_ERROR;
     }
 
@@ -1285,7 +1286,7 @@ OcspResponse* wolfSSL_d2i_OCSP_RESPONSE(OcspResponse** response,
     }
 
     if (GetSequence(*data, &idx, &length, (word32)len) >= 0)
-        (*data) += (unsigned char) ((int)idx + length);
+        (*data) += idx + length;
 
     if (response != NULL && *response == NULL)
         *response = resp;
@@ -1296,6 +1297,9 @@ OcspResponse* wolfSSL_d2i_OCSP_RESPONSE(OcspResponse** response,
 int wolfSSL_i2d_OCSP_RESPONSE(OcspResponse* response,
     unsigned char** data)
 {
+    if (response == NULL)
+        return BAD_FUNC_ARG;
+
     if (data == NULL)
         return (int)response->maxIdx;
 
@@ -1366,7 +1370,11 @@ int wolfSSL_i2d_OCSP_REQUEST(OcspRequest* request, unsigned char** data)
     if (size <= 0 || data == NULL)
         return size;
 
-    return EncodeOcspRequest(request, *data, (word32) size);
+    size = EncodeOcspRequest(request, *data, (word32) size);
+    if (size > 0)
+        *data += size;
+
+    return size;
 }
 
 WOLFSSL_OCSP_ONEREQ* wolfSSL_OCSP_request_add0_id(OcspRequest *req,
@@ -1405,9 +1413,35 @@ WOLFSSL_OCSP_CERTID* wolfSSL_OCSP_CERTID_dup(WOLFSSL_OCSP_CERTID* id)
 
     certId = (WOLFSSL_OCSP_CERTID*)XMALLOC(sizeof(WOLFSSL_OCSP_CERTID),
         NULL, DYNAMIC_TYPE_OPENSSL);
-    if (certId) {
-        XMEMCPY(certId, id, sizeof(WOLFSSL_OCSP_CERTID));
+    if (certId == NULL)
+        return NULL;
+
+    XMEMCPY(certId, id, sizeof(WOLFSSL_OCSP_CERTID));
+    certId->next = NULL;
+    certId->rawCertId = NULL;
+    certId->rawCertIdSize = 0;
+
+    /* Deep-copy the status to avoid double-free */
+    if (id->status != NULL) {
+        certId->status = (CertStatus*)XMALLOC(sizeof(CertStatus),
+            NULL, DYNAMIC_TYPE_OCSP_STATUS);
+        if (certId->status == NULL) {
+            XFREE(certId, NULL, DYNAMIC_TYPE_OPENSSL);
+            return NULL;
+        }
+        XMEMCPY(certId->status, id->status, sizeof(CertStatus));
+        certId->status->next = NULL;
+        /* Don't share dynamically allocated fields */
+        certId->status->rawOcspResponse = NULL;
+        certId->status->rawOcspResponseSz = 0;
+        certId->status->serialInt = NULL;
+#ifdef WOLFSSL_OCSP_PARSE_STATUS
+        certId->status->thisDateAsn = NULL;
+        certId->status->nextDateAsn = NULL;
+#endif
     }
+    certId->ownStatus = 1;
+
     return certId;
 }
 
@@ -1429,7 +1463,9 @@ int wolfSSL_i2d_OCSP_REQUEST_bio(WOLFSSL_BIO* out,
     }
 
     if (data != NULL) {
+        unsigned char* dataOrig = data;
         size = wolfSSL_i2d_OCSP_REQUEST(req, &data);
+        data = dataOrig;
     }
 
     if (size <= 0) {

tests/api.c

@@ -2989,6 +2989,7 @@ static int test_wolfSSL_CheckOCSPResponse(void)
 
     pt = data;
     ExpectNotNull(res = wolfSSL_d2i_OCSP_RESPONSE(NULL, &pt, dataSz));
+    ExpectPtrEq(pt, data + dataSz);
     ExpectNotNull(issuer = wolfSSL_X509_load_certificate_file(caFile,
         SSL_FILETYPE_PEM));
     ExpectNotNull(st = wolfSSL_X509_STORE_new());
@@ -3013,6 +3014,7 @@ static int test_wolfSSL_CheckOCSPResponse(void)
 
     pt = data;
     ExpectNotNull(res = wolfSSL_d2i_OCSP_RESPONSE(NULL, &pt, dataSz));
+    ExpectPtrEq(pt, data + dataSz);
     wolfSSL_OCSP_RESPONSE_free(res);
     res = NULL;
 
@@ -3124,6 +3126,7 @@ static int test_wolfSSL_CheckOCSPResponse(void)
 
         pt = data;
         ExpectNotNull(res = wolfSSL_d2i_OCSP_RESPONSE(NULL, &pt, dataSz));
+        ExpectPtrEq(pt, data + dataSz);
 
         /* try to verify the response */
         ExpectNotNull(issuer = wolfSSL_X509_load_certificate_file(caFile,
@@ -35489,6 +35492,7 @@ TEST_CASE testCases[] = {
     TEST_DECL(test_ocsp_basic_verify),
     TEST_DECL(test_ocsp_response_parsing),
     TEST_DECL(test_ocsp_certid_enc_dec),
+    TEST_DECL(test_ocsp_certid_dup),
     TEST_DECL(test_ocsp_tls_cert_cb),
     TEST_DECL(test_ocsp_cert_unknown_crl_fallback),
     TEST_DECL(test_ocsp_cert_unknown_crl_fallback_nonleaf),

tests/api/test_ocsp.c

@@ -215,6 +215,7 @@ int test_ocsp_basic_verify(void)
     ptr = (const unsigned char*)resp;
     ExpectNotNull(
         response = wolfSSL_d2i_OCSP_RESPONSE(NULL, &ptr, sizeof(resp)));
+    ExpectPtrEq(ptr, (const unsigned char*)resp + sizeof(resp));
     ExpectIntEQ(response->responseStatus, 0);
     ExpectIntEQ(response->responderIdType, OCSP_RESPONDER_ID_NAME);
     ExpectBufEQ(response->responderId.nameHash, cert.subjectHash,
@@ -225,6 +226,8 @@ int test_ocsp_basic_verify(void)
     ptr = (const unsigned char*)resp_rid_bykey;
     ExpectNotNull(response = wolfSSL_d2i_OCSP_RESPONSE(NULL, &ptr,
                       sizeof(resp_rid_bykey)));
+    ExpectPtrEq(ptr, (const unsigned char*)resp_rid_bykey +
+        sizeof(resp_rid_bykey));
     ExpectIntEQ(response->responseStatus, 0);
     ExpectIntEQ(response->responderIdType, OCSP_RESPONDER_ID_KEY);
     ExpectBufEQ(response->responderId.keyHash, cert.subjectKeyHash,
@@ -235,6 +238,7 @@ int test_ocsp_basic_verify(void)
     ptr = (const unsigned char*)resp_nocert;
     ExpectNotNull(
         response = wolfSSL_d2i_OCSP_RESPONSE(NULL, &ptr, sizeof(resp_nocert)));
+    ExpectPtrEq(ptr, (const unsigned char*)resp_nocert + sizeof(resp_nocert));
     ExpectIntEQ(response->responseStatus, 0);
     wolfSSL_OCSP_RESPONSE_free(response);
 
@@ -246,6 +250,7 @@ int test_ocsp_basic_verify(void)
     ptr = (const unsigned char*)resp;
     ExpectNotNull(
         response = wolfSSL_d2i_OCSP_RESPONSE(NULL, &ptr, sizeof(resp)));
+    ExpectPtrEq(ptr, (const unsigned char*)resp + sizeof(resp));
     /* no verify signer certificate */
     ExpectIntEQ(wolfSSL_OCSP_basic_verify(response, NULL, NULL, OCSP_NOVERIFY),
         WOLFSSL_SUCCESS);
@@ -272,6 +277,7 @@ int test_ocsp_basic_verify(void)
     ptr = (const unsigned char*)resp_nocert;
     ExpectNotNull(
         response = wolfSSL_d2i_OCSP_RESPONSE(NULL, &ptr, sizeof(resp_nocert)));
+    ExpectPtrEq(ptr, (const unsigned char*)resp_nocert + sizeof(resp_nocert));
     ExpectIntEQ(wolfSSL_OCSP_basic_verify(response, certs, store, 0),
         WOLFSSL_SUCCESS);
     wolfSSL_OCSP_RESPONSE_free(response);
@@ -281,6 +287,7 @@ int test_ocsp_basic_verify(void)
     ptr = (const unsigned char*)resp;
     ExpectNotNull(
         response = wolfSSL_d2i_OCSP_RESPONSE(NULL, &ptr, sizeof(resp)));
+    ExpectPtrEq(ptr, (const unsigned char*)resp + sizeof(resp));
     ExpectIntEQ(wolfSSL_OCSP_basic_verify(response, NULL, store, 0),
         WOLFSSL_SUCCESS);
     /* make invalid signature */
@@ -311,6 +318,7 @@ int test_ocsp_basic_verify(void)
     ptr = (const unsigned char*)resp_nocert;
     ExpectNotNull(
         response = wolfSSL_d2i_OCSP_RESPONSE(NULL, &ptr, sizeof(resp_nocert)));
+    ExpectPtrEq(ptr, (const unsigned char*)resp_nocert + sizeof(resp_nocert));
     ExpectIntNE(wolfSSL_OCSP_basic_verify(response, NULL, store, 0),
         WOLFSSL_SUCCESS);
     wolfSSL_OCSP_RESPONSE_free(response);
@@ -332,6 +340,7 @@ int test_ocsp_basic_verify(void)
     ptr = (const unsigned char*)resp_multi;
     ExpectNotNull(
         response = wolfSSL_d2i_OCSP_RESPONSE(NULL, &ptr, sizeof(resp_multi)));
+    ExpectPtrEq(ptr, (const unsigned char*)resp_multi + sizeof(resp_multi));
     ExpectIntEQ(wolfSSL_OCSP_basic_verify(response, certs, store, 0),
         WOLFSSL_SUCCESS);
     wolfSSL_OCSP_RESPONSE_free(response);
@@ -342,6 +351,8 @@ int test_ocsp_basic_verify(void)
     ptr = (const unsigned char*)resp_bad_noauth;
     ExpectNotNull(response = wolfSSL_d2i_OCSP_RESPONSE(NULL, &ptr,
                       sizeof(resp_bad_noauth)));
+    ExpectPtrEq(ptr, (const unsigned char*)resp_bad_noauth +
+        sizeof(resp_bad_noauth));
 
     expectedRet = WOLFSSL_FAILURE;
 #ifdef WOLFSSL_NO_OCSP_ISSUER_CHECK
@@ -665,11 +676,51 @@ int test_ocsp_certid_enc_dec(void)
     wolfSSL_X509_free(issuer);
     return EXPECT_SUCCESS();
 }
+int test_ocsp_certid_dup(void)
+{
+    EXPECT_DECLS;
+    WOLFSSL_OCSP_CERTID* certId = NULL;
+    WOLFSSL_OCSP_CERTID* certIdDup = NULL;
+    WOLFSSL_X509* subject = NULL;
+    WOLFSSL_X509* issuer = NULL;
+
+    /* Load test certificates */
+    ExpectNotNull(
+        subject = wolfSSL_X509_load_certificate_file(
+            "./certs/ocsp/intermediate1-ca-cert.pem", WOLFSSL_FILETYPE_PEM));
+    ExpectNotNull(issuer = wolfSSL_X509_load_certificate_file(
+                      "./certs/ocsp/root-ca-cert.pem", WOLFSSL_FILETYPE_PEM));
+
+    /* Create CERTID from certificates */
+    ExpectNotNull(certId = wolfSSL_OCSP_cert_to_id(NULL, subject, issuer));
+
+    /* Dup */
+    ExpectNotNull(certIdDup = wolfSSL_OCSP_CERTID_dup(certId));
+
+    /* Verify the dup compares equal */
+    ExpectIntEQ(wolfSSL_OCSP_id_cmp(certId, certIdDup), 0);
+
+    /* Verify status is a distinct allocation (deep copy) */
+    ExpectPtrNE(certId->status, certIdDup->status);
+
+    /* Freeing both must not double-free (ASAN will catch it) */
+    wolfSSL_OCSP_CERTID_free(certId);
+    wolfSSL_OCSP_CERTID_free(certIdDup);
+
+    wolfSSL_X509_free(subject);
+    wolfSSL_X509_free(issuer);
+    return EXPECT_SUCCESS();
+}
+
 #else /* !NO_SHA && OPENSSL_ALL && HAVE_OCSP && !WOLFSSL_SM3 && !WOLFSSL_SM2 */
 int test_ocsp_certid_enc_dec(void)
 {
     return TEST_SKIPPED;
 }
+int test_ocsp_certid_dup(void)
+{
+    return TEST_SKIPPED;
+}
 #endif
 
 #if defined(HAVE_OCSP) && defined(WOLFSSL_CERT_SETUP_CB) && \

tests/api/test_ocsp.h

@@ -23,6 +23,7 @@
 #define WOLFSSL_TEST_OCSP_H
 
 int test_ocsp_certid_enc_dec(void);
+int test_ocsp_certid_dup(void);
 int test_ocsp_status_callback(void);
 int test_ocsp_basic_verify(void);
 int test_ocsp_response_parsing(void);