From 7cbc71b024e41a8c4864b55a1034c31a916a8805 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Tue, 1 Apr 2025 16:13:59 +0200 Subject: [PATCH 1/4] Refactor *_push and *_pop compat API --- src/conf.c | 21 +------ src/internal.c | 110 ++-------------------------------- src/ssl.c | 129 ++++++++++++++++++++++++++++++---------- src/x509.c | 75 +---------------------- tests/api.c | 2 +- wolfcrypt/src/logging.c | 2 +- wolfssl/internal.h | 1 + wolfssl/ssl.h | 1 + 8 files changed, 113 insertions(+), 228 deletions(-) diff --git a/src/conf.c b/src/conf.c index 239486a6c..a30be3829 100644 --- a/src/conf.c +++ b/src/conf.c @@ -984,8 +984,6 @@ void wolfSSL_NCONF_free(WOLFSSL_CONF *conf) void wolfSSL_X509V3_conf_free(WOLFSSL_CONF_VALUE *val) { - WOLF_STACK_OF(WOLFSSL_CONF_VALUE) *sk = NULL; - if (val) { if (val->name) { /* Not a section. Don't free section as it is a shared pointer. */ @@ -997,12 +995,7 @@ void wolfSSL_X509V3_conf_free(WOLFSSL_CONF_VALUE *val) XFREE(val->section, NULL, DYNAMIC_TYPE_OPENSSL); /* Only free the stack structures. The contained conf values * will be freed in wolfSSL_NCONF_free */ - sk = (WOLF_STACK_OF(WOLFSSL_CONF_VALUE)*)val->value; - while (sk) { - WOLF_STACK_OF(WOLFSSL_CONF_VALUE) *tmp = sk->next; - XFREE(sk, NULL, DYNAMIC_TYPE_OPENSSL); - sk = tmp; - } + wolfSSL_sk_free((WOLF_STACK_OF(WOLFSSL_CONF_VALUE)*)val->value); } XFREE(val, NULL, DYNAMIC_TYPE_OPENSSL); } @@ -1028,19 +1021,9 @@ WOLFSSL_STACK *wolfSSL_sk_CONF_VALUE_new( */ void wolfSSL_sk_CONF_VALUE_free(WOLF_STACK_OF(WOLFSSL_CONF_VALUE)* sk) { - WOLFSSL_STACK* tmp; WOLFSSL_ENTER("wolfSSL_sk_CONF_VALUE_free"); - if (sk == NULL) - return; - - /* parse through stack freeing each node */ - while (sk) { - tmp = sk->next; - wolfSSL_X509V3_conf_free(sk->data.conf); - XFREE(sk, NULL, DYNAMIC_TYPE_OPENSSL); - sk = tmp; - } + wolfSSL_sk_pop_free(sk, NULL); } int wolfSSL_sk_CONF_VALUE_num(const WOLFSSL_STACK *sk) diff --git a/src/internal.c b/src/internal.c index dd00d6499..436565353 100644 --- a/src/internal.c +++ b/src/internal.c @@ -42369,32 +42369,7 @@ WOLFSSL_BY_DIR_HASH* wolfSSL_sk_BY_DIR_HASH_value( WOLFSSL_BY_DIR_HASH* wolfSSL_sk_BY_DIR_HASH_pop( WOLF_STACK_OF(WOLFSSL_BY_DIR_HASH)* sk) { - WOLFSSL_STACK* node; - WOLFSSL_BY_DIR_HASH* hash; - - WOLFSSL_ENTER("wolfSSL_sk_BY_DIR_HASH_pop"); - - if (sk == NULL) { - return NULL; - } - - node = sk->next; - hash = sk->data.dir_hash; - - if (node != NULL) { /* update sk and remove node from stack */ - sk->data.dir_hash = node->data.dir_hash; - sk->next = node->next; - wolfSSL_sk_free_node(node); - } - else { /* last x509 in stack */ - sk->data.dir_hash = NULL; - } - - if (sk->num > 0) { - sk->num -= 1; - } - - return hash; + return wolfSSL_sk_pop(sk); } /* release all contents in stack, and then release stack itself. */ /* Second argument is a function pointer to release resources. */ @@ -42449,39 +42424,13 @@ void wolfSSL_sk_BY_DIR_HASH_free(WOLF_STACK_OF(WOLFSSL_BY_DIR_HASH) *sk) int wolfSSL_sk_BY_DIR_HASH_push(WOLF_STACK_OF(WOLFSSL_BY_DIR_HASH)* sk, WOLFSSL_BY_DIR_HASH* in) { - WOLFSSL_STACK* node; - WOLFSSL_ENTER("wolfSSL_sk_BY_DIR_HASH_push"); if (sk == NULL || in == NULL) { return WOLFSSL_FAILURE; } - /* no previous values in stack */ - if (sk->data.dir_hash == NULL) { - sk->data.dir_hash = in; - sk->num += 1; - return WOLFSSL_SUCCESS; - } - - /* stack already has value(s) create a new node and add more */ - node = (WOLFSSL_STACK*)XMALLOC(sizeof(WOLFSSL_STACK), NULL, - DYNAMIC_TYPE_OPENSSL); - if (node == NULL) { - WOLFSSL_MSG("Memory error"); - return WOLFSSL_FAILURE; - } - XMEMSET(node, 0, sizeof(WOLFSSL_STACK)); - - /* push new obj onto head of stack */ - node->data.dir_hash = sk->data.dir_hash; - node->next = sk->next; - node->type = sk->type; - sk->next = node; - sk->data.dir_hash = in; - sk->num += 1; - - return WOLFSSL_SUCCESS; + return wolfSSL_sk_push(sk, in); } /* create an instance of WOLFSSL_BY_DIR_entry structure */ WOLFSSL_BY_DIR_entry* wolfSSL_BY_DIR_entry_new(void) @@ -42552,32 +42501,7 @@ WOLFSSL_BY_DIR_entry* wolfSSL_sk_BY_DIR_entry_value( WOLFSSL_BY_DIR_entry* wolfSSL_sk_BY_DIR_entry_pop( WOLF_STACK_OF(WOLFSSL_BY_DIR_entry)* sk) { - WOLFSSL_STACK* node; - WOLFSSL_BY_DIR_entry* entry; - - WOLFSSL_ENTER("wolfSSL_sk_BY_DIR_entry_pop"); - - if (sk == NULL) { - return NULL; - } - - node = sk->next; - entry = sk->data.dir_entry; - - if (node != NULL) { /* update sk and remove node from stack */ - sk->data.dir_entry = node->data.dir_entry; - sk->next = node->next; - wolfSSL_sk_free_node(node); - } - else { /* last x509 in stack */ - sk->data.dir_entry = NULL; - } - - if (sk->num > 0) { - sk->num -= 1; - } - - return entry; + return wolfSSL_sk_pop(sk); } /* release all contents in stack, and then release stack itself. */ /* Second argument is a function pointer to release resources. */ @@ -42633,37 +42557,13 @@ void wolfSSL_sk_BY_DIR_entry_free(WOLF_STACK_OF(wolfSSL_BY_DIR_entry) *sk) int wolfSSL_sk_BY_DIR_entry_push(WOLF_STACK_OF(WOLFSSL_BY_DIR_entry)* sk, WOLFSSL_BY_DIR_entry* in) { - WOLFSSL_STACK* node; + WOLFSSL_ENTER("wolfSSL_sk_BY_DIR_entry_push"); if (sk == NULL || in == NULL) { return WOLFSSL_FAILURE; } - /* no previous values in stack */ - if (sk->data.dir_entry == NULL) { - sk->data.dir_entry = in; - sk->num += 1; - return WOLFSSL_SUCCESS; - } - - /* stack already has value(s) create a new node and add more */ - node = (WOLFSSL_STACK*)XMALLOC(sizeof(WOLFSSL_STACK), NULL, - DYNAMIC_TYPE_OPENSSL); - if (node == NULL) { - WOLFSSL_MSG("Memory error"); - return WOLFSSL_FAILURE; - } - XMEMSET(node, 0, sizeof(WOLFSSL_STACK)); - - /* push new obj onto head of stack */ - node->data.dir_entry = sk->data.dir_entry; - node->next = sk->next; - node->type = sk->type; - sk->next = node; - sk->data.dir_entry = in; - sk->num += 1; - - return WOLFSSL_SUCCESS; + return wolfSSL_sk_push(sk, in); } #endif /* OPENSSL_ALL */ diff --git a/src/ssl.c b/src/ssl.c index 6148ad7f7..bdbd3ee26 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -14746,6 +14746,13 @@ int wolfSSL_sk_push(WOLFSSL_STACK* sk, const void *data) return wolfSSL_sk_insert(sk, data, 0); } +void* wolfSSL_sk_pop(WOLFSSL_STACK* sk) +{ + WOLFSSL_ENTER("wolfSSL_sk_pop"); + + return wolfSSL_sk_pop_node(sk, 0); +} + /* return number of elements on success 0 on fail */ int wolfSSL_sk_insert(WOLFSSL_STACK *sk, const void *data, int idx) { @@ -14917,10 +14924,8 @@ int wolfSSL_sk_insert(WOLFSSL_STACK *sk, const void *data, int idx) { /* insert node into stack. not using sk since we return sk->num after */ WOLFSSL_STACK* prev_node = sk; - while (idx != 0 && prev_node->next != NULL) { + while (--idx != 0 && prev_node->next != NULL) prev_node = prev_node->next; - idx--; - } node->next = prev_node->next; prev_node->next = node; } @@ -14928,6 +14933,93 @@ int wolfSSL_sk_insert(WOLFSSL_STACK *sk, const void *data, int idx) return (int)sk->num; } +void* wolfSSL_sk_pop_node(WOLFSSL_STACK* sk, int idx) +{ + void* ret = NULL; + WOLFSSL_STACK* tmp = NULL; + + if (!sk) + return NULL; + if (sk->num == 0) + return NULL; + + sk->num--; + if (idx == 0 || sk->next == NULL) { + switch (sk->type) { + case STACK_TYPE_CIPHER: + /* Can't return cipher type */ + break; + case STACK_TYPE_X509: + case STACK_TYPE_GEN_NAME: + case STACK_TYPE_BIO: + case STACK_TYPE_OBJ: + case STACK_TYPE_STRING: + case STACK_TYPE_ACCESS_DESCRIPTION: + case STACK_TYPE_X509_EXT: + case STACK_TYPE_X509_REQ_ATTR: + case STACK_TYPE_NULL: + case STACK_TYPE_X509_NAME: + case STACK_TYPE_X509_NAME_ENTRY: + case STACK_TYPE_CONF_VALUE: + case STACK_TYPE_X509_INFO: + case STACK_TYPE_BY_DIR_entry: + case STACK_TYPE_BY_DIR_hash: + case STACK_TYPE_X509_OBJ: + case STACK_TYPE_DIST_POINT: + case STACK_TYPE_X509_CRL: + default: + ret = sk->data.generic; + sk->data.generic = NULL; + break; + } + if (sk->next) { + tmp = sk->next; + sk->next = tmp->next; + XMEMCPY(&sk->data, &tmp->data, sizeof(sk->data)); + wolfSSL_sk_free_node(tmp); + } + return ret; + } + + { + WOLFSSL_STACK* prev_node = sk; + tmp = sk->next; + while (--idx != 0 && tmp->next != NULL) { + prev_node = tmp; + tmp = tmp->next; + } + prev_node->next = tmp->next; + switch (sk->type) { + case STACK_TYPE_CIPHER: + /* Can't return cipher type */ + break; + case STACK_TYPE_X509: + case STACK_TYPE_GEN_NAME: + case STACK_TYPE_BIO: + case STACK_TYPE_OBJ: + case STACK_TYPE_STRING: + case STACK_TYPE_ACCESS_DESCRIPTION: + case STACK_TYPE_X509_EXT: + case STACK_TYPE_X509_REQ_ATTR: + case STACK_TYPE_NULL: + case STACK_TYPE_X509_NAME: + case STACK_TYPE_X509_NAME_ENTRY: + case STACK_TYPE_CONF_VALUE: + case STACK_TYPE_X509_INFO: + case STACK_TYPE_BY_DIR_entry: + case STACK_TYPE_BY_DIR_hash: + case STACK_TYPE_X509_OBJ: + case STACK_TYPE_DIST_POINT: + case STACK_TYPE_X509_CRL: + default: + ret = tmp->data.generic; + break; + } + wolfSSL_sk_free_node(tmp); + } + return ret; +} + #endif /* OPENSSL_EXTRA || WOLFSSL_WPAS_SMALL */ #ifdef OPENSSL_EXTRA @@ -18063,7 +18155,7 @@ void wolfSSL_sk_free(WOLFSSL_STACK* sk) while (sk != NULL) { WOLFSSL_STACK* next = sk->next; - XFREE(sk, NULL, DYNAMIC_TYPE_OPENSSL); + wolfSSL_sk_free_node(sk); sk = next; } } @@ -18098,34 +18190,11 @@ void wolfSSL_sk_GENERIC_free(WOLFSSL_STACK* sk) */ void* wolfssl_sk_pop_type(WOLFSSL_STACK* sk, WOLF_STACK_TYPE type) { - WOLFSSL_STACK* node; void* data = NULL; /* Check we have a stack passed in of the right type. */ - if ((sk != NULL) && (sk->type == type)) { - /* Get the next node to become the new first node. */ - node = sk->next; - /* Get the ASN.1 OBJECT_ID object in the first node. */ - data = sk->data.generic; - - /* Check whether there is a next node. */ - if (node != NULL) { - /* Move content out of next node into current node. */ - sk->data.obj = node->data.obj; - sk->next = node->next; - /* Dispose of node. */ - XFREE(node, NULL, DYNAMIC_TYPE_ASN1); - } - else { - /* No more nodes - clear out data. */ - sk->data.obj = NULL; - } - - /* Decrement count as long as we thought we had nodes. */ - if (sk->num > 0) { - sk->num -= 1; - } - } + if ((sk != NULL) && (sk->type == type)) + data = wolfSSL_sk_pop(sk); return data; } @@ -18249,7 +18318,7 @@ void wolfSSL_sk_pop_free(WOLF_STACK_OF(WOLFSSL_ASN1_OBJECT)* sk, if (sk->type != STACK_TYPE_CIPHER) func(sk->data.generic); } - XFREE(sk, NULL, DYNAMIC_TYPE_OPENSSL); + XFREE(sk, sk->heap, DYNAMIC_TYPE_OPENSSL); sk = next; } } diff --git a/src/x509.c b/src/x509.c index 8178fc04f..dc87ab117 100644 --- a/src/x509.c +++ b/src/x509.c @@ -4203,30 +4203,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) { - WOLFSSL_STACK* node; - WOLFSSL_X509* x509; - - if (sk == NULL) { - return NULL; - } - - node = sk->next; - x509 = sk->data.x509; - - if (node != NULL) { /* update sk and remove node from stack */ - sk->data.x509 = node->data.x509; - sk->next = node->next; - XFREE(node, NULL, DYNAMIC_TYPE_X509); - } - else { /* last x509 in stack */ - sk->data.x509 = NULL; - } - - if (sk->num > 0) { - sk->num--; - } - - return x509; + return wolfSSL_sk_pop(sk); } /* Getter function for WOLFSSL_X509 pointer @@ -13448,30 +13425,7 @@ WOLFSSL_X509_NAME* wolfSSL_sk_X509_NAME_value( WOLFSSL_X509_NAME* wolfSSL_sk_X509_NAME_pop( WOLF_STACK_OF(WOLFSSL_X509_NAME)* sk) { - WOLFSSL_STACK* node; - WOLFSSL_X509_NAME* name; - - if (sk == NULL) { - return NULL; - } - - node = sk->next; - name = sk->data.name; - - if (node != NULL) { /* update sk and remove node from stack */ - sk->data.name = node->data.name; - sk->next = node->next; - XFREE(node, NULL, DYNAMIC_TYPE_OPENSSL); - } - else { /* last x509 in stack */ - sk->data.name = NULL; - } - - if (sk->num > 0) { - sk->num -= 1; - } - - return name; + return wolfSSL_sk_pop(sk); } void wolfSSL_sk_X509_NAME_pop_free(WOLF_STACK_OF(WOLFSSL_X509_NAME)* sk, @@ -13622,30 +13576,7 @@ WOLFSSL_X509_INFO* wolfSSL_sk_X509_INFO_value( WOLFSSL_X509_INFO* wolfSSL_sk_X509_INFO_pop( WOLF_STACK_OF(WOLFSSL_X509_INFO)* sk) { - WOLFSSL_STACK* node; - WOLFSSL_X509_INFO* info; - - if (sk == NULL) { - return NULL; - } - - node = sk->next; - info = sk->data.info; - - if (node != NULL) { /* update sk and remove node from stack */ - sk->data.info = node->data.info; - sk->next = node->next; - wolfSSL_sk_free_node(node); - } - else { /* last x509 in stack */ - sk->data.info = NULL; - } - - if (sk->num > 0) { - sk->num -= 1; - } - - return info; + return wolfSSL_sk_pop(sk); } #if defined(OPENSSL_ALL) diff --git a/tests/api.c b/tests/api.c index 0a0913586..680886b35 100644 --- a/tests/api.c +++ b/tests/api.c @@ -28646,7 +28646,7 @@ static int test_X509_STORE_get0_objects(void) #endif ExpectNotNull(objsCopy = sk_X509_OBJECT_deep_copy(objs, NULL, NULL)); ExpectIntEQ(sk_X509_OBJECT_num(objs), sk_X509_OBJECT_num(objsCopy)); - for (i = 0; i < sk_X509_OBJECT_num(objs); i++) { + for (i = 0; i < sk_X509_OBJECT_num(objs) && EXPECT_SUCCESS(); i++) { obj = (X509_OBJECT*)sk_X509_OBJECT_value(objs, i); #ifdef HAVE_CRL objCopy = (X509_OBJECT*)sk_X509_OBJECT_value(objsCopy, i); diff --git a/wolfcrypt/src/logging.c b/wolfcrypt/src/logging.c index 4aee6dcb2..35617683f 100644 --- a/wolfcrypt/src/logging.c +++ b/wolfcrypt/src/logging.c @@ -158,7 +158,7 @@ wolfSSL_Logging_cb wolfSSL_GetLoggingCb(void) int wolfSSL_Debugging_ON(void) { #ifdef DEBUG_WOLFSSL - loggingEnabled = 1; + loggingEnabled = 0; #if defined(WOLFSSL_APACHE_MYNEWT) log_register("wolfcrypt", &mynewt_log, &log_console_handler, NULL, LOG_SYSLEVEL); #endif /* WOLFSSL_APACHE_MYNEWT */ diff --git a/wolfssl/internal.h b/wolfssl/internal.h index e8dac34a1..a9838509f 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -7189,6 +7189,7 @@ WOLFSSL_LOCAL int TranslateErrorToAlert(int err); #if defined(OPENSSL_EXTRA) || defined(WOLFSSL_WPAS_SMALL) WOLFSSL_LOCAL void* wolfssl_sk_pop_type(WOLFSSL_STACK* sk, WOLF_STACK_TYPE type); +WOLFSSL_LOCAL void* wolfSSL_sk_pop_node(WOLFSSL_STACK* sk, int idx); WOLFSSL_LOCAL WOLFSSL_STACK* wolfssl_sk_new_type(WOLF_STACK_TYPE type); WOLFSSL_LOCAL int wolfssl_asn1_obj_set(WOLFSSL_ASN1_OBJECT* obj, diff --git a/wolfssl/ssl.h b/wolfssl/ssl.h index 982371bea..908d5c6e8 100644 --- a/wolfssl/ssl.h +++ b/wolfssl/ssl.h @@ -1853,6 +1853,7 @@ 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); WOLFSSL_API int wolfSSL_sk_insert(WOLFSSL_STACK *sk, const void *data, int idx); +WOLFSSL_API void* wolfSSL_sk_pop(WOLFSSL_STACK* sk); #if defined(OPENSSL_ALL) || defined(OPENSSL_EXTRA) || defined(WOLFSSL_QT) WOLFSSL_API int wolfSSL_sk_ACCESS_DESCRIPTION_push( From 5f13aebd5f94dc92c82d4651ae13114f9e51da11 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Tue, 1 Apr 2025 17:36:44 +0200 Subject: [PATCH 2/4] Push/pop to/from the end of the list object The last object pushed should be visible in the highest index --- src/ssl.c | 4 ++-- src/x509.c | 58 +++++++---------------------------------------------- tests/api.c | 36 ++++++++++++++++----------------- 3 files changed, 27 insertions(+), 71 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index bdbd3ee26..2600858ef 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -14743,14 +14743,14 @@ int wolfSSL_sk_push(WOLFSSL_STACK* sk, const void *data) { WOLFSSL_ENTER("wolfSSL_sk_push"); - return wolfSSL_sk_insert(sk, data, 0); + return wolfSSL_sk_insert(sk, data, -1); } void* wolfSSL_sk_pop(WOLFSSL_STACK* sk) { WOLFSSL_ENTER("wolfSSL_sk_pop"); - return wolfSSL_sk_pop_node(sk, 0); + return wolfSSL_sk_pop_node(sk, -1); } /* return number of elements on success 0 on fail */ diff --git a/src/x509.c b/src/x509.c index dc87ab117..2a79e39a3 100644 --- a/src/x509.c +++ b/src/x509.c @@ -4230,38 +4230,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) { - WOLFSSL_STACK* node; - WOLFSSL_X509* x509; - - if (sk == NULL) { - return NULL; - } - - node = sk->next; - x509 = sk->data.x509; - - if (node != NULL) { - /* walk to end of stack to first node pushed, and remove it */ - WOLFSSL_STACK* prevNode = sk; - - while (node->next != NULL) { - prevNode = node; - node = node->next; - } - - x509 = node->data.x509; - prevNode->next = NULL; - XFREE(node, NULL, DYNAMIC_TYPE_X509); - } - else { /* only one x509 in stack */ - sk->data.x509 = NULL; - } - - if (sk->num > 0) { - sk->num -= 1; - } - - return x509; + return wolfSSL_sk_pop_node(sk, 0); } #endif /* OPENSSL_EXTRA */ @@ -15318,7 +15287,6 @@ WOLFSSL_X509_ATTRIBUTE *wolfSSL_X509_REQ_get_attr( int wolfSSL_X509_REQ_get_attr_by_NID(const WOLFSSL_X509 *req, int nid, int lastpos) { - WOLFSSL_STACK* sk; int idx; WOLFSSL_ENTER("wolfSSL_X509_REQ_get_attr_by_NID"); @@ -15329,26 +15297,14 @@ int wolfSSL_X509_REQ_get_attr_by_NID(const WOLFSSL_X509 *req, } /* search through stack for first matching nid */ - idx = lastpos + 1; - do { - sk = wolfSSL_sk_get_node(req->reqAttributes, idx); - if (sk != NULL) { - WOLFSSL_X509_ATTRIBUTE* attr; - attr = (WOLFSSL_X509_ATTRIBUTE*)sk->data.generic; - if (nid == attr->object->nid) { - /* found a match */ - break; - } - } - idx++; - } while (sk != NULL); - - /* no matches found */ - if (sk == NULL) { - idx = WOLFSSL_FATAL_ERROR; + for (idx = lastpos + 1; idx < wolfSSL_sk_num(req->reqAttributes); idx++) { + WOLFSSL_X509_ATTRIBUTE* attr = + (WOLFSSL_X509_ATTRIBUTE*)wolfSSL_sk_value(req->reqAttributes, idx); + if (attr != NULL && attr->object != NULL && attr->object->nid == nid) + return idx; } - return idx; + return WOLFSSL_FATAL_ERROR; } WOLFSSL_X509_ATTRIBUTE* wolfSSL_X509_ATTRIBUTE_new(void) diff --git a/tests/api.c b/tests/api.c index 680886b35..23695d1e9 100644 --- a/tests/api.c +++ b/tests/api.c @@ -22069,7 +22069,7 @@ static int test_wolfSSL_X509_INFO_multiple_info(void) ExpectNotNull(info = sk_X509_INFO_value(info_stack, i)); ExpectNotNull(info->x509); ExpectNull(info->crl); - if (i != 0) { + if (i != 2) { ExpectNotNull(info->x_pkey); ExpectIntEQ(X509_check_private_key(info->x509, info->x_pkey->dec_pkey), 1); @@ -36213,7 +36213,7 @@ static int test_GENERAL_NAME_set0_othername(void) ExpectNull(sk_GENERAL_NAME_value(NULL, 0)); ExpectNull(sk_GENERAL_NAME_value(gns, 20)); - ExpectNotNull(gn = sk_GENERAL_NAME_value(gns, 2)); + ExpectNotNull(gn = sk_GENERAL_NAME_value(gns, 0)); ExpectIntEQ(gn->type, 0); sk_GENERAL_NAME_pop_free(gns, GENERAL_NAME_free); @@ -46197,8 +46197,8 @@ static int test_sk_X509(void) sk_X509_push(s, &x2); ExpectIntEQ(sk_X509_num(s), 2); ExpectNull(sk_X509_value(s, 2)); - ExpectIntEQ((sk_X509_value(s, 0) == &x2), 1); - ExpectIntEQ((sk_X509_value(s, 1) == &x1), 1); + ExpectIntEQ((sk_X509_value(s, 0) == &x1), 1); + ExpectIntEQ((sk_X509_value(s, 1) == &x2), 1); sk_X509_push(s, &x2); sk_X509_pop_free(s, free_x509); @@ -46223,20 +46223,20 @@ static int test_sk_X509(void) for (i = 0; i < len; ++i) { sk_X509_push(s, xList[i]); ExpectIntEQ(sk_X509_num(s), i + 1); - ExpectIntEQ((sk_X509_value(s, 0) == xList[i]), 1); - ExpectIntEQ((sk_X509_value(s, i) == xList[0]), 1); + ExpectIntEQ((sk_X509_value(s, 0) == xList[0]), 1); + ExpectIntEQ((sk_X509_value(s, i) == xList[i]), 1); } - /* pop returns and removes last pushed on stack, which is index 0 + /* pop returns and removes last pushed on stack, which is the last index * in sk_x509_value */ - for (i = 0; i < len; ++i) { - X509 * x = sk_X509_value(s, 0); + for (i = len-1; i >= 0; --i) { + X509 * x = sk_X509_value(s, i); X509 * y = sk_X509_pop(s); - X509 * z = xList[len - 1 - i]; + X509 * z = xList[i]; - ExpectIntEQ((x == y), 1); - ExpectIntEQ((x == z), 1); - ExpectIntEQ(sk_X509_num(s), len - 1 - i); + ExpectPtrEq(x, y); + ExpectPtrEq(x, z); + ExpectIntEQ(sk_X509_num(s), i); } sk_free(s); @@ -46248,14 +46248,14 @@ static int test_sk_X509(void) for (i = 0; i < len; ++i) { sk_X509_push(s, xList[i]); ExpectIntEQ(sk_X509_num(s), i + 1); - ExpectIntEQ((sk_X509_value(s, 0) == xList[i]), 1); - ExpectIntEQ((sk_X509_value(s, i) == xList[0]), 1); + ExpectIntEQ((sk_X509_value(s, 0) == xList[0]), 1); + ExpectIntEQ((sk_X509_value(s, i) == xList[i]), 1); } /* shift returns and removes first pushed on stack, which is index i * in sk_x509_value() */ for (i = 0; i < len; ++i) { - X509 * x = sk_X509_value(s, len - 1 - i); + X509 * x = sk_X509_value(s, 0); X509 * y = sk_X509_shift(s); X509 * z = xList[i]; @@ -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), 1); + -1), 0); 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, 1)); + ExpectNotNull(attr = X509_REQ_get_attr(req, 0)); ExpectNull(X509_ATTRIBUTE_get0_type(NULL, 1)); ExpectNull(X509_ATTRIBUTE_get0_type(attr, 1)); ExpectNull(X509_ATTRIBUTE_get0_type(NULL, 0)); From 8b7e1be6945d935863b0c36ad7b7c29594afce9e Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Thu, 3 Apr 2025 20:59:04 +0200 Subject: [PATCH 3/4] Maintain backwards compatible order of SAN Maintain previous order in X509_get_ext_d2i(cert, NID_subject_alt_name, NULL, NULL). Tested for in Python osp port (test_ssl.py:test_parse_all_sans). --- src/x509.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/x509.c b/src/x509.c index 2a79e39a3..a063f5899 100644 --- a/src/x509.c +++ b/src/x509.c @@ -2359,7 +2359,11 @@ void* wolfSSL_X509_get_ext_d2i(const WOLFSSL_X509* x509, int nid, int* c, } dns = dns->next; - if (wolfSSL_sk_GENERAL_NAME_push(sk, gn) <= 0) { + /* Using wolfSSL_sk_insert to maintain backwards + * compatiblity 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) { WOLFSSL_MSG("Error pushing ASN1 object onto stack"); goto err; } From 56263d9577417c2280ab01816a787524b8e95eac Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Fri, 4 Apr 2025 10:20:27 +0200 Subject: [PATCH 4/4] fixup! Push/pop to/from the end of the list object --- src/ssl.c | 55 +++++++++++++-------------------------- src/x509.c | 33 ++++++++---------------- src/x509_str.c | 70 ++++++++++++++++++++++++-------------------------- tests/api.c | 8 +++--- 4 files changed, 66 insertions(+), 100 deletions(-) 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));