From 4a2446ea276062f4393f54ed1bb3ae28015d95b2 Mon Sep 17 00:00:00 2001
From: levy <zlw5307@163.com>
Date: Wed, 2 Sep 2020 18:06:37 +0800
Subject: [PATCH 01/19] feat(security): implement on_mechanism_selected

---
 src/runtime/security/client_negotiation.cpp | 58 ++++++++++++++++++++-
 src/runtime/security/client_negotiation.h   |  3 ++
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/src/runtime/security/client_negotiation.cpp b/src/runtime/security/client_negotiation.cpp
index f9f64d85de..907cb58c89 100644
--- a/src/runtime/security/client_negotiation.cpp
+++ b/src/runtime/security/client_negotiation.cpp
@@ -66,7 +66,7 @@ void client_negotiation::handle_response(error_code err, const negotiation_respo
         on_recv_mechanisms(response);
         break;
     case negotiation_status::type::SASL_SELECT_MECHANISMS:
-        // TBD(zlw)
+        on_mechanism_selected(response);
         break;
     case negotiation_status::type::SASL_INITIATE:
     case negotiation_status::type::SASL_CHALLENGE_RESP:
@@ -111,6 +111,20 @@ void client_negotiation::on_recv_mechanisms(const negotiation_response &resp)
     select_mechanism(match_mechanism);
 }
 
+void client_negotiation::on_mechanism_selected(const negotiation_response &resp)
+{
+    if (resp.status == negotiation_status::type::SASL_SELECT_MECHANISMS_RESP) {
+        initiate_negotiation();
+    } else {
+        dwarn_f("{}: select mechanism({}) from server failed, type({}), reason({})",
+                _name,
+                _selected_mechanism,
+                enum_to_string(resp.status),
+                resp.msg);
+        fail_negotiation();
+    }
+}
+
 void client_negotiation::select_mechanism(const std::string &mechanism)
 {
     _selected_mechanism = mechanism;
@@ -121,6 +135,48 @@ void client_negotiation::select_mechanism(const std::string &mechanism)
     send(std::move(req));
 }
 
+void client_negotiation::initiate_negotiation()
+{
+    auto err_s = _sasl->init();
+    if (!err_s.is_ok()) {
+        dassert_f(false,
+                  "{}: initiaze sasl client failed, error = {}, reason = {}",
+                  _name,
+                  err_s.code().to_string(),
+                  err_s.description());
+        fail_negotiation();
+        return;
+    }
+
+    err_s = send_sasl_initiate_msg();
+    const std::string &desc = err_s.description();
+    if (err_s.code() == ERR_SASL_INTERNAL && desc.find("Ticket expired") != std::string::npos) {
+        derror_f("{}: start client negotiation with ticket expire, waiting on ticket renew", _name);
+        fail_negotiation();
+    } else if (!err_s.is_ok() && err_s.code() != ERR_NOT_IMPLEMENTED) {
+        dassert_f(false,
+                  "{}: client_negotiation: send sasl_client_start failed, error = {}, reason = {}",
+                  _name,
+                  err_s.code().to_string(),
+                  desc);
+        fail_negotiation();
+    }
+}
+
+error_s client_negotiation::send_sasl_initiate_msg()
+{
+    std::string resp_msg;
+    auto err = _sasl->start(_selected_mechanism, "", resp_msg);
+    if (err.is_ok() || err.code() == ERR_NOT_IMPLEMENTED) {
+        auto req = dsn::make_unique<negotiation_request>();
+        _status = req->status = negotiation_status::type::SASL_INITIATE;
+        req->msg = resp_msg;
+        send(std::move(req));
+    }
+
+    return err;
+}
+
 void client_negotiation::send(std::unique_ptr<negotiation_request> request)
 {
     negotiation_rpc rpc(std::move(request), RPC_NEGOTIATION);
diff --git a/src/runtime/security/client_negotiation.h b/src/runtime/security/client_negotiation.h
index a45b4084d3..241447b11e 100644
--- a/src/runtime/security/client_negotiation.h
+++ b/src/runtime/security/client_negotiation.h
@@ -32,9 +32,12 @@ 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 on_mechanism_selected(const negotiation_response &resp);
 
     void list_mechanisms();
     void select_mechanism(const std::string &mechanism);
+    void initiate_negotiation();
+    error_s send_sasl_initiate_msg();
     void send(std::unique_ptr<negotiation_request> request);
     void succ_negotiation();
 };

From e0292cdc04006501c6d11499a6b5036b3ea047e2 Mon Sep 17 00:00:00 2001
From: levy <zlw5307@163.com>
Date: Wed, 2 Sep 2020 18:12:38 +0800
Subject: [PATCH 02/19] fix

---
 src/runtime/security/sasl_client_wrapper.cpp | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/runtime/security/sasl_client_wrapper.cpp b/src/runtime/security/sasl_client_wrapper.cpp
index 1d584af121..792b9aa44a 100644
--- a/src/runtime/security/sasl_client_wrapper.cpp
+++ b/src/runtime/security/sasl_client_wrapper.cpp
@@ -17,6 +17,7 @@
 
 #include "sasl_client_wrapper.h"
 
+#include <sasl/sasl.h>
 #include <dsn/utility/flags.h>
 
 namespace dsn {
@@ -26,16 +27,23 @@ DSN_DECLARE_string(service_name);
 
 error_s sasl_client_wrapper::init()
 {
-    // TBD(zlw)
-    return error_s::make(ERR_OK);
+    int sasl_err = sasl_client_new(
+        FLAGS_service_name, FLAGS_service_fqdn, nullptr, nullptr, nullptr, 0, &_conn);
+    return wrap_error(sasl_err);
 }
 
 error_s sasl_client_wrapper::start(const std::string &mechanism,
                                    const std::string &input,
                                    std::string &output)
 {
-    // TBD(zlw)
-    return error_s::make(ERR_OK);
+    const char *msg = nullptr;
+    unsigned msg_len = 0;
+    const char *client_mech = nullptr;
+    int sasl_err =
+        sasl_client_start(_conn, mechanism.c_str(), nullptr, &msg, &msg_len, &client_mech);
+
+    output.assign(msg, msg_len);
+    return wrap_error(sasl_err);
 }
 
 error_s sasl_client_wrapper::step(const std::string &input, std::string &output)

From 9d05c17180d16ae28dac638ce0d1f52ece8e3c21 Mon Sep 17 00:00:00 2001
From: levy <zlw5307@163.com>
Date: Thu, 3 Sep 2020 14:56:04 +0800
Subject: [PATCH 03/19] refactor

---
 include/dsn/utility/error_code.h            |  1 +
 src/runtime/security/client_negotiation.cpp | 42 ++++++++-------------
 src/runtime/security/client_negotiation.h   |  1 -
 src/runtime/security/sasl_wrapper.cpp       |  2 +-
 4 files changed, 17 insertions(+), 29 deletions(-)

diff --git a/include/dsn/utility/error_code.h b/include/dsn/utility/error_code.h
index 61be86f868..951b48876a 100644
--- a/include/dsn/utility/error_code.h
+++ b/include/dsn/utility/error_code.h
@@ -124,4 +124,5 @@ DEFINE_ERR_CODE(ERR_UNAUTHENTICATED)
 DEFINE_ERR_CODE(ERR_KRB5_INTERNAL)
 
 DEFINE_ERR_CODE(ERR_SASL_INTERNAL)
+DEFINE_ERR_CODE(ERR_NOT_COMPLEMENTED)
 } // namespace dsn
diff --git a/src/runtime/security/client_negotiation.cpp b/src/runtime/security/client_negotiation.cpp
index 907cb58c89..7e95bab17f 100644
--- a/src/runtime/security/client_negotiation.cpp
+++ b/src/runtime/security/client_negotiation.cpp
@@ -113,16 +113,16 @@ void client_negotiation::on_recv_mechanisms(const negotiation_response &resp)
 
 void client_negotiation::on_mechanism_selected(const negotiation_response &resp)
 {
-    if (resp.status == negotiation_status::type::SASL_SELECT_MECHANISMS_RESP) {
-        initiate_negotiation();
-    } else {
-        dwarn_f("{}: select mechanism({}) from server failed, type({}), reason({})",
+    if (resp.status != negotiation_status::type::SASL_SELECT_MECHANISMS_RESP) {
+        dwarn_f("{}: get message({}) while expect({})",
                 _name,
-                _selected_mechanism,
                 enum_to_string(resp.status),
-                resp.msg);
+                enum_to_string(negotiation_status::type::SASL_SELECT_MECHANISMS_RESP));
         fail_negotiation();
+        return;
     }
+
+    initiate_negotiation();
 }
 
 void client_negotiation::select_mechanism(const std::string &mechanism)
@@ -148,35 +148,23 @@ void client_negotiation::initiate_negotiation()
         return;
     }
 
-    err_s = send_sasl_initiate_msg();
-    const std::string &desc = err_s.description();
-    if (err_s.code() == ERR_SASL_INTERNAL && desc.find("Ticket expired") != std::string::npos) {
-        derror_f("{}: start client negotiation with ticket expire, waiting on ticket renew", _name);
-        fail_negotiation();
-    } else if (!err_s.is_ok() && err_s.code() != ERR_NOT_IMPLEMENTED) {
+    std::string start_output;
+    err_s = _sasl->start(_selected_mechanism, "", start_output);
+    if (err_s.is_ok() || err_s.code() == ERR_NOT_COMPLEMENTED) {
+        auto req = dsn::make_unique<negotiation_request>();
+        _status = req->status = negotiation_status::type::SASL_INITIATE;
+        req->msg = start_output;
+        send(std::move(req));
+    } else {
         dassert_f(false,
                   "{}: client_negotiation: send sasl_client_start failed, error = {}, reason = {}",
                   _name,
                   err_s.code().to_string(),
-                  desc);
+                  err_s.description());
         fail_negotiation();
     }
 }
 
-error_s client_negotiation::send_sasl_initiate_msg()
-{
-    std::string resp_msg;
-    auto err = _sasl->start(_selected_mechanism, "", resp_msg);
-    if (err.is_ok() || err.code() == ERR_NOT_IMPLEMENTED) {
-        auto req = dsn::make_unique<negotiation_request>();
-        _status = req->status = negotiation_status::type::SASL_INITIATE;
-        req->msg = resp_msg;
-        send(std::move(req));
-    }
-
-    return err;
-}
-
 void client_negotiation::send(std::unique_ptr<negotiation_request> request)
 {
     negotiation_rpc rpc(std::move(request), RPC_NEGOTIATION);
diff --git a/src/runtime/security/client_negotiation.h b/src/runtime/security/client_negotiation.h
index 241447b11e..a7e94952df 100644
--- a/src/runtime/security/client_negotiation.h
+++ b/src/runtime/security/client_negotiation.h
@@ -37,7 +37,6 @@ class client_negotiation : public negotiation
     void list_mechanisms();
     void select_mechanism(const std::string &mechanism);
     void initiate_negotiation();
-    error_s send_sasl_initiate_msg();
     void send(std::unique_ptr<negotiation_request> request);
     void succ_negotiation();
 };
diff --git a/src/runtime/security/sasl_wrapper.cpp b/src/runtime/security/sasl_wrapper.cpp
index 3412a3df77..fc61f69ba4 100644
--- a/src/runtime/security/sasl_wrapper.cpp
+++ b/src/runtime/security/sasl_wrapper.cpp
@@ -46,7 +46,7 @@ error_s sasl_wrapper::wrap_error(int sasl_err)
     case SASL_OK:
         return error_s::make(ERR_OK);
     case SASL_CONTINUE:
-        return error_s::make(ERR_NOT_IMPLEMENTED);
+        return error_s::make(ERR_NOT_COMPLEMENTED);
     case SASL_FAIL:      // Generic failure (encompasses missing krb5 credentials).
     case SASL_BADAUTH:   // Authentication failure.
     case SASL_BADMAC:    // Decode failure.

From 0684bfb3fd7380eab3d7fccb080c6fe3ad58afbb Mon Sep 17 00:00:00 2001
From: levy <zlw5307@163.com>
Date: Thu, 3 Sep 2020 15:22:53 +0800
Subject: [PATCH 04/19] add unit test

---
 src/runtime/security/sasl_client_wrapper.cpp |  7 +++++
 src/runtime/test/client_negotiation_test.cpp | 33 ++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/src/runtime/security/sasl_client_wrapper.cpp b/src/runtime/security/sasl_client_wrapper.cpp
index 792b9aa44a..161a7fb80f 100644
--- a/src/runtime/security/sasl_client_wrapper.cpp
+++ b/src/runtime/security/sasl_client_wrapper.cpp
@@ -19,6 +19,7 @@
 
 #include <sasl/sasl.h>
 #include <dsn/utility/flags.h>
+#include <dsn/utility/fail_point.h>
 
 namespace dsn {
 namespace security {
@@ -27,6 +28,9 @@ DSN_DECLARE_string(service_name);
 
 error_s sasl_client_wrapper::init()
 {
+    FAIL_POINT_INJECT_F("sasl_client_wrapper_init",
+                        [](dsn::string_view) { return error_s::make(ERR_OK); });
+
     int sasl_err = sasl_client_new(
         FLAGS_service_name, FLAGS_service_fqdn, nullptr, nullptr, nullptr, 0, &_conn);
     return wrap_error(sasl_err);
@@ -36,6 +40,9 @@ error_s sasl_client_wrapper::start(const std::string &mechanism,
                                    const std::string &input,
                                    std::string &output)
 {
+    FAIL_POINT_INJECT_F("sasl_client_wrapper_start",
+                        [](dsn::string_view) { return error_s::make(ERR_OK); });
+
     const char *msg = nullptr;
     unsigned msg_len = 0;
     const char *client_mech = nullptr;
diff --git a/src/runtime/test/client_negotiation_test.cpp b/src/runtime/test/client_negotiation_test.cpp
index 5158c45a75..d96d23d92c 100644
--- a/src/runtime/test/client_negotiation_test.cpp
+++ b/src/runtime/test/client_negotiation_test.cpp
@@ -21,6 +21,7 @@
 
 #include <gtest/gtest.h>
 #include <dsn/utility/flags.h>
+#include <dsn/utility/fail_point.h>
 
 namespace dsn {
 namespace security {
@@ -45,6 +46,11 @@ class client_negotiation_test : public testing::Test
         _client_negotiation->handle_response(err, std::move(resp));
     }
 
+    void on_mechanism_selected(const negotiation_response &resp)
+    {
+        _client_negotiation->on_mechanism_selected(resp);
+    }
+
     const std::string &get_selected_mechanism() { return _client_negotiation->_selected_mechanism; }
 
     negotiation_status::type get_negotiation_status() { return _client_negotiation->_status; }
@@ -112,5 +118,32 @@ TEST_F(client_negotiation_test, handle_response)
         ASSERT_EQ(get_negotiation_status(), test.neg_status);
     }
 }
+
+TEST_F(client_negotiation_test, on_mechanism_selected)
+{
+    struct
+    {
+        negotiation_status::type resp_status;
+        negotiation_status::type neg_status;
+    } tests[] = {{negotiation_status::type::SASL_SELECT_MECHANISMS,
+                  negotiation_status::type::SASL_AUTH_FAIL},
+                 {negotiation_status::type::SASL_LIST_MECHANISMS_RESP,
+                  negotiation_status::type::SASL_INITIATE}};
+
+    fail::setup();
+    fail::cfg("sasl_client_wrapper_init", "return()");
+    fail::cfg("sasl_client_wrapper_start", "return()");
+    RPC_MOCKING(negotiation_rpc)
+    {
+        for (const auto &test : tests) {
+            negotiation_response resp;
+            resp.status = test.resp_status;
+            on_mechanism_selected(resp);
+
+            ASSERT_EQ(get_negotiation_status(), test.neg_status);
+        }
+    }
+    fail::teardown();
+}
 } // namespace security
 } // namespace dsn

From 9d2928359a15c9807326ae42e0d6759286c386a3 Mon Sep 17 00:00:00 2001
From: levy <zlw5307@163.com>
Date: Thu, 3 Sep 2020 15:34:37 +0800
Subject: [PATCH 05/19] fix

---
 src/runtime/security/client_negotiation.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/runtime/security/client_negotiation.cpp b/src/runtime/security/client_negotiation.cpp
index c85414ad30..e901a5172d 100644
--- a/src/runtime/security/client_negotiation.cpp
+++ b/src/runtime/security/client_negotiation.cpp
@@ -150,7 +150,7 @@ void client_negotiation::initiate_negotiation()
 
     std::string start_output;
     err_s = _sasl->start(_selected_mechanism, "", start_output);
-    if (err_s.is_ok() || err_s.code() == ERR_NOT_COMPLEMENTED) {
+    if (err_s.is_ok() || ERR_NOT_COMPLEMENTED == err_s.code()) {
         auto req = dsn::make_unique<negotiation_request>();
         _status = req->status = negotiation_status::type::SASL_INITIATE;
         req->msg = start_output;

From a169af9291b78024fe8b52b4c228d871e7a57a69 Mon Sep 17 00:00:00 2001
From: levy <zlw5307@163.com>
Date: Thu, 3 Sep 2020 15:35:58 +0800
Subject: [PATCH 06/19] fix

---
 src/runtime/security/client_negotiation.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/runtime/security/client_negotiation.cpp b/src/runtime/security/client_negotiation.cpp
index e901a5172d..00515ada39 100644
--- a/src/runtime/security/client_negotiation.cpp
+++ b/src/runtime/security/client_negotiation.cpp
@@ -157,7 +157,7 @@ void client_negotiation::initiate_negotiation()
         send(std::move(req));
     } else {
         dassert_f(false,
-                  "{}: client_negotiation: send sasl_client_start failed, error = {}, reason = {}",
+                  "{}: client_negotiation: sasl client start failed, error = {}, reason = {}",
                   _name,
                   err_s.code().to_string(),
                   err_s.description());

From 49c6d8d5e79ad4f07a3ebcaa5d56eb7d18ff0e11 Mon Sep 17 00:00:00 2001
From: levy <zlw5307@163.com>
Date: Thu, 3 Sep 2020 15:37:21 +0800
Subject: [PATCH 07/19] fix

---
 src/runtime/security/client_negotiation.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/runtime/security/client_negotiation.cpp b/src/runtime/security/client_negotiation.cpp
index 00515ada39..13cbfd2b8e 100644
--- a/src/runtime/security/client_negotiation.cpp
+++ b/src/runtime/security/client_negotiation.cpp
@@ -137,6 +137,7 @@ void client_negotiation::select_mechanism(const std::string &mechanism)
 
 void client_negotiation::initiate_negotiation()
 {
+    // init client sasl
     auto err_s = _sasl->init();
     if (!err_s.is_ok()) {
         dassert_f(false,
@@ -148,6 +149,7 @@ void client_negotiation::initiate_negotiation()
         return;
     }
 
+    // start client sasl, and send `SASL_INITIATE` to `server_negotiation` if it returns ok
     std::string start_output;
     err_s = _sasl->start(_selected_mechanism, "", start_output);
     if (err_s.is_ok() || ERR_NOT_COMPLEMENTED == err_s.code()) {

From 098d261fdb392895cffe2502c7cf63a67348a38a Mon Sep 17 00:00:00 2001
From: levy <zlw5307@163.com>
Date: Thu, 3 Sep 2020 16:09:48 +0800
Subject: [PATCH 08/19] fix

---
 src/runtime/security/client_negotiation.cpp | 39 +++++---------
 src/runtime/security/client_negotiation.h   |  1 -
 src/runtime/security/negotiation.cpp        | 15 ++++++
 src/runtime/security/negotiation.h          |  5 ++
 src/runtime/security/server_negotiation.cpp | 58 +++++++++------------
 5 files changed, 58 insertions(+), 60 deletions(-)

diff --git a/src/runtime/security/client_negotiation.cpp b/src/runtime/security/client_negotiation.cpp
index 13cbfd2b8e..687e597d02 100644
--- a/src/runtime/security/client_negotiation.cpp
+++ b/src/runtime/security/client_negotiation.cpp
@@ -79,11 +79,7 @@ void client_negotiation::handle_response(error_code err, const negotiation_respo
 
 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({})",
-                _name,
-                enum_to_string(resp.status),
-                enum_to_string(negotiation_status::type::SASL_LIST_MECHANISMS_RESP));
+    if (!check_status(resp.status, negotiation_status::type::SASL_LIST_MECHANISMS_RESP)) {
         fail_negotiation();
         return;
     }
@@ -113,30 +109,11 @@ void client_negotiation::on_recv_mechanisms(const negotiation_response &resp)
 
 void client_negotiation::on_mechanism_selected(const negotiation_response &resp)
 {
-    if (resp.status != negotiation_status::type::SASL_SELECT_MECHANISMS_RESP) {
-        dwarn_f("{}: get message({}) while expect({})",
-                _name,
-                enum_to_string(resp.status),
-                enum_to_string(negotiation_status::type::SASL_SELECT_MECHANISMS_RESP));
+    if (!check_status(resp.status, negotiation_status::type::SASL_SELECT_MECHANISMS_RESP)) {
         fail_negotiation();
         return;
     }
 
-    initiate_negotiation();
-}
-
-void client_negotiation::select_mechanism(const std::string &mechanism)
-{
-    _selected_mechanism = mechanism;
-
-    auto req = dsn::make_unique<negotiation_request>();
-    _status = req->status = negotiation_status::type::SASL_SELECT_MECHANISMS;
-    req->msg = mechanism;
-    send(std::move(req));
-}
-
-void client_negotiation::initiate_negotiation()
-{
     // init client sasl
     auto err_s = _sasl->init();
     if (!err_s.is_ok()) {
@@ -149,7 +126,7 @@ void client_negotiation::initiate_negotiation()
         return;
     }
 
-    // start client sasl, and send `SASL_INITIATE` to `server_negotiation` if it returns ok
+    // start client sasl, and send `SASL_INITIATE` to `server_negotiation` if everything is ok
     std::string start_output;
     err_s = _sasl->start(_selected_mechanism, "", start_output);
     if (err_s.is_ok() || ERR_NOT_COMPLEMENTED == err_s.code()) {
@@ -167,6 +144,16 @@ void client_negotiation::initiate_negotiation()
     }
 }
 
+void client_negotiation::select_mechanism(const std::string &mechanism)
+{
+    _selected_mechanism = mechanism;
+
+    auto req = dsn::make_unique<negotiation_request>();
+    _status = req->status = negotiation_status::type::SASL_SELECT_MECHANISMS;
+    req->msg = mechanism;
+    send(std::move(req));
+}
+
 void client_negotiation::send(std::unique_ptr<negotiation_request> request)
 {
     negotiation_rpc rpc(std::move(request), RPC_NEGOTIATION);
diff --git a/src/runtime/security/client_negotiation.h b/src/runtime/security/client_negotiation.h
index 94df96b9bf..8a4dd7f38e 100644
--- a/src/runtime/security/client_negotiation.h
+++ b/src/runtime/security/client_negotiation.h
@@ -36,7 +36,6 @@ class client_negotiation : public negotiation
 
     void list_mechanisms();
     void select_mechanism(const std::string &mechanism);
-    void initiate_negotiation();
     void send(std::unique_ptr<negotiation_request> request);
     void succ_negotiation();
 
diff --git a/src/runtime/security/negotiation.cpp b/src/runtime/security/negotiation.cpp
index e1c73e3fc5..fc7148f3f7 100644
--- a/src/runtime/security/negotiation.cpp
+++ b/src/runtime/security/negotiation.cpp
@@ -18,9 +18,11 @@
 #include "negotiation.h"
 #include "client_negotiation.h"
 #include "server_negotiation.h"
+#include "negotiation_utils.h"
 
 #include <dsn/utility/flags.h>
 #include <dsn/utility/smart_pointers.h>
+#include <dsn/dist/fmt_logging.h>
 
 namespace dsn {
 namespace security {
@@ -48,5 +50,18 @@ void negotiation::fail_negotiation()
     _session->on_failure(true);
 }
 
+bool negotiation::check_status(negotiation_status::type status,
+                               negotiation_status::type expect_status)
+{
+    if (status != expect_status) {
+        dwarn_f("{}: get message({}) while expect({})",
+                _name,
+                enum_to_string(status),
+                enum_to_string(expect_status));
+        return false;
+    }
+
+    return true;
+}
 } // namespace security
 } // namespace dsn
diff --git a/src/runtime/security/negotiation.h b/src/runtime/security/negotiation.h
index 20bac462c2..c77b2d93b4 100644
--- a/src/runtime/security/negotiation.h
+++ b/src/runtime/security/negotiation.h
@@ -42,6 +42,11 @@ class negotiation
     virtual void start() = 0;
     bool negotiation_succeed() const { return _status == negotiation_status::type::SASL_SUCC; }
     void fail_negotiation();
+    // check whether the status is equal to expect_status
+    // ret value:
+    //   true:  status == expect_status
+    //   false: status != expect_status
+    bool check_status(negotiation_status::type status, negotiation_status::type expect_status);
 
 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 130a538d53..890cf79224 100644
--- a/src/runtime/security/server_negotiation.cpp
+++ b/src/runtime/security/server_negotiation.cpp
@@ -60,52 +60,44 @@ void server_negotiation::handle_request(negotiation_rpc rpc)
 
 void server_negotiation::on_list_mechanisms(negotiation_rpc rpc)
 {
-    if (rpc.request().status == negotiation_status::type::SASL_LIST_MECHANISMS) {
-        std::string mech_list = boost::join(supported_mechanisms, ",");
-        negotiation_response &response = rpc.response();
-        _status = response.status = negotiation_status::type::SASL_LIST_MECHANISMS_RESP;
-        response.msg = std::move(mech_list);
-    } else {
-        ddebug_f("{}: got message({}) while expect({})",
-                 _name,
-                 enum_to_string(rpc.request().status),
-                 enum_to_string(negotiation_status::type::SASL_LIST_MECHANISMS));
+    if (!check_status(rpc.request().status, negotiation_status::type::SASL_LIST_MECHANISMS)) {
         fail_negotiation();
+        return;
     }
-    return;
+
+    std::string mech_list = boost::join(supported_mechanisms, ",");
+    negotiation_response &response = rpc.response();
+    _status = response.status = negotiation_status::type::SASL_LIST_MECHANISMS_RESP;
+    response.msg = std::move(mech_list);
 }
 
 void server_negotiation::on_select_mechanism(negotiation_rpc rpc)
 {
     const negotiation_request &request = rpc.request();
-    if (request.status == negotiation_status::type::SASL_SELECT_MECHANISMS) {
-        _selected_mechanism = request.msg;
-        if (supported_mechanisms.find(_selected_mechanism) == supported_mechanisms.end()) {
-            dwarn_f("the mechanism of {} is not supported", _selected_mechanism);
-            fail_negotiation();
-            return;
-        }
+    if (!check_status(rpc.request().status, negotiation_status::type::SASL_SELECT_MECHANISMS)) {
+        fail_negotiation();
+        return;
+    }
 
-        error_s err_s = _sasl->init();
-        if (!err_s.is_ok()) {
-            dwarn_f("{}: server initialize sasl failed, error = {}, msg = {}",
-                    _name,
-                    err_s.code().to_string(),
-                    err_s.description());
-            fail_negotiation();
-            return;
-        }
+    _selected_mechanism = request.msg;
+    if (supported_mechanisms.find(_selected_mechanism) == supported_mechanisms.end()) {
+        dwarn_f("the mechanism of {} is not supported", _selected_mechanism);
+        fail_negotiation();
+        return;
+    }
 
-        negotiation_response &response = rpc.response();
-        _status = response.status = negotiation_status::type::SASL_SELECT_MECHANISMS_RESP;
-    } else {
-        dwarn_f("{}: got message({}) while expect({})",
+    error_s err_s = _sasl->init();
+    if (!err_s.is_ok()) {
+        dwarn_f("{}: server initialize sasl failed, error = {}, msg = {}",
                 _name,
-                enum_to_string(request.status),
-                negotiation_status::type::SASL_SELECT_MECHANISMS);
+                err_s.code().to_string(),
+                err_s.description());
         fail_negotiation();
         return;
     }
+
+    negotiation_response &response = rpc.response();
+    _status = response.status = negotiation_status::type::SASL_SELECT_MECHANISMS_RESP;
 }
 } // namespace security
 } // namespace dsn

From f8d1ec95fe87e299abafd0f5c1435a19e0e747d5 Mon Sep 17 00:00:00 2001
From: levy <zlw5307@163.com>
Date: Thu, 3 Sep 2020 16:53:59 +0800
Subject: [PATCH 09/19] fix

---
 src/runtime/security/client_negotiation.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/runtime/security/client_negotiation.cpp b/src/runtime/security/client_negotiation.cpp
index 687e597d02..ea274c7b7f 100644
--- a/src/runtime/security/client_negotiation.cpp
+++ b/src/runtime/security/client_negotiation.cpp
@@ -136,7 +136,7 @@ void client_negotiation::on_mechanism_selected(const negotiation_response &resp)
         send(std::move(req));
     } else {
         dassert_f(false,
-                  "{}: client_negotiation: sasl client start failed, error = {}, reason = {}",
+                  "{}: start sasl client failed, error = {}, reason = {}",
                   _name,
                   err_s.code().to_string(),
                   err_s.description());

From 071c8ef5a6c482bd52632f0db22e11dc90ceb6bc Mon Sep 17 00:00:00 2001
From: levy <zlw5307@163.com>
Date: Thu, 3 Sep 2020 16:58:27 +0800
Subject: [PATCH 10/19] fix

---
 src/runtime/test/client_negotiation_test.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/runtime/test/client_negotiation_test.cpp b/src/runtime/test/client_negotiation_test.cpp
index d96d23d92c..80e2c62061 100644
--- a/src/runtime/test/client_negotiation_test.cpp
+++ b/src/runtime/test/client_negotiation_test.cpp
@@ -127,7 +127,7 @@ TEST_F(client_negotiation_test, on_mechanism_selected)
         negotiation_status::type neg_status;
     } tests[] = {{negotiation_status::type::SASL_SELECT_MECHANISMS,
                   negotiation_status::type::SASL_AUTH_FAIL},
-                 {negotiation_status::type::SASL_LIST_MECHANISMS_RESP,
+                 {negotiation_status::type::SASL_SELECT_MECHANISMS_RESP,
                   negotiation_status::type::SASL_INITIATE}};
 
     fail::setup();

From cfa6b2c8c09d3bc4f2b261eafbfe4187383779f6 Mon Sep 17 00:00:00 2001
From: levy <zlw5307@163.com>
Date: Thu, 3 Sep 2020 17:43:45 +0800
Subject: [PATCH 11/19] fix

---
 src/runtime/security/client_negotiation.cpp | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/src/runtime/security/client_negotiation.cpp b/src/runtime/security/client_negotiation.cpp
index ea274c7b7f..ad3a004055 100644
--- a/src/runtime/security/client_negotiation.cpp
+++ b/src/runtime/security/client_negotiation.cpp
@@ -117,11 +117,10 @@ void client_negotiation::on_mechanism_selected(const negotiation_response &resp)
     // init client sasl
     auto err_s = _sasl->init();
     if (!err_s.is_ok()) {
-        dassert_f(false,
-                  "{}: initiaze sasl client failed, error = {}, reason = {}",
-                  _name,
-                  err_s.code().to_string(),
-                  err_s.description());
+        dwarn_f("{}: initiaze sasl client failed, error = {}, reason = {}",
+                _name,
+                err_s.code().to_string(),
+                err_s.description());
         fail_negotiation();
         return;
     }
@@ -135,11 +134,10 @@ void client_negotiation::on_mechanism_selected(const negotiation_response &resp)
         req->msg = start_output;
         send(std::move(req));
     } else {
-        dassert_f(false,
-                  "{}: start sasl client failed, error = {}, reason = {}",
-                  _name,
-                  err_s.code().to_string(),
-                  err_s.description());
+        dwarn_f("{}: start sasl client failed, error = {}, reason = {}",
+                _name,
+                err_s.code().to_string(),
+                err_s.description());
         fail_negotiation();
     }
 }

From 873cf950c61b9e6791d9e45f38268b442c3069df Mon Sep 17 00:00:00 2001
From: levy <zlw5307@163.com>
Date: Thu, 3 Sep 2020 18:07:21 +0800
Subject: [PATCH 12/19] add unit tests

---
 src/runtime/security/sasl_client_wrapper.cpp | 18 +++++++---
 src/runtime/test/client_negotiation_test.cpp | 35 +++++++++++++++-----
 2 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/src/runtime/security/sasl_client_wrapper.cpp b/src/runtime/security/sasl_client_wrapper.cpp
index 161a7fb80f..bde255d793 100644
--- a/src/runtime/security/sasl_client_wrapper.cpp
+++ b/src/runtime/security/sasl_client_wrapper.cpp
@@ -28,8 +28,12 @@ DSN_DECLARE_string(service_name);
 
 error_s sasl_client_wrapper::init()
 {
-    FAIL_POINT_INJECT_F("sasl_client_wrapper_init",
-                        [](dsn::string_view) { return error_s::make(ERR_OK); });
+    FAIL_POINT_INJECT_F("sasl_client_wrapper_init", [](dsn::string_view str) {
+        if (0 == str.compare("ERR_OK")) {
+            return error_s::make(ERR_OK);
+        }
+        return error_s::make(ERR_UNKNOWN);
+    });
 
     int sasl_err = sasl_client_new(
         FLAGS_service_name, FLAGS_service_fqdn, nullptr, nullptr, nullptr, 0, &_conn);
@@ -40,8 +44,14 @@ error_s sasl_client_wrapper::start(const std::string &mechanism,
                                    const std::string &input,
                                    std::string &output)
 {
-    FAIL_POINT_INJECT_F("sasl_client_wrapper_start",
-                        [](dsn::string_view) { return error_s::make(ERR_OK); });
+    FAIL_POINT_INJECT_F("sasl_client_wrapper_start", [](dsn::string_view str) {
+        if (0 == str.compare("ERR_OK")) {
+            return error_s::make(ERR_OK);
+        } else if (0 == str.compare("ERR_NOT_COMPLEMENTED")) {
+            return error_s::make(ERR_NOT_COMPLEMENTED);
+        }
+        return error_s::make(ERR_UNKNOWN);
+    });
 
     const char *msg = nullptr;
     unsigned msg_len = 0;
diff --git a/src/runtime/test/client_negotiation_test.cpp b/src/runtime/test/client_negotiation_test.cpp
index 80e2c62061..d82a0ba38d 100644
--- a/src/runtime/test/client_negotiation_test.cpp
+++ b/src/runtime/test/client_negotiation_test.cpp
@@ -123,27 +123,46 @@ TEST_F(client_negotiation_test, on_mechanism_selected)
 {
     struct
     {
+        std::string sasl_init_return_value;
+        std::string sasl_start_return_value;
         negotiation_status::type resp_status;
         negotiation_status::type neg_status;
-    } tests[] = {{negotiation_status::type::SASL_SELECT_MECHANISMS,
+    } tests[] = {{"ERR_OK",
+                  "ERR_OK",
+                  negotiation_status::type::SASL_SELECT_MECHANISMS_RESP,
+                  negotiation_status::type::SASL_INITIATE},
+                 {"ERR_OK",
+                  "ERR_NOT_COMPLEMENTED",
+                  negotiation_status::type::SASL_SELECT_MECHANISMS_RESP,
+                  negotiation_status::type::SASL_INITIATE},
+                 {"ERR_OK",
+                  "ERR_TIMEOUT",
+                  negotiation_status::type::SASL_SELECT_MECHANISMS_RESP,
                   negotiation_status::type::SASL_AUTH_FAIL},
-                 {negotiation_status::type::SASL_SELECT_MECHANISMS_RESP,
-                  negotiation_status::type::SASL_INITIATE}};
+                 {"ERR_TIMEOUT",
+                  "ERR_OK",
+                  negotiation_status::type::SASL_SELECT_MECHANISMS_RESP,
+                  negotiation_status::type::SASL_AUTH_FAIL},
+                 {"ERR_OK",
+                  "ERR_OK",
+                  negotiation_status::type::SASL_SELECT_MECHANISMS,
+                  negotiation_status::type::SASL_AUTH_FAIL}};
 
-    fail::setup();
-    fail::cfg("sasl_client_wrapper_init", "return()");
-    fail::cfg("sasl_client_wrapper_start", "return()");
     RPC_MOCKING(negotiation_rpc)
     {
         for (const auto &test : tests) {
+            fail::setup();
+            fail::cfg("sasl_client_wrapper_init", "return(" + test.sasl_init_return_value + ")");
+            fail::cfg("sasl_client_wrapper_start", "return(" + test.sasl_start_return_value + ")");
+
             negotiation_response resp;
             resp.status = test.resp_status;
             on_mechanism_selected(resp);
-
             ASSERT_EQ(get_negotiation_status(), test.neg_status);
+
+            fail::teardown();
         }
     }
-    fail::teardown();
 }
 } // namespace security
 } // namespace dsn

From dff761f297740e2b902f31b0aee9efccd713a0c6 Mon Sep 17 00:00:00 2001
From: levy <zlw5307@163.com>
Date: Fri, 4 Sep 2020 10:18:04 +0800
Subject: [PATCH 13/19] fix by review

---
 src/runtime/security/sasl_client_wrapper.cpp | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/src/runtime/security/sasl_client_wrapper.cpp b/src/runtime/security/sasl_client_wrapper.cpp
index bde255d793..07871c6828 100644
--- a/src/runtime/security/sasl_client_wrapper.cpp
+++ b/src/runtime/security/sasl_client_wrapper.cpp
@@ -29,10 +29,8 @@ DSN_DECLARE_string(service_name);
 error_s sasl_client_wrapper::init()
 {
     FAIL_POINT_INJECT_F("sasl_client_wrapper_init", [](dsn::string_view str) {
-        if (0 == str.compare("ERR_OK")) {
-            return error_s::make(ERR_OK);
-        }
-        return error_s::make(ERR_UNKNOWN);
+        error_code err = error_code::try_get(str.data(), ERR_UNKNOWN);
+        return error_s::make(err);
     });
 
     int sasl_err = sasl_client_new(
@@ -45,12 +43,8 @@ error_s sasl_client_wrapper::start(const std::string &mechanism,
                                    std::string &output)
 {
     FAIL_POINT_INJECT_F("sasl_client_wrapper_start", [](dsn::string_view str) {
-        if (0 == str.compare("ERR_OK")) {
-            return error_s::make(ERR_OK);
-        } else if (0 == str.compare("ERR_NOT_COMPLEMENTED")) {
-            return error_s::make(ERR_NOT_COMPLEMENTED);
-        }
-        return error_s::make(ERR_UNKNOWN);
+        error_code err = error_code::try_get(str.data(), ERR_UNKNOWN);
+        return error_s::make(err);
     });
 
     const char *msg = nullptr;

From a331e4dc979ec582ce7d2ed93d40936ba1a45719 Mon Sep 17 00:00:00 2001
From: levy <zlw5307@163.com>
Date: Fri, 4 Sep 2020 11:03:26 +0800
Subject: [PATCH 14/19] refactor by review

---
 include/dsn/utility/error_code.h             | 2 +-
 src/runtime/security/client_negotiation.cpp  | 4 ++--
 src/runtime/security/sasl_wrapper.cpp        | 2 +-
 src/runtime/test/client_negotiation_test.cpp | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/dsn/utility/error_code.h b/include/dsn/utility/error_code.h
index 951b48876a..361abe8187 100644
--- a/include/dsn/utility/error_code.h
+++ b/include/dsn/utility/error_code.h
@@ -124,5 +124,5 @@ DEFINE_ERR_CODE(ERR_UNAUTHENTICATED)
 DEFINE_ERR_CODE(ERR_KRB5_INTERNAL)
 
 DEFINE_ERR_CODE(ERR_SASL_INTERNAL)
-DEFINE_ERR_CODE(ERR_NOT_COMPLEMENTED)
+DEFINE_ERR_CODE(ERR_SASL_INCOMPLEMENT)
 } // namespace dsn
diff --git a/src/runtime/security/client_negotiation.cpp b/src/runtime/security/client_negotiation.cpp
index ad3a004055..d31c7f95b2 100644
--- a/src/runtime/security/client_negotiation.cpp
+++ b/src/runtime/security/client_negotiation.cpp
@@ -117,7 +117,7 @@ void client_negotiation::on_mechanism_selected(const negotiation_response &resp)
     // init client sasl
     auto err_s = _sasl->init();
     if (!err_s.is_ok()) {
-        dwarn_f("{}: initiaze sasl client failed, error = {}, reason = {}",
+        dwarn_f("{}: initialize sasl client failed, error = {}, reason = {}",
                 _name,
                 err_s.code().to_string(),
                 err_s.description());
@@ -128,7 +128,7 @@ void client_negotiation::on_mechanism_selected(const negotiation_response &resp)
     // start client sasl, and send `SASL_INITIATE` to `server_negotiation` if everything is ok
     std::string start_output;
     err_s = _sasl->start(_selected_mechanism, "", start_output);
-    if (err_s.is_ok() || ERR_NOT_COMPLEMENTED == err_s.code()) {
+    if (err_s.is_ok() || ERR_SASL_INCOMPLEMENT == err_s.code()) {
         auto req = dsn::make_unique<negotiation_request>();
         _status = req->status = negotiation_status::type::SASL_INITIATE;
         req->msg = start_output;
diff --git a/src/runtime/security/sasl_wrapper.cpp b/src/runtime/security/sasl_wrapper.cpp
index fc61f69ba4..bfcdf93100 100644
--- a/src/runtime/security/sasl_wrapper.cpp
+++ b/src/runtime/security/sasl_wrapper.cpp
@@ -46,7 +46,7 @@ error_s sasl_wrapper::wrap_error(int sasl_err)
     case SASL_OK:
         return error_s::make(ERR_OK);
     case SASL_CONTINUE:
-        return error_s::make(ERR_NOT_COMPLEMENTED);
+        return error_s::make(ERR_SASL_INCOMPLEMENT);
     case SASL_FAIL:      // Generic failure (encompasses missing krb5 credentials).
     case SASL_BADAUTH:   // Authentication failure.
     case SASL_BADMAC:    // Decode failure.
diff --git a/src/runtime/test/client_negotiation_test.cpp b/src/runtime/test/client_negotiation_test.cpp
index d82a0ba38d..e18662488d 100644
--- a/src/runtime/test/client_negotiation_test.cpp
+++ b/src/runtime/test/client_negotiation_test.cpp
@@ -132,7 +132,7 @@ TEST_F(client_negotiation_test, on_mechanism_selected)
                   negotiation_status::type::SASL_SELECT_MECHANISMS_RESP,
                   negotiation_status::type::SASL_INITIATE},
                  {"ERR_OK",
-                  "ERR_NOT_COMPLEMENTED",
+                  "ERR_SASL_INCOMPLEMENT",
                   negotiation_status::type::SASL_SELECT_MECHANISMS_RESP,
                   negotiation_status::type::SASL_INITIATE},
                  {"ERR_OK",

From ca6f21808ccbfa7d63c1fec75003be2583eb5dd8 Mon Sep 17 00:00:00 2001
From: levy <zlw5307@163.com>
Date: Fri, 4 Sep 2020 12:10:49 +0800
Subject: [PATCH 15/19] refactor

---
 include/dsn/utility/error_code.h             | 2 +-
 src/runtime/security/client_negotiation.cpp  | 2 +-
 src/runtime/security/sasl_wrapper.cpp        | 2 +-
 src/runtime/test/client_negotiation_test.cpp | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/dsn/utility/error_code.h b/include/dsn/utility/error_code.h
index 361abe8187..ee0703643e 100644
--- a/include/dsn/utility/error_code.h
+++ b/include/dsn/utility/error_code.h
@@ -124,5 +124,5 @@ DEFINE_ERR_CODE(ERR_UNAUTHENTICATED)
 DEFINE_ERR_CODE(ERR_KRB5_INTERNAL)
 
 DEFINE_ERR_CODE(ERR_SASL_INTERNAL)
-DEFINE_ERR_CODE(ERR_SASL_INCOMPLEMENT)
+DEFINE_ERR_CODE(ERR_SASL_INCOMPLETE)
 } // namespace dsn
diff --git a/src/runtime/security/client_negotiation.cpp b/src/runtime/security/client_negotiation.cpp
index d31c7f95b2..306273569c 100644
--- a/src/runtime/security/client_negotiation.cpp
+++ b/src/runtime/security/client_negotiation.cpp
@@ -128,7 +128,7 @@ void client_negotiation::on_mechanism_selected(const negotiation_response &resp)
     // start client sasl, and send `SASL_INITIATE` to `server_negotiation` if everything is ok
     std::string start_output;
     err_s = _sasl->start(_selected_mechanism, "", start_output);
-    if (err_s.is_ok() || ERR_SASL_INCOMPLEMENT == err_s.code()) {
+    if (err_s.is_ok() || ERR_SASL_INCOMPLETE == err_s.code()) {
         auto req = dsn::make_unique<negotiation_request>();
         _status = req->status = negotiation_status::type::SASL_INITIATE;
         req->msg = start_output;
diff --git a/src/runtime/security/sasl_wrapper.cpp b/src/runtime/security/sasl_wrapper.cpp
index bfcdf93100..91cc05079d 100644
--- a/src/runtime/security/sasl_wrapper.cpp
+++ b/src/runtime/security/sasl_wrapper.cpp
@@ -46,7 +46,7 @@ error_s sasl_wrapper::wrap_error(int sasl_err)
     case SASL_OK:
         return error_s::make(ERR_OK);
     case SASL_CONTINUE:
-        return error_s::make(ERR_SASL_INCOMPLEMENT);
+        return error_s::make(ERR_SASL_INCOMPLETE);
     case SASL_FAIL:      // Generic failure (encompasses missing krb5 credentials).
     case SASL_BADAUTH:   // Authentication failure.
     case SASL_BADMAC:    // Decode failure.
diff --git a/src/runtime/test/client_negotiation_test.cpp b/src/runtime/test/client_negotiation_test.cpp
index e18662488d..e828ae70c4 100644
--- a/src/runtime/test/client_negotiation_test.cpp
+++ b/src/runtime/test/client_negotiation_test.cpp
@@ -132,7 +132,7 @@ TEST_F(client_negotiation_test, on_mechanism_selected)
                   negotiation_status::type::SASL_SELECT_MECHANISMS_RESP,
                   negotiation_status::type::SASL_INITIATE},
                  {"ERR_OK",
-                  "ERR_SASL_INCOMPLEMENT",
+                  "ERR_SASL_INCOMPLETE",
                   negotiation_status::type::SASL_SELECT_MECHANISMS_RESP,
                   negotiation_status::type::SASL_INITIATE},
                  {"ERR_OK",

From b8948262addbe327d006bc3c617810b35d89f6b0 Mon Sep 17 00:00:00 2001
From: levy <zlw5307@163.com>
Date: Fri, 4 Sep 2020 12:14:39 +0800
Subject: [PATCH 16/19] fix

---
 src/runtime/security/negotiation.cpp | 6 +++---
 src/runtime/security/negotiation.h   | 8 ++++----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/runtime/security/negotiation.cpp b/src/runtime/security/negotiation.cpp
index fc7148f3f7..57bc0ac3b6 100644
--- a/src/runtime/security/negotiation.cpp
+++ b/src/runtime/security/negotiation.cpp
@@ -51,13 +51,13 @@ void negotiation::fail_negotiation()
 }
 
 bool negotiation::check_status(negotiation_status::type status,
-                               negotiation_status::type expect_status)
+                               negotiation_status::type expected_status)
 {
-    if (status != expect_status) {
+    if (status != expected_status) {
         dwarn_f("{}: get message({}) while expect({})",
                 _name,
                 enum_to_string(status),
-                enum_to_string(expect_status));
+                enum_to_string(expected_status));
         return false;
     }
 
diff --git a/src/runtime/security/negotiation.h b/src/runtime/security/negotiation.h
index c77b2d93b4..9b31b7626b 100644
--- a/src/runtime/security/negotiation.h
+++ b/src/runtime/security/negotiation.h
@@ -42,11 +42,11 @@ class negotiation
     virtual void start() = 0;
     bool negotiation_succeed() const { return _status == negotiation_status::type::SASL_SUCC; }
     void fail_negotiation();
-    // check whether the status is equal to expect_status
+    // check whether the status is equal to expected_status
     // ret value:
-    //   true:  status == expect_status
-    //   false: status != expect_status
-    bool check_status(negotiation_status::type status, negotiation_status::type expect_status);
+    //   true:  status == expected_status
+    //   false: status != expected_status
+    bool check_status(negotiation_status::type status, negotiation_status::type expected_status);
 
 protected:
     // The ownership of the negotiation instance is held by rpc_session.

From c020e1389a8f0ee7167a6c3fca7d0f9ac136b1ce Mon Sep 17 00:00:00 2001
From: levy <zlw5307@163.com>
Date: Fri, 4 Sep 2020 12:20:02 +0800
Subject: [PATCH 17/19] fix

---
 include/dsn/utility/error_code.h             | 2 +-
 src/runtime/security/client_negotiation.cpp  | 2 +-
 src/runtime/security/sasl_wrapper.cpp        | 2 +-
 src/runtime/test/client_negotiation_test.cpp | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/dsn/utility/error_code.h b/include/dsn/utility/error_code.h
index ee0703643e..6e42a8abf3 100644
--- a/include/dsn/utility/error_code.h
+++ b/include/dsn/utility/error_code.h
@@ -124,5 +124,5 @@ DEFINE_ERR_CODE(ERR_UNAUTHENTICATED)
 DEFINE_ERR_CODE(ERR_KRB5_INTERNAL)
 
 DEFINE_ERR_CODE(ERR_SASL_INTERNAL)
-DEFINE_ERR_CODE(ERR_SASL_INCOMPLETE)
+DEFINE_ERR_CODE(ERR_SASL_INCOMPLETED)
 } // namespace dsn
diff --git a/src/runtime/security/client_negotiation.cpp b/src/runtime/security/client_negotiation.cpp
index 306273569c..3f75be7ccc 100644
--- a/src/runtime/security/client_negotiation.cpp
+++ b/src/runtime/security/client_negotiation.cpp
@@ -128,7 +128,7 @@ void client_negotiation::on_mechanism_selected(const negotiation_response &resp)
     // start client sasl, and send `SASL_INITIATE` to `server_negotiation` if everything is ok
     std::string start_output;
     err_s = _sasl->start(_selected_mechanism, "", start_output);
-    if (err_s.is_ok() || ERR_SASL_INCOMPLETE == err_s.code()) {
+    if (err_s.is_ok() || ERR_SASL_INCOMPLETED == err_s.code()) {
         auto req = dsn::make_unique<negotiation_request>();
         _status = req->status = negotiation_status::type::SASL_INITIATE;
         req->msg = start_output;
diff --git a/src/runtime/security/sasl_wrapper.cpp b/src/runtime/security/sasl_wrapper.cpp
index 91cc05079d..d06d2dcec5 100644
--- a/src/runtime/security/sasl_wrapper.cpp
+++ b/src/runtime/security/sasl_wrapper.cpp
@@ -46,7 +46,7 @@ error_s sasl_wrapper::wrap_error(int sasl_err)
     case SASL_OK:
         return error_s::make(ERR_OK);
     case SASL_CONTINUE:
-        return error_s::make(ERR_SASL_INCOMPLETE);
+        return error_s::make(ERR_SASL_INCOMPLETED);
     case SASL_FAIL:      // Generic failure (encompasses missing krb5 credentials).
     case SASL_BADAUTH:   // Authentication failure.
     case SASL_BADMAC:    // Decode failure.
diff --git a/src/runtime/test/client_negotiation_test.cpp b/src/runtime/test/client_negotiation_test.cpp
index e828ae70c4..1821a86151 100644
--- a/src/runtime/test/client_negotiation_test.cpp
+++ b/src/runtime/test/client_negotiation_test.cpp
@@ -132,7 +132,7 @@ TEST_F(client_negotiation_test, on_mechanism_selected)
                   negotiation_status::type::SASL_SELECT_MECHANISMS_RESP,
                   negotiation_status::type::SASL_INITIATE},
                  {"ERR_OK",
-                  "ERR_SASL_INCOMPLETE",
+                  "ERR_SASL_INCOMPLETED",
                   negotiation_status::type::SASL_SELECT_MECHANISMS_RESP,
                   negotiation_status::type::SASL_INITIATE},
                  {"ERR_OK",

From d592e39b45f0f0369a10148e4cb068259924e77f Mon Sep 17 00:00:00 2001
From: levy <zlw5307@163.com>
Date: Fri, 4 Sep 2020 12:21:05 +0800
Subject: [PATCH 18/19] Revert "fix"

This reverts commit c020e1389a8f0ee7167a6c3fca7d0f9ac136b1ce.
---
 include/dsn/utility/error_code.h             | 2 +-
 src/runtime/security/client_negotiation.cpp  | 2 +-
 src/runtime/security/sasl_wrapper.cpp        | 2 +-
 src/runtime/test/client_negotiation_test.cpp | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/dsn/utility/error_code.h b/include/dsn/utility/error_code.h
index 6e42a8abf3..ee0703643e 100644
--- a/include/dsn/utility/error_code.h
+++ b/include/dsn/utility/error_code.h
@@ -124,5 +124,5 @@ DEFINE_ERR_CODE(ERR_UNAUTHENTICATED)
 DEFINE_ERR_CODE(ERR_KRB5_INTERNAL)
 
 DEFINE_ERR_CODE(ERR_SASL_INTERNAL)
-DEFINE_ERR_CODE(ERR_SASL_INCOMPLETED)
+DEFINE_ERR_CODE(ERR_SASL_INCOMPLETE)
 } // namespace dsn
diff --git a/src/runtime/security/client_negotiation.cpp b/src/runtime/security/client_negotiation.cpp
index 3f75be7ccc..306273569c 100644
--- a/src/runtime/security/client_negotiation.cpp
+++ b/src/runtime/security/client_negotiation.cpp
@@ -128,7 +128,7 @@ void client_negotiation::on_mechanism_selected(const negotiation_response &resp)
     // start client sasl, and send `SASL_INITIATE` to `server_negotiation` if everything is ok
     std::string start_output;
     err_s = _sasl->start(_selected_mechanism, "", start_output);
-    if (err_s.is_ok() || ERR_SASL_INCOMPLETED == err_s.code()) {
+    if (err_s.is_ok() || ERR_SASL_INCOMPLETE == err_s.code()) {
         auto req = dsn::make_unique<negotiation_request>();
         _status = req->status = negotiation_status::type::SASL_INITIATE;
         req->msg = start_output;
diff --git a/src/runtime/security/sasl_wrapper.cpp b/src/runtime/security/sasl_wrapper.cpp
index d06d2dcec5..91cc05079d 100644
--- a/src/runtime/security/sasl_wrapper.cpp
+++ b/src/runtime/security/sasl_wrapper.cpp
@@ -46,7 +46,7 @@ error_s sasl_wrapper::wrap_error(int sasl_err)
     case SASL_OK:
         return error_s::make(ERR_OK);
     case SASL_CONTINUE:
-        return error_s::make(ERR_SASL_INCOMPLETED);
+        return error_s::make(ERR_SASL_INCOMPLETE);
     case SASL_FAIL:      // Generic failure (encompasses missing krb5 credentials).
     case SASL_BADAUTH:   // Authentication failure.
     case SASL_BADMAC:    // Decode failure.
diff --git a/src/runtime/test/client_negotiation_test.cpp b/src/runtime/test/client_negotiation_test.cpp
index 1821a86151..e828ae70c4 100644
--- a/src/runtime/test/client_negotiation_test.cpp
+++ b/src/runtime/test/client_negotiation_test.cpp
@@ -132,7 +132,7 @@ TEST_F(client_negotiation_test, on_mechanism_selected)
                   negotiation_status::type::SASL_SELECT_MECHANISMS_RESP,
                   negotiation_status::type::SASL_INITIATE},
                  {"ERR_OK",
-                  "ERR_SASL_INCOMPLETED",
+                  "ERR_SASL_INCOMPLETE",
                   negotiation_status::type::SASL_SELECT_MECHANISMS_RESP,
                   negotiation_status::type::SASL_INITIATE},
                  {"ERR_OK",

From bafb0822bbb16bcfda243452b6b5967c502410b5 Mon Sep 17 00:00:00 2001
From: levy <zlw5307@163.com>
Date: Fri, 4 Sep 2020 13:56:02 +0800
Subject: [PATCH 19/19] fix

---
 src/runtime/test/client_negotiation_test.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/runtime/test/client_negotiation_test.cpp b/src/runtime/test/client_negotiation_test.cpp
index e828ae70c4..7df6334953 100644
--- a/src/runtime/test/client_negotiation_test.cpp
+++ b/src/runtime/test/client_negotiation_test.cpp
@@ -123,8 +123,8 @@ TEST_F(client_negotiation_test, on_mechanism_selected)
 {
     struct
     {
-        std::string sasl_init_return_value;
-        std::string sasl_start_return_value;
+        std::string sasl_init_result;
+        std::string sasl_start_result;
         negotiation_status::type resp_status;
         negotiation_status::type neg_status;
     } tests[] = {{"ERR_OK",
@@ -152,8 +152,8 @@ TEST_F(client_negotiation_test, on_mechanism_selected)
     {
         for (const auto &test : tests) {
             fail::setup();
-            fail::cfg("sasl_client_wrapper_init", "return(" + test.sasl_init_return_value + ")");
-            fail::cfg("sasl_client_wrapper_start", "return(" + test.sasl_start_return_value + ")");
+            fail::cfg("sasl_client_wrapper_init", "return(" + test.sasl_init_result + ")");
+            fail::cfg("sasl_client_wrapper_start", "return(" + test.sasl_start_result + ")");
 
             negotiation_response resp;
             resp.status = test.resp_status;