Skip to content

Comments

Let --ssl_mode=auto prefer SSL only for TCP/IP#1600

Merged
rolandwalker merged 1 commit intomainfrom
RW/ssl-auto-except-local-socket
Feb 23, 2026
Merged

Let --ssl_mode=auto prefer SSL only for TCP/IP#1600
rolandwalker merged 1 commit intomainfrom
RW/ssl-auto-except-local-socket

Conversation

@rolandwalker
Copy link
Contributor

@rolandwalker rolandwalker commented Feb 21, 2026

Description

While it is possible to negotiate an SSL connection to MySQL over a local socket, it shouldn't be needed (should it?).

Nothing is transmitted over a network in that scenario.

If the user still does need SSL/TLS, --ssl-mode=on is available to force it.

@scottnemes does this PR make sense? I'm open to being convinced otherwise. It just made sense to me. (There could even be a fourth ssl_mode but that seemed like needless complexity.)

Checklist

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

@github-actions
Copy link

Findings

  1. Correctness: The new ssl_mode == "auto" socket check only runs in cli() before MyCli.connect() resolves a default socket. For the common case where the user does not pass --socket and connect() later picks one, SSL will still be attempted in auto mode, which contradicts the PR intent. Consider moving the “auto + socket => no SSL” decision into MyCli.connect() after socket resolution, or re-check once the effective socket is known. mycli/main.py:2011, mycli/main.py:576
  2. Correctness/behavior change: If a user explicitly sets --ssl (or DSN ssl=true) but leaves --ssl-mode=auto, the new code silently disables SSL whenever a socket is used. If that’s intended, add a warning like the existing override warning so the user understands why SSL was ignored. If not intended, you should treat explicit --ssl as a stronger signal than auto for sockets. mycli/main.py:2001, mycli/main.py:2011

Tests

  1. Add a test for socket + auto behavior. Ideally cover both:
  • --ssl-mode=auto with a socket (explicit --socket or implicit socket resolution) should result in no SSL (Ssl_cipher empty).
  • --ssl-mode=on with a socket should still negotiate SSL if the server supports it.

No security concerns found.

@rolandwalker
Copy link
Contributor Author

I think bot's suggestion 1 is wrong, and I'm not concerned about the interaction with the deprecated --ssl option.

@rolandwalker rolandwalker self-assigned this Feb 21, 2026
Copy link
Contributor

@scottnemes scottnemes left a comment

Choose a reason for hiding this comment

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

Yeah according to "experts" SSL isn't needed for a socket connection, so makes sense for that. Likely little impact in reality for our use case, but since it's a fairly straightforward fix then might as well.

Verified SSL is no longer used when connecting via socket and ssl-mode=auto and SSL is used with socket and ssl-mode=on.

While it is _possible_ to negotiate an SSL connection to MySQL over a
local socket, it shouldn't be needed (should it?).

Nothing is transmitted over a network in that scenario.

If the user still does need SSL/TLS, --ssl-mode=on is available to
force it.
@rolandwalker rolandwalker force-pushed the RW/ssl-auto-except-local-socket branch from 3f7f9b8 to b72d325 Compare February 23, 2026 09:23
@rolandwalker rolandwalker merged commit 459b692 into main Feb 23, 2026
8 checks passed
@rolandwalker rolandwalker deleted the RW/ssl-auto-except-local-socket branch February 23, 2026 09:28
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.

2 participants