-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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, privilege: Socket authentication #27561
Conversation
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
b60c8d4
to
0829868
Compare
12d48c5
to
abbe998
Compare
bb425fc
to
13ff4f7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, minor nits.
/run-check_dev_2 |
Tested on macOS 11.6 on Apple M1 (arm cpu):
with
Which is the same when |
Same test environment but with > mysql -S /tmp/tidb.sock -u root
ERROR 1045 (28000): Access denied for user 'root'@'localhost' (using password: NO) I assume that
|
Same result on Ubuntu 21.04 x64 on
|
Looks like PeerHost is called too late, so cc.peerHost is not yet set which makes the |
@mjonss does the |
@@ -414,6 +414,7 @@ func (s *Server) startNetworkListener(listener net.Listener, isUnixSocket bool, | |||
} | |||
|
|||
clientConn.isUnixSocket = true | |||
clientConn.peerHost = "localhost" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a constant you can use for "localhost": variable.DefHostname
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not changing that as the "Default Hostname" is not guaranteed to remain "localhost" in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Tested on macOS 11.6 on Apple M1 cpu and ubuntu 21.04 x64, and it now works also for 'proxy' user.
/merge |
This pull request has been accepted and is ready to merge. Commit hash: d50ac88
|
Related PR: pingcap/tidb#27561
can not build on windows after this pr |
While Windows now seems to have Can you try dveeden@3d40baf and see if that fixes the build for the windows target for you? |
@dveeden it works! thank you. |
What problem does this PR solve?
Support MySQL compatible Socket Authentication (
auth_socket
).This could be used to provide a more secure alternative to the default account.
See also:
What is changed and how it works?
Check List
Tests
Documentation
Release note
Notes for reviews
tidb-server
withsocket = "/tmp/tidb.sock"
in your config.CREATE USER 'myuser'@'%' IDENTIFIED WITH 'auth_socket'
(replacemyuser
with your UNIX username)mysqlsh --sql mysql://myuser@(/tmp/tidb.sock)/
(shoul work)mysqlsh --sql mysql://myuser@127.0.0.1:4000/
CREATE USER 'my_other_user'@'%' IDENTIFIED WITH 'auth_socket' AS 'myuser'
(replacemyuser
with your UNIX username)mysqlsh --sql mysql://my_other_user@(/tmp/tidb.sock)/
as UNIX usermyuser
. This tests the mapping between UNIX and TiDB users.