Skip to content
This repository has been archived by the owner on Oct 30, 2018. It is now read-only.

mysql_user: fix user_mod on MySQL(-like) 5.7+ (Fixes #3003) #5388

Merged
merged 1 commit into from Nov 29, 2016

Conversation

wouteroostervld
Copy link
Contributor

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

mysql_user

ANSIBLE VERSION
ansible 2.3.0
  config file = 
  configured module search path = Default w/o overrides
SUMMARY

FIxes #3003. This patch fixes adding a cleartext password for a user which has configured auth_plugin other than mysql_native_password on MySQL-(like) 5.7+.

@gregdek
Copy link
Contributor

gregdek commented Oct 26, 2016

Thanks @wouteroostervld. To the current maintainers, @Jmainguy please review according to guidelines (http://docs.ansible.com/ansible/developing_modules.html#module-checklist) and comment with text 'shipit', 'needs_revision' or 'close_me' as appropriate.

[This message brought to you by your friendly Ansibull-bot.]

@wouteroostervld
Copy link
Contributor Author

wouteroostervld commented Nov 3, 2016

This is a security issue because setting a cleartext password for users with auth_plugin other than mysql_native_password in MySQL 5.7+ does nothing, completes 'succesfully' and generates no error or warning.

This specifically goes wrong with percona 5.7. The root-user deafult has the unix-socket plugin configured, so setting pw is a no-op.

@abadger
Copy link
Contributor

abadger commented Nov 3, 2016

@wouteroostervld Does this work on older mysql versions?

@wouteroostervld
Copy link
Contributor Author

wouteroostervld commented Nov 3, 2016

@abadger: no the special syntax is only executed on 5.7+, otherwise it falls back to the old syntax. The MySQL-version check was already available.

@wouteroostervld
Copy link
Contributor Author

But maybe the capability check variable "old_user_mgmt" could be named clearer.

The original intent was to check only the removal of old style user management in 5.7+. But that is not the only thing that is different. Suggestions?

@Jmainguy
Copy link
Contributor

Jmainguy commented Nov 3, 2016

Looks good to my eyeballs, I will test tonight (in a few hours) and update on the results of my testing.

@Jmainguy
Copy link
Contributor

Jmainguy commented Nov 4, 2016

Notes from my testing.

On a default install of mysql (percona in my tests) 5.7
a temporary password is set, and can be viewed in the mysql log.
The password is technically expired (although the mysql command allows you to login with it)
trying to login using ansible will throw the following error.

fatal: [localhost]: FAILED! => {"changed": false, "failed": true, "msg": "unable to connect to database, check login_user and login_password are correct or /root/.my.cnf has the credentials. Exception message: (1862, 'Your password has expired. To log in you must change it using a client that supports expired passwords.')"}

So the password must be changed manually before ansible will work.

Trying to update the password following examples from mysql.com will give the following errors


mysql> SET PASSWORD FOR 'root'@'localhost' = PASSWORD('MyNewPass');
ERROR 1819 (HY000): Your password does not satisfy the current policy requirements
mysql> SET PASSWORD FOR 'root'@'localhost' = PASSWORD('MyNewPass1');
ERROR 1819 (HY000): Your password does not satisfy the current policy requirements
mysql> SET PASSWORD FOR 'root'@'localhost' = PASSWORD('MyNewPass1@#');
Query OK, 0 rows affected, 1 warning (0.00 sec)

Now that the password is changed, ansible will work

[root@centos7 ansible]# vi playbook.yaml                                                                                                                                                      
[root@centos7 ansible]# ansible-playbook -i localhost playbook.yaml                                                                                                                           
 [WARNING]: Host file not found: localhost

 [WARNING]: provided hosts list is empty, only localhost is available


PLAY [Test] **********************************************************************************************************************************************************************************

TASK [mysql_user] ****************************************************************************************************************************************************************************
changed: [localhost]

PLAY RECAP ***********************************************************************************************************************************************************************************
localhost                  : ok=1    changed=1    unreachable=0    failed=0   

[root@centos7 ansible]# ansible-playbook -i localhost playbook.yaml 
 [WARNING]: Host file not found: localhost

 [WARNING]: provided hosts list is empty, only localhost is available


PLAY [Test] **********************************************************************************************************************************************************************************

TASK [mysql_user] ****************************************************************************************************************************************************************************
fatal: [localhost]: FAILED! => {"changed": false, "failed": true, "msg": "unable to connect to database, check login_user and login_password are correct or /root/.my.cnf has the credentials. Exception message: (1045, \"Access denied for user 'root'@'localhost' (using password: YES)\")"}
        to retry, use: --limit @/tmp/ansible/playbook.retry

PLAY RECAP ***********************************************************************************************************************************************************************************
localhost                  : ok=0    changed=0    unreachable=0    failed=1   

[root@centos7 ansible]# vi playbook.^C
[root@centos7 ansible]# cat playbook.yaml 
---

- name: Test
  hosts: localhost
  gather_facts: false
  tasks:
    - mysql_user: name=root password=jimboadasdadasd!$(AA123
[root@centos7 ansible]# mysql -u root -p 
Enter password: 
Welcome to the MySQL monitor.  Commands end with ; or \g.
Your MySQL connection id is 11
Server version: 5.7.15-9 Percona Server (GPL), Release 9, Revision 9f0fd0a

Copyright (c) 2009-2016 Percona LLC and/or its affiliates
Copyright (c) 2000, 2016, Oracle and/or its affiliates. All rights reserved.

Oracle is a registered trademark of Oracle Corporation and/or its
affiliates. Other names may be trademarks of their respective
owners.

Type 'help;' or '\h' for help. Type '\c' to clear the current input statement.

mysql> Bye

https://docs.ansible.com/ansible/mysql_user_module.html doesnt mention that auth_plugin is a configurable option at the moment

@Jmainguy
Copy link
Contributor

Jmainguy commented Nov 4, 2016

@wouteroostervld is it safe to say, this is waiting on #3589 to be merged before this can be merged?

@wouteroostervld
Copy link
Contributor Author

wouteroostervld commented Nov 4, 2016

@Jmainguy: Looks you tested another problem. The test is if you can set a password for a user if it has auth_sock enabled. (This was the case in my 'default install' using the PPA in Ubuntu from Percona.)

My patch just fixed this specific case for a user with auth_socket plugin configured:

https://www.percona.com/blog/2016/03/16/change-user-password-in-mysql-5-7-with-plugin-auth_socket/

It's not depending on #3589. If you look at the code you see it already uses 'ALTER USER ... WITH mysql_native_password' in some cases but not all. It just fixes mysql_user with cleartextpw's to behave the same way as with hashes.

So it's not a feature but a bug.

@Jmainguy
Copy link
Contributor

Jmainguy commented Nov 17, 2016

So, as usual I was super confused.

This PR affects setting the mysql_native_password (which ansible supports atm), and it affects mysql / forks greater than or equal to 5.7, and also all of mariadb. Before we were not explicitly calling out the plugin to use, this PR fixes that so that if another plugin is the default, that the native_password plugin still gets updated (which is the default for everything I have used personally). I tested the PR and it works.

[root@phy01 ansible]# ansible -i hosts all -m mysql_user -a 'name=root password="jimboadasdadasd!$(AA123aa"'                                                                                  
centos7.soh.re | SUCCESS => {
    "changed": true, 
    "user": "root"
}
[root@phy01 ansible]# ansible -i hosts all -m mysql_user -a 'name=root password="jimboadasdadasd!$(AA123aa"'
centos7.soh.re | FAILED! => {
    "changed": false, 
    "failed": true, 
    "msg": "unable to connect to database, check login_user and login_password are correct or /root/.my.cnf has the credentials. Exception message: (1045, \"Access denied for user 'root'@'localhost' (using password: YES)\")"
}
[root@centos7 ~]# mysql -p
Enter password: 
Welcome to the MySQL monitor.  Commands end with ; or \g.
Your MySQL connection id is 21
Server version: 5.7.15-9 Percona Server (GPL), Release 9, Revision 9f0fd0a

Copyright (c) 2009-2016 Percona LLC and/or its affiliates
Copyright (c) 2000, 2016, Oracle and/or its affiliates. All rights reserved.

Oracle is a registered trademark of Oracle Corporation and/or its
affiliates. Other names may be trademarks of their respective
owners.

Type 'help;' or '\h' for help. Type '\c' to clear the current input statement.

I recommend merging this and backporting it as needed.

@ansibot :shipit:

@wouteroostervld Thanks for working this up and explaining it to me.

@jctanner @jimi-c thanks for reminding me to test this and do my job.

@gregdek
Copy link
Contributor

gregdek commented Nov 17, 2016

Thanks again to @wouteroostervld for this PR, and thanks @Jmainguy for reviewing. Marking for inclusion.

[This message brought to you by your friendly Ansibull-bot.]

@jimi-c jimi-c merged commit 30fb384 into ansible:devel Nov 29, 2016
jimi-c pushed a commit that referenced this pull request Nov 29, 2016
jimi-c pushed a commit that referenced this pull request Nov 29, 2016
@jimi-c
Copy link
Member

jimi-c commented Nov 29, 2016

Merged and cherry-picked to stable-2.2 and stable-2.1.

@wouteroostervld wouteroostervld deleted the mysql-57-plus-fix-alter-user branch January 4, 2017 16:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add plugin as parameter for mysql_user
5 participants