diff --git a/docs/architecture/compat-cleanup-register.md b/docs/architecture/compat-cleanup-register.md index 582e61fdd..54b3e5d15 100644 --- a/docs/architecture/compat-cleanup-register.md +++ b/docs/architecture/compat-cleanup-register.md @@ -12,7 +12,12 @@ for later deletion. ## Open Items -No compatibility code has been introduced in this branch. +- `RUSTFS_COMPAT_TODO(S-012)` + - Task: `S-012` + - File: `rustfs/src/admin/handlers/kms_keys.rs` + - Why: legacy KMS create-key and key-status admin grants must keep working during the dedicated KMS policy migration. + - Removal condition: remove after KMS admin clients and built-in policies use `kms:Configure`, `kms:DescribeKey`, and `kms:ListKeys`. + - Status: planned cleanup. ## Review Checklist diff --git a/docs/architecture/migration-progress.md b/docs/architecture/migration-progress.md index 1d8513df8..8d8e81422 100644 --- a/docs/architecture/migration-progress.md +++ b/docs/architecture/migration-progress.md @@ -5,14 +5,16 @@ 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-kms-action-taxonomy` -- Baseline: `upstream/main` at `51c26278a4907449c565c7f1f52c1d9ae0616486` -- PR type for this branch: `contract` -- Runtime behavior changes: none -- Rust code changes: extend `KmsAction` with the dedicated policy action - taxonomy required before handler-level KMS authorization migration. +- Branch: `overtrue/arch-kms-handler-actions` +- Baseline: `upstream/main` at `4fec606dc4f92b19e085f1609a188e82a72720ff` +- PR type for this branch: `security-change` +- Runtime behavior changes: high-risk KMS admin handlers now authorize through + dedicated `kms:*` actions instead of broad `ServerInfoAdminAction`. +- Rust code changes: migrate KMS handler action lists and route policy inventory + to dedicated KMS actions, while keeping temporary legacy create/status admin + action compatibility with `RUSTFS_COMPAT_TODO(S-012)`. - CI/script changes: none -- Docs changes: record the S-011 KMS action taxonomy handoff. +- Docs changes: record S-012 action migration status and temporary compatibility cleanup. ## Phase 0 Tasks @@ -98,51 +100,58 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block service-control, clear-cache, generate-data-key, delete, rotate, list, and describe actions; wildcard matching still works. - Verification: `cargo test -p rustfs-policy action --no-fail-fast`. +- [x] `S-012` Migrate KMS handlers to dedicated actions. + - Acceptance: KMS data-key, delete/cancel-delete, cache, configure, + service-control, list, and describe handlers use dedicated `kms:*` actions. + - Compatibility: legacy KMS create/status admin actions are retained only as + temporary compatibility paths and registered in + [`compat-cleanup-register.md`](compat-cleanup-register.md). + - Verification: focused handler and route policy tests, migration rules, + formatting, and `make pre-commit`. ## Next PRs -1. `security-change`: migrate KMS handlers to dedicated actions with explicit - legacy compatibility where required. -2. `contract`: add initial policy inventory tables for redaction, serde, or +1. `contract`: add initial policy inventory tables for redaction, serde, or supply-chain governance only after the contract shape remains stable. +2. `security-change`: apply KMS response/config redaction after action + migration settles. ## Pre-Push Review Log | Expert | Status | Notes | |---|---|---| -| Quality/architecture | pass | Pure policy taxonomy extension; names follow existing `KmsAction` variant style and the branch stays a single `contract` PR. | -| Migration preservation | pass | No handler authorization, route policy inventory, startup, global state, crate split, or storage hot-path behavior changes. | -| Testing/verification | pass | Baseline action tests, focused policy tests, full `rustfs-policy`, migration guard scripts, `make pre-commit`, nextest, and doctests pass. | +| Quality/architecture | pass | Single `security-change` PR; KMS auth action lists are local helper functions, names match `KmsAction`, and no storage/startup/global-state logic is touched. | +| Migration preservation | pass | Legacy create/status admin actions are retained only behind `RUSTFS_COMPAT_TODO(S-012)` and registered for cleanup; broad `ServerInfoAdminAction` is intentionally not retained for high-risk KMS operations. | +| Testing/verification | pass | Focused handler/route-policy tests cover dedicated actions and broad-action negative cases; migration rules, formatting, full `make pre-commit`, nextest, and doctests pass. | ## Verification Notes Passed: -- Baseline `cargo test -p rustfs-policy action --no-fail-fast` +- Baseline `cargo test -p rustfs admin::handlers::kms --no-fail-fast` +- Baseline `cargo test -p rustfs admin::route_policy --no-fail-fast` - `cargo fmt --all --check` -- `cargo test -p rustfs-policy action --no-fail-fast` -- `cargo test -p rustfs-policy` +- `cargo test -p rustfs admin::handlers::kms --no-fail-fast` +- `cargo test -p rustfs admin::route_policy --no-fail-fast` - `./scripts/check_architecture_migration_rules.sh` -- `./scripts/check_layer_dependencies.sh` -- `./scripts/check_metrics_migration_refs.sh` - `git diff --check` - `make pre-commit` Notes: -- This branch only extends KMS policy taxonomy. It does not change KMS handler - authorization, route policy inventory, runtime state, startup order, storage - paths, or compatibility mappings. -- `make pre-commit` passed all checks, including 5672 nextest tests and +- This branch changes only KMS admin authorization action selection and route + policy inventory. It does not change KMS runtime defaults, redaction, startup + order, global state, storage paths, or crate boundaries. +- `make pre-commit` passed all checks, including 5682 nextest tests and workspace doctests. ## Handoff Notes -- Keep this S-011 branch as a pure `contract` PR. Do not change KMS handler - authorization, admin route registration, route policy inventory, Config moves, - Storage API moves, Runtime moves, or ECStore moves. +- Keep this S-012 branch as a focused `security-change` PR. Do not change KMS + defaults, redaction, 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. -- S-012 must decide any legacy compatibility mapping explicitly instead of - silently replacing existing admin actions in this taxonomy PR. +- The next KMS security PR should handle redaction or production default + hardening separately; do not bundle those with this action migration. diff --git a/rustfs/src/admin/handlers/kms_dynamic.rs b/rustfs/src/admin/handlers/kms_dynamic.rs index 557c1e56a..7bb056553 100644 --- a/rustfs/src/admin/handlers/kms_dynamic.rs +++ b/rustfs/src/admin/handlers/kms_dynamic.rs @@ -28,7 +28,7 @@ use rustfs_kms::{ ConfigureKmsRequest, ConfigureKmsResponse, KmsConfig, KmsConfigSummary, KmsServiceStatus, KmsStatusResponse, StartKmsRequest, StartKmsResponse, StopKmsResponse, }; -use rustfs_policy::policy::action::{Action, AdminAction}; +use rustfs_policy::policy::action::{Action, KmsAction}; use s3s::{Body, S3Request, S3Response, S3Result, s3_error}; use tracing::{error, info, instrument, warn}; @@ -57,6 +57,14 @@ fn existing_vault_auth(config: &KmsConfig) -> Option Vec { + vec![Action::KmsAction(KmsAction::ConfigureAction)] +} + +fn kms_service_control_actions() -> Vec { + vec![Action::KmsAction(KmsAction::ServiceControlAction)] +} + fn normalize_configure_request_auth( request: &mut ConfigureKmsRequest, existing_config: Option<&KmsConfig>, @@ -184,7 +192,7 @@ impl Operation for ConfigureKmsHandler { &cred, owner, false, - vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)], + kms_configure_actions(), req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)), ) .await?; @@ -284,7 +292,7 @@ impl Operation for StartKmsHandler { &cred, owner, false, - vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)], + kms_service_control_actions(), req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)), ) .await?; @@ -414,7 +422,7 @@ impl Operation for StopKmsHandler { &cred, owner, false, - vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)], + kms_service_control_actions(), req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)), ) .await?; @@ -476,7 +484,7 @@ impl Operation for GetKmsStatusHandler { &cred, owner, false, - vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)], + kms_service_control_actions(), req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)), ) .await?; @@ -544,7 +552,7 @@ impl Operation for ReconfigureKmsHandler { &cred, owner, false, - vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)], + kms_configure_actions(), req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)), ) .await?; @@ -625,3 +633,29 @@ impl Operation for ReconfigureKmsHandler { Ok(S3Response::new((StatusCode::OK, Body::from(json_response)))) } } + +#[cfg(test)] +mod tests { + use super::{kms_configure_actions, kms_service_control_actions}; + use rustfs_policy::policy::action::{Action, AdminAction, KmsAction}; + + fn assert_has_action(actions: &[Action], action: Action) { + assert!(actions.contains(&action), "expected action list to contain {action:?}"); + } + + fn assert_lacks_action(actions: &[Action], action: Action) { + assert!(!actions.contains(&action), "expected action list not to contain {action:?}"); + } + + #[test] + fn kms_dynamic_auth_actions_use_dedicated_kms_actions() { + assert_has_action(&kms_configure_actions(), Action::KmsAction(KmsAction::ConfigureAction)); + assert_has_action(&kms_service_control_actions(), Action::KmsAction(KmsAction::ServiceControlAction)); + } + + #[test] + fn kms_dynamic_actions_reject_server_info_fallback() { + assert_lacks_action(&kms_configure_actions(), Action::AdminAction(AdminAction::ServerInfoAdminAction)); + assert_lacks_action(&kms_service_control_actions(), Action::AdminAction(AdminAction::ServerInfoAdminAction)); + } +} diff --git a/rustfs/src/admin/handlers/kms_keys.rs b/rustfs/src/admin/handlers/kms_keys.rs index 98a6b5d44..7b5064737 100644 --- a/rustfs/src/admin/handlers/kms_keys.rs +++ b/rustfs/src/admin/handlers/kms_keys.rs @@ -24,7 +24,7 @@ use hyper::{HeaderMap, Method, StatusCode}; use matchit::Params; use rustfs_config::MAX_ADMIN_REQUEST_BODY_SIZE; use rustfs_kms::{KmsError, init_global_kms_service_manager, types::*}; -use rustfs_policy::policy::action::{Action, AdminAction}; +use rustfs_policy::policy::action::{Action, AdminAction, KmsAction}; use s3s::header::CONTENT_TYPE; use s3s::{Body, S3Request, S3Response, S3Result, s3_error}; use serde::{Deserialize, Serialize}; @@ -118,6 +118,38 @@ async fn kms_encryption_service_from_context() -> Option Vec { + // RUSTFS_COMPAT_TODO(S-012): keep legacy KMS create-key grants during KMS policy migration. Remove after KMS admin clients and built-in policies use kms:Configure. + vec![ + Action::KmsAction(KmsAction::ConfigureAction), + Action::AdminAction(AdminAction::KMSCreateKeyAdminAction), + ] +} + +fn kms_describe_key_actions() -> Vec { + // RUSTFS_COMPAT_TODO(S-012): keep legacy KMS key-status grants during KMS policy migration. Remove after KMS admin clients and built-in policies use kms:DescribeKey. + vec![ + Action::KmsAction(KmsAction::DescribeKeyAction), + Action::AdminAction(AdminAction::KMSKeyStatusAdminAction), + ] +} + +fn kms_list_keys_actions() -> Vec { + // RUSTFS_COMPAT_TODO(S-012): keep legacy KMS key-status grants during KMS policy migration. Remove after KMS admin clients and built-in policies use kms:ListKeys. + vec![ + Action::KmsAction(KmsAction::ListKeysAction), + Action::AdminAction(AdminAction::KMSKeyStatusAdminAction), + ] +} + +fn kms_generate_data_key_actions() -> Vec { + vec![Action::KmsAction(KmsAction::GenerateDataKeyAction)] +} + +fn kms_delete_key_actions() -> Vec { + vec![Action::KmsAction(KmsAction::DeleteKeyAction)] +} + pub fn register_kms_key_route(r: &mut S3Router) -> std::io::Result<()> { r.insert( Method::POST, @@ -170,7 +202,7 @@ impl Operation for CreateKeyHandler { &cred, owner, false, - vec![Action::AdminAction(AdminAction::KMSCreateKeyAdminAction)], + kms_create_key_actions(), req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)), ) .await?; @@ -249,7 +281,7 @@ impl Operation for DescribeKeyHandler { &cred, owner, false, - vec![Action::AdminAction(AdminAction::KMSKeyStatusAdminAction)], + kms_describe_key_actions(), req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)), ) .await?; @@ -288,8 +320,20 @@ impl Operation for DescribeKeyHandler { #[cfg(test)] mod tests { - use super::extract_key_id; + use super::{ + extract_key_id, kms_create_key_actions, kms_delete_key_actions, kms_describe_key_actions, kms_generate_data_key_actions, + kms_list_keys_actions, + }; use http::Uri; + use rustfs_policy::policy::action::{Action, AdminAction, KmsAction}; + + fn assert_has_action(actions: &[Action], action: Action) { + assert!(actions.contains(&action), "expected action list to contain {action:?}"); + } + + fn assert_lacks_action(actions: &[Action], action: Action) { + assert!(!actions.contains(&action), "expected action list not to contain {action:?}"); + } #[test] fn test_extract_key_id_supports_minio_aliases() { @@ -331,6 +375,31 @@ mod tests { assert_eq!(extract_key_id(&uri).as_deref(), expected); } } + + #[test] + fn kms_key_auth_actions_use_dedicated_kms_actions() { + let create_actions = kms_create_key_actions(); + assert_has_action(&create_actions, Action::KmsAction(KmsAction::ConfigureAction)); + assert_has_action(&create_actions, Action::AdminAction(AdminAction::KMSCreateKeyAdminAction)); + + let describe_actions = kms_describe_key_actions(); + assert_has_action(&describe_actions, Action::KmsAction(KmsAction::DescribeKeyAction)); + assert_has_action(&describe_actions, Action::AdminAction(AdminAction::KMSKeyStatusAdminAction)); + + let list_actions = kms_list_keys_actions(); + assert_has_action(&list_actions, Action::KmsAction(KmsAction::ListKeysAction)); + assert_has_action(&list_actions, Action::AdminAction(AdminAction::KMSKeyStatusAdminAction)); + } + + #[test] + fn kms_sensitive_key_actions_reject_server_info_fallback() { + for actions in [kms_generate_data_key_actions(), kms_delete_key_actions()] { + assert_lacks_action(&actions, Action::AdminAction(AdminAction::ServerInfoAdminAction)); + } + + assert_has_action(&kms_generate_data_key_actions(), Action::KmsAction(KmsAction::GenerateDataKeyAction)); + assert_has_action(&kms_delete_key_actions(), Action::KmsAction(KmsAction::DeleteKeyAction)); + } } /// List KMS keys (legacy endpoint) @@ -351,7 +420,7 @@ impl Operation for ListKeysHandler { &cred, owner, false, - vec![Action::AdminAction(AdminAction::KMSKeyStatusAdminAction)], + kms_list_keys_actions(), req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)), ) .await?; @@ -413,7 +482,7 @@ impl Operation for GenerateDataKeyHandler { &cred, owner, false, - vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)], + kms_generate_data_key_actions(), req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)), ) .await?; @@ -479,7 +548,7 @@ impl Operation for CreateKmsKeyHandler { &cred, owner, false, - vec![Action::AdminAction(AdminAction::KMSCreateKeyAdminAction)], + kms_create_key_actions(), req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)), ) .await?; @@ -613,7 +682,7 @@ impl Operation for DeleteKmsKeyHandler { &cred, owner, false, - vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)], + kms_delete_key_actions(), req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)), ) .await?; @@ -762,7 +831,7 @@ impl Operation for CancelKmsKeyDeletionHandler { &cred, owner, false, - vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)], + kms_delete_key_actions(), req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)), ) .await?; @@ -891,7 +960,7 @@ impl Operation for ListKmsKeysHandler { &cred, owner, false, - vec![Action::AdminAction(AdminAction::KMSKeyStatusAdminAction)], + kms_list_keys_actions(), req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)), ) .await?; @@ -1003,7 +1072,7 @@ impl Operation for DescribeKmsKeyHandler { &cred, owner, false, - vec![Action::AdminAction(AdminAction::KMSKeyStatusAdminAction)], + kms_describe_key_actions(), req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)), ) .await?; diff --git a/rustfs/src/admin/handlers/kms_management.rs b/rustfs/src/admin/handlers/kms_management.rs index 29599cf69..10442598a 100644 --- a/rustfs/src/admin/handlers/kms_management.rs +++ b/rustfs/src/admin/handlers/kms_management.rs @@ -23,7 +23,7 @@ use crate::server::{ADMIN_PREFIX, RemoteAddr}; use hyper::{HeaderMap, Method, StatusCode}; use matchit::Params; use rustfs_kms::{KmsBackend, init_global_kms_service_manager}; -use rustfs_policy::policy::action::{Action, AdminAction}; +use rustfs_policy::policy::action::{Action, KmsAction}; use s3s::header::CONTENT_TYPE; use s3s::{Body, S3Request, S3Response, S3Result, s3_error}; use serde::{Deserialize, Serialize}; @@ -52,6 +52,18 @@ fn backend_name(backend: &KmsBackend) -> &'static str { } } +fn kms_service_control_actions() -> Vec { + vec![Action::KmsAction(KmsAction::ServiceControlAction)] +} + +fn kms_configure_actions() -> Vec { + vec![Action::KmsAction(KmsAction::ConfigureAction)] +} + +fn kms_clear_cache_actions() -> Vec { + vec![Action::KmsAction(KmsAction::ClearCacheAction)] +} + #[derive(Debug, Serialize, Deserialize)] pub struct KmsStatusResponse { pub backend_type: String, @@ -158,7 +170,7 @@ impl Operation for KmsStatusHandler { &cred, owner, false, - vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)], + kms_service_control_actions(), req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)), ) .await?; @@ -220,7 +232,7 @@ impl Operation for KmsConfigHandler { &cred, owner, false, - vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)], + kms_configure_actions(), req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)), ) .await?; @@ -269,7 +281,7 @@ impl Operation for KmsClearCacheHandler { &cred, owner, false, - vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)], + kms_clear_cache_actions(), req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)), ) .await?; @@ -301,3 +313,29 @@ impl Operation for KmsClearCacheHandler { } } } + +#[cfg(test)] +mod tests { + use super::{kms_clear_cache_actions, kms_configure_actions, kms_service_control_actions}; + use rustfs_policy::policy::action::{Action, AdminAction, KmsAction}; + + fn assert_has_action(actions: &[Action], action: Action) { + assert!(actions.contains(&action), "expected action list to contain {action:?}"); + } + + fn assert_lacks_action(actions: &[Action], action: Action) { + assert!(!actions.contains(&action), "expected action list not to contain {action:?}"); + } + + #[test] + fn kms_management_auth_actions_use_dedicated_kms_actions() { + assert_has_action(&kms_service_control_actions(), Action::KmsAction(KmsAction::ServiceControlAction)); + assert_has_action(&kms_configure_actions(), Action::KmsAction(KmsAction::ConfigureAction)); + assert_has_action(&kms_clear_cache_actions(), Action::KmsAction(KmsAction::ClearCacheAction)); + } + + #[test] + fn kms_clear_cache_rejects_server_info_fallback() { + assert_lacks_action(&kms_clear_cache_actions(), Action::AdminAction(AdminAction::ServerInfoAdminAction)); + } +} diff --git a/rustfs/src/admin/route_policy.rs b/rustfs/src/admin/route_policy.rs index 693bca2d5..fe362863e 100644 --- a/rustfs/src/admin/route_policy.rs +++ b/rustfs/src/admin/route_policy.rs @@ -45,8 +45,13 @@ const GET_TABLE_NAMESPACE: AdminActionRef = AdminActionRef::new("GetTableNamespa const HEAL: AdminActionRef = AdminActionRef::new("HealAdminAction"); const IMPORT_BUCKET_METADATA: AdminActionRef = AdminActionRef::new("ImportBucketMetadataAction"); const IMPORT_IAM: AdminActionRef = AdminActionRef::new("ImportIAMAction"); -const KMS_CREATE_KEY: AdminActionRef = AdminActionRef::new("KMSCreateKeyAdminAction"); -const KMS_KEY_STATUS: AdminActionRef = AdminActionRef::new("KMSKeyStatusAdminAction"); +const KMS_CLEAR_CACHE: AdminActionRef = AdminActionRef::new("kms:ClearCache"); +const KMS_CONFIGURE: AdminActionRef = AdminActionRef::new("kms:Configure"); +const KMS_DELETE_KEY: AdminActionRef = AdminActionRef::new("kms:DeleteKey"); +const KMS_DESCRIBE_KEY: AdminActionRef = AdminActionRef::new("kms:DescribeKey"); +const KMS_GENERATE_DATA_KEY: AdminActionRef = AdminActionRef::new("kms:GenerateDataKey"); +const KMS_LIST_KEYS: AdminActionRef = AdminActionRef::new("kms:ListKeys"); +const KMS_SERVICE_CONTROL: AdminActionRef = AdminActionRef::new("kms:ServiceControl"); const LIST_GROUPS: AdminActionRef = AdminActionRef::new("ListGroupsAdminAction"); const LIST_TEMPORARY_ACCOUNTS: AdminActionRef = AdminActionRef::new("ListTemporaryAccountsAdminAction"); const LIST_TIER: AdminActionRef = AdminActionRef::new("ListTierAction"); @@ -475,59 +480,74 @@ pub const ADMIN_ROUTE_POLICY_SPECS: &[AdminRouteSpec] = &[ admin(HttpMethod::Get, "/rustfs/admin/debug/pprof/profile", PROFILING, RouteRiskLevel::High), admin(HttpMethod::Get, "/rustfs/admin/debug/pprof/status", PROFILING, RouteRiskLevel::High), admin(HttpMethod::Get, "/rustfs/admin/debug/tls/status", PROFILING, RouteRiskLevel::High), - admin(HttpMethod::Post, "/rustfs/admin/v3/kms/create-key", KMS_CREATE_KEY, RouteRiskLevel::High), - admin(HttpMethod::Post, "/rustfs/admin/v3/kms/key/create", KMS_CREATE_KEY, RouteRiskLevel::High), + admin(HttpMethod::Post, "/rustfs/admin/v3/kms/create-key", KMS_CONFIGURE, RouteRiskLevel::High), + admin(HttpMethod::Post, "/rustfs/admin/v3/kms/key/create", KMS_CONFIGURE, RouteRiskLevel::High), admin( HttpMethod::Get, "/rustfs/admin/v3/kms/describe-key", - KMS_KEY_STATUS, + KMS_DESCRIBE_KEY, RouteRiskLevel::Sensitive, ), admin( HttpMethod::Get, "/rustfs/admin/v3/kms/key/status", - KMS_KEY_STATUS, + KMS_DESCRIBE_KEY, RouteRiskLevel::Sensitive, ), admin( HttpMethod::Get, "/rustfs/admin/v3/kms/list-keys", - KMS_KEY_STATUS, + KMS_LIST_KEYS, RouteRiskLevel::Sensitive, ), admin( HttpMethod::Post, "/rustfs/admin/v3/kms/generate-data-key", - SERVER_INFO, + KMS_GENERATE_DATA_KEY, RouteRiskLevel::High, ), - admin(HttpMethod::Get, "/rustfs/admin/v3/kms/status", SERVER_INFO, RouteRiskLevel::Sensitive), - admin(HttpMethod::Post, "/rustfs/admin/v3/kms/status", SERVER_INFO, RouteRiskLevel::High), - admin(HttpMethod::Get, "/rustfs/admin/v3/kms/config", SERVER_INFO, RouteRiskLevel::Sensitive), - admin(HttpMethod::Post, "/rustfs/admin/v3/kms/clear-cache", SERVER_INFO, RouteRiskLevel::High), - admin(HttpMethod::Post, "/rustfs/admin/v3/kms/configure", SERVER_INFO, RouteRiskLevel::High), - admin(HttpMethod::Post, "/rustfs/admin/v3/kms/start", SERVER_INFO, RouteRiskLevel::High), - admin(HttpMethod::Post, "/rustfs/admin/v3/kms/stop", SERVER_INFO, RouteRiskLevel::High), + admin( + HttpMethod::Get, + "/rustfs/admin/v3/kms/status", + KMS_SERVICE_CONTROL, + RouteRiskLevel::Sensitive, + ), + admin(HttpMethod::Post, "/rustfs/admin/v3/kms/status", KMS_SERVICE_CONTROL, RouteRiskLevel::High), + admin(HttpMethod::Get, "/rustfs/admin/v3/kms/config", KMS_CONFIGURE, RouteRiskLevel::Sensitive), + admin( + HttpMethod::Post, + "/rustfs/admin/v3/kms/clear-cache", + KMS_CLEAR_CACHE, + RouteRiskLevel::High, + ), + admin(HttpMethod::Post, "/rustfs/admin/v3/kms/configure", KMS_CONFIGURE, RouteRiskLevel::High), + admin(HttpMethod::Post, "/rustfs/admin/v3/kms/start", KMS_SERVICE_CONTROL, RouteRiskLevel::High), + admin(HttpMethod::Post, "/rustfs/admin/v3/kms/stop", KMS_SERVICE_CONTROL, RouteRiskLevel::High), admin( HttpMethod::Get, "/rustfs/admin/v3/kms/service-status", - SERVER_INFO, + KMS_SERVICE_CONTROL, RouteRiskLevel::Sensitive, ), - admin(HttpMethod::Post, "/rustfs/admin/v3/kms/reconfigure", SERVER_INFO, RouteRiskLevel::High), - admin(HttpMethod::Post, "/rustfs/admin/v3/kms/keys", KMS_CREATE_KEY, RouteRiskLevel::High), - admin(HttpMethod::Delete, "/rustfs/admin/v3/kms/keys/delete", SERVER_INFO, RouteRiskLevel::High), + admin(HttpMethod::Post, "/rustfs/admin/v3/kms/reconfigure", KMS_CONFIGURE, RouteRiskLevel::High), + admin(HttpMethod::Post, "/rustfs/admin/v3/kms/keys", KMS_CONFIGURE, RouteRiskLevel::High), + admin( + HttpMethod::Delete, + "/rustfs/admin/v3/kms/keys/delete", + KMS_DELETE_KEY, + RouteRiskLevel::High, + ), admin( HttpMethod::Post, "/rustfs/admin/v3/kms/keys/cancel-deletion", - SERVER_INFO, + KMS_DELETE_KEY, RouteRiskLevel::High, ), - admin(HttpMethod::Get, "/rustfs/admin/v3/kms/keys", KMS_KEY_STATUS, RouteRiskLevel::Sensitive), + admin(HttpMethod::Get, "/rustfs/admin/v3/kms/keys", KMS_LIST_KEYS, RouteRiskLevel::Sensitive), admin( HttpMethod::Get, "/rustfs/admin/v3/kms/keys/{key_id}", - KMS_KEY_STATUS, + KMS_DESCRIBE_KEY, RouteRiskLevel::Sensitive, ), public( @@ -817,6 +837,33 @@ mod tests { ); } + #[test] + fn route_policy_records_dedicated_kms_actions() { + assert_action(HttpMethod::Post, "/rustfs/admin/v3/kms/create-key", KMS_CONFIGURE); + assert_action(HttpMethod::Get, "/rustfs/admin/v3/kms/describe-key", KMS_DESCRIBE_KEY); + assert_action(HttpMethod::Get, "/rustfs/admin/v3/kms/list-keys", KMS_LIST_KEYS); + assert_action(HttpMethod::Post, "/rustfs/admin/v3/kms/generate-data-key", KMS_GENERATE_DATA_KEY); + assert_action(HttpMethod::Post, "/rustfs/admin/v3/kms/clear-cache", KMS_CLEAR_CACHE); + assert_action(HttpMethod::Post, "/rustfs/admin/v3/kms/configure", KMS_CONFIGURE); + assert_action(HttpMethod::Post, "/rustfs/admin/v3/kms/start", KMS_SERVICE_CONTROL); + assert_action(HttpMethod::Delete, "/rustfs/admin/v3/kms/keys/delete", KMS_DELETE_KEY); + assert_action(HttpMethod::Post, "/rustfs/admin/v3/kms/keys/cancel-deletion", KMS_DELETE_KEY); + assert_action(HttpMethod::Get, "/rustfs/admin/v3/kms/keys/{key_id}", KMS_DESCRIBE_KEY); + } + + #[test] + fn route_policy_rejects_server_info_for_sensitive_kms_actions() { + for (method, path) in [ + (HttpMethod::Post, "/rustfs/admin/v3/kms/generate-data-key"), + (HttpMethod::Post, "/rustfs/admin/v3/kms/clear-cache"), + (HttpMethod::Post, "/rustfs/admin/v3/kms/configure"), + (HttpMethod::Delete, "/rustfs/admin/v3/kms/keys/delete"), + (HttpMethod::Post, "/rustfs/admin/v3/kms/keys/cancel-deletion"), + ] { + assert_not_action(method, path, SERVER_INFO); + } + } + #[test] fn route_policy_keeps_contextual_auth_deferred() { assert_deferred( @@ -856,6 +903,14 @@ mod tests { assert_eq!(spec.access().admin_action(), Some(action)); } + fn assert_not_action(method: HttpMethod, path: &str, action: AdminActionRef) { + let spec = ADMIN_ROUTE_POLICY_SPECS + .iter() + .find(|spec| spec.method() == method && spec.path() == path) + .expect("expected direct route policy"); + assert_ne!(spec.access().admin_action(), Some(action)); + } + fn assert_public(method: HttpMethod, path: &str, kind: PublicRouteKind) { let spec = ADMIN_ROUTE_POLICY_SPECS .iter()