Merge commit from fork

* fix(policy): quantify negated string conditions per value ForAllValues:/ForAnyValue: negated string operators computed the positive quantified match and then negated the aggregate. That yields NOT(all match) and NOT(any match), which is the semantics of the *other* quantifier, so ForAllValues:StringNotEquals and ForAnyValue:StringNotEquals were exactly transposed. The same applied to StringNotEqualsIgnoreCase, StringNotLike, ArnNotEquals and ArnNotLike. Introduce an explicit Quantifier and push negation into the per-value predicate for the qualified forms, so ForAllValues requires every request value to satisfy the operator and ForAnyValue requires at least one. Unqualified operators keep negating the aggregate, preserving AWS single-valued-key semantics. Absent keys now follow AWS: ForAllValues is vacuously satisfied, ForAnyValue is not. A request value set that is fully contained in or fully disjoint from the policy set cannot distinguish the two quantifiers, which is why the existing cases missed this; the new tests use partially overlapping sets. * fix(auth): keep request headers out of server-derived condition keys get_condition_values folded every remaining request header into the policy condition map. HeaderMap lowercases header names, and the server-derived keys userid, username, principaltype, versionid and signatureversion are lowercase too, so a header of the same name collided with them. The collision branch used extend(), and non-quantified string operators match if ANY value in the vector matches, so sending `userid: admin` was enough to satisfy a condition on aws:userid. The jwt:/ldap: claim keys were reachable the same way whenever the credential carried no such claim. groups was worse than an append: the header loop ran before the cred.groups block, which is gated on !args.contains_key("groups"), so a `groups:` header both injected a value and suppressed the credential's real group list. Resolve claims and group membership before merging headers, then skip any header naming a key the server already derived or a well-known identity/context key. The reserved set comes from KeyName so it tracks the key registry; s3:x-amz-* keys stay mergeable because they mirror request headers by design. * fix(access): gate the ListBucketVersions fallback on public-access checks An anonymous request for ListObjectVersions that the bucket policy does not grant directly falls back to re-checking the grant as s3:ListBucket. That fallback returned Ok(()) straight away, skipping the two gates the direct grant passes through: deny_anonymous_table_data_plane_if_needed and the RestrictPublicBuckets check on the bucket's public-access block. So a bucket whose policy allows anonymous s3:ListBucket kept serving anonymous version listings after an operator enabled RestrictPublicBuckets, even though the equivalent GetObject was correctly denied. Fold the fallback into policy_allowed so both routes reach the same gates. * fix(ftps): authorize MKD against the CreateBucket boundary FTPS MKD creates a bucket but ran no authorization check, so any principal that could open an FTPS session could create buckets regardless of policy. Every other operation in this driver authorizes first — LIST, RETR, STOR, DELE and RMD all call authorize_operation — and the WebDAV gateway checks S3Action::CreateBucket on the equivalent path. Add the matching check so MKD clears the same boundary as an S3 CreateBucket.
2026-08-08 06:13:14 +00:00 · 2026-07-26 06:14:01 +08:00
parent 21787a4742
commit 92f83bfe15
10 changed files with 477 additions and 87 deletions
@@ -20,7 +20,7 @@ use rustfs_iam::error::Error as IamError;
 use rustfs_iam::sys::{
    SESSION_POLICY_NAME, get_claims_from_token_with_secret, get_claims_from_token_with_secret_allow_missing_exp,
 };
-use rustfs_policy::policy::{ClaimLookup, get_claim_case_insensitive};
+use rustfs_policy::policy::{ClaimLookup, get_claim_case_insensitive, is_server_derived_condition_key};
 use rustfs_trusted_proxies::ClientInfo;
 use rustfs_utils::MaskedAccessKey;
 use rustfs_utils::http::{AMZ_OBJECT_LOCK_LEGAL_HOLD_LOWER, AMZ_OBJECT_LOCK_MODE_LOWER, AMZ_OBJECT_LOCK_RETAIN_UNTIL_DATE_LOWER};
@@ -744,24 +744,8 @@ pub fn get_condition_values_with_query_and_client_info(
        clone_header.remove(*grant_header);
    }

-    for (key, _values) in clone_header.iter() {
-        if key.as_str().eq_ignore_ascii_case("x-amz-tagging") {
-            continue;
-        }
-        if let Some(existing_values) = args.get_mut(key.as_str()) {
-            existing_values.extend(clone_header.get_all(key).iter().map(|v| v.to_str().unwrap_or("").to_string()));
-        } else {
-            args.insert(
-                key.as_str().to_string(),
-                header
-                    .get_all(key)
-                    .iter()
-                    .map(|v| v.to_str().unwrap_or("").to_string())
-                    .collect(),
-            );
-        }
-    }
-
+    // Claims and group membership are part of the verified identity, so they are
+    // resolved before request headers are merged in below.
    if let Some(claims) = &cred.claims {
        for (k, v) in claims {
            if let Some(v_str) = v.as_str() {
@@ -786,9 +770,40 @@ pub fn get_condition_values_with_query_and_client_info(
        args.insert("groups".to_string(), groups.clone());
    }

+    // Every remaining header is attacker-controlled. A header must never contribute
+    // to a condition key that describes the caller's own identity or the connection,
+    // otherwise sending `userid: admin` (or any `jwt:`/`ldap:` claim name) would let a
+    // request satisfy a policy condition about itself. Reject those names outright --
+    // both the ones already populated above and the well-known identity keys that are
+    // absent for this credential, since an absent key is exactly what a spoofed header
+    // would fill in.
+    for key in clone_header.keys() {
+        if key.as_str().eq_ignore_ascii_case("x-amz-tagging") {
+            continue;
+        }
+        if is_reserved_condition_key(key.as_str(), &args) {
+            continue;
+        }
+        args.insert(
+            key.as_str().to_string(),
+            header
+                .get_all(key)
+                .iter()
+                .map(|v| v.to_str().unwrap_or("").to_string())
+                .collect(),
+        );
+    }
+
    args
 }

+/// Whether a request header is forbidden from contributing to policy condition key
+/// `key`, either because the server already derived that key from verified state or
+/// because it is a well-known identity/context key that only the server may populate.
+fn is_reserved_condition_key(key: &str, server_derived: &HashMap<String, Vec<String>>) -> bool {
+    server_derived.contains_key(key) || is_server_derived_condition_key(key)
+}
+
 /// Get request authentication type
 ///
 /// # Arguments
@@ -1329,6 +1344,62 @@ mod tests {
        assert_eq!(conditions.get("principaltype"), Some(&vec!["AssumedRole".to_string()]));
    }

+    #[test]
+    fn test_identity_condition_keys_ignore_spoofed_headers() {
+        let cred = create_test_credentials();
+        let mut headers = HeaderMap::new();
+        // A caller naming its headers after identity condition keys must not be able
+        // to add or replace values the server derives from the credential.
+        headers.insert("userid", "admin".parse().unwrap());
+        headers.insert("username", "admin".parse().unwrap());
+        headers.insert("principaltype", "Account".parse().unwrap());
+        headers.insert("signatureversion", "AWS4-HMAC-SHA256".parse().unwrap());
+
+        let conditions = get_condition_values(&headers, &cred, None, None, None);
+
+        assert_eq!(conditions.get("userid"), Some(&vec!["test-access-key".to_string()]));
+        assert_eq!(conditions.get("username"), Some(&vec!["test-access-key".to_string()]));
+        assert_eq!(conditions.get("principaltype"), Some(&vec!["User".to_string()]));
+        assert!(
+            !conditions
+                .get("signatureversion")
+                .is_some_and(|v| v.iter().any(|s| s == "AWS4-HMAC-SHA256")),
+            "an unsigned request must not gain a signatureversion from a header"
+        );
+    }
+
+    #[test]
+    fn test_claim_condition_keys_ignore_spoofed_headers() {
+        // The credential carries no groups/roles claims, so these keys are absent --
+        // precisely the case a spoofed header would otherwise fill in.
+        let cred = create_test_credentials();
+        let mut headers = HeaderMap::new();
+        headers.insert("groups", "admins".parse().unwrap());
+        headers.insert("roles", "RustFS.ConsoleAdmin".parse().unwrap());
+        headers.insert("sub", "someone-else".parse().unwrap());
+
+        let conditions = get_condition_values(&headers, &cred, None, None, None);
+
+        assert_eq!(conditions.get("groups"), None, "groups must come from the credential only");
+        assert_eq!(conditions.get("roles"), None, "roles must come from claims only");
+        assert_eq!(conditions.get("sub"), None, "jwt claim keys must come from claims only");
+    }
+
+    #[test]
+    fn test_request_headers_still_reach_conditions() {
+        // The reserved list must stay narrow: ordinary request headers, including the
+        // `s3:x-amz-*` condition keys, are still expected to be available to policies.
+        let cred = create_test_credentials();
+        let mut headers = HeaderMap::new();
+        headers.insert("x-amz-content-sha256", "UNSIGNED-PAYLOAD".parse().unwrap());
+        headers.insert("x-amz-server-side-encryption", "AES256".parse().unwrap());
+
+        let conditions = get_condition_values(&headers, &cred, None, None, None);
+
+        assert_eq!(conditions.get("x-amz-content-sha256"), Some(&vec!["UNSIGNED-PAYLOAD".to_string()]));
+        assert_eq!(conditions.get("x-amz-server-side-encryption"), Some(&vec!["AES256".to_string()]));
+    }
+
    #[test]
    fn test_get_condition_values_with_object_lock_headers() {
        let cred = create_test_credentials();
@@ -731,6 +731,23 @@ pub async fn authorize_request<T>(req: &mut S3Request<T>, action: Action) -> S3R
            })
            .await;

+            // A bucket policy granting s3:ListBucket also covers listing versions. This
+            // fallback has to feed the same post-authorization gates as the direct grant
+            // below, otherwise a public bucket keeps serving anonymous
+            // ListObjectVersions after RestrictPublicBuckets is turned on.
+            let policy_allowed = policy_allowed
+                || (action == Action::S3Action(S3Action::ListBucketVersionsAction)
+                    && PolicySys::is_allowed(&BucketPolicyArgs {
+                        bucket: bucket.as_str(),
+                        action: Action::S3Action(S3Action::ListBucketAction),
+                        is_owner: false,
+                        account: "",
+                        groups: &None,
+                        conditions: &conditions,
+                        object: "",
+                    })
+                    .await);
+
            if policy_allowed {
                deny_anonymous_table_data_plane_if_needed(action, bucket.as_str(), object.as_str()).await?;
                // RestrictPublicBuckets: when true, deny public access even if bucket policy allows it.
@@ -747,21 +764,6 @@ pub async fn authorize_request<T>(req: &mut S3Request<T>, action: Action) -> S3R
                }
                return Ok(());
            }
-
-            if action == Action::S3Action(S3Action::ListBucketVersionsAction)
-                && PolicySys::is_allowed(&BucketPolicyArgs {
-                    bucket: bucket.as_str(),
-                    action: Action::S3Action(S3Action::ListBucketAction),
-                    is_owner: false,
-                    account: "",
-                    groups: &None,
-                    conditions: &conditions,
-                    object: "",
-                })
-                .await
-            {
-                return Ok(());
-            }
        }
    }