Browse Source

Fix to catch the issue in this PR with alt cert chains, which only happens if the verify callback is used and the chain is long enough. Cleanup of the myVerify callback to allow specific actions. Fix the suites.c to not crash if no newline at end of file. Added helpful debug message to show that a CA was found.

David Garske 3 years ago
parent
commit
667d9ca896
7 changed files with 166 additions and 75 deletions
  1. 15 10
      examples/client/client.c
  2. 7 5
      examples/server/server.c
  3. 4 4
      tests/api.c
  4. 2 2
      tests/suites.c
  5. 119 31
      tests/test-altchains.conf
  6. 3 0
      wolfcrypt/src/asn.c
  7. 16 23
      wolfssl/test.h

+ 15 - 10
examples/client/client.c

@@ -1444,7 +1444,6 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args)
     int    pkCallbacks   = 0;
     PkCbInfo pkCbInfo;
 #endif
-    int    overrideDateErrors = 0;
     int    minDhKeyBits  = DEFAULT_MIN_DHKEY_BITS;
     char*  alpnList = NULL;
     unsigned char alpn_opt = 0;
@@ -1575,7 +1574,6 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args)
     (void)ourCert;
     (void)verifyCert;
     (void)useClientCert;
-    (void)overrideDateErrors;
     (void)disableCRL;
     (void)minDhKeyBits;
     (void)alpnList;
@@ -1621,7 +1619,7 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args)
                 XEXIT_T(EXIT_SUCCESS);
 
             case 'D' :
-                overrideDateErrors = 1;
+                myVerifyAction = VERIFY_OVERRIDE_DATE_ERR;
                 break;
 
             case 'C' :
@@ -1733,7 +1731,11 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args)
                 }
                 else if (XSTRNCMP(myoptarg, "verifyFail", 10) == 0) {
                     printf("Verify should fail\n");
-                    myVerifyFail = 1;
+                    myVerifyAction = VERIFY_FORCE_FAIL;
+                }
+                else if (XSTRNCMP(myoptarg, "verifyInfo", 10) == 0) {
+                    printf("Verify should not override error\n");
+                    myVerifyAction = VERIFY_USE_PREVERFIY;
                 }
                 else if (XSTRNCMP(myoptarg, "useSupCurve", 11) == 0) {
                     printf("Attempting to test use supported curve\n");
@@ -2445,7 +2447,7 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args)
     #endif
     }
 
-    if (!usePsk && !useAnon && !useVerifyCb && !myVerifyFail) {
+    if (!usePsk && !useAnon && !useVerifyCb && myVerifyAction != VERIFY_FORCE_FAIL) {
     #ifndef TEST_LOAD_BUFFER
         unsigned int verify_flags = 0;
     #ifdef TEST_BEFORE_DATE
@@ -2482,12 +2484,16 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args)
         }
     #endif /* WOLFSSL_TRUST_PEER_CERT && !NO_FILESYSTEM */
     }
-    if (useVerifyCb || myVerifyFail)
+    if (useVerifyCb || myVerifyAction == VERIFY_OVERRIDE_ERROR || 
+            myVerifyAction == VERIFY_USE_PREVERFIY) {
         wolfSSL_CTX_set_verify(ctx, WOLFSSL_VERIFY_PEER, myVerify);
-    else if (!usePsk && !useAnon && doPeerCheck == 0)
+    }
+    else if (!usePsk && !useAnon && doPeerCheck == 0) {
         wolfSSL_CTX_set_verify(ctx, WOLFSSL_VERIFY_NONE, 0);
-    else if (!usePsk && !useAnon && overrideDateErrors == 1)
-        wolfSSL_CTX_set_verify(ctx, WOLFSSL_VERIFY_PEER, myDateCb);
+    }
+    else if (!usePsk && !useAnon && myVerifyAction == VERIFY_OVERRIDE_DATE_ERR) {
+        wolfSSL_CTX_set_verify(ctx, WOLFSSL_VERIFY_PEER, myVerify);
+    }
 #endif /* !NO_CERTS */
 
 #ifdef WOLFSSL_ASYNC_CRYPT
@@ -3452,7 +3458,6 @@ exit:
     /* There are use cases  when these assignments are not read. To avoid
      * potential confusion those warnings have been handled here.
      */
-    (void) overrideDateErrors;
     (void) useClientCert;
     (void) verifyCert;
     (void) ourCert;

+ 7 - 5
examples/server/server.c

@@ -957,7 +957,6 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args)
     unsigned char alpn_opt = 0;
     char*  cipherList = NULL;
     int    useDefCipherList = 0;
-    int    overrideDateErrors = 0;
     const char* verifyCert;
     const char* ourCert;
     const char* ourKey;
@@ -1089,7 +1088,6 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args)
     (void)postHandAuth;
     (void)mcastID;
     (void)loadCertKeyIntoSSLObj;
-    (void)overrideDateErrors;
     (void)nonBlocking;
 
 #ifdef WOLFSSL_TIRTOS
@@ -1221,7 +1219,11 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args)
                 }
                 else if (XSTRNCMP(myoptarg, "verifyFail", 10) == 0) {
                     printf("Verify should fail\n");
-                    myVerifyFail = 1;
+                    myVerifyAction = VERIFY_FORCE_FAIL;
+                }
+                else if (XSTRNCMP(myoptarg, "verifyInfo", 10) == 0) {
+                    printf("Verify should use preverify (just show info)\n");
+                    myVerifyAction = VERIFY_USE_PREVERFIY;
                 }
                 else if (XSTRNCMP(myoptarg, "loadSSL", 7) == 0) {
                     printf("Also load cert/key into wolfSSL object\n");
@@ -1243,7 +1245,7 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args)
                 }
                 else if (XSTRNCMP(myoptarg, "overrideDateErr", 15) == 0) {
                 #if !defined(NO_FILESYSTEM) && !defined(NO_CERTS)
-                    overrideDateErrors = 1;
+                    myVerifyAction = VERIFY_OVERRIDE_DATE_ERR;
                 #endif
                 }
                 else {
@@ -1805,7 +1807,7 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args)
         SSL_CTX_set_verify(ctx, WOLFSSL_VERIFY_PEER |
                             (usePskPlus ? WOLFSSL_VERIFY_FAIL_EXCEPT_PSK :
                                 WOLFSSL_VERIFY_FAIL_IF_NO_PEER_CERT),
-                            overrideDateErrors == 1 ? myDateCb : NULL);
+                  myVerifyAction == VERIFY_OVERRIDE_DATE_ERR ? myVerify : NULL);
 
     #ifdef TEST_BEFORE_DATE
         verify_flags |= WOLFSSL_LOAD_FLAG_DATE_ERR_OKAY;

+ 4 - 4
tests/api.c

@@ -1172,7 +1172,7 @@ static int test_wolfSSL_CertManagerSetVerify(void)
 #if !defined(NO_FILESYSTEM) && !defined(NO_CERTS) && \
     !defined(NO_WOLFSSL_CM_VERIFY) && !defined(NO_RSA)
     WOLFSSL_CERT_MANAGER* cm;
-    int tmp = myVerifyFail;
+    int tmp = myVerifyAction;
     const char* ca_cert = "./certs/ca-cert.pem";
     const char* expiredCert = "./certs/test/expired/expired-cert.pem";
 
@@ -1185,7 +1185,7 @@ static int test_wolfSSL_CertManagerSetVerify(void)
     AssertIntEQ(ret, WOLFSSL_SUCCESS);
 
     /* Use the test CB that always accepts certs */
-    myVerifyFail = 0;
+    myVerifyAction = VERIFY_OVERRIDE_ERROR;
 
     ret = wolfSSL_CertManagerVerify(cm, expiredCert, WOLFSSL_FILETYPE_PEM);
     AssertIntEQ(ret, WOLFSSL_SUCCESS);
@@ -1194,7 +1194,7 @@ static int test_wolfSSL_CertManagerSetVerify(void)
     {
         const char* verifyCert = "./certs/server-cert.pem";
         /* Use the test CB that always fails certs */
-        myVerifyFail = 1;
+        myVerifyAction = VERIFY_FORCE_FAIL;
 
         ret = wolfSSL_CertManagerVerify(cm, verifyCert, WOLFSSL_FILETYPE_PEM);
         AssertIntEQ(ret, VERIFY_CERT_ERROR);
@@ -1202,7 +1202,7 @@ static int test_wolfSSL_CertManagerSetVerify(void)
 #endif
 
     wolfSSL_CertManagerFree(cm);
-    myVerifyFail = tmp;
+    myVerifyAction = tmp;
 #endif
 
     return ret;

+ 2 - 2
tests/suites.c

@@ -572,7 +572,7 @@ static void test_harness(void* vargs)
     cliArgsSz = 1;
     cliArgs[0] = args->argv[0];
 
-    while (*cursor != 0) {
+    while (cursor && *cursor != 0) {
         switch (*cursor) {
             case '\n':
                 /* A blank line triggers test case execution or switches
@@ -611,7 +611,7 @@ static void test_harness(void* vargs)
                     cliArgs[cliArgsSz++] = XSTRSEP(&cursor, " \n");
                 else
                     svrArgs[svrArgsSz++] = XSTRSEP(&cursor, " \n");
-                if (*cursor == '\0') /* eof */
+                if (cursor == NULL || *cursor == '\0') /* eof */
                     do_it = 1;
                 break;
         }

+ 119 - 31
tests/test-altchains.conf

@@ -166,6 +166,88 @@
 -C
 
 
+# Test will use alternate chain where chain contains extra cert
+# server TLSv1.2 DHE-RSA-AES128-GCM-SHA256 RSA Alt Chain
+-v 3
+-l DHE-RSA-AES128-GCM-SHA256
+-A ./certs/ca-cert.pem
+-k ./certs/server-key.pem
+-c ./certs/intermediate/server-chain-alt.pem
+-V
+
+# client TLSv1.2 DHE-RSA-AES128-GCM-SHA256 RSA Alt Chain
+-v 3
+-l DHE-RSA-AES128-GCM-SHA256
+-A ./certs/ca-cert.pem
+-k ./certs/client-key.pem
+-c ./certs/intermediate/client-chain-alt.pem
+-C
+
+# server TLSv1.2 ECDHE-RSA-AES128-GCM-SHA256 RSA Alt Chain
+-v 3
+-l ECDHE-RSA-AES128-GCM-SHA256
+-A ./certs/ca-cert.pem
+-k ./certs/server-key.pem
+-c ./certs/intermediate/server-chain-alt.pem
+-V
+
+# client TLSv1.2 ECDHE-RSA-AES128-GCM-SHA256 RSA Alt Chain
+-v 3
+-l ECDHE-RSA-AES128-GCM-SHA256
+-A ./certs/ca-cert.pem
+-k ./certs/client-key.pem
+-c ./certs/intermediate/client-chain-alt.pem
+-C
+
+# server TLSv1.2 ECDHE-ECDSA-AES128-GCM-SHA256 ECC Alt Chain
+-v 3
+-l ECDHE-ECDSA-AES128-GCM-SHA256
+-A ./certs/ca-ecc-cert.pem
+-k ./certs/ecc-key.pem
+-c ./certs/intermediate/server-chain-alt-ecc.pem
+-V
+
+# client TLSv1.2 ECDHE-ECDSA-AES128-GCM-SHA256 ECC Alt Chain
+-v 3
+-l ECDHE-ECDSA-AES128-GCM-SHA256
+-A ./certs/ca-ecc-cert.pem
+-k ./certs/ecc-client-key.pem
+-c ./certs/intermediate/client-chain-alt-ecc.pem
+-C
+
+# server TLSv1.3 TLS13-AES128-GCM-SHA256 RSA Alt Chain
+-v 4
+-l TLS13-AES128-GCM-SHA256
+-A ./certs/ca-cert.pem
+-k ./certs/server-key.pem
+-c ./certs/intermediate/server-chain-alt.pem
+-V
+
+# client TLSv1.3 TLS13-AES128-GCM-SHA256 RSA Alt Chain
+-v 4
+-l TLS13-AES128-GCM-SHA256
+-A ./certs/ca-cert.pem
+-k ./certs/client-key.pem
+-c ./certs/intermediate/client-chain-alt.pem
+-C
+
+# server TLSv1.3 TLS13-AES128-GCM-SHA256 ECC Alt Chain
+-v 4
+-l TLS13-AES128-GCM-SHA256
+-A ./certs/ca-ecc-cert.pem
+-k ./certs/ecc-key.pem
+-c ./certs/intermediate/server-chain-alt-ecc.pem
+-V
+
+# client TLSv1.3 TLS13-AES128-GCM-SHA256 ECC Alt Chain
+-v 4
+-l TLS13-AES128-GCM-SHA256
+-A ./certs/ca-ecc-cert.pem
+-k ./certs/ecc-client-key.pem
+-c ./certs/intermediate/client-chain-alt-ecc.pem
+-C
+
+
 # Test will load intermediate2 CA as trusted and present full chain (where intermediate CA is not trusted)
 # server TLSv1.2 DHE-RSA-AES128-GCM-SHA256 RSA Partial Trusted Chain
 -v 3
@@ -248,83 +330,89 @@
 -C
 
 
-# Test will use alternate chain where chain contains extra cert
-# server TLSv1.2 DHE-RSA-AES128-GCM-SHA256 RSA Alt Chain
+# Test will load intermediate2 CA as trusted and present full chain (where intermediate CA is not trusted)
+# These tests use the verify callback, but pass the preverify as result in myVerify callback
+# server TLSv1.2 DHE-RSA-AES128-GCM-SHA256 RSA Partial Trusted Chain
 -v 3
 -l DHE-RSA-AES128-GCM-SHA256
--A ./certs/ca-cert.pem
+-A ./certs/intermediate/ca-int2-cert.pem
 -k ./certs/server-key.pem
--c ./certs/intermediate/server-chain-alt.pem
+-c ./certs/intermediate/server-chain.pem
 -V
 
-# client TLSv1.2 DHE-RSA-AES128-GCM-SHA256 RSA Alt Chain
+# client TLSv1.2 DHE-RSA-AES128-GCM-SHA256 RSA Partial Trusted Chain
 -v 3
 -l DHE-RSA-AES128-GCM-SHA256
--A ./certs/ca-cert.pem
+-A ./certs/intermediate/ca-int2-cert.pem
 -k ./certs/client-key.pem
--c ./certs/intermediate/client-chain-alt.pem
+-c ./certs/intermediate/client-chain.pem
 -C
+-H verifyInfo
 
-# server TLSv1.2 ECDHE-RSA-AES128-GCM-SHA256 RSA Alt Chain
+# server TLSv1.2 ECDHE-RSA-AES128-GCM-SHA256 RSA Partial Trusted Chain
 -v 3
 -l ECDHE-RSA-AES128-GCM-SHA256
--A ./certs/ca-cert.pem
+-A ./certs/intermediate/ca-int2-cert.pem
 -k ./certs/server-key.pem
--c ./certs/intermediate/server-chain-alt.pem
+-c ./certs/intermediate/server-chain.pem
 -V
 
-# client TLSv1.2 ECDHE-RSA-AES128-GCM-SHA256 RSA Alt Chain
+# client TLSv1.2 ECDHE-RSA-AES128-GCM-SHA256 RSA Partial Trusted Chain
 -v 3
 -l ECDHE-RSA-AES128-GCM-SHA256
--A ./certs/ca-cert.pem
+-A ./certs/intermediate/ca-int2-cert.pem
 -k ./certs/client-key.pem
--c ./certs/intermediate/client-chain-alt.pem
+-c ./certs/intermediate/client-chain.pem
 -C
+-H verifyInfo
 
-# server TLSv1.2 ECDHE-ECDSA-AES128-GCM-SHA256 ECC Alt Chain
+# server TLSv1.2 ECDHE-ECDSA-AES128-GCM-SHA256 ECC Partial Trusted Chain
 -v 3
 -l ECDHE-ECDSA-AES128-GCM-SHA256
--A ./certs/ca-ecc-cert.pem
+-A ./certs/intermediate/ca-int2-ecc-cert.pem
 -k ./certs/ecc-key.pem
--c ./certs/intermediate/server-chain-alt-ecc.pem
+-c ./certs/intermediate/server-chain-ecc.pem
 -V
 
-# client TLSv1.2 ECDHE-ECDSA-AES128-GCM-SHA256 ECC Alt Chain
+# client TLSv1.2 ECDHE-ECDSA-AES128-GCM-SHA256 ECC Partial Trusted Chain
 -v 3
 -l ECDHE-ECDSA-AES128-GCM-SHA256
--A ./certs/ca-ecc-cert.pem
+-A ./certs/intermediate/ca-int2-ecc-cert.pem
 -k ./certs/ecc-client-key.pem
--c ./certs/intermediate/client-chain-alt-ecc.pem
+-c ./certs/intermediate/client-chain-ecc.pem
 -C
+-H verifyInfo
 
-# server TLSv1.3 TLS13-AES128-GCM-SHA256 RSA Alt Chain
+# server TLSv1.3 TLS13-AES128-GCM-SHA256 RSA Partial Trusted Chain
 -v 4
 -l TLS13-AES128-GCM-SHA256
--A ./certs/ca-cert.pem
+-A ./certs/intermediate/ca-int2-cert.pem
 -k ./certs/server-key.pem
--c ./certs/intermediate/server-chain-alt.pem
+-c ./certs/intermediate/server-chain.pem
 -V
 
-# client TLSv1.3 TLS13-AES128-GCM-SHA256 RSA Alt Chain
+# client TLSv1.3 TLS13-AES128-GCM-SHA256 RSA Partial Trusted Chain
 -v 4
 -l TLS13-AES128-GCM-SHA256
--A ./certs/ca-cert.pem
+-A ./certs/intermediate/ca-int2-cert.pem
 -k ./certs/client-key.pem
--c ./certs/intermediate/client-chain-alt.pem
+-c ./certs/intermediate/client-chain.pem
 -C
+-H verifyInfo
 
-# server TLSv1.3 TLS13-AES128-GCM-SHA256 ECC Alt Chain
+# server TLSv1.3 TLS13-AES128-GCM-SHA256 ECC Partial Trusted Chain
 -v 4
 -l TLS13-AES128-GCM-SHA256
--A ./certs/ca-ecc-cert.pem
+-A ./certs/intermediate/ca-int2-ecc-cert.pem
 -k ./certs/ecc-key.pem
--c ./certs/intermediate/server-chain-alt-ecc.pem
+-c ./certs/intermediate/server-chain-ecc.pem
 -V
 
-# client TLSv1.3 TLS13-AES128-GCM-SHA256 ECC Alt Chain
+# client TLSv1.3 TLS13-AES128-GCM-SHA256 ECC Partial Trusted Chain
 -v 4
 -l TLS13-AES128-GCM-SHA256
--A ./certs/ca-ecc-cert.pem
+-A ./certs/intermediate/ca-int2-ecc-cert.pem
 -k ./certs/ecc-client-key.pem
--c ./certs/intermediate/client-chain-alt-ecc.pem
+-c ./certs/intermediate/client-chain-ecc.pem
 -C
+-H verifyInfo

+ 3 - 0
wolfcrypt/src/asn.c

@@ -9353,6 +9353,9 @@ int ParseCertRelative(DecodedCert* cert, int type, int verify, void* cm)
     #else
             cert->ca = GetCA(cm, cert->issuerHash);
     #endif /* !NO_SKID */
+
+            if (cert->ca)
+                WOLFSSL_MSG("CA found");
         }
 
         if (cert->selfSigned) {

+ 16 - 23
wolfssl/test.h

@@ -1677,7 +1677,13 @@ static WC_INLINE void OCSPRespFreeCb(void* ioCtx, unsigned char* response)
     #endif /* !NO_FILESYSTEM || (NO_FILESYSTEM && FORCE_BUFFER_TEST) */
 #endif /* !NO_CERTS */
 
-static int myVerifyFail = 0;
+enum {
+    VERIFY_OVERRIDE_ERROR,
+    VERIFY_FORCE_FAIL,
+    VERIFY_USE_PREVERFIY,
+    VERIFY_OVERRIDE_DATE_ERR,
+};
+static int myVerifyAction = VERIFY_OVERRIDE_ERROR;
 
 /* The verify callback is called for every certificate only when
  * --enable-opensslextra is defined because it sets WOLFSSL_ALWAYS_VERIFY_CB and
@@ -1764,37 +1770,24 @@ static WC_INLINE int myVerify(int preverify, WOLFSSL_X509_STORE_CTX* store)
     printf("\tSubject's domain name at %d is %s\n", store->error_depth, store->domain);
 
     /* Testing forced fail case by return zero */
-    if (myVerifyFail) {
+    if (myVerifyAction == VERIFY_FORCE_FAIL) {
         return 0; /* test failure case */
     }
 
+    if (myVerifyAction == VERIFY_OVERRIDE_DATE_ERR && 
+        (store->error == ASN_BEFORE_DATE_E || store->error == ASN_AFTER_DATE_E)) {
+        printf("Overriding cert date error as example for bad clock testing\n");
+        return 1;
+    }
+
     /* If error indicate we are overriding it for testing purposes */
-    if (store->error != 0) {
+    if (store->error != 0 && myVerifyAction == VERIFY_OVERRIDE_ERROR) {
         printf("\tAllowing failed certificate check, testing only "
             "(shouldn't do this in production)\n");
     }
 
     /* A non-zero return code indicates failure override */
-    return 1;
-}
-
-
-static WC_INLINE int myDateCb(int preverify, WOLFSSL_X509_STORE_CTX* store)
-{
-    char buffer[WOLFSSL_MAX_ERROR_SZ];
-    (void)preverify;
-
-    printf("In verification callback, error = %d, %s\n", store->error,
-                                 wolfSSL_ERR_error_string(store->error, buffer));
-    printf("Subject's domain name is %s\n", store->domain);
-
-    if (store->error == ASN_BEFORE_DATE_E || store->error == ASN_AFTER_DATE_E) {
-        printf("Overriding cert date error as example for bad clock testing\n");
-        return 1;
-    }
-    printf("Cert error is not date error, not overriding\n");
-
-    return 0;
+    return (myVerifyAction == VERIFY_OVERRIDE_ERROR) ? 1 : preverify;
 }