Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions .github/workflows/no-malloc.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
name: No Malloc Tests

# START OF COMMON SECTION
Expand Down Expand Up @@ -74,6 +74,12 @@
"--enable-curve448", "--enable-mlkem", "--enable-staticmemory",
"CFLAGS=-DWOLFSSL_NO_MALLOC -pedantic -Wdeclaration-after-statement -Wnull-dereference -DTEST_LIBWOLFSSL_SOURCES_INCLUSION_SEQUENCE"],
"check": false,
"run": [["./wolfcrypt/test/testwolfcrypt"]]},
{"name": "no-heap-cert", "minutes": 0.8,
"configure": ["--enable-rsa", "--enable-keygen", "--enable-ecc",
"--enable-acert", "--disable-dh", "--disable-filesystem",
"CFLAGS=-DWOLFSSL_NO_MALLOC -DNO_WOLFSSL_MEMORY -DRSA_MIN_SIZE=1024 -DWOLFSSL_TEST_CERT -DUSE_CERT_BUFFERS_2048 -DUSE_CERT_BUFFERS_256 -pedantic -Wdeclaration-after-statement -Wnull-dereference -DTEST_LIBWOLFSSL_SOURCES_INCLUSION_SEQUENCE"],
"check": false,
"run": [["./wolfcrypt/test/testwolfcrypt"]]}
]
EOF
Expand Down
168 changes: 132 additions & 36 deletions wolfcrypt/src/asn.c
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,15 @@ ASN Options:
* WOLFSSL_ALLOW_CRIT_AKID: Allow critical Auth Key Identifier
* WOLFSSL_ALLOW_CRIT_SKID: Allow critical Subject Key Identifier
* WC_ASN_UNKNOWN_EXT_CB: Callback for unknown extensions
* WC_ASN_NO_HEAP: Zero-allocation cert parse: reference key/alt-name
data in the source DER instead of heap copies, so the source buffer must
outlive the DecodedCert. Auto-defined when WOLFSSL_NO_MALLOC and
NO_WOLFSSL_MEMORY are set without XMALLOC_USER or WOLFSSL_STATIC_MEMORY.
Limitation: IP and registeredID SAN entries need a parsed string form that
has no in-place source, so such certs are rejected with ASN_PARSE_E. SAN
DNS_entry.name is NOT NUL-terminated in this mode; only .len is authoritative.
* WC_ASN_MAX_ALTNAMES: No-heap SAN pool slot count (default 8); excess
subject alternative names are rejected.
*
* ASN.1 Parsing:
* WOLFSSL_ASN_ALL: Enable all ASN.1 features
Expand Down Expand Up @@ -12471,7 +12480,14 @@ void FreeAltNames(DNS_entry* altNames, void* heap)
altNames->ridStringStored = 0;
}
#endif
XFREE(altNames, heap, DYNAMIC_TYPE_ALTNAME);
#ifdef WC_ASN_NO_HEAP
/* Only free heap nodes; no-heap pool nodes aren't owned. */
if (altNames->entryStored) {
XFREE(altNames, heap, DYNAMIC_TYPE_ALTNAME);
}
#else
XFREE(altNames, heap, DYNAMIC_TYPE_ALTNAME);
#endif
altNames = tmp;
}
}
Expand All @@ -12483,6 +12499,9 @@ DNS_entry* AltNameNew(void* heap)
ret = (DNS_entry*)XMALLOC(sizeof(DNS_entry), heap, DYNAMIC_TYPE_ALTNAME);
if (ret != NULL) {
XMEMSET(ret, 0, sizeof(DNS_entry));
#ifdef WC_ASN_NO_HEAP
ret->entryStored = 1; /* heap-allocated node; FreeAltNames frees it */
#endif
}
(void)heap;
return ret;
Expand Down Expand Up @@ -12631,7 +12650,9 @@ static int StoreKey(DecodedCert* cert, const byte* source, word32* srcIdx,
{
int ret;
int length;
#ifndef WC_ASN_NO_HEAP
byte* publicKey;
#endif

ret = CheckBitString(source, srcIdx, &length, maxIdx, 1, NULL);
if (ret == 0) {
Expand All @@ -12641,6 +12662,17 @@ static int StoreKey(DecodedCert* cert, const byte* source, word32* srcIdx,
}
if (ret == 0) {
#endif
#ifdef WC_ASN_NO_HEAP
/* No heap: reference the key in place; source must outlive the cert. */
cert->publicKey = (byte*)&source[*srcIdx];
cert->pubKeyStored = 0;
cert->pubKeySize = (word32)length;
#ifdef HAVE_OCSP_RESPONDER
cert->publicKeyForHash = cert->publicKey;
cert->pubKeyForHashSize = cert->pubKeySize;
#endif
*srcIdx += (word32)length;
#else
publicKey = (byte*)XMALLOC((size_t)length, cert->heap,
DYNAMIC_TYPE_PUBLIC_KEY);
if (publicKey == NULL) {
Expand All @@ -12659,6 +12691,7 @@ static int StoreKey(DecodedCert* cert, const byte* source, word32* srcIdx,

*srcIdx += (word32)length;
}
#endif
}

return ret;
Expand Down Expand Up @@ -13299,7 +13332,9 @@ static int StoreEccKey(DecodedCert* cert, const byte* source, word32* srcIdx,
{
int ret = 0;
DECL_ASNGETDATA(dataASN, eccCertKeyASN_Length);
#ifndef WC_ASN_NO_HEAP
byte* publicKey;
#endif

/* Validate parameters. */
if (pubKey == NULL) {
Expand Down Expand Up @@ -13369,6 +13404,12 @@ static int StoreEccKey(DecodedCert* cert, const byte* source, word32* srcIdx,
#endif
/* Store public key data length. */
cert->pubKeySize = pubKeyLen;
#ifdef WC_ASN_NO_HEAP
/* No heap: point at pubKey in the input DER, so that buffer must stay
* valid for the DecodedCert's lifetime. */
cert->publicKey = (byte*)pubKey;
cert->pubKeyStored = 0;
#else
/* Must allocated space for key.
* Don't memcpy into constant pointer so use temp. */
publicKey = (byte*)XMALLOC(cert->pubKeySize, cert->heap,
Expand All @@ -13383,6 +13424,7 @@ static int StoreEccKey(DecodedCert* cert, const byte* source, word32* srcIdx,
/* Indicate publicKey needs to be freed. */
cert->pubKeyStored = 1;
}
#endif
}
FREE_ASNGETDATA(dataASN, cert->heap);

Expand Down Expand Up @@ -14249,7 +14291,7 @@ static const byte rdnChoice[] = {
};
#endif

#ifdef WOLFSSL_IP_ALT_NAME
#if defined(WOLFSSL_IP_ALT_NAME) && !defined(WC_ASN_NO_HEAP)
/* used to set the human readable string for the IP address with a ASN_IP_TYPE
* DNS entry
* return 0 on success
Expand Down Expand Up @@ -14318,9 +14360,9 @@ static int GenerateDNSEntryIPString(DNS_entry* entry, void* heap)

return ret;
}
#endif /* WOLFSSL_IP_ALT_NAME */
#endif /* WOLFSSL_IP_ALT_NAME && !WC_ASN_NO_HEAP */

#ifdef WOLFSSL_RID_ALT_NAME
#if defined(WOLFSSL_RID_ALT_NAME) && !defined(WC_ASN_NO_HEAP)
/* used to set the human readable string for the registeredID with an
* ASN_RID_TYPE DNS entry
* return 0 on success
Expand Down Expand Up @@ -14417,7 +14459,7 @@ static int GenerateDNSEntryRIDString(DNS_entry* entry, void* heap)

return ret;
}
#endif /* WOLFSSL_RID_ALT_NAME */
#endif /* WOLFSSL_RID_ALT_NAME && !WC_ASN_NO_HEAP */

#ifdef WOLFSSL_ASN_TEMPLATE

Expand Down Expand Up @@ -14466,13 +14508,57 @@ static int AddDNSEntryToList(DNS_entry** lst, DNS_entry* entry)
* @return 0 on success.
* @return MEMORY_E when dynamic memory allocation fails.
*/
static int SetDNSEntry(void* heap, const char* str, int strLen,
int type, DNS_entry** entries)
/* No-heap SAN entries come from a caller pool; pass NULL,NULL if none. */
#ifdef WC_ASN_NO_HEAP
#define WC_DNS_POOL(obj) (obj)->altNamePool, &(obj)->altNamePoolUsed
#else
#define WC_DNS_POOL(obj) NULL, NULL
#endif

static int SetDNSEntry(void* heap, DNS_entry* pool, word32* poolUsed,
const char* str, int strLen, int type,
DNS_entry** entries)
{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🔴 [High] Unused static function build break under WC_ASN_NO_HEAP + WOLFSSL_IP_ALT_NAME/WOLFSSL_RID_ALT_NAME · bug

GenerateDNSEntryIPString (guarded only by #ifdef WOLFSSL_IP_ALT_NAME, line 14284) and GenerateDNSEntryRIDString (#ifdef WOLFSSL_RID_ALT_NAME, line 14355) are static, and their ONLY call sites are lines 14577 and 14583, both of which the PR moved into the new #else (non-WC_ASN_NO_HEAP) branch of SetDNSEntry (the #ifdef WC_ASN_NO_HEAP ... #else ... #endif closes at line 14594). When WC_ASN_NO_HEAP is defined together with WOLFSSL_IP_ALT_NAME or WOLFSSL_RID_ALT_NAME, those callers are compiled out but the function definitions remain, producing defined but not used static functions. Under wolfSSL's common -Werror build jobs this -Wunused-function warning is a hard compile failure. This is not a hypothetical combination: the PR's own WC_ASN_NO_HEAP branch contains explicit #ifdef WOLFSSL_IP_ALT_NAME/#ifdef WOLFSSL_RID_ALT_NAME reject code (asn.c:14521-14531), so the combo is intended to be buildable.

Fix: Gate the two helper definitions with && !defined(WC_ASN_NO_HEAP) (e.g. #if defined(WOLFSSL_IP_ALT_NAME) && !defined(WC_ASN_NO_HEAP) and likewise for GenerateDNSEntryRIDString) so the no-heap + IP/RID-alt-name configuration compiles cleanly under -Werror. Add a CI build combining WC_ASN_NO_HEAP with WOLFSSL_IP_ALT_NAME to catch this.

DNS_entry* dnsEntry;
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. */
(void)heap;
#ifdef WOLFSSL_IP_ALT_NAME
/* No-heap path can't parse an ipString/ridString; reject rather than skip. */
if (type == ASN_IP_TYPE) {
return ASN_PARSE_E;
}
#endif

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🔵 [Low] No-heap SAN name is not NUL-terminated (contract divergence from heap path) · Security

The heap path NUL-terminates dnsEntry->name (dnsEntry_name[strLen] = '\0', line 14572), but the new WC_ASN_NO_HEAP path sets dnsEntry->name = (char*)str pointing directly into the source DER with NO terminator (the following byte is the next ASN.1 element), relying solely on len. Length-based consumers are safe: the core verification path (CheckForAltNames/MatchDomainName in src/internal.c) and the copy paths in src/x509.c use altName->len/dns->len. However, several OpenSSL-compat consumers treat entry->name as a NUL-terminated C string, e.g. X509PrintName (src/x509.c:6956 XSNPRINTF(scratch, MAX_WIDTH, "DNS:%s", entry->name), and the RFC822/URI cases at 6974/6981). Under no-heap those would read past the SAN value into adjacent DER bytes until a stray 0x00, potentially past the end of the cert buffer (out-of-bounds read). Reachability is low because those consumers require OPENSSL_EXTRA/OPENSSL_ALL, which in practice cannot be built under a genuine no-allocator configuration. Flagged because the heap path's NUL-termination is an implicit contract the new path silently breaks.

Fix: Document explicitly (in asn.h near WC_ASN_NO_HEAP and on struct DNS_entry) that under WC_ASN_NO_HEAP name is NOT NUL-terminated and only len is authoritative, mirroring the subjectCN contract. Audit entry->name string consumers (e.g. src/x509.c X509PrintName) to use %.*s with entry->len instead of %s, or compile-guard those print helpers out when WC_ASN_NO_HEAP is defined.

#ifdef WOLFSSL_RID_ALT_NAME
if (type == ASN_RID_TYPE) {
return ASN_PARSE_E;
}
#endif
if ((pool == NULL) || (*poolUsed >= WC_ASN_MAX_ALTNAMES)) {
/* Distinguish pool exhaustion from a real allocation failure. */
WOLFSSL_MSG("No-heap SAN pool exhausted; raise WC_ASN_MAX_ALTNAMES");
ret = MEMORY_E;
dnsEntry = NULL;
}
else {
dnsEntry = &pool[(*poolUsed)++];
XMEMSET(dnsEntry, 0, sizeof(*dnsEntry));
dnsEntry->type = type;
dnsEntry->len = strLen;
dnsEntry->name = (char*)str; /* points into the source DER */
dnsEntry->nameStored = 0;
}
if (ret == 0) {
ret = AddDNSEntryToList(entries, dnsEntry);
}
#else
(void)pool;
(void)poolUsed;
/* TODO: consider one malloc. */
/* Allocate DNS Entry object. */
dnsEntry = AltNameNew(heap);
Expand Down Expand Up @@ -14517,6 +14603,7 @@ static int SetDNSEntry(void* heap, const char* str, int strLen,
XFREE(dnsEntry_name, heap, DYNAMIC_TYPE_ALTNAME);
XFREE(dnsEntry, heap, DYNAMIC_TYPE_ALTNAME);
}
#endif

return ret;
}
Expand Down Expand Up @@ -18988,7 +19075,8 @@ static int DecodeOtherHelper(ASNGetData* dataASN, DecodedCert* cert, int oid)
}

if (ret == 0) {
ret = SetDNSEntry(cert->heap, buf, (int)bufLen, ASN_OTHER_TYPE, &entry);
ret = SetDNSEntry(cert->heap, WC_DNS_POOL(cert), buf, (int)bufLen,
ASN_OTHER_TYPE, &entry);
if (ret == 0) {
#ifdef WOLFSSL_FPKI
entry->oidSum = oid;
Expand Down Expand Up @@ -19050,8 +19138,8 @@ static int DecodeOtherName(DecodedCert* cert, const byte* input,
break;
default:
WOLFSSL_MSG("\tadding unsupported OID");
ret = SetDNSEntry(cert->heap, name, len, ASN_OTHER_TYPE,
&cert->altNames);
ret = SetDNSEntry(cert->heap, WC_DNS_POOL(cert), name, len,
ASN_OTHER_TYPE, &cert->altNames);
break;
}
}
Expand Down Expand Up @@ -19089,8 +19177,9 @@ static int DecodeGeneralName(const byte* input, word32* inOutIdx, byte tag,
* reference hostname. The result is DOMAIN_NAME_MISMATCH at verification
* time rather than ASN_PARSE_E at parse time. */
if (tag == (ASN_CONTEXT_SPECIFIC | ASN_DNS_TYPE)) {
ret = SetDNSEntry(cert->heap, (const char*)(input + idx), len,
ASN_DNS_TYPE, &cert->altNames);
ret = SetDNSEntry(cert->heap, WC_DNS_POOL(cert),
(const char*)(input + idx), len, ASN_DNS_TYPE,
&cert->altNames);
if (ret == 0) {
idx += (word32)len;
}
Expand All @@ -19108,16 +19197,18 @@ static int DecodeGeneralName(const byte* input, word32* inOutIdx, byte tag,
return ASN_PARSE_E;
}

ret = SetDNSEntry(cert->heap, (const char*)(input + idxDir), strLen,
ASN_DIR_TYPE, &cert->altDirNames);
ret = SetDNSEntry(cert->heap, WC_DNS_POOL(cert),
(const char*)(input + idxDir), strLen, ASN_DIR_TYPE,
&cert->altDirNames);
if (ret == 0) {
idx += (word32)len;
}
}
/* GeneralName choice: rfc822Name */
else if (tag == (ASN_CONTEXT_SPECIFIC | ASN_RFC822_TYPE)) {
ret = SetDNSEntry(cert->heap, (const char*)(input + idx), len,
ASN_RFC822_TYPE, &cert->altEmailNames);
ret = SetDNSEntry(cert->heap, WC_DNS_POOL(cert),
(const char*)(input + idx), len, ASN_RFC822_TYPE,
&cert->altEmailNames);
if (ret == 0) {
idx += (word32)len;
}
Expand Down Expand Up @@ -19163,8 +19254,9 @@ static int DecodeGeneralName(const byte* input, word32* inOutIdx, byte tag,
}
#endif

ret = SetDNSEntry(cert->heap, (const char*)(input + idx), len,
ASN_URI_TYPE, &cert->altNames);
ret = SetDNSEntry(cert->heap, WC_DNS_POOL(cert),
(const char*)(input + idx), len, ASN_URI_TYPE,
&cert->altNames);
if (ret == 0) {
idx += (word32)len;
}
Expand Down Expand Up @@ -19196,8 +19288,9 @@ static int DecodeGeneralName(const byte* input, word32* inOutIdx, byte tag,
* ASN_IP_TYPE case under WOLFSSL_GEN_IPADD in src/x509.c).
*/
else if (tag == (ASN_CONTEXT_SPECIFIC | ASN_IP_TYPE)) {
ret = SetDNSEntry(cert->heap, (const char*)(input + idx), len,
ASN_IP_TYPE, &cert->altNames);
ret = SetDNSEntry(cert->heap, WC_DNS_POOL(cert),
(const char*)(input + idx), len, ASN_IP_TYPE,
&cert->altNames);
if (ret == 0) {
idx += (word32)len;
}
Expand Down Expand Up @@ -19228,8 +19321,9 @@ static int DecodeGeneralName(const byte* input, word32* inOutIdx, byte tag,
* when ridString is not generated, instead of failing the
* whole print operation. */
else if (tag == (ASN_CONTEXT_SPECIFIC | ASN_RID_TYPE)) {
ret = SetDNSEntry(cert->heap, (const char*)(input + idx), len,
ASN_RID_TYPE, &cert->altNames);
ret = SetDNSEntry(cert->heap, WC_DNS_POOL(cert),
(const char*)(input + idx), len, ASN_RID_TYPE,
&cert->altNames);
if (ret == 0) {
idx += (word32)len;
}
Expand All @@ -19244,8 +19338,9 @@ static int DecodeGeneralName(const byte* input, word32* inOutIdx, byte tag,
* the public altNames view (used by OpenSSL-compat APIs) reflects
* exactly what the SAN extension carries. */
else if (tag == (ASN_CONTEXT_SPECIFIC | ASN_CONSTRUCTED | ASN_OTHER_TYPE)) {
ret = SetDNSEntry(cert->heap, (const char*)(input + idx), len,
ASN_OTHER_TYPE, &cert->altOtherNamesRaw);
ret = SetDNSEntry(cert->heap, WC_DNS_POOL(cert),
(const char*)(input + idx), len, ASN_OTHER_TYPE,
&cert->altOtherNamesRaw);
if (ret != 0) {
return ret;
}
Expand Down Expand Up @@ -24205,6 +24300,7 @@ int FillSigner(Signer* signer, DecodedCert* cert, int type, DerBuffer *der)
(void)der;
#endif
signer->keyOID = cert->keyOID;
/* pubKeyStored stays 0 under WC_ASN_NO_HEAP (signer uses heap). */
if (cert->pubKeyStored) {
signer->publicKey = cert->publicKey;
signer->pubKeySize = cert->pubKeySize;
Expand Down Expand Up @@ -38800,8 +38896,8 @@ static int DecodeAcertGeneralName(const byte* input, word32* inOutIdx,

/* GeneralName choice: dnsName */
if (tag == (ASN_CONTEXT_SPECIFIC | ASN_DNS_TYPE)) {
ret = SetDNSEntry(acert->heap, (const char*)(input + idx), len,
ASN_DNS_TYPE, entries);
ret = SetDNSEntry(acert->heap, WC_DNS_POOL(acert),
(const char*)(input + idx), len, ASN_DNS_TYPE, entries);
if (ret == 0) {
idx += (word32)len;
}
Expand All @@ -38819,16 +38915,16 @@ static int DecodeAcertGeneralName(const byte* input, word32* inOutIdx,
return ASN_PARSE_E;
}

ret = SetDNSEntry(acert->heap, (const char*)(input + idxDir), strLen,
ASN_DIR_TYPE, entries);
ret = SetDNSEntry(acert->heap, WC_DNS_POOL(acert),
(const char*)(input + idxDir), strLen, ASN_DIR_TYPE, entries);
if (ret == 0) {
idx += (word32)len;
}
}
/* GeneralName choice: rfc822Name */
else if (tag == (ASN_CONTEXT_SPECIFIC | ASN_RFC822_TYPE)) {
ret = SetDNSEntry(acert->heap, (const char*)(input + idx), len,
ASN_RFC822_TYPE, entries);
ret = SetDNSEntry(acert->heap, WC_DNS_POOL(acert),
(const char*)(input + idx), len, ASN_RFC822_TYPE, entries);
if (ret == 0) {
idx += (word32)len;
}
Expand Down Expand Up @@ -38874,8 +38970,8 @@ static int DecodeAcertGeneralName(const byte* input, word32* inOutIdx,
}
#endif

ret = SetDNSEntry(acert->heap, (const char*)(input + idx), len,
ASN_URI_TYPE, entries);
ret = SetDNSEntry(acert->heap, WC_DNS_POOL(acert),
(const char*)(input + idx), len, ASN_URI_TYPE, entries);
if (ret == 0) {
idx += (word32)len;
}
Expand All @@ -38892,8 +38988,8 @@ static int DecodeAcertGeneralName(const byte* input, word32* inOutIdx,
* IP-SAN compat layer). If iPAddress name-constraint enforcement is
* ever extended to attribute certificates, this gate must drop. */
else if (tag == (ASN_CONTEXT_SPECIFIC | ASN_IP_TYPE)) {
ret = SetDNSEntry(acert->heap, (const char*)(input + idx), len,
ASN_IP_TYPE, entries);
ret = SetDNSEntry(acert->heap, WC_DNS_POOL(acert),
(const char*)(input + idx), len, ASN_IP_TYPE, entries);
if (ret == 0) {
idx += (word32)len;
}
Expand All @@ -38903,8 +38999,8 @@ static int DecodeAcertGeneralName(const byte* input, word32* inOutIdx,
#ifdef OPENSSL_ALL
/* GeneralName choice: registeredID */
else if (tag == (ASN_CONTEXT_SPECIFIC | ASN_RID_TYPE)) {
ret = SetDNSEntry(acert->heap, (const char*)(input + idx), len,
ASN_RID_TYPE, entries);
ret = SetDNSEntry(acert->heap, WC_DNS_POOL(acert),
(const char*)(input + idx), len, ASN_RID_TYPE, entries);
if (ret == 0) {
idx += (word32)len;
}
Expand Down
Loading
Loading