Bug#26576989 DROP ROLE CAN MAKE SERVER UNUSABLE

This patch fix two issues with the global variable mandatory_roles: * It was possible to setting anonymous authorization IDs as mandatory roles. This could lead to server instability * The additional privilege ROLE_ADMIN is now required to set the mandatory_roles variable.
HopHippy · Aug 24, 2017 · 5231f2d · 5231f2d
1 parent a5116d4
commit 5231f2d
Show file tree

Hide file tree

Showing 5 changed files with 124 additions and 8 deletions.
diff --git a/mysql-test/suite/auth_sec/r/mandatory_roles.result b/mysql-test/suite/auth_sec/r/mandatory_roles.result
@@ -206,3 +206,41 @@ GRANT USAGE ON *.* TO `u1`@`localhost`
 GRANT `r1`@`%` TO `u1`@`localhost`
 SET GLOBAL mandatory_roles="PUBLIC";
 DROP USER u1@localhost, r1, r2, r3;
+
+# Anonymous roles aren't allowed
+SET GLOBAL mandatory_roles='';
+CREATE USER ''@'localhost';
+GRANT ALL ON sys.* TO ''@'localhost';
+SET GLOBAL mandatory_roles='``@`localhost`';
+ERROR 42000: Variable 'mandatory_roles' can't be set to the value of '``@`localhost`'
+SHOW GRANTS;
+Grants for root@localhost
+GRANT SELECT, INSERT, UPDATE, DELETE, CREATE, DROP, RELOAD, SHUTDOWN, PROCESS, FILE, REFERENCES, INDEX, ALTER, SHOW DATABASES, SUPER, CREATE TEMPORARY TABLES, LOCK TABLES, EXECUTE, REPLICATION SLAVE, REPLICATION CLIENT, CREATE VIEW, SHOW VIEW, CREATE ROUTINE, ALTER ROUTINE, CREATE USER, EVENT, TRIGGER, CREATE TABLESPACE, CREATE ROLE, DROP ROLE ON *.* TO `root`@`localhost` WITH GRANT OPTION
+GRANT BACKUP_ADMIN,BINLOG_ADMIN,CONNECTION_ADMIN,ENCRYPTION_KEY_ADMIN,GROUP_REPLICATION_ADMIN,PERSIST_RO_VARIABLES_ADMIN,REPLICATION_SLAVE_ADMIN,RESOURCE_GROUP_ADMIN,RESOURCE_GROUP_USER,ROLE_ADMIN,SET_USER_ID,SYSTEM_VARIABLES_ADMIN,XA_RECOVER_ADMIN ON *.* TO `root`@`localhost` WITH GRANT OPTION
+GRANT PROXY ON ''@'' TO 'root'@'localhost' WITH GRANT OPTION
+CREATE USER u1@localhost identified by 'foo';
+GRANT SELECT ON *.* To u1@localhost;
+SELECT CURRENT_USER();
+CURRENT_USER()
+u1@localhost
+# Should not show any anonymous mandatory role
+SHOW GRANTS;
+Grants for u1@localhost
+GRANT SELECT ON *.* TO `u1`@`localhost`
+GRANT SYSTEM_VARIABLES_ADMIN ON *.* TO u1@localhost;
+SELECT CURRENT_USER();
+CURRENT_USER()
+u1@localhost
+SHOW GRANTS;
+Grants for u1@localhost
+GRANT SELECT ON *.* TO `u1`@`localhost`
+GRANT SYSTEM_VARIABLES_ADMIN ON *.* TO `u1`@`localhost`
+SET GLOBAL mandatory_roles='';
+ERROR 42000: Access denied; you need (at least one of) the ROLE_ADMIN, SUPER or SYSTEM_VARIABLES_ADMIN privilege(s) for this operation
+SET GLOBAL mandatory_roles='foo@localhost';
+ERROR 42000: Access denied; you need (at least one of) the ROLE_ADMIN, SUPER or SYSTEM_VARIABLES_ADMIN privilege(s) for this operation
+GRANT ''@'localhost' TO u1@localhost;
+ERROR HY000: Failed to grant ``@`localhost`` to `u1`@`localhost`
+SET GLOBAL mandatory_roles='';
+DROP USER u1@localhost, ''@'localhost';
+SET GLOBAL mandatory_roles="PUBLIC";
diff --git a/mysql-test/suite/auth_sec/t/mandatory_roles.test b/mysql-test/suite/auth_sec/t/mandatory_roles.test
@@ -150,3 +150,33 @@ connection default;
 SET GLOBAL mandatory_roles="PUBLIC";
 DROP USER u1@localhost, r1, r2, r3;
 
+--echo
+--echo # Anonymous roles aren't allowed
+SET GLOBAL mandatory_roles='';
+CREATE USER ''@'localhost';
+GRANT ALL ON sys.* TO ''@'localhost';
+--error ER_WRONG_VALUE_FOR_VAR
+SET GLOBAL mandatory_roles='``@`localhost`';
+SHOW GRANTS;
+CREATE USER u1@localhost identified by 'foo';
+GRANT SELECT ON *.* To u1@localhost;
+connect(con2,localhost,u1,foo,test,,,);
+SELECT CURRENT_USER();
+--echo # Should not show any anonymous mandatory role
+SHOW GRANTS;
+connection default;
+GRANT SYSTEM_VARIABLES_ADMIN ON *.* TO u1@localhost;
+connection con2;
+SELECT CURRENT_USER();
+SHOW GRANTS;
+--error ER_SPECIFIC_ACCESS_DENIED_ERROR
+SET GLOBAL mandatory_roles='';
+--error ER_SPECIFIC_ACCESS_DENIED_ERROR
+SET GLOBAL mandatory_roles='foo@localhost';
+connection default;
+--error ER_FAILED_ROLE_GRANT
+GRANT ''@'localhost' TO u1@localhost;
+SET GLOBAL mandatory_roles='';
+DROP USER u1@localhost, ''@'localhost';
+SET GLOBAL mandatory_roles="PUBLIC";
+
diff --git a/sql/auth/auth_common.h b/sql/auth/auth_common.h
@@ -865,6 +865,7 @@ bool do_auto_cert_generation(ssl_artifacts_status auto_detection_status);
 #define DEFAULT_SSL_SERVER_KEY  "server-key.pem"
 
 void update_mandatory_roles(void);
+bool check_authorization_id_string(const char *buffer, size_t length);
 String *func_current_role(THD *thd, String *str, String *active_role);
 
 extern volatile uint32 global_password_history, global_password_reuse_interval;

diff --git a/sql/auth/sql_authorization.cc b/sql/auth/sql_authorization.cc
@@ -3204,8 +3204,17 @@ bool mysql_grant_role(THD *thd, const List <LEX_USER > *users,
     while ((role= roles_it++) && !errors)
     {
       ACL_USER *acl_role;
-      if ((acl_role= find_acl_user(role->host.str,
-                                   role->user.str, true)) == NULL)
+      if (role->user.length == 0 || *(role->user.str) == '\0')
+      {
+        /* Anonymous roles aren't allowed */
+        errors= true;
+        std::string user_str= create_authid_str_from(acl_user);
+        std::string role_str= create_authid_str_from(role);
+        my_error(ER_FAILED_ROLE_GRANT, MYF(0), role_str.c_str(),
+                 user_str.c_str());
+        break;
+      } else if ((acl_role= find_acl_user(role->host.str,
+                                          role->user.str, true)) == NULL)
       {
         my_error(ER_UNKNOWN_AUTHID, MYF(0),
                  const_cast<char *>(role->user.str),
@@ -7343,6 +7352,20 @@ bool assert_valid_privilege_id(const List<st_lex_user>* priv_list)
   return true;
 }
 
+bool check_authorization_id_string(const char *buffer, size_t length)
+{
+  bool error= false;
+  std::string authid_str(buffer, length);
+  iterate_comma_separated_quoated_string(authid_str,
+            [&error](const std::string item){
+              auto el= get_authid_from_quoted_string(item);
+              if (el.second != "" && el.first == "")
+                error= true;
+              return error;
+    });
+  return error;
+}
+
 void get_mandatory_roles(std::vector< Role_id > *mandatory_roles)
 {
   mysql_mutex_lock(&LOCK_mandatory_roles);
@@ -7368,8 +7391,16 @@ void get_mandatory_roles(std::vector< Role_id > *mandatory_roles)
             if (el.second == "")
               el.second= "%";
             Role_id role_id(el.first, el.second);
-            if (find_acl_user(role_id.host().c_str(), role_id.user().c_str(),
-                              true) != NULL)
+            if (role_id.user() == "")
+            {
+              sql_print_warning("Can't set mandatory_role %s@%s: Anonymous "
+                                "authorization IDs are not allowed as roles.",
+                                role_id.user().c_str(),
+                                role_id.host().c_str());
+            }
+            else if (find_acl_user(role_id.host().c_str(),
+                                   role_id.user().c_str(),
+                                   true) != NULL)
             {
               if (std::find(g_mandatory_roles->begin(),
                             g_mandatory_roles->end(),

diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc
@@ -6406,17 +6406,33 @@ static Sys_var_bool Sys_persisted_globals_load(
        ON_CHECK(0),
        ON_UPDATE(0));
 
-static bool check_authid_string(sys_var*, THD*, set_var *var)
+static bool sysvar_check_authid_string(sys_var*, THD *thd, set_var *var)
 {
+  /*
+    Since mandatory_roles is similar to a GRANT role statement without a
+    GRANT ADMIN privilege, setting this variable requires both the
+    ROLE_ADMIN and the SYSTEM_VARIABLES_ADMIN.
+  */
+  Security_context *sctx= thd->security_context();
+  DBUG_ASSERT(sctx != 0);
+  if (sctx && !sctx->has_global_grant(STRING_WITH_LEN("ROLE_ADMIN")).first)
+  {
+    my_error(ER_SPECIFIC_ACCESS_DENIED_ERROR, MYF(0),
+             "ROLE_ADMIN, SUPER or SYSTEM_VARIABLES_ADMIN");
+    /* No privilege access error */
+    return true;
+  }
   if (var->save_result.string_value.str == 0)
   {
     var->save_result.string_value.str= const_cast<char*>("");
     var->save_result.string_value.length= 0;
   }
-  return false;
+  return check_authorization_id_string(var->save_result.string_value.str,
+                                       var->save_result.string_value.length);
 }
 
-static bool sysvar_update_mandatory_roles(sys_var*, THD*, enum_var_type)
+static bool sysvar_update_mandatory_roles(sys_var *, THD *,
+                                          enum_var_type)
 {
   update_mandatory_roles();
   return false;
@@ -6430,7 +6446,7 @@ static Sys_var_lexstring Sys_mandatory_roles(
   "default roles. The granted roles will not be visible in the mysql.role_edges"
   " table.", GLOBAL_VAR(opt_mandatory_roles), CMD_LINE(REQUIRED_ARG),
   IN_SYSTEM_CHARSET, DEFAULT(""), &PLock_sys_mandatory_roles, NOT_IN_BINLOG,
-  ON_CHECK(check_authid_string), ON_UPDATE(sysvar_update_mandatory_roles));
+  ON_CHECK(sysvar_check_authid_string), ON_UPDATE(sysvar_update_mandatory_roles));
 
 static Sys_var_bool Sys_always_activate_granted_roles(
        "activate_all_roles_on_login",