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
…ogin (#54032) (#54049)

close #54031
  • Loading branch information
ti-chi-bot authored Jun 20, 2024
1 parent ff2feb6 commit 4ffd88c
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 17 deletions.
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@%")
})
}

0 comments on commit 4ffd88c

Please sign in to comment.