From e5c5e83303171935f1087bce40b41ca1be55ef97 Mon Sep 17 00:00:00 2001 From: levy Date: Thu, 30 Jul 2020 16:08:21 +0800 Subject: [PATCH 01/15] feat(security): start negotiation when rpc_session is connecting --- include/dsn/tool-api/network.h | 16 +++++++++++ src/runtime/CMakeLists.txt | 2 ++ src/runtime/rpc/asio_net_provider.cpp | 3 ++ src/runtime/rpc/asio_rpc_session.cpp | 5 ++++ src/runtime/rpc/network.cpp | 31 +++++++++++++++++++++ src/runtime/rpc/rpc_engine.cpp | 2 ++ src/runtime/rpc/rpc_engine.h | 4 +++ src/runtime/security/CMakeLists.txt | 21 ++++++++++++++ src/runtime/security/client_negotiation.cpp | 20 +++++++++++++ src/runtime/security/client_negotiation.h | 24 ++++++++++++++++ src/runtime/security/negotiation.cpp | 28 +++++++++++++++++++ src/runtime/security/negotiation.h | 25 +++++++++++++++++ src/runtime/security/server_negotiation.cpp | 21 ++++++++++++++ src/runtime/security/server_negotiation.h | 24 ++++++++++++++++ 14 files changed, 226 insertions(+) create mode 100644 src/runtime/security/CMakeLists.txt create mode 100644 src/runtime/security/client_negotiation.cpp create mode 100644 src/runtime/security/client_negotiation.h create mode 100644 src/runtime/security/negotiation.cpp create mode 100644 src/runtime/security/negotiation.h create mode 100644 src/runtime/security/server_negotiation.cpp create mode 100644 src/runtime/security/server_negotiation.h diff --git a/include/dsn/tool-api/network.h b/include/dsn/tool-api/network.h index 0b3cf6645e..ef6d368a69 100644 --- a/include/dsn/tool-api/network.h +++ b/include/dsn/tool-api/network.h @@ -174,6 +174,8 @@ class connection_oriented_network : public network // to be defined virtual rpc_session_ptr create_client_session(::dsn::rpc_address server_addr) = 0; + bool need_auth(); + protected: typedef std::unordered_map<::dsn::rpc_address, rpc_session_ptr> client_sessions; client_sessions _clients; // to_address => rpc_session @@ -191,6 +193,11 @@ class connection_oriented_network : public network /*! session managements (both client and server types) */ + +namespace security { + class negotiation; +} + class rpc_client_matcher; class rpc_session : public ref_counter { @@ -227,6 +234,10 @@ class rpc_session : public ref_counter bool delay_recv(int delay_ms); bool on_recv_message(message_ex *msg, int delay_ms); + /// for negotiation + void negotiation(); + void auth_negotiation(); + public: /// /// for subclass to implement receiving message @@ -256,6 +267,7 @@ class rpc_session : public ref_counter enum session_state { SS_CONNECTING, + SS_NEGOTIATION, SS_CONNECTED, SS_DISCONNECTED }; @@ -286,6 +298,7 @@ class rpc_session : public ref_counter bool set_connecting(); // return true when it is permitted bool set_disconnected(); + void set_negotiation(); void set_connected(); void clear_send_queue(bool resend_msgs); @@ -304,6 +317,9 @@ class rpc_session : public ref_counter rpc_client_matcher *_matcher; std::atomic_int _delay_server_receive_ms; + + /// for negotiation + std::unique_ptr _negotiation; }; // --------- inline implementation -------------- diff --git a/src/runtime/CMakeLists.txt b/src/runtime/CMakeLists.txt index 5ebbeda09d..d369f70f96 100644 --- a/src/runtime/CMakeLists.txt +++ b/src/runtime/CMakeLists.txt @@ -1,9 +1,11 @@ add_subdirectory(test) add_subdirectory(rpc) add_subdirectory(task) +add_subdirectory(security) # TODO(zlw) remove perf_counter from dsn_runtime after the refactor by WuTao add_library(dsn_runtime STATIC + $ $ $ $ diff --git a/src/runtime/rpc/asio_net_provider.cpp b/src/runtime/rpc/asio_net_provider.cpp index 8d2f0fb095..901e429acf 100644 --- a/src/runtime/rpc/asio_net_provider.cpp +++ b/src/runtime/rpc/asio_net_provider.cpp @@ -153,6 +153,9 @@ void asio_network_provider::do_accept() null_parser, false); + // start negotiation when server accept the connection + s->negotiation(); + // when server connection threshold is hit, close the session, otherwise accept it if (check_if_conn_threshold_exceeded(s->remote_address())) { dwarn("close rpc connection from %s to %s due to hitting server " diff --git a/src/runtime/rpc/asio_rpc_session.cpp b/src/runtime/rpc/asio_rpc_session.cpp index f68187485c..1a61592ff5 100644 --- a/src/runtime/rpc/asio_rpc_session.cpp +++ b/src/runtime/rpc/asio_rpc_session.cpp @@ -199,6 +199,11 @@ void asio_rpc_session::connect() dinfo("client session %s connected", _remote_addr.to_string()); set_options(); + + // start auth negotiation when client is connecting to server + set_negotiation(); + negotiation(); + set_connected(); on_send_completed(); start_read_next(); diff --git a/src/runtime/rpc/network.cpp b/src/runtime/rpc/network.cpp index d2456790a6..f625eb9746 100644 --- a/src/runtime/rpc/network.cpp +++ b/src/runtime/rpc/network.cpp @@ -26,6 +26,7 @@ #include #include +#include #include "message_parser_manager.h" #include "runtime/rpc/rpc_engine.h" @@ -75,6 +76,17 @@ void rpc_session::set_connected() on_rpc_session_connected.execute(this); } +void rpc_session::set_negotiation() +{ + dassert(is_client(), "must be client session"); + + { + utils::auto_lock l(_lock); + dassert(_connect_state == SS_CONNECTING, "session must be connecting"); + _connect_state = SS_NEGOTIATION; + } +} + bool rpc_session::set_disconnected() { { @@ -414,6 +426,19 @@ bool rpc_session::on_recv_message(message_ex *msg, int delay_ms) return true; } +void rpc_session::negotiation() +{ + if (_net.need_auth()) { + auth_negotiation(); + } +} + +void rpc_session::auth_negotiation() +{ + _negotiation = security::create_negotiation(is_client(), this); + _negotiation->start_negotiate(); +} + //////////////////////////////////////////////////////////////////////////////////////////////// network::network(rpc_engine *srv, network *inner_provider) : _engine(srv), _client_hdr_format(NET_HDR_DSN), _unknown_msg_header_format(NET_HDR_INVALID) @@ -720,4 +745,10 @@ void connection_oriented_network::on_client_session_disconnected(rpc_session_ptr scount); } } + +bool connection_oriented_network::need_auth() +{ + return engine()->need_auth(); +} + } // namespace dsn diff --git a/src/runtime/rpc/rpc_engine.cpp b/src/runtime/rpc/rpc_engine.cpp index 2bf70dec71..6e4312eff5 100644 --- a/src/runtime/rpc/rpc_engine.cpp +++ b/src/runtime/rpc/rpc_engine.cpp @@ -409,6 +409,8 @@ rpc_engine::rpc_engine(service_node *node) : _node(node), _rpc_matcher(this) dassert(_node != nullptr, ""); _is_running = false; _is_serving = false; + + _need_auth = dsn_config_get_value_bool("security", "open_auth", false, "whether open auth"); } // diff --git a/src/runtime/rpc/rpc_engine.h b/src/runtime/rpc/rpc_engine.h index 3e33fe23f6..090e793395 100644 --- a/src/runtime/rpc/rpc_engine.h +++ b/src/runtime/rpc/rpc_engine.h @@ -173,6 +173,7 @@ class rpc_engine // call with explicit address void call_address(rpc_address addr, message_ex *request, const rpc_response_task_ptr &call); + bool need_auth() { return _need_auth; } private: network *create_network(const network_server_config &netcs, bool client_only, @@ -190,6 +191,9 @@ class rpc_engine volatile bool _is_running; volatile bool _is_serving; + + // read from config file + bool _need_auth; }; // ------------------------ inline implementations -------------------- diff --git a/src/runtime/security/CMakeLists.txt b/src/runtime/security/CMakeLists.txt new file mode 100644 index 0000000000..47c08e80c1 --- /dev/null +++ b/src/runtime/security/CMakeLists.txt @@ -0,0 +1,21 @@ +set(MY_PROJ_NAME dsn.security) + +# Source files under CURRENT project directory will be automatically included. +# You can manually set MY_PROJ_SRC to include source files under other directories. +set(MY_PROJ_SRC "") + +# Search mode for source files under CURRENT project directory? +# "GLOB_RECURSE" for recursive search +# "GLOB" for non-recursive search +set(MY_SRC_SEARCH_MODE "GLOB") + +set(MY_PROJ_INC_PATH "") + +set(MY_PROJ_LIBS "") + +set(MY_PROJ_LIB_PATH "") + +# Extra files that will be installed +set(MY_BINPLACES "") + +dsn_add_object() diff --git a/src/runtime/security/client_negotiation.cpp b/src/runtime/security/client_negotiation.cpp new file mode 100644 index 0000000000..0d9a62b5ad --- /dev/null +++ b/src/runtime/security/client_negotiation.cpp @@ -0,0 +1,20 @@ +// Copyright (c) 2017, Xiaomi, Inc. All rights reserved. +// This source code is licensed under the Apache License Version 2.0, which +// can be found in the LICENSE file in the root directory of this source tree. + +#include + +namespace dsn { +namespace security { + +client_negotiation::client_negotiation(rpc_session *session) : negotiation(), _session(session) +{ +} + +void client_negotiation::start_negotiate() +{ + // TBD(zlw) +} + +} // namespace security +} // namespace dsn diff --git a/src/runtime/security/client_negotiation.h b/src/runtime/security/client_negotiation.h new file mode 100644 index 0000000000..89b9db010c --- /dev/null +++ b/src/runtime/security/client_negotiation.h @@ -0,0 +1,24 @@ +// Copyright (c) 2017, Xiaomi, Inc. All rights reserved. +// This source code is licensed under the Apache License Version 2.0, which +// can be found in the LICENSE file in the root directory of this source tree. + +#pragma once + +#include "negotiation.h" + +namespace dsn { +namespace security { + +class client_negotiation : public negotiation +{ +public: + client_negotiation(rpc_session *session); + void start_negotiate(); + +private: + // the lifetime of _session should be longer than client_negotiation + rpc_session *_session; +}; + +} // namespace security +} // namespace dsn diff --git a/src/runtime/security/negotiation.cpp b/src/runtime/security/negotiation.cpp new file mode 100644 index 0000000000..d2a5a2ba9f --- /dev/null +++ b/src/runtime/security/negotiation.cpp @@ -0,0 +1,28 @@ +// Copyright (c) 2017, Xiaomi, Inc. All rights reserved. +// This source code is licensed under the Apache License Version 2.0, which +// can be found in the LICENSE file in the root directory of this source tree. + +#include "negotiation.h" +#include "client_negotiation.h" +#include "server_negotiation.h" + +#include + +namespace dsn { +class rpc_session; + +namespace security { + +negotiation::~negotiation() {} + +std::unique_ptr create_negotiation(bool is_client, rpc_session *session) +{ + if (is_client) { + return dsn::make_unique(session); + } else { + return dsn::make_unique(session); + } +} + +} // namespace security +} // namespace dsn diff --git a/src/runtime/security/negotiation.h b/src/runtime/security/negotiation.h new file mode 100644 index 0000000000..d9528f017d --- /dev/null +++ b/src/runtime/security/negotiation.h @@ -0,0 +1,25 @@ +// Copyright (c) 2017, Xiaomi, Inc. All rights reserved. +// This source code is licensed under the Apache License Version 2.0, which +// can be found in the LICENSE file in the root directory of this source tree. + +#pragma once + +#include + +namespace dsn { +class rpc_session; + +namespace security { + +class negotiation +{ +public: + negotiation() = default; + virtual ~negotiation() = 0; + + virtual void start_negotiate() = 0; +}; + +std::unique_ptr create_negotiation(bool is_client, rpc_session *session); +} // namespace security +} // namespace dsn diff --git a/src/runtime/security/server_negotiation.cpp b/src/runtime/security/server_negotiation.cpp new file mode 100644 index 0000000000..c98e6bb6d2 --- /dev/null +++ b/src/runtime/security/server_negotiation.cpp @@ -0,0 +1,21 @@ +// Copyright (c) 2017, Xiaomi, Inc. All rights reserved. +// This source code is licensed under the Apache License Version 2.0, which +// can be found in the LICENSE file in the root directory of this source tree. + +#include +#include "negotiation.h" + +namespace dsn { +namespace security { + +server_negotiation::server_negotiation(rpc_session *session) : negotiation(), _session(session) +{ +} + +void server_negotiation::start_negotiate() +{ + // TBD(zlw) +} + +} // namespace security +} // namespace dsn diff --git a/src/runtime/security/server_negotiation.h b/src/runtime/security/server_negotiation.h new file mode 100644 index 0000000000..e31db08236 --- /dev/null +++ b/src/runtime/security/server_negotiation.h @@ -0,0 +1,24 @@ +// Copyright (c) 2017, Xiaomi, Inc. All rights reserved. +// This source code is licensed under the Apache License Version 2.0, which +// can be found in the LICENSE file in the root directory of this source tree. + +#pragma once + +#include "negotiation.h" + +namespace dsn { +namespace security { + +class server_negotiation : public negotiation +{ +public: + server_negotiation(rpc_session *session); + void start_negotiate(); + +private: + // the lifetime of _session should be longer than client_negotiation + rpc_session *_session; +}; + +} // namespace security +} // namespace dsn From 882a5e3f0d80a35786ce666bedaa27f59c557ac5 Mon Sep 17 00:00:00 2001 From: levy Date: Thu, 30 Jul 2020 17:05:01 +0800 Subject: [PATCH 02/15] fix set_connected --- src/runtime/rpc/network.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/runtime/rpc/network.cpp b/src/runtime/rpc/network.cpp index f625eb9746..80e8d8d142 100644 --- a/src/runtime/rpc/network.cpp +++ b/src/runtime/rpc/network.cpp @@ -66,7 +66,9 @@ void rpc_session::set_connected() { utils::auto_lock l(_lock); - dassert(_connect_state == SS_CONNECTING, "session must be connecting"); + dassert((_connect_state == SS_NEGOTIATION && net().need_auth()) || + (_connect_state == SS_CONNECTING && !net().need_auth()), + "wrong session state"); _connect_state = SS_CONNECTED; } @@ -746,9 +748,6 @@ void connection_oriented_network::on_client_session_disconnected(rpc_session_ptr } } -bool connection_oriented_network::need_auth() -{ - return engine()->need_auth(); -} +bool connection_oriented_network::need_auth() { return engine()->need_auth(); } } // namespace dsn From 0d77ed4d7ca0dc5a8bc55bc9f68f833bacb34627 Mon Sep 17 00:00:00 2001 From: levy Date: Thu, 30 Jul 2020 17:34:14 +0800 Subject: [PATCH 03/15] refactor --- src/runtime/rpc/asio_rpc_session.cpp | 1 - src/runtime/rpc/network.cpp | 5 +++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/runtime/rpc/asio_rpc_session.cpp b/src/runtime/rpc/asio_rpc_session.cpp index 1a61592ff5..507c2862eb 100644 --- a/src/runtime/rpc/asio_rpc_session.cpp +++ b/src/runtime/rpc/asio_rpc_session.cpp @@ -201,7 +201,6 @@ void asio_rpc_session::connect() set_options(); // start auth negotiation when client is connecting to server - set_negotiation(); negotiation(); set_connected(); diff --git a/src/runtime/rpc/network.cpp b/src/runtime/rpc/network.cpp index 80e8d8d142..78f5b92ebe 100644 --- a/src/runtime/rpc/network.cpp +++ b/src/runtime/rpc/network.cpp @@ -431,6 +431,11 @@ bool rpc_session::on_recv_message(message_ex *msg, int delay_ms) void rpc_session::negotiation() { if (_net.need_auth()) { + // set the negotiation state if it's a client rpc_session + if (is_client()) { + set_negotiation(); + } + auth_negotiation(); } } From 9fa1eeaf1157ea4ff106c7d285220f3b33077d2f Mon Sep 17 00:00:00 2001 From: levy Date: Thu, 30 Jul 2020 18:16:26 +0800 Subject: [PATCH 04/15] use CFLAG to define security configuration --- include/dsn/tool-api/network.h | 4 +--- src/runtime/rpc/network.cpp | 8 +++----- src/runtime/rpc/rpc_engine.cpp | 2 -- src/runtime/rpc/rpc_engine.h | 4 ---- src/runtime/security/negotiation.cpp | 5 +++-- src/runtime/security/negotiation.h | 2 ++ 6 files changed, 9 insertions(+), 16 deletions(-) diff --git a/include/dsn/tool-api/network.h b/include/dsn/tool-api/network.h index ef6d368a69..39790cc493 100644 --- a/include/dsn/tool-api/network.h +++ b/include/dsn/tool-api/network.h @@ -174,8 +174,6 @@ class connection_oriented_network : public network // to be defined virtual rpc_session_ptr create_client_session(::dsn::rpc_address server_addr) = 0; - bool need_auth(); - protected: typedef std::unordered_map<::dsn::rpc_address, rpc_session_ptr> client_sessions; client_sessions _clients; // to_address => rpc_session @@ -195,7 +193,7 @@ class connection_oriented_network : public network */ namespace security { - class negotiation; +class negotiation; } class rpc_client_matcher; diff --git a/src/runtime/rpc/network.cpp b/src/runtime/rpc/network.cpp index 78f5b92ebe..444bad12bf 100644 --- a/src/runtime/rpc/network.cpp +++ b/src/runtime/rpc/network.cpp @@ -66,8 +66,8 @@ void rpc_session::set_connected() { utils::auto_lock l(_lock); - dassert((_connect_state == SS_NEGOTIATION && net().need_auth()) || - (_connect_state == SS_CONNECTING && !net().need_auth()), + dassert((_connect_state == SS_NEGOTIATION && security::FLAGS_open_auth) || + (_connect_state == SS_CONNECTING && !security::FLAGS_open_auth), "wrong session state"); _connect_state = SS_CONNECTED; } @@ -430,7 +430,7 @@ bool rpc_session::on_recv_message(message_ex *msg, int delay_ms) void rpc_session::negotiation() { - if (_net.need_auth()) { + if (security::FLAGS_open_auth) { // set the negotiation state if it's a client rpc_session if (is_client()) { set_negotiation(); @@ -753,6 +753,4 @@ void connection_oriented_network::on_client_session_disconnected(rpc_session_ptr } } -bool connection_oriented_network::need_auth() { return engine()->need_auth(); } - } // namespace dsn diff --git a/src/runtime/rpc/rpc_engine.cpp b/src/runtime/rpc/rpc_engine.cpp index 6e4312eff5..2bf70dec71 100644 --- a/src/runtime/rpc/rpc_engine.cpp +++ b/src/runtime/rpc/rpc_engine.cpp @@ -409,8 +409,6 @@ rpc_engine::rpc_engine(service_node *node) : _node(node), _rpc_matcher(this) dassert(_node != nullptr, ""); _is_running = false; _is_serving = false; - - _need_auth = dsn_config_get_value_bool("security", "open_auth", false, "whether open auth"); } // diff --git a/src/runtime/rpc/rpc_engine.h b/src/runtime/rpc/rpc_engine.h index 090e793395..3e33fe23f6 100644 --- a/src/runtime/rpc/rpc_engine.h +++ b/src/runtime/rpc/rpc_engine.h @@ -173,7 +173,6 @@ class rpc_engine // call with explicit address void call_address(rpc_address addr, message_ex *request, const rpc_response_task_ptr &call); - bool need_auth() { return _need_auth; } private: network *create_network(const network_server_config &netcs, bool client_only, @@ -191,9 +190,6 @@ class rpc_engine volatile bool _is_running; volatile bool _is_serving; - - // read from config file - bool _need_auth; }; // ------------------------ inline implementations -------------------- diff --git a/src/runtime/security/negotiation.cpp b/src/runtime/security/negotiation.cpp index d2a5a2ba9f..6137b84ad5 100644 --- a/src/runtime/security/negotiation.cpp +++ b/src/runtime/security/negotiation.cpp @@ -6,13 +6,14 @@ #include "client_negotiation.h" #include "server_negotiation.h" +#include #include namespace dsn { -class rpc_session; - namespace security { +DSN_DEFINE_bool("security", open_auth, false, "whether open auth or not"); + negotiation::~negotiation() {} std::unique_ptr create_negotiation(bool is_client, rpc_session *session) diff --git a/src/runtime/security/negotiation.h b/src/runtime/security/negotiation.h index d9528f017d..90c1ddb45c 100644 --- a/src/runtime/security/negotiation.h +++ b/src/runtime/security/negotiation.h @@ -11,6 +11,8 @@ class rpc_session; namespace security { +extern bool FLAGS_open_auth; + class negotiation { public: From 1d92bc052626920574f995695fe91553f49c2417 Mon Sep 17 00:00:00 2001 From: levy Date: Thu, 30 Jul 2020 18:29:21 +0800 Subject: [PATCH 05/15] refactor --- include/dsn/tool-api/network.h | 4 +++- src/runtime/security/client_negotiation.cpp | 4 ++-- src/runtime/security/client_negotiation.h | 4 ---- src/runtime/security/negotiation.h | 8 +++++++- src/runtime/security/server_negotiation.cpp | 5 ++--- src/runtime/security/server_negotiation.h | 4 ---- 6 files changed, 14 insertions(+), 15 deletions(-) diff --git a/include/dsn/tool-api/network.h b/include/dsn/tool-api/network.h index 39790cc493..eaf1f38a4f 100644 --- a/include/dsn/tool-api/network.h +++ b/include/dsn/tool-api/network.h @@ -234,7 +234,6 @@ class rpc_session : public ref_counter /// for negotiation void negotiation(); - void auth_negotiation(); public: /// @@ -310,6 +309,9 @@ class rpc_session : public ref_counter message_reader _reader; message_parser_ptr _parser; +private: + void auth_negotiation(); + private: const bool _is_client; rpc_client_matcher *_matcher; diff --git a/src/runtime/security/client_negotiation.cpp b/src/runtime/security/client_negotiation.cpp index 0d9a62b5ad..84b17d9e46 100644 --- a/src/runtime/security/client_negotiation.cpp +++ b/src/runtime/security/client_negotiation.cpp @@ -2,12 +2,12 @@ // This source code is licensed under the Apache License Version 2.0, which // can be found in the LICENSE file in the root directory of this source tree. -#include +#include "client_negotiation.h" namespace dsn { namespace security { -client_negotiation::client_negotiation(rpc_session *session) : negotiation(), _session(session) +client_negotiation::client_negotiation(rpc_session *session) : negotiation(session) { } diff --git a/src/runtime/security/client_negotiation.h b/src/runtime/security/client_negotiation.h index 89b9db010c..5d82b048ff 100644 --- a/src/runtime/security/client_negotiation.h +++ b/src/runtime/security/client_negotiation.h @@ -14,10 +14,6 @@ class client_negotiation : public negotiation public: client_negotiation(rpc_session *session); void start_negotiate(); - -private: - // the lifetime of _session should be longer than client_negotiation - rpc_session *_session; }; } // namespace security diff --git a/src/runtime/security/negotiation.h b/src/runtime/security/negotiation.h index 90c1ddb45c..a697f5796c 100644 --- a/src/runtime/security/negotiation.h +++ b/src/runtime/security/negotiation.h @@ -16,10 +16,16 @@ extern bool FLAGS_open_auth; class negotiation { public: - negotiation() = default; + negotiation(rpc_session *session) : _session(session) {} virtual ~negotiation() = 0; virtual void start_negotiate() = 0; + +protected: + // In rpc_session, there is a pointer to a negotiation which type is `std::shared_ptr`, + // if we use `std::shared _session`, it will case a circular reference between + // these two `std::shared_ptr` + rpc_session *_session; }; std::unique_ptr create_negotiation(bool is_client, rpc_session *session); diff --git a/src/runtime/security/server_negotiation.cpp b/src/runtime/security/server_negotiation.cpp index c98e6bb6d2..160e14d262 100644 --- a/src/runtime/security/server_negotiation.cpp +++ b/src/runtime/security/server_negotiation.cpp @@ -2,13 +2,12 @@ // This source code is licensed under the Apache License Version 2.0, which // can be found in the LICENSE file in the root directory of this source tree. -#include -#include "negotiation.h" +#include "server_negotiation.h" namespace dsn { namespace security { -server_negotiation::server_negotiation(rpc_session *session) : negotiation(), _session(session) +server_negotiation::server_negotiation(rpc_session *session) : negotiation(session) { } diff --git a/src/runtime/security/server_negotiation.h b/src/runtime/security/server_negotiation.h index e31db08236..f376978080 100644 --- a/src/runtime/security/server_negotiation.h +++ b/src/runtime/security/server_negotiation.h @@ -14,10 +14,6 @@ class server_negotiation : public negotiation public: server_negotiation(rpc_session *session); void start_negotiate(); - -private: - // the lifetime of _session should be longer than client_negotiation - rpc_session *_session; }; } // namespace security From 2cda288e37e0f19e65fe89026600af2a784b3e77 Mon Sep 17 00:00:00 2001 From: levy Date: Thu, 30 Jul 2020 18:30:34 +0800 Subject: [PATCH 06/15] fix --- include/dsn/tool-api/network.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/dsn/tool-api/network.h b/include/dsn/tool-api/network.h index eaf1f38a4f..a0fc6cd45d 100644 --- a/include/dsn/tool-api/network.h +++ b/include/dsn/tool-api/network.h @@ -318,7 +318,6 @@ class rpc_session : public ref_counter std::atomic_int _delay_server_receive_ms; - /// for negotiation std::unique_ptr _negotiation; }; From 9b4e7777a51817c8706287e520f46fcbec9d5b68 Mon Sep 17 00:00:00 2001 From: levy Date: Thu, 30 Jul 2020 18:31:54 +0800 Subject: [PATCH 07/15] fix --- src/runtime/rpc/network.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/runtime/rpc/network.cpp b/src/runtime/rpc/network.cpp index 444bad12bf..2fd364eb26 100644 --- a/src/runtime/rpc/network.cpp +++ b/src/runtime/rpc/network.cpp @@ -24,12 +24,13 @@ * THE SOFTWARE. */ -#include -#include -#include +#include "runtime/security/negotiation.h" #include "message_parser_manager.h" #include "runtime/rpc/rpc_engine.h" +#include +#include + namespace dsn { /*static*/ join_point rpc_session::on_rpc_session_connected("rpc.session.connected"); From f8b65ff91df856e2f52ab013aeced420029b959f Mon Sep 17 00:00:00 2001 From: levy Date: Thu, 30 Jul 2020 18:32:59 +0800 Subject: [PATCH 08/15] fix --- src/runtime/rpc/asio_net_provider.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/rpc/asio_net_provider.cpp b/src/runtime/rpc/asio_net_provider.cpp index 901e429acf..1178c9c477 100644 --- a/src/runtime/rpc/asio_net_provider.cpp +++ b/src/runtime/rpc/asio_net_provider.cpp @@ -153,7 +153,7 @@ void asio_network_provider::do_accept() null_parser, false); - // start negotiation when server accept the connection + // start negotiation when server accepts the connection s->negotiation(); // when server connection threshold is hit, close the session, otherwise accept it From c13d90b8555df14adee7f04cd27e520473a1c378 Mon Sep 17 00:00:00 2001 From: levy Date: Thu, 30 Jul 2020 18:45:15 +0800 Subject: [PATCH 09/15] format --- src/runtime/security/client_negotiation.cpp | 4 +--- src/runtime/security/server_negotiation.cpp | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/runtime/security/client_negotiation.cpp b/src/runtime/security/client_negotiation.cpp index 84b17d9e46..fe6705366b 100644 --- a/src/runtime/security/client_negotiation.cpp +++ b/src/runtime/security/client_negotiation.cpp @@ -7,9 +7,7 @@ namespace dsn { namespace security { -client_negotiation::client_negotiation(rpc_session *session) : negotiation(session) -{ -} +client_negotiation::client_negotiation(rpc_session *session) : negotiation(session) {} void client_negotiation::start_negotiate() { diff --git a/src/runtime/security/server_negotiation.cpp b/src/runtime/security/server_negotiation.cpp index 160e14d262..d3a8c6056f 100644 --- a/src/runtime/security/server_negotiation.cpp +++ b/src/runtime/security/server_negotiation.cpp @@ -7,9 +7,7 @@ namespace dsn { namespace security { -server_negotiation::server_negotiation(rpc_session *session) : negotiation(session) -{ -} +server_negotiation::server_negotiation(rpc_session *session) : negotiation(session) {} void server_negotiation::start_negotiate() { From 1f205f070b455b82d07a30f29d766075d6e01383 Mon Sep 17 00:00:00 2001 From: levy Date: Mon, 3 Aug 2020 15:02:10 +0800 Subject: [PATCH 10/15] fix by review --- include/dsn/tool-api/network.h | 2 +- src/runtime/rpc/asio_net_provider.cpp | 2 +- src/runtime/rpc/asio_rpc_session.cpp | 2 +- src/runtime/rpc/network.cpp | 8 ++++---- src/runtime/security/client_negotiation.cpp | 19 ++++++++++++++++--- src/runtime/security/client_negotiation.h | 19 ++++++++++++++++--- src/runtime/security/negotiation.cpp | 21 +++++++++++++++++---- src/runtime/security/negotiation.h | 21 +++++++++++++++++---- src/runtime/security/server_negotiation.cpp | 19 ++++++++++++++++--- src/runtime/security/server_negotiation.h | 19 ++++++++++++++++--- 10 files changed, 105 insertions(+), 27 deletions(-) diff --git a/include/dsn/tool-api/network.h b/include/dsn/tool-api/network.h index a0fc6cd45d..743329eb2a 100644 --- a/include/dsn/tool-api/network.h +++ b/include/dsn/tool-api/network.h @@ -233,7 +233,7 @@ class rpc_session : public ref_counter bool on_recv_message(message_ex *msg, int delay_ms); /// for negotiation - void negotiation(); + void start_negotiation(); public: /// diff --git a/src/runtime/rpc/asio_net_provider.cpp b/src/runtime/rpc/asio_net_provider.cpp index 1178c9c477..2f1e29e114 100644 --- a/src/runtime/rpc/asio_net_provider.cpp +++ b/src/runtime/rpc/asio_net_provider.cpp @@ -154,7 +154,7 @@ void asio_network_provider::do_accept() false); // start negotiation when server accepts the connection - s->negotiation(); + s->start_negotiation(); // when server connection threshold is hit, close the session, otherwise accept it if (check_if_conn_threshold_exceeded(s->remote_address())) { diff --git a/src/runtime/rpc/asio_rpc_session.cpp b/src/runtime/rpc/asio_rpc_session.cpp index 507c2862eb..b18122f985 100644 --- a/src/runtime/rpc/asio_rpc_session.cpp +++ b/src/runtime/rpc/asio_rpc_session.cpp @@ -201,7 +201,7 @@ void asio_rpc_session::connect() set_options(); // start auth negotiation when client is connecting to server - negotiation(); + start_negotiation(); set_connected(); on_send_completed(); diff --git a/src/runtime/rpc/network.cpp b/src/runtime/rpc/network.cpp index 2fd364eb26..30b50322fc 100644 --- a/src/runtime/rpc/network.cpp +++ b/src/runtime/rpc/network.cpp @@ -67,8 +67,8 @@ void rpc_session::set_connected() { utils::auto_lock l(_lock); - dassert((_connect_state == SS_NEGOTIATION && security::FLAGS_open_auth) || - (_connect_state == SS_CONNECTING && !security::FLAGS_open_auth), + dassert((_connect_state == SS_NEGOTIATION && security::FLAGS_enable_auth) || + (_connect_state == SS_CONNECTING && !security::FLAGS_enable_auth), "wrong session state"); _connect_state = SS_CONNECTED; } @@ -429,9 +429,9 @@ bool rpc_session::on_recv_message(message_ex *msg, int delay_ms) return true; } -void rpc_session::negotiation() +void rpc_session::start_negotiation() { - if (security::FLAGS_open_auth) { + if (security::FLAGS_enable_auth) { // set the negotiation state if it's a client rpc_session if (is_client()) { set_negotiation(); diff --git a/src/runtime/security/client_negotiation.cpp b/src/runtime/security/client_negotiation.cpp index fe6705366b..52794731c2 100644 --- a/src/runtime/security/client_negotiation.cpp +++ b/src/runtime/security/client_negotiation.cpp @@ -1,6 +1,19 @@ -// Copyright (c) 2017, Xiaomi, Inc. All rights reserved. -// This source code is licensed under the Apache License Version 2.0, which -// can be found in the LICENSE file in the root directory of this source tree. +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. #include "client_negotiation.h" diff --git a/src/runtime/security/client_negotiation.h b/src/runtime/security/client_negotiation.h index 5d82b048ff..c4ed683cc1 100644 --- a/src/runtime/security/client_negotiation.h +++ b/src/runtime/security/client_negotiation.h @@ -1,6 +1,19 @@ -// Copyright (c) 2017, Xiaomi, Inc. All rights reserved. -// This source code is licensed under the Apache License Version 2.0, which -// can be found in the LICENSE file in the root directory of this source tree. +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. #pragma once diff --git a/src/runtime/security/negotiation.cpp b/src/runtime/security/negotiation.cpp index 6137b84ad5..6fc3b385f9 100644 --- a/src/runtime/security/negotiation.cpp +++ b/src/runtime/security/negotiation.cpp @@ -1,6 +1,19 @@ -// Copyright (c) 2017, Xiaomi, Inc. All rights reserved. -// This source code is licensed under the Apache License Version 2.0, which -// can be found in the LICENSE file in the root directory of this source tree. +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. #include "negotiation.h" #include "client_negotiation.h" @@ -12,7 +25,7 @@ namespace dsn { namespace security { -DSN_DEFINE_bool("security", open_auth, false, "whether open auth or not"); +DSN_DEFINE_bool("security", enable_auth, false, "whether open auth or not"); negotiation::~negotiation() {} diff --git a/src/runtime/security/negotiation.h b/src/runtime/security/negotiation.h index a697f5796c..aab5160f3a 100644 --- a/src/runtime/security/negotiation.h +++ b/src/runtime/security/negotiation.h @@ -1,6 +1,19 @@ -// Copyright (c) 2017, Xiaomi, Inc. All rights reserved. -// This source code is licensed under the Apache License Version 2.0, which -// can be found in the LICENSE file in the root directory of this source tree. +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. #pragma once @@ -11,7 +24,7 @@ class rpc_session; namespace security { -extern bool FLAGS_open_auth; +extern bool FLAGS_enable_auth; class negotiation { diff --git a/src/runtime/security/server_negotiation.cpp b/src/runtime/security/server_negotiation.cpp index d3a8c6056f..cbeb049c32 100644 --- a/src/runtime/security/server_negotiation.cpp +++ b/src/runtime/security/server_negotiation.cpp @@ -1,6 +1,19 @@ -// Copyright (c) 2017, Xiaomi, Inc. All rights reserved. -// This source code is licensed under the Apache License Version 2.0, which -// can be found in the LICENSE file in the root directory of this source tree. +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. #include "server_negotiation.h" diff --git a/src/runtime/security/server_negotiation.h b/src/runtime/security/server_negotiation.h index f376978080..197edf3a54 100644 --- a/src/runtime/security/server_negotiation.h +++ b/src/runtime/security/server_negotiation.h @@ -1,6 +1,19 @@ -// Copyright (c) 2017, Xiaomi, Inc. All rights reserved. -// This source code is licensed under the Apache License Version 2.0, which -// can be found in the LICENSE file in the root directory of this source tree. +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. #pragma once From 16b075c50ad8cb2198cbece21930fd62047a97aa Mon Sep 17 00:00:00 2001 From: levy Date: Mon, 3 Aug 2020 15:30:33 +0800 Subject: [PATCH 11/15] fix by review --- src/runtime/rpc/network.cpp | 4 ++++ src/runtime/security/negotiation.h | 2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/runtime/rpc/network.cpp b/src/runtime/rpc/network.cpp index 30b50322fc..e8969d7dfc 100644 --- a/src/runtime/rpc/network.cpp +++ b/src/runtime/rpc/network.cpp @@ -37,6 +37,10 @@ namespace dsn { /*static*/ join_point rpc_session::on_rpc_session_disconnected("rpc.session.disconnected"); +namespace security { + extern bool FLAGS_enable_auth; +} // namespace security + rpc_session::~rpc_session() { clear_send_queue(false); diff --git a/src/runtime/security/negotiation.h b/src/runtime/security/negotiation.h index aab5160f3a..ce84f12f59 100644 --- a/src/runtime/security/negotiation.h +++ b/src/runtime/security/negotiation.h @@ -24,8 +24,6 @@ class rpc_session; namespace security { -extern bool FLAGS_enable_auth; - class negotiation { public: From a79b8c6f9dc15e7d6c35f8ea54c2938884725b06 Mon Sep 17 00:00:00 2001 From: levy Date: Mon, 3 Aug 2020 16:06:01 +0800 Subject: [PATCH 12/15] format --- src/runtime/rpc/network.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/rpc/network.cpp b/src/runtime/rpc/network.cpp index e8969d7dfc..acbd0aa1ea 100644 --- a/src/runtime/rpc/network.cpp +++ b/src/runtime/rpc/network.cpp @@ -38,7 +38,7 @@ namespace dsn { rpc_session::on_rpc_session_disconnected("rpc.session.disconnected"); namespace security { - extern bool FLAGS_enable_auth; +extern bool FLAGS_enable_auth; } // namespace security rpc_session::~rpc_session() From 6dcfb2cd7ed9e6ba72124842a3e190ad6b8544c5 Mon Sep 17 00:00:00 2001 From: levy Date: Tue, 4 Aug 2020 10:24:54 +0800 Subject: [PATCH 13/15] fix --- src/runtime/security/negotiation.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/runtime/security/negotiation.cpp b/src/runtime/security/negotiation.cpp index 6fc3b385f9..2cf8f16c3b 100644 --- a/src/runtime/security/negotiation.cpp +++ b/src/runtime/security/negotiation.cpp @@ -32,9 +32,9 @@ negotiation::~negotiation() {} std::unique_ptr create_negotiation(bool is_client, rpc_session *session) { if (is_client) { - return dsn::make_unique(session); + return make_unique(session); } else { - return dsn::make_unique(session); + return make_unique(session); } } From c443c088bb568e2d9f3b0f58611d8b9e08a0dfa8 Mon Sep 17 00:00:00 2001 From: levy Date: Tue, 4 Aug 2020 11:25:39 +0800 Subject: [PATCH 14/15] fix by review --- include/dsn/tool-api/network.h | 2 +- src/runtime/rpc/network.cpp | 6 +++--- src/runtime/security/negotiation.h | 5 ++--- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/include/dsn/tool-api/network.h b/include/dsn/tool-api/network.h index 743329eb2a..8e630b1af6 100644 --- a/include/dsn/tool-api/network.h +++ b/include/dsn/tool-api/network.h @@ -264,7 +264,7 @@ class rpc_session : public ref_counter enum session_state { SS_CONNECTING, - SS_NEGOTIATION, + SS_NEGOTIATING, SS_CONNECTED, SS_DISCONNECTED }; diff --git a/src/runtime/rpc/network.cpp b/src/runtime/rpc/network.cpp index acbd0aa1ea..284f3c30f9 100644 --- a/src/runtime/rpc/network.cpp +++ b/src/runtime/rpc/network.cpp @@ -71,8 +71,8 @@ void rpc_session::set_connected() { utils::auto_lock l(_lock); - dassert((_connect_state == SS_NEGOTIATION && security::FLAGS_enable_auth) || - (_connect_state == SS_CONNECTING && !security::FLAGS_enable_auth), + dassert((_connect_state == SS_NEGOTIATING && security::FLAGS_enable_auth) || + (_connect_state == SS_CONNECTING && !security::FLAGS_enable_auth), "wrong session state"); _connect_state = SS_CONNECTED; } @@ -90,7 +90,7 @@ void rpc_session::set_negotiation() { utils::auto_lock l(_lock); dassert(_connect_state == SS_CONNECTING, "session must be connecting"); - _connect_state = SS_NEGOTIATION; + _connect_state = SS_NEGOTIATING; } } diff --git a/src/runtime/security/negotiation.h b/src/runtime/security/negotiation.h index ce84f12f59..d7f5c49272 100644 --- a/src/runtime/security/negotiation.h +++ b/src/runtime/security/negotiation.h @@ -33,9 +33,8 @@ class negotiation virtual void start_negotiate() = 0; protected: - // In rpc_session, there is a pointer to a negotiation which type is `std::shared_ptr`, - // if we use `std::shared _session`, it will case a circular reference between - // these two `std::shared_ptr` + // The ownership of the negotiation instance is held by rpc_session. + // So negotiation keeps only a raw pointer. rpc_session *_session; }; From cc759c9ebfd9be99b500ec0561989409254cf8d4 Mon Sep 17 00:00:00 2001 From: levy Date: Tue, 4 Aug 2020 11:29:22 +0800 Subject: [PATCH 15/15] format --- src/runtime/rpc/network.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/rpc/network.cpp b/src/runtime/rpc/network.cpp index 284f3c30f9..b24760f75a 100644 --- a/src/runtime/rpc/network.cpp +++ b/src/runtime/rpc/network.cpp @@ -72,7 +72,7 @@ void rpc_session::set_connected() { utils::auto_lock l(_lock); dassert((_connect_state == SS_NEGOTIATING && security::FLAGS_enable_auth) || - (_connect_state == SS_CONNECTING && !security::FLAGS_enable_auth), + (_connect_state == SS_CONNECTING && !security::FLAGS_enable_auth), "wrong session state"); _connect_state = SS_CONNECTED; }