Repository navigation
Conversation
|
Can one of the admins verify this patch? |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused validation is correct and covered by positive and negative regression cases.
Review effort: Balanced
Findings: None
What changed in this PR
Rejects trailing bytes in TLS 1.3 ClientHello pre_shared_key extensions, enforcing RFC 8446 parsing requirements.
Changes:
- Validates complete consumption of
OfferedPsks. - Adds malformed-input and valid-control regression tests.
| File | Description |
|---|---|
src/tls.c |
Rejects data remaining after PSK binder parsing. |
tests/api/test_tls_msgtype.c |
Tests trailing-byte rejection and valid parsing. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
d4afa61 to
236f5f4
Compare
|
Hi @ducanhmit , Sorry for the slow reply. Thank you for this PR! We require a signed contributor agreement to accept third-party code (please see our https://github.com/wolfSSL/wolfssh/blob/master/CONTRIBUTING.md). Best, |
|
OK to test- contributor agreement approved |
|
Hi @ducanhmit , can you rebase this PR, it's a good bit behind master. |
|
TLSX_PreSharedKey_Parse_ClientHello() parsed the identities and binders vectors of OfferedPsks but did not check that they consumed the whole extension_data, so extra bytes after a valid OfferedPsks were silently accepted. RFC 8446 Section 4.2 requires a decode_error in this case. Return BUFFER_E when bytes remain after the binders, and add a test that a valid OfferedPsks followed by one extra byte is rejected through TLSX_Parse() while the same body without the byte is accepted. Fixes wolfSSL#11134
236f5f4 to
be5b775
Compare
|
@claude review |
Description
TLSX_PreSharedKey_Parse_ClientHello()parsed the identities and binders vectors ofOfferedPsksbut did not check that they consumed the wholeextension_data. Extra bytes after a validOfferedPskswere silently accepted, andTLSX_Parse()then skipped over them. RFC 8446 Section 4.2 requires the receiver to abort withdecode_errorin this case.The parser now returns
BUFFER_Ewhen bytes remain after the binders.Fixes #11134
Testing
test_tls_msgtype_psk_ch_binder_gates: a validOfferedPsksfollowed by one extra byte is rejected throughTLSX_Parse()withBUFFER_E, and the same body without the byte is still accepted.tests/unit.testpasses with./configure --enable-psk --enable-session-ticket --enable-debug.Checklist