From 76f807055aafd9c8214f13609f5b42b022b2e97d Mon Sep 17 00:00:00 2001 From: levy Date: Thu, 27 Aug 2020 17:21:36 +0800 Subject: [PATCH 1/3] refactor(security): close the connection when something is wrong with server_negotiation --- src/runtime/security/client_negotiation.cpp | 6 ------ src/runtime/security/client_negotiation.h | 1 - src/runtime/security/negotiation.cpp | 5 +++++ src/runtime/security/negotiation.h | 1 + src/runtime/security/server_negotiation.cpp | 17 +++++------------ src/runtime/security/server_negotiation.h | 1 - 6 files changed, 11 insertions(+), 20 deletions(-) diff --git a/src/runtime/security/client_negotiation.cpp b/src/runtime/security/client_negotiation.cpp index 9326fa8562..06b1f1adfa 100644 --- a/src/runtime/security/client_negotiation.cpp +++ b/src/runtime/security/client_negotiation.cpp @@ -129,12 +129,6 @@ void client_negotiation::send(std::unique_ptr request) }); } -void client_negotiation::fail_negotiation() -{ - _status = negotiation_status::type::SASL_AUTH_FAIL; - _session->on_failure(true); -} - void client_negotiation::succ_negotiation() { _status = negotiation_status::type::SASL_SUCC; diff --git a/src/runtime/security/client_negotiation.h b/src/runtime/security/client_negotiation.h index 8366931e4b..d3f150c845 100644 --- a/src/runtime/security/client_negotiation.h +++ b/src/runtime/security/client_negotiation.h @@ -35,7 +35,6 @@ class client_negotiation : public negotiation void recv_mechanisms(const negotiation_response &resp); void select_mechanism(const std::string &mechanism); void send(std::unique_ptr request); - void fail_negotiation(); void succ_negotiation(); }; diff --git a/src/runtime/security/negotiation.cpp b/src/runtime/security/negotiation.cpp index d9f59b1074..65d13facd1 100644 --- a/src/runtime/security/negotiation.cpp +++ b/src/runtime/security/negotiation.cpp @@ -42,5 +42,10 @@ std::unique_ptr create_negotiation(bool is_client, rpc_session *ses } } +void negotiation::fail_negotiation() { + _status = negotiation_status::type::SASL_AUTH_FAIL; + _session->on_failure(true); +} + } // namespace security } // namespace dsn diff --git a/src/runtime/security/negotiation.h b/src/runtime/security/negotiation.h index 74a72d149e..4caf690e15 100644 --- a/src/runtime/security/negotiation.h +++ b/src/runtime/security/negotiation.h @@ -39,6 +39,7 @@ class negotiation virtual void start() = 0; bool negotiation_succeed() const { return _status == negotiation_status::type::SASL_SUCC; } + void fail_negotiation(); protected: // The ownership of the negotiation instance is held by rpc_session. diff --git a/src/runtime/security/server_negotiation.cpp b/src/runtime/security/server_negotiation.cpp index 3268386bbf..dd2b81b9a8 100644 --- a/src/runtime/security/server_negotiation.cpp +++ b/src/runtime/security/server_negotiation.cpp @@ -50,7 +50,7 @@ void server_negotiation::handle_request(negotiation_rpc rpc) // TBD(zlw) break; default: - fail_negotiation(rpc, "wrong status"); + fail_negotiation(); } } @@ -66,7 +66,7 @@ void server_negotiation::on_list_mechanisms(negotiation_rpc rpc) _name, enum_to_string(rpc.request().status), enum_to_string(negotiation_status::type::SASL_LIST_MECHANISMS)); - fail_negotiation(rpc, "invalid_client_message_status"); + fail_negotiation(); } return; } @@ -80,7 +80,7 @@ void server_negotiation::on_select_mechanism(negotiation_rpc rpc) std::string error_msg = fmt::format("the mechanism of {} is not supported", _selected_mechanism); dwarn_f("{}", error_msg); - fail_negotiation(rpc, error_msg); + fail_negotiation(); return; } @@ -90,7 +90,7 @@ void server_negotiation::on_select_mechanism(negotiation_rpc rpc) _name, err_s.code().to_string(), err_s.description()); - fail_negotiation(rpc, err_s.description()); + fail_negotiation(); return; } @@ -101,7 +101,7 @@ void server_negotiation::on_select_mechanism(negotiation_rpc rpc) _name, enum_to_string(request.status), negotiation_status::type::SASL_SELECT_MECHANISMS); - fail_negotiation(rpc, "invalid_client_message_status"); + fail_negotiation(); return; } } @@ -112,12 +112,5 @@ error_s server_negotiation::do_sasl_server_init() return error_s::make(ERR_OK); } -void server_negotiation::fail_negotiation(negotiation_rpc rpc, const std::string &reason) -{ - negotiation_response &response = rpc.response(); - _status = response.status = negotiation_status::type::SASL_AUTH_FAIL; - response.msg = reason; -} - } // namespace security } // namespace dsn diff --git a/src/runtime/security/server_negotiation.h b/src/runtime/security/server_negotiation.h index 928bb22c34..f3fd0e5350 100644 --- a/src/runtime/security/server_negotiation.h +++ b/src/runtime/security/server_negotiation.h @@ -37,7 +37,6 @@ class server_negotiation : public negotiation void on_list_mechanisms(negotiation_rpc rpc); void on_select_mechanism(negotiation_rpc rpc); error_s do_sasl_server_init(); - void fail_negotiation(negotiation_rpc rpc, const std::string &reason); }; } // namespace security From d86e76be6f1527ecd036e4ae679b8665653d1327 Mon Sep 17 00:00:00 2001 From: levy Date: Thu, 27 Aug 2020 17:23:32 +0800 Subject: [PATCH 2/3] fix --- src/runtime/security/negotiation.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/runtime/security/negotiation.cpp b/src/runtime/security/negotiation.cpp index 65d13facd1..e1c73e3fc5 100644 --- a/src/runtime/security/negotiation.cpp +++ b/src/runtime/security/negotiation.cpp @@ -42,7 +42,8 @@ std::unique_ptr create_negotiation(bool is_client, rpc_session *ses } } -void negotiation::fail_negotiation() { +void negotiation::fail_negotiation() +{ _status = negotiation_status::type::SASL_AUTH_FAIL; _session->on_failure(true); } From 5ba03f16f4dd431f1f119c75ca81843195bafb11 Mon Sep 17 00:00:00 2001 From: levy Date: Thu, 27 Aug 2020 17:41:57 +0800 Subject: [PATCH 3/3] fix --- src/runtime/security/client_negotiation.cpp | 4 ++-- src/runtime/security/client_negotiation.h | 3 ++- src/runtime/security/server_negotiation.h | 1 + 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/runtime/security/client_negotiation.cpp b/src/runtime/security/client_negotiation.cpp index 06b1f1adfa..f9f64d85de 100644 --- a/src/runtime/security/client_negotiation.cpp +++ b/src/runtime/security/client_negotiation.cpp @@ -63,7 +63,7 @@ void client_negotiation::handle_response(error_code err, const negotiation_respo switch (_status) { case negotiation_status::type::SASL_LIST_MECHANISMS: - recv_mechanisms(response); + on_recv_mechanisms(response); break; case negotiation_status::type::SASL_SELECT_MECHANISMS: // TBD(zlw) @@ -77,7 +77,7 @@ void client_negotiation::handle_response(error_code err, const negotiation_respo } } -void client_negotiation::recv_mechanisms(const negotiation_response &resp) +void client_negotiation::on_recv_mechanisms(const negotiation_response &resp) { if (resp.status != negotiation_status::type::SASL_LIST_MECHANISMS_RESP) { dwarn_f("{}: get message({}) while expect({})", diff --git a/src/runtime/security/client_negotiation.h b/src/runtime/security/client_negotiation.h index d3f150c845..a45b4084d3 100644 --- a/src/runtime/security/client_negotiation.h +++ b/src/runtime/security/client_negotiation.h @@ -31,8 +31,9 @@ class client_negotiation : public negotiation private: void handle_response(error_code err, const negotiation_response &&response); + void on_recv_mechanisms(const negotiation_response &resp); + void list_mechanisms(); - void recv_mechanisms(const negotiation_response &resp); void select_mechanism(const std::string &mechanism); void send(std::unique_ptr request); void succ_negotiation(); diff --git a/src/runtime/security/server_negotiation.h b/src/runtime/security/server_negotiation.h index f3fd0e5350..330c458526 100644 --- a/src/runtime/security/server_negotiation.h +++ b/src/runtime/security/server_negotiation.h @@ -36,6 +36,7 @@ class server_negotiation : public negotiation private: void on_list_mechanisms(negotiation_rpc rpc); void on_select_mechanism(negotiation_rpc rpc); + error_s do_sasl_server_init(); };