diff --git a/docs/architecture/kms-development-defaults-inventory.md b/docs/architecture/kms-development-defaults-inventory.md new file mode 100644 index 000000000..a880a574d --- /dev/null +++ b/docs/architecture/kms-development-defaults-inventory.md @@ -0,0 +1,77 @@ +# KMS Development Defaults Inventory + +This inventory tracks `KMSD-001` for +[`rustfs/backlog#660`](https://github.com/rustfs/backlog/issues/660). It records +current KMS development defaults before any production default hardening. + +## Scope + +- Source files reviewed: `crates/kms/src/config.rs` and + `crates/kms/src/api_types.rs`. +- This is a `docs-only` task. +- No runtime behavior, config serialization, admin authorization, startup order, + global state, storage path, or crate boundary changes are included. +- Follow-up hardening must be done in separate `security-change` PRs with focused + tests. + +## Current Defaults + +| Source | Default | Current behavior | Classification | +|---|---|---|---| +| `KmsConfig::default()` | Local backend | Uses `LocalConfig::default()` and validates successfully. | dev-only | +| `LocalConfig::default().key_dir` | OS temp dir plus `rustfs_kms_keys` | Keys are stored under the process temp directory. | dev-only | +| `LocalConfig::default().master_key` | `None` | Local key files are stored in plaintext when no master key is configured. | invalid for production | +| `LocalConfig::default().file_permissions` | `0o600` | Owner read/write only for key files. | production-safe as a permission default, but not sufficient without encrypted key storage | +| `KmsConfig::local(key_dir)` | caller-provided key dir, default local fields | Keeps `master_key = None` unless the caller supplies one later. | dev-only unless explicit encryption material is configured | +| `KmsConfig::from_env()` local key dir | `./kms_keys` | The env loader builds a relative path, then existing validation rejects it because local key dirs must be absolute. | invalid as a standalone default | +| `KmsConfig::from_env()` local master key | absent | Leaves `master_key = None`. | invalid for production | +| `VaultConfig::default().address` | `http://localhost:8200` | HTTP is accepted by validation. | dev-only | +| `VaultTransitConfig::default().address` | `http://localhost:8200` | HTTP is accepted by validation. | dev-only | +| `VaultConfig::default().auth_method` | token dev-token | The default token is accepted if used as-is. | invalid for production | +| `VaultTransitConfig::default().auth_method` | token dev-token | The default token is accepted if used as-is. | invalid for production | +| `VaultConfig::default().tls` | `None` | No custom TLS settings. HTTPS without custom TLS relies on system CA when no skip flag is set. | production-safe only when HTTPS and system trust are intended | +| `VaultTransitConfig::default().tls` | `None` | Same TLS behavior as Vault KV2. | production-safe only when HTTPS and system trust are intended | +| `ConfigureVaultKmsRequest.skip_tls_verify` | omitted means false | `to_kms_config()` leaves TLS config as `None` unless the request explicitly sets true. | production-safe when omitted | +| `ConfigureVaultTransitKmsRequest.skip_tls_verify` | omitted means false | Same behavior as Vault KV2 configure requests. | production-safe when omitted | +| `skip_tls_verify = true` in configure requests | explicit insecure opt-in | Creates a TLS config with `skip_verify = true`. | invalid for production | + +## Existing Validation Boundary + +- Local key directories must be absolute. +- Timeout and retry attempts must be greater than zero. +- Vault addresses must use HTTP or HTTPS. +- Vault mount paths must be non-empty. +- HTTPS with custom TLS config and verification enabled warns when relying on + system CA instead of custom CA/client certificates. +- Existing validation does not fail closed for HTTP Vault addresses, dev-token, + missing local master key, temp key dirs, or explicit `skip_tls_verify = true`. + +## Hardening Follow-Ups + +`KMSD-002` should make Local KMS unsafe defaults explicit development opt-ins or +production failures: + +- no local master key; +- local key directory under the process temp directory; +- local env defaults that cannot pass validation without an absolute path. + +`KMSD-003` should make Vault unsafe defaults explicit development opt-ins or +production failures: + +- HTTP Vault addresses; +- default dev-token credentials; +- explicit `skip_tls_verify = true`. + +Both follow-ups must preserve existing development workflows through documented +compatibility behavior or explicit development mode. They must not modify KMS +runtime logic only to satisfy tests. + +## Test Expectations For Follow-Ups + +- Add focused negative tests before changing production default behavior. +- Keep persisted config serialization compatibility tests separate from runtime + fail-closed tests. +- Cover both env-loaded configuration and admin configure request conversion. +- Prove development opt-in paths remain explicit and searchable. +- Do not alter KMS key operation behavior, authorization actions, cache behavior, + or storage hot paths while hardening defaults. diff --git a/docs/architecture/migration-progress.md b/docs/architecture/migration-progress.md index bd7fead22..9eb2e3660 100644 --- a/docs/architecture/migration-progress.md +++ b/docs/architecture/migration-progress.md @@ -5,14 +5,15 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block ## Current Context - Issue: [`rustfs/backlog#660`](https://github.com/rustfs/backlog/issues/660) -- Branch: `overtrue/arch-config-consumer-inventory` -- Baseline: `upstream/main` at `a73c90c813bba16e668be090c5c4ca22c765b81b` +- Branch: `overtrue/arch-kms-defaults-inventory` +- Baseline: `upstream/main` at `f80162b5c99d6825b489ad94b81dfaeca6bfa874` (after CFG-002) - PR type for this branch: `docs-only` -- Runtime behavior changes: none. -- Rust code changes: none. +- Runtime behavior changes: none +- Rust code changes: none - CI/script changes: none -- Docs changes: add the CFG-002 config model boundary ADR and link it from the - architecture overview, crate-boundary guardrails, and this progress handoff. +- Docs changes: add KMSD-001 inventory for current KMS development defaults, + classify each default as production-safe, dev-only, or invalid for + production, and record follow-up hardening boundaries. ## Phase 0 Tasks @@ -134,6 +135,15 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block and persisted config serialization still writes the original secret values. - Verification: focused KMS redaction/status tests, full KMS tests, migration guards, Rust quality scan, clippy, and `make pre-commit` passed. +- [x] `KMSD-001` Inventory KMS development defaults. + - Acceptance: + [`kms-development-defaults-inventory.md`](kms-development-defaults-inventory.md) + records Local and Vault defaults for missing master keys, temp key dirs, + HTTP Vault addresses, default dev-token credentials, and skip-TLS behavior. + - Must preserve: no KMS runtime behavior, config serialization, + authorization, startup order, storage path, or crate boundary changes. + - Verification: docs diff review, migration guards, metrics reference guard, + and `git diff --check`. ## Phase 2 Storage API Tasks @@ -193,14 +203,18 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block RUSTFS_COMPAT_TODO(CFG-004) and cleanup-register coverage. 6. `consumer-migration`: migrate external consumers one group at a time only after the model path and compatibility shim are stable. +7. `security-change`: make Local KMS unsafe defaults explicit development + opt-ins or production failures in KMSD-002. +8. `security-change`: make Vault unsafe defaults explicit development opt-ins + or production failures in KMSD-003. ## Pre-Push Review Log | Expert | Status | Notes | |---|---|---| -| Quality/architecture | pass | Single `docs-only` PR; ADR chooses existing rustfs-config, records module path and dependency boundaries, and avoids a speculative new crate. | -| Migration preservation | pass | No code movement; ADR explicitly keeps persistence helpers, global server-config state, startup order, and old-path compatibility requirements out of CFG-002. | -| Testing/verification | pass | Docs-only verification uses migration guard scripts, metrics reference guard, layer dependency guard, and whitespace checks. | +| Quality/architecture | pass | Single `docs-only` PR; the inventory is isolated under `docs/architecture`, uses existing KMS source files as evidence, and introduces no new abstraction or dependency edge. | +| Migration preservation | pass | No runtime code, config persistence, admin authorization, startup order, storage path, global state, or crate boundary changes are made. | +| Testing/verification | pass | Docs-only verification is bounded to diff review, architecture migration rules, metrics reference guard, layer dependency guard, and whitespace checks. | ## Verification Notes @@ -219,14 +233,18 @@ Notes: ## Handoff Notes -- Keep this CFG-002 branch as a focused `docs-only` PR. Do not move - `Config`, `KV`, `KVS`, persistence helpers, global server-config state, - Storage API code, startup code, or target/notify/audit/IAM consumers in this - branch. -- The next extraction PR must preserve the tuple-struct shape, serde fields, - `hiddenIfEmpty` alias, `Config::new` default behavior, marshal/unmarshal - behavior, and old `rustfs_ecstore::config::*` path. +- Keep this KMSD-001 branch as a focused `docs-only` PR. Do not change KMS + defaults, admin authorization, admin route registration shape, config moves, + Storage API moves, runtime moves, or ECStore moves. +- `rustfs` may depend on `rustfs-security-governance` for contract metadata; + the security-governance crate must stay independent from implementation + crates and runtime state. - Do not add temporary compatibility code without a matching `RUSTFS_COMPAT_TODO()` marker and cleanup-register entry. +- KMS production default hardening remains a separate task group; do not bundle + it with this inventory PR. +- The CFG-003 extraction PR must preserve the tuple-struct shape, serde fields, + `hiddenIfEmpty` alias, `Config::new` default behavior, marshal/unmarshal + behavior, and old `rustfs_ecstore::config::*` path. - Do not create a new config-model crate unless a later implementation attempt proves `rustfs-config` cannot hold the pure model boundary.