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 126a9a668e1682b64b97bbb7e789ce9a621559d3..7542befdc556c6b8db30880fc4a4a6d30b241963 100644 (file)
--- a/src/mds/MDSDaemon.cc
+++ b/src/mds/MDSDaemon.cc
@@ -1083,11 +1083,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 525f732fbe6db34207910c406c1f890cecae2153..15bd98a923d552885cfe0c9d9256cdd9d4665250 100644 (file)
--- a/src/mgr/DaemonServer.cc
+++ b/src/mgr/DaemonServer.cc
@@ -180,7 +180,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);
@@ -205,17 +205,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 fa74acb16192ce8bbbfc5a7d8dca22d33d02a664..21c0a82ef23566e9df3e55f3922fbe547223af0a 100644 (file)
--- a/src/mon/MonClient.cc
+++ b/src/mon/MonClient.cc
@@ -1601,8 +1601,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;
   }
@@ -1639,8 +1640,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 0f6edb6070191ce32132440c0fd8a70a6d0b20f0..1c0423f5e571a7f83f01f731fa6c5a1966316092 100644 (file)
--- a/src/mon/Monitor.cc
+++ b/src/mon/Monitor.cc
@@ -6361,7 +6361,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) {
@@ -6482,7 +6484,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";
@@ -6520,12 +6524,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();
@@ -6548,11 +6552,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;
@@ -6561,22 +6564,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 030e094784fbee6beb681f00ec5bcc3a22e2f6f1..e49525a6ca4272381a0934794817803ae28707d8 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 885f1843b31c417c35f766fbecbb390f4ef0510c..21c92f193c05882065c9a301948d0bb11c87b047 100644 (file)
--- a/src/msg/Dispatcher.h
+++ b/src/msg/Dispatcher.h
@@ -209,12 +209,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 8c937ea1d87f7529cbecf01d4f20f5581a1b7f0e..480df63f3d133e8cb7d5b51569c7a456f9efed79 100644 (file)
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -7594,9 +7594,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);
@@ -7614,6 +7613,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;
@@ -7623,23 +7623,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::do_waiters()

diff --git a/src/osd/OSD.h b/src/osd/OSD.h
index 52880bf4acea27a5142e46ee8826fb05bbcc983c..cceb1a47e82259fa7739d0bc17a6309c531c4e44 100644 (file)
--- a/src/osd/OSD.h
+++ b/src/osd/OSD.h
@@ -1530,7 +1530,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;
@@ -1989,7 +1989,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 84c55176bd7ee2021520d5c7655bce2cd6146f14..ab8b53a714ee293d25d9c6b59bf9cc4dab0ed0c2 100644 (file)
--- a/src/test/fio/fio_ceph_messenger.cc
+++ b/src/test/fio/fio_ceph_messenger.cc
@@ -270,8 +270,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 8766ac30e7b66b0cf982f2e06be3d5b0ef431de2..5ebe33a7255879338a5ad17e510bd9a13cb39f21 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) {
@@ -2267,8 +2267,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;
   }
 };