Skip to content

fix(hsm): review fixes for Cosmian/kms#1210 (FIPS SHA-1 gate, hash inference, RotateName cache) - #2

Merged
Manuthor merged 7 commits into
pr-1210-basefrom
claude/optimistic-carson-li1owh
Sep 29, 2026
Merged

Manuthor merged 7 commits into
pr-1210-basefrom
claude/optimistic-carson-li1owh

Conversation

@Manuthor

@Manuthor Manuthor commented Sep 29, 2026 •

Copy link
Copy Markdown
Owner

Summary

Review fixes on top of Cosmian/kms#1210. The base branch pr-1210-base is that PR's head (220e0e89), so this diff contains only the fixes.

Signing (6bcbb441)

  • FIPS: reject SHA-1 RSA signatures on the pre-hashed path as well. A 20-byte digested_data was being signed as a SHA-1 DigestInfo through raw CKM_RSA_PKCS, bypassing the FIPS gate.
  • Infer the RSA PKCS#1 v1.5 hash from the input length only when the input is a digest. A raw message with no explicit hash always uses SHA-256, instead of SHA-1/384/512 for 20/48/64-byte messages.
  • Reject a CryptographicAlgorithm that doesn't match the key type (e.g. ECDSA on an RSA key) with a clear KMIP error.
  • HSM SignatureVerify reports a malformed DER ECDSA signature as invalid instead of returning an error.
  • Merge the duplicated CKM_EDDSA code and fix a stale comment.

RotateName cache (69d9e645)

  • Re-key eligibility checks and HSM latest-generation selection now use find_by_rotate_name_uncached, so they never act on stale data (e.g. a rotation made by another server node).
  • The cache is cleared by keyset name for all owners and generations. It is also cleared by member UID on update, state change (revoke/destroy), delete and HSM re-label.
  • Empty results are no longer cached.
  • Docs updated.

PKCS#11 (436c48e5)

  • A signature cached by a C_Sign length query is reused only for the same data; different data is signed again.

HSM sessions (9a7644d1)

  • The session pool holds only read-write sessions (removes an ignored read_write flag).
  • Document the virtual-memory cost of the 16 MiB RUST_MIN_STACK default.

Tests

  • New unit tests for each fix; the unit tests of all changed crates pass in both FIPS and non-FIPS builds.
  • Clippy is clean.
  • The server's non-FIPS unit tests pass (pre-commit hook).
  • SoftHSM2: 27/27 non-FIPS tests pass. In FIPS, 24/25 pass: rsa_oaep_encrypt fails on the SoftHSM2 2.6.1 used locally, and that failure is already in feat: add key tagging for HSM keys + RotateName cache Cosmian/kms#1210.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AZtgA9JVDCciqEsKuGUJfL

- Only infer the RSA PKCS#1 v1.5 hash from the input length when the input
  is a caller-supplied digest; raw messages always default to SHA-256.
- Reject SHA-1 RSA signatures in FIPS mode for the pre-hashed DigestInfo
  path too (both in SigningAlgorithm resolution and the HSM session gate).
- Reject a cryptographic_algorithm that does not match the key family.
- Treat a malformed DER ECDSA signature as invalid rather than an error.
- Deduplicate the CKM_EDDSA sign/verify arms; fix a stale comment.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01AZtgA9JVDCciqEsKuGUJfL
…cached

- Re-key eligibility (enforce_keyset_latest) and HSM latest-generation
  selection now use Database::find_by_rotate_name_uncached, so they are
  never decided on stale (e.g. other-node) keyset state.
- Invalidate by keyset name across all owners and generation filters,
  and by member UID on update/state change/delete/label writes, so a
  revoked or destroyed generation is not served for the rest of the TTL.
- Never cache empty results.
- Document the invalidation and multi-node consistency model.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01AZtgA9JVDCciqEsKuGUJfL
A signature cached by a C_Sign length query is now only reused when the
follow-up call signs identical data; otherwise the module signs again
instead of returning a signature computed over the earlier data.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01AZtgA9JVDCciqEsKuGUJfL
- SlotManager::checkout_session no longer takes a read_write flag it
  ignored for pooled sessions: the pool only ever holds read-write sessions.
- Document the virtual-memory implication of the 16 MiB RUST_MIN_STACK
  default (it also applies to the blocking pools running HSM calls) and
  that operators can override it.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01AZtgA9JVDCciqEsKuGUJfL
@Manuthor
Manuthor changed the base branch from develop to pr-1210-base September 29, 2026 05:54
@Manuthor Manuthor changed the title Migrate build scripts from Nix to MISE task framework fix(hsm): review fixes for Cosmian/kms#1210 (FIPS SHA-1 gate, hash inference, RotateName cache) Sep 29, 2026
Gate the jobs that need upstream-only secrets or infrastructure on
`github.repository == 'Cosmian/kms'`: AWS XKS remote server, google-cse,
secret_vault/secret_aws/secret_azure and aws-cloudhsm. Matrix entries
cannot be filtered by a job-level `if`, so they move to dedicated
`test-nix-upstream` and `hsm-aws-cloudhsm` jobs; cargo-publish still runs
its dry-run when those jobs are skipped.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01AZtgA9JVDCciqEsKuGUJfL
…ngelog

Replace the inline `#[cfg(not(feature = "non-fips"))]` block inside
`rsa_pkcs1_from_hash` with a function-level gated
`check_rsa_signature_hash_allowed`, per the no-inline-feature-gating rule,
and add the branch CHANGELOG entry.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01AZtgA9JVDCciqEsKuGUJfL
The Proteccio and Crypt2Pay jobs need upstream-only secrets and network
access (Proteccio IP/password/slot, Crypt2Pay password and OpenVPN
profile). On forks they ran with empty credentials and failed
(`C_Login` CKR_ARGUMENTS_BAD, `C_GenerateKeyPair` CKR_MECHANISM_INVALID).

Move them, together with aws-cloudhsm, into one `hsm-upstream` job gated
on `github.repository == 'Cosmian/kms'`, keeping a fixed concurrency
group per hardware HSM and the existing job names. The `hsm` job keeps
the software/simulated HSMs (utimaco, softhsm2, kryoptic) and no longer
receives hardware HSM secrets.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01AZtgA9JVDCciqEsKuGUJfL
@Manuthor
Manuthor merged commit 031c6b4 into pr-1210-base Sep 29, 2026
70 checks passed
@Manuthor
Manuthor deleted the claude/optimistic-carson-li1owh branch September 29, 2026 08:16
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