mon/MonClient: handle ms_handle_fast_authentication return

Fixes: https://tracker.ceph.com/issues/62382
Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
(cherry picked from commit bfbfbbfed6c02a913d0fcfd6a97071acafb95378)

Conflicts:
	src/mon/Monitor.cc: missing state shutdown check

diff --git a/src/mds/MDSDaemon.cc b/src/mds/MDSDaemon.cc
index 483f51db724aafe48f56b506fd9e1b92b8e47f33..33588b32e3bea5fa6179afb0aa9dd8b802f54271 100644 (file)
--- a/src/mds/MDSDaemon.cc
+++ b/src/mds/MDSDaemon.cc
@@ -1091,11 +1091,11 @@ bool MDSDaemon::parse_caps(const AuthCapsInfo& info, MDSAuthCaps& caps)
   }
 }
 
-int MDSDaemon::ms_handle_fast_authentication(Connection *con)
+bool MDSDaemon::ms_handle_fast_authentication(Connection *con)
 {
   /* N.B. without mds_lock! */
   MDSAuthCaps caps;
-  return parse_caps(con->get_peer_caps_info(), caps) ? 0 : -1;
+  return parse_caps(con->get_peer_caps_info(), caps);
 }
 
 void MDSDaemon::ms_handle_accept(Connection *con)

diff --git a/src/mds/MDSDaemon.h b/src/mds/MDSDaemon.h
index e7929d2c37deccd23ffa3fa271ae85cd62082937..d97b0ebf7e0a48e6b5eb571e98f5243215cb0871 100644 (file)
--- a/src/mds/MDSDaemon.h
+++ b/src/mds/MDSDaemon.h
@@ -146,7 +146,7 @@ class MDSDaemon : public Dispatcher {
 
  private:
   bool ms_dispatch2(const ref_t<Message> &m) override;
-  int ms_handle_fast_authentication(Connection *con) override;
+  bool ms_handle_fast_authentication(Connection *con) override;
   void ms_handle_accept(Connection *con) override;
   void ms_handle_connect(Connection *con) override;
   bool ms_handle_reset(Connection *con) override;

diff --git a/src/mgr/DaemonServer.cc b/src/mgr/DaemonServer.cc
index 5b2457278f9426271bea1888b4d7296be0526a16..5656fdafd15edfb3eeeac33474058ec0d8cb7d1c 100644 (file)
--- a/src/mgr/DaemonServer.cc
+++ b/src/mgr/DaemonServer.cc
@@ -179,7 +179,7 @@ entity_addrvec_t DaemonServer::get_myaddrs() const
   return msgr->get_myaddrs();
 }
 
-int DaemonServer::ms_handle_fast_authentication(Connection *con)
+bool DaemonServer::ms_handle_fast_authentication(Connection *con)
 {
   auto s = ceph::make_ref<MgrSession>(cct);
   con->set_priv(s);
@@ -204,17 +204,17 @@ int DaemonServer::ms_handle_fast_authentication(Connection *con)
     catch (buffer::error& e) {
       dout(10) << " session " << s << " " << s->entity_name
                << " failed to decode caps" << dendl;
-      return -EACCES;
+      return false;
     }
     if (!s->caps.parse(str)) {
       dout(10) << " session " << s << " " << s->entity_name
               << " failed to parse caps '" << str << "'" << dendl;
-      return -EACCES;
+      return false;
     }
     dout(10) << " session " << s << " " << s->entity_name
              << " has caps " << s->caps << " '" << str << "'" << dendl;
   }
-  return 1;
+  return true;
 }
 
 void DaemonServer::ms_handle_accept(Connection* con)

diff --git a/src/mgr/DaemonServer.h b/src/mgr/DaemonServer.h
index a7b6456100439ec3f8d20058a9c34ee59fc6589d..615057d7af1db26ed0b8805f3777df92a08cdb05 100644 (file)
--- a/src/mgr/DaemonServer.h
+++ b/src/mgr/DaemonServer.h
@@ -269,7 +269,7 @@ public:
   ~DaemonServer() override;
 
   bool ms_dispatch2(const ceph::ref_t<Message>& m) override;
-  int ms_handle_fast_authentication(Connection *con) override;
+  bool ms_handle_fast_authentication(Connection *con) override;
   void ms_handle_accept(Connection *con) override;
   bool ms_handle_reset(Connection *con) override;
   void ms_handle_remote_reset(Connection *con) override {}

diff --git a/src/mon/MonClient.cc b/src/mon/MonClient.cc
index 7584fe2c43997d9478d81652490b10e958a91299..41793c72a1c510e2d031996b47772ec6dbbfcba5 100644 (file)
--- a/src/mon/MonClient.cc
+++ b/src/mon/MonClient.cc
@@ -1603,8 +1603,9 @@ int MonClient::handle_auth_request(
     // for some channels prior to nautilus (osd heartbeat), we
     // tolerate the lack of an authorizer.
     if (!con->get_messenger()->require_authorizer) {
-      handle_authentication_dispatcher->ms_handle_fast_authentication(con);
-      return 1;
+      if (handle_authentication_dispatcher->ms_handle_fast_authentication(con)) {
+        return 1;
+      }
     }
     return -EACCES;
   }
@@ -1641,8 +1642,10 @@ int MonClient::handle_auth_request(
     &auth_meta->connection_secret,
     ac);
   if (isvalid) {
-    handle_authentication_dispatcher->ms_handle_fast_authentication(con);
-    return 1;
+    if (handle_authentication_dispatcher->ms_handle_fast_authentication(con)) {
+      return 1;
+    }
+    return -EACCES;
   }
   if (!more && !was_challenge && auth_meta->authorizer_challenge) {
     ldout(cct,10) << __func__ << " added challenge on " << con << dendl;

diff --git a/src/mon/Monitor.cc b/src/mon/Monitor.cc
index 002a02fe00edbe347102415b285721ed30ad7a0e..503b572968830fec0a23b93f311b965b22fc5d44 100644 (file)
--- a/src/mon/Monitor.cc
+++ b/src/mon/Monitor.cc
@@ -6381,7 +6381,9 @@ int Monitor::handle_auth_request(
       &auth_meta->connection_secret,
       &auth_meta->authorizer_challenge);
     if (isvalid) {
-      ms_handle_fast_authentication(con);
+      if (!ms_handle_fast_authentication(con)) {
+        return -EACCES;
+      }
       return 1;
     }
     if (!more && !was_challenge && auth_meta->authorizer_challenge) {
@@ -6502,7 +6504,9 @@ int Monitor::handle_auth_request(
   }
   if (r > 0 &&
       !s->authenticated) {
-    ms_handle_fast_authentication(con);
+    if (!ms_handle_fast_authentication(con)) {
+      return -EACCES;
+    }
   }
 
   dout(30) << " r " << r << " reply:\n";
@@ -6540,12 +6544,12 @@ void Monitor::ms_handle_accept(Connection *con)
   }
 }
 
-int Monitor::ms_handle_fast_authentication(Connection *con)
+bool Monitor::ms_handle_fast_authentication(Connection *con)
 {
   if (con->get_peer_type() == CEPH_ENTITY_TYPE_MON) {
     // mon <-> mon connections need no Session, and setting one up
     // creates an awkward ref cycle between Session and Connection.
-    return 1;
+    return true;
   }
 
   auto priv = con->get_priv();
@@ -6568,11 +6572,10 @@ int Monitor::ms_handle_fast_authentication(Connection *con)
           << " " << *s << dendl;
 
   AuthCapsInfo &caps_info = con->get_peer_caps_info();
-  int ret = 0;
   if (caps_info.allow_all) {
     s->caps.set_allow_all();
     s->authenticated = true;
-    ret = 1;
+    return true;
   } else if (caps_info.caps.length()) {
     bufferlist::const_iterator p = caps_info.caps.cbegin();
     string str;
@@ -6581,22 +6584,19 @@ int Monitor::ms_handle_fast_authentication(Connection *con)
     } catch (const ceph::buffer::error &err) {
       derr << __func__ << " corrupt cap data for " << con->get_peer_entity_name()
           << " in auth db" << dendl;
-      str.clear();
-      ret = -EACCES;
+      return false;
     }
-    if (ret >= 0) {
-      if (s->caps.parse(str, NULL)) {
-       s->authenticated = true;
-       ret = 1;
-      } else {
-       derr << __func__ << " unparseable caps '" << str << "' for "
-            << con->get_peer_entity_name() << dendl;
-       ret = -EACCES;
-      }
+    if (s->caps.parse(str, NULL)) {
+      s->authenticated = true;
+      return true;
+    } else {
+      derr << __func__ << " unparseable caps '" << str << "' for "
+           << con->get_peer_entity_name() << dendl;
+      return false;
     }
+  } else {
+    return false;
   }
-
-  return ret;
 }
 
 void Monitor::set_mon_crush_location(const string& loc)

diff --git a/src/mon/Monitor.h b/src/mon/Monitor.h
index 7f9a16a9a36c8709fd339ac1b1d24929c49f0e33..79256ef4e2250e97417e3424f39e20158fa1ce0d 100644 (file)
--- a/src/mon/Monitor.h
+++ b/src/mon/Monitor.h
@@ -957,7 +957,7 @@ public:
   MonCap mon_caps;
   bool get_authorizer(int dest_type, AuthAuthorizer **authorizer);
 public: // for AuthMonitor msgr1:
-  int ms_handle_fast_authentication(Connection *con) override;
+  bool ms_handle_fast_authentication(Connection *con) override;
 private:
   void ms_handle_accept(Connection *con) override;
   bool ms_handle_reset(Connection *con) override;

diff --git a/src/msg/Dispatcher.h b/src/msg/Dispatcher.h
index b04ba36148572ab52553c45d02ce110f63ccf6e2..8b3c80542ac6c2a29581df6d2eade25289d2c050 100644 (file)
--- a/src/msg/Dispatcher.h
+++ b/src/msg/Dispatcher.h
@@ -215,12 +215,14 @@ public:
    *
    * Do not acquire locks in this method! It is considered "fast" delivery.
    *
-   * return 1 for success
-   * return 0 for no action (let another Dispatcher handle it)
-   * return <0 for failure (failure to parse caps, for instance)
+   * Note: MonClient is the only caller of this method and it is configured
+   *       to only call a single dispatcher.
+   *
+   * return true for success (auth succeeds for this stage of session construction)
+   * return false for failure (failure to parse caps, for instance)
    */
-  virtual int ms_handle_fast_authentication(Connection *con) {
-    return 0;
+  [[nodiscard]] virtual bool ms_handle_fast_authentication(Connection *con) {
+    return false;
   }
 
   /**

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 69e9c364496bf13482b1c584e967bdc10885f46c..4b8102065632ab5510ac299096ea9d6d419942e0 100644 (file)
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -7586,9 +7586,8 @@ void OSD::ms_fast_dispatch(Message *m)
   OID_EVENT_TRACE_WITH_MSG(m, "MS_FAST_DISPATCH_END", false);
 }
 
-int OSD::ms_handle_fast_authentication(Connection *con)
+bool OSD::ms_handle_fast_authentication(Connection *con)
 {
-  int ret = 0;
   auto s = ceph::ref_cast<Session>(con->get_priv());
   if (!s) {
     s = ceph::make_ref<Session>(cct, con);
@@ -7606,6 +7605,7 @@ int OSD::ms_handle_fast_authentication(Connection *con)
   AuthCapsInfo &caps_info = con->get_peer_caps_info();
   if (caps_info.allow_all) {
     s->caps.set_allow_all();
+    return true;
   } else if (caps_info.caps.length() > 0) {
     bufferlist::const_iterator p = caps_info.caps.cbegin();
     string str;
@@ -7615,23 +7615,22 @@ int OSD::ms_handle_fast_authentication(Connection *con)
     catch (ceph::buffer::error& e) {
       dout(10) << __func__ << " session " << s << " " << s->entity_name
               << " failed to decode caps string" << dendl;
-      ret = -EACCES;
-    }
-    if (!ret) {
-      bool success = s->caps.parse(str);
-      if (success) {
-       dout(10) << __func__ << " session " << s
-                << " " << s->entity_name
-                << " has caps " << s->caps << " '" << str << "'" << dendl;
-       ret = 1;
-      } else {
-       dout(10) << __func__ << " session " << s << " " << s->entity_name
-                << " failed to parse caps '" << str << "'" << dendl;
-       ret = -EACCES;
-      }
+      return false;
     }
+    bool success = s->caps.parse(str);
+    if (success) {
+      dout(10) << __func__ << " session " << s
+               << " " << s->entity_name
+               << " has caps " << s->caps << " '" << str << "'" << dendl;
+      return true;
+    } else {
+      dout(10) << __func__ << " session " << s << " " << s->entity_name
+               << " failed to parse caps '" << str << "'" << dendl;
+      return false;
+    }
+  } else {
+    return false;
   }
-  return ret;
 }
 
 void OSD::_dispatch(Message *m)

diff --git a/src/osd/OSD.h b/src/osd/OSD.h
index d25879001528926b3d28c138deffe07d43a49aef..7a5a589eda056925c49fc672f5ad281b892760d9 100644 (file)
--- a/src/osd/OSD.h
+++ b/src/osd/OSD.h
@@ -1528,7 +1528,7 @@ public:
     bool ms_handle_refused(Connection *con) override {
       return osd->ms_handle_refused(con);
     }
-    int ms_handle_fast_authentication(Connection *con) override {
+    bool ms_handle_fast_authentication(Connection *con) override {
       return true;
     }
   } heartbeat_dispatcher;
@@ -1951,7 +1951,7 @@ private:
   void ms_handle_connect(Connection *con) override;
   void ms_handle_fast_connect(Connection *con) override;
   void ms_handle_fast_accept(Connection *con) override;
-  int ms_handle_fast_authentication(Connection *con) override;
+  bool ms_handle_fast_authentication(Connection *con) override;
   bool ms_handle_reset(Connection *con) override;
   void ms_handle_remote_reset(Connection *con) override {}
   bool ms_handle_refused(Connection *con) override;

diff --git a/src/test/fio/fio_ceph_messenger.cc b/src/test/fio/fio_ceph_messenger.cc
index 81680f102dd13cc10e4c701c21086757bec1f0c4..604a92339b63e06760b707be4b20bcaef8494188 100644 (file)
--- a/src/test/fio/fio_ceph_messenger.cc
+++ b/src/test/fio/fio_ceph_messenger.cc
@@ -271,8 +271,8 @@ public:
   bool ms_handle_refused(Connection *con) override {
     return false;
   }
-  int ms_handle_fast_authentication(Connection *con) override {
-    return 1;
+  bool ms_handle_fast_authentication(Connection *con) override {
+    return true;
   }
 };
 

diff --git a/src/test/msgr/perf_msgr_client.cc b/src/test/msgr/perf_msgr_client.cc
index ffbfc1614fe737e0f5a3034664cfa07880538334..003a9ade2f331490419144e6c2511261a9a21b87 100644 (file)
--- a/src/test/msgr/perf_msgr_client.cc
+++ b/src/test/msgr/perf_msgr_client.cc
@@ -57,8 +57,8 @@ class MessengerClient {
     bool ms_handle_reset(Connection *con) override { return true; }
     void ms_handle_remote_reset(Connection *con) override {}
     bool ms_handle_refused(Connection *con) override { return false; }
-    int ms_handle_fast_authentication(Connection *con) override {
-      return 1;
+    bool ms_handle_fast_authentication(Connection *con) override {
+      return true;
     }
   };
 

diff --git a/src/test/msgr/perf_msgr_server.cc b/src/test/msgr/perf_msgr_server.cc
index 0c492ab174b7bfe2a0c707050090476a0d893fc5..1d2267e1ab8cacde503b2473b0aa84779e1cf126 100644 (file)
--- a/src/test/msgr/perf_msgr_server.cc
+++ b/src/test/msgr/perf_msgr_server.cc
@@ -100,8 +100,8 @@ class ServerDispatcher : public Dispatcher {
     //cerr << __func__ << " reply message=" << m << std::endl;
     op_wq.queue(m);
   }
-  int ms_handle_fast_authentication(Connection *con) override {
-    return 1;
+  bool ms_handle_fast_authentication(Connection *con) override {
+    return true;
   }
 };
 

diff --git a/src/test/msgr/test_msgr.cc b/src/test/msgr/test_msgr.cc
index f702cc288caedd5ab5e1c3750bca67d5b06c80f4..07f812589d117b6488dfc51a646b927d285368ab 100644 (file)
--- a/src/test/msgr/test_msgr.cc
+++ b/src/test/msgr/test_msgr.cc
@@ -220,8 +220,8 @@ class FakeDispatcher : public Dispatcher {
     cond.notify_all();
   }
 
-  int ms_handle_fast_authentication(Connection *con) override {
-    return 1;
+  bool ms_handle_fast_authentication(Connection *con) override {
+    return true;
   }
 
   void reply_message(Message *m) {
@@ -1709,8 +1709,8 @@ class SyntheticDispatcher : public Dispatcher {
     }
   }
 
-  int ms_handle_fast_authentication(Connection *con) override {
-    return 1;
+  bool ms_handle_fast_authentication(Connection *con) override {
+    return true;
   }
 
   void reply_message(const Message *m, Payload& pl) {
@@ -2322,8 +2322,8 @@ class MarkdownDispatcher : public Dispatcher {
   void ms_fast_dispatch(Message *m) override {
     ceph_abort();
   }
-  int ms_handle_fast_authentication(Connection *con) override {
-    return 1;
+  bool ms_handle_fast_authentication(Connection *con) override {
+    return true;
   }
 };