From 923e35efa04f541aa144e2dd49c5d271eb49dfa5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=94=90=E5=B0=8F=E9=B8=AD?= Date: Thu, 6 Aug 2026 08:29:17 +0800 Subject: [PATCH] fix(site-replication): use MinIO-compatible sts-account IAM item type (#5750) * test(site-replication): expect MinIO sts-account IAM item type MinIO madmin-go replicates STS credentials with SRIAMItem type "sts-account" (SRIAMItemSTSAcc), but RustFS emits and accepts only "sts-credential", so cross-implementation STS replication fails in both directions (MinIO returns errSRInvalidRequest, RustFS returns NotImplemented). Red-light tests: - pin the outbound AssumeRole replication item type to "sts-account" (construction extracted into assume_role_site_replication_item so it is testable, behavior unchanged in this commit) - update the federated identity replication item snapshot to "sts-account" - inbound apply_iam_item must dispatch both "sts-account" and the legacy "sts-credential" alias to the STS arm instead of the unknown-type NotImplemented fallback * fix(site-replication): use MinIO-compatible sts-account IAM item type MinIO madmin-go replicates STS credentials with SRIAMItem type "sts-account" (SRIAMItemSTSAcc). RustFS emitted "sts-credential" and accepted only that value inbound, so STS credential replication with MinIO peers failed in both directions: MinIO rejected RustFS items as errSRInvalidRequest and RustFS answered MinIO items with NotImplemented. - define SR_IAM_ITEM_STS_ACC ("sts-account") and SR_IAM_ITEM_STS_ACC_LEGACY ("sts-credential") in rustfs-madmin - emit "sts-account" from both outbound sites (AssumeRole hook and federated identity OIDC hook) - accept both types inbound; the legacy alias remains permanently for mixed-version RustFS rolling upgrades Token verification and the retry/event mechanism are unchanged. --- crates/madmin/src/site_replication.rs | 9 ++ rustfs/src/admin/handlers/site_replication.rs | 108 +++++++++++++++++- rustfs/src/admin/handlers/sts.rs | 60 +++++++--- .../src/admin/service/federated_identity.rs | 6 +- 4 files changed, 158 insertions(+), 25 deletions(-) diff --git a/crates/madmin/src/site_replication.rs b/crates/madmin/src/site_replication.rs index 259b9d7ff..b22b7a58b 100644 --- a/crates/madmin/src/site_replication.rs +++ b/crates/madmin/src/site_replication.rs @@ -21,6 +21,15 @@ use time::OffsetDateTime; pub const SITE_REPL_API_VERSION: &str = "1"; +/// `SRIAMItem` type for replicated STS credentials, matching MinIO madmin-go +/// `SRIAMItemSTSAcc`. MinIO peers reject any other value as an invalid request. +pub const SR_IAM_ITEM_STS_ACC: &str = "sts-account"; + +/// STS item type emitted by RustFS releases prior to the MinIO alignment. +/// Never emitted anymore, but accepted inbound permanently so mixed-version +/// RustFS sites keep replicating STS credentials during rolling upgrades. +pub const SR_IAM_ITEM_STS_ACC_LEGACY: &str = "sts-credential"; + #[derive(Clone, Serialize, Deserialize, Default)] pub struct PeerSite { #[serde(default)] diff --git a/rustfs/src/admin/handlers/site_replication.rs b/rustfs/src/admin/handlers/site_replication.rs index 81104c5df..983c39d4d 100644 --- a/rustfs/src/admin/handlers/site_replication.rs +++ b/rustfs/src/admin/handlers/site_replication.rs @@ -70,11 +70,11 @@ use rustfs_iam::sys::{ use rustfs_madmin::{ AddOrUpdateUserReq, BucketBandwidth, GroupAddRemove, GroupStatus, IDPSettings, InProgressMetric, InQueueMetric, LDAPConfigSettings, LDAPSettings, OpenIDProviderSettings, PeerInfo, PeerSite, QStat, ReplProxyMetric, ReplicateAddStatus, - ReplicateEditStatus, ReplicateRemoveStatus, ResyncBucketStatus, SITE_REPL_API_VERSION, SRBucketInfo, SRBucketMeta, - SRBucketStatsSummary, SRGroupInfo, SRGroupStatsSummary, SRIAMItem, SRIAMPolicy, SRILMExpiryStatsSummary, SRInfo, SRMetric, - SRMetricsSummary, SRPeerError, SRPeerJoinReq, SRPendingOperation, SRPolicyMapping, SRPolicyStatsSummary, SRRemoveReq, - SRResyncOpStatus, SRRetryStats, SRSessionPolicy, SRSiteSummary, SRStateEditReq, SRStateInfo, SRStatusInfo, SRSvcAccCreate, - SRUserStatsSummary, SiteReplicationInfo, SyncStatus, WorkerStat, + ReplicateEditStatus, ReplicateRemoveStatus, ResyncBucketStatus, SITE_REPL_API_VERSION, SR_IAM_ITEM_STS_ACC, + SR_IAM_ITEM_STS_ACC_LEGACY, SRBucketInfo, SRBucketMeta, SRBucketStatsSummary, SRGroupInfo, SRGroupStatsSummary, SRIAMItem, + SRIAMPolicy, SRILMExpiryStatsSummary, SRInfo, SRMetric, SRMetricsSummary, SRPeerError, SRPeerJoinReq, SRPendingOperation, + SRPolicyMapping, SRPolicyStatsSummary, SRRemoveReq, SRResyncOpStatus, SRRetryStats, SRSessionPolicy, SRSiteSummary, + SRStateEditReq, SRStateInfo, SRStatusInfo, SRSvcAccCreate, SRUserStatsSummary, SiteReplicationInfo, SyncStatus, WorkerStat, }; use rustfs_policy::policy::{ Policy, @@ -7854,7 +7854,11 @@ async fn apply_iam_item(item: SRIAMItem) -> S3Result<()> { .map_err(ApiError::from)?; Ok(()) } - "sts-credential" => { + // MinIO madmin-go sends `SRIAMItemSTSAcc = "sts-account"`. The legacy alias + // `sts-credential` (emitted by older RustFS releases) stays accepted permanently + // so mixed-version RustFS sites keep replicating STS credentials during rolling + // upgrades; it is a compatibility layer, not temporary code. + SR_IAM_ITEM_STS_ACC | SR_IAM_ITEM_STS_ACC_LEGACY => { let Some(sts_credential) = item.sts_credential else { return Err(s3_error!(InvalidRequest, "stsCredential is required")); }; @@ -9584,6 +9588,98 @@ mod tests { assert_eq!(sts_replication_compatibility_policy(&legacy_claims, "readonly"), Some("readonly")); } + /// Publish a ready IAM app context so `apply_iam_item` gets past its IAM guard. + async fn publish_ready_iam_context() { + use crate::admin::runtime_sources::{AppContext, publish_test_app_context}; + use rustfs_iam::store::{Store as _, object::IAM_CONFIG_PREFIX}; + + let _ = rustfs_credentials::init_global_action_credentials( + Some("TESTROOTACCESSKEY".to_string()), + Some("TESTROOTSECRET123".to_string()), + ); + if current_iam_handle().is_none() { + let env = rustfs_test_utils::TestECStoreEnv::builder() + .prefix("site_replication_iam_item") + .disk_count(1) + .init_bucket_metadata(false) + .build() + .await; + rustfs_iam::store::object::ObjectStore::new(Arc::clone(&env.ecstore)) + .save_iam_config(serde_json::json!({"version": 1}), format!("{}/format.json", *IAM_CONFIG_PREFIX)) + .await + .expect("seed IAM format"); + let iam = rustfs_iam::build_iam_sys(Arc::clone(&env.ecstore)) + .await + .expect("build test IAM"); + publish_test_app_context(Arc::new(AppContext::with_default_interfaces( + env.ecstore, + iam, + Arc::new(rustfs_kms::KmsServiceManager::new()), + ))); + } + assert!(current_iam_handle().is_some(), "test IAM should be published"); + } + + fn replicated_sts_item(item_type: &str) -> SRIAMItem { + SRIAMItem { + r#type: item_type.to_string(), + sts_credential: Some(rustfs_madmin::SRSTSCredential { + access_key: "REPLICATEDSTSACCESS".to_string(), + secret_key: "replicatedStsSecret123".to_string(), + session_token: "not-a-valid-session-token".to_string(), + parent_user: "replicated-sts-parent".to_string(), + parent_policy_mapping: String::new(), + api_version: Some(SITE_REPL_API_VERSION.to_string()), + }), + updated_at: Some(OffsetDateTime::UNIX_EPOCH), + api_version: Some(SITE_REPL_API_VERSION.to_string()), + ..Default::default() + } + } + + #[tokio::test] + #[serial] + async fn apply_iam_item_accepts_minio_sts_account_item_type() { + publish_ready_iam_context().await; + + // MinIO madmin-go sends `SRIAMItemSTSAcc = "sts-account"`. The bogus session token + // must reach token verification — falling into the unknown-type NotImplemented arm + // means MinIO-originated STS replication would be rejected. + let err = apply_iam_item(replicated_sts_item("sts-account")) + .await + .expect_err("bogus session token must fail verification"); + assert_ne!( + *err.code(), + S3ErrorCode::NotImplemented, + "sts-account must be dispatched to the STS credential arm, got: {err:?}" + ); + assert!( + err.message().unwrap_or_default().contains("invalid STS session token"), + "expected a token verification error, got: {err:?}" + ); + } + + #[tokio::test] + #[serial] + async fn apply_iam_item_still_accepts_legacy_sts_credential_item_type() { + publish_ready_iam_context().await; + + // Older RustFS peers emit `sts-credential`; the alias stays accepted permanently + // so mixed-version RustFS sites keep replicating STS credentials. + let err = apply_iam_item(replicated_sts_item("sts-credential")) + .await + .expect_err("bogus session token must fail verification"); + assert_ne!( + *err.code(), + S3ErrorCode::NotImplemented, + "legacy sts-credential must stay accepted, got: {err:?}" + ); + assert!( + err.message().unwrap_or_default().contains("invalid STS session token"), + "expected a token verification error, got: {err:?}" + ); + } + #[test] fn oidc_service_account_envelope_round_trips_actual_policy() { let actual_policy = r#"{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":["s3:GetObject"],"Resource":["arn:aws:s3:::bucket/*"]}]}"#; diff --git a/rustfs/src/admin/handlers/sts.rs b/rustfs/src/admin/handlers/sts.rs index 60a1911c9..a0a0135f9 100644 --- a/rustfs/src/admin/handlers/sts.rs +++ b/rustfs/src/admin/handlers/sts.rs @@ -32,7 +32,7 @@ use hyper::Method; use matchit::Params; use rustfs_config::MAX_ADMIN_REQUEST_BODY_SIZE; use rustfs_iam::federation::{FederatedSessionBindingError, FederationError}; -use rustfs_madmin::{SITE_REPL_API_VERSION, SRIAMItem, SRSTSCredential}; +use rustfs_madmin::{SITE_REPL_API_VERSION, SR_IAM_ITEM_STS_ACC, SRIAMItem, SRSTSCredential}; use rustfs_policy::{ auth::get_new_credentials_with_metadata, policy::{ @@ -76,6 +76,24 @@ fn clamp_assume_role_duration(duration_seconds: usize) -> usize { } } +/// Build the site-replication IAM item that mirrors an AssumeRole temporary credential to peers. +fn assume_role_site_replication_item(cred: &rustfs_credentials::Credentials, updated_at: OffsetDateTime) -> SRIAMItem { + SRIAMItem { + r#type: SR_IAM_ITEM_STS_ACC.to_string(), + sts_credential: Some(SRSTSCredential { + access_key: cred.access_key.clone(), + secret_key: cred.secret_key.clone(), + session_token: cred.session_token.clone(), + parent_user: cred.parent_user.clone(), + api_version: Some(SITE_REPL_API_VERSION.to_string()), + ..Default::default() + }), + updated_at: Some(updated_at), + api_version: Some(SITE_REPL_API_VERSION.to_string()), + ..Default::default() + } +} + fn web_identity_federation_error(error: FederationError) -> S3Error { match error { FederationError::TokenVerification(message) => { @@ -244,21 +262,7 @@ async fn handle_assume_role( let root_access_key = current_action_credentials().map(|cred| cred.access_key); if root_access_key.as_deref() != Some(new_cred.parent_user.as_str()) - && let Err(err) = site_replication_iam_change_hook(SRIAMItem { - r#type: "sts-credential".to_string(), - sts_credential: Some(SRSTSCredential { - access_key: new_cred.access_key.clone(), - secret_key: new_cred.secret_key.clone(), - session_token: new_cred.session_token.clone(), - parent_user: new_cred.parent_user.clone(), - api_version: Some(SITE_REPL_API_VERSION.to_string()), - ..Default::default() - }), - updated_at: Some(updated_at), - api_version: Some(SITE_REPL_API_VERSION.to_string()), - ..Default::default() - }) - .await + && let Err(err) = site_replication_iam_change_hook(assume_role_site_replication_item(&new_cred, updated_at)).await { warn!("site replication STS hook failed, err: {err}"); } @@ -422,6 +426,30 @@ mod tests { assert!(exp - now <= STS_MAX_DURATION_SECS as i64); } + #[test] + fn assume_role_replication_item_uses_minio_sts_account_type() { + let cred = rustfs_credentials::Credentials { + access_key: "ASSUMEROLETESTACCESS".to_string(), + secret_key: "assumeRoleTestSecret123".to_string(), + session_token: "assume-role-test-session-token".to_string(), + parent_user: "assume-role-parent".to_string(), + ..Default::default() + }; + + let item = assume_role_site_replication_item(&cred, OffsetDateTime::UNIX_EPOCH); + + // MinIO madmin-go `SRIAMItemSTSAcc`: any other value is rejected by MinIO peers. + assert_eq!(item.r#type, "sts-account"); + assert_eq!(item.updated_at, Some(OffsetDateTime::UNIX_EPOCH)); + assert_eq!(item.api_version.as_deref(), Some(SITE_REPL_API_VERSION)); + let sts = item.sts_credential.expect("replication item should carry the STS credential"); + assert_eq!(sts.access_key, cred.access_key); + assert_eq!(sts.secret_key, cred.secret_key); + assert_eq!(sts.session_token, cred.session_token); + assert_eq!(sts.parent_user, cred.parent_user); + assert_eq!(sts.api_version.as_deref(), Some(SITE_REPL_API_VERSION)); + } + #[test] fn web_identity_errors_preserve_existing_s3_semantics() { let cases = [ diff --git a/rustfs/src/admin/service/federated_identity.rs b/rustfs/src/admin/service/federated_identity.rs index b6e25244e..82bead063 100644 --- a/rustfs/src/admin/service/federated_identity.rs +++ b/rustfs/src/admin/service/federated_identity.rs @@ -22,7 +22,7 @@ use rustfs_iam::{ store::{MappedPolicy, object::ObjectStore}, sys::{IamSys, is_safe_claim_policy_name}, }; -use rustfs_madmin::{SITE_REPL_API_VERSION, SRIAMItem, SRSTSCredential}; +use rustfs_madmin::{SITE_REPL_API_VERSION, SR_IAM_ITEM_STS_ACC, SRIAMItem, SRSTSCredential}; use rustfs_policy::auth::get_new_credentials_with_metadata; use s3s::{S3Error, S3ErrorCode}; use serde_json::Value; @@ -238,7 +238,7 @@ fn issue_credentials( fn site_replication_item(credentials: &rustfs_credentials::Credentials, updated_at: OffsetDateTime) -> SRIAMItem { SRIAMItem { - r#type: "sts-credential".to_string(), + r#type: SR_IAM_ITEM_STS_ACC.to_string(), sts_credential: Some(SRSTSCredential { access_key: credentials.access_key.clone(), secret_key: credentials.secret_key.clone(), @@ -494,7 +494,7 @@ mod tests { assert_eq!( serde_json::to_value(item).expect("replication item should serialize"), serde_json::json!({ - "type": "sts-credential", + "type": "sts-account", "name": "", "stsCredential": { "accessKey": "",