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

Add TLS connection parameters #369

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
1e7badc
Add TLS connection parameters
Jorge-Rodriguez May 19, 2020
c5138a9
Add changelog fragment
Jorge-Rodriguez May 19, 2020
9b93097
Add deprecation notice
Jorge-Rodriguez Jun 11, 2020
0b63322
Add tests
Jorge-Rodriguez Jun 11, 2020
c0bc459
Remove deprecation notice
Jorge-Rodriguez Jun 11, 2020
87729f1
Do not attempt parsing unspecified tls_requires
Jorge-Rodriguez Jun 11, 2020
d01b1d2
Format SQL queries to handle optional TLS requires
Jorge-Rodriguez Jun 11, 2020
9ab6e25
Bug fixes
Jorge-Rodriguez Jun 12, 2020
fc3baea
Fix sanity errors
Jorge-Rodriguez Jun 12, 2020
9f70d2f
Remove JSON_OBJECT function from test query as it is not always avail…
Jorge-Rodriguez Jun 12, 2020
bbdb787
Run grant query with TLS requires only if specified
Jorge-Rodriguez Jun 12, 2020
3cc3501
Debug test
Jorge-Rodriguez Jun 12, 2020
a15fbea
Construct SQL queries properly
Jorge-Rodriguez Jun 15, 2020
76b0f17
debug test
Jorge-Rodriguez Jun 16, 2020
8c9c631
Modify code to accomodate requires statements to different db versions
Jorge-Rodriguez Jun 17, 2020
af34ea6
Fix quotations
Jorge-Rodriguez Jun 17, 2020
1433239
Fix PEP errors
Jorge-Rodriguez Jun 17, 2020
e852758
Don't query requires for blank users
Jorge-Rodriguez Jun 17, 2020
0b62975
Use old style formatting
Jorge-Rodriguez Jun 17, 2020
98e4f4a
Fix filter handling
Jorge-Rodriguez Jun 17, 2020
2957ffd
Handle match objects properly
Jorge-Rodriguez Jun 17, 2020
e9a3ae1
Handle existing grants when managing TLS requires in old db versions
Jorge-Rodriguez Jun 18, 2020
e9269db
Fix typo
Jorge-Rodriguez Jun 18, 2020
104a0d1
Change select test
Jorge-Rodriguez Jun 22, 2020
6d03c69
Fix show create user statement
Jorge-Rodriguez Jun 22, 2020
e17ad26
Fix variable references
Jorge-Rodriguez Jun 22, 2020
c469538
Fix assertion variable definitions
Jorge-Rodriguez Jun 22, 2020
efc7aa5
Fix test variables
Jorge-Rodriguez Jun 22, 2020
ce7e4cc
Fix tls_requires parameter
Jorge-Rodriguez Jun 22, 2020
247d4dc
Don't modify dictionary while iterating it
Jorge-Rodriguez Jun 23, 2020
95199dd
Add tls_requires to all privileges_grant calls
Jorge-Rodriguez Jun 23, 2020
ef43b48
Refactor tests to reduce duplication
Jorge-Rodriguez Jun 23, 2020
4cbd9e0
Fix conditional clause
Jorge-Rodriguez Jun 23, 2020
eaeaee1
Improve changelog message
Jorge-Rodriguez Jun 23, 2020
0fcf827
Properly set results variable
Jorge-Rodriguez Jun 23, 2020
b72d2a1
trigger shippable
Jorge-Rodriguez Jun 23, 2020
f582451
Handle TLS requires after privileges
Jorge-Rodriguez Jun 24, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelogs/fragments/369-mysql_user_add_tls_requires.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
minor_changes:
- mysql_user - add TLS REQUIRES parameters (https://github.com/ansible-collections/community.general/pull/369).
deprecated_features:
- mysql_user - using ``REQUIRESSL`` in ``priv`` is deprecated in favor of ``tls_requires`` (https://github.com/ansible-collections/community.general/pull/369).
Comment on lines +3 to +4
Copy link
Contributor

@Andersson007 Andersson007 Jun 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
deprecated_features:
- mysql_user - using ``REQUIRESSL`` in ``priv`` is deprecated in favor of ``tls_requires`` (https://github.com/ansible-collections/community.general/pull/369).
deprecated_features:
- mysql_user - using ``REQUIRESSL`` in ``priv`` is deprecated in favor of ``tls_requires`` (https://github.com/ansible-collections/community.general/pull/369).

is it necessary to deprecate this feature? I'd avoid deprecations (if possible).
Can we make priv: REQUIRESSL and tls_requires mutually exclusive?
I don't insist, it's just topic to discuss

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just reordered the code, now TLS requires are handled after privileges so if both priv:REQUIRESSL and tls_requires are specified, tls_requires takes precedence.
It's not really query optimal as it means that first we'd set REQUIRE SSL and modify it immediately after.
I could remove the deprecation notice and replace it with a 'try to use tls_requires instead, please' note.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good (imo).
Still waiting for @bmalynovytch 's opinion because he's a MySQL WG leader

185 changes: 163 additions & 22 deletions plugins/modules/database/mysql/mysql_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,14 @@
user instead of overwriting existing ones.
type: bool
default: no
tls_requires:
description:
- Set requirement for secure transport as a dictionary of requirements (see the examples).
- Valid requirements are SSL, X509, SUBJECT, ISSUER, CIPHER.
- SUBJECT, ISSUER and CIPHER are complementary, and mutually exclusive with SSL and X509.
- U(https://mariadb.com/kb/en/securing-connections-for-client-and-server/#requiring-tls).
type: dict
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
type: dict
type: dict
version_added: 1.0.0

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@felixfontein should I add this verbatim? will the version number be adjusted automatically? I don't know how that works.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add this verbatim. If this PR is merged before 1.0.0 is released, this is what is needed. If it is not merged in time, the version number needs to be bumped to 1.1.0. (We'll tell you if that's the case.)

version_added: 1.0.0
sql_log_bin:
description:
- Whether binary logging should be enabled or disabled for the connection.
Expand Down Expand Up @@ -176,13 +184,28 @@
'db2.*': 'ALL,GRANT'

# Note that REQUIRESSL is a special privilege that should only apply to *.* by itself.
# Setting this privilege in this manner is deprecated. Use 'tls_requires' instead.
- name: Modify user to require SSL connections.
mysql_user:
name: bob
append_privs: yes
priv: '*.*:REQUIRESSL'
state: present

- name: Modify user to require TLS connection with a valid client certificate
mysql_user:
name: bob
tls_requires:
x509:
state: present

- name: Modify user to require TLS connection with a specific client certificate and cipher
mysql_user:
name: bob
tls_requires:
subject: '/CN=alice/O=MyDom, Inc./C=US/ST=Oregon/L=Portland'
cipher: 'ECDHE-ECDSA-AES256-SHA384'

- name: Ensure no user named 'sally'@'localhost' exists, also passing in the auth credentials.
mysql_user:
login_user: root
Expand Down Expand Up @@ -269,7 +292,6 @@
from ansible.module_utils.six import iteritems
from ansible.module_utils._text import to_native


VALID_PRIVS = frozenset(('CREATE', 'DROP', 'GRANT', 'GRANT OPTION',
'LOCK TABLES', 'REFERENCES', 'EVENT', 'ALTER',
'DELETE', 'INDEX', 'INSERT', 'SELECT', 'UPDATE',
Expand Down Expand Up @@ -344,30 +366,97 @@ def user_exists(cursor, user, host, host_all):
return count[0] > 0


def user_add(cursor, user, host, host_all, password, encrypted,
plugin, plugin_hash_string, plugin_auth_string, new_priv, check_mode):
def sanitize_requires(tls_requires):
sanitized_requires = {}
if tls_requires:
for key in tls_requires.keys():
sanitized_requires[key.upper()] = tls_requires[key]
if any([key in ['CIPHER', 'ISSUER', 'SUBJECT'] for key in sanitized_requires.keys()]):
sanitized_requires.pop('SSL', None)
sanitized_requires.pop('X509', None)
return sanitized_requires

if 'X509' in sanitized_requires.keys():
sanitized_requires = 'X509'
else:
sanitized_requires = 'SSL'

return sanitized_requires
return None


def mogrify_requires(query, params, tls_requires):
if tls_requires:
if isinstance(tls_requires, dict):
k, v = zip(*tls_requires.items())
requires_query = ' AND '.join(('%s %%s' % key for key in k))
params += v
else:
requires_query = tls_requires
query = ' REQUIRE '.join((query, requires_query))
return query, params


def do_not_mogrify_requires(query, params, tls_requires):
return query, params


def get_tls_requires(cursor, user, host):
if user:
if server_suports_requires_create(cursor):
query = "SHOW CREATE USER '%s'@'%s'" % (user, host)
else:
query = "SHOW GRANTS for '%s'@'%s'" % (user, host)

cursor.execute(query)
require_list = list(filter(lambda x: 'REQUIRE' in x, cursor.fetchall()))
require_line = require_list[0] if require_list else ''
pattern = r"(?<=\bREQUIRE\b)(.*?)(?=(?:\bPASSWORD\b|$))"
requires_match = re.search(pattern, require_line)
requires = requires_match.group().strip() if requires_match else ''
if len(requires.split()) > 1:
import shlex
items = iter(shlex.split(requires))
requires = dict(zip(items, items))
return requires or None


def get_grants(cursor, user, host):
cursor.execute('SHOW GRANTS FOR %s@%s', (user, host))
grants_line = list(filter(lambda x: 'ON *.*' in x[0], cursor.fetchall()))[0]
pattern = r"(?<=\bGRANT\b)(.*?)(?=(?:\bON\b))"
grants = re.search(pattern, grants_line[0]).group().strip()
return grants.split(', ')


def user_add(cursor, user, host, host_all, password, encrypted, plugin, plugin_hash_string,
plugin_auth_string, new_priv, tls_requires, check_mode):
# we cannot create users without a proper hostname
if host_all:
return False

if check_mode:
return True

mogrify = mogrify_requires if server_suports_requires_create(cursor) else do_not_mogrify_requires

if password and encrypted:
cursor.execute("CREATE USER %s@%s IDENTIFIED BY PASSWORD %s", (user, host, password))
cursor.execute(*mogrify("CREATE USER %s@%s IDENTIFIED BY PASSWORD %s", (user, host, password), tls_requires))
elif password and not encrypted:
cursor.execute("CREATE USER %s@%s IDENTIFIED BY %s", (user, host, password))
cursor.execute(*mogrify("CREATE USER %s@%s IDENTIFIED BY %s", (user, host, password), tls_requires))
elif plugin and plugin_hash_string:
cursor.execute("CREATE USER %s@%s IDENTIFIED WITH %s AS %s", (user, host, plugin, plugin_hash_string))
cursor.execute(*mogrify("CREATE USER %s@%s IDENTIFIED WITH %s AS %s", (user, host, plugin, plugin_hash_string), tls_requires))
elif plugin and plugin_auth_string:
cursor.execute("CREATE USER %s@%s IDENTIFIED WITH %s BY %s", (user, host, plugin, plugin_auth_string))
cursor.execute(*mogrify("CREATE USER %s@%s IDENTIFIED WITH %s BY %s", (user, host, plugin, plugin_auth_string), tls_requires))
elif plugin:
cursor.execute("CREATE USER %s@%s IDENTIFIED WITH %s", (user, host, plugin))
cursor.execute(*mogrify("CREATE USER %s@%s IDENTIFIED WITH %s", (user, host, plugin), tls_requires))
else:
cursor.execute("CREATE USER %s@%s", (user, host))
cursor.execute(*mogrify("CREATE USER %s@%s", (user, host), tls_requires))
if new_priv is not None:
for db_table, priv in iteritems(new_priv):
privileges_grant(cursor, user, host, db_table, priv)
privileges_grant(cursor, user, host, db_table, priv, tls_requires)
if tls_requires is not None:
privileges_grant(cursor, user, host, '*.*', get_grants(cursor, user, host), tls_requires)
return True


Expand All @@ -379,8 +468,8 @@ def is_hash(password):
return ishash


def user_mod(cursor, user, host, host_all, password, encrypted,
plugin, plugin_hash_string, plugin_auth_string, new_priv, append_privs, module):
def user_mod(cursor, user, host, host_all, password, encrypted, plugin, plugin_hash_string,
plugin_auth_string, new_priv, append_privs, tls_requires, module):
changed = False
msg = "User unchanged"
grant_option = False
Expand Down Expand Up @@ -453,6 +542,7 @@ def user_mod(cursor, user, host, host_all, password, encrypted,
"UPDATE mysql.user SET plugin = %s, authentication_string = %s, Password = '' WHERE User = %s AND Host = %s",
('mysql_native_password', encrypted_password, user, host)
)
cursor.execute("GRANT USAGE on *.* to '%s'@'%s'", (user, host))
cursor.execute("FLUSH PRIVILEGES")
msg = "Password forced update"
else:
Expand All @@ -461,8 +551,7 @@ def user_mod(cursor, user, host, host_all, password, encrypted,

# Handle plugin authentication
if plugin:
cursor.execute("SELECT plugin, authentication_string FROM mysql.user "
"WHERE user = %s AND host = %s", (user, host))
cursor.execute("SELECT plugin, authentication_string FROM mysql.user WHERE user = %s AND host = %s", (user, host))
current_plugin = cursor.fetchone()

update = False
Expand Down Expand Up @@ -514,7 +603,7 @@ def user_mod(cursor, user, host, host_all, password, encrypted,
msg = "New privileges granted"
if module.check_mode:
return (True, msg)
privileges_grant(cursor, user, host, db_table, priv)
privileges_grant(cursor, user, host, db_table, priv, tls_requires)
changed = True

# If the db.table specification exists in both the user's current privileges
Expand All @@ -528,9 +617,28 @@ def user_mod(cursor, user, host, host_all, password, encrypted,
return (True, msg)
if not append_privs:
privileges_revoke(cursor, user, host, db_table, curr_priv[db_table], grant_option)
privileges_grant(cursor, user, host, db_table, new_priv[db_table])
privileges_grant(cursor, user, host, db_table, new_priv[db_table], tls_requires)
changed = True

# Handle TLS requirements
current_requires = get_tls_requires(cursor, user, host)
if current_requires != tls_requires:
msg = "TLS requires updated"
if module.check_mode:
return (True, msg)
if server_suports_requires_create(cursor):
pre_query = "ALTER USER"
else:
pre_query = 'GRANT %s ON *.* TO' % ','.join(get_grants(cursor, user, host))

if tls_requires is not None:
query = ' '.join((pre_query, '%s@%s'))
cursor.execute(*mogrify_requires(query, (user, host), tls_requires))
else:
query = ' '.join(pre_query, '%s@%s REQUIRE NONE')
cursor.execute(query, (user, host))
changed = True

return (changed, msg)


Expand Down Expand Up @@ -667,19 +775,23 @@ def privileges_revoke(cursor, user, host, db_table, priv, grant_option):
cursor.execute(query, (user, host))


def privileges_grant(cursor, user, host, db_table, priv):
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')])
query = ["GRANT %s ON %s" % (priv_string, db_table)]
query.append("TO %s@%s")
if 'REQUIRESSL' in priv:
params = (user, host)
if tls_requires and not server_suports_requires_create(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)
cursor.execute(query, (user, host))
cursor.execute(query, params)


def convert_priv_dict_to_str(priv):
Expand All @@ -696,6 +808,33 @@ def convert_priv_dict_to_str(priv):
return '/'.join(priv_list)


# TLS requires on user create statement is supported since MySQL 5.7 and MariaDB 10.2
def server_suports_requires_create(cursor):
"""Check if the server supports REQUIRES on the CREATE USER statement or doesn't.

Args:
cursor (cursor): DB driver cursor object.

Returns: True if supports, False otherwise.
"""
cursor.execute("SELECT VERSION()")
version_str = cursor.fetchone()[0]
version = version_str.split('.')

if 'mariadb' in version_str.lower():
# MariaDB 10.2 and later
if int(version[0]) * 1000 + int(version[1]) >= 10002:
return True
else:
return False
else:
# MySQL 5.6 and later
if int(version[0]) * 1000 + int(version[1]) >= 5007:
return True
else:
return False


# 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.
Expand Down Expand Up @@ -847,6 +986,7 @@ def main():
host_all=dict(type="bool", default=False),
state=dict(type='str', default='present', choices=['absent', 'present']),
priv=dict(type='raw'),
tls_requires=dict(type='dict'),
append_privs=dict(type='bool', default=False),
check_implicit_admin=dict(type='bool', default=False),
update_password=dict(type='str', default='always', choices=['always', 'on_create'], no_log=False),
Expand All @@ -872,6 +1012,7 @@ def main():
host_all = module.params["host_all"]
state = module.params["state"]
priv = module.params["priv"]
tls_requires = sanitize_requires(module.params["tls_requires"])
check_implicit_admin = module.params['check_implicit_admin']
connect_timeout = module.params['connect_timeout']
config_file = module.params['config_file']
Expand Down Expand Up @@ -930,11 +1071,11 @@ def main():
if update_password == 'always':
changed, msg = user_mod(cursor, user, host, host_all, password, encrypted,
plugin, plugin_hash_string, plugin_auth_string,
priv, append_privs, module)
priv, append_privs, tls_requires, module)
else:
changed, msg = user_mod(cursor, user, host, host_all, None, encrypted,
plugin, plugin_hash_string, plugin_auth_string,
priv, append_privs, module)
priv, append_privs, tls_requires, module)

except (SQLParseError, InvalidPrivsError, mysql_driver.Error) as e:
module.fail_json(msg=to_native(e))
Expand All @@ -944,7 +1085,7 @@ def main():
try:
changed = user_add(cursor, user, host, host_all, password, encrypted,
plugin, plugin_hash_string, plugin_auth_string,
priv, module.check_mode)
priv, tls_requires, module.check_mode)
if changed:
msg = "User added"

Expand Down
Loading