Fix hmac_smoke sign-permission test to run on real HW (not just emu) by netsweng · Pull Request #741 · Azure/azihsm-sdk

Stuart R. Anderson (netsweng) · 2026-09-23T15:44:09Z

Summary

test_hmac_requires_sign_permission_smoke is supposed to prove a derive-only HMAC key can't produce a MAC, but it hung off a cfg gate (#[cfg(not(feature = "mock"))]) that also included real hardware (nix backend), and its body assumed the emu-only rejection point. Real HSM firmware rejects the derive-only VarHmac256 HKDF-derive key's creation outright (every HMAC-class key requires SignVerify, with no derive-only carve-out) before the test ever reaches its intended sign-permission-on-MAC assertion, causing spurious failures on real hardware.

The intent of this PR is to make the test actually pass on real HW, not merely to cherry-pick #740's original (incorrect) fix as-is — that fix narrowed the gate to #[cfg(feature = "emu")], which hides the real-HW path instead of fixing it to work there too.

Fix

Instead of gating the test to emu-only, relaxed the assertion to accept either backend's valid rejection point:

emu: permits creating the derive-only key, then rejects the MAC attempt with InvalidPermissions (the original scenario this test was written for).
real HW: rejects the derive-only key's creation with InvalidPermissions before a MAC is ever attempted.

Both are valid proof that a key lacking sign permission can't produce a MAC, so the test now runs on both backends instead of being emu-only.

Testing

cargo xtask nextest --features emu --package azihsm_ddi_mbor_types (emu path): passed
Re-ran on real HW via SSH to a hardware-attached VM with AZIHSM_USE_TPM=1: passed, exercising the real-HW InvalidPermissions-on-creation rejection path

Copilot

Copilot review overview

🟢 Approval recommended

The documented emulator-only test scope is correctly enforced.

Review effort: Lite
Findings: None

What changed in this PR

Restricts the HMAC permission smoke test to emulator builds, preventing failures on real hardware.

Changes:

Replaced the not(mock) gate with feature = "emu".

File	Description
`ddi/mbor/types/tests/integration/hmac_smoke.rs`	Limits the sign-permission test to emulator builds.

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

Stuart R. Anderson (netsweng) · 2026-09-23T17:44:18Z

This needs to be rebased to pick up a fix form PR #742 once it merges.

… only (#740) The test's module doc already documents this as an emu-only test (deriving a derive-only VarHmac256 key via HKDF then attempting to sign), but the #[cfg] gate used not(feature = "mock") which also included real hardware (nix backend). Real HSM firmware rejects the derive-only VarHmac256 HKDF-derive step with InvalidPermissions before the test reaches its intended sign-permission assertion, causing spurious failures on real hardware. Verified: all 616 tests pass with --features mock, and a locally built azihsm_ddi_tests binary (nix/real-hardware backend) run against real AZIHSM hardware now passes 599/599 (0 failed). Co-authored-by: Stuart R. Anderson <stuanderson@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🟢 Approval recommended

The change matches the documented test scope and has no unresolved review issues.

Review effort: Lite
Findings: None

Stuart R. Anderson (netsweng) · 2026-10-09T16:35:49Z

Follow-up: confirmed real-HW behavior is a deliberate firmware design divergence, not a stale build

Retested this PR's fix on mock, emu, and real hardware:

mock: 2/2 pass. test_hmac_requires_sign_permission_smoke is correctly compiled out entirely (not just skipped), confirming the emu-only gate works as intended.
emu: 4/4 pass, including test_hmac_requires_sign_permission_smoke.
real HW (nix backend, AZIHSM_USE_TPM=1): normal run correctly excludes the test. I temporarily re-enabled it to confirm the original failure mode described in this PR — it fails with DdiStatus::InvalidPermissions (0x08700022) directly from the HkdfDerive DDI command (opcode 1075), before the test ever reaches its intended sign-permission assertion. This exactly reproduces the bug this PR fixes.

Root cause, traced to the real firmware source

I dug further into why real HW rejects this (initially suspected a stale firmware build lagging the SDK's fw/core emulator logic — that's not the case). Checking the actual production firmware source (azihsm-fw, projects/microsoft/hsp/manticore/cp/hsm):

hkdf_inner (hsm/src/partition/session/app_sess/kdf.rs:159) builds derived HMAC/VarHmac keys via VarLenHmacShaKeyImported::new(kind, key_properties.key_usage.try_into()?, ...).

That usage conversion goes through HmacKeyUsage's TryFrom<DdiKeyUsage> (hsm/src/partition/vault/key/hmac.rs:62-67), shared by both fixed and variable-length HMAC keys:

impl TryFrom<DdiKeyUsage> for HmacKeyUsage {
    fn try_from(usage: DdiKeyUsage) -> Result<Self, Self::Error> {
        match usage {
            DdiKeyUsage::SignVerify => Ok(HmacKeyUsage::SignVerify),
            _ => Err(HsmErr::InvalidPermissions),
        }
    }
}

There is no Derive-only carve-out anywhere in real firmware for HMAC-class keys — it unconditionally requires SignVerify.

This means the SDK's fw/core software emulator (what emu runs) is more permissive than real production firmware has ever been for this scenario — derive-only VarHmac key creation is a deliberate policy difference, not a version-lag bug that a firmware re-flash would fix.

Conclusion

The #[cfg(feature = "emu")] gate in this PR is correct and should be considered permanent, not a stopgap pending a firmware update. This test validates emulator-only permission behavior that real hardware firmware was never designed to support. Recommend tracking the fw/core vs. real-firmware policy divergence separately (emulator/firmware parity backlog) rather than trying to make this specific test pass everywhere.

test_hmac_requires_sign_permission_smoke was gated to emu only because real firmware rejects the derive-only VarHmac key at creation time (InvalidPermissions from HkdfDerive), rather than at the later MAC-sign attempt like the emu backend. Change the gate to not(feature = "mock") and accept InvalidPermissions at either the key-derivation step (real HW) or the MAC-sign step (emu), so the test exercises the same permission-enforcement guarantee uniformly across both non-mock backends. Verified: mock 2/2 (test still excluded), emu 4/4, real HW 4/4 (previously 3/4 with the unconditional emu-only gate removed).

Copilot

🟡 Changes recommended

The test still runs on real hardware instead of applying the emulator-only gate promised by the cherry-pick.

1 open finding

Gate MAC permission test behind the emu feature · New

🧠 Review effort: Lite

Copilot

🟡 Changes recommended

The reported mock-feature run excludes this test, so the claimed emulator-path validation still needs to be run or corrected.

1 open finding

Reported mock-feature run omits the emu-gated test · New

1 resolved since last review

Gate MAC permission test behind the emu feature

🧠 Review effort: Lite

Stuart R. Anderson (netsweng) · 2026-10-09T22:22:16Z

-//! - **Sign permission** (emu only): a `derive`-only HMAC key cannot
-//!   generate a MAC — rejected with `InvalidPermissions` (MAC
-//!   generation is a PKCS#11 `C_Sign` operation requiring `CKA_SIGN`).
+//! - **Sign permission** (emu + real HW, not mock): a `derive`-only


Good catch — the PR description's testing command was wrong. I re-ran it correctly as cargo xtask nextest --features emu --package azihsm_ddi_mbor_types (the test is gated #[cfg(not(feature = "mock"))], so --features mock skips it entirely; --features emu is the correct flag to exercise the emu rejection-of-MAC-attempt path). Confirmed it passes. Updated the PR description accordingly.

Copilot AI lite review requested due to automatic review settings September 23, 2026 15:44

Copilot started reviewing on behalf of Stuart R. Anderson (netsweng) September 23, 2026 15:44 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Stuart R. Anderson (netsweng) enabled auto-merge September 23, 2026 15:47

Stuart R. Anderson (netsweng) requested a review from Jayant Gandhi (jaygmsft) September 23, 2026 16:52

Copilot AI review requested due to automatic review settings September 23, 2026 19:43

Stuart R. Anderson (netsweng) force-pushed the stuanderson/cherry-pick-hmac-fix-main branch from 30767f2 to 2a01220 Compare September 23, 2026 19:43

Copilot started reviewing on behalf of Stuart R. Anderson (netsweng) September 23, 2026 19:44 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Stuart R. Anderson (netsweng) requested a review from Rajesh Gali (rajesh-gali) September 23, 2026 23:35

Rajesh Gali (rajesh-gali) requested changes Oct 8, 2026

View reviewed changes

Comment thread ddi/mbor/types/tests/integration/hmac_smoke.rs Outdated

Copilot AI lite review requested due to automatic review settings October 9, 2026 16:55

Copilot started reviewing on behalf of Stuart R. Anderson (netsweng) October 9, 2026 16:55 View session

Copilot AI reviewed Oct 9, 2026

View reviewed changes

Comment thread ddi/mbor/types/tests/integration/hmac_smoke.rs

Stuart R. Anderson (netsweng) changed the title ~~Cherry-pick: Fix hmac_smoke sign-permission test to run on emu only~~ Fix hmac_smoke sign-permission test to run on real HW (not just emu) Oct 9, 2026

Stuart R. Anderson (netsweng) requested a lite review from Copilot October 9, 2026 19:57

Copilot started reviewing on behalf of Stuart R. Anderson (netsweng) October 9, 2026 19:58 View session

Copilot AI reviewed Oct 9, 2026

View reviewed changes

Navigation Menu

Repository navigation

Fix hmac_smoke sign-permission test to run on real HW (not just emu) - #741

Fix hmac_smoke sign-permission test to run on real HW (not just emu)#741
Stuart R. Anderson (netsweng) wants to merge 2 commits into
mainfrom
stuanderson/cherry-pick-hmac-fix-main

Stuart R. Anderson (netsweng) commented Sep 23, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Stuart R. Anderson (netsweng) commented Sep 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Stuart R. Anderson (netsweng) commented Oct 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Stuart R. Anderson (netsweng) Oct 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Stuart R. Anderson (netsweng) commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Fix

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

Stuart R. Anderson (netsweng) commented Sep 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

Uh oh!

Stuart R. Anderson (netsweng) commented Oct 9, 2026

Follow-up: confirmed real-HW behavior is a deliberate firmware design divergence, not a stale build

Root cause, traced to the real firmware source

Conclusion

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Stuart R. Anderson (netsweng) Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Stuart R. Anderson (netsweng) commented Sep 23, 2026 •

edited

Loading