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

*: support require-secure-transport startup option #15341

Merged
merged 4 commits into from
Mar 17, 2020
Merged

*: support require-secure-transport startup option #15341

merged 4 commits into from
Mar 17, 2020

Conversation

lysu
Copy link
Contributor

@lysu lysu commented Mar 12, 2020

What problem does this PR solve?

fixes #15306

What is changed and how it works?

  • add --require-secure-transport startup option
  • reject non-TLS client connect to TiDB if require-secure-transport be enabled
  • startup failure if require-secure-transport be enabled but no avaliable cert/key/ca
  • reload failure if require-secure-transport be enabled event if with no rollback on error

Check List

Tests

  • Unit test
  • Integration test
  • Manual test
enable `require-secure-transport` and try non-TLS client

Code changes

  • imple change

Side effects

  • n/a

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation
  • Need to update the tidb-ansible repository

Release note

  • support --require-secure-transport startup option.

This change is Reviewable

@lysu lysu requested a review from a team as a code owner March 12, 2020 11:44
@ghost ghost requested review from SunRunAway and wshwsh12 and removed request for a team March 12, 2020 11:44
@lysu
Copy link
Contributor Author

lysu commented Mar 12, 2020

/run-all-tests

@github-actions github-actions bot added the sig/execution SIG execution label Mar 12, 2020
@lysu lysu requested review from kolbe and jackysp and removed request for SunRunAway and wshwsh12 March 12, 2020 11:45
@lysu
Copy link
Contributor Author

lysu commented Mar 12, 2020

/run-unit-test

nmPluginLoad = "plugin-load"
nmRepairMode = "repair-mode"
nmRepairList = "repair-list"
nmRequireSecureTransport = "require-secure-transport"
Copy link
Member

Choose a reason for hiding this comment

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

I know it is the same name as MySQL's, but I'm not sure adding it is approved.
\cc @siddontang

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

I use mysql --help but don't find hat it uses this in command flags.

Copy link
Member

Choose a reason for hiding this comment

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

It is a flag of mysqld, try mysqld --verbose --help

server/conn.go Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Mar 13, 2020

Codecov Report

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

@@               Coverage Diff               @@
##            master     #15341        +/-   ##
===============================================
+ Coverage   80.532%   80.6704%   +0.1383%     
===============================================
  Files          502        503         +1     
  Lines       134534     135968      +1434     
===============================================
+ Hits        108343     109686      +1343     
- Misses       17765      17845        +80     
- Partials      8426       8437        +11

@shenli
Copy link
Member

shenli commented Mar 15, 2020

@kolbe @jackysp PTAL

Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

@jackysp jackysp added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 16, 2020
Copy link
Contributor

@kolbe kolbe left a comment

Choose a reason for hiding this comment

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

LGTM

@zz-jason zz-jason merged commit aec6143 into pingcap:master Mar 17, 2020
sre-bot pushed a commit to sre-bot/tidb that referenced this pull request Mar 17, 2020
Signed-off-by: sre-bot <sre-bot@pingcap.com>
@sre-bot
Copy link
Contributor

sre-bot commented Mar 17, 2020

cherry pick to release-3.0 in PR #15415

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/config component/server security Everything related with security sig/execution SIG execution status/LGT1 Indicates that a PR has LGTM 1.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add require_secure_transport
7 participants