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)

Conflicts:
	src/rgw/rgw_swift_auth.h
	  - only need to modify the user related code to rgw_user construct

diff --git a/src/rgw/rgw_auth.h b/src/rgw/rgw_auth.h
index dad0897697f00c2e541a03d64823012be312c36d..be7a102a714990b11fedbbe68bda76217ee0df6b 100644 (file)
--- a/src/rgw/rgw_auth.h
+++ b/src/rgw/rgw_auth.h
@@ -57,7 +57,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. */
@@ -491,7 +491,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 a2b62e2cff58e74db40db16199c5290b7301fea3..58436022efccedbf39e83da2ef793cbd6482331b 100644 (file)
--- 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 b350c3e134289d91554b57b119c2a8e6570669aa..33d1cb2289da4c67f473b1b31aa9c3998e5b9ed8 100644 (file)
--- a/src/rgw/rgw_swift_auth.h
+++ b/src/rgw/rgw_swift_auth.h
@@ -151,7 +151,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 {
@@ -206,11 +206,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(store, user,
+      rgw::auth::add_3rdparty(store, rgw_user(s->account_name),
         rgw::auth::add_sysreq(cct, store, s,
           rgw::auth::RemoteApplier(cct, store, std::move(extra_acl_strategy), info,
                                    implicit_tenant_context,
@@ -224,11 +221,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(store, user,
+      rgw::auth::add_3rdparty(store, rgw_user(s->account_name),
         rgw::auth::add_sysreq(cct, store, s,
           rgw::auth::LocalApplier(cct, user_info, subuser, perm_mask)));
     /* TODO(rzarzynski): replace with static_ptr. */
@@ -262,7 +256,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. */