tidb-dashboard icon indicating copy to clipboard operation
tidb-dashboard copied to clipboard

Unpermitted users can sign in

Open mikecaat opened this issue 3 years ago • 0 comments

Bug Report

Hi, thanks for developing the useful dashboard. I'm evaluating whether this can be used in production environments. I found the behavior that unpermitted users can sign in. Does it expect results?

What did you do?

  • execute as root user
CREATE USER test;
CREATE DATABASE test_db;
GRANT ALL ON `test_db`.* TO test;    -- only  *database-level*

What did you expect to see?

The test user can not sign in.

I expected the test user must be rejected because the user doesn't have at least PROCESS and CONFIG privileges. (By the way, I couldn't find how to check if it has DASHBOARD_CLIENT or not. If you know, please let me know.)

MySQL [(none)]> SELECT * FROM mysql.user WHERE User = 'test'\G
*************************** 1. row ***************************
                  Host: %
                  User: test
 authentication_string:
                plugin: mysql_native_password
           Select_priv: N
           Insert_priv: N
           Update_priv: N
           Delete_priv: N
           Create_priv: N
             Drop_priv: N
          Process_priv: N
            Grant_priv: N
       References_priv: N
            Alter_priv: N
          Show_db_priv: N
            Super_priv: N
 Create_tmp_table_priv: N
      Lock_tables_priv: N
          Execute_priv: N
      Create_view_priv: N
        Show_view_priv: N
   Create_routine_priv: N
    Alter_routine_priv: N
            Index_priv: N
      Create_user_priv: N
            Event_priv: N
       Repl_slave_priv: N
      Repl_client_priv: N
          Trigger_priv: N
      Create_role_priv: N
        Drop_role_priv: N
        Account_locked: N
         Shutdown_priv: N
           Reload_priv: N
             FILE_priv: N
           Config_priv: N
Create_Tablespace_Priv: N
1 row in set (0.04 sec)

What did you see instead?

The user can sign in.

I assume that ALL PRIVILEGES can sign in in the following document is the point. But, I think it's better to add the condition if the user has global level ALL PRIVILEGES or not. I don't think it's a good behavior to allow a user with privileges only on a specific database to view cluster-level information.

Users with high privileges such as ALL PRIVILEGES or SUPER can sign in to TiDB Dashboard as well. Therefore, to comply with the least privilege principle, it is highly recommended that you create users with the required privileges only to prevent unintended operations. See Privilege Management for more information on these privileges.

https://docs.pingcap.com/tidb/v6.1/dashboard-user

The logic doesn't consider the ALL PRIVILEGES is global level or not.

https://github.com/pingcap/tidb-dashboard/blob/d02b6af853a3cac31c7a4c031347b8f6f9be9732/pkg/apiserver/user/verify_sql_user.go#L71

https://github.com/pingcap/tidb-dashboard/blob/d02b6af853a3cac31c7a4c031347b8f6f9be9732/pkg/apiserver/user/verify_sql_user.go#L122

What version of TiDB Dashboard are you using (./tidb-dashboard --version)?

MySQL [(none)]> SHOW GLOBAL VARIABLES LIKE 'tidb_enable_enhanced_security';
+-------------------------------+-------+
| Variable_name                 | Value |
+-------------------------------+-------+
| tidb_enable_enhanced_security | OFF   |
+-------------------------------+-------+
1 row in set (0.00 sec)

MySQL [(none)]> SELECT tidb_version();
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| tidb_version()                                                                                                                                                                                                                                                                                                       |
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| Release Version: v6.1.1
Edition: Community
Git Commit Hash: 5263a0abda61f102122735049fd0dfadc7b7f8b2
Git Branch: heads/refs/tags/v6.1.1
UTC Build Time: 2022-08-25 10:42:41
GoVersion: go1.18.5
Race Enabled: false
TiKV Min Version: v3.0.0-60965b006877ca7234adaced7890d7b029ed1306
Check Table Before Drop: false |
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+

mikecaat avatar Oct 20 '22 02:10 mikecaat