From dc522cc5d3d00a15d0564955c53019ae58881535 Mon Sep 17 00:00:00 2001 From: "Jorge Rodriguez (A.K.A. Tiriel)" Date: Sat, 10 Apr 2021 07:01:15 +0200 Subject: [PATCH] Deprecate REQUIRESSL privilege (#132) * Deprecate REQUIRESSL privilege * Add missing whitespace * Fix according to PR review * Fix conditional check * Fix privilege string parsing * Add unit tests for the new function * Add integration tests * Fix parentheses indentation * Cover alternative error message * Fix privileges * Limit verification of access denied to pymysql connector * Fix REQUIRE SSL verification tests --- .../fragments/101-drop-requiressl-support.yml | 4 + plugins/modules/mysql_user.py | 42 +++++-- .../test_mysql_user/tasks/issue-121.yml | 115 ++++++++++++++++++ .../targets/test_mysql_user/tasks/main.yml | 2 + tests/unit/plugins/modules/test_mysql_user.py | 70 +++++++++++ 5 files changed, 220 insertions(+), 13 deletions(-) create mode 100644 changelogs/fragments/101-drop-requiressl-support.yml create mode 100644 tests/integration/targets/test_mysql_user/tasks/issue-121.yml diff --git a/changelogs/fragments/101-drop-requiressl-support.yml b/changelogs/fragments/101-drop-requiressl-support.yml new file mode 100644 index 0000000..fded3e3 --- /dev/null +++ b/changelogs/fragments/101-drop-requiressl-support.yml @@ -0,0 +1,4 @@ +minor_changes: + - mysql_user - deprecate the ``REQUIRESSL`` privilege (https://github.com/ansible-collections/community.mysql/issues/101). +major_changes: + - mysql_user - the ``REQUIRESSL`` is an alias for the ``ssl`` key in the ``tls_requires`` option in ``community.mysql`` 2.0.0 and support will be dropped altogether in ``community.mysql`` 3.0.0 (https://github.com/ansible-collections/community.mysql/issues/121). diff --git a/plugins/modules/mysql_user.py b/plugins/modules/mysql_user.py index bb0f5a0..b39b11a 100644 --- a/plugins/modules/mysql_user.py +++ b/plugins/modules/mysql_user.py @@ -189,7 +189,7 @@ EXAMPLES = r''' 'db2.*': 'ALL,GRANT' # Note that REQUIRESSL is a special privilege that should only apply to *.* by itself. -# Setting this privilege in this manner is supported for backwards compatibility only. +# Setting this privilege in this manner is deprecated. # Use 'tls_requires' instead. - name: Modify user to require SSL connections community.mysql.mysql_user: @@ -316,7 +316,8 @@ VALID_PRIVS = frozenset(('CREATE', 'DROP', 'GRANT', 'GRANT OPTION', 'EXECUTE', 'FILE', 'CREATE TABLESPACE', 'CREATE USER', 'PROCESS', 'PROXY', 'RELOAD', 'REPLICATION CLIENT', 'REPLICATION SLAVE', 'SHOW DATABASES', 'SHUTDOWN', - 'SUPER', 'ALL', 'ALL PRIVILEGES', 'USAGE', 'REQUIRESSL', + 'SUPER', 'ALL', 'ALL PRIVILEGES', 'USAGE', + 'REQUIRESSL', # Deprecated, to be removed in version 3.0.0 'CREATE ROLE', 'DROP ROLE', 'APPLICATION_PASSWORD_ADMIN', 'AUDIT_ADMIN', 'BACKUP_ADMIN', 'BINLOG_ADMIN', 'BINLOG_ENCRYPTION_ADMIN', 'CLONE_ADMIN', 'CONNECTION_ADMIN', @@ -745,8 +746,6 @@ def privileges_get(cursor, user, host): if "WITH GRANT OPTION" in res.group(7): privileges.append('GRANT') - if 'REQUIRE SSL' in res.group(7): - privileges.append('REQUIRESSL') db = res.group(2) output.setdefault(db, []).extend(privileges) return output @@ -919,11 +918,6 @@ def privileges_unpack(priv, mode): if '*.*' not in output: output['*.*'] = ['USAGE'] - # if we are only specifying something like REQUIRESSL and/or GRANT (=WITH GRANT OPTION) in *.* - # we still need to add USAGE as a privilege to avoid syntax errors - if 'REQUIRESSL' in priv and not set(output['*.*']).difference(set(['GRANT', 'REQUIRESSL'])): - output['*.*'].append('USAGE') - return output @@ -935,7 +929,7 @@ def privileges_revoke(cursor, user, host, db_table, priv, grant_option): query.append("FROM %s@%s") query = ' '.join(query) cursor.execute(query, (user, host)) - priv_string = ",".join([p for p in priv if p not in ('GRANT', 'REQUIRESSL')]) + priv_string = ",".join([p for p in priv if p not in ('GRANT', )]) query = ["REVOKE %s ON %s" % (priv_string, db_table)] query.append("FROM %s@%s") query = ' '.join(query) @@ -946,15 +940,13 @@ def privileges_grant(cursor, user, host, db_table, priv, tls_requires): # Escape '%' since mysql db.execute uses a format string and the # specification of db and table often use a % (SQL wildcard) db_table = db_table.replace('%', '%%') - priv_string = ",".join([p for p in priv if p not in ('GRANT', 'REQUIRESSL')]) + priv_string = ",".join([p for p in priv if p not in ('GRANT', )]) query = ["GRANT %s ON %s" % (priv_string, db_table)] query.append("TO %s@%s") params = (user, host) if tls_requires and impl.use_old_user_mgmt(cursor): query, params = mogrify_requires(" ".join(query), params, tls_requires) query = [query] - if 'REQUIRESSL' in priv and not tls_requires: - query.append("REQUIRE SSL") if 'GRANT' in priv: query.append("WITH GRANT OPTION") query = ' '.join(query) @@ -975,6 +967,27 @@ def convert_priv_dict_to_str(priv): return '/'.join(priv_list) +def handle_requiressl_in_priv_string(module, priv, tls_requires): + module.deprecate('The "REQUIRESSL" privilege is deprecated, use the "tls_requires" option instead.', + version='3.0.0', collection_name='community.mysql') + priv_groups = re.search(r"(.*?)(\*\.\*:)([^/]*)(.*)", priv) + if priv_groups.group(3) == "REQUIRESSL": + priv = priv_groups.group(1) + priv_groups.group(4) or None + else: + inner_priv_groups = re.search(r"(.*?),?REQUIRESSL,?(.*)", priv_groups.group(3)) + priv = '{0}{1}{2}{3}'.format( + priv_groups.group(1), + priv_groups.group(2), + ','.join(filter(None, (inner_priv_groups.group(1), inner_priv_groups.group(2)))), + priv_groups.group(4) + ) + if not tls_requires: + tls_requires = "SSL" + else: + module.warn('Ignoring "REQUIRESSL" privilege as "tls_requires" is defined and it takes precedence.') + return priv, tls_requires + + # Alter user is supported since MySQL 5.6 and MariaDB 10.2.0 def server_supports_alter_user(cursor): """Check if the server supports ALTER USER statement or doesn't. @@ -1167,6 +1180,9 @@ def main(): if priv and isinstance(priv, dict): priv = convert_priv_dict_to_str(priv) + if priv and "REQUIRESSL" in priv: + priv, tls_requires = handle_requiressl_in_priv_string(module, priv, tls_requires) + if mysql_driver is None: module.fail_json(msg=mysql_driver_fail_msg) diff --git a/tests/integration/targets/test_mysql_user/tasks/issue-121.yml b/tests/integration/targets/test_mysql_user/tasks/issue-121.yml new file mode 100644 index 0000000..d8a9d9e --- /dev/null +++ b/tests/integration/targets/test_mysql_user/tasks/issue-121.yml @@ -0,0 +1,115 @@ +--- +- 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: + + # ============================================================ + - shell: pip show pymysql | awk '/Version/ {print $2}' + register: pymysql_version + + - name: get server certificate + copy: + content: "{{ lookup('pipe', \"openssl s_client -starttls mysql -connect localhost:3307 -showcerts 2>/dev/null