SCRAM verifier & md5 credential storage with SCRAM/md5 backend pass-through - PostgreSQL#5865
SCRAM verifier & md5 credential storage with SCRAM/md5 backend pass-through - PostgreSQL#5865rahim-kanji wants to merge 8 commits into
Conversation
…oor auth selection, anti-enumeration Store a SCRAM-SHA-256 verifier or md5 hash (or plaintext) in pgsql_users.password, detected by prefix. Pick the client auth method from the stored type via a minimum-strength floor (pgsql-authentication_method): a SCRAM verifier always uses SCRAM (no per-auth PBKDF2), an md5 hash uses md5, plaintext follows the floor; a secret too weak for the floor is rejected. Unknown / too-weak users run the existing SCRAM mock to a generic failure (no 'User not found' enumeration leak). Malformed SCRAM verifiers are rejected at LOAD. Pure B-floor logic lives in a dependency-free PgSQL_AuthReconcile.cpp. No libpq/backend changes (that is Phase 2).
…d5 auth pgsql_reconcile_unit-t: 9 cases over the B-floor reconcile matrix (unit-tests-g1). pgsql-verifier_auth-t: 8 integration assertions (legacy-g4) — verifier SCRAM auth, wrong-password reject, md5-hash auth, anti-enumeration, PLUS unselectable, malformed-verifier rejected at load.
…tication ProxySQL now authenticates to PostgreSQL backends using the stored verifier instead of a plaintext password. During the client's frontend SCRAM login the recovered ClientKey and the verifier's ServerKey are harvested onto the session's backend userinfo (skipped for adhoc/plaintext-derived keys, whose salt would not match the backend's rolpassword). On the backend connection PgSQL_Connection::connect_start injects them into the bundled libpq via new conninfo params (scram_client_key/scram_server_key), or md5_secret for md5-stored users; plaintext users keep the original password path unchanged. The bundled libpq is patched (deps/postgresql/scram_verifier_auth.patch, applied by deps/Makefile) to accept the new params: scram_client_key skips saslprep+PBKDF2 and derives StoredKey=SHA256(ClientKey); scram_server_key verifies the backend ServerSignature and fails closed if absent; md5_secret is used as the inner md5 hash. The keys are excluded from PgSQL_Connection_userinfo::compute_hash so connection-pool reuse is unchanged. Precondition (spec section 9): the verifier stored in ProxySQL must be byte-identical to the backend's pg_authid.rolpassword (same salt), and all backends a user reaches must share that rolpassword. No frontend/auth-selection changes (that is Phase 1).
pgsql-verifier_passthrough-t: 3 integration assertions (legacy-g4) — the backend role's exact SCRAM verifier is stored in pgsql_users and a query reaches the backend (proving the harvested ClientKey authenticates the backend leg, no plaintext/PBKDF2), plus a salt-mismatch negative case (a fresh verifier with a different salt is rejected). LOAD ... TO RUNTIME only; backend roles dropped and runtime restored at the end.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (5)
📝 WalkthroughWalkthroughThis PR adds PostgreSQL verifier and md5 secret handling, patches bundled libpq for injected auth material, reconciles frontend auth selection with stored secret type, and adds unit and integration coverage for verifier handling and pass-through authentication. ChangesSCRAM/MD5 verifier pass-through authentication
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant PgSQL_Protocol
participant PgSQL_Connection
participant libpq
participant BackendPostgres
Client->>PgSQL_Protocol: connect request
PgSQL_Protocol->>PgSQL_Protocol: lookup stored secret and reconcile auth method
PgSQL_Protocol->>Client: challenge
Client->>PgSQL_Protocol: auth response
PgSQL_Protocol->>PgSQL_Protocol: verify or mock response handling
PgSQL_Protocol->>PgSQL_Connection: prepare backend connection
PgSQL_Connection->>libpq: pass scram_client_key / scram_server_key / md5_secret
libpq->>BackendPostgres: authenticate using injected material
BackendPostgres->>PgSQL_Connection: auth result
Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements PostgreSQL frontend authentication using stored SCRAM verifiers and MD5 secrets, along with backend SCRAM pass-through authentication to avoid sending plaintext passwords. It introduces anti-enumeration mechanisms to prevent user discovery, adds validation for SCRAM verifiers at load time, and includes comprehensive integration and unit tests. A security review identified that base64-encoded sensitive key material in local stack buffers (ck_b64 and sk_b64) should be explicitly cleared before exiting the block to prevent potential memory-disclosure vulnerabilities.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| char ck_b64[64] = { 0 }; | ||
| char sk_b64[64] = { 0 }; | ||
| int n1 = pg_b64_encode((const char*)userinfo->scram_ClientKey, | ||
| (int)sizeof(userinfo->scram_ClientKey), ck_b64, (int)sizeof(ck_b64) - 1); | ||
| int n2 = pg_b64_encode((const char*)userinfo->scram_ServerKey, | ||
| (int)sizeof(userinfo->scram_ServerKey), sk_b64, (int)sizeof(sk_b64) - 1); | ||
| if (n1 > 0) ck_b64[n1] = '\0'; | ||
| if (n2 > 0) sk_b64[n2] = '\0'; | ||
| append_conninfo_param(conninfo, "scram_client_key", ck_b64); | ||
| append_conninfo_param(conninfo, "scram_server_key", sk_b64); |
There was a problem hiding this comment.
Security Issue: The local buffers ck_b64 and sk_b64 contain base64-encoded sensitive key material (scram_ClientKey is password-equivalent). Leaving these buffers on the stack without clearing them can expose sensitive credentials to memory scraping or other memory-disclosure attacks.
Recommendation: Explicitly clear ck_b64 and sk_b64 using memset before exiting the block.
char ck_b64[64] = { 0 };
char sk_b64[64] = { 0 };
int n1 = pg_b64_encode((const char*)userinfo->scram_ClientKey,
(int)sizeof(userinfo->scram_ClientKey), ck_b64, (int)sizeof(ck_b64) - 1);
int n2 = pg_b64_encode((const char*)userinfo->scram_ServerKey,
(int)sizeof(userinfo->scram_ServerKey), sk_b64, (int)sizeof(sk_b64) - 1);
if (n1 > 0) ck_b64[n1] = '\0';
if (n2 > 0) sk_b64[n2] = '\0';
append_conninfo_param(conninfo, "scram_client_key", ck_b64);
append_conninfo_param(conninfo, "scram_server_key", sk_b64);
memset(ck_b64, 0, sizeof(ck_b64));
memset(sk_b64, 0, sizeof(sk_b64));![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
include/PgSQL_Connection.h (1)
243-244: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse snake_case for the new member fields.
Rename the new members to
scram_client_keyandscram_server_key, updating the.cppcall sites at the same time. As per coding guidelines, “Member variables usesnake_case.”♻️ Proposed rename
- uint8_t scram_ClientKey[32]; - uint8_t scram_ServerKey[32]; + uint8_t scram_client_key[32]; + uint8_t scram_server_key[32];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@include/PgSQL_Connection.h` around lines 243 - 244, The new PgSQL_Connection member fields use mixed-case names instead of the project’s snake_case convention. Rename the fields in PgSQL_Connection to scram_client_key and scram_server_key, and update every corresponding use site in the implementation files that reads or writes these members so the class and its .cpp references stay consistent.Source: Coding guidelines
test/tap/tests/unit/pgsql_reconcile_unit-t.cpp (1)
31-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the returned method on the reject path too.
These lines only verify
rj. A regression that still setsreject=truebut stops returningAM_SCRAMwould pass this test even though the frontend challenge behavior changed.Suggested fix
- pgsql_reconcile_auth_method(3, PT_MD5, &rj); - ok(rj, "md5 secret, scram floor -> REJECT"); + ok(pgsql_reconcile_auth_method(3, PT_MD5, &rj) == AM_SCRAM && rj, + "md5 secret, scram floor -> SCRAM + REJECT");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/tap/tests/unit/pgsql_reconcile_unit-t.cpp` around lines 31 - 32, The reject-path test for pgsql_reconcile_auth_method only checks the `rj` flag, so it can miss regressions in the returned auth method. Update the assertion near `pgsql_reconcile_auth_method` in `pgsql_reconcile_unit-t` to verify both the reject result and that the returned method is still `AM_SCRAM`, matching the existing non-reject expectations and covering the frontend challenge behavior.test/tap/tests/pgsql-verifier_auth-t.cpp (1)
79-82: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the same client-visible failure for anti-enumeration.
The linked requirement is that unknown users, wrong passwords, and floor/secret mismatches look identical to clients. These checks only ban two substrings, so a different user-specific error would still pass. Reuse the wrong-password failure as the baseline and compare the unknown-user and md5-under-SCRAM-floor paths against that same visible error surface.
Also applies to: 94-101, 111-121
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/tap/tests/pgsql-verifier_auth-t.cpp` around lines 79 - 82, The anti-enumeration checks in the pgsql verifier auth tests currently only exclude a couple of substrings, so they may still allow user-specific failures to differ. In the verifier test cases around frontendConn and PQstatus, reuse the wrong-password case as the baseline client-visible failure, then assert the unknown-user path and the md5-under-SCRAM-floor path produce the same visible error surface as that baseline rather than just banning specific substrings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/PgSQL_Connection.cpp`:
- Around line 50-52: Replace the destructor’s plain memset-based clears for
scram_ClientKey and scram_ServerKey with a non-elidable secure wipe such as
explicit_bzero() or the project’s equivalent secure-zero helper. Update the
cleanup logic in the PgSQL_Connection destructor so the SCRAM key material is
always overwritten in a way the compiler cannot optimize away.
In `@lib/PgSQL_Protocol.cpp`:
- Around line 1002-1013: The handshake logic in
PgSQL_Protocol::generate_pkt_initial_handshake and the later password-packet
handling are using different sources of truth for auth selection, which can flip
a login into the mock-fail path if pgsql_thread___authentication_method changes
mid-flight. Stop recomputing mock from the live floor in the password handling
block and instead derive it from the stored handshake decision in
(*myds)->auth_method, or persist the reject bit when the initial handshake is
chosen so both halves use the same auth contract.
In `@test/tap/tests/pgsql-verifier_auth-t.cpp`:
- Around line 43-56: The helper functions addUser(), delUser(), and setFloor()
are swallowing execAdmin() failures, which hides setup errors and makes later
auth checks misleading. Update these test helpers so each admin command result
is checked and failures are surfaced immediately, either by returning a checked
status from addUser()/setFloor() and handling it at the call sites, or by
calling BAIL_OUT with a clear message from addUser(), delUser(), and setFloor()
when execAdmin() fails. Also update the call sites that invoke addUser() to stop
ignoring its return value.
In `@test/tap/tests/pgsql-verifier_passthrough-t.cpp`:
- Around line 61-67: The negative passthrough test in select1ThroughProxySQL is
currently too broad because it treats any connection failure as a pass, which
can hide frontend SCRAM auth regressions. Update the logic so the connection
step explicitly asserts PQstatus(c.get()) == CONNECTION_OK, then separately
assert that execScalar(c.get(), "SELECT 1") fails for the negative case; apply
the same split to the related assertions around the other referenced checks so
frontend-auth success and backend-query failure are validated independently.
- Around line 100-102: The negative passthrough test is allowing an empty
verifier when PQencryptPasswordConn fails, which lets the assertion pass without
actually testing the salt-mismatch path. Update the pgsql-verifier_passthrough-t
test around the PQencryptPasswordConn call and storeUser helper usage to fail
closed unless a SCRAM verifier is returned, so the test aborts or explicitly
fails when mismatch generation returns nullptr instead of falling back to an
empty string.
---
Nitpick comments:
In `@include/PgSQL_Connection.h`:
- Around line 243-244: The new PgSQL_Connection member fields use mixed-case
names instead of the project’s snake_case convention. Rename the fields in
PgSQL_Connection to scram_client_key and scram_server_key, and update every
corresponding use site in the implementation files that reads or writes these
members so the class and its .cpp references stay consistent.
In `@test/tap/tests/pgsql-verifier_auth-t.cpp`:
- Around line 79-82: The anti-enumeration checks in the pgsql verifier auth
tests currently only exclude a couple of substrings, so they may still allow
user-specific failures to differ. In the verifier test cases around frontendConn
and PQstatus, reuse the wrong-password case as the baseline client-visible
failure, then assert the unknown-user path and the md5-under-SCRAM-floor path
produce the same visible error surface as that baseline rather than just banning
specific substrings.
In `@test/tap/tests/unit/pgsql_reconcile_unit-t.cpp`:
- Around line 31-32: The reject-path test for pgsql_reconcile_auth_method only
checks the `rj` flag, so it can miss regressions in the returned auth method.
Update the assertion near `pgsql_reconcile_auth_method` in
`pgsql_reconcile_unit-t` to verify both the reject result and that the returned
method is still `AM_SCRAM`, matching the existing non-reject expectations and
covering the frontend challenge behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 15a25310-334d-4fd9-bc3a-e54b4c48819e
📒 Files selected for processing (11)
deps/Makefiledeps/postgresql/scram_verifier_auth.patchinclude/PgSQL_Connection.hinclude/PgSQL_Protocol.hlib/PgSQL_Authentication.cpplib/PgSQL_Connection.cpplib/PgSQL_Protocol.cpptest/tap/groups/groups.jsontest/tap/tests/pgsql-verifier_auth-t.cpptest/tap/tests/pgsql-verifier_passthrough-t.cpptest/tap/tests/unit/pgsql_reconcile_unit-t.cpp
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: CI-builds / builds (ubuntu22,-tap)
- GitHub Check: CI-builds / builds (ubuntu24,-tap-genai-gcov)
- GitHub Check: CI-builds / builds (debian12,-dbg)
- GitHub Check: CI-builds / builds (ubuntu22,-tap-mysqlx)
- GitHub Check: run / trigger
⚠️ CI failures not shown inline (2)
GitHub Actions: CI-lint-groups-json / lint: SCRAM verifier & md5 credential storage with SCRAM/md5 backend pass-through - PostgreSQL
Conclusion: failure
##[group]Run python3 test/tap/groups/lint_groups_json.py
�[36;1mpython3 test/tap/groups/lint_groups_json.py�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
groups.json format lint: 1 error(s) found:
Keys not sorted: 'basic-t' should come before 'pgsql_reconcile_unit-t'
Hint: run 'python3 /home/runner/work/proxysql/proxysql/test/tap/groups/lint_groups_json.py --fix' to auto-fix
##[error]Process completed with exit code 1.
GitHub Actions: CI-lint-groups-json / 0_lint.txt: SCRAM verifier & md5 credential storage with SCRAM/md5 backend pass-through - PostgreSQL
Conclusion: failure
##[group]Run python3 test/tap/groups/lint_groups_json.py
�[36;1mpython3 test/tap/groups/lint_groups_json.py�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
groups.json format lint: 1 error(s) found:
Keys not sorted: 'basic-t' should come before 'pgsql_reconcile_unit-t'
Hint: run 'python3 /home/runner/work/proxysql/proxysql/test/tap/groups/lint_groups_json.py --fix' to auto-fix
##[error]Process completed with exit code 1.
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{cpp,h,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{cpp,h,hpp}: Feature tiers are controlled via flags:PROXYSQL31=1for v3.1.x features (FFTO, TSDB),PROXYSQL40=1for v4.0.x features (plugin loader).PROXYSQL40=1implies bothPROXYSQL31=1andPROXYSQLFFTO=1andPROXYSQLTSDB=1. Use conditional compilation with#ifdef PROXYSQL31,#ifdef PROXYSQL40,#ifdef PROXYSQLFFTO,#ifdef PROXYSQLTSDB,#ifdef PROXYSQLCLICKHOUSE.
Class names usePascalCasewith protocol prefixes:MySQL_,PgSQL_, orProxySQL_(e.g.,MySQL_Protocol,PgSQL_Session).
Member variables usesnake_case.
Constants and macros useUPPER_SNAKE_CASE.
Use C++17; conditional compilation for feature tiers via#ifdef PROXYSQL31,#ifdef PROXYSQL40,#ifdef PROXYSQLFFTO,#ifdef PROXYSQLTSDB,#ifdef PROXYSQLCLICKHOUSE.
Use RAII for resource management; use jemalloc for memory allocation.
Use pthread mutexes for synchronization; usestd::atomic<>for counters.
Files:
include/PgSQL_Connection.hinclude/PgSQL_Protocol.hlib/PgSQL_Authentication.cpptest/tap/tests/unit/pgsql_reconcile_unit-t.cpplib/PgSQL_Connection.cpptest/tap/tests/pgsql-verifier_passthrough-t.cpptest/tap/tests/pgsql-verifier_auth-t.cpplib/PgSQL_Protocol.cpp
include/**/*.{h,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
Include guards in headers use
#ifndef __CLASS_*_Hformat (e.g.,#ifndef __MYSQL_PROTOCOL_H).
Files:
include/PgSQL_Connection.hinclude/PgSQL_Protocol.h
{lib,src}/**/*.{cpp,h,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
GenAI/MCP/RAG/LLM features live entirely in
plugins/genai/and load as a.soat runtime viadlopen. Do not guard withPROXYSQLGENAIin core code — that flag no longer guards any core code as of Step 7 of the GenAI plugin carve-out.
Files:
lib/PgSQL_Authentication.cpplib/PgSQL_Connection.cpplib/PgSQL_Protocol.cpp
test/tap/tests/**/*.cpp
📄 CodeRabbit inference engine (CLAUDE.md)
Test files follow naming pattern
test_*.cppor*-t.cppintest/tap/tests/. Test binaries are built via pattern rulemake <testname>-twhich compiles<testname>-t.cppinto<testname>-t. Register new tests ingroups.json.
Files:
test/tap/tests/unit/pgsql_reconcile_unit-t.cpptest/tap/tests/pgsql-verifier_passthrough-t.cpptest/tap/tests/pgsql-verifier_auth-t.cpp
test/tap/tests/unit/**/*.cpp
📄 CodeRabbit inference engine (CLAUDE.md)
Unit tests must use
test_globals.handtest_init.hand link againstlibproxysql.avia the custom test harness defined indoc/agents/project-conventions.md.
Files:
test/tap/tests/unit/pgsql_reconcile_unit-t.cpp
🧠 Learnings (2)
📚 Learning: 2026-01-20T09:34:19.124Z
Learnt from: yuji-hatakeyama
Repo: sysown/proxysql PR: 5307
File: test/tap/tests/reg_test_5306-show_warnings_with_comment-t.cpp:39-48
Timestamp: 2026-01-20T09:34:19.124Z
Learning: In ProxySQL's TAP test suite, resource leaks (e.g., not calling mysql_close() on early return paths) are commonly tolerated because test processes are short-lived and OS frees resources on exit. This pattern applies to all C++ test files under test/tap/tests. When reviewing, recognize this as a project-wide test convention and focus on test correctness and isolation rather than insisting on fixing such leaks in these test files.
Applied to files:
test/tap/tests/unit/pgsql_reconcile_unit-t.cpptest/tap/tests/pgsql-verifier_passthrough-t.cpptest/tap/tests/pgsql-verifier_auth-t.cpp
📚 Learning: 2026-04-01T21:27:00.297Z
Learnt from: wazir-ahmed
Repo: sysown/proxysql PR: 5557
File: test/tap/tests/unit/gtid_set_unit-t.cpp:14-17
Timestamp: 2026-04-01T21:27:00.297Z
Learning: In ProxySQL unit tests under test/tap/tests/unit/, include test_globals.h and test_init.h only for tests that depend on ProxySQL runtime globals/initialization (i.e., tests that exercise components linked against libproxysql.a). For “pure” data-structure/utility tests (e.g., ezoption_parser_unit-t.cpp, gtid_set_unit-t.cpp, gtid_trxid_interval_unit-t.cpp) that do not require runtime globals/initialization, it is correct to omit test_globals.h and test_init.h and instead include only tap.h plus the relevant project header(s).
Applied to files:
test/tap/tests/unit/pgsql_reconcile_unit-t.cpp
🪛 ast-grep (0.44.0)
lib/PgSQL_Protocol.cpp
[error] 1050-1050: Use of an unbounded buffer function that can overflow the destination; use a size-bounded equivalent (fgets, strncpy/strlcpy, strncat/strlcat, snprintf).
Context: sprintf(&md5_string[i * 2], "%02x", (unsigned int)md5_digest[i])
Note: [CWE-120] Buffer Copy without Checking Size of Input ('Classic Buffer Overflow').
(dangerous-buffer-functions-cpp)
🔇 Additional comments (14)
deps/Makefile (1)
398-398: LGTM!deps/postgresql/scram_verifier_auth.patch (2)
7-84: LGTM!Also applies to: 101-156
166-188: 🎯 Functional CorrectnessThe MD5 bypass belongs in the later MD5 branch, not this guard. This check is for SCRAM (
scram_client_key);md5_secretis handled separately later, so widening this condition here is misplaced.> Likely an incorrect or invalid review comment.lib/PgSQL_Connection.cpp (1)
39-41: LGTM!Also applies to: 133-136, 956-958, 987-1014
lib/PgSQL_Authentication.cpp (1)
11-11: LGTM!Also applies to: 91-98
include/PgSQL_Protocol.h (1)
55-59: LGTM!lib/PgSQL_Protocol.cpp (2)
381-469: LGTM!Also applies to: 931-932, 1075-1075, 1102-1103, 1156-1196
1041-1045: 🩺 Stability & AvailabilityThe MD5 fast-path is length-checked in the classifier.
get_password_type()only returnsPASSWORD_TYPE_MD5for secrets matchingmd5+ 32 hex chars, so shorter or malformed values fall through to the plaintext path and never reach thismemcpy.> Likely an incorrect or invalid review comment.test/tap/tests/unit/pgsql_reconcile_unit-t.cpp (1)
1-30: LGTM!Also applies to: 34-40
test/tap/groups/groups.json (1)
16-16: LGTM!Also applies to: 180-181
test/tap/tests/pgsql-verifier_passthrough-t.cpp (4)
1-39: LGTM!
40-58: LGTM!
70-92: LGTM!
104-111: LGTM!
|
1 participant
Problem
ProxySQL stores PostgreSQL user passwords as plaintext in
pgsql_users.password.That has two costs: cleartext secrets at rest in the admin DB and in memory, and a
PBKDF2 (4096 HMAC-SHA256 iterations) run for every SCRAM client — paid again per
backend connection, since libpq re-derives the keys from the plaintext on each new
server connection.
PostgreSQL itself never stores the password; it stores a verifier in
pg_authid.rolpassword:SCRAM-SHA-256$<iterations>:<salt>$<StoredKey>:<ServerKey>What this PR does
Lets
pgsql_users.passwordhold the backend's actual secret — a SCRAM verifier, anmd5 hash, or (as today) plaintext — auto-detected by prefix. Clients are then
verified with no per-auth PBKDF2, and backends are authenticated by reusing key
material instead of re-deriving it from a plaintext password.
pgsql-authentication_method:1=cleartext,2=md5,3=scram) selects the challenge method from the stored secrettype. A SCRAM verifier is parsed directly (no PBKDF2); a plaintext user pays PBKDF2 once
per thread (cached thereafter).
ClientKeyas a by-product of the proof check, and replays it (with the verifier'sServerKey) on the backend leg via a bundled-libpq patch — authenticating to the backendwith no plaintext and no PBKDF2.
md5…hash is reused directly on both legs.identically (same handshake shape, one generic error; real reason logged server-side only).
Behavior matrices
Frontend (client → ProxySQL) — challenge method / outcome:
Backend (ProxySQL → PostgreSQL) — depends only on the stored type:
md5_secretA session succeeds only if both legs in its row succeed.
libpq patch (
deps/postgresql/scram_verifier_auth.patch)New conninfo params
scram_client_key/scram_server_key/md5_secret(markedsensitive). In
fe-auth-scram.c: when a ClientKey is injected, skip SASLprep + PBKDF2 andderive
StoredKey = SHA256(ClientKey); verify the backend'sServerSignatureagainst theinjected ServerKey. Fail-closed fences: ClientKey and ServerKey must both be present or
both absent; invalid/short keys are rejected. Injected key material is
explicit_bzero'd infreePGconn.Preconditions & limitations
backend's
rolpassword(same salt and iterations), and the connecting user to map to thesame backend user (no user remapping). All backends a user reaches in a hostgroup must
share the same verifier — automatic for physical/streaming replicas; for logical
replication / independent clusters, pin one identical verifier or use per-hostgroup
credentials.
Closes #5863
Summary by CodeRabbit