mycli icon indicating copy to clipboard operation
mycli copied to clipboard

Add cli option --tls-version to control tls version used

Open meldafert opened this issue 2 years ago • 2 comments

Description

Fixes #1053.

This PR adds a --tls-version option, parallel to the option of the vanilla mysql client. It was implemented by bypassing PyMySQL's ssl logic and creating the SSLContext ourselves.

This PR is currently a draft, as it would require raising the minimum python version to 3.7, as 3.6 has no way to force TLSv1.3. Specifically, SSLContext.minimum_version was only added in 3.7. However, PyMySQL's next update will require 3.7 as well.

This could alternatively be partially implemented inside PyMySQL as well. However, it is a documented option to pass a SSLContext to PyMySQL directly, so this seemed like the easier option. One downside is that the _create_ssl_context method was strongly influenced by the one in PyMySQL - does this need to be noted somewhere for copyright reasons?

Checklist

  • [x] I've added this contribution to the changelog.md.
  • [x] I've added my name to the AUTHORS file (or it's already there).

meldafert avatar Aug 19 '22 12:08 meldafert

Codecov Report

Merging #1073 (1561834) into main (48bd4c9) will decrease coverage by 0.81%. The diff coverage is 14.28%.

@@            Coverage Diff             @@
##             main    #1073      +/-   ##
==========================================
- Coverage   67.92%   67.10%   -0.82%     
==========================================
  Files          26       26              
  Lines        2756     2791      +35     
==========================================
+ Hits         1872     1873       +1     
- Misses        884      918      +34     
Impacted Files Coverage Δ
mycli/sqlexecute.py 74.40% <9.37%> (-9.55%) :arrow_down:
mycli/packages/completion_engine.py 61.90% <50.00%> (-0.17%) :arrow_down:
mycli/main.py 65.32% <100.00%> (+0.04%) :arrow_up:
mycli/sqlcompleter.py 82.53% <0.00%> (-1.75%) :arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov-commenter avatar Aug 27 '22 04:08 codecov-commenter

Might be good to consider lists/ranges of TLS versions.

  • --tls-version TLSv1.2,TLSv1.3 Allowing these two versions only
  • --min-tls-version TLSv1.2 Allowing TLSv1.2 and up

Might be good to by default only allow TLSv1.2 and up. See RFC 8996

dveeden avatar Sep 23 '22 07:09 dveeden

Thanks!

meldafert avatar Apr 03 '24 09:04 meldafert