gsadmin/rustfs
1
0
Fork 0
mirror of https://github.com/rustfs/rustfs.git synced 2026-07-26 16:28:15 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: honor bucket-scoped ListBucket policies with s3:prefix (#2707)

Browse Source 
Co-authored-by: houseme <housemecn@gmail.com>
This commit is contained in:
安正超
2026-04-27 22:13:22 +08:00
committed by GitHub
parent a68fe1601f
commit 159ddd5bac
4 changed files with 293 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/iam/src/oidc.rs
+16 -3
View File
@@ -1599,7 +1599,9 @@ mod tests {
use std::time::{Duration, Instant};
// After the last completed response, exit if no new connection arrives within this window.
const IDLE_SHUTDOWN: Duration = Duration::from_millis(500);
// Keep the mock server alive long enough for slower CI/macOS test environments to finish
// discovery + JWKS requests without racing the shutdown timer.
const IDLE_SHUTDOWN: Duration = Duration::from_secs(1);
const ABSOLUTE_CAP: Duration = Duration::from_secs(5);
let listener = TcpListener::bind("127.0.0.1:0").unwrap();
@@ -1701,6 +1703,17 @@ mod tests {
err.contains(base) && err.contains(&format!("{base}/")) && err.contains("discovery failed for all issuer variants")
}
async fn validate_mocked_oidc_provider_config(config: &OidcProviderConfig) -> Result<OidcProviderValidationResult, String> {
let http_client = ReqwestHttpClient::new()?;
let state = OidcSys::discover_provider(config, &http_client).await?;
Ok(OidcProviderValidationResult {
issuer: state.metadata.issuer().to_string(),
authorization_endpoint: state.metadata.authorization_endpoint().to_string(),
token_endpoint: state.metadata.token_endpoint().map(ToString::to_string),
})
}
#[tokio::test]
async fn test_validate_oidc_provider_config_retries_with_issuer_candidates() {
// Discovery document must advertise the canonical issuer path. The first candidate has no
@@ -1709,7 +1722,7 @@ mod tests {
let config_url = format!("{base}/application/o/rustfs");
let config = build_mocked_oidc_provider_config("default", &config_url);
let result = validate_oidc_provider_config(&config).await;
let result = validate_mocked_oidc_provider_config(&config).await;
let validation_result = result.expect("OIDC provider validation should succeed");
assert_eq!(validation_result.issuer, format!("{base}/application/o/rustfs/"));
@@ -1722,7 +1735,7 @@ mod tests {
let config_url = format!("{base}/application/o/rustfs");
let config = build_mocked_oidc_provider_config("default", &config_url);
let err = validate_oidc_provider_config(&config)
let err = validate_mocked_oidc_provider_config(&config)
.await
.expect_err("OIDC provider validation should fail");
assert!(discovery_error_contains_all_variants(&err, &base));
crates/policy/src/policy/policy.rs
+179
View File
@@ -798,6 +798,139 @@ mod test {
Ok(())
}
#[tokio::test]
async fn test_list_bucket_prefix_condition_uses_bucket_resource() -> Result<()> {
let policy = Policy::parse_config(
br#"{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": ["s3:ListBucket"],
"Resource": ["arn:aws:s3:::polaris-test-bucket"],
"Condition": {
"StringLike": {
"s3:prefix": [
"polaris_test/snowflake_catalog/db1/schema/iceberg_table/*"
]
}
}
}
]
}"#,
)?;
let mut conditions = HashMap::new();
conditions.insert(
"prefix".to_string(),
vec!["polaris_test/snowflake_catalog/db1/schema/iceberg_table/metadata/".to_string()],
);
let claims = HashMap::new();
let args = Args {
account: "polaris-session",
groups: &None,
action: Action::S3Action(crate::policy::action::S3Action::ListBucketAction),
bucket: "polaris-test-bucket",
conditions: &conditions,
is_owner: false,
object: "polaris_test/snowflake_catalog/db1/schema/iceberg_table/metadata/",
claims: &claims,
deny_only: false,
};
assert!(
policy.is_allowed(&args).await,
"ListBucket should match the bucket resource and apply the prefix through the condition, not by converting the prefix into an object resource"
);
Ok(())
}
#[tokio::test]
async fn test_list_bucket_versions_prefix_condition_uses_bucket_resource() -> Result<()> {
let policy = Policy::parse_config(
br#"{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": ["s3:ListBucketVersions"],
"Resource": ["arn:aws:s3:::polaris-test-bucket"],
"Condition": {
"StringLike": {
"s3:prefix": [
"polaris_test/snowflake_catalog/db1/schema/iceberg_table/*"
]
}
}
}
]
}"#,
)?;
let mut conditions = HashMap::new();
conditions.insert(
"prefix".to_string(),
vec!["polaris_test/snowflake_catalog/db1/schema/iceberg_table/metadata/".to_string()],
);
let claims = HashMap::new();
let args = Args {
account: "polaris-session",
groups: &None,
action: Action::S3Action(crate::policy::action::S3Action::ListBucketVersionsAction),
bucket: "polaris-test-bucket",
conditions: &conditions,
is_owner: false,
object: "polaris_test/snowflake_catalog/db1/schema/iceberg_table/metadata/",
claims: &claims,
deny_only: false,
};
assert!(
policy.is_allowed(&args).await,
"ListBucketVersions should match the bucket resource and apply the prefix through the condition"
);
Ok(())
}
#[tokio::test]
async fn test_list_bucket_gateway_prefix_uses_object_resource_when_condition_missing() -> Result<()> {
let policy = Policy::parse_config(
br#"{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": ["s3:ListBucket"],
"Resource": ["arn:aws:s3:::polaris-test-bucket/home/alice/*"]
}
]
}"#,
)?;
let conditions = HashMap::new();
let claims = HashMap::new();
let args = Args {
account: "polaris-session",
groups: &None,
action: Action::S3Action(crate::policy::action::S3Action::ListBucketAction),
bucket: "polaris-test-bucket",
conditions: &conditions,
is_owner: false,
object: "home/alice/projects/",
claims: &claims,
deny_only: false,
};
assert!(
policy.is_allowed(&args).await,
"Gateway ListBucket auth without an s3:prefix condition should continue matching prefix-scoped resources via args.object"
);
Ok(())
}
#[tokio::test]
async fn test_aws_username_policy_variable() -> Result<()> {
let data = r#"
@@ -1408,6 +1541,52 @@ mod test {
assert_eq!(arr[0].as_str().unwrap(), "s3:ListBucket");
}
#[tokio::test]
async fn test_bucket_policy_list_bucket_prefix_condition_uses_bucket_resource() -> Result<()> {
let bucket_policy: BucketPolicy = serde_json::from_str(
r#"{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Principal": {"AWS": "*"},
"Action": ["s3:ListBucket"],
"Resource": ["arn:aws:s3:::polaris-test-bucket"],
"Condition": {
"StringLike": {
"s3:prefix": [
"polaris_test/snowflake_catalog/db1/schema/iceberg_table/*"
]
}
}
}
]
}"#,
)?;
let mut conditions = HashMap::new();
conditions.insert(
"prefix".to_string(),
vec!["polaris_test/snowflake_catalog/db1/schema/iceberg_table/metadata/".to_string()],
);
let args = BucketPolicyArgs {
account: "polaris-session",
groups: &None,
action: Action::S3Action(crate::policy::action::S3Action::ListBucketAction),
bucket: "polaris-test-bucket",
conditions: &conditions,
is_owner: false,
object: "polaris_test/snowflake_catalog/db1/schema/iceberg_table/metadata/",
};
assert!(
bucket_policy.is_allowed(&args).await,
"Bucket policy ListBucket should match the bucket resource and apply the prefix through the condition"
);
Ok(())
}
#[tokio::test]
async fn test_bucket_policy_deny_with_string_not_equals() -> Result<()> {
let data = r#"
crates/policy/src/policy/statement.rs
+31 -21
View File
@@ -14,7 +14,7 @@
use super::{
ActionSet, Args, BucketPolicyArgs, Effect, Error as IamError, Functions, ID, Principal, ResourceSet, Validator,
action::Action,
action::{Action, S3Action},
variables::{VariableContext, VariableResolver},
};
use crate::error::{Error, Result};
@@ -56,6 +56,34 @@ pub(crate) fn variable_resolver_for_policy_args(args: &Args<'_>) -> VariableReso
VariableResolver::new(context)
}
const LIST_BUCKET_PREFIX_CONDITION_KEY: &str = "prefix";
fn build_resource(
action: &Action,
bucket: &str,
object: &str,
conditions: &std::collections::HashMap<String, Vec<String>>,
) -> String {
let bucket_resource_only = matches!(
action,
Action::S3Action(
S3Action::ListBucketAction | S3Action::ListBucketVersionsAction | S3Action::ListBucketMultipartUploadsAction
)
) && conditions.contains_key(LIST_BUCKET_PREFIX_CONDITION_KEY);
let mut resource = String::from(bucket);
if bucket_resource_only || object.is_empty() {
resource.push('/');
return resource;
}
if !object.starts_with('/') {
resource.push('/');
}
resource.push_str(object);
resource
}
impl Statement {
fn is_kms(&self) -> bool {
for act in self.actions.iter() {
@@ -94,16 +122,7 @@ impl Statement {
return false;
}
let mut resource = String::from(args.bucket);
if !args.object.is_empty() {
if !args.object.starts_with('/') {
resource.push('/');
}
resource.push_str(args.object);
} else {
resource.push('/');
}
let resource = build_resource(&args.action, args.bucket, args.object, args.conditions);
if self.is_kms() && (resource == "/" || self.resources.is_empty()) {
return true;
@@ -230,16 +249,7 @@ impl BPStatement {
return false;
}
let mut resource = String::from(args.bucket);
if !args.object.is_empty() {
if !args.object.starts_with('/') {
resource.push('/');
}
resource.push_str(args.object);
} else {
resource.push('/');
}
let resource = build_resource(&args.action, args.bucket, args.object, args.conditions);
if !self.resources.is_empty() && !self.resources.is_match(&resource, args.conditions).await {
return false;
rustfs/src/storage/access.rs
+67 -1
View File
@@ -35,7 +35,7 @@ use s3s::access::{S3Access, S3AccessContext};
use s3s::{S3Error, S3ErrorCode, S3Request, S3Result, dto::*, s3_error};
use std::collections::HashMap;
use std::sync::OnceLock;
use url::Url;
use url::{Url, form_urlencoded};
#[derive(Default, Clone, Debug)]
pub(crate) struct ReqInfo {
@@ -220,6 +220,30 @@ fn action_tag_metric_label(action: &Action) -> &'static str {
}
}
fn merge_list_bucket_query_conditions(action: Action, query: Option<&str>, conditions: &mut HashMap<String, Vec<String>>) {
if !matches!(
action,
Action::S3Action(
S3Action::ListBucketAction | S3Action::ListBucketVersionsAction | S3Action::ListBucketMultipartUploadsAction
)
) {
return;
}
let Some(query) = query else {
return;
};
for (key, value) in form_urlencoded::parse(query.as_bytes()) {
match key.as_ref() {
"prefix" | "delimiter" | "max-keys" => {
conditions.entry(key.into_owned()).or_default().push(value.into_owned());
}
_ => {}
}
}
}
fn auth_fs() -> &'static FS {
static AUTH_FS: OnceLock<FS> = OnceLock::new();
AUTH_FS.get_or_init(FS::new)
@@ -312,6 +336,7 @@ pub async fn authorize_request<T>(req: &mut S3Request<T>, action: Action) -> S3R
let claims = cred.claims.as_ref().unwrap_or(&default_claims);
let mut conditions =
get_condition_values_with_query(&req.headers, cred, version_id.as_deref(), None, remote_addr, req.uri.query());
merge_list_bucket_query_conditions(action, req.uri.query(), &mut conditions);
let action_args = Args {
account: &cred.access_key,
@@ -526,6 +551,7 @@ pub async fn authorize_request<T>(req: &mut S3Request<T>, action: Action) -> S3R
remote_addr,
req.uri.query(),
);
merge_list_bucket_query_conditions(action, req.uri.query(), &mut conditions);
let no_groups: Option<Vec<String>> = None;
let bucket_tag_hint = if !bucket.is_empty() && !object.is_empty() {
@@ -1999,6 +2025,46 @@ mod tests {
/// Object tag conditions must use keys like ExistingObjectTag/<tag-key> so that
/// bucket policy conditions (e.g. s3:ExistingObjectTag/security) are evaluated correctly.
#[test]
fn test_merge_list_bucket_query_conditions_extracts_supported_keys() {
let mut conditions = HashMap::new();
merge_list_bucket_query_conditions(
Action::S3Action(S3Action::ListBucketAction),
Some("prefix=photos%2F2024%2F&delimiter=%2F&max-keys=10&encoding-type=url"),
&mut conditions,
);
assert_eq!(conditions.get("prefix"), Some(&vec!["photos/2024/".to_string()]));
assert_eq!(conditions.get("delimiter"), Some(&vec!["/".to_string()]));
assert_eq!(conditions.get("max-keys"), Some(&vec!["10".to_string()]));
assert!(!conditions.contains_key("encoding-type"));
}
#[test]
fn test_merge_list_bucket_query_conditions_preserves_empty_prefix_signal() {
let mut conditions = HashMap::new();
merge_list_bucket_query_conditions(
Action::S3Action(S3Action::ListBucketVersionsAction),
Some("prefix=&delimiter=%2F"),
&mut conditions,
);
assert_eq!(conditions.get("prefix"), Some(&vec![String::new()]));
assert_eq!(conditions.get("delimiter"), Some(&vec!["/".to_string()]));
}
#[test]
fn test_merge_list_bucket_query_conditions_ignores_non_list_actions() {
let mut conditions = HashMap::new();
merge_list_bucket_query_conditions(
Action::S3Action(S3Action::GetObjectAction),
Some("prefix=photos%2F2024%2F&delimiter=%2F&max-keys=10"),
&mut conditions,
);
assert!(conditions.is_empty());
}
#[test]
fn test_object_tag_conditions_key_format() {
let mut tags = HashMap::new();