auth,mon: print key loading failures once at init

The routine was structured to print warnings frequently whenever the
config is refreshed (manually or normally). This confused Rook with
verbose debugging warnings so I tried to clean that up in an earlier
commit. This unfortunately broke command-line keyfile/key arguments.
I've resolved that now robustly by putting the error message in a
throwaway buffer that can be printed only from the init routines.

Lastly, this commit changes the preference for sourcing the key to
configs "key", then "keyfile", then "keyring". Before, it checked the
keyring file but, as this is usually set in the ceph.conf, that prevents
overriding with another key.

Signed-off-by: Patrick Donnelly <pdonnell@ibm.com>

diff --git a/src/auth/AuthRegistry.cc b/src/auth/AuthRegistry.cc
index 7c8a72b7b797e51793a4e1bcf8d6c8f59830f97e..5200ff4698975554e1feb971f5a18b63938199bf 100644 (file)
--- a/src/auth/AuthRegistry.cc
+++ b/src/auth/AuthRegistry.cc
@@ -9,6 +9,7 @@
 #endif
 #include "none/AuthNoneAuthorizeHandler.h"
 #include "common/ceph_context.h"
+#include "common/StackStringStream.h"
 #include "common/debug.h"
 #include "auth/KeyRing.h"
 
@@ -164,9 +165,12 @@ void AuthRegistry::_refresh_config()
     }
   }
   if (any_cephx) {
+    ldout(cct, 20) << "attempting to load cephx key" << dendl;
     KeyRing k;
-    int r = k.from_ceph_context(cct);
-    if (r == -ENOENT) {
+    CachedStackStringStream css;
+    int r = k.from_ceph_context(cct, css.get());
+    if (r < 0) {
+      ldout(cct, 5) << "removing any cephx auth method as loading cephx key failed: " << css->strv() << dendl;
       for (auto *p : {&cluster_methods, &service_methods, &client_methods}) {
        auto q = std::find(p->begin(), p->end(), CEPH_AUTH_CEPHX);
        if (q != p->end()) {

diff --git a/src/auth/KeyRing.cc b/src/auth/KeyRing.cc
index 37c1d67316ad7f3f0c3729a6d3f73495c4be350c..c2b5222bed0f938943c7e621d2d8450e043507c0 100644 (file)
--- a/src/auth/KeyRing.cc
+++ b/src/auth/KeyRing.cc
@@ -40,20 +40,9 @@ using std::string;
 using ceph::bufferlist;
 using ceph::Formatter;
 
-int KeyRing::from_ceph_context(CephContext *cct)
+int KeyRing::from_ceph_context(CephContext *cct, std::ostream* os)
 {
   const auto& conf = cct->_conf;
-  string filename;
-
-  int ret = ceph_resolve_file_search(conf->keyring, filename);
-  if (!ret) {
-    ret = load(cct, filename);
-    if (ret < 0) {
-      lderr(cct) << "failed to load " << filename
-                << ": " << cpp_strerror(ret) << dendl;
-    }
-    return ret;
-  }
 
   if (!conf->key.empty()) {
     EntityAuth ea;
@@ -63,7 +52,7 @@ int KeyRing::from_ceph_context(CephContext *cct)
       return 0;
     }
     catch (ceph::buffer::error& e) {
-      lderr(cct) << "failed to decode key '" << conf->key << "'" << dendl;
+      *os << "failed to decode key from 'key' config";
       return -EINVAL;
     }
   }
@@ -73,24 +62,40 @@ int KeyRing::from_ceph_context(CephContext *cct)
     string err;
     int r = bl.read_file(conf->keyfile.c_str(), &err);
     if (r < 0) {
-      lderr(cct) << err << dendl;
+      *os << "unable to read keyfile '" << conf->keyfile << "': " << err;
       return r;
+    } else {
+      string k(bl.c_str(), bl.length());
+      EntityAuth ea;
+      try {
+        ea.key.decode_base64(k);
+        add(conf->name, ea);
+        return 0;
+      }
+      catch (ceph::buffer::error& e) {
+        *os << "failed to decode key from 'keyfile' config: '" << conf->keyfile << "'";
+        return -EINVAL;
+      }
     }
-    string k(bl.c_str(), bl.length());
-    EntityAuth ea;
-    try {
-      ea.key.decode_base64(k);
-      add(conf->name, ea);
-      return 0;
-    }
-    catch (ceph::buffer::error& e) {
-      lderr(cct) << "failed to decode key '" << k << "'" << dendl;
-      return -EINVAL;
+  }
+
+  if (!conf->keyring.empty()) {
+    string filename;
+    int ret = ceph_resolve_file_search(conf->keyring, filename);
+    if (!ret) {
+      ret = load(cct, filename);
+      if (ret < 0) {
+        *os << "failed to load 'keyring' config file '" << filename << "': " << cpp_strerror(ret);
+      }
+      return ret;
+    } else {
+      *os << "unable to find a keyring via 'keyring' config " << conf->keyring << ": " << cpp_strerror(ret);
+      return -ENOENT;
     }
   }
 
-  /* this can happen during startup when configs are still being set */
-  ldout(cct, 2) << "unable to find a keyring on " << conf->keyring << ": " << cpp_strerror(ret) << dendl;
+  /* Note: this can happen during startup when configs are still being set */
+  *os << "unable to find suitable key from configs 'keyring', 'keyfile', or 'key'";
   return -ENOENT;
 }
 

diff --git a/src/auth/KeyRing.h b/src/auth/KeyRing.h
index 2e61245b83e1e70541e244378b779d794542b5a8..2ded794178f4a450fc8650deacb27332ffda46c7 100644 (file)
--- a/src/auth/KeyRing.h
+++ b/src/auth/KeyRing.h
@@ -27,7 +27,7 @@ public:
   void decode_plaintext(ceph::buffer::list::const_iterator& bl);
   /* Create a KeyRing from a Ceph context.
    * We will use the configuration stored inside the context. */
-  int from_ceph_context(CephContext *cct);
+  int from_ceph_context(CephContext* cct, std::ostream* os);
 
   std::map<EntityName, EntityAuth>& get_keys() { return keys; }  // yuck
 

diff --git a/src/krbd.cc b/src/krbd.cc
index f232fdf8c3ee3028391ff98564a1a0185fb16a15..45e91b0a77663f458e65e39fe0fa327b303e39a0 100644 (file)
--- a/src/krbd.cc
+++ b/src/krbd.cc
@@ -238,13 +238,14 @@ static int build_map_buf(CephContext *cct, const krbd_spec& spec,
   auto auth_client_required =
     cct->_conf.get_val<std::string>("auth_client_required");
   if (auth_client_required != "none") {
-    r = keyring.from_ceph_context(cct);
+    std::ostringstream err;
+    r = keyring.from_ceph_context(cct, &err);
     auto keyfile = cct->_conf.get_val<std::string>("keyfile");
     auto key = cct->_conf.get_val<std::string>("key");
     if (r == -ENOENT && keyfile.empty() && key.empty())
       r = 0;
     if (r < 0) {
-      std::cerr << "rbd: failed to get secret" << std::endl;
+      std::cerr << "rbd: failed to lookup key: " << err.view() << std::endl;
       return r;
     }
   }

diff --git a/src/mon/MonClient.cc b/src/mon/MonClient.cc
index c0de55abf421866e3ad1044d25719a18178beb39..dbe0953e1dd9977a76e61efc37d864298774ef15 100644 (file)
--- a/src/mon/MonClient.cc
+++ b/src/mon/MonClient.cc
@@ -271,7 +271,12 @@ int MonClient::ping_monitor(const string &mon_id, string *result_reply)
   auth_registry.refresh_config();
 
   KeyRing keyring;
-  keyring.from_ceph_context(cct);
+  CachedStackStringStream css;
+  int keyload = keyring.from_ceph_context(cct, css.get());
+  if (keyload < 0) {
+    ldout(cct, 10) << "could not load key: " << css->strv() << dendl;
+    return keyload;
+  }
   RotatingKeyRing rkeyring(cct, cct->get_module_type(), &keyring);
 
   MonClientPinger *pinger = new MonClientPinger(cct,
@@ -512,18 +517,24 @@ int MonClient::init()
 
   std::lock_guard l(monc_lock);
   keyring.reset(new KeyRing);
+  CachedStackStringStream css;
+  int keyload = keyring->from_ceph_context(cct, css.get());
   if (auth_registry.is_supported_method(messenger->get_mytype(),
                                        CEPH_AUTH_CEPHX)) {
     // this should succeed, because auth_registry just checked!
-    int r = keyring->from_ceph_context(cct);
-    if (r != 0) {
+    if (keyload < 0) {
       // but be somewhat graceful in case there was a race condition
-      lderr(cct) << "keyring not found" << dendl;
-      return r;
+      lderr(cct) << css->strv() << dendl;
+      return keyload;
     }
+  } else if (keyload < 0) {
+    ldout(cct, 5) << "Key loading failed with: " << css->strv() << dendl;
   }
   if (!auth_registry.any_supported_methods(messenger->get_mytype())) {
-    lderr(cct) << "no supported authentication method found! Is the keyring missing?" << dendl;
+    lderr(cct) << "No supported authentication method found! Is the keyring missing?" << dendl;
+    if (keyload < 0) {
+      lderr(cct) << "Key loading failed with: " << css->strv() << dendl;
+    }
     lderr(cct) << "Try debugging using arguments: --debug_monc=20 --debug_auth=5" << dendl;
     return -ENOENT;
   }

diff --git a/src/mount/conf.cc b/src/mount/conf.cc
index f48b84e07853cd6008f3aa9ffea146e89865c4b1..06cc8680f1182311479adcde07c838abed79a799 100644 (file)
--- a/src/mount/conf.cc
+++ b/src/mount/conf.cc
@@ -89,10 +89,13 @@ extern "C" void mount_ceph_get_config_info(const char *config_file,
     mount_ceph_debug("Could not discover monitor addresses\n");
 
 scrape_keyring:
-  err = keyring.from_ceph_context(cct.get());
-  if (err) {
-    mount_ceph_debug("keyring.from_ceph_context failed: %d\n", err);
-    return;
+  {
+    CachedStackStringStream css;
+    int keyload = keyring.from_ceph_context(cct.get(), css.get());
+    if (keyload < 0) {
+      mount_ceph_debug("loading key failed: %s\n", css->strv().data());
+      return;
+    }
   }
 
   if (!keyring.get_secret(conf->name, secret)) {