From b3df1ec97aacc27678c44eefe56ea8680456d608 Mon Sep 17 00:00:00 2001 From: Tingyao Nian Date: Thu, 19 May 2022 19:18:26 +0000 Subject: [PATCH] MDEV-24815 Add 'allow-suspicious-udfs' and 'skip-grant-tables' to system variables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make two existing command line options "allow-suspicious-udfs" and "skip-grant-tables" visible as global system variables. Both options have security implications, but users were not able to check their states in the server prior to this change. This was a security issue, as the user may not be aware if the options are enabled. By adding them into system variables, it increases users’ visibility into their security configurations. Create new MTR tests to verify that the system variables align with the command line options. Minor adjustments to the existing MTR due to the new members in system variables. Before: mysql> SHOW VARIABLES WHERE Variable_Name LIKE 'allow_suspicious_udfs' OR Variable_Name LIKE 'skip_grant_tables'; Empty set (0.000 sec) After: mysql> SHOW VARIABLES WHERE Variable_Name LIKE 'allow_suspicious_udfs' OR Variable_Name LIKE 'skip_grant_tables'; +-----------------------+-------+ | Variable_name | Value | +-----------------------+-------+ | allow_suspicious_udfs | OFF | | skip_grant_tables | OFF | +-----------------------+-------+ All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc. --- mysql-test/main/mysql_upgrade-6984.result | 1 + mysql-test/main/mysql_upgrade-6984.test | 4 ++++ mysql-test/main/mysqld--help.result | 9 ++++---- mysql-test/main/skip_grants.result | 14 +++++++++++++ mysql-test/main/skip_grants.test | 21 ++++++++++++++++++- .../sys_vars/r/allow_suspicious_udfs.result | 11 ++++++++++ .../sys_vars/r/sysvars_server_embedded.result | 20 ++++++++++++++++++ .../r/sysvars_server_notembedded.result | 20 ++++++++++++++++++ .../sys_vars/t/allow_suspicious_udfs.test | 14 +++++++++++++ sql/mysqld.cc | 13 ------------ sql/sys_vars.cc | 14 +++++++++++++ 11 files changed, 123 insertions(+), 18 deletions(-) create mode 100644 mysql-test/suite/sys_vars/r/allow_suspicious_udfs.result create mode 100644 mysql-test/suite/sys_vars/t/allow_suspicious_udfs.test diff --git a/mysql-test/main/mysql_upgrade-6984.result b/mysql-test/main/mysql_upgrade-6984.result index a0ea4607b24..301fdfc3bfd 100644 --- a/mysql-test/main/mysql_upgrade-6984.result +++ b/mysql-test/main/mysql_upgrade-6984.result @@ -168,3 +168,4 @@ connect con1,localhost,root,foo,,,; update mysql.global_priv set priv=json_compact(json_remove(priv, '$.plugin', '$.authentication_string')) where user='root'; flush privileges; set global event_scheduler=OFF; +# restart diff --git a/mysql-test/main/mysql_upgrade-6984.test b/mysql-test/main/mysql_upgrade-6984.test index 48a06bbd542..034310e036f 100644 --- a/mysql-test/main/mysql_upgrade-6984.test +++ b/mysql-test/main/mysql_upgrade-6984.test @@ -26,3 +26,7 @@ set global event_scheduler=OFF; let MYSQLD_DATADIR= `select @@datadir`; --remove_file $MYSQLD_DATADIR/mysql_upgrade_info + +# --skip-grant-tables state may changed during the test. Need to restart the server +# to restore the --skip-grant-tables state. Otherwise MTR's internal check will fail +--source include/restart_mysqld.inc diff --git a/mysql-test/main/mysqld--help.result b/mysql-test/main/mysqld--help.result index 684db62d770..5ada9729b4c 100644 --- a/mysql-test/main/mysqld--help.result +++ b/mysql-test/main/mysqld--help.result @@ -8,10 +8,11 @@ The following specify which files/extra groups are read (specified before remain --defaults-group-suffix=# Additionally read default groups with # appended as a suffix. --allow-suspicious-udfs - Allows use of UDFs consisting of only one symbol xxx() - without corresponding xxx_init() or xxx_deinit(). That - also means that one can load any function from any - library, for example exit() from libc.so + Allows use of user-defined functions (UDFs) consisting of + only one symbol xxx() without corresponding xxx_init() or + xxx_deinit(). That also means that one can load any + function from any library, for example exit() from + libc.so --alter-algorithm[=name] Specify the alter table algorithm. One of: DEFAULT, COPY, INPLACE, NOCOPY, INSTANT diff --git a/mysql-test/main/skip_grants.result b/mysql-test/main/skip_grants.result index f21bfa1da41..fdd7be41095 100644 --- a/mysql-test/main/skip_grants.result +++ b/mysql-test/main/skip_grants.result @@ -137,3 +137,17 @@ drop user baz@baz; # # End of 10.3 tests # +# +# MDEV-24815 Show "--skip-grant-tables" state in SYSTEM VARIABLES +# +SELECT @@skip_grant_tables AS EXPECT_1; +EXPECT_1 +1 +# restart: --skip-skip-grant-tables +SELECT @@skip_grant_tables AS EXPECT_0; +EXPECT_0 +0 +# restart: --skip-grant-tables +# +# End of 10.10 tests +# diff --git a/mysql-test/main/skip_grants.test b/mysql-test/main/skip_grants.test index 7594285aed7..b74cd41b039 100644 --- a/mysql-test/main/skip_grants.test +++ b/mysql-test/main/skip_grants.test @@ -160,7 +160,7 @@ alter user baz@baz identified with mysql_native_password as password("baz"); show create user baz@baz; drop user bar@foo; drop user baz@baz; -# need to restart the server to restore the --skip-grant state +# Need to restart the server to restore the "--skip-grant-tables" state --source include/restart_mysqld.inc --enable_ps_protocol @@ -168,3 +168,22 @@ drop user baz@baz; --echo # --echo # End of 10.3 tests --echo # + +--echo # +--echo # MDEV-24815 Show "--skip-grant-tables" state in SYSTEM VARIABLES +--echo # + +SELECT @@skip_grant_tables AS EXPECT_1; + +# Also check when the server starts without "--skip-grant-table" option +--let $restart_parameters = "--skip-skip-grant-tables" +--source include/restart_mysqld.inc +SELECT @@skip_grant_tables AS EXPECT_0; + +# Need to restart the server to restore the "--skip-grant-tables" state +--let $restart_parameters = "--skip-grant-tables" +--source include/restart_mysqld.inc + +--echo # +--echo # End of 10.10 tests +--echo # diff --git a/mysql-test/suite/sys_vars/r/allow_suspicious_udfs.result b/mysql-test/suite/sys_vars/r/allow_suspicious_udfs.result new file mode 100644 index 00000000000..db0b4749e69 --- /dev/null +++ b/mysql-test/suite/sys_vars/r/allow_suspicious_udfs.result @@ -0,0 +1,11 @@ +# +# MDEV-24815 Show "--allow-suspicious-udfs" state in SYSTEM VARIABLES +# +SELECT @@allow_suspicious_udfs AS EXPECT_0; +EXPECT_0 +0 +# restart: --allow-suspicious-udfs +SELECT @@allow_suspicious_udfs AS EXPECT_1; +EXPECT_1 +1 +# restart: --skip-allow-suspicious-udfs diff --git a/mysql-test/suite/sys_vars/r/sysvars_server_embedded.result b/mysql-test/suite/sys_vars/r/sysvars_server_embedded.result index 906da9edace..d8a76f6719d 100644 --- a/mysql-test/suite/sys_vars/r/sysvars_server_embedded.result +++ b/mysql-test/suite/sys_vars/r/sysvars_server_embedded.result @@ -12,6 +12,16 @@ variable_name not in ( 'log_tc_size','have_sanitizer' ) order by variable_name; +VARIABLE_NAME ALLOW_SUSPICIOUS_UDFS +VARIABLE_SCOPE GLOBAL +VARIABLE_TYPE BOOLEAN +VARIABLE_COMMENT Allows use of user-defined functions (UDFs) consisting of only one symbol xxx() without corresponding xxx_init() or xxx_deinit(). That also means that one can load any function from any library, for example exit() from libc.so +NUMERIC_MIN_VALUE NULL +NUMERIC_MAX_VALUE NULL +NUMERIC_BLOCK_SIZE NULL +ENUM_VALUE_LIST OFF,ON +READ_ONLY YES +COMMAND_LINE_ARGUMENT OPTIONAL VARIABLE_NAME ALTER_ALGORITHM VARIABLE_SCOPE SESSION VARIABLE_TYPE ENUM @@ -3082,6 +3092,16 @@ NUMERIC_BLOCK_SIZE NULL ENUM_VALUE_LIST OFF,ON READ_ONLY YES COMMAND_LINE_ARGUMENT NULL +VARIABLE_NAME SKIP_GRANT_TABLES +VARIABLE_SCOPE GLOBAL +VARIABLE_TYPE BOOLEAN +VARIABLE_COMMENT Start without grant tables. This gives all users FULL ACCESS to all tables. +NUMERIC_MIN_VALUE NULL +NUMERIC_MAX_VALUE NULL +NUMERIC_BLOCK_SIZE NULL +ENUM_VALUE_LIST OFF,ON +READ_ONLY YES +COMMAND_LINE_ARGUMENT OPTIONAL VARIABLE_NAME SKIP_NAME_RESOLVE VARIABLE_SCOPE GLOBAL VARIABLE_TYPE BOOLEAN diff --git a/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result b/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result index b9444a92cbe..27f15844b33 100644 --- a/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result +++ b/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result @@ -12,6 +12,16 @@ variable_name not in ( 'log_tc_size','have_sanitizer' ) order by variable_name; +VARIABLE_NAME ALLOW_SUSPICIOUS_UDFS +VARIABLE_SCOPE GLOBAL +VARIABLE_TYPE BOOLEAN +VARIABLE_COMMENT Allows use of user-defined functions (UDFs) consisting of only one symbol xxx() without corresponding xxx_init() or xxx_deinit(). That also means that one can load any function from any library, for example exit() from libc.so +NUMERIC_MIN_VALUE NULL +NUMERIC_MAX_VALUE NULL +NUMERIC_BLOCK_SIZE NULL +ENUM_VALUE_LIST OFF,ON +READ_ONLY YES +COMMAND_LINE_ARGUMENT OPTIONAL VARIABLE_NAME ALTER_ALGORITHM VARIABLE_SCOPE SESSION VARIABLE_TYPE ENUM @@ -3582,6 +3592,16 @@ NUMERIC_BLOCK_SIZE NULL ENUM_VALUE_LIST OFF,ON READ_ONLY YES COMMAND_LINE_ARGUMENT NULL +VARIABLE_NAME SKIP_GRANT_TABLES +VARIABLE_SCOPE GLOBAL +VARIABLE_TYPE BOOLEAN +VARIABLE_COMMENT Start without grant tables. This gives all users FULL ACCESS to all tables. +NUMERIC_MIN_VALUE NULL +NUMERIC_MAX_VALUE NULL +NUMERIC_BLOCK_SIZE NULL +ENUM_VALUE_LIST OFF,ON +READ_ONLY YES +COMMAND_LINE_ARGUMENT OPTIONAL VARIABLE_NAME SKIP_NAME_RESOLVE VARIABLE_SCOPE GLOBAL VARIABLE_TYPE BOOLEAN diff --git a/mysql-test/suite/sys_vars/t/allow_suspicious_udfs.test b/mysql-test/suite/sys_vars/t/allow_suspicious_udfs.test new file mode 100644 index 00000000000..9179cfbef4d --- /dev/null +++ b/mysql-test/suite/sys_vars/t/allow_suspicious_udfs.test @@ -0,0 +1,14 @@ +--echo # +--echo # MDEV-24815 Show "--allow-suspicious-udfs" state in SYSTEM VARIABLES +--echo # + +SELECT @@allow_suspicious_udfs AS EXPECT_0; + +# Restart the server the server with "--allow-suspicious-udfs" option +--let $restart_parameters = "--allow-suspicious-udfs" +--source include/restart_mysqld.inc +SELECT @@allow_suspicious_udfs AS EXPECT_1; + +# Disable "--allow-suspicious-udfs" to restore the original state +--let $restart_parameters = "--skip-allow-suspicious-udfs" +--source include/restart_mysqld.inc diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 9a64fbf2c52..1e30d8f3644 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -6342,13 +6342,6 @@ struct my_option my_long_options[]= {"help", '?', "Display this help and exit.", &opt_help, &opt_help, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0}, - {"allow-suspicious-udfs", 0, - "Allows use of UDFs consisting of only one symbol xxx() " - "without corresponding xxx_init() or xxx_deinit(). That also means " - "that one can load any function from any library, for example exit() " - "from libc.so", - &opt_allow_suspicious_udfs, &opt_allow_suspicious_udfs, - 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0}, {"ansi", 'a', "Use ANSI SQL syntax instead of MySQL syntax. This mode " "will also set transaction isolation level 'serializable'.", 0, 0, 0, GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0}, @@ -6673,12 +6666,6 @@ struct my_option my_long_options[]= GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0}, {"silent-startup", OPT_SILENT, "Don't print [Note] to the error log during startup.", &opt_silent_startup, &opt_silent_startup, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0}, -#ifndef DISABLE_GRANT_OPTIONS - {"skip-grant-tables", 0, - "Start without grant tables. This gives all users FULL ACCESS to all tables.", - &opt_noacl, &opt_noacl, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, - 0}, -#endif {"skip-host-cache", OPT_SKIP_HOST_CACHE, "Don't cache host names.", 0, 0, 0, GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0}, {"skip-slave-start", 0, diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc index da308f5b734..fdc17fddd7e 100644 --- a/sql/sys_vars.cc +++ b/sql/sys_vars.cc @@ -2677,6 +2677,20 @@ static Sys_var_mybool Sys_old_mode( SESSION_VAR(old_mode), CMD_LINE(OPT_ARG), DEFAULT(FALSE), 0, NOT_IN_BINLOG, ON_CHECK(0), ON_UPDATE(set_old_mode), DEPRECATED("'@@old_mode'")); +static Sys_var_mybool Sys_opt_allow_suspicious_udfs( + "allow_suspicious_udfs", + "Allows use of user-defined functions (UDFs) consisting of only one symbol xxx() without corresponding xxx_init() or xxx_deinit(). That also means that one can load any function from any library, for example exit() from libc.so", + READ_ONLY GLOBAL_VAR(opt_allow_suspicious_udfs), + CMD_LINE(OPT_ARG), DEFAULT(FALSE)); + +#ifndef DISABLE_GRANT_OPTIONS +static Sys_var_mybool Sys_skip_grant_tables( + "skip_grant_tables", + "Start without grant tables. This gives all users FULL ACCESS to all tables.", + READ_ONLY GLOBAL_VAR(opt_noacl), + CMD_LINE(OPT_ARG), DEFAULT(FALSE)); +#endif + static const char *alter_algorithm_modes[]= {"DEFAULT", "COPY", "INPLACE", "NOCOPY", "INSTANT", NULL};