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

privilege: fix auth_socket bug, should only allow os user name to login (#54032) #54050

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 9 additions & 11 deletions pkg/privilege/privileges/privileges.go
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,13 @@ func (p *UserPrivileges) ConnectionVerification(user *auth.UserIdentity, authUse
logutil.BgLogger().Warn("verify through LDAP Simple failed", zap.String("username", user.Username), zap.Error(err))
return info, ErrAccessDenied.FastGenByArgs(user.Username, user.Hostname, hasPassword)
}
} else if record.AuthPlugin == mysql.AuthSocket {
if string(authentication) != authUser && string(authentication) != pwd {
logutil.BgLogger().Error("Failed socket auth", zap.String("authUser", authUser),
zap.String("socket_user", string(authentication)),
zap.String("authentication_string", pwd))
return info, ErrAccessDenied.FastGenByArgs(user.Username, user.Hostname, hasPassword)
}
} else if len(pwd) > 0 && len(authentication) > 0 {
switch record.AuthPlugin {
// NOTE: If the checking of the clear-text password fails, please set `info.FailedDueToWrongPassword = true`.
Expand All @@ -606,22 +613,13 @@ func (p *UserPrivileges) ConnectionVerification(user *auth.UserIdentity, authUse
info.FailedDueToWrongPassword = true
return info, ErrAccessDenied.FastGenByArgs(user.Username, user.Hostname, hasPassword)
}
case mysql.AuthSocket:
if string(authentication) != authUser && string(authentication) != pwd {
logutil.BgLogger().Error("Failed socket auth", zap.String("authUser", authUser),
zap.String("socket_user", string(authentication)),
zap.String("authentication_string", pwd))
return info, ErrAccessDenied.FastGenByArgs(user.Username, user.Hostname, hasPassword)
}
default:
logutil.BgLogger().Error("unknown authentication plugin", zap.String("authUser", authUser), zap.String("plugin", record.AuthPlugin))
return info, ErrAccessDenied.FastGenByArgs(user.Username, user.Hostname, hasPassword)
}
} else if len(pwd) > 0 || len(authentication) > 0 {
if record.AuthPlugin != mysql.AuthSocket {
info.FailedDueToWrongPassword = true
return info, ErrAccessDenied.FastGenByArgs(user.Username, user.Hostname, hasPassword)
}
info.FailedDueToWrongPassword = true
return info, ErrAccessDenied.FastGenByArgs(user.Username, user.Hostname, hasPassword)
}

// Login a locked account is not allowed.
Expand Down
13 changes: 12 additions & 1 deletion pkg/server/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -803,6 +803,9 @@ func (cc *clientConn) openSessionAndDoAuth(authData []byte, authPlugin string, z
return nil
}

// mockOSUserForAuthSocketTest should only be used in test
var mockOSUserForAuthSocketTest atomic.Pointer[string]

// Check if the Authentication Plugin of the server, client and user configuration matches
func (cc *clientConn) checkAuthPlugin(ctx context.Context, resp *handshake.Response41) ([]byte, error) {
// Open a context unless this was done before.
Expand Down Expand Up @@ -851,7 +854,15 @@ func (cc *clientConn) checkAuthPlugin(ctx context.Context, resp *handshake.Respo
if err != nil {
return nil, err
}
return []byte(user.Username), nil
uname := user.Username

if intest.InTest {
if p := mockOSUserForAuthSocketTest.Load(); p != nil {
uname = *p
}
}

return []byte(uname), nil
}
if len(userplugin) == 0 {
// No user plugin set, assuming MySQL Native Password
Expand Down
10 changes: 10 additions & 0 deletions pkg/server/mock_conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,3 +140,13 @@ func CreateMockConn(t *testing.T, server *Server) MockConn {
t: t,
}
}

// MockOSUserForAuthSocket mocks the OS user for AUTH_SOCKET plugin
func MockOSUserForAuthSocket(uname string) {
mockOSUserForAuthSocketTest.Store(&uname)
}

// ClearOSUserForAuthSocket clears the mocked OS user for AUTH_SOCKET plugin
func ClearOSUserForAuthSocket() {
mockOSUserForAuthSocketTest.Store(nil)
}
70 changes: 70 additions & 0 deletions pkg/server/tests/commontest/tidb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"io"
"net"
"os"
"path/filepath"
"runtime"
"strings"
"sync"
Expand Down Expand Up @@ -3087,3 +3088,72 @@ func TestTypeAndCharsetOfSendLongData(t *testing.T) {
ts := servertestkit.CreateTidbTestSuite(t)
ts.RunTestTypeAndCharsetOfSendLongData(t)
}

func TestAuthSocket(t *testing.T) {
defer server2.ClearOSUserForAuthSocket()

cfg := util2.NewTestConfig()
cfg.Socket = filepath.Join(t.TempDir(), "authsock.sock")
cfg.Port = 0
cfg.Status.StatusPort = 0
ts := servertestkit.CreateTidbTestSuiteWithCfg(t, cfg)
ts.WaitUntilServerCanConnect()

ts.RunTests(t, nil, func(dbt *testkit.DBTestKit) {
dbt.MustExec("CREATE USER 'u1'@'%' IDENTIFIED WITH auth_socket;")
dbt.MustExec("CREATE USER 'u2'@'%' IDENTIFIED WITH auth_socket AS 'sockuser'")
dbt.MustExec("CREATE USER 'sockuser'@'%' IDENTIFIED WITH auth_socket;")
})

// network login should be denied
for _, uname := range []string{"u1", "u2", "u3"} {
server2.MockOSUserForAuthSocket(uname)
db, err := sql.Open("mysql", ts.GetDSN(func(config *mysql.Config) {
config.User = uname
}))
require.NoError(t, err)
_, err = db.Conn(context.TODO())
require.EqualError(t,
err,
fmt.Sprintf("Error 1045 (28000): Access denied for user '%s'@'127.0.0.1' (using password: NO)", uname),
)
require.NoError(t, db.Close())
}

socketAuthConf := func(user string) func(*mysql.Config) {
return func(config *mysql.Config) {
config.User = user
config.Net = "unix"
config.Addr = cfg.Socket
config.DBName = ""
}
}

server2.MockOSUserForAuthSocket("sockuser")

// mysql username that is different with the OS user should be rejected.
db, err := sql.Open("mysql", ts.GetDSN(socketAuthConf("u1")))
require.NoError(t, err)
_, err = db.Conn(context.TODO())
require.EqualError(t, err, "Error 1045 (28000): Access denied for user 'u1'@'localhost' (using password: YES)")
require.NoError(t, db.Close())

// mysql username that is the same with the OS user should be accepted.
ts.RunTests(t, socketAuthConf("sockuser"), func(dbt *testkit.DBTestKit) {
rows := dbt.MustQuery("select current_user();")
ts.CheckRows(t, rows, "sockuser@%")
})

// When a user is created with `IDENTIFIED WITH auth_socket AS ...`.
// It should be accepted when username or as string is the same with OS user.
ts.RunTests(t, socketAuthConf("u2"), func(dbt *testkit.DBTestKit) {
rows := dbt.MustQuery("select current_user();")
ts.CheckRows(t, rows, "u2@%")
})

server2.MockOSUserForAuthSocket("u2")
ts.RunTests(t, socketAuthConf("u2"), func(dbt *testkit.DBTestKit) {
rows := dbt.MustQuery("select current_user();")
ts.CheckRows(t, rows, "u2@%")
})
}
1 change: 1 addition & 0 deletions pkg/server/tests/servertestkit/testkit.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ func CreateTidbTestSuiteWithCfg(t *testing.T, cfg *config.Config) *TidbTestSuite
require.NoError(t, err)
ts.Tidbdrv = srv.NewTiDBDriver(ts.Store)

srv.RunInGoTestChan = make(chan struct{})
server, err := srv.NewServer(cfg, ts.Tidbdrv)
require.NoError(t, err)

Expand Down