*: Support for caching_sha2_password authentication#24991
*: Support for caching_sha2_password authentication#24991ti-chi-bot merged 4 commits intopingcap:masterfrom
Conversation
|
Please follow PR Title Format:
Or if the count of mainly changed packages are more than 3, use
|
|
No release note, Please follow https://github.com/pingcap/community/blob/master/contributors/release-note-checker.md |
server/conn.go
Outdated
There was a problem hiding this comment.
You could potentially use an authSwitchRequest (from deleted code above) if the plugin is unknown. The downside is that this is hard to test.
There was a problem hiding this comment.
If the client presents an authentication plugin that doesn't match the plugin used by the server checkAuthPlugin will do the authSwitchRequest so I don't think this code will be reached during normal operation. It may be reached if someone bypasses the GRANT/REVOKE etc and changes mysql.user directly.
ddd7e50 to
517069e
Compare
517069e to
1b57a5d
Compare
1b57a5d to
9b5bd45
Compare
9b5bd45 to
1246b7a
Compare
server/conn.go
Outdated
There was a problem hiding this comment.
If open session success, but do auth fails, will there be a potential resource leak?
server/conn.go
Outdated
There was a problem hiding this comment.
So, there are:
- the default auth plugin (we set in the initial handshake packet)
- the user's auth plugin, which stores in mysql.user authentication related column
- the auth plugin from the client response packet
Which one is which? I'm a bit confused.
Pass *string as both input and output of a function is an idiom in C.
Go support multiple return, you don't need to do that.
1246b7a to
05b10b4
Compare
Issue link: pingcap#9411 What this does: - Check the `plugin` column of the `mysql.user` table. - Based on the plugin from the user record and the plugin the client sent we may need to switch the authentication plugin to match the one from the user record - For accounts with `caching_sha2_password` send the "fast authentication failed" response to trigger full authentication. - call `auth.CheckShaPassword` to validate the user. Implemented functionality: - Full authentication with `caching_sha2_password` over TLS - The `default_authentication_plugin` variable - `CREATE USER... IDENTIFIED WITH 'caching_sha2_password'...` - `SET PASSWORD...` - `ALTER USER ... IDENTIFIED BY...` Missing functionality: - Support for the RSA public key request packet & response - Support for RSA key based secret exchange - Fast authentication (validate against cached entry) Related: - Requires pingcap/parser#1232 - pingcap#24141 makes testing of this easier, but this is not required.
21af7cc to
82f365c
Compare
|
/assign @tiancaiamao @morgo |
| if err != nil { | ||
| return err | ||
| } | ||
| cc.ctx = nil |
There was a problem hiding this comment.
Just a hint: open a new session may need to load the global variables (maybe not true when they're cached), and do it once again could slow down the connection speed.
There was a problem hiding this comment.
I agree with your comment.
The cache is improved with sysvar cache, but it is still expensive. The current code will re-run validation on all of the sysvars. This can be improved though: in future we can cache the validated values in the sysvar cache, and when a session is initialized just copy the sysvar cache map to it.
The complicated case will be eliminating the need to call the 'init functions' (i.e. calling sv.SetSession() so that s.VarXXX is set.) I think we can remove a lot of SetSession() funcs by adding something like sv.ToBool(), sv.ToUint64().
There was a problem hiding this comment.
Indeed, creating a new session for handshake is relatively heavy, indirectly leading to the occurrence of #44502.
|
@morgo would you like to take another look? or I'll merge this one. |
It LGTM. There are some known issues with improving fast connection/disconnection rate, but these are existing. We should suggest the QA team have scenario testing for it because it will affect connectors like php and perl. Issues like #24326 could also be discovered quicker. |
|
/merge |
|
This pull request has been accepted and is ready to merge. DetailsCommit hash: e2e9861 |
|
@dveeden: Your PR was out of date, I have automatically updated it for you. At the same time I will also trigger all tests for you: /run-all-tests If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository. |
Related to pingcap/tidb#24991 Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>
What problem does this PR solve?
Issue Number: close #9411
Problem Summary:
mysql_native_passworduses the SHA1 hash algorithm which is not future proofcaching_sha2_passwordfrom MySQL to TiDB doesn't workWhat is changed and how it works?
What this does:
plugincolumn of themysql.usertable.sent we may need to switch the authentication plugin to match the
one from the user record
caching_sha2_passwordsend the "fastauthentication failed" response to trigger full authentication.
auth.CheckShaPasswordto validate the user.Implemented functionality:
caching_sha2_passwordover TLSdefault_authentication_pluginvariableCREATE USER... IDENTIFIED WITH 'caching_sha2_password'...Missing functionality:
Related changes
of this easier, but this is not required.
Tests
Create users with:
mysql_native_passwordcaching_sha2_passwordTry to authenticate with all of them.
Side effects
Release note
caching_sha2_passwordauthentication