diff --git a/test/infra/docker-pgsql16-single/bin/docker-pgsql-post.bash b/test/infra/docker-pgsql16-single/bin/docker-pgsql-post.bash index bee2ed3c7b..c065de1b68 100755 --- a/test/infra/docker-pgsql16-single/bin/docker-pgsql-post.bash +++ b/test/infra/docker-pgsql16-single/bin/docker-pgsql-post.bash @@ -25,6 +25,19 @@ for PGUSER in ${PGUSERS}; do docker exec "${CONTAINER}" psql -X -Upostgres -dpostgres -c "SET client_min_messages = 'error';" -c "SET lock_timeout = '10s';" -c "GRANT ALL ON SCHEMA public TO $PGUSER;" done +# md5-auth user for #5865 md5 backend pass-through (pgsql-md5_passthrough-t). +# password_encryption MUST be set to 'md5' in the SAME psql session BEFORE CREATE USER so +# pg_authid.rolpassword is stored as an 'md5...' hash (not a SCRAM verifier). pg_hba.conf grants +# 'md5user' the md5 method (above the scram-sha-256 catch-all). Password mirrors the loop above +# (password == username) so tests can supply the known plaintext. +echo "Creating md5-auth user: md5user" +docker exec "${CONTAINER}" psql -X -Upostgres -c "SET client_min_messages = 'error';" -c "SET lock_timeout = '10s';" -c "SET password_encryption = 'md5';" -c "DROP USER IF EXISTS md5user;" -c "CREATE USER md5user WITH PASSWORD 'md5user';" +docker exec "${CONTAINER}" psql -X -Upostgres -c "SET client_min_messages = 'error';" -c "SET lock_timeout = '10s';" -c "CREATE DATABASE md5user;" +docker exec "${CONTAINER}" psql -X -Upostgres -c "SET client_min_messages = 'error';" -c "SET lock_timeout = '10s';" -c "GRANT ALL PRIVILEGES ON DATABASE md5user TO md5user;" +docker exec "${CONTAINER}" psql -X -Upostgres -c "SET client_min_messages = 'error';" -c "SET lock_timeout = '10s';" -c "GRANT pg_write_server_files,pg_read_server_files TO md5user;" +docker exec "${CONTAINER}" psql -X -Upostgres -dmd5user -c "SET client_min_messages = 'error';" -c "SET lock_timeout = '10s';" -c "GRANT ALL ON SCHEMA public TO md5user;" +docker exec "${CONTAINER}" psql -X -Upostgres -dpostgres -c "SET client_min_messages = 'error';" -c "SET lock_timeout = '10s';" -c "GRANT ALL ON SCHEMA public TO md5user;" + # Ensure postgres user has the ROOT_PASSWORD docker exec "${CONTAINER}" psql -X -Upostgres -c "SET client_min_messages = 'error';" -c "SET lock_timeout = '10s';" -c "ALTER USER postgres WITH PASSWORD '${ROOT_PASSWORD}';" diff --git a/test/infra/docker-pgsql16-single/conf/pgsql/pgsql1/pg_hba.conf b/test/infra/docker-pgsql16-single/conf/pgsql/pgsql1/pg_hba.conf index 6372b3e173..4c0036f079 100644 --- a/test/infra/docker-pgsql16-single/conf/pgsql/pgsql1/pg_hba.conf +++ b/test/infra/docker-pgsql16-single/conf/pgsql/pgsql1/pg_hba.conf @@ -16,6 +16,12 @@ # "local" is for Unix domain socket connections only local all all trust +# md5-auth user for #5865 md5 backend pass-through (pgsql-md5_passthrough-t). +# pg_hba is first-match-wins, so these md5 rules MUST precede the scram-sha-256 +# catch-alls below; only 'md5user' is affected, all other users still use scram. +host all md5user 127.0.0.1/32 md5 +host all md5user ::1/128 md5 +host all md5user all md5 # IPv4 local connections: host all all 127.0.0.1/32 scram-sha-256 # IPv6 local connections: diff --git a/test/tap/groups/groups.json b/test/tap/groups/groups.json index 31f97f00d6..75babe3abf 100644 --- a/test/tap/groups/groups.json +++ b/test/tap/groups/groups.json @@ -153,6 +153,8 @@ "pgsql-extended_query_protocol_query_rules_test-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4" ], "pgsql-extended_query_protocol_test-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4" ], "pgsql-issue5384-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4" ], + "pgsql-libpq_scram_params-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4" ], + "pgsql-md5_passthrough-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4" ], "pgsql-monitor_ssl_connections_test-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4" ], "pgsql-multiplex_status_test-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4" ], "pgsql-notice_test-t" : [ "legacy-g6","mysql-auto_increment_delay_multiplex=0-g1","mysql-multiplexing=false-g1","mysql-query_digests=0-g1","mysql-query_digests_keep_comment=1-g1" ], @@ -176,6 +178,7 @@ "pgsql-reg_test_5801_options_startup_param-t" : [ "legacy-g1","mysql-auto_increment_delay_multiplex=0-g1","mysql-multiplexing=false-g1","mysql-query_digests=0-g1","mysql-query_digests_keep_comment=1-g1" ], "pgsql-retry_guard_in_txn_on_broken_backend-t" : [ "legacy-g2" ], "pgsql-scram_cache_invalidation-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4" ], + "pgsql-scram_reload_midhandshake-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4" ], "pgsql-servers_ssl_params-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4" ], "pgsql-set_parameter_validation_test-t" : [ "legacy-g6","mysql-auto_increment_delay_multiplex=0-g1","mysql-multiplexing=false-g1","mysql-query_digests=0-g1","mysql-query_digests_keep_comment=1-g1","set_parser_algorithm_3-g1" ], "pgsql-set_statement_test-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4","set_parser_algorithm_3-g1" ], @@ -189,7 +192,9 @@ "pgsql-unix_socket-t" : [ "pgsql-socket-g1" ], "pgsql-unsupported_feature_test-t" : [ "legacy-g6","mysql-auto_increment_delay_multiplex=0-g1","mysql-multiplexing=false-g1","mysql-query_digests=0-g1","mysql-query_digests_keep_comment=1-g1" ], "pgsql-verifier_auth-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4" ], + "pgsql-verifier_backend_kill-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4" ], "pgsql-verifier_passthrough-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4" ], + "pgsql-verifier_pool_rotation-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4" ], "pgsql-watchdog_test-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4" ], "pgsql_command_complete_unit-t" : [ "unit-tests-g1" ], "pgsql_error_classifier_unit-t" : [ "unit-tests-g1" ], diff --git a/test/tap/tests/Makefile b/test/tap/tests/Makefile index c402d543e9..21cf92eb12 100644 --- a/test/tap/tests/Makefile +++ b/test/tap/tests/Makefile @@ -363,19 +363,25 @@ test_wexecvp_syscall_failures-t: test_wexecvp_syscall_failures-t.cpp $(TAP_LDIR) $(CXX) $< $(IDIRS) $(LDIRS) $(OPT) $(MYLIBS) -Wl,--wrap=pipe,--wrap=fcntl,--wrap=read,--wrap=poll $(STATIC_LIBS) -o $@ pgsql-extended_query_protocol_test-t: pgsql-extended_query_protocol_test-t.cpp pg_lite_client.cpp $(TAP_LDIR)/libtap.so - $(CXX) $< pg_lite_client.cpp $(IDIRS) $(LDIRS) $(OPT) $(MYLIBS) $(STATIC_LIBS) -o $@ + $(CXX) $< pg_lite_client.cpp $(IDIRS) $(LDIRS) $(OPT) $(MYLIBS) $(STATIC_LIBS) -lscram -lusual -Wl,--allow-multiple-definition -o $@ pgsql-reg_test_5273_bind_parameter_format-t: pgsql-reg_test_5273_bind_parameter_format-t.cpp pg_lite_client.cpp $(TAP_LDIR)/libtap.so - $(CXX) $< pg_lite_client.cpp $(IDIRS) $(LDIRS) $(OPT) $(MYLIBS) $(STATIC_LIBS) -o $@ + $(CXX) $< pg_lite_client.cpp $(IDIRS) $(LDIRS) $(OPT) $(MYLIBS) $(STATIC_LIBS) -lscram -lusual -Wl,--allow-multiple-definition -o $@ pgsql-reg_test_5300_threshold_resultset_deadlock-t: pgsql-reg_test_5300_threshold_resultset_deadlock-t.cpp pg_lite_client.cpp $(TAP_LDIR)/libtap.so - $(CXX) $< pg_lite_client.cpp $(IDIRS) $(LDIRS) $(OPT) $(MYLIBS) $(STATIC_LIBS) -o $@ + $(CXX) $< pg_lite_client.cpp $(IDIRS) $(LDIRS) $(OPT) $(MYLIBS) $(STATIC_LIBS) -lscram -lusual -Wl,--allow-multiple-definition -o $@ test_ffto_pgsql_pipeline-t: test_ffto_pgsql_pipeline-t.cpp pg_lite_client.cpp $(TAP_LDIR)/libtap.so - $(CXX) $< pg_lite_client.cpp $(IDIRS) $(LDIRS) $(OPT) $(MYLIBS) $(STATIC_LIBS) -o $@ + $(CXX) $< pg_lite_client.cpp $(IDIRS) $(LDIRS) $(OPT) $(MYLIBS) $(STATIC_LIBS) -lscram -lusual -Wl,--allow-multiple-definition -o $@ test_ffto_pgsql_stmt_portal-t: test_ffto_pgsql_stmt_portal-t.cpp pg_lite_client.cpp $(TAP_LDIR)/libtap.so - $(CXX) $< pg_lite_client.cpp $(IDIRS) $(LDIRS) $(OPT) $(MYLIBS) $(STATIC_LIBS) -o $@ + $(CXX) $< pg_lite_client.cpp $(IDIRS) $(LDIRS) $(OPT) $(MYLIBS) $(STATIC_LIBS) -lscram -lusual -Wl,--allow-multiple-definition -o $@ + +# -lscram/-lusual: pg_lite_client.cpp uses libscram client SCRAM (stepwise saslBegin/saslFinish for the +# mid-handshake reload test); --allow-multiple-definition resolves duplicate symbols between libscram/libusual +# and other vendored static libs (test binaries only). +pgsql-scram_reload_midhandshake-t: pgsql-scram_reload_midhandshake-t.cpp pg_lite_client.cpp $(TAP_LDIR)/libtap.so + $(CXX) $< pg_lite_client.cpp $(IDIRS) $(LDIRS) $(OPT) $(MYLIBS) $(STATIC_LIBS) -lscram -lusual -Wl,--allow-multiple-definition -o $@ MYSQLX_PROTO_DIR := $(PROXYSQL_PATH)/plugins/mysqlx/proto diff --git a/test/tap/tests/pg_lite_client.cpp b/test/tap/tests/pg_lite_client.cpp index f423d6e305..8edd97a0d4 100644 --- a/test/tap/tests/pg_lite_client.cpp +++ b/test/tap/tests/pg_lite_client.cpp @@ -12,6 +12,11 @@ #include #include #include +#include + +extern "C" { +#include "scram.h" +} // Buffer writing helpers static void writeInt32ToBuffer(std::vector& buffer, int32_t value) { @@ -33,6 +38,8 @@ static void writeStringToBuffer(std::vector& buffer, const std::string& buffer.push_back(0); // Null terminator } +static std::string extractErrorMessage(const std::vector& buffer); + // ===== Connection Implementation ===== // Message helpers @@ -178,6 +185,7 @@ PgConnection::PgConnection(int timeout_ms) { } PgConnection::~PgConnection() { + freeSaslState(); disconnect(); } @@ -303,6 +311,7 @@ void PgConnection::handleAuthentication(const std::string& password) { if (type == AUTH_TYPE) { if (buffer.size() < 4) throw PgException("Invalid authentication message"); int32_t authType = ntohl(*reinterpret_cast(buffer.data())); + if (last_auth_type_ == 0 && authType != 0) last_auth_type_ = authType; if (authType == 0) { // AuthenticationOK return; } @@ -310,11 +319,30 @@ void PgConnection::handleAuthentication(const std::string& password) { sendPassword(password); // After sending password, we need to wait for auth result readMessage(type, buffer); + if (type == ERROR_RESPONSE) + throw PgException("Authentication error: " + extractErrorMessage(buffer)); if (type == AUTH_TYPE) { authType = ntohl(*reinterpret_cast(buffer.data())); if (authType == 0) return; } } + else if (authType == 5) { // AuthenticationMD5Password (4-byte salt follows) + if (buffer.size() < 8) throw PgException("Invalid MD5 auth message"); + uint8_t salt[4]; + memcpy(salt, buffer.data() + 4, 4); + sendMD5Password(password, salt); + readMessage(type, buffer); + if (type == ERROR_RESPONSE) + throw PgException("Authentication error: " + extractErrorMessage(buffer)); + if (type == AUTH_TYPE) { + authType = ntohl(*reinterpret_cast(buffer.data())); + if (authType == 0) return; + } + } + else if (authType == 10) { // AuthenticationSASL (mechanism list follows) + doSASLAuth(password, buffer); + return; // doSASLAuth consumes through AuthenticationOk + } else { throw PgException("Unsupported authentication method: " + std::to_string(authType)); } @@ -342,6 +370,314 @@ void PgConnection::sendPassword(const std::string& password) { sendMessage('p', packet); } +static std::string md5_hex(const std::string& in) { + unsigned char digest[MD5_DIGEST_LENGTH]; + MD5(reinterpret_cast(in.data()), in.size(), digest); + static const char* hx = "0123456789abcdef"; + std::string out; + out.reserve(MD5_DIGEST_LENGTH * 2); + for (int i = 0; i < MD5_DIGEST_LENGTH; ++i) { + out.push_back(hx[digest[i] >> 4]); + out.push_back(hx[digest[i] & 0x0f]); + } + return out; +} + +// PostgreSQL MD5 auth: "md5" + md5( md5(password + user) + salt ) +void PgConnection::sendMD5Password(const std::string& password, const uint8_t salt[4]) { + std::string inner = md5_hex(password + user_); + std::string with_salt = inner; + with_salt.append(reinterpret_cast(salt), 4); + std::string token = "md5" + md5_hex(with_salt); + std::vector packet; + writeStringToBuffer(packet, token); // null-terminated C string + sendMessage('p', packet); +} + +// Parse the human-readable message ('M' field) out of an ErrorResponse ('E') payload. +// ErrorResponse body is a sequence of (1-byte field-type, null-terminated string), +// terminated by a zero field-type byte. +static std::string extractErrorMessage(const std::vector& buffer) { + size_t i = 0; + while (i < buffer.size() && buffer[i] != 0) { + char field = static_cast(buffer[i]); + ++i; + const char* start = reinterpret_cast(buffer.data() + i); + size_t len = 0; + while (i + len < buffer.size() && buffer[i + len] != 0) ++len; + std::string value(start, len); + i += len + 1; // skip the value and its null terminator + if (field == 'M') return value; + } + return std::string(); +} + +// Completes a SCRAM-SHA-256 SASL exchange as the CLIENT, reusing deps/libscram. +// mechListMsg is the AuthenticationSASL(10) payload after the 4-byte authType: +// a sequence of null-terminated mechanism names terminated by an extra null. +// (We do not parse it; ProxySQL offers SCRAM-SHA-256 and we answer with that.) +void PgConnection::doSASLAuth(const std::string& password, + const std::vector& /*mechListMsg*/) { + ScramState* st = scram_state_init(); + PgCredentials cred; + memset(&cred, 0, sizeof(cred)); + snprintf(cred.name, sizeof(cred.name), "%s", user_.c_str()); + snprintf(cred.passwd, sizeof(cred.passwd), "%s", password.c_str()); + cred.has_scram_keys = false; + + char type; + std::vector buffer; + char* client_first = nullptr; + char* client_final = nullptr; + + // 1) SASLInitialResponse ('p'): mechanism name + Int32 length + client-first-message. + // libscram's build_client_first_message already includes the "n,,"" GS2 header + // (it returns "n,,n=,r="), so we send it verbatim. + client_first = build_client_first_message(st); + if (!client_first) { free_scram_state(st); throw PgException(std::string("scram client-first: ") + scram_error()); } + { + std::vector pkt; + writeStringToBuffer(pkt, "SCRAM-SHA-256"); // null-terminated mechanism name + int32_t clen = htonl((int32_t)strlen(client_first)); + const uint8_t* cp = reinterpret_cast(&clen); + pkt.insert(pkt.end(), cp, cp + 4); // Int32 length of client-first + pkt.insert(pkt.end(), client_first, client_first + strlen(client_first)); + sendMessage('p', pkt); + } + + // 2) Expect AuthenticationSASLContinue (authType 11) with the server-first-message. + readMessage(type, buffer); + if (type == ERROR_RESPONSE) { + free(client_first); free_scram_state(st); + throw PgException("scram: " + extractErrorMessage(buffer)); + } + if (type != AUTH_TYPE || buffer.size() < 4 || + ntohl(*reinterpret_cast(buffer.data())) != 11) { + free(client_first); free_scram_state(st); + throw PgException("expected AuthenticationSASLContinue(11)"); + } + std::string server_first(reinterpret_cast(buffer.data()) + 4, buffer.size() - 4); + char* server_nonce = nullptr; char* salt = nullptr; int saltlen = 0; int iterations = 0; + if (!read_server_first_message(st, const_cast(server_first.c_str()), + &server_nonce, &salt, &saltlen, &iterations)) { + free(client_first); free_scram_state(st); + throw PgException(std::string("scram read server-first: ") + scram_error()); + } + + // 3) SASLResponse ('p'): client-final-message (with proof derived from plaintext passwd). + client_final = build_client_final_message(st, &cred, server_nonce, salt, saltlen, iterations); + free(salt); // read_server_first_message malloc'd salt and handed us ownership; + salt = nullptr; // build_client_final_message is its only consumer (just read above). + if (!client_final) { free(client_first); free_scram_state(st); throw PgException(std::string("scram client-final: ") + scram_error()); } + { + std::vector pkt(client_final, client_final + strlen(client_final)); + sendMessage('p', pkt); + } + + // 4) Expect AuthenticationSASLFinal (authType 12) with server-final (v=ServerSignature). + // A wrong password surfaces here as an ErrorResponse instead. + readMessage(type, buffer); + if (type == ERROR_RESPONSE) { + free(client_first); free(client_final); free_scram_state(st); + throw PgException("scram: " + extractErrorMessage(buffer)); + } + if (type != AUTH_TYPE || buffer.size() < 4 || + ntohl(*reinterpret_cast(buffer.data())) != 12) { + free(client_first); free(client_final); free_scram_state(st); + throw PgException("expected AuthenticationSASLFinal(12)"); + } + { + std::string server_final(reinterpret_cast(buffer.data()) + 4, buffer.size() - 4); + char server_sig[256] = {0}; + if (!read_server_final_message(const_cast(server_final.c_str()), server_sig) || + !verify_server_signature(st, &cred, server_sig)) { + free(client_first); free(client_final); free_scram_state(st); + throw PgException("scram server signature verification failed"); + } + } + free(client_first); free(client_final); free_scram_state(st); + + // 5) Expect AuthenticationOk (0). + readMessage(type, buffer); + if (type == ERROR_RESPONSE) throw PgException("scram: " + extractErrorMessage(buffer)); + if (type == AUTH_TYPE && buffer.size() >= 4 && + ntohl(*reinterpret_cast(buffer.data())) == 0) return; + throw PgException("scram: no AuthenticationOk after SASLFinal"); +} + +// ===== Stepwise SCRAM (mid-handshake reload test, #5865 review ask 2) ===== +// These mirror doSASLAuth() but pause between server-first and client-final so a test can rotate +// the runtime verifier in the gap. saslBegin() drives phases 1-2; saslFinish() drives phases 3-5. + +void PgConnection::freeSaslState() { + if (sasl_client_first_) { free(sasl_client_first_); sasl_client_first_ = nullptr; } + if (sasl_salt_) { free(sasl_salt_); sasl_salt_ = nullptr; } + if (sasl_st_) { free_scram_state(sasl_st_); sasl_st_ = nullptr; } + sasl_server_nonce_ = nullptr; // non-owned (points into sasl_st_) + sasl_saltlen_ = 0; + sasl_iterations_ = 0; + sasl_password_.clear(); +} + +// Open the socket + send the startup packet, but do NOT read or drive authentication. +// (Copy of connect()'s socket/startup prologue, minus handleAuthentication()/waitForReady().) +void PgConnection::rawConnectStartup(const std::string& host, int port, + const std::string& dbname, const std::string& user) { + sock_ = socket(AF_INET, SOCK_STREAM, 0); + if (sock_ < 0) throw PgException("Socket creation failed"); + + struct addrinfo hints{}, *res; + hints.ai_family = AF_INET; + hints.ai_socktype = SOCK_STREAM; + std::string port_str = std::to_string(port); + int status = getaddrinfo(host.c_str(), port_str.c_str(), &hints, &res); + if (status != 0) { + close(sock_); sock_ = -1; + throw PgException("Failed to resolve host: " + host + " (" + std::string(gai_strerror(status)) + ")"); + } + if (::connect(sock_, res->ai_addr, res->ai_addrlen) < 0) { + freeaddrinfo(res); + close(sock_); sock_ = -1; + throw PgException("Connection failed to " + host + ":" + std::to_string(port)); + } + freeaddrinfo(res); + + user_ = user; + dbname_ = dbname; + sendStartupPacket(); +} + +// Phase 1-2: read AuthenticationSASL(10), send client-first, read AuthenticationSASLContinue(11). +// Returns the server-first-message. The parsed server_nonce/salt/iterations are stashed for +// saslFinish(). Skips any ParameterStatus/etc. is NOT expected here — the server sends the auth +// challenge immediately after startup. +std::string PgConnection::saslBegin(const std::string& user, const std::string& password) { + freeSaslState(); // reset any prior stepwise state on this connection + sasl_password_ = password; + + char type; + std::vector buffer; + + // Read the initial auth request; it must be AuthenticationSASL(10). + readMessage(type, buffer); + if (type == ERROR_RESPONSE) + throw PgException("saslBegin: " + extractErrorMessage(buffer)); + if (type != AUTH_TYPE || buffer.size() < 4) + throw PgException("saslBegin: expected AuthenticationSASL, got message type '" + std::string(1, type) + "'"); + int32_t authType = ntohl(*reinterpret_cast(buffer.data())); + if (authType != 10) + throw PgException("saslBegin: expected AuthenticationSASL(10), got authType " + std::to_string(authType)); + last_auth_type_ = 10; + + sasl_st_ = scram_state_init(); + // We authenticate as `user`; the startup packet already used the same name. + (void)user; + + // 1) SASLInitialResponse ('p'): mechanism name + Int32 length + client-first-message. + sasl_client_first_ = build_client_first_message(sasl_st_); + if (!sasl_client_first_) { + std::string e = scram_error(); freeSaslState(); + throw PgException(std::string("scram client-first: ") + e); + } + { + std::vector pkt; + writeStringToBuffer(pkt, "SCRAM-SHA-256"); + int32_t clen = htonl((int32_t)strlen(sasl_client_first_)); + const uint8_t* cp = reinterpret_cast(&clen); + pkt.insert(pkt.end(), cp, cp + 4); + pkt.insert(pkt.end(), sasl_client_first_, sasl_client_first_ + strlen(sasl_client_first_)); + sendMessage('p', pkt); + } + + // 2) AuthenticationSASLContinue(11): server-first-message. + readMessage(type, buffer); + if (type == ERROR_RESPONSE) { + std::string e = extractErrorMessage(buffer); freeSaslState(); + throw PgException("scram: " + e); + } + if (type != AUTH_TYPE || buffer.size() < 4 || + ntohl(*reinterpret_cast(buffer.data())) != 11) { + freeSaslState(); + throw PgException("expected AuthenticationSASLContinue(11)"); + } + std::string server_first(reinterpret_cast(buffer.data()) + 4, buffer.size() - 4); + if (!read_server_first_message(sasl_st_, const_cast(server_first.c_str()), + &sasl_server_nonce_, &sasl_salt_, &sasl_saltlen_, &sasl_iterations_)) { + std::string e = scram_error(); freeSaslState(); + throw PgException(std::string("scram read server-first: ") + e); + } + return server_first; +} + +// Phase 3-5: build+send client-final (proof from the ORIGINAL password), read SASLFinal(12) + +// AuthenticationOk(0). Returns 0 on success, SASL_FINISH_REJECTED on a clean ErrorResponse. +// A server-signature mismatch or unexpected message throws; IO/timeout errors propagate. +int PgConnection::saslFinish() { + if (!sasl_st_) throw PgException("saslFinish: saslBegin() was not called"); + + PgCredentials cred; + memset(&cred, 0, sizeof(cred)); + snprintf(cred.name, sizeof(cred.name), "%s", user_.c_str()); + snprintf(cred.passwd, sizeof(cred.passwd), "%s", sasl_password_.c_str()); + cred.has_scram_keys = false; + + char type; + std::vector buffer; + + // 3) SASLResponse ('p'): client-final-message. + char* client_final = build_client_final_message(sasl_st_, &cred, sasl_server_nonce_, + sasl_salt_, sasl_saltlen_, sasl_iterations_); + if (!client_final) { + std::string e = scram_error(); freeSaslState(); + throw PgException(std::string("scram client-final: ") + e); + } + { + std::vector pkt(client_final, client_final + strlen(client_final)); + sendMessage('p', pkt); + } + + // 4) AuthenticationSASLFinal(12) OR a clean ErrorResponse (rejected verifier). + readMessage(type, buffer); + if (type == ERROR_RESPONSE) { + last_auth_type_ = -1; + last_error_ = extractErrorMessage(buffer); // clean server rejection of the client-final + free(client_final); freeSaslState(); + return SASL_FINISH_REJECTED; + } + if (type != AUTH_TYPE || buffer.size() < 4 || + ntohl(*reinterpret_cast(buffer.data())) != 12) { + free(client_final); freeSaslState(); + throw PgException("expected AuthenticationSASLFinal(12)"); + } + { + std::string server_final(reinterpret_cast(buffer.data()) + 4, buffer.size() - 4); + char server_sig[256] = {0}; + if (!read_server_final_message(const_cast(server_final.c_str()), server_sig) || + !verify_server_signature(sasl_st_, &cred, server_sig)) { + free(client_final); freeSaslState(); + throw PgException("scram server signature verification failed"); + } + } + free(client_final); + + // 5) AuthenticationOk(0). + readMessage(type, buffer); + if (type == ERROR_RESPONSE) { + last_auth_type_ = -1; + last_error_ = extractErrorMessage(buffer); // rejected between SASLFinal and AuthenticationOk + freeSaslState(); + return SASL_FINISH_REJECTED; + } + if (type == AUTH_TYPE && buffer.size() >= 4 && + ntohl(*reinterpret_cast(buffer.data())) == 0) { + last_auth_type_ = 0; + freeSaslState(); + return 0; + } + freeSaslState(); + throw PgException("scram: no AuthenticationOk after SASLFinal"); +} + void PgConnection::waitForReady() { char type; std::vector buffer; diff --git a/test/tap/tests/pg_lite_client.h b/test/tap/tests/pg_lite_client.h index 189c2e928a..d653d90c09 100644 --- a/test/tap/tests/pg_lite_client.h +++ b/test/tap/tests/pg_lite_client.h @@ -142,6 +142,24 @@ class PgConnection { void disconnect(); bool isConnected() const; inline int getSocket() const { return sock_; } + inline int getLastAuthType() const { return last_auth_type_; } + + // ---- Stepwise SCRAM for mid-handshake tests (#5865 review ask 2) ---- + // Unlike connect(), which drives the whole SASL exchange atomically, these split it so a + // test can mutate runtime credentials BETWEEN server-first and client-final. + // rawConnectStartup(): open the socket + send the startup packet; do NOT read/drive auth. + void rawConnectStartup(const std::string& host, int port, + const std::string& dbname, const std::string& user); + // saslBegin(): read AuthenticationSASL(10), send client-first (SASLInitialResponse), + // read AuthenticationSASLContinue(11); return the server-first-message string. + std::string saslBegin(const std::string& user, const std::string& password); + // saslFinish(): build the client-final from the state saslBegin() left, send it, and read the + // result. Returns 0 on AuthenticationOk, SASL_FINISH_REJECTED on a clean ErrorResponse from the + // server; any genuine IO/timeout/protocol error propagates as PgException. + int saslFinish(); + static const int SASL_FINISH_REJECTED = -1; + // Human-readable server error captured when saslFinish() returns SASL_FINISH_REJECTED. + inline const std::string& getLastError() const { return last_error_; } void execute(const std::string& query); void executeParams( @@ -207,10 +225,25 @@ class PgConnection { int timeout_ms_ = 0; std::string user_; std::string dbname_; - + int last_auth_type_ = 0; + + // Persisted stepwise-SASL state between saslBegin() and saslFinish() (see the two-phase + // methods above). Pointers are forward-declared (struct defs live in scram.h, .cpp only). + struct ScramState* sasl_st_ = nullptr; + std::string sasl_password_; // to rebuild PgCredentials for the client-final proof + char* sasl_client_first_ = nullptr; // owned by us; freed in saslFinish()/destructor + char* sasl_server_nonce_ = nullptr; // non-owned: points into sasl_st_ + char* sasl_salt_ = nullptr; // owned by us (read_server_first_message malloc'd it) + int sasl_saltlen_ = 0; + int sasl_iterations_ = 0; + std::string last_error_; // last server error message (set by saslFinish()) + void freeSaslState(); // release stepwise-SASL resources idempotently + void sendStartupPacket(); void handleAuthentication(const std::string& password); void sendPassword(const std::string& password); + void sendMD5Password(const std::string& password, const uint8_t salt[4]); + void doSASLAuth(const std::string& password, const std::vector& mechListMsg); void sendParse(const std::string& query, const std::string& stmtName, const std::vector& paramType); diff --git a/test/tap/tests/pgsql-libpq_scram_params-t.cpp b/test/tap/tests/pgsql-libpq_scram_params-t.cpp new file mode 100644 index 0000000000..722f91ed14 --- /dev/null +++ b/test/tap/tests/pgsql-libpq_scram_params-t.cpp @@ -0,0 +1,260 @@ +/** + * @file pgsql-libpq_scram_params-t.cpp + * @brief Regression (PR #5865 review ask #5): patched libpq SCRAM/md5 conninfo params. + * + * Connects DIRECTLY to the backend PostgreSQL (no ProxySQL in the path) to validate the accept/reject + * behaviour of the new conninfo parameters the #5865 patch adds to libpq + * (`deps/postgresql/scram_verifier_auth.patch`): + * + * - scram_client_key : base64 of the 32-byte SCRAM ClientKey (verifier pass-through) + * - scram_server_key : base64 of the 32-byte SCRAM ServerKey (verifier pass-through) + * - md5_secret : the stored "md5"+32hex secret (reused for md5 backend auth) + * + * What the patch actually validates (read from the patch + built fe-auth-scram.c / fe-auth.c): + * * build_client_first_message(): require BOTH ClientKey and ServerKey, or NEITHER + * (has_ck != has_sk -> return NULL -> the connection cannot authenticate). This is the + * mutual-auth guard: a lone key can never silently skip server verification. + * * calculate_client_proof(): pg_b64_decode(scram_client_key) MUST yield exactly key_length (32) + * bytes, else errstr "invalid scram_client_key" and auth fails. StoredKey = SHA256(ClientKey). + * * verify_server_signature(): pg_b64_decode(scram_server_key) MUST yield exactly 32 bytes, else + * "invalid scram_server_key". + * * fe-auth.c: with scram_client_key injected, an empty password no longer aborts with + * "no password supplied" (the keys stand in for the password) -- but a SCRAM *verifier* string + * handed in via password= is still treated as a plaintext password (run through SASLprep+PBKDF2) + * and therefore does NOT authenticate. + * + * This test is also the CANARY that the vendored libpq is the PATCHED build: an unpatched libpq + * rejects these keywords with "invalid connection option". + * + * The valid ClientKey/ServerKey pair is derived exactly the way ProxySQL derives it + * (lib/PgSQL_Connection.cpp connect_start + lib/PgSQL_Protocol.cpp harvest): from the role's real + * stored verifier we take the server-chosen salt + iteration count, compute + * SaltedPassword = PBKDF2-HMAC-SHA256(password, salt, iters, 32), then + * ClientKey = HMAC-SHA256(SaltedPassword, "Client Key") and + * ServerKey = HMAC-SHA256(SaltedPassword, "Server Key"), base64-encoded with libpq's own + * pg_b64_encode. (We also cross-check the derived ServerKey against the verifier's stored ServerKey.) + * + * md5_secret backend auth is NOT exercised end-to-end here: the legacy-g4 backend's pg_hba.conf is + * scram-sha-256-only (documented in pgsql-verifier_passthrough-t.cpp), so an md5 exchange never + * happens. The md5_secret keyword is still proven RECOGNISED by the patched libpq in the canary. + * + * Direct-to-backend host/port/superuser come from cl.pgsql_server_* . No ProxySQL runtime state is + * touched; the one backend role this test creates is dropped at the end. + */ +#include +#include +#include +#include +#include + +#include +#include + +#include "libpq-fe.h" +#include "command_line.h" +#include "tap.h" +#include "utils.h" + +// libpq's own base64 (from libpgcommon) -- ProxySQL uses these exact functions to (de)serialise the +// key material, so we mirror them rather than OpenSSL base64 (which pads differently on odd inputs). +extern "C" int pg_b64_encode(const char* src, int len, char* dst, int dstlen); +extern "C" int pg_b64_decode(const char* src, int len, char* dst, int dstlen); + +using PGConnPtr = std::unique_ptr; +CommandLine cl; + +// Direct backend connection with an arbitrary trailing param string (no implicit password: the +// verifier-pass-through cases must send NO password, exactly like ProxySQL's backend leg). +static PGConnPtr connBE(const std::string& params) { + std::stringstream ss; + ss << "host='" << cl.pgsql_server_host << "' port=" << cl.pgsql_server_port + << " dbname=postgres sslmode=disable " << params; + return PGConnPtr(PQconnectdb(ss.str().c_str()), &PQfinish); +} +// Superuser backend connection for DDL / reading pg_authid. +static PGConnPtr connBEsuper() { + std::stringstream ss; + ss << "user='" << cl.pgsql_server_username << "' password='" << cl.pgsql_server_password << "'"; + return connBE(ss.str()); +} +static bool execOk_local(PGconn* c, const std::string& q) { + PGresult* r = PQexec(c, q.c_str()); + bool okk = (PQresultStatus(r) == PGRES_COMMAND_OK || PQresultStatus(r) == PGRES_TUPLES_OK); + if (!okk) diag("query failed: %s -- %s", q.c_str(), PQerrorMessage(c)); + PQclear(r); + return okk; +} +static std::string execScalar_local(PGconn* c, const std::string& q) { + PGresult* r = PQexec(c, q.c_str()); + std::string v = (PQresultStatus(r) == PGRES_TUPLES_OK && PQntuples(r) > 0 && !PQgetisnull(r, 0, 0)) + ? PQgetvalue(r, 0, 0) : ""; + PQclear(r); + return v; +} + +// Parse "SCRAM-SHA-256$:$:". +static bool parseVerifier(const std::string& v, int& iters, std::string& saltb64, std::string& serverb64) { + const std::string pfx = "SCRAM-SHA-256$"; + if (v.compare(0, pfx.size(), pfx) != 0) return false; + std::string s = v.substr(pfx.size()); + auto dollar = s.find('$'); + if (dollar == std::string::npos) return false; + std::string left = s.substr(0, dollar); // ":" + std::string right = s.substr(dollar + 1); // ":" + auto c1 = left.find(':'); + auto c2 = right.find(':'); + if (c1 == std::string::npos || c2 == std::string::npos) return false; + iters = atoi(left.substr(0, c1).c_str()); + saltb64 = left.substr(c1 + 1); + serverb64 = right.substr(c2 + 1); + return iters > 0 && !saltb64.empty() && !serverb64.empty(); +} + +// b64-encode a 32-byte key with libpq's pg_b64_encode (NUL-terminated). +static std::string b64key(const unsigned char* key) { + char buf[128] = {0}; + int n = pg_b64_encode((const char*)key, 32, buf, (int)sizeof(buf) - 1); + if (n <= 0) return ""; + buf[n] = '\0'; + return std::string(buf); +} + +// Derive the ClientKey/ServerKey pair ProxySQL would inject, from the role's real verifier + password. +// Returns false only on a crypto/parse failure (which would be a TEST bug, not a patch bug). +static bool deriveScramKeys(const std::string& verifier, const std::string& password, + std::string& ck_b64, std::string& sk_b64, std::string& derivedServerMatchesStored) { + int iters = 0; + std::string saltb64, storedServerb64; + if (!parseVerifier(verifier, iters, saltb64, storedServerb64)) { + diag("could not parse verifier: %s", verifier.c_str()); + return false; + } + unsigned char salt[256]; + int saltlen = pg_b64_decode(saltb64.c_str(), (int)saltb64.size(), (char*)salt, (int)sizeof(salt)); + if (saltlen <= 0) { diag("salt b64 decode failed"); return false; } + + unsigned char SaltedPassword[32]; + if (PKCS5_PBKDF2_HMAC(password.c_str(), (int)password.size(), salt, saltlen, iters, + EVP_sha256(), 32, SaltedPassword) != 1) { + diag("PBKDF2 failed"); return false; + } + unsigned char ClientKey[32], ServerKey[32]; + unsigned int mlen = 0; + if (!HMAC(EVP_sha256(), SaltedPassword, 32, (const unsigned char*)"Client Key", 10, ClientKey, &mlen) || mlen != 32) { + diag("HMAC ClientKey failed"); return false; + } + if (!HMAC(EVP_sha256(), SaltedPassword, 32, (const unsigned char*)"Server Key", 10, ServerKey, &mlen) || mlen != 32) { + diag("HMAC ServerKey failed"); return false; + } + ck_b64 = b64key(ClientKey); + sk_b64 = b64key(ServerKey); + // sanity: our derived ServerKey must equal the verifier's stored ServerKey. + derivedServerMatchesStored = (sk_b64 == storedServerb64) ? "yes" : "NO"; + if (sk_b64 != storedServerb64) { + diag("derived ServerKey (%s) != verifier stored ServerKey (%s)", sk_b64.c_str(), storedServerb64.c_str()); + } + return !ck_b64.empty() && !sk_b64.empty(); +} + +static bool connOk(const PGConnPtr& c) { return c && PQstatus(c.get()) == CONNECTION_OK; } + +int main(int, char**) { + plan(6); + if (cl.getEnv()) return exit_status(); + + // (0) CANARY: the patched libpq must RECOGNISE all three new keywords. Empty values are inert + // (has_ck==has_sk==false, md5_secret ignored), so this connects normally as the superuser. + // An UNPATCHED libpq returns CONNECTION_BAD with "invalid connection option". + { + std::stringstream p; + p << "user='" << cl.pgsql_server_username << "' password='" << cl.pgsql_server_password + << "' scram_client_key='' scram_server_key='' md5_secret=''"; + auto c = connBE(p.str()); + std::string e = c ? PQerrorMessage(c.get()) : "(null conn)"; + bool recognised = (e.find("invalid connection option") == std::string::npos); + ok(recognised, + "patched libpq RECOGNISES scram_client_key/scram_server_key/md5_secret (else deps libpq is UNPATCHED). status=%s msg=%s", + connOk(c) ? "OK" : "not-OK", e.c_str()); + if (!recognised) { + diag("The vendored libpq is not the #5865 patched build -- rebuild the postgresql dep. Bailing."); + return exit_status(); + } + } + + // Setup: create a scram-sha-256 backend role we control, read its real verifier, derive the pair. + auto be = connBEsuper(); + if (!connOk(be)) BAIL_OUT("no superuser backend connection: %s", be ? PQerrorMessage(be.get()) : "(null)"); + const std::string ROLE = "lp_scram"; + const std::string PW = "libpqparam_pw"; + execOk_local(be.get(), "SET password_encryption TO 'scram-sha-256'"); + execOk_local(be.get(), "DROP ROLE IF EXISTS " + ROLE); + if (!execOk_local(be.get(), "CREATE ROLE " + ROLE + " LOGIN PASSWORD '" + PW + "'")) + BAIL_OUT("could not create backend role %s", ROLE.c_str()); + + std::string verifier = execScalar_local(be.get(), + "SELECT rolpassword FROM pg_authid WHERE rolname='" + ROLE + "'"); + diag("stored verifier for %s: %s", ROLE.c_str(), verifier.c_str()); + + std::string ck_b64, sk_b64, skMatch; + bool derived = deriveScramKeys(verifier, PW, ck_b64, sk_b64, skMatch); + diag("derived ClientKey(b64)=%s ServerKey(b64)=%s serverkey_matches_verifier=%s", + ck_b64.c_str(), sk_b64.c_str(), skMatch.c_str()); + if (!derived) { + diag("KEY DERIVATION FAILED -- this is a TEST-side problem, not a patch defect."); + } + + // (1) POSITIVE: a correctly-derived ClientKey+ServerKey pair (and NO password, exactly like + // ProxySQL's backend leg) authenticates and can run a query. + { + std::string params = "user='" + ROLE + "' scram_client_key='" + ck_b64 + "' scram_server_key='" + sk_b64 + "'"; + auto c = connBE(params); + bool authed = connOk(c) && (execScalar_local(c.get(), "SELECT 1") == "1"); + if (!authed) diag("valid-pair connect/SELECT failed: %s", c ? PQerrorMessage(c.get()) : "(null)"); + ok(authed, "valid scram_client_key+scram_server_key pair authenticates and SELECT 1 works (no password sent)"); + } + + // (2) client key WITHOUT server key -> rejected (mutual-auth guard: has_ck != has_sk -> NULL). + { + std::string params = "user='" + ROLE + "' scram_client_key='" + ck_b64 + "'"; + auto c = connBE(params); + bool rejected = !connOk(c); + diag("(2) scram_client_key alone -> %s : %s", rejected ? "rejected" : "ACCEPTED", + c ? PQerrorMessage(c.get()) : "(null)"); + ok(rejected, "scram_client_key without scram_server_key is rejected"); + } + + // (3) server key WITHOUT client key -> rejected (guard, or 'no password'/auth failure -- any clean reject). + { + std::string params = "user='" + ROLE + "' scram_server_key='" + sk_b64 + "'"; + auto c = connBE(params); + bool rejected = !connOk(c); + diag("(3) scram_server_key alone -> %s : %s", rejected ? "rejected" : "ACCEPTED", + c ? PQerrorMessage(c.get()) : "(null)"); + ok(rejected, "scram_server_key without scram_client_key is rejected"); + } + + // (4) malformed base64 key material -> rejected cleanly ("invalid scram_client_key"), not a crash. + { + std::string params = "user='" + ROLE + "' scram_client_key='@@@notbase64@@@' scram_server_key='@@@notbase64@@@'"; + auto c = connBE(params); + bool rejected = !connOk(c); + diag("(4) malformed base64 keys -> %s : %s", rejected ? "rejected" : "ACCEPTED", + c ? PQerrorMessage(c.get()) : "(null)"); + ok(rejected, "invalid base64 SCRAM key material is rejected (no crash/UB)"); + } + + // (5) SECURITY: the raw SCRAM verifier string handed in via password= is NOT a plaintext password. + // libpq runs SASLprep+PBKDF2 over the verifier text -> wrong proof -> auth fails. + { + std::string params = "user='" + ROLE + "' password='" + verifier + "'"; + auto c = connBE(params); + bool rejected = !connOk(c); + diag("(5) verifier-as-password -> %s : %s", rejected ? "rejected" : "ACCEPTED (SECURITY FINDING)", + c ? PQerrorMessage(c.get()) : "(null)"); + ok(rejected, "SCRAM verifier string used as plaintext password is rejected (not equivalent)"); + } + + // Cleanup: all ROLE connections above are closed (scoped); drop the role. + execOk_local(be.get(), "DROP ROLE IF EXISTS " + ROLE); + return exit_status(); +} diff --git a/test/tap/tests/pgsql-md5_passthrough-t.cpp b/test/tap/tests/pgsql-md5_passthrough-t.cpp new file mode 100644 index 0000000000..7d217b52b0 --- /dev/null +++ b/test/tap/tests/pgsql-md5_passthrough-t.cpp @@ -0,0 +1,141 @@ +/** + * @file pgsql-md5_passthrough-t.cpp + * @brief Regression (PR #5865 review ask #4): direct md5 BACKEND pass-through. + * + * Proves ProxySQL authenticates to a REAL md5-auth backend without a plaintext password: for a + * user stored ONLY as an 'md5' hash (pg_authid.rolpassword, = 'md5'||md5(password||user)), + * ProxySQL injects that hash into libpq via the patched 'md5_secret' conninfo param + * (PgSQL_Connection.cpp: append_conninfo_param(conninfo,"md5_secret",userinfo->password)), so the + * backend leg completes an md5 handshake with no plaintext. A SELECT round-tripping to the backend + * is the proof. + * + * This requires an md5-capable backend: the docker-pgsql16-single infra provisions role 'md5user' + * (password_encryption='md5', plaintext 'md5user') and a pg_hba.conf 'host all md5user md5' + * rule above the scram-sha-256 catch-all (added for this test). The frontend leg also needs the + * auth-method floor lowered to md5 (pgsql-authentication_method=2): an md5 secret cannot satisfy a + * SCRAM floor (see pgsql_reconcile_auth_method in PgSQL_Protocol.cpp), so under the default floor=3 + * an md5-stored user is rejected on the FRONTEND before any backend leg. The floor is restored. + * + * Self-gates: if the backend does not permit md5 for md5user (e.g. an older infra without the + * pg_hba/role additions), the whole test skips cleanly rather than failing the suite. + * + * Per project rule: only LOAD ... TO RUNTIME (never SAVE ... TO DISK); runtime state (the injected + * pgsql_user and the auth-method floor) is restored at the end. The infra-owned 'md5user' backend + * role is left intact. + */ +#include +#include +#include +#include "libpq-fe.h" +#include "command_line.h" +#include "tap.h" +#include "utils.h" + +using PGConnPtr = std::unique_ptr; +CommandLine cl; + +static PGConnPtr openConn(const char* host, int port, const char* user, const char* pass, const char* db) { + std::stringstream ss; + ss << "host=" << host << " port=" << port << " user=" << user << " password=" << pass; + if (db && *db) ss << " dbname=" << db; + ss << " sslmode=disable"; + return PGConnPtr(PQconnectdb(ss.str().c_str()), &PQfinish); +} +static bool execOk(PGconn* c, const std::string& q) { + PGresult* r = PQexec(c, q.c_str()); + bool okk = (PQresultStatus(r) == PGRES_COMMAND_OK || PQresultStatus(r) == PGRES_TUPLES_OK); + if (!okk) diag("query failed: %s -- %s", q.c_str(), PQerrorMessage(c)); + PQclear(r); + return okk; +} +static std::string execScalar(PGconn* c, const std::string& q) { + PGresult* r = PQexec(c, q.c_str()); + std::string v = (PQresultStatus(r) == PGRES_TUPLES_OK && PQntuples(r) > 0 && !PQgetisnull(r, 0, 0)) + ? PQgetvalue(r, 0, 0) : ""; + PQclear(r); + return v; +} +// Store ONLY the md5 hash (no plaintext) in pgsql_users. DELETE-then-INSERT so a pre-existing +// runtime entry for the user cannot cause a UNIQUE conflict. +static void storeUser(PGconn* admin, const char* user, const std::string& secret) { + execOk(admin, std::string("DELETE FROM pgsql_users WHERE username='") + user + "'"); + execOk(admin, std::string("INSERT INTO pgsql_users (username,password,active,default_hostgroup) " + "VALUES ('") + user + "','" + secret + "',1,0)"); + execOk(admin, "LOAD PGSQL USERS TO RUNTIME"); +} +// Open a FRESH ProxySQL frontend connection as user/pass and return true iff `SELECT 1` returns 1 -- +// which requires the BACKEND leg to authenticate (md5_secret pass-through), the property under test. +static bool select_reaches_backend(const char* user, const char* pass) { + auto c = openConn(cl.pgsql_host, cl.pgsql_port, user, pass, "postgres"); + if (!c || PQstatus(c.get()) != CONNECTION_OK) { + diag("frontend connect failed for '%s': %s", user, c ? PQerrorMessage(c.get()) : "(null)"); + return false; + } + return execScalar(c.get(), "SELECT 1") == "1"; +} + +int main(int, char**) { + plan(2); + if (cl.getEnv()) return exit_status(); + + auto admin = openConn(cl.pgsql_admin_host, cl.pgsql_admin_port, cl.admin_username, cl.admin_password, nullptr); + if (!admin || PQstatus(admin.get()) != CONNECTION_OK) BAIL_OUT("no admin connection"); + auto be = openConn(cl.pgsql_server_host, cl.pgsql_server_port, + cl.pgsql_server_username, cl.pgsql_server_password, "postgres"); + if (!be || PQstatus(be.get()) != CONNECTION_OK) BAIL_OUT("no backend connection"); + + // The infra-provisioned md5 user: plaintext == username ('md5user'), stored md5 in the backend. + const char* U = "md5user"; + const char* P = "md5user"; + + // Read the backend's stored md5 hash (what ProxySQL will store and inject as md5_secret). + std::string M = execScalar(be.get(), + std::string("SELECT rolpassword FROM pg_authid WHERE rolname='") + U + "'"); + + // Gate: can we auth DIRECTLY to the backend with md5? If the role is missing or its rolpassword + // isn't an md5 hash, or pg_hba forbids md5 (older infra without the #5865 additions), skip cleanly. + bool md5_backend_ok = false; + if (M.rfind("md5", 0) == 0) { + auto probe = openConn(cl.pgsql_server_host, cl.pgsql_server_port, U, P, "postgres"); + md5_backend_ok = probe && PQstatus(probe.get()) == CONNECTION_OK; + } + if (!md5_backend_ok) { + skip(2, "backend does not permit md5 auth for '%s' (rolpassword='%.4s', probe %s) -- " + "md5 pass-through not exercised; infra needs the #5865 md5user + pg_hba md5 rule", + U, M.empty() ? "(none)" : M.c_str(), M.rfind("md5", 0) == 0 ? "failed" : "skipped"); + return exit_status(); + } + + // Lower the FRONTEND auth-method floor to md5 (an md5 secret is below a SCRAM floor and would be + // rejected before any backend leg). Snapshot the original FIRST and require it non-empty BEFORE any + // mutation: if we can't read it we must not touch the floor, else a silently-skipped restore would + // leave the global floor at MD5 and weaken auth for every subsequent legacy-g4 pgsql test. + std::string orig_floor = execScalar(admin.get(), + "SELECT variable_value FROM runtime_global_variables WHERE variable_name='pgsql-authentication_method'"); + if (orig_floor.empty()) + BAIL_OUT("could not read original pgsql-authentication_method -- refusing to mutate the floor"); + diag("original pgsql-authentication_method = '%s'", orig_floor.c_str()); + execOk(admin.get(), "SET pgsql-authentication_method='2'"); // 2 = MD5 + execOk(admin.get(), "LOAD PGSQL VARIABLES TO RUNTIME"); + + // Store ONLY the md5 hash (no plaintext); a query must reach the backend via md5_secret pass-through. + storeUser(admin.get(), U, M); + ok(select_reaches_backend(U, P), + "md5-only stored user '%s': SELECT 1 reaches the backend via md5_secret pass-through (no plaintext)", U); + + // Wrong password: the FRONTEND md5 handshake must fail, so the connection is rejected. + { + auto c = openConn(cl.pgsql_host, cl.pgsql_port, U, "wrong-pw", "postgres"); + ok(!c || PQstatus(c.get()) != CONNECTION_OK, + "md5-only stored user '%s': wrong password rejected at the frontend", U); + } + + // --- restore runtime (user + floor); leave the infra-owned backend role intact --- + execOk(admin.get(), std::string("DELETE FROM pgsql_users WHERE username='") + U + "'"); + execOk(admin.get(), "LOAD PGSQL USERS TO RUNTIME"); + if (!orig_floor.empty()) { + execOk(admin.get(), std::string("SET pgsql-authentication_method='") + orig_floor + "'"); + execOk(admin.get(), "LOAD PGSQL VARIABLES TO RUNTIME"); + } + return exit_status(); +} diff --git a/test/tap/tests/pgsql-scram_reload_midhandshake-t.cpp b/test/tap/tests/pgsql-scram_reload_midhandshake-t.cpp new file mode 100644 index 0000000000..5a8328f013 --- /dev/null +++ b/test/tap/tests/pgsql-scram_reload_midhandshake-t.cpp @@ -0,0 +1,162 @@ +/** + * @file pgsql-scram_reload_midhandshake-t.cpp + * @brief Regression (PR #5865 review ask #2): reload pgsql_users mid-SCRAM-handshake. + * + * ProxySQL's process_handshake_response_packet() does a FRESH credential lookup for each auth + * packet. A raw client can therefore pause a SCRAM exchange after server-first, rotate the stored + * verifier (pgsql_users.password) + LOAD PGSQL USERS TO RUNTIME, then send the client-final it + * computed for the ORIGINAL verifier. libpq cannot express this (it drives SASL atomically), so we + * drive the exchange stepwise with pg_lite_client's saslBegin()/saslFinish(). + * + * frontend ProxySQL + * |-- startup ------------------->| + * |<-- AuthenticationSASL(10) ----| + * |-- client-first (for A) ------>| (ProxySQL builds server-first from verifier A) + * |<-- SASLContinue(11) ----------| <-- saslBegin() returns this server-first + * | [TEST rotates pgsql_users to verifier B + LOAD ... TO RUNTIME] + * |-- client-final (proof of A) ->| (ProxySQL now looks up verifier B on this packet) + * |<-- ??? -----------------------| <-- saslFinish() + * + * ============================================================================================ + * PINNED CONTRACT (maintainer decision 2026-07-11: "PIN OBSERVED BEHAVIOR"). + * ============================================================================================ + * The review ask states the acceptable contract is EITHER of: + * (A) bound-to-original: the handshake stays bound to the verifier it started with, so the + * client-final computed for A succeeds and the post-auth protocol stays in sync; OR + * (B) fail-closed: the client-final for A is rejected cleanly (ErrorResponse / auth failure). + * Both are correct. What would be a FINDING (a real bug) is a THIRD outcome: a hang/timeout, a + * crash, a protocol desync, or a session left unusable afterwards. + * + * This test does NOT decide between (A) and (B); it observes which the code implements today and + * pins THAT as the regression baseline, reporting it via diag() for the maintainer to bless. + * OBSERVED ON 2026-07-11 (legacy-g4 / docker-pgsql16-single, PR #5865 head): see the run log / + * the ledger entry for the recorded outcome. If this assertion ever flips A<->B, that is a + * behavior change to review, not necessarily a bug; if it becomes the third outcome, it is a bug. + * ============================================================================================ + */ +#include +#include +#include +#include "libpq-fe.h" // admin path only (matches #5865's libpq-admin convention) +#include "pg_lite_client.h" // raw stepwise SASL frontend (MUST precede utils.h: mysql.h clash) +#include "command_line.h" +#include "tap.h" +#include "utils.h" + +using PGConnPtr = std::unique_ptr; +CommandLine cl; + +static PGConnPtr adminConn() { + std::stringstream ss; + ss << "host=" << cl.pgsql_admin_host << " port=" << cl.pgsql_admin_port + << " user=" << cl.admin_username << " password=" << cl.admin_password + << " sslmode=disable"; + return PGConnPtr(PQconnectdb(ss.str().c_str()), &PQfinish); +} +static bool execAdmin(PGconn* a, const std::string& q) { + PGresult* r = PQexec(a, q.c_str()); + bool okk = (PQresultStatus(r) == PGRES_COMMAND_OK || PQresultStatus(r) == PGRES_TUPLES_OK); + if (!okk) diag("admin query failed: %s -- %s", q.c_str(), PQerrorMessage(a)); + PQclear(r); + return okk; +} +static void setVerifier(PGconn* a, const char* user, const std::string& verifier) { + execAdmin(a, std::string("DELETE FROM pgsql_users WHERE username='") + user + "'"); + execAdmin(a, std::string("INSERT INTO pgsql_users (username,password,active,default_hostgroup) VALUES ('") + + user + "','" + verifier + "',1,0)"); + execAdmin(a, "LOAD PGSQL USERS TO RUNTIME"); +} + +int main(int, char**) { + plan(3); + if (cl.getEnv()) return exit_status(); + + auto admin = adminConn(); + if (!admin || PQstatus(admin.get()) != CONNECTION_OK) BAIL_OUT("no admin connection"); + + const char* USER = "reload_user"; + const char* PA = "reload_pw_A"; + const char* PB = "reload_pw_B"; + char* vA = PQencryptPasswordConn(admin.get(), PA, USER, "scram-sha-256"); + char* vB = PQencryptPasswordConn(admin.get(), PB, USER, "scram-sha-256"); + if (!vA || !vB) BAIL_OUT("could not generate SCRAM verifiers"); + diag("verifier A = %s", vA); + diag("verifier B = %s", vB); + + setVerifier(admin.get(), USER, vA); + + // --- Drive a raw SCRAM handshake, pausing between server-first and client-final. --- + bool contract_held = false; // assertion 2: (A) bound+in-sync OR (B) fail-closed; NOT a 3rd outcome + std::string observed = "unknown"; + try { + PgConnection c(4000); // 4s read timeout: a hung handshake surfaces as "Read timed out" + c.rawConnectStartup(cl.pgsql_host, cl.pgsql_port, USER /*db*/, USER); + std::string server_first = c.saslBegin(USER, PA); // ProxySQL builds this from verifier A + ok(!server_first.empty(), "server-first received for verifier A (server-first='%s')", + server_first.c_str()); + + // --- MUTATE runtime creds mid-handshake: rotate the stored verifier A -> B. --- + setVerifier(admin.get(), USER, vB); + diag("rotated pgsql_users['%s'] to verifier B + LOAD PGSQL USERS TO RUNTIME (mid-handshake)", USER); + + // --- Send client-final computed for the ORIGINAL verifier A. --- + int final_type = c.saslFinish(); + if (final_type == 0) { + // (A) bound-to-original: client-final for A ACCEPTED despite the rotation to B. + // Prove the post-auth protocol is in sync (ReadyForQuery) -- i.e. not a desync/unusable + // session. We deliberately do NOT run a backend query: reload_user is a frontend-only + // user with no backend role, so a query would fail at the BACKEND for reasons unrelated + // to the mid-handshake contract. ReadyForQuery from ProxySQL is the correct in-sync proof. + c.waitForReady(); + contract_held = true; + observed = "A: bound-to-original (client-final for A ACCEPTED after reload to B; " + "ReadyForQuery received, session in sync)"; + } else { + // (B) fail-closed: ProxySQL's fresh lookup of verifier B rejected the A-proof cleanly. + contract_held = true; + observed = std::string("B: fail-closed (client-final for A REJECTED after reload to B: ") + + c.getLastError() + ")"; + } + } catch (const PgException& e) { + std::string what = e.what(); + if (what.find("timed out") != std::string::npos) { + // Hang: the handshake neither completed nor was rejected -> this is the FINDING. + contract_held = false; + observed = std::string("FINDING (hang): the mid-handshake reload left the SASL exchange " + "stalled -- ") + what; + } else { + // A thrown ErrorResponse / peer-close is a clean fail-closed outcome == contract (B). + // (A libscram server-signature mismatch would also land here; still a rejection, not a + // success -- the session never becomes usable, so it is NOT the "unusable-after-success" + // third outcome.) + contract_held = true; + observed = std::string("B: fail-closed (handshake threw a clean rejection: ") + what + ")"; + } + } + diag("================================================================================="); + diag("OBSERVED CONTRACT (pin this / maintainer to bless): %s", observed.c_str()); + diag("================================================================================="); + ok(contract_held, + "mid-handshake reload: connection is EITHER rejected (fail-closed) OR stays bound to the " + "verifier used for server-first -- no hang/crash/desync [%s]", observed.c_str()); + + // --- assertion 3: ProxySQL is unharmed by the episode -- a fresh full login with the CURRENT + // verifier (B) still authenticates cleanly (proves no crash / no lasting protocol damage). --- + bool healthy_after = false; + try { + PgConnection h(4000); + h.connect(cl.pgsql_host, cl.pgsql_port, USER /*db*/, USER, PB); // full atomic SASL for B + healthy_after = true; + } catch (const PgException& e) { + diag("post-episode fresh login with verifier B threw: %s", e.what()); + } + ok(healthy_after, + "after the mid-handshake episode ProxySQL is healthy: a fresh login with the current verifier B succeeds"); + + // --- restore --- + execAdmin(admin.get(), std::string("DELETE FROM pgsql_users WHERE username='") + USER + "'"); + execAdmin(admin.get(), "LOAD PGSQL USERS TO RUNTIME"); + PQfreemem(vA); + PQfreemem(vB); + return exit_status(); +} diff --git a/test/tap/tests/pgsql-verifier_auth-t.cpp b/test/tap/tests/pgsql-verifier_auth-t.cpp index 797f0688a5..cf29e27aa2 100644 --- a/test/tap/tests/pgsql-verifier_auth-t.cpp +++ b/test/tap/tests/pgsql-verifier_auth-t.cpp @@ -81,6 +81,17 @@ int main(int, char**) { auto admin = adminConn(); if (!admin || PQstatus(admin.get()) != CONNECTION_OK) BAIL_OUT("no admin connection"); + // Suite isolation: remember the floor we started with and restore exactly that at the end + // (the suite default is not guaranteed to be 3). + std::string orig_floor; + { + PGresult* r = PQexec(admin.get(), + "SELECT variable_value FROM runtime_global_variables WHERE variable_name='pgsql-authentication_method'"); + if (PQresultStatus(r) == PGRES_TUPLES_OK && PQntuples(r) > 0) orig_floor = PQgetvalue(r, 0, 0); + PQclear(r); + } + if (orig_floor.empty()) BAIL_OUT("could not read original pgsql-authentication_method (empty)"); + const char* P = "verifierpass123"; // Generate a SCRAM verifier and an md5 hash for the same password (computed locally by libpq). @@ -112,7 +123,8 @@ int main(int, char**) { auto c = frontendConn("scram_user", P); ok(c && PQstatus(c.get()) == CONNECTION_OK, "verifier-stored user uses SCRAM even under a cleartext floor (no downgrade)"); } - setFloor(admin.get(), "3"); // restore scram-sha-256 + setFloor(admin.get(), orig_floor.c_str()); // restore original floor (needed so the anti-enumeration + // baseline below stays comparable to the floor it was captured under) delUser(admin.get(), "scram_user"); // (4) Anti-enumeration: an unknown user must fail with the SAME client-visible error TEMPLATE as a @@ -169,7 +181,7 @@ int main(int, char**) { ok(!v.empty() && v != "4", "SCRAM-SHA-256-PLUS (4) not selectable; runtime value clamped to 1..3 (got '%s')", v.c_str()); } - setFloor(admin.get(), "3"); // restore + setFloor(admin.get(), orig_floor.c_str()); // restore original floor // (9) Load-time validation: a malformed SCRAM verifier is rejected at LOAD (not silently stored // as plaintext) — so it must NOT appear in the runtime user set. @@ -186,6 +198,10 @@ int main(int, char**) { execAdmin(admin.get(), "DELETE FROM pgsql_users WHERE username='bad_verifier_user'"); execAdmin(admin.get(), "LOAD PGSQL USERS TO RUNTIME"); + // Unconditional final restore: guarantee the suite-wide floor is left exactly as found, + // regardless of the scenario sequence above. + if (!orig_floor.empty()) setFloor(admin.get(), orig_floor.c_str()); + if (scram) PQfreemem(scram); if (md5) PQfreemem(md5); return exit_status(); diff --git a/test/tap/tests/pgsql-verifier_backend_kill-t.cpp b/test/tap/tests/pgsql-verifier_backend_kill-t.cpp new file mode 100644 index 0000000000..3b563149f9 --- /dev/null +++ b/test/tap/tests/pgsql-verifier_backend_kill-t.cpp @@ -0,0 +1,219 @@ +/** + * @file pgsql-verifier_backend_kill-t.cpp + * @brief Regression (PR #5865 review ask #1): the auxiliary backend-termination + * connection must pass-through verifier/md5 credentials, not a plaintext password. + * + * With pgsql-kill_backend_connection_when_disconnect=true, dropping the frontend abruptly + * while a backend query runs must actually KILL the backend PID -- even when the user is + * stored as a SCRAM verifier (or md5 hash), where ProxySQL has no plaintext to fall back on. + * + * The auxiliary termination connection is built in PgSQL_backend_kill_thread() + * (lib/PgSQL_Connection.cpp). Unlike PgSQL_Connection::connect_start(), which hands libpq the + * harvested scram_client_key / md5_secret, the kill path ships the stored secret as an ordinary + * `password=` conninfo param. For a verifier/md5-stored user that stored secret is NOT the + * plaintext, so the kill connection cannot authenticate and pg_terminate_backend never runs -> + * the backend PID survives. This test proves whether that termination actually happens. + * + * Assertions: + * 1. backend role kill_scram is stored as a SCRAM verifier (test precondition). + * 2. kill=true : SCRAM-verifier user -> backend PID terminated on frontend disconnect. + * 3. kill=false: negative control -> backend PID SURVIVES the disconnect (proves the kill in #2 + * is caused by the variable, not by pg_sleep ending or the backend noticing the closed socket). + * 4. kill=true : md5-hash user -> backend PID terminated (or clean skip if the backend's pg_hba + * does not permit md5, e.g. the scram-only legacy-g4 infra). + */ +#include +#include +#include +#include +#include +#include +#include "libpq-fe.h" +#include "command_line.h" +#include "tap.h" +#include "utils.h" + +using PGConnPtr = std::unique_ptr; +CommandLine cl; + +// Runtime admin variable that gates the auxiliary kill/terminate connection. +static const char* KILL_VAR = "pgsql-kill_backend_connection_when_disconnect"; + +static PGConnPtr openConn(const char* host, int port, const char* user, const char* pass, const char* db) { + std::stringstream ss; + ss << "host=" << host << " port=" << port << " user=" << user << " password=" << pass; + if (db && *db) ss << " dbname=" << db; + ss << " sslmode=disable"; + return PGConnPtr(PQconnectdb(ss.str().c_str()), &PQfinish); +} +static bool execOk(PGconn* c, const std::string& q) { + PGresult* r = PQexec(c, q.c_str()); + bool okk = (PQresultStatus(r) == PGRES_COMMAND_OK || PQresultStatus(r) == PGRES_TUPLES_OK); + if (!okk) diag("query failed: %s -- %s", q.c_str(), PQerrorMessage(c)); + PQclear(r); + return okk; +} +static std::string execScalar(PGconn* c, const std::string& q) { + PGresult* r = PQexec(c, q.c_str()); + std::string v = (PQresultStatus(r) == PGRES_TUPLES_OK && PQntuples(r) > 0 && !PQgetisnull(r, 0, 0)) + ? PQgetvalue(r, 0, 0) : ""; + PQclear(r); + return v; +} +static void storeUser(PGconn* a, const char* user, const std::string& secret) { + execOk(a, std::string("INSERT INTO pgsql_users (username,password,active,default_hostgroup) VALUES ('") + + user + "','" + secret + "',1,0)"); + execOk(a, "LOAD PGSQL USERS TO RUNTIME"); +} + +// The backend PID running our `SELECT pg_sleep(60)` for `user`, observed from a DIRECT backend +// connection. Found via pg_stat_activity rather than pg_backend_pid() through the proxy: with +// multiplexing on, the pid a proxied SELECT pg_backend_pid() reports need not be the connection the +// later async pg_sleep runs on -- looking it up by (usename, running query) pins the real one. +static std::string find_sleep_pid(PGconn* obs, const char* user) { + return execScalar(obs, std::string( + "SELECT pid FROM pg_stat_activity WHERE usename='") + user + + "' AND query LIKE '%pg_sleep%' AND pid<>pg_backend_pid() ORDER BY backend_start DESC LIMIT 1"); +} +static std::string wait_for_sleep_pid(PGconn* obs, const char* user, int timeout_ms) { + for (int i = 0; i < timeout_ms / 100; ++i) { + std::string pid = find_sleep_pid(obs, user); + if (!pid.empty()) return pid; + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + } + return ""; +} +static bool pid_present(PGconn* obs, const std::string& pid) { + return execScalar(obs, std::string("SELECT count(*) FROM pg_stat_activity WHERE pid=") + pid) != "0"; +} + +// Open a frontend through ProxySQL as `user`, start `SELECT pg_sleep(60)` async, wait for it to +// reach a backend, then drop the frontend abruptly (PQfinish closes the socket, no graceful +// terminate). Returns the backend PID that was running the query (empty on setup failure). +static std::string start_query_and_drop_frontend(PGconn* obs, const char* user, const char* pass) { + auto fe = openConn(cl.pgsql_host, cl.pgsql_port, user, pass, "postgres"); + if (!fe || PQstatus(fe.get()) != CONNECTION_OK) { + diag("frontend connect failed for '%s': %s", user, fe ? PQerrorMessage(fe.get()) : "(null)"); + return ""; + } + if (!PQsendQuery(fe.get(), "SELECT pg_sleep(60)")) { + diag("failed to start pg_sleep for '%s': %s", user, PQerrorMessage(fe.get())); + return ""; + } + std::string pid = wait_for_sleep_pid(obs, user, 5000); + if (pid.empty()) { diag("pg_sleep for '%s' never reached a backend", user); return ""; } + fe.reset(); // abrupt frontend disconnect + return pid; +} + +// Terminate any lingering pg_sleep backends left by a test phase (e.g. the kill=false control), +// so nothing survives past the test. +static void reap_sleepers(PGconn* obs, const char* user) { + execScalar(obs, std::string( + "SELECT count(pg_terminate_backend(pid)) FROM pg_stat_activity WHERE usename='") + user + + "' AND query LIKE '%pg_sleep%' AND pid<>pg_backend_pid()"); +} + +int main(int, char**) { + plan(4); + if (cl.getEnv()) return exit_status(); + + auto admin = openConn(cl.pgsql_admin_host, cl.pgsql_admin_port, cl.admin_username, cl.admin_password, nullptr); + if (!admin || PQstatus(admin.get()) != CONNECTION_OK) BAIL_OUT("no admin connection"); + auto be = openConn(cl.pgsql_server_host, cl.pgsql_server_port, + cl.pgsql_server_username, cl.pgsql_server_password, "postgres"); + if (!be || PQstatus(be.get()) != CONNECTION_OK) BAIL_OUT("no backend connection"); + + // Capture the ORIGINAL runtime value so we restore exactly that (the suite default is `true`, + // not `false`; the same isolation discipline the maintainer flagged for ask #6). + std::string orig_kill = execScalar(admin.get(), + std::string("SELECT variable_value FROM runtime_global_variables WHERE variable_name='") + KILL_VAR + "'"); + diag("original %s = '%s'", KILL_VAR, orig_kill.c_str()); + + const char* P = "killpass_1"; + execOk(be.get(), "SET password_encryption TO 'scram-sha-256'"); + + // --- SCRAM-verifier user: store the backend's EXACT verifier (byte-identical, needed for pass-through) --- + execOk(be.get(), "DROP ROLE IF EXISTS kill_scram"); + execOk(be.get(), std::string("CREATE ROLE kill_scram LOGIN PASSWORD '") + P + "'"); + std::string V = execScalar(be.get(), "SELECT rolpassword FROM pg_authid WHERE rolname='kill_scram'"); + ok(V.rfind("SCRAM-SHA-256$", 0) == 0, "backend role kill_scram is stored as a SCRAM verifier"); + storeUser(admin.get(), "kill_scram", V); + + // (2) kill=true : the PID must disappear (the aux termination conn authenticated via pass-through). + execOk(admin.get(), std::string("SET ") + KILL_VAR + "='true'"); + execOk(admin.get(), "LOAD PGSQL VARIABLES TO RUNTIME"); + { + std::string pid = start_query_and_drop_frontend(be.get(), "kill_scram", P); + bool terminated = false; + if (!pid.empty()) { + for (int i = 0; i < 10 * 5; ++i) { // up to ~10s + if (!pid_present(be.get(), pid)) { terminated = true; break; } + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + } + } + ok(terminated, + "SCRAM verifier user (kill=true): backend PID %s terminated on frontend disconnect (aux conn used pass-through)", + pid.empty() ? "(none)" : pid.c_str()); + reap_sleepers(be.get(), "kill_scram"); + } + + // (3) Negative control kill=false : the SAME user's PID must SURVIVE the disconnect. + execOk(admin.get(), std::string("SET ") + KILL_VAR + "='false'"); + execOk(admin.get(), "LOAD PGSQL VARIABLES TO RUNTIME"); + { + std::string pid = start_query_and_drop_frontend(be.get(), "kill_scram", P); + bool survived = false; + if (!pid.empty()) { + std::this_thread::sleep_for(std::chrono::milliseconds(3000)); // give any (wrong) kill time to act + survived = pid_present(be.get(), pid); + } + ok(survived, + "negative control (kill=false): backend PID %s survives frontend disconnect (no spurious kill)", + pid.empty() ? "(none)" : pid.c_str()); + reap_sleepers(be.get(), "kill_scram"); + } + + // --- md5 user: only if the backend's pg_hba permits md5 (legacy-g4 is scram-only -> skip cleanly) --- + execOk(admin.get(), std::string("SET ") + KILL_VAR + "='true'"); + execOk(admin.get(), "LOAD PGSQL VARIABLES TO RUNTIME"); + execOk(be.get(), "SET password_encryption TO 'md5'"); + execOk(be.get(), "DROP ROLE IF EXISTS kill_md5"); + execOk(be.get(), std::string("CREATE ROLE kill_md5 LOGIN PASSWORD '") + P + "'"); + std::string M = execScalar(be.get(), "SELECT rolpassword FROM pg_authid WHERE rolname='kill_md5'"); + bool md5_backend_ok = false; + { + auto probe = openConn(cl.pgsql_server_host, cl.pgsql_server_port, "kill_md5", P, "postgres"); + md5_backend_ok = probe && PQstatus(probe.get()) == CONNECTION_OK; + } + if (M.rfind("md5", 0) == 0 && md5_backend_ok) { + storeUser(admin.get(), "kill_md5", M); + std::string pid = start_query_and_drop_frontend(be.get(), "kill_md5", P); + bool terminated = false; + if (!pid.empty()) { + for (int i = 0; i < 10 * 5; ++i) { + if (!pid_present(be.get(), pid)) { terminated = true; break; } + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + } + } + ok(terminated, + "md5-hash user (kill=true): backend PID %s terminated on frontend disconnect (aux conn used md5_secret)", + pid.empty() ? "(none)" : pid.c_str()); + reap_sleepers(be.get(), "kill_md5"); + } else { + skip(1, "backend pg_hba does not permit md5 auth (legacy-g4 is scram-only) -- md5 kill path not exercised here"); + } + + // --- restore runtime + drop roles (runs on all paths) --- + reap_sleepers(be.get(), "kill_scram"); + reap_sleepers(be.get(), "kill_md5"); + execOk(admin.get(), "DELETE FROM pgsql_users WHERE username IN ('kill_scram','kill_md5')"); + execOk(admin.get(), "LOAD PGSQL USERS TO RUNTIME"); + if (!orig_kill.empty()) { + execOk(admin.get(), std::string("SET ") + KILL_VAR + "='" + orig_kill + "'"); + execOk(admin.get(), "LOAD PGSQL VARIABLES TO RUNTIME"); + } + execOk(be.get(), "DROP ROLE IF EXISTS kill_scram"); + execOk(be.get(), "DROP ROLE IF EXISTS kill_md5"); + return exit_status(); +} diff --git a/test/tap/tests/pgsql-verifier_pool_rotation-t.cpp b/test/tap/tests/pgsql-verifier_pool_rotation-t.cpp new file mode 100644 index 0000000000..709607e85d --- /dev/null +++ b/test/tap/tests/pgsql-verifier_pool_rotation-t.cpp @@ -0,0 +1,216 @@ +/** + * @file pgsql-verifier_pool_rotation-t.cpp + * @brief Regression (PR #5865 review ask #3): pool isolation across SCRAM password rotation. + * + * A backend connection authenticated under verifier A must never be reused to serve a client that + * authenticated under verifier B after a password rotation. This is the credential-rotation + * pool-poisoning concern: if a stale A-authenticated backend session lingers in ProxySQL's pool and + * is handed to a B-authenticated frontend, the rotation did not fully flush the pool. + * + * Method (mirrors the DIRECT-backend-observer technique from pgsql-verifier_backend_kill-t.cpp): + * - Create role rot_user with password A (stored as a real SCRAM verifier), store verifier A in + * ProxySQL. Authenticate a frontend with A, run a distinctive async query, and via a DIRECT + * backend observer connection capture the backend connection IDENTITY (pid + backend_start) that + * served it -- NOT pg_backend_pid() over the proxied conn, which under multiplexing need not name + * the connection the query actually ran on. Let A's frontend close gracefully so its backend + * connection returns to ProxySQL's pool, idle. + * - Rotate: ALTER ROLE rot_user PASSWORD B on the backend AND update the ProxySQL stored verifier to + * B, LOAD PGSQL USERS TO RUNTIME. + * - Prove rotation took effect: the OLD password A is now rejected at the ProxySQL frontend. + * - Authenticate a new frontend with B, run a distinctive async query, capture the backend + * connection identity that served it. + * - Assert isolation: the B-serving backend connection is NOT the exact connection that served A + * (identity = pid + backend_start). Reuse of the A-established connection for B is a real #5865 + * pool-poisoning finding and MUST stay red. + * + * Only LOAD ... TO RUNTIME is used (never SAVE ... TO DISK); runtime state is restored and the backend + * role is dropped at the end. Guarded by cl.getEnv(). + */ +#include +#include +#include +#include +#include +#include "libpq-fe.h" +#include "command_line.h" +#include "tap.h" +#include "utils.h" + +using PGConnPtr = std::unique_ptr; +CommandLine cl; + +static PGConnPtr openConn(const char* host, int port, const char* user, const char* pass, const char* db) { + std::stringstream ss; + ss << "host=" << host << " port=" << port << " user=" << user << " password=" << pass; + if (db && *db) ss << " dbname=" << db; + ss << " sslmode=disable"; + return PGConnPtr(PQconnectdb(ss.str().c_str()), &PQfinish); +} +static bool execOk(PGconn* c, const std::string& q) { + PGresult* r = PQexec(c, q.c_str()); + bool okk = (PQresultStatus(r) == PGRES_COMMAND_OK || PQresultStatus(r) == PGRES_TUPLES_OK); + if (!okk) diag("query failed: %s -- %s", q.c_str(), PQerrorMessage(c)); + PQclear(r); + return okk; +} +static std::string execScalar(PGconn* c, const std::string& q) { + PGresult* r = PQexec(c, q.c_str()); + std::string v = (PQresultStatus(r) == PGRES_TUPLES_OK && PQntuples(r) > 0 && !PQgetisnull(r, 0, 0)) + ? PQgetvalue(r, 0, 0) : ""; + PQclear(r); + return v; +} +static void storeUser(PGconn* a, const char* user, const std::string& secret) { + execOk(a, std::string("INSERT INTO pgsql_users (username,password,active,default_hostgroup) VALUES ('") + + user + "','" + secret + "',1,0)"); + execOk(a, "LOAD PGSQL USERS TO RUNTIME"); +} + +// Identity of a backend connection as seen by postgres: pid + backend_start. backend_start is a +// per-connection constant, so (pid,backend_start) uniquely names a physical session even across PID +// recycling -- exactly what we need to tell "the A connection reused for B" from "a fresh connection". +static std::string capture_backend_identity(PGconn* obs, const char* user, const char* marker) { + return execScalar(obs, std::string( + "SELECT pid || '|' || backend_start FROM pg_stat_activity WHERE usename='") + user + + "' AND query LIKE '%" + marker + "%' AND pid<>pg_backend_pid() ORDER BY backend_start DESC LIMIT 1"); +} +static std::string wait_backend_identity(PGconn* obs, const char* user, const char* marker, int timeout_ms) { + for (int i = 0; i < timeout_ms / 100; ++i) { + std::string id = capture_backend_identity(obs, user, marker); + if (!id.empty()) return id; + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + } + return ""; +} +static bool identity_present(PGconn* obs, const std::string& identity) { + size_t bar = identity.find('|'); + if (bar == std::string::npos) return false; + std::string pid = identity.substr(0, bar); + std::string start = identity.substr(bar + 1); + return execScalar(obs, std::string( + "SELECT count(*) FROM pg_stat_activity WHERE pid=") + pid + + " AND backend_start='" + start + "'") != "0"; +} +static void reap(PGconn* obs, const char* user) { + execScalar(obs, std::string( + "SELECT count(pg_terminate_backend(pid)) FROM pg_stat_activity WHERE usename='") + user + + "' AND pid<>pg_backend_pid()"); +} + +// Acquire a frontend connection through ProxySQL that can actually run a query. Right after the +// harness reconfigures ProxySQL the backend pool is cold and the monitor has not yet validated the +// backend, so the very first connection/query for a user can fail transiently; retry until one works. +// This is connection-establishment hygiene only -- it does not touch the rotation/isolation assertions. +static PGConnPtr acquire_frontend(const char* user, const char* pass, int attempts) { + for (int i = 0; i < attempts; ++i) { + auto c = openConn(cl.pgsql_host, cl.pgsql_port, user, pass, "postgres"); + if (c && PQstatus(c.get()) == CONNECTION_OK && execScalar(c.get(), "SELECT 1") == "1") + return c; + if (i == 0) diag("frontend for '%s' not ready yet (attempt %d): %s", user, i + 1, + c ? PQerrorMessage(c.get()) : "(null)"); + std::this_thread::sleep_for(std::chrono::milliseconds(500)); + } + return PGConnPtr(nullptr, &PQfinish); +} + +// Fire a distinctive async query on an already-working frontend (so it lingers in pg_stat_activity +// long enough to be observed), capture the serving backend connection identity via the observer, then +// drain the result so the query finishes and the frontend can close GRACEFULLY -- returning that +// backend connection to ProxySQL's pool, idle (not killed). Returns the identity, or "" on failure. +static std::string serve_marked_query(PGconn* obs, PGconn* fe, const char* user, const char* marker) { + std::string q = std::string("SELECT pg_sleep(5) /* ") + marker + " */"; + if (!PQsendQuery(fe, q.c_str())) { + diag("failed to start marked query for '%s': %s", user, PQerrorMessage(fe)); + return ""; + } + std::string id = wait_backend_identity(obs, user, marker, 10000); + if (id.empty()) diag("marked query '%s' for '%s' never observed on a backend", marker, user); + // Drain: block until the async query completes, so the frontend can close gracefully. + while (PGresult* r = PQgetResult(fe)) PQclear(r); + return id; +} + +int main(int, char**) { + plan(4); + if (cl.getEnv()) return exit_status(); + + auto admin = openConn(cl.pgsql_admin_host, cl.pgsql_admin_port, cl.admin_username, cl.admin_password, nullptr); + if (!admin || PQstatus(admin.get()) != CONNECTION_OK) BAIL_OUT("no admin connection"); + auto be = openConn(cl.pgsql_server_host, cl.pgsql_server_port, + cl.pgsql_server_username, cl.pgsql_server_password, "postgres"); + if (!be || PQstatus(be.get()) != CONNECTION_OK) BAIL_OUT("no backend connection"); + + const char* PA = "rot_pw_A"; + const char* PB = "rot_pw_B"; + execOk(be.get(), "SET password_encryption TO 'scram-sha-256'"); + execOk(be.get(), "DROP ROLE IF EXISTS rot_user"); + execOk(be.get(), std::string("CREATE ROLE rot_user LOGIN PASSWORD '") + PA + "'"); + std::string VA = execScalar(be.get(), "SELECT rolpassword FROM pg_authid WHERE rolname='rot_user'"); + if (VA.rfind("SCRAM-SHA-256$", 0) != 0) BAIL_OUT("backend role rot_user is not stored as a SCRAM verifier: '%s'", VA.c_str()); + storeUser(admin.get(), "rot_user", VA); + + // clean slate: no stale rot_user backends in the pool from a prior run + reap(be.get(), "rot_user"); + + // (1) Authenticate with verifier A; capture the backend connection that serves it, then close the + // frontend gracefully so that exact backend connection returns to ProxySQL's pool, idle. + std::string idA; + { + auto c = acquire_frontend("rot_user", PA, 20); + bool auth_a = c && PQstatus(c.get()) == CONNECTION_OK; + if (auth_a) idA = serve_marked_query(be.get(), c.get(), "rot_user", "rot_marker_A"); + ok(auth_a && !idA.empty(), + "verifier A: authenticate + query through ProxySQL (A-backend identity: %s)", + idA.empty() ? "(none)" : idA.c_str()); + // c closes here (graceful PQfinish) -> A's backend conn returns to the pool. + } + // Precondition for a meaningful test: the A-authenticated backend conn should now be lingering idle + // in the pool. If ProxySQL already tore it down, poisoning is impossible and the isolation check is + // vacuously clean -- diag it so the result is interpretable either way. + { + std::this_thread::sleep_for(std::chrono::milliseconds(500)); + bool lingering = !idA.empty() && identity_present(be.get(), idA); + diag("A-authenticated backend conn %s after frontend close: %s", + idA.c_str(), lingering ? "LINGERING in pool (poisoning is possible)" : "already gone (pool flushed)"); + } + + // (2) Rotate: new backend password AND new ProxySQL stored verifier B. After this, only PB + // authenticates at the ProxySQL frontend; the old password PA must be rejected -- proving the + // rotation actually took effect (a necessary isolation proof independent of PID observation). + execOk(be.get(), std::string("ALTER ROLE rot_user PASSWORD '") + PB + "'"); + std::string VB = execScalar(be.get(), "SELECT rolpassword FROM pg_authid WHERE rolname='rot_user'"); + if (VB.rfind("SCRAM-SHA-256$", 0) != 0 || VB == VA) BAIL_OUT("verifier B not a fresh SCRAM verifier: '%s'", VB.c_str()); + // Rotate the STORED verifier in place (UPDATE, not INSERT: rot_user already exists), then reload. + execOk(admin.get(), std::string("UPDATE pgsql_users SET password='") + VB + "' WHERE username='rot_user'"); + execOk(admin.get(), "LOAD PGSQL USERS TO RUNTIME"); + { + auto c = openConn(cl.pgsql_host, cl.pgsql_port, "rot_user", PA, "postgres"); + ok(c && PQstatus(c.get()) != CONNECTION_OK, + "post-rotation: OLD password A is rejected at the ProxySQL frontend (rotation took effect)"); + } + + // (3) Authenticate with verifier B; capture the backend connection that serves it. + std::string idB; + { + auto c = acquire_frontend("rot_user", PB, 20); + bool auth_b = c && PQstatus(c.get()) == CONNECTION_OK; + if (auth_b) idB = serve_marked_query(be.get(), c.get(), "rot_user", "rot_marker_B"); + ok(auth_b && !idB.empty(), + "verifier B: authenticate + query after rotation (B-backend identity: %s)", + idB.empty() ? "(none)" : idB.c_str()); + } + + // (4) Isolation: the B-serving backend connection must NOT be the exact connection that served A. + // Identity = pid + backend_start; equality means the A-authenticated physical session was + // reused to serve B -- credential-rotation pool poisoning. Keep this RED if it happens. + ok(!idB.empty() && idB != idA, + "isolation: B is served by a DIFFERENT backend connection than A (A=%s B=%s)", + idA.empty() ? "(none)" : idA.c_str(), idB.empty() ? "(none)" : idB.c_str()); + + // cleanup: reap any lingering rot_user backends, drop the ProxySQL user and the backend role. + reap(be.get(), "rot_user"); + execOk(admin.get(), "DELETE FROM pgsql_users WHERE username='rot_user'"); + execOk(admin.get(), "LOAD PGSQL USERS TO RUNTIME"); + execOk(be.get(), "DROP ROLE IF EXISTS rot_user"); + return exit_status(); +}