module.mysql.user_present and state.mysql_user.present can't properly check user passord

[alexises]

Description of Issue/Question

For mariadb version > 10.2.0 the state salt.state.mysql_user.present and module salt.module.mysql.user_exist can't check properly the user password

on theses version, password check is done trough a simple connection on the mariadbl server using the connection_args dict. This dict contain the connection_host attribute that set the address of mysql server used to check this password.

In case of the user is not authorized to connect trough this address, the password is detected as invalid and state will try to change it.

This issue break idempotence when the admin user and the managed user can't use the same interface to connect.

Setup

Create root user:
  mysql_user.present:
    - host: "localhost"
    - name: root
    - password: rootP@ssword

Create external user:
  mysql_user.present:
    - host: "10.0.0.1"
    - name: externalUser
    - password: externalUserP@ssword
    - connection_user: root
    - connection_host: localhost
    - connection_pass: "rootP@ssword"

Steps to Reproduce Issue

• have a recent mariadb with root login only authorized to login to localhost
• have a user on this server only authorized to login on the host ip
• use the mysql_user.present to manage this host or use module.user_exist to check the user password

an connection_usercheck_host parameter should be added to provides a reliable way to set the proper address to use to check the user authentication.

Versions Report

Salt Version:
           Salt: 2019.2.0
 
Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.5.3
      docker-py: Not Installed
          gitdb: 2.0.0
      gitpython: 2.1.1
          ioflo: Not Installed
         Jinja2: 2.9.4
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.8
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.13 (default, Sep 26 2018, 18:42:22)
   python-gnupg: Not Installed
         PyYAML: 3.12
          PyZMQ: 16.0.2
           RAET: Not Installed
          smmap: 2.0.1
        timelib: Not Installed
        Tornado: 4.4.3
            ZMQ: 4.2.1
 
System Versions:
           dist: debian 9.3 
         locale: UTF-8
        machine: x86_64
        release: 4.9.0-4-amd64
         system: Linux
        version: debian 9.3 

issue is also present on salt 2018.3.4

[Ch3LL]

ping @garethgreenaway i know you have been doing some work in these modules lately any ideas here?

[Reiner030]

Hi @Ch3LL and @garethgreenaway ,

I was first positive astonished that for my upgrade from 2017.7.8 to 2018.3.4 I had only to rename all occurrences of the opts['environment'] to opts['saltenv'].
But when do finally test i was trapped by issue #51258 which got luckily already a patch in #51298 (Fix edge case when minion ID is a 16-character string).

After rollout to production servers which are secured I stepped finally in this weird behavior that MySQL states did't check correctly.

E.g these states:

mysql_users-readonly@127.0.0.1-production:
  mysql_user.present:
    - name: 'readonly'
    - host: '127.0.0.1'
    - password: 'some_pass'
    # connection_default_file isn't working (anymore)? Get with secured root access:
    # MySQL Error 1045: Access denied for user 'root'@'localhost' (using password: NO)
    #- connection_default_file: '/etc/mysql/debian.cnf'
    - connection_host: 'localhost'
    - connection_user: 'debian-sys-maint'
    - connection_pass: 'maintain_pass'
    - connection_unix_socket: '/var/run/mysqld/mysqld.sock'

# create account 'readonly'@'10.30.2.0/255.255.255.0' in environment production
mysql_users-readonly@10.30.2.0/255.255.255.0-production:
  mysql_user.present:
    - name: 'readonly'
    - host: '10.30.2.0/255.255.255.0'
    - password: 'some_pass'
    # connection_default_file isn't working (anymore)? Get with secured root access:
    # MySQL Error 1045: Access denied for user 'root'@'localhost' (using password: NO)
    #- connection_default_file: '/etc/mysql/debian.cnf'
    - connection_host: 'localhost'
    - connection_user: 'debian-sys-maint'
    - connection_pass: 'maintain_pass'
    - connection_unix_socket: '/var/run/mysqld/mysqld.sock'

produces in state run following log lines:

[DEBUG   ] LazyLoaded config.option
[DEBUG   ] LazyLoaded mysql.user_create
[DEBUG   ] LazyLoaded mysql_user.present
[INFO    ] Running state [readonly] at time 01:25:13.919236
[INFO    ] Executing state mysql_user.present for [readonly]
[DEBUG   ] Doing query: SELECT VERSION()
[DEBUG   ] Doing query: SELECT column_name from information_schema.COLUMNS WHERE table_schema=%(schema)s and table_name=%(table)s and column_name=%(column)s args: {u'column': u'Password', u'table': u'user', u'schema': u'mysql'}
[ERROR   ] MySQL Error 1045: Access denied for user 'readonly'@'localhost' (using password: YES)
[DEBUG   ] LazyLoaded test.ping
[DEBUG   ] Doing query: SELECT VERSION()
[DEBUG   ] Doing query: SELECT column_name from information_schema.COLUMNS WHERE table_schema=%(schema)s and table_name=%(table)s and column_name=%(column)s args: {u'column': u'Password', u'table': u'user', u'schema': u'mysql'}
[DEBUG   ] Doing query: SELECT User,Host FROM mysql.user WHERE User = %(user)s AND Host = %(host)s args: {u'host': u'127.0.0.1', u'user': u'readonly'}
[DEBUG   ] Doing query: SELECT VERSION()
[DEBUG   ] Doing query: SELECT column_name from information_schema.COLUMNS WHERE table_schema=%(schema)s and table_name=%(table)s and column_name=%(column)s args: {u'column': u'Password', u'table': u'user', u'schema': u'mysql'}
[DEBUG   ] Doing query: ALTER USER %(user)s@%(host)s IDENTIFIED BY %(password)s; args: {u'host': u'127.0.0.1', u'password': u'some_pass', u'user': u'readonly'}
[DEBUG   ] Doing query: FLUSH PRIVILEGES;
[INFO    ] Password for user 'readonly'@'127.0.0.1' has been changed
[INFO    ] {u'readonly': u'Updated'}
[INFO    ] Completed state [readonly] at time 01:25:13.957097 (duration_in_ms=37.862)

[DEBUG   ] LazyLoaded mysql_grants.present
[INFO    ] Running state [readonly] at time 01:25:13.971022
[INFO    ] Executing state mysql_user.present for [readonly]
[DEBUG   ] Doing query: SELECT VERSION()
[DEBUG   ] Doing query: SELECT column_name from information_schema.COLUMNS WHERE table_schema=%(schema)s and table_name=%(table)s and column_name=%(column)s args: {u'column': u'Password', u'table': u'user', u'schema': u'mysql'}
[ERROR   ] MySQL Error 1045: Access denied for user 'readonly'@'localhost' (using password: YES)
[DEBUG   ] Doing query: SELECT VERSION()
[DEBUG   ] Doing query: SELECT column_name from information_schema.COLUMNS WHERE table_schema=%(schema)s and table_name=%(table)s and column_name=%(column)s args: {u'column': u'Password', u'table': u'user', u'schema': u'mysql'}
[DEBUG   ] Doing query: SELECT User,Host FROM mysql.user WHERE User = %(user)s AND Host = %(host)s args: {u'host': u'10.30.2.0/255.255.255.0', u'user': u'readonly'}
[DEBUG   ] Doing query: SELECT VERSION()
[DEBUG   ] Doing query: SELECT column_name from information_schema.COLUMNS WHERE table_schema=%(schema)s and table_name=%(table)s and column_name=%(column)s args: {u'column': u'Password', u'table': u'user', u'schema': u'mysql'}
[DEBUG   ] Doing query: ALTER USER %(user)s@%(host)s IDENTIFIED BY %(password)s; args: {u'host': u'10.30.2.0/255.255.255.0', u'password': u'some_pass', u'user': u'readonly'}
[DEBUG   ] Doing query: FLUSH PRIVILEGES;
[INFO    ] Password for user 'readonly'@'10.30.2.0/255.255.255.0' has been changed
[INFO    ] {u'readonly': u'Updated'}
[INFO    ] Completed state [readonly] at time 01:25:13.988843 (duration_in_ms=17.821)

After several hours of unsuccessful searching for issues (where #50744 is also related to this bug), all latest changes in 2018.3 brach I made a local checkout and run a git blame salt/modules/mysql.py | grep -v 201[1-6] where I luckily found this ill-conceived patch:

https://github.com/saltstack/salt/blame/9bd0b7a3f0ef12574c58b6ee80a5c48f38e2154d/salt/modules/mysql.py#L1305..L1325

6930776d756 (Erik Johnson               2013-07-15 18:38:15 -0500 1305)     elif password:
d31c902931f (Gareth J. Greenaway        2019-02-06 22:13:09 +0100 1306)         if salt.utils.versions.version_cmp(server_version, compare_version) >= 0:
dd96c130fb1 (Gareth J. Greenaway        2018-10-05 22:27:52 -0700 1307)             run_verify = True
1b2ffcef1d0 (Gareth J. Greenaway        2018-06-20 11:41:14 -0700 1308)         else:
8b542e1745d (Gareth J. Greenaway        2018-06-22 12:41:55 -0700 1309)             _password = password
1b2ffcef1d0 (Gareth J. Greenaway        2018-06-20 11:41:14 -0700 1310)             qry += ' AND ' + password_column + ' = PASSWORD(%(password)s)'
9265195deb0 (Gareth J. Greenaway        2018-10-05 18:06:48 -0700 1311)             args['password'] = six.text_type(_password)
0d7e42bba0c (Jose Ignacio Galarza       2013-02-23 16:24:23 +0100 1312)     elif password_hash:
61217a73a2c (Michal Suba                2016-01-25 16:48:56 +0100 1313)         qry += ' AND ' + password_column + ' = %(password)s'
bfd47930812 (regilero                   2013-12-04 22:38:26 +0100 1314)         args['password'] = password_hash
0d7e42bba0c (Jose Ignacio Galarza       2013-02-23 16:24:23 +0100 1315)
9265195deb0 (Gareth J. Greenaway        2018-10-05 18:06:48 -0700 1316)     if run_verify:
0284323d505 (Gareth J. Greenaway        2018-11-16 14:01:31 -0800 1317)         if not verify_login(user, password, **connection_args):
9265195deb0 (Gareth J. Greenaway        2018-10-05 18:06:48 -0700 1318)             return False
6930776d756 (Erik Johnson               2013-07-15 18:38:15 -0500 1319)     try:
46084656009 (regilero                   2013-12-10 22:51:01 +0100 1320)         _execute(cur, qry, args)
6930776d756 (Erik Johnson               2013-07-15 18:38:15 -0500 1321)     except MySQLdb.OperationalError as exc:
c90d2019c8f (Peter Schutt               2018-12-14 10:12:26 +1100 1322)         err = 'MySQL Error {0}: {1}'.format(*exc.args)
6930776d756 (Erik Johnson               2013-07-15 18:38:15 -0500 1323)         __context__['mysql.error'] = err
6930776d756 (Erik Johnson               2013-07-15 18:38:15 -0500 1324)         log.error(err)
6930776d756 (Erik Johnson               2013-07-15 18:38:15 -0500 1325)         return False

https://github.com/saltstack/salt/blame/7914da24eb2d91c1858854da008e8d882dc64201/salt/modules/mysql.py#L2338..L2360

0284323d505 (Gareth J. Greenaway        2018-11-16 14:01:31 -0800 2338) def verify_login(user, password=None, **connection_args):
9265195deb0 (Gareth J. Greenaway        2018-10-05 18:06:48 -0700 2339)     '''
9265195deb0 (Gareth J. Greenaway        2018-10-05 18:06:48 -0700 2340)     Attempt to login using the provided credentials.
9265195deb0 (Gareth J. Greenaway        2018-10-05 18:06:48 -0700 2341)     If successful, return true.  Otherwise, return False.
9265195deb0 (Gareth J. Greenaway        2018-10-05 18:06:48 -0700 2342)
9265195deb0 (Gareth J. Greenaway        2018-10-05 18:06:48 -0700 2343)     CLI Example:
9265195deb0 (Gareth J. Greenaway        2018-10-05 18:06:48 -0700 2344)
9265195deb0 (Gareth J. Greenaway        2018-10-05 18:06:48 -0700 2345)     .. code-block:: bash
9265195deb0 (Gareth J. Greenaway        2018-10-05 18:06:48 -0700 2346)
0284323d505 (Gareth J. Greenaway        2018-11-16 14:01:31 -0800 2347)         salt '*' mysql.verify_login root password
9265195deb0 (Gareth J. Greenaway        2018-10-05 18:06:48 -0700 2348)     '''
0284323d505 (Gareth J. Greenaway        2018-11-16 14:01:31 -0800 2349)     # Override the connection args for username and password
9265195deb0 (Gareth J. Greenaway        2018-10-05 18:06:48 -0700 2350)     connection_args['connection_user'] = user
9265195deb0 (Gareth J. Greenaway        2018-10-05 18:06:48 -0700 2351)     connection_args['connection_pass'] = password
9265195deb0 (Gareth J. Greenaway        2018-10-05 18:06:48 -0700 2352)
9265195deb0 (Gareth J. Greenaway        2018-10-05 18:06:48 -0700 2353)     dbc = _connect(**connection_args)
9265195deb0 (Gareth J. Greenaway        2018-10-05 18:06:48 -0700 2354)     if dbc is None:
9265195deb0 (Gareth J. Greenaway        2018-10-05 18:06:48 -0700 2355)         # Clear the mysql.error if unable to connect
9265195deb0 (Gareth J. Greenaway        2018-10-05 18:06:48 -0700 2356)         # if the connection fails, we simply return False
9265195deb0 (Gareth J. Greenaway        2018-10-05 18:06:48 -0700 2357)         if 'mysql.error' in __context__:
9265195deb0 (Gareth J. Greenaway        2018-10-05 18:06:48 -0700 2358)             del __context__['mysql.error']
9265195deb0 (Gareth J. Greenaway        2018-10-05 18:06:48 -0700 2359)         return False
9265195deb0 (Gareth J. Greenaway        2018-10-05 18:06:48 -0700 2360)     return True

This is a typical developer "I-run-all-states on-localhost-and-thats-sufficient-for-all-cases" thinking failure.

1. When python works like C this function get connection_args as reference but not as copy in line 2338.
   So made changes in lines 2350 and 2351 are used for every request after this function call!

2. Values of user, pass are overwritten in lines 2350 and 2351 but not host which make sense because it has to use the connection_host and not the allowed network host/mask.
   But here was missed that this source/destination match is only available from/for localhost (= socket) or 127.0.0.1 (=TCP) connections.
   So this function could only verify logins from localhost or if salt-minion host is in allowed MySQL host (mask) but not from different locations of given MySQL host entry!
   These states are not catched in lines 1316 - 1318 and in other parts/function calls of verify_login.
   Perhaps it makes also sense to setup such check in verify_login and give a third value (None) back if verification isn't possible.

---

BTW:

• As you can see I have also commented out because this isn't working since years:

    # connection_default_file isn't working (anymore)? Get with secured root access:
    # MySQL Error 1045: Access denied for user 'root'@'localhost' (using password: NO)
    #- connection_default_file: '/etc/mysql/debian.cnf'
  

  As long as user/pass/host entries were working and your bugs often celebrates several birthdays till they are fixed I wasn't in the mood to open an issue for it that time.

• While "blaming" the file I found also the meaningless question: "Rumpelstiltskin, what's your name?" implemented by @rallytime which is logged as MySQL call before the ERROR lines:
  https://github.com/saltstack/salt/blame/7914da24eb2d91c1858854da008e8d882dc64201/salt/modules/mysql.py#L316..L323

  216d9fdc9a4 (Nicole Thomas              2016-04-11 17:07:15 -0600  316)     qry = ('SELECT column_name from information_schema.COLUMNS '
  216d9fdc9a4 (Nicole Thomas              2016-04-11 17:07:15 -0600  317)            'WHERE table_schema=%(schema)s and table_name=%(table)s '
  216d9fdc9a4 (Nicole Thomas              2016-04-11 17:07:15 -0600  318)            'and column_name=%(column)s')
  216d9fdc9a4 (Nicole Thomas              2016-04-11 17:07:15 -0600  319)     args = {
  216d9fdc9a4 (Nicole Thomas              2016-04-11 17:07:15 -0600  320)       'schema': 'mysql',
  216d9fdc9a4 (Nicole Thomas              2016-04-11 17:07:15 -0600  321)       'table':  'user',
  216d9fdc9a4 (Nicole Thomas              2016-04-11 17:07:15 -0600  322)       'column': 'Password'
  216d9fdc9a4 (Nicole Thomas              2016-04-11 17:07:15 -0600  323)     }
  

  The answer (for MySQL based databases) can be only the column given in request if there is not a newer function implemented in latest MySQL service which allows multiple passwords and allow different content override in information schema column.

  MariaDB [(none)]> SELECT column_name from information_schema.COLUMNS WHERE table_schema='mysql' and table_name='user' and column_name='Password';
  +-------------+
  | column_name |
  +-------------+
  | Password    |
  +-------------+
  

  Even it's nice to double-verify settings for changes it seems senseless.

---

The common problem also at Saltstack is the difference in thinking between feature orientated developers and stable running orientated admins/operations development which has to be used for systems like Saltstack and is very good explained in the speeches of: "Kristian Köhntopp: Go Away Or I Will Replace You With A Very Little Shell Script" where all thee public ones are sadly only available in German lanaguage.

Here the visual picture of the difference ;)

• Developer: https://youtu.be/e0CCv7pSK4s?t=1058
• Infrastrucute Development: https://youtu.be/e0CCv7pSK4s?t=1084

Happy Easter

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[Reiner030]

Hello @Ch3LL and @garethgreenaway ,

Where is the fix for this bug?

Is it in #50744 (comment) ?

thanks for pointing that out. Can anyone try the fixes here: #53418

And if this is the case, why it isn't working in released 2018.3.5 version ?

[sc-kanga]

Am I correct in presuming there still isn't a fix for this?