From: Sage Weil <sage@redhat.com>
Date: Wed, 28 Feb 2018 21:59:25 +0000 (-0600)
Subject: common/config: intercept "keyfile", translate into "key"
X-Git-Tag: v13.0.2~78^2~6
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=91e8da14312b62f07d68b3aebca3bfb00a9cfc6e;p=ceph.git

common/config: intercept "keyfile", translate into "key"

The keyfile arg might be - (stdin), which we can only read once.  Ensure
that we consume it once by intercepting the CLI value early and inserting
the value into the 'key' option.

This robs future code of the knowledge that the key came from --keyfile
and not --key, but avoids the issue of multiple users (notably, KeyRing.cc
and the OSD mkfs code).

Remove the - special case from OSD at the same time, since it can no
longer be reached (unless something other than the CLI specified '-', but
neither ceph.conf nor the mon config make sense here).

Signed-off-by: Sage Weil <sage@redhat.com>
---

diff --git a/src/common/config.cc b/src/common/config.cc
index 30142575c28b..5e8f7452c3fa 100644
--- a/src/common/config.cc
+++ b/src/common/config.cc
@@ -588,7 +588,18 @@ int md_config_t::parse_argv(std::vector<const char*>& args, int level)
       set_val_or_die("public_addr", val.c_str());
     }
     else if (ceph_argparse_witharg(args, i, &val, "--keyfile", "-K", (char*)NULL)) {
-      set_val_or_die("keyfile", val.c_str());
+      bufferlist bl;
+      string err;
+      int r;
+      if (val == "-") {
+	r = bl.read_fd(STDIN_FILENO, 1024);
+      } else {
+	r = bl.read_file(val.c_str(), &err);
+      }
+      if (r >= 0) {
+	string k(bl.c_str(), bl.length());
+	set_val_or_die("key", k.c_str());
+      }
     }
     else if (ceph_argparse_witharg(args, i, &val, "--keyring", "-k", (char*)NULL)) {
       set_val_or_die("keyring", val.c_str());
diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index bf1659fba218..f2ad6778f345 100644
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -1881,15 +1881,7 @@ int OSD::write_meta(CephContext *cct, ObjectStore *store, uuid_d& cluster_fsid,
     if (!keyfile.empty()) {
       bufferlist keybl;
       string err;
-      if (keyfile == "-") {
-	static_assert(1024 * 1024 >
-		      (sizeof(CryptoKey) - sizeof(bufferptr) +
-		       sizeof(__u16) + 16 /* AES_KEY_LEN */ + 3 - 1) / 3. * 4.,
-		      "1MB should be enough for a base64 encoded CryptoKey");
-	r = keybl.read_fd(STDIN_FILENO, 1024 * 1024);
-      } else {
-	r = keybl.read_file(keyfile.c_str(), &err);
-      }
+      r = keybl.read_file(keyfile.c_str(), &err);
       if (r < 0) {
 	derr << __func__ << " failed to read keyfile " << keyfile << ": "
 	     << err << ": " << cpp_strerror(r) << dendl;
diff --git a/src/test/daemon_config.cc b/src/test/daemon_config.cc
index 221b854d635c..df59c32b1628 100644
--- a/src/test/daemon_config.cc
+++ b/src/test/daemon_config.cc
@@ -128,7 +128,7 @@ TEST(DaemonConfig, ArgV) {
 
   int ret;
   const char *argv[] = { "foo", "--log-graylog-port", "22",
-			 "--keyfile", "/tmp/my-keyfile", NULL };
+			 "--key", "my-key", NULL };
   size_t argc = (sizeof(argv) / sizeof(argv[0])) - 1;
   vector<const char*> args;
   argv_to_vec(argc, argv, args);
@@ -138,9 +138,9 @@ TEST(DaemonConfig, ArgV) {
   char buf[128];
   char *tmp = buf;
   memset(buf, 0, sizeof(buf));
-  ret = g_ceph_context->_conf->get_val("keyfile", &tmp, sizeof(buf));
+  ret = g_ceph_context->_conf->get_val("key", &tmp, sizeof(buf));
   ASSERT_EQ(0, ret);
-  ASSERT_EQ(string("/tmp/my-keyfile"), string(buf));
+  ASSERT_EQ(string("my-key"), string(buf));
 
   memset(buf, 0, sizeof(buf));
   ret = g_ceph_context->_conf->get_val("log_graylog_port", &tmp, sizeof(buf));
diff --git a/src/test/libcephfs_config.cc b/src/test/libcephfs_config.cc
index d55a5fa563b8..dd8acc835b4d 100644
--- a/src/test/libcephfs_config.cc
+++ b/src/test/libcephfs_config.cc
@@ -44,15 +44,15 @@ TEST(LibCephConfig, ArgV) {
   ASSERT_EQ(ret, 0);
 
   const char *argv[] = { "foo", "--leveldb-max-open-files", "2",
-			 "--keyfile", "/tmp/my-keyfile", NULL };
+			 "--key", "my-key", NULL };
   size_t argc = (sizeof(argv) / sizeof(argv[0])) - 1;
   ceph_conf_parse_argv(cmount, argc, argv);
 
   char buf[128];
   memset(buf, 0, sizeof(buf));
-  ret = ceph_conf_get(cmount, "keyfile", buf, sizeof(buf));
+  ret = ceph_conf_get(cmount, "key", buf, sizeof(buf));
   ASSERT_EQ(ret, 0);
-  ASSERT_EQ(string("/tmp/my-keyfile"), string(buf));
+  ASSERT_EQ(string("my-key"), string(buf));
 
   memset(buf, 0, sizeof(buf));
   ret = ceph_conf_get(cmount, "leveldb_max_open_files", buf, sizeof(buf));
diff --git a/src/test/librados/librados_config.cc b/src/test/librados/librados_config.cc
index 623454be59e0..d30fb30efbfc 100644
--- a/src/test/librados/librados_config.cc
+++ b/src/test/librados/librados_config.cc
@@ -45,15 +45,15 @@ TEST(LibRadosConfig, ArgV) {
   ASSERT_EQ(ret, 0);
 
   const char *argv[] = { "foo", "--leveldb-max-open-files", "2",
-			 "--keyfile", "/tmp/my-keyfile", NULL };
+			 "--key", "my-key", NULL };
   size_t argc = (sizeof(argv) / sizeof(argv[0])) - 1;
   rados_conf_parse_argv(cl, argc, argv);
 
   char buf[128];
   memset(buf, 0, sizeof(buf));
-  ret = rados_conf_get(cl, "keyfile", buf, sizeof(buf));
+  ret = rados_conf_get(cl, "key", buf, sizeof(buf));
   ASSERT_EQ(ret, 0);
-  ASSERT_EQ(string("/tmp/my-keyfile"), string(buf));
+  ASSERT_EQ(string("my-key"), string(buf));
 
   memset(buf, 0, sizeof(buf));
   ret = rados_conf_get(cl, "leveldb_max_open_files", buf, sizeof(buf));