Allow serial number 0 for root CA certificates

[jackctj117]

Fixes #8615

This pull request updates the logic for validating X.509 certificate serial numbers in wolfcrypt/src/asn.c. The main change is to improve compliance with RFC 5280 while allowing for real-world exceptions involving root CAs. The previous strict check for zero serial numbers is now more nuanced, permitting serial number 0 for self-signed CA certificates but still rejecting it for other certificates.

Certificate serial number validation improvements:

• Moved and updated the check for serial numbers of 0 to allow self-signed CA (root) certificates to have a serial number of 0, while still treating serial 0 as an error for all other certificates. This better aligns with RFC 5280 and real-world trust store practices. [1] [2]
• Removed the previous check that always treated a serial number of 0 as an error, regardless of certificate type.

Testing

Tested with certificates generated using OpenSSL to verify all scenarios:

• Root CA with serial 0 loads successfully (previously failed, now passes)
• End-entity cert with serial 0 is correctly rejected
• Normal end-entity cert signed by root CA with serial 0 verifies successfully
• Self-signed non-CA cert with serial 0 is correctly rejected

[kareem-wolfssl]

Changes look good.

Can you add some test cases with a CA and leaf cert with a serial of 0?

[jackctj117]

@kareem-wolfssl it looks like the failures are looking for an expected failure due to a self-signed CA certificate with serial number 0.

[kareem-wolfssl]

@kareem-wolfssl it looks like the failures are looking for an expected failure due to a self-signed CA certificate with serial number 0.

Yes, you will need to update the failing test test_MakeCertWith0Ser since we no longer expect a root CA with serial 0 to fail.

[dgarske]

@jackctj117 looks like some valid unit test failures you will need to look into. All related to --enable-asn=original.

FAILURES:
   390: test_SerialNumber0_RootCA

[jackctj117]

Jenkins retest this please. History lost.

[jackctj117]

Jenkins retest this please. History lost.

[jackctj117]

Jenkins retest this please

[jackctj117]

Jenkins retest this please

[jackctj117]

Jenkins retest this

[jackctj117]

Jenkins retest this please

[kareem-wolfssl]

Jenkins retest this please

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

wolfcrypt/src/asn.c:23904

• The log message for cert->serialSz == 0 is confusing: it suggests a certificate might legitimately have “no serial number”, but X.509 certificates always have a serial number and RFC 5280 requires it to be present. Consider rewording to clearly state that an empty/zero-length serial is invalid, and (if appropriate in this file) emit the verbose error macro consistently with other parse failures.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (6)

wolfcrypt/src/asn.c:1

• The PR description is focused on certificate serial-number validation, but this hunk also changes BIT STRING parsing behavior by removing the previous compile-time guard around the zero-length check. If the guard existed to preserve behavior for selftests/FIPS (or specific configurations), this could be an unintended behavior change; consider restoring the conditional compilation (or narrowing the change) unless this is explicitly part of the intended fix.
  wolfcrypt/src/asn.c:1
• The comment states the exception is for certs “being loaded as trust anchors”, but the condition implemented here only checks cert->isCA && cert->selfSigned (i.e., it’s not tied to a “trust-anchor load” context). Either adjust the comment to match the actual behavior, or tighten the condition so the exception is only applied in the trust-anchor loading path (to avoid misleading future maintainers).
  wolfcrypt/src/asn.c:1
• The “serial number == 0” policy appears implemented in multiple places (e.g., both decoding and relative parsing paths) with slightly different context checks and error handling (ret = ASN_PARSE_E vs return ASN_PARSE_E). Consider centralizing this into a shared helper (or a single policy function) so future tweaks don’t drift and so messaging/behavior stays consistent across code paths.
  tests/api.c:1
• This change makes the test depend on RSA (!defined(NO_RSA)) where it previously depended on ECC (defined(HAVE_ECC)). On configurations that build ECC but disable RSA, this test will no longer compile/run, reducing coverage for serial-0 certificate generation/decoding scenarios. If the intent is just to switch algorithms, consider supporting both (conditional RSA/ECC branches) so the test continues to exercise the behavior across common build configurations.
  tests/api.c:1
• This change makes the test depend on RSA (!defined(NO_RSA)) where it previously depended on ECC (defined(HAVE_ECC)). On configurations that build ECC but disable RSA, this test will no longer compile/run, reducing coverage for serial-0 certificate generation/decoding scenarios. If the intent is just to switch algorithms, consider supporting both (conditional RSA/ECC branches) so the test continues to exercise the behavior across common build configurations.
  tests/api.c:1
• This change makes the test depend on RSA (!defined(NO_RSA)) where it previously depended on ECC (defined(HAVE_ECC)). On configurations that build ECC but disable RSA, this test will no longer compile/run, reducing coverage for serial-0 certificate generation/decoding scenarios. If the intent is just to switch algorithms, consider supporting both (conditional RSA/ECC branches) so the test continues to exercise the behavior across common build configurations.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ColtonWilley]

Jenkins retest this please

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #9567

Scan targets checked: wolfcrypt-api_misuse, wolfcrypt-bugs, wolfcrypt-compliance, wolfcrypt-concurrency, wolfcrypt-consttime, wolfcrypt-defaults, wolfcrypt-mutation, wolfcrypt-portability, wolfcrypt-proptest, wolfcrypt-src, wolfcrypt-zeroize

Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

[github-actions]

MemBrowse Memory Report

No memory changes detected for:

• gcc-arm-cortex-m4
• gcc-arm-cortex-m4-baremetal
• gcc-arm-cortex-m4-min-ecc
• gcc-arm-cortex-m4-tls12