From 4ffd88c4ca5a9a150129c2ff184c0f34b275f8a2 Mon Sep 17 00:00:00 2001 From: Ti Chi Robot Date: Thu, 20 Jun 2024 20:50:48 +0800 Subject: [PATCH] privilege: fix `auth_socket` bug, should only allow os user name to login (#54032) (#54049) close pingcap/tidb#54031 --- privilege/privileges/privileges.go | 20 +++---- server/conn.go | 13 ++++- server/tidb_test.go | 89 ++++++++++++++++++++++++++++-- 3 files changed, 105 insertions(+), 17 deletions(-) diff --git a/privilege/privileges/privileges.go b/privilege/privileges/privileges.go index 544a0e21ac259..94ca09159485c 100644 --- a/privilege/privileges/privileges.go +++ b/privilege/privileges/privileges.go @@ -542,6 +542,13 @@ func (p *UserPrivileges) ConnectionVerification(user *auth.UserIdentity, authUse logutil.BgLogger().Error("check claims failed", 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`. @@ -567,22 +574,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. diff --git a/server/conn.go b/server/conn.go index b65538e999f88..4599c1609d3bb 100644 --- a/server/conn.go +++ b/server/conn.go @@ -892,6 +892,9 @@ func (cc *clientConn) openSessionAndDoAuth(authData []byte, authPlugin string) e 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 *handshakeResponse41) ([]byte, error) { // Open a context unless this was done before. @@ -940,7 +943,15 @@ func (cc *clientConn) checkAuthPlugin(ctx context.Context, resp *handshakeRespon if err != nil { return nil, err } - return []byte(user.Username), nil + uname := user.Username + + failpoint.Inject("MockOSUserForAuthSocket", func() { + if p := mockOSUserForAuthSocketTest.Load(); p != nil { + uname = *p + } + }) + + return []byte(uname), nil } if len(userplugin) == 0 { // No user plugin set, assuming MySQL Native Password diff --git a/server/tidb_test.go b/server/tidb_test.go index 259942d929a52..0394a8bdd25f0 100644 --- a/server/tidb_test.go +++ b/server/tidb_test.go @@ -83,6 +83,15 @@ type tidbTestSuite struct { } func createTidbTestSuite(t *testing.T) *tidbTestSuite { + cfg := newTestConfig() + cfg.Port = 0 + cfg.Status.ReportStatus = true + cfg.Status.StatusPort = 0 + cfg.Performance.TCPKeepAlive = true + return createTidbTestSuiteWithCfg(t, cfg) +} + +func createTidbTestSuiteWithCfg(t *testing.T, cfg *config.Config) *tidbTestSuite { ts := &tidbTestSuite{testServerClient: newTestServerClient()} // setup tidbTestSuite @@ -93,11 +102,6 @@ func createTidbTestSuite(t *testing.T) *tidbTestSuite { ts.domain, err = session.BootstrapSession(ts.store) require.NoError(t, err) ts.tidbdrv = NewTiDBDriver(ts.store) - cfg := newTestConfig() - cfg.Port = ts.port - cfg.Status.ReportStatus = true - cfg.Status.StatusPort = ts.statusPort - cfg.Performance.TCPKeepAlive = true RunInGoTestChan = make(chan struct{}) server, err := NewServer(cfg, ts.tidbdrv) require.NoError(t, err) @@ -3229,3 +3233,78 @@ func TestLoadData(t *testing.T) { ts.runTestLoadDataReplace(t) ts.runTestLoadDataReplaceNonclusteredPK(t) } + +func TestAuthSocket(t *testing.T) { + require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/server/MockOSUserForAuthSocket", "return(true)")) + defer func() { + mockOSUserForAuthSocketTest.Store(nil) + require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/server/MockOSUserForAuthSocket")) + }() + + cfg := newTestConfig() + cfg.Socket = filepath.Join(t.TempDir(), "authsock.sock") + cfg.Port = 0 + cfg.Status.StatusPort = 0 + ts := 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"} { + mockOSUserForAuthSocketTest.Store(&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: 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 = "" + } + } + + mockOSUser := "sockuser" + mockOSUserForAuthSocketTest.Store(&mockOSUser) + + // 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: 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@%") + }) + + mockOSUser = "u2" + mockOSUserForAuthSocketTest.Store(&mockOSUser) + ts.runTests(t, socketAuthConf("u2"), func(dbt *testkit.DBTestKit) { + rows := dbt.MustQuery("select current_user();") + ts.checkRows(t, rows, "u2@%") + }) +}