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) #54049

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 privilege/privileges/privileges.go
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand All @@ -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.
Expand Down
13 changes: 12 additions & 1 deletion server/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
89 changes: 84 additions & 5 deletions server/tidb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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@%")
})
}
Loading