From bb60c588003e8ea9a1a8f85fdad23fdc9d3bce82 Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Tue, 23 Jul 2024 15:37:41 +0000 Subject: [PATCH 1/3] ocsp: don't free ocsp request if saved in ssl->ctx->certOcspRequest --- src/internal.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/internal.c b/src/internal.c index 324ec932cd..ece556c8f4 100644 --- a/src/internal.c +++ b/src/internal.c @@ -23310,8 +23310,10 @@ int SendFinished(WOLFSSL* ssl) * Returns 0 on success */ static int CreateOcspRequest(WOLFSSL* ssl, OcspRequest* request, - DecodedCert* cert, byte* certData, word32 length) + DecodedCert* cert, byte* certData, word32 length, + byte *takeOwnership) { + byte ctxOwnsRequest = 0; int ret; if (request != NULL) @@ -23330,14 +23332,18 @@ static int CreateOcspRequest(WOLFSSL* ssl, OcspRequest* request, if (!ssl->buffers.weOwnCert) { wolfSSL_Mutex* ocspLock = &SSL_CM(ssl)->ocsp_stapling->ocspLock; if (wc_LockMutex(ocspLock) == 0) { - if (ssl->ctx->certOcspRequest == NULL) + if (ssl->ctx->certOcspRequest == NULL) { ssl->ctx->certOcspRequest = request; + ctxOwnsRequest = 1; + } wc_UnLockMutex(ocspLock); } } } FreeDecodedCert(cert); + if (takeOwnership != NULL) + *takeOwnership = ctxOwnsRequest; return ret; } @@ -23360,6 +23366,7 @@ int CreateOcspResponse(WOLFSSL* ssl, OcspRequest** ocspRequest, int ret = 0; OcspRequest* request = NULL; byte createdRequest = 0; + byte ctxOwnsRequest = 0; if (ssl == NULL || ocspRequest == NULL || response == NULL) return BAD_FUNC_ARG; @@ -23397,7 +23404,7 @@ int CreateOcspResponse(WOLFSSL* ssl, OcspRequest** ocspRequest, createdRequest = 1; if (ret == 0) { ret = CreateOcspRequest(ssl, request, cert, der->buffer, - der->length); + der->length, &ctxOwnsRequest); } if (ret != 0) { @@ -23424,7 +23431,7 @@ int CreateOcspResponse(WOLFSSL* ssl, OcspRequest** ocspRequest, } /* free request up if error case found otherwise return it */ - if (ret != 0 && createdRequest) { + if (ret != 0 && createdRequest && !ctxOwnsRequest) { FreeOcspRequest(request); XFREE(request, ssl->heap, DYNAMIC_TYPE_OCSP_REQUEST); } @@ -24119,6 +24126,7 @@ int SendCertificateStatus(WOLFSSL* ssl) { OcspRequest* request = ssl->ctx->certOcspRequest; buffer responses[1 + MAX_CHAIN_DEPTH]; + byte ctxOwnsRequest = 0; int i = 0; XMEMSET(responses, 0, sizeof(responses)); @@ -24177,7 +24185,7 @@ int SendCertificateStatus(WOLFSSL* ssl) break; ret = CreateOcspRequest(ssl, request, cert, der.buffer, - der.length); + der.length, &ctxOwnsRequest); if (ret == 0) { request->ssl = ssl; ret = CheckOcspRequest(SSL_CM(ssl)->ocsp_stapling, @@ -24192,12 +24200,13 @@ int SendCertificateStatus(WOLFSSL* ssl) i++; - FreeOcspRequest(request); + if (!ctxOwnsRequest) + FreeOcspRequest(request); } } } - - XFREE(request, ssl->heap, DYNAMIC_TYPE_OCSP_REQUEST); + if (!ctxOwnsRequest) + XFREE(request, ssl->heap, DYNAMIC_TYPE_OCSP_REQUEST); #ifdef WOLFSSL_SMALL_STACK XFREE(cert, ssl->heap, DYNAMIC_TYPE_DCERT); #endif From a1fbfa94d20a3610021a4d7907233b4c04914ed0 Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Wed, 24 Jul 2024 10:56:22 +0000 Subject: [PATCH 2/3] tests: add OCSP callback fails test --- tests/api.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/tests/api.c b/tests/api.c index 8a140e1819..61083e4b29 100644 --- a/tests/api.c +++ b/tests/api.c @@ -82888,6 +82888,60 @@ static int test_wolfSSL_SendUserCanceled(void) #endif return EXPECT_RESULT(); } +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + defined(HAVE_OCSP) && \ + defined(HAVE_CERTIFICATE_STATUS_REQUEST) && \ + !defined(WOLFSSL_NO_TLS12) +static int test_ocsp_callback_fails_cb(void* ctx, const char* url, int urlSz, + byte* ocspReqBuf, int ocspReqSz, byte** ocspRespBuf) +{ + (void)ctx; + (void)url; + (void)urlSz; + (void)ocspReqBuf; + (void)ocspReqSz; + (void)ocspRespBuf; + return -1; +} +static int test_ocsp_callback_fails(void) +{ + WOLFSSL_CTX *ctx_c = NULL; + WOLFSSL_CTX *ctx_s = NULL; + WOLFSSL *ssl_c = NULL; + WOLFSSL *ssl_s = NULL; + struct test_memio_ctx test_ctx; + EXPECT_DECLS; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0); + ExpectIntEQ(wolfSSL_CTX_EnableOCSPStapling(ctx_c), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_CTX_EnableOCSPStapling(ctx_s), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_UseOCSPStapling(ssl_c, WOLFSSL_CSR_OCSP,0), WOLFSSL_SUCCESS); + /* override URL to avoid exing from SendCertificateStatus because of no AuthInfo on the certificate */ + ExpectIntEQ(wolfSSL_CTX_SetOCSP_OverrideURL(ctx_s, "http://dummy.test"), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_CTX_EnableOCSP(ctx_s, WOLFSSL_OCSP_NO_NONCE | WOLFSSL_OCSP_URL_OVERRIDE), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_CTX_load_verify_locations(ctx_s, caCertFile, 0), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_SetOCSP_Cb(ssl_s, test_ocsp_callback_fails_cb, NULL, NULL), WOLFSSL_SUCCESS); + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), OCSP_INVALID_STATUS); + + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); + + return EXPECT_RESULT(); +} +#else +static int test_ocsp_callback_fails(void) +{ + return TEST_SKIPPED; +} +#endif /* defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + defined(HAVE_OCSP) && \ + defined(HAVE_CERTIFICATE_STATUS_REQUEST) */ + /*----------------------------------------------------------------------------* | Main @@ -84120,6 +84174,7 @@ TEST_CASE testCases[] = { TEST_DECL(test_wolfSSL_UseOCSPStapling), TEST_DECL(test_wolfSSL_UseOCSPStaplingV2), TEST_DECL(test_self_signed_stapling), + TEST_DECL(test_ocsp_callback_fails), /* Multicast */ TEST_DECL(test_wolfSSL_mcast), From 31380aca13ca4a380b546c320ba544347663d47e Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Mon, 29 Jul 2024 15:00:41 +0000 Subject: [PATCH 3/3] fixup! ocsp: don't free ocsp request if saved in ssl->ctx->certOcspRequest --- src/internal.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/internal.c b/src/internal.c index ece556c8f4..0509918f50 100644 --- a/src/internal.c +++ b/src/internal.c @@ -23311,14 +23311,16 @@ int SendFinished(WOLFSSL* ssl) */ static int CreateOcspRequest(WOLFSSL* ssl, OcspRequest* request, DecodedCert* cert, byte* certData, word32 length, - byte *takeOwnership) + byte *ctxOwnsRequest) { - byte ctxOwnsRequest = 0; int ret; if (request != NULL) XMEMSET(request, 0, sizeof(OcspRequest)); + if (ctxOwnsRequest!= NULL) + *ctxOwnsRequest = 0; + InitDecodedCert(cert, certData, length, ssl->heap); /* TODO: Setup async support here */ ret = ParseCertRelative(cert, CERT_TYPE, VERIFY, SSL_CM(ssl), NULL); @@ -23334,7 +23336,8 @@ static int CreateOcspRequest(WOLFSSL* ssl, OcspRequest* request, if (wc_LockMutex(ocspLock) == 0) { if (ssl->ctx->certOcspRequest == NULL) { ssl->ctx->certOcspRequest = request; - ctxOwnsRequest = 1; + if (ctxOwnsRequest!= NULL) + *ctxOwnsRequest = 1; } wc_UnLockMutex(ocspLock); } @@ -23342,8 +23345,6 @@ static int CreateOcspRequest(WOLFSSL* ssl, OcspRequest* request, } FreeDecodedCert(cert); - if (takeOwnership != NULL) - *takeOwnership = ctxOwnsRequest; return ret; }