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

Migration script does not respect db.use_ssl property #10275

Closed
pvannierop opened this issue Jul 14, 2023 · 0 comments · Fixed by #10353
Closed

Migration script does not respect db.use_ssl property #10275

pvannierop opened this issue Jul 14, 2023 · 0 comments · Fixed by #10353
Assignees

Comments

@pvannierop
Copy link
Contributor

pvannierop commented Jul 14, 2023

Affected PR: #10037

Problem

When migrating the database the migration.py script tries to connect to the MySQL database over SSL, despite the db.use_ssl property having value false.

Analysis

The current and broken logic in migration.py to configure SSL is:

connection_kwargs = {here
if portal_properties.database_use_ssl == 'true':
    connection_kwargs['ssl'] = {"ssl_mode": True}

When I change this to the following, the db.use_ssl property is respected:

connection_kwargs = {}
if portal_properties.database_use_ssl == 'true':
   connection_kwargs["ssl_mode"] = "REQUIRED"
else:
   connection_kwargs["ssl_mode"] = "DISABLED"

mysqlclient python library

Official documentation of mysqlclient python library states (see here):

ssl_mode
If present, specify the security settings for the connection to the server. For more information on ssl_mode, see the MySQL documentation. Only one of ‘DISABLED’, ‘PREFERRED’, ‘REQUIRED’, ‘VERIFY_CA’, ‘VERIFY_IDENTITY’ can be specified.
If not present, the session ssl_mode will be unchanged, but in version 5.7 and later, the default is PREFERRED.
This must be a keyword parameter.

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.

The ssl parameter can reference values mentioned here.

Discussion

  • Should we accept the proposed fix to the problem?
  • Does connection over SSL work at all with the current script? I have a hard time believing this because the ssl parameter does not receive correct parameters.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant