[MDEV-24114] show create user cannot display password expired status and password expire interval Created: 2020-11-04  Updated: 2021-02-23  Resolved: 2021-02-23

Status: Closed
Project: MariaDB Server
Component/s: Authentication and Privilege System
Affects Version/s: 10.4.16
Fix Version/s: 10.4.19

Type: Bug Priority: Major
Reporter: Daniel Black Assignee: Robert Bindar
Resolution: Fixed Votes: 0
Labels: None


 Description   

A user that has been set to PASSWORD EXPIRE and not have it unlocked by any of:

  • alter user user2@localhost PASSWORD EXPIRE NEVER
  • alter user user2@localhost PASSWORD EXPIRE INTERVAL 60 DAY
  • alter user user2@localhost PASSWORD EXPIRE DEFAULT

10.4-4d6c6611443f1e0e1cdab34ac6e320031e7f980b

 
MariaDB [(none)]> create user user2@localhost PASSWORD EXPIRE NEVER; show create user user2@localhost; select * from mysql.global_priv where user='user2';
 
Query OK, 0 rows affected (0.001 sec)
 
 
 
+-------------------------------------------------------+
 
| CREATE USER for user2@localhost                       |
 
+-------------------------------------------------------+
 
| CREATE USER `user2`@`localhost` PASSWORD EXPIRE NEVER |
 
+-------------------------------------------------------+
 
1 row in set (0.000 sec)
 
 
 
+-----------+-------+-----------------------------------------------------------------------------------------------------------------------------------+
 
| Host      | User  | Priv                                                                                                                              |
 
+-----------+-------+-----------------------------------------------------------------------------------------------------------------------------------+
 
| localhost | user2 | {"access":0,"plugin":"mysql_native_password","authentication_string":"","password_last_changed":1604464098,"password_lifetime":0} |
 
+-----------+-------+-----------------------------------------------------------------------------------------------------------------------------------+
 
1 row in set (0.001 sec)
 
 
 
MariaDB [(none)]> alter user user2@localhost PASSWORD EXPIRE ; show create user user2@localhost;  select * from mysql.global_priv where user='user2';
 
Query OK, 0 rows affected (0.001 sec)
 
 
 
+-------------------------------------------------+
 
| CREATE USER for user2@localhost                 |
 
+-------------------------------------------------+
 
| CREATE USER `user2`@`localhost` PASSWORD EXPIRE |
 
+-------------------------------------------------+
 
1 row in set (0.000 sec)
 
 
 
+-----------+-------+--------------------------------------------------------------------------------------------------------------------------+
 
| Host      | User  | Priv                                                                                                                     |
 
+-----------+-------+--------------------------------------------------------------------------------------------------------------------------+
 
| localhost | user2 | {"access":0,"plugin":"mysql_native_password","authentication_string":"","password_last_changed":0,"password_lifetime":0} |
 
+-----------+-------+--------------------------------------------------------------------------------------------------------------------------+
 
1 row in set (0.001 sec)
 
 
 
MariaDB [(none)]> alter user user2@localhost PASSWORD EXPIRE NEVER; show create user user2@localhost; select * from mysql.global_priv where user='user2';
 
Query OK, 0 rows affected (0.001 sec)
 
 
 
+-------------------------------------------------+
 
| CREATE USER for user2@localhost                 |
 
+-------------------------------------------------+
 
| CREATE USER `user2`@`localhost` PASSWORD EXPIRE |
 
+-------------------------------------------------+
 
1 row in set (0.000 sec)
 
 
 
+-----------+-------+--------------------------------------------------------------------------------------------------------------------------+
 
| Host      | User  | Priv                                                                                                                     |
 
+-----------+-------+--------------------------------------------------------------------------------------------------------------------------+
 
| localhost | user2 | {"access":0,"plugin":"mysql_native_password","authentication_string":"","password_last_changed":0,"password_lifetime":0} |
 
+-----------+-------+--------------------------------------------------------------------------------------------------------------------------+
 
1 row in set (0.001 sec)
 
 
 
MariaDB [(none)]> alter user user2@localhost PASSWORD EXPIRE INTERVAL 60 DAY; show create user user2@localhost; select * from mysql.global_priv where user='user2
 
';
 
Query OK, 0 rows affected (0.000 sec)
 
 
 
+-------------------------------------------------+
 
| CREATE USER for user2@localhost                 |
 
+-------------------------------------------------+
 
| CREATE USER `user2`@`localhost` PASSWORD EXPIRE |
 
+-------------------------------------------------+
 
1 row in set (0.000 sec)
 
 
 
+-----------+-------+---------------------------------------------------------------------------------------------------------------------------+
 
| Host      | User  | Priv                                                                                                                      |
 
+-----------+-------+---------------------------------------------------------------------------------------------------------------------------+
 
| localhost | user2 | {"access":0,"plugin":"mysql_native_password","authentication_string":"","password_last_changed":0,"password_lifetime":60} |
 
+-----------+-------+---------------------------------------------------------------------------------------------------------------------------+
 
1 row in set (0.001 sec)
 
 
 
MariaDB [(none)]> alter user user2@localhost PASSWORD EXPIRE DEFAULT; show create user user2@localhost; select * from mysql.global_priv where user='user2';
 
Query OK, 0 rows affected (0.000 sec)
 
 
 
+-------------------------------------------------+
 
| CREATE USER for user2@localhost                 |
 
+-------------------------------------------------+
 
| CREATE USER `user2`@`localhost` PASSWORD EXPIRE |
 
+-------------------------------------------------+
 
1 row in set (0.000 sec)
 
 
 
+-----------+-------+---------------------------------------------------------------------------------------------------------------------------+
 
| Host      | User  | Priv                                                                                                                      |
 
+-----------+-------+---------------------------------------------------------------------------------------------------------------------------+
 
| localhost | user2 | {"access":0,"plugin":"mysql_native_password","authentication_string":"","password_last_changed":0,"password_lifetime":-1} |
 
+-----------+-------+---------------------------------------------------------------------------------------------------------------------------+

So I'm assuming that any of the unexpired variants should set password_last_changed=NOW if its 0. At a minimum the `show create user` is incorrect.

 Comments   
Comment by Daniel Black [ 2020-11-04 ]

note after fixing this remove the `select` from the test lock_user https://github.com/mariadb/server/commit/add67826361738688bc08dc3582eac8a0be5e92e#diff-b8b08d216af90c90811fe1d979f72d3359c049ff67ae51cfe1cdfa2d57fd5607R163 and re-record.

Comment by Robert Bindar [ 2020-11-06 ]

I hope I've followed the examples carefully enough and understood properly what you're saying.

This is not a bug, it was a design decision based on mysql compatibility requirements.
PASSWORD EXPIRE can be seen as a manual expiration mechanism, whilst the others, PASSWORD EXPIRE NEVER, PASSWORD EXPIRE DEFAULT, PASSWORD EXPIRE INTERVAL are mechanisms for enforcing automatic expiration policies. These two are separate mechanisms , PASSWORD EXPIRE takes precedence and that's why it appears in the result of SHOW CREATE even when automatic expiration policies are being set at a later point.

About unexpiring a password. The only solution is to just SET PASSWORD, it wasn't a PASSWORD UNEXPIRE option in mysql so we chose to not add one as well. Is it standing in your way somehow?

Comment by Daniel Black [ 2020-11-07 ]

Ok, I didn't watch the video however may the "unexpire" mechanism could be added to https://mariadb.com/kb/en/user-password-expiry/

Comment by Daniel Black [ 2020-11-07 ]

Oh, and the SHOW CREATE USER isn't displaying the updated status by ALTER USER of the user. So a recreation of the user with the SHOW CREATE USER output will be wrong.

Comment by Daniel Black [ 2020-11-16 ]

robertbindar, so we have two concepts of password expiry, its interval, and is current status.

What if `SHOW CREATE USER` was change to output two rows like:

CREATE USER `user2`@`localhost` PASSWORD EXPIRE [NEVER | INTERVAL X DAY]
 
ALTER USER `user2`@`localhost` PASSWORD EXPIRE

This way it captures the creation of the interval and the state. We've still lost the last_password_changed, but its an improvement in restoring the original.

Comment by Robert Bindar [ 2020-12-02 ]

Hi danblack!

I'm not sure how it would be possible to restore `password_last_changed` given how the feature is implemented.
In any case, when you restore an user account based on output from `SHOW CREATE`, I don't think you should expect to get all the info about the user account as it was originally (e.g. get hidden state info like timestamps and so on).

But having both a `CREATE USER` and an `ALTER USER` statement in the output of `SHOW CREATE USER` sounds like a good solution to me, but I'm worried that it would be a bit confusing to users. I'd say better a bit confusing than wrong though.

One question, are there any precedents to having multiple statements in the output of `SHOW CREATE`? I don't know whether this is an accepted practice, I don't see why it wouldn't be, I just want to make sure we keep the behavior of SHOW consistent.

Comment by Daniel Black [ 2020-12-15 ]

Happy to leave `password_last_changed` as a separate requirement. If you are restoring from backup there may be changes since last backup so could result in an earlier than expected expiry.

User's can look up the documentation and its easily explainable. I'm concerned about tools that use `SHOW CREATE USER` and way to recreate the user and it not being correct.

While none of the SHOW CREATE syntaxes have encounters a CREATE statement that can't reproduce the database item as it was before before (well SHOW CREATE SEQUENCE fails too), SHOW GRANTS certainly does output multiple statements. So its not unexpected.

Comment by Robert Bindar [ 2020-12-15 ]

Hey danblack, I created a fix for this here, thanks a lot for the patience: https://github.com/MariaDB/server/commit/5c39194fbcee92768fff236faf356075bbc7442c

Please feel free to review it, maybe also cvicentiu or serg can take a quick look (nothing too fancy here, I would love to hear their opinion on adding an additional ALTER USER statement in the output).

Comment by Daniel Black [ 2020-12-16 ]

Tested ok from me. Minor style fixes of `{` not on a new line. First use of c++11 closures I've seen. I assume that's supported in 10.4. Thanks Robert.

Comment by Sergei Golubchik [ 2020-12-18 ]

robertbindar:

  • password expiration is a compatibility feature. What does MySQL do here?
  • why did you need a lambda?
  • in our coding style, multi-line comment syntax is /* ... */
Comment by Robert Bindar [ 2020-12-22 ]

Bright mind serg, I got carried away and I forgot the basic argument for this feature. Indeed it is a compatibility feature and MySQL does what we currently do, if a password is both expired and has an automatic INTERVAL set, mysql only prints "PASSWORD EXPIRED". What do you suggest we should do here, leave the feature as is?

I used the lambda to avoid code duplication. I also didn't want to pollute the file with a new function just for that purpose, because that little piece of code was very specific to the show_create function.

Comment by Daniel Black [ 2021-01-13 ]

robertbindar thanks for describing MySQL behavior. While password expiration is compatibility implemented, and in this MDEV we want to ensure that a save/restore generates the same user especially as this part of MDEV-23630. We don't have to implement their bugs/deficiencies.

ref: MDEV-24103 for last_password_changed save/restore, though I'm almost tempted to WONTFIX MDEV-24103 even though the restore isn't perfect and restored users are given extra grace for their password changes.

On lambda's, there's other very specific parts of code in a function. Probably a static function is sufficient. Save lambdas where there's a need to change the function ptr/assignment.

Comment by Robert Bindar [ 2021-01-25 ]

I agree with you, this issue should be fixed as we discussed, but again, not sure this should be a bug fix in 10.4, I'm more inclined to say it should go in 10.6, if we change behavior of existing feature, we should at least do it in our latest version.

About the lambda here, I don't see any problem with it, I see it as a perfectly valid case especially given sql_acl.cc is 14k+ lines of code and a new function would complicate the file one step further for someone that browses through the file looking for some random functionality.

Let me know if the patch is ok to push (except the two comments) and in which version you think we should push it.

Comment by Daniel Black [ 2021-01-27 ]

If we push to 10.6, for users (and our mysqldump) we've created a need to a) implement a 10.4 work around. b) implement a 10.6 special case to back to what they probably have now. Code that takes SHOW CREATE USER output and only reads the the first row is no worse off. For those that read all rows it becomes a compatible enough to recreate the user as it was previously. I'm not sure I see a case of writing code where the additional row actually breaks anything (maybe if a coder added appended ";" outside the loop, not sure accounting for that case is worth it). So please, 10.4.

GeoffMontee, I believe SkySQL might use this. Do you see a problem with the proposed patch?

Lambdas, I can't tell if serg still has a objection after your rational.

Last Review comment on patch:
add_user_parameters isn't needed for the ALTER USER part. Its already included in the `CREATE USER` output and we'd just be duplicating any configuration there. So remove that one line.
Don't forget multi-line comment syntax above.

Comment by Robert Bindar [ 2021-02-01 ]

Thanks for the thorough answer danblack and good catch on add_user_parameters (i removed it but I needed to add the hostname because that function was adding the hostname inside). Once tests are ready I will push the patch if no more objections arrive.

Comment by Robert Bindar [ 2021-02-23 ]

Pushed to 10.4

Generated at Thu Feb 08 09:27:32 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.