Skip to content

Fix TLS CBC MAC verify and benchmark PQ option table for crypto callback builds - #11447

Open
dgarske wants to merge 2 commits into
wolfSSL:masterfrom
dgarske:cryptocb_fixes
Open

Fix TLS CBC MAC verify and benchmark PQ option table for crypto callback builds#11447
dgarske wants to merge 2 commits into
wolfSSL:masterfrom
dgarske:cryptocb_fixes

Conversation

@dgarske

@dgarske dgarske commented Sep 11, 2026

Copy link
Copy Markdown
Member

PR: crypto callback TLS CBC MAC verify and benchmark NO_MAIN_DRIVER fixes

Two independent fixes for crypto callback devices backing the TLS and hash path, such as a wolfHSM offload port.

1. TLS CBC record MAC verification with a crypto callback device

TLS_hmac() verifies CBC record MACs with Hmac_UpdateFinal_CT(), which reads the raw hash state via wc_Sha*FinalRaw(). A callback device that computes the HMAC never populates that software state, so the MAC comes out wrong and the peer reports a decrypt error on any MAC-then-Encrypt CBC suite. When the HMAC's devId is a callback device, TLS_hmac() now uses Hmac_UpdateFinal() instead: the public update/final path, which works for device-backed and software HMAC alike and keeps the Lucky13 padding-time equalization. It is the same variant FIPS and self-test builds already use.

2. benchmark.c PQ option table under NO_MAIN_DRIVER

bench_pq_asym_opt[] is guarded by !WOLFSSL_BENCHMARK_ALL && !MAIN_NO_ARGS, but its only readers are inside #ifndef NO_MAIN_DRIVER, so a NO_MAIN_DRIVER build hits -Werror=unused-const-variable. Add !NO_MAIN_DRIVER to the guard.

Test status

test_wc_CryptoCb_TLS_CBC_HMAC (tests/api.c) drives a TLS 1.2 CBC handshake through a callback device that owns the HMAC key and never populates the software hash state; it fails without fix 1 and passes with it. It needs WOLF_CRYPTO_CB_SETKEY, so a new .github/workflows/cryptocb-setkey.yml runs make check under --enable-cryptocbutils=setkey. check-source-text clean.

Copilot AI lite review requested due to automatic review settings September 11, 2026 21:38
@dgarske dgarske closed this Sep 11, 2026

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.

🟡 Changes recommended

Find-mapped callback devices can still select the raw-state verification path, and regression coverage is still needed.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This pull request updates TLS CBC MAC verification for crypto-callback devices and fixes PQ benchmark compilation for NO_MAIN_DRIVER builds.

Changes:

  • Routes callback-backed HMAC verification through public update/final operations.
  • Aligns the PQ benchmark option-table guard with its use sites.
File summaries
File Description
wolfcrypt/benchmark/benchmark.c Fixes conditional compilation of the PQ option table.
src/tls.c Adjusts TLS CBC HMAC verification dispatch.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/tls.c Outdated
Comment thread src/tls.c
Comment on lines +1483 to +1484
ret = Hmac_UpdateFinal(hmac, digest, in,
totalSz, myInner, innerSz);
@dgarske dgarske reopened this Sep 11, 2026
…vice is registered since the raw-hash constant-time path reads software hash state a device does not maintain

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.

🟡 Changes recommended

Critical TLS CBC HMAC callback handling remains unresolved for INVALID_DEVID find mappings and needs regression coverage.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread src/tls.c
* software threat model. A device that declines HMAC and falls
* through to software then runs the same update/final variant
* FIPS and selftest builds always use (see the #else branch). */
if (hmac->devId != INVALID_DEVID) {
@dgarske dgarske self-assigned this Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants