Skip to content

Navigation Menu

Sign in
Sign up

gmac: clear the cached message on init - #474

Open
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:master from
yosuke-wolfssl:fix/f_11551
Open

gmac: clear the cached message on init #474
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:master from
yosuke-wolfssl:fix/f_11551

Conversation

@yosuke-wolfssl

@yosuke-wolfssl yosuke-wolfssl commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Problem

wp_gmac_init() never cleared the cached message buffer. wolfSSL has no
streaming GMAC API, so wp_gmac_update() accumulates the whole message in
macCtx->data / macCtx->dataLen and wp_gmac_final() feeds it to the
one-shot wc_GmacUpdate(); only wp_gmac_free() and wp_gmac_dup() ever
touched that buffer. Re-initializing a used EVP_MAC_CTX — the documented way
to reuse one, and the natural GMAC pattern since every message needs a fresh IV
— therefore authenticated previous_message || new_message, reporting success
at every call. A wrong tag with no error.

OpenSSL's default provider resets on every init path (gmac_init() always
calls EVP_EncryptInit_ex()), and OpenSSL's evp_test re-inits and re-runs
every MAC KAT, so scripts/test-openssl.sh reproduces this today: all nine
GMAC entries in scripts/evp_test/evpmac_common.txt fail with TEST_MAC_ERR.

Same defect class as f_11550 (CMAC, PR #473); this is its GMAC sibling.

Fix (src/wp_gmac.c)

wp_gmac_init() discards the cached message once the running check passes and
before the parameters and key are applied, so every init starts a new message:

  • OPENSSL_clear_free() cleanses the plaintext and releases the
    allocation, keeping dataLen equal to the allocated size — the invariant
    wp_gmac_free() and wp_gmac_dup() both rely on for a correct cleanse and
    memdup.
  • data = NULL; dataLen = 0 so wp_gmac_update()'s next
    OPENSSL_realloc() starts clean.

Clearing in init covers both a reset after final and a reset mid-stream, so
wp_gmac_final() is unchanged. No re-keying is needed — wc_GmacUpdate() is a
one-shot over an already-keyed Gmac.

Closes f_11551.

Tests

test_gmac_reinit drives one EVP_MAC_CTX through init/update/final rounds
and an init mid-stream, re-supplying the IV on each init as evp_test does,
and compares every MAC against OpenSSL.

Verification

  • Build clean under -Werror.
  • Negative control fails before the fix — GMAC after reset doesn't match the first MAC — while the OpenSSL half of the same test passes.
  • 212/212 unit tests pass; ASan + UBSan clean.
  • evp_test on evpmac_common.txt: 11 errors → 2. All nine GMAC failures
    clear; the two remaining CMAC failures are PR cmac: take a NULL key in init as a restart with the cached key #473 .

- wp_gmac_init() releases macCtx->data with OPENSSL_clear_free() and
 resets data and dataLen once the running check passes, before the
 parameters and key are applied.
- test_gmac_reinit drives one EVP_MAC_CTX through init/update/final
 rounds plus an init mid-stream, re-supplying the IV on each init,
 and compares every MAC against OpenSSL.
- test/unit.c and test/unit.h register the new test case.
Issue: F-11551
Copilot AI lite review requested due to automatic review settings August 26, 2026 05:53

Copilot AI 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.

Pull request overview

This pull request fixes a correctness bug in the GMAC implementation where re-initializing an EVP_MAC_CTX could unintentionally authenticate previous_message || new_message due to a stale cached message buffer. It aligns wolfProvider behavior with OpenSSL’s expected "init resets state" semantics and adds a regression test to cover both post-final and mid-stream re-init cases.

Changes:

  • Clear and free the cached GMAC message buffer during wp_gmac_init() so each init starts a fresh message.
  • Add a new unit test (test_gmac_reinit) that exercises re-init after EVP_MAC_final() and re-init mid-stream while reusing the cached key, comparing results against OpenSSL.
  • Register the new GMAC re-init test in the unit test harness.

Reviewed changes

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

File Description
src/wp_gmac.c Clears cached message data in wp_gmac_init() to prevent stale-data MAC concatenation across re-inits.
test/test_gmac.c Adds test_gmac_reinit to validate correct behavior across GMAC context resets vs OpenSSL.
test/unit.h Declares the new GMAC re-init unit test.
test/unit.c Registers the new GMAC re-init unit test in the test case list.

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

@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 #474

Scan targets checked: wolfprovider-bugs, wolfprovider-src

Fenrir result: Approved ✅

No new issues found in the changed files.

Advisory only — this automated result does not count as a GitHub approval.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left review comments

At least 1 approving review is required to merge this pull request.

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /