rgw: Minor Fixes.

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 8641d110917469fa103bba1f5223104d2007f198..593d018f70ceb96409dde1abb0eacd75b18fd08f 100644 (file)
--- a/src/rgw/rgw_iam_policy.cc
+++ b/src/rgw/rgw_iam_policy.cc
@@ -468,6 +468,7 @@ static const actpair actpairs[] =
  { "iam:GetRolePolicy", iamGetRolePolicy},
  { "iam:ListRolePolicies", iamListRolePolicies},
  { "iam:DeleteRolePolicy", iamDeleteRolePolicy},
+ { "sts:AssumeRole", stsAssumeRole},
 };
 
 struct PolicyParser;
@@ -1309,6 +1310,14 @@ Effect Statement::eval(const Environment& e,
   return Effect::Pass;
 }
 
+Effect Statement::eval_principal(const Environment& e,
+                      boost::optional<const rgw::auth::Identity&> ida) const {
+  if (ida && (!ida->is_identity(princ) || ida->is_identity(noprinc))) {
+    return Effect::Deny;
+  }
+  return Effect::Allow;
+}
+
 namespace {
 const char* action_bit_string(uint64_t action) {
   switch (action) {
@@ -1511,6 +1520,9 @@ const char* action_bit_string(uint64_t action) {
 
   case iamDeleteRolePolicy:
     return "iam:DeleteRolePolicy";
+
+  case stsAssumeRole:
+    return "sts:AssumeRole";
   }
   return "s3Invalid";
 }
@@ -1636,6 +1648,20 @@ Effect Policy::eval(const Environment& e,
   return allowed ? Effect::Allow : Effect::Pass;
 }
 
+Effect Policy::eval_principal(const Environment& e,
+                   boost::optional<const rgw::auth::Identity&> ida) const {
+  auto allowed = false;
+  for (auto& s : statements) {
+    auto g = s.eval_principal(e, ida);
+    if (g == Effect::Deny) {
+      return g;
+    } else if (g == Effect::Allow) {
+      allowed = true;
+    }
+  }
+  return allowed ? Effect::Allow : Effect::Deny;
+}
+
 ostream& operator <<(ostream& m, const Policy& p) {
   m << "{ Version: "
     << (p.version == Version::v2008_10_17 ? "2008-10-17" : "2012-10-17");

diff --git a/src/rgw/rgw_iam_policy.h b/src/rgw/rgw_iam_policy.h
index b7fe5599635c84ebfb36689dbf1e6631583a7774..6c032c2a5d65563a8fb9c2b14e9db5cfac07cd5c 100644 (file)
--- a/src/rgw/rgw_iam_policy.h
+++ b/src/rgw/rgw_iam_policy.h
@@ -125,6 +125,8 @@ static const Action_t iamAllValue("111111111111100000000000000000000000000000000
 //Modify allValue if more Actions are added
 static const Action_t allValue("111111111111111111111111111111111111111111111111111111111111111111111");
 
+static constexpr std::uint64_t stsAssumeRole = 1ULL << 55;
+
 namespace {
 inline int op_to_perm(std::uint64_t op) {
   switch (op) {
@@ -459,6 +461,9 @@ struct Statement {
   Effect eval(const Environment& e,
              boost::optional<const rgw::auth::Identity&> ida,
              std::uint64_t action, const ARN& resource) const;
+
+  Effect eval_principal(const Environment& e,
+                      boost::optional<const rgw::auth::Identity&> ida) const;
 };
 
 std::ostream& operator <<(ostream& m, const Statement& s);
@@ -487,6 +492,9 @@ struct Policy {
              boost::optional<const rgw::auth::Identity&> ida,
              std::uint64_t action, const ARN& resource) const;
 
+  Effect eval_principal(const Environment& e,
+             boost::optional<const rgw::auth::Identity&> ida) const;
+
   template <typename F>
   bool has_conditional(const string& conditional, F p) const {
     for (const auto&s: statements){

diff --git a/src/rgw/rgw_rest_sts.cc b/src/rgw/rgw_rest_sts.cc
index 5cf2620c382028b668f2021a66d8ff42fa264fdc..a790478e44eb50308edad0fb7b5b6dcaa46455dc 100644 (file)
--- a/src/rgw/rgw_rest_sts.cc
+++ b/src/rgw/rgw_rest_sts.cc
@@ -19,6 +19,8 @@
 
 #include "rgw_request.h"
 #include "rgw_process.h"
+#include "rgw_iam_policy.h"
+#include "rgw_iam_policy_keywords.h"
 
 #include "sts-assume-role.h"
 
@@ -33,6 +35,32 @@
 
 int RGWREST_STS::verify_permission()
 {
+  STS::STSService _sts(s->cct, store);
+  sts = std::move(_sts);
+
+  string rArn = s->info.args.get("RoleArn");
+  const auto& [ret, role] = sts.getRoleInfo(rArn);
+  if (ret < 0) {
+    return ret;
+  }
+  string policy = role.get_assume_role_policy();
+  bufferlist bl = bufferlist::static_from_string(policy);
+
+  //Parse the policy
+  //TODO - This step should be part of Role Creation
+  try {
+    const rgw::IAM::Policy p(s->cct, s->user->user_id.tenant, bl);
+    //Check if the input role arn is there as one of the Principals in the policy,
+  // If yes, then return 0, else -EPERM
+    auto res = p.eval_principal(s->env, *s->auth.identity);
+    if (res == rgw::IAM::Effect::Deny) {
+      return -EPERM;
+    }
+  } catch (rgw::IAM::PolicyParseException& e) {
+    ldout(s->cct, 20) << "failed to parse policy: " << e.what() << dendl;
+    return -EPERM;
+  }
+
   return 0;
 }
 
@@ -60,10 +88,12 @@ int RGWSTSAssumeRole::get_params()
     return -EINVAL;
   }
 
-  JSONParser p;
-  if (!p.parse(policy.c_str(), policy.length())) {
-    ldout(s->cct, 20) << "ERROR: failed to parse policy doc" << dendl;
-    return -ERR_MALFORMED_DOC;
+  if (! policy.empty()) {
+    JSONParser p;
+    if (!p.parse(policy.c_str(), policy.length())) {
+      ldout(s->cct, 20) << "ERROR: failed to parse policy doc" << dendl;
+      return -ERR_MALFORMED_DOC;
+    }
   }
 
   return 0;
@@ -71,11 +101,14 @@ int RGWSTSAssumeRole::get_params()
 
 void RGWSTSAssumeRole::execute()
 {
+  if (op_ret = get_params(); op_ret < 0) {
+    return;
+  }
+
   STS::AssumeRoleRequest req(duration, externalId, policy, roleArn,
                         roleSessionName, serialNumber, tokenCode);
-  STS::STSService sts(s->cct, store);
-  const auto& [op_ret, assumedRoleUser, creds, packedPolicySize] = sts.assumeRole(req);
-
+  const auto& [ret, assumedRoleUser, creds, packedPolicySize] = sts.assumeRole(req);
+  op_ret = std::move(ret);
   //Dump the output
   if (op_ret == 0) {
     s->formatter->open_object_section("AssumeRole");

diff --git a/src/rgw/rgw_rest_sts.h b/src/rgw/rgw_rest_sts.h
index ba95308ff3038722a6a00a0bb5831a664766eb80..d2f9e9b29239c1db3166436e24f8e43227a77dbd 100644 (file)
--- a/src/rgw/rgw_rest_sts.h
+++ b/src/rgw/rgw_rest_sts.h
@@ -1,8 +1,13 @@
 #ifndef CEPH_RGW_REST_STS_H
 #define CEPH_RGW_REST_STS_H
 
+#include "sts-assume-role.h"
+
 class RGWREST_STS : public RGWRESTOp {
+protected:
+  STS::STSService sts;
 public:
+  RGWREST_STS() = default;
   int verify_permission() override;
   void send_response() override;
 };

diff --git a/src/rgw/rgw_role.h b/src/rgw/rgw_role.h
index 3124d59877fe0ae628d2f28b86a0037a69f23f68..d9d960d8b9f2e3d45c8c2f0af8673225713f9022 100644 (file)
--- a/src/rgw/rgw_role.h
+++ b/src/rgw/rgw_role.h
@@ -58,7 +58,7 @@ public:
     if (this->path.empty())
       this->path = "/";
     extract_name_tenant(this->name);
-    if (! max_session_duration_str.empty()) {
+    if (max_session_duration_str.empty()) {
       max_session_duration = SESSION_DURATION_MIN;
     } else {
       max_session_duration = std::stoull(max_session_duration_str);

diff --git a/src/rgw/sts-assume-role.cc b/src/rgw/sts-assume-role.cc
index 69b8b40d10af7a37c5bbd5ce51db33488b4e6c71..fe848cc3a3fff40ab6bf9444f161d661017b23f9 100644 (file)
--- a/src/rgw/sts-assume-role.cc
+++ b/src/rgw/sts-assume-role.cc
@@ -99,10 +99,9 @@ int Credentials::generateCredentials(CephContext* cct, const uint64_t& duration)
   enc_output.append('\0');
   encrypted_str = enc_output.c_str();
 
-  std::string decoded_str;
   bufferlist enc_bp, encoded_op;
   enc_bp.append(encrypted_str);
-  encoded_op.encode_base64(enc_bp);
+  enc_bp.encode_base64(encoded_op);
   encoded_op.append('\0');
   sessionToken = encoded_op.c_str();
 
@@ -118,23 +117,17 @@ void AssumedRoleUser::dump(Formatter *f) const
 int AssumedRoleUser::generateAssumedRoleUser(CephContext* cct,
                                               RGWRados *store,
                                               const string& roleId,
-                                              const boost::optional<rgw::IAM::ARN>& roleArn,
+                                              const rgw::IAM::ARN& roleArn,
                                               const string& roleSessionName)
 {
-  string resource, account;
-  rgw::IAM::ARN r_arn;
-  if (roleArn) {
-    r_arn = roleArn.get();
-    resource = r_arn.resource;
-    boost::replace_first(resource, "role", "assumed-role");
-    resource.append("/");
-    resource.append(roleSessionName);
-    account = r_arn.account;
-  }
+  string resource = std::move(roleArn.resource);
+  boost::replace_first(resource, "role", "assumed-role");
+  resource.append("/");
+  resource.append(roleSessionName);
   
   rgw::IAM::ARN assumed_role_arn(rgw::IAM::Partition::aws,
                                   rgw::IAM::Service::sts,
-                                  "", account, resource);
+                                  "", roleArn.account, resource);
   arn = assumed_role_arn.to_string();
 
   //Assumeroleid = roleid:rolesessionname
@@ -170,7 +163,7 @@ int AssumeRoleRequest::validate_input() const
       return -EINVAL;
     }
 
-    std::regex regex_externalId("[A-Za-z0-9_+=,.@:/-");
+    std::regex regex_externalId("[A-Za-z0-9_=,.@:/-]+");
     if (! std::regex_match(externalId, regex_externalId)) {
       return -EINVAL;
     }
@@ -190,7 +183,7 @@ int AssumeRoleRequest::validate_input() const
       return -EINVAL;
     }
 
-    std::regex regex_roleSession("[A-Za-z0-9_+=,.@-");
+    std::regex regex_roleSession("[A-Za-z0-9_=,.@-]+");
     if (! std::regex_match(roleSessionName, regex_roleSession)) {
       return -EINVAL;
     }
@@ -200,7 +193,7 @@ int AssumeRoleRequest::validate_input() const
       return -EINVAL;
     }
 
-    std::regex regex_serialNumber("[A-Za-z0-9_=/:,.@-");
+    std::regex regex_serialNumber("[A-Za-z0-9_=/:,.@-]+");
     if (! std::regex_match(serialNumber, regex_serialNumber)) {
       return -EINVAL;
     }
@@ -212,20 +205,23 @@ int AssumeRoleRequest::validate_input() const
   return 0;
 }
 
-std::tuple<int, boost::optional<rgw::IAM::ARN>, RGWRole> STSService::_getRoleInfo(const string& arn)
+std::tuple<int, RGWRole> STSService::getRoleInfo(const string& arn)
 {
   if (auto r_arn = rgw::IAM::ARN::parse(arn); r_arn) {
     auto pos = r_arn->resource.find_last_of('/');
     string roleName = r_arn->resource.substr(pos + 1);
     RGWRole role(cct, store, roleName, r_arn->account);
     if (int ret = role.get(); ret < 0) {
-      return make_tuple(ret, r_arn, role);
+      if (ret == -ENOENT) {
+        ret = -ERR_NO_ROLE_FOUND;
+      }
+      return make_tuple(ret, this->role);
     } else {
-      return make_tuple(0, r_arn, role);
+      this->role = std::move(role);
+      return make_tuple(0, this->role);
     }
   } else {
-    RGWRole dummyRole;
-    return make_tuple(-EINVAL, r_arn, dummyRole);
+    return make_tuple(-EINVAL, this->role);
   }
 }
 
@@ -237,16 +233,14 @@ AssumeRoleResponse STSService::assumeRole(AssumeRoleRequest& req)
   string roleId;
 
   //Get the role info which is being assumed
-  const auto& [ret_val, r_arn, role] = _getRoleInfo(req.getRoleARN());
-  if (ret_val < 0) {
-    return make_tuple(ret_val, user, cred, packedPolicySize);
+  boost::optional<rgw::IAM::ARN> r_arn;
+  if (r_arn = rgw::IAM::ARN::parse(req.getRoleARN()); r_arn == boost::none) {
+    return make_tuple(-EINVAL, user, cred, packedPolicySize);
   }
 
-  if (r_arn) {
-    roleId = role.get_id();
-    roleMaxSessionDuration = role.get_max_session_duration();
-    req.setMaxDuration(roleMaxSessionDuration);
-  }
+  roleId = role.get_id();
+  roleMaxSessionDuration = role.get_max_session_duration();
+  req.setMaxDuration(roleMaxSessionDuration);
 
   //Validate input
   int ret = 0;
@@ -259,7 +253,7 @@ AssumeRoleResponse STSService::assumeRole(AssumeRoleRequest& req)
   packedPolicySize = (policy.size() / req.getMaxPolicySize()) * 100;
 
   //Generate Assumed Role User
-  if (ret = user.generateAssumedRoleUser(cct, store, roleId, r_arn, req.getRoleSessionName()); ret < 0) {
+  if (ret = user.generateAssumedRoleUser(cct, store, roleId, r_arn.get(), req.getRoleSessionName()); ret < 0) {
     return make_tuple(ret, user, cred, packedPolicySize);
   }
 

diff --git a/src/rgw/sts-assume-role.h b/src/rgw/sts-assume-role.h
index 138a4513f4f45bcaeb59994dd40cf726cde0e69a..49620e623269f731c87da737de302080ee74bc16 100644 (file)
--- a/src/rgw/sts-assume-role.h
+++ b/src/rgw/sts-assume-role.h
@@ -52,7 +52,7 @@ public:
   int generateAssumedRoleUser( CephContext* cct,
                                 RGWRados *store,
                                 const string& roleId,
-                                const boost::optional<rgw::IAM::ARN>& roleArn,
+                                const rgw::IAM::ARN& roleArn,
                                 const string& roleSessionName);
   const string& getARN() const { return arn; }
   const string& getAssumeRoleId() const { return assumeRoleId; }
@@ -80,9 +80,11 @@ using AssumeRoleResponse = std::tuple<int, AssumedRoleUser, Credentials, uint64_
 class STSService {
   CephContext* cct;
   RGWRados *store;
-  std::tuple<int, boost::optional<rgw::IAM::ARN>, RGWRole> _getRoleInfo(const string& arn);
+  RGWRole role;
 public:
+  STSService() = default;
   STSService(CephContext* _cct, RGWRados *_store) : cct(_cct), store(_store) {}
+  std::tuple<int, RGWRole> getRoleInfo(const string& arn);
   AssumeRoleResponse assumeRole(AssumeRoleRequest& req);
 };
 }