Skip to content

Commit

Permalink
privilege: fix auth_socket bug, should only allow os user name to l…
Browse files Browse the repository at this point in the history
  • Loading branch information
ti-chi-bot authored Jun 18, 2024
1 parent 1563cc5 commit d179476
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 12 deletions.
20 changes: 9 additions & 11 deletions pkg/privilege/privileges/privileges.go
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,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 @@ -608,22 +615,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
14 changes: 13 additions & 1 deletion pkg/server/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ import (
"github.com/pingcap/tidb/pkg/util/dbterror/exeerrors"
"github.com/pingcap/tidb/pkg/util/execdetails"
"github.com/pingcap/tidb/pkg/util/hack"
"github.com/pingcap/tidb/pkg/util/intest"
"github.com/pingcap/tidb/pkg/util/logutil"
"github.com/pingcap/tidb/pkg/util/memory"
"github.com/pingcap/tidb/pkg/util/resourcegrouptag"
Expand Down Expand Up @@ -771,6 +772,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 *handshake.Response41) ([]byte, error) {
// Open a context unless this was done before.
Expand Down Expand Up @@ -819,7 +823,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)
}
69 changes: 69 additions & 0 deletions pkg/server/tests/tidb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3144,3 +3144,72 @@ func TestConnectionCount(t *testing.T) {
ts := servertestkit.CreateTidbTestSuite(t)
ts.RunTestConnectionCount(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@%")
})
}

0 comments on commit d179476

Please sign in to comment.