Skip to content

Commit 17401da

Browse files
dgarskedgarske
authored
Merge pull request #9678 from cconlon/otherNameSan
Fix GENERAL_NAME memory management for otherName and RID SANs
2 parents 3520b4c + 0f395a5 commit 17401da

2 files changed

Lines changed: 172 additions & 17 deletions

File tree

‎src/x509.c‎

Lines changed: 72 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,6 @@ static int wolfssl_dns_entry_othername_to_gn(DNS_entry* dns,
571571
tag = WOLFSSL_V_ASN1_SEQUENCE;
572572
}
573573

574-
575574
/* Create a WOLFSSL_ASN1_STRING from the DER. */
576575
str = wolfSSL_ASN1_STRING_type_new(tag);
577576
if (str == NULL) {
@@ -584,15 +583,23 @@ static int wolfssl_dns_entry_othername_to_gn(DNS_entry* dns,
584583
if (type == NULL)
585584
goto err;
586585
wolfSSL_ASN1_TYPE_set(type, tag, str);
586+
str = NULL; /* type now owns str */
587+
588+
if (wolfSSL_GENERAL_NAME_set_type(gn, WOLFSSL_GEN_OTHERNAME)
589+
!= WOLFSSL_SUCCESS) {
590+
goto err;
591+
}
587592

588593
/* Store the object and string in general name. */
589594
gn->d.otherName->type_id = obj;
590595
gn->d.otherName->value = type;
596+
type = NULL; /* gn->d.otherName owns type */
591597

592598
ret = 1;
593599
err:
594600
if (ret != 1) {
595601
wolfSSL_ASN1_OBJECT_free(obj);
602+
wolfSSL_ASN1_TYPE_free(type);
596603
wolfSSL_ASN1_STRING_free(str);
597604
}
598605
return ret;
@@ -602,30 +609,32 @@ static int wolfssl_dns_entry_othername_to_gn(DNS_entry* dns,
602609
#if defined(OPENSSL_ALL) || defined(OPENSSL_EXTRA)
603610
static int DNS_to_GENERAL_NAME(WOLFSSL_GENERAL_NAME* gn, DNS_entry* dns)
604611
{
605-
gn->type = dns->type;
606-
switch (gn->type) {
612+
switch (dns->type) {
607613
case WOLFSSL_GEN_OTHERNAME:
608-
if (!wolfssl_dns_entry_othername_to_gn(dns, gn)) {
609-
WOLFSSL_MSG("OTHERNAME set failed");
610-
return WOLFSSL_FAILURE;
611-
}
614+
/* Sets gn->type internally */
615+
if (!wolfssl_dns_entry_othername_to_gn(dns, gn)) {
616+
WOLFSSL_MSG("OTHERNAME set failed");
617+
return WOLFSSL_FAILURE;
618+
}
612619
break;
613620

614621
case WOLFSSL_GEN_EMAIL:
615622
case WOLFSSL_GEN_DNS:
616623
case WOLFSSL_GEN_URI:
617624
case WOLFSSL_GEN_IPADD:
618625
case WOLFSSL_GEN_IA5:
619-
gn->d.ia5->length = dns->len;
620-
if (wolfSSL_ASN1_STRING_set(gn->d.ia5, dns->name,
621-
gn->d.ia5->length) != WOLFSSL_SUCCESS) {
622-
WOLFSSL_MSG("ASN1_STRING_set failed");
623-
return WOLFSSL_FAILURE;
624-
}
625-
break;
626+
gn->type = dns->type;
627+
gn->d.ia5->length = dns->len;
628+
if (wolfSSL_ASN1_STRING_set(gn->d.ia5, dns->name,
629+
gn->d.ia5->length) != WOLFSSL_SUCCESS) {
630+
WOLFSSL_MSG("ASN1_STRING_set failed");
631+
return WOLFSSL_FAILURE;
632+
}
633+
break;
626634

627635

628636
case WOLFSSL_GEN_DIRNAME:
637+
gn->type = dns->type;
629638
/* wolfSSL_GENERAL_NAME_new() mallocs this by default */
630639
wolfSSL_ASN1_STRING_free(gn->d.ia5);
631640
gn->d.ia5 = NULL;
@@ -636,6 +645,7 @@ static int DNS_to_GENERAL_NAME(WOLFSSL_GENERAL_NAME* gn, DNS_entry* dns)
636645

637646
#ifdef WOLFSSL_RID_ALT_NAME
638647
case WOLFSSL_GEN_RID:
648+
gn->type = dns->type;
639649
/* wolfSSL_GENERAL_NAME_new() mallocs this by default */
640650
wolfSSL_ASN1_STRING_free(gn->d.ia5);
641651
gn->d.ia5 = NULL;
@@ -2310,9 +2320,9 @@ void* wolfSSL_X509_get_ext_d2i(const WOLFSSL_X509* x509, int nid, int* c,
23102320
goto err;
23112321
}
23122322

2313-
gn->type = dns->type;
2314-
switch (gn->type) {
2323+
switch (dns->type) {
23152324
case ASN_DIR_TYPE:
2325+
gn->type = dns->type;
23162326
{
23172327
int localIdx = 0;
23182328
unsigned char* n = (unsigned char*)XMALLOC(
@@ -2336,12 +2346,14 @@ void* wolfSSL_X509_get_ext_d2i(const WOLFSSL_X509* x509, int nid, int* c,
23362346
break;
23372347

23382348
case ASN_OTHER_TYPE:
2349+
/* gn->type set internally */
23392350
if (!wolfssl_dns_entry_othername_to_gn(dns, gn)) {
23402351
goto err;
23412352
}
23422353
break;
23432354

23442355
case ASN_IP_TYPE:
2356+
gn->type = dns->type;
23452357
if (wolfSSL_ASN1_STRING_set(gn->d.iPAddress,
23462358
dns->name, dns->len) != WOLFSSL_SUCCESS) {
23472359
WOLFSSL_MSG("ASN1_STRING_set failed");
@@ -2350,7 +2362,35 @@ void* wolfSSL_X509_get_ext_d2i(const WOLFSSL_X509* x509, int nid, int* c,
23502362
gn->d.iPAddress->type = WOLFSSL_V_ASN1_OCTET_STRING;
23512363
break;
23522364

2365+
#ifdef WOLFSSL_RID_ALT_NAME
2366+
case ASN_RID_TYPE:
2367+
gn->type = dns->type;
2368+
/* Free ia5 before using union for registeredID */
2369+
wolfSSL_ASN1_STRING_free(gn->d.ia5);
2370+
gn->d.ia5 = NULL;
2371+
2372+
gn->d.registeredID = wolfSSL_ASN1_OBJECT_new();
2373+
if (gn->d.registeredID == NULL) {
2374+
goto err;
2375+
}
2376+
gn->d.registeredID->obj =
2377+
(const unsigned char*)XMALLOC(dns->len,
2378+
gn->d.registeredID->heap, DYNAMIC_TYPE_ASN1);
2379+
if (gn->d.registeredID->obj == NULL) {
2380+
goto err;
2381+
}
2382+
gn->d.registeredID->dynamic |=
2383+
WOLFSSL_ASN1_DYNAMIC_DATA;
2384+
XMEMCPY((byte*)gn->d.registeredID->obj,
2385+
dns->ridString, dns->len);
2386+
gn->d.registeredID->objSz = dns->len;
2387+
gn->d.registeredID->grp = oidCertExtType;
2388+
gn->d.registeredID->nid = WC_NID_registeredAddress;
2389+
break;
2390+
#endif /* WOLFSSL_RID_ALT_NAME */
2391+
23532392
default:
2393+
gn->type = dns->type;
23542394
if (wolfSSL_ASN1_STRING_set(gn->d.dNSName,
23552395
dns->name, dns->len) != WOLFSSL_SUCCESS) {
23562396
WOLFSSL_MSG("ASN1_STRING_set failed");
@@ -4643,7 +4683,12 @@ int wolfSSL_GENERAL_NAME_set0_othername(WOLFSSL_GENERAL_NAME* gen,
46434683
return WOLFSSL_FAILURE;
46444684
}
46454685

4646-
gen->type = WOLFSSL_GEN_OTHERNAME;
4686+
if (wolfSSL_GENERAL_NAME_set_type(gen, WOLFSSL_GEN_OTHERNAME)
4687+
!= WOLFSSL_SUCCESS) {
4688+
wolfSSL_ASN1_OBJECT_free(x);
4689+
return WOLFSSL_FAILURE;
4690+
}
4691+
46474692
gen->d.otherName->type_id = x;
46484693
gen->d.otherName->value = value;
46494694
return WOLFSSL_SUCCESS;
@@ -4975,6 +5020,16 @@ int wolfSSL_GENERAL_NAME_set_type(WOLFSSL_GENERAL_NAME* name, int typ)
49755020
if (name->d.uniformResourceIdentifier == NULL)
49765021
ret = MEMORY_E;
49775022
break;
5023+
case WOLFSSL_GEN_OTHERNAME:
5024+
name->d.otherName = (WOLFSSL_ASN1_OTHERNAME*)XMALLOC(
5025+
sizeof(WOLFSSL_ASN1_OTHERNAME), NULL, DYNAMIC_TYPE_ASN1);
5026+
if (name->d.otherName == NULL) {
5027+
ret = MEMORY_E;
5028+
}
5029+
else {
5030+
XMEMSET(name->d.otherName, 0, sizeof(WOLFSSL_ASN1_OTHERNAME));
5031+
}
5032+
break;
49785033
default:
49795034
name->type = WOLFSSL_GEN_IA5;
49805035
name->d.ia5 = wolfSSL_ASN1_STRING_new();

‎tests/api.c‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15548,6 +15548,104 @@ static int test_GENERAL_NAME_set0_othername(void)
1554815548
return EXPECT_RESULT();
1554915549
}
1555015550

15551+
/* Test RID (Registered ID) GENERAL_NAME creation and freeing */
15552+
static int test_RID_GENERAL_NAME_free(void)
15553+
{
15554+
EXPECT_DECLS;
15555+
#if defined(OPENSSL_ALL) && defined(WOLFSSL_RID_ALT_NAME)
15556+
/* RID OID: 1.2.3.4.5 in DER */
15557+
const unsigned char ridData[] = { 0x06, 0x04, 0x2a, 0x03, 0x04, 0x05 };
15558+
const unsigned char* p = ridData;
15559+
GENERAL_NAME* gn = NULL;
15560+
GENERAL_NAMES* gns = NULL;
15561+
ASN1_OBJECT* ridObj = NULL;
15562+
15563+
/* Create RID ASN1_OBJECT from DER */
15564+
ExpectNotNull(ridObj = wolfSSL_d2i_ASN1_OBJECT(NULL, &p, sizeof(ridData)));
15565+
15566+
/* Create GENERAL_NAME and set up as RID */
15567+
ExpectNotNull(gn = GENERAL_NAME_new());
15568+
if (gn != NULL) {
15569+
/* GENERAL_NAME_new allocates ia5, must free before using as RID */
15570+
gn->type = GEN_RID;
15571+
wolfSSL_ASN1_STRING_free(gn->d.ia5);
15572+
gn->d.ia5 = NULL;
15573+
gn->d.registeredID = ridObj;
15574+
ridObj = NULL; /* gn owns */
15575+
}
15576+
if (EXPECT_FAIL()) {
15577+
wolfSSL_ASN1_OBJECT_free(ridObj);
15578+
}
15579+
15580+
/* Add to stack */
15581+
ExpectNotNull(gns = sk_GENERAL_NAME_new(NULL));
15582+
ExpectIntEQ(sk_GENERAL_NAME_push(gns, gn), 1);
15583+
if (EXPECT_FAIL()) {
15584+
GENERAL_NAME_free(gn);
15585+
gn = NULL;
15586+
}
15587+
15588+
/* Verify RID is set up correctly */
15589+
ExpectNotNull(gn = sk_GENERAL_NAME_value(gns, 0));
15590+
ExpectIntEQ(gn->type, GEN_RID);
15591+
ExpectNotNull(gn->d.registeredID);
15592+
15593+
/* Free via sk_GENERAL_NAME_pop_free, exercises type_free for RID */
15594+
sk_GENERAL_NAME_pop_free(gns, GENERAL_NAME_free);
15595+
#endif
15596+
return EXPECT_RESULT();
15597+
}
15598+
15599+
/* Test RID (Registered ID) SAN parsing via X509_get_ext_d2i().
15600+
* Uses rid-cert.der which contains a RID SAN with OID 1.2.3.4.5. This tests
15601+
* that ASN_RID_TYPE case in wolfSSL_X509_get_ext_d2i() frees ia5 before
15602+
* allocating registeredID. */
15603+
static int test_RID_X509_get_ext_d2i(void)
15604+
{
15605+
EXPECT_DECLS;
15606+
#if defined(OPENSSL_ALL) && defined(WOLFSSL_RID_ALT_NAME) && \
15607+
!defined(NO_RSA) && !defined(NO_FILESYSTEM)
15608+
int i;
15609+
int numNames;
15610+
int foundRID = 0;
15611+
const char* ridCert = "./certs/rid-cert.der";
15612+
X509* x509 = NULL;
15613+
GENERAL_NAMES* gns = NULL;
15614+
GENERAL_NAME* gn = NULL;
15615+
XFILE f = XBADFILE;
15616+
15617+
ExpectTrue((f = XFOPEN(ridCert, "rb")) != XBADFILE);
15618+
ExpectNotNull(x509 = d2i_X509_fp(f, NULL));
15619+
if (f != XBADFILE) {
15620+
XFCLOSE(f);
15621+
f = XBADFILE;
15622+
}
15623+
15624+
/* Get SANs, will exercise ASN_RID_TYPE case */
15625+
ExpectNotNull(gns = (GENERAL_NAMES*)X509_get_ext_d2i(x509,
15626+
NID_subject_alt_name, NULL, NULL));
15627+
15628+
/* rid-cert.der contains: UPN, RID (1.2.3.4.5), DNS, URI, othername */
15629+
numNames = sk_GENERAL_NAME_num(gns);
15630+
ExpectIntGE(numNames, 2);
15631+
15632+
for (i = 0; i < numNames; i++) {
15633+
gn = sk_GENERAL_NAME_value(gns, i);
15634+
if (gn != NULL && gn->type == GEN_RID) {
15635+
ExpectNotNull(gn->d.registeredID);
15636+
foundRID = 1;
15637+
break;
15638+
}
15639+
}
15640+
ExpectIntEQ(foundRID, 1);
15641+
15642+
/* Free via sk_GENERAL_NAME_pop_free, exercises type_free for RID */
15643+
sk_GENERAL_NAME_pop_free(gns, GENERAL_NAME_free);
15644+
X509_free(x509);
15645+
#endif
15646+
return EXPECT_RESULT();
15647+
}
15648+
1555115649
/* Note the lack of wolfSSL_ prefix...this is a compatibility layer test. */
1555215650
static int test_othername_and_SID_ext(void)
1555315651
{
@@ -31205,6 +31303,8 @@ TEST_CASE testCases[] = {
3120531303
TEST_OSSL_X509_LOOKUP_DECLS,
3120631304

3120731305
TEST_DECL(test_GENERAL_NAME_set0_othername),
31306+
TEST_DECL(test_RID_GENERAL_NAME_free),
31307+
TEST_DECL(test_RID_X509_get_ext_d2i),
3120831308
TEST_DECL(test_othername_and_SID_ext),
3120931309
TEST_DECL(test_wolfSSL_dup_CA_list),
3121031310
/* OpenSSL sk_X509 API test */

0 commit comments

Comments
 (0)