Skip to content

Fix odd-length CertificateRequest signature_algorithms acceptance#10630

Merged
douzzer merged 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_4890
Jun 8, 2026
Merged

Fix odd-length CertificateRequest signature_algorithms acceptance#10630
douzzer merged 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_4890

Conversation

@yosuke-wolfssl

Copy link
Copy Markdown
Contributor

Description

The TLS 1.2 CertificateRequest parser bounds-checked the supported_signature_algorithms vector but never verified that its length is an even multiple of the 2-byte SignatureAndHashAlgorithm element size. PickHashSigAlgo iterates while (i+1) < hashSigAlgoSz, so an odd trailing byte was silently ignored — a malformed vector (e.g. length 3: one valid pair + 1 junk byte) could select an algorithm and return success instead of being rejected.

This adds the missing parity check, matching what the TLS 1.3 signature_algorithms extension parser already does (TLSX_SignatureAlgorithms_Parse in src/tls.c).

Addressed by f_4890.

Changes

  • src/internal.c — DoCertificateRequest: reject TLS 1.2 supported_signature_algorithms lengths not divisible by HELLO_EXT_SIGALGO_SZ with BUFFER_ERROR, before PickHashSigAlgo. The check sits ahead of the client-cert/key handling so the malformed message is rejected consistently even when no local client certificate is configured.
  • tests/api/test_tls.c / tests/api/test_tls.h — new regression test test_tls12_certreq_odd_sigalgs. It drives a real TLS 1.2 memio handshake (server requesting a client cert), locates the server's CertificateRequest, shrinks the sig-algs vector by one byte while decrementing the record/handshake/sig-algs length fields so the message stays self-consistent (only the length parity is wrong), and asserts the client rejects with BUFFER_ERROR.

Testing

  • New test fails without the src/internal.c fix (client silently accepts the odd-length vector and continues the handshake) and passes with it — confirming it catches the regression rather than passing incidentally.
  • Full tests/unit.test suite passes: Success for all configured tests (1428 passed, 0 failed).

@yosuke-wolfssl yosuke-wolfssl self-assigned this Jun 8, 2026
Copilot AI review requested due to automatic review settings June 8, 2026 02:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This pull request hardens the TLS 1.2 CertificateRequest parser by rejecting malformed supported_signature_algorithms vectors whose length is not an even multiple of the 2‑byte SignatureAndHashAlgorithm element size, aligning behavior with existing TLS 1.3 parsing.

Changes:

  • Reject odd-length TLS 1.2 supported_signature_algorithms in DoCertificateRequest() with BUFFER_ERROR before algorithm selection.
  • Add a regression test that performs a real TLS 1.2 memio handshake, corrupts the CertificateRequest sig-algs vector length parity while keeping length fields self-consistent, and asserts the client fails with BUFFER_ERROR.
  • Register the new test in the TLS API test list.

Reviewed changes

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

File Description
src/internal.c Adds an explicit modulo check to reject malformed TLS 1.2 sig-algs vectors before calling PickHashSigAlgo().
tests/api/test_tls.c Adds a memio-based regression test that mutates a server CertificateRequest to have an odd sig-algs vector length and expects BUFFER_ERROR.
tests/api/test_tls.h Declares and registers the new regression test in the TLS test group.

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

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #10630

Scan targets checked: none
Failed targets: wolfcrypt-rs-bugs, wolfssl-bugs, wolfssl-src

⚠️ Review incomplete — one or more scan targets failed before findings could be produced. See the Fenrir PR review detail page for logs.

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #10630

Scan targets checked: none
Failed targets: wolfcrypt-rs-bugs, wolfssl-bugs, wolfssl-src

⚠️ Review incomplete — one or more scan targets failed before findings could be produced. See the Fenrir PR review detail page for logs.

@douzzer douzzer added the Staged Staged for merge pending final test results and review label Jun 8, 2026
@douzzer douzzer merged commit e513172 into wolfSSL:master Jun 8, 2026
485 of 490 checks passed
@yosuke-wolfssl yosuke-wolfssl deleted the fix/f_4890 branch June 8, 2026 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Staged Staged for merge pending final test results and review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants