Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mysql_user: Granting all privileges at a global level always results in a change in MySQL 8.0 #77

Open
steveteahan opened this issue Dec 28, 2020 · 27 comments · Fixed by #438
Labels
help wanted Extra attention is needed

Comments

@steveteahan
Copy link
Contributor

steveteahan commented Dec 28, 2020

SUMMARY

Granting ALL PRIVILEGES at a global level will always result in a change being detected. I came across this issue when trying to re-enable the user_password_update_test.yml test for another change. This appears to be linked to the following change in MySQL 8.0:

In MySQL 8.0 compared to previous series, SHOW GRANTS no longer displays ALL PRIVILEGES in its global-privileges output because the meaning of ALL PRIVILEGES at the global level varies depending on which dynamic privileges are defined. Instead, SHOW GRANTS explictly lists each granted global privilege
(https://dev.mysql.com/doc/refman/8.0/en/show-grants.html)

So, because the expanded list of all privileges doesn't match "ALL PRIVILEGES", a change will always be made when applying *.*:ALL. Privileges at the database and/or table level are unaffected (hopefully the more common use case).

This issue appears to be complicated by the fact that dynamic privileges are defined at runtime as per the docs. This isn't my area of expertise, but my understanding of the implication of this is that we cannot reliably hard-code dynamic privileges to check. SHOW PRIVILEGES seems to get us the list of privileges that would be applied to a user with *.*:ALL (in addition to GRANT, PROXY, and USAGE).

I can think of a couple of options:

  1. This is the desired behavior. It can be added to the documentation and the test below can be added so the intent is clear. Possibly a warning?
  2. Build the list of privileges to check against using SHOW PRIVILEGES when priv is *.*:ALL

Like I mentioned, this isn't my area of expertise, so it is possible that hard-coding the privileges will really work, but purely based on the documentation it seems like it wouldn't.

ISSUE TYPE
  • Bug Report
COMPONENT NAME

mysql_user

ANSIBLE VERSION

Development

CONFIGURATION

Development environment

OS / ENVIRONMENT

Development environment

STEPS TO REPRODUCE

The following test will fail and reproduce the issue:

- vars:
    mysql_parameters: &mysql_params
      login_user: '{{ mysql_user }}'
      login_password: '{{ mysql_password }}'
      login_host: 127.0.0.1
      login_port: '{{ mysql_primary_port }}'

  block:

    - name: Create a user with all privileges
      mysql_user:
        <<: *mysql_params
        name: '{{ user_name_5 }}'
        password: '{{ user_password_5 }}'
        priv: '*.*:ALL'
        state: present
      register: result

    - name: Assert that there was a change because the permissions did not exist previously
      assert:
        that:
          - "result.changed == true"

    - name: Test idempotency when nothing has changed and we're still granting ALL privileges
      mysql_user:
        <<: *mysql_params
        name: '{{ user_name_5 }}'
        password: '{{ user_password_5 }}'
        priv: '*.*:ALL'
        state: present
      register: result

    - name: Assert that there wasn't a change because the user already had ALL privileges
      assert:
        that:
          - "result.changed == false"

    ##########
    # Clean up

    - name: Drop test user
      mysql_user:
        <<: *mysql_params
        name: '{{ user_name_5 }}'
        state: absent
EXPECTED RESULTS

The test above should pass because no change was made to the privileges.

ACTUAL RESULTS
TASK [test_mysql_user : Test idempotency when nothing has changed and we're still granting ALL privileges] ***
task path: /root/ansible_collections/community/mysql/tests/output/.tmp/integration/test_mysql_user-sw27zu6h-ÅÑŚÌβŁÈ/tests/integration/targets/test_mysql_user/tasks/test_grant_all_privileges.yml:33
<testhost> ESTABLISH LOCAL CONNECTION FOR USER: root
<testhost> EXEC /bin/sh -c 'echo ~root && sleep 0'
<testhost> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo /root/.ansible/tmp `"&& mkdir "` echo /root/.ansible/tmp/ansible-tmp-1609109000.0398564-8365-196033537627436 `" && echo ansible-tmp-1609109000.0398564-8365-196033537627436="` echo /root/.ansible/tmp/ansible-tmp-1609109000.0398564-8365-196033537627436 `" ) && sleep 0'
Using module file /root/ansible_collections/community/mysql/plugins/modules/mysql_user.py
<testhost> PUT /root/.ansible/tmp/ansible-local-101glv7bsmi/tmpd7a9ul1t TO /root/.ansible/tmp/ansible-tmp-1609109000.0398564-8365-196033537627436/AnsiballZ_mysql_user.py
<testhost> EXEC /bin/sh -c 'chmod u+x /root/.ansible/tmp/ansible-tmp-1609109000.0398564-8365-196033537627436/ /root/.ansible/tmp/ansible-tmp-1609109000.0398564-8365-196033537627436/AnsiballZ_mysql_user.py && sleep 0'
<testhost> EXEC /bin/sh -c '/tmp/python-ypi9raas-ansible/python /root/.ansible/tmp/ansible-tmp-1609109000.0398564-8365-196033537627436/AnsiballZ_mysql_user.py && sleep 0'
<testhost> EXEC /bin/sh -c 'rm -f -r /root/.ansible/tmp/ansible-tmp-1609109000.0398564-8365-196033537627436/ > /dev/null 2>&1 && sleep 0'
changed: [testhost] => {
    "changed": true,
    "invocation": {
        "module_args": {
            "append_privs": false,
            "ca_cert": null,
            "check_hostname": null,
            "check_implicit_admin": false,
            "client_cert": null,
            "client_key": null,
            "config_file": "/root/.my.cnf",
            "connect_timeout": 30,
            "encrypted": false,
            "host": "localhost",
            "host_all": false,
            "login_host": "127.0.0.1",
            "login_password": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER",
            "login_port": 3307,
            "login_unix_socket": null,
            "login_user": "root",
            "name": "db_user5",
            "password": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER",
            "plugin": null,
            "plugin_auth_string": null,
            "plugin_hash_string": null,
            "priv": "*.*:ALL",
            "resource_limits": null,
            "sql_log_bin": true,
            "state": "present",
            "tls_requires": null,
            "update_password": "always",
            "user": "db_user5"
        }
    },
    "msg": "Privileges updated",
    "user": "db_user5"
}

TASK [test_mysql_user : Assert that there wasn't a change because the user already had ALL privileges] ***
task path: /root/ansible_collections/community/mysql/tests/output/.tmp/integration/test_mysql_user-sw27zu6h-ÅÑŚÌβŁÈ/tests/integration/targets/test_mysql_user/tasks/test_grant_all_privileges.yml:42
fatal: [testhost]: FAILED! => {
    "assertion": "result.changed == false",
    "changed": false,
    "evaluated_to": false,
    "msg": "Assertion failed"
}
@Andersson007
Copy link
Collaborator

Andersson007 commented Dec 28, 2020

This is not my area too. I'm against hard-coding (who will maintain this?),
so IMO better just to describe this nuance in the option description, it could be something very roughly like

- When granting all privileges using C(*.*:ALL) in MySQL 8 and later, the module always reports that the state has been changed.

@tgadiev
Copy link

tgadiev commented Feb 10, 2021

I faced the same issue. Idempotence test fails on the following task:

    - name: Update mysql root password for all root accounts
      mysql_user:
        name: root
        host_all: true
        password: '{{ mysql_root_password }}'
        check_implicit_admin: true
        priv: '*.*:ALL,GRANT'
        login_user: '{{ mysql_root_username }}'
        login_password: '{{ mysql_root_password }}'

@Jorge-Rodriguez
Copy link
Contributor

Related to #62 #99 #89

According to the docs MySQL 8 no longer reports ALL privileges as ALL but as the explicit list of granted privileges.

This causes the privilege parser to always detect a change in privileges

@rsicart
Copy link
Contributor

rsicart commented Sep 21, 2021

Same problem here. Someone knows if the fix for this issue is in the roadmap?

I tried to define specific global privileges (static, dynamic) to bypass this ALL PRIVILEGES <-> "list of specific privileges" translation problem, but I've seen that there's a list of hard-coded VALID_PRIVS which doesn't contain all privileges.

@Andersson007
Copy link
Collaborator

Andersson007 commented Sep 21, 2021

Same problem here. Someone knows if the fix for this issue is in the roadmap?

Not in mine unfortunately (at least in the nearest future) but I'd be happy to review things if someone picks it up and submits a fix.

I'm thinking of determining the list of possible privileges dynamically (i.e. to fetch them from a server) instead of having the hardcoded ones. Though not sure if it's possible. If anyone shares the SQL query that works on all supported versions, it would be great (if exists).
I always think that it would be nice to get rid of that frozenset when adding a new privilege to it...

@Andersson007
Copy link
Collaborator

Though it can be painful to implement OR can lead to breaking changes introduction which i'd generally like to avoid but this frozenset is a really annoying thing.

@Andersson007
Copy link
Collaborator

@rsicart , can this issue get closed?

@Jorge-Rodriguez
Copy link
Contributor

If I understood the PR correctly, the actual modification checks weren't updated, so I'd say the behaviour remains unchanged

@rsicart
Copy link
Contributor

rsicart commented Sep 23, 2021

As Tiriel said, PR #217 doesn't solve this is issue, but perhaps fixes this other issue #110

@Andersson007
Copy link
Collaborator

Closed that one, thanks for pointing out!

@tchernomax
Copy link

tchernomax commented Aug 9, 2022

I don't know if mysql change it's behavior since the report of this issue but for me it me it was more than a "always results in a change".

I use the module to set the root@localhost user privileges. root@localhost is also the user I use to connect to mysql.

But from what I understand, because of the diff between ['ALL', 'GRANT'] and [<all the privileges>, 'GRANT'] the module revoked <all the privileges> and 'GRANT' , then flush privileges, before trying to grant 'ALL' … but at this point my user didn't have any privileges anymore and the GRANT fails.

So now my root@localhost is useless, and I don't have any other user with the GRANT privilege… so I am "locked out" of my mysql.
Fortunately I just started building this instance, so no harm done… but I wouldn't want this to happen in my production environment :)

What I suggest : if 'ALL' is in curr_priv[db_table] then don't revoke any privileges, do the privileges_grant, redo the privileges_get and compare the privileges list with the one before privileges_grant to detect any change.
(need some tweaking for the GRANT OPTION)

@Jorge-Rodriguez
Copy link
Contributor

@Andersson007 let's prioritize this, @tchernomax issue is quite serious.

@Andersson007
Copy link
Collaborator

@tchernomax thanks for reporting this!

@Andersson007 let's prioritize this, @tchernomax issue is quite serious.

@Jorge-Rodriguez agreed! Would you like to pick it up?

@Jorge-Rodriguez
Copy link
Contributor

@tchernomax thanks for reporting this!

@Andersson007 let's prioritize this, @tchernomax issue is quite serious.

@Jorge-Rodriguez agreed! Would you like to pick it up?

I can have a stab at it yes. Although another pair of eyes couldn't hurt.

@Andersson007
Copy link
Collaborator

I can have a stab at it yes. Although another pair of eyes couldn't hurt.

Cool, thanks a lot!

@ocfmatt
Copy link

ocfmatt commented Aug 15, 2022

I've bumped into this bug using MySQL 8.

I have a play updating the root password and ensuring the privileges are correct using

  - name: Ensure root user is present 
    community.mysql.mysql_user:
      name: "{{ mysql.database_root_user }}"
      host: 'localhost'
      password: "{{ mysql.database_root_password }}"
      priv: '*.*:ALL,GRANT'
      state: present
      login_user: root
      login_password: "AnInsecurePassword"

I have plays later on trying to create users and grant permissions to databases however, this wasn't working regardless of what priv vars I tried. After finding this issue I did find the grant option was being removed from the root user in the exact same manner as described above.

Checking MySQL the Grant permission is removed:

mysql> SELECT Host,User,Grant_priv FROM mysql.user;
+---------------------------+------------------+------------+
| Host                      | User             | Grant_priv |
+---------------------------+------------------+------------+
| localhost                 | mysql.infoschema | N          |
| localhost                 | mysql.session    | N          |
| localhost                 | mysql.sys        | N          |
| localhost                 | root             | N          |
+---------------------------+------------------+------------+
4 rows in set (0.00 sec)

I did still have all other permissions as the root user so I was able to add grant back to root using an SQL update query and confirm the permission was effectively restored. @tchernomax the update query below will get you up and running again if you did get stuck where no users have the grant priv.

mysql> UPDATE mysql.user SET Grant_priv = 'Y' where User = 'root' AND Host = 'localhost';
mysql> FLUSH PRIVILEGES;

mysql> SELECT Host,User,Grant_priv FROM mysql.user;
+---------------------------+------------------+------------+
| Host                      | User             | Grant_priv |
+---------------------------+------------------+------------+
| localhost                 | mysql.infoschema | N          |
| localhost                 | mysql.session    | N          |
| localhost                 | mysql.sys        | N          |
| localhost                 | root             | Y          |
+---------------------------+------------------+------------+
4 rows in set (0.00 sec)

After running my playbook again, the grant permission was removed from the root user.

mysql> SELECT Host,User,Grant_priv FROM mysql.user;
+---------------------------+------------------+------------+
| Host                      | User             | Grant_priv |
+---------------------------+------------------+------------+
| localhost                 | mysql.infoschema | N          |
| localhost                 | mysql.session    | N          |
| localhost                 | mysql.sys        | N          |
| localhost                 | root             | N          |
+---------------------------+------------------+------------+
4 rows in set (0.00 sec)

This is happening with all users where ALL,GRANT is defined. The only workaround I can use for now will be the good old fall back to the shell module and executing commands outside the mysql_user module until resolved.

Regards,
Matt.

@rsicart
Copy link
Contributor

rsicart commented Aug 29, 2022

Just clarify that the Grant Priv revocation problem seems to be present in collection 3.4.1, but not in 2.3.8.

Problem is not present when creating new users (user_add only grants privs), but present on user modification, where grants and revokes are done.

@betanummeric
Copy link
Member

@tchernomax
FYI, in case you lock yourself out of a mysql/mariadb you can recover by restarting it with --skip-grant-tables --skip-networking, connect from localhost and update the mysql.user or mysql.global_priv table, something like

update mysql.global_priv set Priv='{"access": 18446744073709551615, "authentication_string": "censored","version_id":100609,"plugin":"mysql_native_password"}' where User='root' and Host='localhost';

Then stop and start normally.
Of course, it should not come to that.

@Andersson007
Copy link
Collaborator

Andersson007 commented Sep 5, 2022

@rsicart should the issue get closed? (i've released the collection version that contains your fix)

@rsicart
Copy link
Contributor

rsicart commented Sep 5, 2022

No, at the moment we always have a change.

As @tchernomax commented, we should try to make a diff of privileges before and after configuring privileges. Perhaps something similar to a deep diff of dicts.

@Andersson007
Copy link
Collaborator

No, at the moment we always have a change.

As @tchernomax commented, we should try to make a diff of privileges before and after configuring privileges. Perhaps something similar to a deep diff of dicts.

@rsicart ah, ok, thanks for clarifying!

@Andersson007
Copy link
Collaborator

Andersson007 commented Sep 9, 2022

Thanks everyone involved!
The fix for this issue is available in the latest collection version, feel free to install it manually from Galaxy and use, thanks!

UPDATE: it's not in the release, i mixed it up with another fix, i apologize

@pgrenaud
Copy link

pgrenaud commented Dec 5, 2022

Hum, I'm a little bit late for that, but the issue doesn't seem entirely fixed. If I run ansible with --check, a change is still reported. Whereas without the --check, no change is reported.

I'm running mysql_user from community.mysql version 3.5.1 with priv: '*.*:ALL,GRANT'.

Should we reopen this issue or should I open a new one?

@Andersson007 Andersson007 reopened this Dec 6, 2022
@Andersson007
Copy link
Collaborator

@pgrenaud hello, thanks for reporting the case!

@Andersson007
Copy link
Collaborator

@rsicart do you have time to take a look? ^
Looks like we forgot to cover check mode.
I think we should add the same task that you added in the PR with check_mode: yes to reproduce the case.
If you have no time, please let me know (now rush with it though)

@rsicart
Copy link
Contributor

rsicart commented Dec 6, 2022

@rsicart do you have time to take a look? ^ Looks like we forgot to cover check mode. I think we should add the same task that you added in the PR with check_mode: yes to reproduce the case. If you have no time, please let me know (now rush with it though)

Hey @Andersson007, thanks for letting me know. I'm a little busy lately, I prefer that someone takes a look (if possible).

@Andersson007
Copy link
Collaborator

@rsicart do you have time to take a look? ^ Looks like we forgot to cover check mode. I think we should add the same task that you added in the PR with check_mode: yes to reproduce the case. If you have no time, please let me know (now rush with it though)

Hey @Andersson007, thanks for letting me know. I'm a little busy lately, I prefer that someone takes a look (if possible).

@rsicart hey, thanks for letting us know!
let's keep it open then, if someone picks it up, it'd be cool.
If it's still open when you have easier times @rsicart, feel free to pick it up!
It doesn't look like a blocker, so it can wait imo.
I'll put help_wanted

@Andersson007 Andersson007 added the help wanted Extra attention is needed label Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants