rgw/sts: code to check IAM policy and return an

appropriate error incase Resource specified in the
IAM policy is incorrect and is discarded. The IAM
policy can be a resource policy or an identity policy.
This is for policies that have already been set.

Fixes: https://tracker.ceph.com/issues/51219
Signed-off-by: Pritha Srivastava <prsrivas@redhat.com>

diff --git a/src/rgw/rgw_common.cc b/src/rgw/rgw_common.cc
index d846aeac99a1b697cf1c81365956d4b07cae6b19..20b126c53d7dded0803a1f40a24b5de4d574d786 100644 (file)
--- a/src/rgw/rgw_common.cc
+++ b/src/rgw/rgw_common.cc
@@ -1053,12 +1053,12 @@ Effect eval_or_pass(const boost::optional<Policy>& policy,
                    const rgw::IAM::Environment& env,
                    boost::optional<const rgw::auth::Identity&> id,
                    const uint64_t op,
-                   const ARN& arn,
+                   const ARN& resource,
                                boost::optional<rgw::IAM::PolicyPrincipal&> princ_type=boost::none) {
   if (!policy)
     return Effect::Pass;
   else
-    return policy->eval(env, id, op, arn, princ_type);
+    return policy->eval(env, id, op, resource, princ_type);
 }
 
 }
@@ -1067,10 +1067,10 @@ Effect eval_identity_or_session_policies(const vector<Policy>& policies,
                           const rgw::IAM::Environment& env,
                           boost::optional<const rgw::auth::Identity&> id,
                           const uint64_t op,
-                          const ARN& arn) {
+                          const ARN& resource) {
   auto policy_res = Effect::Pass, prev_res = Effect::Pass;
   for (auto& policy : policies) {
-    if (policy_res = eval_or_pass(policy, env, id, op, arn); policy_res == Effect::Deny)
+    if (policy_res = eval_or_pass(policy, env, id, op, resource); policy_res == Effect::Deny)
       return policy_res;
     else if (policy_res == Effect::Allow)
       prev_res = Effect::Allow;

diff --git a/src/rgw/rgw_iam_policy.cc b/src/rgw/rgw_iam_policy.cc
index 4a5595b22b3a4505e7fa1c055db99512483d46e5..1ecb74281a73bc884f14f88ac36ff37c784b45a9 100644 (file)
--- a/src/rgw/rgw_iam_policy.cc
+++ b/src/rgw/rgw_iam_policy.cc
@@ -1027,23 +1027,29 @@ ostream& operator <<(ostream& m, const Condition& c) {
 
 Effect Statement::eval(const Environment& e,
                       boost::optional<const rgw::auth::Identity&> ida,
-                      uint64_t act, const ARN& res, boost::optional<PolicyPrincipal&> princ_type) const {
+                      uint64_t act, boost::optional<const ARN&> res, boost::optional<PolicyPrincipal&> princ_type) const {
 
   if (eval_principal(e, ida, princ_type) == Effect::Deny) {
     return Effect::Pass;
   }
 
-  if (!resource.empty()) {
+  if (res && resource.empty() && notresource.empty()) {
+    return Effect::Pass;
+  }
+  if (!res && (!resource.empty() || !notresource.empty())) {
+    return Effect::Pass;
+  }
+  if (!resource.empty() && res) {
     if (!std::any_of(resource.begin(), resource.end(),
           [&res](const ARN& pattern) {
-            return pattern.match(res);
+            return pattern.match(*res);
           })) {
       return Effect::Pass;
     }
-  } else if (!notresource.empty()) {
+  } else if (!notresource.empty() && res) {
     if (std::any_of(notresource.begin(), notresource.end(),
           [&res](const ARN& pattern) {
-            return pattern.match(res);
+            return pattern.match(*res);
           })) {
       return Effect::Pass;
     }
@@ -1481,7 +1487,7 @@ Policy::Policy(CephContext* cct, const string& tenant,
 
 Effect Policy::eval(const Environment& e,
                    boost::optional<const rgw::auth::Identity&> ida,
-                   std::uint64_t action, const ARN& resource,
+                   std::uint64_t action, boost::optional<const ARN&> resource,
         boost::optional<PolicyPrincipal&> princ_type) const {
   auto allowed = false;
   for (auto& s : statements) {

diff --git a/src/rgw/rgw_iam_policy.h b/src/rgw/rgw_iam_policy.h
index 3689657e4681cc7e9ebcba28f77f8ed46b35bc79..2de1b7bd58b9f83a5e73334bc76b3bc7d9a7b8f1 100644 (file)
--- a/src/rgw/rgw_iam_policy.h
+++ b/src/rgw/rgw_iam_policy.h
@@ -483,7 +483,7 @@ struct Statement {
 
   Effect eval(const Environment& e,
              boost::optional<const rgw::auth::Identity&> ida,
-             std::uint64_t action, const ARN& resource, boost::optional<PolicyPrincipal&> princ_type=boost::none) const;
+             std::uint64_t action, boost::optional<const ARN&> resource, boost::optional<PolicyPrincipal&> princ_type=boost::none) const;
 
   Effect eval_principal(const Environment& e,
                       boost::optional<const rgw::auth::Identity&> ida, boost::optional<PolicyPrincipal&> princ_type=boost::none) const;
@@ -515,7 +515,7 @@ struct Policy {
 
   Effect eval(const Environment& e,
              boost::optional<const rgw::auth::Identity&> ida,
-             std::uint64_t action, const ARN& resource, boost::optional<PolicyPrincipal&> princ_type=boost::none) const;
+             std::uint64_t action, boost::optional<const ARN&> resource, boost::optional<PolicyPrincipal&> princ_type=boost::none) const;
 
   Effect eval_principal(const Environment& e,
              boost::optional<const rgw::auth::Identity&> ida, boost::optional<PolicyPrincipal&> princ_type=boost::none) const;

diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc
index 99c3d36d236436436e340c6fdc5653e2ff04ed44..25bf48daff905193b8933af4f97d344c51e18b08 100644 (file)
--- a/src/rgw/rgw_op.cc
+++ b/src/rgw/rgw_op.cc
@@ -418,7 +418,8 @@ static int read_obj_policy(const DoutPrefixProvider *dpp,
       if (r == Effect::Deny)
         return -EACCES;
       if (policy) {
-        r = policy->eval(s->env, *s->auth.identity, rgw::IAM::s3ListBucket, ARN(bucket->get_key()));
+        ARN b_arn(bucket->get_key());
+        r = policy->eval(s->env, *s->auth.identity, rgw::IAM::s3ListBucket, b_arn);
         if (r == Effect::Allow)
           return -ENOENT;
         if (r == Effect::Deny)
@@ -3564,23 +3565,25 @@ int RGWPutObj::verify_permission(optional_yield y)
         if (has_s3_existing_tag || has_s3_resource_tag)
           rgw_iam_add_objtags(this, s, cs_object.get(), has_s3_existing_tag, has_s3_resource_tag);
         auto usr_policy_res = Effect::Pass;
+        rgw::ARN obj_arn(cs_object->get_obj());
         for (auto& user_policy : s->iam_user_policies) {
           if (usr_policy_res = user_policy.eval(s->env, *s->auth.identity,
                              cs_object->get_instance().empty() ?
                              rgw::IAM::s3GetObject :
                              rgw::IAM::s3GetObjectVersion,
-                             rgw::ARN(cs_object->get_obj())); usr_policy_res == Effect::Deny)
+                             obj_arn); usr_policy_res == Effect::Deny)
             return -EACCES;
           else if (usr_policy_res == Effect::Allow)
             break;
         }
   rgw::IAM::Effect e = Effect::Pass;
   if (policy) {
+    rgw::ARN obj_arn(cs_object->get_obj());
          e = policy->eval(s->env, *s->auth.identity,
                              cs_object->get_instance().empty() ?
                              rgw::IAM::s3GetObject :
                              rgw::IAM::s3GetObjectVersion,
-                             rgw::ARN(cs_object->get_obj()));
+                             obj_arn);
   }
        if (e == Effect::Deny) {
          return -EACCES; 
@@ -3651,9 +3654,10 @@ int RGWPutObj::verify_permission(optional_yield y)
     rgw::IAM::Effect e = Effect::Pass;
     rgw::IAM::PolicyPrincipal princ_type = rgw::IAM::PolicyPrincipal::Other;
     if (s->iam_policy) {
+      ARN obj_arn(s->object->get_obj());
       e = s->iam_policy->eval(s->env, *s->auth.identity,
           rgw::IAM::s3PutObject,
-          s->object->get_obj(),
+          obj_arn,
           princ_type);
     }
     if (e == Effect::Deny) {
@@ -4232,9 +4236,10 @@ void RGWPostObj::execute(optional_yield y)
     rgw::IAM::Effect e = Effect::Pass;
     rgw::IAM::PolicyPrincipal princ_type = rgw::IAM::PolicyPrincipal::Other;
     if (s->iam_policy) {
+      ARN obj_arn(s->object->get_obj());
       e = s->iam_policy->eval(s->env, *s->auth.identity,
                                 rgw::IAM::s3PutObject,
-                                s->object->get_obj(),
+                                obj_arn,
          princ_type);
     }
     if (e == Effect::Deny) {
@@ -4803,8 +4808,8 @@ int RGWDeleteObj::verify_permission(optional_yield y)
       if (r == Effect::Deny) {
         bypass_perm = false;
       } else if (r == Effect::Pass && s->iam_policy) {
-        r = s->iam_policy->eval(s->env, *s->auth.identity, rgw::IAM::s3BypassGovernanceRetention,
-                                     ARN(s->bucket->get_key(), s->object->get_name()));
+        ARN obj_arn(ARN(s->bucket->get_key(), s->object->get_name()));
+        r = s->iam_policy->eval(s->env, *s->auth.identity, rgw::IAM::s3BypassGovernanceRetention, obj_arn);
         if (r == Effect::Deny) {
           bypass_perm = false;
         }
@@ -4828,12 +4833,13 @@ int RGWDeleteObj::verify_permission(optional_yield y)
 
     rgw::IAM::Effect r = Effect::Pass;
     rgw::IAM::PolicyPrincipal princ_type = rgw::IAM::PolicyPrincipal::Other;
+    ARN obj_arn(ARN(s->bucket->get_key(), s->object->get_name()));
     if (s->iam_policy) {
       r = s->iam_policy->eval(s->env, *s->auth.identity,
                                 s->object->get_instance().empty() ?
                                 rgw::IAM::s3DeleteObject :
                                 rgw::IAM::s3DeleteObjectVersion,
-                                ARN(s->bucket->get_key(), s->object->get_name()),
+                                obj_arn,
          princ_type);
     }
     if (r == Effect::Deny)
@@ -4845,7 +4851,7 @@ int RGWDeleteObj::verify_permission(optional_yield y)
                                               s->object->get_instance().empty() ?
                                               rgw::IAM::s3DeleteObject :
                                               rgw::IAM::s3DeleteObjectVersion,
-                                              ARN(s->bucket->get_key(), s->object->get_name()));
+                                              obj_arn);
       if (session_policy_res == Effect::Deny) {
           return -EACCES;
       }
@@ -5134,12 +5140,13 @@ int RGWCopyObj::verify_permission(optional_yield y)
         if (has_s3_existing_tag || has_s3_resource_tag)
           rgw_iam_add_objtags(this, s, s->src_object.get(), has_s3_existing_tag, has_s3_resource_tag);
 
+        ARN obj_arn(s->src_object->get_obj());
         auto identity_policy_res = eval_identity_or_session_policies(s->iam_user_policies, s->env,
                                                   boost::none,
                                                   s->src_object->get_instance().empty() ?
                                                   rgw::IAM::s3GetObject :
                                                   rgw::IAM::s3GetObjectVersion,
-                                                  ARN(s->src_object->get_obj()));
+                                                  obj_arn);
         if (identity_policy_res == Effect::Deny) {
           return -EACCES;
         }
@@ -5150,7 +5157,7 @@ int RGWCopyObj::verify_permission(optional_yield y)
             s->src_object->get_instance().empty() ?
             rgw::IAM::s3GetObject :
             rgw::IAM::s3GetObjectVersion,
-            ARN(s->src_object->get_obj()),
+            obj_arn,
             princ_type);
         }
        if (e == Effect::Deny) {
@@ -5162,7 +5169,7 @@ int RGWCopyObj::verify_permission(optional_yield y)
                                                   s->src_object->get_instance().empty() ?
                                                   rgw::IAM::s3GetObject :
                                                   rgw::IAM::s3GetObjectVersion,
-                                                  ARN(s->src_object->get_obj()));
+                                                  obj_arn);
         if (session_policy_res == Effect::Deny) {
             return -EACCES;
         }
@@ -5240,10 +5247,11 @@ int RGWCopyObj::verify_permission(optional_yield y)
        rgw_add_to_iam_environment(s->env, "s3:x-amz-metadata-directive",
                                   *md_directive);
 
+      ARN obj_arn(dest_object->get_obj());
       auto identity_policy_res = eval_identity_or_session_policies(s->iam_user_policies,
                                                                   s->env, boost::none,
                                                                   rgw::IAM::s3PutObject,
-                                                                  ARN(dest_object->get_obj()));
+                                                                  obj_arn);
       if (identity_policy_res == Effect::Deny) {
         return -EACCES;
       }
@@ -5252,14 +5260,14 @@ int RGWCopyObj::verify_permission(optional_yield y)
       if (dest_iam_policy) {
         e = dest_iam_policy->eval(s->env, *s->auth.identity,
                                       rgw::IAM::s3PutObject,
-                                      ARN(dest_object->get_obj()),
+                                      obj_arn,
                                       princ_type);
       }
       if (e == Effect::Deny) {
         return -EACCES;
       }
       if (!s->session_policies.empty()) {
-        auto session_policy_res = eval_identity_or_session_policies(s->session_policies, s->env, boost::none, rgw::IAM::s3PutObject, ARN(dest_object->get_obj()));
+        auto session_policy_res = eval_identity_or_session_policies(s->session_policies, s->env, boost::none, rgw::IAM::s3PutObject, obj_arn);
         if (session_policy_res == Effect::Deny) {
             return false;
         }
@@ -6050,10 +6058,11 @@ int RGWInitMultipart::verify_permission(optional_yield y)
 
     rgw::IAM::Effect e = Effect::Pass;
     rgw::IAM::PolicyPrincipal princ_type = rgw::IAM::PolicyPrincipal::Other;
+    ARN obj_arn(s->object->get_obj());
     if (s->iam_policy) {
       e = s->iam_policy->eval(s->env, *s->auth.identity,
                                 rgw::IAM::s3PutObject,
-                                s->object->get_obj(),
+                                obj_arn,
          princ_type);
     }
     if (e == Effect::Deny) {
@@ -6158,10 +6167,11 @@ int RGWCompleteMultipart::verify_permission(optional_yield y)
 
     rgw::IAM::Effect e = Effect::Pass;
     rgw::IAM::PolicyPrincipal princ_type = rgw::IAM::PolicyPrincipal::Other;
+    rgw::ARN obj_arn(s->object->get_obj());
     if (s->iam_policy) {
       e = s->iam_policy->eval(s->env, *s->auth.identity,
                                 rgw::IAM::s3PutObject,
-                                s->object->get_obj(),
+                                obj_arn,
          princ_type);
     }
     if (e == Effect::Deny) {
@@ -6406,10 +6416,11 @@ int RGWAbortMultipart::verify_permission(optional_yield y)
 
     rgw::IAM::Effect e = Effect::Pass;
     rgw::IAM::PolicyPrincipal princ_type = rgw::IAM::PolicyPrincipal::Other;
+    ARN obj_arn(s->object->get_obj());
     if (s->iam_policy) {
       e = s->iam_policy->eval(s->env, *s->auth.identity,
                                 rgw::IAM::s3AbortMultipartUpload,
-                                s->object->get_obj(), princ_type);
+                                obj_arn, princ_type);
     }
 
     if (e == Effect::Deny) {
@@ -6594,13 +6605,14 @@ int RGWDeleteMultiObj::verify_permission(optional_yield y)
 
   if (s->iam_policy || ! s->iam_user_policies.empty() || ! s->session_policies.empty()) {
     if (s->bucket->get_info().obj_lock_enabled() && bypass_governance_mode) {
+      ARN bucket_arn(s->bucket->get_key());
       auto r = eval_identity_or_session_policies(s->iam_user_policies, s->env, boost::none,
                                                rgw::IAM::s3BypassGovernanceRetention, ARN(s->bucket->get_key()));
       if (r == Effect::Deny) {
         bypass_perm = false;
       } else if (r == Effect::Pass && s->iam_policy) {
         r = s->iam_policy->eval(s->env, *s->auth.identity, rgw::IAM::s3BypassGovernanceRetention,
-                                     ARN(s->bucket->get_key()));
+                                     bucket_arn);
         if (r == Effect::Deny) {
           bypass_perm = false;
         }
@@ -6627,12 +6639,13 @@ int RGWDeleteMultiObj::verify_permission(optional_yield y)
 
     rgw::IAM::Effect r = Effect::Pass;
     rgw::IAM::PolicyPrincipal princ_type = rgw::IAM::PolicyPrincipal::Other;
+    rgw::ARN bucket_arn(s->bucket->get_key());
     if (s->iam_policy) {
       r = s->iam_policy->eval(s->env, *s->auth.identity,
                                 not_versioned ?
                                 rgw::IAM::s3DeleteObject :
                                 rgw::IAM::s3DeleteObjectVersion,
-                                ARN(s->bucket->get_key()),
+                                bucket_arn,
          princ_type);
     }
     if (r == Effect::Deny)
@@ -6766,12 +6779,13 @@ void RGWDeleteMultiObj::execute(optional_yield y)
       rgw::IAM::Effect e = Effect::Pass;
       rgw::IAM::PolicyPrincipal princ_type = rgw::IAM::PolicyPrincipal::Other;
       if (s->iam_policy) {
+        ARN obj_arn(obj->get_obj());
         e = s->iam_policy->eval(s->env,
                                   *s->auth.identity,
                                   iter->instance.empty() ?
                                   rgw::IAM::s3DeleteObject :
                                   rgw::IAM::s3DeleteObjectVersion,
-                                  ARN(obj->get_obj()),
+                                  obj_arn,
            princ_type);
       }
       if (e == Effect::Deny) {
@@ -7254,8 +7268,9 @@ bool RGWBulkUploadOp::handle_file_verify_permission(RGWBucketInfo& binfo,
     }
 
     rgw::IAM::PolicyPrincipal princ_type = rgw::IAM::PolicyPrincipal::Other;
+    ARN obj_arn(obj);
     auto e = policy->eval(s->env, *s->auth.identity,
-                         rgw::IAM::s3PutObject, obj, princ_type);
+                         rgw::IAM::s3PutObject, obj_arn, princ_type);
     if (e == Effect::Deny) {
       return false;
     }

diff --git a/src/rgw/rgw_rest_sts.cc b/src/rgw/rgw_rest_sts.cc
index 3e623e299d00f92393294026933dfca4d930aca8..8f2a0f1d47f156a91177466b2b453559299fa018 100644 (file)
--- a/src/rgw/rgw_rest_sts.cc
+++ b/src/rgw/rgw_rest_sts.cc
@@ -528,7 +528,7 @@ int RGWREST_STS::verify_permission(optional_yield y)
   try {
     const rgw::IAM::Policy p(s->cct, s->user->get_tenant(), bl);
     if (!s->principal_tags.empty()) {
-      auto res = p.eval(s->env, *s->auth.identity, rgw::IAM::stsTagSession, rgw::ARN());
+      auto res = p.eval(s->env, *s->auth.identity, rgw::IAM::stsTagSession, boost::none);
       if (res != rgw::IAM::Effect::Allow) {
         ldout(s->cct, 0) << "evaluating policy for stsTagSession returned deny/pass" << dendl;
         return -EPERM;
@@ -541,7 +541,7 @@ int RGWREST_STS::verify_permission(optional_yield y)
       op = rgw::IAM::stsAssumeRole;
     }
 
-    auto res = p.eval(s->env, *s->auth.identity, op, rgw::ARN());
+    auto res = p.eval(s->env, *s->auth.identity, op, boost::none);
     if (res != rgw::IAM::Effect::Allow) {
       ldout(s->cct, 0) << "evaluating policy for op: " << op << " returned deny/pass" << dendl;
       return -EPERM;

diff --git a/src/test/rgw/test_rgw_iam_policy.cc b/src/test/rgw/test_rgw_iam_policy.cc
index 600f152258c04cee216feee48c9a47cb27175efd..dac4d2cef3889c57c7efe318da5ec5659a83ba30 100644 (file)
--- a/src/test/rgw/test_rgw_iam_policy.cc
+++ b/src/test/rgw/test_rgw_iam_policy.cc
@@ -199,19 +199,19 @@ TEST_F(PolicyTest, Eval1) {
                   bufferlist::static_from_string(example1));
   Environment e;
 
-  EXPECT_EQ(p.eval(e, none, s3ListBucket,
-                  ARN(Partition::aws, Service::s3,
-                      "", arbitrary_tenant, "example_bucket")),
+  ARN arn1(Partition::aws, Service::s3,
+                      "", arbitrary_tenant, "example_bucket");
+  EXPECT_EQ(p.eval(e, none, s3ListBucket, arn1),
            Effect::Allow);
 
-  EXPECT_EQ(p.eval(e, none, s3PutBucketAcl,
-                  ARN(Partition::aws, Service::s3,
-                      "", arbitrary_tenant, "example_bucket")),
+  ARN arn2(Partition::aws, Service::s3,
+                      "", arbitrary_tenant, "example_bucket");
+  EXPECT_EQ(p.eval(e, none, s3PutBucketAcl, arn2),
            Effect::Pass);
 
-  EXPECT_EQ(p.eval(e, none, s3ListBucket,
-                  ARN(Partition::aws, Service::s3,
-                      "", arbitrary_tenant, "erroneous_bucket")),
+  ARN arn3(Partition::aws, Service::s3,
+                      "", arbitrary_tenant, "erroneous_bucket");
+  EXPECT_EQ(p.eval(e, none, s3ListBucket, arn3),
            Effect::Pass);
 
 }
@@ -271,31 +271,29 @@ TEST_F(PolicyTest, Eval2) {
   auto notacct = FakeIdentity(
     Principal::tenant("some-other-account"));
   for (auto i = 0ULL; i < s3Count; ++i) {
-    EXPECT_EQ(p.eval(e, trueacct, i,
-                    ARN(Partition::aws, Service::s3,
-                        "", arbitrary_tenant, "mybucket")),
+    ARN arn1(Partition::aws, Service::s3,
+                        "", arbitrary_tenant, "mybucket");
+    EXPECT_EQ(p.eval(e, trueacct, i, arn1),
              Effect::Allow);
-    EXPECT_EQ(p.eval(e, trueacct, i,
-                    ARN(Partition::aws, Service::s3,
-                        "", arbitrary_tenant, "mybucket/myobject")),
+    ARN arn2(Partition::aws, Service::s3,
+                        "", arbitrary_tenant, "mybucket/myobject");
+    EXPECT_EQ(p.eval(e, trueacct, i, arn2),
              Effect::Allow);
-
-    EXPECT_EQ(p.eval(e, notacct, i,
-                    ARN(Partition::aws, Service::s3,
-                        "", arbitrary_tenant, "mybucket")),
+    ARN arn3(Partition::aws, Service::s3,
+                        "", arbitrary_tenant, "mybucket");
+    EXPECT_EQ(p.eval(e, notacct, i, arn3),
              Effect::Pass);
-    EXPECT_EQ(p.eval(e, notacct, i,
-                    ARN(Partition::aws, Service::s3,
-                        "", arbitrary_tenant, "mybucket/myobject")),
+    ARN arn4(Partition::aws, Service::s3,
+                        "", arbitrary_tenant, "mybucket/myobject");
+    EXPECT_EQ(p.eval(e, notacct, i, arn4),
              Effect::Pass);
-
-    EXPECT_EQ(p.eval(e, trueacct, i,
-                    ARN(Partition::aws, Service::s3,
-                        "", arbitrary_tenant, "notyourbucket")),
+    ARN arn5(Partition::aws, Service::s3,
+                        "", arbitrary_tenant, "notyourbucket");
+    EXPECT_EQ(p.eval(e, trueacct, i, arn5),
              Effect::Pass);
-    EXPECT_EQ(p.eval(e, trueacct, i,
-                    ARN(Partition::aws, Service::s3,
-                        "", arbitrary_tenant, "notyourbucket/notyourobject")),
+    ARN arn6(Partition::aws, Service::s3,
+                        "", arbitrary_tenant, "notyourbucket/notyourobject");
+    EXPECT_EQ(p.eval(e, trueacct, i, arn6),
              Effect::Pass);
 
   }
@@ -459,14 +457,14 @@ TEST_F(PolicyTest, Eval3) {
   s3allow[s3GetPublicAccessBlock] = 1;
   s3allow[s3GetBucketEncryption] = 1;
 
-  EXPECT_EQ(p.eval(em, none, s3PutBucketPolicy,
-                  ARN(Partition::aws, Service::s3,
-                      "", arbitrary_tenant, "mybucket")),
+  ARN arn1(Partition::aws, Service::s3,
+                      "", arbitrary_tenant, "mybucket");
+  EXPECT_EQ(p.eval(em, none, s3PutBucketPolicy, arn1),
            Effect::Allow);
 
-  EXPECT_EQ(p.eval(em, none, s3PutBucketPolicy,
-                  ARN(Partition::aws, Service::s3,
-                      "", arbitrary_tenant, "mybucket")),
+  ARN arn2(Partition::aws, Service::s3,
+                      "", arbitrary_tenant, "mybucket");
+  EXPECT_EQ(p.eval(em, none, s3PutBucketPolicy, arn2),
            Effect::Allow);
 
 
@@ -474,57 +472,54 @@ TEST_F(PolicyTest, Eval3) {
     if ((op == s3ListAllMyBuckets) || (op == s3PutBucketPolicy)) {
       continue;
     }
-    EXPECT_EQ(p.eval(em, none, op,
-                    ARN(Partition::aws, Service::s3,
-                        "", arbitrary_tenant, "confidential-data")),
+    ARN arn3(Partition::aws, Service::s3,
+                        "", arbitrary_tenant, "confidential-data");
+    EXPECT_EQ(p.eval(em, none, op, arn3),
              Effect::Pass);
-    EXPECT_EQ(p.eval(tr, none, op,
-                    ARN(Partition::aws, Service::s3,
-                        "", arbitrary_tenant, "confidential-data")),
+    ARN arn4(Partition::aws, Service::s3,
+                        "", arbitrary_tenant, "confidential-data");
+    EXPECT_EQ(p.eval(tr, none, op, arn4),
              s3allow[op] ? Effect::Allow : Effect::Pass);
-    EXPECT_EQ(p.eval(fa, none, op,
-                    ARN(Partition::aws, Service::s3,
-                        "", arbitrary_tenant, "confidential-data")),
+    ARN arn5(Partition::aws, Service::s3,
+                        "", arbitrary_tenant, "confidential-data");
+    EXPECT_EQ(p.eval(fa, none, op, arn5),
              Effect::Pass);
-
-    EXPECT_EQ(p.eval(em, none, op,
-                    ARN(Partition::aws, Service::s3,
-                        "", arbitrary_tenant, "confidential-data/moo")),
+    ARN arn6(Partition::aws, Service::s3,
+                        "", arbitrary_tenant, "confidential-data/moo");
+    EXPECT_EQ(p.eval(em, none, op, arn6),
              Effect::Pass);
-    EXPECT_EQ(p.eval(tr, none, op,
-                    ARN(Partition::aws, Service::s3,
-                        "", arbitrary_tenant, "confidential-data/moo")),
+    ARN arn7(Partition::aws, Service::s3,
+                        "", arbitrary_tenant, "confidential-data/moo");
+    EXPECT_EQ(p.eval(tr, none, op, arn7),
              s3allow[op] ? Effect::Allow : Effect::Pass);
-    EXPECT_EQ(p.eval(fa, none, op,
-                    ARN(Partition::aws, Service::s3,
-                        "", arbitrary_tenant, "confidential-data/moo")),
+    ARN arn8(Partition::aws, Service::s3,
+                        "", arbitrary_tenant, "confidential-data/moo");
+    EXPECT_EQ(p.eval(fa, none, op, arn8),
              Effect::Pass);
-
-    EXPECT_EQ(p.eval(em, none, op,
-                    ARN(Partition::aws, Service::s3,
-                        "", arbitrary_tenant, "really-confidential-data")),
+    ARN arn9(Partition::aws, Service::s3,
+                        "", arbitrary_tenant, "really-confidential-data");
+    EXPECT_EQ(p.eval(em, none, op, arn9),
              Effect::Pass);
-    EXPECT_EQ(p.eval(tr, none, op,
-                    ARN(Partition::aws, Service::s3,
-                        "", arbitrary_tenant, "really-confidential-data")),
+    ARN arn10(Partition::aws, Service::s3,
+                        "", arbitrary_tenant, "really-confidential-data");
+    EXPECT_EQ(p.eval(tr, none, op, arn10),
              Effect::Pass);
-    EXPECT_EQ(p.eval(fa, none, op,
-                    ARN(Partition::aws, Service::s3,
-                        "", arbitrary_tenant, "really-confidential-data")),
+    ARN arn11(Partition::aws, Service::s3,
+                        "", arbitrary_tenant, "really-confidential-data");
+    EXPECT_EQ(p.eval(fa, none, op, arn11),
              Effect::Pass);
-
-    EXPECT_EQ(p.eval(em, none, op,
-                    ARN(Partition::aws, Service::s3,
+    ARN arn12(Partition::aws, Service::s3,
                         "", arbitrary_tenant,
-                        "really-confidential-data/moo")), Effect::Pass);
-    EXPECT_EQ(p.eval(tr, none, op,
-                    ARN(Partition::aws, Service::s3,
+                        "really-confidential-data/moo");
+    EXPECT_EQ(p.eval(em, none, op, arn12), Effect::Pass);
+    ARN arn13(Partition::aws, Service::s3,
                         "", arbitrary_tenant,
-                        "really-confidential-data/moo")), Effect::Pass);
-    EXPECT_EQ(p.eval(fa, none, op,
-                    ARN(Partition::aws, Service::s3,
+                        "really-confidential-data/moo");
+    EXPECT_EQ(p.eval(tr, none, op, arn13), Effect::Pass);
+    ARN arn14(Partition::aws, Service::s3,
                         "", arbitrary_tenant,
-                        "really-confidential-data/moo")), Effect::Pass);
+                        "really-confidential-data/moo");
+    EXPECT_EQ(p.eval(fa, none, op, arn14), Effect::Pass);
 
   }
 }
@@ -565,14 +560,14 @@ TEST_F(PolicyTest, Eval4) {
                   bufferlist::static_from_string(example4));
   Environment e;
 
-  EXPECT_EQ(p.eval(e, none, iamCreateRole,
-                  ARN(Partition::aws, Service::iam,
-                      "", arbitrary_tenant, "role/example_role")),
+  ARN arn1(Partition::aws, Service::iam,
+                      "", arbitrary_tenant, "role/example_role");
+  EXPECT_EQ(p.eval(e, none, iamCreateRole, arn1),
            Effect::Allow);
 
-  EXPECT_EQ(p.eval(e, none, iamDeleteRole,
-                  ARN(Partition::aws, Service::iam,
-                      "", arbitrary_tenant, "role/example_role")),
+  ARN arn2(Partition::aws, Service::iam,
+                      "", arbitrary_tenant, "role/example_role");
+  EXPECT_EQ(p.eval(e, none, iamDeleteRole, arn2),
            Effect::Pass);
 }
 
@@ -612,19 +607,19 @@ TEST_F(PolicyTest, Eval5) {
                   bufferlist::static_from_string(example5));
   Environment e;
 
-  EXPECT_EQ(p.eval(e, none, iamCreateRole,
-                  ARN(Partition::aws, Service::iam,
-                      "", arbitrary_tenant, "role/example_role")),
+  ARN arn1(Partition::aws, Service::iam,
+                      "", arbitrary_tenant, "role/example_role");
+  EXPECT_EQ(p.eval(e, none, iamCreateRole, arn1),
            Effect::Allow);
 
-  EXPECT_EQ(p.eval(e, none, s3ListBucket,
-                  ARN(Partition::aws, Service::iam,
-                      "", arbitrary_tenant, "role/example_role")),
+  ARN arn2(Partition::aws, Service::iam,
+                      "", arbitrary_tenant, "role/example_role");
+  EXPECT_EQ(p.eval(e, none, s3ListBucket, arn2),
            Effect::Pass);
 
-  EXPECT_EQ(p.eval(e, none, iamCreateRole,
-                  ARN(Partition::aws, Service::iam,
-                      "", "", "role/example_role")),
+  ARN arn3(Partition::aws, Service::iam,
+                      "", "", "role/example_role");
+  EXPECT_EQ(p.eval(e, none, iamCreateRole, arn3),
            Effect::Pass);
 }
 
@@ -664,14 +659,14 @@ TEST_F(PolicyTest, Eval6) {
                   bufferlist::static_from_string(example6));
   Environment e;
 
-  EXPECT_EQ(p.eval(e, none, iamCreateRole,
-                  ARN(Partition::aws, Service::iam,
-                      "", arbitrary_tenant, "user/A")),
+  ARN arn1(Partition::aws, Service::iam,
+                      "", arbitrary_tenant, "user/A");
+  EXPECT_EQ(p.eval(e, none, iamCreateRole, arn1),
            Effect::Allow);
 
-  EXPECT_EQ(p.eval(e, none, s3ListBucket,
-                  ARN(Partition::aws, Service::iam,
-                      "", arbitrary_tenant, "user/A")),
+  ARN arn2(Partition::aws, Service::iam,
+                      "", arbitrary_tenant, "user/A");
+  EXPECT_EQ(p.eval(e, none, s3ListBucket, arn2),
            Effect::Allow);
 }
 
@@ -721,19 +716,19 @@ TEST_F(PolicyTest, Eval7) {
   auto sub2acct = FakeIdentity(
     Principal::user(std::move(""), "A:sub2A"));
 
-  EXPECT_EQ(p.eval(e, subacct, s3ListBucket,
-                  ARN(Partition::aws, Service::s3,
-                      "", arbitrary_tenant, "mybucket/*")),
+  ARN arn1(Partition::aws, Service::s3,
+                      "", arbitrary_tenant, "mybucket/*");
+  EXPECT_EQ(p.eval(e, subacct, s3ListBucket, arn1),
            Effect::Allow);
   
-  EXPECT_EQ(p.eval(e, parentacct, s3ListBucket,
-                  ARN(Partition::aws, Service::s3,
-                      "", arbitrary_tenant, "mybucket/*")),
+  ARN arn2(Partition::aws, Service::s3,
+                      "", arbitrary_tenant, "mybucket/*");
+  EXPECT_EQ(p.eval(e, parentacct, s3ListBucket, arn2),
            Effect::Pass);
-  
-  EXPECT_EQ(p.eval(e, sub2acct, s3ListBucket,
-                  ARN(Partition::aws, Service::s3,
-                      "", arbitrary_tenant, "mybucket/*")),
+
+  ARN arn3(Partition::aws, Service::s3,
+                      "", arbitrary_tenant, "mybucket/*");
+  EXPECT_EQ(p.eval(e, sub2acct, s3ListBucket, arn3),
            Effect::Pass);
 }
 
@@ -1029,94 +1024,93 @@ TEST_F(IPPolicyTest, EvalIPAddress) {
   auto trueacct = FakeIdentity(
     Principal::tenant("ACCOUNT-ID-WITHOUT-HYPHENS"));
   // Without an IP address in the environment then evaluation will always pass
-  EXPECT_EQ(allowp.eval(e, trueacct, s3ListBucket,
-                       ARN(Partition::aws, Service::s3,
-                           "", arbitrary_tenant, "example_bucket")),
+  ARN arn1(Partition::aws, Service::s3,
+                           "", arbitrary_tenant, "example_bucket");
+  EXPECT_EQ(allowp.eval(e, trueacct, s3ListBucket, arn1),
            Effect::Pass);
-  EXPECT_EQ(fullp.eval(e, trueacct, s3ListBucket,
-                      ARN(Partition::aws, Service::s3,
-                          "", arbitrary_tenant, "example_bucket/myobject")),
+  ARN arn2(Partition::aws, Service::s3,
+      "", arbitrary_tenant, "example_bucket/myobject");
+  EXPECT_EQ(fullp.eval(e, trueacct, s3ListBucket, arn2),
            Effect::Pass);
 
-  EXPECT_EQ(allowp.eval(allowedIP, trueacct, s3ListBucket,
-                       ARN(Partition::aws, Service::s3,
-                           "", arbitrary_tenant, "example_bucket")),
+  ARN arn3(Partition::aws, Service::s3,
+                           "", arbitrary_tenant, "example_bucket");
+  EXPECT_EQ(allowp.eval(allowedIP, trueacct, s3ListBucket, arn3),
            Effect::Allow);
-  EXPECT_EQ(allowp.eval(blocklistedIPv6, trueacct, s3ListBucket,
-                       ARN(Partition::aws, Service::s3,
-                           "", arbitrary_tenant, "example_bucket")),
+  ARN arn4(Partition::aws, Service::s3,
+                           "", arbitrary_tenant, "example_bucket");
+  EXPECT_EQ(allowp.eval(blocklistedIPv6, trueacct, s3ListBucket, arn4),
            Effect::Pass);
 
-
-  EXPECT_EQ(denyp.eval(allowedIP, trueacct, s3ListBucket,
-                      ARN(Partition::aws, Service::s3,
-                          "", arbitrary_tenant, "example_bucket")),
+  ARN arn5(Partition::aws, Service::s3,
+                          "", arbitrary_tenant, "example_bucket");
+  EXPECT_EQ(denyp.eval(allowedIP, trueacct, s3ListBucket, arn5),
            Effect::Deny);
-  EXPECT_EQ(denyp.eval(allowedIP, trueacct, s3ListBucket,
-                      ARN(Partition::aws, Service::s3,
-                          "", arbitrary_tenant, "example_bucket/myobject")),
+  ARN arn6(Partition::aws, Service::s3,
+                          "", arbitrary_tenant, "example_bucket/myobject");
+  EXPECT_EQ(denyp.eval(allowedIP, trueacct, s3ListBucket, arn6),
            Effect::Deny);
 
-  EXPECT_EQ(denyp.eval(blocklistedIP, trueacct, s3ListBucket,
-                      ARN(Partition::aws, Service::s3,
-                          "", arbitrary_tenant, "example_bucket")),
+  ARN arn7(Partition::aws, Service::s3,
+                          "", arbitrary_tenant, "example_bucket");
+  EXPECT_EQ(denyp.eval(blocklistedIP, trueacct, s3ListBucket, arn7),
            Effect::Pass);
-  EXPECT_EQ(denyp.eval(blocklistedIP, trueacct, s3ListBucket,
-                      ARN(Partition::aws, Service::s3,
-                          "", arbitrary_tenant, "example_bucket/myobject")),
+  ARN arn8(Partition::aws, Service::s3,
+                          "", arbitrary_tenant, "example_bucket/myobject");
+  EXPECT_EQ(denyp.eval(blocklistedIP, trueacct, s3ListBucket, arn8),
            Effect::Pass);
 
-  EXPECT_EQ(denyp.eval(blocklistedIPv6, trueacct, s3ListBucket,
-                      ARN(Partition::aws, Service::s3,
-                          "", arbitrary_tenant, "example_bucket")),
+  ARN arn9(Partition::aws, Service::s3,
+                          "", arbitrary_tenant, "example_bucket");
+  EXPECT_EQ(denyp.eval(blocklistedIPv6, trueacct, s3ListBucket, arn9),
            Effect::Pass);
-  EXPECT_EQ(denyp.eval(blocklistedIPv6, trueacct, s3ListBucket,
-                      ARN(Partition::aws, Service::s3,
-                          "", arbitrary_tenant, "example_bucket/myobject")),
+  ARN arn10(Partition::aws, Service::s3,
+                          "", arbitrary_tenant, "example_bucket/myobject");
+  EXPECT_EQ(denyp.eval(blocklistedIPv6, trueacct, s3ListBucket, arn10),
            Effect::Pass);
-  EXPECT_EQ(denyp.eval(allowedIPv6, trueacct, s3ListBucket,
-                      ARN(Partition::aws, Service::s3,
-                          "", arbitrary_tenant, "example_bucket")),
+  ARN arn11(Partition::aws, Service::s3,
+                          "", arbitrary_tenant, "example_bucket");
+  EXPECT_EQ(denyp.eval(allowedIPv6, trueacct, s3ListBucket, arn11),
            Effect::Deny);
-  EXPECT_EQ(denyp.eval(allowedIPv6, trueacct, s3ListBucket,
-                      ARN(Partition::aws, Service::s3,
-                          "", arbitrary_tenant, "example_bucket/myobject")),
+  ARN arn12(Partition::aws, Service::s3,
+                          "", arbitrary_tenant, "example_bucket/myobject");
+  EXPECT_EQ(denyp.eval(allowedIPv6, trueacct, s3ListBucket, arn12),
            Effect::Deny);
 
-  EXPECT_EQ(fullp.eval(allowedIP, trueacct, s3ListBucket,
-                      ARN(Partition::aws, Service::s3,
-                          "", arbitrary_tenant, "example_bucket")),
+  ARN arn13(Partition::aws, Service::s3,
+                          "", arbitrary_tenant, "example_bucket");
+  EXPECT_EQ(fullp.eval(allowedIP, trueacct, s3ListBucket, arn13),
            Effect::Allow);
-  EXPECT_EQ(fullp.eval(allowedIP, trueacct, s3ListBucket,
-                      ARN(Partition::aws, Service::s3,
-                          "", arbitrary_tenant, "example_bucket/myobject")),
+  ARN arn14(Partition::aws, Service::s3,
+      "", arbitrary_tenant, "example_bucket/myobject");
+  EXPECT_EQ(fullp.eval(allowedIP, trueacct, s3ListBucket, arn14),
            Effect::Allow);
 
-  EXPECT_EQ(fullp.eval(blocklistedIP, trueacct, s3ListBucket,
-                      ARN(Partition::aws, Service::s3,
-                          "", arbitrary_tenant, "example_bucket")),
+  ARN arn15(Partition::aws, Service::s3,
+                          "", arbitrary_tenant, "example_bucket");
+  EXPECT_EQ(fullp.eval(blocklistedIP, trueacct, s3ListBucket, arn15),
            Effect::Pass);
-  EXPECT_EQ(fullp.eval(blocklistedIP, trueacct, s3ListBucket,
-                      ARN(Partition::aws, Service::s3,
-                          "", arbitrary_tenant, "example_bucket/myobject")),
+  ARN arn16(Partition::aws, Service::s3,
+                          "", arbitrary_tenant, "example_bucket/myobject");
+  EXPECT_EQ(fullp.eval(blocklistedIP, trueacct, s3ListBucket, arn16),
            Effect::Pass);
 
-  EXPECT_EQ(fullp.eval(allowedIPv6, trueacct, s3ListBucket,
-                      ARN(Partition::aws, Service::s3,
-                          "", arbitrary_tenant, "example_bucket")),
+  ARN arn17(Partition::aws, Service::s3,
+                          "", arbitrary_tenant, "example_bucket");
+  EXPECT_EQ(fullp.eval(allowedIPv6, trueacct, s3ListBucket, arn17),
            Effect::Allow);
-  EXPECT_EQ(fullp.eval(allowedIPv6, trueacct, s3ListBucket,
-                      ARN(Partition::aws, Service::s3,
-                          "", arbitrary_tenant, "example_bucket/myobject")),
+  ARN arn18(Partition::aws, Service::s3,
+                          "", arbitrary_tenant, "example_bucket/myobject");
+  EXPECT_EQ(fullp.eval(allowedIPv6, trueacct, s3ListBucket, arn18),
            Effect::Allow);
 
-  EXPECT_EQ(fullp.eval(blocklistedIPv6, trueacct, s3ListBucket,
-                      ARN(Partition::aws, Service::s3,
-                          "", arbitrary_tenant, "example_bucket")),
+  ARN arn19(Partition::aws, Service::s3,
+                          "", arbitrary_tenant, "example_bucket");
+  EXPECT_EQ(fullp.eval(blocklistedIPv6, trueacct, s3ListBucket, arn19),
            Effect::Pass);
-  EXPECT_EQ(fullp.eval(blocklistedIPv6, trueacct, s3ListBucket,
-                      ARN(Partition::aws, Service::s3,
-                          "", arbitrary_tenant, "example_bucket/myobject")),
+  ARN arn20(Partition::aws, Service::s3,
+                          "", arbitrary_tenant, "example_bucket/myobject");
+  EXPECT_EQ(fullp.eval(blocklistedIPv6, trueacct, s3ListBucket, arn20),
            Effect::Pass);
 }