feat(kms): migrate KMS handlers to dedicated actions (#3298)

feat: migrate KMS handlers to dedicated actions
This commit is contained in:
安正超
2026-06-09 20:01:02 +08:00
committed by GitHub
parent 4fec606dc4
commit f5bb034ec8
6 changed files with 281 additions and 71 deletions
+6 -1
View File
@@ -12,7 +12,12 @@ for later deletion.
## Open Items ## 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 ## Review Checklist
+36 -27
View File
@@ -5,14 +5,16 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
## Current Context ## Current Context
- Issue: [`rustfs/backlog#660`](https://github.com/rustfs/backlog/issues/660) - Issue: [`rustfs/backlog#660`](https://github.com/rustfs/backlog/issues/660)
- Branch: `overtrue/arch-kms-action-taxonomy` - Branch: `overtrue/arch-kms-handler-actions`
- Baseline: `upstream/main` at `51c26278a4907449c565c7f1f52c1d9ae0616486` - Baseline: `upstream/main` at `4fec606dc4f92b19e085f1609a188e82a72720ff`
- PR type for this branch: `contract` - PR type for this branch: `security-change`
- Runtime behavior changes: none - Runtime behavior changes: high-risk KMS admin handlers now authorize through
- Rust code changes: extend `KmsAction` with the dedicated policy action dedicated `kms:*` actions instead of broad `ServerInfoAdminAction`.
taxonomy required before handler-level KMS authorization migration. - 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 - 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 ## 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 service-control, clear-cache, generate-data-key, delete, rotate, list, and
describe actions; wildcard matching still works. describe actions; wildcard matching still works.
- Verification: `cargo test -p rustfs-policy action --no-fail-fast`. - 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 ## Next PRs
1. `security-change`: migrate KMS handlers to dedicated actions with explicit 1. `contract`: add initial policy inventory tables for redaction, serde, or
legacy compatibility where required.
2. `contract`: add initial policy inventory tables for redaction, serde, or
supply-chain governance only after the contract shape remains stable. 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 ## Pre-Push Review Log
| Expert | Status | Notes | | Expert | Status | Notes |
|---|---|---| |---|---|---|
| Quality/architecture | pass | Pure policy taxonomy extension; names follow existing `KmsAction` variant style and the branch stays a single `contract` PR. | | 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 | No handler authorization, route policy inventory, startup, global state, crate split, or storage hot-path behavior changes. | | 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 | Baseline action tests, focused policy tests, full `rustfs-policy`, migration guard scripts, `make pre-commit`, nextest, and doctests pass. | | 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 ## Verification Notes
Passed: 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 fmt --all --check`
- `cargo test -p rustfs-policy action --no-fail-fast` - `cargo test -p rustfs admin::handlers::kms --no-fail-fast`
- `cargo test -p rustfs-policy` - `cargo test -p rustfs admin::route_policy --no-fail-fast`
- `./scripts/check_architecture_migration_rules.sh` - `./scripts/check_architecture_migration_rules.sh`
- `./scripts/check_layer_dependencies.sh`
- `./scripts/check_metrics_migration_refs.sh`
- `git diff --check` - `git diff --check`
- `make pre-commit` - `make pre-commit`
Notes: Notes:
- This branch only extends KMS policy taxonomy. It does not change KMS handler - This branch changes only KMS admin authorization action selection and route
authorization, route policy inventory, runtime state, startup order, storage policy inventory. It does not change KMS runtime defaults, redaction, startup
paths, or compatibility mappings. order, global state, storage paths, or crate boundaries.
- `make pre-commit` passed all checks, including 5672 nextest tests and - `make pre-commit` passed all checks, including 5682 nextest tests and
workspace doctests. workspace doctests.
## Handoff Notes ## Handoff Notes
- Keep this S-011 branch as a pure `contract` PR. Do not change KMS handler - Keep this S-012 branch as a focused `security-change` PR. Do not change KMS
authorization, admin route registration, route policy inventory, Config moves, defaults, redaction, admin route registration shape, Config moves, Storage API
Storage API moves, Runtime moves, or ECStore moves. moves, Runtime moves, or ECStore moves.
- `rustfs` may depend on `rustfs-security-governance` for contract metadata; - `rustfs` may depend on `rustfs-security-governance` for contract metadata;
the security-governance crate must stay independent from implementation the security-governance crate must stay independent from implementation
crates and runtime state. crates and runtime state.
- Do not add temporary compatibility code without a matching - Do not add temporary compatibility code without a matching
`RUSTFS_COMPAT_TODO(<task-id>)` marker and cleanup-register entry. `RUSTFS_COMPAT_TODO(<task-id>)` marker and cleanup-register entry.
- S-012 must decide any legacy compatibility mapping explicitly instead of - The next KMS security PR should handle redaction or production default
silently replacing existing admin actions in this taxonomy PR. hardening separately; do not bundle those with this action migration.
+40 -6
View File
@@ -28,7 +28,7 @@ use rustfs_kms::{
ConfigureKmsRequest, ConfigureKmsResponse, KmsConfig, KmsConfigSummary, KmsServiceStatus, KmsStatusResponse, StartKmsRequest, ConfigureKmsRequest, ConfigureKmsResponse, KmsConfig, KmsConfigSummary, KmsServiceStatus, KmsStatusResponse, StartKmsRequest,
StartKmsResponse, StopKmsResponse, 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 s3s::{Body, S3Request, S3Response, S3Result, s3_error};
use tracing::{error, info, instrument, warn}; use tracing::{error, info, instrument, warn};
@@ -57,6 +57,14 @@ fn existing_vault_auth(config: &KmsConfig) -> Option<rustfs_kms::config::VaultAu
} }
} }
fn kms_configure_actions() -> Vec<Action> {
vec![Action::KmsAction(KmsAction::ConfigureAction)]
}
fn kms_service_control_actions() -> Vec<Action> {
vec![Action::KmsAction(KmsAction::ServiceControlAction)]
}
fn normalize_configure_request_auth( fn normalize_configure_request_auth(
request: &mut ConfigureKmsRequest, request: &mut ConfigureKmsRequest,
existing_config: Option<&KmsConfig>, existing_config: Option<&KmsConfig>,
@@ -184,7 +192,7 @@ impl Operation for ConfigureKmsHandler {
&cred, &cred,
owner, owner,
false, false,
vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)], kms_configure_actions(),
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)), req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
) )
.await?; .await?;
@@ -284,7 +292,7 @@ impl Operation for StartKmsHandler {
&cred, &cred,
owner, owner,
false, false,
vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)], kms_service_control_actions(),
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)), req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
) )
.await?; .await?;
@@ -414,7 +422,7 @@ impl Operation for StopKmsHandler {
&cred, &cred,
owner, owner,
false, false,
vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)], kms_service_control_actions(),
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)), req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
) )
.await?; .await?;
@@ -476,7 +484,7 @@ impl Operation for GetKmsStatusHandler {
&cred, &cred,
owner, owner,
false, false,
vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)], kms_service_control_actions(),
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)), req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
) )
.await?; .await?;
@@ -544,7 +552,7 @@ impl Operation for ReconfigureKmsHandler {
&cred, &cred,
owner, owner,
false, false,
vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)], kms_configure_actions(),
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)), req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
) )
.await?; .await?;
@@ -625,3 +633,29 @@ impl Operation for ReconfigureKmsHandler {
Ok(S3Response::new((StatusCode::OK, Body::from(json_response)))) 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));
}
}
+80 -11
View File
@@ -24,7 +24,7 @@ use hyper::{HeaderMap, Method, StatusCode};
use matchit::Params; use matchit::Params;
use rustfs_config::MAX_ADMIN_REQUEST_BODY_SIZE; use rustfs_config::MAX_ADMIN_REQUEST_BODY_SIZE;
use rustfs_kms::{KmsError, init_global_kms_service_manager, types::*}; 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::header::CONTENT_TYPE;
use s3s::{Body, S3Request, S3Response, S3Result, s3_error}; use s3s::{Body, S3Request, S3Response, S3Result, s3_error};
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
@@ -118,6 +118,38 @@ async fn kms_encryption_service_from_context() -> Option<std::sync::Arc<rustfs_k
manager.get_encryption_service().await manager.get_encryption_service().await
} }
fn kms_create_key_actions() -> Vec<Action> {
// 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<Action> {
// 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<Action> {
// 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<Action> {
vec![Action::KmsAction(KmsAction::GenerateDataKeyAction)]
}
fn kms_delete_key_actions() -> Vec<Action> {
vec![Action::KmsAction(KmsAction::DeleteKeyAction)]
}
pub fn register_kms_key_route(r: &mut S3Router<AdminOperation>) -> std::io::Result<()> { pub fn register_kms_key_route(r: &mut S3Router<AdminOperation>) -> std::io::Result<()> {
r.insert( r.insert(
Method::POST, Method::POST,
@@ -170,7 +202,7 @@ impl Operation for CreateKeyHandler {
&cred, &cred,
owner, owner,
false, false,
vec![Action::AdminAction(AdminAction::KMSCreateKeyAdminAction)], kms_create_key_actions(),
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)), req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
) )
.await?; .await?;
@@ -249,7 +281,7 @@ impl Operation for DescribeKeyHandler {
&cred, &cred,
owner, owner,
false, false,
vec![Action::AdminAction(AdminAction::KMSKeyStatusAdminAction)], kms_describe_key_actions(),
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)), req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
) )
.await?; .await?;
@@ -288,8 +320,20 @@ impl Operation for DescribeKeyHandler {
#[cfg(test)] #[cfg(test)]
mod tests { 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 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] #[test]
fn test_extract_key_id_supports_minio_aliases() { fn test_extract_key_id_supports_minio_aliases() {
@@ -331,6 +375,31 @@ mod tests {
assert_eq!(extract_key_id(&uri).as_deref(), expected); 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) /// List KMS keys (legacy endpoint)
@@ -351,7 +420,7 @@ impl Operation for ListKeysHandler {
&cred, &cred,
owner, owner,
false, false,
vec![Action::AdminAction(AdminAction::KMSKeyStatusAdminAction)], kms_list_keys_actions(),
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)), req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
) )
.await?; .await?;
@@ -413,7 +482,7 @@ impl Operation for GenerateDataKeyHandler {
&cred, &cred,
owner, owner,
false, false,
vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)], kms_generate_data_key_actions(),
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)), req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
) )
.await?; .await?;
@@ -479,7 +548,7 @@ impl Operation for CreateKmsKeyHandler {
&cred, &cred,
owner, owner,
false, false,
vec![Action::AdminAction(AdminAction::KMSCreateKeyAdminAction)], kms_create_key_actions(),
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)), req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
) )
.await?; .await?;
@@ -613,7 +682,7 @@ impl Operation for DeleteKmsKeyHandler {
&cred, &cred,
owner, owner,
false, false,
vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)], kms_delete_key_actions(),
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)), req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
) )
.await?; .await?;
@@ -762,7 +831,7 @@ impl Operation for CancelKmsKeyDeletionHandler {
&cred, &cred,
owner, owner,
false, false,
vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)], kms_delete_key_actions(),
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)), req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
) )
.await?; .await?;
@@ -891,7 +960,7 @@ impl Operation for ListKmsKeysHandler {
&cred, &cred,
owner, owner,
false, false,
vec![Action::AdminAction(AdminAction::KMSKeyStatusAdminAction)], kms_list_keys_actions(),
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)), req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
) )
.await?; .await?;
@@ -1003,7 +1072,7 @@ impl Operation for DescribeKmsKeyHandler {
&cred, &cred,
owner, owner,
false, false,
vec![Action::AdminAction(AdminAction::KMSKeyStatusAdminAction)], kms_describe_key_actions(),
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)), req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
) )
.await?; .await?;
+42 -4
View File
@@ -23,7 +23,7 @@ use crate::server::{ADMIN_PREFIX, RemoteAddr};
use hyper::{HeaderMap, Method, StatusCode}; use hyper::{HeaderMap, Method, StatusCode};
use matchit::Params; use matchit::Params;
use rustfs_kms::{KmsBackend, init_global_kms_service_manager}; 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::header::CONTENT_TYPE;
use s3s::{Body, S3Request, S3Response, S3Result, s3_error}; use s3s::{Body, S3Request, S3Response, S3Result, s3_error};
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
@@ -52,6 +52,18 @@ fn backend_name(backend: &KmsBackend) -> &'static str {
} }
} }
fn kms_service_control_actions() -> Vec<Action> {
vec![Action::KmsAction(KmsAction::ServiceControlAction)]
}
fn kms_configure_actions() -> Vec<Action> {
vec![Action::KmsAction(KmsAction::ConfigureAction)]
}
fn kms_clear_cache_actions() -> Vec<Action> {
vec![Action::KmsAction(KmsAction::ClearCacheAction)]
}
#[derive(Debug, Serialize, Deserialize)] #[derive(Debug, Serialize, Deserialize)]
pub struct KmsStatusResponse { pub struct KmsStatusResponse {
pub backend_type: String, pub backend_type: String,
@@ -158,7 +170,7 @@ impl Operation for KmsStatusHandler {
&cred, &cred,
owner, owner,
false, false,
vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)], kms_service_control_actions(),
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)), req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
) )
.await?; .await?;
@@ -220,7 +232,7 @@ impl Operation for KmsConfigHandler {
&cred, &cred,
owner, owner,
false, false,
vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)], kms_configure_actions(),
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)), req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
) )
.await?; .await?;
@@ -269,7 +281,7 @@ impl Operation for KmsClearCacheHandler {
&cred, &cred,
owner, owner,
false, false,
vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)], kms_clear_cache_actions(),
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)), req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
) )
.await?; .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));
}
}
+77 -22
View File
@@ -45,8 +45,13 @@ const GET_TABLE_NAMESPACE: AdminActionRef = AdminActionRef::new("GetTableNamespa
const HEAL: AdminActionRef = AdminActionRef::new("HealAdminAction"); const HEAL: AdminActionRef = AdminActionRef::new("HealAdminAction");
const IMPORT_BUCKET_METADATA: AdminActionRef = AdminActionRef::new("ImportBucketMetadataAction"); const IMPORT_BUCKET_METADATA: AdminActionRef = AdminActionRef::new("ImportBucketMetadataAction");
const IMPORT_IAM: AdminActionRef = AdminActionRef::new("ImportIAMAction"); const IMPORT_IAM: AdminActionRef = AdminActionRef::new("ImportIAMAction");
const KMS_CREATE_KEY: AdminActionRef = AdminActionRef::new("KMSCreateKeyAdminAction"); const KMS_CLEAR_CACHE: AdminActionRef = AdminActionRef::new("kms:ClearCache");
const KMS_KEY_STATUS: AdminActionRef = AdminActionRef::new("KMSKeyStatusAdminAction"); 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_GROUPS: AdminActionRef = AdminActionRef::new("ListGroupsAdminAction");
const LIST_TEMPORARY_ACCOUNTS: AdminActionRef = AdminActionRef::new("ListTemporaryAccountsAdminAction"); const LIST_TEMPORARY_ACCOUNTS: AdminActionRef = AdminActionRef::new("ListTemporaryAccountsAdminAction");
const LIST_TIER: AdminActionRef = AdminActionRef::new("ListTierAction"); 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/profile", PROFILING, RouteRiskLevel::High),
admin(HttpMethod::Get, "/rustfs/admin/debug/pprof/status", 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::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/create-key", KMS_CONFIGURE, RouteRiskLevel::High),
admin(HttpMethod::Post, "/rustfs/admin/v3/kms/key/create", KMS_CREATE_KEY, RouteRiskLevel::High), admin(HttpMethod::Post, "/rustfs/admin/v3/kms/key/create", KMS_CONFIGURE, RouteRiskLevel::High),
admin( admin(
HttpMethod::Get, HttpMethod::Get,
"/rustfs/admin/v3/kms/describe-key", "/rustfs/admin/v3/kms/describe-key",
KMS_KEY_STATUS, KMS_DESCRIBE_KEY,
RouteRiskLevel::Sensitive, RouteRiskLevel::Sensitive,
), ),
admin( admin(
HttpMethod::Get, HttpMethod::Get,
"/rustfs/admin/v3/kms/key/status", "/rustfs/admin/v3/kms/key/status",
KMS_KEY_STATUS, KMS_DESCRIBE_KEY,
RouteRiskLevel::Sensitive, RouteRiskLevel::Sensitive,
), ),
admin( admin(
HttpMethod::Get, HttpMethod::Get,
"/rustfs/admin/v3/kms/list-keys", "/rustfs/admin/v3/kms/list-keys",
KMS_KEY_STATUS, KMS_LIST_KEYS,
RouteRiskLevel::Sensitive, RouteRiskLevel::Sensitive,
), ),
admin( admin(
HttpMethod::Post, HttpMethod::Post,
"/rustfs/admin/v3/kms/generate-data-key", "/rustfs/admin/v3/kms/generate-data-key",
SERVER_INFO, KMS_GENERATE_DATA_KEY,
RouteRiskLevel::High, RouteRiskLevel::High,
), ),
admin(HttpMethod::Get, "/rustfs/admin/v3/kms/status", SERVER_INFO, RouteRiskLevel::Sensitive), admin(
admin(HttpMethod::Post, "/rustfs/admin/v3/kms/status", SERVER_INFO, RouteRiskLevel::High), HttpMethod::Get,
admin(HttpMethod::Get, "/rustfs/admin/v3/kms/config", SERVER_INFO, RouteRiskLevel::Sensitive), "/rustfs/admin/v3/kms/status",
admin(HttpMethod::Post, "/rustfs/admin/v3/kms/clear-cache", SERVER_INFO, RouteRiskLevel::High), KMS_SERVICE_CONTROL,
admin(HttpMethod::Post, "/rustfs/admin/v3/kms/configure", SERVER_INFO, RouteRiskLevel::High), RouteRiskLevel::Sensitive,
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::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( admin(
HttpMethod::Get, HttpMethod::Get,
"/rustfs/admin/v3/kms/service-status", "/rustfs/admin/v3/kms/service-status",
SERVER_INFO, KMS_SERVICE_CONTROL,
RouteRiskLevel::Sensitive, RouteRiskLevel::Sensitive,
), ),
admin(HttpMethod::Post, "/rustfs/admin/v3/kms/reconfigure", 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_CREATE_KEY, RouteRiskLevel::High), admin(HttpMethod::Post, "/rustfs/admin/v3/kms/keys", KMS_CONFIGURE, RouteRiskLevel::High),
admin(HttpMethod::Delete, "/rustfs/admin/v3/kms/keys/delete", SERVER_INFO, RouteRiskLevel::High), admin(
HttpMethod::Delete,
"/rustfs/admin/v3/kms/keys/delete",
KMS_DELETE_KEY,
RouteRiskLevel::High,
),
admin( admin(
HttpMethod::Post, HttpMethod::Post,
"/rustfs/admin/v3/kms/keys/cancel-deletion", "/rustfs/admin/v3/kms/keys/cancel-deletion",
SERVER_INFO, KMS_DELETE_KEY,
RouteRiskLevel::High, 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( admin(
HttpMethod::Get, HttpMethod::Get,
"/rustfs/admin/v3/kms/keys/{key_id}", "/rustfs/admin/v3/kms/keys/{key_id}",
KMS_KEY_STATUS, KMS_DESCRIBE_KEY,
RouteRiskLevel::Sensitive, RouteRiskLevel::Sensitive,
), ),
public( 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] #[test]
fn route_policy_keeps_contextual_auth_deferred() { fn route_policy_keeps_contextual_auth_deferred() {
assert_deferred( assert_deferred(
@@ -856,6 +903,14 @@ mod tests {
assert_eq!(spec.access().admin_action(), Some(action)); 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) { fn assert_public(method: HttpMethod, path: &str, kind: PublicRouteKind) {
let spec = ADMIN_ROUTE_POLICY_SPECS let spec = ADMIN_ROUTE_POLICY_SPECS
.iter() .iter()