Skip to content

In wolfSSL_CTX_set_cert_store, send certificates into the CertMgr #8708

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
+103 −33
Open

In wolfSSL_CTX_set_cert_store, send certificates into the CertMgr #8708

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
66dc2b2
In wolfSSL_CTX_set_cert_store, send certificates into the CertMgr
anhu Apr 23, 2025
5e78122
WOLFSSL_WPAS_SMALL needs this as well.
anhu May 15, 2025
a9dd008
Update src/ssl.c
anhu Jun 6, 2025
8468fe3
C89
anhu Jun 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions src/ssl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -12946,6 +12947,26 @@ 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) {
int ret;
x = wolfSSL_sk_X509_pop(str->certs);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cert store has 3 X509 stacks, one for certs, one for trusted, and one named owned. Are we sure that it is the certs stack that should get added as CA's?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, certs should get added but trusted should also.

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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the certs stack need free'd and emptied here? Is it being converted over to a WOLFSSL_CTX cert store, and that future calls to wolfSSL_X509_STORE_add_cert would then load certs as a CA?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also it would be worth checking if this behavior conforms to the expected OpenSSL compat behavior. Attempting untrusted vs trusted self signed certificate loads after the store is set to the WOLFSSL_CTX struct.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a concept of SSL_CTX owned X509_STORE vs non-owned. While confusing, it was done this way to leave all existing code for TLS and CM usage unmodified. To determine if X509_STORE is owned by SSL_CTX or not, we do NULL checks on str->certs. So that is why it needs to be pop_freed here, otherwise logic in X509_STORE wont work right.

str->certs = NULL;
}

/* free existing store if it exists */
wolfSSL_X509_STORE_free(ctx->x509_store_pt);
ctx->x509_store.cache = str->cache;
Expand Down
21 changes: 12 additions & 9 deletions src/x509.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
{
Expand All @@ -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,
Expand Down
45 changes: 22 additions & 23 deletions src/x509_str.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -1321,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)
Expand Down Expand Up @@ -1367,27 +1387,6 @@ 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 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)
{
Expand Down
42 changes: 42 additions & 0 deletions tests/api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was looking over the support case referenced and I don't think this test reflects the edge case reported. It looks like we should be testing the order of :

wolfSSL_CTX_set_cert_store(ctx, store);

then

X509_STORE_add_cert(store, cert)

Where a cert store is assigned to the WOLFSSL_CTX struct and after it is assigned to it there is certificates added.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to be tricky or even impossible to fully support with the current design. X509_STORE owned by SSL_CTX expects everything to be in underlying CM, while X509_STORE not-owned expects them in str->certs. I should say this flow will work for TLS, but setting a X509_STORE inside a SSL_CTX, then expecting direct X509_STORE APIs to work right is likely not going to work with current model.

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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be testing a successful verification making use of the loaded CA's.


/* Clean up */
wolfSSL_CTX_free(ctx);
X509_free(cert);

#endif
return EXPECT_RESULT();
}


static int test_wolfSSL_X509_STORE(void)
{
EXPECT_DECLS;
Expand Down Expand Up @@ -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),
Expand Down
5 changes: 5 additions & 0 deletions wolfssl/internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2781,6 +2781,11 @@ WOLFSSL_LOCAL int X509StoreLoadCertBuffer(WOLFSSL_X509_STORE *str,
byte *buf, word32 bufLen, int type);
#endif /* !defined NO_CERTS */

#if defined(OPENSSL_EXTRA) || defined(WOLFSSL_WPAS_SMALL)
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 */
Expand Down
2 changes: 1 addition & 1 deletion wolfssl/ssl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
Loading