From cd3831365fe2b846dfe47d07d85a48d0d87858e2 Mon Sep 17 00:00:00 2001 From: Yichen Wang <18348405+Aiee@users.noreply.github.com> Date: Fri, 11 Mar 2022 13:54:14 +0800 Subject: [PATCH] Fix service crash caused by connecting using a pre-v2.6.0 client (#3942) * Reject clients with a version lower than 2.6.0 * Add TTL for clientAddr_ * Fix tests * Use client_idle_timeout_secs as the clientAddrTimeout * Change the param of authCheckFromCache() --- src/clients/meta/MetaClient.cpp | 22 ++++++++++++++ src/clients/meta/MetaClient.h | 21 +++++++++++++ src/graph/service/GraphService.cpp | 37 +++++++++++++++++++++-- src/graph/service/GraphService.h | 2 +- src/graph/service/PasswordAuthenticator.h | 1 + 5 files changed, 79 insertions(+), 4 deletions(-) diff --git a/src/clients/meta/MetaClient.cpp b/src/clients/meta/MetaClient.cpp index 4c6ff5f0cb3..ae9186c3021 100644 --- a/src/clients/meta/MetaClient.cpp +++ b/src/clients/meta/MetaClient.cpp @@ -2298,6 +2298,7 @@ Status MetaClient::authCheckFromCache(const std::string& account, const std::str if (!ready_) { return Status::Error("Meta Service not ready"); } + folly::rcu_reader guard; const auto& metadata = *metadata_.load(); auto iter = metadata.userPasswordMap_.find(account); @@ -2468,6 +2469,11 @@ folly::Future> MetaClient::heartbeat() { } } + // TTL for clientAddrMap + // If multiple connections are created but do not authenticate, the clientAddrMap_ will keep + // growing. This is to clear the clientAddrMap_ regularly. + clearClientAddrMap(); + // info used in the agent, only set once // TOOD(spw): if we could add data path(disk) dynamicly in the future, it should be // reported every time it changes @@ -3619,5 +3625,21 @@ Status MetaClient::verifyVersion() { } return Status::OK(); } + +void MetaClient::clearClientAddrMap() { + if (clientAddrMap_.size() == 0) { + return; + } + + auto curTimestamp = time::WallClock::fastNowInSec(); + for (auto it = clientAddrMap_.cbegin(); it != clientAddrMap_.cend();) { + // The clientAddr is expired + if (it->second < curTimestamp) { + it = clientAddrMap_.erase(it); + } else { + ++it; + } + } +} } // namespace meta } // namespace nebula diff --git a/src/clients/meta/MetaClient.h b/src/clients/meta/MetaClient.h index 01584a974f7..b3959c899c4 100644 --- a/src/clients/meta/MetaClient.h +++ b/src/clients/meta/MetaClient.h @@ -154,6 +154,8 @@ using MetaConfigMap = using FTIndexMap = std::unordered_map; using SessionMap = std::unordered_map; + +using clientAddrMap = folly::ConcurrentHashMap; class MetaChangedListener { public: virtual ~MetaChangedListener() = default; @@ -641,6 +643,10 @@ class MetaClient { return options_.localHost_.toString(); } + clientAddrMap& getClientAddrMap() { + return clientAddrMap_; + } + protected: // Return true if load succeeded. bool loadData(); @@ -727,6 +733,9 @@ class MetaClient { Status verifyVersion(); + // Removes expired keys in the clientAddrMap_ + void clearClientAddrMap(); + private: std::shared_ptr ioThreadPool_; std::shared_ptr> clientsMan_; @@ -807,6 +816,18 @@ class MetaClient { NameIndexMap tagNameIndexMap_; NameIndexMap edgeNameIndexMap_; + // TODO(Aiee) This is a walkaround to address the problem that using a lower version(< v2.6.0) + // client to connect with higher version(>= v3.0.0) Nebula service will cause a crash. + // + // The key here is the host of the client that sends the request, and the value indicates the + // expiration of the key because we don't want to keep the key forever. + // + // The assumption here is that there is ONLY ONE VERSION of the client in the host. + // + // This map will be updated when verifyVersion() is called. Only the clients since v2.6.0 will + // call verifyVersion(), thus we could determine whether the client version is lower than v2.6.0 + clientAddrMap clientAddrMap_; + // Global service client ServiceClientsList serviceClientList_; FTIndexMap fulltextIndexMap_; diff --git a/src/graph/service/GraphService.cpp b/src/graph/service/GraphService.cpp index 4deaafc372b..97b83ea6d6f 100644 --- a/src/graph/service/GraphService.cpp +++ b/src/graph/service/GraphService.cpp @@ -24,6 +24,9 @@ namespace nebula { namespace graph { +// The default value is 28800 seconds +const int64_t clientAddrTimeout = FLAGS_client_idle_timeout_secs; + Status GraphService::init(std::shared_ptr ioExecutor, const HostAddr& hostAddr) { auto addrs = network::NetworkUtils::toHosts(FLAGS_meta_server_addrs); @@ -69,8 +72,10 @@ folly::Future GraphService::future_authenticate(const std::string& auto ctx = std::make_unique>(); auto future = ctx->future(); - // check username and password failed - auto authResult = auth(username, password); + // Check username and password failed + // Check whether the client has called verifyClientVersion() + auto clientAddr = HostAddr(peer->getAddressStr(), peer->getPort()); + auto authResult = auth(username, password, clientAddr); if (!authResult.ok()) { ctx->resp().errorCode = ErrorCode::E_BAD_USERNAME_PASSWORD; ctx->resp().errorMsg.reset(new std::string(authResult.toString())); @@ -202,12 +207,29 @@ folly::Future GraphService::future_executeJsonWithParameter( }); } -Status GraphService::auth(const std::string& username, const std::string& password) { +Status GraphService::auth(const std::string& username, + const std::string& password, + const HostAddr& clientIp) { if (!FLAGS_enable_authorize) { return Status::OK(); } if (FLAGS_auth_type == "password") { + auto metaClient = queryEngine_->metaClient(); + // TODO(Aiee) This is a walkaround to address the problem that using a lower version(< v2.6.0) + // client to connect with higher version(>= v3.0.0) Nebula service will cause a crash. + // + // Only the clients since v2.6.0 will call verifyVersion(), thus we could determine whether the + // client version is lower than v2.6.0 + auto clientAddrIt = metaClient->getClientAddrMap().find(clientIp); + if (clientAddrIt == metaClient->getClientAddrMap().end()) { + return Status::Error( + folly::sformat("The version of the client sending request from {} is lower than v2.6.0, " + "please update the client.", + clientIp.toString())); + } + + // Auth with PasswordAuthenticator auto authenticator = std::make_unique(queryEngine_->metaClient()); return authenticator->auth(username, proxygen::md5Encode(folly::StringPiece(password))); } else if (FLAGS_auth_type == "cloud") { @@ -230,6 +252,7 @@ folly::Future GraphService::future_verifyClientVe folly::splitTo( ":", FLAGS_client_white_list, std::inserter(whiteList, whiteList.begin())); cpp2::VerifyClientVersionResp resp; + if (FLAGS_enable_client_white_list && whiteList.find(req.get_version()) == whiteList.end()) { resp.error_code_ref() = nebula::cpp2::ErrorCode::E_CLIENT_SERVER_INCOMPATIBLE; resp.error_msg_ref() = folly::stringPrintf( @@ -239,6 +262,14 @@ folly::Future GraphService::future_verifyClientVe } else { resp.error_code_ref() = nebula::cpp2::ErrorCode::SUCCEEDED; } + + // The client sent request has a version >= v2.6.0, mark the address as valid + auto* peer = getRequestContext()->getPeerAddress(); + auto clientAddr = HostAddr(peer->getAddressStr(), peer->getPort()); + + auto ttlTimestamp = time::WallClock::fastNowInSec() + clientAddrTimeout; + auto clientAddrMap = &metaClient_->getClientAddrMap(); + clientAddrMap->insert_or_assign(clientAddr, ttlTimestamp); return folly::makeFuture(std::move(resp)); } } // namespace graph diff --git a/src/graph/service/GraphService.h b/src/graph/service/GraphService.h index 85ed21a6ce4..d96d6385004 100644 --- a/src/graph/service/GraphService.h +++ b/src/graph/service/GraphService.h @@ -54,7 +54,7 @@ class GraphService final : public cpp2::GraphServiceSvIf { std::unique_ptr metaClient_; private: - Status auth(const std::string& username, const std::string& password); + Status auth(const std::string& username, const std::string& password, const HostAddr& clientIp); std::unique_ptr sessionManager_; std::unique_ptr queryEngine_; diff --git a/src/graph/service/PasswordAuthenticator.h b/src/graph/service/PasswordAuthenticator.h index 7bb3e792251..98996271655 100644 --- a/src/graph/service/PasswordAuthenticator.h +++ b/src/graph/service/PasswordAuthenticator.h @@ -16,6 +16,7 @@ class PasswordAuthenticator final : public Authenticator { public: explicit PasswordAuthenticator(meta::MetaClient* client); + // Authenticates the user by checking the user/password cache in the meta Status auth(const std::string& user, const std::string& password) override; private: