Store an alt name with its entry in one allocation

src/x509.c

@@ -3119,33 +3119,24 @@ int wolfSSL_X509_add_altname_ex(WOLFSSL_X509* x509, const char* name,
         word32 nameSz, int type)
 {
     DNS_entry* newAltName = NULL;
-    char* nameCopy = NULL;
 
     if (x509 == NULL)
         return WOLFSSL_FAILURE;
 
     if ((name == NULL) || (nameSz == 0))
         return WOLFSSL_SUCCESS;
 
-    newAltName = AltNameNew(x509->heap);
-    if (newAltName == NULL)
+    /* AltNameNewEx() takes a signed length. */
+    if (nameSz > (word32)INT_MAX)
         return WOLFSSL_FAILURE;
 
-    nameCopy = (char*)XMALLOC(nameSz + 1, x509->heap, DYNAMIC_TYPE_ALTNAME);
-    if (nameCopy == NULL) {
-        XFREE(newAltName, x509->heap, DYNAMIC_TYPE_ALTNAME);
+    /* One block, so nothing to unwind on failure. */
+    newAltName = AltNameNewEx(name, (int)nameSz, x509->heap);
+    if (newAltName == NULL)
         return WOLFSSL_FAILURE;
-    }
-
-    XMEMCPY(nameCopy, name, nameSz);
-
-    nameCopy[nameSz] = '\0';
 
     newAltName->next = x509->altNames;
     newAltName->type = type;
-    newAltName->len = (int)nameSz;
-    newAltName->name = nameCopy;
-    newAltName->nameStored = 1;
     x509->altNames = newAltName;
 
     return WOLFSSL_SUCCESS;

tests/api/test_asn.c

@@ -5072,3 +5072,52 @@ int test_wc_AsnFeatureCoverage(void)
 #endif /* !NO_ASN && HAVE_ECC && USE_CERT_BUFFERS_256 && !HAVE_FIPS */
     return EXPECT_RESULT();
 }
+
+/* AltNameNewEx() stores the name inside the entry's own allocation, so
+ * FreeAltNames() releases both with one free. Check the copy, the length, the
+ * terminator, and that a NULL or empty name still yields a usable entry.
+ */
+int test_wc_AltNameNewEx(void)
+{
+    EXPECT_DECLS;
+#if !defined(NO_ASN) && !defined(NO_CERTS) && \
+    (defined(WOLFSSL_TEST_CERT) || defined(OPENSSL_EXTRA) || \
+     defined(OPENSSL_EXTRA_X509_SMALL) || defined(WOLFSSL_PUBLIC_ASN))
+    const char  name[] = "example.com";
+    DNS_entry*  entry = NULL;
+
+    ExpectNotNull(entry = AltNameNewEx(name, (int)XSTRLEN(name), NULL));
+    if (entry != NULL) {
+        ExpectIntEQ(entry->len, (int)XSTRLEN(name));
+        ExpectNotNull(entry->name);
+        ExpectIntEQ(XMEMCMP(entry->name, name, XSTRLEN(name)), 0);
+        /* The name is NUL terminated and part of the entry's allocation. */
+        ExpectIntEQ(entry->name[XSTRLEN(name)], '\0');
+        ExpectIntEQ(entry->nameStored, 0);
+        /* The name region sits after the struct in the entry's own block,
+         * which is the property that makes one allocation and one free
+         * correct. */
+        ExpectTrue((const char*)entry < entry->name);
+        ExpectTrue(entry->name <
+            (const char*)entry + sizeof(DNS_entry) + XSTRLEN(name) + 1);
+    }
+    FreeAltNames(entry, NULL);
+    entry = NULL;
+
+    /* A length with no name to go with it is rejected rather than leaving
+     * len covering bytes that were never written, and a negative length is
+     * rejected rather than used as a size. */
+    ExpectNull(AltNameNewEx(NULL, 1, NULL));
+    ExpectNull(AltNameNewEx(name, -1, NULL));
+
+    /* An empty name is still a valid entry with a terminated string. */
+    ExpectNotNull(entry = AltNameNewEx(NULL, 0, NULL));
+    if (entry != NULL) {
+        ExpectIntEQ(entry->len, 0);
+        ExpectNotNull(entry->name);
+        ExpectIntEQ(entry->name[0], '\0');
+    }
+    FreeAltNames(entry, NULL);
+#endif
+    return EXPECT_RESULT();
+}

tests/api/test_asn.h

@@ -59,6 +59,7 @@ int test_wc_DecodeExtKeyUsage_ssh_oid_collision(void);
 int test_wc_DecodeExtKeyUsage_oidSumCollision(void);
 int test_wc_AsnDecisionCoverage(void);
 int test_wc_AsnFeatureCoverage(void);
+int test_wc_AltNameNewEx(void);
 
 #define TEST_ASN_DECLS                                              \
     TEST_DECL_GROUP("asn", test_SetAsymKeyDer),                     \
@@ -95,6 +96,7 @@ int test_wc_AsnFeatureCoverage(void);
     TEST_DECL_GROUP("asn", test_wc_DecodeExtKeyUsage_ssh_oid_collision), \
     TEST_DECL_GROUP("asn", test_wc_DecodeExtKeyUsage_oidSumCollision), \
     TEST_DECL_GROUP("asn", test_wc_AsnDecisionCoverage),           \
-    TEST_DECL_GROUP("asn", test_wc_AsnFeatureCoverage)
+    TEST_DECL_GROUP("asn", test_wc_AsnFeatureCoverage),             \
+    TEST_DECL_GROUP("asn", test_wc_AltNameNewEx)
 
 #endif /* WOLFCRYPT_TEST_ASN_H */

wolfcrypt/src/asn.c

@@ -13134,6 +13134,43 @@ void FreeAltNames(DNS_entry* altNames, void* heap)
     }
 }
 
+/* Allocate an alt name entry holding a NUL terminated copy of the name in the
+ * same block. nameStored stays 0, so the name is never freed on its own.
+ *
+ * str     Name to store. May be NULL only when strLen is 0.
+ * strLen  Length of the name in bytes, zero or more.
+ * heap    Heap hint for the allocation.
+ * returns the new alt name, or NULL on failure.
+ */
+DNS_entry* AltNameNewEx(const char* str, int strLen, void* heap)
+{
+    DNS_entry* ret;
+    char* name;
+
+    /* len would otherwise cover bytes that were never written. */
+    if ((strLen < 0) || ((str == NULL) && (strLen > 0)))
+        return NULL;
+
+    ret = (DNS_entry*)XMALLOC(sizeof(DNS_entry) + (size_t)strLen + 1, heap,
+                              DYNAMIC_TYPE_ALTNAME);
+    if (ret == NULL)
+        return NULL;
+
+    XMEMSET(ret, 0, sizeof(DNS_entry));
+    name = (char*)ret + sizeof(DNS_entry);
+    if ((str != NULL) && (strLen > 0))
+        XMEMCPY(name, str, (size_t)strLen);
+    name[strLen] = '\0';
+    ret->name = name;
+    ret->len = strLen;
+#ifdef WC_ASN_NO_HEAP
+    ret->entryStored = 1;   /* heap-allocated node; FreeAltNames frees it */
+#endif
+
+    (void)heap;
+    return ret;
+}
+
 /* malloc and initialize a new alt name structure */
 DNS_entry* AltNameNew(void* heap)
 {
@@ -15269,11 +15306,8 @@ static int SetDNSEntry(void* heap, DNS_entry* pool, word32* poolUsed,
                        const char* str, int strLen, int type,
                        DNS_entry** entries)
 {
-    DNS_entry* dnsEntry;
+    DNS_entry* dnsEntry = NULL;
     int ret = 0;
-#ifndef WC_ASN_NO_HEAP
-    char *dnsEntry_name = NULL;
-#endif
 
 #ifdef WC_ASN_NO_HEAP
     /* No heap: borrow a pool slot; name points into the source DER. */
@@ -15309,31 +15343,20 @@ static int SetDNSEntry(void* heap, DNS_entry* pool, word32* poolUsed,
 #else
     (void)pool;
     (void)poolUsed;
-    /* TODO: consider one malloc. */
-    /* Allocate DNS Entry object. */
-    dnsEntry = AltNameNew(heap);
-    if (dnsEntry == NULL) {
-        ret = MEMORY_E;
+    /* Screened here so a NULL from AltNameNewEx() means out of memory and
+     * nothing else. */
+    if ((strLen < 0) || ((str == NULL) && (strLen > 0))) {
+        ret = BAD_FUNC_ARG;
     }
     if (ret == 0) {
-        /* Allocate DNS Entry name - length of string plus 1 for NUL. */
-        dnsEntry->name = dnsEntry_name = (char*)XMALLOC((size_t)strLen + 1,
-                                                    heap, DYNAMIC_TYPE_ALTNAME);
-        if (dnsEntry->name == NULL) {
+        /* Allocate DNS Entry object holding the name. */
+        dnsEntry = AltNameNewEx(str, strLen, heap);
+        if (dnsEntry == NULL) {
             ret = MEMORY_E;
         }
-        else {
-            dnsEntry->nameStored = 1;
-        }
     }
     if (ret == 0) {
-        /* Set tag type, name length, name and NUL terminate name. */
         dnsEntry->type = type;
-        dnsEntry->len = strLen;
-        if (str != NULL && strLen > 0) {
-            XMEMCPY(dnsEntry_name, str, (size_t)strLen);
-        }
-        dnsEntry_name[strLen] = '\0';
 
 #ifdef WOLFSSL_RID_ALT_NAME
         /* store registeredID as a string */
@@ -15348,12 +15371,16 @@ static int SetDNSEntry(void* heap, DNS_entry* pool, word32* poolUsed,
 #endif
     if (ret == 0) {
         ret = AddDNSEntryToList(entries, dnsEntry);
+        /* The list owns it from here. */
+        if (ret == 0)
+            dnsEntry = NULL;
     }
 
-    /* failure cleanup */
+    /* Only reached when the entry was never linked. Clear next so
+     * FreeAltNames() frees this node alone. */
     if (ret != 0 && dnsEntry != NULL) {
-        XFREE(dnsEntry_name, heap, DYNAMIC_TYPE_ALTNAME);
-        XFREE(dnsEntry, heap, DYNAMIC_TYPE_ALTNAME);
+        dnsEntry->next = NULL;
+        FreeAltNames(dnsEntry, heap);
     }
 #endif
 

wolfssl/wolfcrypt/asn.h

@@ -1548,8 +1548,12 @@ struct DNS_entry {
     int        type;   /* i.e. ASN_DNS_TYPE */
     int        len;    /* actual DNS len */
     const char*
-               name;   /* actual DNS name; under WC_ASN_NO_HEAP this points into
-                        * the source DER and is NOT NUL-terminated - use len */
+               name;   /* actual DNS name; under WC_ASN_NO_HEAP a parsed entry
+                        * points into the source DER and is NOT NUL-terminated
+                        * - use len */
+    /* 1 = name is its own allocation and FreeAltNames() releases it.
+     * 0 = name is borrowed from the DER, or inside the entry's own
+     * block. Either way, never free it on its own. */
     int        nameStored;
 #ifdef WOLFSSL_IP_ALT_NAME
     char*      ipString; /* human readable form of IP address */
@@ -2438,6 +2442,7 @@ typedef enum MimeStatus
 #ifdef WOLFSSL_API_PREFIX_MAP
     #define FreeAltNames wc_FreeAltNames
     #define AltNameNew wc_AltNameNew
+    #define AltNameNewEx wc_AltNameNewEx
     #define AltNameDup wc_AltNameDup
     #ifndef IGNORE_NAME_CONSTRAINTS
         #define FreeNameSubtrees wc_FreeNameSubtrees
@@ -2498,6 +2503,8 @@ WOLFSSL_LOCAL int StreamOctetString(const byte* inBuf, word32 inBufSz,
 
 WOLFSSL_ASN_API void FreeAltNames(DNS_entry* altNames, void* heap);
 WOLFSSL_ASN_API DNS_entry* AltNameNew(void* heap);
+WOLFSSL_ASN_API DNS_entry* AltNameNewEx(const char* str, int strLen,
+                                        void* heap);
 WOLFSSL_ASN_API DNS_entry* AltNameDup(DNS_entry* from, void* heap);
 #if defined(WOLFSSL_ASN_TEMPLATE) && defined(WOLFSSL_CERT_GEN) && \
     defined(WOLFSSL_ALT_NAMES)