Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix service crash caused by connecting using a pre-v2.6.0 client #3942

Merged
merged 5 commits into from
Mar 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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