diff --git a/src/ssl.c b/src/ssl.c index 2600858ef..c16c5ff66 100644 --- a/src/ssl.c +++ b/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 - * WOLFSSL_FAILURE on no issuer found + * @return 0 on success or 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_SUCCESS) + WOLFSSL_X509* issuer = NULL; + if (x509GetIssuerFromCM(&issuer, cm, x) != WOLFSSL_SUCCESS) break; - x = issuer[i]; - } - if (i == 0) /* No further chain found */ - 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]); + if (wolfSSL_sk_X509_push(sk, issuer) <= 0) { + wolfSSL_X509_free(issuer); + return WOLFSSL_FATAL_ERROR; } + 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--) { + for (i = 0; i < ssl->session->chain.count; 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) - == WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR)) { - ret = WOLFSSL_FATAL_ERROR; - } + err = PushCAx509Chain(SSL_CM(ssl), x509, sk); } - - if (ret != 0 || wolfSSL_sk_X509_push(sk, x509) <= 0) { + if (err != 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) diff --git a/src/x509.c b/src/x509.c index a063f5899..47d8a3529 100644 --- a/src/x509.c +++ b/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; - - if (sk == NULL) { - return NULL; - } - - ret = wolfSSL_sk_get_node(sk, idx); - if (ret != NULL) { - return ret->data.gn; - } - return NULL; + return (WOLFSSL_GENERAL_NAME*)wolfSSL_sk_value(sk, idx); } /* 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_FATAL_ERROR; - } - - return (int)sk->num; + return wolfSSL_sk_num(sk); } /* 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 { diff --git a/src/x509_str.c b/src/x509_str.c index b67582ce1..f16131d74 100644 --- a/src/x509_str.c +++ b/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) || \ - 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--) { + for (i = 0; i < c->count; 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; diff --git a/tests/api.c b/tests/api.c index 23695d1e9..feeebf84d 100644 --- a/tests/api.c +++ b/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));