From daf5f52c64ce9d373b09a8dfbd4979e656b10fd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Mon, 10 Sep 2018 15:28:37 +0300 Subject: [PATCH] Pass raw password to users_auth By passing the raw password deeper into the authentication code, it can be used to verify the user can access some systems. Right now, this is not required by the simple salted password comparison done in MaxScale. --- include/maxscale/adminusers.h.in | 2 +- include/maxscale/users.h | 7 ++++--- server/core/admin.cc | 2 +- server/core/adminusers.cc | 20 +++++++++----------- server/core/users.cc | 11 +++++++---- server/modules/routing/debugcli/debugcmd.cc | 2 +- 6 files changed, 23 insertions(+), 21 deletions(-) diff --git a/include/maxscale/adminusers.h.in b/include/maxscale/adminusers.h.in index cc5d042b6..4025c32a4 100644 --- a/include/maxscale/adminusers.h.in +++ b/include/maxscale/adminusers.h.in @@ -76,7 +76,7 @@ const char* admin_add_inet_user(const char *uname, const char *password, enum us const char* admin_remove_inet_user(const char* uname); bool admin_inet_user_exists(const char *uname); bool admin_verify_inet_user(const char *uname, const char *password); -bool admin_user_is_inet_admin(const char* username); +bool admin_user_is_inet_admin(const char* username, const char *password); bool admin_user_is_unix_admin(const char* username); bool admin_have_admin(); bool admin_is_last_admin(const char* user); diff --git a/include/maxscale/users.h b/include/maxscale/users.h index 0f6255cb4..be3d91106 100644 --- a/include/maxscale/users.h +++ b/include/maxscale/users.h @@ -100,12 +100,13 @@ bool users_find(USERS* users, const char* user); /** * Check if user is an administrator * - * @param users The users table - * @param user User to check + * @param users The users table + * @param user User to check + * @param password Password of the user or NULL if password isn't available * * @return True if user is an administrator */ -bool users_is_admin(USERS* users, const char* user); +bool users_is_admin(USERS* users, const char* user, const char* password); /** * Check how many admin account exists diff --git a/server/core/admin.cc b/server/core/admin.cc index 838b2e1ff..4df576d4c 100644 --- a/server/core/admin.cc +++ b/server/core/admin.cc @@ -192,7 +192,7 @@ bool Client::auth(MHD_Connection* connection, const char* url, const char* metho send_auth_error(connection); rval = false; } - else if (!admin_user_is_inet_admin(user) && modifies_data(connection, method)) + else if (!admin_user_is_inet_admin(user, pw) && modifies_data(connection, method)) { if (config_get_global_options()->admin_log_auth_failures) { diff --git a/server/core/adminusers.cc b/server/core/adminusers.cc index de339d3c0..56ea8fc6a 100644 --- a/server/core/adminusers.cc +++ b/server/core/adminusers.cc @@ -27,6 +27,7 @@ #include #include #include +#include /** * @file adminusers.c - Administration user account management @@ -209,7 +210,7 @@ static std::string path_from_type(enum user_type type) json_t* admin_user_to_json(const char* host, const char* user, enum user_type type) { user_account_type account = USER_ACCOUNT_BASIC; - if ((type == USER_TYPE_INET && admin_user_is_inet_admin(user)) + if ((type == USER_TYPE_INET && admin_user_is_inet_admin(user, nullptr)) || (type == USER_TYPE_UNIX && admin_user_is_unix_admin(user))) { account = USER_ACCOUNT_ADMIN; @@ -431,9 +432,8 @@ bool admin_linux_account_enabled(const char* uname) */ const char* admin_add_inet_user(const char* uname, const char* password, enum user_account_type type) { - char cpassword[MXS_CRYPT_SIZE]; - mxs_crypt(password, ADMIN_SALT, cpassword); - return admin_add_user(&inet_users, INET_USERS_FILE_NAME, uname, cpassword, type); + auto cpassword = mxs::crypt(password, ADMIN_SALT); + return admin_add_user(&inet_users, INET_USERS_FILE_NAME, uname, cpassword.c_str(), type); } /** @@ -482,21 +482,19 @@ bool admin_verify_inet_user(const char* username, const char* password) if (inet_users) { - char cpassword[MXS_CRYPT_SIZE]; - mxs_crypt(password, ADMIN_SALT, cpassword); - rv = users_auth(inet_users, username, cpassword); + rv = users_auth(inet_users, username, password); } return rv; } -bool admin_user_is_inet_admin(const char* username) +bool admin_user_is_inet_admin(const char* username, const char* password) { bool rval = false; if (inet_users) { - rval = users_is_admin(inet_users, username); + rval = users_is_admin(inet_users, username, password); } return rval; @@ -508,7 +506,7 @@ bool admin_user_is_unix_admin(const char* username) if (linux_users) { - rval = users_is_admin(linux_users, username); + rval = users_is_admin(linux_users, username, nullptr); } return rval; @@ -521,7 +519,7 @@ bool admin_have_admin() bool admin_is_last_admin(const char* user) { - return (admin_user_is_inet_admin(user) || admin_user_is_unix_admin(user)) + return (admin_user_is_inet_admin(user, nullptr) || admin_user_is_unix_admin(user)) && (users_admin_count(inet_users) + users_admin_count(linux_users)) == 1; } diff --git a/server/core/users.cc b/server/core/users.cc index edb97c747..26b5e27a3 100644 --- a/server/core/users.cc +++ b/server/core/users.cc @@ -20,6 +20,7 @@ #include #include +#include #include #include #include @@ -109,7 +110,9 @@ public: return std::count_if(m_data.begin(), m_data.end(), is_admin); } - bool check_permissions(std::string user, user_account_type perm) const + bool check_permissions(const std::string& user, + const std::string& password, + user_account_type perm) const { std::lock_guard guard(m_lock); UserMap::const_iterator it = m_data.find(user); @@ -295,16 +298,16 @@ bool users_auth(USERS* users, const char* user, const char* password) if (u->get(user, &info)) { - rval = strcmp(password, info.password.c_str()) == 0; + rval = info.password == mxs::crypt(password, ADMIN_SALT); } return rval; } -bool users_is_admin(USERS* users, const char* user) +bool users_is_admin(USERS* users, const char* user, const char* password) { Users* u = reinterpret_cast(users); - return u->check_permissions(user, USER_ACCOUNT_ADMIN); + return u->check_permissions(user, password ? password : "", USER_ACCOUNT_ADMIN); } int users_admin_count(USERS* users) diff --git a/server/modules/routing/debugcli/debugcmd.cc b/server/modules/routing/debugcli/debugcmd.cc index cf19e7830..713e2ce9f 100644 --- a/server/modules/routing/debugcli/debugcmd.cc +++ b/server/modules/routing/debugcli/debugcmd.cc @@ -2046,7 +2046,7 @@ static bool user_is_authorized(DCB* dcb) } else { - if (!admin_user_is_inet_admin(dcb->user)) + if (!admin_user_is_inet_admin(dcb->user, nullptr)) { rval = false; }