Added to pass ssl_mode in configuration. #347

migimigi · 2019-03-19T09:20:56Z

SSL was enabled by default when connecting to MySQL 5.7.
See https://dev.mysql.com/worklog/task/?id=7712

Added to configuration to enable/disable SSL.
The reason for not using --ssl or --skip-ssl described in MySQL Worklog, --ssl option is deprecated as of MySQL 5.7.11.
See https://dev.mysql.com/doc/refman/5.7/en/encrypted-connection-options.html

codecov · 2019-03-19T09:33:22Z

Codecov Report

Merging #347 into master will increase coverage by 0.04%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #347      +/-   ##
==========================================
+ Coverage   87.14%   87.18%   +0.04%     
==========================================
  Files          13       14       +1     
  Lines        1556     1561       +5     
==========================================
+ Hits         1356     1361       +5     
  Misses        200      200

Impacted Files	Coverage Δ
MySQLdb/__init__.py	`84.78% <100%> (ø)`	⬆️
MySQLdb/constants/__init__.py	`100% <100%> (ø)`	⬆️
MySQLdb/constants/SSL_MODE.py	`100% <100%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e04c597...fd02b7d. Read the comment docs.

codecov · 2019-03-19T09:33:22Z

Codecov Report

Merging #347 into master will increase coverage by 0.14%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #347      +/-   ##
==========================================
+ Coverage   86.61%   86.75%   +0.14%     
==========================================
  Files          12       12              
  Lines        1531     1533       +2     
==========================================
+ Hits         1326     1330       +4     
+ Misses        205      203       -2

Impacted Files	Coverage Δ
MySQLdb/connections.py	`81.81% <ø> (+1.67%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8fd628e...bd87447. Read the comment docs.

methane · 2019-11-28T09:01:26Z

I think there is no MYSQL_OPT_SSL_MODE in old MySQL Connector/C or MariaDB.
Would you test it?

migimigi · 2019-11-28T11:15:47Z

MYSQL_OPT_SSL_MODE exists from MySQL5.5.55.
https://dev.mysql.com/doc/relnotes/mysql/5.5/en/news-5-5-55.html#mysqld-5-5-55-security
I don't know about MariaDB.
I'm going to investigate, fix and test.
I couldn't find MySQL and MariaDB versions supported by mysqlclient-python at https://mysqlclient.readthedocs.io/
Is it there?

methane · 2019-11-28T11:25:48Z

MySQL 5.5+ are supported.

methane · 2019-11-28T11:29:22Z

I used MYSQL_DEFAULT_AUTH which is introduced in MySQL 5.5.10. (see #396)
So MySQL 5.5.10+ are supported now.

I wll drop Python 2 support. So I am OK to drop some old MySQL support.
But 5.5.55 looks too new.

migimigi · 2019-11-28T11:32:57Z

I see.

… MYSQL_OPT_SSL_MODE is defined

migimigi · 2019-11-29T10:45:51Z

The version in which MYSQL_OPT_SSL_MODE is defined
5.5.55- https://dev.mysql.com/doc/relnotes/mysql/5.5/en/news-5-5-55.html#mysqld-5-5-55-security
5.6.36- https://dev.mysql.com/doc/relnotes/mysql/5.6/en/news-5-6-36.html#mysqld-5-6-36-security
5.7.11- https://dev.mysql.com/doc/relnotes/mysql/5.7/en/news-5-7-11.html#mysqld-5-7-11-security
8.0.0- https://dev.mysql.com/doc/relnotes/mysql/8.0/en/news-8-0-0.html#mysqld-8-0-0-security
Other than these versions were disabled with #if.

MYSQL_OPT_SSL_MODE was found to be not defined in MariaDB.
So it was disabled in MariaDB.
https://github.com/MariaDB/server/blob/10.5/include/mysql.h#L184
brianmario/mysql2#879 (comment)

methane · 2019-11-29T12:21:13Z

Options should be similar to my.cnf file. Imagine that users may load options from their config file.
So ssl_mode="VERIFY_CA", not ssl_mode=SSL_MODE.VERIFY_CA.

migimigi · 2019-12-02T08:07:28Z

It has modified

methane · 2019-12-02T09:26:43Z

I don't like this. This implementation ignores the ssl_mode option silently when it is not supported by the underlying library.

There are two options to fix this:

Raise an error if ssl_mode is not supported by the underlying library.
Support ssl_mode using legacy options.

migimigi · 2019-12-02T12:21:34Z

I see.
Choose to raises an error if the ssl_mode setting is not supported.
The legacy option was deprecated and was not used.

methane · 2019-12-02T23:32:11Z

MySQLdb/_mysql.c

+        (PyCFunction)_mysql_get_have_enum_mysql_opt_ssl_mode,
+        METH_NOARGS,
+        _mysql_get_have_enum_mysql_opt_ssl_mode__doc__
+    },


Don't add new API.

methane · 2019-12-02T23:32:24Z

MySQLdb/connections.py

@@ -102,6 +102,13 @@ class object, used to create cursors (keyword only)
        :param int client_flag:
            flags to use or 0 (see MySQL docs or constants/CLIENTS.py)

+        :param str ssl_mode


Suggested change

:param str ssl_mode

:param str ssl_mode:

methane · 2019-12-02T23:32:53Z

MySQLdb/connections.py

+        if 'ssl_mode' in kwargs:
+            if not _mysql.get_have_enum_mysql_opt_ssl_mode():
+                raise NotSupportedError('Does not support ssl_mode specification: %s' % _mysql.get_client_info())
+            if hasattr(SSL_MODE, kwargs['ssl_mode']):
+                kwargs2['ssl_mode'] = getattr(SSL_MODE, kwargs['ssl_mode'])
+            else:
+                raise NotSupportedError('Unknown MySQL ssl_mode specification: %s' % kwargs['ssl_mode'])
+


Implement this block in C.

Implemented error checking on the C lang side.
And removed MySQLdb.constants.SSL_MODE, which is no longer needed.

This reverts commit e57c474.

methane · 2019-12-03T10:57:03Z

Thanks!

Jun Yamasaki added 2 commits March 19, 2019 16:44

Add ssl_mode can be passed.

b28e723

Add a constant to specify SSL_MODE.

fd02b7d

fix typo

35b74ce

migimigi changed the title ~~Added to pass sql_mode in configuration.~~ Added to pass ssl_mode in configuration. Mar 20, 2019

methane added this to the v2.0 milestone Nov 18, 2019

Jun Yamasaki and others added 2 commits November 29, 2019 19:33

Fix to enable the ssl_mode setting only for MySQL versions where enum…

53e19a7

… MYSQL_OPT_SSL_MODE is defined

Merge branch 'master' into feature/add_ssl_mode

73c4296

Modify ssl_mode setting from constant to string

08680b1

Raises an error if the ssl_mode setting is not supported

e57c474

methane reviewed Dec 2, 2019

View reviewed changes

Jun Yamasaki added 4 commits December 3, 2019 12:25

Revert "Raises an error if the ssl_mode setting is not supported"

8335d70

This reverts commit e57c474.

fix typo

09dc7e9

Remove MySQLdb.constants.SSL_MODE

67deee2

Check ssl_mode setting on C lang side

bd87447

methane merged commit 1a5ae1d into PyMySQL:master Dec 3, 2019

migimigi deleted the feature/add_ssl_mode branch December 3, 2019 11:17

migimigi restored the feature/add_ssl_mode branch December 4, 2019 01:27

parafoxx mentioned this pull request Dec 23, 2019

Defectdojo cant handle secure connections to DB DefectDojo/django-DefectDojo#1716

Closed

3 tasks

Added to pass ssl_mode in configuration. #347

Added to pass ssl_mode in configuration. #347

Uh oh!

Conversation

migimigi commented Mar 19, 2019

Uh oh!

codecov bot commented Mar 19, 2019

Codecov Report

Uh oh!

codecov bot commented Mar 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

methane commented Nov 28, 2019

Uh oh!

migimigi commented Nov 28, 2019

Uh oh!

methane commented Nov 28, 2019

Uh oh!

methane commented Nov 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

migimigi commented Nov 28, 2019

Uh oh!

migimigi commented Nov 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

methane commented Nov 29, 2019

Uh oh!

migimigi commented Dec 2, 2019

Uh oh!

methane commented Dec 2, 2019

Uh oh!

migimigi commented Dec 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

methane Dec 2, 2019

Choose a reason for hiding this comment

Uh oh!

migimigi Dec 3, 2019

Choose a reason for hiding this comment

Uh oh!

methane Dec 2, 2019

Choose a reason for hiding this comment

Uh oh!

migimigi Dec 3, 2019

Choose a reason for hiding this comment

Uh oh!

methane Dec 2, 2019

Choose a reason for hiding this comment

Uh oh!

migimigi Dec 3, 2019

Choose a reason for hiding this comment

Uh oh!

methane commented Dec 3, 2019

Uh oh!

Uh oh!

codecov bot commented Mar 19, 2019 •

edited

Loading

methane commented Nov 28, 2019 •

edited

Loading

migimigi commented Nov 29, 2019 •

edited

Loading

migimigi commented Dec 2, 2019 •

edited

Loading