Skip to content

Commit

Permalink
Merge pull request wolfSSL#7779 from rizlik/ocsp-dfree-fix
Browse files Browse the repository at this point in the history 
ocsp: don't free ocsp request if saved in ssl->ctx->certOcspRequest
  • Loading branch information
@SparkiDev
SparkiDev authored Jul 30, 2024
2 parents ad76038 + 31380ac commit dbf88e4
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 8 deletions.
26 changes: 18 additions & 8 deletions src/internal.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23313,13 +23313,17 @@ 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 *ctxOwnsRequest)
{
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);
Expand All @@ -23333,8 +23337,11 @@ 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;
if (ctxOwnsRequest!= NULL)
*ctxOwnsRequest = 1;
}
wc_UnLockMutex(ocspLock);
}
}
Expand Down Expand Up @@ -23363,6 +23370,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;
Expand Down Expand Up @@ -23400,7 +23408,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) {
Expand All @@ -23427,7 +23435,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);
}
Expand Down Expand Up @@ -24122,6 +24130,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));
Expand Down Expand Up @@ -24180,7 +24189,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,
Expand All @@ -24195,12 +24204,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
Expand Down
55 changes: 55 additions & 0 deletions tests/api.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84316,6 +84316,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
Expand Down Expand Up @@ -85556,6 +85610,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),
Expand Down

0 comments on commit dbf88e4

Please sign in to comment.