From: Matthew Oliver <moliver@suse.com>
Date: Thu, 9 Jul 2020 06:13:05 +0000 (+0000)
Subject: rgw: Swift API anonymous access should 401
X-Git-Tag: v15.2.9~122^2~85^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F37339%2Fhead;p=ceph.git

rgw: Swift API anonymous access should 401

There was a previous patch to fix this but turns out that only fixed it
for the Swift V1 auth. And it actaully broke keystone because it didn't
take into account the idiosyncrasies of multi tenancy. Which resulted in
the incorect behaviour for keystone. Worse, because it didn't take
tenants properly into account keystone ACLs where broken.

This patch reworks, and simplifies the original patch to work for both
auths. It even extends the ThirdPartyAccountApplier to check for an ANON
user and properly scope it to a tenant.

Fixes: https://tracker.ceph.com/issues/46295
Signed-off-by: Matthew Oliver <moliver@suse.com>
(cherry picked from commit 67081098dc2dddd80d52d5acd166e68954cae618)
---

diff --git a/src/rgw/rgw_auth.h b/src/rgw/rgw_auth.h
index 37971119be6c..fda81e8423a9 100644
--- a/src/rgw/rgw_auth.h
+++ b/src/rgw/rgw_auth.h
@@ -58,7 +58,7 @@ public:
    * with the reason. */
   virtual uint32_t get_perm_mask() const = 0;
 
-  virtual bool is_anonymous() const final {
+  virtual bool is_anonymous() const {
     /* If the identity owns the anonymous account (rgw_user), it's considered
      * the anonymous identity. On error throws rgw::auth::Exception storing
      * the reason. */
@@ -498,7 +498,6 @@ public:
       is_admin(acct_privilege_t::IS_ADMIN_ACCT == level),
       acct_type(acct_type) {
     }
-    bool is_anon() const {return (acct_name.compare(RGW_USER_ANON_ID) == 0);}
   };
 
   using aclspec_t = rgw::auth::Identity::aclspec_t;
diff --git a/src/rgw/rgw_auth_filters.h b/src/rgw/rgw_auth_filters.h
index 8a5bf80644a7..9dbcf677ac0a 100644
--- a/src/rgw/rgw_auth_filters.h
+++ b/src/rgw/rgw_auth_filters.h
@@ -76,6 +76,10 @@ public:
     return get_decoratee().is_owner_of(uid);
   }
 
+  bool is_anonymous() const override {
+    return get_decoratee().is_anonymous();
+  }
+
   uint32_t get_perm_mask() const override {
     return get_decoratee().get_perm_mask();
   }
@@ -159,6 +163,13 @@ void ThirdPartyAccountApplier<T>::load_acct_info(const DoutPrefixProvider* dpp,
     /* The override has been specified but the account belongs to the authenticated
      * identity. We may safely forward the call to a next stage. */
     DecoratedApplier<T>::load_acct_info(dpp, user_info);
+  } else if (this->is_anonymous()) {
+    /* If the user was authed by the anonymous engine then scope the ANON user
+     * to the correct tenant */
+    if (acct_user_override.tenant.empty())
+      user_info.user_id = rgw_user(acct_user_override.id, RGW_USER_ANON_ID);
+    else
+      user_info.user_id = rgw_user(acct_user_override.tenant, RGW_USER_ANON_ID);
   } else {
     /* Compatibility mechanism for multi-tenancy. For more details refer to
      * load_acct_info method of rgw::auth::RemoteApplier. */
diff --git a/src/rgw/rgw_swift_auth.h b/src/rgw/rgw_swift_auth.h
index 8cefbf1a4880..fe1553bf322f 100644
--- a/src/rgw/rgw_swift_auth.h
+++ b/src/rgw/rgw_swift_auth.h
@@ -152,7 +152,7 @@ class SwiftAnonymousApplier : public rgw::auth::LocalApplier {
       : LocalApplier(cct, user_info, LocalApplier::NO_SUBUSER, boost::none) {
       };
     bool is_admin_of(const rgw_user& uid) const {return false;}
-    bool is_owner_of(const rgw_user& uid) const {return false;}
+    bool is_owner_of(const rgw_user& uid) const {return uid.id.compare(RGW_USER_ANON_ID) == 0;}
 };
 
 class SwiftAnonymousEngine : public rgw::auth::AnonymousEngine {
@@ -207,11 +207,8 @@ class DefaultStrategy : public rgw::auth::Strategy,
                              const req_state* const s,
                              acl_strategy_t&& extra_acl_strategy,
                              const rgw::auth::RemoteApplier::AuthInfo &info) const override {
-    rgw_user user(s->account_name);
-    if (info.is_anon())
-      user = rgw_user(RGW_USER_ANON_ID);
     auto apl = \
-      rgw::auth::add_3rdparty(ctl, user,
+      rgw::auth::add_3rdparty(ctl, rgw_user(s->account_name),
         rgw::auth::add_sysreq(cct, ctl, s,
           rgw::auth::RemoteApplier(cct, ctl, std::move(extra_acl_strategy), info,
                                    implicit_tenant_context,
@@ -225,11 +222,8 @@ class DefaultStrategy : public rgw::auth::Strategy,
                             const RGWUserInfo& user_info,
                             const std::string& subuser,
                             const boost::optional<uint32_t>& perm_mask) const override {
-    rgw_user user(s->account_name);
-    if (user_info.user_id.compare(RGW_USER_ANON_ID) == 0)
-      user = rgw_user(user_info.user_id);
     auto apl = \
-      rgw::auth::add_3rdparty(ctl, user,
+      rgw::auth::add_3rdparty(ctl, rgw_user(s->account_name),
         rgw::auth::add_sysreq(cct, ctl, s,
           rgw::auth::LocalApplier(cct, user_info, subuser, perm_mask)));
     /* TODO(rzarzynski): replace with static_ptr. */
@@ -263,7 +257,7 @@ public:
                       static_cast<rgw::auth::TokenExtractor*>(this),
                       static_cast<rgw::auth::LocalApplier::Factory*>(this)),
       anon_engine(cct,
-                  static_cast<rgw::auth::LocalApplier::Factory*>(this),
+                  static_cast<SwiftAnonymousApplier::Factory*>(this),
                   static_cast<rgw::auth::TokenExtractor*>(this)) {
     /* When the constructor's body is being executed, all member engines
      * should be initialized. Thus, we can safely add them. */