gsadmin/rustfs
1
0
Fork 0
mirror of https://github.com/rustfs/rustfs.git synced 2026-08-18 10:43:15 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(admin): route kms management auth through shared gate (#6194)

Browse Source
This commit is contained in:
Zhengchao An
2026-08-18 12:21:49 +08:00
committed by GitHub
parent de9145e87a
commit b825c54850
1 changed files with 75 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files
rustfs/src/admin/handlers/kms_management.rs
+75 -52
View File
@@ -16,13 +16,12 @@
use super::kms_dynamic::current_kms_config_fingerprint;
use super::kms_keys::{CreateKeyHandler, DescribeKeyHandler, GenerateDataKeyHandler, ListKeysHandler};
use crate::admin::auth::validate_admin_request;
use crate::admin::auth::authorize_admin_request;
use crate::admin::router::{AdminOperation, Operation, S3Router};
use crate::admin::runtime_sources::{
current_kms_runtime_service_manager, current_notification_system, current_or_init_kms_runtime_service_manager,
};
use crate::auth::{check_key_valid, get_session_token};
use crate::server::{ADMIN_PREFIX, RemoteAddr};
use crate::server::ADMIN_PREFIX;
use hyper::{HeaderMap, Method, StatusCode};
use matchit::Params;
use rustfs_kms::KmsBackend;
@@ -69,6 +68,18 @@ fn kms_clear_cache_actions() -> Vec<Action> {
vec![Action::KmsAction(KmsAction::ClearCacheAction)]
}
/// Admin gate for the KMS management endpoints, none of which act on a key.
///
/// The pre-check keeps these endpoints' historical missing-credentials message;
/// the shared gate reports "get cred failed".
async fn authorize_kms_management_request(req: &S3Request<Body>, actions: Vec<Action>) -> S3Result<()> {
if req.credentials.is_none() {
return Err(s3_error!(InvalidRequest, "authentication required"));
}
authorize_admin_request(req, actions).await?;
Ok(())
}
/// Response of `POST /kms/clear-cache`.
///
/// Declared rather than built inline so the shape the console already depends
@@ -260,22 +271,7 @@ pub struct KmsStatusHandler {}
#[async_trait::async_trait]
impl Operation for KmsStatusHandler {
async fn call(&self, req: S3Request<Body>, _params: Params<'_, '_>) -> S3Result<S3Response<(StatusCode, Body)>> {
let Some(cred) = req.credentials else {
return Err(s3_error!(InvalidRequest, "authentication required"));
};
let (cred, owner) =
check_key_valid(get_session_token(&req.uri, &req.headers).unwrap_or_default(), &cred.access_key).await?;
validate_admin_request(
&req.headers,
&cred,
owner,
false,
kms_service_control_actions(),
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
)
.await?;
authorize_kms_management_request(&req, kms_service_control_actions()).await?;
let Some(service) = kms_encryption_service_from_context().await else {
return Err(s3_error!(InternalError, "KMS service not initialized"));
@@ -326,22 +322,7 @@ pub struct KmsConfigHandler {}
#[async_trait::async_trait]
impl Operation for KmsConfigHandler {
async fn call(&self, req: S3Request<Body>, _params: Params<'_, '_>) -> S3Result<S3Response<(StatusCode, Body)>> {
let Some(cred) = req.credentials else {
return Err(s3_error!(InvalidRequest, "authentication required"));
};
let (cred, owner) =
check_key_valid(get_session_token(&req.uri, &req.headers).unwrap_or_default(), &cred.access_key).await?;
validate_admin_request(
&req.headers,
&cred,
owner,
false,
kms_configure_actions(),
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
)
.await?;
authorize_kms_management_request(&req, kms_configure_actions()).await?;
let Some(service) = kms_encryption_service_from_context().await else {
return Err(s3_error!(InternalError, "KMS service not initialized"));
@@ -375,22 +356,7 @@ pub struct KmsClearCacheHandler {}
#[async_trait::async_trait]
impl Operation for KmsClearCacheHandler {
async fn call(&self, req: S3Request<Body>, _params: Params<'_, '_>) -> S3Result<S3Response<(StatusCode, Body)>> {
let Some(cred) = req.credentials else {
return Err(s3_error!(InvalidRequest, "authentication required"));
};
let (cred, owner) =
check_key_valid(get_session_token(&req.uri, &req.headers).unwrap_or_default(), &cred.access_key).await?;
validate_admin_request(
&req.headers,
&cred,
owner,
false,
kms_clear_cache_actions(),
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
)
.await?;
authorize_kms_management_request(&req, kms_clear_cache_actions()).await?;
let Some(service) = kms_encryption_service_from_context().await else {
return Err(s3_error!(InternalError, "KMS service not initialized"));
@@ -422,9 +388,14 @@ impl Operation for KmsClearCacheHandler {
#[cfg(test)]
mod tests {
use super::{KmsClearCacheResponse, kms_clear_cache_actions, kms_configure_actions, kms_service_control_actions};
use super::{
KmsClearCacheResponse, authorize_kms_management_request, kms_clear_cache_actions, kms_configure_actions,
kms_service_control_actions,
};
use crate::admin::handlers::kms_keys::stable_json_value;
use hyper::HeaderMap;
use rustfs_policy::policy::action::{Action, AdminAction, KmsAction};
use s3s::{Body, S3Request};
fn assert_has_action(actions: &[Action], action: Action) {
assert!(actions.contains(&action), "expected action list to contain {action:?}");
@@ -434,6 +405,58 @@ mod tests {
assert!(!actions.contains(&action), "expected action list not to contain {action:?}");
}
/// These endpoints authorize through the shared admin gate, which reports
/// "get cred failed" for a credential-less request. The pre-check keeps the
/// message these endpoints have always returned (rustfs/backlog#1829).
#[tokio::test]
async fn kms_management_gate_keeps_its_missing_credentials_message() {
let req = S3Request {
input: Body::from(String::new()),
method: http::Method::GET,
uri: "/rustfs/admin/v3/kms/status".parse().expect("uri should parse"),
headers: HeaderMap::new(),
extensions: http::Extensions::new(),
credentials: None,
region: None,
service: None,
trailing_headers: None,
};
let err = authorize_kms_management_request(&req, kms_service_control_actions())
.await
.expect_err("a request without credentials must be rejected");
assert_eq!(err.code(), &s3s::S3ErrorCode::InvalidRequest);
assert_eq!(err.message(), Some("authentication required"));
}
/// Every management endpoint must reach the shared gate, each with its own
/// action set. The action lists are pinned above, but nothing else checks
/// which handler asks for which, and a handler that lost its gate entirely
/// would still serve its response.
#[test]
fn management_handlers_authorize_with_their_dedicated_actions() {
let src = include_str!("kms_management.rs");
for (handler, actions) in [
("KmsStatusHandler", "kms_service_control_actions()"),
("KmsConfigHandler", "kms_configure_actions()"),
("KmsClearCacheHandler", "kms_clear_cache_actions()"),
] {
let block = src
.split_once(&format!("impl Operation for {handler}"))
.unwrap_or_else(|| panic!("{handler} impl should exist"))
.1;
let end = block
.find("\nimpl Operation for")
.or_else(|| block.find("\n#[cfg(test)]"))
.unwrap_or(block.len());
assert!(
block[..end].contains(&format!("authorize_kms_management_request(&req, {actions})")),
"{handler} must authorize through the shared gate with {actions}"
);
}
}
#[test]
fn kms_management_auth_actions_use_dedicated_kms_actions() {
assert_has_action(&kms_service_control_actions(), Action::KmsAction(KmsAction::ServiceControlAction));