Skip to content

fix(mysql): require TLS for SPIFFE users (#5929) #6059

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 1 commit into from
Aug 14, 2026
Merged

fix(mysql): require TLS for SPIFFE users (#5929) #6059

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 14 additions & 1 deletion lib/MySQL_Authentication.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@
}
#endif
uint64_t hash1, hash2;
bool effective_use_ssl = use_ssl;
SpookyHash myhash;
myhash.Init(1,2);
myhash.Update(username,strlen(username));
Expand Down Expand Up @@ -306,7 +307,19 @@
// FIXME: if the password is a clear text password, automatically generate sha1_pass and clear_text_password
}

ad->use_ssl=use_ssl;
if (ad->attributes && strlen(ad->attributes)) {

Check warning on line 310 in lib/MySQL_Authentication.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Make sure use of "strlen" is safe here. 


See more on https://sonarcloud.io/project/issues?id=sysown_proxysql&issues=AZ_8W9ZWXfszIsLsJ_TE&open=AZ_8W9ZWXfszIsLsJ_TE&pullRequest=6059
try {
nlohmann::json valid=nlohmann::json::parse(ad->attributes);

@cubic-dev-ai cubic-dev-ai Bot Aug 13, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: This re-parses ad->attributes that was just parsed and validated a few lines above in the same add() call (in both the new-account and attribute-change branches). Every add() for a user with non-empty attributes now parses the same JSON twice. The spiffe check could reuse the already-validated JSON instead of parsing again; as-is it's redundant work done on every user load (e.g. LOAD MYSQL USERS TO RUNTIME iterating many users).

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At lib/MySQL_Authentication.cpp, line 312:

<comment>This re-parses `ad->attributes` that was just parsed and validated a few lines above in the same `add()` call (in both the new-account and attribute-change branches). Every `add()` for a user with non-empty attributes now parses the same JSON twice. The spiffe check could reuse the already-validated JSON instead of parsing again; as-is it's redundant work done on every user load (e.g. `LOAD MYSQL USERS TO RUNTIME` iterating many users).</comment>

<file context>
@@ -306,7 +307,19 @@ bool MySQL_Authentication::add(char * username, char * password, enum cred_usern
-	ad->use_ssl=use_ssl;
+	if (ad->attributes && strlen(ad->attributes)) {
+		try {
+			nlohmann::json valid=nlohmann::json::parse(ad->attributes);
+			if (valid.find("spiffe_id") != valid.end()) {
+				effective_use_ssl = true;
</file context>
Fix with cubic

if (valid.find("spiffe_id") != valid.end()) {

@cubic-dev-ai cubic-dev-ai Bot Aug 13, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: The TLS-forcing path runs for every credential type, not only frontend users. In ProxySQL_Admin.cpp the LOAD MYSQL USERS handler calls add() once per usertype in usertypes, and a single mysql_users row flagged with both frontend_=1 and backend_=1 passes the same attributes (including spiffe_id) for the USERNAME_BACKEND credential too. As a result the backend account also gets use_ssl=true, forcing TLS on the proxy→MySQL backend connection for that user. This contradicts the PR scope ("require TLS for MySQL frontend users") and can break backend connectivity for a SPIFFE user that is also a backend user. Guard the spiffe check with usertype == USERNAME_FRONTEND.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At lib/MySQL_Authentication.cpp, line 313:

<comment>The TLS-forcing path runs for every credential type, not only frontend users. In ProxySQL_Admin.cpp the LOAD MYSQL USERS handler calls `add()` once per usertype in `usertypes`, and a single `mysql_users` row flagged with both `frontend_=1` and `backend_=1` passes the same `attributes` (including `spiffe_id`) for the USERNAME_BACKEND credential too. As a result the backend account also gets `use_ssl=true`, forcing TLS on the proxy→MySQL backend connection for that user. This contradicts the PR scope ("require TLS for MySQL frontend users") and can break backend connectivity for a SPIFFE user that is also a backend user. Guard the spiffe check with `usertype == USERNAME_FRONTEND`.</comment>

<file context>
@@ -306,7 +307,19 @@ bool MySQL_Authentication::add(char * username, char * password, enum cred_usern
+	if (ad->attributes && strlen(ad->attributes)) {
+		try {
+			nlohmann::json valid=nlohmann::json::parse(ad->attributes);
+			if (valid.find("spiffe_id") != valid.end()) {
+				effective_use_ssl = true;
+			}
</file context>
Fix with cubic

effective_use_ssl = true;
}
}
catch(nlohmann::json::exception&) {
// Invalid attributes do not require TLS.
}
}

ad->use_ssl=effective_use_ssl;
ad->default_hostgroup=default_hostgroup;
ad->schema_locked=schema_locked;
ad->transaction_persistent=transaction_persistent;
Expand Down
99 changes: 98 additions & 1 deletion test/tap/tests/unit/auth_unit-t.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,15 @@
);
}

static bool mysql_add_frontend_with_attributes(MySQL_Authentication *auth,
const char *user, bool use_ssl, const char *attributes)
{
return auth->add(
(char *)user, (char *)"pass", USERNAME_FRONTEND,

Check failure on line 59 in test/tap/tests/unit/auth_unit-t.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

C-style cast removing const qualification from the type of a pointer may lead to undefined behavior. 


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

Check failure on line 59 in test/tap/tests/unit/auth_unit-t.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

C-style cast removing const qualification from the type of a pointer may lead to undefined behavior. 


See more on https://sonarcloud.io/project/issues?id=sysown_proxysql&issues=AZ_8W9QQXfszIsLsJ_S5&open=AZ_8W9QQXfszIsLsJ_S5&pullRequest=6059
use_ssl, 0, (char *)"", false, false, false,

Check failure on line 60 in test/tap/tests/unit/auth_unit-t.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

C-style cast removing const qualification from the type of a pointer may lead to undefined behavior. 


See more on https://sonarcloud.io/project/issues?id=sysown_proxysql&issues=AZ_8W9QQXfszIsLsJ_S6&open=AZ_8W9QQXfszIsLsJ_S6&pullRequest=6059
100, (char *)attributes, (char *)"");

Check failure on line 61 in test/tap/tests/unit/auth_unit-t.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

C-style cast removing const qualification from the type of a pointer may lead to undefined behavior. 


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

Check failure on line 61 in test/tap/tests/unit/auth_unit-t.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

C-style cast removing const qualification from the type of a pointer may lead to undefined behavior. 


See more on https://sonarcloud.io/project/issues?id=sysown_proxysql&issues=AZ_8W9QQXfszIsLsJ_S8&open=AZ_8W9QQXfszIsLsJ_S8&pullRequest=6059
}

// ============================================================================
// Helper: add a MySQL backend user
// ============================================================================
Expand Down Expand Up @@ -449,6 +458,90 @@
free_account_details(be);
}

static void test_mysql_spiffe_requires_ssl() {
GloMyAuth->reset();

mysql_add_frontend_with_attributes(
GloMyAuth, "spiffe-user", false,
R"({"spiffe_id":"spiffe://example.org/ns/default/sa/client"})");
account_details_t account = GloMyAuth->lookup(
(char *)"spiffe-user", USERNAME_FRONTEND, { false, false, false });

Check failure on line 468 in test/tap/tests/unit/auth_unit-t.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

C-style cast removing const qualification from the type of a pointer may lead to undefined behavior. 


See more on https://sonarcloud.io/project/issues?id=sysown_proxysql&issues=AZ_8W9QQXfszIsLsJ_S9&open=AZ_8W9QQXfszIsLsJ_S9&pullRequest=6059
ok(account.use_ssl, "MySQL: SPIFFE user requires TLS");
free_account_details(account);

mysql_add_frontend_with_attributes(
GloMyAuth, "spiffe-user", false,
R"({"spiffe_id":"spiffe://example.org/ns/default/sa/client"})");
account = GloMyAuth->lookup(
(char *)"spiffe-user", USERNAME_FRONTEND, { false, false, false });

Check failure on line 476 in test/tap/tests/unit/auth_unit-t.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

C-style cast removing const qualification from the type of a pointer may lead to undefined behavior. 


See more on https://sonarcloud.io/project/issues?id=sysown_proxysql&issues=AZ_8W9QQXfszIsLsJ_S-&open=AZ_8W9QQXfszIsLsJ_S-&pullRequest=6059
ok(account.use_ssl, "MySQL: SPIFFE user remains TLS-required on update");
free_account_details(account);

mysql_add_frontend_with_attributes(
GloMyAuth, "plain-user", false, R"({"role":"client"})");
account = GloMyAuth->lookup(
(char *)"plain-user", USERNAME_FRONTEND, { false, false, false });

Check failure on line 483 in test/tap/tests/unit/auth_unit-t.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

C-style cast removing const qualification from the type of a pointer may lead to undefined behavior. 


See more on https://sonarcloud.io/project/issues?id=sysown_proxysql&issues=AZ_8W9QQXfszIsLsJ_S_&open=AZ_8W9QQXfszIsLsJ_S_&pullRequest=6059
ok(!account.use_ssl, "MySQL: non-SPIFFE user preserves explicit TLS setting");
free_account_details(account);
}

static void test_mysql_invalid_spiffe_attributes_do_not_require_ssl() {
GloMyAuth->reset();

mysql_add_frontend_with_attributes(
GloMyAuth, "invalid-spiffe-user", false, R"({"role":"client"})");
mysql_add_frontend_with_attributes(
GloMyAuth, "invalid-spiffe-user", false,
R"({"spiffe_id":"spiffe://example.org/ns/default/sa/client","default-transaction_isolation":123})");
account_details_t account = GloMyAuth->lookup(
(char *)"invalid-spiffe-user", USERNAME_FRONTEND, { false, false, true });

Check failure on line 497 in test/tap/tests/unit/auth_unit-t.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

C-style cast removing const qualification from the type of a pointer may lead to undefined behavior. 


See more on https://sonarcloud.io/project/issues?id=sysown_proxysql&issues=AZ_8W9QQXfszIsLsJ_TA&open=AZ_8W9QQXfszIsLsJ_TA&pullRequest=6059
ok(!account.use_ssl, "MySQL: invalid SPIFFE attributes preserve explicit TLS setting");
ok(account.attributes != nullptr && account.attributes[0] == '\0',
"MySQL: invalid SPIFFE attributes are cleared");
free_account_details(account);
}

static void test_mysql_spiffe_attributes_case_variant_requires_ssl() {
GloMyAuth->reset();

mysql_add_frontend_with_attributes(
GloMyAuth, "case-variant-spiffe-user", false,
R"({"spiffe_id":"spiffe://example.org/ns/default/sa/client"})");
mysql_add_frontend_with_attributes(
GloMyAuth, "case-variant-spiffe-user", false,
R"({"SPIFFE_ID":"spiffe://example.org/ns/default/sa/client"})");
account_details_t account = GloMyAuth->lookup(
(char *)"case-variant-spiffe-user", USERNAME_FRONTEND,

Check failure on line 514 in test/tap/tests/unit/auth_unit-t.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

C-style cast removing const qualification from the type of a pointer may lead to undefined behavior. 


See more on https://sonarcloud.io/project/issues?id=sysown_proxysql&issues=AZ_8W9QQXfszIsLsJ_TB&open=AZ_8W9QQXfszIsLsJ_TB&pullRequest=6059
{ false, false, true });
ok(account.use_ssl, "MySQL: retained SPIFFE attributes require TLS");
ok(account.attributes != nullptr &&
strcmp(account.attributes,
R"({"spiffe_id":"spiffe://example.org/ns/default/sa/client"})") == 0,
"MySQL: case-variant update retains recognized SPIFFE attributes");
free_account_details(account);
}

static void test_mysql_spiffe_changed_attributes_toggle_ssl() {
GloMyAuth->reset();

mysql_add_frontend_with_attributes(
GloMyAuth, "changed-spiffe-user", false, R"({"role":"client"})");
mysql_add_frontend_with_attributes(
GloMyAuth, "changed-spiffe-user", false,
R"({"spiffe_id":"spiffe://example.org/ns/default/sa/client","role":"client"})");
account_details_t account = GloMyAuth->lookup(
(char *)"changed-spiffe-user", USERNAME_FRONTEND, { false, false, false });

Check failure on line 533 in test/tap/tests/unit/auth_unit-t.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

C-style cast removing const qualification from the type of a pointer may lead to undefined behavior. 


See more on https://sonarcloud.io/project/issues?id=sysown_proxysql&issues=AZ_8W9QQXfszIsLsJ_TC&open=AZ_8W9QQXfszIsLsJ_TC&pullRequest=6059
ok(account.use_ssl, "MySQL: changed SPIFFE attributes require TLS");
free_account_details(account);

mysql_add_frontend_with_attributes(
GloMyAuth, "changed-spiffe-user", false, R"({"role":"updated"})");
account = GloMyAuth->lookup(
(char *)"changed-spiffe-user", USERNAME_FRONTEND, { false, false, false });

Check failure on line 540 in test/tap/tests/unit/auth_unit-t.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

C-style cast removing const qualification from the type of a pointer may lead to undefined behavior. 


See more on https://sonarcloud.io/project/issues?id=sysown_proxysql&issues=AZ_8W9QQXfszIsLsJ_TD&open=AZ_8W9QQXfszIsLsJ_TD&pullRequest=6059
ok(!account.use_ssl, "MySQL: changed non-SPIFFE attributes clear TLS requirement");
free_account_details(account);
}

// ============================================================================
// 8. PgSQL_Authentication: Core CRUD
// ============================================================================
Expand Down Expand Up @@ -558,7 +651,7 @@
// ============================================================================

int main() {
plan(60);
plan(69);

test_init_minimal();
test_init_auth();
Expand All @@ -577,6 +670,10 @@
test_mysql_checksums(); // 4 tests
test_mysql_memory(); // 3 tests
test_mysql_frontend_backend_separation();// 4 tests
test_mysql_spiffe_requires_ssl(); // 3 tests
test_mysql_invalid_spiffe_attributes_do_not_require_ssl(); // 2 tests
test_mysql_spiffe_attributes_case_variant_requires_ssl(); // 2 tests
test_mysql_spiffe_changed_attributes_toggle_ssl(); // 2 tests

// PgSQL tests
test_pgsql_add_exists_lookup(); // 5 tests
Expand Down
Loading