Merge pull request #1759 from dgarske/verifycbfail

Fix to resolve issue with verify callback not causing an error

examples/client/client.c

@@ -887,7 +887,7 @@ static void Usage(void)
 #ifdef HAVE_WNR
     printf("-q <file>   Whitewood config file,      default %s\n", wnrConfig);
 #endif
-    printf("-H <arg>    Internal tests [defCipherList, exitWithRet]\n");
+    printf("-H <arg>    Internal tests [defCipherList, exitWithRet, verifyFail]\n");
 #ifdef WOLFSSL_TLS13
     printf("-J          Use HelloRetryRequest to choose group for KE\n");
     printf("-K          Key Exchange for PSK not using (EC)DHE\n");
@@ -1211,10 +1211,14 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args)
                     printf("Using default cipher list for testing\n");
                     useDefCipherList = 1;
                 }
-                else if (XSTRNCMP(myoptarg, "exitWithRet", 7) == 0) {
+                else if (XSTRNCMP(myoptarg, "exitWithRet", 11) == 0) {
                     printf("Skip exit() for testing\n");
                     exitWithRet = 1;
                 }
+                else if (XSTRNCMP(myoptarg, "verifyFail", 10) == 0) {
+                    printf("Verify should fail\n");
+                    myVerifyFail = 1;
+                }
                 else {
                     Usage();
                     XEXIT_T(MY_EX_USAGE);
@@ -1821,9 +1825,9 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args)
     #endif
     }
 
-    if (!usePsk && !useAnon && !useVerifyCb) {
+    if (!usePsk && !useAnon && (!useVerifyCb || myVerifyFail)) {
     #if !defined(NO_FILESYSTEM)
-        if (wolfSSL_CTX_load_verify_locations(ctx, verifyCert,0)
+        if (wolfSSL_CTX_load_verify_locations(ctx, verifyCert, 0)
                                                            != WOLFSSL_SUCCESS) {
             wolfSSL_CTX_free(ctx); ctx = NULL;
             err_sys("can't load ca file, Please run from wolfSSL home dir");

examples/server/server.c

@@ -416,7 +416,7 @@ static void Usage(void)
 #endif
     printf("-g          Return basic HTML web page\n");
     printf("-C <num>    The number of connections to accept, default: 1\n");
-    printf("-H <arg>    Internal tests [defCipherList, exitWithRet]\n");
+    printf("-H <arg>    Internal tests [defCipherList, exitWithRet, verifyFail]\n");
 #ifdef WOLFSSL_TLS13
     printf("-U          Update keys and IVs before sending\n");
     printf("-K          Key Exchange for PSK not using (EC)DHE\n");
@@ -714,10 +714,14 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args)
                     printf("Using default cipher list for testing\n");
                     useDefCipherList = 1;
                 }
-                else if (XSTRNCMP(myoptarg, "exitWithRet", 7) == 0) {
+                else if (XSTRNCMP(myoptarg, "exitWithRet", 11) == 0) {
                     printf("Skip exit() for testing\n");
                     exitWithRet = 1;
                 }
+                else if (XSTRNCMP(myoptarg, "verifyFail", 10) == 0) {
+                    printf("Verify should fail\n");
+                    myVerifyFail = 1;
+                }
                 else {
                     Usage();
                     XEXIT_T(MY_EX_USAGE);

src/internal.c

@@ -8454,9 +8454,7 @@ typedef struct ProcPeerCertArgs {
     word32 begin;
     int    totalCerts; /* number of certs in certs buffer */
     int    count;
-    int    dCertInit;
     int    certIdx;
-    int    fatal;
     int    lastErr;
 #ifdef WOLFSSL_ALT_CERT_CHAINS
     int    lastCaErr;
@@ -8464,11 +8462,14 @@ typedef struct ProcPeerCertArgs {
 #ifdef WOLFSSL_TLS13
     byte   ctxSz;
 #endif
-#ifdef WOLFSSL_TRUST_PEER_CERT
-    byte haveTrustPeer; /* was cert verified by loaded trusted peer cert */
-#endif
 #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL)
     char   untrustedDepth;
+#endif
+    word16 fatal:1;
+    word16 verifyErr:1;
+    word16 dCertInit:1;
+#ifdef WOLFSSL_TRUST_PEER_CERT
+    word16 haveTrustPeer:1; /* was cert verified by loaded trusted peer cert */
 #endif
 } ProcPeerCertArgs;
 
@@ -8608,6 +8609,15 @@ static int DoVerifyCallback(WOLFSSL* ssl, int ret, ProcPeerCertArgs* args)
             WOLFSSL_MSG("Verify callback overriding error!");
             ret = 0;
         }
+        else {
+            /* induce error if one not present */
+            if (ret == 0) {
+                ret = VERIFY_CERT_ERROR;
+            }
+
+            /* mark as verify error */
+            args->verifyErr = 1;
+        }
     #ifdef OPENSSL_EXTRA
         if (args->certIdx > 0)
             FreeX509(x509);
@@ -8616,14 +8626,6 @@ static int DoVerifyCallback(WOLFSSL* ssl, int ret, ProcPeerCertArgs* args)
         wolfSSL_sk_X509_free(store->chain);
         store->chain = NULL;
     #endif
-    #ifdef WOLFSSL_SMALL_STACK
-        XFREE(domain, ssl->heap, DYNAMIC_TYPE_STRING);
-        #ifdef OPENSSL_EXTRA
-        XFREE(x509, ssl->heap, DYNAMIC_TYPE_X509);
-        #endif
-        XFREE(store, ssl->heap, DYNAMIC_TYPE_X509_STORE);
-    #endif
-
     #ifdef SESSION_CERTS
         if (store->discardSessionCerts) {
             WOLFSSL_MSG("Verify callback requested discard sess certs");
@@ -8633,6 +8635,13 @@ static int DoVerifyCallback(WOLFSSL* ssl, int ret, ProcPeerCertArgs* args)
         #endif
         }
     #endif /* SESSION_CERTS */
+    #ifdef WOLFSSL_SMALL_STACK
+        XFREE(domain, ssl->heap, DYNAMIC_TYPE_STRING);
+        #ifdef OPENSSL_EXTRA
+        XFREE(x509, ssl->heap, DYNAMIC_TYPE_X509);
+        #endif
+        XFREE(store, ssl->heap, DYNAMIC_TYPE_X509_STORE);
+    #endif
     }
 
     if (ret != 0) {
@@ -9231,7 +9240,8 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx,
                             ssl->options.usingAltCertChain = 1;
 
                             /* clear last CA fail since CA cert was validated */
-                            args->lastCaErr = 0;
+                            if (!args->verifyErr)
+                                args->lastCaErr = 0;
 
                         #ifdef SESSION_CERTS
                             AddSessionCertToChain(&ssl->session.altChain,
@@ -9242,16 +9252,6 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx,
                     }
                     else if (ret != 0) {
                         WOLFSSL_MSG("Failed to verify CA from chain");
-                    #ifdef WOLFSSL_ALT_CERT_CHAINS
-                        if (args->lastCaErr == 0) {
-                            /* store CA error and proceed to next cert */
-                            args->lastCaErr = ret;
-                            ret = 0;
-                        }
-                        else {
-                            args->lastErr = args->lastCaErr;
-                        }
-                    #endif
                     #ifdef OPENSSL_EXTRA
                         ssl->peerVerifyRet = X509_V_ERR_INVALID_CA;
                     #endif
@@ -9322,6 +9322,16 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx,
                     ret = DoVerifyCallback(ssl, ret, args);
 
                     /* Handle error codes */
+                #ifdef WOLFSSL_ALT_CERT_CHAINS
+                    if (args->lastCaErr == 0) {
+                        /* capture CA error and proceed to next cert */
+                        args->lastCaErr = ret;
+                        ret = 0;
+                    }
+                    else {
+                        args->lastErr = args->lastCaErr;
+                    }
+                #endif
                     if (ret != 0 && args->lastErr == 0) {
                         args->lastErr = ret;   /* save error from last time */
                         ret = 0; /* reset error */
@@ -9424,7 +9434,22 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx,
                             cert->buffer, cert->length);
                     }
                 #endif /* SESSION_CERTS && WOLFSSL_ALT_CERT_CHAINS */
-                    args->fatal = 0;
+
+                    /* check if fatal error */
+                    if (args->verifyErr) {
+                        args->fatal = 1;
+                        if (ret == 0) {
+                            ret = args->lastErr;
+                        }
+                    #ifdef WOLFSSL_ALT_CERT_CHAINS
+                        if (ret == 0) {
+                            ret = args->lastCaErr;
+                        }
+                    #endif
+                    }
+                    else {
+                        args->fatal = 0;
+                    }
                 }
                 else if (ret == ASN_PARSE_E || ret == BUFFER_E) {
                     WOLFSSL_MSG("Got Peer cert ASN PARSE or BUFFER ERROR");
@@ -9442,7 +9467,8 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx,
                     if (ssl->verifyCallback) {
                         WOLFSSL_MSG(
                             "\tCallback override available, will continue");
-                        args->fatal = 0;
+                        /* check if fatal error */
+                        args->fatal = (args->verifyErr) ? 1 : 0;
                     }
                     else {
                         WOLFSSL_MSG("\tNo callback override available, fatal");

tests/test-fails.conf

@@ -107,3 +107,63 @@
 -A ./certs/test/server-garbage.pem
 -m
 
+# Verify Callback Failure Tests
+# no error going into callback, return error
+# server
+-v 3
+-l ECDHE-RSA-AES128-GCM-SHA256
+
+# client verify should fail
+-v 3
+-l ECDHE-RSA-AES128-GCM-SHA256
+-H verifyFail
+
+# server verify should fail
+-v 3
+-l ECDHE-RSA-AES128-GCM-SHA256
+-H verifyFail
+
+# client
+-v 3
+-l ECDHE-RSA-AES128-GCM-SHA256
+
+# server
+-v 3
+-l ECDHE-ECDSA-AES128-GCM-SHA256
+
+# client verify should fail
+-v 3
+-l ECDHE-ECDSA-AES128-GCM-SHA256
+-H verifyFail
+
+# server verify should fail
+-v 3
+-l ECDHE-ECDSA-AES128-GCM-SHA256
+-H verifyFail
+
+# client
+-v 3
+-l ECDHE-ECDSA-AES128-GCM-SHA256
+
+# error going into callback, return error
+# server
+-v 3
+-l ECDHE-RSA-AES128-GCM-SHA256
+-c ./certs/test/server-cert-rsa-badsig.pem
+-k ./certs/server-key.pem
+
+# client verify should fail
+-v 3
+-l ECDHE-RSA-AES128-GCM-SHA256
+-H verifyFail
+
+# server
+-v 3
+-l ECDHE-ECDSA-AES128-GCM-SHA256
+-c ./certs/test/server-cert-ecc-badsig.pem
+-k ./certs/ecc-key.pem
+
+# client verify should fail
+-v 3
+-l ECDHE-ECDSA-AES128-GCM-SHA256
+-H verifyFail

tests/test.conf

@@ -2199,13 +2199,50 @@
 -v 3
 -l NTRU-AES128-SHA
 
+# error going into callback, return ok
 # server TLSv1.2 verify callback override
 -v 3
--l ECDHE-RSA-AES128-SHA256
+-l ECDHE-RSA-AES128-GCM-SHA256
+-c ./certs/test/server-cert-rsa-badsig.pem
 
 # client TLSv1.2 verify callback override
 -v 3
--l ECDHE-RSA-AES128-SHA256
+-l ECDHE-RSA-AES128-GCM-SHA256
+-j
+
+# server TLSv1.2 verify callback override
+-v 3
+-l ECDHE-ECDSA-AES128-GCM-SHA256
+-c ./certs/test/server-cert-ecc-badsig.pem
+-k ./certs/ecc-key.pem
+
+# client TLSv1.2 verify callback override
+-v 3
+-l ECDHE-ECDSA-AES128-GCM-SHA256
+-A ./certs/ca-ecc-cert.pem
+-j
+
+# no error going into callback, return ok
+# server TLSv1.2 verify callback override
+-v 3
+-l ECDHE-RSA-AES128-GCM-SHA256
+-c ./certs/server-cert.pem
+
+# client TLSv1.2 verify callback override
+-v 3
+-l ECDHE-RSA-AES128-GCM-SHA256
+-j
+
+# server TLSv1.2 verify callback override
+-v 3
+-l ECDHE-ECDSA-AES128-GCM-SHA256
+-c ./certs/test/server-ecc.pem
+-k ./certs/ecc-key.pem
+
+# client TLSv1.2 verify callback override
+-v 3
+-l ECDHE-ECDSA-AES128-GCM-SHA256
+-A ./certs/ca-ecc-cert.pem
 -j
 
 # server TLSv1.2 ECDHE-EDCSA-CHACHA20-POLY1305

wolfssl/test.h

@@ -1452,6 +1452,7 @@ 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;
 static WC_INLINE int myVerify(int preverify, WOLFSSL_X509_STORE_CTX* store)
 {
     char buffer[WOLFSSL_MAX_ERROR_SZ];
@@ -1470,6 +1471,8 @@ static WC_INLINE int myVerify(int preverify, WOLFSSL_X509_STORE_CTX* store)
      * store->store:        WOLFSSL_X509_STORE with CA cert chain
      * store->store->cm:    WOLFSSL_CERT_MANAGER
      * store->ex_data:      The WOLFSSL object pointer
+     * store->discardSessionCerts: When set to non-zero value session certs 
+        will be discarded (only with SESSION_CERTS)
      */
 
     printf("In verification callback, error = %d, %s\n", store->error,
@@ -1505,6 +1508,9 @@ static WC_INLINE int myVerify(int preverify, WOLFSSL_X509_STORE_CTX* store)
     printf("\tAllowing to continue anyway (shouldn't do this)\n");
 
     /* A non-zero return code indicates failure override */
+    if (myVerifyFail)
+        return 0; /* test failure case */
+
     return 1;
 }