From c99f33b68041b8a34764cf1a45e4d6772aa84a41 Mon Sep 17 00:00:00 2001 From: Matthew Oliver Date: Wed, 26 Feb 2020 06:15:22 +0000 Subject: [PATCH] rgw: anonomous swift to obj that dont exist should 401 Currently, if you attempt to GET and object in the Swift API that doesn't exist and you don't pass a `X-Auth-Token` it will 404 instead of 401. This is actually a rather big problem as it means someone can leak data out of the cluster, not object data itself, but if an object exists or not. This is caused by the SwiftAnonymousEngine's, frankly wide open is_applicable acceptance. When we get to checking the bucket or object for user acceptance we deal with it properly, but if the object doesn't exsit, because the user has been "authorised" rgw returns a 404. Why? Because we always override the user with the Swift account. Meaning as far as checks are concerned the auth user is the user, not and anonymous user. I assume this is because a swift container could have world readable reads or writes and in slight s3 and swift api divergents can make these interesting edge cases leak in. This patch doesn't change the user to the swift account if they are anonymous. So we can do some anonymous checks when it suits later in the request processing path. Fixes: https://tracker.ceph.com/issues/43617 Signed-off-by: Matthew Oliver (cherry picked from commit b03d9754e113d24221f1ce0bac17556ab0017a8a) Conflicts: src/rgw/rgw_swift_auth.h - where master has "rgw_user(s->account_name)", nautilus has "s->account_name" only --- src/rgw/rgw_auth.h | 1 + src/rgw/rgw_op.cc | 2 ++ src/rgw/rgw_swift_auth.h | 24 +++++++++++++++++++++--- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/rgw/rgw_auth.h b/src/rgw/rgw_auth.h index a6612ac44caca..2aaa1a5ac1f87 100644 --- a/src/rgw/rgw_auth.h +++ b/src/rgw/rgw_auth.h @@ -484,6 +484,7 @@ 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_op.cc b/src/rgw/rgw_op.cc index 72bf870a5736b..ae666a726f510 100644 --- a/src/rgw/rgw_op.cc +++ b/src/rgw/rgw_op.cc @@ -7431,6 +7431,8 @@ int RGWHandler::do_read_permissions(RGWOp *op, bool only_bucket) << " ret=" << ret << dendl; if (ret == -ENODATA) ret = -EACCES; + if (s->auth.identity->is_anonymous() && ret == -EACCES) + ret = -EPERM; } return ret; diff --git a/src/rgw/rgw_swift_auth.h b/src/rgw/rgw_swift_auth.h index b14133f4a9d9e..b350c3e134289 100644 --- a/src/rgw/rgw_swift_auth.h +++ b/src/rgw/rgw_swift_auth.h @@ -4,6 +4,8 @@ #ifndef CEPH_RGW_SWIFT_AUTH_H #define CEPH_RGW_SWIFT_AUTH_H +#include "rgw_common.h" +#include "rgw_user.h" #include "rgw_op.h" #include "rgw_rest.h" #include "rgw_auth.h" @@ -141,6 +143,16 @@ public: } }; +/* SwiftAnonymous: applier. */ +class SwiftAnonymousApplier : public rgw::auth::LocalApplier { + public: + SwiftAnonymousApplier(CephContext* const cct, + const RGWUserInfo& user_info) + : 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;} +}; class SwiftAnonymousEngine : public rgw::auth::AnonymousEngine { const rgw::auth::TokenExtractor* const extractor; @@ -151,7 +163,7 @@ class SwiftAnonymousEngine : public rgw::auth::AnonymousEngine { public: SwiftAnonymousEngine(CephContext* const cct, - const rgw::auth::LocalApplier::Factory* const apl_factory, + const SwiftAnonymousApplier::Factory* const apl_factory, const rgw::auth::TokenExtractor* const extractor) : AnonymousEngine(cct, apl_factory), extractor(extractor) { @@ -194,8 +206,11 @@ 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, s->account_name, + rgw::auth::add_3rdparty(store, user, rgw::auth::add_sysreq(cct, store, s, rgw::auth::RemoteApplier(cct, store, std::move(extra_acl_strategy), info, implicit_tenant_context, @@ -209,8 +224,11 @@ class DefaultStrategy : public rgw::auth::Strategy, const RGWUserInfo& user_info, const std::string& subuser, const boost::optional& 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, s->account_name, + rgw::auth::add_3rdparty(store, user, rgw::auth::add_sysreq(cct, store, s, rgw::auth::LocalApplier(cct, user_info, subuser, perm_mask))); /* TODO(rzarzynski): replace with static_ptr. */ -- 2.39.5