From f44bbe9ba387dfc9b3ecaa72d6b19d8d6ff68029 Mon Sep 17 00:00:00 2001 From: toddouska Date: Fri, 27 Jan 2017 15:42:00 -0800 Subject: [PATCH 1/3] Better CheckOcspRequest error detection on retry --- src/ocsp.c | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/src/ocsp.c b/src/ocsp.c index e8de3d512..5d5f8f6ab 100644 --- a/src/ocsp.c +++ b/src/ocsp.c @@ -251,6 +251,7 @@ int CheckOcspRequest(WOLFSSL_OCSP* ocsp, OcspRequest* ocspRequest, CertStatus* status = NULL; byte* request = NULL; int requestSz = 2048; + int responseSz = 0; byte* response = NULL; const char* url = NULL; int urlSz = 0; @@ -319,31 +320,40 @@ int CheckOcspRequest(WOLFSSL_OCSP* ocsp, OcspRequest* ocspRequest, #endif requestSz = EncodeOcspRequest(ocspRequest, request, requestSz); + if (requestSz > 0 && ocsp->cm->ocspIOCb) { + responseSz = ocsp->cm->ocspIOCb(ocsp->cm->ocspIOCtx, url, urlSz, + request, requestSz, &response); + if (responseSz < 0) { + ret = responseSz; /* because ret was used for multiple purposes */ + } + } - if (ocsp->cm->ocspIOCb) - ret = ocsp->cm->ocspIOCb(ocsp->cm->ocspIOCtx, url, urlSz, - request, requestSz, &response); - - if (ret >= 0 && response) { + if (responseSz >= 0 && response) { XMEMSET(newStatus, 0, sizeof(CertStatus)); - InitOcspResponse(ocspResponse, newStatus, response, ret); - OcspResponseDecode(ocspResponse, ocsp->cm, ocsp->cm->heap); - - if (ocspResponse->responseStatus != OCSP_SUCCESSFUL) + InitOcspResponse(ocspResponse, newStatus, response, responseSz); + if (OcspResponseDecode(ocspResponse, ocsp->cm, ocsp->cm->heap) != 0) { ret = OCSP_LOOKUP_FAIL; + WOLFSSL_MSG("OcspResponseDecode failed"); + } + else if (ocspResponse->responseStatus != OCSP_SUCCESSFUL) { + ret = OCSP_LOOKUP_FAIL; + WOLFSSL_MSG("OcspResponse status bad"); + } else { + ret = OCSP_LOOKUP_FAIL; /* make sure in fail state */ if (CompareOcspReqResp(ocspRequest, ocspResponse) == 0) { if (responseBuffer) { - responseBuffer->buffer = (byte*)XMALLOC(ret, ocsp->cm->heap, - DYNAMIC_TYPE_TMP_BUFFER); + responseBuffer->buffer = (byte*)XMALLOC(responseSz, + ocsp->cm->heap, DYNAMIC_TYPE_TMP_BUFFER); if (responseBuffer->buffer) { - responseBuffer->length = ret; - XMEMCPY(responseBuffer->buffer, response, ret); + responseBuffer->length = responseSz; + XMEMCPY(responseBuffer->buffer, response, responseSz); } } + /* only way to get to good state */ ret = xstat2err(ocspResponse->status->status); if (wc_LockMutex(&ocsp->ocspLock) != 0) From a10d4641268b2ce65e60ba092db45324cf7c4b3f Mon Sep 17 00:00:00 2001 From: toddouska Date: Fri, 27 Jan 2017 17:07:31 -0800 Subject: [PATCH 2/3] fix scan-build warning and simplify CheckOcspRequest validation --- src/ocsp.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/ocsp.c b/src/ocsp.c index 5d5f8f6ab..6a41c34a7 100644 --- a/src/ocsp.c +++ b/src/ocsp.c @@ -244,6 +244,7 @@ static int GetOcspStatus(WOLFSSL_OCSP* ocsp, OcspRequest* request, return ret; } +/* 0 on success */ int CheckOcspRequest(WOLFSSL_OCSP* ocsp, OcspRequest* ocspRequest, buffer* responseBuffer) { @@ -256,6 +257,7 @@ int CheckOcspRequest(WOLFSSL_OCSP* ocsp, OcspRequest* ocspRequest, const char* url = NULL; int urlSz = 0; int ret = -1; + int validated = 0; /* ocsp validation flag */ #ifdef WOLFSSL_SMALL_STACK CertStatus* newStatus; @@ -323,9 +325,6 @@ int CheckOcspRequest(WOLFSSL_OCSP* ocsp, OcspRequest* ocspRequest, if (requestSz > 0 && ocsp->cm->ocspIOCb) { responseSz = ocsp->cm->ocspIOCb(ocsp->cm->ocspIOCtx, url, urlSz, request, requestSz, &response); - if (responseSz < 0) { - ret = responseSz; /* because ret was used for multiple purposes */ - } } if (responseSz >= 0 && response) { @@ -333,15 +332,12 @@ int CheckOcspRequest(WOLFSSL_OCSP* ocsp, OcspRequest* ocspRequest, InitOcspResponse(ocspResponse, newStatus, response, responseSz); if (OcspResponseDecode(ocspResponse, ocsp->cm, ocsp->cm->heap) != 0) { - ret = OCSP_LOOKUP_FAIL; WOLFSSL_MSG("OcspResponseDecode failed"); } else if (ocspResponse->responseStatus != OCSP_SUCCESSFUL) { - ret = OCSP_LOOKUP_FAIL; WOLFSSL_MSG("OcspResponse status bad"); } else { - ret = OCSP_LOOKUP_FAIL; /* make sure in fail state */ if (CompareOcspReqResp(ocspRequest, ocspResponse) == 0) { if (responseBuffer) { responseBuffer->buffer = (byte*)XMALLOC(responseSz, @@ -355,6 +351,9 @@ int CheckOcspRequest(WOLFSSL_OCSP* ocsp, OcspRequest* ocspRequest, /* only way to get to good state */ ret = xstat2err(ocspResponse->status->status); + if (ret == 0) { + validated = 1; + } if (wc_LockMutex(&ocsp->ocspLock) != 0) ret = BAD_MUTEX_E; @@ -396,12 +395,8 @@ int CheckOcspRequest(WOLFSSL_OCSP* ocsp, OcspRequest* ocspRequest, wc_UnLockMutex(&ocsp->ocspLock); } } - else - ret = OCSP_LOOKUP_FAIL; } } - else - ret = OCSP_LOOKUP_FAIL; #ifdef WOLFSSL_SMALL_STACK XFREE(newStatus, NULL, DYNAMIC_TYPE_TMP_BUFFER); @@ -411,6 +406,12 @@ int CheckOcspRequest(WOLFSSL_OCSP* ocsp, OcspRequest* ocspRequest, if (response != NULL && ocsp->cm->ocspRespFreeCb) ocsp->cm->ocspRespFreeCb(ocsp->cm->ocspIOCtx, response); + if (ret == 0 && validated == 1) { + ret = 0; + } else { + ret = OCSP_LOOKUP_FAIL; + } + WOLFSSL_LEAVE("CheckOcspRequest", ret); return ret; } From ea96fa95b300da2112425853d1c67e1d105d6cbc Mon Sep 17 00:00:00 2001 From: toddouska Date: Sat, 28 Jan 2017 11:11:25 -0800 Subject: [PATCH 3/3] add new OCSP response validated debug message and remove redundant ret set --- src/ocsp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ocsp.c b/src/ocsp.c index 6a41c34a7..b87077ea5 100644 --- a/src/ocsp.c +++ b/src/ocsp.c @@ -407,7 +407,7 @@ int CheckOcspRequest(WOLFSSL_OCSP* ocsp, OcspRequest* ocspRequest, ocsp->cm->ocspRespFreeCb(ocsp->cm->ocspIOCtx, response); if (ret == 0 && validated == 1) { - ret = 0; + WOLFSSL_MSG("New OcspResponse validated"); } else { ret = OCSP_LOOKUP_FAIL; }