fixup! Push/pop to/from the end of the list object

src/ssl.c

@@ -14489,40 +14489,24 @@ static int x509GetIssuerFromCM(WOLFSSL_X509 **issuer, WOLFSSL_CERT_MANAGER* cm,
  * @param cm The cert manager that is queried for the issuer
  * @param x  This cert's issuer will be queried in cm
  * @param sk The issuer is pushed onto this stack
- * @return WOLFSSL_SUCCESS on success
+ * @return 0 on success or no issuer found
- *         WOLFSSL_FAILURE on no issuer found
  *         WOLFSSL_FATAL_ERROR on a fatal error
  */
 static int PushCAx509Chain(WOLFSSL_CERT_MANAGER* cm,
         WOLFSSL_X509 *x, WOLFSSL_STACK* sk)
 {
-    WOLFSSL_X509* issuer[MAX_CHAIN_DEPTH];
     int i;
-    int push = 1;
-    int ret = WOLFSSL_SUCCESS;
-
     for (i = 0; i < MAX_CHAIN_DEPTH; i++) {
-        if (x509GetIssuerFromCM(&issuer[i], cm, x)
+        WOLFSSL_X509* issuer = NULL;
-                != WOLFSSL_SUCCESS)
+        if (x509GetIssuerFromCM(&issuer, cm, x) != WOLFSSL_SUCCESS)
             break;
-        x = issuer[i];
+        if (wolfSSL_sk_X509_push(sk, issuer) <= 0) {
-    }
+            wolfSSL_X509_free(issuer);
-    if (i == 0) /* No further chain found */
+            return WOLFSSL_FATAL_ERROR;
-        return WOLFSSL_FAILURE;
-    i--;
-    for (; i >= 0; i--) {
-        if (push) {
-            if (wolfSSL_sk_X509_push(sk, issuer[i]) <= 0) {
-                wolfSSL_X509_free(issuer[i]);
-                ret = WOLFSSL_FATAL_ERROR;
-                push = 0; /* Free the rest of the unpushed certs */
-            }
-        }
-        else {
-            wolfSSL_X509_free(issuer[i]);
         }
+        x = issuer;
     }
-    return ret;
+    return 0;
 }
 
 
@@ -14536,34 +14520,31 @@ static WOLF_STACK_OF(WOLFSSL_X509)* CreatePeerCertChain(const WOLFSSL* ssl,
     WOLFSSL_STACK* sk;
     WOLFSSL_X509* x509;
     int i = 0;
-    int ret;
+    int err;
 
     WOLFSSL_ENTER("wolfSSL_set_peer_cert_chain");
     if ((ssl == NULL) || (ssl->session->chain.count == 0))
         return NULL;
 
     sk = wolfSSL_sk_X509_new_null();
-    i = ssl->session->chain.count-1;
+    for (i = 0; i < ssl->session->chain.count; i++) {
-    for (; i >= 0; i--) {
         x509 = wolfSSL_X509_new_ex(ssl->heap);
         if (x509 == NULL) {
             WOLFSSL_MSG("Error Creating X509");
             wolfSSL_sk_X509_pop_free(sk, NULL);
             return NULL;
         }
-        ret = DecodeToX509(x509, ssl->session->chain.certs[i].buffer,
+        err = DecodeToX509(x509, ssl->session->chain.certs[i].buffer,
                              ssl->session->chain.certs[i].length);
-        if (ret == 0 && i == ssl->session->chain.count-1 && verifiedFlag) {
+        if (err == 0 && wolfSSL_sk_X509_push(sk, x509) <= 0)
+            err = WOLFSSL_FATAL_ERROR;
+        if (err == 0 && i == ssl->session->chain.count-1 && verifiedFlag) {
             /* On the last element in the verified chain try to add the CA chain
-             * first if we have one for this cert */
+             * if we have one for this cert */
             SSL_CM_WARNING(ssl);
-            if (PushCAx509Chain(SSL_CM(ssl), x509, sk)
+            err = PushCAx509Chain(SSL_CM(ssl), x509, sk);
-                    == WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR)) {
-                ret = WOLFSSL_FATAL_ERROR;
-            }
         }
-
+        if (err != 0) {
-        if (ret != 0 || wolfSSL_sk_X509_push(sk, x509) <= 0) {
             WOLFSSL_MSG("Error decoding cert");
             wolfSSL_X509_free(x509);
             wolfSSL_sk_X509_pop_free(sk, NULL);
@@ -14595,7 +14576,7 @@ WOLF_STACK_OF(WOLFSSL_X509)* wolfSSL_set_peer_cert_chain(WOLFSSL* ssl)
             if (ssl->session->peer)
                 wolfSSL_X509_free(ssl->session->peer);
 
-            ssl->session->peer = wolfSSL_sk_X509_pop(sk);
+            ssl->session->peer = wolfSSL_sk_X509_shift(sk);
             ssl->session->peerVerifyRet = ssl->peerVerifyRet;
         }
         if (ssl->peerCertChain != NULL)

src/x509.c

@@ -2360,7 +2360,7 @@ void* wolfSSL_X509_get_ext_d2i(const WOLFSSL_X509* x509, int nid, int* c,
 
                     dns = dns->next;
                     /* Using wolfSSL_sk_insert to maintain backwards
-                     * compatiblity with earlier versions of _push API that
+                     * compatibility with earlier versions of _push API that
                      * pushed items to the start of the list instead of the
                      * end. */
                     if (wolfSSL_sk_insert(sk, gn, 0) <= 0) {
@@ -4207,7 +4207,7 @@ int wolfSSL_sk_X509_push(WOLF_STACK_OF(WOLFSSL_X509_NAME)* sk,
 /* Return and remove the last x509 pushed on stack */
 WOLFSSL_X509* wolfSSL_sk_X509_pop(WOLF_STACK_OF(WOLFSSL_X509_NAME)* sk)
 {
-    return wolfSSL_sk_pop(sk);
+    return (WOLFSSL_X509*)wolfSSL_sk_pop(sk);
 }
 
 /* Getter function for WOLFSSL_X509 pointer
@@ -4234,7 +4234,7 @@ WOLFSSL_X509* wolfSSL_sk_X509_value(WOLF_STACK_OF(WOLFSSL_X509)* sk, int i)
 /* Return and remove the first x509 pushed on stack */
 WOLFSSL_X509* wolfSSL_sk_X509_shift(WOLF_STACK_OF(WOLFSSL_X509)* sk)
 {
-    return wolfSSL_sk_pop_node(sk, 0);
+    return (WOLFSSL_X509*)wolfSSL_sk_pop_node(sk, 0);
 }
 
 #endif /* OPENSSL_EXTRA */
@@ -4547,17 +4547,7 @@ int wolfSSL_sk_GENERAL_NAME_push(WOLFSSL_GENERAL_NAMES* sk,
  */
 WOLFSSL_GENERAL_NAME* wolfSSL_sk_GENERAL_NAME_value(WOLFSSL_STACK* sk, int idx)
 {
-    WOLFSSL_STACK* ret;
+    return (WOLFSSL_GENERAL_NAME*)wolfSSL_sk_value(sk, idx);
-
-    if (sk == NULL) {
-        return NULL;
-    }
-
-    ret = wolfSSL_sk_get_node(sk, idx);
-    if (ret != NULL) {
-        return ret->data.gn;
-    }
-    return NULL;
 }
 
 /* Gets the number of nodes in the stack
@@ -4570,11 +4560,7 @@ int wolfSSL_sk_GENERAL_NAME_num(WOLFSSL_STACK* sk)
 {
     WOLFSSL_ENTER("wolfSSL_sk_GENERAL_NAME_num");
 
-    if (sk == NULL) {
+    return wolfSSL_sk_num(sk);
-        return WOLFSSL_FATAL_ERROR;
-    }
-
-    return (int)sk->num;
 }
 
 /* Allocates an empty GENERAL NAME stack */
@@ -13398,7 +13384,7 @@ WOLFSSL_X509_NAME* wolfSSL_sk_X509_NAME_value(
 WOLFSSL_X509_NAME* wolfSSL_sk_X509_NAME_pop(
     WOLF_STACK_OF(WOLFSSL_X509_NAME)* sk)
 {
-    return wolfSSL_sk_pop(sk);
+    return (WOLFSSL_X509_NAME*)wolfSSL_sk_pop(sk);
 }
 
 void wolfSSL_sk_X509_NAME_pop_free(WOLF_STACK_OF(WOLFSSL_X509_NAME)* sk,
@@ -13549,7 +13535,7 @@ WOLFSSL_X509_INFO* wolfSSL_sk_X509_INFO_value(
 WOLFSSL_X509_INFO* wolfSSL_sk_X509_INFO_pop(
         WOLF_STACK_OF(WOLFSSL_X509_INFO)* sk)
 {
-    return wolfSSL_sk_pop(sk);
+    return (WOLFSSL_X509_INFO*)wolfSSL_sk_pop(sk);
 }
 
 #if defined(OPENSSL_ALL)
@@ -15206,7 +15192,10 @@ int wolfSSL_X509_REQ_add1_attr_by_NID(WOLFSSL_X509 *req,
         }
         if ((req->reqAttributes != NULL) &&
                 (req->reqAttributes->type == STACK_TYPE_X509_REQ_ATTR)) {
-            ret = wolfSSL_sk_push(req->reqAttributes, attr) > 0
+            /* Using wolfSSL_sk_insert to maintain backwards compatibility with
+             * earlier versions of _push API that pushed items to the start of
+             * the list instead of the end. */
+            ret = wolfSSL_sk_insert(req->reqAttributes, attr, 0) > 0
                     ? WOLFSSL_SUCCESS : WOLFSSL_FAILURE;
         }
         else {

src/x509_str.c

@@ -803,43 +803,7 @@ WOLFSSL_STACK* wolfSSL_X509_STORE_CTX_get_chain(WOLFSSL_X509_STORE_CTX* ctx)
         if (sk == NULL)
             return NULL;
 
-#if defined(WOLFSSL_NGINX) || defined(WOLFSSL_HAPROXY) || \
+        for (i = 0; i < c->count; i++) {
-    defined(OPENSSL_EXTRA)
-        /* add CA used to verify top of chain to the list */
-        if (c->count > 0) {
-            WOLFSSL_X509* x509 = wolfSSL_get_chain_X509(c, c->count - 1);
-            WOLFSSL_X509* issuer = NULL;
-            if (x509 != NULL) {
-                if (wolfSSL_X509_STORE_CTX_get1_issuer(&issuer, ctx, x509)
-                        == WOLFSSL_SUCCESS) {
-                    /* check that the certificate being looked up is not self
-                     * signed and that a issuer was found */
-                    if (issuer != NULL && wolfSSL_X509_NAME_cmp(&x509->issuer,
-                                &x509->subject) != 0) {
-                        if (wolfSSL_sk_X509_push(sk, issuer) <= 0) {
-                            WOLFSSL_MSG("Unable to load CA x509 into stack");
-                            error = 1;
-                        }
-                    }
-                    else {
-                        WOLFSSL_MSG("Certificate is self signed");
-                        wolfSSL_X509_free(issuer);
-                    }
-                }
-                else {
-                    WOLFSSL_MSG("Could not find CA for certificate");
-                }
-            }
-            wolfSSL_X509_free(x509);
-            if (error) {
-                wolfSSL_sk_X509_pop_free(sk, NULL);
-                wolfSSL_X509_free(issuer);
-                return NULL;
-            }
-        }
-#endif
-
-        for (i = c->count - 1; i >= 0; i--) {
             WOLFSSL_X509* x509 = wolfSSL_get_chain_X509(c, i);
 
             if (x509 == NULL) {
@@ -855,6 +819,38 @@ WOLFSSL_STACK* wolfSSL_X509_STORE_CTX_get_chain(WOLFSSL_X509_STORE_CTX* ctx)
                 break;
             }
         }
+
+#if defined(WOLFSSL_NGINX) || defined(WOLFSSL_HAPROXY) || \
+    defined(OPENSSL_EXTRA)
+        /* add CA used to verify top of chain to the list */
+        if (!error && c->count > 0) {
+            WOLFSSL_X509* x509 = wolfSSL_get_chain_X509(c, c->count - 1);
+            WOLFSSL_X509* issuer = NULL;
+            if (x509 != NULL) {
+                if (wolfSSL_X509_STORE_CTX_get1_issuer(&issuer, ctx, x509)
+                        == WOLFSSL_SUCCESS) {
+                    /* check that the certificate being looked up is not self
+                     * signed and that a issuer was found */
+                    if (issuer != NULL && wolfSSL_X509_NAME_cmp(&x509->issuer,
+                                &x509->subject) != 0) {
+                        if (wolfSSL_sk_X509_push(sk, issuer) <= 0) {
+                            WOLFSSL_MSG("Unable to load CA x509 into stack");
+                            error = 1;
+                            wolfSSL_X509_free(issuer);
+                        }
+                    }
+                    else {
+                        WOLFSSL_MSG("Certificate is self signed");
+                        wolfSSL_X509_free(issuer);
+                    }
+                }
+                else {
+                    WOLFSSL_MSG("Could not find CA for certificate");
+                }
+            }
+            wolfSSL_X509_free(x509);
+        }
+#endif
         if (error) {
             wolfSSL_sk_X509_pop_free(sk, NULL);
             return NULL;

tests/api.c

@@ -36208,12 +36208,12 @@ static int test_GENERAL_NAME_set0_othername(void)
     ExpectNotNull(gns = (GENERAL_NAMES*)X509_get_ext_d2i(x509,
         NID_subject_alt_name, NULL, NULL));
 
-    ExpectIntEQ(sk_GENERAL_NAME_num(NULL), WOLFSSL_FATAL_ERROR);
+    ExpectIntEQ(sk_GENERAL_NAME_num(NULL), 0);
     ExpectIntEQ(sk_GENERAL_NAME_num(gns), 3);
 
     ExpectNull(sk_GENERAL_NAME_value(NULL, 0));
     ExpectNull(sk_GENERAL_NAME_value(gns, 20));
-    ExpectNotNull(gn = sk_GENERAL_NAME_value(gns, 0));
+    ExpectNotNull(gn = sk_GENERAL_NAME_value(gns, 2));
     ExpectIntEQ(gn->type, 0);
 
     sk_GENERAL_NAME_pop_free(gns, GENERAL_NAME_free);
@@ -49234,12 +49234,12 @@ static int test_wolfSSL_d2i_X509_REQ(void)
         ExpectIntEQ(X509_REQ_get_attr_by_NID(NULL, NID_pkcs9_challengePassword,
             -1), -1);
         ExpectIntEQ(X509_REQ_get_attr_by_NID(req, NID_pkcs9_challengePassword,
-            -1), 0);
+            -1), 1);
         ExpectNull(X509_REQ_get_attr(NULL, 3));
         ExpectNull(X509_REQ_get_attr(req, 3));
         ExpectNull(X509_REQ_get_attr(NULL, 0));
         ExpectNull(X509_REQ_get_attr(empty, 0));
-        ExpectNotNull(attr = X509_REQ_get_attr(req, 0));
+        ExpectNotNull(attr = X509_REQ_get_attr(req, 1));
         ExpectNull(X509_ATTRIBUTE_get0_type(NULL, 1));
         ExpectNull(X509_ATTRIBUTE_get0_type(attr, 1));
         ExpectNull(X509_ATTRIBUTE_get0_type(NULL, 0));