From 160b54da8008c1fb3cca15f99c38d3885a4cd82f Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Tue, 29 Jan 2019 11:46:48 -0600 Subject: [PATCH] auth/Auth{Client,Server}: pass auth_meta in explicitly This removes the wonky accessor on Connection, and most importantly allows the caller to control the lifecycle of the AuthConnectionMeta. Signed-off-by: Sage Weil --- src/auth/AuthClient.h | 4 ++++ src/auth/AuthServer.h | 1 + src/mon/MonClient.cc | 21 +++++++++++---------- src/mon/MonClient.h | 7 +++++++ src/mon/Monitor.cc | 9 +++++---- src/mon/Monitor.h | 7 ++++++- src/msg/Connection.h | 4 ---- src/msg/async/AsyncConnection.cc | 5 ----- src/msg/async/AsyncConnection.h | 2 -- src/msg/async/Protocol.h | 3 --- src/msg/async/ProtocolV2.cc | 13 +++++++++---- 11 files changed, 43 insertions(+), 33 deletions(-) diff --git a/src/auth/AuthClient.h b/src/auth/AuthClient.h index 79912a3188b..8977772cf5c 100644 --- a/src/auth/AuthClient.h +++ b/src/auth/AuthClient.h @@ -14,15 +14,18 @@ public: virtual int get_auth_request( Connection *con, + AuthConnectionMeta *auth_meta, uint32_t *method, std::vector *preferred_modes, bufferlist *out) = 0; virtual int handle_auth_reply_more( Connection *con, + AuthConnectionMeta *auth_meta, const bufferlist& bl, bufferlist *reply) = 0; virtual int handle_auth_done( Connection *con, + AuthConnectionMeta *auth_meta, uint64_t global_id, uint32_t con_mode, const bufferlist& bl, @@ -30,6 +33,7 @@ public: std::string *connection_secret) = 0; virtual int handle_auth_bad_method( Connection *con, + AuthConnectionMeta *auth_meta, uint32_t old_auth_method, int result, const std::vector& allowed_methods, diff --git a/src/auth/AuthServer.h b/src/auth/AuthServer.h index a762dfb2110..fb54b0a5943 100644 --- a/src/auth/AuthServer.h +++ b/src/auth/AuthServer.h @@ -38,6 +38,7 @@ public: virtual int handle_auth_request( Connection *con, + AuthConnectionMeta *auth_meta, bool more, uint32_t auth_method, const bufferlist& bl, diff --git a/src/mon/MonClient.cc b/src/mon/MonClient.cc index 8eab711c95b..a51209124de 100644 --- a/src/mon/MonClient.cc +++ b/src/mon/MonClient.cc @@ -1212,6 +1212,7 @@ void MonClient::handle_get_version_reply(MMonGetVersionReply* m) int MonClient::get_auth_request( Connection *con, + AuthConnectionMeta *auth_meta, uint32_t *auth_method, std::vector *preferred_modes, bufferlist *bl) @@ -1222,7 +1223,7 @@ int MonClient::get_auth_request( // connection to mon? if (con->get_peer_type() == CEPH_ENTITY_TYPE_MON) { - ceph_assert(!con->get_auth_meta()->authorizer); + ceph_assert(!auth_meta->authorizer); for (auto& i : pending_cons) { if (i.second.is_con(con)) { return i.second.get_auth_request( @@ -1234,7 +1235,6 @@ int MonClient::get_auth_request( } // generate authorizer - auto auth_meta = con->get_auth_meta(); if (!auth) { lderr(cct) << __func__ << " but no auth handler is set up" << dendl; return -EACCES; @@ -1255,6 +1255,7 @@ int MonClient::get_auth_request( int MonClient::handle_auth_reply_more( Connection *con, + AuthConnectionMeta *auth_meta, const bufferlist& bl, bufferlist *reply) { @@ -1263,14 +1264,13 @@ int MonClient::handle_auth_reply_more( if (con->get_peer_type() == CEPH_ENTITY_TYPE_MON) { for (auto& i : pending_cons) { if (i.second.is_con(con)) { - return i.second.handle_auth_reply_more(bl, reply); + return i.second.handle_auth_reply_more(auth_meta, bl, reply); } } return -ENOENT; } // authorizer challenges - auto auth_meta = con->get_auth_meta(); if (!auth || !auth_meta->authorizer) { lderr(cct) << __func__ << " no authorizer?" << dendl; return -1; @@ -1282,6 +1282,7 @@ int MonClient::handle_auth_reply_more( int MonClient::handle_auth_done( Connection *con, + AuthConnectionMeta *auth_meta, uint64_t global_id, uint32_t con_mode, const bufferlist& bl, @@ -1293,7 +1294,7 @@ int MonClient::handle_auth_done( for (auto& i : pending_cons) { if (i.second.is_con(con)) { int r = i.second.handle_auth_done( - global_id, bl, + auth_meta, global_id, bl, session_key, connection_secret); if (r) { pending_cons.erase(i.first); @@ -1313,7 +1314,6 @@ int MonClient::handle_auth_done( return -ENOENT; } else { // verify authorizer reply - auto auth_meta = con->get_auth_meta(); auto p = bl.begin(); if (!auth_meta->authorizer->verify_reply(p, &auth_meta->connection_secret)) { ldout(cct, 0) << __func__ << " failed verifying authorizer reply" @@ -1327,12 +1327,13 @@ int MonClient::handle_auth_done( int MonClient::handle_auth_bad_method( Connection *con, + AuthConnectionMeta *auth_meta, uint32_t old_auth_method, int result, const std::vector& allowed_methods, const std::vector& allowed_modes) { - con->get_auth_meta()->allowed_methods = allowed_methods; + auth_meta->allowed_methods = allowed_methods; std::lock_guard l(monc_lock); if (con->get_peer_type() == CEPH_ENTITY_TYPE_MON) { @@ -1367,12 +1368,12 @@ int MonClient::handle_auth_bad_method( int MonClient::handle_auth_request( Connection *con, + AuthConnectionMeta *auth_meta, bool more, uint32_t auth_method, const bufferlist& payload, bufferlist *reply) { - auto auth_meta = con->get_auth_meta(); auth_meta->auth_mode = payload[0]; if (auth_meta->auth_mode != AUTH_MODE_AUTHORIZER) { return -EACCES; @@ -1512,6 +1513,7 @@ int MonConnection::get_auth_request( } int MonConnection::handle_auth_reply_more( + AuthConnectionMeta *auth_meta, const bufferlist& bl, bufferlist *reply) { @@ -1522,7 +1524,6 @@ int MonConnection::handle_auth_reply_more( auto p = bl.cbegin(); ldout(cct, 10) << __func__ << " payload_len " << bl.length() << dendl; - auto auth_meta = con->get_auth_meta(); int r = auth->handle_response(0, p, &auth_meta->session_key, &auth_meta->connection_secret); if (r == -EAGAIN) { @@ -1542,6 +1543,7 @@ int MonConnection::handle_auth_reply_more( } int MonConnection::handle_auth_done( + AuthConnectionMeta *auth_meta, uint64_t new_global_id, const bufferlist& bl, CryptoKey *session_key, @@ -1553,7 +1555,6 @@ int MonConnection::handle_auth_done( global_id = new_global_id; auth->set_global_id(global_id); auto p = bl.begin(); - auto auth_meta = con->get_auth_meta(); int auth_err = auth->handle_response(0, p, &auth_meta->session_key, &auth_meta->connection_secret); if (auth_err >= 0) { diff --git a/src/mon/MonClient.h b/src/mon/MonClient.h index d79234d2d43..19b921defc6 100644 --- a/src/mon/MonClient.h +++ b/src/mon/MonClient.h @@ -135,9 +135,11 @@ public: uint32_t want_keys, RotatingKeyRing* keyring); int handle_auth_reply_more( + AuthConnectionMeta *auth_meta, const bufferlist& bl, bufferlist *reply); int handle_auth_done( + AuthConnectionMeta *auth_meta, uint64_t global_id, const bufferlist& bl, CryptoKey *session_key, @@ -271,15 +273,18 @@ public: // AuthClient int get_auth_request( Connection *con, + AuthConnectionMeta *auth_meta, uint32_t *method, std::vector *preferred_modes, bufferlist *bl) override; int handle_auth_reply_more( Connection *con, + AuthConnectionMeta *auth_meta, const bufferlist& bl, bufferlist *reply) override; int handle_auth_done( Connection *con, + AuthConnectionMeta *auth_meta, uint64_t global_id, uint32_t con_mode, const bufferlist& bl, @@ -287,6 +292,7 @@ public: std::string *connection_secret) override; int handle_auth_bad_method( Connection *con, + AuthConnectionMeta *auth_meta, uint32_t old_auth_method, int result, const std::vector& allowed_methods, @@ -294,6 +300,7 @@ public: // AuthServer int handle_auth_request( Connection *con, + AuthConnectionMeta *auth_meta, bool more, uint32_t auth_method, const bufferlist& bl, diff --git a/src/mon/Monitor.cc b/src/mon/Monitor.cc index 1b456434a75..cac25ec1617 100644 --- a/src/mon/Monitor.cc +++ b/src/mon/Monitor.cc @@ -5867,6 +5867,7 @@ void Monitor::extract_save_mon_key(KeyRing& keyring) // AuthClient methods -- for mon <-> mon communication int Monitor::get_auth_request( Connection *con, + AuthConnectionMeta *auth_meta, uint32_t *method, vector *preferred_modes, bufferlist *out) @@ -5878,7 +5879,6 @@ int Monitor::get_auth_request( if (con->get_peer_type() != CEPH_ENTITY_TYPE_MON) { return -EACCES; } - auto auth_meta = con->get_auth_meta(); auth_meta->authorizer.reset(auth); *method = auth->protocol; auth_registry.get_supported_modes(CEPH_ENTITY_TYPE_MON, auth->protocol, @@ -5889,10 +5889,10 @@ int Monitor::get_auth_request( int Monitor::handle_auth_reply_more( Connection *con, + AuthConnectionMeta *auth_meta, const bufferlist& bl, bufferlist *reply) { - auto auth_meta = con->get_auth_meta(); if (!auth_meta->authorizer) { derr << __func__ << " no authorizer?" << dendl; return -EACCES; @@ -5904,6 +5904,7 @@ int Monitor::handle_auth_reply_more( int Monitor::handle_auth_done( Connection *con, + AuthConnectionMeta *auth_meta, uint64_t global_id, uint32_t con_mode, const bufferlist& bl, @@ -5911,7 +5912,6 @@ int Monitor::handle_auth_done( std::string *connection_secret) { // verify authorizer reply - auto auth_meta = con->get_auth_meta(); auto p = bl.begin(); if (!auth_meta->authorizer->verify_reply(p, connection_secret)) { dout(0) << __func__ << " failed verifying authorizer reply" << dendl; @@ -5923,6 +5923,7 @@ int Monitor::handle_auth_done( int Monitor::handle_auth_bad_method( Connection *con, + AuthConnectionMeta *auth_meta, uint32_t old_auth_method, int result, const std::vector& allowed_methods, @@ -6028,6 +6029,7 @@ KeyStore *Monitor::ms_get_auth1_authorizer_keystore() int Monitor::handle_auth_request( Connection *con, + AuthConnectionMeta *auth_meta, bool more, uint32_t auth_method, const bufferlist &payload, @@ -6040,7 +6042,6 @@ int Monitor::handle_auth_request( << " method " << auth_method << " payload " << payload.length() << dendl; - auto auth_meta = con->get_auth_meta(); if (!more) { auth_meta->auth_mode = payload[0]; } diff --git a/src/mon/Monitor.h b/src/mon/Monitor.h index d79249edb8e..b4c773390c0 100644 --- a/src/mon/Monitor.h +++ b/src/mon/Monitor.h @@ -909,15 +909,18 @@ private: // AuthClient int get_auth_request( Connection *con, + AuthConnectionMeta *auth_meta, uint32_t *method, vector *preferred_modes, bufferlist *out) override; int handle_auth_reply_more( Connection *con, - const bufferlist& bl, + AuthConnectionMeta *auth_meta, + const bufferlist& bl, bufferlist *reply) override; int handle_auth_done( Connection *con, + AuthConnectionMeta *auth_meta, uint64_t global_id, uint32_t con_mode, const bufferlist& bl, @@ -925,6 +928,7 @@ private: std::string *connection_secret) override; int handle_auth_bad_method( Connection *con, + AuthConnectionMeta *auth_meta, uint32_t old_auth_method, int result, const std::vector& allowed_methods, @@ -933,6 +937,7 @@ private: // AuthServer int handle_auth_request( Connection *con, + AuthConnectionMeta *auth_meta, bool more, uint32_t auth_method, const bufferlist& bl, diff --git a/src/msg/Connection.h b/src/msg/Connection.h index 0f7c764a528..44cc5102fe0 100644 --- a/src/msg/Connection.h +++ b/src/msg/Connection.h @@ -104,10 +104,6 @@ public: return msgr; } - virtual AuthConnectionMeta *get_auth_meta() { - return nullptr; - } - /** * Queue the given Message to send out on the given Connection. * Success in this function does not guarantee Message delivery, only diff --git a/src/msg/async/AsyncConnection.cc b/src/msg/async/AsyncConnection.cc index 7e81dfd37bd..92b0df9cfb7 100644 --- a/src/msg/async/AsyncConnection.cc +++ b/src/msg/async/AsyncConnection.cc @@ -445,11 +445,6 @@ bool AsyncConnection::is_connected() { return protocol->is_connected(); } -AuthConnectionMeta *AsyncConnection::get_auth_meta() -{ - return &protocol->auth_meta; -} - void AsyncConnection::connect(const entity_addrvec_t &addrs, int type, entity_addr_t &target) { diff --git a/src/msg/async/AsyncConnection.h b/src/msg/async/AsyncConnection.h index d96b8da20af..4b089fa02d5 100644 --- a/src/msg/async/AsyncConnection.h +++ b/src/msg/async/AsyncConnection.h @@ -136,8 +136,6 @@ class AsyncConnection : public Connection { return target_addr; } - AuthConnectionMeta *get_auth_meta() override; - private: enum { STATE_NONE, diff --git a/src/msg/async/Protocol.h b/src/msg/async/Protocol.h index cf9005b1b66..d327e525a15 100644 --- a/src/msg/async/Protocol.h +++ b/src/msg/async/Protocol.h @@ -105,9 +105,6 @@ public: virtual void write_event() = 0; virtual bool is_queued() = 0; - virtual AuthConnectionMeta *get_auth_meta() { - return nullptr; - } }; #endif /* _MSG_ASYNC_PROTOCOL_ */ diff --git a/src/msg/async/ProtocolV2.cc b/src/msg/async/ProtocolV2.cc index 9ec9d9fc809..12f700ba87d 100644 --- a/src/msg/async/ProtocolV2.cc +++ b/src/msg/async/ProtocolV2.cc @@ -2099,7 +2099,8 @@ CtPtr ProtocolV2::send_auth_request(std::vector &allowed_methods) { vector preferred_modes; connection->lock.unlock(); int r = messenger->auth_client->get_auth_request( - connection, &auth_meta.auth_method, &preferred_modes, &bl); + connection, &auth_meta, + &auth_meta.auth_method, &preferred_modes, &bl); connection->lock.lock(); if (state != State::CONNECTING) { return _fault(); @@ -2127,7 +2128,9 @@ CtPtr ProtocolV2::handle_auth_bad_method(char *payload, uint32_t length) { ceph_assert(messenger->auth_client); connection->lock.unlock(); int r = messenger->auth_client->handle_auth_bad_method( - connection, bad_method.method(), bad_method.result(), + connection, + &auth_meta, + bad_method.method(), bad_method.result(), bad_method.allowed_methods(), bad_method.allowed_modes()); connection->lock.lock(); @@ -2150,7 +2153,7 @@ CtPtr ProtocolV2::handle_auth_reply_more(char *payload, uint32_t length) bufferlist reply; connection->lock.unlock(); int r = messenger->auth_client->handle_auth_reply_more( - connection, auth_more.auth_payload(), &reply); + connection, &auth_meta, auth_more.auth_payload(), &reply); connection->lock.lock(); if (state != State::CONNECTING) { return _fault(); @@ -2173,6 +2176,7 @@ CtPtr ProtocolV2::handle_auth_done(char *payload, uint32_t length) { connection->lock.unlock(); int r = messenger->auth_client->handle_auth_done( connection, + &auth_meta, auth_done.global_id(), auth_done.con_mode(), auth_done.auth_payload(), @@ -2469,7 +2473,8 @@ CtPtr ProtocolV2::_handle_auth_request(bufferlist& auth_payload, bool more) bufferlist reply; connection->lock.unlock(); int r = messenger->auth_server->handle_auth_request( - connection, more, auth_meta.auth_method, auth_payload, + connection, &auth_meta, + more, auth_meta.auth_method, auth_payload, &reply); connection->lock.lock(); if (state != ACCEPTING) { -- 2.39.5