11 Commits (docs/passthrough-auth-spec)

Author	SHA1	Message	Date
Rene Cannao	f85f22681f	docs(passthrough-auth): align evict_oldest comment with actual behavior ... The previous comment said "if after sweeping zombies we're still above the cap, drop the oldest real entry" -- implying a conditional second eviction. The code unconditionally erases the oldest real entry after the zombie sweep, regardless of whether the sweep alone brought size below the cap. Functionally this is defensible: evict_oldest is called only when size() > cap at the call site, so we're already over the limit when the function runs. But the comment mismatch was caught by the round-4 fix-verification subagent (R4-nit) and the holistic- readiness subagent both noted that the COMMENT was misleading even if the behavior is acceptable. Update the comment to: - state plainly that phase 2 is unconditional given the call-site contract - explain the trade-off (slight over-eviction under zombie-heavy workloads vs. simpler API) - flag a Phase-2 alternative if real-world telemetry shows the over-eviction matters - preserve the original observation that the just-inserted entry is never the eviction target No code change.	1 month ago
Rene Cannao	4ee7543f52	feat(passthrough-auth): mysql-passthrough_auth_failure_map_cap variable ... The failure-counter map size limit was a hardcoded FAILURE_MAP_HARD_CAP=100000 constant. The round-3 production- readiness review flagged this as Phase-2 work: operators on unusually-sized hosts (very large or very small memory budgets) have no way to tune it without rebuilding. Promote to an operator-tunable variable @c mysql-passthrough_auth_failure_map_cap with the same default (100000) and range [1000 .. 100M]. Threaded through: include/MySQL_Thread.h struct field include/proxysql_structs.h thread-local + extern lib/MySQL_Thread.cpp: - registered in mysql_thread_variables_names - init in init_variables - VariablesPointers_int entry with [1000 .. 100M] range - REFRESH_VARIABLE_INT in refresh_variables include/MySQL_Passthrough_Auth_Cache.h record_failure signature gains @c max_keys lib/MySQL_Passthrough_Auth_Cache.cpp uses parameter instead of the constant lib/MySQL_Session.cpp call site passes mysql_thread___... etc/proxysql.cnf operator-visible doc Default unchanged at 100000; existing deployments see no behavior change. Operators who need to tune it can do so via SET / LOAD MYSQL VARIABLES TO RUNTIME like any other passthrough variable. P2-B from the round-3 review of PR #5810.	1 month ago
Rene Cannao	4b923f6361	perf(passthrough-auth): rwlock fast-path for cache lookup ... lookup() previously took the write lock unconditionally because of the TTL-eviction branch. Cache reads happen on every passthrough- eligible client connect (the cache is read BEFORE we know whether to dispatch a probe), so the write lock serializes worker threads in the most common case: cached entry present, not expired. Split into the standard double-checked-locking pattern: Reader fast-path (the dominant case): - rdlock the cache - find() the entry - if missing → unlock and return false (miss) - if found and NOT expired → copy out the cleartext, unlock, return true (hit). Multiple concurrent readers run in parallel. Writer slow-path (entry expired, requires eviction): - drop rdlock, acquire wrlock - re-check the entry under wrlock: between the lock release and re-acquire another thread could have evicted it OR inserted a fresh entry. Handle both: + still missing → return false + still expired → erase and return false + now fresh → return the new cleartext (race-resolved hit) - unlock Doesn't change observable behavior. The fast path is now read- parallel across worker threads; only TTL-expired-entry lookups take the write lock, and those are rare relative to total lookup rate. P2-A from the round-3 review of PR #5810. Matches the same pattern already applied to username_allowed's pattern_lock in commit c73d1be69.	1 month ago
Rene Cannao	ee6d1dc466	fix(passthrough-auth): evict_oldest sweeps empty-deque zombies ... Under a sustained-churn attack (each unique username/IP appears exactly once before the window expires), failure-counter map entries accumulate. The would_lockout_* path's erase_if_empty fires only on the specific key being checked, and a one-shot key is never checked twice, so its zombie deque (after the window expires) lingers indefinitely. When the map then reaches FAILURE_MAP_HARD_CAP, the previous evict_oldest's "find the oldest non-empty deque" search would skip every zombie (their front() check would crash, so we correctly filter to non-empty). With every other deque empty, the only non-empty candidate IS the entry we just inserted via record_failure -- and evict_oldest would discard the freshly-recorded failure signal. Result before this fix: under exactly the workload the cap was meant to defend against (username-churn credential stuffing), the lockout state is silently dropped. Fix: split the scan into two phases under failure_lock. Phase 1 -- "find oldest non-empty AND collect every empty deque encountered". Iterator-safe (we don't erase mid-iteration; we collect iterators into a local vector). Phase 2 -- reclaim every empty zombie (the work erase_if_empty would have done lazily if would_lockout_* had ever revisited those keys), then if we're still above the cap drop the oldest real entry. The just-inserted entry has the latest timestamp at its front(), so it can only be evicted in the degenerate case where every other entry has been swept (i.e. the cap was never really exceeded by genuine activity). Worst-case complexity stays O(N) per record_failure call when the cap is hit (one pass to scan, one pass to erase, both bounded by the current map size). Best case is strictly better because subsequent evict_oldest calls walk a smaller map after zombies are reaped. Discovered as B5 by the new-code subagent in the round-3 review of PR #5810. Production-readiness rated this as a real correctness defect because it broke the lockout under attack.	1 month ago
Rene Cannao	8dfe921ad4	feat(passthrough-auth): counters + stats_mysql_passthrough_auth_metrics ... Adds operational-visibility metrics for the pass-through auth feature. Previously the only diagnostics available to an SRE were the audit log (optional, often off) and proxy_warning (added in the preceding commit). Both are good but neither lets an operator answer "is anything happening?" / "how often does pass-through succeed?" at a glance. Nine atomic counters added to MySQL_Passthrough_Auth_Cache, incremented at the single well-defined call site each: probes_attempted -- a real probe was about to run (past all eligibility gates, past inflight cap, past rate limits) probes_ok -- probe succeeded, cache populated probes_failed_credentials -- probe got 1045/1698/1130 from backend probes_failed_transport -- probe got 2xxx / unknown errno lockouts_user -- per-user lockout fired lockouts_ip -- per-IP lockout fired inflight_cap_rejects -- max_inflight_probes saturated cache_hits -- PPHR_verify_password used the cache cache_invalidations -- backend ER 1045 evicted a cached entry Plus two current-state gauges (read on demand from existing methods): inflight_probes -- current Glo->inflight() cache_entries -- current Glo->size() Exposed via a new admin virtual table: stats_mysql_passthrough_auth_metrics( metric_name VARCHAR NOT NULL PRIMARY KEY, metric_value BIGINT NOT NULL ) stats___mysql_passthrough_auth_metrics is registered alongside the existing stats___mysql_passthrough_auth_cache; both refresh from GloMyPTAuthCache on SELECT. All counter ops use std::memory_order_relaxed -- stats are advisory and don't need to synchronize with the increments. Counters are monotonic since process start; reset only by process restart. Phase 2 may add a Prometheus exposer alongside this; for now the admin table is the canonical surface for monitoring integrations. Discovered by the production-readiness subagent during the second deep review of PR #5810. Listed there as a blocker for production GA because operational visibility into pass-through misbehavior was effectively zero.	1 month ago
Rene Cannao	c73d1be692	fix(passthrough-auth): pattern_lock to rwlock with reader fast-path ... username_allowed previously took a pthread_mutex_t for the entire compile-or-FullMatch operation. The compile is rare (only on pattern-string change) but the FullMatch fires for every probe- eligible connect. Worker threads serialized through this single mutex; under high connect rate this is contention on a global lock per connection. Switch pattern_lock to pthread_rwlock and split the access: Reader fast-path (the dominant case): rdlock; if cached pattern still matches; FullMatch under rdlock (re2::RE2::FullMatch on a const RE2 is documented thread-safe); unlock; return. Many readers can run concurrently. Writer slow-path (rare): pattern changed -> wrlock; re-check under wrlock (another thread may have compiled while we were upgrading); compile if still needed; FullMatch; unlock; return. Serialized only with other writers and any in-flight reader. The re-check after wrlock acquisition is critical: between the rdlock release and wrlock acquire, another thread might have already compiled the pattern we wanted. Without the re-check we'd allocate, free, and re-assign needlessly (and could lose data if the third thread used the soon-to-be-deleted RE2 instance). Header bumps the lock type to pthread_rwlock_t; init/destroy adjusted accordingly. Discovered by the fix-verification and new-code-quality subagents during the second deep review of PR #5810. Flagged as a performance risk under high connect rate.	1 month ago
Rene Cannao	3b9bf35899	fix(passthrough-auth): bound failure-counter map growth ... Without this, the per-user / per-IP failure maps grow line-rate as an attacker churns random usernames/IPs (or a misconfigured client retries with new ephemeral IDs). Each unique key leaves an entry in the map; prune_and_count trimmed timestamps off the deque but never erased the map entry itself, even when the deque emptied. Long-uptime deployments under hostile traffic could exhaust memory. Two bounds added: 1. erase_if_empty on the would_lockout_* path The natural eviction point: as soon as we observe a deque has pruned down to empty, drop the map entry. Cost is one extra unordered_map::erase per query that prunes to empty; happens under the same failure_lock the check already holds. The map shrinks back down whenever traffic shifts and old keys' windows expire. 2. FAILURE_MAP_HARD_CAP defense-in-depth on the record_failure path When an attacker can sustain enough wrong-pw attempts per second to keep every distinct username/IP entry within its window (erase_if_empty cannot evict anything), the map can still grow to the population of distinct keys seen in window_s. The hard cap (100,000 entries) fires when the map exceeds that; we linearly scan for the oldest deque's front()-timestamp and drop it. We lose rate-limit state for one historical key; preferred to unbounded growth. Linear scan is fine because the eviction path is rare (happens only above the cap). The cap is well above any plausible legitimate active workload (an active 100k distinct usernames/IPs would be a meaningfully sized deployment) but well below the typical proxy host's memory ceiling (each entry is ~50 bytes for the string + deque overhead; 100k entries ≈ 5MB per map). Tunable in code only for Phase 1; operator-tunable variable could be added later if hosts with unusual sizing need it. Discovered by the production-readiness subagent during the second deep review of PR #5810.	1 month ago
Rene Cannao	38ac143f4d	fix(passthrough-auth): implement username_pattern allowlist ... mysql-passthrough_auth_username_pattern was registered as a global variable (spec §3.3) and documented as the primary mitigation for unknown-user enumeration (spec §7.1, security model), but never read anywhere in the auth path. Dead code: an admin could SET the pattern and observe no behavioral change. With passthrough_auth_unknown_users=true and a permissive default per-IP limit (10/60s), that was a meaningful enumeration surface. Implement: MySQL_Passthrough_Auth_Cache::username_allowed(username, pattern) Uses RE2 (already a project dependency, used by query rules) with FullMatch semantics. The compiled regex is cached on the class behind a dedicated pattern_lock; the cache key is the pattern string itself, so changing mysql-passthrough_auth_username_pattern at runtime picks up on the next probe attempt without a restart or FLUSH. Failure modes are fail-safe: - empty pattern -> allow all (preserves back-compat default) - non-compiling regex -> deny all (a misconfigured allowlist must NOT default to permitting every username; that would re-open exactly the surface the pattern exists to gate) Hooked into PPHR_verify_password BEFORE the cache lookup so a denied username cannot be probed AND cannot be enumerated by timing the cache-hit response vs the cache-miss response. Applies to both empty_pw_case and unknown_user_case -- admins may want to constrain which empty-pw rows can pass-through too, not just unknown-user opt-in. Discovered by the security, integration, AND spec-vs-impl subagents during the PR #5810 deep review -- triple convergence on "dead variable".	1 month ago
Rene Cannao	d8525aeb0c	feat(passthrough-auth): per-user / per-IP rate limiting ... Spec §7.2: sliding-window failure counters on each side of the pass-through probe path. When either threshold is exceeded the session is rejected with the generic 'Access denied' ERR and the backend probe is NOT issued. Without these counters ProxySQL would become an open credential-stuffing amplifier against the configured backends. MySQL_Passthrough_Auth_Cache::would_lockout_user(user, max, window) MySQL_Passthrough_Auth_Cache::would_lockout_ip(ip, max, window) MySQL_Passthrough_Auth_Cache::record_failure(user, ip) The data structures (failures_by_user / failures_by_ip) hold monotonic timestamps in std::deque keyed by username / source IP. Lookups lazily prune timestamps older than the configured window; no background sweeper needed in Phase 1. A separate mutex (failure_lock) protects the maps, distinct from the cache rwlock, since these are write-mostly on the failure path. Configuration (spec §3.3): mysql-passthrough_auth_max_failures_per_user (default 3) mysql-passthrough_auth_max_failures_per_ip (default 10) mysql-passthrough_auth_failure_window_s (default 60) Rejects that happen before the probe (lockout hit, inflight cap, missing inputs) do NOT record a failure — we don't extend the lockout indefinitely. Only an actual backend rejection of the probe records a new failure.	1 month ago
Rene Cannao	cd44e3f68e	feat(passthrough-auth): global in-flight probe cap ... Spec §7.3: bound concurrent backend probes. When the configured cap (mysql-passthrough_auth_max_inflight_probes, default 100) is reached, additional sessions wishing to start a probe receive a generic 'Access denied' ERR instead. MySQL_Passthrough_Auth_Cache::try_acquire_inflight(max) — atomic compare-and-increment; returns false when at the cap. MySQL_Passthrough_Auth_Cache::release_inflight() — pair with every successful acquire. MySQL_Passthrough_Auth_Cache::inflight() — current count, for future stats exposure. In the session handler the slot is held by a local RAII guard so every return path (auth success, backend rejected, init failure, no healthy backend, ...) releases cleanly. Also bounds same-username races: even without a per-username lock, the global cap limits how many concurrent probes for the same user can fire. Minor duplicate probing is accepted as a v1 trade-off (spec §7.3).	1 month ago
Rene Cannao	4f4d5f3493	feat(passthrough-auth): scaffold MySQL_Passthrough_Auth_Cache ... First commit of the pass-through authentication implementation (spec: doc/internal/passthrough_authentication.md, PR #5810). Adds an in-memory cache class that will later hold credentials learned via pass-through authentication: - include/MySQL_Passthrough_Auth_Cache.h — class definition with lookup/insert/evict/clear/snapshot API and lazy TTL expiry on lookup - lib/MySQL_Passthrough_Auth_Cache.cpp — implementation using an unordered_map keyed by username and a pthread_rwlock - src/main.cpp — singleton GloMyPTAuthCache constructed in ProxySQL_Main_init_Auth_module() and destroyed during shutdown - lib/Makefile — link the new object No consumers yet; the cache is constructed empty and not touched by the auth path. Library and binary build cleanly.	1 month ago