Skip to content

Commit

Permalink
Fix service crash caused by connecting using a pre-v2.6.0 client (#3942)
Browse files Browse the repository at this point in the history
* 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()
  • Loading branch information
Aiee authored Mar 11, 2022
1 parent 138e587 commit cd38313
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 4 deletions.
22 changes: 22 additions & 0 deletions src/clients/meta/MetaClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -2468,6 +2469,11 @@ folly::Future<StatusOr<bool>> 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
Expand Down Expand Up @@ -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
21 changes: 21 additions & 0 deletions src/clients/meta/MetaClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ using MetaConfigMap =
using FTIndexMap = std::unordered_map<std::string, cpp2::FTIndex>;

using SessionMap = std::unordered_map<SessionID, cpp2::Session>;

using clientAddrMap = folly::ConcurrentHashMap<HostAddr, int64_t>;
class MetaChangedListener {
public:
virtual ~MetaChangedListener() = default;
Expand Down Expand Up @@ -641,6 +643,10 @@ class MetaClient {
return options_.localHost_.toString();
}

clientAddrMap& getClientAddrMap() {
return clientAddrMap_;
}

protected:
// Return true if load succeeded.
bool loadData();
Expand Down Expand Up @@ -727,6 +733,9 @@ class MetaClient {

Status verifyVersion();

// Removes expired keys in the clientAddrMap_
void clearClientAddrMap();

private:
std::shared_ptr<folly::IOThreadPoolExecutor> ioThreadPool_;
std::shared_ptr<thrift::ThriftClientManager<cpp2::MetaServiceAsyncClient>> clientsMan_;
Expand Down Expand Up @@ -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_;
Expand Down
37 changes: 34 additions & 3 deletions src/graph/service/GraphService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<folly::IOThreadPoolExecutor> ioExecutor,
const HostAddr& hostAddr) {
auto addrs = network::NetworkUtils::toHosts(FLAGS_meta_server_addrs);
Expand Down Expand Up @@ -69,8 +72,10 @@ folly::Future<AuthResponse> GraphService::future_authenticate(const std::string&

auto ctx = std::make_unique<RequestContext<AuthResponse>>();
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()));
Expand Down Expand Up @@ -202,12 +207,29 @@ folly::Future<std::string> 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<PasswordAuthenticator>(queryEngine_->metaClient());
return authenticator->auth(username, proxygen::md5Encode(folly::StringPiece(password)));
} else if (FLAGS_auth_type == "cloud") {
Expand All @@ -230,6 +252,7 @@ folly::Future<cpp2::VerifyClientVersionResp> GraphService::future_verifyClientVe
folly::splitTo<std::string>(
":", 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(
Expand All @@ -239,6 +262,14 @@ folly::Future<cpp2::VerifyClientVersionResp> 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<cpp2::VerifyClientVersionResp>(std::move(resp));
}
} // namespace graph
Expand Down
2 changes: 1 addition & 1 deletion src/graph/service/GraphService.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class GraphService final : public cpp2::GraphServiceSvIf {
std::unique_ptr<meta::MetaClient> 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<GraphSessionManager> sessionManager_;
std::unique_ptr<QueryEngine> queryEngine_;
Expand Down
1 change: 1 addition & 0 deletions src/graph/service/PasswordAuthenticator.h
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit cd38313

Please sign in to comment.