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) (#54051)

close #54031
  • Loading branch information
ti-chi-bot authored Jun 18, 2024
1 parent 2c0584a commit b75a314
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 12 deletions.
20 changes: 9 additions & 11 deletions 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 server/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ import (
"github.com/pingcap/tidb/util/dbterror/exeerrors"
"github.com/pingcap/tidb/util/execdetails"
"github.com/pingcap/tidb/util/hack"
"github.com/pingcap/tidb/util/intest"
"github.com/pingcap/tidb/util/logutil"
"github.com/pingcap/tidb/util/memory"
"github.com/pingcap/tidb/util/resourcegrouptag"
Expand Down Expand Up @@ -877,6 +878,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 @@ -925,7 +929,15 @@ func (cc *clientConn) checkAuthPlugin(ctx context.Context, resp *handshakeRespon
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
71 changes: 71 additions & 0 deletions server/tidb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3220,3 +3220,74 @@ func TestProxyProtocolWithIpNoFallbackable(t *testing.T) {
require.NotNil(t, err)
db.Close()
}

func TestAuthSocket(t *testing.T) {
defer mockOSUserForAuthSocketTest.Store(nil)

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 (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 = ""
}
}

mockUser := "sockuser"
mockOSUserForAuthSocketTest.Store(&mockUser)
// 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@%")
})

mockUser = "u2"
mockOSUserForAuthSocketTest.Store(&mockUser)
ts.runTests(t, socketAuthConf("u2"), func(dbt *testkit.DBTestKit) {
rows := dbt.MustQuery("select current_user();")
ts.checkRows(t, rows, "u2@%")
})
}

0 comments on commit b75a314

Please sign in to comment.