From 66dc2b229ab093e985ab6175b9bcb45e4d50ac02 Mon Sep 17 00:00:00 2001 From: Anthony Hu Date: Wed, 23 Apr 2025 18:51:03 -0400 Subject: [PATCH 1/4] In wolfSSL_CTX_set_cert_store, send certificates into the CertMgr --- src/ssl.c | 15 +++++++++++++++ src/x509_str.c | 5 +---- tests/api.c | 42 ++++++++++++++++++++++++++++++++++++++++++ wolfssl/internal.h | 5 +++++ 4 files changed, 63 insertions(+), 4 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 80e55cf865..ba3ea3207e 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -12930,6 +12930,7 @@ int wolfSSL_set_compression(WOLFSSL* ssl) void wolfSSL_CTX_set_cert_store(WOLFSSL_CTX* ctx, WOLFSSL_X509_STORE* str) { + WOLFSSL_X509 *x = NULL; WOLFSSL_ENTER("wolfSSL_CTX_set_cert_store"); if (ctx == NULL || str == NULL || ctx->cm == str->cm) { return; @@ -12946,6 +12947,20 @@ int wolfSSL_set_compression(WOLFSSL* ssl) ctx->cm = str->cm; ctx->x509_store.cm = str->cm; + /* wolfSSL_CTX_set_cert_store() (this function) associates str with the + * wolfSSL_CTX. It is clear that this is a TLS use case which means we + * should move all the certs, if any, into the CertMgr and set + * str->certs to NULL as that will allow the certs to be properly + * processed. */ + if (str->certs != NULL) { + while (wolfSSL_sk_X509_num(str->certs) > 0) { + x = wolfSSL_sk_X509_pop(str->certs); + X509StoreAddCa(str, x, WOLFSSL_USER_CA); + } + wolfSSL_sk_X509_pop_free(str->certs, NULL); + str->certs = NULL; + } + /* free existing store if it exists */ wolfSSL_X509_STORE_free(ctx->x509_store_pt); ctx->x509_store.cache = str->cache; diff --git a/src/x509_str.c b/src/x509_str.c index 9fec690b38..74b3c6c6df 100644 --- a/src/x509_str.c +++ b/src/x509_str.c @@ -34,8 +34,6 @@ #ifdef OPENSSL_EXTRA static int X509StoreGetIssuerEx(WOLFSSL_X509 **issuer, WOLFSSL_STACK *certs, WOLFSSL_X509 *x); -static int X509StoreAddCa(WOLFSSL_X509_STORE* store, - WOLFSSL_X509* x509, int type); #endif /* Based on OpenSSL default max depth */ @@ -1367,8 +1365,7 @@ WOLFSSL_X509_LOOKUP* wolfSSL_X509_STORE_add_lookup(WOLFSSL_X509_STORE* store, return &store->lookup; } -static int X509StoreAddCa(WOLFSSL_X509_STORE* store, - WOLFSSL_X509* x509, int type) +int X509StoreAddCa(WOLFSSL_X509_STORE* store, WOLFSSL_X509* x509, int type) { int result = WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR); DerBuffer* derCert = NULL; diff --git a/tests/api.c b/tests/api.c index 5a1db8a337..2314bff1ee 100644 --- a/tests/api.c +++ b/tests/api.c @@ -28772,6 +28772,47 @@ static int test_wolfSSL_CTX_set_srp_password(void) return EXPECT_RESULT(); } +static int test_wolfSSL_CTX_set_cert_store_null_certs(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_EXTRA) && !defined(NO_RSA) && !defined(NO_TLS) && \ + !defined(NO_WOLFSSL_SERVER) + X509_STORE *store = NULL; + WOLFSSL_CTX *ctx = NULL; + WOLFSSL_METHOD *method = NULL; + X509 *cert = NULL; + const char caCert[] = "./certs/ca-cert.pem"; + + /* Create a new X509_STORE */ + ExpectNotNull(store = X509_STORE_new()); + + /* Load a certificate */ + ExpectNotNull(cert = wolfSSL_X509_load_certificate_file(caCert, + SSL_FILETYPE_PEM)); + + /* Add the certificate to the store */ + ExpectIntEQ(X509_STORE_add_cert(store, cert), SSL_SUCCESS); + ExpectNotNull(store->certs); + + /* Create a new SSL_CTX */ + ExpectNotNull(method = wolfSSLv23_server_method()); + ExpectNotNull(ctx = wolfSSL_CTX_new(method)); + + /* Set the store in the SSL_CTX */ + wolfSSL_CTX_set_cert_store(ctx, store); + + /* Verify that the certs member of the store is null */ + ExpectNull(store->certs); + + /* Clean up */ + wolfSSL_CTX_free(ctx); + X509_free(cert); + +#endif + return EXPECT_RESULT(); +} + + static int test_wolfSSL_X509_STORE(void) { EXPECT_DECLS; @@ -67534,6 +67575,7 @@ TEST_CASE testCases[] = { TEST_DECL(test_wolfSSL_X509_VERIFY_PARAM_set1_ip), TEST_DECL(test_wolfSSL_X509_STORE_CTX_get0_store), TEST_DECL(test_wolfSSL_X509_STORE), + TEST_DECL(test_wolfSSL_CTX_set_cert_store_null_certs), TEST_DECL(test_wolfSSL_X509_STORE_load_locations), TEST_DECL(test_X509_STORE_get0_objects), TEST_DECL(test_wolfSSL_X509_load_crl_file), diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 52a075efd8..797a703d72 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -2781,6 +2781,11 @@ WOLFSSL_LOCAL int X509StoreLoadCertBuffer(WOLFSSL_X509_STORE *str, byte *buf, word32 bufLen, int type); #endif /* !defined NO_CERTS */ +#ifdef OPENSSL_EXTRA +WOLFSSL_LOCAL int X509StoreAddCa(WOLFSSL_X509_STORE* store, + WOLFSSL_X509* x509, int type); +#endif + /* wolfSSL Sock Addr */ struct WOLFSSL_SOCKADDR { unsigned int sz; /* sockaddr size */ From 5e781221a02bd8f584e01d8905f20784d630f6ef Mon Sep 17 00:00:00 2001 From: Anthony Hu Date: Thu, 15 May 2025 15:00:38 -0400 Subject: [PATCH 2/4] WOLFSSL_WPAS_SMALL needs this as well. --- src/x509.c | 21 ++++++++++++--------- src/x509_str.c | 42 ++++++++++++++++++++++-------------------- wolfssl/internal.h | 2 +- wolfssl/ssl.h | 2 +- 4 files changed, 36 insertions(+), 31 deletions(-) diff --git a/src/x509.c b/src/x509.c index 2dbc7587cd..ca6467e895 100644 --- a/src/x509.c +++ b/src/x509.c @@ -4173,6 +4173,16 @@ byte* wolfSSL_X509_get_hw_serial_number(WOLFSSL_X509* x509,byte* in, #endif /* WOLFSSL_SEP */ #endif /* OPENSSL_EXTRA */ + + +#if defined(OPENSSL_EXTRA) || defined(WOLFSSL_WPAS_SMALL) +/* Return and remove the last x509 pushed on stack */ +WOLFSSL_X509* wolfSSL_sk_X509_pop(WOLF_STACK_OF(WOLFSSL_X509_NAME)* sk) +{ + return (WOLFSSL_X509*)wolfSSL_sk_pop(sk); +} +#endif /* OPENSSL_EXTRA || WOLFSSL_WPAS_SMALL */ + /* require OPENSSL_EXTRA since wolfSSL_X509_free is wrapped by OPENSSL_EXTRA */ #if defined(OPENSSL_EXTRA) @@ -4211,13 +4221,6 @@ int wolfSSL_sk_X509_push(WOLF_STACK_OF(WOLFSSL_X509_NAME)* sk, return wolfSSL_sk_push(sk, x509); } - -/* Return and remove the last x509 pushed on stack */ -WOLFSSL_X509* wolfSSL_sk_X509_pop(WOLF_STACK_OF(WOLFSSL_X509_NAME)* sk) -{ - return (WOLFSSL_X509*)wolfSSL_sk_pop(sk); -} - /* Getter function for WOLFSSL_X509 pointer * * sk is the stack to retrieve pointer from @@ -14059,7 +14062,7 @@ WOLFSSL_X509_CRL *wolfSSL_X509_OBJECT_get0_X509_CRL(WOLFSSL_X509_OBJECT *obj) * HAVE_SBLIM_SFCB)) */ -#if defined(OPENSSL_EXTRA) +#if defined(OPENSSL_EXTRA) || defined(WOLFSSL_WPAS_SMALL) int wolfSSL_sk_X509_num(const WOLF_STACK_OF(WOLFSSL_X509) *s) { @@ -14070,7 +14073,7 @@ int wolfSSL_sk_X509_num(const WOLF_STACK_OF(WOLFSSL_X509) *s) return (int)s->num; } -#endif /* OPENSSL_EXTRA */ +#endif /* OPENSSL_EXTRA || WOLFSSL_WPAS_SMALL */ #ifdef HAVE_EX_DATA_CRYPTO int wolfSSL_X509_get_ex_new_index(int idx, void *arg, diff --git a/src/x509_str.c b/src/x509_str.c index 74b3c6c6df..7d72ffddd7 100644 --- a/src/x509_str.c +++ b/src/x509_str.c @@ -1319,6 +1319,28 @@ int wolfSSL_X509_STORE_set_ex_data_with_cleanup( #endif /* OPENSSL_EXTRA || HAVE_WEBSERVER || WOLFSSL_WPAS_SMALL */ +#if defined(OPENSSL_EXTRA) || defined(WOLFSSL_WPAS_SMALL) +int X509StoreAddCa(WOLFSSL_X509_STORE* store, WOLFSSL_X509* x509, int type) +{ + int result = WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR); + DerBuffer* derCert = NULL; + + WOLFSSL_ENTER("X509StoreAddCa"); + if (store != NULL && x509 != NULL && x509->derCert != NULL) { + result = AllocDer(&derCert, x509->derCert->length, + x509->derCert->type, NULL); + if (result == 0) { + /* AddCA() frees the buffer. */ + XMEMCPY(derCert->buffer, + x509->derCert->buffer, x509->derCert->length); + result = AddCA(store->cm, &derCert, type, VERIFY); + } + } + + return result; +} +#endif /* OPENSSL_EXTRA || WOLFSSL_WPAS_SMALL */ + #ifdef OPENSSL_EXTRA #if defined(WOLFSSL_QT) || defined(OPENSSL_ALL) @@ -1365,26 +1387,6 @@ WOLFSSL_X509_LOOKUP* wolfSSL_X509_STORE_add_lookup(WOLFSSL_X509_STORE* store, return &store->lookup; } -int X509StoreAddCa(WOLFSSL_X509_STORE* store, WOLFSSL_X509* x509, int type) -{ - int result = WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR); - DerBuffer* derCert = NULL; - - WOLFSSL_ENTER("X509StoreAddCa"); - if (store != NULL && x509 != NULL && x509->derCert != NULL) { - result = AllocDer(&derCert, x509->derCert->length, - x509->derCert->type, NULL); - if (result == 0) { - /* AddCA() frees the buffer. */ - XMEMCPY(derCert->buffer, - x509->derCert->buffer, x509->derCert->length); - result = AddCA(store->cm, &derCert, type, VERIFY); - } - } - - return result; -} - int wolfSSL_X509_STORE_add_cert(WOLFSSL_X509_STORE* store, WOLFSSL_X509* x509) { diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 797a703d72..d1ae5c638e 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -2781,7 +2781,7 @@ WOLFSSL_LOCAL int X509StoreLoadCertBuffer(WOLFSSL_X509_STORE *str, byte *buf, word32 bufLen, int type); #endif /* !defined NO_CERTS */ -#ifdef OPENSSL_EXTRA +#if defined(OPENSSL_EXTRA) || defined(WOLFSSL_WPAS_SMALL) WOLFSSL_LOCAL int X509StoreAddCa(WOLFSSL_X509_STORE* store, WOLFSSL_X509* x509, int type); #endif diff --git a/wolfssl/ssl.h b/wolfssl/ssl.h index 908d5c6e82..2e77bc1114 100644 --- a/wolfssl/ssl.h +++ b/wolfssl/ssl.h @@ -5324,7 +5324,6 @@ WOLFSSL_API int wolfSSL_CIPHER_get_bits(const WOLFSSL_CIPHER *c, int *alg_bits); WOLFSSL_API WOLFSSL_STACK* wolfSSL_sk_X509_new( WOLF_SK_COMPARE_CB(WOLFSSL_X509, cb)); WOLFSSL_API WOLFSSL_STACK* wolfSSL_sk_X509_new_null(void); -WOLFSSL_API int wolfSSL_sk_X509_num(const WOLF_STACK_OF(WOLFSSL_X509) *s); WOLFSSL_API WOLFSSL_STACK* wolfSSL_sk_X509_OBJECT_new(void); WOLFSSL_API void wolfSSL_sk_X509_OBJECT_free(WOLFSSL_STACK* s); @@ -5408,6 +5407,7 @@ WOLFSSL_API int wolfSSL_i2d_ASN1_BIT_STRING(const WOLFSSL_ASN1_BIT_STRING* bstr, unsigned char** pp); WOLFSSL_API WOLFSSL_ASN1_BIT_STRING* wolfSSL_d2i_ASN1_BIT_STRING( WOLFSSL_ASN1_BIT_STRING** out, const byte** src, long len); +WOLFSSL_API int wolfSSL_sk_X509_num(const WOLF_STACK_OF(WOLFSSL_X509) *s); #endif /* OPENSSL_EXTRA || WOLFSSL_WPAS_SMALL */ WOLFSSL_API int wolfSSL_version(WOLFSSL* ssl); From a9dd00899a7d800b5152b7f0d85698409cc10f49 Mon Sep 17 00:00:00 2001 From: Anthony Hu Date: Fri, 6 Jun 2025 16:23:37 -0400 Subject: [PATCH 3/4] Update src/ssl.c Don't ignore return code. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/ssl.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/ssl.c b/src/ssl.c index ba3ea3207e..05e6f6d1be 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -12955,7 +12955,12 @@ int wolfSSL_set_compression(WOLFSSL* ssl) if (str->certs != NULL) { while (wolfSSL_sk_X509_num(str->certs) > 0) { x = wolfSSL_sk_X509_pop(str->certs); - X509StoreAddCa(str, x, WOLFSSL_USER_CA); + int ret = X509StoreAddCa(str, x, WOLFSSL_USER_CA); + if (ret != WOLFSSL_SUCCESS) { + WOLFSSL_MSG("Error adding CA certificate to store"); + wolfSSL_X509_free(x); /* Free the certificate to avoid memory leaks */ + break; /* Exit the loop on failure */ + } } wolfSSL_sk_X509_pop_free(str->certs, NULL); str->certs = NULL; From 8468fe3b17a708ca5a70efcfeacd4bc660c80d6b Mon Sep 17 00:00:00 2001 From: Anthony Hu Date: Fri, 6 Jun 2025 16:29:55 -0400 Subject: [PATCH 4/4] C89 --- src/ssl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ssl.c b/src/ssl.c index 05e6f6d1be..42cf528403 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -12954,8 +12954,9 @@ int wolfSSL_set_compression(WOLFSSL* ssl) * processed. */ if (str->certs != NULL) { while (wolfSSL_sk_X509_num(str->certs) > 0) { + int ret; x = wolfSSL_sk_X509_pop(str->certs); - int ret = X509StoreAddCa(str, x, WOLFSSL_USER_CA); + ret = X509StoreAddCa(str, x, WOLFSSL_USER_CA); if (ret != WOLFSSL_SUCCESS) { WOLFSSL_MSG("Error adding CA certificate to store"); wolfSSL_X509_free(x); /* Free the certificate to avoid memory leaks */