From d6cba8e8adf35490a62a3541ae992b5ef36108cd Mon Sep 17 00:00:00 2001
From: Pritha Srivastava <prsrivas@redhat.com>
Date: Mon, 27 Aug 2018 21:07:34 +0530
Subject: [PATCH] rgw: STS auth changes to make sure that other engines are not
 aware of session token.

Signed-off-by: Pritha Srivastava <prsrivas@redhat.com>
---
 src/common/options.cc        |  2 +-
 src/rgw/rgw_auth_keystone.cc | 12 ------------
 src/rgw/rgw_auth_s3.h        | 11 ++++++-----
 src/rgw/rgw_rest_s3.cc       | 17 +++++------------
 4 files changed, 12 insertions(+), 30 deletions(-)

diff --git a/src/common/options.cc b/src/common/options.cc
index 39ebc1a932d..8f80b0cbb53 100644
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -5372,7 +5372,7 @@ std::vector<Option> get_rgw_options() {
     .set_description("Should S3 authentication use Keystone."),
 
     Option("rgw_s3_auth_order", Option::TYPE_STR, Option::LEVEL_ADVANCED)
-     .set_default("external, local, sts")
+     .set_default("sts, external, local")
      .set_description("Authentication strategy order to use for s3 authentication")
      .set_long_description(
 	  "Order of authentication strategies to try for s3 authentication, the allowed "
diff --git a/src/rgw/rgw_auth_keystone.cc b/src/rgw/rgw_auth_keystone.cc
index 04faae07fa9..03a39610eec 100644
--- a/src/rgw/rgw_auth_keystone.cc
+++ b/src/rgw/rgw_auth_keystone.cc
@@ -208,12 +208,6 @@ TokenEngine::result_t
 TokenEngine::authenticate(const std::string& token,
                           const req_state* const s) const
 {
-  //If Keystone is enabled and the request has a session token, then keystone shouldn't authenticate it.
-  if (s->info.args.exists("X-Amz-Security-Token") ||
-      s->info.env->exists("HTTP_X_AMZ_SECURITY_TOKEN")) {
-    return result_t::deny();
-  }
-
   boost::optional<TokenEngine::token_envelope_t> t;
 
   /* This will be initialized on the first call to this method. In C++11 it's
@@ -433,12 +427,6 @@ rgw::auth::Engine::result_t EC2Engine::authenticate(
   /* Passthorugh only! */
   const req_state* s) const
 {
-  //If Keystone is enabled and the request has a session token, then Keystone shouldn't authenticate it.
-  if (s->info.args.exists("X-Amz-Security-Token") ||
-      s->info.env->exists("HTTP_X_AMZ_SECURITY_TOKEN")) {
-    return result_t::deny();
-  }
-
   /* This will be initialized on the first call to this method. In C++11 it's
    * also thread-safe. */
   static const struct RolesCacher {
diff --git a/src/rgw/rgw_auth_s3.h b/src/rgw/rgw_auth_s3.h
index 68e4057491d..cbf2a3f9ee4 100644
--- a/src/rgw/rgw_auth_s3.h
+++ b/src/rgw/rgw_auth_s3.h
@@ -219,6 +219,12 @@ public:
 
     auto auth_order = parse_auth_order(cct);
     engine_map_t engine_map;
+
+    /* STS Auth*/
+    if (! sts_engine.is_empty()) {
+      engine_map.insert(std::make_pair("sts", std::cref(sts_engine)));
+    }
+
     /* The external auth. */
     if (! external_engines.is_empty()) {
       engine_map.insert(std::make_pair("external", std::cref(external_engines)));
@@ -228,11 +234,6 @@ public:
       engine_map.insert(std::make_pair("local", std::cref(local_engine)));
     }
 
-    /* STS Auth*/
-    if (! sts_engine.is_empty()) {
-      engine_map.insert(std::make_pair("sts", std::cref(sts_engine)));
-    }
-
     add_engines(auth_order, engine_map);
   }
 
diff --git a/src/rgw/rgw_rest_s3.cc b/src/rgw/rgw_rest_s3.cc
index d5a1bed14c5..857cb49a522 100644
--- a/src/rgw/rgw_rest_s3.cc
+++ b/src/rgw/rgw_rest_s3.cc
@@ -4238,12 +4238,6 @@ rgw::auth::s3::LDAPEngine::authenticate(
   const completer_factory_t& completer_factory,
   const req_state* const s) const
 {
-  //If LDAP is enabled and the request has a session token, then LDAP shouldn't authenticate it.
-  if (s->info.args.exists("X-Amz-Security-Token") ||
-      s->info.env->exists("HTTP_X_AMZ_SECURITY_TOKEN")) {
-    return result_t::deny();
-  }
-
   /* boost filters and/or string_ref may throw on invalid input */
   rgw::RGWToken base64_token;
   try {
@@ -4295,12 +4289,6 @@ rgw::auth::s3::LocalEngine::authenticate(
   const completer_factory_t& completer_factory,
   const req_state* const s) const
 {
-  //If LocalAuth is enabled and the request has a session token, then LocalEngine shouldn't authenticate it.
-  if (s->info.args.exists("X-Amz-Security-Token") ||
-      s->info.env->exists("HTTP_X_AMZ_SECURITY_TOKEN")) {
-    return result_t::deny();
-  }
-
   /* get the user info */
   RGWUserInfo user_info;
   /* TODO(rzarzynski): we need to have string-view taking variant. */
@@ -4410,6 +4398,11 @@ rgw::auth::s3::STSEngine::authenticate(
   const completer_factory_t& completer_factory,
   const req_state* const s) const
 {
+  if (! s->info.args.exists("X-Amz-Security-Token") &&
+      ! s->info.env->exists("HTTP_X_AMZ_SECURITY_TOKEN")) {
+    return result_t::deny();
+  }
+
   STS::SessionToken token;
   if (int ret = get_session_token(session_token, token); ret < 0) {
     return result_t::deny(ret);
-- 
2.39.5