From 53e901662641b70a78e1a601ab1ec57a4dddfe85 Mon Sep 17 00:00:00 2001 From: Steve Teahan <75569952+steveteahan@users.noreply.github.com> Date: Sat, 16 Jan 2021 08:24:36 -0500 Subject: [PATCH 1/2] mysql_user - added queries to module output and set no_log option on plugin_hash_string and plugin_auth_string (#75) --- .../75-mysql_user_queries_output.yml | 2 + plugins/modules/mysql_user.py | 85 +++++- .../targets/test_mysql_user/tasks/main.yml | 4 + .../tasks/test_queries_output.yml | 280 ++++++++++++++++++ 4 files changed, 360 insertions(+), 11 deletions(-) create mode 100644 changelogs/fragments/75-mysql_user_queries_output.yml create mode 100644 tests/integration/targets/test_mysql_user/tasks/test_queries_output.yml diff --git a/changelogs/fragments/75-mysql_user_queries_output.yml b/changelogs/fragments/75-mysql_user_queries_output.yml new file mode 100644 index 00000000..329b9e3f --- /dev/null +++ b/changelogs/fragments/75-mysql_user_queries_output.yml @@ -0,0 +1,2 @@ +minor_changes: +- mysql_user - added queries to module output and set no_log option on plugin_hash_string and plugin_auth_string (https://github.com/ansible-collections/community.mysql/issues/75). diff --git a/plugins/modules/mysql_user.py b/plugins/modules/mysql_user.py index 0c757fa8..a9f843ad 100644 --- a/plugins/modules/mysql_user.py +++ b/plugins/modules/mysql_user.py @@ -294,7 +294,15 @@ # password=n<_665{vS43y ''' -RETURN = '''#''' +RETURN = r''' +queries: + description: List of executed queries which modified DB's state. + returned: if executed successfully + type: list + sample: ["CREATE USER 'db_user2'@'localhost' IDENTIFIED WITH mysql_native_password AS '********'"] + version_added: '1.1.0' +''' + import re import string @@ -347,11 +355,26 @@ class InvalidPrivsError(Exception): pass + +# Same as what is used by Ansible no_log parameters for consistency. +REDACTED_AUTH_STRING = "*" * 8 + +_executed_queries = [] + # =========================================== # MySQL module specific support methods. # +def log_query(cursor, query, args=None): + """ + Logs the query including the args provided. + + Note: check that all args handle credentials properly (parameters should have no_log set or other mitigations). + """ + _executed_queries.append(cursor.mogrify(query, args)) + + # User Authentication Management changed in MySQL 5.7 and MariaDB 10.2.0 def use_old_user_mgmt(cursor): cursor.execute("SELECT VERSION()") @@ -490,6 +513,8 @@ def user_add(cursor, user, host, host_all, password, encrypted, mogrify = do_not_mogrify_requires if old_user_mgmt else mogrify_requires + sanitized_query_with_args = "" + if password and encrypted: if supports_identified_by_password(cursor): query_with_args = "CREATE USER %s@%s IDENTIFIED BY PASSWORD %s", (user, host, password) @@ -502,6 +527,14 @@ def user_add(cursor, user, host, host_all, password, encrypted, cursor.execute("SELECT CONCAT('*', UCASE(SHA1(UNHEX(SHA1(%s)))))", (password,)) encrypted_password = cursor.fetchone()[0] query_with_args = "CREATE USER %s@%s IDENTIFIED WITH mysql_native_password AS %s", (user, host, encrypted_password) + + # encrypted_password above is not a module argument and therefore cannot use no_log to prevent logging. This + # is a workaround for that. Using 'BY' instead of 'AS' in the statement appears to allow passing in + # plaintext values for MySQL 5.7 and 8.0. It is not clear whether this was avoided for a specific reason. + # This special case can be removed if the change described above is implemented in the future. + + sanitized_query_with_args = "CREATE USER %s@%s IDENTIFIED WITH mysql_native_password AS %s",\ + (user, host, REDACTED_AUTH_STRING) elif plugin and plugin_hash_string: query_with_args = "CREATE USER %s@%s IDENTIFIED WITH %s AS %s", (user, host, plugin, plugin_hash_string) elif plugin and plugin_auth_string: @@ -512,6 +545,14 @@ def user_add(cursor, user, host, host_all, password, encrypted, query_with_args = "CREATE USER %s@%s", (user, host) query_with_args_and_tls_requires = query_with_args + (tls_requires,) + + # Handle the special case where a no_log module parameter isn't being used. + if sanitized_query_with_args: + sanitized_query_with_args_and_tls = sanitized_query_with_args + (tls_requires,) + log_query(cursor, *mogrify(*sanitized_query_with_args_and_tls)) + else: + log_query(cursor, *mogrify(*query_with_args_and_tls_requires)) + cursor.execute(*mogrify(*query_with_args_and_tls_requires)) if new_priv is not None: @@ -592,11 +633,15 @@ def user_mod(cursor, user, host, host_all, password, encrypted, if module.check_mode: return (True, msg) if old_user_mgmt: - cursor.execute("SET PASSWORD FOR %s@%s = %s", (user, host, encrypted_password)) + query = "SET PASSWORD FOR %s@%s = %s" + log_query(cursor, query, (user, host, REDACTED_AUTH_STRING)) + cursor.execute(query, (user, host, encrypted_password)) msg = "Password updated (old style)" else: try: - cursor.execute("ALTER USER %s@%s IDENTIFIED WITH mysql_native_password AS %s", (user, host, encrypted_password)) + query = "ALTER USER %s@%s IDENTIFIED WITH mysql_native_password AS %s" + log_query(cursor, query, (user, host, REDACTED_AUTH_STRING)) + cursor.execute(query, (user, host, encrypted_password)) msg = "Password updated (new style)" except (mysql_driver.Error) as e: # https://stackoverflow.com/questions/51600000/authentication-string-of-root-user-on-mysql @@ -641,6 +686,7 @@ def user_mod(cursor, user, host, host_all, password, encrypted, else: query_with_args = "ALTER USER %s@%s IDENTIFIED WITH %s", (user, host, plugin) + log_query(cursor, *query_with_args) cursor.execute(*query_with_args) changed = True @@ -711,6 +757,7 @@ def user_mod(cursor, user, host, host_all, password, encrypted, query = " ".join((pre_query, "%s@%s REQUIRE NONE")) query_with_args = query, (user, host) + log_query(cursor, *query_with_args) cursor.execute(*query_with_args) changed = True @@ -727,7 +774,9 @@ def user_delete(cursor, user, host, host_all, check_mode): hostnames = [host] for hostname in hostnames: - cursor.execute("DROP USER %s@%s", (user, hostname)) + query_with_args = "DROP USER %s@%s", (user, hostname) + log_query(cursor, *query_with_args) + cursor.execute(*query_with_args) return True @@ -842,12 +891,16 @@ def privileges_revoke(cursor, user, host, db_table, priv, grant_option): query = ["REVOKE GRANT OPTION ON %s" % db_table] query.append("FROM %s@%s") query = ' '.join(query) - cursor.execute(query, (user, host)) + query_with_args = query, (user, host) + log_query(cursor, *query_with_args) + cursor.execute(*query_with_args) priv_string = ",".join([p for p in priv if p not in ('GRANT', 'REQUIRESSL')]) query = ["REVOKE %s ON %s" % (priv_string, db_table)] query.append("FROM %s@%s") query = ' '.join(query) - cursor.execute(query, (user, host)) + query_with_args = query, (user, host) + log_query(cursor, *query_with_args) + cursor.execute(*query_with_args) def privileges_grant(cursor, user, host, db_table, priv, tls_requires): @@ -866,7 +919,9 @@ def privileges_grant(cursor, user, host, db_table, priv, tls_requires): if 'GRANT' in priv: query.append("WITH GRANT OPTION") query = ' '.join(query) - cursor.execute(query, params) + query_with_args = query, params + log_query(cursor, *query_with_args) + cursor.execute(*query_with_args) def convert_priv_dict_to_str(priv): @@ -1011,7 +1066,9 @@ def limit_resources(module, cursor, user, host, resource_limits, check_mode): query = "ALTER USER %s@%s" query += ' WITH %s' % ' '.join(tmp) - cursor.execute(query, (user, host)) + query_with_args = query, (user, host) + log_query(cursor, *query_with_args) + cursor.execute(*query_with_args) return True @@ -1036,8 +1093,8 @@ def main(): update_password=dict(type='str', default='always', choices=['always', 'on_create'], no_log=False), sql_log_bin=dict(type='bool', default=True), plugin=dict(default=None, type='str'), - plugin_hash_string=dict(default=None, type='str'), - plugin_auth_string=dict(default=None, type='str'), + plugin_hash_string=dict(default=None, type='str', no_log=True), + plugin_auth_string=dict(default=None, type='str', no_log=True), resource_limits=dict(type='dict'), ) module = AnsibleModule( @@ -1144,7 +1201,13 @@ def main(): else: changed = False msg = "User doesn't exist" - module.exit_json(changed=changed, user=user, msg=msg) + + module_output = {"changed": changed, "user": user, "msg": msg} + + if not module.check_mode: + module_output["queries"] = _executed_queries + + module.exit_json(**module_output) if __name__ == '__main__': diff --git a/tests/integration/targets/test_mysql_user/tasks/main.yml b/tests/integration/targets/test_mysql_user/tasks/main.yml index a744050a..9000c090 100644 --- a/tests/integration/targets/test_mysql_user/tasks/main.yml +++ b/tests/integration/targets/test_mysql_user/tasks/main.yml @@ -273,6 +273,10 @@ # Tests for the TLS requires dictionary - include: tls_requirements.yml + # Tests for queries module output + - include: test_queries_output.yml enable_check_mode=no + - include: test_queries_output.yml enable_check_mode=yes + - import_tasks: issue-29511.yaml tags: - issue-29511 diff --git a/tests/integration/targets/test_mysql_user/tasks/test_queries_output.yml b/tests/integration/targets/test_mysql_user/tasks/test_queries_output.yml new file mode 100644 index 00000000..75cd2efd --- /dev/null +++ b/tests/integration/targets/test_mysql_user/tasks/test_queries_output.yml @@ -0,0 +1,280 @@ +# Tests for ensuring that "queries" is in the output of the mysql_user module and that credentials are not being logged + +- vars: + mysql_parameters: &mysql_params + login_user: '{{ mysql_user }}' + login_password: '{{ mysql_password }}' + login_host: 127.0.0.1 + login_port: '{{ mysql_primary_port }}' + test_user_name: 'test_queries_output' + test_user_password: 'Uq109*yhQ5KybNerab' + test_user_hash: '*4BA890B12C1DF5C88D7B1B0B0BE8FF800AACAED9' + test_user_new_pasword: 'mE6M^83ET&##midxeE' + test_user_new_hash: '*D5F6294364E7E732DD8C81FB5BE1D175DB341A0C' + + block: + - name: Create test databases + mysql_db: + <<: *mysql_params + name: '{{ item }}' + state: present + loop: + - queries_output_1 + - queries_output_2 + + # ============================================================ + # Test a simple sequence of creating, modifying, and dropping a user. These tests exclude exact query syntax + # because the scope of the test is simply that queries exists, rather than that a specific statement is executed + # (that should be covered in other tests). + + - name: Create a user + mysql_user: + <<: *mysql_params + name: '{{ test_user_name }}' + password: '{{ test_user_password }}' + priv: 'queries_output_1.*:SELECT,INSERT/queries_output_2.*:SELECT,DELETE' + state: present + check_mode: '{{ enable_check_mode }}' + register: result + + - name: Join queries list from result + set_fact: + result_queries: "{{ result.queries|join('; ') }}" + when: enable_check_mode == 'no' + + - name: Assert that queries are in the result with expected patterns and password isn't logged + assert: + that: + - "result.queries is defined" + - "'CREATE USER' in result_queries" + - "'GRANT' in result_queries" + - "'{{ test_user_password }}' not in result_queries" + - "'********' in result_queries" + when: enable_check_mode == 'no' + + - name: Assert that queries isn't present in result in check mode + assert: + that: + - "result.queries is undefined" + when: enable_check_mode == 'yes' + + - name: Modify privileges for existing user + mysql_user: + <<: *mysql_params + name: '{{ test_user_name }}' + password: '{{ test_user_password }}' + priv: 'queries_output_1.*:SELECT/queries_output_2.*:SELECT' + state: present + check_mode: '{{ enable_check_mode }}' + register: result + + - name: Join queries list from result + set_fact: + result_queries: "{{ result.queries|join('; ') }}" + when: enable_check_mode == 'no' + + - name: Assert that queries are in the result with expected patterns + assert: + that: + - "result.queries is defined" + - "'GRANT' in result_queries" + - "'REVOKE' in result_queries" + when: enable_check_mode == 'no' + + - name: Assert that queries isn't present in result in check mode + assert: + that: + - "result.queries is undefined" + when: enable_check_mode == 'yes' + + - name: Modify password for existing user + mysql_user: + <<: *mysql_params + name: '{{ test_user_name }}' + password: '{{ test_user_new_pasword }}' + priv: 'queries_output_1.*:SELECT/queries_output_2.*:SELECT' + state: present + check_mode: '{{ enable_check_mode }}' + register: result + + - name: Join queries list from result + set_fact: + result_queries: "{{ result.queries|join('; ') }}" + when: enable_check_mode == 'no' + + - name: Assert that queries are in the result with expected patterns + assert: + that: + - "result.queries is defined" + - "'ALTER USER' in result_queries" + - "'{{ test_user_new_pasword }}' not in result_queries" + - "'********' in result_queries" + when: enable_check_mode == 'no' + + - name: Assert that queries isn't present in result in check mode + assert: + that: + - "result.queries is undefined" + when: enable_check_mode == 'yes' + + - name: Drop test user + mysql_user: + <<: *mysql_params + name: '{{ test_user_name }}' + state: absent + check_mode: '{{ enable_check_mode }}' + register: result + + - name: Assert that queries are in the result with expected patterns + assert: + that: + - "result.queries is defined" + - "'DROP USER' in result.queries|join('; ')" + when: enable_check_mode == 'no' + + - name: Assert that queries isn't present in result in check mode + assert: + that: + - "result.queries is undefined" + when: enable_check_mode == 'yes' + + # ============================================================ + # Test that credentials aren't logged when using plugin auth and plugin_auth_string. + + - name: Create a user with plugin_auth_string + mysql_user: + <<: *mysql_params + name: '{{ test_user_name }}' + plugin: mysql_native_password + plugin_auth_string: '{{ test_user_password }}' + state: present + check_mode: '{{ enable_check_mode }}' + register: result + + - name: Join queries list from result + set_fact: + result_queries: "{{ result.queries|join('; ') }}" + when: enable_check_mode == 'no' + + - name: Assert that queries are in the result with expected patterns and password isn't logged + assert: + that: + - "result.queries is defined" + - "'CREATE USER' in result_queries" + - "'{{ test_user_password }}' not in result_queries" + - "'********' in result_queries" + when: enable_check_mode == 'no' + + - name: Assert that queries isn't present in result in check mode + assert: + that: + - "result.queries is undefined" + when: enable_check_mode == 'yes' + + - name: Modify password for existing user using plugin_auth_string + mysql_user: + <<: *mysql_params + name: '{{ test_user_name }}' + plugin: mysql_native_password + plugin_auth_string: '{{ test_user_new_pasword }}' + state: present + check_mode: '{{ enable_check_mode }}' + register: result + + - name: Join queries list from result + set_fact: + result_queries: "{{ result.queries|join('; ') }}" + when: enable_check_mode == 'no' + + - name: Assert that queries are in the result with expected patterns + assert: + that: + - "result.queries is defined" + - "'ALTER USER' in result_queries" + - "'{{ test_user_new_pasword }}' not in result_queries" + - "'********' in result_queries" + when: enable_check_mode == 'no' + + - name: Drop test user + mysql_user: + <<: *mysql_params + name: '{{ test_user_name }}' + state: absent + check_mode: '{{ enable_check_mode }}' + register: result + + # ============================================================ + # Test that credentials aren't logged when using plugin auth and plugin_hash_string + + - name: Create a user + mysql_user: + <<: *mysql_params + name: '{{ test_user_name }}' + plugin: mysql_native_password + plugin_hash_string: '{{ test_user_hash }}' + state: present + check_mode: '{{ enable_check_mode }}' + register: result + + - name: Join queries list from result + set_fact: + result_queries: "{{ result.queries|join('; ') }}" + when: enable_check_mode == 'no' + + - name: Assert that queries are in the result with expected patterns and password isn't logged + assert: + that: + - "result.queries is defined" + - "'CREATE USER' in result_queries" + - "'{{ test_user_hash }}' not in result_queries" + - "'********' in result_queries" + when: enable_check_mode == 'no' + + - name: Assert that queries isn't present in result in check mode + assert: + that: + - "result.queries is undefined" + when: enable_check_mode == 'yes' + + - name: Modify password for existing user using plugin_hash_string + mysql_user: + <<: *mysql_params + name: '{{ test_user_name }}' + plugin: mysql_native_password + plugin_hash_string: '{{ test_user_new_hash }}' + state: present + check_mode: '{{ enable_check_mode }}' + register: result + + - name: Join queries list from result + set_fact: + result_queries: "{{ result.queries|join('; ') }}" + when: enable_check_mode == 'no' + + - name: Assert that queries are in the result with expected patterns + assert: + that: + - "result.queries is defined" + - "'ALTER USER' in result_queries" + - "'{{ test_user_new_hash }}' not in result_queries" + - "'********' in result_queries" + when: enable_check_mode == 'no' + + - name: Drop test user + mysql_user: + <<: *mysql_params + name: '{{ test_user_name }}' + state: absent + check_mode: '{{ enable_check_mode }}' + register: result + + ########## + # Clean up + - name: Drop test databases + mysql_db: + <<: *mysql_params + name: '{{ item }}' + state: present + loop: + - queries_output_1 + - queries_output_2 From 34743200d8f09c70ca620412f089879ad3518a02 Mon Sep 17 00:00:00 2001 From: Steve Teahan <75569952+steveteahan@users.noreply.github.com> Date: Wed, 20 Jan 2021 06:31:58 -0500 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Andrew Klychkov --- changelogs/fragments/75-mysql_user_queries_output.yml | 2 +- plugins/modules/mysql_user.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/changelogs/fragments/75-mysql_user_queries_output.yml b/changelogs/fragments/75-mysql_user_queries_output.yml index 329b9e3f..79654d83 100644 --- a/changelogs/fragments/75-mysql_user_queries_output.yml +++ b/changelogs/fragments/75-mysql_user_queries_output.yml @@ -1,2 +1,2 @@ minor_changes: -- mysql_user - added queries to module output and set no_log option on plugin_hash_string and plugin_auth_string (https://github.com/ansible-collections/community.mysql/issues/75). +- mysql_user - added queries to module output and set `no_log` flag on `plugin_hash_string` and `plugin_auth_string` (https://github.com/ansible-collections/community.mysql/issues/75). diff --git a/plugins/modules/mysql_user.py b/plugins/modules/mysql_user.py index a9f843ad..5bbe218e 100644 --- a/plugins/modules/mysql_user.py +++ b/plugins/modules/mysql_user.py @@ -300,7 +300,7 @@ returned: if executed successfully type: list sample: ["CREATE USER 'db_user2'@'localhost' IDENTIFIED WITH mysql_native_password AS '********'"] - version_added: '1.1.0' + version_added: '1.3.0' '''