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 module doesn't support TLS/SSL connections #50106

Closed
travispaul opened this issue Oct 18, 2018 · 10 comments
Closed

MySQL module doesn't support TLS/SSL connections #50106

travispaul opened this issue Oct 18, 2018 · 10 comments
Labels
Bug broken, incorrect, or confusing behavior Confirmed Salt engineer has confirmed bug/feature - often including a MCVE severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Milestone

Comments

@travispaul
Copy link
Contributor

Description of Issue/Question

The MySQL module does not expose the TLS/SSL connection options from the MySQLdb library.

ssl
This parameter takes a dictionary or mapping, where the keys are parameter names used by the mysql_ssl_set MySQL C API call. If this is set, it initiates an SSL connection to the server; if there is no SSL support in the client, an exception is raised. This must be a keyword parameter.

These connection options are absent from the module as well (not just the documentation):
https://github.com/saltstack/salt/blob/develop/salt/modules/mysql.py#L314

@Ch3LL
Copy link
Contributor

Ch3LL commented Oct 18, 2018

i'm wondering if this is what you are looking for: https://github.com/saltstack/salt/blob/develop/salt/modules/mysql.py#L115-L123

and it looks like there is a ssl sanitize function here: https://github.com/saltstack/salt/blob/develop/salt/modules/mysql.py#L1665

@Ch3LL Ch3LL added the info-needed waiting for more info label Oct 18, 2018
@Ch3LL Ch3LL added this to the Blocked milestone Oct 18, 2018
@travispaul
Copy link
Contributor Author

Hi @Ch3LL ,

Sorry if my initial details were vague. The keywords you linked to are used as part of a query with the REQUIRE clause in the CREATE USER or GRANT syntax (e.g: GRANT SELECT ON `someuser`.* TO 'somedb'@'192.168.1.100' REQUIRE SSL). I'm not aware a situation where those keywords will change the connection settings.

What I'm looking for (and might take a stab at adding this weekend) is the ability to provide SSL/TLS settings before the connection is initiated so that queries and their responses are not sent over the network in plain-text when using any of the MySQL modules.

Below are some examples of the settings I need. One wouldn't necessarily use all of these settings at the same time. Most often we only need to set mysql.ssl_ca for Amazon RDS though we do use the other settings in some cases:

mysql.host: 'some-remote-host.tld'
mysql.port: 3306
mysql.user: 'some-user'
mysql.pass: 'some-pass'
mysql.db: 'mysql'
mysql.charset: 'utf8'

# desired options:
mysql.ssl_key: '/path/to/client.key'
mysql.ssl_cert: '/path/to/client.cert'
mysql.ssl_ca: '/path/to/ca.cert'
mysql.ssl_capath: '/path/to/ca-dir'
mysql.ssl_cipher: 'DHE-RSA-AES128-GCM-SHA256'

@travispaul
Copy link
Contributor Author

Just wanted to add an update from my attempts over the weekend. I thought I might be able to get away with setting the SSL options in a cnf file using the mysql.default_file option. However in every case I tried the SSL options seem to be ignored. This appears to be a problem in the MySQLdb library as far as I could tell.

@Ch3LL
Copy link
Contributor

Ch3LL commented Oct 22, 2018

thanks for clarifying @travispaul . I took a look at the mysql docs: https://dev.mysql.com/doc/connector-python/en/connector-python-connectargs.html

and it looks like we just need to add those ssl options to the connection args list there. Want to try that out and see if it works. We are willing to accept a PR on this.

@Ch3LL Ch3LL added Bug broken, incorrect, or confusing behavior severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around P3 Priority 3 and removed info-needed waiting for more info labels Oct 22, 2018
@Ch3LL Ch3LL modified the milestones: Blocked, Approved Oct 22, 2018
@travispaul
Copy link
Contributor Author

@Ch3LL it's definitely on my shortlist but it might be a week or two given some upcoming travel. I'll continue to update this issue if I make any progress.

@kveroneau
Copy link

kveroneau commented Nov 14, 2019

Is there any updates to this by chance? I am working with an Azure MySQL managed DB at work, and Azure Databases requires SSL to work, thus the Salt states I created and tested in my local VM do not work whatsoever against the Azure Database. I am sure this is also the case for Amazon RDS as well. I will need to do this salt states manually for the time being until this issue is resolved.

Small update, apparently in the Azure control panel, I can disable SSL enforcement. Although it still does not solve the problem that our application's data is flowing freely in plain-text over the public internet in some instances.

Hmm, nevermind. Although SSL enforcement is turned off, I still receive the following error: InterfaceError (-1, 'error totally whack'). I am using python-mysqldb 1.3.7-1.1, which works fine with a locally installed MySQL 5.7 instance. I will try python-pymysql next.

@travispaul
Copy link
Contributor Author

@kveroneau just an FYI, I ended up taking another approach for my use-case and never followed up with trying to implement MySQL over SSL within Salt

@stale
Copy link

stale bot commented Jan 7, 2020

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.

@stale stale bot added the stale label Jan 7, 2020
@waynew waynew added the Confirmed Salt engineer has confirmed bug/feature - often including a MCVE label Jan 8, 2020
@stale
Copy link

stale bot commented Jan 8, 2020

Thank you for updating this issue. It is no longer marked as stale.

@stale stale bot removed the stale label Jan 8, 2020
@sagetherage sagetherage removed the P3 Priority 3 label Jun 3, 2020
@twangboy
Copy link
Contributor

twangboy commented Sep 6, 2023

Closing this issue due to age and lack of activity. Please test this on version 3006.2 and create a new issue if the problem persists. The new issue template has more information and will allow us to track and reproduce the issue more effectively. Thanks!

@twangboy twangboy closed this as completed Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Confirmed Salt engineer has confirmed bug/feature - often including a MCVE severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Projects
None yet
Development

No branches or pull requests

6 participants