Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ocsp: don't free ocsp request if saved in ssl->ctx->certOcspRequest #7779

Merged
merged 3 commits into from
Jul 30, 2024
Merged

ocsp: don't free ocsp request if saved in ssl->ctx->certOcspRequest #7779

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bb60c58
ocsp: don't free ocsp request if saved in ssl->ctx->certOcspRequest
rizlik Jul 23, 2024
a1fbfa9
tests: add OCSP callback fails test
rizlik Jul 24, 2024
31380ac
fixup! ocsp: don't free ocsp request if saved in ssl->ctx->certOcspRe…
rizlik Jul 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23310,13 +23310,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)
SparkiDev marked this conversation as resolved.
Show resolved Hide resolved
{
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 @@ -23330,8 +23334,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 @@ -23360,6 +23367,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 @@ -23397,7 +23405,7 @@ int CreateOcspResponse(WOLFSSL* ssl, OcspRequest** ocspRequest,
createdRequest = 1;
if (ret == 0) {
ret = CreateOcspRequest(ssl, request, cert, der->buffer,
SparkiDev marked this conversation as resolved.
Show resolved Hide resolved
der->length);
der->length, &ctxOwnsRequest);
}

if (ret != 0) {
Expand All @@ -23424,7 +23432,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 @@ -24119,6 +24127,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 @@ -24177,7 +24186,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 @@ -24192,12 +24201,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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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),
Expand Down