Skip to content

refactor X509PrintSubjAltName #9238

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

Merged
dgarske merged 1 commit into from
Oct 2, 2025
Merged

refactor X509PrintSubjAltName #9238

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
175 changes: 17 additions & 158 deletions src/x509.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6099,7 +6099,6 @@ int wolfSSL_X509_cmp(const WOLFSSL_X509 *a, const WOLFSSL_X509 *b)
#define MAX_WIDTH 80
#endif

#if defined(WOLFSSL_ACERT)
#define ACERT_NUM_DIR_TAGS 4

/* Convenience struct and function for printing the Holder sub fields
Expand Down Expand Up @@ -6209,9 +6208,8 @@ static int X509PrintDirType(char * dst, int max_len, const DNS_entry * entry)

return total_len;
}

static int X509_ACERT_print_name_entry(WOLFSSL_BIO* bio,
const DNS_entry* entry, int indent)
static int X509_print_name_entry(WOLFSSL_BIO* bio,
const DNS_entry* entry, int indent)
{
int ret = WOLFSSL_SUCCESS;
int nameCount = 0;
Expand Down Expand Up @@ -6242,68 +6240,43 @@ static int X509_ACERT_print_name_entry(WOLFSSL_BIO* bio,

if (entry->type == ASN_DNS_TYPE) {
len = XSNPRINTF(scratch, MAX_WIDTH, "DNS:%s", entry->name);
if (len >= MAX_WIDTH) {
ret = WOLFSSL_FAILURE;
break;
}
}
#if defined(OPENSSL_ALL) || defined(WOLFSSL_IP_ALT_NAME)
else if (entry->type == ASN_IP_TYPE) {
len = XSNPRINTF(scratch, MAX_WIDTH, "IP Address:%s",
entry->ipString);
if (len >= MAX_WIDTH) {
ret = WOLFSSL_FAILURE;
break;
}
}
#endif /* OPENSSL_ALL || WOLFSSL_IP_ALT_NAME */
else if (entry->type == ASN_RFC822_TYPE) {
len = XSNPRINTF(scratch, MAX_WIDTH, "email:%s",
entry->name);
if (len >= MAX_WIDTH) {
ret = WOLFSSL_FAILURE;
break;
}
}
else if (entry->type == ASN_DIR_TYPE) {
len = X509PrintDirType(scratch, MAX_WIDTH, entry);
if (len >= MAX_WIDTH) {
ret = WOLFSSL_FAILURE;
break;
}
}
else if (entry->type == ASN_URI_TYPE) {
len = XSNPRINTF(scratch, MAX_WIDTH, "URI:%s",
entry->name);
if (len >= MAX_WIDTH) {
ret = WOLFSSL_FAILURE;
break;
}
}
#if defined(OPENSSL_ALL)
else if (entry->type == ASN_RID_TYPE) {
len = XSNPRINTF(scratch, MAX_WIDTH, "Registered ID:%s",
entry->ridString);
if (len >= MAX_WIDTH) {
ret = WOLFSSL_FAILURE;
break;
}
}
#endif
else if (entry->type == ASN_OTHER_TYPE) {
len = XSNPRINTF(scratch, MAX_WIDTH,
"othername <unsupported>");
Comment on lines 6242 to 6269
Copy link

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

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

The previous bounds checks for truncation (len >= MAX_WIDTH) after each formatting call were removed, which silently allows truncated output. To keep behavior robust without duplicating checks, add a single post-selection check like: if (len < 0 || len >= MAX_WIDTH) { ret = WOLFSSL_FAILURE; break; } before writing to the BIO.

Copilot uses AI. Check for mistakes.
if (len >= MAX_WIDTH) {
ret = WOLFSSL_FAILURE;
break;
}
}
else {
WOLFSSL_MSG("Bad alt name type.");
ret = WOLFSSL_FAILURE;
break;
}

if (len >= MAX_WIDTH) {
ret = WOLFSSL_FAILURE;
break;
}
if (wolfSSL_BIO_write(bio, scratch, (int)XSTRLEN(scratch))
<= 0) {
ret = WOLFSSL_FAILURE;
Expand All @@ -6320,128 +6293,6 @@ static int X509_ACERT_print_name_entry(WOLFSSL_BIO* bio,
return ret;
}

#endif /* if WOLFSSL_ACERT*/

static int X509PrintSubjAltName(WOLFSSL_BIO* bio, WOLFSSL_X509* x509,
int indent)
{
int ret = WOLFSSL_SUCCESS;
DNS_entry* entry;

if (bio == NULL || x509 == NULL) {
ret = WOLFSSL_FAILURE;
}

if (ret == WOLFSSL_SUCCESS && x509->subjAltNameSet &&
x509->altNames != NULL) {
char scratch[MAX_WIDTH];
int len;

len = XSNPRINTF(scratch, MAX_WIDTH, "%*s", indent, "");
if (len >= MAX_WIDTH)
ret = WOLFSSL_FAILURE;
if (ret == WOLFSSL_SUCCESS) {
if (wolfSSL_BIO_write(bio, scratch, (int)XSTRLEN(scratch)) <= 0) {
ret = WOLFSSL_FAILURE;
}
}
if (ret == WOLFSSL_SUCCESS) {
int nameCount = 0;

entry = x509->altNames;
while (entry != NULL) {
++nameCount;
if (nameCount > 1) {
if (wolfSSL_BIO_write(bio, ", ", 2) <= 0) {
ret = WOLFSSL_FAILURE;
break;
}
}

if (entry->type == ASN_DNS_TYPE) {
len = XSNPRINTF(scratch, MAX_WIDTH, "DNS:%s", entry->name);
if (len >= MAX_WIDTH) {
ret = WOLFSSL_FAILURE;
break;
}
}
#ifdef WOLFSSL_IP_ALT_NAME
else if (entry->type == ASN_IP_TYPE) {
len = XSNPRINTF(scratch, MAX_WIDTH, "IP Address:%s",
entry->ipString);
if (len >= MAX_WIDTH) {
ret = WOLFSSL_FAILURE;
break;
}
}
#endif /* OPENSSL_ALL || WOLFSSL_IP_ALT_NAME */
else if (entry->type == ASN_RFC822_TYPE) {
len = XSNPRINTF(scratch, MAX_WIDTH, "email:%s",
entry->name);
if (len >= MAX_WIDTH) {
ret = WOLFSSL_FAILURE;
break;
}
}
else if (entry->type == ASN_DIR_TYPE) {
/* @TODO entry->name in ASN1 syntax */
len = XSNPRINTF(scratch, MAX_WIDTH,
"DirName:<print out not supported yet>");
if (len >= MAX_WIDTH) {
ret = WOLFSSL_FAILURE;
break;
}
}
else if (entry->type == ASN_URI_TYPE) {
len = XSNPRINTF(scratch, MAX_WIDTH, "URI:%s",
entry->name);
if (len >= MAX_WIDTH) {
ret = WOLFSSL_FAILURE;
break;
}
}
#if defined(OPENSSL_ALL)
else if (entry->type == ASN_RID_TYPE) {
len = XSNPRINTF(scratch, MAX_WIDTH, "Registered ID:%s",
entry->ridString);
if (len >= MAX_WIDTH) {
ret = WOLFSSL_FAILURE;
break;
}
}
#endif
else if (entry->type == ASN_OTHER_TYPE) {
len = XSNPRINTF(scratch, MAX_WIDTH,
"othername <unsupported>");
if (len >= MAX_WIDTH) {
ret = WOLFSSL_FAILURE;
break;
}
}
else {
WOLFSSL_MSG("Bad alt name type.");
ret = WOLFSSL_FAILURE;
break;
}

if (wolfSSL_BIO_write(bio, scratch, (int)XSTRLEN(scratch))
<= 0) {
ret = WOLFSSL_FAILURE;
break;
}

entry = entry->next;
}
}

if (ret == WOLFSSL_SUCCESS && wolfSSL_BIO_write(bio, "\n", 1) <= 0) {
ret = WOLFSSL_FAILURE;
}
}

return ret;
}

#ifdef XSNPRINTF
static int X509PrintKeyUsage(WOLFSSL_BIO* bio, WOLFSSL_X509* x509, int indent)
{
Expand Down Expand Up @@ -6711,6 +6562,14 @@ static int X509PrintValidity(WOLFSSL_BIO* bio, WOLFSSL_ASN1_TIME * notBefore,
}
#endif /* ifndef NO_ASN_TIME */

static int X509PrintSubjAltName(WOLFSSL_BIO* bio, WOLFSSL_X509* x509,
int indent)
{
if (!x509 || !x509->altNames || !x509->subjAltNameSet)
return WOLFSSL_FAILURE;
Comment on lines +6568 to +6569
Copy link

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

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

The refactor changes behavior by returning WOLFSSL_FAILURE when no SAN is present (altNames is NULL or subjAltNameSet is false). Previously, the function treated absence of SAN as a no-op and returned success, which avoids failing overall certificate printing for a valid cert without SAN. Also, the prior implementation guarded against a NULL bio, which is no longer checked here. Suggest restoring prior semantics and input validation: return success when SAN isn't set/present, and fail early on NULL bio.

Suggested change
if (!x509 || !x509->altNames || !x509->subjAltNameSet)
return WOLFSSL_FAILURE;
if (bio == NULL)
return WOLFSSL_FAILURE;
if (!x509 || !x509->altNames || !x509->subjAltNameSet)
return WOLFSSL_SUCCESS; /* No SAN present, treat as no-op */

Copilot uses AI. Check for mistakes.
return X509_print_name_entry(bio, x509->altNames, indent);
}

/* iterate through certificate extensions printing them out in human readable
* form
* return WOLFSSL_SUCCESS on success
Expand Down Expand Up @@ -7586,7 +7445,7 @@ int wolfSSL_X509_ACERT_print(WOLFSSL_BIO* bio, WOLFSSL_X509_ACERT* x509)
return WOLFSSL_FAILURE;
}

if (X509_ACERT_print_name_entry(bio, x509->holderEntityName, 1)
if (X509_print_name_entry(bio, x509->holderEntityName, 1)
!= WOLFSSL_SUCCESS) {
return WOLFSSL_FAILURE;
}
Expand All @@ -7599,7 +7458,7 @@ int wolfSSL_X509_ACERT_print(WOLFSSL_BIO* bio, WOLFSSL_X509_ACERT* x509)
return WOLFSSL_FAILURE;
}

if (X509_ACERT_print_name_entry(bio, x509->holderIssuerName, 1)
if (X509_print_name_entry(bio, x509->holderIssuerName, 1)
!= WOLFSSL_SUCCESS) {
return WOLFSSL_FAILURE;
}
Expand All @@ -7617,7 +7476,7 @@ int wolfSSL_X509_ACERT_print(WOLFSSL_BIO* bio, WOLFSSL_X509_ACERT* x509)
}

if (x509->AttCertIssuerName != NULL) {
if (X509_ACERT_print_name_entry(bio, x509->AttCertIssuerName, 1)
if (X509_print_name_entry(bio, x509->AttCertIssuerName, 1)
!= WOLFSSL_SUCCESS) {
return WOLFSSL_FAILURE;
}
Expand Down
Loading