From a896c16ebd67fbf06d74d603fc6467a86ef85cc2 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Wed, 6 Nov 2024 17:10:54 -0700 Subject: [PATCH 1/2] fix for memory leak due to missed WOLFSSL_GENERAL_NAME capability changes --- src/x509.c | 92 +++++++++++++++++++++++++++++++++-------- tests/api.c | 5 +++ wolfssl/openssl/ssl.h | 2 +- wolfssl/wolfcrypt/asn.h | 1 + 4 files changed, 81 insertions(+), 19 deletions(-) diff --git a/src/x509.c b/src/x509.c index 274c40120a..a74f5093a8 100644 --- a/src/x509.c +++ b/src/x509.c @@ -587,6 +587,76 @@ static int wolfssl_dns_entry_othername_to_gn(DNS_entry* dns, #endif /* OPENSSL_ALL || WOLFSSL_WPAS_SMALL */ #if defined(OPENSSL_ALL) || defined(OPENSSL_EXTRA) +static int DNS_to_GENERAL_NAME(WOLFSSL_GENERAL_NAME* gn, DNS_entry* dns) +{ + gn->type = dns->type; + switch (gn->type) { + case WOLFSSL_GEN_OTHERNAME: + if (!wolfssl_dns_entry_othername_to_gn(dns, gn)) { + WOLFSSL_MSG("OTHERNAME set failed"); + return WOLFSSL_FAILURE; + } + break; + + case WOLFSSL_GEN_EMAIL: + case WOLFSSL_GEN_DNS: + case WOLFSSL_GEN_URI: + case WOLFSSL_GEN_IPADD: + case WOLFSSL_GEN_IA5: + gn->d.ia5->length = dns->len; + if (wolfSSL_ASN1_STRING_set(gn->d.ia5, dns->name, + gn->d.ia5->length) != WOLFSSL_SUCCESS) { + WOLFSSL_MSG("ASN1_STRING_set failed"); + return WOLFSSL_FAILURE; + } + break; + + + case WOLFSSL_GEN_DIRNAME: + /* wolfSSL_GENERAL_NAME_new() mallocs this by default */ + wolfSSL_ASN1_STRING_free(gn->d.ia5); + gn->d.ia5 = NULL; + + gn->d.dirn = wolfSSL_X509_NAME_new();; + /* @TODO extract dir name info from DNS_entry */ + break; + +#ifdef WOLFSSL_RID_ALT_NAME + case WOLFSSL_GEN_RID: + /* wolfSSL_GENERAL_NAME_new() mallocs this by default */ + wolfSSL_ASN1_STRING_free(gn->d.ia5); + gn->d.ia5 = NULL; + + gn->d.registeredID = wolfSSL_ASN1_OBJECT_new(); + if (gn->d.registeredID == NULL) { + return WOLFSSL_FAILURE; + } + gn->d.registeredID->obj = XMALLOC(dns->len, + gn->d.registeredID->heap, DYNAMIC_TYPE_ASN1); + if (gn->d.registeredID->obj == NULL) { + /* registeredID gets free'd up by caller after failure */ + return WOLFSSL_FAILURE; + } + gn->d.registeredID->dynamic |= WOLFSSL_ASN1_DYNAMIC_DATA; + XMEMCPY((byte*)gn->d.registeredID->obj, dns->ridString, dns->len); + gn->d.registeredID->objSz = dns->len; + gn->d.registeredID->grp = oidCertExtType; + gn->d.registeredID->nid = WC_NID_registeredAddress; + break; +#endif + + case WOLFSSL_GEN_X400: + /* Unsupported: fall through */ + case WOLFSSL_GEN_EDIPARTY: + /* Unsupported: fall through */ + default: + WOLFSSL_MSG("Unsupported type conversion"); + return WOLFSSL_FAILURE; + } + return WOLFSSL_SUCCESS; +} + + static int wolfssl_x509_alt_names_to_gn(WOLFSSL_X509* x509, WOLFSSL_X509_EXTENSION* ext) { @@ -624,24 +694,10 @@ static int wolfssl_x509_alt_names_to_gn(WOLFSSL_X509* x509, goto err; } - gn->type = dns->type; - if (gn->type == WOLFSSL_GEN_OTHERNAME) { - if (!wolfssl_dns_entry_othername_to_gn(dns, gn)) { - WOLFSSL_MSG("OTHERNAME set failed"); - wolfSSL_GENERAL_NAME_free(gn); - wolfSSL_sk_pop_free(sk, NULL); - goto err; - } - } - else { - gn->d.ia5->length = dns->len; - if (wolfSSL_ASN1_STRING_set(gn->d.ia5, dns->name, - gn->d.ia5->length) != WOLFSSL_SUCCESS) { - WOLFSSL_MSG("ASN1_STRING_set failed"); - wolfSSL_GENERAL_NAME_free(gn); - wolfSSL_sk_pop_free(sk, NULL); - goto err; - } + if (DNS_to_GENERAL_NAME(gn, dns) != WOLFSSL_SUCCESS) { + wolfSSL_GENERAL_NAME_free(gn); + wolfSSL_sk_pop_free(sk, NULL); + goto err; } if (wolfSSL_sk_GENERAL_NAME_push(sk, gn) <= 0) { diff --git a/tests/api.c b/tests/api.c index c08d0987d1..efd7af41d6 100644 --- a/tests/api.c +++ b/tests/api.c @@ -77998,6 +77998,7 @@ static int test_X509_REQ(void) #ifdef HAVE_ECC const unsigned char* ecPriv = (const unsigned char*)ecc_clikey_der_256; const unsigned char* ecPub = (unsigned char*)ecc_clikeypub_der_256; + BIO* bio = NULL; #endif ExpectNotNull(name = X509_NAME_new()); @@ -78089,6 +78090,10 @@ static int test_X509_REQ(void) /* Signature is random and may be shorter or longer. */ ExpectIntGE((len = i2d_X509_REQ(req, &der)), 245); ExpectIntLE(len, 253); + ExpectNotNull(bio = BIO_new_fp(stderr, BIO_NOCLOSE)); + ExpectIntEQ(X509_REQ_print(bio, req), WOLFSSL_SUCCESS); + ExpectIntEQ(X509_REQ_print(bio, NULL), WOLFSSL_FAILURE); + BIO_free(bio); XFREE(der, NULL, DYNAMIC_TYPE_OPENSSL); X509_REQ_free(req); EVP_PKEY_free(pub); diff --git a/wolfssl/openssl/ssl.h b/wolfssl/openssl/ssl.h index 8133a42ef1..449db9b302 100644 --- a/wolfssl/openssl/ssl.h +++ b/wolfssl/openssl/ssl.h @@ -567,7 +567,7 @@ typedef STACK_OF(ACCESS_DESCRIPTION) AUTHORITY_INFO_ACCESS; #define X509_sign wolfSSL_X509_sign #define X509_sign_ctx wolfSSL_X509_sign_ctx #define X509_print wolfSSL_X509_print -#define X509_REQ_print wolfSSL_X509_print +#define X509_REQ_print wolfSSL_X509_REQ_print #define X509_print_ex wolfSSL_X509_print_ex #define X509_print_fp wolfSSL_X509_print_fp #define X509_CRL_print wolfSSL_X509_CRL_print diff --git a/wolfssl/wolfcrypt/asn.h b/wolfssl/wolfcrypt/asn.h index f9d1eb79cf..d6f63ba7bb 100644 --- a/wolfssl/wolfcrypt/asn.h +++ b/wolfssl/wolfcrypt/asn.h @@ -912,6 +912,7 @@ extern const WOLFSSL_ObjectInfo wolfssl_object_info[]; #define WC_NID_postalCode ASN_POSTAL_CODE /* postalCode */ #define WC_NID_favouriteDrink 462 #define WC_NID_userId 458 +#define WC_NID_registeredAddress 870 #define WC_NID_emailAddress 0x30 /* emailAddress */ #define WC_NID_id_on_dnsSRV 82 /* 1.3.6.1.5.5.7.8.7 */ #define WC_NID_ms_upn 265 /* 1.3.6.1.4.1.311.20.2.3 */ From ce935fddadc0958a7902576c3dcf039a99f62685 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Mon, 11 Nov 2024 09:57:33 -0700 Subject: [PATCH 2/2] cast return of XMALLOC --- src/x509.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/x509.c b/src/x509.c index a74f5093a8..0f39c21f66 100644 --- a/src/x509.c +++ b/src/x509.c @@ -631,7 +631,7 @@ static int DNS_to_GENERAL_NAME(WOLFSSL_GENERAL_NAME* gn, DNS_entry* dns) if (gn->d.registeredID == NULL) { return WOLFSSL_FAILURE; } - gn->d.registeredID->obj = XMALLOC(dns->len, + gn->d.registeredID->obj = (const unsigned char*)XMALLOC(dns->len, gn->d.registeredID->heap, DYNAMIC_TYPE_ASN1); if (gn->d.registeredID->obj == NULL) { /* registeredID gets free'd up by caller after failure */