Skip to content

Conversation

@scottnemes
Copy link
Contributor

Description

What:
Currently mycli does not connect via SSL by default, while the official MySQL client does.

Why:
Connecting via SSL by default is more secure.

Resolves #760

Checklist

  • I've added this contribution to the changelog.md.
  • I've added my name to the AUTHORS file (or it's already there).
  • I ran uv run ruff check && uv run ruff format && uv run mypy --install-types . to lint and format the code.

@rolandwalker
Copy link
Contributor

While I favor this, there is a lot to consider.

The vendor client does not simply use SSL/TLS by default, but tries encrypted transport first, then retries without encryption on failure (https://dev.mysql.com/doc/refman/9.0/en/encrypted-connections.html). That's probably the minimum we need to do, to avoid a flood of incoming issues.

To help the user out, and to do better than the vendor client, we could also have a config-file option for encrypted transport with three states: on/off/auto, with the default of "auto".

_connect() already uses a try/except formulation to work out whether a password prompt is needed:

  • def _connect() -> None:

    so it might get complicated to also do the same thing for encrypted transport.

@scottnemes
Copy link
Contributor Author

While I favor this, there is a lot to consider.

The vendor client does not simply use SSL/TLS by default, but tries encrypted transport first, then retries without encryption on failure (https://dev.mysql.com/doc/refman/9.0/en/encrypted-connections.html). That's probably the minimum we need to do, to avoid a flood of incoming issues.

To help the user out, and to do better than the vendor client, we could also have a config-file option for encrypted transport with three states: on/off/auto, with the default of "auto".

_connect() already uses a try/except formulation to work out whether a password prompt is needed:

  • def _connect() -> None:

    so it might get complicated to also do the same thing for encrypted transport.

Good callout. I actually thought it would downgrade automatically as long as the server wasn't requiring SSL, so definitely did not expect that; I will rework that bit.

RE: the config, I did think about adding a config file option but thought it might be scope creep. But if you also think it's a good idea I will work on that bit as well!

@rolandwalker
Copy link
Contributor

I actually thought it would downgrade automatically as long as the server wasn't requiring SSL

Ah, well I haven't check pymysql docs/internals before writing that — maybe the library does!

@scottnemes
Copy link
Contributor Author

I actually thought it would downgrade automatically as long as the server wasn't requiring SSL

Ah, well I haven't check pymysql docs/internals before writing that — maybe the library does!

It doesn't apparently; that's why I was surprised anyway, hah! So it will need a decent amount of work to add in the config and retry logic. Will take a look though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Access denied for user

2 participants