addressed review comments

wolfSSL · Aug 7, 2024 · 9918de4 · 9918de4
1 parent e94d5e0
commit 9918de4
Show file tree

Hide file tree

Showing 8 changed files with 125 additions and 120 deletions.
diff --git a/SCRIPTS-LIST b/SCRIPTS-LIST
@@ -35,7 +35,7 @@ scripts/
  google.test   - example client test against google, part of tests
  resume.test   - example sessoin resume test, part of tests
  ocsp-stapling.test - example client test against globalsign, part of tests
- ocsp-stapling1_tls13.text - exmaple client test against example server, part of tests
+ ocsp-stapling1_tls13.text - example client test against example server, part of tests
  ocsp-stapling2.test - example client test against example server, part of tests
  sniffer-testsuite.test - runs snifftest on a pcap of testsuite, part of tests
                           in sniffer mode

diff --git a/scripts/ocsp-stapling2.test b/scripts/ocsp-stapling2.test
@@ -43,7 +43,7 @@ fi
 if ./examples/client/client '-#' | fgrep -q -e ' -DTEST_IPV6 '; then
     if [[ "$IPV6_SUPPORTED" == "no" ]]; then
         echo 'Skipping IPV6 test in environment lacking IPV6 support.'
-        exit 0
+        exit 77
     fi
     LOCALHOST='[::1]'
     LOCALHOST_FOR_NC='-6 ::1'

diff --git a/scripts/ocsp-stapling_tls13multi.test b/scripts/ocsp-stapling_tls13multi.test
@@ -59,7 +59,7 @@ fi
 if ./examples/client/client '-#' | fgrep -q -e ' -DTEST_IPV6 '; then
     if [[ "$IPV6_SUPPORTED" == "no" ]]; then
         echo 'Skipping IPV6 test in environment lacking IPV6 support.'
-        exit 0
+        exit 77
     fi
     LOCALHOST='[::1]'
     LOCALHOST_FOR_NC='-6 ::1'
@@ -74,9 +74,11 @@ PARENTDIR="$PWD"
 # to make this instance orthogonal to any others running, even on same repo.
 # TCP ports are also carefully formed below from the PID, to minimize conflicts.
 
-WORKSPACE="${PARENTDIR}/workspace.pid$$"
+#WORKSPACE="${PARENTDIR}/workspace.pid$$"
+#mkdir "${WORKSPACE}" || exit $?
+
+WORKSPACE="$(mktemp -d -p ${PARENTDIR})"
 
-mkdir "${WORKSPACE}" || exit $?
 cp -pR ${SCRIPT_DIR}/../certs "${WORKSPACE}"/ || exit $?
 cd "$WORKSPACE" || exit $?
 ln -s ../examples
@@ -128,54 +130,54 @@ remove_single_rF(){
         rm $1
     fi
 }
-
 #create a configure file for cert generation with the port 0 solution
 create_new_cnf() {
-    printf '%s\n' "Random Ports Selected: $1 $2 $3 $4"
-
-    printf '%s\n' "#" > $test_cnf
-    printf '%s\n' "# openssl configuration file for OCSP certificates" >> $test_cnf
-    printf '%s\n' "#" >> $test_cnf
-    printf '%s\n' "" >> $test_cnf
-    printf '%s\n' "# Extensions to add to a certificate request (intermediate1-ca)" >> $test_cnf
-    printf '%s\n' "[ v3_req1 ]" >> $test_cnf
-    printf '%s\n' "basicConstraints       = CA:false" >> $test_cnf
-    printf '%s\n' "subjectKeyIdentifier   = hash" >> $test_cnf
-    printf '%s\n' "authorityKeyIdentifier = keyid:always,issuer:always" >> $test_cnf
-    printf '%s\n' "keyUsage               = nonRepudiation, digitalSignature, keyEncipherment" >> $test_cnf
-    printf '%s\n' "authorityInfoAccess    = OCSP;URI:http://127.0.0.1:$1" >> $test_cnf
-    printf '%s\n' "" >> $test_cnf
-    printf '%s\n' "# Extensions to add to a certificate request (intermediate2-ca)" >> $test_cnf
-    printf '%s\n' "[ v3_req2 ]" >> $test_cnf
-    printf '%s\n' "basicConstraints       = CA:false" >> $test_cnf
-    printf '%s\n' "subjectKeyIdentifier   = hash" >> $test_cnf
-    printf '%s\n' "authorityKeyIdentifier = keyid:always,issuer:always" >> $test_cnf
-    printf '%s\n' "keyUsage               = nonRepudiation, digitalSignature, keyEncipherment" >> $test_cnf
-    printf '%s\n' "authorityInfoAccess    = OCSP;URI:http://127.0.0.1:$2" >> $test_cnf
-    printf '%s\n' "" >> $test_cnf
-    printf '%s\n' "# Extensions to add to a certificate request (intermediate3-ca)" >> $test_cnf
-    printf '%s\n' "[ v3_req3 ]" >> $test_cnf
-    printf '%s\n' "basicConstraints       = CA:false" >> $test_cnf
-    printf '%s\n' "subjectKeyIdentifier   = hash" >> $test_cnf
-    printf '%s\n' "authorityKeyIdentifier = keyid:always,issuer:always" >> $test_cnf
-    printf '%s\n' "keyUsage               = nonRepudiation, digitalSignature, keyEncipherment" >> $test_cnf
-    printf '%s\n' "authorityInfoAccess    = OCSP;URI:http://127.0.0.1:$3" >> $test_cnf
-    printf '%s\n' "" >> $test_cnf
-    printf '%s\n' "# Extensions for a typical CA" >> $test_cnf
-    printf '%s\n' "[ v3_ca ]" >> $test_cnf
-    printf '%s\n' "basicConstraints       = CA:true" >> $test_cnf
-    printf '%s\n' "subjectKeyIdentifier   = hash" >> $test_cnf
-    printf '%s\n' "authorityKeyIdentifier = keyid:always,issuer:always" >> $test_cnf
-    printf '%s\n' "keyUsage               = keyCertSign, cRLSign" >> $test_cnf
-    printf '%s\n' "authorityInfoAccess    = OCSP;URI:http://127.0.0.1:$4" >> $test_cnf
-    printf '%s\n' "" >> $test_cnf
-    printf '%s\n' "# OCSP extensions." >> $test_cnf
-    printf '%s\n' "[ v3_ocsp ]" >> $test_cnf
-    printf '%s\n' "basicConstraints       = CA:false" >> $test_cnf
-    printf '%s\n' "subjectKeyIdentifier   = hash" >> $test_cnf
-    printf '%s\n' "authorityKeyIdentifier = keyid:always,issuer:always" >> $test_cnf
-    printf '%s\n' "extendedKeyUsage       = OCSPSigning" >> $test_cnf
-
+    echo "Random Ports Selected: $1 $2 $3 $4"
+
+    cat <<- EOF > $test_cnf
+    #
+    # openssl configuration file for OCSP certificates
+    #
+
+    # Extensions to add to a certificate request (intermediate1-ca)
+    [ v3_req1 ]
+    basicConstraints       = CA:false
+    subjectKeyIdentifier   = hash
+    authorityKeyIdentifier = keyid:always,issuer:always
+    keyUsage               = nonRepudiation, digitalSignature, keyEncipherment
+    authorityInfoAccess    = OCSP;URI:http://127.0.0.1:$1
+
+    # Extensions to add to a certificate request (intermediate2-ca)
+    [ v3_req2 ]
+    basicConstraints       = CA:false
+    subjectKeyIdentifier   = hash
+    authorityKeyIdentifier = keyid:always,issuer:always
+    keyUsage               = nonRepudiation, digitalSignature, keyEncipherment
+    authorityInfoAccess    = OCSP;URI:http://127.0.0.1:$2
+
+    # Extensions to add to a certificate request (intermediate3-ca)
+    [ v3_req3 ]
+    basicConstraints       = CA:false
+    subjectKeyIdentifier   = hash
+    authorityKeyIdentifier = keyid:always,issuer:always
+    keyUsage               = nonRepudiation, digitalSignature, keyEncipherment
+    authorityInfoAccess    = OCSP;URI:http://127.0.0.1:$3
+
+    # Extensions for a typical CA
+    [ v3_ca ]
+    basicConstraints       = CA:true
+    subjectKeyIdentifier   = hash
+    authorityKeyIdentifier = keyid:always,issuer:always
+    keyUsage               = keyCertSign, cRLSign
+    authorityInfoAccess    = OCSP;URI:http://127.0.0.1:$4
+
+    # OCSP extensions.
+    [ v3_ocsp ]
+    basicConstraints       = CA:false
+    subjectKeyIdentifier   = hash
+    authorityKeyIdentifier = keyid:always,issuer:always
+    extendedKeyUsage       = OCSPSigning
+EOF
     mv $test_cnf $CERT_DIR/$test_cnf
     cd $CERT_DIR
     CURR_LOC="$PWD"

diff --git a/src/internal.c b/src/internal.c
@@ -13222,7 +13222,12 @@ static int ProcessCSR_ex(WOLFSSL* ssl, byte* input, word32* inOutIdx,
 {
     int ret = 0;
     OcspRequest* request;
-
+#if defined(HAVE_CERTIFICATE_STATUS_REQUEST)
+    TLSX* ext =  TLSX_Find(ssl->extensions, TLSX_STATUS_REQUEST);
+    CertificateStatusRequest* csr;
+#else
+    (void)idx;
+#endif
     #ifdef WOLFSSL_SMALL_STACK
         CertStatus* status;
         OcspEntry* single;
@@ -13234,7 +13239,14 @@ static int ProcessCSR_ex(WOLFSSL* ssl, byte* input, word32* inOutIdx,
     #endif
 
     WOLFSSL_ENTER("ProcessCSR");
-
+#if defined(HAVE_CERTIFICATE_STATUS_REQUEST)
+    if (ext) {
+        /* status request */
+        csr = (CertificateStatusRequest*)ext->data;
+        if (csr && !csr->ssl)
+            csr->ssl = ssl;
+    }
+#endif
     do {
         #ifdef HAVE_CERTIFICATE_STATUS_REQUEST
             if (ssl->status_request) {
@@ -14381,20 +14393,28 @@ static int ProcessPeerCertCheckKey(WOLFSSL* ssl, ProcPeerCertArgs* args)
 
 #if defined(HAVE_OCSP) && defined(WOLFSSL_TLS13) \
         && defined(HAVE_CERTIFICATE_STATUS_REQUEST)
-static int ProcessPeerCersChainOCSPStausCheck(WOLFSSL* ssl)
+static int ProcessPeerCertsChainOCSPStausCheck(WOLFSSL* ssl)
 {
     int ret = 0;
     word32 i;
     word32 idx = 0;
     TLSX* ext =  TLSX_Find(ssl->extensions, TLSX_STATUS_REQUEST);
     CertificateStatusRequest* csr;
 
-    if (ext == NULL) {
+    if (ext) {
+        csr = (CertificateStatusRequest*)ext->data;
+        if (csr == NULL) {
+            return 0;
+        }
+    } else
         return 0;
-    }
-    csr = (CertificateStatusRequest*)ext->data;
 
-    for (i = 0; i < (csr->requests); i++) {
+    for (i = 0; i < csr->requests; i++) {
+        /* error when leaf cert doesn't have certificate status */
+        if (i ==0 && csr->responses[i].length == 0) {
+            WOLFSSL_MSG("Leaf cert doesn't have certificate status.");
+            return BAD_CERTIFICATE_STATUS_ERROR;
+        }
         if (csr->responses[i].length != 0) {
             ssl->status_request = 1;
             idx = 0;
@@ -14406,6 +14426,9 @@ static int ProcessPeerCersChainOCSPStausCheck(WOLFSSL* ssl)
                 break;
             }
         }
+        else {
+            WOLFSSL_MSG("Intermediate cert doesn't have certificate status.");
+        }
     }
 
     return ret;
@@ -14892,8 +14915,8 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx,
                         }
                         else /* skips OCSP and force CRL check */
                     #endif /* HAVE_CERTIFICATE_STATUS_REQUEST_V2 */
-                    #if defined(WOLFSSL_TLS13) && defined(HAVE_CERTIFICATE_STATUS_REQUEST)
-                        if (ssl->options.tls1_3) {
+                    #if defined(HAVE_CERTIFICATE_STATUS_REQUEST)
+                        if (IsAtLeastTLSv1_3(ssl->version)) {
                             ret = TLSX_CSR_InitRequest(ssl->extensions,
                                     args->dCert, ssl->heap);
                         }
@@ -15386,7 +15409,7 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx,
                             WOLFSSL_MSG("\tHave status request");
                         #if defined(WOLFSSL_TLS13)
                             if (ssl->options.tls1_3) {
-                                ret = ProcessPeerCersChainOCSPStausCheck(ssl);
+                                ret = ProcessPeerCertsChainOCSPStausCheck(ssl);
                                 if (ret < 0) {
                                     WOLFSSL_ERROR_VERBOSE(ret);
                                     goto exit_ppc;

diff --git a/src/ssl.c b/src/ssl.c
@@ -13108,7 +13108,11 @@ size_t wolfSSL_get_client_random(const WOLFSSL* ssl, unsigned char* out,
 #ifdef WOLFSSL_QUIC
         wolfSSL_quic_clear(ssl);
 #endif
-
+#ifdef HAVE_OCSP
+#if defined(WOLFSSL_TLS13) && defined(HAVE_CERTIFICATE_STATUS_REQUEST)
+    ssl->response_idx = 0;
+#endif
+#endif
         return WOLFSSL_SUCCESS;
     }
 

diff --git a/src/tls.c b/src/tls.c
@@ -3117,23 +3117,17 @@ int TLSX_UseTruncatedHMAC(TLSX** extensions, void* heap)
 
 static void TLSX_CSR_Free(CertificateStatusRequest* csr, void* heap)
 {
-#ifdef WOLFSSL_TLS13
     int i;
-#endif
+
     switch (csr->status_type) {
         case WOLFSSL_CSR_OCSP:
-        #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
         break;
     }
-
 #ifdef WOLFSSL_TLS13
-    for (i = 0; i < (1 + MAX_CHAIN_DEPTH); i++) {
+    for (i = 0; i < MAX_CERT_EXTENSIONS; i++) {
         if (csr->responses[i].buffer != NULL) {
             XFREE(csr->responses[i].buffer, heap,
                 DYNAMIC_TYPE_TMP_BUFFER);
@@ -3164,7 +3158,7 @@ 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 (!isRequest && IsAtLeastTLSv1_3(csr->ssl->version)) {
         return (word16)(OPAQUE8_LEN + OPAQUE24_LEN +
                 csr->responses[idx].length);
     }
@@ -3224,7 +3218,7 @@ int TLSX_CSR_Write_ex(CertificateStatusRequest* csr, byte* output,
     }
 #endif
 #if defined(WOLFSSL_TLS13) && !defined(NO_WOLFSSL_SERVER)
-    if (!isRequest && csr->ssl->options.tls1_3) {
+    if (!isRequest && IsAtLeastTLSv1_3(csr->ssl->version)) {
         word16 offset = 0;
         output[offset++] = csr->status_type;
         c32to24(csr->responses[idx].length, output + offset);
@@ -3581,30 +3575,21 @@ int TLSX_CSR_InitRequest(TLSX* extensions, DecodedCert* cert, void* heap)
         switch (csr->status_type) {
             case WOLFSSL_CSR_OCSP: {
                 byte nonce[MAX_OCSP_NONCE_SZ];
-            #if defined(WOLFSSL_TLS13)
                 int  req_cnt = csr->requests;
-            #else
-                int  req_cnt = 0;
-            #endif
                 int  nonceSz = csr->request.ocsp[0].nonceSz;
 
                 /* preserve nonce */
                 XMEMCPY(nonce, csr->request.ocsp[0].nonce, nonceSz);
 
-            #if defined(WOLFSSL_TLS13)
-                if (req_cnt < 1 + MAX_CHAIN_DEPTH)
-            #endif
-                {
+                if (req_cnt < MAX_CERT_EXTENSIONS) {
                     if ((ret = InitOcspRequest(&csr->request.ocsp[req_cnt],
                                     cert, 0, heap)) != 0)
                         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++;
-                #endif
                 }
             }
             break;
@@ -3620,16 +3605,17 @@ void* TLSX_CSR_GetRequest_ex(TLSX* extensions, int idx)
     CertificateStatusRequest* csr = extension ?
                               (CertificateStatusRequest*)extension->data : NULL;
 
-    if (csr) {
+    if (csr && csr->ssl) {
         switch (csr->status_type) {
             case WOLFSSL_CSR_OCSP:
-            #if defined(WOLFSSL_TLS13)
+            if (IsAtLeastTLSv1_3(csr->ssl->version)) {
                 return idx < csr->requests ?
-                    &csr->request.ocsp[csr->requests - idx - 1] : NULL;
-            #else
-                return idx < 1 ?
-                    &csr->request.ocsp[idx] : NULL;
-            #endif
+                   &csr->request.ocsp[csr->requests - idx - 1] : NULL;
+            }
+            else {
+                return idx == 0 ?
+                    &csr->request.ocsp[0] : NULL;
+            }
         }
     }
 
@@ -3671,9 +3657,6 @@ int TLSX_UseCertificateStatusRequest(TLSX** extensions, byte status_type,
 {
     CertificateStatusRequest* csr = NULL;
     int ret = 0;
-#if defined(WOLFSSL_TLS13)
-    int i;
-#endif
 
     if (!extensions || status_type != WOLFSSL_CSR_OCSP)
         return BAD_FUNC_ARG;
@@ -3685,10 +3668,7 @@ int TLSX_UseCertificateStatusRequest(TLSX** extensions, byte status_type,
 
     ForceZero(csr, sizeof(CertificateStatusRequest));
 #if defined(WOLFSSL_TLS13)
-    for(i = 0; i < (1 + MAX_CHAIN_DEPTH); i++) {
-        csr->responses[i].length = 0;
-        csr->responses[i].buffer = NULL;
-    }
+    XMEMSET(csr->responses, 0, sizeof(csr->responses));
 #endif
     csr->status_type = status_type;
     csr->options     = options;