rgw: Using bitset for Actions to allow for more action op codes.

Signed-off-by: Pritha Srivastava <prsrivas@redhat.com>

diff --git a/src/rgw/rgw_iam_policy.cc b/src/rgw/rgw_iam_policy.cc
index 00e9ac693365fb700d6c81d5852b8a259015e9fb..ceb83104d62861167e0a34956713be93141aa79b 100644 (file)
--- a/src/rgw/rgw_iam_policy.cc
+++ b/src/rgw/rgw_iam_policy.cc
@@ -825,11 +825,19 @@ bool ParseState::do_string(CephContext* cct, const char* s, size_t l) {
     is_action = true;
     if (*s == '*') {
       is_validaction = true;
+      (w->id == TokenID::Action ?
+        t->action = iamAllValue : t->notaction = iamAllValue);
     } else {
       for (auto& p : actpairs) {
         if (match_policy({s, l}, p.name, MATCH_POLICY_ACTION)) {
           is_validaction = true;
-    (w->id == TokenID::Action ? t->action : t->notaction) |= p.bit;
+          (w->id == TokenID::Action ? t->action[p.bit] = 1 : t->notaction[p.bit] = 1);
+        }
+        if ((t->action & s3AllValue) == s3AllValue) {
+          t->action[s3All] = 1;
+        }
+        if ((t->notaction & s3AllValue) == s3AllValue) {
+          t->notaction[s3All] = 1;
         }
       }
     }
@@ -1281,7 +1289,7 @@ Effect Statement::eval(const Environment& e,
     return Effect::Pass;
   }
 
-  if (!(action & act) || (notaction & act)) {
+  if (!(action[act] == 1) || (notaction[act] == 1)) {
     return Effect::Pass;
   }
 
@@ -1500,17 +1508,17 @@ const char* action_bit_string(uint64_t action) {
   return "s3Invalid";
 }
 
-ostream& print_actions(ostream& m, const uint64_t a) {
+ostream& print_actions(ostream& m, const Action_t a) {
   bool begun = false;
   m << "[ ";
-  for (auto i = 0U; i < s3Count; ++i) {
-    if (a & (1ULL << i)) {
+  for (auto i = 0U; i < allCount; ++i) {
+    if (a[i] == 1) {
       if (begun) {
         m << ", ";
       } else {
         begun = true;
       }
-      m << action_bit_string(1ULL << i);
+      m << action_bit_string(i);
     }
   }
   if (begun) {
@@ -1543,22 +1551,22 @@ ostream& operator <<(ostream& m, const Statement& s) {
      (const char*) "Allow" :
      (const char*) "Deny");
 
-  if (s.action || s.notaction || !s.resource.empty() ||
+  if (s.action.any() || s.notaction.any() || !s.resource.empty() ||
       !s.notresource.empty() || !s.conditions.empty()) {
     m << ", ";
   }
 
-  if (s.action) {
+  if (s.action.any()) {
     m << "Action: ";
     print_actions(m, s.action);
 
-    if (s.notaction || !s.resource.empty() ||
+    if (s.notaction.any() || !s.resource.empty() ||
        !s.notresource.empty() || !s.conditions.empty()) {
       m << ", ";
     }
   }
 
-  if (s.notaction) {
+  if (s.notaction.any()) {
     m << "NotAction: ";
     print_actions(m, s.notaction);
 

diff --git a/src/rgw/rgw_iam_policy.h b/src/rgw/rgw_iam_policy.h
index 111084381fd436f66c9e681043277a30bfabd93f..759b5f084021de0173ef06da5c536a1fcf0aa1a1 100644 (file)
--- a/src/rgw/rgw_iam_policy.h
+++ b/src/rgw/rgw_iam_policy.h
@@ -9,6 +9,7 @@
 #include <cstdint>
 #include <iostream>
 #include <string>
+#include <bitset>
 
 #include <boost/algorithm/string/predicate.hpp>
 #include <boost/container/flat_map.hpp>
@@ -40,77 +41,88 @@ struct rgw_bucket;
 
 namespace rgw {
 namespace IAM {
-static constexpr std::uint64_t s3None = 0;
-static constexpr std::uint64_t s3GetObject = 1ULL << 0;
-static constexpr std::uint64_t s3GetObjectVersion = 1ULL << 1;
-static constexpr std::uint64_t s3PutObject = 1ULL << 2;
-static constexpr std::uint64_t s3GetObjectAcl = 1ULL << 3;
-static constexpr std::uint64_t s3GetObjectVersionAcl = 1ULL << 4;
-static constexpr std::uint64_t s3PutObjectAcl = 1ULL << 5;
-static constexpr std::uint64_t s3PutObjectVersionAcl = 1ULL << 6;
-static constexpr std::uint64_t s3DeleteObject = 1ULL << 7;
-static constexpr std::uint64_t s3DeleteObjectVersion = 1ULL << 8;
-static constexpr std::uint64_t s3ListMultipartUploadParts = 1ULL << 9;
-static constexpr std::uint64_t s3AbortMultipartUpload = 1ULL << 10;
-static constexpr std::uint64_t s3GetObjectTorrent = 1ULL << 11;
-static constexpr std::uint64_t s3GetObjectVersionTorrent = 1ULL << 12;
-static constexpr std::uint64_t s3RestoreObject = 1ULL << 13;
-static constexpr std::uint64_t s3CreateBucket = 1ULL << 14;
-static constexpr std::uint64_t s3DeleteBucket = 1ULL << 15;
-static constexpr std::uint64_t s3ListBucket = 1ULL << 16;
-static constexpr std::uint64_t s3ListBucketVersions = 1ULL << 17;
-static constexpr std::uint64_t s3ListAllMyBuckets = 1ULL << 18;
-static constexpr std::uint64_t s3ListBucketMultipartUploads = 1ULL << 19;
-static constexpr std::uint64_t s3GetAccelerateConfiguration = 1ULL << 20;
-static constexpr std::uint64_t s3PutAccelerateConfiguration = 1ULL << 21;
-static constexpr std::uint64_t s3GetBucketAcl = 1ULL << 22;
-static constexpr std::uint64_t s3PutBucketAcl = 1ULL << 23;
-static constexpr std::uint64_t s3GetBucketCORS = 1ULL << 24;
-static constexpr std::uint64_t s3PutBucketCORS = 1ULL << 25;
-static constexpr std::uint64_t s3GetBucketVersioning = 1ULL << 26;
-static constexpr std::uint64_t s3PutBucketVersioning = 1ULL << 27;
-static constexpr std::uint64_t s3GetBucketRequestPayment = 1ULL << 28;
-static constexpr std::uint64_t s3PutBucketRequestPayment = 1ULL << 29;
-static constexpr std::uint64_t s3GetBucketLocation = 1ULL << 30;
-static constexpr std::uint64_t s3GetBucketPolicy = 1ULL << 31;
-static constexpr std::uint64_t s3DeleteBucketPolicy = 1ULL << 32;
-static constexpr std::uint64_t s3PutBucketPolicy = 1ULL << 33;
-static constexpr std::uint64_t s3GetBucketNotification = 1ULL << 34;
-static constexpr std::uint64_t s3PutBucketNotification = 1ULL << 35;
-static constexpr std::uint64_t s3GetBucketLogging = 1ULL << 36;
-static constexpr std::uint64_t s3PutBucketLogging = 1ULL << 37;
-static constexpr std::uint64_t s3GetBucketTagging = 1ULL << 38;
-static constexpr std::uint64_t s3PutBucketTagging = 1ULL << 39;
-static constexpr std::uint64_t s3GetBucketWebsite = 1ULL << 40;
-static constexpr std::uint64_t s3PutBucketWebsite = 1ULL << 41;
-static constexpr std::uint64_t s3DeleteBucketWebsite = 1ULL << 42;
-static constexpr std::uint64_t s3GetLifecycleConfiguration = 1ULL << 43;
-static constexpr std::uint64_t s3PutLifecycleConfiguration = 1ULL << 44;
-static constexpr std::uint64_t s3PutReplicationConfiguration = 1ULL << 45;
-static constexpr std::uint64_t s3GetReplicationConfiguration = 1ULL << 46;
-static constexpr std::uint64_t s3DeleteReplicationConfiguration = 1ULL << 47;
-static constexpr std::uint64_t s3GetObjectTagging = 1ULL << 48;
-static constexpr std::uint64_t s3PutObjectTagging = 1ULL << 49;
-static constexpr std::uint64_t s3DeleteObjectTagging = 1ULL << 50;
-static constexpr std::uint64_t s3GetObjectVersionTagging = 1ULL << 51;
-static constexpr std::uint64_t s3PutObjectVersionTagging = 1ULL << 52;
-static constexpr std::uint64_t s3DeleteObjectVersionTagging = 1ULL << 53;
-static constexpr std::uint64_t s3Count = 54;
-static constexpr std::uint64_t s3All = (1ULL << s3Count) - 1;
-static constexpr std::uint64_t iamPutUserPolicy = 1ULL << 56;
-static constexpr std::uint64_t iamGetUserPolicy = 1ULL << 57;
-static constexpr std::uint64_t iamDeleteUserPolicy = 1ULL << 58;
-static constexpr std::uint64_t iamListUserPolicies = 1ULL << 59;
-
-static constexpr std::uint64_t iamCreateRole = 1ULL << 60;
-static constexpr std::uint64_t iamDeleteRole = 1ULL << 61;
-static constexpr std::uint64_t iamModifyRole = 1ULL << 62;
-static constexpr std::uint64_t iamGetRole = 1ULL << 63;
-static constexpr std::uint64_t iamListRoles = 3ULL;
-static constexpr std::uint64_t iamPutRolePolicy = 5ULL;
-static constexpr std::uint64_t iamGetRolePolicy = 6ULL;
-static constexpr std::uint64_t iamListRolePolicies = 7ULL;
-static constexpr std::uint64_t iamDeleteRolePolicy = 9ULL;
+
+static constexpr std::uint64_t s3GetObject = 0;
+static constexpr std::uint64_t s3GetObjectVersion = 1;
+static constexpr std::uint64_t s3PutObject = 2;
+static constexpr std::uint64_t s3GetObjectAcl = 3;
+static constexpr std::uint64_t s3GetObjectVersionAcl = 4;
+static constexpr std::uint64_t s3PutObjectAcl = 5;
+static constexpr std::uint64_t s3PutObjectVersionAcl = 6;
+static constexpr std::uint64_t s3DeleteObject = 7;
+static constexpr std::uint64_t s3DeleteObjectVersion = 8;
+static constexpr std::uint64_t s3ListMultipartUploadParts = 9;
+static constexpr std::uint64_t s3AbortMultipartUpload = 10;
+static constexpr std::uint64_t s3GetObjectTorrent = 11;
+static constexpr std::uint64_t s3GetObjectVersionTorrent = 12;
+static constexpr std::uint64_t s3RestoreObject = 13;
+static constexpr std::uint64_t s3CreateBucket = 14;
+static constexpr std::uint64_t s3DeleteBucket = 15;
+static constexpr std::uint64_t s3ListBucket = 16;
+static constexpr std::uint64_t s3ListBucketVersions = 17;
+static constexpr std::uint64_t s3ListAllMyBuckets = 18;
+static constexpr std::uint64_t s3ListBucketMultipartUploads = 19;
+static constexpr std::uint64_t s3GetAccelerateConfiguration = 20;
+static constexpr std::uint64_t s3PutAccelerateConfiguration = 21;
+static constexpr std::uint64_t s3GetBucketAcl = 22;
+static constexpr std::uint64_t s3PutBucketAcl = 23;
+static constexpr std::uint64_t s3GetBucketCORS = 24;
+static constexpr std::uint64_t s3PutBucketCORS = 25;
+static constexpr std::uint64_t s3GetBucketVersioning = 26;
+static constexpr std::uint64_t s3PutBucketVersioning = 27;
+static constexpr std::uint64_t s3GetBucketRequestPayment = 28;
+static constexpr std::uint64_t s3PutBucketRequestPayment = 29;
+static constexpr std::uint64_t s3GetBucketLocation = 30;
+static constexpr std::uint64_t s3GetBucketPolicy = 31;
+static constexpr std::uint64_t s3DeleteBucketPolicy = 32;
+static constexpr std::uint64_t s3PutBucketPolicy = 33;
+static constexpr std::uint64_t s3GetBucketNotification = 34;
+static constexpr std::uint64_t s3PutBucketNotification = 35;
+static constexpr std::uint64_t s3GetBucketLogging = 36;
+static constexpr std::uint64_t s3PutBucketLogging = 37;
+static constexpr std::uint64_t s3GetBucketTagging = 38;
+static constexpr std::uint64_t s3PutBucketTagging = 39;
+static constexpr std::uint64_t s3GetBucketWebsite = 40;
+static constexpr std::uint64_t s3PutBucketWebsite = 41;
+static constexpr std::uint64_t s3DeleteBucketWebsite = 42;
+static constexpr std::uint64_t s3GetLifecycleConfiguration = 43;
+static constexpr std::uint64_t s3PutLifecycleConfiguration = 44;
+static constexpr std::uint64_t s3PutReplicationConfiguration = 45;
+static constexpr std::uint64_t s3GetReplicationConfiguration = 46;
+static constexpr std::uint64_t s3DeleteReplicationConfiguration = 47;
+static constexpr std::uint64_t s3GetObjectTagging = 48;
+static constexpr std::uint64_t s3PutObjectTagging = 49;
+static constexpr std::uint64_t s3DeleteObjectTagging = 50;
+static constexpr std::uint64_t s3GetObjectVersionTagging = 51;
+static constexpr std::uint64_t s3PutObjectVersionTagging = 52;
+static constexpr std::uint64_t s3DeleteObjectVersionTagging = 53;
+static constexpr std::uint64_t s3All = 54;
+
+static constexpr std::uint64_t iamPutUserPolicy = 55;
+static constexpr std::uint64_t iamGetUserPolicy = 56;
+static constexpr std::uint64_t iamDeleteUserPolicy = 57;
+static constexpr std::uint64_t iamListUserPolicies = 58;
+
+static constexpr std::uint64_t iamCreateRole = 59;
+static constexpr std::uint64_t iamDeleteRole = 60;
+static constexpr std::uint64_t iamModifyRole = 61;
+static constexpr std::uint64_t iamGetRole = 62;
+static constexpr std::uint64_t iamListRoles = 63;
+static constexpr std::uint64_t iamPutRolePolicy = 64;
+static constexpr std::uint64_t iamGetRolePolicy = 65;
+static constexpr std::uint64_t iamListRolePolicies = 66;
+static constexpr std::uint64_t iamDeleteRolePolicy = 67;
+
+static constexpr std::uint64_t s3Count = s3DeleteObjectVersionTagging + 1;
+static constexpr std::uint64_t allCount = iamDeleteRolePolicy + 1;
+
+using Action_t = bitset<allCount>;
+using NotAction_t = Action_t;
+
+static const Action_t None(0);
+static const Action_t s3AllValue("111111111111111111111111111111111111111111111111111111");
+//Modify iamAllValue if more IAM actions are added
+static const Action_t iamAllValue("11111111111111111111111111111111111111111111111111111111111111111111");
 
 namespace {
 inline int op_to_perm(std::uint64_t op) {
@@ -435,8 +447,8 @@ struct Statement {
   // deny as defensive programming.
   Effect effect = Effect::Deny;
 
-  std::uint64_t action = 0;
-  std::uint64_t notaction = 0;
+  Action_t action = 0;
+  NotAction_t notaction = 0;
 
   boost::container::flat_set<ARN> resource;
   boost::container::flat_set<ARN> notresource;

diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc
index cae79ee057a081458b69cc39db6f557614177421..73868467b8fae88b93ce38ba571789d59455afd1 100644 (file)
--- a/src/rgw/rgw_op.cc
+++ b/src/rgw/rgw_op.cc
@@ -536,7 +536,6 @@ int rgw_build_bucket_policies(RGWRados* store, struct req_state* s)
     }
   }
 
-  
   /* handle user ACL only for those APIs which support it */
   if (s->user_acl) {
     map<string, bufferlist> uattrs;

diff --git a/src/rgw/rgw_rest_user_policy.cc b/src/rgw/rgw_rest_user_policy.cc
index 213db72358d5f1cd1eeb1fdb530413e0216cc494..962cb5a601d24b691f7457d87bcb671890b8c46d 100644 (file)
--- a/src/rgw/rgw_rest_user_policy.cc
+++ b/src/rgw/rgw_rest_user_policy.cc
@@ -292,7 +292,7 @@ void RGWDeleteUserPolicy::execute()
   if (op_ret < 0) {
     return;
   }
-  
+
   RGWUserInfo info;
   rgw_user user_id(user_name);
   op_ret = rgw_get_user_info_by_uid(store, user_id, info);
@@ -300,7 +300,7 @@ void RGWDeleteUserPolicy::execute()
     op_ret = -ERR_NO_SUCH_ENTITY;
     return;
   }
-  
+
   map<string, bufferlist> uattrs;
   op_ret = rgw_get_user_attrs_by_uid(store, user_id, uattrs);
   if (op_ret == -ENOENT) {

diff --git a/src/test/rgw/test_rgw_iam_policy.cc b/src/test/rgw/test_rgw_iam_policy.cc
index 164e9bbcde730e7deb24d5c5200b4684cfc2d2ef..9b553da4fbbf4c78e86d82b862789e52d01e5f7b 100644 (file)
--- a/src/test/rgw/test_rgw_iam_policy.cc
+++ b/src/test/rgw/test_rgw_iam_policy.cc
@@ -73,12 +73,14 @@ using rgw::IAM::s3ListBucket;
 using rgw::IAM::s3ListBucketMultipartUploads;
 using rgw::IAM::s3ListBucketVersions;
 using rgw::IAM::s3ListMultipartUploadParts;
-using rgw::IAM::s3None;
+using rgw::IAM::None;
 using rgw::IAM::s3PutBucketAcl;
 using rgw::IAM::s3PutBucketPolicy;
 using rgw::IAM::Service;
 using rgw::IAM::TokenID;
 using rgw::IAM::Version;
+using rgw::IAM::Action_t;
+using rgw::IAM::NotAction_t;
 
 class FakeIdentity : public Identity {
   const Principal id;
@@ -146,8 +148,10 @@ TEST_F(PolicyTest, Parse1) {
   EXPECT_TRUE(p->statements[0].princ.empty());
   EXPECT_TRUE(p->statements[0].noprinc.empty());
   EXPECT_EQ(p->statements[0].effect, Effect::Allow);
-  EXPECT_EQ(p->statements[0].action, s3ListBucket);
-  EXPECT_EQ(p->statements[0].notaction, s3None);
+  Action_t act;
+  act[s3ListBucket] = 1;
+  EXPECT_EQ(p->statements[0].action, act);
+  EXPECT_EQ(p->statements[0].notaction, None);
   ASSERT_FALSE(p->statements[0].resource.empty());
   ASSERT_EQ(p->statements[0].resource.size(), 1U);
   EXPECT_EQ(p->statements[0].resource.begin()->partition, Partition::aws);
@@ -200,8 +204,12 @@ TEST_F(PolicyTest, Parse2) {
            Principal::tenant("ACCOUNT-ID-WITHOUT-HYPHENS"));
   EXPECT_TRUE(p->statements[0].noprinc.empty());
   EXPECT_EQ(p->statements[0].effect, Effect::Allow);
-  EXPECT_EQ(p->statements[0].action, s3All);
-  EXPECT_EQ(p->statements[0].notaction, s3None);
+  Action_t act;
+  for (auto i = 0ULL; i < s3Count; i++)
+    act[i] = 1;
+  act[s3All] = 1;
+  EXPECT_EQ(p->statements[0].action, act);
+  EXPECT_EQ(p->statements[0].notaction, None);
   ASSERT_FALSE(p->statements[0].resource.empty());
   ASSERT_EQ(p->statements[0].resource.size(), 2U);
   EXPECT_EQ(p->statements[0].resource.begin()->partition, Partition::aws);
@@ -232,29 +240,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, 1ULL << i,
+    EXPECT_EQ(p.eval(e, trueacct, i,
                     ARN(Partition::aws, Service::s3,
                         "", arbitrary_tenant, "mybucket")),
              Effect::Allow);
-    EXPECT_EQ(p.eval(e, trueacct, 1ULL << i,
+    EXPECT_EQ(p.eval(e, trueacct, i,
                     ARN(Partition::aws, Service::s3,
                         "", arbitrary_tenant, "mybucket/myobject")),
              Effect::Allow);
 
-    EXPECT_EQ(p.eval(e, notacct, 1ULL << i,
+    EXPECT_EQ(p.eval(e, notacct, i,
                     ARN(Partition::aws, Service::s3,
                         "", arbitrary_tenant, "mybucket")),
              Effect::Pass);
-    EXPECT_EQ(p.eval(e, notacct, 1ULL << i,
+    EXPECT_EQ(p.eval(e, notacct, i,
                     ARN(Partition::aws, Service::s3,
                         "", arbitrary_tenant, "mybucket/myobject")),
              Effect::Pass);
 
-    EXPECT_EQ(p.eval(e, trueacct, 1ULL << i,
+    EXPECT_EQ(p.eval(e, trueacct, i,
                     ARN(Partition::aws, Service::s3,
                         "", arbitrary_tenant, "notyourbucket")),
              Effect::Pass);
-    EXPECT_EQ(p.eval(e, trueacct, 1ULL << i,
+    EXPECT_EQ(p.eval(e, trueacct, i,
                     ARN(Partition::aws, Service::s3,
                         "", arbitrary_tenant, "notyourbucket/notyourobject")),
              Effect::Pass);
@@ -279,8 +287,10 @@ TEST_F(PolicyTest, Parse3) {
   EXPECT_TRUE(p->statements[0].princ.empty());
   EXPECT_TRUE(p->statements[0].noprinc.empty());
   EXPECT_EQ(p->statements[0].effect, Effect::Allow);
-  EXPECT_EQ(p->statements[0].action, s3PutBucketPolicy);
-  EXPECT_EQ(p->statements[0].notaction, s3None);
+  Action_t act;
+  act[s3PutBucketPolicy] = 1;
+  EXPECT_EQ(p->statements[0].action, act);
+  EXPECT_EQ(p->statements[0].notaction, None);
   ASSERT_FALSE(p->statements[0].resource.empty());
   ASSERT_EQ(p->statements[0].resource.size(), 1U);
   EXPECT_EQ(p->statements[0].resource.begin()->partition, Partition::wildcard);
@@ -295,8 +305,10 @@ TEST_F(PolicyTest, Parse3) {
   EXPECT_TRUE(p->statements[1].princ.empty());
   EXPECT_TRUE(p->statements[1].noprinc.empty());
   EXPECT_EQ(p->statements[1].effect, Effect::Allow);
-  EXPECT_EQ(p->statements[1].action, s3ListAllMyBuckets);
-  EXPECT_EQ(p->statements[1].notaction, s3None);
+  Action_t act1;
+  act1[s3ListAllMyBuckets] = 1;
+  EXPECT_EQ(p->statements[1].action, act1);
+  EXPECT_EQ(p->statements[1].notaction, None);
   ASSERT_FALSE(p->statements[1].resource.empty());
   ASSERT_EQ(p->statements[1].resource.size(), 1U);
   EXPECT_EQ(p->statements[1].resource.begin()->partition, Partition::wildcard);
@@ -311,29 +323,36 @@ TEST_F(PolicyTest, Parse3) {
   EXPECT_TRUE(p->statements[2].princ.empty());
   EXPECT_TRUE(p->statements[2].noprinc.empty());
   EXPECT_EQ(p->statements[2].effect, Effect::Allow);
-  EXPECT_EQ(p->statements[2].action, (s3ListMultipartUploadParts |
-                                     s3ListBucket | s3ListBucketVersions |
-                                     s3ListAllMyBuckets |
-                                     s3ListBucketMultipartUploads |
-                                     s3GetObject | s3GetObjectVersion |
-                                     s3GetObjectAcl | s3GetObjectVersionAcl |
-                                     s3GetObjectTorrent |
-                                     s3GetObjectVersionTorrent |
-                                     s3GetAccelerateConfiguration |
-                                     s3GetBucketAcl | s3GetBucketCORS |
-                                     s3GetBucketVersioning |
-                                     s3GetBucketRequestPayment |
-                                     s3GetBucketLocation |
-                                     s3GetBucketPolicy |
-                                     s3GetBucketNotification |
-                                     s3GetBucketLogging |
-                                     s3GetBucketTagging |
-                                     s3GetBucketWebsite |
-                                     s3GetLifecycleConfiguration |
-                                     s3GetReplicationConfiguration |
-                                     s3GetObjectTagging |
-                                     s3GetObjectVersionTagging));
-  EXPECT_EQ(p->statements[2].notaction, s3None);
+  Action_t act2;
+  act2[s3ListMultipartUploadParts] = 1;
+  act2[s3ListBucket] = 1;
+  act2[s3ListBucketVersions] = 1;
+  act2[s3ListAllMyBuckets] = 1;
+  act2[s3ListBucketMultipartUploads] = 1;
+  act2[s3GetObject] = 1;
+  act2[s3GetObjectVersion] = 1;
+  act2[s3GetObjectAcl] = 1;
+  act2[s3GetObjectVersionAcl] = 1;
+  act2[s3GetObjectTorrent] = 1;
+  act2[s3GetObjectVersionTorrent] = 1;
+  act2[s3GetAccelerateConfiguration] = 1;
+  act2[s3GetBucketAcl] = 1;
+  act2[s3GetBucketCORS] = 1;
+  act2[s3GetBucketVersioning] = 1;
+  act2[s3GetBucketRequestPayment] = 1;
+  act2[s3GetBucketLocation] = 1;
+  act2[s3GetBucketPolicy] = 1;
+  act2[s3GetBucketNotification] = 1;
+  act2[s3GetBucketLogging] = 1;
+  act2[s3GetBucketTagging] = 1;
+  act2[s3GetBucketWebsite] = 1;
+  act2[s3GetLifecycleConfiguration] = 1;
+  act2[s3GetReplicationConfiguration] = 1;
+  act2[s3GetObjectTagging] = 1;
+  act2[s3GetObjectVersionTagging] = 1;
+
+  EXPECT_EQ(p->statements[2].action, act2);
+  EXPECT_EQ(p->statements[2].notaction, None);
   ASSERT_FALSE(p->statements[2].resource.empty());
   ASSERT_EQ(p->statements[2].resource.size(), 2U);
   EXPECT_EQ(p->statements[2].resource.begin()->partition, Partition::aws);
@@ -367,19 +386,33 @@ TEST_F(PolicyTest, Eval3) {
   Environment tr = { { "aws:MultiFactorAuthPresent", "true" } };
   Environment fa = { { "aws:MultiFactorAuthPresent", "false" } };
 
-  auto s3allow = (s3ListMultipartUploadParts | s3ListBucket |
-                 s3ListBucketVersions | s3ListAllMyBuckets |
-                 s3ListBucketMultipartUploads | s3GetObject |
-                 s3GetObjectVersion | s3GetObjectAcl | s3GetObjectVersionAcl |
-                 s3GetObjectTorrent | s3GetObjectVersionTorrent |
-                 s3GetAccelerateConfiguration | s3GetBucketAcl |
-                 s3GetBucketCORS | s3GetBucketVersioning |
-                 s3GetBucketRequestPayment | s3GetBucketLocation |
-                 s3GetBucketPolicy | s3GetBucketNotification |
-                 s3GetBucketLogging | s3GetBucketTagging |
-                 s3GetBucketWebsite | s3GetLifecycleConfiguration |
-                 s3GetReplicationConfiguration |
-                 s3GetObjectTagging | s3GetObjectVersionTagging);
+  Action_t s3allow;
+  s3allow[s3ListMultipartUploadParts] = 1;
+  s3allow[s3ListBucket] = 1;
+  s3allow[s3ListBucketVersions] = 1;
+  s3allow[s3ListAllMyBuckets] = 1;
+  s3allow[s3ListBucketMultipartUploads] = 1;
+  s3allow[s3GetObject] = 1;
+  s3allow[s3GetObjectVersion] = 1;
+  s3allow[s3GetObjectAcl] = 1;
+  s3allow[s3GetObjectVersionAcl] = 1;
+  s3allow[s3GetObjectTorrent] = 1;
+  s3allow[s3GetObjectVersionTorrent] = 1;
+  s3allow[s3GetAccelerateConfiguration] = 1;
+  s3allow[s3GetBucketAcl] = 1;
+  s3allow[s3GetBucketCORS] = 1;
+  s3allow[s3GetBucketVersioning] = 1;
+  s3allow[s3GetBucketRequestPayment] = 1;
+  s3allow[s3GetBucketLocation] = 1;
+  s3allow[s3GetBucketPolicy] = 1;
+  s3allow[s3GetBucketNotification] = 1;
+  s3allow[s3GetBucketLogging] = 1;
+  s3allow[s3GetBucketTagging] = 1;
+  s3allow[s3GetBucketWebsite] = 1;
+  s3allow[s3GetLifecycleConfiguration] = 1;
+  s3allow[s3GetReplicationConfiguration] = 1;
+  s3allow[s3GetObjectTagging] = 1;
+  s3allow[s3GetObjectVersionTagging] = 1;
 
   EXPECT_EQ(p.eval(em, none, s3PutBucketPolicy,
                   ARN(Partition::aws, Service::s3,
@@ -392,12 +425,10 @@ TEST_F(PolicyTest, Eval3) {
            Effect::Allow);
 
 
-  for (auto i = 0ULL; i < s3Count; ++i) {
-    auto op = 1ULL << i;
+  for (auto op = 0ULL; op < s3Count; ++op) {
     if ((op == s3ListAllMyBuckets) || (op == s3PutBucketPolicy)) {
       continue;
     }
-
     EXPECT_EQ(p.eval(em, none, op,
                     ARN(Partition::aws, Service::s3,
                         "", arbitrary_tenant, "confidential-data")),
@@ -405,7 +436,7 @@ TEST_F(PolicyTest, Eval3) {
     EXPECT_EQ(p.eval(tr, none, op,
                     ARN(Partition::aws, Service::s3,
                         "", arbitrary_tenant, "confidential-data")),
-             op & s3allow ? Effect::Allow : Effect::Pass);
+             s3allow[op] ? Effect::Allow : Effect::Pass);
     EXPECT_EQ(p.eval(fa, none, op,
                     ARN(Partition::aws, Service::s3,
                         "", arbitrary_tenant, "confidential-data")),
@@ -418,7 +449,7 @@ TEST_F(PolicyTest, Eval3) {
     EXPECT_EQ(p.eval(tr, none, op,
                     ARN(Partition::aws, Service::s3,
                         "", arbitrary_tenant, "confidential-data/moo")),
-             op & s3allow ? Effect::Allow : Effect::Pass);
+             s3allow[op] ? Effect::Allow : Effect::Pass);
     EXPECT_EQ(p.eval(fa, none, op,
                     ARN(Partition::aws, Service::s3,
                         "", arbitrary_tenant, "confidential-data/moo")),
@@ -639,8 +670,10 @@ TEST_F(IPPolicyTest, ParseIPAddress) {
            Principal::wildcard());
   EXPECT_TRUE(p->statements[0].noprinc.empty());
   EXPECT_EQ(p->statements[0].effect, Effect::Allow);
-  EXPECT_EQ(p->statements[0].action, s3ListBucket);
-  EXPECT_EQ(p->statements[0].notaction, s3None);
+  Action_t act;
+  act[s3ListBucket] = 1;
+  EXPECT_EQ(p->statements[0].action, act);
+  EXPECT_EQ(p->statements[0].notaction, None);
   ASSERT_FALSE(p->statements[0].resource.empty());
   ASSERT_EQ(p->statements[0].resource.size(), 2U);
   EXPECT_EQ(p->statements[0].resource.begin()->partition, Partition::aws);