Skip to content

fix(auth): allow mysql-monitor_* to authenticate under caching_sha2_password #5991

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.

Sign up for GitHub

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

Merged
renecannao merged 2 commits into from
Aug 9, 2026
Merged

fix(auth): allow mysql-monitor_* to authenticate under caching_sha2_password #5991

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9421d44
fix(test): restore the exact pre-existing state in the #5363/#5985 re…
renecannao Aug 8, 2026
d768c6e
Merge branch 'v3.0' into fix/5363-monitor-caching-sha2
renecannao Aug 9, 2026
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
2 changes: 1 addition & 1 deletion lib/MySQL_Protocol.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1709,7 +1709,7 @@ int MySQL_Protocol::PPHR_1(unsigned char *pkt, unsigned int len, bool& ret, MyPr
if ((*myds)->switching_auth_stage == 5 && *pkt == 2) {
proxy_debug(PROXY_DEBUG_MYSQL_AUTH, 5,
"Session=%p , DS=%p , user='%s' . Client requested the caching_sha2_password RSA public key\n",
(*myds), (*myds)->sess, vars1.user);
(*myds)->sess, (*myds), vars1.user);
proxy_error(
"User '%s'@'%s' requested the caching_sha2_password RSA public key, which ProxySQL does not"
" serve. Connect using TLS instead.\n",
Expand Down
2 changes: 1 addition & 1 deletion test/repro/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ needs debug-only admin commands.

## COLD_START

```
```text
COLD_START=0 (default) use the existing ProxySQL instance
COLD_START=1 destroy and recreate it first
```
Expand Down
84 changes: 50 additions & 34 deletions test/repro/reg_test_5363_admin_monitor_caching_sha2.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,36 +66,45 @@
WORKSPACE="${WORKSPACE:-$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)}"
INFRA_ID="${INFRA_ID:-dev-$USER}"
TAP_GROUP="${TAP_GROUP:-no-infra-g1}"
COLD_START="${COLD_START:-0}"
COLD_START="-e"

@coderabbitai coderabbitai Bot Aug 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Preserve the caller-provided COLD_START value.

Line 69 overwrites COLD_START=1 with -e. The cold-start branch at Line 116 can never run. The usage message at Line 124 is therefore incorrect.

Proposed fix 
-COLD_START="-e"
+COLD_START="${COLD_START:-0}"

Also applies to: 116-126

🤖 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 `@test/repro/reg_test_5363_admin_monitor_caching_sha2.bash` at line 69,
Preserve the caller-provided COLD_START value instead of unconditionally
assigning “-e” near the script’s initialization. Update the cold-start branching
logic around COLD_START and its usage message so the branch remains reachable
when the caller requests a cold start and the displayed invocation matches the
actual accepted value.

export WORKSPACE INFRA_ID TAP_GROUP

CTR="proxysql.${INFRA_ID}"
DATADIR="${WORKSPACE}/ci_infra_logs/${INFRA_ID}/proxysql"
MON_USER='monitor'
MON_PASS='monitorpass'
BASE_CREDS='admin:admin;radmin:radmin;cluster1:secret1pass'
LOAD_MYSQL_VARS='LOAD MYSQL VARIABLES TO RUNTIME;'

PASS=0; FAIL=0; BUGS=0
ok() { PASS=$((PASS+1)); printf ' ok %d - %s\n' "$((PASS+FAIL))" "$1"; }
nok() { FAIL=$((FAIL+1)); printf ' NOT OK %d - %s\n' "$((PASS+FAIL))" "$1"; }
bug() { FAIL=$((FAIL+1)); BUGS=$((BUGS+1)); printf ' NOT OK %d - [BUG #5363] %s\n' "$((PASS+FAIL))" "$1"; }
hdr() { printf '\n== %s ==\n' "$1"; }
die() { printf '\nFATAL: %s\n' "$1" >&2; exit 2; }
ok() { local msg="$1"; PASS=$((PASS+1)); printf ' ok %d - %s\n' "$((PASS+FAIL))" "$msg"; return 0; }
nok() { local msg="$1"; FAIL=$((FAIL+1)); printf ' NOT OK %d - %s\n' "$((PASS+FAIL))" "$msg"; return 0; }
bug() { local msg="$1"; FAIL=$((FAIL+1)); BUGS=$((BUGS+1)); printf ' NOT OK %d - [BUG #5363] %s\n' "$((PASS+FAIL))" "$msg"; return 0; }
hdr() { local msg="$1"; printf '\n== %s ==\n' "$msg"; return 0; }
die() { local msg="$1"; printf '\nFATAL: %s\n' "$msg" >&2; exit 2; }

Check warning on line 83 in test/repro/reg_test_5363_admin_monitor_caching_sha2.bash

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Add an explicit return statement at the end of the function. 


See more on https://sonarcloud.io/project/issues?id=sysown_proxysql&issues=AZ_g450_jmTxNrSpeKt0&open=AZ_g450_jmTxNrSpeKt0&pullRequest=5991

IP=''
adm() { mysql -h"$IP" -P6032 -uradmin -pradmin --protocol=TCP -NBe "$1" 2>/dev/null; }
adm() {
local sql="$1"
mysql -h"$IP" -P6032 -uradmin -pradmin --protocol=TCP -NBe "$sql" 2>/dev/null
return $?
}
setvar() { # setvar <variable_name> <value> <LOAD statement>
adm "UPDATE global_variables SET variable_value='$2' WHERE variable_name='$1'; $3" >/dev/null
local name="$1" value="$2" load_stmt="$3"
adm "UPDATE global_variables SET variable_value='$value' WHERE variable_name='$name'; $load_stmt" >/dev/null
return $?
Comment on lines +91 to +94

@coderabbitai coderabbitai Bot Aug 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Serialize saved variable values safely before restoring them.

Line 93 embeds ORIG_MON_USER and ORIG_MON_PASS directly in an SQL literal. A pre-existing value containing a quote or backslash can produce invalid SQL or restore a different value. This defeats the exact-state restoration added in Lines 157-164.

Encode SQL literal values before constructing the UPDATE statement. Apply the same encoding to name if this helper remains generic.

Also applies to: 161-167

🤖 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 `@test/repro/reg_test_5363_admin_monitor_caching_sha2.bash` around lines 91 -
94, Update the setvar() helper to SQL-escape or otherwise safely serialize both
value and name before interpolating them into the UPDATE statement, preserving
exact restoration of values containing quotes or backslashes; ensure the same
encoding is used for all calls that restore ORIG_MON_USER and ORIG_MON_PASS.

}
# login_local <user> <pass> <ssl-mode> -- runs the client INSIDE the container
login_local() {
docker exec "$CTR" mysql -h127.0.0.1 -P6032 -u"$1" -p"$2" \
--protocol=TCP --ssl-mode="$3" -NBe "SELECT 1;" >/dev/null 2>&1
local user="$1" pass="$2" ssl_mode="$3"
docker exec "$CTR" mysql -h127.0.0.1 -P6032 -u"$user" -p"$pass" \
--protocol=TCP --ssl-mode="$ssl_mode" -NBe "SELECT 1;" >/dev/null 2>&1
return $?
}

# ------------------------------------------------------------- preflight ---
command -v docker >/dev/null || die "docker not found"
command -v mysql >/dev/null || die "mysql client not found"
[ -x "${WORKSPACE}/src/proxysql" ] || die "no binary at ${WORKSPACE}/src/proxysql -- build it first"
[[ -x "${WORKSPACE}/src/proxysql" ]] || die "no binary at ${WORKSPACE}/src/proxysql -- build it first"
"${WORKSPACE}/src/proxysql" --version 2>&1 | grep -q '_DEBUG' \
|| die "${WORKSPACE}/src/proxysql is not a DEBUG build"

Expand All @@ -104,24 +113,21 @@
# PPHR_5passwordFalse_0(), which never calls set_clear_text_password(), so the
# caching_sha2 cleartext cache plays no part in the result. COLD_START is offered
# only for symmetry with the #5985 companion and for a guaranteed-clean run.
if [ "$COLD_START" = "1" ]; then
if [[ "$COLD_START" = "1" ]]; then
hdr "Cold start: destroying and recreating ProxySQL for INFRA_ID=${INFRA_ID}"
WORKSPACE="$WORKSPACE" INFRA_ID="$INFRA_ID" TAP_GROUP="$TAP_GROUP" \
"${WORKSPACE}/test/infra/control/stop-proxysql-isolated.bash" >/dev/null 2>&1
"${WORKSPACE}/test/infra/control/stop-proxysql-isolated.bash" >/dev/null 2>&1
rm -f "${DATADIR}"/proxysql.db "${DATADIR}"/proxysql_debug.db \
"${DATADIR}"/proxysql_stats.db "${DATADIR}"/sqlite3server.db 2>/dev/null
WORKSPACE="$WORKSPACE" INFRA_ID="$INFRA_ID" TAP_GROUP="$TAP_GROUP" \
"${WORKSPACE}/test/infra/control/start-proxysql-isolated.bash" >/dev/null 2>&1 \
"${WORKSPACE}/test/infra/control/start-proxysql-isolated.bash" >/dev/null 2>&1 \
|| die "start-proxysql-isolated.bash failed"
else
hdr "Using the existing ProxySQL instance (set COLD_START=1 to recreate it)"
WORKSPACE="$WORKSPACE" INFRA_ID="$INFRA_ID" TAP_GROUP="$TAP_GROUP" \
"${WORKSPACE}/test/infra/control/ensure-infras.bash" >/dev/null 2>&1 \
"${WORKSPACE}/test/infra/control/ensure-infras.bash" >/dev/null 2>&1 \
|| die "ensure-infras.bash failed"
fi

IP="$(docker inspect "$CTR" --format '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}' 2>/dev/null)"
[ -n "$IP" ] || die "could not determine IP of ${CTR}"
[[ -n "$IP" ]] || die "could not determine IP of ${CTR}"
docker exec "$CTR" which mysql >/dev/null 2>&1 || die "no mysql client inside ${CTR}"

# Guard against testing a STALE binary. The container bind-mounts
Expand All @@ -134,7 +140,7 @@
BIN_MTIME="$(stat -c %Y "${WORKSPACE}/src/proxysql" 2>/dev/null || echo 0)"
CTR_STARTED="$(docker inspect -f '{{.State.StartedAt}}' "$CTR" 2>/dev/null)"
CTR_EPOCH="$(date -d "$CTR_STARTED" +%s 2>/dev/null || echo 0)"
if [ "$BIN_MTIME" -gt "$CTR_EPOCH" ] 2>/dev/null; then
if [[ "$BIN_MTIME" -gt "$CTR_EPOCH" ]] 2>/dev/null; then
die "the running ProxySQL container predates ${WORKSPACE}/src/proxysql, so it is
NOT running the binary you just built. Re-run with COLD_START=1, or refresh
the container with:
Expand All @@ -143,32 +149,42 @@
fi
echo " ${CTR} at ${IP} -- $(adm 'SELECT @@admin-version;')"

# Capture the instance's ACTUAL pre-existing values so the EXIT handler can put
# back exactly what was there. Restoring hardcoded defaults instead would
# silently rewrite the configuration of a shared instance -- see the "Restore any
# global state that was changed" rule in test/repro/README.md. Note this script
# never modifies 'admin-admin_credentials', so it must not reset it either.
ORIG_AUTH_PLUGIN="$(adm 'SELECT @@mysql-default_authentication_plugin;')"
ORIG_MON_USER="$(adm 'SELECT @@mysql-monitor_username;')"
ORIG_MON_PASS="$(adm 'SELECT @@mysql-monitor_password;')"

restore() {
setvar 'admin-admin_credentials' "$BASE_CREDS" 'LOAD ADMIN VARIABLES TO RUNTIME;'
setvar 'mysql-default_authentication_plugin' 'mysql_native_password' 'LOAD MYSQL VARIABLES TO RUNTIME;'
setvar 'mysql-monitor_password' 'monitor' 'LOAD MYSQL VARIABLES TO RUNTIME;'
setvar 'mysql-default_authentication_plugin' "$ORIG_AUTH_PLUGIN" "$LOAD_MYSQL_VARS"
setvar 'mysql-monitor_username' "$ORIG_MON_USER" "$LOAD_MYSQL_VARS"
setvar 'mysql-monitor_password' "$ORIG_MON_PASS" "$LOAD_MYSQL_VARS"
return 0
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
trap restore EXIT

# ----------------------------------------------------------- preconditions ---
hdr "Preconditions"
[ "$(adm 'SELECT count(*) FROM runtime_mysql_users;')" = "0" ] \
[[ "$(adm 'SELECT count(*) FROM runtime_mysql_users;')" = "0" ]] \
&& ok "no mysql_users rows (nothing can shadow the monitor credential)" \
|| nok "mysql_users is not empty -- result would be confounded"
adm "SELECT @@admin-admin_credentials;" | grep -q "${MON_USER}:" \
&& nok "an admin credential named '${MON_USER}' exists -- wrong code path" \
|| ok "no admin credential named '${MON_USER}' (PPHR_5passwordFalse_0 is the path)"

setvar 'mysql-monitor_username' "$MON_USER" 'LOAD MYSQL VARIABLES TO RUNTIME;'
setvar 'mysql-monitor_password' "$MON_PASS" 'LOAD MYSQL VARIABLES TO RUNTIME;'
[ "$(adm 'SELECT @@mysql-monitor_username;')" = "$MON_USER" ] &&
[ "$(adm 'SELECT @@mysql-monitor_password;')" = "$MON_PASS" ] \
setvar 'mysql-monitor_username' "$MON_USER" "$LOAD_MYSQL_VARS"
setvar 'mysql-monitor_password' "$MON_PASS" "$LOAD_MYSQL_VARS"
[[ "$(adm 'SELECT @@mysql-monitor_username;')" = "$MON_USER" ]] &&
[[ "$(adm 'SELECT @@mysql-monitor_password;')" = "$MON_PASS" ]] \
&& ok "mysql-monitor_username/password set to ${MON_USER}/${MON_PASS} (cleartext, as documented)" \
|| nok "failed to set mysql-monitor_* variables"

# ------------------------------------------------- baseline: native works ---
hdr "Baseline: mysql_native_password"
setvar 'mysql-default_authentication_plugin' 'mysql_native_password' 'LOAD MYSQL VARIABLES TO RUNTIME;'
setvar 'mysql-default_authentication_plugin' 'mysql_native_password' "$LOAD_MYSQL_VARS"
login_local "$MON_USER" "$MON_PASS" DISABLED \
&& ok "${MON_USER} authenticates on :6032 over plaintext" \
|| nok "${MON_USER} FAILED under native/plaintext -- baseline broken, investigate before trusting the rest"
Expand All @@ -178,8 +194,8 @@

# ------------------------------------------------------------- the bug ---
hdr "caching_sha2_password"
setvar 'mysql-default_authentication_plugin' 'caching_sha2_password' 'LOAD MYSQL VARIABLES TO RUNTIME;'
[ "$(adm 'SELECT @@mysql-default_authentication_plugin;')" = "caching_sha2_password" ] \
setvar 'mysql-default_authentication_plugin' 'caching_sha2_password' "$LOAD_MYSQL_VARS"
[[ "$(adm 'SELECT @@mysql-default_authentication_plugin;')" = "caching_sha2_password" ]] \
&& ok "mysql-default_authentication_plugin = caching_sha2_password" \
|| nok "failed to switch the default authentication plugin"

Expand All @@ -204,12 +220,12 @@
# --------------------------------------------------------------- summary ---
hdr "Summary"
printf ' passed: %d\n failed: %d (of which %d are the reported bug)\n' "$PASS" "$FAIL" "$BUGS"
if [ "$FAIL" -eq 0 ]; then
if [[ "$FAIL" -eq 0 ]]; then
echo
echo " All assertions pass: #5363 appears FIXED on this build."
exit 0
fi
if [ "$FAIL" -eq "$BUGS" ]; then
if [[ "$FAIL" -eq "$BUGS" ]]; then
cat <<'EOS'

REPRODUCED. Every failure is a [BUG #5363] assertion:
Expand Down
Loading
Loading