Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TestAbnormalMySQLTable has multiple issues. #54924

Closed
dveeden opened this issue Jul 26, 2024 · 0 comments · Fixed by #54925
Closed

TestAbnormalMySQLTable has multiple issues. #54924

dveeden opened this issue Jul 26, 2024 · 0 comments · Fixed by #54925
Labels
severity/minor type/bug The issue is confirmed as a bug.

Comments

@dveeden
Copy link
Contributor

dveeden commented Jul 26, 2024

Bug Report

TestAbnormalMySQLTable:

func TestAbnormalMySQLTable(t *testing.T) {
store := createStoreAndPrepareDB(t)
tk := testkit.NewTestKit(t, store)
// Simulate the case mysql.user is synchronized from MySQL.
tk.MustExec("DROP TABLE mysql.user;")
tk.MustExec("USE mysql;")
tk.MustExec(`CREATE TABLE user (
Host char(60) COLLATE utf8_bin NOT NULL DEFAULT '',
User char(16) COLLATE utf8_bin NOT NULL DEFAULT '',
Password char(41) CHARACTER SET latin1 COLLATE latin1_bin NOT NULL DEFAULT '',
Select_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Insert_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Update_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Delete_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Create_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Drop_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Reload_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Shutdown_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Process_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
File_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Config_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Grant_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
References_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Index_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Alter_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Show_db_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Super_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Create_tmp_table_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Lock_tables_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Execute_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Repl_slave_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Repl_client_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Create_view_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Show_view_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Create_routine_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Alter_routine_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Create_user_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Event_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Trigger_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Create_tablespace_priv enum('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
Create_role_priv ENUM('N','Y') NOT NULL DEFAULT 'N',
Drop_role_priv ENUM('N','Y') NOT NULL DEFAULT 'N',
Account_locked ENUM('N','Y') NOT NULL DEFAULT 'N',
ssl_type enum('','ANY','X509','SPECIFIED') CHARACTER SET utf8 NOT NULL DEFAULT '',
ssl_cipher blob NOT NULL,
x509_issuer blob NOT NULL,
x509_subject blob NOT NULL,
max_questions int(11) unsigned NOT NULL DEFAULT '0',
max_updates int(11) unsigned NOT NULL DEFAULT '0',
max_connections int(11) unsigned NOT NULL DEFAULT '0',
max_user_connections int(11) unsigned NOT NULL DEFAULT '0',
plugin char(64) COLLATE utf8_bin DEFAULT 'mysql_native_password',
authentication_string text COLLATE utf8_bin,
token_issuer varchar(255),
user_attributes json,
password_expired ENUM('N','Y') CHARACTER SET utf8 NOT NULL DEFAULT 'N',
password_last_changed TIMESTAMP DEFAULT CURRENT_TIMESTAMP(),
password_lifetime SMALLINT UNSIGNED,
PRIMARY KEY (Host,User)
) ENGINE=MyISAM DEFAULT CHARSET=utf8 COLLATE=utf8_bin COMMENT='Users and global privileges';`)
tk.MustExec(`INSERT INTO user VALUES ('localhost','root','','Y','Y','Y','Y','Y','Y','Y','Y','Y','Y','Y','Y','Y','Y','Y','Y','Y','Y','Y','Y','Y','Y','Y','Y','Y','Y','Y','Y','Y','Y','Y','Y','Y','','','','',0,0,0,0,'mysql_native_password','', '', 'null', 'N', current_timestamp(), null);
`)
var p privileges.MySQLPrivilege
require.NoError(t, p.LoadUserTable(tk.Session()))
activeRoles := make([]*auth.RoleIdentity, 0)
// MySQL mysql.user table schema is not identical to TiDB, check it doesn't break privilege.
require.True(t, p.RequestVerification(activeRoles, "root", "localhost", "test", "", "", mysql.SelectPriv))
// Absent of those tables doesn't cause error.
tk.MustExec("DROP TABLE mysql.db;")
tk.MustExec("DROP TABLE mysql.tables_priv;")
tk.MustExec("DROP TABLE mysql.columns_priv;")
require.NoError(t, p.LoadAll(tk.Session()))
}

To me it looks like this test was meant to test if authentication in TiDB would break if someone/something would accidentally sync the mysql.user table from MySQL. However the current version doesn't match a MySQL mysql.user table anymore.

  • If something like DM or any other tool would accidentaly sync the mysql.user table from MySQL to TiDB this would be a serious problem. So it might be good to test to make sure at least root can still authenticate.
  • Then we should test with the mysql.user structure from MySQL 5.7, 8.0 and 8.4. Maybe also MariaDB 11.x?
  • However if the table is synced then the data in it might be synced as well. This might work for caching_sha2_password or mysql_native_password. However root might use socket auth in some situations or be restricted to 127.0.0.1/::1 which might make this more difficult. And at least MariaDB has some different auth plugins that we don't support.
  • There is another solution, which I think might be better: Rely on skip_grant_tables. This should work for all root authentication issues including this one.
  • I don't think we would ever officially support syncing mysql.user from a non-TiDB upstream. So this would only be a best effort action to reduce the damage.
  • The test currently doesn't include versions of the mysql.user table from previous versions of TiDB.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
severity/minor type/bug The issue is confirmed as a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant