From 6303aa9a42569325b3022979389df3b7fe115fa4 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 22:00:28 +0800 Subject: [PATCH] fix(site-replication): translate policy mapping userType at MinIO wire boundary (#5751) * test(site-replication): pin MinIO IAMUserType wire semantics for policy mappings Red tests for P0-4: MinIO peers send SRPolicyMapping.UserType using the madmin IAMUserType table (unknown=-1, regUser=0, stsUser=1, svcUser=2), while RustFS deserializes the field as u64 and decodes it with the internal RPC table (None=0, Svc=1, Sts=2, Reg=3). - userType -1 (MinIO group mappings) fails to deserialize, rejecting the whole IAM item: group mappings never sync from MinIO. - stsUser=1 decodes as Svc, landing federated STS mappings under the wrong prefix and silently dropping their effect. * fix(site-replication): translate policy mapping userType at MinIO wire boundary SRPolicyMapping.userType travels on the wire using MinIO's IAMUserType table (unknown=-1, regUser=0, stsUser=1, svcUser=2), but RustFS stored the field as u64 and reused the internal RPC encoding UserType::to_u64/from_u64 (None=0, Svc=1, Sts=2, Reg=3) at the site replication boundary. Consequences: MinIO group mappings (userType -1) failed to deserialize and the whole IAM item was rejected, and MinIO STS mappings (1) were stored as service-account mappings, silently dropping federated users' policies. - Widen SRPolicyMapping.user_type and SRCredInfo.iam_user_type to i64 so MinIO's -1 deserializes. - Add sr_wire_user_type / user_type_from_sr_wire in rustfs-iam as the dedicated SR wire codec: MinIO table on both directions, groups always encoded as 0, and wire value 3 kept forever as an alias for Reg so mappings from pre-fix RustFS peers still decode; unknown values fail closed. - Route the SR inbound (apply_iam_item) and outbound (mapped_policy_to_sr_mapping, policy-mapping change hooks) paths through the codec. The internal UserType::to_u64/from_u64 encoding is untouched: it is the intra-cluster node RPC contract and changing it would break rolling restarts. Outbound compatibility with old RustFS peers is preserved because UserType::None and Reg share the users prefix in get_mapped_policy_path, so wire 0 lands in the same location Reg=3 did. --- crates/iam/src/store.rs | 120 +++++++++++++++++- crates/madmin/src/site_replication.rs | 41 +++++- rustfs/src/admin/handlers/policies.rs | 4 +- rustfs/src/admin/handlers/site_replication.rs | 9 +- 4 files changed, 164 insertions(+), 10 deletions(-) diff --git a/crates/iam/src/store.rs b/crates/iam/src/store.rs index 529c5401c..6c5d7b2f6 100644 --- a/crates/iam/src/store.rs +++ b/crates/iam/src/store.rs @@ -139,6 +139,59 @@ impl UserType { } } +/// Encode a [`UserType`] as the site-replication wire value for +/// `SRPolicyMapping.userType` / `SRCredInfo.iamUserType`. +/// +/// The wire uses MinIO's `IAMUserType` table (cmd/iam.go): +/// +/// | wire | MinIO meaning | +/// |------|---------------| +/// | -1 | unknown | +/// | 0 | regUser | +/// | 1 | stsUser | +/// | 2 | svcUser | +/// +/// This is deliberately distinct from the internal encoding +/// [`UserType::to_u64`]/[`UserType::from_u64`] (None=0, Svc=1, Sts=2, Reg=3), +/// which is used by intra-cluster node RPC and must never change (a rolling +/// restart mixes old and new nodes on that RPC). Do not "unify" the two +/// tables: internal values on the SR wire mislabel users on MinIO peers. +/// +/// Group mappings always encode as 0: MinIO routes group mappings by the +/// `isGroup` flag (userType is effectively ignored), and pre-fix RustFS peers +/// sent 0 for groups, so 0 is the one value every peer generation accepts. +pub fn sr_wire_user_type(user_type: UserType, is_group: bool) -> i64 { + if is_group { + return 0; + } + match user_type { + UserType::Reg | UserType::None => 0, + UserType::Sts => 1, + UserType::Svc => 2, + } +} + +/// Decode a site-replication wire `userType` value (see [`sr_wire_user_type`] +/// for the table) into a [`UserType`]. +/// +/// - `-1` (MinIO unknown, sent for group mappings) maps to [`UserType::None`]; +/// `policy_db_set` routes group items by `is_group`, and for non-group items +/// `None` shares the users prefix with `Reg`. +/// - `3` is a permanent alias for [`UserType::Reg`]: pre-fix RustFS peers sent +/// the internal encoding (`Reg.to_u64() == 3`) on the wire. Keep it forever +/// for mixed-version site replication; do not remove. +/// - Anything else is unknown and rejected (`None`), so callers fail closed. +pub fn user_type_from_sr_wire(v: i64) -> Option { + match v { + -1 => Some(UserType::None), + 0 => Some(UserType::Reg), + 1 => Some(UserType::Sts), + 2 => Some(UserType::Svc), + 3 => Some(UserType::Reg), + _ => None, + } +} + #[derive(Serialize, Deserialize, Clone)] pub struct MappedPolicy { pub version: i64, @@ -214,7 +267,72 @@ impl GroupInfo { #[cfg(test)] mod tests { - use super::{GroupInfo, MappedPolicy}; + use super::{GroupInfo, MappedPolicy, UserType, sr_wire_user_type, user_type_from_sr_wire}; + + /// Site-replication inbound decode of `SRPolicyMapping.userType` must + /// follow MinIO IAMUserType wire semantics (cmd/iam.go): stsUser = 1. + /// The internal `UserType::from_u64` table maps 1 to Svc — reusing it at + /// the SR boundary lands federated STS mappings under the wrong prefix + /// and silently drops their effect. + #[test] + fn sr_inbound_decodes_minio_sts_wire_value_as_sts() { + assert_eq!(user_type_from_sr_wire(1), Some(UserType::Sts)); + } + + /// Wire-constant contract: literal MinIO IAMUserType values (cmd/iam.go). + /// WARNING: these literals are the cross-vendor wire format. Never "tidy" + /// them to match `UserType::to_u64`/`from_u64` — that internal table + /// (None=0, Svc=1, Sts=2, Reg=3) belongs to intra-cluster node RPC only. + #[test] + fn sr_wire_decode_matches_minio_iam_user_type_table() { + assert_eq!(user_type_from_sr_wire(-1), Some(UserType::None)); // MinIO unknown (group mappings) + assert_eq!(user_type_from_sr_wire(0), Some(UserType::Reg)); // MinIO regUser + assert_eq!(user_type_from_sr_wire(1), Some(UserType::Sts)); // MinIO stsUser + assert_eq!(user_type_from_sr_wire(2), Some(UserType::Svc)); // MinIO svcUser + // Permanent alias: pre-fix RustFS peers sent internal Reg=3 on the wire. + assert_eq!(user_type_from_sr_wire(3), Some(UserType::Reg)); + // Unknown values fail closed. + assert_eq!(user_type_from_sr_wire(4), None); + assert_eq!(user_type_from_sr_wire(-2), None); + } + + /// Wire-constant contract for the outbound direction. + #[test] + fn sr_wire_encode_matches_minio_iam_user_type_table() { + assert_eq!(sr_wire_user_type(UserType::Reg, false), 0); // MinIO regUser + assert_eq!(sr_wire_user_type(UserType::Sts, false), 1); // MinIO stsUser + assert_eq!(sr_wire_user_type(UserType::Svc, false), 2); // MinIO svcUser + assert_eq!(sr_wire_user_type(UserType::None, false), 0); + // Group mappings always go out as 0 — the value both MinIO (routes by + // isGroup) and pre-fix RustFS peers accept. + for ut in [UserType::Reg, UserType::Sts, UserType::Svc, UserType::None] { + assert_eq!(sr_wire_user_type(ut, true), 0); + } + } + + /// Mixed-version matrix: every value a peer generation can emit decodes to + /// a `UserType` the receiver stores correctly. + #[test] + fn sr_wire_round_trip_covers_old_rustfs_and_minio_peers() { + // Old RustFS outbound: user mappings as internal Reg=3, groups as 0. + assert_eq!(user_type_from_sr_wire(3), Some(UserType::Reg)); + assert_eq!(user_type_from_sr_wire(0), Some(UserType::Reg)); + // New RustFS outbound decodes on its own kind (self round-trip). + for (ut, is_group) in [ + (UserType::Reg, false), + (UserType::Sts, false), + (UserType::Svc, false), + (UserType::None, true), + ] { + assert!(user_type_from_sr_wire(sr_wire_user_type(ut, is_group)).is_some()); + } + // Internal RPC encoding is untouched (rolling-restart contract). + assert_eq!(UserType::None.to_u64(), 0); + assert_eq!(UserType::Svc.to_u64(), 1); + assert_eq!(UserType::Sts.to_u64(), 2); + assert_eq!(UserType::Reg.to_u64(), 3); + assert_eq!(UserType::from_u64(1), Some(UserType::Svc)); + } /// uses RFC3339 for updatedAt. MappedPolicy must serialize as RFC3339. #[test] diff --git a/crates/madmin/src/site_replication.rs b/crates/madmin/src/site_replication.rs index b22b7a58b..a8813f99c 100644 --- a/crates/madmin/src/site_replication.rs +++ b/crates/madmin/src/site_replication.rs @@ -171,8 +171,12 @@ impl fmt::Debug for PeerInfo { pub struct SRPolicyMapping { #[serde(rename = "userOrGroup", default)] pub user_or_group: String, + /// MinIO IAMUserType wire value (cmd/iam.go): unknown = -1, regUser = 0, + /// stsUser = 1, svcUser = 2. Signed because MinIO sends -1 for group + /// mappings. This is NOT the RustFS-internal `UserType` encoding; translate + /// at the boundary with `rustfs_iam::store::{sr_wire_user_type, user_type_from_sr_wire}`. #[serde(rename = "userType", default)] - pub user_type: u64, + pub user_type: i64, #[serde(rename = "isGroup", default)] pub is_group: bool, #[serde(default)] @@ -330,8 +334,10 @@ pub struct SRSvcAccChange { pub struct SRCredInfo { #[serde(rename = "accessKey", default)] pub access_key: String, + /// MinIO IAMUserType wire value (same table as `SRPolicyMapping::user_type`); + /// signed because MinIO's unknown is -1. #[serde(rename = "iamUserType", default)] - pub iam_user_type: u64, + pub iam_user_type: i64, #[serde(rename = "isDeleteReq", default)] pub is_delete_req: bool, #[serde(rename = "userIdentityJSON", default, skip_serializing_if = "Option::is_none")] @@ -1399,7 +1405,7 @@ pub struct SiteNetPerfResult { #[cfg(test)] mod tests { - use super::{PeerInfo, PeerSite, SRInfo, SRResyncOpStatus}; + use super::{PeerInfo, PeerSite, SRCredInfo, SRInfo, SRPolicyMapping, SRResyncOpStatus}; use serde_json::{Value, json}; const TEST_CA_CERT: &str = "-----BEGIN CERTIFICATE-----\ntest-ca\n-----END CERTIFICATE-----"; @@ -1500,6 +1506,35 @@ mod tests { assert!(peer_debug.contains("has_custom_ca: true")); } + /// MinIO IAMUserType wire semantics (cmd/iam.go): unknown = -1, + /// regUser = 0, stsUser = 1, svcUser = 2. MinIO group policy mappings + /// arrive with `userType: -1`; the wire field must accept negatives. + #[test] + fn sr_policy_mapping_accepts_minio_negative_user_type() { + let mapping: SRPolicyMapping = serde_json::from_value(json!({ + "userOrGroup": "devs", + "userType": -1, + "isGroup": true, + "policy": "readwrite" + })) + .expect("MinIO group mapping with userType -1 must deserialize"); + assert_eq!(mapping.user_type, -1); + assert!(mapping.is_group); + assert_eq!(mapping.policy, "readwrite"); + } + + /// Same IAMUserType family as SRPolicyMapping: MinIO may send -1 (unknown). + #[test] + fn sr_cred_info_accepts_minio_negative_iam_user_type() { + let cred: SRCredInfo = serde_json::from_value(json!({ + "accessKey": "replicated-user", + "iamUserType": -1 + })) + .expect("SRCredInfo with iamUserType -1 must deserialize"); + assert_eq!(cred.iam_user_type, -1); + assert_eq!(cred.access_key, "replicated-user"); + } + #[test] fn resync_status_legacy_json_defaults_new_lifecycle_fields() { let legacy_json = json!({ diff --git a/rustfs/src/admin/handlers/policies.rs b/rustfs/src/admin/handlers/policies.rs index 354920acb..19bbee516 100644 --- a/rustfs/src/admin/handlers/policies.rs +++ b/rustfs/src/admin/handlers/policies.rs @@ -571,7 +571,7 @@ impl Operation for SetPolicyForUserOrGroup { r#type: "policy-mapping".to_string(), policy_mapping: Some(SRPolicyMapping { user_or_group: query.user_or_group.clone(), - user_type: rustfs_iam::store::UserType::Reg.to_u64(), + user_type: rustfs_iam::store::sr_wire_user_type(rustfs_iam::store::UserType::Reg, query.is_group), is_group: query.is_group, policy: query.policy_name.clone(), updated_at: Some(updated_at), @@ -1060,7 +1060,7 @@ pub(crate) async fn handle_builtin_policy_association( r#type: "policy-mapping".to_string(), policy_mapping: Some(SRPolicyMapping { user_or_group: target_name.clone(), - user_type: rustfs_iam::store::UserType::Reg.to_u64(), + user_type: rustfs_iam::store::sr_wire_user_type(rustfs_iam::store::UserType::Reg, is_group), is_group, policy: updated_policies.join(","), updated_at: Some(updated_at), diff --git a/rustfs/src/admin/handlers/site_replication.rs b/rustfs/src/admin/handlers/site_replication.rs index 983c39d4d..10fd7f2fc 100644 --- a/rustfs/src/admin/handlers/site_replication.rs +++ b/rustfs/src/admin/handlers/site_replication.rs @@ -63,7 +63,7 @@ use rustfs_config::{ }; use rustfs_iam::error::is_err_no_such_service_account; use rustfs_iam::federation::OIDC_VIRTUAL_PARENT_CLAIM; -use rustfs_iam::store::{MappedPolicy, UserType}; +use rustfs_iam::store::{MappedPolicy, UserType, sr_wire_user_type, user_type_from_sr_wire}; use rustfs_iam::sys::{ NewServiceAccountOpts, SITE_REPLICATOR_SERVICE_ACCOUNT, UpdateServiceAccountOpts, get_claims_from_token_with_secret, }; @@ -4303,7 +4303,7 @@ fn local_idp_settings() -> IDPSettings { fn mapped_policy_to_sr_mapping(name: String, is_group: bool, user_type: UserType, mapping: MappedPolicy) -> SRPolicyMapping { SRPolicyMapping { user_or_group: name, - user_type: user_type.to_u64(), + user_type: sr_wire_user_type(user_type, is_group), is_group, policy: mapping.policies, updated_at: Some(mapping.update_at), @@ -7824,7 +7824,8 @@ async fn apply_iam_item(item: SRIAMItem) -> S3Result<()> { let Some(mapping) = item.policy_mapping else { return Err(s3_error!(InvalidRequest, "policyMapping is required")); }; - let user_type = UserType::from_u64(mapping.user_type).ok_or_else(|| s3_error!(InvalidRequest, "invalid userType"))?; + let user_type = + user_type_from_sr_wire(mapping.user_type).ok_or_else(|| s3_error!(InvalidRequest, "invalid userType"))?; iam_sys .policy_db_set(&mapping.user_or_group, user_type, mapping.is_group, &mapping.policy) .await @@ -11910,7 +11911,7 @@ mod tests { "alice".to_string(), SRPolicyMapping { user_or_group: "alice".to_string(), - user_type: UserType::Reg.to_u64(), + user_type: sr_wire_user_type(UserType::Reg, false), policy: "readwrite".to_string(), updated_at: Some(OffsetDateTime::UNIX_EPOCH), ..Default::default()