[auto-rotation feature] feat: generalize objects rotation - all key types + certificates#968
[auto-rotation feature] feat: generalize objects rotation - all key types + certificates#968Manuthor wants to merge 30 commits into
Conversation
fa12cc6 to
5f27e21
Compare
There was a problem hiding this comment.
Pull request overview
This PR is the first in the key auto-rotation feature stack. It introduces the canonical auto-rotation specification document and lands the underlying manual rotation implementation for keys and certificates (ReKey, ReKeyKeyPair, ReCertify), plus the database plumbing and test vectors needed for wrapping-key dependant rewrites and certificate renewal link chains.
Changes:
- Add comprehensive documentation for rotation policy attributes, scheduler semantics, and rotation/renewal scenarios (with diagrams and attribute tables).
- Refactor and extend server KMIP operations to implement manual rotation flows for symmetric keys, key pairs, and certificate renewal (
ReCertify) using a shared orchestration trait. - Add
ObjectsStore::find_wrapped_by()across SQL backends to support wrapping-key rotations that must re-wrap dependants, and extend test vectors/runner docs accordingly.
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Adds a documentation link for scheduled key auto-rotation. |
| documentation/mkdocs.yml | Registers the new Key Auto-Rotation documentation page in the nav. |
| documentation/docs/kmip_support/key_auto_rotation.md | Adds the full auto-rotation specification and scenarios (policy attributes, flows, roadmap). |
| crate/test_kms_server/src/vector_runner.rs | Registers new negative and positive vectors for ReCertify and offset state verification. |
| crate/test_kms_server/README.md | Updates vector counts and documents newly added vectors. |
| crate/server/src/core/wrapping/wrap.rs | Tightens self-wrap handling and bypasses KEK ownership checks for server-wide KEK. |
| crate/server/src/core/operations/rekey/* | Introduces a new rekey/ module with shared orchestration + symmetric/keypair flows. |
| crate/server/src/core/operations/recertify.rs | Adds server-side ReCertify implementation and dependant relinking behavior. |
| crate/server/src/core/operations/{mod.rs,message.rs,dispatch.rs} | Wires ReCertify through dispatch and operation processing. |
| crate/server/src/core/operations/key_ops/mod.rs | Fixes lifecycle setup so PreActive objects retain a future activation_date. |
| crate/server/src/core/operations/certify/* | Broadens visibility of certify helpers/types for reuse by ReCertify. |
| crate/server/src/core/kms/kmip.rs | Adds the KMS::recertify entry point. |
| crate/interfaces/src/stores/objects_store.rs | Adds find_wrapped_by() to the store trait (default empty). |
| crate/server_database/src/core/database_objects.rs | Exposes Database::find_wrapped_by() across backends. |
| crate/server_database/src/stores/sql/{sqlite,pgsql,mysql}.rs | Implements find_wrapped_by() using JSON queries in each SQL backend. |
| crate/kmip/src/kmip_2_1/{kmip_operations,kmip_messages}.rs | Adds KMIP 2.1 ReCertify request/response types + message (de)serialization. |
| crate/kmip/src/kmip_1_4/kmip_operations.rs | Adds 1.4↔2.1 conversions for ReCertify types and operation mapping. |
| CHANGELOG/feat_key-rotation-manual.md | Adds the required branch-specific changelog for manual rotation changes. |
| CHANGELOG/docs_key-autorotation-spec.md | Adds the required branch-specific changelog for the spec doc addition. |
291bfbd to
1b09922
Compare
tbrezot
left a comment
There was a problem hiding this comment.
I could not go through all diffs, but thanks to your documentation I could grasp enough of the idea to have a few questions (cf my comment of the doc). I will complete the review once we have discussed those points.
9eb009a to
854aa02
Compare
5911dc5 to
dcc5aa5
Compare
tbrezot
left a comment
There was a problem hiding this comment.
A lot of effort has been put into this MR, and it greatly improves the initial implementation. I still have a few comments though:
- a few nitpicks that are trivial to correct;
- maybe an issue concerning the key-selection procedure;
- some deeper concerns about concurrency management, which are orthogonal to this feature and may be addressed in a dedicated MR.
Also, since it is structuring, it might gain from requesting a review from someone else. In any case, I may spend more time on it if you delay the merge (since it is huge, I could not read everything).
d7c575c to
f610a78
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add pin_fn_depth macro variant in dispatch for functions returning (Resp, Option<u32>) - Switch MACVerify dispatch arm to pin_fn_depth to unpack mac_verify tuple return - Remove unused kms.message() wrapper (route calls operations::message() directly) - Remove unused RequestMessage/ResponseMessage imports in kms/kmip.rs - Fix google_cse encrypt/decrypt callers to unpack (Resp, Option<u32>) tuples - Fix kmip.rs single-op dispatch path (dispatch() returns KResult<Operation>, not tuple) - Fix ui.vendor.non-fips.sha256 Nix hash Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The method was removed but is called by 6+ test files in crate/server/src/tests/. Restore it with #[allow(dead_code)] since it is only called from cfg(test) code and cargo build (without --tests) cannot see those callers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ckms subprocess cannot negotiate ML-DSA-44 TLS handshake without calling init_openssl_providers() at startup. Mark as ignored until the ckms binary is updated to support PQC TLS client connections. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move the canonical script from scripts/ to .mise/scripts/docs/ and update all callers to the new path: - .mise/tasks/docs/log-index-check - .pre-commit-config.yaml (update-log-index hook entry) - .mise/scripts/docs/generate_docs.sh (step 6 invocation) - .mise/scripts/docs/update_log_index.py (epilog / docstring path hints) The old scripts/update_log_index.py is removed via git rm. The .github/workflows/main_base.yml already used the new path.
54a61cc to
411a05d
Compare
5f8b4fa to
fba296b
Compare
Summary
Delivers the complete key auto-rotation specification and the full manual-rotation
implementation for all key types, as PR 1 of 3 in the auto-rotation feature stack.
Documentation
📄 Key Auto-Rotation Policy — published after merge.
Source:
documentation/docs/kmip_support/key_auto_rotation.mdWhat the doc covers
x-rotate-interval,x-rotate-name,x-rotate-offset,x-rotate-generation,x-rotate-date,x-rotate-latest).x-rotate-generationandx-rotate-dateare server-managed and read-only to prevent chain corruption.my-keyset) andmy-keyset@N/my-keyset@latestaddressing. Encrypt/Sign always use the latest generation; Decrypt/Verify walk newest-to-oldest so in-flight ciphertexts survive rotation.hsm::…UIDs) support manualRe-Key(callsC_GenerateKeyon the same PKCS#11 slot) and keysets (state stored inCKA_LABEL), but are excluded from the auto-rotation scheduler.--auto-rotation-check-interval-secs. FiresRe-Keyautomatically whenx-rotate-date + x-rotate-intervalelapses.ReCertify), server-wide KEK.Implementation —
Re-Key,Re-Key Key Pair,Re-CertifyComplete manual-rotation implementation for all six scenarios:
Implements
ReCertify(KMIP 2.1 §6.1.45): self-signed and CA-signed renewal, replacement link chain, offset-basedPreActivestate.Code quality
implblocks — rotation helpers moved ontoimpl Attributes,impl Object,impl UniqueIdentifierincrate/kmip; crypto-op helpers moved ontoimpl KMSincrate/server.Breaking changes
None.
Reviewer notes
This PR is the canonical reference for all subsequent PRs in this stack.
Please review terminology and attribute semantics carefully — changes here cascade to the implementation PRs.