From 6cc3986db5d2b0bee4e2f37b0fc698a8930c11f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 17 Aug 2017 09:59:30 +0300 Subject: [PATCH] MXS-1354: Prevent removal of last admin account Removing the last admin account is now forbidden. This should prevent most cases where users could lock themselves out of the administrative interface. This change does allow a non-root network user to be the last admin account. In practice this does not prevent the root user from gaining access to maxadmin. Access can be gained by removing the users file and restarting MaxScale or by editing the users file by hand. --- include/maxscale/adminusers.h.in | 1 + include/maxscale/users.h | 6 ++--- server/core/adminusers.cc | 19 +++++++-------- server/core/users.cc | 8 +++---- server/modules/routing/debugcli/debugcmd.c | 27 +++++++++++++--------- 5 files changed, 34 insertions(+), 27 deletions(-) diff --git a/include/maxscale/adminusers.h.in b/include/maxscale/adminusers.h.in index 94b137a94..1141d1dfc 100644 --- a/include/maxscale/adminusers.h.in +++ b/include/maxscale/adminusers.h.in @@ -85,6 +85,7 @@ bool admin_verify_inet_user(const char *uname, const char *password); bool admin_user_is_inet_admin(const char* username); bool admin_user_is_unix_admin(const char* username); bool admin_have_admin(); +bool admin_is_last_admin(const char* user); /** * @brief Convert all admin users to JSON diff --git a/include/maxscale/users.h b/include/maxscale/users.h index 5af73815b..0ca53f5d8 100644 --- a/include/maxscale/users.h +++ b/include/maxscale/users.h @@ -109,13 +109,13 @@ bool users_find(USERS* users, const char* user); bool users_is_admin(USERS* users, const char* user); /** - * Check if at least one admin account exists + * Check how many admin account exists * * @param users Users to check * - * @return True if at least one admin account exists + * @return Number of admin accounts */ -bool users_have_admin(USERS* users); +int users_admin_count(USERS* users); /** * Dump users as JSON diff --git a/server/core/adminusers.cc b/server/core/adminusers.cc index 8d3143964..da7cf54fa 100644 --- a/server/core/adminusers.cc +++ b/server/core/adminusers.cc @@ -102,12 +102,6 @@ static const char *admin_add_user(USERS** pusers, const char* fname, static const char* admin_remove_user(USERS *users, const char* fname, const char *uname) { - if (strcmp(uname, DEFAULT_ADMIN_USER) == 0) - { - MXS_WARNING("Attempt to delete the default admin user '%s'.", uname); - return ADMIN_ERR_DELROOT; - } - if (!users_delete(users, uname)) { MXS_ERROR("Couldn't find user %s. Removing user failed.", uname); @@ -369,7 +363,7 @@ bool admin_linux_account_enabled(const char *uname) { bool rv = false; - if (strcmp(uname, DEFAULT_ADMIN_USER) == 0) + if (!linux_users && strcmp(uname, DEFAULT_ADMIN_USER) == 0) { rv = true; } @@ -511,8 +505,15 @@ bool admin_user_is_unix_admin(const char* username) bool admin_have_admin() { - return (inet_users && users_have_admin(inet_users)) || - (linux_users && users_have_admin(linux_users)); + return (inet_users && users_admin_count(inet_users) > 0) || + (linux_users && users_admin_count(linux_users) > 0); +} + +bool admin_is_last_admin(const char* user) +{ + return (admin_user_is_inet_admin(user) || admin_user_is_unix_admin(user)) && + ((inet_users ? users_admin_count(inet_users) : 1) + + (linux_users ? users_admin_count(linux_users) : 1)) == 1; } /** diff --git a/server/core/users.cc b/server/core/users.cc index 188fd5a76..436d07b79 100644 --- a/server/core/users.cc +++ b/server/core/users.cc @@ -104,9 +104,9 @@ public: return rval; } - bool have_admin() const + int admin_count() const { - return std::find_if(m_data.begin(), m_data.end(), is_admin) != m_data.end(); + return std::count_if(m_data.begin(), m_data.end(), is_admin); } bool check_permissions(std::string user, user_account_type perm) const @@ -302,10 +302,10 @@ bool users_is_admin(USERS* users, const char* user) return u->check_permissions(user, USER_ACCOUNT_ADMIN); } -bool users_have_admin(USERS* users) +int users_admin_count(USERS* users) { Users* u = reinterpret_cast(users); - return u->have_admin(); + return u->admin_count(); } void users_diagnostic(DCB* dcb, USERS* users) diff --git a/server/modules/routing/debugcli/debugcmd.c b/server/modules/routing/debugcli/debugcmd.c index 50120129b..425cb59d8 100644 --- a/server/modules/routing/debugcli/debugcmd.c +++ b/server/modules/routing/debugcli/debugcmd.c @@ -2343,17 +2343,19 @@ static void telnetdRemoveUser(DCB *dcb, char *user) if (!admin_inet_user_exists(user)) { - dcb_printf(dcb, "Account %s for remote (network) usage does not exist.\n", user); - return; + dcb_printf(dcb, "Account '%s' for remote usage does not exist.\n", user); } - - if ((err = admin_remove_inet_user(user)) == NULL) + else if (admin_is_last_admin(user)) { - dcb_printf(dcb, "Account %s for remote (network) usage has been successfully removed.\n", user); + dcb_printf(dcb, "Cannot remove the last admin account '%s'.\n", user); + } + else if ((err = admin_remove_inet_user(user))) + { + dcb_printf(dcb, "Failed to remove remote account '%s': %s\n", user, err); } else { - dcb_printf(dcb, "Failed to remove remote account %s: %s\n", user, err); + dcb_printf(dcb, "Account '%s' for remote usage has been successfully removed.\n", user); } } @@ -2674,16 +2676,19 @@ disable_account(DCB *dcb, char *user) if (!admin_linux_account_enabled(user)) { - dcb_printf(dcb, "The Linux user %s has not been enabled.\n", user); + dcb_printf(dcb, "The Linux user '%s' has not been enabled.\n", user); return; } - - if ((err = admin_disable_linux_account(user)) == NULL) + else if (admin_is_last_admin(user)) { - dcb_printf(dcb, "The Linux user %s has successfully been disabled.\n", user); + dcb_printf(dcb, "Cannot remove the last admin account '%s'.\n", user); + } + else if ((err = admin_disable_linux_account(user))) + { + dcb_printf(dcb, "Failed to disable the Linux user '%s': %s\n", user, err); } else { - dcb_printf(dcb, "Failed to disable the Linux user %s: %s\n", user, err); + dcb_printf(dcb, "The Linux user '%s' has successfully been disabled.\n", user); } }