auth: msgr2 can return incorrect allowed_modes through AuthBadMethodFrame

Updating AuthServer interface to return correct modes by dividing function
get_supported_auth_methods() into two functions to get methods and modes separately.

Fixes: https://tracker.ceph.com/issues/72265
Signed-off-by: Miki Patel <miki.patel132@gmail.com>
(cherry picked from commit 5de7e746b79ca454010e1358d198beb625cb93e9)

diff --git a/src/auth/AuthServer.h b/src/auth/AuthServer.h
index a71f6a70a6cccf6769b126d70b52c20a2dcbb8b7..3d567eaca15ac7b558d6ab3c465e39bfc7a810f9 100644 (file)
--- a/src/auth/AuthServer.h
+++ b/src/auth/AuthServer.h
@@ -17,15 +17,22 @@ public:
   AuthServer(CephContext *cct) : auth_registry(cct) {}
   virtual ~AuthServer() {}
 
-  /// Get authentication methods and connection modes for the given peer type
+  /// Get authentication methods for the given peer type
   virtual void get_supported_auth_methods(
     int peer_type,
-    std::vector<uint32_t> *methods,
-    std::vector<uint32_t> *modes = nullptr) {
-    auth_registry.get_supported_methods(peer_type, methods, modes);
+    std::vector<uint32_t> *methods) {
+    auth_registry.get_supported_methods(peer_type, methods, nullptr);
   }
 
-  /// Get support connection modes for the given peer type and auth method
+  /// Get supported connection modes for the given peer type and auth method
+  virtual void get_supported_con_modes(
+    int peer_type,
+    uint32_t auth_method,
+    std::vector<uint32_t> *modes) {
+    auth_registry.get_supported_modes(peer_type, auth_method, modes);
+  }
+
+  /// Choose a connection mode for the given peer type and auth method
   virtual uint32_t pick_con_mode(
     int peer_type,
     uint32_t auth_method,

diff --git a/src/crimson/auth/AuthServer.h b/src/crimson/auth/AuthServer.h
index a808410d2d5b75357f4fcc7628f57ccec9d70df3..d7fc979769f374fe449a875d2104735598e5ed51 100644 (file)
--- a/src/crimson/auth/AuthServer.h
+++ b/src/crimson/auth/AuthServer.h
@@ -16,10 +16,15 @@ class AuthServer {
 public:
   virtual ~AuthServer() {}
 
-  // Get authentication methods and connection modes for the given peer type
-  virtual std::pair<std::vector<uint32_t>, std::vector<uint32_t>>
+  // Get authentication methods for the given peer type
+  virtual std::vector<uint32_t>
   get_supported_auth_methods(int peer_type) = 0;
-  // Get support connection modes for the given peer type and auth method
+  // Get supported connection modes for the given peer type and auth method 
+  virtual std::vector<uint32_t>
+  get_supported_con_modes(
+    int peer_type,
+    uint32_t auth_method) = 0;
+  // Choose a connection mode for the given peer type and auth method
   virtual uint32_t pick_con_mode(
     int peer_type,
     uint32_t auth_method,

diff --git a/src/crimson/auth/DummyAuth.h b/src/crimson/auth/DummyAuth.h
index 7a3dd7ec4d6a4b9801366b0e4d6e8c05ff3b4f11..9e57a729882ab1276206d6ef7a5cd06a4017f482 100644 (file)
--- a/src/crimson/auth/DummyAuth.h
+++ b/src/crimson/auth/DummyAuth.h
@@ -12,9 +12,15 @@ public:
   DummyAuthClientServer() {}
 
   // client
-  std::pair<std::vector<uint32_t>, std::vector<uint32_t>>
+  std::vector<uint32_t>
   get_supported_auth_methods(int peer_type) final {
-    return {{CEPH_AUTH_NONE}, {CEPH_AUTH_NONE}};
+    return {CEPH_AUTH_NONE};
+  }
+
+  std::vector<uint32_t>
+  get_supported_con_modes(int peer_type,
+                         uint32_t auth_method) final {
+    return {CEPH_CON_MODE_CRC};
   }
 
   uint32_t pick_con_mode(int peer_type,

diff --git a/src/crimson/mon/MonClient.cc b/src/crimson/mon/MonClient.cc
index 7be09915a94618fe3b3cc02daa38ebdec9edd480..eeca189c423cb3228e4a5700de6deb13e47380cd 100644 (file)
--- a/src/crimson/mon/MonClient.cc
+++ b/src/crimson/mon/MonClient.cc
@@ -561,13 +561,21 @@ void Client::ms_handle_reset(crimson::net::ConnectionRef conn, bool /* is_replac
   });
 }
 
-std::pair<std::vector<uint32_t>, std::vector<uint32_t>>
+std::vector<uint32_t>
 Client::get_supported_auth_methods(int peer_type)
 {
     std::vector<uint32_t> methods;
+    auth_registry.get_supported_methods(peer_type, &methods, nullptr);
+    return methods;
+}
+
+std::vector<uint32_t>
+Client::get_supported_con_modes(int peer_type,
+                               uint32_t auth_method)
+{
     std::vector<uint32_t> modes;
-    auth_registry.get_supported_methods(peer_type, &methods, &modes);
-    return {methods, modes};
+    auth_registry.get_supported_modes(peer_type, auth_method, &modes);
+    return modes;
 }
 
 uint32_t Client::pick_con_mode(int peer_type,

diff --git a/src/crimson/mon/MonClient.h b/src/crimson/mon/MonClient.h
index 1228ecd0bba2762754225147bf809a345ae67d4c..325f6ff3f1f249cb56de3b15c8cb74fc26e2fdd3 100644 (file)
--- a/src/crimson/mon/MonClient.h
+++ b/src/crimson/mon/MonClient.h
@@ -119,8 +119,9 @@ public:
   void print(std::ostream&) const;
 private:
   // AuthServer methods
-  std::pair<std::vector<uint32_t>, std::vector<uint32_t>>
-  get_supported_auth_methods(int peer_type) final;
+  std::vector<uint32_t> get_supported_auth_methods(int peer_type) final;
+  std::vector<uint32_t> get_supported_con_modes(int peer_type,
+                                               uint32_t auth_method) final;
   uint32_t pick_con_mode(int peer_type,
                         uint32_t auth_method,
                         const std::vector<uint32_t>& preferred_modes) final;

diff --git a/src/crimson/net/ProtocolV2.cc b/src/crimson/net/ProtocolV2.cc
index 55b669384ed3bfd588e97192b00030217c48e4df..7134bc33d7a8c89e495c6eb33930c9e64377e77d 100644 (file)
--- a/src/crimson/net/ProtocolV2.cc
+++ b/src/crimson/net/ProtocolV2.cc
@@ -1056,8 +1056,11 @@ seastar::future<> ProtocolV2::_auth_bad_method(int r)
 {
   // _auth_bad_method() logic
   ceph_assert(r < 0);
-  auto [allowed_methods, allowed_modes] =
+  auto allowed_methods =
       messenger.get_auth_server()->get_supported_auth_methods(conn.get_peer_type());
+  auto allowed_modes =
+      messenger.get_auth_server()->get_supported_con_modes(conn.get_peer_type(),
+                                                         auth_meta->auth_method);
   auto bad_method = AuthBadMethodFrame::Encode(
       auth_meta->auth_method, r, allowed_methods, allowed_modes);
   logger().warn("{} WRITE AuthBadMethodFrame: method={}, result={}, "

diff --git a/src/msg/async/ProtocolV2.cc b/src/msg/async/ProtocolV2.cc
index 7c4a4d0fe94137df39e12234ecfd986e1dbb3979..83c9557ca4fd915cc64e1ff33994176e2b572276 100644 (file)
--- a/src/msg/async/ProtocolV2.cc
+++ b/src/msg/async/ProtocolV2.cc
@@ -2225,7 +2225,9 @@ CtPtr ProtocolV2::_auth_bad_method(int r)
   std::vector<uint32_t> allowed_methods;
   std::vector<uint32_t> allowed_modes;
   messenger->auth_server->get_supported_auth_methods(
-    connection->get_peer_type(), &allowed_methods, &allowed_modes);
+    connection->get_peer_type(), &allowed_methods);
+  messenger->auth_server->get_supported_con_modes(
+    connection->get_peer_type(), auth_meta->auth_method, &allowed_modes);
   ldout(cct, 1) << __func__ << " auth_method " << auth_meta->auth_method
                << " r " << cpp_strerror(r)
                << ", allowed_methods " << allowed_methods