From: Patrick Donnelly Date: Tue, 6 Jan 2026 15:50:09 +0000 (-0500) Subject: auth,mon: print key loading failures once at init X-Git-Tag: testing/wip-pdonnell-testing-20260126.152838~2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=653f978e8c9340a1b73f9fda756a863fdcef57ec;p=ceph-ci.git 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 --- diff --git a/src/auth/AuthRegistry.cc b/src/auth/AuthRegistry.cc index 7c8a72b7b79..5200ff46989 100644 --- 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 37c1d67316a..c2b5222bed0 100644 --- 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 2e61245b83e..2ded794178f 100644 --- 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& get_keys() { return keys; } // yuck diff --git a/src/krbd.cc b/src/krbd.cc index f232fdf8c3e..45e91b0a776 100644 --- 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("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("keyfile"); auto key = cct->_conf.get_val("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 c0de55abf42..dbe0953e1dd 100644 --- 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 f48b84e0785..06cc8680f11 100644 --- 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)) {