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

Add constraints on invalid password attempts #3573

Merged
merged 14 commits into from
Dec 30, 2021
124 changes: 120 additions & 4 deletions src/clients/meta/MetaClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@
#include <thrift/lib/cpp/util/EnumUtils.h>

#include <boost/filesystem.hpp>
#include <unordered_set>

#include "clients/meta/FileBasedClusterIdMan.h"
#include "clients/meta/stats/MetaClientStats.h"
#include "common/base/Base.h"
#include "common/base/MurmurHash2.h"
#include "common/base/Status.h"
#include "common/conf/Configuration.h"
#include "common/http/HttpClient.h"
#include "common/meta/NebulaSchemaProvider.h"
Expand All @@ -37,6 +39,27 @@ DEFINE_int32(meta_client_retry_interval_secs, 1, "meta client sleep interval bet
DEFINE_int32(meta_client_timeout_ms, 60 * 1000, "meta client timeout");
DEFINE_string(cluster_id_path, "cluster.id", "file path saved clusterId");
DEFINE_int32(check_plan_killed_frequency, 8, "check plan killed every 1<<n times");
DEFINE_uint32(failed_login_attempts,
0,
HarrisChu marked this conversation as resolved.
Show resolved Hide resolved
"how many consecutive incorrect passwords input to a SINGLE graph service node cause "
"the account to become locked.");
DEFINE_uint32(
password_lock_time_in_secs,
0,
HarrisChu marked this conversation as resolved.
Show resolved Hide resolved
"how long in seconds to lock the account after too many consecutive login attempts provide an "
"incorrect password.");

// Sanity-checking Flag Values
static bool ValidateFailedLoginAttempts(const char* flagname, uint32_t value) {
if (value <= 32767) // value is ok
return true;

FLOG_WARN("Invalid value for --%s: %d, the timeout should be an integer between 0 and 32767\n",
flagname,
(int)value);
return false;
}
DEFINE_validator(failed_login_attempts, &ValidateFailedLoginAttempts);

namespace nebula {
namespace meta {
Expand Down Expand Up @@ -152,6 +175,11 @@ bool MetaClient::loadUsersAndRoles() {
}
decltype(userRolesMap_) userRolesMap;
decltype(userPasswordMap_) userPasswordMap;
decltype(userPasswordAttemptsRemain_) userPasswordAttemptsRemain;
decltype(userLoginLockTime_) userLoginLockTime;
// List of username
std::unordered_set<std::string> userNameList;

for (auto& user : userRoleRet.value()) {
auto rolesRet = getUserRoles(user.first).get();
if (!rolesRet.ok()) {
Expand All @@ -160,11 +188,40 @@ bool MetaClient::loadUsersAndRoles() {
}
userRolesMap[user.first] = rolesRet.value();
userPasswordMap[user.first] = user.second;
userPasswordAttemptsRemain[user.first] = FLAGS_failed_login_attempts;
userLoginLockTime[user.first] = 0;
userNameList.emplace(user.first);
}
{
folly::RWSpinLock::WriteHolder holder(localCacheLock_);
userRolesMap_ = std::move(userRolesMap);
userPasswordMap_ = std::move(userPasswordMap);

// This method is called periodically by the heartbeat thread, but we don't want to reset the
// failed login attempts every time. Remove expired users from cache
for (auto& ele : userPasswordAttemptsRemain) {
if (userNameList.count(ele.first) == 0) {
userPasswordAttemptsRemain.erase(ele.first);
}
}
for (auto& ele : userLoginLockTime) {
if (userNameList.count(ele.first) == 0) {
userLoginLockTime.erase(ele.first);
}
}

// If the user is not in the map, insert value with the default value
for (const auto& user : userNameList) {
if (userPasswordAttemptsRemain.count(user) == 0) {
userPasswordAttemptsRemain[user] = FLAGS_failed_login_attempts;
}
if (userLoginLockTime.count(user) == 0) {
userLoginLockTime[user] = 0;
}
}

userPasswordAttemptsRemain_ = std::move(userPasswordAttemptsRemain);
userLoginLockTime_ = std::move(userLoginLockTime);
}
return true;
}
Expand Down Expand Up @@ -1370,6 +1427,8 @@ folly::Future<StatusOr<bool>> MetaClient::multiPut(

cpp2::MultiPutReq req;
std::vector<nebula::KeyValue> data;
data.reserve(pairs.size());

for (auto& element : pairs) {
data.emplace_back(std::move(element));
}
Expand Down Expand Up @@ -2354,16 +2413,73 @@ std::vector<cpp2::RoleItem> MetaClient::getRolesByUserFromCache(const std::strin
return iter->second;
}

bool MetaClient::authCheckFromCache(const std::string& account, const std::string& password) {
Status MetaClient::authCheckFromCache(const std::string& account, const std::string& password) {
// Check meta service status
if (!ready_) {
return false;
return Status::Error("Meta Service not ready");
}

const ThreadLocalInfo& threadLocalInfo = getThreadLocalInfo();
// Check user existence
auto iter = threadLocalInfo.userPasswordMap_.find(account);
if (iter == threadLocalInfo.userPasswordMap_.end()) {
return false;
return Status::Error("User not exist");
}

folly::RWSpinLock::WriteHolder holder(localCacheLock_);

auto& lockedSince = userLoginLockTime_[account];
Aiee marked this conversation as resolved.
Show resolved Hide resolved
auto& passwordAttemtRemain = userPasswordAttemptsRemain_[account];
LOG(INFO) << "Thread id: " << std::this_thread::get_id()
<< " ,passwordAttemtRemain: " << passwordAttemtRemain;
// lockedSince is non-zero means the account has been locked
if (lockedSince != 0) {
auto remainingLockTime =
(lockedSince + FLAGS_password_lock_time_in_secs) - time::WallClock::fastNowInSec();
// If the account is still locked, there is no need to check the password
if (remainingLockTime > 0) {
return Status::Error(
"%d times consecutive incorrect passwords has been input, user name: %s has been "
"locked, try again in %ld seconds",
FLAGS_failed_login_attempts,
account.c_str(),
remainingLockTime);
}
// Clear lock state and reset attempts
lockedSince = 0;
passwordAttemtRemain = FLAGS_failed_login_attempts;
}
Aiee marked this conversation as resolved.
Show resolved Hide resolved
return iter->second == password;

if (iter->second != password) {
// By default there is no limit of login attempts if any of these 2 flags is unset
if (FLAGS_failed_login_attempts == 0 || FLAGS_password_lock_time_in_secs == 0) {
return Status::Error("Invalid password");
}

// If the password is not correct and passwordAttemtRemain > 0,
// Allow another attemp
if (passwordAttemtRemain > 0) {
--passwordAttemtRemain;
if (passwordAttemtRemain == 0) {
// If the remaining attemps is 0, failed to authenticate
// Block user login
lockedSince = time::WallClock::fastNowInSec();
return Status::Error(
"%d times consecutive incorrect passwords has been input, user name: %s has been "
"locked, try again in %d seconds",
FLAGS_failed_login_attempts,
account.c_str(),
FLAGS_password_lock_time_in_secs);
}
LOG(ERROR) << "Invalid password, remaining attempts: " << passwordAttemtRemain;
return Status::Error("Invalid password, remaining attempts: %d", passwordAttemtRemain);
}
}

// Reset password attempts
passwordAttemtRemain = FLAGS_failed_login_attempts;
lockedSince = 0;
return Status::OK();
}

bool MetaClient::checkShadowAccountFromCache(const std::string& account) {
Expand Down
15 changes: 11 additions & 4 deletions src/clients/meta/MetaClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <gtest/gtest_prod.h>

#include <atomic>
#include <cstdint>

#include "common/base/Base.h"
#include "common/base/ObjectPool.h"
Expand Down Expand Up @@ -143,6 +144,10 @@ using IndexStatus = std::tuple<std::string, std::string, std::string>;
using UserRolesMap = std::unordered_map<std::string, std::vector<cpp2::RoleItem>>;
// get user password by account
using UserPasswordMap = std::unordered_map<std::string, std::string>;
// Mapping of user name and remaining wrong password attempts
using UserPasswordAttemptsRemain = std::unordered_map<std::string, uint32>;
// Mapping of user name and the timestamp when the account is locked
using UserLoginLockTime = std::unordered_map<std::string, uint32>;

Comment on lines 146 to 151
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could these 3 map be merged to one map?
eg.
std::unordered_map<std::string, PassInfo>.

Copy link
Contributor Author

@Aiee Aiee Dec 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've considered this, but UserPasswordMap is used in the localThreadCache as a read-only object, and the rest two objects require both read and write. I believe we could keep it like this for now.

// config cache, get config via module and name
using MetaConfigMap =
Expand Down Expand Up @@ -199,13 +204,13 @@ struct MetaClientOptions {
std::string serviceName_ = "";
// Whether to skip the config manager
bool skipConfig_ = false;
// host role(graph/meta/storage) using this client
// Host role(graph/meta/storage) using this client
cpp2::HostRole role_ = cpp2::HostRole::UNKNOWN;
// gitInfoSHA of Host using this client
std::string gitInfoSHA_{""};
// data path list, used in storaged
// Data path list, used in storaged
std::vector<std::string> dataPaths_;
// install path, used in metad/graphd/storaged
// Install path, used in metad/graphd/storaged
std::string rootPath_;
};

Expand Down Expand Up @@ -593,7 +598,7 @@ class MetaClient {

std::vector<cpp2::RoleItem> getRolesByUserFromCache(const std::string& user);

bool authCheckFromCache(const std::string& account, const std::string& password);
Status authCheckFromCache(const std::string& account, const std::string& password);

StatusOr<TermID> getTermFromCache(GraphSpaceID spaceId, PartitionID);

Expand Down Expand Up @@ -821,6 +826,8 @@ class MetaClient {

UserRolesMap userRolesMap_;
UserPasswordMap userPasswordMap_;
UserPasswordAttemptsRemain userPasswordAttemptsRemain_;
UserLoginLockTime userLoginLockTime_;

NameIndexMap tagNameIndexMap_;
NameIndexMap edgeNameIndexMap_;
Expand Down
2 changes: 1 addition & 1 deletion src/common/graph/Response.h
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ struct AuthResponse {
if (!checkPointer(timeZoneOffsetSeconds.get(), rhs.timeZoneOffsetSeconds.get())) {
return false;
}
return checkPointer(timeZoneName.get(), timeZoneName.get());
return checkPointer(timeZoneName.get(), rhs.timeZoneName.get());
}

ErrorCode errorCode{ErrorCode::SUCCEEDED};
Expand Down
3 changes: 2 additions & 1 deletion src/graph/service/Authenticator.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#define GRAPH_SERVICE_AUTHENTICATOR_H_

#include "common/base/Base.h"
#include "common/base/Status.h"

namespace nebula {
namespace graph {
Expand All @@ -15,7 +16,7 @@ class Authenticator {
public:
virtual ~Authenticator() {}

virtual bool NG_MUST_USE_RESULT auth(const std::string &user, const std::string &password) = 0;
virtual Status NG_MUST_USE_RESULT auth(const std::string &user, const std::string &password) = 0;
};

} // namespace graph
Expand Down
13 changes: 7 additions & 6 deletions src/graph/service/CloudAuthenticator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include "graph/service/CloudAuthenticator.h"

#include "common/base/Status.h"
#include "common/encryption/Base64.h"
#include "common/http/HttpClient.h"
#include "graph/service/GraphFlags.h"
Expand All @@ -16,13 +17,13 @@ CloudAuthenticator::CloudAuthenticator(meta::MetaClient* client) {
metaClient_ = client;
}

bool CloudAuthenticator::auth(const std::string& user, const std::string& password) {
Status CloudAuthenticator::auth(const std::string& user, const std::string& password) {
// The shadow account on the nebula side has been created
// Normal passwords and tokens use different prefixes

// First, go to meta to check if the shadow account exists
if (!metaClient_->checkShadowAccountFromCache(user)) {
return false;
return Status::Error("Shadow account not exist");
}

// Second, use user + password authentication methods
Expand All @@ -35,20 +36,20 @@ bool CloudAuthenticator::auth(const std::string& user, const std::string& passwo

if (!result.ok()) {
LOG(ERROR) << result.status();
return false;
return result.status();
}

try {
auto json = folly::parseJson(result.value());
if (json["code"].asString().compare("0") != 0) {
LOG(ERROR) << "Cloud authentication failed, user: " << user;
return false;
return Status::Error("Cloud authentication failed, user: %s", user.c_str());
}
} catch (std::exception& e) {
LOG(ERROR) << "Invalid json: " << e.what();
return false;
return Status::Error("Invalid json: %s", e.what());
}
return true;
return Status::OK();
}

} // namespace graph
Expand Down
2 changes: 1 addition & 1 deletion src/graph/service/CloudAuthenticator.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class CloudAuthenticator final : public Authenticator {
public:
explicit CloudAuthenticator(meta::MetaClient* client);

bool auth(const std::string& user, const std::string& password) override;
Status auth(const std::string& user, const std::string& password) override;

private:
meta::MetaClient* metaClient_;
Expand Down
8 changes: 8 additions & 0 deletions src/graph/service/GraphFlags.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ DECLARE_string(auth_type);
DECLARE_string(cloud_http_url);
DECLARE_uint32(max_allowed_statements);

// Failed login attempt
// value of failed_login_attempts is in the range from 0 to 32767.
// The deault value is 0. A value of 0 disables the option.
DECLARE_uint32(failed_login_attempts);
// value of password_lock_time_in_secs is in the range from 0 to 32767[secs].
// The deault value is 0. A value of 0 disables the option.
DECLARE_uint32(password_lock_time_in_secs);

// optimizer
DECLARE_bool(enable_optimizer);

Expand Down
15 changes: 7 additions & 8 deletions src/graph/service/GraphService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,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
if (!auth(username, password)) {
auto authResult = auth(username, password);
if (!authResult.ok()) {
ctx->resp().errorCode = ErrorCode::E_BAD_USERNAME_PASSWORD;
ctx->resp().errorMsg.reset(new std::string("Bad username/password"));
ctx->resp().errorMsg.reset(new std::string(authResult.toString()));
ctx->finish();
stats::StatsManager::addValue(kNumAuthFailedSessions);
stats::StatsManager::addValue(kNumAuthFailedSessionsBadUserNamePassword);
Expand Down Expand Up @@ -200,9 +201,9 @@ folly::Future<std::string> GraphService::future_executeJsonWithParameter(
});
}

bool GraphService::auth(const std::string& username, const std::string& password) {
Status GraphService::auth(const std::string& username, const std::string& password) {
if (!FLAGS_enable_authorize) {
return true;
return Status::OK();
}

if (FLAGS_auth_type == "password") {
Expand All @@ -214,14 +215,12 @@ bool GraphService::auth(const std::string& username, const std::string& password
// There is no way to identify which one is in the graph layer,
// let's check the native user's password first, then cloud user.
auto pwdAuth = std::make_unique<PasswordAuthenticator>(queryEngine_->metaClient());
if (pwdAuth->auth(username, encryption::MD5Utils::md5Encode(password))) {
return true;
}
return pwdAuth->auth(username, encryption::MD5Utils::md5Encode(password));
auto cloudAuth = std::make_unique<CloudAuthenticator>(queryEngine_->metaClient());
return cloudAuth->auth(username, password);
}
LOG(WARNING) << "Unknown auth type: " << FLAGS_auth_type;
return false;
return Status::Error("Unknown auth type: %s", FLAGS_auth_type.c_str());
}

folly::Future<cpp2::VerifyClientVersionResp> GraphService::future_verifyClientVersion(
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:
bool auth(const std::string& username, const std::string& password);
Status auth(const std::string& username, const std::string& password);

std::unique_ptr<GraphSessionManager> sessionManager_;
std::unique_ptr<QueryEngine> queryEngine_;
Expand Down
2 changes: 1 addition & 1 deletion src/graph/service/PasswordAuthenticator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ PasswordAuthenticator::PasswordAuthenticator(meta::MetaClient* client) {
metaClient_ = client;
}

bool PasswordAuthenticator::auth(const std::string& user, const std::string& password) {
Status PasswordAuthenticator::auth(const std::string& user, const std::string& password) {
return metaClient_->authCheckFromCache(user, password);
}

Expand Down
Loading