Merge pull request #6887 from julek-wolfssl/zd/16849

Implement untrusted certs in wolfSSL_X509_STORE_CTX_init

src/ssl.c

@@ -24285,6 +24285,38 @@ error:
     return NULL;
 }
 
+
+WOLFSSL_STACK* wolfSSL_shallow_sk_dup(WOLFSSL_STACK* sk)
+{
+
+    WOLFSSL_STACK* ret = NULL;
+    WOLFSSL_STACK** prev = &ret;
+
+    WOLFSSL_ENTER("wolfSSL_shallow_sk_dup");
+
+    for (; sk != NULL; sk = sk->next) {
+        WOLFSSL_STACK* cur = wolfSSL_sk_new_node(sk->heap);
+
+        if (!cur) {
+            WOLFSSL_MSG("wolfSSL_sk_new_node error");
+            goto error;
+        }
+
+        XMEMCPY(cur, sk, sizeof(WOLFSSL_STACK));
+        cur->next = NULL;
+
+        *prev = cur;
+        prev = &cur->next;
+    }
+    return ret;
+
+error:
+    if (ret) {
+        wolfSSL_sk_free(ret);
+    }
+    return NULL;
+}
+
 /* Free the just the stack structure */
 void wolfSSL_sk_free(WOLFSSL_STACK* sk)
 {

src/ssl_certman.c

@@ -1648,6 +1648,7 @@ int wolfSSL_CertManagerDisableCRL(WOLFSSL_CERT_MANAGER* cm)
     if (ret == WOLFSSL_SUCCESS) {
         /* Disable CRL checking. */
         cm->crlEnabled = 0;
+        cm->crlCheckAll = 0;
     }
 
     return ret;

src/x509_str.c

@@ -84,16 +84,40 @@ int wolfSSL_X509_STORE_CTX_init(WOLFSSL_X509_STORE_CTX* ctx,
         #endif
 
         ctx->chain  = sk;
-        /* Add intermediate certificates from stack to store */
+        /* Add intermediate certs, that verify to a loaded CA, to the store */
-        while (sk != NULL) {
+        if (sk != NULL) {
-            WOLFSSL_X509* x509_cert = sk->data.x509;
+            byte addedAtLeastOne = 1;
-            if (x509_cert != NULL && x509_cert->isCa) {
+            WOLF_STACK_OF(WOLFSSL_X509)* head = wolfSSL_shallow_sk_dup(sk);
-                ret = wolfSSL_X509_STORE_add_cert(store, x509_cert);
+            if (head == NULL)
-                if (ret < 0) {
+                return WOLFSSL_FAILURE;
-                    return WOLFSSL_FAILURE;
+            while (addedAtLeastOne) {
+                WOLF_STACK_OF(WOLFSSL_X509)* cur = head;
+                WOLF_STACK_OF(WOLFSSL_X509)** prev = &head;
+                addedAtLeastOne = 0;
+                while (cur) {
+                    WOLFSSL_X509* cert = cur->data.x509;
+                    if (cert != NULL && cert->derCert != NULL &&
+                            wolfSSL_CertManagerVerifyBuffer(store->cm,
+                                    cert->derCert->buffer,
+                                    cert->derCert->length,
+                                    WOLFSSL_FILETYPE_ASN1) == WOLFSSL_SUCCESS) {
+                        ret = wolfSSL_X509_STORE_add_cert(store, cert);
+                        if (ret < 0) {
+                            wolfSSL_sk_free(head);
+                            return WOLFSSL_FAILURE;
+                        }
+                        addedAtLeastOne = 1;
+                        *prev = cur->next;
+                        wolfSSL_sk_free_node(cur);
+                        cur = *prev;
+                    }
+                    else {
+                        prev = &cur->next;
+                        cur = cur->next;
+                    }
                 }
             }
-            sk = sk->next;
+            wolfSSL_sk_free(head);
         }
 
         ctx->sesChain = NULL;
@@ -140,7 +164,9 @@ void wolfSSL_X509_STORE_CTX_free(WOLFSSL_X509_STORE_CTX* ctx)
     }
 }
 
-
+/* Its recommended to use a full free -> init cycle of all the objects
+ * because wolfSSL_X509_STORE_CTX_init may modify the store too which doesn't
+ * get reset here. */
 void wolfSSL_X509_STORE_CTX_cleanup(WOLFSSL_X509_STORE_CTX* ctx)
 {
     if (ctx != NULL) {
@@ -168,7 +194,7 @@ int GetX509Error(int e)
 {
     switch (e) {
         case ASN_BEFORE_DATE_E:
-            return WOLFSSL_X509_V_ERR_ERROR_IN_CERT_NOT_BEFORE_FIELD;
+            return WOLFSSL_X509_V_ERR_CERT_NOT_YET_VALID;
         case ASN_AFTER_DATE_E:
             return WOLFSSL_X509_V_ERR_CERT_HAS_EXPIRED;
         case ASN_NO_SIGNER_E: /* get issuer error if no CA found locally */
@@ -185,6 +211,9 @@ int GetX509Error(int e)
             return WOLFSSL_X509_V_ERR_CERT_SIGNATURE_FAILURE;
         case CRL_CERT_REVOKED:
             return WOLFSSL_X509_V_ERR_CERT_REVOKED;
+        case 0:
+        case 1:
+            return 0;
         default:
 #ifdef HAVE_WOLFSSL_MSG_EX
             WOLFSSL_MSG_EX("Error not configured or implemented yet: %d", e);
@@ -195,6 +224,19 @@ int GetX509Error(int e)
     }
 }
 
+static void SetupStoreCtxError(WOLFSSL_X509_STORE_CTX* ctx, int ret)
+{
+    int depth = 0;
+    int error = GetX509Error(ret);
+
+    /* Set error depth */
+    if (ctx->chain)
+        depth = (int)ctx->chain->num;
+
+    wolfSSL_X509_STORE_CTX_set_error(ctx, error);
+    wolfSSL_X509_STORE_CTX_set_error_depth(ctx, depth);
+}
+
 /* Verifies certificate chain using WOLFSSL_X509_STORE_CTX
  * returns 0 on success or < 0 on failure.
  */
@@ -204,66 +246,39 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx)
 
     if (ctx != NULL && ctx->store != NULL && ctx->store->cm != NULL
          && ctx->current_cert != NULL && ctx->current_cert->derCert != NULL) {
-        int ret = 0;
+        int ret = wolfSSL_CertManagerVerifyBuffer(ctx->store->cm,
-        int depth = 0;
-        int error;
-    #ifndef NO_ASN_TIME
-        byte *afterDate, *beforeDate;
-    #endif
-
-        ret = wolfSSL_CertManagerVerifyBuffer(ctx->store->cm,
                 ctx->current_cert->derCert->buffer,
                 ctx->current_cert->derCert->length,
                 WOLFSSL_FILETYPE_ASN1);
-        /* If there was an error, process it and add it to CTX */
+        SetupStoreCtxError(ctx, ret);
-        if (ret < 0) {
-            /* Get corresponding X509 error */
-            error = GetX509Error(ret);
-            /* Set error depth */
-            if (ctx->chain)
-                depth = (int)ctx->chain->num;
-
-            wolfSSL_X509_STORE_CTX_set_error(ctx, error);
-            wolfSSL_X509_STORE_CTX_set_error_depth(ctx, depth);
-        #if defined(OPENSSL_ALL) || defined(WOLFSSL_QT)
-            if (ctx->store && ctx->store->verify_cb)
-                ctx->store->verify_cb(0, ctx);
-        #endif
-        }
 
     #ifndef NO_ASN_TIME
-        error = 0;
+        if (ret != ASN_BEFORE_DATE_E && ret != ASN_AFTER_DATE_E) {
-        /* wolfSSL_CertManagerVerifyBuffer only returns ASN_AFTER_DATE_E or
+            /* wolfSSL_CertManagerVerifyBuffer only returns ASN_AFTER_DATE_E or
-         ASN_BEFORE_DATE_E if there are no additional errors found in the
+             ASN_BEFORE_DATE_E if there are no additional errors found in the
-         cert. Therefore, check if the cert is expired or not yet valid
+             cert. Therefore, check if the cert is expired or not yet valid
-         in order to return the correct expected error. */
+             in order to return the correct expected error. */
-        afterDate = ctx->current_cert->notAfter.data;
+            byte *afterDate = ctx->current_cert->notAfter.data;
-        beforeDate = ctx->current_cert->notBefore.data;
+            byte *beforeDate = ctx->current_cert->notBefore.data;
 
-        if (XVALIDATE_DATE(afterDate, (byte)ctx->current_cert->notAfter.type,
+            if (XVALIDATE_DATE(afterDate,
-                                                                   AFTER) < 1) {
+                        (byte)ctx->current_cert->notAfter.type, AFTER) < 1) {
-            error = WOLFSSL_X509_V_ERR_CERT_HAS_EXPIRED;
+                ret = ASN_AFTER_DATE_E;
-        }
+            }
-        else if (XVALIDATE_DATE(beforeDate,
+            else if (XVALIDATE_DATE(beforeDate,
-                    (byte)ctx->current_cert->notBefore.type, BEFORE) < 1) {
+                        (byte)ctx->current_cert->notBefore.type, BEFORE) < 1) {
-            error = WOLFSSL_X509_V_ERR_CERT_NOT_YET_VALID;
+                ret = ASN_BEFORE_DATE_E;
-        }
+            }
-
+            SetupStoreCtxError(ctx, ret);
-        if (error != 0 ) {
-            wolfSSL_X509_STORE_CTX_set_error(ctx, error);
-            wolfSSL_X509_STORE_CTX_set_error_depth(ctx, depth);
-        #if defined(OPENSSL_ALL) || defined(WOLFSSL_QT)
-            if (ctx->store && ctx->store->verify_cb)
-                ctx->store->verify_cb(0, ctx);
-        #endif
         }
     #endif
 
-        /* OpenSSL returns 0 when a chain can't be built */
+    #if defined(OPENSSL_ALL) || defined(WOLFSSL_QT)
-        if (ret == ASN_NO_SIGNER_E)
+        if (ctx->store && ctx->store->verify_cb)
-            return WOLFSSL_FAILURE;
+            ret = ctx->store->verify_cb(ret >= 0 ? 1 : 0, ctx) == 1 ? 0 : -1;
-        else
+    #endif
-            return ret;
+
+        return ret >= 0 ? WOLFSSL_SUCCESS : WOLFSSL_FAILURE;
     }
     return WOLFSSL_FATAL_ERROR;
 }
@@ -1029,8 +1044,11 @@ WOLFSSL_API int wolfSSL_X509_STORE_load_locations(WOLFSSL_X509_STORE *str,
 
 #ifdef HAVE_CRL
     if (str->cm->crl == NULL) {
+        /* Workaround to allocate the internals to load CRL's but don't enable
+         * CRL checking by default */
         if (wolfSSL_CertManagerEnableCRL(str->cm, WOLFSSL_CRL_CHECK)
-            != WOLFSSL_SUCCESS) {
+                != WOLFSSL_SUCCESS ||
+                wolfSSL_CertManagerDisableCRL(str->cm) != WOLFSSL_SUCCESS) {
             WOLFSSL_MSG("Enable CRL failed");
             wolfSSL_CTX_free(ctx);
             return WOLFSSL_FAILURE;

tests/api.c

@@ -35531,6 +35531,118 @@ static int test_wolfSSL_X509_STORE_CTX(void)
     return EXPECT_RESULT();
 }
 
+#if defined(OPENSSL_EXTRA) && !defined(NO_RSA)
+static int test_X509_STORE_untrusted_load_cert_to_stack(const char* filename,
+        STACK_OF(X509)* chain)
+{
+    EXPECT_DECLS;
+    XFILE fp = XBADFILE;
+    X509* cert = NULL;
+
+    ExpectTrue((fp = XFOPEN(filename, "rb"))
+            != XBADFILE);
+    ExpectNotNull(cert = PEM_read_X509(fp, 0, 0, 0 ));
+    if (fp != XBADFILE) {
+        XFCLOSE(fp);
+        fp = XBADFILE;
+    }
+    ExpectIntEQ(sk_X509_push(chain, cert), 1);
+    if (EXPECT_FAIL())
+        X509_free(cert);
+
+    return EXPECT_RESULT();
+}
+
+static int test_X509_STORE_untrusted_certs(const char** filenames, int ret,
+        int err, int loadCA)
+{
+    EXPECT_DECLS;
+    X509_STORE_CTX* ctx = NULL;
+    X509_STORE* str = NULL;
+    XFILE fp = XBADFILE;
+    X509* cert = NULL;
+    STACK_OF(X509)* untrusted = NULL;
+
+    ExpectTrue((fp = XFOPEN("./certs/intermediate/server-int-cert.pem", "rb"))
+            != XBADFILE);
+    ExpectNotNull(cert = PEM_read_X509(fp, 0, 0, 0 ));
+    if (fp != XBADFILE) {
+        XFCLOSE(fp);
+        fp = XBADFILE;
+    }
+
+    ExpectNotNull(str = X509_STORE_new());
+    ExpectNotNull(ctx = X509_STORE_CTX_new());
+    ExpectNotNull(untrusted = sk_X509_new_null());
+
+    ExpectIntEQ(X509_STORE_set_flags(str, 0), 1);
+    if (loadCA) {
+        ExpectIntEQ(X509_STORE_load_locations(str, "./certs/ca-cert.pem", NULL),
+                1);
+    }
+    for (; *filenames; filenames++) {
+        ExpectIntEQ(test_X509_STORE_untrusted_load_cert_to_stack(*filenames,
+                untrusted), TEST_SUCCESS);
+    }
+
+    ExpectIntEQ(X509_STORE_CTX_init(ctx, str, cert, untrusted), 1);
+    ExpectIntEQ(X509_verify_cert(ctx), ret);
+    ExpectIntEQ(X509_STORE_CTX_get_error(ctx), err);
+
+    X509_free(cert);
+    X509_STORE_free(str);
+    X509_STORE_CTX_free(ctx);
+    sk_X509_pop_free(untrusted, NULL);
+
+    return EXPECT_RESULT();
+}
+#endif
+
+static int test_X509_STORE_untrusted(void)
+{
+    EXPECT_DECLS;
+#if defined(OPENSSL_EXTRA) && !defined(NO_RSA)
+    const char* untrusted1[] = {
+        "./certs/intermediate/ca-int2-cert.pem",
+        NULL
+    };
+    const char* untrusted2[] = {
+        "./certs/intermediate/ca-int-cert.pem",
+        "./certs/intermediate/ca-int2-cert.pem",
+        NULL
+    };
+    const char* untrusted3[] = {
+        "./certs/intermediate/ca-int-cert.pem",
+        "./certs/intermediate/ca-int2-cert.pem",
+        "./certs/ca-cert.pem",
+        NULL
+    };
+    /* Adding unrelated certs that should be ignored */
+    const char* untrusted4[] = {
+        "./certs/client-ca.pem",
+        "./certs/intermediate/ca-int-cert.pem",
+        "./certs/server-cert.pem",
+        "./certs/intermediate/ca-int2-cert.pem",
+        NULL
+    };
+
+    /* Only immediate issuer in untrusted chaing. Fails since can't build chain
+     * to loaded CA. */
+    ExpectIntEQ(test_X509_STORE_untrusted_certs(untrusted1, 0,
+            X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY, 1), TEST_SUCCESS);
+    /* Succeeds because path to loaded CA is available. */
+    ExpectIntEQ(test_X509_STORE_untrusted_certs(untrusted2, 1, 0, 1),
+            TEST_SUCCESS);
+    /* Fails because root CA is in the untrusted stack */
+    ExpectIntEQ(test_X509_STORE_untrusted_certs(untrusted3, 0,
+            X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY, 0), TEST_SUCCESS);
+    /* Succeeds because path to loaded CA is available. */
+    ExpectIntEQ(test_X509_STORE_untrusted_certs(untrusted4, 1, 0, 1),
+            TEST_SUCCESS);
+#endif
+    return EXPECT_RESULT();
+}
+
 static int test_wolfSSL_X509_STORE_set_flags(void)
 {
     EXPECT_DECLS;
@@ -51492,7 +51604,8 @@ static int test_X509_LOOKUP_add_dir(void)
 
     /* Now we SHOULD get CRL_MISSING, because we looked for PEM
      * in dir containing only ASN1/DER. */
-    ExpectIntEQ(X509_verify_cert(storeCtx), CRL_MISSING);
+    ExpectIntEQ(X509_verify_cert(storeCtx), WOLFSSL_FAILURE);
+    ExpectIntEQ(X509_STORE_CTX_get_error(storeCtx), CRL_MISSING);
 
     X509_CRL_free(crl);
     X509_STORE_free(store);
@@ -67697,6 +67810,7 @@ TEST_CASE testCases[] = {
     TEST_DECL(test_wolfSSL_TBS),
 
     TEST_DECL(test_wolfSSL_X509_STORE_CTX),
+    TEST_DECL(test_X509_STORE_untrusted),
     TEST_DECL(test_wolfSSL_X509_STORE_CTX_trusted_stack_cleanup),
     TEST_DECL(test_wolfSSL_X509_STORE_CTX_get0_current_issuer),
     TEST_DECL(test_wolfSSL_X509_STORE_set_flags),

wolfssl/ssl.h

@@ -1526,6 +1526,7 @@ WOLFSSL_API WOLFSSL_STACK* wolfSSL_sk_new_node(void* heap);
 WOLFSSL_API void wolfSSL_sk_free(WOLFSSL_STACK* sk);
 WOLFSSL_API void wolfSSL_sk_free_node(WOLFSSL_STACK* in);
 WOLFSSL_API WOLFSSL_STACK* wolfSSL_sk_dup(WOLFSSL_STACK* sk);
+WOLFSSL_API WOLFSSL_STACK* wolfSSL_shallow_sk_dup(WOLFSSL_STACK* sk);
 WOLFSSL_API int wolfSSL_sk_push_node(WOLFSSL_STACK** stack, WOLFSSL_STACK* in);
 WOLFSSL_API WOLFSSL_STACK* wolfSSL_sk_get_node(WOLFSSL_STACK* sk, int idx);
 WOLFSSL_API int wolfSSL_sk_push(WOLFSSL_STACK *st, const void *data);