From 27b5c54f11e9544e90a2914687359b0759e1620f Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Tue, 9 Apr 2019 11:37:54 +0800 Subject: [PATCH] crimson/auth: refactor AuthClient interface * we'd better return value by "return" not by passing output prameters by pointers. * remove unused parameters from AuthClient::handle_auth_done(), `session_key` and `connection_secret` are returned by setting corresponding member variables of `auth_meta`. Signed-off-by: Kefu Chai --- src/crimson/auth/AuthClient.h | 43 ++++++++++++++++++++++------------- src/crimson/auth/DummyAuth.h | 20 +++++----------- src/crimson/net/ProtocolV2.cc | 38 ++++++++++++------------------- 3 files changed, 48 insertions(+), 53 deletions(-) diff --git a/src/crimson/auth/AuthClient.h b/src/crimson/auth/AuthClient.h index 236707b66ea..f8d0685673b 100644 --- a/src/crimson/auth/AuthClient.h +++ b/src/crimson/auth/AuthClient.h @@ -3,7 +3,11 @@ #pragma once +#include +#include +#include #include +#include "include/buffer_fwd.h" #include "crimson/net/Fwd.h" class CryptoKey; @@ -15,37 +19,44 @@ public: using std::logic_error::logic_error; }; +using method_t = uint32_t; + // TODO: revisit interfaces for non-dummy implementations class AuthClient { public: virtual ~AuthClient() {} - // Build an authentication request to begin the handshake - virtual int get_auth_request( - ceph::net::ConnectionRef conn, - AuthConnectionMetaRef auth_meta, - uint32_t *method, - std::vector *preferred_modes, - bufferlist *out) = 0; + /// Build an authentication request to begin the handshake + /// + /// @throw auth::error if unable to build the request + virtual std::tuple, // preferred modes + ceph::bufferlist> // auth bl + get_auth_request(ceph::net::ConnectionRef conn, + AuthConnectionMetaRef auth_meta) = 0; - // Handle server's request to continue the handshake - virtual int handle_auth_reply_more( + /// Handle server's request to continue the handshake + /// + /// @throw auth::error if unable to build the request + virtual ceph::bufferlist handle_auth_reply_more( ceph::net::ConnectionRef conn, AuthConnectionMetaRef auth_meta, - const bufferlist& bl, - bufferlist *reply) = 0; + const ceph::bufferlist& bl) = 0; - // Handle server's indication that authentication succeeded + /// Handle server's indication that authentication succeeded + /// + /// @return 0 if authenticated, a negative number otherwise virtual int handle_auth_done( ceph::net::ConnectionRef conn, AuthConnectionMetaRef auth_meta, uint64_t global_id, uint32_t con_mode, - const bufferlist& bl, - CryptoKey *session_key, - std::string *connection_secret) = 0; + const bufferlist& bl) = 0; - // Handle server's indication that the previous auth attempt failed + /// Handle server's indication that the previous auth attempt failed + /// + /// @return 0 if will try next auth method, a negative number if we have no + /// more options virtual int handle_auth_bad_method( ceph::net::ConnectionRef conn, AuthConnectionMetaRef auth_meta, diff --git a/src/crimson/auth/DummyAuth.h b/src/crimson/auth/DummyAuth.h index b3b2dc62089..1f51b1aa38c 100644 --- a/src/crimson/auth/DummyAuth.h +++ b/src/crimson/auth/DummyAuth.h @@ -31,22 +31,16 @@ public: return nullptr; } - int get_auth_request( + std::tuple, bufferlist> get_auth_request( ceph::net::ConnectionRef conn, - AuthConnectionMetaRef auth_meta, - uint32_t *method, - std::vector *preferred_modes, - bufferlist *out) override { - *method = CEPH_AUTH_NONE; - *preferred_modes = { CEPH_CON_MODE_CRC }; - return 0; + AuthConnectionMetaRef auth_meta) override { + return {CEPH_AUTH_NONE, {CEPH_CON_MODE_CRC}, {}}; } - int handle_auth_reply_more( + ceph::bufferlist handle_auth_reply_more( ceph::net::ConnectionRef conn, AuthConnectionMetaRef auth_meta, - const bufferlist& bl, - bufferlist *reply) override { + const bufferlist& bl) override { ceph_abort(); } @@ -55,9 +49,7 @@ public: AuthConnectionMetaRef auth_meta, uint64_t global_id, uint32_t con_mode, - const bufferlist& bl, - CryptoKey *session_key, - std::string *connection_secret) { + const bufferlist& bl) override { return 0; } diff --git a/src/crimson/net/ProtocolV2.cc b/src/crimson/net/ProtocolV2.cc index 5d85349883c..c76fb0c8ded 100644 --- a/src/crimson/net/ProtocolV2.cc +++ b/src/crimson/net/ProtocolV2.cc @@ -481,14 +481,9 @@ seastar::future<> ProtocolV2::handle_auth_reply() logger().debug("{} auth reply more len={}", conn, auth_more.auth_payload().length()); ceph_assert(messenger.get_auth_client()); - ceph::bufferlist reply; - int r = messenger.get_auth_client()->handle_auth_reply_more( - conn.shared_from_this(), auth_meta, auth_more.auth_payload(), &reply); - if (r < 0) { - logger().error("{} auth_client handle_auth_reply_more returned {}", - conn, r); - abort_in_fault(); - } + // 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()); auto more_reply = AuthRequestMoreFrame::Encode(reply); return write_frame(more_reply); }).then([this] { @@ -503,9 +498,7 @@ seastar::future<> ProtocolV2::handle_auth_reply() conn.shared_from_this(), auth_meta, auth_done.global_id(), auth_done.con_mode(), - auth_done.auth_payload(), - &auth_meta->session_key, - &auth_meta->connection_secret); + auth_done.auth_payload()); if (r < 0) { logger().error("{} auth_client handle_auth_done returned {}", conn, r); abort_in_fault(); @@ -528,21 +521,20 @@ seastar::future<> ProtocolV2::client_auth(std::vector &allowed_methods // send_auth_request() logic ceph_assert(messenger.get_auth_client()); - bufferlist bl; - vector preferred_modes; - int r = messenger.get_auth_client()->get_auth_request( - conn.shared_from_this(), auth_meta, &auth_meta->auth_method, - &preferred_modes, &bl); - if (r < 0) { - logger().error("{} get_initial_auth_request returned {}", conn, r); + try { + auto [auth_method, preferred_modes, bl] = + messenger.get_auth_client()->get_auth_request(conn.shared_from_this(), auth_meta); + auth_meta->auth_method = auth_method; + auto frame = AuthRequestFrame::Encode(auth_method, preferred_modes, bl); + return write_frame(frame).then([this] { + return handle_auth_reply(); + }); + } catch (const ceph::auth::error& e) { + logger().error("{} get_initial_auth_request returned {}", conn, e); dispatch_reset(); abort_in_close(); + return seastar::now(); } - - auto frame = AuthRequestFrame::Encode(auth_meta->auth_method, preferred_modes, bl); - return write_frame(frame).then([this] { - return handle_auth_reply(); - }); } seastar::future ProtocolV2::process_wait() -- 2.39.5