From 828649ba996f17b91e54d67a3a462cde1b1bf759 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Wed, 16 Aug 2017 14:29:58 +0300 Subject: [PATCH] MXS-1354: Add user authorization to maxadmin All commands that modify the internal state of MaxScale now require admin level authorization. --- include/maxscale/adminusers.h.in | 3 +- server/core/admin.cc | 2 +- server/core/adminusers.cc | 26 ++++++++++++-- server/modules/protocol/maxscaled/maxscaled.c | 7 +++- server/modules/routing/debugcli/debugcmd.c | 34 +++++++++++++++++++ 5 files changed, 66 insertions(+), 6 deletions(-) diff --git a/include/maxscale/adminusers.h.in b/include/maxscale/adminusers.h.in index 2a39bc71a..94b137a94 100644 --- a/include/maxscale/adminusers.h.in +++ b/include/maxscale/adminusers.h.in @@ -82,7 +82,8 @@ 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_is_admin_user(const char* username); +bool admin_user_is_inet_admin(const char* username); +bool admin_user_is_unix_admin(const char* username); bool admin_have_admin(); /** diff --git a/server/core/admin.cc b/server/core/admin.cc index 9b9b1cb06..a113ac6ed 100644 --- a/server/core/admin.cc +++ b/server/core/admin.cc @@ -191,7 +191,7 @@ bool do_auth(MHD_Connection* connection, const char* url, const char* method) send_auth_error(connection); rval = false; } - else if (!admin_is_admin_user(user) && modifies_data(connection, method)) + else if (!admin_user_is_inet_admin(user) && 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 59fe582db..8d3143964 100644 --- a/server/core/adminusers.cc +++ b/server/core/adminusers.cc @@ -177,7 +177,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 = admin_is_admin_user(user) ? USER_ACCOUNT_ADMIN : USER_ACCOUNT_BASIC; + user_account_type account = admin_user_is_inet_admin(user) ? USER_ACCOUNT_ADMIN : USER_ACCOUNT_BASIC; std::string path = path_from_type(type); path += "/"; path += user; @@ -477,14 +477,34 @@ admin_verify_inet_user(const char *username, const char *password) return rv; } -bool admin_is_admin_user(const char* username) +bool admin_user_is_inet_admin(const char* username) { - bool rval = true; // The default `admin:mariadb` user has all permissions + bool rval = false; if (inet_users) { rval = users_is_admin(inet_users, username); } + else if (strcmp(INET_DEFAULT_USERNAME, username) == 0) + { + rval = true; + } + + return rval; +} + +bool admin_user_is_unix_admin(const char* username) +{ + bool rval = false; + + if (linux_users) + { + rval = users_is_admin(linux_users, username); + } + else if (strcmp(DEFAULT_ADMIN_USER, username) == 0) + { + rval = true; + } return rval; } diff --git a/server/modules/protocol/maxscaled/maxscaled.c b/server/modules/protocol/maxscaled/maxscaled.c index 91c7fcab3..100913cd0 100644 --- a/server/modules/protocol/maxscaled/maxscaled.c +++ b/server/modules/protocol/maxscaled/maxscaled.c @@ -241,7 +241,12 @@ static int maxscaled_read_event(DCB* dcb) { case MAXSCALED_STATE_LOGIN: { - maxscaled->username = strndup((char*)GWBUF_DATA(head), GWBUF_LENGTH(head)); + size_t len = GWBUF_LENGTH(head); + char user[len + 1]; + memcpy(user, GWBUF_DATA(head), len); + user[len] = '\0'; + maxscaled->username = MXS_STRDUP_A(user); + dcb->user = MXS_STRDUP_A(user); maxscaled->state = MAXSCALED_STATE_PASSWD; dcb_printf(dcb, MAXADMIN_AUTH_PASSWORD_PROMPT); gwbuf_free(head); diff --git a/server/modules/routing/debugcli/debugcmd.c b/server/modules/routing/debugcli/debugcmd.c index 243564717..50120129b 100644 --- a/server/modules/routing/debugcli/debugcmd.c +++ b/server/modules/routing/debugcli/debugcmd.c @@ -1774,6 +1774,11 @@ static struct { NULL, NULL } }; +static bool command_requires_admin_privileges(const char* cmd) +{ + return strcmp(cmd, "list") != 0 && strcmp(cmd, "show") != 0; +} + /** * Convert a string argument to a numeric, observing prefixes * for number bases, e.g. 0x for hex, 0 for octal @@ -1846,6 +1851,28 @@ static void free_arg(int arg_type, void *value) } } +static bool user_is_authorized(DCB* dcb) +{ + bool rval = true; + + if (strcmp(dcb->remote, "localhost") == 0) + { + if (!admin_user_is_unix_admin(dcb->user)) + { + rval = false; + } + } + else + { + if (!admin_user_is_inet_admin(dcb->user)) + { + rval = false; + } + } + + return rval; +} + static SPINLOCK debugcmd_lock = SPINLOCK_INIT; static const char item_separator[] = @@ -2006,6 +2033,13 @@ execute_cmd(CLI_SESSION *cli) { found = 1; /**< command and sub-command match */ + if (command_requires_admin_privileges(cmds[i].cmd) && + !user_is_authorized(dcb)) + { + dcb_printf(dcb, "Access denied, administrative privileges required.\n"); + break; + } + if (cmds[i].options[j].argc_min == cmds[i].options[j].argc_max && argc != cmds[i].options[j].argc_min) {