Hi, Nikita, On Mar 10, Nikita Malyavin wrote:
revision-id: 9e5d4dfc49b (mariadb-11.4.1-10-g9e5d4dfc49b) parent(s): 929c2e06aae author: Nikita Malyavin committer: Nikita Malyavin timestamp: 2024-02-29 17:24:27 +0100 message:
MDEV-23729 MDEV-32218 INFORMATION_SCHEMA table for user login data
* A new table INFORMATION_SCHEMA.LOGON is introduced.
There were many cases where we lack an INFORMATION_SCHEMA with all user accounts. Forcing users to select from mysql.user is a shame. Let's use the chance and create a table USERS. It doesn't need more columns now than what you've created, but we'll likely create more in the future.
* Upon idea, it stores auxiliary user data related to login/security/resources * An unprivileged user can access their own data, and that is the main difference with what mysql.global_priv provides
exactly!
* The fields are currently: USER, WRONG_PASSWORD_ATTEMPTS, EXPIRATION_TIME
I wanted to write about splitting USER into USER and HOST, but indeed it's just USER everywhere else in INFORMATION_SCHEMA (e.g. DEFINER and GRANTEE columns). And functions like USER() and CURRENT_USER() don't split either. So, you're right, let's consistently use user@host everywhere.
diff --git a/mysql-test/main/information_schema_stats.result b/mysql-test/main/information_schema_stats.result index 352bcbab823..e38788872fa 100644 --- a/mysql-test/main/information_schema_stats.result +++ b/mysql-test/main/information_schema_stats.result ... +connect(localhost,naughty_user,wrong_passwd,test,16000,/home/nik/mariadb/bld/mysql-test/var/tmp/mysqld.1.sock);
here and below - you forgot to replace the path with MASTER_MYSOCK
diff --git a/mysql-test/main/information_schema_stats.test b/mysql-test/main/information_schema_stats.test index fd5171c3fb4..dbcabe45965 100644 --- a/mysql-test/main/information_schema_stats.test +++ b/mysql-test/main/information_schema_stats.test @@ -47,3 +47,69 @@ select * from information_schema.index_statistics where table_schema='test' and select * from information_schema.table_statistics where table_schema='test' and table_name='just_a_test'; set global userstat=@save_userstat; --enable_ps2_protocol + +--echo # +--echo # MDEV-23729 INFORMATION_SCHEMA Table info. about user locked due to +--echo # max_password_errors +--echo # +--echo # MDEV-32218 message to notify end-user N-days prior the password get +--echo # expired +--echo #
I don't see how you test "info about user locked due to max_password_errors". This is the reason for implementing this new table, it has to be tested. At least add select * from information_schema.logon where wrong_password_attemps >= @max_password_errors; and show it it's empty at first and then not empty.
diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 14450a5a610..4d199bf0e61 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -12956,6 +12956,87 @@ int fill_schema_column_privileges(THD *thd, TABLE_LIST *tables, COND *cond) #endif }
+namespace Show +{ + ST_FIELD_INFO users_fields_info[] = + { + Column("USER", Userhost(), NOT_NULL), + Column("WRONG_PASSWORD_ATTEMPTS", SLonglong(), NULLABLE),
@@max_password_errors is documented as "If there is more than this number of failed connect attempts due to invalid password, user will be blocked from further connections until FLUSH_PRIVILEGES" Let's use call incorrect password consistently everywhere. It could be "bad password", "wrong password", "incorrect password", "invalid password", but it should be the same everywhere. I personally think that "invalid" implies some kind if validity check, e.g. not shorter than 10 characters, not equal to the username, etc and a password that doesn't satisfy these validity rules is "invalid". Trying to login with an incorrect password is a different kind of error, a password can be valid but wrong. May be, ask Ian and/or Daniel about it? And then don't forget to update @@max_password_errors help text to match. Also, you don't have a test case for FLUSH PRIVILEGES.
+ Column("EXPIRATION_TIME", SLonglong(), NULLABLE),
PASSWORD_EXPIRATION_TIME, the user account itself does not expire.
+ CEnd() + }; +}; + +static bool ignore_max_password_errors(const ACL_USER *acl_user); + +static int fill_logon_schema_record(THD *thd, TABLE * table, ACL_USER *user) +{ + ulonglong lifetime= user->password_lifetime < 0 + ? default_password_lifetime + : user->password_lifetime; + + bool ignore_password_errors= ignore_max_password_errors(user);
why? I think it's still useful to show how many wrong password attempts were there for an account even if it doesn't get blocked.
+ bool ignore_expiration_date= lifetime == 0; + + /* Skip user if nothing to show */ + if (ignore_password_errors && ignore_expiration_date) + return 0; + + Grantee_str grantee(user->user.str, safe_str(user->host.hostname)); + table->field[0]->store(grantee, strlen(grantee), system_charset_info); + if (ignore_password_errors) + { + table->field[1]->set_null(); + } + else + { + table->field[1]->set_notnull(); + table->field[1]->store(user->password_errors); + } + if (ignore_expiration_date) + { + table->field[2]->set_null(); + } + else + { + table->field[2]->set_notnull(); + table->field[2]->store(user->password_last_changed + + user->password_lifetime * 3600 * 24, true);
I think it'd be more generally useful to show password_last_changed and expiration period separately.
+ } + + return schema_table_store_record(thd, table); +} + +int fill_logon_schema_table(THD *thd, TABLE_LIST *tables, COND *cond) +{ + int res= 0; +#ifndef NO_EMBEDDED_ACCESS_CHECKS + bool see_whole_table= check_global_access(thd, PROCESS_ACL, true) == 0;
I don't think PROCESS_ACL is a very logical choice here. And there's nothing better as far as I can see. May be let's just do if (check_access(thd, SELECT_ACL, "mysql", NULL, NULL, 1, 1)) ? There are many checks like that in e.g. sql_parse.cc
+ + TABLE *table= tables->table; + + if (!see_whole_table) + { + mysql_mutex_lock(&acl_cache->lock); + ACL_USER *cur_user= find_user_exact(thd->security_ctx->priv_host, + thd->security_ctx->priv_user);
1. cur_user can be NULL if someone dropped it while there was an active connection for this user 2. add a test for it
+ + res= fill_logon_schema_record(thd, table, cur_user); + mysql_mutex_unlock(&acl_cache->lock); + return res; + } + + mysql_mutex_lock(&acl_cache->lock); + for (size_t i= 0; res == 0 && i < acl_users.elements; i++) + { + ACL_USER *user= dynamic_element(&acl_users, i, ACL_USER*); + res= fill_logon_schema_record(thd, table, user); + } + mysql_mutex_unlock(&acl_cache->lock); +#endif + return res; +} +
Regards, Sergei Chief Architect, MariaDB Server and security@mariadb.org