*: support MySQL dual passwords (release-8.5)#69735
Conversation
… DISCARD OLD PASSWORD) (pingcap#68028) ref pingcap#60587 (cherry picked from commit b254c43) Signed-off-by: bb7133 <bb7133@gmail.com>
…sts) (pingcap#68393) ref pingcap#60587 (cherry picked from commit 60996fd) Signed-off-by: bb7133 <bb7133@gmail.com>
ti-chi-bot
Bot
added
release-note
📝 WalkthroughWalkthroughAdds MySQL 8.0 dual-password support across SQL parsing, executor handling, privilege decoding, authentication fallback, error reporting, and test coverage. The change tracks retained secondary passwords in ChangesDual-Password Feature
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/privilege/privileges/cache.go (1)
1016-1029: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winHoist
defaultAuthPluginresolution out of the per-row closure.
decodeUserTableRowreturns a closure that's invoked once per row inmysql.user. ThedefaultAuthPluginlookup (p.globalVars.GetGlobalSysVar(...)) is computed inside that closure, so it re-runs for every user row even though the value is invariant for the whole table load. Move it out to beforereturn func(...)so it's computed once perdecodeUserTableRowcall instead of once per row.♻️ Proposed fix
func (p *MySQLPrivilege) decodeUserTableRow(userList map[string]struct{}) func(chunk.Row, []*resolve.ResultField) error { + defaultAuthPlugin := "" + if p.globalVars != nil { + if val, err := p.globalVars.GetGlobalSysVar(variable.DefaultAuthPlugin); err == nil { + defaultAuthPlugin = val + } + } + if defaultAuthPlugin == "" { + defaultAuthPlugin = mysql.AuthNativePassword + } return func(row chunk.Row, fs []*resolve.ResultField) error { var value UserRecord - defaultAuthPlugin := "" - if p.globalVars != nil { - val, err := p.globalVars.GetGlobalSysVar(variable.DefaultAuthPlugin) - if err == nil { - defaultAuthPlugin = val - } - } - if defaultAuthPlugin == "" { - defaultAuthPlugin = mysql.AuthNativePassword - } for i, f := range fs {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/privilege/privileges/cache.go` around lines 1016 - 1029, `decodeUserTableRow` is recalculating `defaultAuthPlugin` inside the returned row-processing closure, which repeats the same lookup for every `mysql.user` row. Move the `p.globalVars.GetGlobalSysVar(variable.DefaultAuthPlugin)` resolution and fallback to `mysql.AuthNativePassword` outside the `return func(row chunk.Row, fs []*resolve.ResultField) error` closure so it runs once per `MySQLPrivilege.decodeUserTableRow` call, then have the closure reuse that captured value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/executor/simple.go`:
- Around line 2748-2766: The RETAIN eligibility check in
isDualPasswordCapablePlugin is using mysql.IsAuthPluginClearText, which matches
the clear-text plugin instead of the dual-password-capable authentication
plugins. Update this helper to use a dedicated allowlist for the supported
password-based plugins (such as mysql_native_password, caching_sha2_password,
and tidb_sm3_password) so RETAIN is accepted for the correct users while still
excluding LDAP/socket/token plugins.
---
Nitpick comments:
In `@pkg/privilege/privileges/cache.go`:
- Around line 1016-1029: `decodeUserTableRow` is recalculating
`defaultAuthPlugin` inside the returned row-processing closure, which repeats
the same lookup for every `mysql.user` row. Move the
`p.globalVars.GetGlobalSysVar(variable.DefaultAuthPlugin)` resolution and
fallback to `mysql.AuthNativePassword` outside the `return func(row chunk.Row,
fs []*resolve.ResultField) error` closure so it runs once per
`MySQLPrivilege.decodeUserTableRow` call, then have the closure reuse that
captured value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 42856281-81c3-4549-bdee-f374e312fe40
📒 Files selected for processing (24)
errors.tomlpkg/domain/domain.gopkg/errno/errcode.gopkg/errno/errname.gopkg/executor/BUILD.bazelpkg/executor/simple.gopkg/executor/simple_internal_test.gopkg/executor/test/passwordtest/BUILD.bazelpkg/executor/test/passwordtest/dual_password_test.gopkg/executor/utils.gopkg/parser/ast/misc.gopkg/parser/ast/misc_test.gopkg/parser/keywords.gopkg/parser/keywords_test.gopkg/parser/misc.gopkg/parser/parser.gopkg/parser/parser.ypkg/parser/parser_test.gopkg/privilege/privileges/cache.gopkg/privilege/privileges/privileges.gopkg/util/dbterror/exeerrors/errors.gotests/integrationtest/r/executor/dual_password.resulttests/integrationtest/r/executor/executor.resulttests/integrationtest/t/executor/dual_password.test
| // isDualPasswordCapablePlugin reports whether a user whose plugin is `plugin` is | ||
| // eligible to hold a secondary ("additional") password. Dual passwords are only | ||
| // meaningful for password-based plugins. LDAP / socket / token plugins are excluded, | ||
| // matching MySQL 8.0 behavior. | ||
| // | ||
| // It delegates to mysql.IsAuthPluginClearText, the canonical predicate for | ||
| // "the plugin derives a server-side hash from a clear-text password" | ||
| // (mysql_native_password / caching_sha2_password / tidb_sm3_password): those | ||
| // are exactly the plugins that can hold a second stored hash. If the two sets | ||
| // ever need to diverge, split this back into its own list and document the | ||
| // difference. | ||
| // | ||
| // Callers MUST pass the resolved plugin (see effectiveAuthPlugin): an empty | ||
| // `plugin` column on a legacy mysql.user row could resolve to anything via | ||
| // `default_authentication_plugin`, and treating "" as natively capable would | ||
| // wrongly allow RETAIN on an LDAP-default deployment. | ||
| func isDualPasswordCapablePlugin(plugin string) bool { | ||
| return mysql.IsAuthPluginClearText(plugin) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect IsAuthPluginClearText definition and the plugins it accepts on this branch.
fd -t f 'const.go|type.go|util.go' pkg/parser/mysql | xargs rg -nP -C4 'func IsAuthPluginClearText'Repository: pingcap/tidb
Length of output: 1976
🌐 Web query:
pingcap tidb release-8.5 mysql IsAuthPluginClearText pkg/parser/mysql auth plugin clear text
💡 Result:
In TiDB, the mysql_clear_password authentication plugin is supported primarily to facilitate the tidb_auth_token authentication mechanism, which allows clients to send authentication tokens in plain text [1][2]. Because this mechanism transmits credentials in plain text, TiDB documentation mandates that TLS must be enabled between the client and the server when using tidb_auth_token to ensure security [2][3]. For MySQL command-line clients and other applications using libmysqlclient, the mysql_clear_password plugin is not enabled by default for security reasons [4]. To use it, you must explicitly enable it: 1. Command-line clients: Use the --enable-cleartext-plugin flag [2][3][4]. 2. Environment configuration: Set the LIBMYSQL_ENABLE_CLEARTEXT_PLUGIN environment variable to 1, Y, or y [4]. 3. Client-specific tools: Tools like mycli typically enable this plugin by default [2][3][5]. Within the TiDB codebase, authentication plugin information is handled within the pkg/parser package, which maintains the Abstract Syntax Tree (AST) definitions and necessary structures for parsing SQL statements (such as CREATE USER) that involve authentication options [6][7]. The parser supports the storage and management of authentication plugin names, enabling compatibility with various MySQL authentication methods [6]. TiDB 8.5 (an LTS release) continues to support these authentication standards to maintain MySQL compatibility while providing its own authentication extensions [8][9]. Developers interacting with pkg/parser or internal authentication logic will find these structures in the ast (Abstract Syntax Tree) and auth related sub-packages within the TiDB repository [6][7].
Citations:
- 1: https://docs.pingcap.com/tidb/stable/security-compatibility-with-mysql/
- 2: https://github.com/pingcap/docs/blob/master/security-compatibility-with-mysql.md
- 3: https://docs.pingcap.com/tidb/dev/security-compatibility-with-mysql/
- 4: https://dev.mysql.com/doc/refman/8.4/en/cleartext-pluggable-authentication.html
- 5: *: authentication using
tidb_auth_tokenplugin #38773 - 6: parser,ast: Support for authentication plugins parser#1236
- 7: *: fix a bug for default_authentication_plugin #57391
- 8: https://docs.pingcap.com/tidb/stable/release-8.5.0
- 9: https://docs.pingcap.com/tidb/v8.5/release-notes
🌐 Web query:
site:github.com/pingcap/tidb "func IsAuthPluginClearText" "pkg/parser/mysql"
💡 Result:
IsAuthPluginClearText(authPlugin string) bool exists in TiDB’s github.com/pingcap/tidb/parser/mysql package (the repo is pingcap/tidb). It’s documented as: “used to indicated that the plugin need clear-text password.” (pkg.go.dev)
If you want, I can also help locate the exact source file and line in the GitHub repo.
Citations:
🌐 Web query:
pingcap tidb parser mysql IsAuthPluginClearText source body switch authPlugin clear text mysql_native_password caching_sha2_password tidb_sm3_password
💡 Result:
TiDB's handling of authentication plugins and the "clear text" authentication flow is designed for MySQL protocol compatibility [1]. Authentication Plugin Support TiDB supports several authentication plugins, including mysql_native_password, caching_sha2_password, and auth_socket [2]. These constants are defined within the TiDB parser package (specifically in parser/mysql/const.go) to facilitate protocol handling [2]. Clear Text Authentication and Auth Switching The TiDB server implementation handles authentication switches—a mechanism in the MySQL protocol—to negotiate authentication methods between the client and server [3][4]. For instance, when a client attempts to connect using a method the server may not fully prefer or when a "change user" command is issued, the server can request a switch to a supported plugin, such as downgrading from caching_sha2_password to mysql_native_password for compatibility [3][4]. Regarding "clear text" (often associated with full authentication in caching_sha2_password): 1. In the MySQL protocol, "full authentication" for caching_sha2_password involves sending the clear-text password over a secure (TLS/SSL) connection [5]. 2. TiDB supports this full authentication flow [5]. 3. Because security is paramount, TiDB requires an encrypted connection for this process [6]. 4. Unlike MySQL, which may utilize a "fast authentication" cache, TiDB has historically prioritized and implemented the "complete authentication" (full authentication) path for caching_sha2_password [6]. Codebase Context Relevant logic for managing these plugins is dispersed across the parser and server packages: - Parser/AST: Updates have been made to the SQL parser to support authentication plugins in statements like CREATE USER, including compatibility for caching_sha2_password hashing formats [7][8]. - Server: The server connection handling logic (e.g., pkg/server/conn.go) manages the actual handshake, including enforcing TLS for certain plugins and handling auth switch requests [3][6][4]. In summary, TiDB manages authentication via its parser's constant definitions [2] and the server's handshake/switch logic, ensuring that protocols like caching_sha2_password are handled securely via encrypted full authentication [5][6].
Citations:
- 1: https://pkg.go.dev/github.com/pingcap/tidb/parser/mysql
- 2: https://github.com/pingcap/tidb/blob/9f68c8e92a994e4790bfd9e567e5ad86c8daa861/parser/mysql/const.go
- 3: server: add auth switch to mysql_native_password (#19603) #19959
- 4: *: support switch auth when change user #38830
- 5: Support for caching_sha2_password #9411
- 6:
default_authentication_pluginis not compatable with MySQL's #54138 - 7: parser,ast: Support for authentication plugins parser#1236
- 8: Caching sha2 parser#1232
🌐 Web query:
site:github.com/pingcap/tidb/blob release-8.5 "IsAuthPluginClearText" "mysql_native_password" "caching_sha2_password" "tidb_sm3_password"
💡 Result:
In TiDB’s pkg/parser/auth for the current release-8.5 line, I found caching_sha2_password, mysql_native_password, and tidb_sm3_password support, with source files for all three auth paths listed. The docs also say CheckHashingPassword handles caching_sha2_password and tidb_sm3_password. (pkg.go.dev)
I did not find IsAuthPluginClearText in that package/docs search. (pkg.go.dev)
Citations:
- 1: https://pkg.go.dev/github.com/pingcap/tidb/pkg/parser/auth
- 2: https://pkg.go.dev/github.com/pingcap/tidb/pkg/parser/auth
🌐 Web query:
site:github.com/pingcap/tidb "func IsAuthPluginClearText" "parser/mysql/const.go" release-8.5
💡 Result:
I cannot provide specific information about a function named IsAuthPluginClearText in the file parser/mysql/const.go for TiDB release-8.5. The provided search results confirm that TiDB release-8.5 exists as a branch [1][2], but there is no documentation or indexed source code confirming the existence or implementation details of a function by that exact name in the specified file path [3][1][4][2][5].
Citations:
- 1: master/release-8.5: upgrade golang to 1.25.10 #68884
- 2: br: deliver "pitr table filter" to release-8.5 #64908
- 3: https://github.com/pingcap/tidb/releases
- 4: Incompatible result from TiDB with MySQL #58370
- 5: Update Go version to 1.25.8 on release-8.5 tidb-dashboard#1878
🌐 Web query:
site:github.com/pingcap/tidb release-8.5 "IsAuthPluginClearText(" "mysql_native_password" "caching_sha2_password" "tidb_sm3_password" "auth_socket" "mysql_clear_password"
💡 Result:
In TiDB 8.5, IsAuthPluginClearText should be true only for mysql_clear_password. TiDB’s docs say tidb_auth_token requires clients to support mysql_clear_password to send the token in plain text; the listed password-based plugins (mysql_native_password, caching_sha2_password, tidb_sm3_password) and auth_socket are not described as clear-text plugins. (docs.pingcap.com)
So for your list:
mysql_native_password→ falsecaching_sha2_password→ falsetidb_sm3_password→ falseauth_socket→ falsemysql_clear_password→ true (docs.pingcap.com)
Citations:
- 1: https://docs.pingcap.com/tidb/v8.1/security-compatibility-with-mysql/
- 2: https://docs.pingcap.com/tidb/v8.1/security-compatibility-with-mysql/
mysql.IsAuthPluginClearText is the wrong gate for RETAIN
On release-8.5 it maps to mysql_clear_password, so this would reject RETAIN for mysql_native_password, caching_sha2_password, and tidb_sm3_password users. Keep a dedicated allowlist for the dual-password-capable plugins here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/executor/simple.go` around lines 2748 - 2766, The RETAIN eligibility
check in isDualPasswordCapablePlugin is using mysql.IsAuthPluginClearText, which
matches the clear-text plugin instead of the dual-password-capable
authentication plugins. Update this helper to use a dedicated allowlist for the
supported password-based plugins (such as mysql_native_password,
caching_sha2_password, and tidb_sm3_password) so RETAIN is accepted for the
correct users while still excluding LDAP/socket/token plugins.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## release-8.5-20260710-v8.5.7 #69735 +/- ##
================================================================
Coverage ? 55.6606%
================================================================
Files ? 1849
Lines ? 666728
Branches ? 0
================================================================
Hits ? 371105
Misses ? 268011
Partials ? 27612
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
wow, I am also taking a look for the test failure |
|
/test check-dev2 |
|
|
Confirmed this backport against the current head of the master PR (#68393 @ Backport content verification
CI failures1.
|
|
🙏 |
Signed-off-by: bb7133 <bb7133@gmail.com>
|
/test check-dev2 |
Signed-off-by: bb7133 <bb7133@gmail.com>
|
thanks so much for the work 🙇 |
ti-chi-bot
Bot
added
lgtm
and removed
needs-1-more-lgtm
[LGTM Timeline notifier]Timeline:
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: D3Hunter, tiancaiamao, yudongusa The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
7e355f5
into
pingcap:release-8.5-20260710-v8.5.7
pingcap/tidb#69735 backported the feature to release-8.5 and it ships in the v8.5.7 train, so "Starting from v9.0.0" becomes "Starting from v8.5.7" in the three version notes (password-management.md intro + section, sql-statement-set-password.md). Co-Authored-By: Claude <noreply@anthropic.com>
5 participants
What problem does this PR solve?
Issue Number: ref #60587
Problem Summary:
This PR backports MySQL 8.0 dual-password support to
release-8.5.It includes two upstream pieces because
release-8.5does not yet have the parser prerequisite:RETAIN CURRENT PASSWORD/DISCARD OLD PASSWORDWhat changed and how does it work?
Backport of #68028 and #68393:
ALTER USER ... RETAIN CURRENT PASSWORD,ALTER USER ... DISCARD OLD PASSWORD, andSET PASSWORD ... RETAIN CURRENT PASSWORD.mysql.user.User_attributes.$.additional_password, matching MySQL.APPLICATION_PASSWORD_ADMINdynamic privilege.Backport-specific notes:
release-8.5grammar instead of copying master output.RETURNINGparser changes and traffic dynamic privileges.default_authentication_pluginhandling was adapted to therelease-8.5privilege/domain code path.Check List
Tests
cd pkg/parser && go test . ./ast -count=1go test --tags=intest ./pkg/executor -run 'Test(AlterUserHasPrivilegedOptions|AuthenticatedUserNameAndHost|BuildAdditionalPasswordEntry|EffectiveAuthPlugin|DualPassword)' -count=1go test --tags=intest ./pkg/executor/test/passwordtest -run 'TestDualPassword|TestSetPassword' -count=1go test --tags=intest ./pkg/executor/test/passwordtest -count=1go test --tags=intest ./pkg/privilege/privileges -run 'Test' -count=1tests/integrationtest/t/executor/dual_password.testmake parser_yaccgit diff --check origin/release-8.5...HEAD -- . ':(exclude)tests/integrationtest/r/**/*.result'Side effects
Documentation
Release note
Summary by CodeRabbit
ALTER USERandSET PASSWORDwithRETAIN CURRENT PASSWORDandDISCARD OLD PASSWORD.APPLICATION_PASSWORD_ADMINsupport for the required authorization paths.CREATE USER/ALTER USERbehavior for unsupported or invalid dual-password combinations.