Merge pull request ceph#58686 from clwluvw/rgw-noprinc-allow

rgw: donot allow NotPrincipal with Allow Effect

Reviewed-by: Casey Bodley <[email protected]>
Reviewed-by: J. Eric Ivancich <[email protected]>

Author: cbodley

src/rgw/rgw_iam_policy.cc

@@ -701,11 +701,10 @@ bool ParseState::do_string(CephContext* cct, const char* s, size_t l) {
       return false;
     }
   } else if (w->kind == TokenKind::cond_key) {
-    auto& t = pp->policy.statements.back();
     if (l > 0 && *s == '$') {
       if (l >= 2 && *(s+1) == '{') {
         if (l > 0 && *(s+l-1) == '}') {
-          t.conditions.back().isruntime = true;
+          t->conditions.back().isruntime = true;
         } else {
 	  annotate(fmt::format("Invalid interpolation `{}`.",
 			       std::string_view{s, l}));
@@ -717,7 +716,7 @@ bool ParseState::do_string(CephContext* cct, const char* s, size_t l) {
         return false;
       }
     }
-    t.conditions.back().vals.emplace_back(s, l);
+    t->conditions.back().vals.emplace_back(s, l);
 
     // Principals
 
@@ -756,6 +755,13 @@ bool ParseState::do_string(CephContext* cct, const char* s, size_t l) {
     return false;
   }
 
+  // NotPrincipal must be used with "Effect":"Deny". Using it with "Effect":"Allow" is not supported.
+  // cf. https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_elements_notprincipal.html
+  if (t && t->effect == Effect::Allow && !t->noprinc.empty()) {
+    annotate("Allow with NotPrincipal is not allowed.");
+    return false;
+  }
+
   return true;
 }
 
@@ -1887,14 +1893,10 @@ struct IsPublicStatement
   bool operator() (const Statement &s) const {
     if (s.effect == Effect::Allow) {
       for (const auto& p : s.princ) {
-	if (p.is_wildcard()) {
-	  return s.eval_conditions(iam_all_env) == Effect::Allow;
-	}
+        if (p.is_wildcard()) {
+          return s.eval_conditions(iam_all_env) == Effect::Allow;
+        }
       }
-      // no princ should not contain fixed values
-      return std::none_of(s.noprinc.begin(), s.noprinc.end(), [](const rgw::auth::Principal& p) {
-								return p.is_wildcard();
-							      });
     }
     return false;
   }