From 5ac3e7d542d5f4542072386d2bdffef750f0b07e Mon Sep 17 00:00:00 2001 From: Tesfa Mael Date: Wed, 14 Oct 2020 01:10:07 -0700 Subject: [PATCH 1/4] Process multiple ocsp responses --- wolfcrypt/src/asn.c | 206 +++++++++++++++++++++++--------------------- 1 file changed, 106 insertions(+), 100 deletions(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 90ae1ba3e..07d04d603 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -8885,6 +8885,7 @@ static int DecodeCertExtensions(DecodedCert* cert) case OCSP_NOCHECK_OID: VERIFY_AND_SET_OID(cert->ocspNoCheckSet); ret = GetASNNull(input, &idx, sz); + length = 0; /* idx is already incremented, reset length to 0 */ if (ret != 0) return ASN_PARSE_E; break; @@ -16546,115 +16547,120 @@ static int DecodeSingleResponse(byte* source, prevIndex = idx; - /* When making a request, we only request one status on one certificate - * at a time. There should only be one SingleResponse */ + /* wolfSSL only requests one status for one certificate at a time but + some OCSP responders can reply with multiple SingleResponse items. + Expect to handle one SingleResponse. Otherwise, we can process the + responses but only the last entry in the list is verified. */ - /* Wrapper around the Single Response */ - if (GetSequence(source, &idx, &length, size) < 0) - return ASN_PARSE_E; + while ((idx-prevIndex) < (word32)wrapperSz) { + /* Wrapper around the Single Response */ + if (GetSequence(source, &idx, &length, size) < 0) + return ASN_PARSE_E; - /* Wrapper around the CertID */ - if (GetSequence(source, &idx, &length, size) < 0) - return ASN_PARSE_E; - /* Skip the hash algorithm */ - if (GetAlgoId(source, &idx, &oid, oidIgnoreType, size) < 0) - return ASN_PARSE_E; - /* Save reference to the hash of CN */ - ret = GetOctetString(source, &idx, &length, size); - if (ret < 0) - return ret; - resp->issuerHash = source + idx; - idx += length; - /* Save reference to the hash of the issuer public key */ - ret = GetOctetString(source, &idx, &length, size); - if (ret < 0) - return ret; - resp->issuerKeyHash = source + idx; - idx += length; + /* Wrapper around the CertID */ + if (GetSequence(source, &idx, &length, size) < 0) + return ASN_PARSE_E; + /* Skip the hash algorithm */ + if (GetAlgoId(source, &idx, &oid, oidIgnoreType, size) < 0) + return ASN_PARSE_E; + /* Save reference to the hash of CN */ + ret = GetOctetString(source, &idx, &length, size); + if (ret < 0) + return ret; + resp->issuerHash = source + idx; + idx += length; + /* Save reference to the hash of the issuer public key */ + ret = GetOctetString(source, &idx, &length, size); + if (ret < 0) + return ret; + resp->issuerKeyHash = source + idx; + idx += length; - /* Get serial number */ - if (GetSerialNumber(source, &idx, cs->serial, &cs->serialSz, size) < 0) - return ASN_PARSE_E; + /* Get serial number */ + if (GetSerialNumber(source, &idx, cs->serial, &cs->serialSz, size) < 0) + return ASN_PARSE_E; - if ( idx >= size ) - return BUFFER_E; + if ( idx >= size ) + return BUFFER_E; - /* CertStatus */ - switch (source[idx++]) - { - case (ASN_CONTEXT_SPECIFIC | CERT_GOOD): - cs->status = CERT_GOOD; + /* CertStatus */ + switch (source[idx++]) + { + case (ASN_CONTEXT_SPECIFIC | CERT_GOOD): + cs->status = CERT_GOOD; + idx++; + break; + case (ASN_CONTEXT_SPECIFIC | ASN_CONSTRUCTED | CERT_REVOKED): + cs->status = CERT_REVOKED; + if (GetLength(source, &idx, &length, size) < 0) + return ASN_PARSE_E; + idx += length; + break; + case (ASN_CONTEXT_SPECIFIC | CERT_UNKNOWN): + cs->status = CERT_UNKNOWN; + idx++; + break; + default: + return ASN_PARSE_E; + } + + #if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) || defined(WOLFSSL_HAPROXY) + cs->thisDateAsn = source + idx; + localIdx = 0; + if (GetDateInfo(cs->thisDateAsn, &localIdx, NULL, + (byte*)&cs->thisDateParsed.type, + &cs->thisDateParsed.length, size) < 0) + return ASN_PARSE_E; + XMEMCPY(cs->thisDateParsed.data, + cs->thisDateAsn + localIdx - cs->thisDateParsed.length, + cs->thisDateParsed.length); + #endif + if (GetBasicDate(source, &idx, cs->thisDate, + &cs->thisDateFormat, size) < 0) + return ASN_PARSE_E; + + #ifndef NO_ASN_TIME + #ifndef WOLFSSL_NO_OCSP_DATE_CHECK + if (!XVALIDATE_DATE(cs->thisDate, cs->thisDateFormat, BEFORE)) + return ASN_BEFORE_DATE_E; + #endif + #endif + + + /* The following items are optional. Only check for them if there is more + * unprocessed data in the singleResponse wrapper. */ + + localIdx = idx; + if (((int)(idx - prevIndex) < wrapperSz) && + GetASNTag(source, &localIdx, &tag, size) == 0 && + tag == (ASN_CONSTRUCTED | ASN_CONTEXT_SPECIFIC | 0)) + { idx++; - break; - case (ASN_CONTEXT_SPECIFIC | ASN_CONSTRUCTED | CERT_REVOKED): - cs->status = CERT_REVOKED; if (GetLength(source, &idx, &length, size) < 0) return ASN_PARSE_E; - idx += length; - break; - case (ASN_CONTEXT_SPECIFIC | CERT_UNKNOWN): - cs->status = CERT_UNKNOWN; - idx++; - break; - default: - return ASN_PARSE_E; - } + #if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) || defined(WOLFSSL_HAPROXY) + cs->nextDateAsn = source + idx; + localIdx = 0; + if (GetDateInfo(cs->nextDateAsn, &localIdx, NULL, + (byte*)&cs->nextDateParsed.type, + &cs->nextDateParsed.length, size) < 0) + return ASN_PARSE_E; + XMEMCPY(cs->nextDateParsed.data, + cs->nextDateAsn + localIdx - cs->nextDateParsed.length, + cs->nextDateParsed.length); + #endif + if (GetBasicDate(source, &idx, cs->nextDate, + &cs->nextDateFormat, size) < 0) + return ASN_PARSE_E; -#if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) || defined(WOLFSSL_HAPROXY) - cs->thisDateAsn = source + idx; - localIdx = 0; - if (GetDateInfo(cs->thisDateAsn, &localIdx, NULL, - (byte*)&cs->thisDateParsed.type, - &cs->thisDateParsed.length, size) < 0) - return ASN_PARSE_E; - XMEMCPY(cs->thisDateParsed.data, - cs->thisDateAsn + localIdx - cs->thisDateParsed.length, - cs->thisDateParsed.length); -#endif - if (GetBasicDate(source, &idx, cs->thisDate, - &cs->thisDateFormat, size) < 0) - return ASN_PARSE_E; - -#ifndef NO_ASN_TIME -#ifndef WOLFSSL_NO_OCSP_DATE_CHECK - if (!XVALIDATE_DATE(cs->thisDate, cs->thisDateFormat, BEFORE)) - return ASN_BEFORE_DATE_E; -#endif -#endif - - /* The following items are optional. Only check for them if there is more - * unprocessed data in the singleResponse wrapper. */ - - localIdx = idx; - if (((int)(idx - prevIndex) < wrapperSz) && - GetASNTag(source, &localIdx, &tag, size) == 0 && - tag == (ASN_CONSTRUCTED | ASN_CONTEXT_SPECIFIC | 0)) - { - idx++; - if (GetLength(source, &idx, &length, size) < 0) - return ASN_PARSE_E; -#if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) || defined(WOLFSSL_HAPROXY) - cs->nextDateAsn = source + idx; - localIdx = 0; - if (GetDateInfo(cs->nextDateAsn, &localIdx, NULL, - (byte*)&cs->nextDateParsed.type, - &cs->nextDateParsed.length, size) < 0) - return ASN_PARSE_E; - XMEMCPY(cs->nextDateParsed.data, - cs->nextDateAsn + localIdx - cs->nextDateParsed.length, - cs->nextDateParsed.length); -#endif - if (GetBasicDate(source, &idx, cs->nextDate, - &cs->nextDateFormat, size) < 0) - return ASN_PARSE_E; - -#ifndef NO_ASN_TIME -#ifndef WOLFSSL_NO_OCSP_DATE_CHECK - if (!XVALIDATE_DATE(cs->nextDate, cs->nextDateFormat, AFTER)) - return ASN_AFTER_DATE_E; -#endif -#endif - } + #ifndef NO_ASN_TIME + #ifndef WOLFSSL_NO_OCSP_DATE_CHECK + if (!XVALIDATE_DATE(cs->nextDate, cs->nextDateFormat, AFTER)) + return ASN_AFTER_DATE_E; + #endif + #endif + } + } /* while, process multiple SingleResponse items */ localIdx = idx; if (((int)(idx - prevIndex) < wrapperSz) && From 10b1884993d49d9e932a0a6437d9f29ab669112d Mon Sep 17 00:00:00 2001 From: David Garske Date: Wed, 14 Oct 2020 14:37:09 -0700 Subject: [PATCH 2/4] Added support for handling an OCSP response with multiple status responses. --- src/internal.c | 6 +- src/ocsp.c | 3 +- wolfcrypt/src/asn.c | 271 +++++++++++++++++++++------------------- wolfssl/wolfcrypt/asn.h | 8 +- 4 files changed, 157 insertions(+), 131 deletions(-) diff --git a/src/internal.c b/src/internal.c index 8a5cc24e0..bf4e6aa5e 100644 --- a/src/internal.c +++ b/src/internal.c @@ -9844,7 +9844,7 @@ static int ProcessCSR(WOLFSSL* ssl, byte* input, word32* inOutIdx, } #endif - InitOcspResponse(response, status, input +*inOutIdx, status_length); + InitOcspResponse(response, status, input +*inOutIdx, status_length, ssl->heap); if (OcspResponseDecode(response, ssl->ctx->cm, ssl->heap, 0) != 0) ret = BAD_CERTIFICATE_STATUS_ERROR; @@ -9864,6 +9864,7 @@ static int ProcessCSR(WOLFSSL* ssl, byte* input, word32* inOutIdx, *inOutIdx += status_length; + FreeOcspResponse(response); #ifdef WOLFSSL_SMALL_STACK XFREE(status, ssl->heap, DYNAMIC_TYPE_OCSP_STATUS); XFREE(response, ssl->heap, DYNAMIC_TYPE_OCSP_REQUEST); @@ -11835,7 +11836,7 @@ static int DoCertificateStatus(WOLFSSL* ssl, byte* input, word32* inOutIdx, if (status_length) { InitOcspResponse(response, status, input +*inOutIdx, - status_length); + status_length, ssl->heap); if ((OcspResponseDecode(response, ssl->ctx->cm, ssl->heap, 0) != 0) @@ -11854,6 +11855,7 @@ static int DoCertificateStatus(WOLFSSL* ssl, byte* input, word32* inOutIdx, else if (idx == 1) /* server cert must be OK */ ret = BAD_CERTIFICATE_STATUS_ERROR; } + FreeOcspResponse(response); *inOutIdx += status_length; list_length -= status_length; diff --git a/src/ocsp.c b/src/ocsp.c index d4002c357..856b2d81c 100644 --- a/src/ocsp.c +++ b/src/ocsp.c @@ -296,7 +296,7 @@ WOLFSSL_LOCAL int CheckOcspResponse(WOLFSSL_OCSP *ocsp, byte *response, int resp #endif XMEMSET(newStatus, 0, sizeof(CertStatus)); - InitOcspResponse(ocspResponse, newStatus, response, responseSz); + InitOcspResponse(ocspResponse, newStatus, response, responseSz, ocsp->cm->heap); ret = OcspResponseDecode(ocspResponse, ocsp->cm, ocsp->cm->heap, 0); if (ret != 0) { ocsp->error = ret; @@ -378,6 +378,7 @@ end: ret = OCSP_LOOKUP_FAIL; } + FreeOcspResponse(ocspResponse); #ifdef WOLFSSL_SMALL_STACK XFREE(newStatus, NULL, DYNAMIC_TYPE_TMP_BUFFER); XFREE(ocspResponse, NULL, DYNAMIC_TYPE_TMP_BUFFER); diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 07d04d603..78f1c18db 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -16531,146 +16531,119 @@ static int GetEnumerated(const byte* input, word32* inOutIdx, int *value, static int DecodeSingleResponse(byte* source, - word32* ioIndex, OcspResponse* resp, word32 size) + word32* ioIndex, OcspResponse* resp, word32 size, CertStatus* cs) { - word32 idx = *ioIndex, prevIndex, oid, localIdx; - int length, wrapperSz; - CertStatus* cs = resp->status; + word32 idx = *ioIndex, oid, localIdx; + int length; int ret; byte tag; WOLFSSL_ENTER("DecodeSingleResponse"); - /* Outer wrapper of the SEQUENCE OF Single Responses. */ - if (GetSequence(source, &idx, &wrapperSz, size) < 0) + /* Wrapper around the Single Response */ + if (GetSequence(source, &idx, &length, size) < 0) return ASN_PARSE_E; - prevIndex = idx; + /* Wrapper around the CertID */ + if (GetSequence(source, &idx, &length, size) < 0) + return ASN_PARSE_E; + /* Skip the hash algorithm */ + if (GetAlgoId(source, &idx, &oid, oidIgnoreType, size) < 0) + return ASN_PARSE_E; + /* Save reference to the hash of CN */ + ret = GetOctetString(source, &idx, &length, size); + if (ret < 0) + return ret; + resp->issuerHash = source + idx; + idx += length; + /* Save reference to the hash of the issuer public key */ + ret = GetOctetString(source, &idx, &length, size); + if (ret < 0) + return ret; + resp->issuerKeyHash = source + idx; + idx += length; - /* wolfSSL only requests one status for one certificate at a time but - some OCSP responders can reply with multiple SingleResponse items. - Expect to handle one SingleResponse. Otherwise, we can process the - responses but only the last entry in the list is verified. */ + /* Get serial number */ + if (GetSerialNumber(source, &idx, cs->serial, &cs->serialSz, size) < 0) + return ASN_PARSE_E; - while ((idx-prevIndex) < (word32)wrapperSz) { - /* Wrapper around the Single Response */ - if (GetSequence(source, &idx, &length, size) < 0) - return ASN_PARSE_E; + if ( idx >= size ) + return BUFFER_E; - /* Wrapper around the CertID */ - if (GetSequence(source, &idx, &length, size) < 0) - return ASN_PARSE_E; - /* Skip the hash algorithm */ - if (GetAlgoId(source, &idx, &oid, oidIgnoreType, size) < 0) - return ASN_PARSE_E; - /* Save reference to the hash of CN */ - ret = GetOctetString(source, &idx, &length, size); - if (ret < 0) - return ret; - resp->issuerHash = source + idx; - idx += length; - /* Save reference to the hash of the issuer public key */ - ret = GetOctetString(source, &idx, &length, size); - if (ret < 0) - return ret; - resp->issuerKeyHash = source + idx; - idx += length; - - /* Get serial number */ - if (GetSerialNumber(source, &idx, cs->serial, &cs->serialSz, size) < 0) - return ASN_PARSE_E; - - if ( idx >= size ) - return BUFFER_E; - - /* CertStatus */ - switch (source[idx++]) - { - case (ASN_CONTEXT_SPECIFIC | CERT_GOOD): - cs->status = CERT_GOOD; - idx++; - break; - case (ASN_CONTEXT_SPECIFIC | ASN_CONSTRUCTED | CERT_REVOKED): - cs->status = CERT_REVOKED; - if (GetLength(source, &idx, &length, size) < 0) - return ASN_PARSE_E; - idx += length; - break; - case (ASN_CONTEXT_SPECIFIC | CERT_UNKNOWN): - cs->status = CERT_UNKNOWN; - idx++; - break; - default: - return ASN_PARSE_E; - } - - #if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) || defined(WOLFSSL_HAPROXY) - cs->thisDateAsn = source + idx; - localIdx = 0; - if (GetDateInfo(cs->thisDateAsn, &localIdx, NULL, - (byte*)&cs->thisDateParsed.type, - &cs->thisDateParsed.length, size) < 0) - return ASN_PARSE_E; - XMEMCPY(cs->thisDateParsed.data, - cs->thisDateAsn + localIdx - cs->thisDateParsed.length, - cs->thisDateParsed.length); - #endif - if (GetBasicDate(source, &idx, cs->thisDate, - &cs->thisDateFormat, size) < 0) - return ASN_PARSE_E; - - #ifndef NO_ASN_TIME - #ifndef WOLFSSL_NO_OCSP_DATE_CHECK - if (!XVALIDATE_DATE(cs->thisDate, cs->thisDateFormat, BEFORE)) - return ASN_BEFORE_DATE_E; - #endif - #endif - - - /* The following items are optional. Only check for them if there is more - * unprocessed data in the singleResponse wrapper. */ - - localIdx = idx; - if (((int)(idx - prevIndex) < wrapperSz) && - GetASNTag(source, &localIdx, &tag, size) == 0 && - tag == (ASN_CONSTRUCTED | ASN_CONTEXT_SPECIFIC | 0)) - { + /* CertStatus */ + switch (source[idx++]) + { + case (ASN_CONTEXT_SPECIFIC | CERT_GOOD): + cs->status = CERT_GOOD; idx++; + break; + case (ASN_CONTEXT_SPECIFIC | ASN_CONSTRUCTED | CERT_REVOKED): + cs->status = CERT_REVOKED; if (GetLength(source, &idx, &length, size) < 0) return ASN_PARSE_E; - #if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) || defined(WOLFSSL_HAPROXY) - cs->nextDateAsn = source + idx; - localIdx = 0; - if (GetDateInfo(cs->nextDateAsn, &localIdx, NULL, - (byte*)&cs->nextDateParsed.type, - &cs->nextDateParsed.length, size) < 0) - return ASN_PARSE_E; - XMEMCPY(cs->nextDateParsed.data, - cs->nextDateAsn + localIdx - cs->nextDateParsed.length, - cs->nextDateParsed.length); - #endif - if (GetBasicDate(source, &idx, cs->nextDate, - &cs->nextDateFormat, size) < 0) - return ASN_PARSE_E; + idx += length; + break; + case (ASN_CONTEXT_SPECIFIC | CERT_UNKNOWN): + cs->status = CERT_UNKNOWN; + idx++; + break; + default: + return ASN_PARSE_E; + } - #ifndef NO_ASN_TIME - #ifndef WOLFSSL_NO_OCSP_DATE_CHECK - if (!XVALIDATE_DATE(cs->nextDate, cs->nextDateFormat, AFTER)) - return ASN_AFTER_DATE_E; - #endif - #endif - } - } /* while, process multiple SingleResponse items */ +#if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) || defined(WOLFSSL_HAPROXY) + cs->thisDateAsn = source + idx; + localIdx = 0; + if (GetDateInfo(cs->thisDateAsn, &localIdx, NULL, + (byte*)&cs->thisDateParsed.type, + &cs->thisDateParsed.length, size) < 0) + return ASN_PARSE_E; + XMEMCPY(cs->thisDateParsed.data, + cs->thisDateAsn + localIdx - cs->thisDateParsed.length, + cs->thisDateParsed.length); +#endif + if (GetBasicDate(source, &idx, cs->thisDate, + &cs->thisDateFormat, size) < 0) + return ASN_PARSE_E; +#ifndef NO_ASN_TIME +#ifndef WOLFSSL_NO_OCSP_DATE_CHECK + if (!XVALIDATE_DATE(cs->thisDate, cs->thisDateFormat, BEFORE)) + return ASN_BEFORE_DATE_E; +#endif +#endif + + /* The following items are optional. Only check for them if there is more + * unprocessed data in the singleResponse wrapper. */ localIdx = idx; - if (((int)(idx - prevIndex) < wrapperSz) && + if (idx < size && GetASNTag(source, &localIdx, &tag, size) == 0 && - tag == (ASN_CONSTRUCTED | ASN_CONTEXT_SPECIFIC | 1)) + tag == (ASN_CONSTRUCTED | ASN_CONTEXT_SPECIFIC | 0)) { idx++; if (GetLength(source, &idx, &length, size) < 0) return ASN_PARSE_E; - idx += length; +#if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) || defined(WOLFSSL_HAPROXY) + cs->nextDateAsn = source + idx; + localIdx = 0; + if (GetDateInfo(cs->nextDateAsn, &localIdx, NULL, + (byte*)&cs->nextDateParsed.type, + &cs->nextDateParsed.length, size) < 0) + return ASN_PARSE_E; + XMEMCPY(cs->nextDateParsed.data, + cs->nextDateAsn + localIdx - cs->nextDateParsed.length, + cs->nextDateParsed.length); +#endif + if (GetBasicDate(source, &idx, cs->nextDate, + &cs->nextDateFormat, size) < 0) + return ASN_PARSE_E; + +#ifndef NO_ASN_TIME +#ifndef WOLFSSL_NO_OCSP_DATE_CHECK + if (!XVALIDATE_DATE(cs->nextDate, cs->nextDateFormat, AFTER)) + return ASN_AFTER_DATE_E; +#endif +#endif } *ioIndex = idx; @@ -16765,6 +16738,8 @@ static int DecodeResponseData(byte* source, int version; int ret; byte tag; + int wrapperSz; + CertStatus* cs; WOLFSSL_ENTER("DecodeResponseData"); @@ -16806,8 +16781,26 @@ static int DecodeResponseData(byte* source, &resp->producedDateFormat, size) < 0) return ASN_PARSE_E; - if ((ret = DecodeSingleResponse(source, &idx, resp, size)) < 0) - return ret; /* ASN_PARSE_E, ASN_BEFORE_DATE_E, ASN_AFTER_DATE_E */ + /* Outer wrapper of the SEQUENCE OF Single Responses. */ + if (GetSequence(source, &idx, &wrapperSz, size) < 0) + return ASN_PARSE_E; + + localIdx = idx; + cs = resp->status; + while (idx - localIdx < (word32)wrapperSz) { + if ((ret = DecodeSingleResponse(source, &idx, resp, localIdx + wrapperSz, cs)) < 0) + return ret; /* ASN_PARSE_E, ASN_BEFORE_DATE_E, ASN_AFTER_DATE_E */ + if (idx - localIdx < (word32)wrapperSz) { + cs->next = (CertStatus*)XMALLOC(sizeof(CertStatus), resp->heap, + DYNAMIC_TYPE_OCSP_STATUS); + if (cs->next == NULL) { + return MEMORY_E; + } + cs = cs->next; + XMEMSET(cs, 0, sizeof(CertStatus)); + cs->isDynamic = 1; + } + } /* * Check the length of the ResponseData against the current index to @@ -16980,7 +16973,7 @@ static int DecodeBasicOcspResponse(byte* source, word32* ioIndex, void InitOcspResponse(OcspResponse* resp, CertStatus* status, - byte* source, word32 inSz) + byte* source, word32 inSz, void* heap) { WOLFSSL_ENTER("InitOcspResponse"); @@ -16991,6 +16984,17 @@ void InitOcspResponse(OcspResponse* resp, CertStatus* status, resp->status = status; resp->source = source; resp->maxIdx = inSz; + resp->heap = heap; +} + +void FreeOcspResponse(OcspResponse* resp) +{ + CertStatus *status, *next; + for (status = resp->status; status; status = next) { + next = status->next; + if (status->isDynamic) + XFREE(status, resp->heap, DYNAMIC_TYPE_OCSP_STATUS); + } } @@ -17265,6 +17269,7 @@ void FreeOcspRequest(OcspRequest* req) int CompareOcspReqResp(OcspRequest* req, OcspResponse* resp) { int cmp; + CertStatus *status, *next, *prev, *top; WOLFSSL_ENTER("CompareOcspReqResp"); @@ -17310,13 +17315,27 @@ int CompareOcspReqResp(OcspRequest* req, OcspResponse* resp) return cmp; } - cmp = req->serialSz - resp->status->serialSz; - if (cmp != 0) { - WOLFSSL_MSG("\tserialSz mismatch"); - return cmp; + /* match based on found status and return */ + for (status = resp->status; status; status = next) { + cmp = req->serialSz - status->serialSz; + if (cmp == 0) { + cmp = XMEMCMP(req->serial, status->serial, req->serialSz); + if (cmp == 0) { + /* match found */ + if (resp->status != status && prev) { + /* move to top of list */ + top = resp->status; + resp->status = status; + prev->next = status->next; + status->next = top; + } + break; + } + } + next = status->next; + prev = status; } - cmp = XMEMCMP(req->serial, resp->status->serial, req->serialSz); if (cmp != 0) { WOLFSSL_MSG("\tserial mismatch"); return cmp; diff --git a/wolfssl/wolfcrypt/asn.h b/wolfssl/wolfcrypt/asn.h index e502e59aa..f02ef3af9 100644 --- a/wolfssl/wolfcrypt/asn.h +++ b/wolfssl/wolfcrypt/asn.h @@ -1267,8 +1267,10 @@ struct CertStatus { byte* rawOcspResponse; word32 rawOcspResponseSz; -}; + /* option bits - using 32-bit for alignment */ + word32 isDynamic:1; /* was allocated cert status */ +}; struct OcspResponse { int responseStatus; /* return code from Responder */ @@ -1300,6 +1302,7 @@ struct OcspResponse { #ifdef OPENSSL_EXTRA int verifyError; #endif + void* heap; }; @@ -1337,7 +1340,8 @@ struct OcspEntry int totalStatus; /* number on list */ }; -WOLFSSL_LOCAL void InitOcspResponse(OcspResponse*, CertStatus*, byte*, word32); +WOLFSSL_LOCAL void InitOcspResponse(OcspResponse*, CertStatus*, byte*, word32, void*); +WOLFSSL_LOCAL void FreeOcspResponse(OcspResponse*); WOLFSSL_LOCAL int OcspResponseDecode(OcspResponse*, void*, void* heap, int); WOLFSSL_LOCAL int InitOcspRequest(OcspRequest*, DecodedCert*, byte, void*); From 751f64b4aa6dc1b722abd287ccd56dfc69c618aa Mon Sep 17 00:00:00 2001 From: David Garske Date: Wed, 14 Oct 2020 14:55:18 -0700 Subject: [PATCH 3/4] Fix for OCSP single response last optional part handling and restore original size arg since its required for the ASN elements. --- wolfcrypt/src/asn.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 78f1c18db..83c51486c 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -16531,15 +16531,18 @@ static int GetEnumerated(const byte* input, word32* inOutIdx, int *value, static int DecodeSingleResponse(byte* source, - word32* ioIndex, OcspResponse* resp, word32 size, CertStatus* cs) + word32* ioIndex, OcspResponse* resp, word32 size, int wrapperSz, + CertStatus* cs) { - word32 idx = *ioIndex, oid, localIdx; + word32 idx = *ioIndex, prevIndex, oid, localIdx; int length; int ret; byte tag; WOLFSSL_ENTER("DecodeSingleResponse"); + prevIndex = idx; + /* Wrapper around the Single Response */ if (GetSequence(source, &idx, &length, size) < 0) return ASN_PARSE_E; @@ -16616,7 +16619,7 @@ static int DecodeSingleResponse(byte* source, /* The following items are optional. Only check for them if there is more * unprocessed data in the singleResponse wrapper. */ localIdx = idx; - if (idx < size && + if (((int)(idx - prevIndex) < wrapperSz) && GetASNTag(source, &localIdx, &tag, size) == 0 && tag == (ASN_CONSTRUCTED | ASN_CONTEXT_SPECIFIC | 0)) { @@ -16788,7 +16791,8 @@ static int DecodeResponseData(byte* source, localIdx = idx; cs = resp->status; while (idx - localIdx < (word32)wrapperSz) { - if ((ret = DecodeSingleResponse(source, &idx, resp, localIdx + wrapperSz, cs)) < 0) + ret = DecodeSingleResponse(source, &idx, resp, size, wrapperSz, cs); + if (ret < 0) return ret; /* ASN_PARSE_E, ASN_BEFORE_DATE_E, ASN_AFTER_DATE_E */ if (idx - localIdx < (word32)wrapperSz) { cs->next = (CertStatus*)XMALLOC(sizeof(CertStatus), resp->heap, From b18d43abb9b847ce28bd647cfcbdd37c5771bee1 Mon Sep 17 00:00:00 2001 From: David Garske Date: Wed, 14 Oct 2020 15:52:51 -0700 Subject: [PATCH 4/4] Fix for possible uninitialized use of `prev`. --- wolfcrypt/src/asn.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 83c51486c..0a7243847 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -17273,7 +17273,7 @@ void FreeOcspRequest(OcspRequest* req) int CompareOcspReqResp(OcspRequest* req, OcspResponse* resp) { int cmp; - CertStatus *status, *next, *prev, *top; + CertStatus *status, *next, *prev = NULL, *top; WOLFSSL_ENTER("CompareOcspReqResp");