Key Delete Guard

[rajesh-gali]

No description provided.

[Copilot]

Pull request overview

This PR introduces a “deleted key” guard across the Rust API layer by adding an explicit validity check to key types and enforcing that check in crypto operations, with new integration tests to validate behavior after key deletion.

Changes:

• Extend HsmKey with is_valid() and implement it for all key wrapper types (including key pairs).
• Add key.is_valid()? checks in RSA/ECC/HMAC/AES operations (single-shot + streaming init paths) plus HKDF/ECDH derive and unwrap entrypoints.
• Add integration tests asserting operations fail with HsmError::InvalidKey after key deletion and that public-key verification remains functional.

Reviewed changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
api/tests/src/algo/rsa/sign_tests.rs	Adds deleted-key sign failure + public-key verify-after-delete tests.
api/tests/src/algo/rsa/hash_sign_tests.rs	Adds deleted-key hash-sign and streaming init failure tests + verify-after-delete test.
api/tests/src/algo/rsa/enc_tests.rs	Adds deleted-key decrypt failure test.
api/tests/src/algo/hmac/hmac_tests.rs	Adds deleted-key HMAC sign/verify init failure tests.
api/tests/src/algo/ecc/sign_tests.rs	Adds deleted-key ECC sign failure + verify-after-delete test.
api/tests/src/algo/ecc/hash_sign_tests.rs	Adds deleted-key ECC hash-sign + streaming init failure tests + verify-after-delete test.
api/tests/src/algo/ecc/ecdh_tests.rs	Adds deleted-key ECDH derive failure test.
api/tests/src/algo/aes/xts_tests.rs	Adds deleted-key AES-XTS encrypt/decrypt + streaming init failure tests.
api/tests/src/algo/aes/gcm_tests.rs	Adds deleted-key AES-GCM encrypt/decrypt + streaming init failure tests.
api/tests/src/algo/aes/cbc_tests.rs	Adds deleted-key AES-CBC encrypt/decrypt + streaming init failure tests.
api/lib/src/traits/key.rs	Adds HsmKey::is_valid() and tightens some key-op error bounds.
api/lib/src/algo/rsa/wrap.rs	Enforces deleted-key check before RSA-AES wrap.
api/lib/src/algo/rsa/sign.rs	Enforces deleted-key check for RSA sign/verify.
api/lib/src/algo/rsa/key.rs	Enforces deleted-key check for RSA key-pair unwrap path.
api/lib/src/algo/rsa/hash_sign.rs	Enforces deleted-key check for RSA hash-sign/verify and streaming init.
api/lib/src/algo/rsa/enc.rs	Enforces deleted-key check for RSA encrypt/decrypt.
api/lib/src/algo/mod.rs	Implements is_valid() for key wrapper macros based on inner deleted state.
api/lib/src/algo/kdf/hkdf.rs	Enforces deleted-key check for HKDF base key.
api/lib/src/algo/hmac/sign.rs	Enforces deleted-key check for HMAC sign/verify and streaming init.
api/lib/src/algo/ecc/sign.rs	Enforces deleted-key check for ECC sign/verify.
api/lib/src/algo/ecc/hash_sign.rs	Enforces deleted-key check for ECC hash-sign/verify and streaming init.
api/lib/src/algo/ecc/ecdh.rs	Enforces deleted-key check for ECDH derive base key.
api/lib/src/algo/aes/xts.rs	Enforces deleted-key check for AES-XTS encrypt/decrypt and streaming init.
api/lib/src/algo/aes/key.rs	Enforces deleted-key check for RSA-based AES unwrap paths.
api/lib/src/algo/aes/gcm.rs	Enforces deleted-key check for AES-GCM encrypt/decrypt and streaming init.
api/lib/src/algo/aes/cbc.rs	Enforces deleted-key check for AES-CBC encrypt/decrypt and streaming init.

[Copilot]

HsmKey::is_valid() only protects call sites that explicitly invoke it. There are still internal call paths that use key.handle()/key.session() directly (e.g., key report generation in api/lib/src/ddi/{rsa,ecc}.rs calls generate_key_report(&key.session(), key.handle(), ...)) and can therefore operate on a deleted key handle, risking use-after-delete if the handle ID is reused. Consider enforcing the deleted check in a single shared place (e.g., make handle access fallible / add handle_checked() and use it in all DDI wrappers, or ensure all DDI wrapper entrypoints call key.is_valid() before using the handle).

[Copilot]

The comment in HsmKey for [<$pub_name>] is incomplete ("remains valid across") and doesn’t explain the intended validity semantics. Either complete the sentence (e.g., explicitly mention deletion of the associated private key) or remove the comment to avoid confusion.

Suggested change

// The public key is a software-only object that remains valid across

[Copilot]

The additional From<HsmError> bound on HsmKeyDeriveOp::Error, HsmKeyUnwrapOp::Error, and HsmKeyPairUnwrapOp::Error is a public API tightening and may break downstream implementations that use custom error types. Since the current implementations already use HsmError directly, consider removing this bound and explicitly mapping is_valid() failures to the operation error type at call sites (or documenting the breaking change if it’s intentional).

[Copilot]

This adds a new deleted-key guard in HsmRsaAesWrapAlgo::encrypt (key.is_valid()?), but there’s no corresponding test asserting that wrapping with a deleted RSA wrapping public key fails with InvalidKey. Please add a regression test to ensure the new behavior is covered and doesn’t get removed accidentally.

[Copilot]

This changes HKDF derivation to fail fast on deleted base keys (base_key.is_valid()?). Please add a test that deletes the base HsmGenericSecretKey and asserts HsmKeyManager::derive_key returns HsmError::InvalidKey, to cover the new behavior.