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

server: add auth switch to mysql_native_password (#19603) #19959

Merged
merged 2 commits into from
Sep 16, 2020

Conversation

ti-srebot
Copy link
Contributor

cherry-pick #19603 to release-4.0


What problem does this PR solve?

Issue Number: Fixes #17875

Problem Summary:

The TiDB server does not allow clients from MySQL 8 servers to connect when they have a password.

What is changed and how it works?

What's Changed:

Support for MySQL 8 clients has been added.

How it Works:

The MySQL protocol allows the server to ask the client to change authentication plugins. This does not explicitly allow support for MySQL 8's caching_sha2_password, but it sidesteps the issue to allow MySQL 8 clients to connect to TiDB. Any client that speaks caching_sha2_password is asked to change to use mysql_native_password.

Related changes

  • Need to cherry-pick to the release branch

(It is certainly not required, butI think this is useful enough to cherry-pick.)

Check List

Tests

  • Manual test (add detailed scripts or steps below)

The go mysql connector does not permit sending caching_sha2_password, and then being downgraded, so it is hard to include in the testsuite directly under ./server/server_test.go. What I did was used a MySQL 8.0 CLI and performed the following:

$ mysql -e "SET password='test'; SELECT 1";
$ mysql -ptest -e "SELECT 1";

This confirms that it both allows non-passworded accounts and passworded to work correctly.

Side effects

  • None

Release note

  • TiDB now accepts connections from clients using connectors from MySQL 8.0.

@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ghost
Copy link

ghost commented Sep 11, 2020

/run-all-tests

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

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 12, 2020
@tiancaiamao
Copy link
Contributor

LGTM

@ti-srebot ti-srebot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Sep 12, 2020
@jackysp
Copy link
Member

jackysp commented Sep 16, 2020

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Sep 16, 2020
@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot
Copy link
Contributor Author

@ti-srebot merge failed.

@jackysp
Copy link
Member

jackysp commented Sep 16, 2020

/run-integration-compatibility-test
/run-sqllogic-test-1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/server status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/4.0-cherry-pick
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants