From e636a56c502815903c95f1d2df0bc818034fea51 Mon Sep 17 00:00:00 2001
From: Matt Benjamin <mbenjamin@redhat.com>
Date: Mon, 29 Oct 2018 21:21:21 -0400
Subject: [PATCH] rgw: move keystone secrets from ceph.conf to files

Permits setting restrictive permissions on these secrets.

Fixes: http://tracker.ceph.com/issues/36621

Signed-off-by: Matt Benjamin <mbenjamin@redhat.com>
---
 doc/radosgw/config-ref.rst      | 29 +++++++++++++++--
 doc/radosgw/keystone.rst        |  2 ++
 src/common/legacy_config_opts.h |  2 ++
 src/common/options.cc           | 12 +++++--
 src/rgw/rgw_keystone.cc         | 57 +++++++++++++++++++++++++++++++++
 src/rgw/rgw_keystone.h          | 14 ++++----
 src/rgw/rgw_main.cc             |  6 ++++
 7 files changed, 109 insertions(+), 13 deletions(-)

diff --git a/doc/radosgw/config-ref.rst b/doc/radosgw/config-ref.rst
index abaeafba821..3e95100772d 100644
--- a/doc/radosgw/config-ref.rst
+++ b/doc/radosgw/config-ref.rst
@@ -763,11 +763,26 @@ Keystone Settings
               authentication with the admin credentials
               (``rgw keystone admin user``, ``rgw keystone admin password``,
               ``rgw keystone admin tenant``, ``rgw keystone admin project``,
-              ``rgw keystone admin domain``). Admin token feature is considered
-              as deprecated.
+              ``rgw keystone admin domain``). The Keystone admin token
+              has been deprecated, but can be used to integrate with
+              older environments.  Prefer ``rgw keystone admin token path``
+              to avoid exposing the token.
 :Type: String
 :Default: None
 
+``rgw keystone admin token path``
+
+:Description: Path to a file containing the Keystone admin token
+	      (shared secret).  In Ceph RadosGW authentication with
+	      the admin token has priority over authentication with
+	      the admin credentials
+              (``rgw keystone admin user``, ``rgw keystone admin password``,
+              ``rgw keystone admin tenant``, ``rgw keystone admin project``,
+              ``rgw keystone admin domain``).
+              The Keystone admin token has been deprecated, but can be
+              used to integrate with older environments.
+:Type: String
+:Default: None
 
 ``rgw keystone admin tenant``
 
@@ -788,7 +803,15 @@ Keystone Settings
 ``rgw keystone admin password``
 
 :Description: The password for OpenStack admin user when using OpenStack
-              Identity API v2
+              Identity API v2.  Prefer ``rgw keystone admin password path``
+              to avoid exposing the token.
+:Type: String
+:Default: None
+
+``rgw keystone admin password path``
+
+:Description: Path to a file containing the password for OpenStack
+              admin user when using OpenStack Identity API v2.
 :Type: String
 :Default: None
 
diff --git a/doc/radosgw/keystone.rst b/doc/radosgw/keystone.rst
index 398276c74a5..db283e04cbf 100644
--- a/doc/radosgw/keystone.rst
+++ b/doc/radosgw/keystone.rst
@@ -14,6 +14,7 @@ The following configuration options are available for Keystone integration::
 	rgw keystone api version = {keystone api version}
 	rgw keystone url = {keystone server url:keystone server admin port}
 	rgw keystone admin token = {keystone admin token}
+	rgw keystone admin token path = {path to keystone admin token} #preferred
 	rgw keystone accepted roles = {accepted user roles}
 	rgw keystone token cache size = {number of tokens to cache}
 	rgw keystone revocation interval = {number of seconds before checking revoked tickets}
@@ -32,6 +33,7 @@ configuration options for are::
 
    rgw keystone admin user = {keystone service tenant user name}
    rgw keystone admin password = {keystone service tenant user password}
+   rgw keystone admin password = {keystone service tenant user password path} # preferred
    rgw keystone admin tenant = {keystone service tenant name}
 
 
diff --git a/src/common/legacy_config_opts.h b/src/common/legacy_config_opts.h
index e1d5e8458d2..b85d7147b5e 100644
--- a/src/common/legacy_config_opts.h
+++ b/src/common/legacy_config_opts.h
@@ -1314,8 +1314,10 @@ OPTION(rgw_swift_account_in_url, OPT_BOOL)  // assume that URL always contain th
 OPTION(rgw_swift_enforce_content_length, OPT_BOOL)  // enforce generation of Content-Length even in cost of performance or scalability
 OPTION(rgw_keystone_url, OPT_STR)  // url for keystone server
 OPTION(rgw_keystone_admin_token, OPT_STR)  // keystone admin token (shared secret)
+OPTION(rgw_keystone_admin_token_path, OPT_STR)  // path to keystone admin token (shared secret)
 OPTION(rgw_keystone_admin_user, OPT_STR)  // keystone admin user name
 OPTION(rgw_keystone_admin_password, OPT_STR)  // keystone admin user password
+OPTION(rgw_keystone_admin_password_path, OPT_STR)  // path to keystone admin user password
 OPTION(rgw_keystone_admin_tenant, OPT_STR)  // keystone admin user tenant (for keystone v2.0)
 OPTION(rgw_keystone_admin_project, OPT_STR)  // keystone admin user project (for keystone v3)
 OPTION(rgw_keystone_admin_domain, OPT_STR)  // keystone admin user domain
diff --git a/src/common/options.cc b/src/common/options.cc
index 49cb29a08f6..d8507244471 100644
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -5296,7 +5296,11 @@ std::vector<Option> get_rgw_options() {
 
     Option("rgw_keystone_admin_token", Option::TYPE_STR, Option::LEVEL_ADVANCED)
     .set_default("")
-    .set_description("The admin token (shared secret) that is used for the Keystone requests."),
+    .set_description("DEPRECATED: The admin token (shared secret) that is used for the Keystone requests."),
+
+    Option("rgw_keystone_admin_token_path", Option::TYPE_STR, Option::LEVEL_ADVANCED)
+    .set_default("")
+    .set_description("Path to a file containing the admin token (shared secret) that is used for the Keystone requests."),
 
     Option("rgw_keystone_admin_user", Option::TYPE_STR, Option::LEVEL_ADVANCED)
     .set_default("")
@@ -5304,7 +5308,11 @@ std::vector<Option> get_rgw_options() {
 
     Option("rgw_keystone_admin_password", Option::TYPE_STR, Option::LEVEL_ADVANCED)
     .set_default("")
-    .set_description("Keystone admin password."),
+    .set_description("DEPRECATED: Keystone admin password."),
+
+    Option("rgw_keystone_admin_password_path", Option::TYPE_STR, Option::LEVEL_ADVANCED)
+    .set_default("")
+    .set_description("Path to a file containing the Keystone admin password."),
 
     Option("rgw_keystone_admin_tenant", Option::TYPE_STR, Option::LEVEL_ADVANCED)
     .set_default("")
diff --git a/src/rgw/rgw_keystone.cc b/src/rgw/rgw_keystone.cc
index dda39475277..b6fe15a8c1e 100644
--- a/src/rgw/rgw_keystone.cc
+++ b/src/rgw/rgw_keystone.cc
@@ -5,6 +5,8 @@
 #include <fnmatch.h>
 
 #include <boost/algorithm/string/predicate.hpp>
+#include <boost/algorithm/string.hpp>
+#include <fstream>
 
 #include "common/errno.h"
 #include "common/ceph_json.h"
@@ -170,6 +172,61 @@ std::string CephCtxConfig::get_endpoint_url() const noexcept
   }
 }
 
+/* secrets */
+const std::string CephCtxConfig::empty{""};
+
+static inline std::string read_secret(const std::string& file_path)
+{
+  using namespace std;
+
+  constexpr int16_t size{1024};
+  char buf[size];
+  string s;
+
+  s.reserve(size);
+  ifstream ifs(file_path, ios::in | ios::binary);
+  if (ifs) {
+    while (true) {
+      auto sbuf = ifs.rdbuf();
+      auto len =  sbuf->sgetn(buf, size);
+      if (!len)
+	break;
+      s.append(buf, len);
+    }
+    boost::algorithm::trim(s);
+    if (s.back() == '\n')
+      s.pop_back();
+  }
+  return s;
+}
+
+std::string CephCtxConfig::get_admin_token() const noexcept
+{
+  auto& atv = g_ceph_context->_conf->rgw_keystone_admin_token_path;
+  if (!atv.empty()) {
+    return read_secret(atv);
+  } else {
+    auto& atv = g_ceph_context->_conf->rgw_keystone_admin_token;
+    if (!atv.empty()) {
+      return atv;
+    }
+  }
+  return empty;
+}
+
+std::string CephCtxConfig::get_admin_password() const noexcept  {
+  auto& apv = g_ceph_context->_conf->rgw_keystone_admin_password_path;
+  if (!apv.empty()) {
+    return read_secret(apv);
+  } else {
+    auto& apv = g_ceph_context->_conf->rgw_keystone_admin_password;
+    if (!apv.empty()) {
+      return apv;
+    }
+  }
+  return empty;
+}
+
 int Service::get_admin_token(CephContext* const cct,
                              TokenCache& token_cache,
                              const Config& config,
diff --git a/src/rgw/rgw_keystone.h b/src/rgw/rgw_keystone.h
index 8790f299903..55ad2f9412e 100644
--- a/src/rgw/rgw_keystone.h
+++ b/src/rgw/rgw_keystone.h
@@ -53,9 +53,9 @@ public:
   virtual std::string get_endpoint_url() const noexcept = 0;
   virtual ApiVersion get_api_version() const noexcept = 0;
 
-  virtual boost::string_ref get_admin_token() const noexcept = 0;
+  virtual std::string get_admin_token() const noexcept = 0;
   virtual boost::string_ref get_admin_user() const noexcept = 0;
-  virtual boost::string_ref get_admin_password() const noexcept = 0;
+  virtual std::string get_admin_password() const noexcept = 0;
   virtual boost::string_ref get_admin_tenant() const noexcept = 0;
   virtual boost::string_ref get_admin_project() const noexcept = 0;
   virtual boost::string_ref get_admin_domain() const noexcept = 0;
@@ -66,6 +66,8 @@ protected:
   CephCtxConfig() = default;
   virtual ~CephCtxConfig() = default;
 
+  const static std::string empty;
+
 public:
   static CephCtxConfig& get_instance() {
     static CephCtxConfig instance;
@@ -75,17 +77,13 @@ public:
   std::string get_endpoint_url() const noexcept override;
   ApiVersion get_api_version() const noexcept override;
 
-  boost::string_ref get_admin_token() const noexcept override {
-    return g_ceph_context->_conf->rgw_keystone_admin_token;
-  }
+  std::string get_admin_token() const noexcept override;
 
   boost::string_ref get_admin_user() const noexcept override {
     return g_ceph_context->_conf->rgw_keystone_admin_user;
   }
 
-  boost::string_ref get_admin_password() const noexcept override {
-    return g_ceph_context->_conf->rgw_keystone_admin_password;
-  }
+  std::string get_admin_password() const noexcept override;
 
   boost::string_ref get_admin_tenant() const noexcept override {
     return g_ceph_context->_conf->rgw_keystone_admin_tenant;
diff --git a/src/rgw/rgw_main.cc b/src/rgw/rgw_main.cc
index 53e65609386..b7513d930f0 100644
--- a/src/rgw/rgw_main.cc
+++ b/src/rgw/rgw_main.cc
@@ -340,6 +340,12 @@ int main(int argc, const char **argv)
     apis_map[*li] = true;
   }
 
+  /* warn about insecure keystone secret config options */
+  if (!(g_ceph_context->_conf->rgw_keystone_admin_token.empty() ||
+	g_ceph_context->_conf->rgw_keystone_admin_password.empty())) {
+    dout(0) << "WARNING: rgw_keystone_admin_token and rgw_keystone_admin_password should be avoided as they can expose secrets.  Prefer the new rgw_keystone_admin_token_path and rgw_keystone_admin_password_path options, which read their secrets from files." << dendl;
+  }
+
   // S3 website mode is a specialization of S3
   const bool s3website_enabled = apis_map.count("s3website") > 0;
   const bool sts_enabled = apis_map.count("sts") > 0;
-- 
2.39.5