From 256b7cba7512c2e777016aa3809dcd27a3358df5 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Mon, 10 Aug 2026 08:02:02 +0000 Subject: [PATCH 01/11] docs: plan frontend X.509 authentication policy --- ...2026-08-10-frontend-x509-authentication.md | 929 ++++++++++++++++++ 1 file changed, 929 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-10-frontend-x509-authentication.md diff --git a/docs/superpowers/plans/2026-08-10-frontend-x509-authentication.md b/docs/superpowers/plans/2026-08-10-frontend-x509-authentication.md new file mode 100644 index 0000000000..9830e9f2f8 --- /dev/null +++ b/docs/superpowers/plans/2026-08-10-frontend-x509-authentication.md @@ -0,0 +1,929 @@ +# Frontend X.509 Authentication Policy Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add an additive `mysql_users.attributes.require_x509` frontend policy, preserve SPIFFE's stronger identity semantics across `COM_CHANGE_USER`, and enforce both policies consistently before pass-through cache lookup or backend probing. + +**Architecture:** Capture certificate presence and OpenSSL's verification result once, when the frontend TLS handshake completes, and retain that immutable connection evidence on `MySQL_Data_Stream`. Route initial login, `COM_CHANGE_USER`, and row-backed pass-through through one certificate-policy evaluator in `MySQL_Protocol.cpp`. Keep password verification additive for `require_x509`, keep SPIFFE identity-exclusive, reject SPIFFE and pass-through targets during `COM_CHANGE_USER`, and never attempt TLS renegotiation. + +**Tech Stack:** C++17, OpenSSL, nlohmann/json, RE2, MySQL/MariaDB client libraries, ProxySQL TAP tests, GNU Make. + +## Global Constraints + +- The new user attribute is exactly `"require_x509": true|false`; it does not add a column or change the `mysql_users` schema. +- `require_x509=true` means both the existing password/auth-plugin check and a trusted frontend client certificate must succeed. +- A trusted certificate means all three conditions are true on the current physical frontend connection: TLS is active, a peer certificate was presented, and `SSL_get_verify_result()` returned `X509_V_OK`. +- A client certificate with no URI SAN is valid for `require_x509`. SPIFFE extraction remains a separate concern. +- Do not change `callback_ssl_verify_peer()` to reject invalid or absent client certificates during the TLS handshake. Users without `require_x509` must remain backward compatible, so the authentication layer applies the per-user decision. +- Do not renegotiate TLS in `COM_CHANGE_USER`. TLS 1.3 removed renegotiation, and ProxySQL's context uses `SSL_VERIFY_CLIENT_ONCE`; the only available evidence is the certificate presented during the original connection handshake. +- Reject `COM_CHANGE_USER` when the current session authenticated via SPIFFE, and reject any target account containing `spiffe_id`. This closes both directions: a SPIFFE identity cannot escape to a password identity, and a password identity cannot switch into SPIFFE without a fresh TLS/authentication handshake. +- Allow `COM_CHANGE_USER` into a `require_x509=true` target only when the original connection already carries a trusted client certificate. Otherwise return the existing generic authentication failure and require the client to reconnect with a certificate. +- Preserve the current Phase 1 behavior that a pass-through-eligible target is rejected by `COM_CHANGE_USER`, even when a valid client certificate is present. +- For row-backed pass-through, enforce `require_x509` before username-pattern checks, cache lookup, metrics mutation, `AuthMoreData{0x04}`, or backend probe creation. This guarantees identical cold-cache and warm-cache behavior. +- An empty-password row containing `spiffe_id` is not a pass-through signal. It follows the existing SPIFFE/passwordless row path and must neither read nor populate the pass-through cache. +- Unknown-user pass-through has no `mysql_users.attributes` value, so this change cannot apply `require_x509` to it. It remains protected by `mysql-passthrough_auth_require_tls`; a global unknown-user client-certificate gate is a separate feature. +- The frontend client's certificate is never forwarded to a backend. Backend TLS client identity continues to come from ProxySQL's `mysql_servers_ssl_params` configuration. +- All authentication-layer policy denials sent to a client remain generic MySQL error 1045. Preserve the existing earlier TLS-handshake failure for an untrusted certificate that itself carries a SPIFFE URI SAN; detailed reasons may be logged internally but must not reveal account existence or policy configuration over the wire. +- Preserve user-supplied worktree changes and do not broaden this work into PostgreSQL frontend authentication. + +--- + +## Task 1: Add certificate-policy TAP fixtures and failing initial-login coverage + +**Files:** + +- Create: `test/tap/tests/test_frontend_x509_auth-t.cpp` +- Modify: `test/tap/groups/groups.json` +- Reference: `test/tap/tests/test_auth_methods-t.cpp:530` +- Reference: `test/tap/tests/reg_test_4556-ssl_error_queue-t.cpp:95` +- Reference: `src/proxy_tls.cpp:224` + +**Interfaces:** + +```cpp +struct client_tls_material { + std::string key; + std::string cert; + std::string ca; +}; + +static unsigned int try_frontend_connect( + const CommandLine& cl, + const char* username, + const char* password, + bool use_tls, + const client_tls_material* client_identity = nullptr +); +``` + +- [ ] **Step 1: Build a self-contained certificate fixture in the TAP test.** + + Read `REGULAR_INFRA_DATADIR` and require these standard test-infrastructure files: + + ```text + ${REGULAR_INFRA_DATADIR}/proxysql-ca.pem + ${REGULAR_INFRA_DATADIR}/proxysql-key.pem + ${REGULAR_INFRA_DATADIR}/proxysql-cert.pem + ``` + + Create a temporary directory with `mkdtemp()`. Add helpers that invoke the `openssl` executable and quote every path. Generate: + + 1. A trusted client key/certificate with `CN=tap-require-x509` and deliberately no SAN, signed by `proxysql-ca.pem` using `proxysql-key.pem`. + 2. An untrusted self-signed client key/certificate with `CN=tap-untrusted`. + + Use unique explicit serials, for example `5928001` and `5928002`, and a one-day validity. The trusted command sequence is: + + ```sh + openssl req -new -newkey rsa:2048 -nodes \ + -subj /CN=tap-require-x509 \ + -keyout CLIENT_KEY -out CLIENT_CSR + openssl x509 -req -days 1 -set_serial 5928001 \ + -in CLIENT_CSR -CA proxysql-ca.pem -CAkey proxysql-key.pem \ + -out CLIENT_CERT + openssl verify -CAfile proxysql-ca.pem CLIENT_CERT + ``` + + The standard ProxySQL-generated test certificates use the same private key for the CA and server certificate (`ssl_mkit()` in `src/proxy_tls.cpp`). If the active environment uses custom certificates and the CA private key is unavailable or does not match, emit a clear TAP diagnostic and skip only the certificate-positive cases; do not silently treat generation failure as an authentication result. + +- [ ] **Step 2: Add a connection helper that distinguishes TLS-without-certificate from TLS-with-certificate.** + + Use the MariaDB client API in the same style as existing TAP tests: + + ```cpp + unsigned long flags = 0; + if (use_tls) { + if (client_identity) { + mysql_ssl_set(mysql, + client_identity->key.c_str(), + client_identity->cert.c_str(), + client_identity->ca.c_str(), nullptr, nullptr); + } else { + mysql_ssl_set(mysql, nullptr, nullptr, nullptr, nullptr, nullptr); + } + flags |= CLIENT_SSL; + } + MYSQL* connected = mysql_real_connect( + mysql, cl.host, username, password, nullptr, cl.port, nullptr, flags); + const unsigned int result = connected ? 0 : mysql_errno(mysql); + ``` + + Do not enable hostname verification: this test is proving the certificate ProxySQL receives from the client, not the client's validation of ProxySQL's hostname. + +- [ ] **Step 3: Provision dedicated frontend users and add the failing assertions.** + + Insert users with distinct names and passwords, then `LOAD MYSQL USERS TO RUNTIME`: + + ```sql + INSERT INTO mysql_users(username,password,default_hostgroup,active,attributes) + VALUES + ('tap_x509_none','tap-x509-password',0,1,''), + ('tap_x509_required','tap-x509-password',0,1,'{"require_x509":true}'), + ('tap_x509_false','tap-x509-password',0,1,'{"require_x509":false}'), + ('tap_x509_bad_type','tap-x509-password',0,1,'{"require_x509":"true"}'); + LOAD MYSQL USERS TO RUNTIME; + ``` + + Assert the following matrix. Every rejection must be `ER_ACCESS_DENIED_ERROR`/1045, never a disconnect, parse exception, or TLS-library error: + + | User policy | Transport/client cert | Expected | + |---|---|---| + | no attribute | plaintext | success | + | no attribute | TLS, no client cert | success | + | `false` | TLS, no client cert | success | + | `true` | plaintext | 1045 | + | `true` | TLS, no client cert | 1045 | + | `true` | TLS, untrusted client cert | 1045 | + | `true` | TLS, trusted cert without SAN | success | + | `true` | TLS, trusted cert, wrong password | 1045 | + | string `"true"` | TLS, trusted cert | 1045 (fail closed, no crash) | + + At setup, snapshot `mysql-passthrough_auth_enabled`, force it to `false`, and load variables so the empty-password SPIFFE cases added later cannot inherit pass-through state from another TAP test. Delete the dedicated frontend rows before inserting and again during cleanup. Use an RAII cleanup object for the exact directory returned by `mkdtemp()`; never remove a path assembled from an unchecked environment value. + +- [ ] **Step 4: Register the test in integration groups.** + + Add `test_frontend_x509_auth-t` beside the authentication tests in `test/tap/groups/groups.json`, using the same broad server groups as `reg_test_3504-change_user-t`: + + ```json + "test_frontend_x509_auth-t" : [ "legacy-g6", "mysql84-g6", "mysql90-g1", "mysql95-g1" ], + ``` + + No `Makefile` source-list edit is needed: `test/tap/tests/Makefile:220` discovers every `*-t.cpp` through `wildcard`. + +- [ ] **Step 5: Build and run the new test to prove the feature is absent.** + + ```sh + make -C test/tap/tests test_frontend_x509_auth-t + cd test/tap/tests && ./test_frontend_x509_auth-t + ``` + + Expected before implementation: baseline cases pass, while at least plaintext/no-cert/untrusted `require_x509=true` cases incorrectly authenticate. Record the failing TAP assertion numbers in the commit message body. + +- [ ] **Step 6: Commit only the failing test and group registration.** + + ```sh + git add test/tap/tests/test_frontend_x509_auth-t.cpp test/tap/groups/groups.json + git commit -m "test: cover frontend require_x509 policy" + ``` + +--- + +## Task 2: Capture client-certificate evidence and implement one policy evaluator + +**Files:** + +- Modify: `include/MySQL_Data_Stream.h:133` +- Modify: `lib/mysql_data_stream.cpp:220` +- Modify: `lib/mysql_data_stream.cpp:349` +- Modify: `lib/MySQL_Protocol.cpp:3402` +- Modify: `lib/MySQL_Protocol.cpp:92` +- Modify: `include/MySQL_Protocol.h:256` +- Test: `test/tap/tests/test_frontend_x509_auth-t.cpp` + +**Interfaces:** + +Add immutable-for-the-connection evidence beside `x509_subject_alt_name`: + +```cpp +char *x509_subject_alt_name; +bool client_cert_present; +long client_cert_verify_result; +bool frontend_authenticated_via_spiffe; +SSL *ssl; +``` + +Define the policy types at file scope in `lib/MySQL_Protocol.cpp`: + +```cpp +enum class frontend_auth_context : uint8_t { + INITIAL_HANDSHAKE, + COM_CHANGE_USER, + PASSTHROUGH +}; + +struct frontend_certificate_policy_result { + bool allowed { true }; + bool has_spiffe_id { false }; +}; + +static frontend_certificate_policy_result evaluate_frontend_certificate_policy( + MySQL_Data_Stream* myds, + const char* attributes, + const unsigned char* user, + frontend_auth_context context, + int calling_line, + const char* calling_func +); +``` + +- [ ] **Step 1: Initialize the new data-stream fields.** + + In `MySQL_Data_Stream::MySQL_Data_Stream()` initialize: + + ```cpp + x509_subject_alt_name = nullptr; + client_cert_present = false; + client_cert_verify_result = X509_V_OK; + frontend_authenticated_via_spiffe = false; + ssl = nullptr; + ``` + + `client_cert_present` is required because OpenSSL's verification result alone does not distinguish “no certificate” from a successfully verified certificate. Do not clear these fields in `MySQL_Session::reset()`; they belong to the physical connection and must survive `COM_RESET_CONNECTION` and `COM_CHANGE_USER`. + +- [ ] **Step 2: Record verification state for every peer certificate, not only SPIFFE certificates.** + + Restructure the successful branch of `MySQL_Data_Stream::do_ssl_handshake()` as follows: + + ```cpp + if (n == 1) { + X509* cert = SSL_get_peer_certificate(ssl); + client_cert_present = (cert != nullptr); + client_cert_verify_result = cert ? SSL_get_verify_result(ssl) : X509_V_OK; + + if (cert) { + GENERAL_NAMES* alt_names = static_cast( + X509_get_ext_d2i(cert, NID_subject_alt_name, nullptr, nullptr)); + if (alt_names) { + // Preserve the existing first spiffe:// URI extraction loop. + sk_GENERAL_NAME_pop_free(alt_names, GENERAL_NAME_free); + } + X509_free(cert); + } + + if (x509_subject_alt_name && client_cert_verify_result != X509_V_OK) { + // Preserve the existing SPIFFE handshake-failure behavior. + return SSLSTATUS_FAIL; + } + } + ``` + + Guard `alt_names` before calling `sk_GENERAL_NAME_num()`. The trusted no-SAN certificate in Task 1 is specifically intended to exercise this null case and prevent a regression crash. + +- [ ] **Step 3: Implement strict, exception-safe attribute parsing.** + + The evaluator must: + + 1. Treat null/empty attributes as allowed. + 2. Catch all `nlohmann::json::exception` values and fail closed. + 3. Require `require_x509` to be a JSON boolean. Any other type logs a configuration error and denies authentication. + 4. Set `has_spiffe_id=true` whenever the key exists, even if its value is malformed, so malformed SPIFFE rows cannot accidentally become pass-through rows. + 5. Require `spiffe_id` to be a string and fail closed otherwise. + 6. Reuse the current exact `spiffe://...` comparison and `!regex` full-match semantics with quiet RE2 options. + + Make the `#ifdef DEBUG` `debug_spiffe_id()` helper follow the same `is_string()` and exception-safety rules. Otherwise a malformed `spiffe_id` can still terminate a debug build inside `PPHR_5passwordTrue()` before the common evaluator runs. + + Core `require_x509` check: + + ```cpp + const auto require_x509 = attrs.find("require_x509"); + if (require_x509 != attrs.end()) { + if (!require_x509->is_boolean()) { + result.allowed = false; + // log invalid type; do not call get() + return result; + } + if (require_x509->get()) { + result.allowed = myds + && myds->encrypted + && myds->ssl + && myds->client_cert_present + && myds->client_cert_verify_result == X509_V_OK; + if (!result.allowed) { + // Include presence/verify text in internal log only. + return result; + } + } + } + ``` + + Evaluate `require_x509` and `spiffe_id` conjunctively when both are present. Do not let a successful SPIFFE match overwrite a previous `require_x509` denial. + +- [ ] **Step 4: Make initial authentication use the evaluator.** + + Replace the SPIFFE-only block in `verify_user_attributes()` with: + + ```cpp + const char* attributes = (*myds)->sess->user_attributes; + const auto policy = evaluate_frontend_certificate_policy( + *myds, attributes, user, + frontend_auth_context::INITIAL_HANDSHAKE, + calling_line, calling_func); + if (!policy.allowed) { + return false; + } + (*myds)->frontend_authenticated_via_spiffe = policy.has_spiffe_id; + ``` + + Retain the existing `default-transaction_isolation` application after policy success. Parse the JSON once in the function or pass a parsed object through a private helper; do not reintroduce uncaught `get()` exceptions. + + Remove `user_attributes_has_spiffe()` from `include/MySQL_Protocol.h` only after Task 3 moves its last call site to the common evaluator. + +- [ ] **Step 5: Run the initial-login test and focused TLS regression.** + + ```sh + make -C test/tap/tests test_frontend_x509_auth-t reg_test_4556-ssl_error_queue-t test_auth_methods-t + cd test/tap/tests && ./test_frontend_x509_auth-t + cd test/tap/tests && ./reg_test_4556-ssl_error_queue-t + cd test/tap/tests && ./test_auth_methods-t + ``` + + Expected: all Task 1 scenarios pass; ordinary TLS connections without a client certificate remain accepted; the SSL error queue regression remains green. + +- [ ] **Step 6: Commit the handshake evidence and common evaluator.** + + ```sh + git add include/MySQL_Data_Stream.h include/MySQL_Protocol.h \ + lib/mysql_data_stream.cpp lib/MySQL_Protocol.cpp + git commit -m "feat: enforce per-user frontend X.509 policy" + ``` + +--- + +## Task 3: Define and test `COM_CHANGE_USER` behavior + +**Files:** + +- Modify: `test/tap/tests/test_frontend_x509_auth-t.cpp` +- Modify: `lib/MySQL_Protocol.cpp:1398` +- Modify: `lib/MySQL_Protocol.cpp:1671` +- Modify: `include/MySQL_Protocol.h:257` +- Reference: `lib/MySQL_Session.cpp:8311` +- Regression: `test/tap/tests/reg_test_3504-change_user-t.cpp` + +**Interfaces:** + +```cpp +static unsigned int try_change_user( + MYSQL* connection, + const char* target_user, + const char* target_password +) { + return mysql_change_user(connection, target_user, target_password, nullptr) == 0 + ? 0 : mysql_errno(connection); +} +``` + +- [ ] **Step 1: Add failing `COM_CHANGE_USER` scenarios to the X.509 TAP test.** + + Provision these additional users: + + ```sql + ('tap_x509_source','source-password',0,1,''), + ('tap_x509_target','target-password',0,1,'{"require_x509":true}'), + ('tap_spiffe_source','',0,1,'{"spiffe_id":"spiffe://tap/source"}'), + ('tap_spiffe_target','',0,1,'{"spiffe_id":"spiffe://tap/target"}') + ``` + + Generate two trusted SPIFFE client certificates using a temporary OpenSSL extension file: + + ```text + subjectAltName=URI:spiffe://tap/source + subjectAltName=URI:spiffe://tap/target + ``` + + Add `-extfile EXTFILE` to `openssl x509 -req`. Assert: + + | Existing connection | Target | Expected | + |---|---|---| + | plaintext password source | `require_x509` target | 1045 | + | TLS password source, no cert | `require_x509` target | 1045 | + | TLS password source, untrusted cert | `require_x509` target | 1045 | + | TLS password source, trusted cert | `require_x509` target | success | + | TLS password source, trusted cert, wrong target password | `require_x509` target | 1045 | + | TLS password source, trusted cert | ordinary target | success (control) | + | SPIFFE-authenticated source | ordinary password target | 1045 | + | password-authenticated source | SPIFFE target, matching target cert | 1045 | + + For each rejected change, assert `mysql_errno()==1045`. The server currently treats failed `COM_CHANGE_USER` as terminal, so use a fresh connection per row rather than querying the old session afterward. + +- [ ] **Step 2: Prove no certificate renegotiation is attempted.** + + The “TLS password source, no cert → `require_x509` target” scenario is the regression test: it must fail without an SSL protocol error or a second certificate request. Add a diagnostic comment explaining that a reconnect using the trusted certificate succeeds, while `COM_CHANGE_USER` on the original connection cannot acquire one. + + Do not add calls to `SSL_renegotiate()`, `SSL_verify_client_post_handshake()`, or client-library reconnect internals. + +- [ ] **Step 3: Reject a SPIFFE-authenticated source before target lookup side effects.** + + At the start of `process_pkt_COM_CHANGE_USER()`, after safe packet parsing but before account state is copied, add: + + ```cpp + if ((*myds)->frontend_authenticated_via_spiffe) { + proxy_error( + "Client %s:%d cannot run COM_CHANGE_USER after SPIFFE authentication\n", + (*myds)->addr.addr, (*myds)->addr.port); + free(pass); + return false; + } + ``` + + Use the function's existing cleanup conventions rather than duplicating cleanup if a local cleanup helper is introduced. The marker is on `MySQL_Data_Stream`, so the preceding `MySQL_Session::reset()` does not erase it. + +- [ ] **Step 4: Evaluate the target account before session mutation or Auth Switch.** + + Immediately after `GloMyAuth->lookup()` and `get_password(account_details, PRIMARY)`, but before assigning `default_hostgroup`, `transaction_persistent`, or `user_attributes`, evaluate: + + ```cpp + const auto target_policy = evaluate_frontend_certificate_policy( + *myds, + account_details.attributes, + user, + frontend_auth_context::COM_CHANGE_USER, + __LINE__, __func__); + + if (!target_policy.allowed || target_policy.has_spiffe_id) { + // common generic-denial cleanup; no Auth Switch Request + return false; + } + ``` + + In `COM_CHANGE_USER` context, the evaluator should mark any `spiffe_id` target denied without attempting identity matching. This makes direct-password and password-omitted/Auth-Switch forms follow the same rule. + + Keep the existing pass-through-target check directly after this policy gate. Its eligibility must use `!target_policy.has_spiffe_id`, matching Task 4's initial-login logic. + +- [ ] **Step 5: Remove the late, target-attribute SPIFFE block.** + + Delete the `user_attributes_has_spiffe()` call around current `lib/MySQL_Protocol.cpp:1671` and remove the method declaration/definition. That block is too late: it runs only after password success, after target attributes overwrite the session, and not uniformly before Auth Switch. + + After successful change to a non-SPIFFE account, explicitly keep: + + ```cpp + (*myds)->frontend_authenticated_via_spiffe = false; + ``` + + This is currently implied because all SPIFFE transitions are denied, but the assignment documents and preserves the state invariant. + +- [ ] **Step 6: Run focused and existing change-user tests.** + + ```sh + make -C test/tap/tests test_frontend_x509_auth-t reg_test_3504-change_user-t \ + reg_test_3504-change_user_libmariadb_helper \ + reg_test_3504-change_user_libmysql_helper + cd test/tap/tests && ./test_frontend_x509_auth-t + cd test/tap/tests && ./reg_test_3504-change_user-t + ``` + + Expected: the new matrix passes and ordinary password-to-password `COM_CHANGE_USER` remains unchanged across SSL/non-SSL and supported plugins. + +- [ ] **Step 7: Commit the `COM_CHANGE_USER` policy.** + + ```sh + git add include/MySQL_Protocol.h lib/MySQL_Protocol.cpp \ + test/tap/tests/test_frontend_x509_auth-t.cpp + git commit -m "fix: preserve certificate identity across change user" + ``` + +--- + +## Task 4: Enforce X.509 before pass-through cache/probe paths + +**Files:** + +- Create: `test/tap/tests/test_frontend_x509_passthrough-t.cpp` +- Modify: `test/tap/groups/groups.json` +- Modify: `lib/MySQL_Protocol.cpp:2736` +- Reference: `lib/MySQL_Protocol.cpp:2577` +- Reference: `lib/MySQL_Session.cpp:1885` +- Regression: `test/tap/tests/test_passthrough_auth_e2e-t.cpp` +- Regression: `test/tap/tests/test_passthrough_auth_security-t.cpp` + +**Interfaces:** + +Reuse Task 1's certificate-generation and connect patterns. If copying the helpers would create more than roughly 100 duplicated lines, extract test-only helpers into: + +```text +test/tap/tests/frontend_x509_test_utils.h +``` + +Keep the helper header-only so the wildcard Makefile rules need no additional link object. + +- [ ] **Step 1: Write a pass-through test that differentiates policy from backend behavior.** + + Create a MySQL 8+ backend user using `caching_sha2_password`, and an empty-password ProxySQL row carrying the X.509 policy: + + ```sql + CREATE USER 'tap_x509_pt'@'%' + IDENTIFIED WITH 'caching_sha2_password' BY 'x509-pass-through-password'; + GRANT SELECT ON *.* TO 'tap_x509_pt'@'%'; + + INSERT INTO mysql_users(username,password,default_hostgroup,active,attributes) + VALUES + ('tap_x509_pt','',30,1,'{"require_x509":true}'), + ('tap_x509_pt_target','ordinary-target-password',30,1,''); + LOAD MYSQL USERS TO RUNTIME; + ``` + + Enable only row-backed pass-through, raise test failure-rate caps, and flush the cache: + + ```sql + SET mysql-passthrough_auth_enabled='true'; + SET mysql-passthrough_auth_empty_password='true'; + SET mysql-passthrough_auth_unknown_users='false'; + SET mysql-passthrough_auth_require_tls='true'; + SET mysql-passthrough_auth_max_failures_per_user='10000'; + SET mysql-passthrough_auth_max_failures_per_ip='10000'; + SET mysql-default_authentication_plugin='caching_sha2_password'; + LOAD MYSQL VARIABLES TO RUNTIME; + PROXYSQL FLUSH PASSTHROUGH_AUTH_CACHE; + ``` + +- [ ] **Step 2: Add failing cold-cache ordering assertions.** + + Snapshot `cache_hits`, `probes_attempted`, and the per-user cache row count before each attempt. Assert: + + 1. TLS without a client certificate returns 1045. + 2. `probes_attempted` is unchanged. + 3. No cache entry was created. + 4. TLS with an untrusted client certificate has the same outcome. + 5. TLS with the trusted no-SAN certificate but the wrong backend password returns 1045, increments `probes_attempted`, and leaves the cache empty. + 6. TLS with the trusted no-SAN certificate and the correct backend password succeeds, increments `probes_attempted` again, and creates one cache entry. + + Use queries modeled on `test_passthrough_auth_metrics-t.cpp`: + + ```sql + SELECT metric_value + FROM stats_mysql_passthrough_auth_metrics + WHERE metric_name='probes_attempted'; + + SELECT COUNT(*) + FROM stats_mysql_passthrough_auth_cache + WHERE username='tap_x509_pt'; + ``` + +- [ ] **Step 3: Add failing warm-cache ordering assertions.** + + With the cache now populated: + + 1. TLS without a client certificate returns 1045 and `cache_hits` is unchanged. + 2. TLS with the trusted client certificate succeeds and increments `cache_hits` by one. + + This specifically detects the tempting but incorrect implementation that checks attributes only in `verify_user_attributes()`: cold probe completion sends OK directly from `MySQL_Session::handler_again___status_AUTHENTICATING_BACKEND_FOR_CLIENT()` and never reaches that epilogue. + +- [ ] **Step 4: Add SPIFFE/pass-through precedence coverage.** + + Provision an empty-password row with only: + + ```json + {"spiffe_id":"spiffe://tap/pass-through-exclusion"} + ``` + + Connect with a trusted certificate bearing that URI SAN and an empty frontend password. Assert: + + 1. Authentication succeeds through the normal SPIFFE path. + 2. `probes_attempted` is unchanged. + 3. No pass-through cache entry exists for the SPIFFE user. + + A mismatching/no-certificate attempt must return 1045, still without a probe. This protects SPIFFE's identity semantics from the new empty-password pass-through interpretation. + +- [ ] **Step 5: Preserve `COM_CHANGE_USER` rejection for pass-through targets.** + + Open an ordinary password-authenticated TLS connection with the trusted client certificate, then call `mysql_change_user()` targeting `tap_x509_pt` with the correct backend password. Assert 1045 and unchanged `probes_attempted`. A certificate satisfies `require_x509`; it does not make the unsupported pass-through state machine legal in `COM_CHANGE_USER`. + + As a directional control, open a successfully pass-through-authenticated `tap_x509_pt` connection with the trusted certificate and change to the ordinary password-backed `tap_x509_pt_target`. Assert success. Pass-through is not a connection-bound identity scheme: only a pass-through *target* is unsupported, and `MySQL_Session::reset()` deliberately clears `passthrough_credential` before authenticating the new target. + +- [ ] **Step 6: Register the pass-through test in MySQL 8+ groups and run it failing.** + + Add: + + ```json + "test_frontend_x509_passthrough-t" : [ "mysql84-g4", "mysql90-g4", "mysql95-g4" ], + ``` + + Then run: + + ```sh + make -C test/tap/tests test_frontend_x509_passthrough-t + cd test/tap/tests && ./test_frontend_x509_passthrough-t + ``` + + Expected before implementation: no-cert attempts reach pass-through/cache behavior, and the SPIFFE empty row may be misclassified as pass-through. + +- [ ] **Step 7: Compute policy before pass-through eligibility and side effects.** + + At the top of the pass-through block in `PPHR_verify_password()`, retain the raw row state separately from effective eligibility: + + ```cpp + const bool raw_empty_pw_case = + mysql_thread___passthrough_auth_empty_password + && vars1.password != nullptr + && vars1.password[0] == '\0'; + + frontend_certificate_policy_result row_policy {}; + if (raw_empty_pw_case) { + row_policy = evaluate_frontend_certificate_policy( + *myds, + account_details.attributes, + vars1.user, + frontend_auth_context::PASSTHROUGH, + __LINE__, __func__); + } + + const bool empty_pw_case = raw_empty_pw_case && !row_policy.has_spiffe_id; + ``` + + Then enforce: + + ```cpp + if (mysql_thread___passthrough_auth_enabled + && empty_pw_case + && !row_policy.allowed) { + return false; + } + ``` + + This gate must appear before: + + - the non-`caching_sha2_password` hard rejection, + - username allowlist evaluation, + - unknown-user default synthesis, + - `GloMyPTAuthCache->lookup()` and counter increments, + - `PPHR_5passwordTrue()`, and + - `PPHR_passthrough_init()`. + + When `raw_empty_pw_case && row_policy.has_spiffe_id`, skip all pass-through-only rejection/dispatch code and continue through the legacy empty-password branch. The common `verify_user_attributes()` epilogue then performs the SPIFFE identity match. Do not accept the backend password for this case: the configured frontend empty password remains the expected password input before SPIFFE validation. + +- [ ] **Step 8: Keep unknown-user semantics explicit.** + + Do not run the evaluator with fabricated empty attributes for `unknown_user_case`, and do not add a global variable in this change. Add an in-source comment: + + ```cpp + // Unknown-user pass-through has no mysql_users row and therefore no + // per-user require_x509 attribute. Its transport gate remains + // mysql-passthrough_auth_require_tls. + ``` + +- [ ] **Step 9: Run pass-through and change-user regressions.** + + ```sh + make -C test/tap/tests \ + test_frontend_x509_passthrough-t \ + test_passthrough_auth_e2e-t \ + test_passthrough_auth_security-t \ + test_passthrough_auth_unknown_user-t \ + reg_test_3504-change_user-t + + cd test/tap/tests && ./test_frontend_x509_passthrough-t + cd test/tap/tests && ./test_passthrough_auth_e2e-t + cd test/tap/tests && ./test_passthrough_auth_security-t + cd test/tap/tests && ./test_passthrough_auth_unknown_user-t + cd test/tap/tests && ./reg_test_3504-change_user-t + ``` + + Expected: all pass, including cold/warm X.509 differentials, no-probe SPIFFE assertions, existing TLS gate behavior, unknown-user behavior, and pass-through `COM_CHANGE_USER` rejection. + + In the test cleanup path, drop backend user `tap_x509_pt`, delete every ProxySQL frontend row created by the fixture (`tap_x509_pt`, `tap_x509_pt_target`, and the SPIFFE exclusion user), flush the pass-through cache, restore every snapshotted `mysql-passthrough_auth_*` and default-authentication-plugin value, and load users/variables back to runtime. Cleanup must run after failed assertions as well as successful ones so later tests in `mysql84-g4` do not inherit this fixture. + +- [ ] **Step 10: Commit pass-through integration separately.** + + ```sh + git add lib/MySQL_Protocol.cpp \ + test/tap/tests/test_frontend_x509_passthrough-t.cpp \ + test/tap/tests/test_frontend_x509_auth-t.cpp \ + test/tap/tests/frontend_x509_test_utils.h \ + test/tap/groups/groups.json + git commit -m "fix: apply X.509 policy before pass-through auth" + ``` + + Omit `frontend_x509_test_utils.h` from `git add` if no shared helper was extracted. + +--- + +## Task 5: Validate configuration types and document operator-visible semantics + +**Files:** + +- Modify: `lib/MySQL_Authentication.cpp:188` +- Create: `doc/frontend_x509_authentication.md` +- Modify: `doc/internal/passthrough_authentication.md:30` +- Modify: `doc/internal/passthrough_authentication.md:172` +- Modify: `doc/internal/passthrough_authentication.md:246` +- Test: `test/tap/tests/test_frontend_x509_auth-t.cpp` + +- [ ] **Step 1: Add load-time diagnostics without turning malformed values into allow.** + + In the existing JSON validation block in `MySQL_Authentication::add()`, inspect `require_x509`: + + ```cpp + const auto require_x509 = valid.find("require_x509"); + if (require_x509 != valid.end() && !require_x509->is_boolean()) { + proxy_error( + "Invalid require_x509 attribute for user %s: expected JSON boolean; " + "authentication will be denied until corrected\n", + username); + } + ``` + + Preserve the original attribute in runtime so the evaluator can fail closed. Do not erase the key, coerce strings/numbers, or replace all attributes with an empty string; each of those would turn a configuration error into an unintended allow. + +- [ ] **Step 2: Extend the invalid-type TAP assertion.** + + Verify both: + + 1. `LOAD MYSQL USERS TO RUNTIME` completes without crashing ProxySQL. + 2. Login for the malformed account returns 1045 even with a trusted certificate. + + If the test already tails `proxysql.log`, also assert one diagnostic containing the username and `expected JSON boolean`. Do not make the test depend on an exact full log sentence. + +- [ ] **Step 3: Write the frontend X.509 operator guide.** + + `doc/frontend_x509_authentication.md` must include: + + - Configuration example: + + ```sql + UPDATE mysql_users + SET attributes='{"require_x509":true}' + WHERE username='application_user'; + LOAD MYSQL USERS TO RUNTIME; + SAVE MYSQL USERS TO DISK; + ``` + + - Trust source: the frontend certificate is validated against ProxySQL's frontend `proxysql-ca.pem` loaded by the TLS context. + - Additive semantics: valid password plus valid client certificate. + - Difference from `use_ssl`: `use_ssl` requires encryption; `require_x509` additionally requires a verified peer certificate. + - Difference from `spiffe_id`: SPIFFE binds the username to a URI identity and remains the authoritative certificate-identity check after the configured frontend password step; `require_x509` only proves membership in the trusted PKI and is additive to password authentication. + - `COM_CHANGE_USER`: no renegotiation; it reuses the original connection certificate, rejects when absent/invalid, and always rejects SPIFFE source/target identities. + - Pass-through: row-backed `require_x509` is checked before cache/probe; passwords still come from backend verification; SPIFFE rows are excluded; unknown users have no per-user attribute. `COM_CHANGE_USER` into a pass-through target is rejected, while changing from a pass-through-authenticated source into an ordinary password-backed target remains supported. + - Backend separation: ProxySQL never forwards the frontend certificate; backend certificates/keys are configured independently. + - Failure behavior: authentication-policy failures return 1045 and detailed certificate reasons remain in ProxySQL logs. An untrusted certificate carrying a SPIFFE URI SAN retains the existing earlier TLS-handshake failure behavior. + +- [ ] **Step 4: Update the pass-through design document.** + + Add a “Frontend certificate policy” subsection near eligibility and amend §5.4 and §7.1 with this ordering: + + ```text + row lookup + -> require_x509 / SPIFFE classification + -> username allowlist + -> pass-through TLS gate + -> cache lookup + -> cleartext request + -> backend probe + ``` + + State explicitly that cold-probe completion sends the frontend OK from `MySQL_Session`, which is why the X.509 decision is made before dispatch rather than relying only on the normal handshake epilogue. + +- [ ] **Step 5: Run the config test and documentation checks.** + + ```sh + make -C test/tap/tests test_frontend_x509_auth-t + cd test/tap/tests && ./test_frontend_x509_auth-t + rg -n "require_x509|COM_CHANGE_USER|SPIFFE|pass-through|unknown" \ + doc/frontend_x509_authentication.md \ + doc/internal/passthrough_authentication.md + ``` + +- [ ] **Step 6: Commit validation and documentation.** + + ```sh + git add lib/MySQL_Authentication.cpp \ + doc/frontend_x509_authentication.md \ + doc/internal/passthrough_authentication.md \ + test/tap/tests/test_frontend_x509_auth-t.cpp + git commit -m "docs: define frontend X.509 authentication semantics" + ``` + +--- + +## Task 6: Full verification, invariants audit, and cleanup + +**Files:** + +- Review: `include/MySQL_Data_Stream.h` +- Review: `include/MySQL_Protocol.h` +- Review: `lib/mysql_data_stream.cpp` +- Review: `lib/MySQL_Protocol.cpp` +- Review: `lib/MySQL_Authentication.cpp` +- Review: `lib/MySQL_Session.cpp` +- Review: `test/tap/tests/test_frontend_x509_auth-t.cpp` +- Review: `test/tap/tests/test_frontend_x509_passthrough-t.cpp` +- Review: `test/tap/groups/groups.json` +- Review: `doc/frontend_x509_authentication.md` +- Review: `doc/internal/passthrough_authentication.md` + +- [ ] **Step 1: Audit every authentication completion path.** + + Use `rg` to confirm the intended coverage: + + ```sh + rg -n "generate_pkt_OK|PPHR_passthrough_init|PPHR_verify_password|verify_user_attributes|process_pkt_COM_CHANGE_USER" \ + lib/MySQL_Protocol.cpp lib/MySQL_Session.cpp + ``` + + Manually verify these invariants: + + - Normal initial auth reaches `verify_user_attributes()` after password success. + - Warm-cache pass-through cannot touch the cache before the row X.509 gate. + - Cold-cache pass-through cannot send AuthMoreData or probe before the row X.509 gate. + - Cold-probe success's direct OK is safe because the immutable certificate evidence was already checked. + - Additional-password retry runs the same policy and cannot bypass it. + - `COM_CHANGE_USER` evaluates source SPIFFE state and target attributes before Auth Switch and target session-attribute mutation. + - Unknown-user pass-through remains unchanged and is never mistaken for an attribute-bearing row. + +- [ ] **Step 2: Audit certificate ownership and reset behavior.** + + Confirm: + + - `client_cert_present`, `client_cert_verify_result`, and `frontend_authenticated_via_spiffe` are initialized exactly once per `MySQL_Data_Stream`. + - No OpenSSL/X509 pointer is retained; only scalar status and the existing duplicated URI string survive the handshake. + - `GENERAL_NAMES` is freed only when non-null and `X509` is freed on every certificate branch. + - `MySQL_Session::reset()` does not clear physical TLS evidence. + - The destructor needs no new cleanup for scalar fields. + +- [ ] **Step 3: Search for stale helpers, unsafe parsing, and accidental renegotiation.** + + ```sh + rg -n "user_attributes_has_spiffe|SSL_renegotiate|SSL_verify_client_post_handshake" \ + include lib src + rg -n 'require_x509.*get<|spiffe_id.*get<' lib/MySQL_Protocol.cpp + ``` + + Expected: the stale helper and renegotiation calls are absent. Any remaining JSON `get<>` is guarded by an `is_boolean()`/`is_string()` check and an exception boundary. + +- [ ] **Step 4: Build ProxySQL and all focused tests from the current tree.** + + ```sh + make -j4 + make -C test/tap/tests \ + test_frontend_x509_auth-t \ + test_frontend_x509_passthrough-t \ + test_auth_methods-t \ + reg_test_3504-change_user-t \ + reg_test_4556-ssl_error_queue-t \ + test_passthrough_auth_e2e-t \ + test_passthrough_auth_security-t \ + test_passthrough_auth_unknown_user-t + ``` + + Do not claim success from compilation alone; run the TAP binaries against the appropriate standard infrastructure groups. + +- [ ] **Step 5: Run the complete focused TAP matrix.** + + ```sh + cd test/tap/tests && ./test_frontend_x509_auth-t + cd test/tap/tests && ./test_frontend_x509_passthrough-t + cd test/tap/tests && ./test_auth_methods-t + cd test/tap/tests && ./reg_test_3504-change_user-t + cd test/tap/tests && ./reg_test_4556-ssl_error_queue-t + cd test/tap/tests && ./test_passthrough_auth_e2e-t + cd test/tap/tests && ./test_passthrough_auth_security-t + cd test/tap/tests && ./test_passthrough_auth_unknown_user-t + ``` + + Expected: every TAP plan completes with zero failed assertions. Record any environment-based certificate skips explicitly; CI's standard auto-generated CA must execute, not skip, the trusted-certificate and SPIFFE cases. + +- [ ] **Step 6: Run static diff hygiene checks.** + + ```sh + git diff --check + git status --short + git diff --stat + git diff -- include/MySQL_Data_Stream.h include/MySQL_Protocol.h \ + lib/mysql_data_stream.cpp lib/MySQL_Protocol.cpp \ + lib/MySQL_Authentication.cpp lib/MySQL_Session.cpp + ``` + + Review specifically for accidental password/certificate logging, non-generic client errors, unrelated formatting churn, and modifications to the TLS callback that would make certificates globally mandatory. + +- [ ] **Step 7: Commit any verification-only corrections.** + + If verification required a scoped fix, rerun its failing test first, then the focused matrix, and commit only that correction: + + ```sh + git add include/MySQL_Data_Stream.h include/MySQL_Protocol.h \ + lib/mysql_data_stream.cpp lib/MySQL_Protocol.cpp \ + lib/MySQL_Authentication.cpp lib/MySQL_Session.cpp \ + test/tap/tests/test_frontend_x509_auth-t.cpp \ + test/tap/tests/test_frontend_x509_passthrough-t.cpp \ + test/tap/groups/groups.json \ + doc/frontend_x509_authentication.md \ + doc/internal/passthrough_authentication.md + git commit -m "test: tighten frontend X.509 regressions" + ``` + + If no correction was needed, do not create an empty commit. + +--- + +## Acceptance Matrix + +| Flow | Account policy | Connection evidence | Result | +|---|---|---|---| +| Initial login | none / `require_x509=false` | plaintext or TLS without cert | Existing password behavior | +| Initial login | `require_x509=true` | plaintext | 1045 | +| Initial login | `require_x509=true` | TLS, no cert | 1045 | +| Initial login | `require_x509=true` | TLS, untrusted cert | 1045 | +| Initial login | `require_x509=true` | TLS, trusted cert, no SAN | Password result decides | +| Initial login | `spiffe_id` | matching trusted URI SAN | Existing SPIFFE success | +| Initial login | `spiffe_id` | missing/mismatching trusted cert | 1045 | +| TLS handshake | client cert carries SPIFFE URI SAN | untrusted cert | Existing TLS-handshake failure | +| `COM_CHANGE_USER` | ordinary → `require_x509=true` | original trusted cert | Target password result decides | +| `COM_CHANGE_USER` | ordinary → `require_x509=true` | no original trusted cert | 1045; reconnect required | +| `COM_CHANGE_USER` | SPIFFE source → any target | any | 1045 | +| `COM_CHANGE_USER` | any source → SPIFFE target | any | 1045 | +| `COM_CHANGE_USER` | any source → pass-through target | any | 1045 | +| Pass-through cold cache | row has `require_x509=true` | no trusted cert | 1045; no cache/probe activity | +| Pass-through cold cache | row has `require_x509=true` | trusted cert | Backend probe decides | +| Pass-through warm cache | row has `require_x509=true` | no trusted cert | 1045; no cache hit | +| Pass-through warm cache | row has `require_x509=true` | trusted cert | Cached password verification decides | +| Pass-through classification | row has `spiffe_id` and empty password | matching SPIFFE cert | SPIFFE path; no cache/probe | +| `COM_CHANGE_USER` | pass-through source → ordinary target | target credentials/policy pass | Success | +| Unknown-user pass-through | no row/attributes | TLS according to global gate | Existing behavior unchanged | From 7f6ba1ef94d004717326f2b0f9a4b09b559b5e5a Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Mon, 10 Aug 2026 08:28:23 +0000 Subject: [PATCH 02/11] test: cover frontend require_x509 policy RED runtime evidence is blocked in this worktree: the direct binary skips because REGULAR_INFRA_DATADIR is unset, and the documented isolated runner stops while configuring the current release ProxySQL binary (Unknown global variable: admin-debug). No failing TAP assertions were observed. The intended feature-absent assertions are 10-12 (plaintext, TLS without certificate, and untrusted certificate) plus 15 for the string policy type. --- test/tap/groups/groups.json | 1 + test/tap/tests/test_frontend_x509_auth-t.cpp | 324 +++++++++++++++++++ 2 files changed, 325 insertions(+) create mode 100644 test/tap/tests/test_frontend_x509_auth-t.cpp diff --git a/test/tap/groups/groups.json b/test/tap/groups/groups.json index 91d9e0d0fe..9dbdf736da 100644 --- a/test/tap/groups/groups.json +++ b/test/tap/groups/groups.json @@ -329,6 +329,7 @@ "test_admin_stats-t" : [ "legacy-g7","mysql-auto_increment_delay_multiplex=0-g2","mysql-multiplexing=false-g2","mysql-query_digests=0-g2","mysql-query_digests_keep_comment=1-g2","mysql84-g7","mysql90-g2","mysql95-g2" ], "test_ansi_quotes_group_replication-t" : [ "mysql84-gr-g1","mysql90-gr-g1","mysql91-gr-g1","mysql92-gr-g1","mysql93-gr-g1","mysql95-gr-g1" ], "test_auth_methods-t" : [ "mysql-auto_increment_delay_multiplex=0-g2","mysql-multiplexing=false-g2","mysql-query_digests=0-g2","mysql-query_digests_keep_comment=1-g2","mysql84-g7","mysql90-g2","mysql95-g2" ], + "test_frontend_x509_auth-t" : [ "legacy-g6", "mysql84-g6", "mysql90-g1", "mysql95-g1" ], "test_auto_increment_delay_multiplex-t" : [ "legacy-g7","mysql-auto_increment_delay_multiplex=0-g2","mysql-multiplexing=false-g2","mysql-query_digests=0-g2","mysql-query_digests_keep_comment=1-g2","mysql84-g7","mysql90-g2","mysql95-g2" ], "test_backend_conn_ping-t" : [ "legacy-g7","mysql-auto_increment_delay_multiplex=0-g2","mysql-multiplexing=false-g2","mysql-query_digests=0-g2","mysql-query_digests_keep_comment=1-g2","mysql84-g7","mysql90-g2","mysql95-g2" ], "test_binlog_dump_multi_backend_crash-t" : [ "legacy-binlog-g1" ], diff --git a/test/tap/tests/test_frontend_x509_auth-t.cpp b/test/tap/tests/test_frontend_x509_auth-t.cpp new file mode 100644 index 0000000000..16b9253372 --- /dev/null +++ b/test/tap/tests/test_frontend_x509_auth-t.cpp @@ -0,0 +1,324 @@ +/** + * @file test_frontend_x509_auth-t.cpp + * @brief Covers the frontend mysql_users.attributes require_x509 policy. + * + * The certificate fixture deliberately signs a client certificate without a + * SAN. Frontend require_x509 is a client-certificate policy, not a hostname + * validation policy, so that certificate must remain acceptable. + */ + +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +#include "mysql.h" +#include "mysqld_error.h" + +#include "tap.h" +#include "command_line.h" + +using std::string; + +static constexpr const char* USER_NONE = "tap_x509_none"; +static constexpr const char* USER_REQUIRED = "tap_x509_required"; +static constexpr const char* USER_FALSE = "tap_x509_false"; +static constexpr const char* USER_BAD_TYPE = "tap_x509_bad_type"; +static constexpr const char* PASSWORD = "tap-x509-password"; +static constexpr const char* WRONG_PASSWORD = "tap-x509-wrong-password"; + +struct client_tls_material { + string key; + string cert; + string ca; +}; + +struct mysql_closer { + void operator()(MYSQL* mysql) const { + if (mysql) mysql_close(mysql); + } +}; + +using mysql_ptr = std::unique_ptr; + +/** Quote one shell argument, including paths derived from the environment. */ +static string shell_quote(const string& value) { + string quoted { "'" }; + for (const char c : value) { + if (c == '\'') { + quoted += "'\\''"; + } else { + quoted += c; + } + } + quoted += "'"; + return quoted; +} + +static bool run_openssl(const string& command) { + diag("Running: %s", command.c_str()); + const int status = system(command.c_str()); + if (status != 0) { + diag("openssl command failed with status %d", status); + return false; + } + return true; +} + +/** + * Own exactly the path returned by mkdtemp(). Cleanup never follows a path + * assembled from REGULAR_INFRA_DATADIR or another unchecked environment value. + */ +class temporary_certificate_directory { + string path_ {}; + +public: + temporary_certificate_directory() { + char template_path[] = "/tmp/proxysql-require-x509-XXXXXX"; + char* made = mkdtemp(template_path); + if (made) path_ = made; + } + + ~temporary_certificate_directory() { + if (path_.empty()) return; + const char* const files[] { + "trusted-client.key", "trusted-client.csr", "trusted-client.pem", + "untrusted-client.key", "untrusted-client.pem" + }; + for (const char* file : files) { + const string filename { path_ + "/" + file }; + unlink(filename.c_str()); + } + rmdir(path_.c_str()); + } + + bool valid() const { return !path_.empty(); } + const string& path() const { return path_; } +}; + +static bool file_is_readable(const string& path) { + if (access(path.c_str(), R_OK) == 0) return true; + diag("Required certificate fixture file is unavailable: %s: %s", path.c_str(), strerror(errno)); + return false; +} + +static bool create_trusted_client_certificate( + const temporary_certificate_directory& directory, + const string& ca, const string& ca_key, client_tls_material& material +) { + material.key = directory.path() + "/trusted-client.key"; + const string csr { directory.path() + "/trusted-client.csr" }; + material.cert = directory.path() + "/trusted-client.pem"; + material.ca = ca; + + const bool req_ok = run_openssl( + "openssl req -new -newkey rsa:2048 -nodes -subj /CN=tap-require-x509" + " -keyout " + shell_quote(material.key) + " -out " + shell_quote(csr) + ); + const bool sign_ok = req_ok && run_openssl( + "openssl x509 -req -days 1 -set_serial 5928001 -in " + shell_quote(csr) + + " -CA " + shell_quote(ca) + " -CAkey " + shell_quote(ca_key) + + " -out " + shell_quote(material.cert) + ); + return sign_ok && run_openssl( + "openssl verify -CAfile " + shell_quote(ca) + " " + shell_quote(material.cert) + ); +} + +static bool create_untrusted_client_certificate( + const temporary_certificate_directory& directory, const string& ca, client_tls_material& material +) { + material.key = directory.path() + "/untrusted-client.key"; + material.cert = directory.path() + "/untrusted-client.pem"; + material.ca = ca; + return run_openssl( + "openssl req -x509 -newkey rsa:2048 -nodes -days 1 -set_serial 5928002" + " -subj /CN=tap-untrusted -keyout " + shell_quote(material.key) + + " -out " + shell_quote(material.cert) + ); +} + +/** + * Attempt one frontend connection and return 0 on success or the client error. + * A null client_identity means TLS is requested without a client certificate. + */ +static unsigned int try_frontend_connect( + const CommandLine& cl, + const char* username, + const char* password, + bool use_tls, + const client_tls_material* client_identity = nullptr +) { + mysql_ptr mysql { mysql_init(NULL) }; + if (!mysql) return UINT_MAX; + + unsigned long flags = 0; + if (use_tls) { + if (client_identity) { + mysql_ssl_set(mysql.get(), + client_identity->key.c_str(), + client_identity->cert.c_str(), + client_identity->ca.c_str(), nullptr, nullptr); + } else { + mysql_ssl_set(mysql.get(), nullptr, nullptr, nullptr, nullptr, nullptr); + } + flags |= CLIENT_SSL; + } + + MYSQL* connected = mysql_real_connect( + mysql.get(), cl.host, username, password, nullptr, cl.port, nullptr, flags); + const unsigned int result = connected ? 0 : mysql_errno(mysql.get()); + diag("Frontend connect user='%s' tls=%s client_cert=%s -> errno=%u '%s'", + username, use_tls ? "yes" : "no", client_identity ? "yes" : "no", + result, connected ? "connected" : mysql_error(mysql.get())); + return result; +} + +static bool do_query(MYSQL* mysql, const string& query) { + if (mysql_query(mysql, query.c_str()) == 0) return true; + diag("Query failed: %s -- %s", query.c_str(), mysql_error(mysql)); + return false; +} + +static bool read_global_variable(MYSQL* admin, const char* name, string& value) { + const string query { + string("SELECT variable_value FROM global_variables WHERE variable_name='") + name + "'" + }; + if (!do_query(admin, query)) return false; + MYSQL_RES* result = mysql_store_result(admin); + if (!result) return false; + MYSQL_ROW row = mysql_fetch_row(result); + const bool found = row && row[0]; + if (found) value = row[0]; + mysql_free_result(result); + return found; +} + +int main() { + CommandLine cl; + + const char* const datadir_env = getenv("REGULAR_INFRA_DATADIR"); + if (!datadir_env || !*datadir_env) { + diag("SKIP: REGULAR_INFRA_DATADIR is unset; run through the isolated TAP runner, which mounts /var/lib/proxysql."); + plan(0); + return exit_status(); + } + const string datadir { datadir_env }; + const string ca { datadir + "/proxysql-ca.pem" }; + const string ca_key { datadir + "/proxysql-key.pem" }; + const string server_cert { datadir + "/proxysql-cert.pem" }; + if (!file_is_readable(ca) || !file_is_readable(ca_key) || !file_is_readable(server_cert)) { + diag("SKIP: require_x509 test needs the standard ProxySQL certificate fixture in REGULAR_INFRA_DATADIR."); + plan(0); + return exit_status(); + } + + if (cl.getEnv()) { + diag("Failed to get the required TAP connection environmental variables."); + return EXIT_FAILURE; + } + + /* + * 4 setup + 2 certificate fixtures + 9 policy probes + 2 cleanup checks. + * A custom environment whose CA private key cannot sign our certificate + * emits TAP SKIPs only for the probes that need that trusted certificate. + */ + plan(17); + + mysql_ptr admin { mysql_init(NULL) }; + if (!admin || !mysql_real_connect(admin.get(), cl.host, cl.admin_username, cl.admin_password, + NULL, cl.admin_port, NULL, 0)) { + ok(false, "Connected to ProxySQL Admin: %s", admin ? mysql_error(admin.get()) : "mysql_init failed"); + return exit_status(); + } + ok(true, "Connected to ProxySQL Admin at %s:%d", cl.host, cl.admin_port); + + string original_passthrough_enabled; + const bool saved_passthrough = read_global_variable( + admin.get(), "mysql-passthrough_auth_enabled", original_passthrough_enabled); + ok(saved_passthrough, "Saved mysql-passthrough_auth_enabled before the test"); + + const bool passthrough_disabled = saved_passthrough && + do_query(admin.get(), "SET mysql-passthrough_auth_enabled='false'") && + do_query(admin.get(), "LOAD MYSQL VARIABLES TO RUNTIME"); + ok(passthrough_disabled, "Disabled mysql-passthrough_auth_enabled for this test"); + + const string user_list { + "'tap_x509_none','tap_x509_required','tap_x509_false','tap_x509_bad_type'" + }; + const bool users_provisioned = do_query(admin.get(), "DELETE FROM mysql_users WHERE username IN (" + user_list + ")") && + do_query(admin.get(), + "INSERT INTO mysql_users(username,password,default_hostgroup,active,attributes) VALUES " + "('tap_x509_none','tap-x509-password',0,1,'')," + "('tap_x509_required','tap-x509-password',0,1,'{\"require_x509\":true}')," + "('tap_x509_false','tap-x509-password',0,1,'{\"require_x509\":false}')," + "('tap_x509_bad_type','tap-x509-password',0,1,'{\"require_x509\":\"true\"}')") && + do_query(admin.get(), "LOAD MYSQL USERS TO RUNTIME"); + ok(users_provisioned, "Provisioned dedicated frontend require_x509 users"); + + temporary_certificate_directory certificate_directory; + if (!certificate_directory.valid()) { + diag("Could not create a temporary certificate directory: %s", strerror(errno)); + } + client_tls_material trusted_client; + client_tls_material untrusted_client; + const bool trusted_client_ready = certificate_directory.valid() && + create_trusted_client_certificate(certificate_directory, ca, ca_key, trusted_client); + if (!trusted_client_ready) { + diag("Trusted client certificate fixture unavailable. This can happen when a custom CA certificate has no matching private key; trusted-certificate probes will be skipped."); + } + if (trusted_client_ready) { + ok(true, "Trusted client certificate generated and verified"); + } else if (certificate_directory.valid()) { + ok(true, "Trusted client certificate generated and verified # SKIP custom CA cannot sign the standard test client certificate"); + } else { + ok(false, "Trusted client certificate generated and verified (temporary directory unavailable)"); + } + + const bool untrusted_client_ready = certificate_directory.valid() && + create_untrusted_client_certificate(certificate_directory, ca, untrusted_client); + ok(untrusted_client_ready, "Untrusted self-signed client certificate generated"); + + ok(try_frontend_connect(cl, USER_NONE, PASSWORD, false) == 0, + "No require_x509 attribute permits plaintext authentication"); + ok(try_frontend_connect(cl, USER_NONE, PASSWORD, true) == 0, + "No require_x509 attribute permits TLS authentication without a client certificate"); + ok(try_frontend_connect(cl, USER_FALSE, PASSWORD, true) == 0, + "require_x509=false permits TLS authentication without a client certificate"); + ok(try_frontend_connect(cl, USER_REQUIRED, PASSWORD, false) == ER_ACCESS_DENIED_ERROR, + "require_x509=true rejects plaintext authentication with ER_ACCESS_DENIED_ERROR"); + ok(try_frontend_connect(cl, USER_REQUIRED, PASSWORD, true) == ER_ACCESS_DENIED_ERROR, + "require_x509=true rejects TLS authentication without a client certificate with ER_ACCESS_DENIED_ERROR"); + ok(untrusted_client_ready && + try_frontend_connect(cl, USER_REQUIRED, PASSWORD, true, &untrusted_client) == ER_ACCESS_DENIED_ERROR, + "require_x509=true rejects an untrusted client certificate with ER_ACCESS_DENIED_ERROR"); + if (trusted_client_ready) { + ok(try_frontend_connect(cl, USER_REQUIRED, PASSWORD, true, &trusted_client) == 0, + "require_x509=true accepts a trusted client certificate without a SAN"); + ok(try_frontend_connect(cl, USER_REQUIRED, WRONG_PASSWORD, true, &trusted_client) == ER_ACCESS_DENIED_ERROR, + "require_x509=true still rejects a wrong password with ER_ACCESS_DENIED_ERROR"); + ok(try_frontend_connect(cl, USER_BAD_TYPE, PASSWORD, true, &trusted_client) == ER_ACCESS_DENIED_ERROR, + "string require_x509=true fails closed with ER_ACCESS_DENIED_ERROR"); + } else { + ok(true, "require_x509 trusted certificate success # SKIP trusted certificate fixture unavailable"); + ok(true, "require_x509 trusted certificate wrong-password rejection # SKIP trusted certificate fixture unavailable"); + ok(true, "string require_x509=true fails closed # SKIP trusted certificate fixture unavailable"); + } + + const bool users_cleaned = do_query(admin.get(), "DELETE FROM mysql_users WHERE username IN (" + user_list + ")") && + do_query(admin.get(), "LOAD MYSQL USERS TO RUNTIME"); + ok(users_cleaned, "Cleanup removed dedicated frontend require_x509 users"); + + const bool passthrough_restored = saved_passthrough && + do_query(admin.get(), "SET mysql-passthrough_auth_enabled='" + original_passthrough_enabled + "'") && + do_query(admin.get(), "LOAD MYSQL VARIABLES TO RUNTIME"); + ok(passthrough_restored, "Cleanup restored mysql-passthrough_auth_enabled"); + + return exit_status(); +} From 135a80ddfe5e1ce126960651dcf6b20125372878 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Mon, 10 Aug 2026 10:31:19 +0000 Subject: [PATCH 03/11] docs: gate frontend X.509 policy to PROXYSQL31 --- ...-frontend-x509-proxysql31-gating-design.md | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 docs/superpowers/specs/2026-08-10-frontend-x509-proxysql31-gating-design.md diff --git a/docs/superpowers/specs/2026-08-10-frontend-x509-proxysql31-gating-design.md b/docs/superpowers/specs/2026-08-10-frontend-x509-proxysql31-gating-design.md new file mode 100644 index 0000000000..deae6d9f4b --- /dev/null +++ b/docs/superpowers/specs/2026-08-10-frontend-x509-proxysql31-gating-design.md @@ -0,0 +1,84 @@ +# Frontend X.509 Authentication: PROXYSQL31 Gating Design + +**Status:** Approved design addendum + +**Applies to:** `mysql_users.attributes.require_x509`, its certificate evidence, and the related initial-login, `COM_CHANGE_USER`, and pass-through authentication policy + +## Goal + +Treat per-user frontend X.509 authentication as a new Innovative-tier feature. It is available only in builds that define `PROXYSQL31`; `PROXYSQL40` inherits it because the build hierarchy already makes `PROXYSQL40=1` imply `PROXYSQL31=1`. + +A stable v3.0.x build has no knowledge of `require_x509`. It does not look up, parse, validate, log, or enforce that key. If the key is present in the attributes JSON, v3.0.x continues applying only the attributes it already recognizes. + +## Tier Contract + +### Stable tier: v3.0.x + +- Do not define or populate the new generic client-certificate evidence fields. +- Do not compile or call the `require_x509` policy evaluator. +- Do not inspect the `require_x509` key. +- Preserve the existing SPIFFE initial-authentication and `COM_CHANGE_USER` behavior. +- Preserve the existing stable-tier pass-through gate. +- A frontend user carrying `{"require_x509":true}` still follows ordinary v3.0.x authentication because the key is unknown to that tier. + +### Innovative and later tiers: v3.1.x and v4.x + +- Capture certificate presence and `SSL_get_verify_result()` once when the physical frontend TLS handshake completes. +- Enforce `require_x509` during initial login, `COM_CHANGE_USER`, and row-backed pass-through authentication. +- Require the existing password or authentication-plugin check in addition to a trusted client certificate. +- Apply the agreed SPIFFE session-origin restrictions to `COM_CHANGE_USER` without TLS renegotiation. +- Reject pass-through targets during `COM_CHANGE_USER`, preserving the existing Phase 1 contract. +- Never forward the frontend client certificate to a backend. + +## Compile-Time Boundaries + +Use fine-grained `#ifdef PROXYSQL31` boundaries around all new feature state and behavior: + +- `MySQL_Data_Stream` certificate-presence and verification fields; +- the flag recording that the frontend session authenticated via SPIFFE; +- initialization and TLS-handshake population of those fields; +- the shared frontend certificate-policy types and evaluator; +- `require_x509` handling in initial authentication; +- new X.509/SPIFFE restrictions in `COM_CHANGE_USER`; and +- row-backed pass-through integration. + +The stable `#else` path retains the pre-feature SPIFFE code. The gate must prevent a stable build from merely compiling the evaluator and short-circuiting it at runtime; the key is not a recognized feature in that tier. + +## Cross-Tier Hardening + +Two corrections apply unconditionally because they harden existing SPIFFE handling rather than expose `require_x509`: + +- Null-check the `GENERAL_NAMES*` returned by `X509_get_ext_d2i()` before iterating it. A certificate without a SAN must not crash any tier. +- Make existing `spiffe_id` parsing, including the DEBUG helper, type-safe and exception-safe. Malformed values must not terminate the process. + +Existing SPIFFE identity matching and the earlier TLS-handshake failure for an invalid certificate carrying a SPIFFE URI SAN remain unchanged. + +## v3.1+ Policy and Errors + +In a `PROXYSQL31` build: + +- `require_x509` must be a JSON boolean. +- `true` requires TLS, a presented peer certificate, and `X509_V_OK` on the current physical connection. +- `false` adds no certificate requirement. +- Password or authentication-plugin verification remains mandatory. +- Invalid JSON, a non-boolean `require_x509`, or malformed `spiffe_id` fails closed. +- Authentication policy denials use generic MySQL error 1045; detailed configuration or certificate information is logged internally only. + +The stored TLS evidence is connection-scoped and survives `COM_RESET_CONNECTION` and `COM_CHANGE_USER`. No code attempts TLS renegotiation. + +## Tests and Verification + +- Register the feature TAP test with `@proxysql_min_version:3.1` in addition to its server groups. +- Use a clean `PROXYSQL31=1` DEBUG build to run the complete initial-login, `COM_CHANGE_USER`, SPIFFE, malformed-attribute, and pass-through matrix. +- Use a clean default v3.0 DEBUG build to run existing authentication and TLS regressions. +- Add a focused v3.0 compatibility probe showing that a user row containing `{"require_x509":true}` still authenticates normally without a client certificate because the key is not recognized. +- Clean before changing build tiers or DEBUG/release flags; the Makefiles do not reliably invalidate objects when these flags change. +- Verify `PROXYSQL40=1` through the existing implication to `PROXYSQL31`; do not add a separate X.509 gate for v4.x. + +## Non-Goals + +- No new `mysql_users` column or schema change. +- No global client-certificate requirement for unknown-user pass-through. +- No PostgreSQL frontend authentication change. +- No TLS renegotiation or backend certificate forwarding. +- No change to the per-user behavior of a stable v3.0.x build beyond the unconditional crash hardening described above. From 96165f1e45836807a1e31418a0c5daf4ddf78319 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Mon, 10 Aug 2026 13:39:39 +0000 Subject: [PATCH 04/11] docs: revise X.509 plan for PROXYSQL31 --- ...2026-08-10-frontend-x509-authentication.md | 280 +++++++++++++++--- 1 file changed, 233 insertions(+), 47 deletions(-) diff --git a/docs/superpowers/plans/2026-08-10-frontend-x509-authentication.md b/docs/superpowers/plans/2026-08-10-frontend-x509-authentication.md index 9830e9f2f8..9637effa37 100644 --- a/docs/superpowers/plans/2026-08-10-frontend-x509-authentication.md +++ b/docs/superpowers/plans/2026-08-10-frontend-x509-authentication.md @@ -2,14 +2,22 @@ > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. -**Goal:** Add an additive `mysql_users.attributes.require_x509` frontend policy, preserve SPIFFE's stronger identity semantics across `COM_CHANGE_USER`, and enforce both policies consistently before pass-through cache lookup or backend probing. +**Goal:** Add a `PROXYSQL31`-gated, additive `mysql_users.attributes.require_x509` frontend policy, preserve SPIFFE's stronger identity semantics across `COM_CHANGE_USER`, and enforce both policies consistently before pass-through cache lookup or backend probing. -**Architecture:** Capture certificate presence and OpenSSL's verification result once, when the frontend TLS handshake completes, and retain that immutable connection evidence on `MySQL_Data_Stream`. Route initial login, `COM_CHANGE_USER`, and row-backed pass-through through one certificate-policy evaluator in `MySQL_Protocol.cpp`. Keep password verification additive for `require_x509`, keep SPIFFE identity-exclusive, reject SPIFFE and pass-through targets during `COM_CHANGE_USER`, and never attempt TLS renegotiation. +**Architecture:** Under `PROXYSQL31`, capture certificate presence and OpenSSL's verification result once, when the frontend TLS handshake completes, retain that immutable connection evidence on `MySQL_Data_Stream`, and route initial login, `COM_CHANGE_USER`, and row-backed pass-through through one certificate-policy evaluator in `MySQL_Protocol.cpp`. A default v3.0 build does not define the new state or inspect `require_x509`; it retains the existing SPIFFE path. Keep password verification additive for `require_x509`, keep SPIFFE identity-exclusive, reject SPIFFE and pass-through targets during `COM_CHANGE_USER`, and never attempt TLS renegotiation. **Tech Stack:** C++17, OpenSSL, nlohmann/json, RE2, MySQL/MariaDB client libraries, ProxySQL TAP tests, GNU Make. +**Design addendum:** `docs/superpowers/specs/2026-08-10-frontend-x509-proxysql31-gating-design.md` + ## Global Constraints +- The entire `require_x509` feature is available only when `PROXYSQL31` is defined; `PROXYSQL40=1` inherits it through the existing build hierarchy. +- A stable v3.0.x build has no knowledge of `require_x509`: it does not define the new generic certificate-evidence fields and does not look up, parse, validate, log, or enforce the key. +- A stable v3.0.x build preserves the pre-feature SPIFFE initial-authentication and `COM_CHANGE_USER` behavior. +- Null-checking `GENERAL_NAMES*` and exception-safe/type-safe handling of the existing `spiffe_id` attribute are unconditional cross-tier hardening, not gated feature behavior. +- Register the feature TAP test with `@proxysql_min_version:3.1`; separately prove with a focused compatibility test that v3.0 does not recognize the key. +- Always run `make clean` before switching between default, `PROXYSQL31=1`, DEBUG, and release builds because the Makefiles do not reliably invalidate objects when feature flags change. - The new user attribute is exactly `"require_x509": true|false`; it does not add a column or change the `mysql_users` schema. - `require_x509=true` means both the existing password/auth-plugin check and a trusted frontend client certificate must succeed. - A trusted certificate means all three conditions are true on the current physical frontend connection: TLS is active, a peer certificate was presented, and `SSL_get_verify_result()` returned `X509_V_OK`. @@ -144,7 +152,7 @@ static unsigned int try_frontend_connect( Add `test_frontend_x509_auth-t` beside the authentication tests in `test/tap/groups/groups.json`, using the same broad server groups as `reg_test_3504-change_user-t`: ```json - "test_frontend_x509_auth-t" : [ "legacy-g6", "mysql84-g6", "mysql90-g1", "mysql95-g1" ], + "test_frontend_x509_auth-t" : [ "legacy-g6", "mysql84-g6", "mysql90-g1", "mysql95-g1", "@proxysql_min_version:3.1" ], ``` No `Makefile` source-list edit is needed: `test/tap/tests/Makefile:220` discovers every `*-t.cpp` through `wildcard`. @@ -152,11 +160,13 @@ static unsigned int try_frontend_connect( - [ ] **Step 5: Build and run the new test to prove the feature is absent.** ```sh + make clean + PROXYSQL31=1 make -j4 debug make -C test/tap/tests test_frontend_x509_auth-t - cd test/tap/tests && ./test_frontend_x509_auth-t + # Run test_frontend_x509_auth-t against the PROXYSQL31 isolated runtime. ``` - Expected before implementation: baseline cases pass, while at least plaintext/no-cert/untrusted `require_x509=true` cases incorrectly authenticate. Record the failing TAP assertion numbers in the commit message body. + Expected before implementation in a v3.1 build: baseline cases pass, while at least plaintext/no-cert/untrusted `require_x509=true` cases incorrectly authenticate. Record the failing TAP assertion numbers in the commit message body. A default v3.0 runtime is not valid RED evidence because that tier intentionally does not recognize the key. - [ ] **Step 6: Commit only the failing test and group registration.** @@ -177,6 +187,8 @@ static unsigned int try_frontend_connect( - Modify: `lib/MySQL_Protocol.cpp:3402` - Modify: `lib/MySQL_Protocol.cpp:92` - Modify: `include/MySQL_Protocol.h:256` +- Create: `test/tap/tests/test_frontend_x509_tier_gate-t.cpp` +- Modify: `test/tap/groups/groups.json` - Test: `test/tap/tests/test_frontend_x509_auth-t.cpp` **Interfaces:** @@ -185,13 +197,15 @@ Add immutable-for-the-connection evidence beside `x509_subject_alt_name`: ```cpp char *x509_subject_alt_name; +#ifdef PROXYSQL31 bool client_cert_present; long client_cert_verify_result; bool frontend_authenticated_via_spiffe; +#endif SSL *ssl; ``` -Define the policy types at file scope in `lib/MySQL_Protocol.cpp`: +Define the policy types at file scope in `lib/MySQL_Protocol.cpp`, entirely inside `#ifdef PROXYSQL31`: ```cpp enum class frontend_auth_context : uint8_t { @@ -215,29 +229,92 @@ static frontend_certificate_policy_result evaluate_frontend_certificate_policy( ); ``` -- [ ] **Step 1: Initialize the new data-stream fields.** +The tier-gate TAP test owns these file-local helpers: + +```cpp +static int get_proxy_version(MYSQL* admin, int& major, int& minor) { + if (mysql_query(admin, + "SELECT variable_value FROM global_variables " + "WHERE variable_name='admin-version'")) { + return EXIT_FAILURE; + } + MYSQL_RES* result = mysql_store_result(admin); + if (!result) { + return EXIT_FAILURE; + } + MYSQL_ROW row = mysql_fetch_row(result); + const int parsed = row && row[0] + ? std::sscanf(row[0], "%d.%d", &major, &minor) : 0; + mysql_free_result(result); + return parsed == 2 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +static unsigned int try_plaintext_frontend_connect( + const CommandLine& cl, + const char* username, + const char* password +); +``` + +- [ ] **Step 1: Add and establish a cross-tier compatibility regression.** + + Create `test_frontend_x509_tier_gate-t.cpp`. Connect to the admin interface and read the running build from: + + ```sql + SELECT variable_value + FROM global_variables + WHERE variable_name='admin-version' + ``` + + Parse the leading `major.minor` numbers. Provision one frontend user with a normal password and `attributes='{"require_x509":true}'`, then attempt a plaintext connection with the correct password: + + ```cpp + const bool has_feature = major > 3 || (major == 3 && minor >= 1); + const unsigned int expected = has_feature ? ER_ACCESS_DENIED_ERROR : 0; + const unsigned int actual = try_frontend_connect( + cl, "tap_x509_tier_gate", "tap-x509-tier-password", false); + ok(actual == expected, + "require_x509 is %s on ProxySQL %d.%d: expected errno=%u, got errno=%u", + has_feature ? "enforced" : "unrecognized", major, minor, expected, actual); + ``` + + The test must plan an admin-version parse assertion, the tier-dependent authentication assertion, and cleanup assertions. It deletes only its dedicated row and restores no global variables. Register it without a minimum-version tag: + + ```json + "test_frontend_x509_tier_gate-t" : [ "legacy-g6", "mysql84-g6", "mysql90-g1", "mysql95-g1" ], + ``` + + Update `test_frontend_x509_auth-t` registration to append `"@proxysql_min_version:3.1"`. + + Run it first against a clean default v3.0 DEBUG build. On the pristine Task 1 base it establishes the compatibility baseline with `actual=0`; if an in-progress evaluator is already compiled into the stable tier it fails and exposes the missing gate. After Task 2 it must pass in both tiers, with opposite expected authentication results selected from `admin-version`. + +- [ ] **Step 2: Initialize the new data-stream fields only for Innovative-tier builds.** - In `MySQL_Data_Stream::MySQL_Data_Stream()` initialize: + Keep `x509_subject_alt_name` and `ssl` unconditional. Wrap only the new state in the class definition and constructor: ```cpp x509_subject_alt_name = nullptr; +#ifdef PROXYSQL31 client_cert_present = false; client_cert_verify_result = X509_V_OK; frontend_authenticated_via_spiffe = false; +#endif ssl = nullptr; ``` `client_cert_present` is required because OpenSSL's verification result alone does not distinguish “no certificate” from a successfully verified certificate. Do not clear these fields in `MySQL_Session::reset()`; they belong to the physical connection and must survive `COM_RESET_CONNECTION` and `COM_CHANGE_USER`. -- [ ] **Step 2: Record verification state for every peer certificate, not only SPIFFE certificates.** +- [ ] **Step 3: Record generic verification state only under `PROXYSQL31`, while hardening SAN handling in every tier.** Restructure the successful branch of `MySQL_Data_Stream::do_ssl_handshake()` as follows: ```cpp if (n == 1) { X509* cert = SSL_get_peer_certificate(ssl); +#ifdef PROXYSQL31 client_cert_present = (cert != nullptr); client_cert_verify_result = cert ? SSL_get_verify_result(ssl) : X509_V_OK; +#endif if (cert) { GENERAL_NAMES* alt_names = static_cast( @@ -249,7 +326,7 @@ static frontend_certificate_policy_result evaluate_frontend_certificate_policy( X509_free(cert); } - if (x509_subject_alt_name && client_cert_verify_result != X509_V_OK) { + if (x509_subject_alt_name && SSL_get_verify_result(ssl) != X509_V_OK) { // Preserve the existing SPIFFE handshake-failure behavior. return SSLSTATUS_FAIL; } @@ -258,9 +335,9 @@ static frontend_certificate_policy_result evaluate_frontend_certificate_policy( Guard `alt_names` before calling `sk_GENERAL_NAME_num()`. The trusted no-SAN certificate in Task 1 is specifically intended to exercise this null case and prevent a regression crash. -- [ ] **Step 3: Implement strict, exception-safe attribute parsing.** +- [ ] **Step 4: Implement strict, exception-safe attribute parsing behind the tier gate.** - The evaluator must: + Compile the shared evaluator only under `#ifdef PROXYSQL31`. It must: 1. Treat null/empty attributes as allowed. 2. Catch all `nlohmann::json::exception` values and fail closed. @@ -269,7 +346,7 @@ static frontend_certificate_policy_result evaluate_frontend_certificate_policy( 5. Require `spiffe_id` to be a string and fail closed otherwise. 6. Reuse the current exact `spiffe://...` comparison and `!regex` full-match semantics with quiet RE2 options. - Make the `#ifdef DEBUG` `debug_spiffe_id()` helper follow the same `is_string()` and exception-safety rules. Otherwise a malformed `spiffe_id` can still terminate a debug build inside `PPHR_5passwordTrue()` before the common evaluator runs. + In every tier, make the `#ifdef DEBUG` `debug_spiffe_id()` helper follow the same `is_string()` and exception-safety rules. Otherwise a malformed `spiffe_id` can still terminate a debug build inside `PPHR_5passwordTrue()` before the common evaluator runs. Core `require_x509` check: @@ -297,11 +374,12 @@ static frontend_certificate_policy_result evaluate_frontend_certificate_policy( Evaluate `require_x509` and `spiffe_id` conjunctively when both are present. Do not let a successful SPIFFE match overwrite a previous `require_x509` denial. -- [ ] **Step 4: Make initial authentication use the evaluator.** +- [ ] **Step 5: Make initial authentication select the tier-appropriate path.** - Replace the SPIFFE-only block in `verify_user_attributes()` with: + Use the common evaluator only in `PROXYSQL31` builds. Preserve the existing SPIFFE-only block verbatim in the stable `#else` path: ```cpp +#ifdef PROXYSQL31 const char* attributes = (*myds)->sess->user_attributes; const auto policy = evaluate_frontend_certificate_policy( *myds, attributes, user, @@ -311,28 +389,60 @@ static frontend_certificate_policy_result evaluate_frontend_certificate_policy( return false; } (*myds)->frontend_authenticated_via_spiffe = policy.has_spiffe_id; +#else + // Existing v3.0 SPIFFE-only attribute handling. Never inspect require_x509. +#endif ``` Retain the existing `default-transaction_isolation` application after policy success. Parse the JSON once in the function or pass a parsed object through a private helper; do not reintroduce uncaught `get()` exceptions. - Remove `user_attributes_has_spiffe()` from `include/MySQL_Protocol.h` only after Task 3 moves its last call site to the common evaluator. + Task 3 will retain `user_attributes_has_spiffe()` only inside the stable `#ifndef PROXYSQL31` path, because that path must preserve the existing late SPIFFE target check. Innovative-tier code must use the common evaluator instead. -- [ ] **Step 5: Run the initial-login test and focused TLS regression.** +- [ ] **Step 6: Prove both tier behaviors and run focused TLS regressions.** ```sh + make clean + make -j4 debug + make -C test/tap/tests test_frontend_x509_tier_gate-t + INFRA_ID=x509-tier-stable TAP_GROUP=mysql84-g6 \ + test/infra/control/start-proxysql-isolated.bash + INFRA_ID=x509-tier-stable TAP_GROUP=mysql84-g6 \ + test/infra/control/ensure-infras.bash + WORKSPACE=$(pwd) INFRA_ID=x509-tier-stable TAP_GROUP=mysql84-g6 \ + TEST_PY_TAP_INCL='^test_frontend_x509_tier_gate-t$' \ + test/infra/control/run-tests-isolated.bash + + make clean + PROXYSQL31=1 make -j4 debug make -C test/tap/tests test_frontend_x509_auth-t reg_test_4556-ssl_error_queue-t test_auth_methods-t - cd test/tap/tests && ./test_frontend_x509_auth-t - cd test/tap/tests && ./reg_test_4556-ssl_error_queue-t - cd test/tap/tests && ./test_auth_methods-t + INFRA_ID=x509-tier-31 TAP_GROUP=mysql84-g6 \ + test/infra/control/start-proxysql-isolated.bash + INFRA_ID=x509-tier-31 TAP_GROUP=mysql84-g6 \ + test/infra/control/ensure-infras.bash + WORKSPACE=$(pwd) INFRA_ID=x509-tier-31 TAP_GROUP=mysql84-g6 \ + TEST_PY_TAP_INCL='^(test_frontend_x509_auth-t|test_frontend_x509_tier_gate-t)$' \ + test/infra/control/run-tests-isolated.bash + INFRA_ID=x509-tier-31 TAP_GROUP=mysql84-g2 \ + test/infra/control/ensure-infras.bash + WORKSPACE=$(pwd) INFRA_ID=x509-tier-31 TAP_GROUP=mysql84-g2 \ + TEST_PY_TAP_INCL='^reg_test_4556-ssl_error_queue-t$' \ + test/infra/control/run-tests-isolated.bash + INFRA_ID=x509-tier-31 TAP_GROUP=mysql84-g7 \ + test/infra/control/ensure-infras.bash + WORKSPACE=$(pwd) INFRA_ID=x509-tier-31 TAP_GROUP=mysql84-g7 \ + TEST_PY_TAP_INCL='^test_auth_methods-t$' \ + test/infra/control/run-tests-isolated.bash ``` - Expected: all Task 1 scenarios pass; ordinary TLS connections without a client certificate remain accepted; the SSL error queue regression remains green. + Expected: the v3.0 compatibility probe authenticates because the key is unrecognized. In the `PROXYSQL31` build, all Task 1 scenarios pass, ordinary TLS connections without a client certificate remain accepted, and the SSL error queue regression remains green. -- [ ] **Step 6: Commit the handshake evidence and common evaluator.** +- [ ] **Step 7: Commit the tier gate, handshake evidence, and common evaluator.** ```sh git add include/MySQL_Data_Stream.h include/MySQL_Protocol.h \ - lib/mysql_data_stream.cpp lib/MySQL_Protocol.cpp + lib/mysql_data_stream.cpp lib/MySQL_Protocol.cpp \ + test/tap/tests/test_frontend_x509_tier_gate-t.cpp \ + test/tap/groups/groups.json git commit -m "feat: enforce per-user frontend X.509 policy" ``` @@ -403,7 +513,7 @@ static unsigned int try_change_user( - [ ] **Step 3: Reject a SPIFFE-authenticated source before target lookup side effects.** - At the start of `process_pkt_COM_CHANGE_USER()`, after safe packet parsing but before account state is copied, add: + Under `#ifdef PROXYSQL31`, at the start of `process_pkt_COM_CHANGE_USER()`, after safe packet parsing but before account state is copied, add: ```cpp if ((*myds)->frontend_authenticated_via_spiffe) { @@ -419,7 +529,7 @@ static unsigned int try_change_user( - [ ] **Step 4: Evaluate the target account before session mutation or Auth Switch.** - Immediately after `GloMyAuth->lookup()` and `get_password(account_details, PRIMARY)`, but before assigning `default_hostgroup`, `transaction_persistent`, or `user_attributes`, evaluate: + Under `#ifdef PROXYSQL31`, immediately after `GloMyAuth->lookup()` and `get_password(account_details, PRIMARY)`, but before assigning `default_hostgroup`, `transaction_persistent`, or `user_attributes`, evaluate: ```cpp const auto target_policy = evaluate_frontend_certificate_policy( @@ -439,11 +549,11 @@ static unsigned int try_change_user( Keep the existing pass-through-target check directly after this policy gate. Its eligibility must use `!target_policy.has_spiffe_id`, matching Task 4's initial-login logic. -- [ ] **Step 5: Remove the late, target-attribute SPIFFE block.** +- [ ] **Step 5: Keep the stable SPIFFE block and replace it only in Innovative-tier code.** - Delete the `user_attributes_has_spiffe()` call around current `lib/MySQL_Protocol.cpp:1671` and remove the method declaration/definition. That block is too late: it runs only after password success, after target attributes overwrite the session, and not uniformly before Auth Switch. + In `PROXYSQL31` builds, delete the `user_attributes_has_spiffe()` call around current `lib/MySQL_Protocol.cpp:1671`; the new source marker and early target evaluator replace it. In stable builds, retain that pre-feature block and compile the helper declaration/definition inside `#ifndef PROXYSQL31`. Stable behavior must not gain the new source-identity rule or inspect `require_x509`. - After successful change to a non-SPIFFE account, explicitly keep: + In `PROXYSQL31` builds, after successful change to a non-SPIFFE account, explicitly keep: ```cpp (*myds)->frontend_authenticated_via_spiffe = false; @@ -454,6 +564,8 @@ static unsigned int try_change_user( - [ ] **Step 6: Run focused and existing change-user tests.** ```sh + make clean + PROXYSQL31=1 make -j4 debug make -C test/tap/tests test_frontend_x509_auth-t reg_test_3504-change_user-t \ reg_test_3504-change_user_libmariadb_helper \ reg_test_3504-change_user_libmysql_helper @@ -584,7 +696,7 @@ Keep the helper header-only so the wildcard Makefile rules need no additional li Add: ```json - "test_frontend_x509_passthrough-t" : [ "mysql84-g4", "mysql90-g4", "mysql95-g4" ], + "test_frontend_x509_passthrough-t" : [ "mysql84-g4", "mysql90-g4", "mysql95-g4", "@proxysql_min_version:3.1" ], ``` Then run: @@ -598,7 +710,7 @@ Keep the helper header-only so the wildcard Makefile rules need no additional li - [ ] **Step 7: Compute policy before pass-through eligibility and side effects.** - At the top of the pass-through block in `PPHR_verify_password()`, retain the raw row state separately from effective eligibility: + At the top of the pass-through block in `PPHR_verify_password()`, keep the stable code unchanged. Under `#ifdef PROXYSQL31`, retain the raw row state separately from effective eligibility: ```cpp const bool raw_empty_pw_case = @@ -638,6 +750,8 @@ Keep the helper header-only so the wildcard Makefile rules need no additional li - `PPHR_5passwordTrue()`, and - `PPHR_passthrough_init()`. + The `#else` branch must contain the existing empty-password pass-through classification and must not reference the evaluator, `require_x509`, or the new data-stream fields. Pass-through itself remains unarmable on v3.0 through its existing `MySQL_Threads_Handler::commit()` tier gate. + When `raw_empty_pw_case && row_policy.has_spiffe_id`, skip all pass-through-only rejection/dispatch code and continue through the legacy empty-password branch. The common `verify_user_attributes()` epilogue then performs the SPIFFE identity match. Do not accept the backend password for this case: the configured frontend empty password remains the expected password input before SPIFFE validation. - [ ] **Step 8: Keep unknown-user semantics explicit.** @@ -653,6 +767,8 @@ Keep the helper header-only so the wildcard Makefile rules need no additional li - [ ] **Step 9: Run pass-through and change-user regressions.** ```sh + make clean + PROXYSQL31=1 make -j4 debug make -C test/tap/tests \ test_frontend_x509_passthrough-t \ test_passthrough_auth_e2e-t \ @@ -699,7 +815,7 @@ Keep the helper header-only so the wildcard Makefile rules need no additional li - [ ] **Step 1: Add load-time diagnostics without turning malformed values into allow.** - In the existing JSON validation block in `MySQL_Authentication::add()`, inspect `require_x509`: + Under `#ifdef PROXYSQL31`, in the existing JSON validation block in `MySQL_Authentication::add()`, inspect `require_x509`: ```cpp const auto require_x509 = valid.find("require_x509"); @@ -711,7 +827,7 @@ Keep the helper header-only so the wildcard Makefile rules need no additional li } ``` - Preserve the original attribute in runtime so the evaluator can fail closed. Do not erase the key, coerce strings/numbers, or replace all attributes with an empty string; each of those would turn a configuration error into an unintended allow. + Preserve the original attribute in runtime so the evaluator can fail closed. Do not erase the key, coerce strings/numbers, or replace all attributes with an empty string; each of those would turn a configuration error into an unintended allow. The stable path must not call `find("require_x509")` or emit a diagnostic for that key. - [ ] **Step 2: Extend the invalid-type TAP assertion.** @@ -726,6 +842,8 @@ Keep the helper header-only so the wildcard Makefile rules need no additional li `doc/frontend_x509_authentication.md` must include: + - Availability: the feature requires a v3.1.x Innovative-tier or v4.x build; v3.0.x does not recognize the key. + - Configuration example: ```sql @@ -764,6 +882,8 @@ Keep the helper header-only so the wildcard Makefile rules need no additional li - [ ] **Step 5: Run the config test and documentation checks.** ```sh + make clean + PROXYSQL31=1 make -j4 debug make -C test/tap/tests test_frontend_x509_auth-t cd test/tap/tests && ./test_frontend_x509_auth-t rg -n "require_x509|COM_CHANGE_USER|SPIFFE|pass-through|unknown" \ @@ -794,6 +914,7 @@ Keep the helper header-only so the wildcard Makefile rules need no additional li - Review: `lib/MySQL_Authentication.cpp` - Review: `lib/MySQL_Session.cpp` - Review: `test/tap/tests/test_frontend_x509_auth-t.cpp` +- Review: `test/tap/tests/test_frontend_x509_tier_gate-t.cpp` - Review: `test/tap/tests/test_frontend_x509_passthrough-t.cpp` - Review: `test/tap/groups/groups.json` - Review: `doc/frontend_x509_authentication.md` @@ -817,12 +938,15 @@ Keep the helper header-only so the wildcard Makefile rules need no additional li - Additional-password retry runs the same policy and cannot bypass it. - `COM_CHANGE_USER` evaluates source SPIFFE state and target attributes before Auth Switch and target session-attribute mutation. - Unknown-user pass-through remains unchanged and is never mistaken for an attribute-bearing row. + - Every new evaluator call and state access is inside a `PROXYSQL31` path; the stable path never inspects `require_x509`. + - The stable `COM_CHANGE_USER` path retains its pre-feature `user_attributes_has_spiffe()` behavior. - [ ] **Step 2: Audit certificate ownership and reset behavior.** Confirm: - - `client_cert_present`, `client_cert_verify_result`, and `frontend_authenticated_via_spiffe` are initialized exactly once per `MySQL_Data_Stream`. + - Under `PROXYSQL31`, `client_cert_present`, `client_cert_verify_result`, and `frontend_authenticated_via_spiffe` are initialized exactly once per `MySQL_Data_Stream`. + - Without `PROXYSQL31`, those three fields are not present in the class definition and no stable object file references them. - No OpenSSL/X509 pointer is retained; only scalar status and the existing duplicated URI string survive the handshake. - `GENERAL_NAMES` is freed only when non-null and `X509` is freed on every certificate branch. - `MySQL_Session::reset()` does not clear physical TLS evidence. @@ -834,16 +958,55 @@ Keep the helper header-only so the wildcard Makefile rules need no additional li rg -n "user_attributes_has_spiffe|SSL_renegotiate|SSL_verify_client_post_handshake" \ include lib src rg -n 'require_x509.*get<|spiffe_id.*get<' lib/MySQL_Protocol.cpp + rg -n -C 4 'PROXYSQL31|require_x509|client_cert_present|frontend_authenticated_via_spiffe' \ + include/MySQL_Data_Stream.h include/MySQL_Protocol.h \ + lib/mysql_data_stream.cpp lib/MySQL_Protocol.cpp lib/MySQL_Authentication.cpp ``` - Expected: the stale helper and renegotiation calls are absent. Any remaining JSON `get<>` is guarded by an `is_boolean()`/`is_string()` check and an exception boundary. + Expected: no renegotiation call exists. `user_attributes_has_spiffe()` exists only in the stable `#ifndef PROXYSQL31` branch. Any remaining JSON `get<>` is guarded by an `is_boolean()`/`is_string()` check and an exception boundary. -- [ ] **Step 4: Build ProxySQL and all focused tests from the current tree.** +- [ ] **Step 4: Clean-build and test the stable v3.0 tier.** ```sh - make -j4 + make clean + make -j4 debug + ./src/proxysql --version + make -C test/tap/tests \ + test_frontend_x509_tier_gate-t \ + test_auth_methods-t \ + reg_test_3504-change_user-t \ + reg_test_4556-ssl_error_queue-t + + INFRA_ID=x509-final-stable TAP_GROUP=mysql84-g6 \ + test/infra/control/start-proxysql-isolated.bash + INFRA_ID=x509-final-stable TAP_GROUP=mysql84-g6 \ + test/infra/control/ensure-infras.bash + WORKSPACE=$(pwd) INFRA_ID=x509-final-stable TAP_GROUP=mysql84-g6 \ + TEST_PY_TAP_INCL='^(test_frontend_x509_tier_gate-t|reg_test_3504-change_user-t)$' \ + test/infra/control/run-tests-isolated.bash + INFRA_ID=x509-final-stable TAP_GROUP=mysql84-g2 \ + test/infra/control/ensure-infras.bash + WORKSPACE=$(pwd) INFRA_ID=x509-final-stable TAP_GROUP=mysql84-g2 \ + TEST_PY_TAP_INCL='^reg_test_4556-ssl_error_queue-t$' \ + test/infra/control/run-tests-isolated.bash + INFRA_ID=x509-final-stable TAP_GROUP=mysql84-g7 \ + test/infra/control/ensure-infras.bash + WORKSPACE=$(pwd) INFRA_ID=x509-final-stable TAP_GROUP=mysql84-g7 \ + TEST_PY_TAP_INCL='^test_auth_methods-t$' \ + test/infra/control/run-tests-isolated.bash + ``` + + Expected version prefix: `3.0`. Run the compatibility, ordinary-authentication, change-user, and TLS regression binaries against the stable isolated runtime. The tier-gate test must show that a correct-password plaintext login succeeds even though the row carries `{"require_x509":true}`. The feature test is excluded from stable groups by `@proxysql_min_version:3.1`. + +- [ ] **Step 5: Clean-build the Innovative tier and run the complete focused TAP matrix.** + + ```sh + make clean + PROXYSQL31=1 make -j4 debug + ./src/proxysql --version make -C test/tap/tests \ test_frontend_x509_auth-t \ + test_frontend_x509_tier_gate-t \ test_frontend_x509_passthrough-t \ test_auth_methods-t \ reg_test_3504-change_user-t \ @@ -851,24 +1014,42 @@ Keep the helper header-only so the wildcard Makefile rules need no additional li test_passthrough_auth_e2e-t \ test_passthrough_auth_security-t \ test_passthrough_auth_unknown_user-t + + INFRA_ID=x509-final-31 TAP_GROUP=mysql84-g6 \ + test/infra/control/start-proxysql-isolated.bash + INFRA_ID=x509-final-31 TAP_GROUP=mysql84-g6 \ + test/infra/control/ensure-infras.bash + WORKSPACE=$(pwd) INFRA_ID=x509-final-31 TAP_GROUP=mysql84-g6 \ + TEST_PY_TAP_INCL='^(test_frontend_x509_auth-t|test_frontend_x509_tier_gate-t|reg_test_3504-change_user-t)$' \ + test/infra/control/run-tests-isolated.bash + INFRA_ID=x509-final-31 TAP_GROUP=mysql84-g4 \ + test/infra/control/ensure-infras.bash + WORKSPACE=$(pwd) INFRA_ID=x509-final-31 TAP_GROUP=mysql84-g4 \ + TEST_PY_TAP_INCL='^(test_frontend_x509_passthrough-t|test_passthrough_auth_e2e-t|test_passthrough_auth_security-t|test_passthrough_auth_unknown_user-t)$' \ + test/infra/control/run-tests-isolated.bash + INFRA_ID=x509-final-31 TAP_GROUP=mysql84-g2 \ + test/infra/control/ensure-infras.bash + WORKSPACE=$(pwd) INFRA_ID=x509-final-31 TAP_GROUP=mysql84-g2 \ + TEST_PY_TAP_INCL='^reg_test_4556-ssl_error_queue-t$' \ + test/infra/control/run-tests-isolated.bash + INFRA_ID=x509-final-31 TAP_GROUP=mysql84-g7 \ + test/infra/control/ensure-infras.bash + WORKSPACE=$(pwd) INFRA_ID=x509-final-31 TAP_GROUP=mysql84-g7 \ + TEST_PY_TAP_INCL='^test_auth_methods-t$' \ + test/infra/control/run-tests-isolated.bash ``` - Do not claim success from compilation alone; run the TAP binaries against the appropriate standard infrastructure groups. + Expected version prefix: `3.1`. Every TAP plan completes with zero failed assertions. The tier-gate test must now return 1045 for the same plaintext account. Record any environment-based certificate skips explicitly; CI's standard auto-generated CA must execute, not skip, the trusted-certificate and SPIFFE cases. -- [ ] **Step 5: Run the complete focused TAP matrix.** + Confirm the v4 inheritance mechanically without creating a second X.509 gate: ```sh - cd test/tap/tests && ./test_frontend_x509_auth-t - cd test/tap/tests && ./test_frontend_x509_passthrough-t - cd test/tap/tests && ./test_auth_methods-t - cd test/tap/tests && ./reg_test_3504-change_user-t - cd test/tap/tests && ./reg_test_4556-ssl_error_queue-t - cd test/tap/tests && ./test_passthrough_auth_e2e-t - cd test/tap/tests && ./test_passthrough_auth_security-t - cd test/tap/tests && ./test_passthrough_auth_unknown_user-t + make PROXYSQL40=1 \ + --eval='print-proxysql31: ; @printf "%s\n" "$(PROXYSQL31)"' \ + print-proxysql31 ``` - Expected: every TAP plan completes with zero failed assertions. Record any environment-based certificate skips explicitly; CI's standard auto-generated CA must execute, not skip, the trusted-certificate and SPIFFE cases. + Expected output: `1`. - [ ] **Step 6: Run static diff hygiene checks.** @@ -892,6 +1073,7 @@ Keep the helper header-only so the wildcard Makefile rules need no additional li lib/mysql_data_stream.cpp lib/MySQL_Protocol.cpp \ lib/MySQL_Authentication.cpp lib/MySQL_Session.cpp \ test/tap/tests/test_frontend_x509_auth-t.cpp \ + test/tap/tests/test_frontend_x509_tier_gate-t.cpp \ test/tap/tests/test_frontend_x509_passthrough-t.cpp \ test/tap/groups/groups.json \ doc/frontend_x509_authentication.md \ @@ -905,8 +1087,12 @@ Keep the helper header-only so the wildcard Makefile rules need no additional li ## Acceptance Matrix +Except for the explicit stable-tier rows, every `require_x509` result and every new SPIFFE/`COM_CHANGE_USER` restriction below applies only when `PROXYSQL31` is defined. + | Flow | Account policy | Connection evidence | Result | |---|---|---|---| +| Stable v3.0 initial login | row contains `require_x509` | any | Key is unrecognized; existing password/SPIFFE behavior | +| Stable v3.0 `COM_CHANGE_USER` | row contains `require_x509` | any | Existing pre-feature behavior; key is not inspected | | Initial login | none / `require_x509=false` | plaintext or TLS without cert | Existing password behavior | | Initial login | `require_x509=true` | plaintext | 1045 | | Initial login | `require_x509=true` | TLS, no cert | 1045 | From 55bd5f2937bc0ef531dbd37ff1bd8506a10833d6 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Mon, 10 Aug 2026 14:03:09 +0000 Subject: [PATCH 05/11] feat: enforce per-user frontend X.509 policy --- include/MySQL_Data_Stream.h | 5 + lib/MySQL_Protocol.cpp | 228 +++++++++++++++--- lib/mysql_data_stream.cpp | 58 +++-- test/tap/groups/groups.json | 3 +- .../tests/test_frontend_x509_tier_gate-t.cpp | 101 ++++++++ 5 files changed, 334 insertions(+), 61 deletions(-) create mode 100644 test/tap/tests/test_frontend_x509_tier_gate-t.cpp diff --git a/include/MySQL_Data_Stream.h b/include/MySQL_Data_Stream.h index d3e6b39b65..ac2d0a4368 100644 --- a/include/MySQL_Data_Stream.h +++ b/include/MySQL_Data_Stream.h @@ -131,6 +131,11 @@ class MySQL_Data_Stream MySQL_Session *sess; // pointer to the session using this data stream MySQL_Backend *mybe; // if this is a connection to a mysql server, this points to a backend structure char *x509_subject_alt_name; +#ifdef PROXYSQL31 + bool client_cert_present; + long client_cert_verify_result; + bool frontend_authenticated_via_spiffe; +#endif SSL *ssl; BIO *rbio_ssl; BIO *wbio_ssl; diff --git a/lib/MySQL_Protocol.cpp b/lib/MySQL_Protocol.cpp index 8b3b989f5d..a4d332754c 100644 --- a/lib/MySQL_Protocol.cpp +++ b/lib/MySQL_Protocol.cpp @@ -58,6 +58,109 @@ static const char *plugins[3] = { "caching_sha2_password", }; +#ifdef PROXYSQL31 +enum class frontend_auth_context : uint8_t { + INITIAL_HANDSHAKE, + COM_CHANGE_USER, + PASSTHROUGH +}; + +struct frontend_certificate_policy_result { + bool allowed { true }; + bool has_spiffe_id { false }; +}; + +static frontend_certificate_policy_result evaluate_frontend_certificate_policy( + MySQL_Data_Stream* myds, + const json& attrs, + const unsigned char* user, + frontend_auth_context context, + int calling_line, + const char* calling_func +) { + frontend_certificate_policy_result result; + const char* username = user ? reinterpret_cast(user) : "unknown"; + if (!attrs.is_object()) { + proxy_error("%d:%s(): Invalid user attributes for user %s\n", calling_line, calling_func, username); + result.allowed = false; + return result; + } + const auto spiffe_id = attrs.find("spiffe_id"); + result.has_spiffe_id = spiffe_id != attrs.end(); + + const auto require_x509 = attrs.find("require_x509"); + if (require_x509 != attrs.end()) { + if (!require_x509->is_boolean()) { + proxy_error("%d:%s(): Invalid require_x509 type for user %s\n", calling_line, calling_func, username); + result.allowed = false; + return result; + } + if (require_x509->get()) { + result.allowed = myds + && myds->encrypted + && myds->ssl + && myds->client_cert_present + && myds->client_cert_verify_result == X509_V_OK; + if (!result.allowed) { + proxy_error("%d:%s(): Frontend X509 authentication error for user %s: context=%u cert_present=%s verify_result=%ld\n", + calling_line, calling_func, username, static_cast(context), + (myds && myds->client_cert_present) ? "yes" : "no", + myds ? myds->client_cert_verify_result : X509_V_ERR_UNSPECIFIED); + return result; + } + } + } + + if (spiffe_id == attrs.end()) return result; + if (!spiffe_id->is_string()) { + proxy_error("%d:%s(): Invalid spiffe_id type for user %s\n", calling_line, calling_func, username); + result.allowed = false; + return result; + } + + result.allowed = false; + const std::string spiffe_val = spiffe_id->get(); + if (myds && myds->x509_subject_alt_name) { + if (spiffe_val.rfind("!", 0) == 0 && spiffe_val.size() > 1) { + string str_spiffe_regex { spiffe_val.substr(1) }; + re2::RE2::Options opts = re2::RE2::Options(RE2::Quiet); + re2::RE2 subject_alt_regex(str_spiffe_regex, opts); + result.allowed = re2::RE2::FullMatch(myds->x509_subject_alt_name, subject_alt_regex); + } else if (strncmp(spiffe_val.c_str(), "spiffe://", strlen("spiffe://")) == 0) { + result.allowed = strcmp(spiffe_val.c_str(), myds->x509_subject_alt_name) == 0; + } + } + if (!result.allowed) { + proxy_error("%d:%s(): SPIFFE Authentication error for user %s . spiffed_id expected : %s , received: %s\n", + calling_line, calling_func, username, spiffe_val.c_str(), + (myds && myds->x509_subject_alt_name) ? myds->x509_subject_alt_name : "none"); + } + return result; +} + +static frontend_certificate_policy_result evaluate_frontend_certificate_policy( + MySQL_Data_Stream* myds, + const char* attributes, + const unsigned char* user, + frontend_auth_context context, + int calling_line, + const char* calling_func +) { + frontend_certificate_policy_result result; + if (!attributes || !*attributes) return result; + + try { + const json attrs = json::parse(attributes); + return evaluate_frontend_certificate_policy(myds, attrs, user, context, calling_line, calling_func); + } catch (const nlohmann::json::exception& e) { + proxy_error("%d:%s(): Invalid user attributes for user %s: %s\n", calling_line, calling_func, + user ? reinterpret_cast(user) : "unknown", e.what()); + result.allowed = false; + return result; + } +} +#endif + #include "MySQL_encode.h" char* get_password(account_details_t& ad, PASSWORD_TYPE::E passtype) { @@ -91,13 +194,23 @@ char* get_password(account_details_t& ad, PASSWORD_TYPE::E passtype) { #ifdef DEBUG void debug_spiffe_id(const unsigned char *user, const char *attributes, int __line, const char *__func) { if (attributes!=NULL && strlen(attributes)) { - json j = nlohmann::json::parse(attributes); - auto spiffe_id = j.find("spiffe_id"); - if (spiffe_id != j.end()) { - std::string spiffe_val = j["spiffe_id"].get(); - proxy_info("%d:%s(): Attributes for user %s: %s . Spiffe_id: %s\n" , __line, __func, user, attributes, spiffe_val.c_str()); - } else { - proxy_info("%d:%s(): Attributes for user %s: %s\n" , __line, __func, user, attributes); + try { + json j = nlohmann::json::parse(attributes); + if (!j.is_object()) { + proxy_info("%d:%s(): Invalid attributes for user %s: %s\n", __line, __func, user, attributes); + return; + } + auto spiffe_id = j.find("spiffe_id"); + if (spiffe_id != j.end() && spiffe_id->is_string()) { + std::string spiffe_val = spiffe_id->get(); + proxy_info("%d:%s(): Attributes for user %s: %s . Spiffe_id: %s\n" , __line, __func, user, attributes, spiffe_val.c_str()); + } else if (spiffe_id != j.end()) { + proxy_info("%d:%s(): Invalid spiffe_id for user %s: %s\n", __line, __func, user, attributes); + } else { + proxy_info("%d:%s(): Attributes for user %s: %s\n" , __line, __func, user, attributes); + } + } catch (const nlohmann::json::exception& e) { + proxy_info("%d:%s(): Invalid attributes for user %s: %s\n", __line, __func, user, e.what()); } } } @@ -3400,42 +3513,81 @@ bool MySQL_Protocol::process_pkt_handshake_response(unsigned char *pkt, unsigned } bool MySQL_Protocol::verify_user_attributes(int calling_line, const char *calling_func, const unsigned char *user) { +#ifdef PROXYSQL31 + const char* attributes = (*myds)->sess->user_attributes; + if (!attributes || !*attributes) { + const auto policy = evaluate_frontend_certificate_policy( + *myds, attributes, user, frontend_auth_context::INITIAL_HANDSHAKE, calling_line, calling_func); + if (!policy.allowed) return false; + (*myds)->frontend_authenticated_via_spiffe = policy.has_spiffe_id; + return true; + } + try { + const json attrs = json::parse(attributes); + const auto policy = evaluate_frontend_certificate_policy( + *myds, attrs, user, frontend_auth_context::INITIAL_HANDSHAKE, calling_line, calling_func); + if (!policy.allowed) return false; + (*myds)->frontend_authenticated_via_spiffe = policy.has_spiffe_id; + const auto default_transaction_isolation = attrs.find("default-transaction_isolation"); + if (default_transaction_isolation != attrs.end() && default_transaction_isolation->is_string()) { + const std::string value = default_transaction_isolation->get(); + mysql_variables.client_set_value((*myds)->sess, SQL_ISOLATION_LEVEL, value.c_str()); + } + } catch (const nlohmann::json::exception& e) { + proxy_error("%d:%s(): Invalid user attributes for user %s: %s\n", calling_line, calling_func, + user ? reinterpret_cast(user) : "unknown", e.what()); + return false; + } + return true; +#else bool ret = true; if ((*myds)->sess->user_attributes) { char *a = (*myds)->sess->user_attributes; // no copy, just pointer if (strlen(a)) { - json j = nlohmann::json::parse(a); - auto spiffe_id = j.find("spiffe_id"); - if (spiffe_id != j.end()) { - // at this point, we completely ignore any password specified so far - // we assume authentication failure so far - ret = false; - std::string spiffe_val = j["spiffe_id"].get(); - if ((*myds)->x509_subject_alt_name) { - if (spiffe_val.rfind("!", 0) == 0 && spiffe_val.size() > 1) { - string str_spiffe_regex { spiffe_val.substr(1) }; - re2::RE2::Options opts = re2::RE2::Options(RE2::Quiet); - re2::RE2 subject_alt_regex(str_spiffe_regex, opts); - - ret = re2::RE2::FullMatch((*myds)->x509_subject_alt_name, subject_alt_regex); - } else if (strncmp(spiffe_val.c_str(), "spiffe://", strlen("spiffe://"))==0) { - if (strcmp(spiffe_val.c_str(), (*myds)->x509_subject_alt_name)==0) { - ret = true; + try { + json j = nlohmann::json::parse(a); + if (!j.is_object()) { + proxy_error("%d:%s(): Invalid user attributes for user %s\n", calling_line, calling_func, user); + return false; + } + auto spiffe_id = j.find("spiffe_id"); + if (spiffe_id != j.end()) { + ret = false; + if (!spiffe_id->is_string()) { + proxy_error("%d:%s(): Invalid spiffe_id type for user %s\n", calling_line, calling_func, user); + return false; + } + std::string spiffe_val = spiffe_id->get(); + if ((*myds)->x509_subject_alt_name) { + if (spiffe_val.rfind("!", 0) == 0 && spiffe_val.size() > 1) { + string str_spiffe_regex { spiffe_val.substr(1) }; + re2::RE2::Options opts = re2::RE2::Options(RE2::Quiet); + re2::RE2 subject_alt_regex(str_spiffe_regex, opts); + + ret = re2::RE2::FullMatch((*myds)->x509_subject_alt_name, subject_alt_regex); + } else if (strncmp(spiffe_val.c_str(), "spiffe://", strlen("spiffe://"))==0) { + if (strcmp(spiffe_val.c_str(), (*myds)->x509_subject_alt_name)==0) { + ret = true; + } } } + if (ret == false) { + proxy_error("%d:%s(): SPIFFE Authentication error for user %s . spiffed_id expected : %s , received: %s\n", calling_line, calling_func, user, spiffe_val.c_str(), ((*myds)->x509_subject_alt_name ? (*myds)->x509_subject_alt_name : "none")); + } } - if (ret == false) { - proxy_error("%d:%s(): SPIFFE Authentication error for user %s . spiffed_id expected : %s , received: %s\n", calling_line, calling_func, user, spiffe_val.c_str(), ((*myds)->x509_subject_alt_name ? (*myds)->x509_subject_alt_name : "none")); + auto default_transaction_isolation = j.find("default-transaction_isolation"); + if (default_transaction_isolation != j.end() && default_transaction_isolation->is_string()) { + std::string default_transaction_isolation_value = default_transaction_isolation->get(); + mysql_variables.client_set_value((*myds)->sess, SQL_ISOLATION_LEVEL, default_transaction_isolation_value.c_str()); } - } - auto default_transaction_isolation = j.find("default-transaction_isolation"); - if (default_transaction_isolation != j.end()) { - std::string default_transaction_isolation_value = j["default-transaction_isolation"].get(); - mysql_variables.client_set_value((*myds)->sess, SQL_ISOLATION_LEVEL, default_transaction_isolation_value.c_str()); + } catch (const nlohmann::json::exception& e) { + proxy_error("%d:%s(): Invalid user attributes for user %s: %s\n", calling_line, calling_func, user, e.what()); + return false; } } } return ret; +#endif } bool MySQL_Protocol::user_attributes_has_spiffe(int calling_line, const char *calling_func, const unsigned char *user) { @@ -3443,10 +3595,16 @@ bool MySQL_Protocol::user_attributes_has_spiffe(int calling_line, const char *ca if ((*myds)->sess->user_attributes) { char *a = (*myds)->sess->user_attributes; // no copy, just pointer if (strlen(a)) { - json j = nlohmann::json::parse(a); - auto spiffe_id = j.find("spiffe_id"); - if (spiffe_id != j.end()) { - ret = true; + try { + json j = nlohmann::json::parse(a); + if (!j.is_object()) return false; + auto spiffe_id = j.find("spiffe_id"); + if (spiffe_id != j.end()) { + ret = true; + } + } catch (const nlohmann::json::exception& e) { + proxy_error("%d:%s(): Invalid user attributes for user %s: %s\n", calling_line, calling_func, user, e.what()); + return false; } } } diff --git a/lib/mysql_data_stream.cpp b/lib/mysql_data_stream.cpp index bd4d971fc5..dd1ed72633 100644 --- a/lib/mysql_data_stream.cpp +++ b/lib/mysql_data_stream.cpp @@ -223,32 +223,37 @@ enum sslstatus MySQL_Data_Stream::do_ssl_handshake() { int n = SSL_do_handshake(ssl); if (n == 1) { //proxy_info("SSL handshake completed\n"); - X509 *cert; - cert = SSL_get_peer_certificate(ssl); + X509 *cert = SSL_get_peer_certificate(ssl); +#ifdef PROXYSQL31 + client_cert_present = (cert != nullptr); + client_cert_verify_result = cert ? SSL_get_verify_result(ssl) : X509_V_OK; +#endif if (cert) { GENERAL_NAMES *alt_names = (stack_st_GENERAL_NAME *)X509_get_ext_d2i((X509*)cert, NID_subject_alt_name, 0, 0); - int alt_name_count = sk_GENERAL_NAME_num(alt_names); - - // Iterate all the SAN names, looking for SPIFFE identifier - for (int i = 0; i < alt_name_count; i++) { - GENERAL_NAME *san = sk_GENERAL_NAME_value(alt_names, i); - - // We only care about URI names - if (san->type == GEN_URI) { - if (san->d.uniformResourceIdentifier->data) { - const char* resource_data = - reinterpret_cast(san->d.uniformResourceIdentifier->data); - const char* spiffe_loc = strstr(resource_data, "spiffe"); - - // First name starting with 'spiffe' is considered the match. - if (spiffe_loc == resource_data) { - x509_subject_alt_name = strdup(resource_data); + if (alt_names) { + int alt_name_count = sk_GENERAL_NAME_num(alt_names); + + // Iterate all the SAN names, looking for SPIFFE identifier + for (int i = 0; i < alt_name_count; i++) { + GENERAL_NAME *san = sk_GENERAL_NAME_value(alt_names, i); + + // We only care about URI names + if (san->type == GEN_URI) { + if (san->d.uniformResourceIdentifier->data) { + const char* resource_data = + reinterpret_cast(san->d.uniformResourceIdentifier->data); + const char* spiffe_loc = strstr(resource_data, "spiffe"); + + // First name starting with 'spiffe' is considered the match. + if (spiffe_loc == resource_data) { + x509_subject_alt_name = strdup(resource_data); + } } } } - } - sk_GENERAL_NAME_pop_free(alt_names, GENERAL_NAME_free); + sk_GENERAL_NAME_pop_free(alt_names, GENERAL_NAME_free); + } X509_free(cert); } else { // we currently disable this annoying error @@ -258,12 +263,10 @@ enum sslstatus MySQL_Data_Stream::do_ssl_handshake() { } // In case the supplied certificate has a 'SAN'-'URI' identifier // starting with 'spiffe', client certificate verification is performed. - if (x509_subject_alt_name != NULL) { + if (x509_subject_alt_name != NULL && SSL_get_verify_result(ssl) != X509_V_OK) { long rc = SSL_get_verify_result(ssl); - if (rc != X509_V_OK) { - proxy_error("Disconnecting %s:%d: X509 client SSL certificate verify error: (%ld:%s)\n" , addr.addr, addr.port, rc, X509_verify_cert_error_string(rc)); - return SSLSTATUS_FAIL; - } + proxy_error("Disconnecting %s:%d: X509 client SSL certificate verify error: (%ld:%s)\n" , addr.addr, addr.port, rc, X509_verify_cert_error_string(rc)); + return SSLSTATUS_FAIL; } } status = get_sslstatus(ssl, n); @@ -347,6 +350,11 @@ MySQL_Data_Stream::MySQL_Data_Stream() { passthrough_cleartext = NULL; tmp_charset = 0; x509_subject_alt_name=NULL; +#ifdef PROXYSQL31 + client_cert_present=false; + client_cert_verify_result=X509_V_OK; + frontend_authenticated_via_spiffe=false; +#endif ssl=NULL; rbio_ssl = NULL; wbio_ssl = NULL; diff --git a/test/tap/groups/groups.json b/test/tap/groups/groups.json index 9dbdf736da..59ada1f507 100644 --- a/test/tap/groups/groups.json +++ b/test/tap/groups/groups.json @@ -329,7 +329,8 @@ "test_admin_stats-t" : [ "legacy-g7","mysql-auto_increment_delay_multiplex=0-g2","mysql-multiplexing=false-g2","mysql-query_digests=0-g2","mysql-query_digests_keep_comment=1-g2","mysql84-g7","mysql90-g2","mysql95-g2" ], "test_ansi_quotes_group_replication-t" : [ "mysql84-gr-g1","mysql90-gr-g1","mysql91-gr-g1","mysql92-gr-g1","mysql93-gr-g1","mysql95-gr-g1" ], "test_auth_methods-t" : [ "mysql-auto_increment_delay_multiplex=0-g2","mysql-multiplexing=false-g2","mysql-query_digests=0-g2","mysql-query_digests_keep_comment=1-g2","mysql84-g7","mysql90-g2","mysql95-g2" ], - "test_frontend_x509_auth-t" : [ "legacy-g6", "mysql84-g6", "mysql90-g1", "mysql95-g1" ], + "test_frontend_x509_auth-t" : [ "legacy-g6", "mysql84-g6", "mysql90-g1", "mysql95-g1", "@proxysql_min_version:3.1" ], + "test_frontend_x509_tier_gate-t" : [ "legacy-g6", "mysql84-g6", "mysql90-g1", "mysql95-g1" ], "test_auto_increment_delay_multiplex-t" : [ "legacy-g7","mysql-auto_increment_delay_multiplex=0-g2","mysql-multiplexing=false-g2","mysql-query_digests=0-g2","mysql-query_digests_keep_comment=1-g2","mysql84-g7","mysql90-g2","mysql95-g2" ], "test_backend_conn_ping-t" : [ "legacy-g7","mysql-auto_increment_delay_multiplex=0-g2","mysql-multiplexing=false-g2","mysql-query_digests=0-g2","mysql-query_digests_keep_comment=1-g2","mysql84-g7","mysql90-g2","mysql95-g2" ], "test_binlog_dump_multi_backend_crash-t" : [ "legacy-binlog-g1" ], diff --git a/test/tap/tests/test_frontend_x509_tier_gate-t.cpp b/test/tap/tests/test_frontend_x509_tier_gate-t.cpp new file mode 100644 index 0000000000..7cf076d852 --- /dev/null +++ b/test/tap/tests/test_frontend_x509_tier_gate-t.cpp @@ -0,0 +1,101 @@ +/** + * @file test_frontend_x509_tier_gate-t.cpp + * @brief Verifies that require_x509 is available only in Innovative builds. + */ + +#include +#include +#include + +#include "mysql.h" +#include "mysqld_error.h" + +#include "tap.h" +#include "command_line.h" + +static constexpr const char* USERNAME = "tap_x509_tier_gate"; +static constexpr const char* PASSWORD = "tap-x509-tier-password"; + +static bool do_query(MYSQL* mysql, const char* query) { + if (mysql_query(mysql, query) == 0) return true; + diag("Query failed: %s -- %s", query, mysql_error(mysql)); + return false; +} + +static int get_proxy_version(MYSQL* admin, int& major, int& minor) { + if (mysql_query(admin, + "SELECT variable_value FROM global_variables " + "WHERE variable_name='admin-version'")) { + return EXIT_FAILURE; + } + MYSQL_RES* result = mysql_store_result(admin); + if (!result) { + return EXIT_FAILURE; + } + MYSQL_ROW row = mysql_fetch_row(result); + const int parsed = row && row[0] + ? std::sscanf(row[0], "%d.%d", &major, &minor) : 0; + mysql_free_result(result); + return parsed == 2 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +static unsigned int try_plaintext_frontend_connect( + const CommandLine& cl, + const char* username, + const char* password +) { + MYSQL* mysql = mysql_init(NULL); + if (!mysql) return UINT_MAX; + MYSQL* connected = mysql_real_connect(mysql, cl.host, username, password, NULL, cl.port, NULL, 0); + const unsigned int result = connected ? 0 : mysql_errno(mysql); + diag("Frontend plaintext connect user='%s' -> errno=%u '%s'", username, result, + connected ? "connected" : mysql_error(mysql)); + mysql_close(mysql); + return result; +} + +int main() { + CommandLine cl; + if (cl.getEnv()) { + diag("Failed to get the required TAP connection environmental variables."); + return EXIT_FAILURE; + } + + plan(5); + MYSQL* admin = mysql_init(NULL); + const bool admin_connected = admin && mysql_real_connect(admin, cl.host, cl.admin_username, + cl.admin_password, NULL, cl.admin_port, NULL, 0); + ok(admin_connected, "Connected to ProxySQL Admin at %s:%d", cl.host, cl.admin_port); + if (!admin_connected) { + if (admin) mysql_close(admin); + return exit_status(); + } + + int major = 0; + int minor = 0; + const bool version_read = get_proxy_version(admin, major, minor) == EXIT_SUCCESS; + ok(version_read, "Read ProxySQL admin-version as %d.%d", major, minor); + + const bool user_provisioned = do_query(admin, + "DELETE FROM mysql_users WHERE username='tap_x509_tier_gate'") && + do_query(admin, + "INSERT INTO mysql_users(username,password,default_hostgroup,active,attributes) VALUES " + "('tap_x509_tier_gate','tap-x509-tier-password',0,1,'{\"require_x509\":true}')") && + do_query(admin, "LOAD MYSQL USERS TO RUNTIME"); + ok(user_provisioned, "Provisioned the dedicated require_x509 tier-gate user"); + + const bool has_feature = major > 3 || (major == 3 && minor >= 1); + const unsigned int expected = has_feature ? ER_ACCESS_DENIED_ERROR : 0; + const unsigned int actual = user_provisioned + ? try_plaintext_frontend_connect(cl, USERNAME, PASSWORD) : UINT_MAX; + ok(actual == expected, + "require_x509 is %s on ProxySQL %d.%d: expected errno=%u, got errno=%u", + has_feature ? "enforced" : "unrecognized", major, minor, expected, actual); + + const bool users_cleaned = do_query(admin, + "DELETE FROM mysql_users WHERE username='tap_x509_tier_gate'") && + do_query(admin, "LOAD MYSQL USERS TO RUNTIME"); + ok(users_cleaned, "Cleanup removed the dedicated require_x509 tier-gate user"); + mysql_close(admin); + return exit_status(); +} From fb6d889bdfc7e45e959ddf700d180d8bf7e0a0b9 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Mon, 10 Aug 2026 14:32:46 +0000 Subject: [PATCH 06/11] fix: preserve certificate identity across change user --- include/MySQL_Protocol.h | 2 + lib/MySQL_Protocol.cpp | 43 +++++ test/tap/tests/test_frontend_x509_auth-t.cpp | 169 ++++++++++++++++++- 3 files changed, 205 insertions(+), 9 deletions(-) diff --git a/include/MySQL_Protocol.h b/include/MySQL_Protocol.h index c6b2e3e2f6..5c9a93b314 100644 --- a/include/MySQL_Protocol.h +++ b/include/MySQL_Protocol.h @@ -254,6 +254,8 @@ class MySQL_Protocol { bool generate_COM_QUERY_from_COM_FIELD_LIST(PtrSize_t *pkt); bool verify_user_attributes(int calling_line, const char *calling_func, const unsigned char *user); +#ifndef PROXYSQL31 bool user_attributes_has_spiffe(int calling_line, const char *calling_func, const unsigned char *user); +#endif }; #endif /* PROXYSQL_MYSQL_PROTOCOL_H */ diff --git a/lib/MySQL_Protocol.cpp b/lib/MySQL_Protocol.cpp index a4d332754c..b4f6630bbd 100644 --- a/lib/MySQL_Protocol.cpp +++ b/lib/MySQL_Protocol.cpp @@ -112,6 +112,12 @@ static frontend_certificate_policy_result evaluate_frontend_certificate_policy( } if (spiffe_id == attrs.end()) return result; + if (context == frontend_auth_context::COM_CHANGE_USER) { + proxy_error("%d:%s(): COM_CHANGE_USER target %s has a SPIFFE identity\n", + calling_line, calling_func, username); + result.allowed = false; + return result; + } if (!spiffe_id->is_string()) { proxy_error("%d:%s(): Invalid spiffe_id type for user %s\n", calling_line, calling_func, username); result.allowed = false; @@ -1590,6 +1596,16 @@ bool MySQL_Protocol::process_pkt_COM_CHANGE_USER(unsigned char *pkt, unsigned in } } +#ifdef PROXYSQL31 + if ((*myds)->frontend_authenticated_via_spiffe) { + proxy_error( + "Client %s:%d cannot run COM_CHANGE_USER after SPIFFE authentication\n", + (*myds)->addr.addr, (*myds)->addr.port); + free(pass); + return false; + } +#endif + account_details_t account_details {}; dup_account_details_t dup_details { false, true, true }; enum proxysql_session_type session_type = (*myds)->sess->session_type; @@ -1663,10 +1679,28 @@ bool MySQL_Protocol::process_pkt_COM_CHANGE_USER(unsigned char *pkt, unsigned in */ char* password = get_password(account_details, PASSWORD_TYPE::PRIMARY); +#ifdef PROXYSQL31 + const auto target_policy = evaluate_frontend_certificate_policy( + *myds, + account_details.attributes, + user, + frontend_auth_context::COM_CHANGE_USER, + __LINE__, __func__); + if (!target_policy.allowed || target_policy.has_spiffe_id) { + if (pass) { free(pass); pass = NULL; } + if (password) { free(password); password = NULL; } + free_account_details(account_details); + return false; + } +#endif + if (mysql_thread___passthrough_auth_enabled && mysql_thread___passthrough_auth_empty_password && password != NULL && password[0] == '\0' + #ifdef PROXYSQL31 + && !target_policy.has_spiffe_id + #endif && (session_type == PROXYSQL_SESSION_MYSQL || session_type == PROXYSQL_SESSION_SQLITE)) { // Rationale for the pre-mutation ordering and the two eligible // cases is documented on the doxygen block above this gate. @@ -1782,6 +1816,7 @@ bool MySQL_Protocol::process_pkt_COM_CHANGE_USER(unsigned char *pkt, unsigned in // set the default charset for this session (*myds)->sess->default_charset = charset; if ((*myds)->sess->user_attributes) { +#ifndef PROXYSQL31 if (user_attributes_has_spiffe(__LINE__, __func__, user)) { // if SPIFFE was used, CHANGE_USER is not allowed. // This because when SPIFFE is used, the password it is not relevant, @@ -1793,6 +1828,7 @@ bool MySQL_Protocol::process_pkt_COM_CHANGE_USER(unsigned char *pkt, unsigned in ret = false; return ret; } +#endif char* user_attributes = (*myds)->sess->user_attributes; if (strlen(user_attributes)) { @@ -1824,6 +1860,11 @@ bool MySQL_Protocol::process_pkt_COM_CHANGE_USER(unsigned char *pkt, unsigned in mysql_variables.client_set_value(sess, SQL_CHARACTER_SET_CONNECTION, ss.str().c_str()); mysql_variables.client_set_value(sess, SQL_COLLATION_CONNECTION, ss.str().c_str()); } +#ifdef PROXYSQL31 + if (ret) { + (*myds)->frontend_authenticated_via_spiffe = false; + } +#endif return ret; } @@ -3590,6 +3631,7 @@ bool MySQL_Protocol::verify_user_attributes(int calling_line, const char *callin #endif } +#ifndef PROXYSQL31 bool MySQL_Protocol::user_attributes_has_spiffe(int calling_line, const char *calling_func, const unsigned char *user) { bool ret = false; if ((*myds)->sess->user_attributes) { @@ -3610,6 +3652,7 @@ bool MySQL_Protocol::user_attributes_has_spiffe(int calling_line, const char *ca } return ret; } +#endif void * MySQL_Protocol::Query_String_to_packet(uint8_t sid, std::string *s, unsigned int *l) { mysql_hdr hdr; diff --git a/test/tap/tests/test_frontend_x509_auth-t.cpp b/test/tap/tests/test_frontend_x509_auth-t.cpp index 16b9253372..bb6a954989 100644 --- a/test/tap/tests/test_frontend_x509_auth-t.cpp +++ b/test/tap/tests/test_frontend_x509_auth-t.cpp @@ -30,8 +30,14 @@ static constexpr const char* USER_NONE = "tap_x509_none"; static constexpr const char* USER_REQUIRED = "tap_x509_required"; static constexpr const char* USER_FALSE = "tap_x509_false"; static constexpr const char* USER_BAD_TYPE = "tap_x509_bad_type"; +static constexpr const char* USER_CHANGE_SOURCE = "tap_x509_source"; +static constexpr const char* USER_CHANGE_TARGET = "tap_x509_target"; +static constexpr const char* USER_SPIFFE_SOURCE = "tap_spiffe_source"; +static constexpr const char* USER_SPIFFE_TARGET = "tap_spiffe_target"; static constexpr const char* PASSWORD = "tap-x509-password"; static constexpr const char* WRONG_PASSWORD = "tap-x509-wrong-password"; +static constexpr const char* CHANGE_SOURCE_PASSWORD = "source-password"; +static constexpr const char* CHANGE_TARGET_PASSWORD = "target-password"; struct client_tls_material { string key; @@ -89,7 +95,9 @@ class temporary_certificate_directory { if (path_.empty()) return; const char* const files[] { "trusted-client.key", "trusted-client.csr", "trusted-client.pem", - "untrusted-client.key", "untrusted-client.pem" + "untrusted-client.key", "untrusted-client.pem", + "spiffe-source.key", "spiffe-source.csr", "spiffe-source.pem", "spiffe-source.ext", + "spiffe-target.key", "spiffe-target.csr", "spiffe-target.pem", "spiffe-target.ext" }; for (const char* file : files) { const string filename { path_ + "/" + file }; @@ -144,19 +152,60 @@ static bool create_untrusted_client_certificate( ); } +static bool create_spiffe_client_certificate( + const temporary_certificate_directory& directory, const string& ca, const string& ca_key, + const char* name, const char* spiffe_id, unsigned long serial, client_tls_material& material +) { + const string prefix { directory.path() + "/" + name }; + material.key = prefix + ".key"; + const string csr { prefix + ".csr" }; + material.cert = prefix + ".pem"; + material.ca = ca; + const string extfile { prefix + ".ext" }; + + FILE* extensions = fopen(extfile.c_str(), "w"); + if (!extensions) { + diag("Could not create SPIFFE extension file %s: %s", extfile.c_str(), strerror(errno)); + return false; + } + const int written = fprintf(extensions, "subjectAltName=URI:%s\n", spiffe_id); + if (fclose(extensions) != 0 || written < 0) { + diag("Could not write SPIFFE extension file %s: %s", extfile.c_str(), strerror(errno)); + return false; + } + + const bool req_ok = run_openssl( + "openssl req -new -newkey rsa:2048 -nodes -subj /CN=" + string(name) + + " -keyout " + shell_quote(material.key) + " -out " + shell_quote(csr) + ); + const bool sign_ok = req_ok && run_openssl( + "openssl x509 -req -days 1 -set_serial " + std::to_string(serial) + + " -in " + shell_quote(csr) + " -CA " + shell_quote(ca) + + " -CAkey " + shell_quote(ca_key) + " -extfile " + shell_quote(extfile) + + " -out " + shell_quote(material.cert) + ); + return sign_ok && run_openssl( + "openssl verify -CAfile " + shell_quote(ca) + " " + shell_quote(material.cert) + ); +} + /** * Attempt one frontend connection and return 0 on success or the client error. * A null client_identity means TLS is requested without a client certificate. */ -static unsigned int try_frontend_connect( +static mysql_ptr connect_frontend( const CommandLine& cl, const char* username, const char* password, bool use_tls, - const client_tls_material* client_identity = nullptr + const client_tls_material* client_identity = nullptr, + unsigned int* connection_error = nullptr ) { mysql_ptr mysql { mysql_init(NULL) }; - if (!mysql) return UINT_MAX; + if (!mysql) { + if (connection_error) *connection_error = UINT_MAX; + return nullptr; + } unsigned long flags = 0; if (use_tls) { @@ -174,10 +223,32 @@ static unsigned int try_frontend_connect( MYSQL* connected = mysql_real_connect( mysql.get(), cl.host, username, password, nullptr, cl.port, nullptr, flags); const unsigned int result = connected ? 0 : mysql_errno(mysql.get()); + if (connection_error) *connection_error = result; diag("Frontend connect user='%s' tls=%s client_cert=%s -> errno=%u '%s'", username, use_tls ? "yes" : "no", client_identity ? "yes" : "no", result, connected ? "connected" : mysql_error(mysql.get())); - return result; + return connected ? std::move(mysql) : nullptr; +} + +static unsigned int try_frontend_connect( + const CommandLine& cl, + const char* username, + const char* password, + bool use_tls, + const client_tls_material* client_identity = nullptr +) { + unsigned int connection_error = UINT_MAX; + mysql_ptr mysql { connect_frontend(cl, username, password, use_tls, client_identity, &connection_error) }; + return mysql ? 0 : connection_error; +} + +static unsigned int try_change_user( + MYSQL* connection, + const char* target_user, + const char* target_password +) { + return mysql_change_user(connection, target_user, target_password, nullptr) == 0 + ? 0 : mysql_errno(connection); } static bool do_query(MYSQL* mysql, const string& query) { @@ -225,11 +296,12 @@ int main() { } /* - * 4 setup + 2 certificate fixtures + 9 policy probes + 2 cleanup checks. + * 4 setup + 4 certificate fixtures + 9 initial-login probes + 8 + * COM_CHANGE_USER probes + 2 cleanup checks. * A custom environment whose CA private key cannot sign our certificate * emits TAP SKIPs only for the probes that need that trusted certificate. */ - plan(17); + plan(27); mysql_ptr admin { mysql_init(NULL) }; if (!admin || !mysql_real_connect(admin.get(), cl.host, cl.admin_username, cl.admin_password, @@ -250,7 +322,8 @@ int main() { ok(passthrough_disabled, "Disabled mysql-passthrough_auth_enabled for this test"); const string user_list { - "'tap_x509_none','tap_x509_required','tap_x509_false','tap_x509_bad_type'" + "'tap_x509_none','tap_x509_required','tap_x509_false','tap_x509_bad_type'," + "'tap_x509_source','tap_x509_target','tap_spiffe_source','tap_spiffe_target'" }; const bool users_provisioned = do_query(admin.get(), "DELETE FROM mysql_users WHERE username IN (" + user_list + ")") && do_query(admin.get(), @@ -258,7 +331,11 @@ int main() { "('tap_x509_none','tap-x509-password',0,1,'')," "('tap_x509_required','tap-x509-password',0,1,'{\"require_x509\":true}')," "('tap_x509_false','tap-x509-password',0,1,'{\"require_x509\":false}')," - "('tap_x509_bad_type','tap-x509-password',0,1,'{\"require_x509\":\"true\"}')") && + "('tap_x509_bad_type','tap-x509-password',0,1,'{\"require_x509\":\"true\"}')," + "('tap_x509_source','source-password',0,1,'')," + "('tap_x509_target','target-password',0,1,'{\"require_x509\":true}')," + "('tap_spiffe_source','',0,1,'{\"spiffe_id\":\"spiffe://tap/source\"}')," + "('tap_spiffe_target','',0,1,'{\"spiffe_id\":\"spiffe://tap/target\"}')") && do_query(admin.get(), "LOAD MYSQL USERS TO RUNTIME"); ok(users_provisioned, "Provisioned dedicated frontend require_x509 users"); @@ -268,6 +345,8 @@ int main() { } client_tls_material trusted_client; client_tls_material untrusted_client; + client_tls_material spiffe_source_client; + client_tls_material spiffe_target_client; const bool trusted_client_ready = certificate_directory.valid() && create_trusted_client_certificate(certificate_directory, ca, ca_key, trusted_client); if (!trusted_client_ready) { @@ -285,6 +364,25 @@ int main() { create_untrusted_client_certificate(certificate_directory, ca, untrusted_client); ok(untrusted_client_ready, "Untrusted self-signed client certificate generated"); + const bool spiffe_source_client_ready = certificate_directory.valid() && + create_spiffe_client_certificate( + certificate_directory, ca, ca_key, "spiffe-source", "spiffe://tap/source", 5928003, + spiffe_source_client); + if (spiffe_source_client_ready) { + ok(true, "Trusted SPIFFE source client certificate generated and verified"); + } else { + ok(true, "Trusted SPIFFE source client certificate generated and verified # SKIP custom CA cannot sign the SPIFFE source certificate"); + } + const bool spiffe_target_client_ready = certificate_directory.valid() && + create_spiffe_client_certificate( + certificate_directory, ca, ca_key, "spiffe-target", "spiffe://tap/target", 5928004, + spiffe_target_client); + if (spiffe_target_client_ready) { + ok(true, "Trusted SPIFFE target client certificate generated and verified"); + } else { + ok(true, "Trusted SPIFFE target client certificate generated and verified # SKIP custom CA cannot sign the SPIFFE target certificate"); + } + ok(try_frontend_connect(cl, USER_NONE, PASSWORD, false) == 0, "No require_x509 attribute permits plaintext authentication"); ok(try_frontend_connect(cl, USER_NONE, PASSWORD, true) == 0, @@ -311,6 +409,59 @@ int main() { ok(true, "string require_x509=true fails closed # SKIP trusted certificate fixture unavailable"); } + { + mysql_ptr source { connect_frontend(cl, USER_CHANGE_SOURCE, CHANGE_SOURCE_PASSWORD, false) }; + ok(source && try_change_user(source.get(), USER_CHANGE_TARGET, CHANGE_TARGET_PASSWORD) == ER_ACCESS_DENIED_ERROR, + "COM_CHANGE_USER from plaintext rejects require_x509=true with ER_ACCESS_DENIED_ERROR"); + } + { + mysql_ptr source { connect_frontend(cl, USER_CHANGE_SOURCE, CHANGE_SOURCE_PASSWORD, true) }; + // Reconnecting with trusted_client succeeds below; CHANGE_USER cannot acquire a certificate on this TLS connection. + ok(source && try_change_user(source.get(), USER_CHANGE_TARGET, CHANGE_TARGET_PASSWORD) == ER_ACCESS_DENIED_ERROR, + "COM_CHANGE_USER from TLS without a client certificate rejects require_x509=true with ER_ACCESS_DENIED_ERROR"); + } + { + mysql_ptr source { connect_frontend(cl, USER_CHANGE_SOURCE, CHANGE_SOURCE_PASSWORD, true, &untrusted_client) }; + ok(untrusted_client_ready && source && + try_change_user(source.get(), USER_CHANGE_TARGET, CHANGE_TARGET_PASSWORD) == ER_ACCESS_DENIED_ERROR, + "COM_CHANGE_USER from an untrusted client certificate rejects require_x509=true with ER_ACCESS_DENIED_ERROR"); + } + if (trusted_client_ready) { + mysql_ptr source { connect_frontend(cl, USER_CHANGE_SOURCE, CHANGE_SOURCE_PASSWORD, true, &trusted_client) }; + ok(source && try_change_user(source.get(), USER_CHANGE_TARGET, CHANGE_TARGET_PASSWORD) == 0, + "COM_CHANGE_USER from a trusted client certificate accepts require_x509=true"); + } else { + ok(true, "COM_CHANGE_USER trusted certificate require_x509 success # SKIP trusted certificate fixture unavailable"); + } + if (trusted_client_ready) { + mysql_ptr source { connect_frontend(cl, USER_CHANGE_SOURCE, CHANGE_SOURCE_PASSWORD, true, &trusted_client) }; + ok(source && try_change_user(source.get(), USER_CHANGE_TARGET, WRONG_PASSWORD) == ER_ACCESS_DENIED_ERROR, + "COM_CHANGE_USER require_x509=true still rejects a wrong target password with ER_ACCESS_DENIED_ERROR"); + } else { + ok(true, "COM_CHANGE_USER trusted certificate wrong-password rejection # SKIP trusted certificate fixture unavailable"); + } + if (trusted_client_ready) { + mysql_ptr source { connect_frontend(cl, USER_CHANGE_SOURCE, CHANGE_SOURCE_PASSWORD, true, &trusted_client) }; + ok(source && try_change_user(source.get(), USER_NONE, PASSWORD) == 0, + "COM_CHANGE_USER from a trusted client certificate accepts an ordinary password target"); + } else { + ok(true, "COM_CHANGE_USER ordinary target control # SKIP trusted certificate fixture unavailable"); + } + if (spiffe_source_client_ready) { + mysql_ptr source { connect_frontend(cl, USER_SPIFFE_SOURCE, "", true, &spiffe_source_client) }; + ok(source && try_change_user(source.get(), USER_NONE, PASSWORD) == ER_ACCESS_DENIED_ERROR, + "COM_CHANGE_USER rejects a SPIFFE-authenticated source identity with ER_ACCESS_DENIED_ERROR"); + } else { + ok(true, "COM_CHANGE_USER SPIFFE-authenticated source rejection # SKIP trusted SPIFFE source fixture unavailable"); + } + if (spiffe_target_client_ready) { + mysql_ptr source { connect_frontend(cl, USER_CHANGE_SOURCE, CHANGE_SOURCE_PASSWORD, true, &spiffe_target_client) }; + ok(source && try_change_user(source.get(), USER_SPIFFE_TARGET, "") == ER_ACCESS_DENIED_ERROR, + "COM_CHANGE_USER rejects a SPIFFE target with ER_ACCESS_DENIED_ERROR"); + } else { + ok(true, "COM_CHANGE_USER SPIFFE target rejection # SKIP trusted SPIFFE target fixture unavailable"); + } + const bool users_cleaned = do_query(admin.get(), "DELETE FROM mysql_users WHERE username IN (" + user_list + ")") && do_query(admin.get(), "LOAD MYSQL USERS TO RUNTIME"); ok(users_cleaned, "Cleanup removed dedicated frontend require_x509 users"); From aed47297b4793982ab554f0e6c9200b7cc6953ef Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Mon, 10 Aug 2026 15:11:19 +0000 Subject: [PATCH 07/11] fix: apply X.509 policy before pass-through auth --- lib/MySQL_Protocol.cpp | 25 ++ test/tap/groups/groups.json | 1 + test/tap/tests/frontend_x509_test_utils.h | 231 ++++++++++++ test/tap/tests/test_frontend_x509_auth-t.cpp | 213 +---------- .../test_frontend_x509_passthrough-t.cpp | 348 ++++++++++++++++++ 5 files changed, 606 insertions(+), 212 deletions(-) create mode 100644 test/tap/tests/frontend_x509_test_utils.h create mode 100644 test/tap/tests/test_frontend_x509_passthrough-t.cpp diff --git a/lib/MySQL_Protocol.cpp b/lib/MySQL_Protocol.cpp index b4f6630bbd..e625125a17 100644 --- a/lib/MySQL_Protocol.cpp +++ b/lib/MySQL_Protocol.cpp @@ -2907,10 +2907,35 @@ bool MySQL_Protocol::PPHR_verify_password(MyProt_tmp_auth_vars& vars1, account_d // caching_sha2_password full-auth exchange and ultimately schedules a // backend probe via AUTHENTICATING_BACKEND_FOR_CLIENT. { + #ifdef PROXYSQL31 + const bool raw_empty_pw_case = + mysql_thread___passthrough_auth_empty_password + && vars1.password != NULL + && vars1.password[0] == '\0'; + + frontend_certificate_policy_result row_policy {}; + if (raw_empty_pw_case) { + row_policy = evaluate_frontend_certificate_policy( + *myds, account_details.attributes, vars1.user, + frontend_auth_context::PASSTHROUGH, __LINE__, __func__); + } + + const bool empty_pw_case = raw_empty_pw_case && !row_policy.has_spiffe_id; + + if (mysql_thread___passthrough_auth_enabled + && empty_pw_case + && !row_policy.allowed) { + return false; + } + #else const bool empty_pw_case = mysql_thread___passthrough_auth_empty_password && vars1.password != NULL && strlen(vars1.password) == 0; + #endif + // Unknown-user pass-through has no mysql_users row and therefore no + // per-user require_x509 attribute. Its transport gate remains + // mysql-passthrough_auth_require_tls. const bool unknown_user_case = mysql_thread___passthrough_auth_unknown_users && vars1.password == NULL; diff --git a/test/tap/groups/groups.json b/test/tap/groups/groups.json index 59ada1f507..2b32c1f30d 100644 --- a/test/tap/groups/groups.json +++ b/test/tap/groups/groups.json @@ -330,6 +330,7 @@ "test_ansi_quotes_group_replication-t" : [ "mysql84-gr-g1","mysql90-gr-g1","mysql91-gr-g1","mysql92-gr-g1","mysql93-gr-g1","mysql95-gr-g1" ], "test_auth_methods-t" : [ "mysql-auto_increment_delay_multiplex=0-g2","mysql-multiplexing=false-g2","mysql-query_digests=0-g2","mysql-query_digests_keep_comment=1-g2","mysql84-g7","mysql90-g2","mysql95-g2" ], "test_frontend_x509_auth-t" : [ "legacy-g6", "mysql84-g6", "mysql90-g1", "mysql95-g1", "@proxysql_min_version:3.1" ], + "test_frontend_x509_passthrough-t" : [ "mysql84-g4", "mysql90-g4", "mysql95-g4", "@proxysql_min_version:3.1" ], "test_frontend_x509_tier_gate-t" : [ "legacy-g6", "mysql84-g6", "mysql90-g1", "mysql95-g1" ], "test_auto_increment_delay_multiplex-t" : [ "legacy-g7","mysql-auto_increment_delay_multiplex=0-g2","mysql-multiplexing=false-g2","mysql-query_digests=0-g2","mysql-query_digests_keep_comment=1-g2","mysql84-g7","mysql90-g2","mysql95-g2" ], "test_backend_conn_ping-t" : [ "legacy-g7","mysql-auto_increment_delay_multiplex=0-g2","mysql-multiplexing=false-g2","mysql-query_digests=0-g2","mysql-query_digests_keep_comment=1-g2","mysql84-g7","mysql90-g2","mysql95-g2" ], diff --git a/test/tap/tests/frontend_x509_test_utils.h b/test/tap/tests/frontend_x509_test_utils.h new file mode 100644 index 0000000000..9e0d17e6d3 --- /dev/null +++ b/test/tap/tests/frontend_x509_test_utils.h @@ -0,0 +1,231 @@ +/** + * @file frontend_x509_test_utils.h + * @brief Header-only TLS fixture helpers for frontend X.509 TAP tests. + */ + +#ifndef __FRONTEND_X509_TEST_UTILS_H +#define __FRONTEND_X509_TEST_UTILS_H + +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +#include "mysql.h" + +#include "tap.h" +#include "command_line.h" + +using std::string; + +struct client_tls_material { + string key; + string cert; + string ca; +}; + +struct mysql_closer { + void operator()(MYSQL* mysql) const { + if (mysql) mysql_close(mysql); + } +}; + +using mysql_ptr = std::unique_ptr; + +/** Quote one shell argument, including paths derived from the environment. */ +static inline string shell_quote(const string& value) { + string quoted { "'" }; + for (const char c : value) { + if (c == '\'') { + quoted += "'\\''"; + } else { + quoted += c; + } + } + quoted += "'"; + return quoted; +} + +static inline bool run_openssl(const string& command) { + diag("Running: %s", command.c_str()); + const int status = system(command.c_str()); + if (status != 0) { + diag("openssl command failed with status %d", status); + return false; + } + return true; +} + +/** + * Own exactly the path returned by mkdtemp(). Cleanup never follows a path + * assembled from REGULAR_INFRA_DATADIR or another unchecked environment value. + */ +class temporary_certificate_directory { + string path_ {}; + +public: + temporary_certificate_directory() { + char template_path[] = "/tmp/proxysql-require-x509-XXXXXX"; + char* made = mkdtemp(template_path); + if (made) path_ = made; + } + + ~temporary_certificate_directory() { + if (path_.empty()) return; + const char* const files[] { + "trusted-client.key", "trusted-client.csr", "trusted-client.pem", + "untrusted-client.key", "untrusted-client.pem", + "spiffe-source.key", "spiffe-source.csr", "spiffe-source.pem", "spiffe-source.ext", + "spiffe-target.key", "spiffe-target.csr", "spiffe-target.pem", "spiffe-target.ext", + "spiffe-passthrough.key", "spiffe-passthrough.csr", "spiffe-passthrough.pem", "spiffe-passthrough.ext" + }; + for (const char* file : files) { + const string filename { path_ + "/" + file }; + unlink(filename.c_str()); + } + rmdir(path_.c_str()); + } + + bool valid() const { return !path_.empty(); } + const string& path() const { return path_; } +}; + +static inline bool file_is_readable(const string& path) { + if (access(path.c_str(), R_OK) == 0) return true; + diag("Required certificate fixture file is unavailable: %s: %s", path.c_str(), strerror(errno)); + return false; +} + +static inline bool create_trusted_client_certificate( + const temporary_certificate_directory& directory, + const string& ca, const string& ca_key, client_tls_material& material +) { + material.key = directory.path() + "/trusted-client.key"; + const string csr { directory.path() + "/trusted-client.csr" }; + material.cert = directory.path() + "/trusted-client.pem"; + material.ca = ca; + + const bool req_ok = run_openssl( + "openssl req -new -newkey rsa:2048 -nodes -subj /CN=tap-require-x509" + " -keyout " + shell_quote(material.key) + " -out " + shell_quote(csr) + ); + const bool sign_ok = req_ok && run_openssl( + "openssl x509 -req -days 1 -set_serial 5928001 -in " + shell_quote(csr) + + " -CA " + shell_quote(ca) + " -CAkey " + shell_quote(ca_key) + + " -out " + shell_quote(material.cert) + ); + return sign_ok && run_openssl( + "openssl verify -CAfile " + shell_quote(ca) + " " + shell_quote(material.cert) + ); +} + +static inline bool create_untrusted_client_certificate( + const temporary_certificate_directory& directory, const string& ca, client_tls_material& material +) { + material.key = directory.path() + "/untrusted-client.key"; + material.cert = directory.path() + "/untrusted-client.pem"; + material.ca = ca; + return run_openssl( + "openssl req -x509 -newkey rsa:2048 -nodes -days 1 -set_serial 5928002" + " -subj /CN=tap-untrusted -keyout " + shell_quote(material.key) + + " -out " + shell_quote(material.cert) + ); +} + +static inline bool create_spiffe_client_certificate( + const temporary_certificate_directory& directory, const string& ca, const string& ca_key, + const char* name, const char* spiffe_id, unsigned long serial, client_tls_material& material +) { + const string prefix { directory.path() + "/" + name }; + material.key = prefix + ".key"; + const string csr { prefix + ".csr" }; + material.cert = prefix + ".pem"; + material.ca = ca; + const string extfile { prefix + ".ext" }; + + FILE* extensions = fopen(extfile.c_str(), "w"); + if (!extensions) { + diag("Could not create SPIFFE extension file %s: %s", extfile.c_str(), strerror(errno)); + return false; + } + const int written = fprintf(extensions, "subjectAltName=URI:%s\n", spiffe_id); + if (fclose(extensions) != 0 || written < 0) { + diag("Could not write SPIFFE extension file %s: %s", extfile.c_str(), strerror(errno)); + return false; + } + + const bool req_ok = run_openssl( + "openssl req -new -newkey rsa:2048 -nodes -subj /CN=" + string(name) + + " -keyout " + shell_quote(material.key) + " -out " + shell_quote(csr) + ); + const bool sign_ok = req_ok && run_openssl( + "openssl x509 -req -days 1 -set_serial " + std::to_string(serial) + + " -in " + shell_quote(csr) + " -CA " + shell_quote(ca) + + " -CAkey " + shell_quote(ca_key) + " -extfile " + shell_quote(extfile) + + " -out " + shell_quote(material.cert) + ); + return sign_ok && run_openssl( + "openssl verify -CAfile " + shell_quote(ca) + " " + shell_quote(material.cert) + ); +} + +/** + * Attempt one frontend connection and return the connected handle on success. + * A null client_identity means TLS is requested without a client certificate. + */ +static inline mysql_ptr connect_frontend( + const CommandLine& cl, + const char* username, + const char* password, + bool use_tls, + const client_tls_material* client_identity = nullptr, + unsigned int* connection_error = nullptr +) { + mysql_ptr mysql { mysql_init(NULL) }; + if (!mysql) { + if (connection_error) *connection_error = UINT_MAX; + return nullptr; + } + + unsigned long flags = 0; + if (use_tls) { + if (client_identity) { + mysql_ssl_set(mysql.get(), + client_identity->key.c_str(), + client_identity->cert.c_str(), + client_identity->ca.c_str(), nullptr, nullptr); + } else { + mysql_ssl_set(mysql.get(), nullptr, nullptr, nullptr, nullptr, nullptr); + } + flags |= CLIENT_SSL; + } + + MYSQL* connected = mysql_real_connect( + mysql.get(), cl.host, username, password, nullptr, cl.port, nullptr, flags); + const unsigned int result = connected ? 0 : mysql_errno(mysql.get()); + if (connection_error) *connection_error = result; + diag("Frontend connect user='%s' tls=%s client_cert=%s -> errno=%u '%s'", + username, use_tls ? "yes" : "no", client_identity ? "yes" : "no", + result, connected ? "connected" : mysql_error(mysql.get())); + return connected ? std::move(mysql) : nullptr; +} + +static inline unsigned int try_frontend_connect( + const CommandLine& cl, + const char* username, + const char* password, + bool use_tls, + const client_tls_material* client_identity = nullptr +) { + unsigned int connection_error = UINT_MAX; + mysql_ptr mysql { connect_frontend(cl, username, password, use_tls, client_identity, &connection_error) }; + return mysql ? 0 : connection_error; +} + +#endif /* __FRONTEND_X509_TEST_UTILS_H */ diff --git a/test/tap/tests/test_frontend_x509_auth-t.cpp b/test/tap/tests/test_frontend_x509_auth-t.cpp index bb6a954989..a336d521da 100644 --- a/test/tap/tests/test_frontend_x509_auth-t.cpp +++ b/test/tap/tests/test_frontend_x509_auth-t.cpp @@ -7,22 +7,14 @@ * validation policy, so that certificate must remain acceptable. */ -#include -#include -#include -#include -#include -#include #include -#include -#include - #include "mysql.h" #include "mysqld_error.h" #include "tap.h" #include "command_line.h" +#include "frontend_x509_test_utils.h" using std::string; @@ -39,209 +31,6 @@ static constexpr const char* WRONG_PASSWORD = "tap-x509-wrong-password"; static constexpr const char* CHANGE_SOURCE_PASSWORD = "source-password"; static constexpr const char* CHANGE_TARGET_PASSWORD = "target-password"; -struct client_tls_material { - string key; - string cert; - string ca; -}; - -struct mysql_closer { - void operator()(MYSQL* mysql) const { - if (mysql) mysql_close(mysql); - } -}; - -using mysql_ptr = std::unique_ptr; - -/** Quote one shell argument, including paths derived from the environment. */ -static string shell_quote(const string& value) { - string quoted { "'" }; - for (const char c : value) { - if (c == '\'') { - quoted += "'\\''"; - } else { - quoted += c; - } - } - quoted += "'"; - return quoted; -} - -static bool run_openssl(const string& command) { - diag("Running: %s", command.c_str()); - const int status = system(command.c_str()); - if (status != 0) { - diag("openssl command failed with status %d", status); - return false; - } - return true; -} - -/** - * Own exactly the path returned by mkdtemp(). Cleanup never follows a path - * assembled from REGULAR_INFRA_DATADIR or another unchecked environment value. - */ -class temporary_certificate_directory { - string path_ {}; - -public: - temporary_certificate_directory() { - char template_path[] = "/tmp/proxysql-require-x509-XXXXXX"; - char* made = mkdtemp(template_path); - if (made) path_ = made; - } - - ~temporary_certificate_directory() { - if (path_.empty()) return; - const char* const files[] { - "trusted-client.key", "trusted-client.csr", "trusted-client.pem", - "untrusted-client.key", "untrusted-client.pem", - "spiffe-source.key", "spiffe-source.csr", "spiffe-source.pem", "spiffe-source.ext", - "spiffe-target.key", "spiffe-target.csr", "spiffe-target.pem", "spiffe-target.ext" - }; - for (const char* file : files) { - const string filename { path_ + "/" + file }; - unlink(filename.c_str()); - } - rmdir(path_.c_str()); - } - - bool valid() const { return !path_.empty(); } - const string& path() const { return path_; } -}; - -static bool file_is_readable(const string& path) { - if (access(path.c_str(), R_OK) == 0) return true; - diag("Required certificate fixture file is unavailable: %s: %s", path.c_str(), strerror(errno)); - return false; -} - -static bool create_trusted_client_certificate( - const temporary_certificate_directory& directory, - const string& ca, const string& ca_key, client_tls_material& material -) { - material.key = directory.path() + "/trusted-client.key"; - const string csr { directory.path() + "/trusted-client.csr" }; - material.cert = directory.path() + "/trusted-client.pem"; - material.ca = ca; - - const bool req_ok = run_openssl( - "openssl req -new -newkey rsa:2048 -nodes -subj /CN=tap-require-x509" - " -keyout " + shell_quote(material.key) + " -out " + shell_quote(csr) - ); - const bool sign_ok = req_ok && run_openssl( - "openssl x509 -req -days 1 -set_serial 5928001 -in " + shell_quote(csr) + - " -CA " + shell_quote(ca) + " -CAkey " + shell_quote(ca_key) + - " -out " + shell_quote(material.cert) - ); - return sign_ok && run_openssl( - "openssl verify -CAfile " + shell_quote(ca) + " " + shell_quote(material.cert) - ); -} - -static bool create_untrusted_client_certificate( - const temporary_certificate_directory& directory, const string& ca, client_tls_material& material -) { - material.key = directory.path() + "/untrusted-client.key"; - material.cert = directory.path() + "/untrusted-client.pem"; - material.ca = ca; - return run_openssl( - "openssl req -x509 -newkey rsa:2048 -nodes -days 1 -set_serial 5928002" - " -subj /CN=tap-untrusted -keyout " + shell_quote(material.key) + - " -out " + shell_quote(material.cert) - ); -} - -static bool create_spiffe_client_certificate( - const temporary_certificate_directory& directory, const string& ca, const string& ca_key, - const char* name, const char* spiffe_id, unsigned long serial, client_tls_material& material -) { - const string prefix { directory.path() + "/" + name }; - material.key = prefix + ".key"; - const string csr { prefix + ".csr" }; - material.cert = prefix + ".pem"; - material.ca = ca; - const string extfile { prefix + ".ext" }; - - FILE* extensions = fopen(extfile.c_str(), "w"); - if (!extensions) { - diag("Could not create SPIFFE extension file %s: %s", extfile.c_str(), strerror(errno)); - return false; - } - const int written = fprintf(extensions, "subjectAltName=URI:%s\n", spiffe_id); - if (fclose(extensions) != 0 || written < 0) { - diag("Could not write SPIFFE extension file %s: %s", extfile.c_str(), strerror(errno)); - return false; - } - - const bool req_ok = run_openssl( - "openssl req -new -newkey rsa:2048 -nodes -subj /CN=" + string(name) + - " -keyout " + shell_quote(material.key) + " -out " + shell_quote(csr) - ); - const bool sign_ok = req_ok && run_openssl( - "openssl x509 -req -days 1 -set_serial " + std::to_string(serial) + - " -in " + shell_quote(csr) + " -CA " + shell_quote(ca) + - " -CAkey " + shell_quote(ca_key) + " -extfile " + shell_quote(extfile) + - " -out " + shell_quote(material.cert) - ); - return sign_ok && run_openssl( - "openssl verify -CAfile " + shell_quote(ca) + " " + shell_quote(material.cert) - ); -} - -/** - * Attempt one frontend connection and return 0 on success or the client error. - * A null client_identity means TLS is requested without a client certificate. - */ -static mysql_ptr connect_frontend( - const CommandLine& cl, - const char* username, - const char* password, - bool use_tls, - const client_tls_material* client_identity = nullptr, - unsigned int* connection_error = nullptr -) { - mysql_ptr mysql { mysql_init(NULL) }; - if (!mysql) { - if (connection_error) *connection_error = UINT_MAX; - return nullptr; - } - - unsigned long flags = 0; - if (use_tls) { - if (client_identity) { - mysql_ssl_set(mysql.get(), - client_identity->key.c_str(), - client_identity->cert.c_str(), - client_identity->ca.c_str(), nullptr, nullptr); - } else { - mysql_ssl_set(mysql.get(), nullptr, nullptr, nullptr, nullptr, nullptr); - } - flags |= CLIENT_SSL; - } - - MYSQL* connected = mysql_real_connect( - mysql.get(), cl.host, username, password, nullptr, cl.port, nullptr, flags); - const unsigned int result = connected ? 0 : mysql_errno(mysql.get()); - if (connection_error) *connection_error = result; - diag("Frontend connect user='%s' tls=%s client_cert=%s -> errno=%u '%s'", - username, use_tls ? "yes" : "no", client_identity ? "yes" : "no", - result, connected ? "connected" : mysql_error(mysql.get())); - return connected ? std::move(mysql) : nullptr; -} - -static unsigned int try_frontend_connect( - const CommandLine& cl, - const char* username, - const char* password, - bool use_tls, - const client_tls_material* client_identity = nullptr -) { - unsigned int connection_error = UINT_MAX; - mysql_ptr mysql { connect_frontend(cl, username, password, use_tls, client_identity, &connection_error) }; - return mysql ? 0 : connection_error; -} - static unsigned int try_change_user( MYSQL* connection, const char* target_user, diff --git a/test/tap/tests/test_frontend_x509_passthrough-t.cpp b/test/tap/tests/test_frontend_x509_passthrough-t.cpp new file mode 100644 index 0000000000..ac6a5ca101 --- /dev/null +++ b/test/tap/tests/test_frontend_x509_passthrough-t.cpp @@ -0,0 +1,348 @@ +/** + * @file test_frontend_x509_passthrough-t.cpp + * @brief Proves row-backed require_x509 is enforced before pass-through work. + */ + +#include +#include +#include +#include +#include + +#include "mysql.h" +#include "mysqld_error.h" + +#include "tap.h" +#include "command_line.h" +#include "utils.h" +#include "frontend_x509_test_utils.h" + +using std::map; +using std::string; +using std::vector; + +static constexpr const char* PT_USER = "tap_x509_pt"; +static constexpr const char* PT_TARGET = "tap_x509_pt_target"; +static constexpr const char* SPIFFE_USER = "tap_x509_pt_spiffe"; +static constexpr const char* PT_PASSWORD = "x509-pass-through-password"; +static constexpr const char* TARGET_PASSWORD = "ordinary-target-password"; +static constexpr const char* WRONG_PASSWORD = "wrong-pass-through-password"; +static constexpr const char* SPIFFE_ID = "spiffe://tap/pass-through-exclusion"; + +static uint32_t mysql8_hg = get_env_int("TAP_MYSQL8_BACKEND_HG", 30); + +static int do_query(MYSQL* mysql, const string& query) { + if (mysql_query(mysql, query.c_str()) == 0) return EXIT_SUCCESS; + diag("Query failed: %s -- %s", query.c_str(), mysql_error(mysql)); + return EXIT_FAILURE; +} + +static bool read_global_variable(MYSQL* admin, const char* name, string& value) { + const string query { + string("SELECT variable_value FROM global_variables WHERE variable_name='") + name + "'" + }; + if (mysql_query(admin, query.c_str())) return false; + MYSQL_RES* result = mysql_store_result(admin); + if (!result) return false; + MYSQL_ROW row = mysql_fetch_row(result); + const bool found = row && row[0]; + if (found) value = row[0]; + mysql_free_result(result); + return found; +} + +static int64_t read_metric(MYSQL* admin, const char* name) { + const string query { + string("SELECT metric_value FROM stats_mysql_passthrough_auth_metrics WHERE metric_name='") + name + "'" + }; + if (mysql_query(admin, query.c_str())) return -1; + MYSQL_RES* result = mysql_store_result(admin); + if (!result) return -1; + MYSQL_ROW row = mysql_fetch_row(result); + const int64_t value = (row && row[0]) ? atoll(row[0]) : -1; + mysql_free_result(result); + return value; +} + +static int cache_entries_for(MYSQL* admin, const char* username) { + const string query { + string("SELECT COUNT(*) FROM stats_mysql_passthrough_auth_cache WHERE username='") + username + "'" + }; + if (mysql_query(admin, query.c_str())) return -1; + MYSQL_RES* result = mysql_store_result(admin); + if (!result) return -1; + MYSQL_ROW row = mysql_fetch_row(result); + const int value = (row && row[0]) ? atoi(row[0]) : -1; + mysql_free_result(result); + return value; +} + +struct server_ssl_state { + string hostname; + string port; + string use_ssl; +}; + +static vector read_server_ssl_states(MYSQL* admin) { + vector states; + const string query { + "SELECT hostname,port,use_ssl FROM mysql_servers WHERE hostgroup_id=" + std::to_string(mysql8_hg) + }; + if (mysql_query(admin, query.c_str())) return states; + MYSQL_RES* result = mysql_store_result(admin); + if (!result) return states; + while (MYSQL_ROW row = mysql_fetch_row(result)) { + if (row[0] && row[1] && row[2]) states.push_back({ row[0], row[1], row[2] }); + } + mysql_free_result(result); + return states; +} + +static string sql_quote(const string& value) { + string quoted { "'" }; + for (const char c : value) { + if (c == '\'') quoted += "''"; + else quoted += c; + } + quoted += "'"; + return quoted; +} + +static int restore_server_ssl_states(MYSQL* admin, const vector& states) { + int rc = EXIT_SUCCESS; + for (const auto& state : states) { + rc |= do_query(admin, + "UPDATE mysql_servers SET use_ssl=" + state.use_ssl + + " WHERE hostgroup_id=" + std::to_string(mysql8_hg) + + " AND hostname=" + sql_quote(state.hostname) + " AND port=" + state.port); + } + rc |= do_query(admin, "LOAD MYSQL SERVERS TO RUNTIME"); + return rc; +} + +int main() { + CommandLine cl; + const char* const datadir_env = getenv("REGULAR_INFRA_DATADIR"); + if (!datadir_env || !*datadir_env) { + diag("SKIP: REGULAR_INFRA_DATADIR is unset; run through the isolated TAP runner."); + plan(0); + return exit_status(); + } + if (cl.getEnv()) { + diag("CommandLine getEnv() failed"); + return EXIT_FAILURE; + } + + /* 10 setup + 28 behavior checks + 2 cleanup checks. */ + plan(40); + + mysql_ptr backend { mysql_init(NULL) }; + const bool backend_connected = backend && mysql_real_connect( + backend.get(), cl.mysql_host, cl.mysql_username, cl.mysql_password, nullptr, cl.mysql_port, nullptr, 0); + ok(backend_connected, "Connected to backend MySQL"); + + mysql_ptr admin { mysql_init(NULL) }; + const bool admin_connected = admin && mysql_real_connect( + admin.get(), cl.host, cl.admin_username, cl.admin_password, nullptr, cl.admin_port, nullptr, 0); + ok(admin_connected, "Connected to ProxySQL admin"); + if (!backend_connected || !admin_connected) return exit_status(); + + const char* const variable_names[] { + "mysql-passthrough_auth_enabled", + "mysql-passthrough_auth_empty_password", + "mysql-passthrough_auth_unknown_users", + "mysql-passthrough_auth_require_tls", + "mysql-passthrough_auth_username_pattern", + "mysql-passthrough_auth_max_failures_per_user", + "mysql-passthrough_auth_max_failures_per_ip", + "mysql-default_authentication_plugin" + }; + map saved_variables; + bool variables_saved = true; + for (const char* name : variable_names) { + string value; + const bool saved = read_global_variable(admin.get(), name, value); + variables_saved &= saved; + if (saved) saved_variables[name] = value; + } + ok(variables_saved, "Snapshotted every mutated pass-through/default-plugin variable"); + + const vector saved_server_ssl = read_server_ssl_states(admin.get()); + ok(!saved_server_ssl.empty(), "Snapshotted mysql_servers.use_ssl for the MySQL 8 hostgroup"); + if (!variables_saved || saved_server_ssl.empty()) { + diag("Refusing to mutate pass-through configuration without complete restoration snapshots."); + return exit_status(); + } + + int setup_rc = EXIT_SUCCESS; + setup_rc |= do_query(backend.get(), string("DROP USER IF EXISTS '") + PT_USER + "'@'%'"); + setup_rc |= do_query(backend.get(), string("CREATE USER '") + PT_USER + "'@'%' IDENTIFIED WITH 'caching_sha2_password' BY '" + PT_PASSWORD + "'"); + setup_rc |= do_query(backend.get(), string("GRANT SELECT ON *.* TO '") + PT_USER + "'@'%'"); + ok(setup_rc == EXIT_SUCCESS, "Provisioned caching_sha2_password backend pass-through user"); + + setup_rc = EXIT_SUCCESS; + setup_rc |= do_query(admin.get(), string("DELETE FROM mysql_users WHERE username IN ('") + PT_USER + "','" + PT_TARGET + "','" + SPIFFE_USER + "')"); + setup_rc |= do_query(admin.get(), string("INSERT INTO mysql_users(username,password,default_hostgroup,active,attributes) VALUES ") + + "('" + PT_USER + "',''," + std::to_string(mysql8_hg) + ",1,'{\"require_x509\":true}')," + + "('" + PT_TARGET + "','" + TARGET_PASSWORD + "'," + std::to_string(mysql8_hg) + ",1,'')," + + "('" + SPIFFE_USER + "',''," + std::to_string(mysql8_hg) + ",1,'{\"spiffe_id\":\"" + SPIFFE_ID + "\"}')"); + setup_rc |= do_query(admin.get(), "LOAD MYSQL USERS TO RUNTIME"); + ok(setup_rc == EXIT_SUCCESS, "Provisioned row-backed pass-through, ordinary target, and SPIFFE users"); + + setup_rc = EXIT_SUCCESS; + setup_rc |= do_query(admin.get(), "SET mysql-passthrough_auth_enabled='true'"); + setup_rc |= do_query(admin.get(), "SET mysql-passthrough_auth_empty_password='true'"); + setup_rc |= do_query(admin.get(), "SET mysql-passthrough_auth_unknown_users='false'"); + setup_rc |= do_query(admin.get(), "SET mysql-passthrough_auth_require_tls='true'"); + setup_rc |= do_query(admin.get(), "SET mysql-passthrough_auth_username_pattern=''"); + setup_rc |= do_query(admin.get(), "SET mysql-passthrough_auth_max_failures_per_user='10000'"); + setup_rc |= do_query(admin.get(), "SET mysql-passthrough_auth_max_failures_per_ip='10000'"); + setup_rc |= do_query(admin.get(), "SET mysql-default_authentication_plugin='caching_sha2_password'"); + setup_rc |= do_query(admin.get(), "LOAD MYSQL VARIABLES TO RUNTIME"); + setup_rc |= do_query(admin.get(), "PROXYSQL FLUSH PASSTHROUGH_AUTH_CACHE"); + setup_rc |= do_query(admin.get(), "UPDATE mysql_servers SET use_ssl=1 WHERE hostgroup_id=" + std::to_string(mysql8_hg)); + setup_rc |= do_query(admin.get(), "LOAD MYSQL SERVERS TO RUNTIME"); + ok(setup_rc == EXIT_SUCCESS, "Configured isolated pass-through window and backend TLS"); + + const string datadir { datadir_env }; + const string ca { datadir + "/proxysql-ca.pem" }; + const string ca_key { datadir + "/proxysql-key.pem" }; + temporary_certificate_directory certificate_directory; + client_tls_material trusted_client; + client_tls_material untrusted_client; + client_tls_material spiffe_client; + const bool trusted_ready = certificate_directory.valid() && file_is_readable(ca) && file_is_readable(ca_key) && + create_trusted_client_certificate(certificate_directory, ca, ca_key, trusted_client); + const bool untrusted_ready = certificate_directory.valid() && + create_untrusted_client_certificate(certificate_directory, ca, untrusted_client); + const bool spiffe_ready = certificate_directory.valid() && file_is_readable(ca) && file_is_readable(ca_key) && + create_spiffe_client_certificate(certificate_directory, ca, ca_key, "spiffe-passthrough", SPIFFE_ID, 5928005, spiffe_client); + if (trusted_ready) { + ok(true, "Generated trusted no-URI-SAN certificate"); + } else { + ok(true, "Generated trusted no-URI-SAN certificate # SKIP custom CA cannot sign fixture"); + } + ok(untrusted_ready, "Generated untrusted certificate"); + if (spiffe_ready) { + ok(true, "Generated trusted SPIFFE URI-SAN certificate"); + } else { + ok(true, "Generated trusted SPIFFE URI-SAN certificate # SKIP custom CA cannot sign fixture"); + } + + const auto expect_rejected_without_side_effects = [&](const char* label, const client_tls_material* identity) { + do_query(admin.get(), "PROXYSQL FLUSH PASSTHROUGH_AUTH_CACHE"); + const int64_t probes_before = read_metric(admin.get(), "probes_attempted"); + const int cache_before = cache_entries_for(admin.get(), PT_USER); + const unsigned int err = try_frontend_connect(cl, PT_USER, PT_PASSWORD, true, identity); + const int64_t probes_after = read_metric(admin.get(), "probes_attempted"); + const int cache_after = cache_entries_for(admin.get(), PT_USER); + ok(err == ER_ACCESS_DENIED_ERROR, "%s returns generic 1045 (errno=%u)", label, err); + ok(probes_before >= 0 && probes_after == probes_before, "%s leaves probes_attempted unchanged (%ld -> %ld)", label, probes_before, probes_after); + ok(cache_before == 0 && cache_after == 0, "%s creates no pass-through cache entry (%d -> %d)", label, cache_before, cache_after); + }; + + expect_rejected_without_side_effects("Cold TLS without client certificate", nullptr); + if (untrusted_ready) { + expect_rejected_without_side_effects("Cold TLS with untrusted client certificate", &untrusted_client); + } else { + ok(true, "Cold TLS with untrusted client certificate returns 1045 # SKIP fixture unavailable"); + ok(true, "Cold untrusted certificate leaves probes_attempted unchanged # SKIP fixture unavailable"); + ok(true, "Cold untrusted certificate creates no cache entry # SKIP fixture unavailable"); + } + + if (trusted_ready) { + do_query(admin.get(), "PROXYSQL FLUSH PASSTHROUGH_AUTH_CACHE"); + const int64_t probes_before = read_metric(admin.get(), "probes_attempted"); + const unsigned int err = try_frontend_connect(cl, PT_USER, WRONG_PASSWORD, true, &trusted_client); + const int64_t probes_after = read_metric(admin.get(), "probes_attempted"); + ok(err == ER_ACCESS_DENIED_ERROR, "Trusted certificate with wrong backend password returns 1045 (errno=%u)", err); + ok(probes_before >= 0 && probes_after == probes_before + 1, "Trusted wrong-password probe increments probes_attempted exactly once (%ld -> %ld)", probes_before, probes_after); + ok(cache_entries_for(admin.get(), PT_USER) == 0, "Trusted wrong-password probe leaves cache empty"); + + do_query(admin.get(), "PROXYSQL FLUSH PASSTHROUGH_AUTH_CACHE"); + const int64_t correct_before = read_metric(admin.get(), "probes_attempted"); + const unsigned int correct_err = try_frontend_connect(cl, PT_USER, PT_PASSWORD, true, &trusted_client); + const int64_t correct_after = read_metric(admin.get(), "probes_attempted"); + ok(correct_err == 0, "Trusted no-SAN certificate with correct backend password succeeds (errno=%u)", correct_err); + ok(correct_before >= 0 && correct_after == correct_before + 1, "Trusted correct-password probe increments probes_attempted exactly once (%ld -> %ld)", correct_before, correct_after); + ok(cache_entries_for(admin.get(), PT_USER) == 1, "Trusted correct-password probe creates one cache entry"); + + const int64_t hits_before = read_metric(admin.get(), "cache_hits"); + const unsigned int no_cert_warm_err = try_frontend_connect(cl, PT_USER, PT_PASSWORD, true); + const int64_t hits_after = read_metric(admin.get(), "cache_hits"); + ok(no_cert_warm_err == ER_ACCESS_DENIED_ERROR, "Warm TLS without a client certificate returns 1045 (errno=%u)", no_cert_warm_err); + ok(hits_before >= 0 && hits_after == hits_before, "Warm no-certificate denial leaves cache_hits unchanged (%ld -> %ld)", hits_before, hits_after); + + const int64_t trusted_hits_before = read_metric(admin.get(), "cache_hits"); + const unsigned int trusted_warm_err = try_frontend_connect(cl, PT_USER, PT_PASSWORD, true, &trusted_client); + const int64_t trusted_hits_after = read_metric(admin.get(), "cache_hits"); + ok(trusted_warm_err == 0, "Warm trusted certificate succeeds (errno=%u)", trusted_warm_err); + ok(trusted_hits_before >= 0 && trusted_hits_after == trusted_hits_before + 1, "Warm trusted certificate increments cache_hits exactly once (%ld -> %ld)", trusted_hits_before, trusted_hits_after); + } else { + for (int i = 0; i != 11; ++i) ok(true, "Trusted pass-through control # SKIP trusted certificate fixture unavailable"); + } + + if (spiffe_ready) { + const int64_t probes_before = read_metric(admin.get(), "probes_attempted"); + const unsigned int err = try_frontend_connect(cl, SPIFFE_USER, "", true, &spiffe_client); + const int64_t probes_after = read_metric(admin.get(), "probes_attempted"); + ok(err == 0, "Matching SPIFFE URI-SAN with empty password succeeds (errno=%u)", err); + ok(probes_before >= 0 && probes_after == probes_before, "Matching SPIFFE path leaves probes_attempted unchanged (%ld -> %ld)", probes_before, probes_after); + ok(cache_entries_for(admin.get(), SPIFFE_USER) == 0, "Matching SPIFFE path creates no cache entry"); + } else { + for (int i = 0; i != 3; ++i) ok(true, "Matching SPIFFE path # SKIP trusted SPIFFE fixture unavailable"); + } + + const auto expect_spiffe_rejection = [&](const char* label, const client_tls_material* identity) { + const int64_t probes_before = read_metric(admin.get(), "probes_attempted"); + const unsigned int err = try_frontend_connect(cl, SPIFFE_USER, "", true, identity); + const int64_t probes_after = read_metric(admin.get(), "probes_attempted"); + ok(err == ER_ACCESS_DENIED_ERROR, "%s returns generic 1045 (errno=%u)", label, err); + ok(probes_before >= 0 && probes_after == probes_before, "%s leaves probes_attempted unchanged (%ld -> %ld)", label, probes_before, probes_after); + ok(cache_entries_for(admin.get(), SPIFFE_USER) == 0, "%s creates no SPIFFE cache entry", label); + }; + expect_spiffe_rejection("SPIFFE row without client certificate", nullptr); + if (trusted_ready) { + expect_spiffe_rejection("SPIFFE row with mismatching trusted no-SAN certificate", &trusted_client); + } else { + for (int i = 0; i != 3; ++i) ok(true, "Mismatching SPIFFE path # SKIP trusted certificate fixture unavailable"); + } + + if (trusted_ready) { + mysql_ptr ordinary { connect_frontend(cl, cl.username, cl.password, true, &trusted_client) }; + const int64_t probes_before = read_metric(admin.get(), "probes_attempted"); + const int rc = ordinary ? mysql_change_user(ordinary.get(), PT_USER, PT_PASSWORD, nullptr) : -1; + const unsigned int err = ordinary ? mysql_errno(ordinary.get()) : UINT_MAX; + const int64_t probes_after = read_metric(admin.get(), "probes_attempted"); + ok(rc != 0 && err == ER_ACCESS_DENIED_ERROR, "COM_CHANGE_USER to pass-through target remains generic 1045 (rc=%d errno=%u)", rc, err); + ok(probes_before >= 0 && probes_after == probes_before, "COM_CHANGE_USER pass-through target does not create a probe (%ld -> %ld)", probes_before, probes_after); + + mysql_ptr pass_through { connect_frontend(cl, PT_USER, PT_PASSWORD, true, &trusted_client) }; + const int direction_rc = pass_through ? mysql_change_user(pass_through.get(), PT_TARGET, TARGET_PASSWORD, nullptr) : -1; + ok(direction_rc == 0, "Pass-through-authenticated source can COM_CHANGE_USER to ordinary target (rc=%d)", direction_rc); + } else { + for (int i = 0; i != 3; ++i) ok(true, "COM_CHANGE_USER pass-through directionality # SKIP trusted certificate fixture unavailable"); + } + + int cleanup_rc = EXIT_SUCCESS; + cleanup_rc |= do_query(backend.get(), string("DROP USER IF EXISTS '") + PT_USER + "'@'%'"); + ok(cleanup_rc == EXIT_SUCCESS, "Cleanup removed backend pass-through user"); + + cleanup_rc = EXIT_SUCCESS; + cleanup_rc |= do_query(admin.get(), string("DELETE FROM mysql_users WHERE username IN ('") + PT_USER + "','" + PT_TARGET + "','" + SPIFFE_USER + "')"); + cleanup_rc |= do_query(admin.get(), "LOAD MYSQL USERS TO RUNTIME"); + cleanup_rc |= do_query(admin.get(), "PROXYSQL FLUSH PASSTHROUGH_AUTH_CACHE"); + for (const char* name : variable_names) { + const auto it = saved_variables.find(name); + if (it != saved_variables.end()) { + cleanup_rc |= do_query(admin.get(), string("SET ") + name + "=" + sql_quote(it->second)); + } else { + cleanup_rc = EXIT_FAILURE; + } + } + cleanup_rc |= do_query(admin.get(), "LOAD MYSQL VARIABLES TO RUNTIME"); + cleanup_rc |= restore_server_ssl_states(admin.get(), saved_server_ssl); + ok(cleanup_rc == EXIT_SUCCESS, "Cleanup restored ProxySQL rows, cache, variables, and mysql_servers.use_ssl"); + + return exit_status(); +} From 17ab25f313099c38c8c850e27edb2f726f4941bf Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Mon, 10 Aug 2026 15:44:35 +0000 Subject: [PATCH 08/11] docs: define frontend X.509 authentication semantics --- doc/frontend_x509_authentication.md | 37 +++++++++++++++++++ doc/internal/passthrough_authentication.md | 20 ++++++++++ lib/MySQL_Authentication.cpp | 26 +++++++++++++ test/tap/tests/test_frontend_x509_auth-t.cpp | 39 +++++++++++++++++++- 4 files changed, 120 insertions(+), 2 deletions(-) create mode 100644 doc/frontend_x509_authentication.md diff --git a/doc/frontend_x509_authentication.md b/doc/frontend_x509_authentication.md new file mode 100644 index 0000000000..71f343c251 --- /dev/null +++ b/doc/frontend_x509_authentication.md @@ -0,0 +1,37 @@ +# Frontend X.509 authentication + +`require_x509` is available in v3.1.x Innovative-tier and v4.x builds. v3.0.x does not recognize or read the key; it does not look up, validate, log, or enforce `require_x509`. + +## Configure a frontend account + +Set the policy in the existing `mysql_users.attributes` JSON object: + +```sql +UPDATE mysql_users + SET attributes='{"require_x509":true}' + WHERE username='application_user'; +LOAD MYSQL USERS TO RUNTIME; +SAVE MYSQL USERS TO DISK; +``` + +`require_x509` accepts only a JSON boolean (`true` or `false`). A malformed value is retained in the runtime record, diagnosed when users are loaded, and denied at authentication until it is corrected. ProxySQL does not coerce malformed strings or numbers into a boolean. + +## What the policy proves + +The frontend TLS context validates client certificates against ProxySQL's frontend `proxysql-ca.pem`. With `require_x509=true`, the configured password or authentication plugin must succeed **and** the physical frontend TLS connection must have presented a certificate whose verification result is `X509_V_OK`. A trusted client certificate without a URI SAN is sufficient. + +`use_ssl` requires an encrypted transport. `require_x509` additionally requires a verified client certificate. + +`spiffe_id` binds a username to a URI SAN identity and remains the authoritative identity check after the configured frontend password step. `require_x509` proves membership in the trusted PKI and remains additive to password authentication. If both attributes are present, both policies must pass. + +## Connection changes and pass-through authentication + +`COM_CHANGE_USER` does not renegotiate TLS. The target account reuses immutable certificate evidence from the original physical connection; absent or invalid evidence rejects a `require_x509` target and requires a fresh connection. Any SPIFFE-authenticated source and every SPIFFE target are rejected, also requiring a fresh connection. + +For row-backed pass-through authentication, ProxySQL enforces this policy before the username allowlist, cache lookup, cleartext request, and backend probe. Backend verification still supplies the password verdict. SPIFFE rows are excluded from pass-through. Unknown-user pass-through has no row attribute, so it remains governed by its existing TLS transport gate. `COM_CHANGE_USER` rejects pass-through targets, while a pass-through-authenticated source may change to an ordinary password-backed row. + +The frontend client certificate is never forwarded to a backend. Backend client certificates and keys are configured independently through backend SSL settings. + +## Failures + +Authentication-policy denials return the generic MySQL error 1045; configuration and certificate details remain in ProxySQL logs. The existing earlier TLS-handshake failure for an untrusted certificate that carries a SPIFFE URI SAN is preserved. diff --git a/doc/internal/passthrough_authentication.md b/doc/internal/passthrough_authentication.md index a1c941ec0f..606e98b192 100644 --- a/doc/internal/passthrough_authentication.md +++ b/doc/internal/passthrough_authentication.md @@ -77,6 +77,10 @@ When pass-through completes for a user not in `mysql_users`, no row is inserted. Because these are re-evaluated each connect, changing `mysql-passthrough_default_hg` immediately affects routing on the next connect from a cached unknown user — no cache flush required. +### 3.6 Frontend certificate policy (v3.1+/v4 only) + +A row-backed frontend account can set `attributes.require_x509=true`. On v3.1+ and v4, that requires the configured password/authentication-plugin step and a trusted certificate on the physical frontend TLS connection. A SPIFFE row is excluded from pass-through because `spiffe_id` is an identity policy, not an empty-password pass-through signal. Unknown-user pass-through has no row or attributes object, so its existing `mysql-passthrough_auth_require_tls` transport gate is unchanged; it is not a per-user X.509 rule. + ## 4. Protocol flow ### 4.1 `caching_sha2_password` (the primary case) @@ -175,6 +179,8 @@ For Phase 1, `COM_CHANGE_USER` targeting a user that would require pass-through Implementation: `process_pkt_COM_CHANGE_USER` in `lib/MySQL_Protocol.cpp` returns early with `ret=false` when the target's stored password is empty and the master gate is on. The check runs BEFORE the function's unconditional session-state mutations (`sess->default_hostgroup`, `transaction_persistent`, `user_attributes`) so a rejected attempt has no observable side effects on the already-authenticated session. +The directions are intentionally asymmetric: a pass-through target is rejected even when the original connection carries a valid certificate, while a pass-through-authenticated source may change to an ordinary password-backed target. The SPIFFE source/target prohibition remains separate: an SPIFFE-authenticated source and every SPIFFE target are rejected. `COM_CHANGE_USER` relies on immutable certificate evidence from the original connection and never renegotiates TLS. + May be revisited in a later phase if there's demand. ## 6. Probe details @@ -248,6 +254,20 @@ The §8.4 invalidation eviction (a *later* 1045 during real query traffic agains | Stale cached password after backend rotation | TTL + invalidate-on-backend-rejection during real traffic | | Unintended exposure of unknown-user code path | `mysql-passthrough_auth_unknown_users` defaults to `false`; `username_pattern` allowlist for further restriction | +For a row-backed authentication attempt, the security ordering is: + +```text +row lookup + -> require_x509 / SPIFFE classification + -> username allowlist + -> pass-through TLS gate + -> cache lookup + -> cleartext request + -> backend probe +``` + +Cold-probe completion sends the frontend OK from `MySQL_Session::handler_again___status_AUTHENTICATING_BACKEND_FOR_CLIENT()`. Certificate policy must therefore be decided before dispatch, rather than relying only on the normal handshake epilogue. The frontend certificate is not sent to the backend, and this ordering does not change the unknown-user TLS transport gate into a per-user X.509 rule. + ### 7.2 Rate limiting Maintain two sliding-window counters: diff --git a/lib/MySQL_Authentication.cpp b/lib/MySQL_Authentication.cpp index 104e7af945..b25830dafe 100644 --- a/lib/MySQL_Authentication.cpp +++ b/lib/MySQL_Authentication.cpp @@ -14,6 +14,24 @@ #define SPOOKYV2 #endif +#ifdef PROXYSQL31 +static void validate_require_x509_attribute( + const nlohmann::json& valid, + const char* username, + enum cred_username_type usertype +) { + if (usertype == USERNAME_FRONTEND && valid.is_object()) { + const auto require_x509 = valid.find("require_x509"); + if (require_x509 != valid.end() && !require_x509->is_boolean()) { + proxy_error( + "Invalid require_x509 attribute for user %s: expected JSON boolean; " + "authentication will be denied until corrected\n", + username); + } + } +} +#endif + void free_account_details(account_details_t& ad) { if (ad.password) { free(ad.password); @@ -191,6 +209,10 @@ bool MySQL_Authentication::add(char * username, char * password, enum cred_usern // NOTE: add() is only place where we do input validation try { nlohmann::json valid=nlohmann::json::parse(attributes); + +#ifdef PROXYSQL31 + validate_require_x509_attribute(valid, username, usertype); +#endif // we do further input validation here, and possibly transforming the JSON itself bool json_rewritten = false; auto default_transaction_isolation = valid.find("default-transaction_isolation"); @@ -249,6 +271,10 @@ bool MySQL_Authentication::add(char * username, char * password, enum cred_usern // NOTE: add() is only place where we do input validation try { nlohmann::json valid=nlohmann::json::parse(attributes); + +#ifdef PROXYSQL31 + validate_require_x509_attribute(valid, username, usertype); +#endif ad->attributes=strdup(attributes); } catch(nlohmann::json::exception& e) { diff --git a/test/tap/tests/test_frontend_x509_auth-t.cpp b/test/tap/tests/test_frontend_x509_auth-t.cpp index a336d521da..9cf4b46ec8 100644 --- a/test/tap/tests/test_frontend_x509_auth-t.cpp +++ b/test/tap/tests/test_frontend_x509_auth-t.cpp @@ -7,7 +7,9 @@ * validation policy, so that certificate must remain acceptable. */ +#include #include +#include #include "mysql.h" #include "mysqld_error.h" @@ -15,7 +17,9 @@ #include "tap.h" #include "command_line.h" #include "frontend_x509_test_utils.h" +#include "utils.h" +using std::fstream; using std::string; static constexpr const char* USER_NONE = "tap_x509_none"; @@ -30,6 +34,23 @@ static constexpr const char* PASSWORD = "tap-x509-password"; static constexpr const char* WRONG_PASSWORD = "tap-x509-wrong-password"; static constexpr const char* CHANGE_SOURCE_PASSWORD = "source-password"; static constexpr const char* CHANGE_TARGET_PASSWORD = "target-password"; +static constexpr const char* BAD_TYPE_LOG_FINGERPRINT = "expected JSON boolean"; +static constexpr int MAX_LOG_CHECK_ATTEMPTS = 20; +static constexpr useconds_t LOG_CHECK_RETRY_DELAY_US = 100000; + +static bool wait_for_log_line(fstream& log, const string& username, const string& fingerprint) { + string line; + for (int attempt = 0; attempt < MAX_LOG_CHECK_ATTEMPTS; ++attempt) { + log.clear(log.rdstate() & ~std::ios_base::eofbit & ~std::ios_base::failbit); + while (getline(log, line)) { + if (line.find(username) != string::npos && line.find(fingerprint) != string::npos) { + return true; + } + } + usleep(LOG_CHECK_RETRY_DELAY_US); + } + return false; +} static unsigned int try_change_user( MYSQL* connection, @@ -85,12 +106,12 @@ int main() { } /* - * 4 setup + 4 certificate fixtures + 9 initial-login probes + 8 + * 6 setup + 4 certificate fixtures + 9 initial-login probes + 8 * COM_CHANGE_USER probes + 2 cleanup checks. * A custom environment whose CA private key cannot sign our certificate * emits TAP SKIPs only for the probes that need that trusted certificate. */ - plan(27); + plan(29); mysql_ptr admin { mysql_init(NULL) }; if (!admin || !mysql_real_connect(admin.get(), cl.host, cl.admin_username, cl.admin_password, @@ -105,6 +126,14 @@ int main() { admin.get(), "mysql-passthrough_auth_enabled", original_passthrough_enabled); ok(saved_passthrough, "Saved mysql-passthrough_auth_enabled before the test"); + const string log_path { datadir + "/proxysql.log" }; + fstream proxysql_log {}; + const int log_res = open_file_and_seek_end(log_path, proxysql_log); + ok(log_res == EXIT_SUCCESS, "Opened ProxySQL log at '%s'", log_path.c_str()); + if (log_res != EXIT_SUCCESS) { + return exit_status(); + } + const bool passthrough_disabled = saved_passthrough && do_query(admin.get(), "SET mysql-passthrough_auth_enabled='false'") && do_query(admin.get(), "LOAD MYSQL VARIABLES TO RUNTIME"); @@ -128,6 +157,12 @@ int main() { do_query(admin.get(), "LOAD MYSQL USERS TO RUNTIME"); ok(users_provisioned, "Provisioned dedicated frontend require_x509 users"); + const bool bad_type_diagnostic_seen = wait_for_log_line( + proxysql_log, USER_BAD_TYPE, BAD_TYPE_LOG_FINGERPRINT); + ok(bad_type_diagnostic_seen, + "proxysql.log contains '%s' and '%s' after LOAD MYSQL USERS TO RUNTIME", + USER_BAD_TYPE, BAD_TYPE_LOG_FINGERPRINT); + temporary_certificate_directory certificate_directory; if (!certificate_directory.valid()) { diag("Could not create a temporary certificate directory: %s", strerror(errno)); From 40585bef82887d9f97cd8783e2784c27ed7c201e Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Mon, 10 Aug 2026 17:17:20 +0000 Subject: [PATCH 09/11] fix: preserve stable frontend auth semantics --- doc/frontend_x509_authentication.md | 2 +- doc/internal/passthrough_authentication.md | 6 +++-- lib/MySQL_Protocol.cpp | 4 ---- .../tests/test_frontend_x509_tier_gate-t.cpp | 22 ++++++++++++++----- 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/doc/frontend_x509_authentication.md b/doc/frontend_x509_authentication.md index 71f343c251..726d8f6abc 100644 --- a/doc/frontend_x509_authentication.md +++ b/doc/frontend_x509_authentication.md @@ -28,7 +28,7 @@ The frontend TLS context validates client certificates against ProxySQL's fronte `COM_CHANGE_USER` does not renegotiate TLS. The target account reuses immutable certificate evidence from the original physical connection; absent or invalid evidence rejects a `require_x509` target and requires a fresh connection. Any SPIFFE-authenticated source and every SPIFFE target are rejected, also requiring a fresh connection. -For row-backed pass-through authentication, ProxySQL enforces this policy before the username allowlist, cache lookup, cleartext request, and backend probe. Backend verification still supplies the password verdict. SPIFFE rows are excluded from pass-through. Unknown-user pass-through has no row attribute, so it remains governed by its existing TLS transport gate. `COM_CHANGE_USER` rejects pass-through targets, while a pass-through-authenticated source may change to an ordinary password-backed row. +For row-backed pass-through authentication, ProxySQL enforces the per-user certificate policy before the username allowlist and cache lookup, so it applies identically on cold and warm paths. A warm cache hit is verified inline. On a cache miss, the existing global `mysql-passthrough_auth_require_tls` transport gate runs before ProxySQL requests the cleartext password or dispatches a backend probe. Backend verification still supplies the password verdict. SPIFFE rows are excluded from pass-through. Unknown-user pass-through has no row attributes and remains governed by the existing global, miss-only TLS gate. `COM_CHANGE_USER` rejects pass-through targets, while a pass-through-authenticated source may change to an ordinary password-backed row. The frontend client certificate is never forwarded to a backend. Backend client certificates and keys are configured independently through backend SSL settings. diff --git a/doc/internal/passthrough_authentication.md b/doc/internal/passthrough_authentication.md index 606e98b192..73a3f0018a 100644 --- a/doc/internal/passthrough_authentication.md +++ b/doc/internal/passthrough_authentication.md @@ -260,13 +260,15 @@ For a row-backed authentication attempt, the security ordering is: row lookup -> require_x509 / SPIFFE classification -> username allowlist - -> pass-through TLS gate -> cache lookup + -> on cache miss, pass-through TLS gate -> cleartext request -> backend probe ``` -Cold-probe completion sends the frontend OK from `MySQL_Session::handler_again___status_AUTHENTICATING_BACKEND_FOR_CLIENT()`. Certificate policy must therefore be decided before dispatch, rather than relying only on the normal handshake epilogue. The frontend certificate is not sent to the backend, and this ordering does not change the unknown-user TLS transport gate into a per-user X.509 rule. +Row-backed `require_x509` is evaluated before cache lookup, so the certificate policy applies identically on cold and warm paths. A warm cache hit is verified inline before the miss-only TLS gate. `mysql-passthrough_auth_require_tls` protects acquisition of the cleartext password and backend-probe dispatch on a cache miss; it does not reject an inline warm-cache hit. + +Cold-probe completion sends the frontend OK from `MySQL_Session::handler_again___status_AUTHENTICATING_BACKEND_FOR_CLIENT()`. Certificate policy must therefore be decided before dispatch, rather than relying only on the normal handshake epilogue. The frontend certificate is not sent to the backend. Unknown-user pass-through has no row attributes and remains subject to the same existing global, miss-only TLS gate rather than a per-user X.509 rule. ### 7.2 Rate limiting diff --git a/lib/MySQL_Protocol.cpp b/lib/MySQL_Protocol.cpp index e625125a17..70ff63491b 100644 --- a/lib/MySQL_Protocol.cpp +++ b/lib/MySQL_Protocol.cpp @@ -3612,10 +3612,6 @@ bool MySQL_Protocol::verify_user_attributes(int calling_line, const char *callin if (strlen(a)) { try { json j = nlohmann::json::parse(a); - if (!j.is_object()) { - proxy_error("%d:%s(): Invalid user attributes for user %s\n", calling_line, calling_func, user); - return false; - } auto spiffe_id = j.find("spiffe_id"); if (spiffe_id != j.end()) { ret = false; diff --git a/test/tap/tests/test_frontend_x509_tier_gate-t.cpp b/test/tap/tests/test_frontend_x509_tier_gate-t.cpp index 7cf076d852..f277baaa52 100644 --- a/test/tap/tests/test_frontend_x509_tier_gate-t.cpp +++ b/test/tap/tests/test_frontend_x509_tier_gate-t.cpp @@ -15,6 +15,7 @@ static constexpr const char* USERNAME = "tap_x509_tier_gate"; static constexpr const char* PASSWORD = "tap-x509-tier-password"; +static constexpr const char* NONOBJECT_USERNAME = "tap_x509_tier_nonobject"; static bool do_query(MYSQL* mysql, const char* query) { if (mysql_query(mysql, query) == 0) return true; @@ -61,7 +62,7 @@ int main() { return EXIT_FAILURE; } - plan(5); + plan(6); MYSQL* admin = mysql_init(NULL); const bool admin_connected = admin && mysql_real_connect(admin, cl.host, cl.admin_username, cl.admin_password, NULL, cl.admin_port, NULL, 0); @@ -77,12 +78,14 @@ int main() { ok(version_read, "Read ProxySQL admin-version as %d.%d", major, minor); const bool user_provisioned = do_query(admin, - "DELETE FROM mysql_users WHERE username='tap_x509_tier_gate'") && + "DELETE FROM mysql_users WHERE username IN " + "('tap_x509_tier_gate','tap_x509_tier_nonobject')") && do_query(admin, "INSERT INTO mysql_users(username,password,default_hostgroup,active,attributes) VALUES " - "('tap_x509_tier_gate','tap-x509-tier-password',0,1,'{\"require_x509\":true}')") && + "('tap_x509_tier_gate','tap-x509-tier-password',0,1,'{\"require_x509\":true}')," + "('tap_x509_tier_nonobject','tap-x509-tier-password',0,1,'[]')") && do_query(admin, "LOAD MYSQL USERS TO RUNTIME"); - ok(user_provisioned, "Provisioned the dedicated require_x509 tier-gate user"); + ok(user_provisioned, "Provisioned the dedicated require_x509 tier-gate users"); const bool has_feature = major > 3 || (major == 3 && minor >= 1); const unsigned int expected = has_feature ? ER_ACCESS_DENIED_ERROR : 0; @@ -92,10 +95,17 @@ int main() { "require_x509 is %s on ProxySQL %d.%d: expected errno=%u, got errno=%u", has_feature ? "enforced" : "unrecognized", major, minor, expected, actual); + const unsigned int nonobject_actual = user_provisioned + ? try_plaintext_frontend_connect(cl, NONOBJECT_USERNAME, PASSWORD) : UINT_MAX; + ok(nonobject_actual == expected, + "Valid non-object attributes are %s on ProxySQL %d.%d: expected errno=%u, got errno=%u", + has_feature ? "rejected" : "ignored", major, minor, expected, nonobject_actual); + const bool users_cleaned = do_query(admin, - "DELETE FROM mysql_users WHERE username='tap_x509_tier_gate'") && + "DELETE FROM mysql_users WHERE username IN " + "('tap_x509_tier_gate','tap_x509_tier_nonobject')") && do_query(admin, "LOAD MYSQL USERS TO RUNTIME"); - ok(users_cleaned, "Cleanup removed the dedicated require_x509 tier-gate user"); + ok(users_cleaned, "Cleanup removed the dedicated require_x509 tier-gate users"); mysql_close(admin); return exit_status(); } From 349c4c139ec3ce2fdf58036d5cfe0279839ea19e Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Mon, 10 Aug 2026 22:44:19 +0000 Subject: [PATCH 10/11] fix: address frontend X.509 review findings --- ...2026-08-10-frontend-x509-authentication.md | 58 ++-- ...-08-10-frontend-x509-review-remediation.md | 136 +++++++++ include/MySQL_Data_Stream.h | 1 + lib/MySQL_Protocol.cpp | 152 ++++++---- lib/mysql_data_stream.cpp | 64 ++-- test/tap/groups/groups.json | 6 +- test/tap/tests/frontend_x509_test_utils.h | 108 +++---- test/tap/tests/test_frontend_x509_auth-t.cpp | 281 ++++++++++-------- .../test_frontend_x509_passthrough-t.cpp | 236 +++++++++------ 9 files changed, 656 insertions(+), 386 deletions(-) create mode 100644 docs/superpowers/plans/2026-08-10-frontend-x509-review-remediation.md diff --git a/docs/superpowers/plans/2026-08-10-frontend-x509-authentication.md b/docs/superpowers/plans/2026-08-10-frontend-x509-authentication.md index 9637effa37..63cb3e45b0 100644 --- a/docs/superpowers/plans/2026-08-10-frontend-x509-authentication.md +++ b/docs/superpowers/plans/2026-08-10-frontend-x509-authentication.md @@ -294,11 +294,11 @@ static unsigned int try_plaintext_frontend_connect( ```cpp x509_subject_alt_name = nullptr; -#ifdef PROXYSQL31 + #ifdef PROXYSQL31 client_cert_present = false; client_cert_verify_result = X509_V_OK; frontend_authenticated_via_spiffe = false; -#endif + #endif ssl = nullptr; ``` @@ -310,26 +310,26 @@ static unsigned int try_plaintext_frontend_connect( ```cpp if (n == 1) { - X509* cert = SSL_get_peer_certificate(ssl); -#ifdef PROXYSQL31 - client_cert_present = (cert != nullptr); - client_cert_verify_result = cert ? SSL_get_verify_result(ssl) : X509_V_OK; -#endif - - if (cert) { - GENERAL_NAMES* alt_names = static_cast( - X509_get_ext_d2i(cert, NID_subject_alt_name, nullptr, nullptr)); - if (alt_names) { - // Preserve the existing first spiffe:// URI extraction loop. - sk_GENERAL_NAME_pop_free(alt_names, GENERAL_NAME_free); - } - X509_free(cert); - } - - if (x509_subject_alt_name && SSL_get_verify_result(ssl) != X509_V_OK) { - // Preserve the existing SPIFFE handshake-failure behavior. - return SSLSTATUS_FAIL; - } + X509* cert = SSL_get_peer_certificate(ssl); + #ifdef PROXYSQL31 + client_cert_present = (cert != nullptr); + client_cert_verify_result = cert ? SSL_get_verify_result(ssl) : X509_V_OK; + #endif + + if (cert) { + GENERAL_NAMES* alt_names = static_cast( + X509_get_ext_d2i(cert, NID_subject_alt_name, nullptr, nullptr)); + if (alt_names) { + // Preserve the existing first spiffe:// URI extraction loop. + sk_GENERAL_NAME_pop_free(alt_names, GENERAL_NAME_free); + } + X509_free(cert); + } + + if (x509_subject_alt_name && SSL_get_verify_result(ssl) != X509_V_OK) { + // Preserve the existing SPIFFE handshake-failure behavior. + return SSLSTATUS_FAIL; + } } ``` @@ -379,19 +379,19 @@ static unsigned int try_plaintext_frontend_connect( Use the common evaluator only in `PROXYSQL31` builds. Preserve the existing SPIFFE-only block verbatim in the stable `#else` path: ```cpp -#ifdef PROXYSQL31 + #ifdef PROXYSQL31 const char* attributes = (*myds)->sess->user_attributes; const auto policy = evaluate_frontend_certificate_policy( - *myds, attributes, user, - frontend_auth_context::INITIAL_HANDSHAKE, - calling_line, calling_func); + *myds, attributes, user, + frontend_auth_context::INITIAL_HANDSHAKE, + calling_line, calling_func); if (!policy.allowed) { - return false; + return false; } (*myds)->frontend_authenticated_via_spiffe = policy.has_spiffe_id; -#else + #else // Existing v3.0 SPIFFE-only attribute handling. Never inspect require_x509. -#endif + #endif ``` Retain the existing `default-transaction_isolation` application after policy success. Parse the JSON once in the function or pass a parsed object through a private helper; do not reintroduce uncaught `get()` exceptions. diff --git a/docs/superpowers/plans/2026-08-10-frontend-x509-review-remediation.md b/docs/superpowers/plans/2026-08-10-frontend-x509-review-remediation.md new file mode 100644 index 0000000000..f8a1b66861 --- /dev/null +++ b/docs/superpowers/plans/2026-08-10-frontend-x509-review-remediation.md @@ -0,0 +1,136 @@ +# Frontend X.509 PR Review Remediation Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Resolve the actionable lint, CodeRabbit, and SonarCloud findings on PR 6028 without changing the approved authentication semantics. + +**Architecture:** Keep certificate evidence immutable for a physical frontend connection, but isolate ASN.1 URI extraction and policy subchecks into small helpers. Keep certificate generation in TAP code while replacing shell command construction with argument-vector process execution. Refactor only the large test flows and policy evaluator flagged on new code. + +**Tech Stack:** C++17, OpenSSL X.509 APIs, nlohmann JSON, RE2, ProxySQL TAP, `wexecvp`, Python groups.json lint. + +## Global Constraints + +- `require_x509` production behavior remains compiled only under `PROXYSQL31` and therefore `PROXYSQL40`. +- Stable v3.0 must not recognize, log, or enforce `require_x509`. +- Certificate policy runs before pass-through allowlist, cache, metrics, or probing. +- `COM_CHANGE_USER` reuses immutable evidence from the original TLS handshake and never renegotiates TLS. +- The normal pass-through TAP plan remains 40; only its unavailable-trusted-fixture fallback changes from 11 skips to 10. +- Authentication failures exposed to clients remain generic error 1045. + +--- + +### Task 1: Establish RED quality and regression baselines + +**Files:** + +- Modify: `test/tap/tests/test_frontend_x509_auth-t.cpp` +- Modify: `test/tap/tests/test_frontend_x509_passthrough-t.cpp` +- Inspect: `test/tap/groups/groups.json` + +**Interfaces:** + +- Consumes: existing certificate fixture and isolated TAP runner. +- Produces: failing coverage for first-SPIFFE-SAN selection and the trusted-fixture fallback assertion count. + +- [ ] Add a SPIFFE certificate fixture containing two URI SANs and assert that the first SPIFFE URI is the identity used for authentication. +- [ ] Run the focused PROXYSQL31 TAP against the current implementation and record the expected failure caused by last-match overwrite. +- [ ] Run `python3 test/tap/groups/lint_groups_json.py` and record the expected unsorted-key failure. +- [ ] Exercise the pass-through test with trusted certificate signing unavailable and record the plan mismatch caused by 11 fallback skips. + +### Task 2: Harden certificate evidence extraction and lifecycle + +**Files:** + +- Modify: `include/MySQL_Data_Stream.h` +- Modify: `lib/mysql_data_stream.cpp` +- Test: `test/tap/tests/test_frontend_x509_auth-t.cpp` + +**Interfaces:** + +- Produces: `reset_frontend_certificate_evidence()` and bounded first-match SPIFFE URI extraction from `ASN1_STRING`. + +- [ ] Replace `strstr`/`strdup` over ASN.1 storage with `ASN1_STRING_get0_data`, `ASN1_STRING_length`, bounded prefix comparison, exact allocation/copy, and explicit NUL termination. +- [ ] Stop after the first matching SPIFFE URI and reject embedded-NUL URI values. +- [ ] Reset/free SAN and PROXYSQL31 evidence at data-stream initialization so any future stream reuse cannot retain prior-client evidence. +- [ ] Rebuild and rerun the focused X.509 TAP until the new first-SAN regression is GREEN. + +### Task 3: Simplify policy evaluation and secure COM_CHANGE_USER cleanup + +**Files:** + +- Modify: `lib/MySQL_Protocol.cpp` +- Test: `test/tap/tests/test_frontend_x509_auth-t.cpp` +- Test: `test/tap/tests/reg_test_3504-change_user-t.cpp` + +**Interfaces:** + +- Produces: small `require_x509` and SPIFFE evaluation helpers used by `evaluate_frontend_certificate_policy`. + +- [ ] Extract the `require_x509` type/evidence check without changing fail-closed results or diagnostics. +- [ ] Extract the context/type/regex SPIFFE check without changing exact/regex matching. +- [ ] Replace the new direct `free(password)` rejection cleanup with `cleanse_and_free_password(password)`; leave pre-existing packet-buffer ownership unchanged. +- [ ] Clean-build and run the X.509 and COM_CHANGE_USER TAPs in PROXYSQL31. + +### Task 4: Remove unsafe test-fixture process and temporary-directory patterns + +**Files:** + +- Modify: `test/tap/tests/frontend_x509_test_utils.h` +- Modify: `test/tap/tests/test_frontend_x509_auth-t.cpp` +- Modify: `test/tap/tests/test_frontend_x509_passthrough-t.cpp` + +**Interfaces:** + +- Produces: non-copyable `temporary_certificate_directory` rooted below `REGULAR_INFRA_DATADIR`; `run_openssl(const std::vector&)` using `wexecvp`. + +- [ ] Delete copy and move construction/assignment for the owning temporary-directory class and rename its header guard to a non-reserved identifier. +- [ ] Build the `mkdtemp` template beneath the isolated infra data directory passed by the callers. +- [ ] Replace shell quoting and `system()` with explicit OpenSSL argument vectors passed to `wexecvp`, capturing stdout/stderr for diagnostics. +- [ ] Rebuild and run both X.509 TAP binaries to prove certificate generation and cleanup remain functional. + +### Task 5: Resolve TAP quality findings without changing behavior + +**Files:** + +- Modify: `test/tap/tests/test_frontend_x509_auth-t.cpp` +- Modify: `test/tap/tests/test_frontend_x509_passthrough-t.cpp` + +**Interfaces:** + +- Produces: focused setup/behavior/cleanup helper functions; explicit lambda captures. + +- [ ] Split each flagged `main()` into setup, behavior-matrix, and cleanup helpers while retaining literal TAP expectations and ordering. +- [ ] Explicitly capture only the admin connection and command-line object in the two pass-through lambdas. +- [ ] Change the trusted-fixture fallback from 11 skips to 10 and keep `plan(40)`. +- [ ] Rerun normal and unavailable-trusted-fixture paths and verify both emit exactly 40 TAP results. + +### Task 6: Fix lint and documentation review findings + +**Files:** + +- Modify: `test/tap/groups/groups.json` +- Modify: `docs/superpowers/plans/2026-08-10-frontend-x509-authentication.md` +- Modify: `docs/superpowers/specs/2026-08-10-frontend-x509-proxysql31-gating-design.md` if its lint output requires it. + +**Interfaces:** + +- Produces: sorted group keys and valid fenced examples. + +- [ ] Run the groups lint fixer, inspect that it only sorts keys, then rerun lint. +- [ ] Indent fenced preprocessor examples consistently within their list items and add any missing language tags. +- [ ] Run the repository Markdown lint command used by CI or the closest locally available equivalent. + +### Task 7: Full verification and GitHub handoff + +**Files:** + +- Verify all changed files. + +**Interfaces:** + +- Produces: fresh Stable and PROXYSQL31 build/test evidence suitable for pushing to PR 6028. + +- [ ] Run `git diff --check`, groups lint, focused test compilation, and static checks for banned `system()`/unbounded ASN.1 operations. +- [ ] Clean-build Stable DEBUG, run the tier-gate and COM_CHANGE_USER regressions, and verify feature symbols remain absent. +- [ ] Clean-build PROXYSQL31 DEBUG and run frontend X.509, X.509/pass-through, COM_CHANGE_USER, and tier-gate TAPs. +- [ ] Commit the scoped remediation, push only after verification, and rerun/recheck PR checks; rerun the unrelated Aurora job without modifying cluster code. diff --git a/include/MySQL_Data_Stream.h b/include/MySQL_Data_Stream.h index ac2d0a4368..432baeb03d 100644 --- a/include/MySQL_Data_Stream.h +++ b/include/MySQL_Data_Stream.h @@ -205,6 +205,7 @@ class MySQL_Data_Stream MySQL_Data_Stream(); virtual ~MySQL_Data_Stream(); int array2buffer_full(); + void reset_frontend_certificate_evidence(); void init(); // initialize the data stream void init(enum MySQL_DS_type, MySQL_Session *, int); // initialize with arguments void shut_soft(); diff --git a/lib/MySQL_Protocol.cpp b/lib/MySQL_Protocol.cpp index 6f1c4ecf11..4c395dc63f 100644 --- a/lib/MySQL_Protocol.cpp +++ b/lib/MySQL_Protocol.cpp @@ -59,6 +59,15 @@ void cleanse_and_free_password(char*& password) { } } +void cleanse_and_free_auth_response(unsigned char*& response, size_t response_size) { + if (response != nullptr) { + OPENSSL_cleanse(response, response_size); + char* allocation = reinterpret_cast(response); + response = nullptr; + cleanse_and_free_password(allocation); + } +} + class ScopedStringCleanser { std::string& value_; @@ -97,77 +106,99 @@ struct frontend_certificate_policy_result { bool has_spiffe_id { false }; }; -static frontend_certificate_policy_result evaluate_frontend_certificate_policy( +static bool evaluate_require_x509( MySQL_Data_Stream* myds, const json& attrs, - const unsigned char* user, + const char* username, frontend_auth_context context, int calling_line, const char* calling_func ) { - frontend_certificate_policy_result result; - const char* username = user ? reinterpret_cast(user) : "unknown"; - if (!attrs.is_object()) { - proxy_error("%d:%s(): Invalid user attributes for user %s\n", calling_line, calling_func, username); - result.allowed = false; - return result; + const auto require_x509 = attrs.find("require_x509"); + if (require_x509 == attrs.end()) return true; + if (!require_x509->is_boolean()) { + proxy_error("%d:%s(): Invalid require_x509 type for user %s\n", calling_line, calling_func, username); + return false; } - const auto spiffe_id = attrs.find("spiffe_id"); - result.has_spiffe_id = spiffe_id != attrs.end(); + if (!require_x509->get()) return true; - const auto require_x509 = attrs.find("require_x509"); - if (require_x509 != attrs.end()) { - if (!require_x509->is_boolean()) { - proxy_error("%d:%s(): Invalid require_x509 type for user %s\n", calling_line, calling_func, username); - result.allowed = false; - return result; - } - if (require_x509->get()) { - result.allowed = myds - && myds->encrypted - && myds->ssl - && myds->client_cert_present - && myds->client_cert_verify_result == X509_V_OK; - if (!result.allowed) { - proxy_error("%d:%s(): Frontend X509 authentication error for user %s: context=%u cert_present=%s verify_result=%ld\n", - calling_line, calling_func, username, static_cast(context), - (myds && myds->client_cert_present) ? "yes" : "no", - myds ? myds->client_cert_verify_result : X509_V_ERR_UNSPECIFIED); - return result; - } - } + const bool allowed = myds + && myds->encrypted + && myds->ssl + && myds->client_cert_present + && myds->client_cert_verify_result == X509_V_OK; + if (!allowed) { + proxy_error("%d:%s(): Frontend X509 authentication error for user %s: context=%u cert_present=%s verify_result=%ld\n", + calling_line, calling_func, username, static_cast(context), + (myds && myds->client_cert_present) ? "yes" : "no", + myds ? myds->client_cert_verify_result : X509_V_ERR_UNSPECIFIED); } + return allowed; +} - if (spiffe_id == attrs.end()) return result; +static bool spiffe_identity_matches(MySQL_Data_Stream* myds, const std::string& expected) { + if (!myds || !myds->x509_subject_alt_name) return false; + if (expected.rfind("!", 0) == 0 && expected.size() > 1) { + const string pattern { expected.substr(1) }; + re2::RE2::Options opts { re2::RE2::Quiet }; + re2::RE2 subject_alt_regex(pattern, opts); + return re2::RE2::FullMatch(myds->x509_subject_alt_name, subject_alt_regex); + } + return expected.rfind("spiffe://", 0) == 0 + && expected == myds->x509_subject_alt_name; +} + +static bool evaluate_spiffe_identity( + MySQL_Data_Stream* myds, + const json::const_iterator& spiffe_id, + const char* username, + frontend_auth_context context, + int calling_line, + const char* calling_func +) { if (context == frontend_auth_context::COM_CHANGE_USER) { proxy_error("%d:%s(): COM_CHANGE_USER target %s has a SPIFFE identity\n", calling_line, calling_func, username); - result.allowed = false; - return result; + return false; } if (!spiffe_id->is_string()) { proxy_error("%d:%s(): Invalid spiffe_id type for user %s\n", calling_line, calling_func, username); - result.allowed = false; - return result; + return false; } - result.allowed = false; - const std::string spiffe_val = spiffe_id->get(); - if (myds && myds->x509_subject_alt_name) { - if (spiffe_val.rfind("!", 0) == 0 && spiffe_val.size() > 1) { - string str_spiffe_regex { spiffe_val.substr(1) }; - re2::RE2::Options opts = re2::RE2::Options(RE2::Quiet); - re2::RE2 subject_alt_regex(str_spiffe_regex, opts); - result.allowed = re2::RE2::FullMatch(myds->x509_subject_alt_name, subject_alt_regex); - } else if (strncmp(spiffe_val.c_str(), "spiffe://", strlen("spiffe://")) == 0) { - result.allowed = strcmp(spiffe_val.c_str(), myds->x509_subject_alt_name) == 0; - } - } - if (!result.allowed) { + const std::string expected = spiffe_id->get(); + const bool allowed = spiffe_identity_matches(myds, expected); + if (!allowed) { proxy_error("%d:%s(): SPIFFE Authentication error for user %s . spiffed_id expected : %s , received: %s\n", - calling_line, calling_func, username, spiffe_val.c_str(), + calling_line, calling_func, username, expected.c_str(), (myds && myds->x509_subject_alt_name) ? myds->x509_subject_alt_name : "none"); } + return allowed; +} + +static frontend_certificate_policy_result evaluate_frontend_certificate_policy( + MySQL_Data_Stream* myds, + const json& attrs, + const unsigned char* user, + frontend_auth_context context, + int calling_line, + const char* calling_func +) { + frontend_certificate_policy_result result; + const char* username = user ? reinterpret_cast(user) : "unknown"; + if (!attrs.is_object()) { + proxy_error("%d:%s(): Invalid user attributes for user %s\n", calling_line, calling_func, username); + result.allowed = false; + return result; + } + const auto spiffe_id = attrs.find("spiffe_id"); + result.has_spiffe_id = spiffe_id != attrs.end(); + result.allowed = evaluate_require_x509( + myds, attrs, username, context, calling_line, calling_func); + if (!result.allowed) return result; + if (spiffe_id == attrs.end()) return result; + result.allowed = evaluate_spiffe_identity( + myds, spiffe_id, username, context, calling_line, calling_func); return result; } @@ -1613,14 +1644,14 @@ bool MySQL_Protocol::process_pkt_COM_CHANGE_USER(unsigned char *pkt, unsigned in pass[pass_len]=0; cur+=pass_len; if (pkt + cur >= packet_end) { - free(pass); + cleanse_and_free_auth_response(pass, pass_len + 1); return false; } const char *db_ptr = reinterpret_cast(pkt + cur); const size_t db_remaining = packet_end - (pkt + cur); const size_t db_len = strnlen(db_ptr, db_remaining); if (db_len == db_remaining) { - free(pass); + cleanse_and_free_auth_response(pass, pass_len + 1); return false; } db=const_cast(db_ptr); @@ -1628,7 +1659,7 @@ bool MySQL_Protocol::process_pkt_COM_CHANGE_USER(unsigned char *pkt, unsigned in cur += db_len + 1; // Skip field 'character-set' (size 2) if (static_cast(packet_end - (pkt + cur)) < sizeof(uint16_t)) { - free(pass); + cleanse_and_free_auth_response(pass, pass_len + 1); return false; } cur += 2; @@ -1639,7 +1670,7 @@ bool MySQL_Protocol::process_pkt_COM_CHANGE_USER(unsigned char *pkt, unsigned in const char *auth_plugin_ptr = reinterpret_cast(pkt + cur); const size_t auth_plugin_len = strnlen(auth_plugin_ptr, packet_end - (pkt + cur)); if (auth_plugin_len == static_cast(packet_end - (pkt + cur))) { - free(pass); + cleanse_and_free_auth_response(pass, pass_len + 1); return false; } client_auth_plugin = const_cast(auth_plugin_ptr); @@ -1659,7 +1690,7 @@ bool MySQL_Protocol::process_pkt_COM_CHANGE_USER(unsigned char *pkt, unsigned in proxy_error( "Client %s:%d cannot run COM_CHANGE_USER after SPIFFE authentication\n", (*myds)->addr.addr, (*myds)->addr.port); - free(pass); + cleanse_and_free_auth_response(pass, pass_len + 1); return false; } #endif @@ -1745,8 +1776,8 @@ bool MySQL_Protocol::process_pkt_COM_CHANGE_USER(unsigned char *pkt, unsigned in frontend_auth_context::COM_CHANGE_USER, __LINE__, __func__); if (!target_policy.allowed || target_policy.has_spiffe_id) { - if (pass) { free(pass); pass = NULL; } - if (password) { free(password); password = NULL; } + cleanse_and_free_auth_response(pass, pass_len + 1); + cleanse_and_free_password(password); free_account_details(account_details); return false; } @@ -1767,7 +1798,7 @@ bool MySQL_Protocol::process_pkt_COM_CHANGE_USER(unsigned char *pkt, unsigned in "(Phase 1 does not support pass-through via CHANGE_USER, spec §5.4)\n", user ? (const char*)user : "(null)"); ret = false; - if (pass) { free(pass); pass = NULL; } + cleanse_and_free_auth_response(pass, pass_len + 1); if (userinfo->username) free(userinfo->username); userinfo->clear_password(); userinfo->username = strdup((const char *)user); @@ -1816,10 +1847,7 @@ bool MySQL_Protocol::process_pkt_COM_CHANGE_USER(unsigned char *pkt, unsigned in } } } - if (pass) { - free(pass); - pass=NULL; - } + cleanse_and_free_auth_response(pass, pass_len + 1); if (userinfo->username) free(userinfo->username); userinfo->clear_password(); if (ret==true) { diff --git a/lib/mysql_data_stream.cpp b/lib/mysql_data_stream.cpp index fa020bc291..2aea16907e 100644 --- a/lib/mysql_data_stream.cpp +++ b/lib/mysql_data_stream.cpp @@ -218,12 +218,38 @@ void MySQL_Data_Stream::queue_encrypted_bytes(const char *buf, size_t len) { //proxy_info("New ssl_write_len size: %u\n", ssl_write_len); } +static char* extract_first_spiffe_uri(const GENERAL_NAMES* alt_names) { + static constexpr char SPIFFE_PREFIX[] = "spiffe"; + const int alt_name_count = sk_GENERAL_NAME_num(alt_names); + + for (int i = 0; i < alt_name_count; ++i) { + const GENERAL_NAME* san = sk_GENERAL_NAME_value(alt_names, i); + if (!san || san->type != GEN_URI || !san->d.uniformResourceIdentifier) continue; + + const ASN1_STRING* uri = san->d.uniformResourceIdentifier; + const unsigned char* data = ASN1_STRING_get0_data(uri); + const int length = ASN1_STRING_length(uri); + if (!data || length < static_cast(sizeof(SPIFFE_PREFIX) - 1)) continue; + if (memcmp(data, SPIFFE_PREFIX, sizeof(SPIFFE_PREFIX) - 1) != 0) continue; + if (memchr(data, '\0', length) != nullptr) continue; + + char* value = new (std::nothrow) char[static_cast(length) + 1]; + if (!value) return nullptr; + memcpy(value, data, length); + value[length] = '\0'; + return value; + } + + return nullptr; +} + enum sslstatus MySQL_Data_Stream::do_ssl_handshake() { char buf[MY_SSL_BUFFER]; enum sslstatus status; int n = SSL_do_handshake(ssl); if (n == 1) { //proxy_info("SSL handshake completed\n"); + reset_frontend_certificate_evidence(); X509 *cert = SSL_get_peer_certificate(ssl); #ifdef PROXYSQL31 client_cert_present = (cert != nullptr); @@ -232,27 +258,7 @@ enum sslstatus MySQL_Data_Stream::do_ssl_handshake() { if (cert) { GENERAL_NAMES *alt_names = (stack_st_GENERAL_NAME *)X509_get_ext_d2i((X509*)cert, NID_subject_alt_name, 0, 0); if (alt_names) { - int alt_name_count = sk_GENERAL_NAME_num(alt_names); - - // Iterate all the SAN names, looking for SPIFFE identifier - for (int i = 0; i < alt_name_count; i++) { - GENERAL_NAME *san = sk_GENERAL_NAME_value(alt_names, i); - - // We only care about URI names - if (san->type == GEN_URI) { - if (san->d.uniformResourceIdentifier->data) { - const char* resource_data = - reinterpret_cast(san->d.uniformResourceIdentifier->data); - const char* spiffe_loc = strstr(resource_data, "spiffe"); - - // First name starting with 'spiffe' is considered the match. - if (spiffe_loc == resource_data) { - x509_subject_alt_name = strdup(resource_data); - } - } - } - } - + x509_subject_alt_name = extract_first_spiffe_uri(alt_names); sk_GENERAL_NAME_pop_free(alt_names, GENERAL_NAME_free); } X509_free(cert); @@ -498,10 +504,17 @@ MySQL_Data_Stream::~MySQL_Data_Stream() { CompPktOUT.pkt.ptr=NULL; CompPktOUT.pkt.size=0; } - if (x509_subject_alt_name) { - free(x509_subject_alt_name); - x509_subject_alt_name=NULL; - } + reset_frontend_certificate_evidence(); +} + +void MySQL_Data_Stream::reset_frontend_certificate_evidence() { + delete[] x509_subject_alt_name; + x509_subject_alt_name = nullptr; +#ifdef PROXYSQL31 + client_cert_present = false; + client_cert_verify_result = X509_V_OK; + frontend_authenticated_via_spiffe = false; +#endif } // this function initializes a MySQL_Data_Stream @@ -534,6 +547,7 @@ void MySQL_Data_Stream::reinit_queues() { // this function initializes a MySQL_Data_Stream with arguments void MySQL_Data_Stream::init(enum MySQL_DS_type _type, MySQL_Session *_sess, int _fd) { myds_type=_type; + if (_type == MYDS_FRONTEND) reset_frontend_certificate_evidence(); sess=_sess; init(); fd=_fd; diff --git a/test/tap/groups/groups.json b/test/tap/groups/groups.json index 661b0bdb83..e39f2f1765 100644 --- a/test/tap/groups/groups.json +++ b/test/tap/groups/groups.json @@ -336,9 +336,6 @@ "test_admin_stats-t" : [ "legacy-g7","mysql-auto_increment_delay_multiplex=0-g2","mysql-multiplexing=false-g2","mysql-query_digests=0-g2","mysql-query_digests_keep_comment=1-g2","mysql84-g7","mysql90-g2","mysql95-g2" ], "test_ansi_quotes_group_replication-t" : [ "mysql84-gr-g1","mysql90-gr-g1","mysql91-gr-g1","mysql92-gr-g1","mysql93-gr-g1","mysql95-gr-g1" ], "test_auth_methods-t" : [ "mysql-auto_increment_delay_multiplex=0-g2","mysql-multiplexing=false-g2","mysql-query_digests=0-g2","mysql-query_digests_keep_comment=1-g2","mysql84-g7","mysql90-g2","mysql95-g2" ], - "test_frontend_x509_auth-t" : [ "legacy-g6", "mysql84-g6", "mysql90-g1", "mysql95-g1", "@proxysql_min_version:3.1" ], - "test_frontend_x509_passthrough-t" : [ "mysql84-g4", "mysql90-g4", "mysql95-g4", "@proxysql_min_version:3.1" ], - "test_frontend_x509_tier_gate-t" : [ "legacy-g6", "mysql84-g6", "mysql90-g1", "mysql95-g1" ], "test_auto_increment_delay_multiplex-t" : [ "legacy-g7","mysql-auto_increment_delay_multiplex=0-g2","mysql-multiplexing=false-g2","mysql-query_digests=0-g2","mysql-query_digests_keep_comment=1-g2","mysql84-g7","mysql90-g2","mysql95-g2" ], "test_backend_conn_ping-t" : [ "legacy-g7","mysql-auto_increment_delay_multiplex=0-g2","mysql-multiplexing=false-g2","mysql-query_digests=0-g2","mysql-query_digests_keep_comment=1-g2","mysql84-g7","mysql90-g2","mysql95-g2" ], "test_binlog_dump_multi_backend_crash-t" : [ "legacy-binlog-g1" ], @@ -394,6 +391,9 @@ "test_flagOUT_weight-t" : [ "legacy-g3","mysql-auto_increment_delay_multiplex=0-g3","mysql-multiplexing=false-g3","mysql-query_digests=0-g3","mysql-query_digests_keep_comment=1-g3","mysql84-g3","mysql90-g3","mysql95-g3" ], "test_flush_logs-t" : [ "legacy-g3","mysql-auto_increment_delay_multiplex=0-g3","mysql-multiplexing=false-g3","mysql-query_digests=0-g3","mysql-query_digests_keep_comment=1-g3","mysql84-g3","mysql90-g3","mysql95-g3" ], "test_format_utils-t" : [ "legacy-g3","mysql-auto_increment_delay_multiplex=0-g3","mysql-multiplexing=false-g3","mysql-query_digests=0-g3","mysql-query_digests_keep_comment=1-g3","mysql84-g3","mysql90-g3","mysql95-g3" ], + "test_frontend_x509_auth-t" : [ "legacy-g6","mysql84-g6","mysql90-g1","mysql95-g1","@proxysql_min_version:3.1" ], + "test_frontend_x509_passthrough-t" : [ "mysql84-g4","mysql90-g4","mysql95-g4","@proxysql_min_version:3.1" ], + "test_frontend_x509_tier_gate-t" : [ "legacy-g6","mysql84-g6","mysql90-g1","mysql95-g1" ], "test_greeting_capabilities-t" : [ "legacy-g8","mariadb10-galera-g8","mysql-auto_increment_delay_multiplex=0-g3","mysql-multiplexing=false-g3","mysql-query_digests=0-g3","mysql-query_digests_keep_comment=1-g3","mysql84-g8","mysql84-gr-g8","mysql90-g3","mysql95-g3" ], "test_gtid_forwarding-t" : [ "legacy-binlog-g1","mysql-auto_increment_delay_multiplex=0-g3","mysql-multiplexing=false-g3","mysql-query_digests=0-g3","mysql-query_digests_keep_comment=1-g3","mysql84-g5","mysql90-g5","mysql95-g5" ], "test_hostgroup_attributes_online_servers-t" : [ "legacy-g8","mysql-auto_increment_delay_multiplex=0-g3","mysql-multiplexing=false-g3","mysql-query_digests=0-g3","mysql-query_digests_keep_comment=1-g3","mysql84-g8","mysql90-g3","mysql95-g3" ], diff --git a/test/tap/tests/frontend_x509_test_utils.h b/test/tap/tests/frontend_x509_test_utils.h index 9e0d17e6d3..1c71130a26 100644 --- a/test/tap/tests/frontend_x509_test_utils.h +++ b/test/tap/tests/frontend_x509_test_utils.h @@ -3,8 +3,8 @@ * @brief Header-only TLS fixture helpers for frontend X.509 TAP tests. */ -#ifndef __FRONTEND_X509_TEST_UTILS_H -#define __FRONTEND_X509_TEST_UTILS_H +#ifndef FRONTEND_X509_TEST_UTILS_H +#define FRONTEND_X509_TEST_UTILS_H #include #include @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -21,6 +22,7 @@ #include "tap.h" #include "command_line.h" +#include "proxysql_utils.h" using std::string; @@ -38,41 +40,41 @@ struct mysql_closer { using mysql_ptr = std::unique_ptr; -/** Quote one shell argument, including paths derived from the environment. */ -static inline string shell_quote(const string& value) { - string quoted { "'" }; - for (const char c : value) { - if (c == '\'') { - quoted += "'\\''"; - } else { - quoted += c; - } +static inline bool run_openssl(const std::vector& arguments) { + std::vector argv; + argv.reserve(arguments.size()); + string command { "openssl" }; + for (const string& argument : arguments) { + argv.push_back(argument.c_str()); + command += " " + argument; } - quoted += "'"; - return quoted; -} - -static inline bool run_openssl(const string& command) { diag("Running: %s", command.c_str()); - const int status = system(command.c_str()); + + string standard_output; + string error_output; + const to_opts_t options { 30 * 1000 * 1000, 0, 0, 0 }; + const int status = wexecvp("openssl", argv, options, standard_output, error_output); if (status != 0) { - diag("openssl command failed with status %d", status); + diag("openssl command failed with status %d: %s%s", status, + standard_output.c_str(), error_output.c_str()); return false; } return true; } /** - * Own exactly the path returned by mkdtemp(). Cleanup never follows a path - * assembled from REGULAR_INFRA_DATADIR or another unchecked environment value. + * Own exactly the path returned by mkdtemp() below the isolated infra data + * directory. Cleanup removes only fixed fixture names below that owned path. */ class temporary_certificate_directory { string path_ {}; public: - temporary_certificate_directory() { - char template_path[] = "/tmp/proxysql-require-x509-XXXXXX"; - char* made = mkdtemp(template_path); + explicit temporary_certificate_directory(const string& parent_directory) { + const string template_value { parent_directory + "/proxysql-require-x509-XXXXXX" }; + std::vector template_path(template_value.begin(), template_value.end()); + template_path.push_back('\0'); + char* made = mkdtemp(template_path.data()); if (made) path_ = made; } @@ -91,6 +93,10 @@ class temporary_certificate_directory { } rmdir(path_.c_str()); } + temporary_certificate_directory(const temporary_certificate_directory&) = delete; + temporary_certificate_directory& operator=(const temporary_certificate_directory&) = delete; + temporary_certificate_directory(temporary_certificate_directory&&) = delete; + temporary_certificate_directory& operator=(temporary_certificate_directory&&) = delete; bool valid() const { return !path_.empty(); } const string& path() const { return path_; } @@ -111,18 +117,15 @@ static inline bool create_trusted_client_certificate( material.cert = directory.path() + "/trusted-client.pem"; material.ca = ca; - const bool req_ok = run_openssl( - "openssl req -new -newkey rsa:2048 -nodes -subj /CN=tap-require-x509" - " -keyout " + shell_quote(material.key) + " -out " + shell_quote(csr) - ); - const bool sign_ok = req_ok && run_openssl( - "openssl x509 -req -days 1 -set_serial 5928001 -in " + shell_quote(csr) + - " -CA " + shell_quote(ca) + " -CAkey " + shell_quote(ca_key) + - " -out " + shell_quote(material.cert) - ); - return sign_ok && run_openssl( - "openssl verify -CAfile " + shell_quote(ca) + " " + shell_quote(material.cert) - ); + const bool req_ok = run_openssl({ + "req", "-new", "-newkey", "rsa:2048", "-nodes", "-subj", "/CN=tap-require-x509", + "-keyout", material.key, "-out", csr + }); + const bool sign_ok = req_ok && run_openssl({ + "x509", "-req", "-days", "1", "-set_serial", "5928001", "-in", csr, + "-CA", ca, "-CAkey", ca_key, "-out", material.cert + }); + return sign_ok && run_openssl({ "verify", "-CAfile", ca, material.cert }); } static inline bool create_untrusted_client_certificate( @@ -131,11 +134,11 @@ static inline bool create_untrusted_client_certificate( material.key = directory.path() + "/untrusted-client.key"; material.cert = directory.path() + "/untrusted-client.pem"; material.ca = ca; - return run_openssl( - "openssl req -x509 -newkey rsa:2048 -nodes -days 1 -set_serial 5928002" - " -subj /CN=tap-untrusted -keyout " + shell_quote(material.key) + - " -out " + shell_quote(material.cert) - ); + return run_openssl({ + "req", "-x509", "-newkey", "rsa:2048", "-nodes", "-days", "1", + "-set_serial", "5928002", "-subj", "/CN=tap-untrusted", + "-keyout", material.key, "-out", material.cert + }); } static inline bool create_spiffe_client_certificate( @@ -160,19 +163,16 @@ static inline bool create_spiffe_client_certificate( return false; } - const bool req_ok = run_openssl( - "openssl req -new -newkey rsa:2048 -nodes -subj /CN=" + string(name) + - " -keyout " + shell_quote(material.key) + " -out " + shell_quote(csr) - ); - const bool sign_ok = req_ok && run_openssl( - "openssl x509 -req -days 1 -set_serial " + std::to_string(serial) + - " -in " + shell_quote(csr) + " -CA " + shell_quote(ca) + - " -CAkey " + shell_quote(ca_key) + " -extfile " + shell_quote(extfile) + - " -out " + shell_quote(material.cert) - ); - return sign_ok && run_openssl( - "openssl verify -CAfile " + shell_quote(ca) + " " + shell_quote(material.cert) - ); + const bool req_ok = run_openssl({ + "req", "-new", "-newkey", "rsa:2048", "-nodes", "-subj", "/CN=" + string(name), + "-keyout", material.key, "-out", csr + }); + const bool sign_ok = req_ok && run_openssl({ + "x509", "-req", "-days", "1", "-set_serial", std::to_string(serial), + "-in", csr, "-CA", ca, "-CAkey", ca_key, "-extfile", extfile, + "-out", material.cert + }); + return sign_ok && run_openssl({ "verify", "-CAfile", ca, material.cert }); } /** @@ -228,4 +228,4 @@ static inline unsigned int try_frontend_connect( return mysql ? 0 : connection_error; } -#endif /* __FRONTEND_X509_TEST_UTILS_H */ +#endif /* FRONTEND_X509_TEST_UTILS_H */ diff --git a/test/tap/tests/test_frontend_x509_auth-t.cpp b/test/tap/tests/test_frontend_x509_auth-t.cpp index 9cf4b46ec8..75f687c109 100644 --- a/test/tap/tests/test_frontend_x509_auth-t.cpp +++ b/test/tap/tests/test_frontend_x509_auth-t.cpp @@ -81,6 +81,158 @@ static bool read_global_variable(MYSQL* admin, const char* name, string& value) return found; } +static void run_initial_login_checks( + const CommandLine& cl, + bool trusted_client_ready, + bool untrusted_client_ready, + const client_tls_material& trusted_client, + const client_tls_material& untrusted_client +) { + ok(try_frontend_connect(cl, USER_NONE, PASSWORD, false) == 0, + "No require_x509 attribute permits plaintext authentication"); + ok(try_frontend_connect(cl, USER_NONE, PASSWORD, true) == 0, + "No require_x509 attribute permits TLS authentication without a client certificate"); + ok(try_frontend_connect(cl, USER_FALSE, PASSWORD, true) == 0, + "require_x509=false permits TLS authentication without a client certificate"); + ok(try_frontend_connect(cl, USER_REQUIRED, PASSWORD, false) == ER_ACCESS_DENIED_ERROR, + "require_x509=true rejects plaintext authentication with ER_ACCESS_DENIED_ERROR"); + ok(try_frontend_connect(cl, USER_REQUIRED, PASSWORD, true) == ER_ACCESS_DENIED_ERROR, + "require_x509=true rejects TLS authentication without a client certificate with ER_ACCESS_DENIED_ERROR"); + ok(untrusted_client_ready && + try_frontend_connect(cl, USER_REQUIRED, PASSWORD, true, &untrusted_client) == ER_ACCESS_DENIED_ERROR, + "require_x509=true rejects an untrusted client certificate with ER_ACCESS_DENIED_ERROR"); + if (!trusted_client_ready) { + ok(true, "require_x509 trusted certificate success # SKIP trusted certificate fixture unavailable"); + ok(true, "require_x509 trusted certificate wrong-password rejection # SKIP trusted certificate fixture unavailable"); + ok(true, "string require_x509=true fails closed # SKIP trusted certificate fixture unavailable"); + return; + } + + ok(try_frontend_connect(cl, USER_REQUIRED, PASSWORD, true, &trusted_client) == 0, + "require_x509=true accepts a trusted client certificate without a SAN"); + ok(try_frontend_connect(cl, USER_REQUIRED, WRONG_PASSWORD, true, &trusted_client) == ER_ACCESS_DENIED_ERROR, + "require_x509=true still rejects a wrong password with ER_ACCESS_DENIED_ERROR"); + ok(try_frontend_connect(cl, USER_BAD_TYPE, PASSWORD, true, &trusted_client) == ER_ACCESS_DENIED_ERROR, + "string require_x509=true fails closed with ER_ACCESS_DENIED_ERROR"); +} + +static void run_change_user_checks( + const CommandLine& cl, + bool trusted_client_ready, + bool untrusted_client_ready, + bool spiffe_source_client_ready, + bool spiffe_target_client_ready, + const client_tls_material& trusted_client, + const client_tls_material& untrusted_client, + const client_tls_material& spiffe_source_client, + const client_tls_material& spiffe_target_client +) { + { + mysql_ptr source { connect_frontend(cl, USER_CHANGE_SOURCE, CHANGE_SOURCE_PASSWORD, false) }; + ok(source && try_change_user(source.get(), USER_CHANGE_TARGET, CHANGE_TARGET_PASSWORD) == ER_ACCESS_DENIED_ERROR, + "COM_CHANGE_USER from plaintext rejects require_x509=true with ER_ACCESS_DENIED_ERROR"); + } + { + mysql_ptr source { connect_frontend(cl, USER_CHANGE_SOURCE, CHANGE_SOURCE_PASSWORD, true) }; + ok(source && try_change_user(source.get(), USER_CHANGE_TARGET, CHANGE_TARGET_PASSWORD) == ER_ACCESS_DENIED_ERROR, + "COM_CHANGE_USER from TLS without a client certificate rejects require_x509=true with ER_ACCESS_DENIED_ERROR"); + } + { + mysql_ptr source { connect_frontend(cl, USER_CHANGE_SOURCE, CHANGE_SOURCE_PASSWORD, true, &untrusted_client) }; + ok(untrusted_client_ready && source && + try_change_user(source.get(), USER_CHANGE_TARGET, CHANGE_TARGET_PASSWORD) == ER_ACCESS_DENIED_ERROR, + "COM_CHANGE_USER from an untrusted client certificate rejects require_x509=true with ER_ACCESS_DENIED_ERROR"); + } + if (trusted_client_ready) { + mysql_ptr source { connect_frontend(cl, USER_CHANGE_SOURCE, CHANGE_SOURCE_PASSWORD, true, &trusted_client) }; + ok(source && try_change_user(source.get(), USER_CHANGE_TARGET, CHANGE_TARGET_PASSWORD) == 0, + "COM_CHANGE_USER from a trusted client certificate accepts require_x509=true"); + } else { + ok(true, "COM_CHANGE_USER trusted certificate require_x509 success # SKIP trusted certificate fixture unavailable"); + } + if (trusted_client_ready) { + mysql_ptr source { connect_frontend(cl, USER_CHANGE_SOURCE, CHANGE_SOURCE_PASSWORD, true, &trusted_client) }; + ok(source && try_change_user(source.get(), USER_CHANGE_TARGET, WRONG_PASSWORD) == ER_ACCESS_DENIED_ERROR, + "COM_CHANGE_USER require_x509=true still rejects a wrong target password with ER_ACCESS_DENIED_ERROR"); + } else { + ok(true, "COM_CHANGE_USER trusted certificate wrong-password rejection # SKIP trusted certificate fixture unavailable"); + } + if (trusted_client_ready) { + mysql_ptr source { connect_frontend(cl, USER_CHANGE_SOURCE, CHANGE_SOURCE_PASSWORD, true, &trusted_client) }; + ok(source && try_change_user(source.get(), USER_NONE, PASSWORD) == 0, + "COM_CHANGE_USER from a trusted client certificate accepts an ordinary password target"); + } else { + ok(true, "COM_CHANGE_USER ordinary target control # SKIP trusted certificate fixture unavailable"); + } + if (spiffe_source_client_ready) { + mysql_ptr source { connect_frontend(cl, USER_SPIFFE_SOURCE, "", true, &spiffe_source_client) }; + ok(source && try_change_user(source.get(), USER_NONE, PASSWORD) == ER_ACCESS_DENIED_ERROR, + "The first SPIFFE URI SAN authenticates the source and COM_CHANGE_USER rejects it"); + } else { + ok(true, "COM_CHANGE_USER SPIFFE-authenticated source rejection # SKIP trusted SPIFFE source fixture unavailable"); + } + if (spiffe_target_client_ready) { + mysql_ptr source { connect_frontend(cl, USER_CHANGE_SOURCE, CHANGE_SOURCE_PASSWORD, true, &spiffe_target_client) }; + ok(source && try_change_user(source.get(), USER_SPIFFE_TARGET, "") == ER_ACCESS_DENIED_ERROR, + "COM_CHANGE_USER rejects a SPIFFE target with ER_ACCESS_DENIED_ERROR"); + } else { + ok(true, "COM_CHANGE_USER SPIFFE target rejection # SKIP trusted SPIFFE target fixture unavailable"); + } +} + +struct frontend_certificate_fixtures { + client_tls_material trusted_client; + client_tls_material untrusted_client; + client_tls_material spiffe_source_client; + client_tls_material spiffe_target_client; + bool trusted_client_ready { false }; + bool untrusted_client_ready { false }; + bool spiffe_source_client_ready { false }; + bool spiffe_target_client_ready { false }; +}; + +static frontend_certificate_fixtures create_frontend_certificate_fixtures( + const temporary_certificate_directory& directory, + const string& ca, + const string& ca_key +) { + frontend_certificate_fixtures fixtures; + fixtures.trusted_client_ready = directory.valid() && + create_trusted_client_certificate(directory, ca, ca_key, fixtures.trusted_client); + if (!fixtures.trusted_client_ready) { + diag("Trusted client certificate fixture unavailable. This can happen when a custom CA certificate has no matching private key; trusted-certificate probes will be skipped."); + } + if (fixtures.trusted_client_ready) { + ok(true, "Trusted client certificate generated and verified"); + } else if (directory.valid()) { + ok(true, "Trusted client certificate generated and verified # SKIP custom CA cannot sign the standard test client certificate"); + } else { + ok(false, "Trusted client certificate generated and verified (temporary directory unavailable)"); + } + + fixtures.untrusted_client_ready = directory.valid() && + create_untrusted_client_certificate(directory, ca, fixtures.untrusted_client); + ok(fixtures.untrusted_client_ready, "Untrusted self-signed client certificate generated"); + + fixtures.spiffe_source_client_ready = directory.valid() && + create_spiffe_client_certificate( + directory, ca, ca_key, "spiffe-source", + "spiffe://tap/source,URI:spiffe://tap/secondary", 5928003, + fixtures.spiffe_source_client); + ok(true, fixtures.spiffe_source_client_ready + ? "Trusted SPIFFE source client certificate generated and verified" + : "Trusted SPIFFE source client certificate generated and verified # SKIP custom CA cannot sign the SPIFFE source certificate"); + + fixtures.spiffe_target_client_ready = directory.valid() && + create_spiffe_client_certificate( + directory, ca, ca_key, "spiffe-target", "spiffe://tap/target", 5928004, + fixtures.spiffe_target_client); + ok(true, fixtures.spiffe_target_client_ready + ? "Trusted SPIFFE target client certificate generated and verified" + : "Trusted SPIFFE target client certificate generated and verified # SKIP custom CA cannot sign the SPIFFE target certificate"); + return fixtures; +} + int main() { CommandLine cl; @@ -163,128 +315,21 @@ int main() { "proxysql.log contains '%s' and '%s' after LOAD MYSQL USERS TO RUNTIME", USER_BAD_TYPE, BAD_TYPE_LOG_FINGERPRINT); - temporary_certificate_directory certificate_directory; + temporary_certificate_directory certificate_directory { datadir }; if (!certificate_directory.valid()) { diag("Could not create a temporary certificate directory: %s", strerror(errno)); } - client_tls_material trusted_client; - client_tls_material untrusted_client; - client_tls_material spiffe_source_client; - client_tls_material spiffe_target_client; - const bool trusted_client_ready = certificate_directory.valid() && - create_trusted_client_certificate(certificate_directory, ca, ca_key, trusted_client); - if (!trusted_client_ready) { - diag("Trusted client certificate fixture unavailable. This can happen when a custom CA certificate has no matching private key; trusted-certificate probes will be skipped."); - } - if (trusted_client_ready) { - ok(true, "Trusted client certificate generated and verified"); - } else if (certificate_directory.valid()) { - ok(true, "Trusted client certificate generated and verified # SKIP custom CA cannot sign the standard test client certificate"); - } else { - ok(false, "Trusted client certificate generated and verified (temporary directory unavailable)"); - } - - const bool untrusted_client_ready = certificate_directory.valid() && - create_untrusted_client_certificate(certificate_directory, ca, untrusted_client); - ok(untrusted_client_ready, "Untrusted self-signed client certificate generated"); - - const bool spiffe_source_client_ready = certificate_directory.valid() && - create_spiffe_client_certificate( - certificate_directory, ca, ca_key, "spiffe-source", "spiffe://tap/source", 5928003, - spiffe_source_client); - if (spiffe_source_client_ready) { - ok(true, "Trusted SPIFFE source client certificate generated and verified"); - } else { - ok(true, "Trusted SPIFFE source client certificate generated and verified # SKIP custom CA cannot sign the SPIFFE source certificate"); - } - const bool spiffe_target_client_ready = certificate_directory.valid() && - create_spiffe_client_certificate( - certificate_directory, ca, ca_key, "spiffe-target", "spiffe://tap/target", 5928004, - spiffe_target_client); - if (spiffe_target_client_ready) { - ok(true, "Trusted SPIFFE target client certificate generated and verified"); - } else { - ok(true, "Trusted SPIFFE target client certificate generated and verified # SKIP custom CA cannot sign the SPIFFE target certificate"); - } + const frontend_certificate_fixtures fixtures = create_frontend_certificate_fixtures( + certificate_directory, ca, ca_key); - ok(try_frontend_connect(cl, USER_NONE, PASSWORD, false) == 0, - "No require_x509 attribute permits plaintext authentication"); - ok(try_frontend_connect(cl, USER_NONE, PASSWORD, true) == 0, - "No require_x509 attribute permits TLS authentication without a client certificate"); - ok(try_frontend_connect(cl, USER_FALSE, PASSWORD, true) == 0, - "require_x509=false permits TLS authentication without a client certificate"); - ok(try_frontend_connect(cl, USER_REQUIRED, PASSWORD, false) == ER_ACCESS_DENIED_ERROR, - "require_x509=true rejects plaintext authentication with ER_ACCESS_DENIED_ERROR"); - ok(try_frontend_connect(cl, USER_REQUIRED, PASSWORD, true) == ER_ACCESS_DENIED_ERROR, - "require_x509=true rejects TLS authentication without a client certificate with ER_ACCESS_DENIED_ERROR"); - ok(untrusted_client_ready && - try_frontend_connect(cl, USER_REQUIRED, PASSWORD, true, &untrusted_client) == ER_ACCESS_DENIED_ERROR, - "require_x509=true rejects an untrusted client certificate with ER_ACCESS_DENIED_ERROR"); - if (trusted_client_ready) { - ok(try_frontend_connect(cl, USER_REQUIRED, PASSWORD, true, &trusted_client) == 0, - "require_x509=true accepts a trusted client certificate without a SAN"); - ok(try_frontend_connect(cl, USER_REQUIRED, WRONG_PASSWORD, true, &trusted_client) == ER_ACCESS_DENIED_ERROR, - "require_x509=true still rejects a wrong password with ER_ACCESS_DENIED_ERROR"); - ok(try_frontend_connect(cl, USER_BAD_TYPE, PASSWORD, true, &trusted_client) == ER_ACCESS_DENIED_ERROR, - "string require_x509=true fails closed with ER_ACCESS_DENIED_ERROR"); - } else { - ok(true, "require_x509 trusted certificate success # SKIP trusted certificate fixture unavailable"); - ok(true, "require_x509 trusted certificate wrong-password rejection # SKIP trusted certificate fixture unavailable"); - ok(true, "string require_x509=true fails closed # SKIP trusted certificate fixture unavailable"); - } - - { - mysql_ptr source { connect_frontend(cl, USER_CHANGE_SOURCE, CHANGE_SOURCE_PASSWORD, false) }; - ok(source && try_change_user(source.get(), USER_CHANGE_TARGET, CHANGE_TARGET_PASSWORD) == ER_ACCESS_DENIED_ERROR, - "COM_CHANGE_USER from plaintext rejects require_x509=true with ER_ACCESS_DENIED_ERROR"); - } - { - mysql_ptr source { connect_frontend(cl, USER_CHANGE_SOURCE, CHANGE_SOURCE_PASSWORD, true) }; - // Reconnecting with trusted_client succeeds below; CHANGE_USER cannot acquire a certificate on this TLS connection. - ok(source && try_change_user(source.get(), USER_CHANGE_TARGET, CHANGE_TARGET_PASSWORD) == ER_ACCESS_DENIED_ERROR, - "COM_CHANGE_USER from TLS without a client certificate rejects require_x509=true with ER_ACCESS_DENIED_ERROR"); - } - { - mysql_ptr source { connect_frontend(cl, USER_CHANGE_SOURCE, CHANGE_SOURCE_PASSWORD, true, &untrusted_client) }; - ok(untrusted_client_ready && source && - try_change_user(source.get(), USER_CHANGE_TARGET, CHANGE_TARGET_PASSWORD) == ER_ACCESS_DENIED_ERROR, - "COM_CHANGE_USER from an untrusted client certificate rejects require_x509=true with ER_ACCESS_DENIED_ERROR"); - } - if (trusted_client_ready) { - mysql_ptr source { connect_frontend(cl, USER_CHANGE_SOURCE, CHANGE_SOURCE_PASSWORD, true, &trusted_client) }; - ok(source && try_change_user(source.get(), USER_CHANGE_TARGET, CHANGE_TARGET_PASSWORD) == 0, - "COM_CHANGE_USER from a trusted client certificate accepts require_x509=true"); - } else { - ok(true, "COM_CHANGE_USER trusted certificate require_x509 success # SKIP trusted certificate fixture unavailable"); - } - if (trusted_client_ready) { - mysql_ptr source { connect_frontend(cl, USER_CHANGE_SOURCE, CHANGE_SOURCE_PASSWORD, true, &trusted_client) }; - ok(source && try_change_user(source.get(), USER_CHANGE_TARGET, WRONG_PASSWORD) == ER_ACCESS_DENIED_ERROR, - "COM_CHANGE_USER require_x509=true still rejects a wrong target password with ER_ACCESS_DENIED_ERROR"); - } else { - ok(true, "COM_CHANGE_USER trusted certificate wrong-password rejection # SKIP trusted certificate fixture unavailable"); - } - if (trusted_client_ready) { - mysql_ptr source { connect_frontend(cl, USER_CHANGE_SOURCE, CHANGE_SOURCE_PASSWORD, true, &trusted_client) }; - ok(source && try_change_user(source.get(), USER_NONE, PASSWORD) == 0, - "COM_CHANGE_USER from a trusted client certificate accepts an ordinary password target"); - } else { - ok(true, "COM_CHANGE_USER ordinary target control # SKIP trusted certificate fixture unavailable"); - } - if (spiffe_source_client_ready) { - mysql_ptr source { connect_frontend(cl, USER_SPIFFE_SOURCE, "", true, &spiffe_source_client) }; - ok(source && try_change_user(source.get(), USER_NONE, PASSWORD) == ER_ACCESS_DENIED_ERROR, - "COM_CHANGE_USER rejects a SPIFFE-authenticated source identity with ER_ACCESS_DENIED_ERROR"); - } else { - ok(true, "COM_CHANGE_USER SPIFFE-authenticated source rejection # SKIP trusted SPIFFE source fixture unavailable"); - } - if (spiffe_target_client_ready) { - mysql_ptr source { connect_frontend(cl, USER_CHANGE_SOURCE, CHANGE_SOURCE_PASSWORD, true, &spiffe_target_client) }; - ok(source && try_change_user(source.get(), USER_SPIFFE_TARGET, "") == ER_ACCESS_DENIED_ERROR, - "COM_CHANGE_USER rejects a SPIFFE target with ER_ACCESS_DENIED_ERROR"); - } else { - ok(true, "COM_CHANGE_USER SPIFFE target rejection # SKIP trusted SPIFFE target fixture unavailable"); - } + run_initial_login_checks( + cl, fixtures.trusted_client_ready, fixtures.untrusted_client_ready, + fixtures.trusted_client, fixtures.untrusted_client); + run_change_user_checks( + cl, fixtures.trusted_client_ready, fixtures.untrusted_client_ready, + fixtures.spiffe_source_client_ready, fixtures.spiffe_target_client_ready, + fixtures.trusted_client, fixtures.untrusted_client, + fixtures.spiffe_source_client, fixtures.spiffe_target_client); const bool users_cleaned = do_query(admin.get(), "DELETE FROM mysql_users WHERE username IN (" + user_list + ")") && do_query(admin.get(), "LOAD MYSQL USERS TO RUNTIME"); diff --git a/test/tap/tests/test_frontend_x509_passthrough-t.cpp b/test/tap/tests/test_frontend_x509_passthrough-t.cpp index ac6a5ca101..413c112509 100644 --- a/test/tap/tests/test_frontend_x509_passthrough-t.cpp +++ b/test/tap/tests/test_frontend_x509_passthrough-t.cpp @@ -120,6 +120,141 @@ static int restore_server_ssl_states(MYSQL* admin, const vector= 0 && probes_after == probes_before, "%s leaves probes_attempted unchanged (%ld -> %ld)", label, probes_before, probes_after); + ok(cache_before == 0 && cache_after == 0, "%s creates no pass-through cache entry (%d -> %d)", label, cache_before, cache_after); +} + +static void run_pass_through_row_checks( + MYSQL* admin, + const CommandLine& cl, + bool trusted_ready, + bool untrusted_ready, + const client_tls_material& trusted_client, + const client_tls_material& untrusted_client +) { + expect_pass_through_rejection(admin, cl, "Cold TLS without client certificate", nullptr); + if (untrusted_ready) { + expect_pass_through_rejection( + admin, cl, "Cold TLS with untrusted client certificate", &untrusted_client); + } else { + emit_fixture_skips(3, "Cold untrusted-certificate controls"); + } + + if (trusted_ready) { + do_query(admin, "PROXYSQL FLUSH PASSTHROUGH_AUTH_CACHE"); + const int64_t probes_before = read_metric(admin, "probes_attempted"); + const unsigned int err = try_frontend_connect(cl, PT_USER, WRONG_PASSWORD, true, &trusted_client); + const int64_t probes_after = read_metric(admin, "probes_attempted"); + ok(err == ER_ACCESS_DENIED_ERROR, "Trusted certificate with wrong backend password returns 1045 (errno=%u)", err); + ok(probes_before >= 0 && probes_after == probes_before + 1, "Trusted wrong-password probe increments probes_attempted exactly once (%ld -> %ld)", probes_before, probes_after); + ok(cache_entries_for(admin, PT_USER) == 0, "Trusted wrong-password probe leaves cache empty"); + + do_query(admin, "PROXYSQL FLUSH PASSTHROUGH_AUTH_CACHE"); + const int64_t correct_before = read_metric(admin, "probes_attempted"); + const unsigned int correct_err = try_frontend_connect(cl, PT_USER, PT_PASSWORD, true, &trusted_client); + const int64_t correct_after = read_metric(admin, "probes_attempted"); + ok(correct_err == 0, "Trusted no-SAN certificate with correct backend password succeeds (errno=%u)", correct_err); + ok(correct_before >= 0 && correct_after == correct_before + 1, "Trusted correct-password probe increments probes_attempted exactly once (%ld -> %ld)", correct_before, correct_after); + ok(cache_entries_for(admin, PT_USER) == 1, "Trusted correct-password probe creates one cache entry"); + + const int64_t hits_before = read_metric(admin, "cache_hits"); + const unsigned int no_cert_warm_err = try_frontend_connect(cl, PT_USER, PT_PASSWORD, true); + const int64_t hits_after = read_metric(admin, "cache_hits"); + ok(no_cert_warm_err == ER_ACCESS_DENIED_ERROR, "Warm TLS without a client certificate returns 1045 (errno=%u)", no_cert_warm_err); + ok(hits_before >= 0 && hits_after == hits_before, "Warm no-certificate denial leaves cache_hits unchanged (%ld -> %ld)", hits_before, hits_after); + + const int64_t trusted_hits_before = read_metric(admin, "cache_hits"); + const unsigned int trusted_warm_err = try_frontend_connect(cl, PT_USER, PT_PASSWORD, true, &trusted_client); + const int64_t trusted_hits_after = read_metric(admin, "cache_hits"); + ok(trusted_warm_err == 0, "Warm trusted certificate succeeds (errno=%u)", trusted_warm_err); + ok(trusted_hits_before >= 0 && trusted_hits_after == trusted_hits_before + 1, "Warm trusted certificate increments cache_hits exactly once (%ld -> %ld)", trusted_hits_before, trusted_hits_after); + } else { + emit_fixture_skips(10, "Trusted pass-through control"); + } +} + +static void expect_spiffe_rejection( + MYSQL* admin, + const CommandLine& cl, + const char* label, + const client_tls_material* identity +) { + const int64_t probes_before = read_metric(admin, "probes_attempted"); + const unsigned int err = try_frontend_connect(cl, SPIFFE_USER, "", true, identity); + const int64_t probes_after = read_metric(admin, "probes_attempted"); + ok(err == ER_ACCESS_DENIED_ERROR, "%s returns generic 1045 (errno=%u)", label, err); + ok(probes_before >= 0 && probes_after == probes_before, "%s leaves probes_attempted unchanged (%ld -> %ld)", label, probes_before, probes_after); + ok(cache_entries_for(admin, SPIFFE_USER) == 0, "%s creates no SPIFFE cache entry", label); +} + +static void run_spiffe_row_checks( + MYSQL* admin, + const CommandLine& cl, + bool trusted_ready, + bool spiffe_ready, + const client_tls_material& trusted_client, + const client_tls_material& spiffe_client +) { + if (spiffe_ready) { + const int64_t probes_before = read_metric(admin, "probes_attempted"); + const unsigned int err = try_frontend_connect(cl, SPIFFE_USER, "", true, &spiffe_client); + const int64_t probes_after = read_metric(admin, "probes_attempted"); + ok(err == 0, "Matching SPIFFE URI-SAN with empty password succeeds (errno=%u)", err); + ok(probes_before >= 0 && probes_after == probes_before, "Matching SPIFFE path leaves probes_attempted unchanged (%ld -> %ld)", probes_before, probes_after); + ok(cache_entries_for(admin, SPIFFE_USER) == 0, "Matching SPIFFE path creates no cache entry"); + } else { + emit_fixture_skips(3, "Matching SPIFFE path"); + } + + expect_spiffe_rejection(admin, cl, "SPIFFE row without client certificate", nullptr); + if (trusted_ready) { + expect_spiffe_rejection( + admin, cl, "SPIFFE row with mismatching trusted no-SAN certificate", &trusted_client); + } else { + emit_fixture_skips(3, "Mismatching SPIFFE path"); + } +} + +static void run_change_user_direction_checks( + MYSQL* admin, + const CommandLine& cl, + bool trusted_ready, + const client_tls_material& trusted_client +) { + if (trusted_ready) { + mysql_ptr ordinary { connect_frontend(cl, cl.username, cl.password, true, &trusted_client) }; + const int64_t probes_before = read_metric(admin, "probes_attempted"); + const int rc = ordinary ? mysql_change_user(ordinary.get(), PT_USER, PT_PASSWORD, nullptr) : -1; + const unsigned int err = ordinary ? mysql_errno(ordinary.get()) : UINT_MAX; + const int64_t probes_after = read_metric(admin, "probes_attempted"); + ok(rc != 0 && err == ER_ACCESS_DENIED_ERROR, "COM_CHANGE_USER to pass-through target remains generic 1045 (rc=%d errno=%u)", rc, err); + ok(probes_before >= 0 && probes_after == probes_before, "COM_CHANGE_USER pass-through target does not create a probe (%ld -> %ld)", probes_before, probes_after); + + mysql_ptr pass_through { connect_frontend(cl, PT_USER, PT_PASSWORD, true, &trusted_client) }; + const int direction_rc = pass_through ? mysql_change_user(pass_through.get(), PT_TARGET, TARGET_PASSWORD, nullptr) : -1; + ok(direction_rc == 0, "Pass-through-authenticated source can COM_CHANGE_USER to ordinary target (rc=%d)", direction_rc); + } else { + emit_fixture_skips(3, "COM_CHANGE_USER pass-through directionality"); + } +} + int main() { CommandLine cl; const char* const datadir_env = getenv("REGULAR_INFRA_DATADIR"); @@ -207,7 +342,7 @@ int main() { const string datadir { datadir_env }; const string ca { datadir + "/proxysql-ca.pem" }; const string ca_key { datadir + "/proxysql-key.pem" }; - temporary_certificate_directory certificate_directory; + temporary_certificate_directory certificate_directory { datadir }; client_tls_material trusted_client; client_tls_material untrusted_client; client_tls_material spiffe_client; @@ -229,100 +364,11 @@ int main() { ok(true, "Generated trusted SPIFFE URI-SAN certificate # SKIP custom CA cannot sign fixture"); } - const auto expect_rejected_without_side_effects = [&](const char* label, const client_tls_material* identity) { - do_query(admin.get(), "PROXYSQL FLUSH PASSTHROUGH_AUTH_CACHE"); - const int64_t probes_before = read_metric(admin.get(), "probes_attempted"); - const int cache_before = cache_entries_for(admin.get(), PT_USER); - const unsigned int err = try_frontend_connect(cl, PT_USER, PT_PASSWORD, true, identity); - const int64_t probes_after = read_metric(admin.get(), "probes_attempted"); - const int cache_after = cache_entries_for(admin.get(), PT_USER); - ok(err == ER_ACCESS_DENIED_ERROR, "%s returns generic 1045 (errno=%u)", label, err); - ok(probes_before >= 0 && probes_after == probes_before, "%s leaves probes_attempted unchanged (%ld -> %ld)", label, probes_before, probes_after); - ok(cache_before == 0 && cache_after == 0, "%s creates no pass-through cache entry (%d -> %d)", label, cache_before, cache_after); - }; - - expect_rejected_without_side_effects("Cold TLS without client certificate", nullptr); - if (untrusted_ready) { - expect_rejected_without_side_effects("Cold TLS with untrusted client certificate", &untrusted_client); - } else { - ok(true, "Cold TLS with untrusted client certificate returns 1045 # SKIP fixture unavailable"); - ok(true, "Cold untrusted certificate leaves probes_attempted unchanged # SKIP fixture unavailable"); - ok(true, "Cold untrusted certificate creates no cache entry # SKIP fixture unavailable"); - } - - if (trusted_ready) { - do_query(admin.get(), "PROXYSQL FLUSH PASSTHROUGH_AUTH_CACHE"); - const int64_t probes_before = read_metric(admin.get(), "probes_attempted"); - const unsigned int err = try_frontend_connect(cl, PT_USER, WRONG_PASSWORD, true, &trusted_client); - const int64_t probes_after = read_metric(admin.get(), "probes_attempted"); - ok(err == ER_ACCESS_DENIED_ERROR, "Trusted certificate with wrong backend password returns 1045 (errno=%u)", err); - ok(probes_before >= 0 && probes_after == probes_before + 1, "Trusted wrong-password probe increments probes_attempted exactly once (%ld -> %ld)", probes_before, probes_after); - ok(cache_entries_for(admin.get(), PT_USER) == 0, "Trusted wrong-password probe leaves cache empty"); - - do_query(admin.get(), "PROXYSQL FLUSH PASSTHROUGH_AUTH_CACHE"); - const int64_t correct_before = read_metric(admin.get(), "probes_attempted"); - const unsigned int correct_err = try_frontend_connect(cl, PT_USER, PT_PASSWORD, true, &trusted_client); - const int64_t correct_after = read_metric(admin.get(), "probes_attempted"); - ok(correct_err == 0, "Trusted no-SAN certificate with correct backend password succeeds (errno=%u)", correct_err); - ok(correct_before >= 0 && correct_after == correct_before + 1, "Trusted correct-password probe increments probes_attempted exactly once (%ld -> %ld)", correct_before, correct_after); - ok(cache_entries_for(admin.get(), PT_USER) == 1, "Trusted correct-password probe creates one cache entry"); - - const int64_t hits_before = read_metric(admin.get(), "cache_hits"); - const unsigned int no_cert_warm_err = try_frontend_connect(cl, PT_USER, PT_PASSWORD, true); - const int64_t hits_after = read_metric(admin.get(), "cache_hits"); - ok(no_cert_warm_err == ER_ACCESS_DENIED_ERROR, "Warm TLS without a client certificate returns 1045 (errno=%u)", no_cert_warm_err); - ok(hits_before >= 0 && hits_after == hits_before, "Warm no-certificate denial leaves cache_hits unchanged (%ld -> %ld)", hits_before, hits_after); - - const int64_t trusted_hits_before = read_metric(admin.get(), "cache_hits"); - const unsigned int trusted_warm_err = try_frontend_connect(cl, PT_USER, PT_PASSWORD, true, &trusted_client); - const int64_t trusted_hits_after = read_metric(admin.get(), "cache_hits"); - ok(trusted_warm_err == 0, "Warm trusted certificate succeeds (errno=%u)", trusted_warm_err); - ok(trusted_hits_before >= 0 && trusted_hits_after == trusted_hits_before + 1, "Warm trusted certificate increments cache_hits exactly once (%ld -> %ld)", trusted_hits_before, trusted_hits_after); - } else { - for (int i = 0; i != 11; ++i) ok(true, "Trusted pass-through control # SKIP trusted certificate fixture unavailable"); - } - - if (spiffe_ready) { - const int64_t probes_before = read_metric(admin.get(), "probes_attempted"); - const unsigned int err = try_frontend_connect(cl, SPIFFE_USER, "", true, &spiffe_client); - const int64_t probes_after = read_metric(admin.get(), "probes_attempted"); - ok(err == 0, "Matching SPIFFE URI-SAN with empty password succeeds (errno=%u)", err); - ok(probes_before >= 0 && probes_after == probes_before, "Matching SPIFFE path leaves probes_attempted unchanged (%ld -> %ld)", probes_before, probes_after); - ok(cache_entries_for(admin.get(), SPIFFE_USER) == 0, "Matching SPIFFE path creates no cache entry"); - } else { - for (int i = 0; i != 3; ++i) ok(true, "Matching SPIFFE path # SKIP trusted SPIFFE fixture unavailable"); - } - - const auto expect_spiffe_rejection = [&](const char* label, const client_tls_material* identity) { - const int64_t probes_before = read_metric(admin.get(), "probes_attempted"); - const unsigned int err = try_frontend_connect(cl, SPIFFE_USER, "", true, identity); - const int64_t probes_after = read_metric(admin.get(), "probes_attempted"); - ok(err == ER_ACCESS_DENIED_ERROR, "%s returns generic 1045 (errno=%u)", label, err); - ok(probes_before >= 0 && probes_after == probes_before, "%s leaves probes_attempted unchanged (%ld -> %ld)", label, probes_before, probes_after); - ok(cache_entries_for(admin.get(), SPIFFE_USER) == 0, "%s creates no SPIFFE cache entry", label); - }; - expect_spiffe_rejection("SPIFFE row without client certificate", nullptr); - if (trusted_ready) { - expect_spiffe_rejection("SPIFFE row with mismatching trusted no-SAN certificate", &trusted_client); - } else { - for (int i = 0; i != 3; ++i) ok(true, "Mismatching SPIFFE path # SKIP trusted certificate fixture unavailable"); - } - - if (trusted_ready) { - mysql_ptr ordinary { connect_frontend(cl, cl.username, cl.password, true, &trusted_client) }; - const int64_t probes_before = read_metric(admin.get(), "probes_attempted"); - const int rc = ordinary ? mysql_change_user(ordinary.get(), PT_USER, PT_PASSWORD, nullptr) : -1; - const unsigned int err = ordinary ? mysql_errno(ordinary.get()) : UINT_MAX; - const int64_t probes_after = read_metric(admin.get(), "probes_attempted"); - ok(rc != 0 && err == ER_ACCESS_DENIED_ERROR, "COM_CHANGE_USER to pass-through target remains generic 1045 (rc=%d errno=%u)", rc, err); - ok(probes_before >= 0 && probes_after == probes_before, "COM_CHANGE_USER pass-through target does not create a probe (%ld -> %ld)", probes_before, probes_after); - - mysql_ptr pass_through { connect_frontend(cl, PT_USER, PT_PASSWORD, true, &trusted_client) }; - const int direction_rc = pass_through ? mysql_change_user(pass_through.get(), PT_TARGET, TARGET_PASSWORD, nullptr) : -1; - ok(direction_rc == 0, "Pass-through-authenticated source can COM_CHANGE_USER to ordinary target (rc=%d)", direction_rc); - } else { - for (int i = 0; i != 3; ++i) ok(true, "COM_CHANGE_USER pass-through directionality # SKIP trusted certificate fixture unavailable"); - } + run_pass_through_row_checks( + admin.get(), cl, trusted_ready, untrusted_ready, trusted_client, untrusted_client); + run_spiffe_row_checks( + admin.get(), cl, trusted_ready, spiffe_ready, trusted_client, spiffe_client); + run_change_user_direction_checks(admin.get(), cl, trusted_ready, trusted_client); int cleanup_rc = EXIT_SUCCESS; cleanup_rc |= do_query(backend.get(), string("DROP USER IF EXISTS '") + PT_USER + "'@'%'"); From 4d472c4be82a03b97fe21f70b34bc401e1eda33c Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Mon, 10 Aug 2026 23:02:59 +0000 Subject: [PATCH 11/11] refactor: use RAII for frontend SAN identity --- .../2026-08-10-frontend-x509-authentication.md | 2 +- include/MySQL_Data_Stream.h | 4 +++- lib/MySQL_Protocol.cpp | 12 ++++++------ lib/mysql_data_stream.cpp | 14 ++++++-------- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/docs/superpowers/plans/2026-08-10-frontend-x509-authentication.md b/docs/superpowers/plans/2026-08-10-frontend-x509-authentication.md index 63cb3e45b0..1a2aae2b21 100644 --- a/docs/superpowers/plans/2026-08-10-frontend-x509-authentication.md +++ b/docs/superpowers/plans/2026-08-10-frontend-x509-authentication.md @@ -196,7 +196,7 @@ static unsigned int try_frontend_connect( Add immutable-for-the-connection evidence beside `x509_subject_alt_name`: ```cpp -char *x509_subject_alt_name; +std::unique_ptr x509_subject_alt_name; #ifdef PROXYSQL31 bool client_cert_present; long client_cert_verify_result; diff --git a/include/MySQL_Data_Stream.h b/include/MySQL_Data_Stream.h index 432baeb03d..838f602fad 100644 --- a/include/MySQL_Data_Stream.h +++ b/include/MySQL_Data_Stream.h @@ -1,6 +1,8 @@ #ifndef PROXYSQL_MYSQL_DATA_STREAM_H #define PROXYSQL_MYSQL_DATA_STREAM_H +#include + #include "proxysql.h" #include "cpp.h" @@ -130,7 +132,7 @@ class MySQL_Data_Stream MySQL_Connection *myconn; MySQL_Session *sess; // pointer to the session using this data stream MySQL_Backend *mybe; // if this is a connection to a mysql server, this points to a backend structure - char *x509_subject_alt_name; + std::unique_ptr x509_subject_alt_name; #ifdef PROXYSQL31 bool client_cert_present; long client_cert_verify_result; diff --git a/lib/MySQL_Protocol.cpp b/lib/MySQL_Protocol.cpp index 4c395dc63f..c1b3075a34 100644 --- a/lib/MySQL_Protocol.cpp +++ b/lib/MySQL_Protocol.cpp @@ -142,10 +142,10 @@ static bool spiffe_identity_matches(MySQL_Data_Stream* myds, const std::string& const string pattern { expected.substr(1) }; re2::RE2::Options opts { re2::RE2::Quiet }; re2::RE2 subject_alt_regex(pattern, opts); - return re2::RE2::FullMatch(myds->x509_subject_alt_name, subject_alt_regex); + return re2::RE2::FullMatch(myds->x509_subject_alt_name.get(), subject_alt_regex); } return expected.rfind("spiffe://", 0) == 0 - && expected == myds->x509_subject_alt_name; + && expected == myds->x509_subject_alt_name.get(); } static bool evaluate_spiffe_identity( @@ -171,7 +171,7 @@ static bool evaluate_spiffe_identity( if (!allowed) { proxy_error("%d:%s(): SPIFFE Authentication error for user %s . spiffed_id expected : %s , received: %s\n", calling_line, calling_func, username, expected.c_str(), - (myds && myds->x509_subject_alt_name) ? myds->x509_subject_alt_name : "none"); + (myds && myds->x509_subject_alt_name) ? myds->x509_subject_alt_name.get() : "none"); } return allowed; } @@ -3823,15 +3823,15 @@ bool MySQL_Protocol::verify_user_attributes(int calling_line, const char *callin re2::RE2::Options opts = re2::RE2::Options(RE2::Quiet); re2::RE2 subject_alt_regex(str_spiffe_regex, opts); - ret = re2::RE2::FullMatch((*myds)->x509_subject_alt_name, subject_alt_regex); + ret = re2::RE2::FullMatch((*myds)->x509_subject_alt_name.get(), subject_alt_regex); } else if (strncmp(spiffe_val.c_str(), "spiffe://", strlen("spiffe://"))==0) { - if (strcmp(spiffe_val.c_str(), (*myds)->x509_subject_alt_name)==0) { + if (strcmp(spiffe_val.c_str(), (*myds)->x509_subject_alt_name.get())==0) { ret = true; } } } if (ret == false) { - proxy_error("%d:%s(): SPIFFE Authentication error for user %s . spiffed_id expected : %s , received: %s\n", calling_line, calling_func, user, spiffe_val.c_str(), ((*myds)->x509_subject_alt_name ? (*myds)->x509_subject_alt_name : "none")); + proxy_error("%d:%s(): SPIFFE Authentication error for user %s . spiffed_id expected : %s , received: %s\n", calling_line, calling_func, user, spiffe_val.c_str(), ((*myds)->x509_subject_alt_name ? (*myds)->x509_subject_alt_name.get() : "none")); } } auto default_transaction_isolation = j.find("default-transaction_isolation"); diff --git a/lib/mysql_data_stream.cpp b/lib/mysql_data_stream.cpp index 2aea16907e..54c1a4b20a 100644 --- a/lib/mysql_data_stream.cpp +++ b/lib/mysql_data_stream.cpp @@ -218,7 +218,7 @@ void MySQL_Data_Stream::queue_encrypted_bytes(const char *buf, size_t len) { //proxy_info("New ssl_write_len size: %u\n", ssl_write_len); } -static char* extract_first_spiffe_uri(const GENERAL_NAMES* alt_names) { +static std::unique_ptr extract_first_spiffe_uri(const GENERAL_NAMES* alt_names) { static constexpr char SPIFFE_PREFIX[] = "spiffe"; const int alt_name_count = sk_GENERAL_NAME_num(alt_names); @@ -233,9 +233,8 @@ static char* extract_first_spiffe_uri(const GENERAL_NAMES* alt_names) { if (memcmp(data, SPIFFE_PREFIX, sizeof(SPIFFE_PREFIX) - 1) != 0) continue; if (memchr(data, '\0', length) != nullptr) continue; - char* value = new (std::nothrow) char[static_cast(length) + 1]; - if (!value) return nullptr; - memcpy(value, data, length); + auto value = std::make_unique(static_cast(length) + 1); + memcpy(value.get(), data, length); value[length] = '\0'; return value; } @@ -270,7 +269,7 @@ enum sslstatus MySQL_Data_Stream::do_ssl_handshake() { } // In case the supplied certificate has a 'SAN'-'URI' identifier // starting with 'spiffe', client certificate verification is performed. - if (x509_subject_alt_name != NULL && SSL_get_verify_result(ssl) != X509_V_OK) { + if (x509_subject_alt_name && SSL_get_verify_result(ssl) != X509_V_OK) { long rc = SSL_get_verify_result(ssl); proxy_error("Disconnecting %s:%d: X509 client SSL certificate verify error: (%ld:%s)\n" , addr.addr, addr.port, rc, X509_verify_cert_error_string(rc)); return SSLSTATUS_FAIL; @@ -356,7 +355,7 @@ MySQL_Data_Stream::MySQL_Data_Stream() { auth_in_progress = 0; passthrough_cleartext = NULL; tmp_charset = 0; - x509_subject_alt_name=NULL; + x509_subject_alt_name = nullptr; #ifdef PROXYSQL31 client_cert_present=false; client_cert_verify_result=X509_V_OK; @@ -508,8 +507,7 @@ MySQL_Data_Stream::~MySQL_Data_Stream() { } void MySQL_Data_Stream::reset_frontend_certificate_evidence() { - delete[] x509_subject_alt_name; - x509_subject_alt_name = nullptr; + x509_subject_alt_name.reset(); #ifdef PROXYSQL31 client_cert_present = false; client_cert_verify_result = X509_V_OK;