From: Miki Patel Date: Mon, 28 Jul 2025 10:31:54 +0000 (+0530) Subject: auth: msgr2 can return incorrect allowed_modes through AuthBadMethodFrame X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=5de7e746b79ca454010e1358d198beb625cb93e9;p=ceph.git 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 --- diff --git a/src/auth/AuthServer.h b/src/auth/AuthServer.h index a71f6a70a6c..3d567eaca15 100644 --- 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 *methods, - std::vector *modes = nullptr) { - auth_registry.get_supported_methods(peer_type, methods, modes); + std::vector *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 *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 a808410d2d5..d7fc979769f 100644 --- 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> + // Get authentication methods for the given peer type + virtual std::vector 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 + 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 7a3dd7ec4d6..9e57a729882 100644 --- a/src/crimson/auth/DummyAuth.h +++ b/src/crimson/auth/DummyAuth.h @@ -12,9 +12,15 @@ public: DummyAuthClientServer() {} // client - std::pair, std::vector> + std::vector get_supported_auth_methods(int peer_type) final { - return {{CEPH_AUTH_NONE}, {CEPH_AUTH_NONE}}; + return {CEPH_AUTH_NONE}; + } + + std::vector + 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 4c076cf43c6..3d6f7fd2f9f 100644 --- a/src/crimson/mon/MonClient.cc +++ b/src/crimson/mon/MonClient.cc @@ -564,13 +564,21 @@ void Client::ms_handle_reset(crimson::net::ConnectionRef conn, bool /* is_replac }); } -std::pair, std::vector> +std::vector Client::get_supported_auth_methods(int peer_type) { std::vector methods; + auth_registry.get_supported_methods(peer_type, &methods, nullptr); + return methods; +} + +std::vector +Client::get_supported_con_modes(int peer_type, + uint32_t auth_method) +{ std::vector 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 20c4d0ba1a1..e5c9eb43cf7 100644 --- 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> - get_supported_auth_methods(int peer_type) final; + std::vector get_supported_auth_methods(int peer_type) final; + std::vector 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& preferred_modes) final; diff --git a/src/crimson/net/ProtocolV2.cc b/src/crimson/net/ProtocolV2.cc index 6f4b2c6d00a..3deb5f039d8 100644 --- 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 58e4f4df21d..310646b69dc 100644 --- a/src/msg/async/ProtocolV2.cc +++ b/src/msg/async/ProtocolV2.cc @@ -2260,7 +2260,9 @@ CtPtr ProtocolV2::_auth_bad_method(int r) std::vector allowed_methods; std::vector 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