addressed review comments

lock certOcspRequest to NULL

src/internal.c

@@ -23360,7 +23360,7 @@ int SendFinished(WOLFSSL* ssl)
  *
  * Returns 0 on success
  */
-static int CreateOcspRequest(WOLFSSL* ssl, OcspRequest* request,
+int CreateOcspRequest(WOLFSSL* ssl, OcspRequest* request,
  DecodedCert* cert, byte* certData, word32 length,
  byte *ctxOwnsRequest)
 {

src/tls.c

@@ -3125,7 +3125,7 @@ static void TLSX_CSR_Free(CertificateStatusRequest* csr, void* heap)
  #ifndef WOLFSSL_TLS13
  FreeOcspRequest(&csr->request.ocsp[0]);
  #else
- for(i = 0; i < csr->requests;i++) {
+ for(i = 0;i < csr->requests;i++) {
  FreeOcspRequest(&csr->request.ocsp[i]);
  }
  #endif
@@ -3165,12 +3165,8 @@ word16 TLSX_CSR_GetSize_ex(CertificateStatusRequest* csr, byte isRequest,
 #endif
 #if defined(WOLFSSL_TLS13) && !defined(NO_WOLFSSL_SERVER)
  if (!isRequest && csr->ssl->options.tls1_3) {
- if (csr->responses[idx].length != 0)
- size = (word16)(OPAQUE8_LEN + OPAQUE24_LEN +
- csr->responses[idx].length);
- else
- size = (word16)OPAQUE16_LEN;
- return size;
+ return (word16)(OPAQUE8_LEN + OPAQUE24_LEN +
+ csr->responses[idx].length);;
  }
 #else
  (void)idx;
@@ -3260,6 +3256,10 @@ static int TLSX_CSR_Parse(WOLFSSL* ssl, const byte* input, word16 length,
  word16 size = 0;
 #if defined(WOLFSSL_TLS13)
  DecodedCert* cert;
+ buffer der;
+ word32 pos = 0;
+ DerBuffer* chain;
+ int i = 0;
 #endif
 #endif
 
@@ -3268,6 +3268,7 @@ static int TLSX_CSR_Parse(WOLFSSL* ssl, const byte* input, word16 length,
  OcspRequest* request;
  TLSX* extension;
  CertificateStatusRequest* csr;
+ byte ctxOwnsRequest = 0;
 #endif
 
 #if !defined(NO_WOLFSSL_CLIENT) && defined(WOLFSSL_TLS13) \
@@ -3422,8 +3423,7 @@ static int TLSX_CSR_Parse(WOLFSSL* ssl, const byte* input, word16 length,
 
  #if defined(WOLFSSL_TLS13)
  if (ssl->options.tls1_3) {
- int i = 0;
- (void)i;
+
  if (ssl->buffers.certificate == NULL) {
  WOLFSSL_MSG("Certificate buffer not set!");
  return BUFFER_ERROR;
@@ -3472,48 +3472,49 @@ static int TLSX_CSR_Parse(WOLFSSL* ssl, const byte* input, word16 length,
  if (csr->responses[0].buffer)
  TLSX_SetResponse(ssl, TLSX_STATUS_REQUEST);
 
- {
- buffer der;
- word32 pos = 0;
- DerBuffer* chain;
- /* use certChain if available, otherwise use peer certificate */
- chain = ssl->buffers.certChain;
- if (chain == NULL) {
- chain = ssl->buffers.certificate;
- }
+ /* use certChain if available, otherwise use peer certificate */
+ chain = ssl->buffers.certChain;
+ if (chain == NULL) {
+ chain = ssl->buffers.certificate;
+ }
 
- if (chain && chain->buffer) {
- while (pos + OPAQUE24_LEN < chain->length) {
- c24to32(chain->buffer + pos, &der.length);
- pos += OPAQUE24_LEN;
-
- der.buffer = chain->buffer + pos;
- pos += der.length;
-
- if (pos > chain->length)
- break;
- request = &csr->request.ocsp[i + 1];
- if (ret == 0) {
- ret = CreateOcspRequest(ssl, request, cert,
- der.buffer, der.length);
- if (ret == 0 &&
- request == ssl->ctx->certOcspRequest) {
+ if (chain && chain->buffer) {
+ while (pos + OPAQUE24_LEN < chain->length) {
+ c24to32(chain->buffer + pos, &der.length);
+ pos += OPAQUE24_LEN;
+
+ der.buffer = chain->buffer + pos;
+ pos += der.length;
+
+ if (pos > chain->length)
+ break;
+ request = &csr->request.ocsp[i + 1];
+ if (ret == 0) {
+ ret = CreateOcspRequest(ssl, request, cert,
+ der.buffer, der.length, &ctxOwnsRequest);
+ if (ctxOwnsRequest) {
+ wolfSSL_Mutex* ocspLock =
+ &SSL_CM(ssl)->ocsp_stapling->ocspLock;
+ if (wc_LockMutex(ocspLock) == 0) {
+ /* the request is ours */
  ssl->ctx->certOcspRequest = NULL;
  }
+ wc_UnLockMutex(ocspLock);
  }
- if (ret == 0) {
- request->ssl = ssl;
- ret = CheckOcspRequest(SSL_CM(ssl)->ocsp_stapling,
+ }
+
+ if (ret == 0) {
+ request->ssl = ssl;
+ ret = CheckOcspRequest(SSL_CM(ssl)->ocsp_stapling,
  request, &csr->responses[i + 1], ssl->heap);
- /* Suppressing, not critical */
- if (ret == WC_NO_ERR_TRACE(OCSP_CERT_REVOKED) ||
- ret == WC_NO_ERR_TRACE(OCSP_CERT_UNKNOWN) ||
- ret == WC_NO_ERR_TRACE(OCSP_LOOKUP_FAIL)) {
- ret = 0;
- }
- i++;
- csr->requests++;
+ /* Suppressing, not critical */
+ if (ret == WC_NO_ERR_TRACE(OCSP_CERT_REVOKED) ||
+ ret == WC_NO_ERR_TRACE(OCSP_CERT_UNKNOWN) ||
+ ret == WC_NO_ERR_TRACE(OCSP_LOOKUP_FAIL)) {
+ ret = 0;
  }
+ i++;
+ csr->requests++;
  }
  }
  XFREE(cert, ssl->heap, DYNAMIC_TYPE_DCERT);
@@ -3551,19 +3552,20 @@ int TLSX_CSR_InitRequest(TLSX* extensions, DecodedCert* cert, void* heap)
  XMEMCPY(nonce, csr->request.ocsp[0].nonce, nonceSz);
 
  #if defined(WOLFSSL_TLS13)
- if (req_cnt < 1 + MAX_CHAIN_DEPTH) {
+ if (req_cnt < 1 + MAX_CHAIN_DEPTH)
  #endif
- if ((ret = InitOcspRequest(&csr->request.ocsp[req_cnt],
+ {
+ if ((ret = InitOcspRequest(&csr->request.ocsp[req_cnt],
  cert, 0, heap)) != 0)
- return ret;
+  return ret;
 
- /* restore nonce */
- XMEMCPY(csr->request.ocsp[req_cnt].nonce, nonce, nonceSz);
- csr->request.ocsp[req_cnt].nonceSz = nonceSz;
- #if defined(WOLFSSL_TLS13)
- csr->requests++;
+ /* restore nonce */
+ XMEMCPY(csr->request.ocsp[req_cnt].nonce, nonce, nonceSz);
+ csr->request.ocsp[req_cnt].nonceSz = nonceSz;
+ #if defined(WOLFSSL_TLS13)
+ csr->requests++;
+ #endif
  }
- #endif
  }
  break;
  }

src/tls13.c

@@ -8403,6 +8403,74 @@ static word32 NextCert(byte* data, word32 length, word32* idx)
  return len;
 }
 
+#if defined(HAVE_CERTIFICATE_STATUS_REQUEST)
+/* Write certificate status request into certificate to buffer.
+ *
+ * ssl SSL/TLS object.
+ * extSz word32 array.
+ * Length of the certificate status request data for the certificate.
+ * extSz_num number of the CSR written
+ * extIdx The index number of certificate status request data
+ * for the certificate.
+ * returns Total number of bytes written.
+ */
+static word32 WriteCSRToBuffer(WOLFSSL* ssl, word16* extSz, word16 extSz_num)
+{
+ int ret = 0;
+ TLSX* ext;
+ CertificateStatusRequest* csr;
+ word32 ex_offset = HELLO_EXT_TYPE_SZ + OPAQUE16_LEN /* extesion type */
+ + OPAQUE16_LEN /* extension length */;
+ word32 totalSz = 0;
+ word32 tmpSz;
+ word32 extIdx;
+ DerBuffer* der;
+
+ ext = TLSX_Find(ssl->extensions, TLSX_STATUS_REQUEST);
+ csr = ext ? (CertificateStatusRequest*)ext->data : NULL;
+
+ if (csr) {
+ for (extIdx = 0; extIdx < (word16)(extSz_num); extIdx++) {
+ tmpSz = TLSX_CSR_GetSize_ex(csr, 0, extIdx + 1);
+
+ if (tmpSz > (OPAQUE8_LEN + OPAQUE24_LEN) &&
+ ssl->buffers.certExts[extIdx + 1] == NULL) {
+ /* csr extension is not zero */
+ extSz[extIdx] = tmpSz;
+
+ ret = AllocDer(&ssl->buffers.certExts[extIdx + 1],
+ extSz[extIdx] + ex_offset,
+ CERT_TYPE, ssl->heap);
+ if (ret < 0)
+ return ret;
+ der = ssl->buffers.certExts[extIdx+1];
+
+ /* write extension type */
+ c16toa(ext->type, der->buffer
+ + OPAQUE16_LEN);
+ /* writes extension data length. */
+ c16toa(extSz[extIdx], der->buffer
+ + HELLO_EXT_TYPE_SZ + OPAQUE16_LEN);
+ /* write extension data */
+ extSz[extIdx] = (word16)TLSX_CSR_Write_ex(csr,
+ der->buffer + ex_offset, 0,
+ extIdx + 1);
+ /* add extension offset */
+ extSz[extIdx] += (word16)ex_offset;
+ /* extension length */
+ c16toa(extSz[extIdx] - OPAQUE16_LEN,
+ der->buffer);
+ }
+ totalSz += extSz[extIdx];
+ }
+ }
+ else {
+ /* chain cert empty extension size */
+ totalSz += OPAQUE16_LEN * ssl->buffers.certChainCnt;
+ }
+ return totalSz;
+}
+#endif /* HAVE_CERTIFICATE_STATUS_REQUEST */
 /* Add certificate data and empty extension to output up to the fragment size.
  *
  * ssl SSL/TLS object.
@@ -8459,14 +8527,7 @@ static int SendTls13Certificate(WOLFSSL* ssl)
 {
  int ret = 0;
  word32 certSz, certChainSz, headerSz, listSz, payloadSz;
-#if defined(HAVE_CERTIFICATE_STATUS_REQUEST)
- TLSX* ext;
- CertificateStatusRequest* csr;
- #define MAX_CERT_EXTENSIONS MAX_CHAIN_DEPTH
-#else
- #define MAX_CERT_EXTENSIONS 1
-#endif
- word16 extSz[1 + MAX_CERT_EXTENSIONS] = { OPAQUE16_LEN };
+ word16 extSz[1 + MAX_CERT_EXTENSIONS];
  word16 extIdx = 0;
  word32 length, maxFragment;
  word32 totalextSz = 0;
@@ -8562,51 +8623,12 @@ static int SendTls13Certificate(WOLFSSL* ssl)
 #if defined(HAVE_CERTIFICATE_STATUS_REQUEST)
  if (ssl->options.side == WOLFSSL_SERVER_END &&
  ssl->buffers.certChainCnt > 0) {
- ext = TLSX_Find(ssl->extensions, TLSX_STATUS_REQUEST);
- csr = ext ? (CertificateStatusRequest*)ext->data : NULL;
+ /* write CSR to the buffer */
+ totalextSz += WriteCSRToBuffer(ssl, &extSz[1],
+ ssl->buffers.certChainCnt);
 
- if (csr) {
- word32 ex_offset;
-
- ex_offset = HELLO_EXT_TYPE_SZ + OPAQUE16_LEN /* extesion type */
- + OPAQUE16_LEN /* extension length */;
- for (extIdx = 1; extIdx <(word16)
- (1 + ssl->buffers.certChainCnt); extIdx++) {
-
- extSz[extIdx] = TLSX_CSR_GetSize_ex(csr, 0, extIdx);
- if (extSz[extIdx] > OPAQUE16_LEN &&
- ssl->buffers.certExts[extIdx] == NULL) {
-
- ret = AllocDer(&ssl->buffers.certExts[extIdx],
- extSz[extIdx] + ex_offset,
- CERT_TYPE, ssl->heap);
- if (ret < 0)
- return ret;
-
- /* write extension type */
- c16toa(ext->type, ssl->buffers.certExts[extIdx]->buffer
- + OPAQUE16_LEN);
- /* writes extension data length. */
- c16toa(extSz[extIdx], ssl->buffers.certExts[extIdx]->buffer
- + HELLO_EXT_TYPE_SZ + OPAQUE16_LEN);
- /* write extension data */
- extSz[extIdx] = (word16)TLSX_CSR_Write_ex(csr,
- ssl->buffers.certExts[extIdx]->buffer + ex_offset, 0,
- extIdx);
- /* add extension offset */
- extSz[extIdx] += (word16)ex_offset;
- /* extension length */
- c16toa(extSz[extIdx] - OPAQUE16_LEN,
- ssl->buffers.certExts[extIdx]->buffer);
- }
- totalextSz += extSz[extIdx];
- }
- }
- else {
- /* chain cert empty extension size */
- totalextSz += OPAQUE16_LEN * ssl->buffers.certChainCnt;
- }
- } else if (ssl->options.side == WOLFSSL_CLIENT_END &&
+ }
+ else if (ssl->options.side == WOLFSSL_CLIENT_END &&
  ssl->buffers.certChainCnt > 0)
 #endif
  {

wolfssl/internal.h

@@ -1989,6 +1989,15 @@ enum Misc {
  #define MAX_CHAIN_DEPTH 9
 #endif
 
+/* Max certificate extensions in TLS1.3 */
+#if defined(HAVE_CERTIFICATE_STATUS_REQUEST)
+ /* Number of extensions to set each OCSP response */
+ #define MAX_CERT_EXTENSIONS MAX_CHAIN_DEPTH
+#else
+ /* Only empty extensions */
+ #define MAX_CERT_EXTENSIONS 1
+#endif
+
 /* max size of a certificate message payload */
 /* assumes MAX_CHAIN_DEPTH number of certificates at 2kb per certificate */
 #ifndef MAX_CERTIFICATE_SZ
@@ -3260,7 +3269,8 @@ WOLFSSL_LOCAL int TLSX_CSR_Write_ex(CertificateStatusRequest* csr, byte* output,
 WOLFSSL_LOCAL void* TLSX_CSR_GetRequest_ex(TLSX* extensions, int idx);
 
 WOLFSSL_LOCAL int CreateOcspRequest(WOLFSSL* ssl, OcspRequest* request,
- DecodedCert* cert, byte* certData, word32 length);
+ DecodedCert* cert, byte* certData, word32 length,
+ byte *ctxOwnsRequest);
 #endif
 
 /** Certificate Status Request v2 - RFC 6961 */