Fixes in TLS ECH, handle empty records, and ASN len check

[embhorn]

Description

• Fix TLSX_Parse to check dup ECH
• Add WOLFSSL_MAX_EMPTY_RECORDS
• Fix ParseCertRelative to check pathlen on self-issued certs

Fixes zd21587

Testing

Added tests:

• test_tls13_duplicate_ech_extension
• test_tls13_empty_record_limit
• test_PathLenSelfIssued and test_PathLenNoKeyUsage

Checklist

• added tests
• updated/added doxygen
• updated appropriate READMEs
• Updated manual and documentation

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR tightens TLS 1.3 parsing and certificate verification logic by adding protections against duplicate extensions (ECH), limiting empty application-data records (DoS mitigation), and correctly enforcing RFC 5280 pathLen constraints in more cases.

Changes:

• Treat TLS ECH as duplicate-detectable in TLSX_Parse() and add a targeted TLS 1.3 test.
• Add an empty TLS application-data record rate limit (WOLFSSL_MAX_EMPTY_RECORDS), new error code, and corresponding TLS 1.3 tests.
• Fix ParseCertRelative() pathLen enforcement for self-issued certificates and when KeyUsage is absent; add regression tests.

Reviewed changes

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

Show a summary per file

File	Description
wolfssl/wolfcrypt/settings.h	Adds configurable WOLFSSL_MAX_EMPTY_RECORDS default for empty-record DoS mitigation.
wolfssl/internal.h	Tracks empty application-data records per connection via emptyRecordCount.
wolfssl/error-ssl.h	Adds EMPTY_RECORD_LIMIT_E error code for empty-record limit violations.
wolfcrypt/src/asn.c	Fixes RFC 5280 pathLen enforcement logic for self-issued certs and missing KeyUsage.
src/tls.c	Includes ECH in extension duplicate-detection gating.
src/internal.c	Enforces empty-record rate limit and exposes the new error reason string.
tests/api/test_tls13.h	Registers new TLS 1.3 tests for duplicate ECH and empty-record limit.
tests/api/test_tls13.c	Adds TLS 1.3 regression tests for duplicate ECH and empty-record limit behavior.
tests/api.c	Adds certificate-generation/verification regression tests for pathLen enforcement cases.

Comments suppressed due to low confidence (2)

src/internal.c:1

• The limit check is off-by-one relative to the macro name: with >= WOLFSSL_MAX_EMPTY_RECORDS after pre-increment, the code rejects the Nth empty record where N == WOLFSSL_MAX_EMPTY_RECORDS. If the intent is “allow up to WOLFSSL_MAX_EMPTY_RECORDS empty records and fail on the next one”, change the comparison logic (e.g., increment then check >, or check first then increment) so the name matches the enforced behavior.
  src/internal.c:1
• The reason string is a bit tautological/uninformative (“error” twice). Consider aligning the text with the enum comment for clarity, e.g. “Too many empty records received” or “Empty record limit exceeded”, which reads better in logs and user-facing diagnostics.

---

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

[Copilot]

emptyRecordCount is a byte, but WOLFSSL_MAX_EMPTY_RECORDS is configurable. If a user sets WOLFSSL_MAX_EMPTY_RECORDS > 255, the counter can wrap and defeat the rate-limit. Consider making this counter a wider type (e.g., word16/word32) or enforcing a compile-time cap (with a preprocessor check/error) to prevent wraparound.

Suggested change

	byte emptyRecordCount; /* detect empty record dos attempt */
	word32 emptyRecordCount; /* detect empty record dos attempt */

[Copilot]

WOLFSSL_MAX_EMPTY_RECORDS is a new public-facing configuration knob that impacts runtime behavior/security posture. It should have a brief doxygen-style comment (what it limits, default, and what happens when exceeded / related error code) so integrators can safely tune it.

Suggested change

	/**
	* Maximum number of consecutive empty TLS records accepted from a peer.
	* Default is 32. If this limit is exceeded, record processing fails and the
	* connection is rejected with the related empty-record error condition.
	*/

[AsafMeizner]

Reviewed all three commits. Fixes look correct.

Fix 1: Duplicate ECH Extension (1f75deb)

Adding TLSX_ECH to the duplicate-detection gate works. Test correctly crafts a ClientHello with two ECH extensions and verifies rejection.

Note: this fixes ECH specifically, but extension types > 62 outside the hardcoded exceptions still bypass duplicate detection. A more general approach (hash set for seen types) would future-proof this, though that's a larger change.

Fix 2: Empty Record Limit (d2a8735)

Counter with default 32 matching OpenSSL, reset on non-empty — correct. Both test cases (exceeded limit + counter reset) have good coverage.

Issue: emptyRecordCount is byte but WOLFSSL_MAX_EMPTY_RECORDS is configurable. Values > 255 wrap the counter and defeat the limit. Either widen to word16/word32 or add:

#if WOLFSSL_MAX_EMPTY_RECORDS > 255
    #error "WOLFSSL_MAX_EMPTY_RECORDS must be <= 255 (counter is byte)"
#endif

Fix 3: pathLen for Self-Issued Certs (d6fa2b1)

This is the most important one. Changes are correct:

• pathLen no longer gated on extKeyUsageSet — handles absent KeyUsage per RFC 5280 4.2.1.3
• Self-issued certs enforce issuer's maxPathLen without decrementing — matches RFC 5280 6.1.4(l)
• maxPathLen widened from byte to word16

Tests test_PathLenSelfIssued and test_PathLenNoKeyUsage cover both attack scenarios and verify ASN_PATHLEN_INV_E.

Thanks for the fast turnaround - all three issues addressed with tests in under a day is impressive.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

---

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