From 7c932c10a63c2db6f32eb1a76785fc0ff573b0e3 Mon Sep 17 00:00:00 2001 From: Yingxin Cheng Date: Thu, 13 Oct 2022 17:38:09 +0800 Subject: [PATCH] crimson/net: replace unnecessary ConnectionRef/AuthConnectionMetaRef Signed-off-by: Yingxin Cheng --- src/crimson/auth/AuthClient.h | 16 ++--- src/crimson/auth/AuthServer.h | 4 +- src/crimson/auth/DummyAuth.h | 20 +++--- src/crimson/mon/MonClient.cc | 112 +++++++++++++++++----------------- src/crimson/mon/MonClient.h | 24 ++++---- src/crimson/net/Fwd.h | 3 - src/crimson/net/Protocol.cc | 3 +- src/crimson/net/Protocol.h | 2 - src/crimson/net/ProtocolV2.cc | 17 ++++-- src/crimson/net/ProtocolV2.h | 4 ++ 10 files changed, 104 insertions(+), 101 deletions(-) diff --git a/src/crimson/auth/AuthClient.h b/src/crimson/auth/AuthClient.h index cd21b3838d7b1..2d970c88c3cd0 100644 --- a/src/crimson/auth/AuthClient.h +++ b/src/crimson/auth/AuthClient.h @@ -34,23 +34,23 @@ public: /// Build an authentication request to begin the handshake /// /// @throw auth::error if unable to build the request - virtual auth_request_t get_auth_request(crimson::net::ConnectionRef conn, - AuthConnectionMetaRef auth_meta) = 0; + virtual auth_request_t get_auth_request(crimson::net::Connection &conn, + AuthConnectionMeta &auth_meta) = 0; /// Handle server's request to continue the handshake /// /// @throw auth::error if unable to build the request virtual ceph::bufferlist handle_auth_reply_more( - crimson::net::ConnectionRef conn, - AuthConnectionMetaRef auth_meta, + crimson::net::Connection &conn, + AuthConnectionMeta &auth_meta, const ceph::bufferlist& bl) = 0; /// Handle server's indication that authentication succeeded /// /// @return 0 if authenticated, a negative number otherwise virtual int handle_auth_done( - crimson::net::ConnectionRef conn, - AuthConnectionMetaRef auth_meta, + crimson::net::Connection &conn, + AuthConnectionMeta &auth_meta, uint64_t global_id, uint32_t con_mode, const bufferlist& bl) = 0; @@ -60,8 +60,8 @@ public: /// @return 0 if will try next auth method, a negative number if we have no /// more options virtual int handle_auth_bad_method( - crimson::net::ConnectionRef conn, - AuthConnectionMetaRef auth_meta, + crimson::net::Connection &conn, + AuthConnectionMeta &auth_meta, uint32_t old_auth_method, int result, const std::vector& allowed_methods, diff --git a/src/crimson/auth/AuthServer.h b/src/crimson/auth/AuthServer.h index d75c8f58649a5..800e2cd6ceb71 100644 --- a/src/crimson/auth/AuthServer.h +++ b/src/crimson/auth/AuthServer.h @@ -30,8 +30,8 @@ public: int auth_method) = 0; // Handle an authentication request on an incoming connection virtual int handle_auth_request( - crimson::net::ConnectionRef conn, - AuthConnectionMetaRef auth_meta, + crimson::net::Connection &conn, + AuthConnectionMeta &auth_meta, bool more, //< true if this is not the first part of the handshake uint32_t auth_method, const bufferlist& bl, diff --git a/src/crimson/auth/DummyAuth.h b/src/crimson/auth/DummyAuth.h index 7c26140a22714..89bb6734f61e3 100644 --- a/src/crimson/auth/DummyAuth.h +++ b/src/crimson/auth/DummyAuth.h @@ -32,21 +32,21 @@ public: } AuthClient::auth_request_t get_auth_request( - crimson::net::ConnectionRef conn, - AuthConnectionMetaRef auth_meta) override { + crimson::net::Connection &conn, + AuthConnectionMeta &auth_meta) override { return {CEPH_AUTH_NONE, {CEPH_CON_MODE_CRC}, {}}; } ceph::bufferlist handle_auth_reply_more( - crimson::net::ConnectionRef conn, - AuthConnectionMetaRef auth_meta, + crimson::net::Connection &conn, + AuthConnectionMeta &auth_meta, const bufferlist& bl) override { ceph_abort(); } int handle_auth_done( - crimson::net::ConnectionRef conn, - AuthConnectionMetaRef auth_meta, + crimson::net::Connection &conn, + AuthConnectionMeta &auth_meta, uint64_t global_id, uint32_t con_mode, const bufferlist& bl) override { @@ -54,8 +54,8 @@ public: } int handle_auth_bad_method( - crimson::net::ConnectionRef conn, - AuthConnectionMetaRef auth_meta, + crimson::net::Connection &conn, + AuthConnectionMeta &auth_meta, uint32_t old_auth_method, int result, const std::vector& allowed_methods, @@ -65,8 +65,8 @@ public: // server int handle_auth_request( - crimson::net::ConnectionRef conn, - AuthConnectionMetaRef auth_meta, + crimson::net::Connection &conn, + AuthConnectionMeta &auth_meta, bool more, uint32_t auth_method, const bufferlist& bl, diff --git a/src/crimson/mon/MonClient.cc b/src/crimson/mon/MonClient.cc index 3d672761aad07..d89c0aded96d4 100644 --- a/src/crimson/mon/MonClient.cc +++ b/src/crimson/mon/MonClient.cc @@ -507,10 +507,10 @@ Client::ms_dispatch(crimson::net::ConnectionRef conn, MessageRef m) // we only care about these message types switch (m->get_type()) { case CEPH_MSG_MON_MAP: - return handle_monmap(conn, boost::static_pointer_cast(m)); + return handle_monmap(*conn, boost::static_pointer_cast(m)); case CEPH_MSG_AUTH_REPLY: return handle_auth_reply( - conn, boost::static_pointer_cast(m)); + *conn, boost::static_pointer_cast(m)); case CEPH_MSG_MON_SUBSCRIBE_ACK: return handle_subscribe_ack( boost::static_pointer_cast(m)); @@ -584,8 +584,8 @@ AuthAuthorizeHandler* Client::get_auth_authorize_handler(int peer_type, } -int Client::handle_auth_request(crimson::net::ConnectionRef con, - AuthConnectionMetaRef auth_meta, +int Client::handle_auth_request(crimson::net::Connection &conn, + AuthConnectionMeta &auth_meta, bool more, uint32_t auth_method, const ceph::bufferlist& payload, @@ -594,21 +594,21 @@ int Client::handle_auth_request(crimson::net::ConnectionRef con, if (payload.length() == 0) { return -EACCES; } - auth_meta->auth_mode = payload[0]; - if (auth_meta->auth_mode < AUTH_MODE_AUTHORIZER || - auth_meta->auth_mode > AUTH_MODE_AUTHORIZER_MAX) { + auth_meta.auth_mode = payload[0]; + if (auth_meta.auth_mode < AUTH_MODE_AUTHORIZER || + auth_meta.auth_mode > AUTH_MODE_AUTHORIZER_MAX) { return -EACCES; } - AuthAuthorizeHandler* ah = get_auth_authorize_handler(con->get_peer_type(), + AuthAuthorizeHandler* ah = get_auth_authorize_handler(conn.get_peer_type(), auth_method); if (!ah) { logger().error("no AuthAuthorizeHandler found for auth method: {}", auth_method); return -EOPNOTSUPP; } - auto authorizer_challenge = &auth_meta->authorizer_challenge; - if (auth_meta->skip_authorizer_challenge) { - logger().info("skipping challenge on {}", con); + auto authorizer_challenge = &auth_meta.authorizer_challenge; + if (auth_meta.skip_authorizer_challenge) { + logger().info("skipping challenge on {}", conn); authorizer_challenge = nullptr; } if (!active_con) { @@ -616,44 +616,44 @@ int Client::handle_auth_request(crimson::net::ConnectionRef con, // let's instruct the client to come back later return -EBUSY; } - bool was_challenge = (bool)auth_meta->authorizer_challenge; + bool was_challenge = (bool)auth_meta.authorizer_challenge; EntityName name; AuthCapsInfo caps_info; bool is_valid = ah->verify_authorizer( &cct, active_con->get_keys(), payload, - auth_meta->get_connection_secret_length(), + auth_meta.get_connection_secret_length(), reply, &name, - &con->peer_global_id, + &conn.peer_global_id, &caps_info, - &auth_meta->session_key, - &auth_meta->connection_secret, + &auth_meta.session_key, + &auth_meta.connection_secret, authorizer_challenge); if (is_valid) { auth_handler.handle_authentication(name, caps_info); return 1; } - if (!more && !was_challenge && auth_meta->authorizer_challenge) { - logger().info("added challenge on {}", con); + if (!more && !was_challenge && auth_meta.authorizer_challenge) { + logger().info("added challenge on {}", conn); return 0; } else { - logger().info("bad authorizer on {}", con); + logger().info("bad authorizer on {}", conn); return -EACCES; } } auth::AuthClient::auth_request_t -Client::get_auth_request(crimson::net::ConnectionRef con, - AuthConnectionMetaRef auth_meta) +Client::get_auth_request(crimson::net::Connection &conn, + AuthConnectionMeta &auth_meta) { - logger().info("get_auth_request(con={}, auth_method={})", - con, auth_meta->auth_method); + logger().info("get_auth_request(conn={}, auth_method={})", + conn, auth_meta.auth_method); // connection to mon? - if (con->get_peer_type() == CEPH_ENTITY_TYPE_MON) { + if (conn.get_peer_type() == CEPH_ENTITY_TYPE_MON) { auto found = std::find_if(pending_conns.begin(), pending_conns.end(), - [peer_addr = con->get_peer_addr()](auto& mc) { + [peer_addr = conn.get_peer_addr()](auto& mc) { return mc->is_my_peer(peer_addr); }); if (found == pending_conns.end()) { @@ -666,90 +666,90 @@ Client::get_auth_request(crimson::net::ConnectionRef con, logger().error(" but no auth handler is set up"); throw crimson::auth::error("no auth available"); } - auto authorizer = active_con->get_authorizer(con->get_peer_type()); + auto authorizer = active_con->get_authorizer(conn.get_peer_type()); if (!authorizer) { logger().error("failed to build_authorizer for type {}", - ceph_entity_type_name(con->get_peer_type())); + ceph_entity_type_name(conn.get_peer_type())); throw crimson::auth::error("unable to build auth"); } - auth_meta->authorizer.reset(authorizer); - auth_meta->auth_method = authorizer->protocol; + auth_meta.authorizer.reset(authorizer); + auth_meta.auth_method = authorizer->protocol; vector modes; - auth_registry.get_supported_modes(con->get_peer_type(), - auth_meta->auth_method, + auth_registry.get_supported_modes(conn.get_peer_type(), + auth_meta.auth_method, &modes); return {authorizer->protocol, modes, authorizer->bl}; } } -ceph::bufferlist Client::handle_auth_reply_more(crimson::net::ConnectionRef conn, - AuthConnectionMetaRef auth_meta, +ceph::bufferlist Client::handle_auth_reply_more(crimson::net::Connection &conn, + AuthConnectionMeta &auth_meta, const bufferlist& bl) { - if (conn->get_peer_type() == CEPH_ENTITY_TYPE_MON) { + if (conn.get_peer_type() == CEPH_ENTITY_TYPE_MON) { auto found = std::find_if(pending_conns.begin(), pending_conns.end(), - [peer_addr = conn->get_peer_addr()](auto& mc) { + [peer_addr = conn.get_peer_addr()](auto& mc) { return mc->is_my_peer(peer_addr); }); if (found == pending_conns.end()) { throw crimson::auth::error{"unknown connection"}; } bufferlist reply; - tie(auth_meta->session_key, auth_meta->connection_secret, reply) = + tie(auth_meta.session_key, auth_meta.connection_secret, reply) = (*found)->handle_auth_reply_more(bl); return reply; } else { // authorizer challenges - if (!active_con || !auth_meta->authorizer) { + if (!active_con || !auth_meta.authorizer) { logger().error("no authorizer?"); throw crimson::auth::error("no auth available"); } - auth_meta->authorizer->add_challenge(&cct, bl); - return auth_meta->authorizer->bl; + auth_meta.authorizer->add_challenge(&cct, bl); + return auth_meta.authorizer->bl; } } -int Client::handle_auth_done(crimson::net::ConnectionRef conn, - AuthConnectionMetaRef auth_meta, +int Client::handle_auth_done(crimson::net::Connection &conn, + AuthConnectionMeta &auth_meta, uint64_t global_id, uint32_t /*con_mode*/, const bufferlist& bl) { - if (conn->get_peer_type() == CEPH_ENTITY_TYPE_MON) { + if (conn.get_peer_type() == CEPH_ENTITY_TYPE_MON) { auto found = std::find_if(pending_conns.begin(), pending_conns.end(), - [peer_addr = conn->get_peer_addr()](auto& mc) { + [peer_addr = conn.get_peer_addr()](auto& mc) { return mc->is_my_peer(peer_addr); }); if (found == pending_conns.end()) { return -ENOENT; } int r = 0; - tie(auth_meta->session_key, auth_meta->connection_secret, r) = + tie(auth_meta.session_key, auth_meta.connection_secret, r) = (*found)->handle_auth_done(global_id, bl); return r; } else { // verify authorizer reply auto p = bl.begin(); - if (!auth_meta->authorizer->verify_reply(p, &auth_meta->connection_secret)) { + if (!auth_meta.authorizer->verify_reply(p, &auth_meta.connection_secret)) { logger().error("failed verifying authorizer reply"); return -EACCES; } - auth_meta->session_key = auth_meta->authorizer->session_key; + auth_meta.session_key = auth_meta.authorizer->session_key; return 0; } } // Handle server's indication that the previous auth attempt failed -int Client::handle_auth_bad_method(crimson::net::ConnectionRef conn, - AuthConnectionMetaRef auth_meta, +int Client::handle_auth_bad_method(crimson::net::Connection &conn, + AuthConnectionMeta &auth_meta, uint32_t old_auth_method, int result, const std::vector& allowed_methods, const std::vector& allowed_modes) { - if (conn->get_peer_type() == CEPH_ENTITY_TYPE_MON) { + if (conn.get_peer_type() == CEPH_ENTITY_TYPE_MON) { auto found = std::find_if(pending_conns.begin(), pending_conns.end(), - [peer_addr = conn->get_peer_addr()](auto& mc) { + [peer_addr = conn.get_peer_addr()](auto& mc) { return mc->is_my_peer(peer_addr); }); if (found != pending_conns.end()) { @@ -767,11 +767,11 @@ int Client::handle_auth_bad_method(crimson::net::ConnectionRef conn, } } -seastar::future<> Client::handle_monmap(crimson::net::ConnectionRef conn, +seastar::future<> Client::handle_monmap(crimson::net::Connection &conn, Ref m) { monmap.decode(m->monmapbl); - const auto peer_addr = conn->get_peer_addr(); + const auto peer_addr = conn.get_peer_addr(); auto cur_mon = monmap.get_name(peer_addr); logger().info("got monmap {}, mon.{}, is now rank {}", monmap.epoch, cur_mon, monmap.get_rank(cur_mon)); @@ -800,13 +800,13 @@ seastar::future<> Client::handle_monmap(crimson::net::ConnectionRef conn, } } -seastar::future<> Client::handle_auth_reply(crimson::net::ConnectionRef conn, +seastar::future<> Client::handle_auth_reply(crimson::net::Connection &conn, Ref m) { logger().info("handle_auth_reply {} returns {}: {}", - *conn, *m, m->result); + conn, *m, m->result); auto found = std::find_if(pending_conns.begin(), pending_conns.end(), - [peer_addr = conn->get_peer_addr()](auto& mc) { + [peer_addr = conn.get_peer_addr()](auto& mc) { return mc->is_my_peer(peer_addr); }); if (found != pending_conns.end()) { @@ -818,7 +818,7 @@ seastar::future<> Client::handle_auth_reply(crimson::net::ConnectionRef conn, active_con->renew_tickets()).discard_result(); }); } else { - logger().error("unknown auth reply from {}", conn->get_peer_addr()); + logger().error("unknown auth reply from {}", conn.get_peer_addr()); return seastar::now(); } } diff --git a/src/crimson/mon/MonClient.h b/src/crimson/mon/MonClient.h index ac50f821392dc..c27291355a383 100644 --- a/src/crimson/mon/MonClient.h +++ b/src/crimson/mon/MonClient.h @@ -126,8 +126,8 @@ private: const std::vector& preferred_modes) final; AuthAuthorizeHandler* get_auth_authorize_handler(int peer_type, int auth_method) final; - int handle_auth_request(crimson::net::ConnectionRef conn, - AuthConnectionMetaRef auth_meta, + int handle_auth_request(crimson::net::Connection &conn, + AuthConnectionMeta &auth_meta, bool more, uint32_t auth_method, const ceph::bufferlist& payload, @@ -139,24 +139,24 @@ private: // AuthClient methods crimson::auth::AuthClient::auth_request_t - get_auth_request(crimson::net::ConnectionRef conn, - AuthConnectionMetaRef auth_meta) final; + get_auth_request(crimson::net::Connection &conn, + AuthConnectionMeta &auth_meta) final; // Handle server's request to continue the handshake - ceph::bufferlist handle_auth_reply_more(crimson::net::ConnectionRef conn, - AuthConnectionMetaRef auth_meta, + ceph::bufferlist handle_auth_reply_more(crimson::net::Connection &conn, + AuthConnectionMeta &auth_meta, const bufferlist& bl) final; // Handle server's indication that authentication succeeded - int handle_auth_done(crimson::net::ConnectionRef conn, - AuthConnectionMetaRef auth_meta, + int handle_auth_done(crimson::net::Connection &conn, + AuthConnectionMeta &auth_meta, uint64_t global_id, uint32_t con_mode, const bufferlist& bl) final; // Handle server's indication that the previous auth attempt failed - int handle_auth_bad_method(crimson::net::ConnectionRef conn, - AuthConnectionMetaRef auth_meta, + int handle_auth_bad_method(crimson::net::Connection &conn, + AuthConnectionMeta &auth_meta, uint32_t old_auth_method, int result, const std::vector& allowed_methods, @@ -169,9 +169,9 @@ private: MessageRef m) override; void ms_handle_reset(crimson::net::ConnectionRef conn, bool is_replace) override; - seastar::future<> handle_monmap(crimson::net::ConnectionRef conn, + seastar::future<> handle_monmap(crimson::net::Connection &conn, Ref m); - seastar::future<> handle_auth_reply(crimson::net::ConnectionRef conn, + seastar::future<> handle_auth_reply(crimson::net::Connection &conn, Ref m); seastar::future<> handle_subscribe_ack(Ref m); seastar::future<> handle_get_version_reply(Ref m); diff --git a/src/crimson/net/Fwd.h b/src/crimson/net/Fwd.h index c4719a3a4cd53..7ccd3fe35dd03 100644 --- a/src/crimson/net/Fwd.h +++ b/src/crimson/net/Fwd.h @@ -26,10 +26,7 @@ #include "crimson/common/errorator.h" -using auth_proto_t = int; - class AuthConnectionMeta; -using AuthConnectionMetaRef = seastar::lw_shared_ptr; namespace crimson::net { diff --git a/src/crimson/net/Protocol.cc b/src/crimson/net/Protocol.cc index 30ee539d2567c..fe5189bb77595 100644 --- a/src/crimson/net/Protocol.cc +++ b/src/crimson/net/Protocol.cc @@ -25,8 +25,7 @@ Protocol::Protocol(proto_t type, SocketConnection& conn) : proto_type(type), dispatchers(dispatchers), - conn(conn), - auth_meta{seastar::make_lw_shared()} + conn(conn) {} Protocol::~Protocol() diff --git a/src/crimson/net/Protocol.h b/src/crimson/net/Protocol.h index 34eb8a60e7a34..e34d2d1d11bf8 100644 --- a/src/crimson/net/Protocol.h +++ b/src/crimson/net/Protocol.h @@ -89,8 +89,6 @@ class Protocol { ChainedDispatchers& dispatchers; SocketConnection &conn; - AuthConnectionMetaRef auth_meta; - private: bool closed = false; // become valid only after closed == true diff --git a/src/crimson/net/ProtocolV2.cc b/src/crimson/net/ProtocolV2.cc index fa47da50832c0..1f5f13cd1d9e5 100644 --- a/src/crimson/net/ProtocolV2.cc +++ b/src/crimson/net/ProtocolV2.cc @@ -153,6 +153,7 @@ ProtocolV2::ProtocolV2(ChainedDispatchers& dispatchers, SocketMessenger& messenger) : Protocol(proto_t::v2, dispatchers, conn), messenger{messenger}, + auth_meta{seastar::make_lw_shared()}, protocol_timer{conn} {} @@ -529,7 +530,7 @@ seastar::future<> ProtocolV2::handle_auth_reply() bad_method.allowed_methods(), bad_method.allowed_modes()); ceph_assert(messenger.get_auth_client()); int r = messenger.get_auth_client()->handle_auth_bad_method( - conn.shared_from_this(), auth_meta, + conn, *auth_meta, bad_method.method(), bad_method.result(), bad_method.allowed_methods(), bad_method.allowed_modes()); if (r < 0) { @@ -548,7 +549,7 @@ seastar::future<> ProtocolV2::handle_auth_reply() ceph_assert(messenger.get_auth_client()); // let execute_connecting() take care of the thrown exception auto reply = messenger.get_auth_client()->handle_auth_reply_more( - conn.shared_from_this(), auth_meta, auth_more.auth_payload()); + conn, *auth_meta, auth_more.auth_payload()); auto more_reply = AuthRequestMoreFrame::Encode(reply); logger().debug("{} WRITE AuthRequestMoreFrame: payload_len={}", conn, reply.length()); @@ -566,7 +567,8 @@ seastar::future<> ProtocolV2::handle_auth_reply() auth_done.auth_payload().length()); ceph_assert(messenger.get_auth_client()); int r = messenger.get_auth_client()->handle_auth_done( - conn.shared_from_this(), auth_meta, + conn, + *auth_meta, auth_done.global_id(), auth_done.con_mode(), auth_done.auth_payload()); @@ -594,7 +596,7 @@ seastar::future<> ProtocolV2::client_auth(std::vector &allowed_methods try { auto [auth_method, preferred_modes, bl] = - messenger.get_auth_client()->get_auth_request(conn.shared_from_this(), auth_meta); + messenger.get_auth_client()->get_auth_request(conn, *auth_meta); auth_meta->auth_method = auth_method; auto frame = AuthRequestFrame::Encode(auth_method, preferred_modes, bl); logger().debug("{} WRITE AuthRequestFrame: method={}," @@ -959,8 +961,11 @@ seastar::future<> ProtocolV2::_handle_auth_request(bufferlist& auth_payload, boo ceph_assert(messenger.get_auth_server()); bufferlist reply; int r = messenger.get_auth_server()->handle_auth_request( - conn.shared_from_this(), auth_meta, - more, auth_meta->auth_method, auth_payload, + conn, + *auth_meta, + more, + auth_meta->auth_method, + auth_payload, &reply); switch (r) { // successful diff --git a/src/crimson/net/ProtocolV2.h b/src/crimson/net/ProtocolV2.h index 2ad0e4941366f..df5e7bbb4bfa4 100644 --- a/src/crimson/net/ProtocolV2.h +++ b/src/crimson/net/ProtocolV2.h @@ -13,6 +13,8 @@ namespace crimson::net { class ProtocolV2 final : public Protocol { + using AuthConnectionMetaRef = seastar::lw_shared_ptr; + public: ProtocolV2(ChainedDispatchers& dispatchers, SocketConnection& conn, @@ -43,6 +45,8 @@ class ProtocolV2 final : public Protocol { private: SocketMessenger &messenger; + AuthConnectionMetaRef auth_meta; + enum class state_t { NONE = 0, ACCEPTING, -- 2.39.5