mon/MonClient: handle ms_handle_fast_authentication return

Fixes: https://tracker.ceph.com/issues/62382
Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

diff --git a/src/mds/MDSDaemon.cc b/src/mds/MDSDaemon.cc
index 206daf9d5d8b33dcf341216e979c3209fb227cdb..f1a95280121a64aee58ad7413d87cd1b6066531c 100644 (file)
--- a/src/mds/MDSDaemon.cc
+++ b/src/mds/MDSDaemon.cc
@@ -1151,11 +1151,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 7f357abdfdd7caa533b58d480e94d8488439729b..c1999a32029c832ecf93232f3dfb0b6f922507c6 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 f914a3152f90ef988df232a511fe533084c63913..ca335a744d496e9517129774fb27f73ae8a0901c 100644 (file)
--- a/src/mgr/DaemonServer.cc
+++ b/src/mgr/DaemonServer.cc
@@ -257,7 +257,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);
@@ -282,17 +282,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 998cc7c8385a8916ec98853bd05bc9df9bf5ebc7..1193805def57277e52b73f0da5fe61c5cfc52de1 100644 (file)
--- a/src/mgr/DaemonServer.h
+++ b/src/mgr/DaemonServer.h
@@ -272,7 +272,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 0667c078b630d7d8952df4ad67119aa76394f876..acda4f6d3914e6a3de33059c064dfdeed928c823 100644 (file)
--- a/src/mon/MonClient.cc
+++ b/src/mon/MonClient.cc
@@ -1609,8 +1609,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;
   }
@@ -1647,8 +1648,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 a70bfbe33c9deb8e5fa0960310c6fe9589d178fb..4f632cc1ab7bf2a249eb88148b0d8ab0426f113e 100644 (file)
--- a/src/mon/Monitor.cc
+++ b/src/mon/Monitor.cc
@@ -6402,7 +6402,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) {
@@ -6523,7 +6525,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";
@@ -6561,12 +6565,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();
@@ -6576,7 +6580,7 @@ int Monitor::ms_handle_fast_authentication(Connection *con)
     if (state == STATE_SHUTDOWN) {
       dout(10) << __func__ << " ignoring new con " << con << " (shutdown)" << dendl;
       con->mark_down();
-      return -EACCES;
+      return false;
     }
     s = session_map.new_session(
       entity_name_t(con->get_peer_type(), -1),  // we don't know yet
@@ -6594,11 +6598,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;
@@ -6607,22 +6610,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 13afacafde7dd6b01ce4450499eb3a121d48eaf3..2958388e83a88ec2ccf20f7dd88b675a712930e6 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 799a4a9de91e8e8636b634e107d7b9aecae3ff8f..8b7a65c795a7910099b0bc7a10f04d9eb5ee1b5e 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 2b59c7c87e4dd7d85c16a5719e9df8b178af2879..b91b147c18977e7cb16eac64b25402f567bd5039 100644 (file)
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -7645,9 +7645,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);
@@ -7665,6 +7664,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;
@@ -7674,23 +7674,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 0ccfdb05d8e6667bea55b431286b68d8a26e7db3..51183bfe3881501e09a600fddf7facb31e6cbbb8 100644 (file)
--- a/src/osd/OSD.h
+++ b/src/osd/OSD.h
@@ -1507,7 +1507,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;
@@ -1945,7 +1945,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;
   }
 };