From 3366bd24648b166a10a549f247363bd8865dc877 Mon Sep 17 00:00:00 2001 From: GatewayJ <835269233@qq.com> Date: Sun, 29 Mar 2026 19:18:16 +0800 Subject: [PATCH] feat(iam,admin): prepared IAM auth, ExistingObjectTag, admin permission checks (#2315) Signed-off-by: GatewayJ <835269233@qq.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: GatewayJ <8352692332qq.com> --- crates/e2e_test/src/common.rs | 65 +- .../src/existing_object_tag_policy_test.rs | 364 ++++++++ crates/e2e_test/src/lib.rs | 4 + .../src/policy/policy_variables_test.rs | 6 +- crates/iam/src/sys.rs | 781 +++++++++++++---- crates/policy/src/policy/policy.rs | 336 +++++++- crates/policy/src/policy/statement.rs | 170 ++-- rustfs/src/admin/auth.rs | 3 - rustfs/src/storage/access.rs | 786 +++++++++++++----- 9 files changed, 2077 insertions(+), 438 deletions(-) create mode 100644 crates/e2e_test/src/existing_object_tag_policy_test.rs diff --git a/crates/e2e_test/src/common.rs b/crates/e2e_test/src/common.rs index c6a268335..67aef5f97 100644 --- a/crates/e2e_test/src/common.rs +++ b/crates/e2e_test/src/common.rs @@ -463,18 +463,18 @@ impl Drop for RustFSTestEnvironment { } } -/// Utility function to execute awscurl commands -pub async fn execute_awscurl( +async fn execute_awscurl_with_service( url: &str, method: &str, body: Option<&str>, access_key: &str, secret_key: &str, + service: &str, ) -> Result> { let mut args = vec![ "--fail-with-body", "--service", - "s3", + service, "--region", "us-east-1", "--access_key", @@ -490,7 +490,64 @@ pub async fn execute_awscurl( args.extend(&["-d", body_content]); } - info!("Executing awscurl: {} {}", method, url); + info!("Executing awscurl: {} {} (service={})", method, url, service); + let awscurl_path = awscurl_binary_path(); + let output = Command::new(&awscurl_path).args(&args).output()?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + let stdout = String::from_utf8_lossy(&output.stdout); + return Err(format!("awscurl failed: stderr='{stderr}', stdout='{stdout}'").into()); + } + + let response = String::from_utf8_lossy(&output.stdout).to_string(); + Ok(response) +} + +/// Utility function to execute awscurl commands (SigV4 service `s3` for admin APIs). +pub async fn execute_awscurl( + url: &str, + method: &str, + body: Option<&str>, + access_key: &str, + secret_key: &str, +) -> Result> { + execute_awscurl_with_service(url, method, body, access_key, secret_key, "s3").await +} + +/// `POST` with SigV4 `--service sts` and explicit `Content-Type: application/x-www-form-urlencoded`. +/// +/// RustFS `AssumeRole` is handled on `POST /` by the admin router; `is_match` requires this +/// content type so s3s routes to the custom handler instead of `Unknown operation`. +pub async fn awscurl_post_sts_form_urlencoded( + url: &str, + body: &str, + access_key: &str, + secret_key: &str, +) -> Result> { + let args = vec![ + "--fail-with-body", + "--service", + "sts", + "--region", + "us-east-1", + "--access_key", + access_key, + "--secret_key", + secret_key, + "-H", + "Content-Type: application/x-www-form-urlencoded", + "-X", + "POST", + url, + "-d", + body, + ]; + + info!( + "Executing awscurl: POST {} (service=sts, Content-Type=application/x-www-form-urlencoded)", + url + ); let awscurl_path = awscurl_binary_path(); let output = Command::new(&awscurl_path).args(&args).output()?; diff --git a/crates/e2e_test/src/existing_object_tag_policy_test.rs b/crates/e2e_test/src/existing_object_tag_policy_test.rs new file mode 100644 index 000000000..6e82a6582 --- /dev/null +++ b/crates/e2e_test/src/existing_object_tag_policy_test.rs @@ -0,0 +1,364 @@ +// Copyright 2024 RustFS Team +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! E2E: `s3:ExistingObjectTag` with **IAM identity policy**, **bucket policy**, and **STS AssumeRole +//! session policy** (`Policy` parameter) via `awscurl --service sts` with explicit +//! `Content-Type: application/x-www-form-urlencoded` on `POST /`. + +use crate::common::{ + RustFSTestEnvironment, awscurl_available, awscurl_delete, awscurl_post_sts_form_urlencoded, awscurl_put, init_logging, +}; +use aws_sdk_s3::config::{Credentials, Region}; +use aws_sdk_s3::primitives::ByteStream; +use aws_sdk_s3::types::{Tag, Tagging}; +use aws_sdk_s3::{Client, Config}; +use serial_test::serial; +use tracing::info; +use uuid::Uuid; + +fn user_client(env: &RustFSTestEnvironment, access_key: &str, secret_key: &str) -> Client { + let credentials = Credentials::new(access_key, secret_key, None, None, "e2e-existing-tag"); + let config = Config::builder() + .credentials_provider(credentials) + .region(Region::new("us-east-1")) + .endpoint_url(&env.url) + .force_path_style(true) + .behavior_version_latest() + .build(); + Client::from_conf(config) +} + +fn sts_session_client(env: &RustFSTestEnvironment, access_key: &str, secret_key: &str, session_token: &str) -> Client { + let credentials = Credentials::new(access_key, secret_key, Some(session_token.into()), None, "e2e-sts-session"); + let config = Config::builder() + .credentials_provider(credentials) + .region(Region::new("us-east-1")) + .endpoint_url(&env.url) + .force_path_style(true) + .behavior_version_latest() + .build(); + Client::from_conf(config) +} + +fn extract_xml_tag(xml: &str, tag: &str) -> Option { + let open = format!("<{tag}>"); + let close = format!(""); + let start = xml.find(&open)? + open.len(); + let end = xml[start..].find(&close)? + start; + Some(xml[start..end].to_string()) +} + +fn parse_assume_role_credentials(xml: &str) -> Result<(String, String, String), Box> { + let ak = extract_xml_tag(xml, "AccessKeyId").ok_or("missing AccessKeyId in AssumeRole response")?; + let sk = extract_xml_tag(xml, "SecretAccessKey").ok_or("missing SecretAccessKey in AssumeRole response")?; + let token = extract_xml_tag(xml, "SessionToken").ok_or("missing SessionToken in AssumeRole response")?; + Ok((ak, sk, token)) +} + +async fn assume_role_with_session_policy( + env: &RustFSTestEnvironment, + parent_ak: &str, + parent_sk: &str, + session_policy_json: &str, +) -> Result<(String, String, String), Box> { + let policy_enc = urlencoding::encode(session_policy_json); + let body = format!("Action=AssumeRole&Version=2011-06-15&DurationSeconds=3600&Policy={}", policy_enc); + let url = format!("{}/", env.url.trim_end_matches('/')); + let xml = awscurl_post_sts_form_urlencoded(&url, &body, parent_ak, parent_sk).await?; + parse_assume_role_credentials(&xml) +} + +async fn admin_create_user( + env: &RustFSTestEnvironment, + username: &str, + password: &str, +) -> Result<(), Box> { + let body = serde_json::json!({ "secretKey": password, "status": "enabled" }).to_string(); + let url = format!("{}/rustfs/admin/v3/add-user?accessKey={}", env.url, username); + awscurl_put(&url, &body, &env.access_key, &env.secret_key).await?; + Ok(()) +} + +async fn admin_add_canned_policy( + env: &RustFSTestEnvironment, + policy_name: &str, + policy_json: &str, +) -> Result<(), Box> { + let url = format!("{}/rustfs/admin/v3/add-canned-policy?name={}", env.url, policy_name); + awscurl_put(&url, policy_json, &env.access_key, &env.secret_key).await?; + Ok(()) +} + +async fn admin_attach_policy_to_user( + env: &RustFSTestEnvironment, + policy_name: &str, + username: &str, +) -> Result<(), Box> { + let url = format!( + "{}/rustfs/admin/v3/set-user-or-group-policy?policyName={}&userOrGroup={}&isGroup=false", + env.url, policy_name, username + ); + awscurl_put(&url, "", &env.access_key, &env.secret_key).await?; + Ok(()) +} + +async fn admin_remove_user(env: &RustFSTestEnvironment, username: &str) { + let url = format!("{}/rustfs/admin/v3/remove-user?accessKey={}", env.url, username); + let _ = awscurl_delete(&url, &env.access_key, &env.secret_key).await; +} + +async fn admin_remove_policy(env: &RustFSTestEnvironment, policy_name: &str) { + let url = format!("{}/rustfs/admin/v3/remove-canned-policy?name={}", env.url, policy_name); + let _ = awscurl_delete(&url, &env.access_key, &env.secret_key).await; +} + +async fn put_object_with_tagging_str( + client: &Client, + bucket: &str, + key: &str, + data: &[u8], + tagging: &str, +) -> Result<(), Box> { + client + .put_object() + .bucket(bucket) + .key(key) + .body(ByteStream::from(data.to_vec())) + .tagging(tagging) + .send() + .await?; + Ok(()) +} + +async fn put_object_tag_kv( + client: &Client, + bucket: &str, + key: &str, + tag_key: &str, + tag_value: &str, +) -> Result<(), Box> { + let tag = Tag::builder() + .key(tag_key) + .value(tag_value) + .build() + .map_err(|e| format!("Tag build: {e}"))?; + let tagging = Tagging::builder() + .tag_set(tag) + .build() + .map_err(|e| format!("Tagging build: {e}"))?; + client + .put_object_tagging() + .bucket(bucket) + .key(key) + .tagging(tagging) + .send() + .await?; + Ok(()) +} + +async fn cleanup_bucket_and_object(admin: &Client, bucket: &str, key: &str) { + let _ = admin.delete_object().bucket(bucket).key(key).send().await; + let _ = admin.delete_bucket().bucket(bucket).send().await; +} + +/// IAM identity policy: GetObject allowed only when `s3:ExistingObjectTag/security` == `public`. +#[tokio::test] +#[serial] +async fn test_e2e_iam_policy_existing_object_tag_get_object() -> Result<(), Box> { + init_logging(); + if !awscurl_available() { + info!("Skipping test_e2e_iam_policy_existing_object_tag_get_object: awscurl not available"); + return Ok(()); + } + + let suffix = Uuid::new_v4(); + let user = format!("e2eiamtag-{suffix}"); + let user_secret = "longSecretKeyForTest123!"; + let policy_name = format!("e2e-iam-tag-pol-{suffix}"); + let bucket = format!("e2e-iam-tag-bkt-{suffix}"); + let key = "tagged-object.txt"; + + let mut env = RustFSTestEnvironment::new().await?; + env.start_rustfs_server(vec![]).await?; + + let admin = env.create_s3_client(); + admin_create_user(&env, &user, user_secret).await?; + + let policy_doc = serde_json::json!({ + "Version": "2012-10-17", + "Statement": [{ + "Effect": "Allow", + "Action": ["s3:GetObject"], + "Resource": [format!("arn:aws:s3:::{}/*", bucket)], + "Condition": { "StringEquals": { "s3:ExistingObjectTag/security": "public" } } + }] + }) + .to_string(); + + admin_add_canned_policy(&env, &policy_name, &policy_doc).await?; + admin_attach_policy_to_user(&env, &policy_name, &user).await?; + + admin.create_bucket().bucket(&bucket).send().await?; + put_object_with_tagging_str(&admin, &bucket, key, b"hello-iam-tag", "security=public").await?; + + let uclient = user_client(&env, &user, user_secret); + let out = uclient.get_object().bucket(&bucket).key(key).send().await?; + let _ = out.body.collect().await?; + + put_object_tag_kv(&admin, &bucket, key, "security", "private").await?; + let denied = uclient.get_object().bucket(&bucket).key(key).send().await; + assert!( + denied.is_err(), + "GetObject must be denied when ExistingObjectTag no longer matches IAM policy" + ); + + cleanup_bucket_and_object(&admin, &bucket, key).await; + admin_remove_user(&env, &user).await; + admin_remove_policy(&env, &policy_name).await; + + info!("test_e2e_iam_policy_existing_object_tag_get_object passed"); + Ok(()) +} + +/// Bucket policy: same `ExistingObjectTag` condition; user has no canned IAM policy attached. +#[tokio::test] +#[serial] +async fn test_e2e_bucket_policy_existing_object_tag_get_object() -> Result<(), Box> { + init_logging(); + if !awscurl_available() { + info!("Skipping test_e2e_bucket_policy_existing_object_tag_get_object: awscurl not available"); + return Ok(()); + } + + let suffix = Uuid::new_v4(); + let user = format!("e2ebptag-{suffix}"); + let user_secret = "longSecretKeyForTest456!"; + let bucket = format!("e2e-bp-tag-bkt-{suffix}"); + let key = "obj.txt"; + + let mut env = RustFSTestEnvironment::new().await?; + env.start_rustfs_server(vec![]).await?; + + let admin = env.create_s3_client(); + admin_create_user(&env, &user, user_secret).await?; + + admin.create_bucket().bucket(&bucket).send().await?; + + let deny_before = user_client(&env, &user, user_secret) + .get_object() + .bucket(&bucket) + .key(key) + .send() + .await; + assert!(deny_before.is_err(), "without bucket policy, user must be denied"); + + let bp = serde_json::json!({ + "Version": "2012-10-17", + "Statement": [{ + "Effect": "Allow", + "Principal": { "AWS": [user.clone()] }, + "Action": ["s3:GetObject"], + "Resource": [format!("arn:aws:s3:::{}/*", bucket)], + "Condition": { "StringEquals": { "s3:ExistingObjectTag/security": "public" } } + }] + }) + .to_string(); + + admin.put_bucket_policy().bucket(&bucket).policy(&bp).send().await?; + put_object_with_tagging_str(&admin, &bucket, key, b"data", "security=public").await?; + + let uclient = user_client(&env, &user, user_secret); + let ok = uclient.get_object().bucket(&bucket).key(key).send().await?; + let _ = ok.body.collect().await?; + + put_object_tag_kv(&admin, &bucket, key, "security", "private").await?; + let denied = uclient.get_object().bucket(&bucket).key(key).send().await; + assert!(denied.is_err(), "GetObject must fail when tag no longer satisfies bucket policy"); + + cleanup_bucket_and_object(&admin, &bucket, key).await; + admin_remove_user(&env, &user).await; + + info!("test_e2e_bucket_policy_existing_object_tag_get_object passed"); + Ok(()) +} + +/// STS `AssumeRole` with inline `Policy` (session policy): GetObject only when `ExistingObjectTag/security` is `public`. +#[tokio::test] +#[serial] +async fn test_e2e_sts_assume_role_session_policy_existing_object_tag() -> Result<(), Box> { + init_logging(); + if !awscurl_available() { + info!("Skipping test_e2e_sts_assume_role_session_policy_existing_object_tag: awscurl not available"); + return Ok(()); + } + + let suffix = Uuid::new_v4(); + let parent = format!("e2e-sts-par-{suffix}"); + let parent_secret = "longSecretKeyForParentSts99!"; + let policy_readwrite = format!("e2e-sts-rw-{suffix}"); + let bucket = format!("e2e-sts-tag-bkt-{suffix}"); + let key = "sts-obj.txt"; + + let mut env = RustFSTestEnvironment::new().await?; + env.start_rustfs_server(vec![]).await?; + + let admin = env.create_s3_client(); + admin_create_user(&env, &parent, parent_secret).await?; + + let rw = serde_json::to_string(&serde_json::json!({ + "Version": "2012-10-17", + "Statement": [{ + "Effect": "Allow", + "Action": ["s3:*"], + "Resource": ["arn:aws:s3:::*"] + }] + }))?; + admin_add_canned_policy(&env, &policy_readwrite, &rw).await?; + admin_attach_policy_to_user(&env, &policy_readwrite, &parent).await?; + + let parent_client = user_client(&env, &parent, parent_secret); + parent_client.create_bucket().bucket(&bucket).send().await?; + put_object_with_tagging_str(&parent_client, &bucket, key, b"sts-e2e-data", "security=public").await?; + + let session_policy = serde_json::json!({ + "Version": "2012-10-17", + "Statement": [{ + "Effect": "Allow", + "Action": ["s3:GetObject"], + "Resource": [format!("arn:aws:s3:::{}/*", bucket)], + "Condition": { "StringEquals": { "s3:ExistingObjectTag/security": "public" } } + }] + }) + .to_string(); + + let (ak, sk, token) = assume_role_with_session_policy(&env, &parent, parent_secret, &session_policy).await?; + + let session_client = sts_session_client(&env, &ak, &sk, &token); + let ok = session_client.get_object().bucket(&bucket).key(key).send().await?; + let _ = ok.body.collect().await?; + + put_object_tag_kv(&parent_client, &bucket, key, "security", "private").await?; + let denied = session_client.get_object().bucket(&bucket).key(key).send().await; + assert!( + denied.is_err(), + "session policy must deny GetObject when ExistingObjectTag no longer matches" + ); + + cleanup_bucket_and_object(&admin, &bucket, key).await; + admin_remove_user(&env, &parent).await; + admin_remove_policy(&env, &policy_readwrite).await; + + info!("test_e2e_sts_assume_role_session_policy_existing_object_tag passed"); + Ok(()) +} diff --git a/crates/e2e_test/src/lib.rs b/crates/e2e_test/src/lib.rs index 97dd2461d..6ac513bcc 100644 --- a/crates/e2e_test/src/lib.rs +++ b/crates/e2e_test/src/lib.rs @@ -40,6 +40,10 @@ mod quota_test; #[cfg(test)] mod bucket_policy_check_test; +/// IAM / bucket / STS session policy with `s3:ExistingObjectTag` conditions (E2E). +#[cfg(test)] +mod existing_object_tag_policy_test; + // Regression tests for Issue #2036: anonymous access with PublicAccessBlock #[cfg(test)] mod anonymous_access_test; diff --git a/crates/e2e_test/src/policy/policy_variables_test.rs b/crates/e2e_test/src/policy/policy_variables_test.rs index 187f355c7..52c6492d0 100644 --- a/crates/e2e_test/src/policy/policy_variables_test.rs +++ b/crates/e2e_test/src/policy/policy_variables_test.rs @@ -14,7 +14,7 @@ //! Tests for AWS IAM policy variables with single-value, multi-value, and nested scenarios -use crate::common::{awscurl_put, init_logging}; +use crate::common::{awscurl_delete, awscurl_put, init_logging}; use crate::policy::test_env::PolicyTestEnvironment; use aws_sdk_s3::primitives::ByteStream; use serial_test::serial; @@ -113,11 +113,11 @@ async fn cleanup_user_and_policy(env: &PolicyTestEnvironment, username: &str, po // Remove user let remove_user_url = format!("{}/rustfs/admin/v3/remove-user?accessKey={}", env.url, username); - let _ = awscurl_put(&remove_user_url, "", &env.access_key, &env.secret_key).await; + let _ = awscurl_delete(&remove_user_url, &env.access_key, &env.secret_key).await; // Remove policy let remove_policy_url = format!("{}/rustfs/admin/v3/remove-canned-policy?name={}", env.url, policy_name); - let _ = awscurl_put(&remove_policy_url, "", &env.access_key, &env.secret_key).await; + let _ = awscurl_delete(&remove_policy_url, &env.access_key, &env.secret_key).await; } /// Test AWS policy variables with single-value scenarios diff --git a/crates/iam/src/sys.rs b/crates/iam/src/sys.rs index dac697633..04e869fb0 100644 --- a/crates/iam/src/sys.rs +++ b/crates/iam/src/sys.rs @@ -35,7 +35,7 @@ use rustfs_policy::auth::{ }; use rustfs_policy::policy::Args; use rustfs_policy::policy::opa; -use rustfs_policy::policy::{Policy, PolicyDoc, iam_policy_claim_name_sa}; +use rustfs_policy::policy::{Policy, PolicyDoc, iam_policy_claim_name_sa, policy_needs_existing_object_tag_for_args}; use serde_json::Value; use serde_json::json; use std::collections::HashMap; @@ -65,6 +65,78 @@ pub struct IamSys { roles_map: HashMap, } +#[derive(Clone)] +enum PreparedSessionPolicy { + None, + DenyAll, + Policy(Policy), +} + +#[derive(Clone, Copy)] +enum PreparedServicePolicyMode { + Inherited, + SessionBound, +} + +#[derive(Clone)] +enum PreparedIamMode { + Opa, + Owner, + Deny, + Regular { + combined_policy: Policy, + }, + Sts { + is_owner: bool, + combined_policy: Policy, + session_policy: PreparedSessionPolicy, + }, + ServiceAccount { + is_owner: bool, + parent_user: String, + combined_policy: Policy, + mode: PreparedServicePolicyMode, + session_policy: PreparedSessionPolicy, + }, +} + +#[derive(Clone)] +pub struct PreparedIamAuth { + pub needs_existing_object_tag: bool, + mode: PreparedIamMode, +} + +impl PreparedIamAuth { + /// Evaluate whether the already-prepared IAM context needs ExistingObjectTag + /// conditions for the provided request args. + pub async fn needs_existing_object_tag_for_args(&self, args: &Args<'_>) -> bool { + match &self.mode { + PreparedIamMode::Opa | PreparedIamMode::Owner | PreparedIamMode::Deny => false, + PreparedIamMode::Regular { combined_policy } => { + policy_needs_existing_object_tag_for_args(combined_policy, args).await + } + PreparedIamMode::Sts { + combined_policy, + session_policy, + .. + } => { + policy_needs_existing_object_tag_for_args(combined_policy, args).await + || prepared_session_policy_needs_existing_object_tag_for_args(session_policy, args).await + } + PreparedIamMode::ServiceAccount { + combined_policy, + mode, + session_policy, + .. + } => { + policy_needs_existing_object_tag_for_args(combined_policy, args).await + || matches!(mode, PreparedServicePolicyMode::SessionBound) + && prepared_session_policy_needs_existing_object_tag_for_args(session_policy, args).await + } + } + } +} + impl IamSys { /// Create a new IamSys instance with the given IamCache store /// @@ -740,14 +812,150 @@ impl IamSys { self.store.policy_db_get(name, groups).await } - pub async fn is_allowed_sts(&self, args: &Args<'_>, parent_user: &str) -> bool { + fn is_safe_claim_policy_name(policy: &str) -> bool { + !policy.is_empty() && policy.chars().all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-') + } + + /// Compatibility wrapper for service-account authorization entry points. + /// The canonical evaluation path is `prepare_service_account_auth + eval_prepared`. + pub async fn is_allowed_service_account(&self, args: &Args<'_>, parent_user: &str) -> bool { + let prepared = self.prepare_service_account_auth(args, parent_user).await; + self.eval_prepared(&prepared, args).await + } + + pub async fn get_combined_policy(&self, policies: &[String]) -> Policy { + self.store.merge_policies(&policies.join(",")).await.1 + } + + /// Prepare IAM authorization context once so callers can: + /// 1) know whether policy evaluation may need `s3:ExistingObjectTag`, and + /// 2) evaluate with final conditions without re-merging identity policies. + pub async fn prepare_auth(&self, args: &Args<'_>) -> PreparedIamAuth { + if args.is_owner { + return PreparedIamAuth { + needs_existing_object_tag: false, + mode: PreparedIamMode::Owner, + }; + } + + if Self::get_policy_plugin_client().await.is_some() { + return PreparedIamAuth { + needs_existing_object_tag: false, + mode: PreparedIamMode::Opa, + }; + } + + let Ok((is_temp, parent_user)) = self.is_temp_user(args.account).await else { + return PreparedIamAuth { + needs_existing_object_tag: false, + mode: PreparedIamMode::Deny, + }; + }; + if is_temp { + return self.prepare_sts_auth(args, &parent_user).await; + } + + let Ok((is_svc, parent_user)) = self.is_service_account(args.account).await else { + return PreparedIamAuth { + needs_existing_object_tag: false, + mode: PreparedIamMode::Deny, + }; + }; + if is_svc { + return self.prepare_service_account_auth(args, &parent_user).await; + } + + self.prepare_regular_auth(args).await + } + + pub async fn eval_prepared(&self, prepared: &PreparedIamAuth, args: &Args<'_>) -> bool { + match &prepared.mode { + PreparedIamMode::Opa => { + let Some(opa_enable) = Self::get_policy_plugin_client().await else { + tracing::warn!("eval_prepared: OPA mode requested but plugin is unavailable"); + return false; + }; + opa_enable.is_allowed(args).await + } + PreparedIamMode::Owner => true, + PreparedIamMode::Deny => false, + PreparedIamMode::Regular { combined_policy } => combined_policy.is_allowed(args).await, + PreparedIamMode::Sts { + is_owner, + combined_policy, + session_policy, + } => { + let session_ok = evaluate_prepared_session_policy(session_policy, args).await; + if let Some(ok) = session_ok { + return ok && (*is_owner || combined_policy.is_allowed(args).await); + } + *is_owner || combined_policy.is_allowed(args).await + } + PreparedIamMode::ServiceAccount { + is_owner, + parent_user, + combined_policy, + mode, + session_policy, + } => { + let mut parent_args = args.clone(); + parent_args.account = parent_user; + + let parent_allowed = *is_owner || combined_policy.is_allowed(&parent_args).await; + match mode { + PreparedServicePolicyMode::Inherited => parent_allowed, + PreparedServicePolicyMode::SessionBound => { + let session_ok = evaluate_prepared_session_policy(session_policy, args).await; + if let Some(ok) = session_ok { + return ok && parent_allowed; + } + parent_allowed + } + } + } + } + } + + async fn prepare_regular_auth(&self, args: &Args<'_>) -> PreparedIamAuth { + let Ok(policies) = self.policy_db_get(args.account, args.groups).await else { + return PreparedIamAuth { + needs_existing_object_tag: false, + mode: PreparedIamMode::Deny, + }; + }; + + if policies.is_empty() { + return PreparedIamAuth { + needs_existing_object_tag: false, + mode: PreparedIamMode::Deny, + }; + } + + let combined_policy = self.get_combined_policy(&policies).await; + PreparedIamAuth { + needs_existing_object_tag: policy_needs_existing_object_tag_for_args(&combined_policy, args).await, + mode: PreparedIamMode::Regular { combined_policy }, + } + } + + pub(crate) async fn prepare_sts_auth(&self, args: &Args<'_>, parent_user: &str) -> PreparedIamAuth { let is_owner = matches!(get_global_action_cred(), Some(cred) if cred.access_key == parent_user); let role_arn = args.get_role_arn(); let (effective_groups, groups_source, policies) = if is_owner { (None, "owner", Vec::new()) } else if let Some(arn_str) = role_arn { - let Ok(arn) = ARN::parse(arn_str) else { return false }; + let Ok(arn) = ARN::parse(arn_str) else { + tracing::warn!( + parent_user = %parent_user, + role_arn = %arn_str, + "prepare_sts_auth: invalid role ARN in STS claims" + ); + return PreparedIamAuth { + needs_existing_object_tag: false, + mode: PreparedIamMode::Deny, + }; + }; let p = MappedPolicy::new(self.roles_map.get(&arn).map_or_else(String::default, |v| v.clone()).as_str()).to_slice(); (None, "role", p) } else { @@ -758,7 +966,7 @@ impl IamSys { None => { tracing::warn!( parent_user = %parent_user, - "is_allowed_sts: groups fallback failed — parent user not found; policy evaluation will use no groups" + "prepare_sts_auth: groups fallback failed, parent user not found" ); (None, "parent_user_credentials") } @@ -768,9 +976,10 @@ impl IamSys { (effective_groups, groups_source, p) }; + let mut combined_policy = Policy::default(); + if !is_owner && policies.is_empty() { // For OIDC/STS users, policies may be specified in JWT claims rather than IAM DB. - // Resolve claim-based policy names against built-in default policies. if let Some(claim_policies) = args.claims.get("policy").and_then(|v| v.as_str()) { use rustfs_policy::policy::default::DEFAULT_POLICIES; let mut resolved = Vec::new(); @@ -786,162 +995,161 @@ impl IamSys { } } if !resolved.is_empty() { - let combined = Policy::merge_policies(resolved); - let (has_session_policy, is_allowed_sp) = is_allowed_by_session_policy(args); - if has_session_policy { - return is_allowed_sp && combined.is_allowed(args).await; - } - return combined.is_allowed(args).await; + combined_policy = Policy::merge_policies(resolved); + } else if args.deny_only { + combined_policy = Policy::default(); + } else { + return PreparedIamAuth { + needs_existing_object_tag: false, + mode: PreparedIamMode::Deny, + }; } + } else if args.deny_only { + combined_policy = Policy::default(); + } else { + return PreparedIamAuth { + needs_existing_object_tag: false, + mode: PreparedIamMode::Deny, + }; } - - if args.deny_only { - let combined_policy = Policy::default(); - let (has_session_policy, is_allowed_sp) = is_allowed_by_session_policy(args); - if has_session_policy { - return is_allowed_sp && combined_policy.is_allowed(args).await; + } else if !is_owner { + let (a, c) = self.store.merge_policies(&policies.join(",")).await; + if a.is_empty() { + if args.deny_only { + combined_policy = Policy::default(); + } else { + return PreparedIamAuth { + needs_existing_object_tag: false, + mode: PreparedIamMode::Deny, + }; } - return combined_policy.is_allowed(args).await; + } else { + combined_policy = c; } - return false; } - let combined_policy = { - if is_owner { - Policy::default() - } else { - let (a, c) = self.store.merge_policies(&policies.join(",")).await; - if a.is_empty() { - if args.deny_only { - Policy::default() - } else { - return false; - } - } else { - c - } - } - }; - - let (has_session_policy, is_allowed_sp) = is_allowed_by_session_policy(args); + let session_policy = prepare_session_policy(args, false); tracing::debug!( - "is_allowed_sts: action={:?}, has_session_policy={}, is_allowed_sp={}, is_owner={}, parent_user={}, groups_source={}, effective_groups={:?}", + "prepare_sts_auth: action={:?}, is_owner={}, parent_user={}, groups_source={}, effective_groups={:?}", args.action, - has_session_policy, - is_allowed_sp, is_owner, parent_user, groups_source, effective_groups ); - if has_session_policy { - return is_allowed_sp && (is_owner || combined_policy.is_allowed(args).await); + PreparedIamAuth { + needs_existing_object_tag: policy_needs_existing_object_tag_for_args(&combined_policy, args).await + || prepared_session_policy_needs_existing_object_tag_for_args(&session_policy, args).await, + mode: PreparedIamMode::Sts { + is_owner, + combined_policy, + session_policy, + }, } - - is_owner || combined_policy.is_allowed(args).await } - fn is_safe_claim_policy_name(policy: &str) -> bool { - !policy.is_empty() && policy.chars().all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-') - } - - pub async fn is_allowed_service_account(&self, args: &Args<'_>, parent_user: &str) -> bool { + async fn prepare_service_account_auth(&self, args: &Args<'_>, parent_user: &str) -> PreparedIamAuth { let Some(p) = args.claims.get("parent") else { - return false; + return PreparedIamAuth { + needs_existing_object_tag: false, + mode: PreparedIamMode::Deny, + }; }; if p.as_str() != Some(parent_user) { - return false; + return PreparedIamAuth { + needs_existing_object_tag: false, + mode: PreparedIamMode::Deny, + }; } let is_owner = matches!(get_global_action_cred(), Some(cred) if cred.access_key == parent_user); - let role_arn = args.get_role_arn(); - let svc_policies = { - if is_owner { - Vec::new() - } else if role_arn.is_some() { - let Ok(arn) = ARN::parse(role_arn.unwrap_or_default()) else { return false }; - MappedPolicy::new(self.roles_map.get(&arn).map_or_else(String::default, |v| v.clone()).as_str()).to_slice() - } else { - let Ok(p) = self.policy_db_get(parent_user, args.groups).await else { return false }; - p - } + let svc_policies = if is_owner { + Vec::new() + } else if role_arn.is_some() { + let Ok(arn) = ARN::parse(role_arn.unwrap_or_default()) else { + tracing::warn!( + parent_user = %parent_user, + role_arn = ?role_arn, + "prepare_service_account_auth: invalid role ARN in service account claims" + ); + return PreparedIamAuth { + needs_existing_object_tag: false, + mode: PreparedIamMode::Deny, + }; + }; + MappedPolicy::new(self.roles_map.get(&arn).map_or_else(String::default, |v| v.clone()).as_str()).to_slice() + } else { + let Ok(policies) = self.policy_db_get(parent_user, args.groups).await else { + return PreparedIamAuth { + needs_existing_object_tag: false, + mode: PreparedIamMode::Deny, + }; + }; + policies }; if !is_owner && svc_policies.is_empty() { - return false; + return PreparedIamAuth { + needs_existing_object_tag: false, + mode: PreparedIamMode::Deny, + }; } - let combined_policy = { - if is_owner { - Policy::default() - } else { - let (a, c) = self.store.merge_policies(&svc_policies.join(",")).await; - if a.is_empty() { - return false; - } - c + let combined_policy = if is_owner { + Policy::default() + } else { + let (a, c) = self.store.merge_policies(&svc_policies.join(",")).await; + if a.is_empty() { + return PreparedIamAuth { + needs_existing_object_tag: false, + mode: PreparedIamMode::Deny, + }; } + c }; - let mut parent_args = args.clone(); - parent_args.account = parent_user; - let Some(sa) = args.claims.get(&iam_policy_claim_name_sa()) else { - return false; + return PreparedIamAuth { + needs_existing_object_tag: false, + mode: PreparedIamMode::Deny, + }; }; - let Some(sa_str) = sa.as_str() else { - return false; + return PreparedIamAuth { + needs_existing_object_tag: false, + mode: PreparedIamMode::Deny, + }; }; - if sa_str == INHERITED_POLICY_TYPE { - return is_owner || combined_policy.is_allowed(&parent_args).await; + let mode = if sa_str == INHERITED_POLICY_TYPE { + PreparedServicePolicyMode::Inherited + } else { + PreparedServicePolicyMode::SessionBound + }; + + let session_policy = prepare_session_policy(args, true); + let needs_existing_object_tag = policy_needs_existing_object_tag_for_args(&combined_policy, args).await + || matches!(mode, PreparedServicePolicyMode::SessionBound) + && prepared_session_policy_needs_existing_object_tag_for_args(&session_policy, args).await; + + PreparedIamAuth { + needs_existing_object_tag, + mode: PreparedIamMode::ServiceAccount { + is_owner, + parent_user: parent_user.to_string(), + combined_policy, + mode, + session_policy, + }, } - - let (has_session_policy, is_allowed_sp) = is_allowed_by_session_policy_for_service_account(args); - if has_session_policy { - return is_allowed_sp && (is_owner || combined_policy.is_allowed(&parent_args).await); - } - - is_owner || combined_policy.is_allowed(&parent_args).await - } - - pub async fn get_combined_policy(&self, policies: &[String]) -> Policy { - self.store.merge_policies(&policies.join(",")).await.1 } pub async fn is_allowed(&self, args: &Args<'_>) -> bool { - if args.is_owner { - return true; - } - - let opa_enable = Self::get_policy_plugin_client().await; - if let Some(opa_enable) = opa_enable { - return opa_enable.is_allowed(args).await; - } - - let Ok((is_temp, parent_user)) = self.is_temp_user(args.account).await else { return false }; - - if is_temp { - return self.is_allowed_sts(args, &parent_user).await; - } - - let Ok((is_svc, parent_user)) = self.is_service_account(args.account).await else { return false }; - - if is_svc { - return self.is_allowed_service_account(args, &parent_user).await; - } - - let Ok(policies) = self.policy_db_get(args.account, args.groups).await else { return false }; - - if policies.is_empty() { - return false; - } - - self.get_combined_policy(&policies).await.is_allowed(args).await + let prepared = self.prepare_auth(args).await; + self.eval_prepared(&prepared, args).await } /// Check if the underlying store is ready @@ -950,55 +1158,56 @@ impl IamSys { } } -fn is_allowed_by_session_policy(args: &Args<'_>) -> (bool, bool) { - let Some(policy) = args.claims.get(SESSION_POLICY_NAME_EXTRACTED) else { - return (false, false); - }; - - let has_session_policy = true; - - let Some(policy_str) = policy.as_str() else { - return (has_session_policy, false); - }; - - let Ok(sub_policy) = Policy::parse_config(policy_str.as_bytes()) else { - return (has_session_policy, false); - }; - - if sub_policy.version.is_empty() { - return (has_session_policy, false); +async fn prepared_session_policy_needs_existing_object_tag_for_args(policy: &PreparedSessionPolicy, args: &Args<'_>) -> bool { + match policy { + PreparedSessionPolicy::Policy(p) => policy_needs_existing_object_tag_for_args(p, args).await, + PreparedSessionPolicy::None | PreparedSessionPolicy::DenyAll => false, } - - let mut session_policy_args = args.clone(); - session_policy_args.is_owner = false; - - (has_session_policy, pollster::block_on(sub_policy.is_allowed(&session_policy_args))) } -fn is_allowed_by_session_policy_for_service_account(args: &Args<'_>) -> (bool, bool) { - let Some(policy) = args.claims.get(SESSION_POLICY_NAME_EXTRACTED) else { - return (false, false); - }; - - let mut has_session_policy = true; - - let Some(policy_str) = policy.as_str() else { - return (has_session_policy, false); +fn prepare_session_policy(args: &Args<'_>, empty_is_none: bool) -> PreparedSessionPolicy { + let Some(policy_str) = extract_session_policy_text(args.claims) else { + return PreparedSessionPolicy::None; }; let Ok(sub_policy) = Policy::parse_config(policy_str.as_bytes()) else { - return (has_session_policy, false); + return PreparedSessionPolicy::DenyAll; }; - if sub_policy.version.is_empty() && sub_policy.statements.is_empty() && sub_policy.id.is_empty() { - has_session_policy = false; - return (has_session_policy, false); + if empty_is_none { + if sub_policy.version.is_empty() && sub_policy.statements.is_empty() && sub_policy.id.is_empty() { + return PreparedSessionPolicy::None; + } + return PreparedSessionPolicy::Policy(sub_policy); } - let mut session_policy_args = args.clone(); - session_policy_args.is_owner = false; + if sub_policy.version.is_empty() { + return PreparedSessionPolicy::DenyAll; + } - (has_session_policy, pollster::block_on(sub_policy.is_allowed(&session_policy_args))) + PreparedSessionPolicy::Policy(sub_policy) +} + +fn extract_session_policy_text(claims: &HashMap) -> Option { + if let Some(policy_str) = claims.get(SESSION_POLICY_NAME_EXTRACTED).and_then(|v| v.as_str()) { + return Some(policy_str.to_string()); + } + + let encoded = claims.get(SESSION_POLICY_NAME).and_then(|v| v.as_str())?; + let bytes = base64_simd::URL_SAFE_NO_PAD.decode_to_vec(encoded.as_bytes()).ok()?; + String::from_utf8(bytes).ok() +} + +async fn evaluate_prepared_session_policy(policy: &PreparedSessionPolicy, args: &Args<'_>) -> Option { + match policy { + PreparedSessionPolicy::None => None, + PreparedSessionPolicy::DenyAll => Some(false), + PreparedSessionPolicy::Policy(p) => { + let mut session_policy_args = args.clone(); + session_policy_args.is_owner = false; + Some(p.is_allowed(&session_policy_args).await) + } + } } #[derive(Debug, Clone, Default)] @@ -1050,6 +1259,7 @@ mod tests { use rustfs_policy::auth::UserIdentity; use rustfs_policy::policy::Args; use rustfs_policy::policy::action::{Action, AdminAction, S3Action}; + use rustfs_policy::policy::policy_uses_existing_object_tag_conditions; use serde_json::Value; use std::collections::HashMap; use time::OffsetDateTime; @@ -1301,7 +1511,8 @@ mod tests { deny_only: false, }; - let allowed = iam_sys.is_allowed_sts(&args, parent_user).await; + let prepared = iam_sys.prepare_sts_auth(&args, parent_user).await; + let allowed = iam_sys.eval_prepared(&prepared, &args).await; assert!( allowed, "STS temp credentials with no groups in args should still be allowed via parent user's group policy (readwrite)" @@ -1342,7 +1553,8 @@ mod tests { deny_only: true, }; - let allowed = iam_sys.is_allowed_sts(&args, parent_user).await; + let prepared = iam_sys.prepare_sts_auth(&args, parent_user).await; + let allowed = iam_sys.eval_prepared(&prepared, &args).await; assert!( !allowed, "session policy Deny must be evaluated even when IAM policies are empty and deny_only is set" @@ -1381,7 +1593,8 @@ mod tests { deny_only: true, }; - let allowed = iam_sys.is_allowed_sts(&args, parent_user).await; + let prepared = iam_sys.prepare_sts_auth(&args, parent_user).await; + let allowed = iam_sys.eval_prepared(&prepared, &args).await; assert!( allowed, "deny_only with no matching Deny in session policy should still allow self-service-style checks" @@ -1412,4 +1625,252 @@ mod tests { "regular user mapped policy must be written to user_policies for bucket user listing" ); } + + #[tokio::test] + async fn test_prepare_auth_eval_matches_prepare_sts_auth_for_parent_policy_fallback() { + let store = StsTestMockStore { empty_policies: false }; + let cache_manager = IamCache::new(store).await; + let iam_sys = IamSys::new(cache_manager); + + let parent_user = "sts-fallback-test-parent"; + let claims = HashMap::new(); + let groups: Option> = None; + let args = Args { + account: parent_user, + groups: &groups, + action: Action::S3Action(S3Action::ListBucketAction), + bucket: "mybucket", + conditions: &HashMap::new(), + is_owner: false, + object: "", + claims: &claims, + deny_only: false, + }; + + let sts_prepared = iam_sys.prepare_sts_auth(&args, parent_user).await; + let sts_eval = iam_sys.eval_prepared(&sts_prepared, &args).await; + let prepared = iam_sys.prepare_auth(&args).await; + let eval = iam_sys.eval_prepared(&prepared, &args).await; + assert_eq!(sts_eval, eval, "prepare_auth must match explicit STS preparation for this identity"); + } + + #[tokio::test] + async fn test_prepare_auth_detects_existing_object_tag_in_session_policy() { + let store = StsTestMockStore { empty_policies: true }; + let cache_manager = IamCache::new(store).await; + let iam_sys = IamSys::new(cache_manager); + let sts_access_key = "sts-session-tag-test-user"; + + let sts_user = UserIdentity::from(Credentials { + access_key: sts_access_key.to_string(), + secret_key: "longenoughsecret".to_string(), + session_token: "sts-token".to_string(), + status: ACCOUNT_ON.to_string(), + parent_user: "sts-empty-parent-policy-test".to_string(), + ..Default::default() + }); + Cache::add_or_update(&iam_sys.store.cache.sts_accounts, sts_access_key, &sts_user, OffsetDateTime::now_utc()); + + let mut claims = HashMap::new(); + claims.insert( + SESSION_POLICY_NAME_EXTRACTED.to_string(), + Value::String( + r#"{ + "Version":"2012-10-17", + "Statement":[{"Effect":"Allow","Action":["s3:GetObject"],"Resource":["arn:aws:s3:::bucket/*"],"Condition":{"StringEquals":{"s3:ExistingObjectTag/security":"public"}}}] +}"# + .to_string(), + ), + ); + + let groups: Option> = None; + let args = Args { + account: sts_access_key, + groups: &groups, + action: Action::S3Action(S3Action::GetObjectAction), + bucket: "bucket", + conditions: &HashMap::new(), + is_owner: false, + object: "obj", + claims: &claims, + deny_only: true, + }; + + let prepared = iam_sys.prepare_auth(&args).await; + assert!( + prepared.needs_existing_object_tag, + "session policy with ExistingObjectTag must request object tag loading" + ); + } + + #[test] + fn test_policy_uses_existing_object_tag_matches_condition_keys_only() { + let with_value_only = Policy::parse_config( + br#"{ + "Version":"2012-10-17", + "Statement":[{ + "Effect":"Allow", + "Action":["s3:GetObject"], + "Resource":["arn:aws:s3:::bucket/*"], + "Condition":{"StringEquals":{"s3:prefix":"ExistingObjectTag/security"}} + }] +}"#, + ) + .expect("policy with value-only ExistingObjectTag text should parse"); + assert!( + !policy_uses_existing_object_tag_conditions(&with_value_only), + "ExistingObjectTag text in values should not trigger tag dependency" + ); + + let with_condition_key = Policy::parse_config( + br#"{ + "Version":"2012-10-17", + "Statement":[{ + "Effect":"Allow", + "Action":["s3:GetObject"], + "Resource":["arn:aws:s3:::bucket/*"], + "Condition":{"StringEquals":{"s3:ExistingObjectTag/security":"public"}} + }] +}"#, + ) + .expect("policy with ExistingObjectTag condition key should parse"); + assert!( + policy_uses_existing_object_tag_conditions(&with_condition_key), + "ExistingObjectTag condition key must trigger tag dependency" + ); + } + + #[test] + fn test_policy_uses_existing_object_tag_when_only_secondary_action_has_tag_condition() { + let split_action_policy = Policy::parse_config( + br#"{ + "Version":"2012-10-17", + "Statement":[ + { + "Effect":"Allow", + "Action":["s3:DeleteObject"], + "Resource":["arn:aws:s3:::bucket/*"] + }, + { + "Effect":"Allow", + "Action":["s3:DeleteObjectVersion"], + "Resource":["arn:aws:s3:::bucket/*"], + "Condition":{"StringEquals":{"s3:ExistingObjectTag/security":"public"}} + } + ] +}"#, + ) + .expect("split-action policy should parse"); + + assert!( + policy_uses_existing_object_tag_conditions(&split_action_policy), + "full merged policy must still be detectable as containing ExistingObjectTag keys" + ); + } + + #[tokio::test] + async fn test_prepare_auth_detects_existing_object_tag_in_encoded_session_policy() { + let store = StsTestMockStore { empty_policies: true }; + let cache_manager = IamCache::new(store).await; + let iam_sys = IamSys::new(cache_manager); + let sts_access_key = "sts-session-tag-encoded-test-user"; + + let sts_user = UserIdentity::from(Credentials { + access_key: sts_access_key.to_string(), + secret_key: "longenoughsecret".to_string(), + session_token: "sts-token".to_string(), + status: ACCOUNT_ON.to_string(), + parent_user: "sts-empty-parent-policy-test".to_string(), + ..Default::default() + }); + Cache::add_or_update(&iam_sys.store.cache.sts_accounts, sts_access_key, &sts_user, OffsetDateTime::now_utc()); + + let session_policy_json = r#"{ + "Version":"2012-10-17", + "Statement":[{"Effect":"Allow","Action":["s3:GetObject"],"Resource":["arn:aws:s3:::bucket/*"],"Condition":{"StringEquals":{"s3:ExistingObjectTag/security":"public"}}}] +}"#; + let mut claims = HashMap::new(); + claims.insert( + SESSION_POLICY_NAME.to_string(), + Value::String(base64_simd::URL_SAFE_NO_PAD.encode_to_string(session_policy_json.as_bytes())), + ); + + let groups: Option> = None; + let args = Args { + account: sts_access_key, + groups: &groups, + action: Action::S3Action(S3Action::GetObjectAction), + bucket: "bucket", + conditions: &HashMap::new(), + is_owner: false, + object: "obj", + claims: &claims, + deny_only: true, + }; + + let prepared = iam_sys.prepare_auth(&args).await; + assert!( + prepared.needs_existing_object_tag, + "base64 sessionPolicy with ExistingObjectTag must request object tag loading" + ); + } + + #[tokio::test] + async fn test_prepare_auth_service_account_inherited_ignores_session_policy_tag_hint() { + let store = StsTestMockStore { empty_policies: false }; + let cache_manager = IamCache::new(store).await; + let iam_sys = IamSys::new(cache_manager); + + let service_account_access_key = "svc-inherited-tag-hint-test-user"; + let parent_user = "sts-fallback-test-parent"; + let mut service_account_claims = HashMap::new(); + service_account_claims.insert(iam_policy_claim_name_sa(), Value::String(INHERITED_POLICY_TYPE.to_string())); + let service_identity = UserIdentity::from(Credentials { + access_key: service_account_access_key.to_string(), + secret_key: "longenoughsecret".to_string(), + status: ACCOUNT_ON.to_string(), + parent_user: parent_user.to_string(), + claims: Some(service_account_claims), + ..Default::default() + }); + Cache::add_or_update( + &iam_sys.store.cache.users, + service_account_access_key, + &service_identity, + OffsetDateTime::now_utc(), + ); + + let mut request_claims = HashMap::new(); + request_claims.insert("parent".to_string(), Value::String(parent_user.to_string())); + request_claims.insert(iam_policy_claim_name_sa(), Value::String(INHERITED_POLICY_TYPE.to_string())); + request_claims.insert( + SESSION_POLICY_NAME_EXTRACTED.to_string(), + Value::String( + r#"{ + "Version":"2012-10-17", + "Statement":[{"Effect":"Allow","Action":["s3:GetObject"],"Resource":["arn:aws:s3:::bucket/*"],"Condition":{"StringEquals":{"s3:ExistingObjectTag/security":"public"}}}] +}"# + .to_string(), + ), + ); + + let groups: Option> = Some(vec!["testgroup".to_string()]); + let args = Args { + account: service_account_access_key, + groups: &groups, + action: Action::S3Action(S3Action::GetObjectAction), + bucket: "bucket", + conditions: &HashMap::new(), + is_owner: false, + object: "obj", + claims: &request_claims, + deny_only: false, + }; + + let prepared = iam_sys.prepare_auth(&args).await; + assert!( + !prepared.needs_existing_object_tag, + "inherited service account should not require object tag fetch based on session policy hint" + ); + } } diff --git a/crates/policy/src/policy/policy.rs b/crates/policy/src/policy/policy.rs index 457356c78..6464fcfd3 100644 --- a/crates/policy/src/policy/policy.rs +++ b/crates/policy/src/policy/policy.rs @@ -12,7 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -use super::{Effect, Error as IamError, ID, Statement, action::Action, statement::BPStatement}; +use super::{ + Effect, Error as IamError, Functions, ID, Statement, action::Action, statement::BPStatement, + statement::variable_resolver_for_policy_args, +}; use crate::error::{Error, Result}; use serde::{Deserialize, Serialize}; use serde_json::Value; @@ -272,6 +275,77 @@ pub fn iam_policy_claim_name_sa() -> String { rustfs_credentials::IAM_POLICY_CLAIM_NAME_SA.to_string() } +#[inline] +pub fn is_existing_object_tag_condition_key(key: &str) -> bool { + matches!(key, "ExistingObjectTag" | "s3:ExistingObjectTag") + || key.starts_with("ExistingObjectTag/") + || key.starts_with("s3:ExistingObjectTag/") +} + +pub fn value_uses_existing_object_tag_condition_key(value: &Value) -> bool { + match value { + Value::Object(obj) => obj + .iter() + .any(|(key, value)| is_existing_object_tag_condition_key(key) || value_uses_existing_object_tag_condition_key(value)), + Value::Array(items) => items.iter().any(value_uses_existing_object_tag_condition_key), + _ => false, + } +} + +/// True if `conditions` JSON references `s3:ExistingObjectTag` / `ExistingObjectTag/...` keys. +pub fn functions_use_existing_object_tag(conditions: &Functions) -> bool { + serde_json::to_value(conditions) + .map(|v| value_uses_existing_object_tag_condition_key(&v)) + .unwrap_or(false) +} + +pub fn policy_uses_existing_object_tag_conditions(policy: &Policy) -> bool { + policy + .statements + .iter() + .any(|statement| functions_use_existing_object_tag(&statement.conditions)) +} + +pub fn bucket_policy_uses_existing_object_tag_conditions(policy: &BucketPolicy) -> bool { + policy + .statements + .iter() + .any(|statement| functions_use_existing_object_tag(&statement.conditions)) +} + +/// True when at least one statement that applies to `args` may evaluate ExistingObjectTag conditions. +pub async fn policy_needs_existing_object_tag_for_args(policy: &Policy, args: &Args<'_>) -> bool { + if !policy_uses_existing_object_tag_conditions(policy) { + return false; + } + let resolver = variable_resolver_for_policy_args(args); + for statement in &policy.statements { + if !functions_use_existing_object_tag(&statement.conditions) { + continue; + } + if statement.request_reaches_condition_eval(args, &resolver).await { + return true; + } + } + false +} + +/// True when at least one bucket-policy statement that applies to `args` may evaluate ExistingObjectTag conditions. +pub async fn bucket_policy_needs_existing_object_tag_for_args(policy: &BucketPolicy, args: &BucketPolicyArgs<'_>) -> bool { + if !bucket_policy_uses_existing_object_tag_conditions(policy) { + return false; + } + for statement in &policy.statements { + if !functions_use_existing_object_tag(&statement.conditions) { + continue; + } + if statement.request_reaches_condition_eval(args).await { + return true; + } + } + false +} + pub mod default { use std::{collections::HashSet, sync::LazyLock}; @@ -1231,6 +1305,61 @@ mod test { assert_eq!(statement["Principal"]["AWS"], "*"); } + #[test] + fn test_existing_object_tag_condition_helpers() { + let identity_policy = Policy::parse_config( + br#"{ + "Version":"2012-10-17", + "Statement":[{ + "Effect":"Allow", + "Action":["s3:GetObject"], + "Resource":["arn:aws:s3:::bucket/*"], + "Condition":{"StringEquals":{"s3:ExistingObjectTag/security":"public"}} + }] +}"#, + ) + .expect("identity policy with ExistingObjectTag key should parse"); + assert!( + policy_uses_existing_object_tag_conditions(&identity_policy), + "identity policy ExistingObjectTag key should be detected" + ); + + let identity_value_only = Policy::parse_config( + br#"{ + "Version":"2012-10-17", + "Statement":[{ + "Effect":"Allow", + "Action":["s3:GetObject"], + "Resource":["arn:aws:s3:::bucket/*"], + "Condition":{"StringEquals":{"s3:prefix":"ExistingObjectTag/security"}} + }] +}"#, + ) + .expect("identity policy with value-only marker should parse"); + assert!( + !policy_uses_existing_object_tag_conditions(&identity_value_only), + "value-only marker must not be treated as ExistingObjectTag condition key" + ); + + let bucket_policy: BucketPolicy = serde_json::from_str( + r#"{ + "Version":"2012-10-17", + "Statement":[{ + "Effect":"Allow", + "Principal":"*", + "Action":["s3:GetObject"], + "Resource":["arn:aws:s3:::bucket/*"], + "Condition":{"StringEquals":{"s3:ExistingObjectTag/security":"public"}} + }] +}"#, + ) + .expect("bucket policy with ExistingObjectTag key should parse"); + assert!( + bucket_policy_uses_existing_object_tag_conditions(&bucket_policy), + "bucket policy ExistingObjectTag key should be detected" + ); + } + #[test] fn test_bucket_policy_serialize_single_action_as_array() { use crate::policy::action::{Action, ActionSet, S3Action}; @@ -1359,4 +1488,209 @@ mod test { Ok(()) } + + #[tokio::test] + async fn test_policy_needs_existing_object_tag_narrows_by_action() { + use crate::policy::Args; + use crate::policy::action::{Action, S3Action}; + use std::collections::HashMap; + + let split_policy = Policy::parse_config( + br#"{ + "Version":"2012-10-17", + "Statement":[ + { + "Effect":"Allow", + "Action":["s3:DeleteObject"], + "Resource":["arn:aws:s3:::bucket/*"] + }, + { + "Effect":"Allow", + "Action":["s3:DeleteObjectVersion"], + "Resource":["arn:aws:s3:::bucket/*"], + "Condition":{"StringEquals":{"s3:ExistingObjectTag/security":"public"}} + } + ] +}"#, + ) + .expect("split-action policy should parse"); + + let groups: Option> = None; + let cond = HashMap::new(); + let claims = HashMap::new(); + + let args_get = Args { + account: "user", + groups: &groups, + action: Action::S3Action(S3Action::GetObjectAction), + bucket: "bucket", + conditions: &cond, + is_owner: false, + object: "k", + claims: &claims, + deny_only: false, + }; + assert!( + !policy_needs_existing_object_tag_for_args(&split_policy, &args_get).await, + "GetObject should not match statements with DeleteObject/DeleteObjectVersion" + ); + + let args_del = Args { + account: "user", + groups: &groups, + action: Action::S3Action(S3Action::DeleteObjectAction), + bucket: "bucket", + conditions: &cond, + is_owner: false, + object: "k", + claims: &claims, + deny_only: false, + }; + assert!( + !policy_needs_existing_object_tag_for_args(&split_policy, &args_del).await, + "DeleteObject matches only the statement without ExistingObjectTag" + ); + + let args_delv = Args { + account: "user", + groups: &groups, + action: Action::S3Action(S3Action::DeleteObjectVersionAction), + bucket: "bucket", + conditions: &cond, + is_owner: false, + object: "k", + claims: &claims, + deny_only: false, + }; + assert!( + policy_needs_existing_object_tag_for_args(&split_policy, &args_delv).await, + "DeleteObjectVersion matches the statement with ExistingObjectTag" + ); + } + + #[tokio::test] + async fn test_policy_needs_existing_object_tag_narrows_by_resource() { + use crate::policy::Args; + use crate::policy::action::{Action, S3Action}; + use std::collections::HashMap; + + let policy = Policy::parse_config( + br#"{ + "Version":"2012-10-17", + "Statement":[ + { + "Effect":"Allow", + "Action":["s3:GetObject"], + "Resource":["arn:aws:s3:::bucket/private/*"], + "Condition":{"StringEquals":{"s3:ExistingObjectTag/security":"public"}} + } + ] +}"#, + ) + .expect("policy should parse"); + + let groups: Option> = None; + let cond = HashMap::new(); + let claims = HashMap::new(); + + let args_public = Args { + account: "user", + groups: &groups, + action: Action::S3Action(S3Action::GetObjectAction), + bucket: "bucket", + conditions: &cond, + is_owner: false, + object: "public/a.txt", + claims: &claims, + deny_only: false, + }; + assert!( + !policy_needs_existing_object_tag_for_args(&policy, &args_public).await, + "resource mismatch should skip ExistingObjectTag fetch hint" + ); + + let args_private = Args { + account: "user", + groups: &groups, + action: Action::S3Action(S3Action::GetObjectAction), + bucket: "bucket", + conditions: &cond, + is_owner: false, + object: "private/a.txt", + claims: &claims, + deny_only: false, + }; + assert!( + policy_needs_existing_object_tag_for_args(&policy, &args_private).await, + "resource match should keep ExistingObjectTag fetch hint" + ); + } + + #[tokio::test] + async fn test_bucket_policy_needs_existing_object_tag_narrows_by_principal() { + use crate::policy::BucketPolicyArgs; + use crate::policy::action::{Action, S3Action}; + use std::collections::HashMap; + + let bucket_policy: BucketPolicy = serde_json::from_str( + r#"{ + "Version":"2012-10-17", + "Statement":[ + { + "Effect":"Allow", + "Principal":{"AWS":["alice"]}, + "Action":["s3:GetObject"], + "Resource":["arn:aws:s3:::bucket/private/*"], + "Condition":{"StringEquals":{"s3:ExistingObjectTag/security":"public"}} + } + ] +}"#, + ) + .expect("bucket policy should parse"); + + let groups: Option> = None; + let cond = HashMap::new(); + + let args_bob = BucketPolicyArgs { + bucket: "bucket", + action: Action::S3Action(S3Action::GetObjectAction), + is_owner: false, + account: "bob", + groups: &groups, + conditions: &cond, + object: "private/a.txt", + }; + assert!( + !bucket_policy_needs_existing_object_tag_for_args(&bucket_policy, &args_bob).await, + "principal mismatch should skip ExistingObjectTag fetch hint" + ); + + let args_alice_public = BucketPolicyArgs { + bucket: "bucket", + action: Action::S3Action(S3Action::GetObjectAction), + is_owner: false, + account: "alice", + groups: &groups, + conditions: &cond, + object: "public/a.txt", + }; + assert!( + !bucket_policy_needs_existing_object_tag_for_args(&bucket_policy, &args_alice_public).await, + "resource mismatch should skip ExistingObjectTag fetch hint" + ); + + let args_alice_private = BucketPolicyArgs { + bucket: "bucket", + action: Action::S3Action(S3Action::GetObjectAction), + is_owner: false, + account: "alice", + groups: &groups, + conditions: &cond, + object: "private/a.txt", + }; + assert!( + bucket_policy_needs_existing_object_tag_for_args(&bucket_policy, &args_alice_private).await, + "principal and resource match should keep ExistingObjectTag fetch hint" + ); + } } diff --git a/crates/policy/src/policy/statement.rs b/crates/policy/src/policy/statement.rs index 3dd18ef9a..df33f3c3e 100644 --- a/crates/policy/src/policy/statement.rs +++ b/crates/policy/src/policy/statement.rs @@ -38,6 +38,24 @@ pub struct Statement { pub conditions: Functions, } +/// Builds the same [`VariableResolver`] as [`Statement::is_allowed`]. +pub(crate) fn variable_resolver_for_policy_args(args: &Args<'_>) -> VariableResolver { + let mut context = VariableContext::new(); + context.claims = Some(args.claims.clone()); + context.conditions = args.conditions.clone(); + context.account_id = Some(args.account.to_string()); + + let username = if let Some(parent) = args.claims.get("parent").and_then(|v| v.as_str()) { + parent.to_string() + } else { + args.account.to_string() + }; + + context.username = Some(username); + + VariableResolver::new(context) +} + impl Statement { fn is_kms(&self) -> bool { for act in self.actions.iter() { @@ -69,67 +87,62 @@ impl Statement { false } - pub async fn is_allowed(&self, args: &Args<'_>) -> bool { - let mut context = VariableContext::new(); - context.claims = Some(args.claims.clone()); - context.conditions = args.conditions.clone(); - context.account_id = Some(args.account.to_string()); + /// Returns true when this statement would reach `conditions.evaluate_with_resolver` in + /// [`Statement::is_allowed`] (including the KMS shortcut path). Does not evaluate conditions. + pub(crate) async fn request_reaches_condition_eval(&self, args: &Args<'_>, resolver: &VariableResolver) -> bool { + if (!self.actions.is_match(&args.action) && !self.actions.is_empty()) || self.not_actions.is_match(&args.action) { + return false; + } - let username = if let Some(parent) = args.claims.get("parent").and_then(|v| v.as_str()) { - // For temp credentials or service account credentials, username is parent_user - parent.to_string() - } else { - // For regular user credentials, username is access_key - args.account.to_string() - }; - - context.username = Some(username); - - let resolver = VariableResolver::new(context); - - let check = 'c: { - if (!self.actions.is_match(&args.action) && !self.actions.is_empty()) || self.not_actions.is_match(&args.action) { - break 'c 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 { + let mut resource = String::from(args.bucket); + if !args.object.is_empty() { + if !args.object.starts_with('/') { resource.push('/'); } - if self.is_kms() && (resource == "/" || self.resources.is_empty()) { - break 'c self.conditions.evaluate_with_resolver(args.conditions, Some(&resolver)).await; - } + resource.push_str(args.object); + } else { + resource.push('/'); + } - if self.resources.is_empty() && self.not_resources.is_empty() && !self.is_admin() && !self.is_sts() { - break 'c false; - } + if self.is_kms() && (resource == "/" || self.resources.is_empty()) { + return true; + } - if !self.resources.is_empty() - && !self - .resources - .is_match_with_resolver(&resource, args.conditions, Some(&resolver)) - .await - && !self.is_admin() - && !self.is_sts() - { - break 'c false; - } + if self.resources.is_empty() && self.not_resources.is_empty() && !self.is_admin() && !self.is_sts() { + return false; + } - if !self.not_resources.is_empty() - && self - .not_resources - .is_match_with_resolver(&resource, args.conditions, Some(&resolver)) - .await - && !self.is_admin() - && !self.is_sts() - { + if !self.resources.is_empty() + && !self + .resources + .is_match_with_resolver(&resource, args.conditions, Some(resolver)) + .await + && !self.is_admin() + && !self.is_sts() + { + return false; + } + + if !self.not_resources.is_empty() + && self + .not_resources + .is_match_with_resolver(&resource, args.conditions, Some(resolver)) + .await + && !self.is_admin() + && !self.is_sts() + { + return false; + } + + true + } + + pub async fn is_allowed(&self, args: &Args<'_>) -> bool { + let resolver = variable_resolver_for_policy_args(args); + + let check = 'c: { + if !self.request_reaches_condition_eval(args, &resolver).await { break 'c false; } @@ -207,32 +220,41 @@ pub struct BPStatement { } impl BPStatement { - pub async fn is_allowed(&self, args: &BucketPolicyArgs<'_>) -> bool { - let check = 'c: { - if !self.principal.is_match(args.account) { - break 'c false; - } + /// Returns true when this statement would reach `conditions.evaluate` in [`BPStatement::is_allowed`]. + pub(crate) async fn request_reaches_condition_eval(&self, args: &BucketPolicyArgs<'_>) -> bool { + if !self.principal.is_match(args.account) { + return false; + } - if (!self.actions.is_match(&args.action) && !self.actions.is_empty()) || self.not_actions.is_match(&args.action) { - break 'c false; - } + if (!self.actions.is_match(&args.action) && !self.actions.is_empty()) || self.not_actions.is_match(&args.action) { + 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 { + let mut resource = String::from(args.bucket); + if !args.object.is_empty() { + if !args.object.starts_with('/') { resource.push('/'); } - if !self.resources.is_empty() && !self.resources.is_match(&resource, args.conditions).await { - break 'c false; - } + resource.push_str(args.object); + } else { + resource.push('/'); + } - if !self.not_resources.is_empty() && self.not_resources.is_match(&resource, args.conditions).await { + if !self.resources.is_empty() && !self.resources.is_match(&resource, args.conditions).await { + return false; + } + + if !self.not_resources.is_empty() && self.not_resources.is_match(&resource, args.conditions).await { + return false; + } + + true + } + + pub async fn is_allowed(&self, args: &BucketPolicyArgs<'_>) -> bool { + let check = 'c: { + if !self.request_reaches_condition_eval(args).await { break 'c false; } diff --git a/rustfs/src/admin/auth.rs b/rustfs/src/admin/auth.rs index 4a91c36c6..318beee97 100644 --- a/rustfs/src/admin/auth.rs +++ b/rustfs/src/admin/auth.rs @@ -132,9 +132,6 @@ pub async fn validate_admin_request_with_bucket( Err(s3_error!(AccessDenied, "Access Denied")) } -/// Unified authentication request handler for both UI and CLI -/// -/// This function provides a single entry point for authentication, /// Unified authentication request handler for both UI and CLI /// /// This function provides a single entry point for authentication, diff --git a/rustfs/src/storage/access.rs b/rustfs/src/storage/access.rs index d6830b03b..8553466bf 100644 --- a/rustfs/src/storage/access.rs +++ b/rustfs/src/storage/access.rs @@ -25,11 +25,15 @@ use rustfs_ecstore::new_object_layer_fn; use rustfs_ecstore::store_api::BucketOperations; use rustfs_iam::error::Error as IamError; use rustfs_policy::policy::action::{Action, S3Action}; -use rustfs_policy::policy::{Args, BucketPolicyArgs}; +use rustfs_policy::policy::{ + Args, BucketPolicy, BucketPolicyArgs, bucket_policy_needs_existing_object_tag_for_args, + bucket_policy_uses_existing_object_tag_conditions, +}; use rustfs_utils::http::AMZ_OBJECT_LOCK_BYPASS_GOVERNANCE; 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; #[derive(Default, Clone, Debug)] @@ -64,7 +68,27 @@ fn ext_req_info_mut(ext: &mut http::Extensions) -> S3Result<&mut ReqInfo> { } #[derive(Clone, Debug)] -pub(crate) struct ObjectTagConditions(pub HashMap>); +pub(crate) struct ObjectTagConditions { + bucket: String, + object: String, + version_id: Option, + values: HashMap>, +} + +impl ObjectTagConditions { + fn new(bucket: &str, object: &str, version_id: Option<&str>, values: HashMap>) -> Self { + Self { + bucket: bucket.to_string(), + object: object.to_string(), + version_id: version_id.map(str::to_string), + values, + } + } + + fn matches(&self, bucket: &str, object: &str, version_id: Option<&str>) -> bool { + self.bucket == bucket && self.object == object && self.version_id.as_deref() == version_id + } +} const AMZ_WRITE_OFFSET_BYTES_HEADER: &str = "x-amz-write-offset-bytes"; @@ -72,38 +96,176 @@ fn has_write_offset_bytes_header(headers: &http::HeaderMap) -> bool { headers.contains_key(AMZ_WRITE_OFFSET_BYTES_HEADER) } -/// Returns true if the bucket has a policy that uses `s3:ExistingObjectTag` (or -/// `ExistingObjectTag/...`) conditions. Used to skip fetching object tags when -/// no tag-based policy is in effect. -async fn bucket_policy_uses_existing_object_tag(bucket: &str) -> bool { - let Ok((policy_str, _)) = metadata_sys::get_bucket_policy_raw(bucket).await else { - return false; - }; - policy_str.contains("ExistingObjectTag") +/// True when the bucket policy may evaluate `s3:ExistingObjectTag` for this request (statement +/// matches principal/action/resource and conditions reference ExistingObjectTag keys). +enum BucketPolicyExistingObjectTagHint { + NoTagRequirement, + ConservativeTagRequired, + Parsed(BucketPolicy), } -impl FS { - /// Fetches object tags (when the bucket policy requires them) and wraps them - /// as `ObjectTagConditions`. Returns `AccessDenied` on transient storage - /// errors to avoid fail-open on Deny policies. - async fn fetch_tag_conditions( - &self, - bucket: &str, - key: &str, - version_id: Option<&str>, - op: &'static str, - ) -> S3Result { - let tag_conditions = if bucket_policy_uses_existing_object_tag(bucket).await { - counter!("rustfs.object_tag_conditions.fetched", "op" => op).increment(1); - self.get_object_tag_conditions_for_policy(bucket, key, version_id).await? - } else { - counter!("rustfs.object_tag_conditions.skipped", "op" => op).increment(1); - HashMap::new() - }; - Ok(ObjectTagConditions(tag_conditions)) +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum BucketPolicyRawLoadErrorKind { + PolicyMissing, + BucketMissing, + Other, +} + +fn classify_bucket_policy_raw_load_error(err: &StorageError) -> BucketPolicyRawLoadErrorKind { + if err == &StorageError::ConfigNotFound { + BucketPolicyRawLoadErrorKind::PolicyMissing + } else if is_err_bucket_not_found(err) { + BucketPolicyRawLoadErrorKind::BucketMissing + } else { + BucketPolicyRawLoadErrorKind::Other } } +/// Load and parse bucket policy once for ExistingObjectTag hint checks. +async fn load_bucket_policy_existing_object_tag_hint(bucket: &str, action: Action) -> BucketPolicyExistingObjectTagHint { + let (policy_str, _) = match metadata_sys::get_bucket_policy_raw(bucket).await { + Ok(v) => v, + Err(err) => match classify_bucket_policy_raw_load_error(&err) { + BucketPolicyRawLoadErrorKind::PolicyMissing => { + tracing::debug!( + bucket = %bucket, + ?action, + "bucket policy not configured while checking ExistingObjectTag hint; treating as no tag requirement" + ); + return BucketPolicyExistingObjectTagHint::NoTagRequirement; + } + BucketPolicyRawLoadErrorKind::BucketMissing => { + tracing::debug!( + bucket = %bucket, + ?action, + error = %err, + "bucket missing while checking ExistingObjectTag hint; treating as no tag requirement" + ); + return BucketPolicyExistingObjectTagHint::NoTagRequirement; + } + BucketPolicyRawLoadErrorKind::Other => { + tracing::warn!( + bucket = %bucket, + ?action, + error = %err, + "failed to load bucket policy while checking ExistingObjectTag hint; conservatively enabling tag fetch" + ); + return BucketPolicyExistingObjectTagHint::ConservativeTagRequired; + } + }, + }; + match serde_json::from_str::(policy_str.as_str()) { + Ok(policy) => { + if bucket_policy_uses_existing_object_tag_conditions(&policy) { + BucketPolicyExistingObjectTagHint::Parsed(policy) + } else { + BucketPolicyExistingObjectTagHint::NoTagRequirement + } + } + Err(err) => { + tracing::warn!( + bucket = %bucket, + ?action, + error = %err, + "malformed bucket policy while checking ExistingObjectTag hint; conservatively enabling tag fetch" + ); + BucketPolicyExistingObjectTagHint::ConservativeTagRequired + } + } +} + +async fn bucket_policy_needs_existing_object_tag_from_hint( + hint: &BucketPolicyExistingObjectTagHint, + args: &BucketPolicyArgs<'_>, +) -> bool { + match hint { + BucketPolicyExistingObjectTagHint::NoTagRequirement => false, + BucketPolicyExistingObjectTagHint::ConservativeTagRequired => true, + BucketPolicyExistingObjectTagHint::Parsed(policy) => bucket_policy_needs_existing_object_tag_for_args(policy, args).await, + } +} + +fn merge_object_tag_conditions(conditions: &mut HashMap>, tags: &HashMap>) { + for (k, v) in tags { + conditions + .entry(k.clone()) + .and_modify(|existing| existing.extend(v.iter().cloned())) + .or_insert_with(|| v.clone()); + } +} + +fn action_tag_metric_label(action: &Action) -> &'static str { + match action { + Action::S3Action(S3Action::GetObjectAction) => "get_object", + Action::S3Action(S3Action::GetObjectAttributesAction) => "get_object_attributes", + Action::S3Action(S3Action::GetObjectVersionAction) => "get_object_version", + Action::S3Action(S3Action::GetObjectVersionAttributesAction) => "get_object_version_attributes", + Action::S3Action(S3Action::GetObjectTaggingAction) => "get_object_tagging", + Action::S3Action(S3Action::DeleteObjectAction) => "delete_object", + Action::S3Action(S3Action::DeleteObjectVersionAction) => "delete_object_version", + Action::S3Action(S3Action::DeleteObjectTaggingAction) => "delete_object_tagging", + Action::S3Action(S3Action::PutObjectTaggingAction) => "put_object_tagging", + _ => "authorize", + } +} + +fn auth_fs() -> &'static FS { + static AUTH_FS: OnceLock = OnceLock::new(); + AUTH_FS.get_or_init(FS::new) +} + +/// Extra action that may be evaluated in the same authorization flow and can +/// independently require `ExistingObjectTag` conditions. +fn secondary_tag_hint_action(action: Action, version_id: Option<&str>) -> Option { + match action { + Action::S3Action(S3Action::DeleteObjectAction) if version_id.is_some() => { + Some(Action::S3Action(S3Action::DeleteObjectVersionAction)) + } + _ => None, + } +} + +async fn get_or_fetch_object_tag_conditions( + req: &mut S3Request, + bucket: &str, + object: &str, + version_id: Option<&str>, + action: Action, +) -> S3Result>> { + if let Some(cached) = req.extensions.get::() + && cached.matches(bucket, object, version_id) + { + return Ok(cached.values.clone()); + } + + counter!("rustfs.object_tag_conditions.fetched", "op" => action_tag_metric_label(&action)).increment(1); + let fetched = auth_fs() + .get_object_tag_conditions_for_policy(bucket, object, version_id) + .await?; + req.extensions + .insert(ObjectTagConditions::new(bucket, object, version_id, fetched.clone())); + Ok(fetched) +} + +async fn maybe_merge_object_tag_conditions( + req: &mut S3Request, + action: Action, + bucket: &str, + object: &str, + version_id: Option<&str>, + conditions: &mut HashMap>, + needs_tag: bool, +) -> S3Result<()> { + if !needs_tag || bucket.is_empty() || object.is_empty() { + counter!("rustfs.object_tag_conditions.skipped", "op" => action_tag_metric_label(&action)).increment(1); + return Ok(()); + } + + let tags = get_or_fetch_object_tag_conditions(req, bucket, object, version_id, action).await?; + merge_object_tag_conditions(conditions, &tags); + Ok(()) +} + /// Returns true when the owner (root or parent=root credentials) may bypass bucket policy /// explicit Deny for this action. Per AWS S3, only GetBucketPolicy, PutBucketPolicy, and /// DeleteBucketPolicy have this bypass so the admin can recover from a misconfigured policy. @@ -120,11 +282,14 @@ pub(crate) fn owner_can_bypass_policy_deny(is_owner: bool, action: &Action) -> b /// Authorizes the request based on the action and credentials. pub async fn authorize_request(req: &mut S3Request, action: Action) -> S3Result<()> { let remote_addr = req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)); - let object_tag_conditions = req.extensions.get::().cloned(); - let req_info = req_info_ref(req)?; + let cred = req_info.cred.clone(); + let is_owner = req_info.is_owner; + let bucket = req_info.bucket.clone().unwrap_or_default(); + let object = req_info.object.clone().unwrap_or_default(); + let version_id = req_info.version_id.clone(); - if let Some(cred) = &req_info.cred { + if let Some(cred) = &cred { let Ok(iam_store) = rustfs_iam::get() else { return Err(S3Error::with_message( S3ErrorCode::InternalError, @@ -134,102 +299,174 @@ pub async fn authorize_request(req: &mut S3Request, action: Action) -> S3R let default_claims = HashMap::new(); let claims = cred.claims.as_ref().unwrap_or(&default_claims); - let mut conditions = get_condition_values_with_query( - &req.headers, - cred, - req_info.version_id.as_deref(), - None, - remote_addr, - req.uri.query(), - ); - // Merge object tag conditions; extend existing values if the same key exists (e.g. from get_condition_values). - if let Some(ref tags) = object_tag_conditions { - for (k, v) in &tags.0 { - conditions - .entry(k.clone()) - .and_modify(|existing| existing.extend(v.iter().cloned())) - .or_insert_with(|| v.clone()); + let mut conditions = + get_condition_values_with_query(&req.headers, cred, version_id.as_deref(), None, remote_addr, req.uri.query()); + + let action_args = Args { + account: &cred.access_key, + groups: &cred.groups, + action, + bucket: bucket.as_str(), + conditions: &conditions, + is_owner, + object: object.as_str(), + claims, + deny_only: false, + }; + let prepared = iam_store.prepare_auth(&action_args).await; + let mut needs_tag_from_iam = prepared.needs_existing_object_tag; + + let bucket_tag_hint = if !bucket.is_empty() && !object.is_empty() { + Some(load_bucket_policy_existing_object_tag_hint(bucket.as_str(), action).await) + } else { + None + }; + let mut needs_tag_from_bucket = if let Some(hint) = bucket_tag_hint.as_ref() { + let bucket_args = BucketPolicyArgs { + bucket: bucket.as_str(), + action, + is_owner, + account: cred.access_key.as_str(), + groups: &cred.groups, + conditions: &conditions, + object: object.as_str(), + }; + bucket_policy_needs_existing_object_tag_from_hint(hint, &bucket_args).await + } else { + false + }; + + let secondary_action = secondary_tag_hint_action(action, version_id.as_deref()); + if let Some(extra_action) = secondary_action { + let extra_args = Args { + account: &cred.access_key, + groups: &cred.groups, + action: extra_action, + bucket: bucket.as_str(), + conditions: &conditions, + is_owner, + object: object.as_str(), + claims, + deny_only: false, + }; + needs_tag_from_iam |= prepared.needs_existing_object_tag_for_args(&extra_args).await; + + if let Some(hint) = bucket_tag_hint.as_ref() { + let extra_bucket_args = BucketPolicyArgs { + bucket: bucket.as_str(), + action: extra_action, + is_owner, + account: cred.access_key.as_str(), + groups: &cred.groups, + conditions: &conditions, + object: object.as_str(), + }; + needs_tag_from_bucket |= bucket_policy_needs_existing_object_tag_from_hint(hint, &extra_bucket_args).await; } } - let bucket_name = req_info.bucket.as_deref().unwrap_or(""); + + let needs_tag = needs_tag_from_iam || needs_tag_from_bucket; + if needs_tag { + tracing::debug!( + bucket = %bucket, + ?action, + ?secondary_action, + needs_tag_from_iam, + needs_tag_from_bucket, + "authorize_request ExistingObjectTag hint requires tag conditions" + ); + } + maybe_merge_object_tag_conditions( + req, + action, + bucket.as_str(), + object.as_str(), + version_id.as_deref(), + &mut conditions, + needs_tag, + ) + .await?; + let bucket_name = bucket.as_str(); // Per AWS S3: root can always perform GetBucketPolicy, PutBucketPolicy, DeleteBucketPolicy // even if bucket policy explicitly denies. Other actions (ListBucket, GetObject, etc.) are // subject to bucket policy Deny for root as well. See: repost.aws/knowledge-center/s3-accidentally-denied-access // Here "owner" means root or credentials whose parent_user is root (e.g. Console admin via STS). - let owner_can_bypass_deny = owner_can_bypass_policy_deny(req_info.is_owner, &action); + let owner_can_bypass_deny = owner_can_bypass_policy_deny(is_owner, &action); if !bucket_name.is_empty() && !owner_can_bypass_deny && !PolicySys::is_allowed(&BucketPolicyArgs { bucket: bucket_name, action, - // Run this early check in deny-only mode so IAM fallback can still grant access. + // Early explicit-deny gate for bucket policy: use owner short-circuit path so + // deny statements are enforced before IAM/bucket allow fallback evaluation. is_owner: true, account: &cred.access_key, groups: &cred.groups, conditions: &conditions, - object: req_info.object.as_deref().unwrap_or(""), + object: object.as_str(), }) .await { return Err(s3_error!(AccessDenied, "Access Denied")); } - if action == Action::S3Action(S3Action::DeleteObjectAction) - && req_info.version_id.is_some() - && !iam_store - .is_allowed(&Args { - account: &cred.access_key, - groups: &cred.groups, - action: Action::S3Action(S3Action::DeleteObjectVersionAction), - bucket: req_info.bucket.as_deref().unwrap_or(""), - conditions: &conditions, - is_owner: req_info.is_owner, - object: req_info.object.as_deref().unwrap_or(""), - claims, - deny_only: false, - }) - .await - && !PolicySys::is_allowed(&BucketPolicyArgs { - bucket: req_info.bucket.as_deref().unwrap_or(""), - action: Action::S3Action(S3Action::DeleteObjectVersionAction), - is_owner: req_info.is_owner, + if action == Action::S3Action(S3Action::DeleteObjectAction) && version_id.is_some() { + let delete_version_args = Args { account: &cred.access_key, groups: &cred.groups, + action: Action::S3Action(S3Action::DeleteObjectVersionAction), + bucket: bucket.as_str(), conditions: &conditions, - object: req_info.object.as_deref().unwrap_or(""), - }) - .await - { - return Err(s3_error!(AccessDenied, "Access Denied")); + is_owner, + object: object.as_str(), + claims, + deny_only: false, + }; + let delete_version_allowed = iam_store.eval_prepared(&prepared, &delete_version_args).await; + if !delete_version_allowed + && !PolicySys::is_allowed(&BucketPolicyArgs { + bucket: bucket.as_str(), + action: Action::S3Action(S3Action::DeleteObjectVersionAction), + is_owner, + account: &cred.access_key, + groups: &cred.groups, + conditions: &conditions, + object: object.as_str(), + }) + .await + { + return Err(s3_error!(AccessDenied, "Access Denied")); + } } - let iam_allowed = iam_store - .is_allowed(&Args { + let iam_allowed = { + let final_args = Args { account: &cred.access_key, groups: &cred.groups, action, - bucket: req_info.bucket.as_deref().unwrap_or(""), + bucket: bucket.as_str(), conditions: &conditions, - is_owner: req_info.is_owner, - object: req_info.object.as_deref().unwrap_or(""), + is_owner, + object: object.as_str(), claims, deny_only: false, - }) - .await; + }; + iam_store.eval_prepared(&prepared, &final_args).await + }; if iam_allowed { return Ok(()); } let policy_allowed_fallback = PolicySys::is_allowed(&BucketPolicyArgs { - bucket: req_info.bucket.as_deref().unwrap_or(""), + bucket: bucket.as_str(), action, - is_owner: req_info.is_owner, + is_owner, account: &cred.access_key, groups: &cred.groups, conditions: &conditions, - object: req_info.object.as_deref().unwrap_or(""), + object: object.as_str(), }) .await; @@ -238,31 +475,30 @@ pub async fn authorize_request(req: &mut S3Request, action: Action) -> S3R } if action == Action::S3Action(S3Action::ListBucketVersionsAction) { - if iam_store - .is_allowed(&Args { - account: &cred.access_key, - groups: &cred.groups, - action: Action::S3Action(S3Action::ListBucketAction), - bucket: req_info.bucket.as_deref().unwrap_or(""), - conditions: &conditions, - is_owner: req_info.is_owner, - object: req_info.object.as_deref().unwrap_or(""), - claims, - deny_only: false, - }) - .await - { + let list_bucket_args = Args { + account: &cred.access_key, + groups: &cred.groups, + action: Action::S3Action(S3Action::ListBucketAction), + bucket: bucket.as_str(), + conditions: &conditions, + is_owner, + object: object.as_str(), + claims, + deny_only: false, + }; + let list_bucket_allowed = iam_store.eval_prepared(&prepared, &list_bucket_args).await; + if list_bucket_allowed { return Ok(()); } if PolicySys::is_allowed(&BucketPolicyArgs { - bucket: req_info.bucket.as_deref().unwrap_or(""), + bucket: bucket.as_str(), action: Action::S3Action(S3Action::ListBucketAction), - is_owner: req_info.is_owner, + is_owner, account: &cred.access_key, groups: &cred.groups, conditions: &conditions, - object: req_info.object.as_deref().unwrap_or(""), + object: object.as_str(), }) .await { @@ -270,35 +506,81 @@ pub async fn authorize_request(req: &mut S3Request, action: Action) -> S3R } } } else { + let default_cred = rustfs_credentials::Credentials::default(); let mut conditions = get_condition_values_with_query( &req.headers, - &rustfs_credentials::Credentials::default(), - req_info.version_id.as_deref(), + &default_cred, + version_id.as_deref(), req.region.clone(), remote_addr, req.uri.query(), ); - // Merge object tag conditions; extend existing values if the same key exists. - if let Some(ref tags) = object_tag_conditions { - for (k, v) in &tags.0 { - conditions - .entry(k.clone()) - .and_modify(|existing| existing.extend(v.iter().cloned())) - .or_insert_with(|| v.clone()); - } + + let no_groups: Option> = None; + let bucket_tag_hint = if !bucket.is_empty() && !object.is_empty() { + Some(load_bucket_policy_existing_object_tag_hint(bucket.as_str(), action).await) + } else { + None + }; + let mut needs_tag_from_bucket = if let Some(hint) = bucket_tag_hint.as_ref() { + let bucket_args = BucketPolicyArgs { + bucket: bucket.as_str(), + action, + is_owner: false, + account: "", + groups: &no_groups, + conditions: &conditions, + object: object.as_str(), + }; + bucket_policy_needs_existing_object_tag_from_hint(hint, &bucket_args).await + } else { + false + }; + let secondary_action = secondary_tag_hint_action(action, version_id.as_deref()); + if let Some(extra_action) = secondary_action + && let Some(hint) = bucket_tag_hint.as_ref() + { + let extra_bucket_args = BucketPolicyArgs { + bucket: bucket.as_str(), + action: extra_action, + is_owner: false, + account: "", + groups: &no_groups, + conditions: &conditions, + object: object.as_str(), + }; + needs_tag_from_bucket |= bucket_policy_needs_existing_object_tag_from_hint(hint, &extra_bucket_args).await; } - let bucket_name = req_info.bucket.as_deref().unwrap_or(""); + if needs_tag_from_bucket { + tracing::debug!( + bucket = %bucket, + ?action, + ?secondary_action, + "anonymous authorize_request ExistingObjectTag hint requires tag conditions" + ); + } + maybe_merge_object_tag_conditions( + req, + action, + bucket.as_str(), + object.as_str(), + version_id.as_deref(), + &mut conditions, + needs_tag_from_bucket, + ) + .await?; + let bucket_name = bucket.as_str(); if !bucket_name.is_empty() && !PolicySys::is_allowed(&BucketPolicyArgs { bucket: bucket_name, action, - // Run this early check in deny-only mode so later policy checks are not bypassed. + // Early explicit-deny gate for bucket policy in anonymous path. is_owner: true, account: "", groups: &None, conditions: &conditions, - object: req_info.object.as_deref().unwrap_or(""), + object: object.as_str(), }) .await { @@ -306,14 +588,30 @@ pub async fn authorize_request(req: &mut S3Request, action: Action) -> S3R } if action != Action::S3Action(S3Action::ListAllMyBucketsAction) { + if action == Action::S3Action(S3Action::DeleteObjectAction) && version_id.is_some() { + let delete_version_allowed = PolicySys::is_allowed(&BucketPolicyArgs { + bucket: bucket.as_str(), + action: Action::S3Action(S3Action::DeleteObjectVersionAction), + is_owner: false, + account: "", + groups: &None, + conditions: &conditions, + object: object.as_str(), + }) + .await; + if !delete_version_allowed { + return Err(s3_error!(AccessDenied, "Access Denied")); + } + } + let policy_allowed = PolicySys::is_allowed(&BucketPolicyArgs { - bucket: req_info.bucket.as_deref().unwrap_or(""), + bucket: bucket.as_str(), action, is_owner: false, account: "", groups: &None, conditions: &conditions, - object: req_info.object.as_deref().unwrap_or(""), + object: object.as_str(), }) .await; @@ -335,7 +633,7 @@ pub async fn authorize_request(req: &mut S3Request, action: Action) -> S3R if action == Action::S3Action(S3Action::ListBucketVersionsAction) && PolicySys::is_allowed(&BucketPolicyArgs { - bucket: req_info.bucket.as_deref().unwrap_or(""), + bucket: bucket.as_str(), action: Action::S3Action(S3Action::ListBucketAction), is_owner: false, account: "", @@ -412,20 +710,6 @@ fn validate_post_object_success_controls(input: &PostObjectInput) -> S3Result<() #[async_trait::async_trait] impl S3Access for FS { - // /// Checks whether the current request has accesses to the resources. - // /// - // /// This method is called before deserializing the operation input. - // /// - // /// By default, this method rejects all anonymous requests - // /// and returns [`AccessDenied`](crate::S3ErrorCode::AccessDenied) error. - // /// - // /// An access control provider can override this method to implement custom logic. - // /// - // /// Common fields in the context: - // /// + [`cx.credentials()`](S3AccessContext::credentials) - // /// + [`cx.s3_path()`](S3AccessContext::s3_path) - // /// + [`cx.s3_op().name()`](crate::S3Operation::name) - // /// + [`cx.extensions_mut()`](S3AccessContext::extensions_mut) async fn check(&self, cx: &mut S3AccessContext<'_>) -> S3Result<()> { // Upper layer has verified ak/sk // info!( @@ -525,11 +809,6 @@ impl S3Access for FS { req_info.object = Some(src_key.clone()); req_info.version_id = version_id.clone(); - let tag_conds = self - .fetch_tag_conditions(&src_bucket, &src_key, version_id.as_deref(), "copy_object_src") - .await?; - req.extensions.insert(tag_conds); - authorize_request(req, Action::S3Action(S3Action::GetObjectAction)).await?; } @@ -719,11 +998,6 @@ impl S3Access for FS { req_info.object = Some(req.input.key.clone()); req_info.version_id = req.input.version_id.clone(); - let tag_conds = self - .fetch_tag_conditions(&req.input.bucket, &req.input.key, req.input.version_id.as_deref(), "delete_object") - .await?; - req.extensions.insert(tag_conds); - authorize_request(req, Action::S3Action(S3Action::DeleteObjectAction)).await?; // S3 Standard: When bypass_governance header is set, must have s3:BypassGovernanceRetention permission @@ -743,16 +1017,6 @@ impl S3Access for FS { req_info.object = Some(req.input.key.clone()); req_info.version_id = req.input.version_id.clone(); - let tag_conds = self - .fetch_tag_conditions( - &req.input.bucket, - &req.input.key, - req.input.version_id.as_deref(), - "delete_object_tagging", - ) - .await?; - req.extensions.insert(tag_conds); - authorize_request(req, Action::S3Action(S3Action::DeleteObjectTaggingAction)).await } @@ -997,11 +1261,6 @@ impl S3Access for FS { req_info.object = Some(req.input.key.clone()); req_info.version_id = req.input.version_id.clone(); - let tag_conds = self - .fetch_tag_conditions(&req.input.bucket, &req.input.key, req.input.version_id.as_deref(), "get_object") - .await?; - req.extensions.insert(tag_conds); - authorize_request(req, Action::S3Action(S3Action::GetObjectAction)).await } @@ -1026,16 +1285,6 @@ impl S3Access for FS { req_info.object = Some(req.input.key.clone()); req_info.version_id = req.input.version_id.clone(); - let tag_conds = self - .fetch_tag_conditions( - &req.input.bucket, - &req.input.key, - req.input.version_id.as_deref(), - "get_object_attributes", - ) - .await?; - req.extensions.insert(tag_conds); - if req.input.version_id.is_some() { authorize_request(req, Action::S3Action(S3Action::GetObjectVersionAttributesAction)).await?; authorize_request(req, Action::S3Action(S3Action::GetObjectVersionAction)).await?; @@ -1090,11 +1339,6 @@ impl S3Access for FS { req_info.object = Some(req.input.key.clone()); req_info.version_id = req.input.version_id.clone(); - let tag_conds = self - .fetch_tag_conditions(&req.input.bucket, &req.input.key, req.input.version_id.as_deref(), "get_object_tagging") - .await?; - req.extensions.insert(tag_conds); - authorize_request(req, Action::S3Action(S3Action::GetObjectTaggingAction)).await } @@ -1134,11 +1378,6 @@ impl S3Access for FS { req_info.object = Some(req.input.key.clone()); req_info.version_id = req.input.version_id.clone(); - let tag_conds = self - .fetch_tag_conditions(&req.input.bucket, &req.input.key, req.input.version_id.as_deref(), "head_object") - .await?; - req.extensions.insert(tag_conds); - authorize_request(req, Action::S3Action(S3Action::GetObjectAction)).await } @@ -1523,11 +1762,6 @@ impl S3Access for FS { req_info.object = Some(req.input.key.clone()); req_info.version_id = req.input.version_id.clone(); - let tag_conds = self - .fetch_tag_conditions(&req.input.bucket, &req.input.key, req.input.version_id.as_deref(), "put_object_tagging") - .await?; - req.extensions.insert(tag_conds); - authorize_request(req, Action::S3Action(S3Action::PutObjectTaggingAction)).await } @@ -1593,11 +1827,6 @@ impl S3Access for FS { req_info.object = Some(src_key.clone()); req_info.version_id = version_id.clone(); - let tag_conds = self - .fetch_tag_conditions(&src_bucket, &src_key, version_id.as_deref(), "upload_part_copy_src") - .await?; - req.extensions.insert(tag_conds); - authorize_request(req, Action::S3Action(S3Action::GetObjectAction)).await?; } @@ -1621,6 +1850,7 @@ impl S3Access for FS { mod tests { use super::*; use http::{HeaderMap, Method, Uri}; + use rustfs_policy::policy::{BucketPolicy, bucket_policy_uses_existing_object_tag_conditions}; use std::collections::HashMap; use time::OffsetDateTime; @@ -1729,10 +1959,10 @@ mod tests { let mut tags = HashMap::new(); tags.insert("ExistingObjectTag/security".to_string(), vec!["public".to_string()]); tags.insert("ExistingObjectTag/project".to_string(), vec!["webapp".to_string()]); - let object_tag_conditions = ObjectTagConditions(tags); + let object_tag_conditions = ObjectTagConditions::new("bucket", "object", None, tags); let mut conditions = HashMap::new(); conditions.insert("delimiter".to_string(), vec!["/".to_string()]); - for (k, v) in &object_tag_conditions.0 { + for (k, v) in &object_tag_conditions.values { conditions.insert(k.clone(), v.clone()); } assert_eq!(conditions.get("ExistingObjectTag/security"), Some(&vec!["public".to_string()])); @@ -1740,11 +1970,100 @@ mod tests { assert_eq!(conditions.get("delimiter"), Some(&vec!["/".to_string()])); } - /// When bucket has no policy or policy fetch fails, tag-based check is skipped (returns false). + /// When policy metadata cannot be loaded, tag-based check is conservative (returns true). #[tokio::test] - async fn test_bucket_policy_uses_existing_object_tag_no_policy() { - let result = bucket_policy_uses_existing_object_tag("test-bucket-no-policy-xyz-absent").await; - assert!(!result, "bucket with no policy should not use ExistingObjectTag"); + async fn test_bucket_policy_needs_existing_object_tag_load_failure_is_conservative() { + let conditions = HashMap::new(); + let hint = load_bucket_policy_existing_object_tag_hint( + "test-bucket-no-policy-xyz-absent", + Action::S3Action(S3Action::GetObjectAction), + ) + .await; + let no_groups: Option> = None; + let args = BucketPolicyArgs { + bucket: "test-bucket-no-policy-xyz-absent", + action: Action::S3Action(S3Action::GetObjectAction), + is_owner: false, + account: "", + groups: &no_groups, + conditions: &conditions, + object: "obj", + }; + let result = bucket_policy_needs_existing_object_tag_from_hint(&hint, &args).await; + assert!( + result, + "when policy metadata cannot be loaded, ExistingObjectTag should be fetched conservatively" + ); + } + + #[test] + fn test_bucket_policy_existing_object_tag_condition_key_detection() { + let condition_key_policy = r#"{ + "Version":"2012-10-17", + "Statement":[{ + "Effect":"Allow", + "Principal":"*", + "Action":["s3:GetObject"], + "Resource":["arn:aws:s3:::bucket/*"], + "Condition":{"StringEquals":{"s3:ExistingObjectTag/security":"public"}} + }] +}"#; + let policy: BucketPolicy = serde_json::from_str(condition_key_policy).expect("valid bucket policy JSON"); + assert!( + bucket_policy_uses_existing_object_tag_conditions(&policy), + "ExistingObjectTag in condition key must be detected" + ); + + let value_only_policy = r#"{ + "Version":"2012-10-17", + "Statement":[{ + "Effect":"Allow", + "Principal":"*", + "Action":["s3:GetObject"], + "Resource":["arn:aws:s3:::bucket/*"], + "Condition":{"StringEquals":{"s3:prefix":"ExistingObjectTag/security"}} + }] +}"#; + let policy: BucketPolicy = serde_json::from_str(value_only_policy).expect("valid bucket policy JSON"); + assert!( + !bucket_policy_uses_existing_object_tag_conditions(&policy), + "ExistingObjectTag text in values should not trigger tag dependency" + ); + } + + #[test] + fn test_unparsable_bucket_policy_json_implies_conservative_existing_object_tag_fetch() { + // Matches `load_bucket_policy_existing_object_tag_hint`: unparsable policy => conservative tag fetch. + let malformed = r#"{"Version":"2012-10-17","Statement":[INVALID]}"#; + assert!(serde_json::from_str::(malformed).is_err()); + let conservative_fetch = serde_json::from_str::(malformed) + .map(|p| bucket_policy_uses_existing_object_tag_conditions(&p)) + .unwrap_or(true); + assert!(conservative_fetch); + + // Invalid JSON that still contains real ExistingObjectTag condition keys (trailing comma). + let malformed_with_tag_keys = r#"{"Version":"2012-10-17","Statement":[{"Condition":{"StringEquals":{"s3:ExistingObjectTag/security":"public"}}},]}"#; + assert!(serde_json::from_str::(malformed_with_tag_keys).is_err()); + let conservative_with_tag_keys = serde_json::from_str::(malformed_with_tag_keys) + .map(|p| bucket_policy_uses_existing_object_tag_conditions(&p)) + .unwrap_or(true); + assert!(conservative_with_tag_keys); + } + + #[test] + fn test_classify_bucket_policy_raw_load_error() { + assert_eq!( + classify_bucket_policy_raw_load_error(&StorageError::ConfigNotFound), + BucketPolicyRawLoadErrorKind::PolicyMissing + ); + assert_eq!( + classify_bucket_policy_raw_load_error(&StorageError::BucketNotFound("b".to_string())), + BucketPolicyRawLoadErrorKind::BucketMissing + ); + assert_eq!( + classify_bucket_policy_raw_load_error(&StorageError::Io(std::io::Error::other("boom"))), + BucketPolicyRawLoadErrorKind::Other + ); } /// Owner can bypass bucket policy Deny only for the three policy management APIs (per AWS S3). @@ -1767,6 +2086,87 @@ mod tests { )); } + #[test] + fn test_secondary_tag_hint_action_for_delete_object_version() { + assert_eq!( + secondary_tag_hint_action(Action::S3Action(S3Action::DeleteObjectAction), Some("v1")), + Some(Action::S3Action(S3Action::DeleteObjectVersionAction)) + ); + assert_eq!(secondary_tag_hint_action(Action::S3Action(S3Action::DeleteObjectAction), None), None); + assert_eq!( + secondary_tag_hint_action(Action::S3Action(S3Action::ListBucketVersionsAction), None), + None + ); + } + + #[tokio::test] + async fn test_anonymous_delete_object_with_version_requires_secondary_policy_and_tag_hint() { + let policy: BucketPolicy = serde_json::from_str( + r#"{ + "Version":"2012-10-17", + "Statement":[ + { + "Effect":"Allow", + "Principal":{"AWS":"*"}, + "Action":["s3:DeleteObject"], + "Resource":["arn:aws:s3:::bucket/*"] + }, + { + "Effect":"Allow", + "Principal":{"AWS":"*"}, + "Action":["s3:DeleteObjectVersion"], + "Resource":["arn:aws:s3:::bucket/*"], + "Condition":{"StringEquals":{"s3:ExistingObjectTag/security":"public"}} + } + ] +}"#, + ) + .expect("bucket policy should parse"); + let hint = BucketPolicyExistingObjectTagHint::Parsed(policy.clone()); + let no_groups: Option> = None; + let conditions = HashMap::new(); + + let args_delete = BucketPolicyArgs { + bucket: "bucket", + action: Action::S3Action(S3Action::DeleteObjectAction), + is_owner: false, + account: "", + groups: &no_groups, + conditions: &conditions, + object: "obj", + }; + assert!( + policy.is_allowed(&args_delete).await, + "anonymous DeleteObject can be allowed by bucket policy" + ); + + let args_delete_version = BucketPolicyArgs { + bucket: "bucket", + action: Action::S3Action(S3Action::DeleteObjectVersionAction), + is_owner: false, + account: "", + groups: &no_groups, + conditions: &conditions, + object: "obj", + }; + assert!( + !policy.is_allowed(&args_delete_version).await, + "DeleteObjectVersion should still be denied without matching ExistingObjectTag conditions" + ); + + let needs_tag_main = bucket_policy_needs_existing_object_tag_from_hint(&hint, &args_delete).await; + let needs_tag_secondary = bucket_policy_needs_existing_object_tag_from_hint(&hint, &args_delete_version).await; + assert!(!needs_tag_main, "DeleteObject statement itself does not require ExistingObjectTag"); + assert!( + needs_tag_secondary, + "DeleteObjectVersion statement requires ExistingObjectTag when version delete is evaluated" + ); + assert!( + needs_tag_main || needs_tag_secondary, + "combined primary+secondary check must require tag fetch for DeleteObject(versionId)" + ); + } + #[tokio::test] async fn post_object_marks_request_extensions() { let input = PostObjectInput::builder()