cmac: take a NULL key in init as a restart with the cached key - #473
cmac: take a NULL key in init as a restart with the cached key#473yosuke-wolfssl wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes CMAC context reinitialization semantics so EVP_MAC_init(ctx, NULL, 0, NULL) correctly resets the CMAC state while retaining the cached key, aligning wolfProvider behavior with the documented EVP MAC contract and OpenSSL’s default provider behavior.
Changes:
- Update
wp_cmac_set_key()/wp_cmac_init()so a NULL key triggers a restart using the cached key (when present). - Add a unit test (
test_cmac_reinit) that exercises reinit-after-final and mid-stream reset behavior and compares outputs against OpenSSL. - Wire the new CMAC reinit test into the unit test harness.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/unit.h | Declares the new test_cmac_reinit unit test. |
| test/unit.c | Registers test_cmac_reinit in the unit test list. |
| test/test_cmac.c | Adds coverage for CMAC reset behavior (post-final and mid-stream) and OpenSSL equivalence checks. |
| src/wp_cmac.c | Implements CMAC restart-on-init with cached key when key == NULL. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #473
Scan targets checked: wolfprovider-bugs, wolfprovider-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
b4e345f to
e9d81e0
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #473
Scan targets checked: wolfprovider-bugs, wolfprovider-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
- wp_cmac_set_key() substitutes the cached length for a NULL key, rejects a zero length, and applies the maximum-length and expKeySize checks only to a supplied key. - wp_cmac_set_key() clears macCtx->keyLen while copying a supplied key into macCtx->key, runs the wolfSSL CMAC init from that buffer, and assigns macCtx->keyLen once that call succeeds. - wp_cmac_init() calls wp_cmac_set_key() with restart set whenever a key is passed in or one is already cached. - The doxygen for the key parameter of both functions covers the NULL case. - test_cmac_reinit drives one EVP_MAC_CTX through init, update and final rounds with no key argument and through an init mid-stream, comparing the MACs with OpenSSL. - test_cmac_reinit_stale_key checks that a key rejected for want of a cipher is not picked up by a later keyless init. - Both new tests run against OpenSSL and wolfProvider; test_cmac_reinit is declared in unit.h and registered in unit.c. Issue: F-11550
e9d81e0 to
24f9585
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #473
Scan targets checked: wolfprovider-bugs, wolfprovider-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
Problem
wp_cmac_init()only restarted the wolfSSL CMAC object when a key was supplied, soEVP_MAC_init(ctx, NULL, 0, NULL)— the documented way to reset a MAC context while keeping the installed key — did nothing. Two failure modes:EVP_MAC_final()wc_CmacFinal()frees and zeroes theCmac, so the nextEVP_MAC_update()returns 0finaldigest/buffer/totalSzsurvive, so the MAC silently coversmsgA‖msgBinstead ofmsgBThe second case reports success at every call — a wrong MAC with no error. OpenSSL's default provider implements the reset explicitly (
cmac_prov.c→CMAC_Init(ctx, NULL, 0, NULL, NULL)), and OpenSSL'sevp_testexercises it for every MAC KAT:scripts/evp_test/evpmac_common.txtcarries three CMAC entries with noNoReinit, soscripts/test-openssl.shreproduces this today.Fix (
src/wp_cmac.c)wp_cmac_set_key()takeskey == NULLas "keep the cached key, just restart". The length andexpKeySizechecks and the cleanse/copy ofmacCtx->keyare gated onkey != NULL; a NULL key with nothing cached fails.wp_cmac_init()calls it whenever a key is supplied or one is already cached, sowc_InitCmac_ex()runs on every init and re-derives the AES key schedule and the k1/k2 subkeys.A context that never received a key still returns success from init, matching
wp_hmac_init()andwp_gmac_init(). OpenSSL fails there, but that divergence is uniform across all three wolfProvider MACs and belongs in its own change.Closes f_11550.
Tests
test_cmac_reinitdrives oneEVP_MAC_CTXthrough init/update/final rounds with no key argument, plus an init mid-stream, and compares every MAC against OpenSSL.Verification
-Werror.EVP_MAC_updatereturns 0 afterfinal, andCMAC after mid-stream reset covers stale dataon the mid-stream path.--enable-debugconfigs; all five CMAC cases pass.