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 21 out of 21 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

wolfcrypt/src/asn.c:1

• This introduces an early return from ParseCertRelative. If the rest of the function relies on a single-exit pattern for consistent cleanup/logging (common in wolfSSL code), this return can bypass it. Prefer setting ret = ASN_PARSE_E and flowing through the function’s existing error/exit handling (e.g., via the established goto exit; pattern if present).
  wolfcrypt/src/asn.c:1
• The updated log message format is inconsistent with nearby wolfSSL messages (it adds an Error: prefix and includes parenthetical text). Consider aligning with existing WOLFSSL_MSG style (e.g., no Error: prefix, keep it short, and/or mention the relevant compile-time knob only where actionable) to keep logs consistent and easier to grep.

---

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

You can also share your feedback on Copilot code review. Take the survey.

[douzzer]

Jenkins retest this please

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (3)

wolfcrypt/src/asn.c:1

• The serial-0 detection only triggers when serialSz == 1, but a value of 0 can also be encoded with leading 0x00 bytes (non-minimal INTEGER encodings). If the policy is “reject value 0 except for self-signed CA trust anchors”, consider checking whether all bytes are 0 (or decoding the INTEGER value) rather than only the 1-byte case.
  wolfcrypt/src/asn.c:1
• This introduces an early return from ParseCertRelative(), while surrounding validation in this function appears to use ret = ... and continue toward a shared exit path. To keep control flow consistent (and avoid skipping any common cleanup/error handling), set ret = ASN_PARSE_E; and follow the function’s existing error/exit pattern instead of returning immediately here.
  wolfcrypt/src/asn.c:1
• The message says “non-root certificate”, but the condition is specifically “serial 0 is only permitted for self-signed CA certificates loaded as trust anchors (CA_TYPE/TRUSTED_PEER_TYPE + isCA + selfSigned)”. Consider updating the message to reflect the actual rule (e.g., “serial number 0 is only allowed for self-signed CA trust anchors”) to make failures easier to diagnose.

---

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

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (2)

wolfcrypt/src/asn.c:1

• The “serial == 0” check only triggers when serialSz == 1. If a non-canonical encoding (or permissive decode path) produces a multi-byte all-zero serial (e.g., 00 00), this logic would not detect it. Consider treating the serial as zero when all serial bytes are 0x00 (for any serialSz > 0) so the allow/deny behavior can’t be bypassed by representation.
  wolfcrypt/src/asn.c:1
• This introduces an early return inside ParseCertRelative, while most error paths appear to flow via ret (and often through any shared return/cleanup/error-trace handling). To keep error handling consistent, prefer setting ret = ASN_PARSE_E; and continuing through the function’s normal exit path (or using the project’s usual error-return macro if applicable) rather than returning directly here.

---

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

[Copilot]

This assertion only checks “not success”, so it could pass for unrelated reasons (e.g., missing fixture file, file read error, unsupported format) and wouldn’t prove the serial-0 rejection logic is what triggered. Prefer asserting the specific expected failure code (e.g., WC_NO_ERR_TRACE(ASN_PARSE_E) or the project’s canonical parse error for this path) to make the test resilient and diagnostic.

Suggested change

	ExpectIntNE(wolfSSL_CertManagerLoadCA(cm, selfSignedNonCASerial0File, NULL),
	WOLFSSL_SUCCESS);
	ExpectIntEQ(wolfSSL_CertManagerLoadCA(cm, selfSignedNonCASerial0File, NULL),
	WC_NO_ERR_TRACE(ASN_PARSE_E));

[jackctj117]

Doesn't work with --enable-krb

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (4)

wolfcrypt/src/asn.c:1

• The WOLFSSL_CERT_REQ && !cert->isCSR clause is currently inside the negated allow-list expression, which makes CSRs more likely to fail this check (i.e., cert->isCSR == 1 forces the allow-list false, then !false triggers the error). If the intent is to skip the serial-0 rejection for CSRs, restructure to guard the whole check with if (!cert->isCSR && ...) (or equivalently move !cert->isCSR outside the negation) so CSRs do not get rejected by this serial-0 logic.
  wolfcrypt/src/asn.c:1
• This introduces an early return from ParseCertRelative, which can bypass any unified cleanup/teardown logic later in the function (and makes future refactors riskier). Prefer setting ret = ASN_PARSE_E; and jumping to the existing common exit/cleanup path (e.g., goto exit;) used elsewhere in the function so resource handling remains consistent.
  wolfcrypt/src/asn.c:1
• The new error message no longer tells users how to intentionally allow serial 0 (the old message referenced WOLFSSL_NO_ASN_STRICT). Since this check is gated by WOLFSSL_NO_ASN_STRICT / WOLFSSL_ASN_ALLOW_0_SERIAL, consider updating the message to mention the relevant build option(s) (e.g., WOLFSSL_ASN_ALLOW_0_SERIAL or disabling strict ASN) to make diagnosing configuration-related failures easier.
  certs/test-serial0/root_serial0.pem:1
• These committed test certificates embed a notBefore of 2026-03-19 (visible in the ASN.1 time encoding), which can make tests time-dependent: any CI runner or developer machine with a clock skewed earlier than that date will fail verification unexpectedly. To keep the test suite robust, regenerate fixtures with a notBefore far enough in the past (or otherwise ensure the cert validity window is not sensitive to current time/clock skew).

-----BEGIN CERTIFICATE-----

---

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

[Copilot]

This adds a private key (root_serial0_key.pem) to EXTRA_DIST, which increases the chance it is redistributed downstream and can trigger secret/key scanners even if it's only for testing. Consider removing the key from distributed artifacts (keep only cert fixtures), and have generate_certs.sh generate keys on demand; alternatively, store the key outside EXTRA_DIST and document that it's a non-sensitive test key.

Suggested change

certs/test-serial0/root_serial0_key.pem \

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (2)

wolfcrypt/src/asn.c:1

• Returning directly from the middle of ParseCertRelative can skip the function’s normal error handling / cleanup path (whatever that is in the surrounding code). Prefer setting ret = ASN_PARSE_E and flowing through the existing function-level return/cleanup pattern (e.g., a shared exit label) instead of an early return.
  wolfcrypt/src/asn.c:1
• The allow/deny logic is correct but hard to read because && !cert->isCSR is conditionally appended outside the parenthesized 'root CA' group. Consider refactoring into a clearly named boolean (e.g., allowZeroSerial) that explicitly encodes: (a) allow when loading trust anchors AND selfSigned AND isCA, OR (b) allow when parsing a CSR (when enabled). This reduces the risk of future regressions when conditions are modified.

---

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

[Copilot]

A private key is being added to the repository and included in EXTRA_DIST. Even if it’s only a test key, shipping private keys can trigger security/compliance tooling and increases the risk of accidental reuse. Consider excluding the key from distribution artifacts (keep it only in the generator workflow/docs), or add prominent 'TEST ONLY / DO NOT USE' documentation and ensure it’s never installed/packaged beyond tests.

Suggested change

	-----BEGIN PRIVATE KEY-----
	MIIEugIBADANBgkqhkiG9w0BAQEFAASCBKQwggSgAgEAAoIBAQChQKbHG0su7Bpy
	nIN8GtTCQk8jNXKR0TnOnCHLyekO8p22QxW+A6r6sfRo/TGxPOqG8VGhFec8bs6w
	v7KX+SbYCxvhZUab4ygnlYCjW/ab6eiRTr4BloBSYWBUqz01k53CDjRD1CK6orZ4
	JTAqi+4qtJkqTupMY1sKyxxw9F69PPqzNDHTsUwfZUYcvHa5VQIdX0k3cFs4Myer
	eNkUvLW5BZrGcDecsZRuvZ9CB5+z6ser2/ROxR3ty6ZgwHA9fLmQoxXor89Gb0mr
	XQLjNTTSDvP3eC4GPqFSLZYFf6aaLx/qFNKICUPEA6Fmk+blliE/c/Z7DquD86OB
	+VqON0e1AgMBAAECgf4QdS4GA0WpWXNZ4lHVqJjGqCLu6ap3HqZDz590p2jsrp21
	opfXXlR77+54p3L5OqavTarh0Ugr1NKOeF5CLkXVD9zgoZPXyzZhakRTOkxlCfo4
	wL6qGE5HPRvwNK/9xV1PJSeOd7+DYEu5rt+wuXYlPxscDPTV+yMrvy8lyOmIjZey
	uS64bMaiWVvuF5KEc3p0+VqX5Q1FzSOBxBMdSuN+5xbckeVMBWWQpBUAitxdpGUs
	koieaE8vjrwAXlgha2iUF9/3WJO+IYRz4e2x/2cU3wAXB/5Ga4VSltHFgHcpahc5
	CN7QxBaMAisKvoGmHL9cmi47KP9+ScUWwNGHgQKBgQDS4oW6oO12W/5+5uGNP6zR
	KtJ4utKwrhSeWpV6qu5cXjYC1pJEM0XN2K+K4KGPUxoNtLpspCJ2SR911y+jhz4a
	oQ2U5l+gHEYjQP7bOHPwN8aCIZj6HHyD8aTcsWl3vb9dizhRvTw8D+vCiNCWM2sQ
	fSdtMLk1Ju7ud5i0onJ+lQKBgQDDv+3RW+7apjuIc5CDlOIG+0Ir2/iyvtkhDRMn
	thEUZYL+a8sTKhOk4SfgDZbxdz9oVJVwXFSV01F5ac/gu0/C0VLYb6vbGr2TPf3K
	RPRQjoJh3XvM6ydx47hO/iUm8E5bEWjYqBXuhinh0lj11zjCtsf+zkhxCy+0i5ot
	WW/8oQKBgD8iGa75pp2chOAw9q12tqIYE9KY+6JxOzL9I2sJ6To16i2HV1qbjvZF
	PKhy/2sNEeuwg28q5DZNReHdfiGSx4DpXkuJfG9Oh6DeQG4YxHzR9dfXfxjBlnVZ
	zmVTp6N1Zuj2WPH/mRzSF16x3uBYnGDfVwJVZ90Fvtoda9YIHAbRAoGBAKvhuacd
	/GvNj3TPVNPVRWsv8PimHIiHgAy/eFRkUDcCs7VHXXekeL9MXUElbab1OJ4Zt2aE
	DFnKxj3AJaKFlxHPz9jwpYysvE2wH0sepRCfMelRG8Xhri8Y79uc2W6Jj6Pzc4ba
	gPeCowABPdAQfWysJoydAYsRcYAtHOI5KFZBAoGAeiLvbtj/odW1tuOcijKVX4kC
	er/cqIYgvgQg1TTMsb02Fv1GBQn8A1C1Jb6TV/cJvxW3TIrqk07cSrM8qIHy1qAk
	omQbnSV6p6c4FaJXBFPk9ileSPwBegxuBSiby2X29cJ88D1Wdq8Osjmc5wK/umuy
	Vowu3kDcLFcpu3v6368=
	-----END PRIVATE KEY-----
	# TEST-ONLY PLACEHOLDER: no private key material is stored in this file.
	#
	# This file previously contained a test private key that was distributed as
	# part of the source tree, which can trigger security/compliance tooling and
	# is against best practices.
	#
	# To run tests that require a private key, generate one locally as part of
	# your test setup or CI workflow (for example, using openssl) and point the
	# tests to that generated key instead of relying on a checked-in key file.
	#
	# Example (PKCS#8 RSA key generation for tests only):
	# openssl genpkey -algorithm RSA -pkeyopt rsa_keygen_bits:2048 \\
	# -out certs/test-serial0/root_serial0_key.pem
	#
	# Do NOT commit the generated key back into the repository.