Hi, Attached are some SCRAM-related patches.
The first patch fixes parsing of SCRAM iteration counts in both the backend SCRAM verifier parser and libpq's server-first-message parser. Previously both paths parsed into a long and then stored the result in an int, so values in (INT_MAX, LONG_MAX] could be accepted by strtol() and then narrowed incorrectly. I don't think this allows for any invalid logins as the password verifier would have tried to verify a different iteration count, but it's still wrong. The new common helper, scram_parse_iterations(), rejects values that are not strictly decimal digits, rejects zero, and rejects values larger than INT_MAX. This means values with leading whitespace, signs, trailing junk, decimal points, hex-style prefixes, and integer overflow are all rejected rather than being partially accepted by strtol() syntax. Patch 0002 adds a small test_scram test module for direct tests of SCRAM helpers in src/common. Currently it covers scram_parse_iterations(). I kept this separate from the parser fix so the bug fix can be backpatched independently if adding a new test module is considered too invasive for older branches. Patches 0003 through 0005 address a separate client-side resource issue. In a SCRAM exchange, the PBKDF2 iteration count is supplied by the server. A hostile or misconfigured server can advertise a very large iteration count and keep a blocking libpq connection inside scram_SaltedPassword() beyond the caller's connect_timeout. The backend has CHECK_FOR_INTERRUPTS() inside the loop, but the frontend previously had no equivalent. These patches mirror the blocking connection attempt's deadline into PGconn, add an optional interrupt callback to the common SCRAM PBKDF2 helper, and have libpq use that callback to abort once the in-flight blocking connection deadline has expired. The test modifies a SCRAM verifier to advertise a very large iteration count and verifies that connect_timeout interrupts the PBKDF2 loop. This protection applies to the blocking connection path, such as PQconnectdb(). It does not make connect_timeout apply automatically to applications driving PQconnectPoll() themselves. I considered passing the connection deadline directly into the SCRAM PBKDF2 helper, but used an interrupt callback instead. That keeps the common SCRAM code independent of libpq's timeout representation and allows the same mechanism to support other frontend abort conditions, such as cancellation of an in-progress connection attempt. Patch 0006 adds a separate libpq connection parameter, scram_max_iterations, with matching PGSCRAMMAXITERATIONS environment variable. This places a hard client-side cap on the server-advertised SCRAM iteration count before PBKDF2 begins. A value of 0 disables the cap. This is useful independently of connect_timeout as it protects clients that do not set a timeout. It also protects async PQconnectPoll() users because it does not depend on the blocking connection deadline. The attached patch currently uses a default cap of 100K. That is well above PostgreSQL's normal SCRAM iteration count (4K), but it is still a client-side behavior change for installations using unusually high SCRAM iteration counts. For reference, we added a similar patch to pgjdbc with the same 100K default. For the connection timeout work, I set up a mock postgres server that advertises a huge SCRAM iteration count. Without this patch it takes multiple seconds to perform the client PBKDF2 derivation. It still provides the "timeout expired" error, but only after the PBKDF2 work is complete: $ time PGCONNECT_TIMEOUT=1 psql postgresql://user:[email protected]/db psql: error: connection to server at "127.0.0.1", port 5432 failed: timeout expired real 0m12.716s user 0m12.613s sys 0m0.015s With the patches it rejects the connection attempt immediately (as it exceeds the 100K default): $ time LD_LIBRARY_PATH=src/interfaces/libpq PGCONNECT_TIMEOUT=1 src/bin/psql/psql postgresql://user:[email protected]/db psql: error: connection to server at "127.0.0.1", port 5432 failed: server requested SCRAM iteration count 10000000, exceeding scram_max_iterations (100000) real 0m0.059s user 0m0.002s sys 0m0.005s Explicitly overriding the max iterations and it respects the 1-second timeout: $ time LD_LIBRARY_PATH=src/interfaces/libpq PGCONNECT_TIMEOUT=1 PGSCRAMMAXITERATIONS=0 src/bin/psql/psql postgresql://user:[email protected]/db psql: error: connection to server at "127.0.0.1", port 5432 failed: could not calculate client proof: connection timeout expired real 0m1.005s user 0m0.930s sys 0m0.003s Some specific things I would particularly like feedback on: - Whether scram_max_iterations should default to 100K, some other value, or 0 to preserve existing behavior. The last option would still provide an opt-in hard cap, but would not protect clients unless they configure it. For reference, pgjdbc ships with a 100K default, but PostgreSQL deployments with tuned scram_iterations in the 250Kâ1M range do exist and would need to set the parameter explicitly on libpq upgrade. - Whether scram_SaltedPassword() should grow the new interrupt callback parameters directly rather than via the scram_SaltedPasswordExt() shim used here. I went with a shim to avoid signature churn for any out-of-tree consumers of src/common, but I have no strong attachment to the shim if changing the signature is preferred. - Whether the in-flight connect deadline should live on PGconn or be passed through fe_scram_state instead. I went with PGconn on the theory that the deadline of the current attempt is generally useful state that future auth paths (GSSAPI, LDAP, etc.) might want to consult, but the narrower scope is also reasonable. - Whether the connect_timeout behavior (0003â0005) should be backpatched, separately from the new scram_max_iterations connection parameter (0006), which I assume is HEAD-only. - Whether the iteration-count parsing fix (0001) should be backpatched. The int-truncation half is safe to backpatch in isolation; tightening parse_scram_secret() to reject zero/negative iteration counts in stored verifiers is technically a behavior change though I think it'd be invalid per the SCRAM RFC. - Whether the TAP test in 0005, which depends on a 1-second connect_timeout interrupting a doctored verifier's PBKDF2 loop, is acceptable as written. The iteration count is large enough that the loop cannot complete in any realistic CI scenario, but the test is timing-sensitive by nature. Regards, -- Sehrope Sarkuni Founder & CEO | JackDB, Inc. | https://www.jackdb.com/
From f455d607af6123c7576c42ce65463f8e3e08378e Mon Sep 17 00:00:00 2001 From: Sehrope Sarkuni <[email protected]> Date: Sat, 23 May 2026 10:11:52 -0400 Subject: [PATCH 5/6] Add test for connect_timeout during SCRAM iteration Exercises the new deadline check inside scram_SaltedPassword(). Creates a role with a normal SCRAM verifier, then doctors the stored iteration count up to a value high enough that even fast modern hardware cannot finish the loop within the 1-second connect_timeout The doctored verifier's StoredKey and ServerKey no longer match the iteration count, but that is irrelevant for this test as the patched client never reaches the end of the proof step. --- src/test/authentication/t/001_password.pl | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl index 69ed4919b16..3194d1224d0 100644 --- a/src/test/authentication/t/001_password.pl +++ b/src/test/authentication/t/001_password.pl @@ -792,4 +792,27 @@ test_conn( qr/connection authenticated: identity="regress_not_member" method=scram-sha-256/ ]); +# Test that connect_timeout interrupts a slow SCRAM auth +reset_pg_hba($node, 'all', 'all', 'trust'); +$node->safe_psql( + 'postgres', + "SET password_encryption='scram-sha-256'; + CREATE ROLE scram_slow LOGIN PASSWORD 'pass';"); +$node->safe_psql( + 'postgres', + q{UPDATE pg_authid + SET rolpassword = regexp_replace(rolpassword, + '^SCRAM-SHA-256\$[0-9]+:', + 'SCRAM-SHA-256$999999999:') + WHERE rolname = 'scram_slow';}); +reset_pg_hba($node, 'all', 'all', 'scram-sha-256'); +{ + $node->connect_fails( + "user=scram_slow connect_timeout=1", + 'connect_timeout aborts SCRAM iteration loop', + expected_stderr => qr/connection timeout expired/); +} +reset_pg_hba($node, 'all', 'all', 'trust'); +$node->safe_psql('postgres', 'DROP ROLE scram_slow;'); + done_testing(); -- 2.43.0
From 375046649a87e29e862db762ec7abc7323728307 Mon Sep 17 00:00:00 2001 From: Sehrope Sarkuni <[email protected]> Date: Sat, 23 May 2026 10:10:06 -0400 Subject: [PATCH 4/6] libpq: honor connect_timeout during SCRAM iteration The PBKDF2 iteration count in a SCRAM exchange is dictated by the server, so a hostile or misconfigured server can make a libpq client spin in scram_SaltedPassword() for an unbounded amount of CPU time beyond any connect_timeout the caller asked for. The backend has CHECK_FOR_INTERRUPTS() inside the iteration loop to make this interruptible but the frontend had no equivalent. Adds scram_SaltedPasswordExt(), a variant of scram_SaltedPassword() that takes an optional interrupt callback. In frontend builds, the PBKDF2 loop checks the callback every SCRAM_INTERRUPT_CHECK_INTERVAL iterations and aborts if the callback returns true. libpq passes a callback that checks the current time against conn->connect_deadline (the connect_timeout deadline of the in-flight attempt) so a long SCRAM exchange no longer outlives the timeout. Backend callsites pass NULL and are unchanged. The existing scram_SaltedPassword() now invokes the new variant with a NULL callback so there is no signature change to anything linking to that library. Async callers that drive PQconnectPoll() directly do not go through pqConnectDBComplete() and therefore never have conn->connect_deadline set. --- doc/src/sgml/libpq.sgml | 11 ++++++ src/common/scram-common.c | 56 ++++++++++++++++++++++++++-- src/include/common/scram-common.h | 24 ++++++++++++ src/interfaces/libpq/fe-auth-scram.c | 31 ++++++++++++--- 4 files changed, 114 insertions(+), 8 deletions(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 7d3c3bb66d8..af73a041762 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -1444,6 +1444,17 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname seconds, so the total time spent waiting for a connection might be up to 10 seconds. </para> + <para> + In addition to bounding network establishment, this deadline + also bounds client-side SCRAM PBKDF2 work performed during + authentication on the blocking connection path + (<xref linkend="libpq-PQconnectdb"/> and friends). Connecting to + a server that advertises a very large SCRAM iteration count under + an unusually small <literal>connect_timeout</literal> may + therefore fail with + <literal>connection timeout expired</literal> during authentication + rather than during network establishment. + </para> </listitem> </varlistentry> diff --git a/src/common/scram-common.c b/src/common/scram-common.c index 0644771b5a6..dc554cbaeef 100644 --- a/src/common/scram-common.c +++ b/src/common/scram-common.c @@ -29,18 +29,54 @@ #endif #include "port/pg_bswap.h" +#ifdef FRONTEND +/* + * Invoke the caller-supplied interrupt callback every this many PBKDF2 + * iterations. + */ +#define SCRAM_INTERRUPT_CHECK_INTERVAL 4096 +#endif + /* * Calculate SaltedPassword. * - * The password should already be normalized by SASLprep. Returns 0 on - * success, -1 on failure with *errstr pointing to a message about the - * error details. + * Equivalent to scram_SaltedPasswordExt() with no interrupt callback. + * Preserved as a stable entry point for backend code and any external + * consumers compiled against the pre-callback signature. */ int scram_SaltedPassword(const char *password, pg_cryptohash_type hash_type, int key_length, const uint8 *salt, int saltlen, int iterations, uint8 *result, const char **errstr) +{ + return scram_SaltedPasswordExt(password, hash_type, key_length, + salt, saltlen, iterations, + NULL, NULL, + result, errstr); +} + +/* + * Calculate SaltedPassword, with an optional caller interrupt callback. + * + * The password should already be normalized by SASLprep. Returns 0 on + * success, -1 on failure with *errstr pointing to a message about the + * error details. + * + * In frontend code, an optional interrupt callback may be supplied. It + * is invoked periodically from within the PBKDF2 iteration loop, and if + * it returns true the loop aborts with the callback-provided errstr. + * The callback is the frontend analogue of the backend's + * CHECK_FOR_INTERRUPTS() check. Pass NULL to disable. Ignored in the + * backend, which uses CHECK_FOR_INTERRUPTS() directly. + */ +int +scram_SaltedPasswordExt(const char *password, + pg_cryptohash_type hash_type, int key_length, + const uint8 *salt, int saltlen, int iterations, + scram_interrupt_callback interrupt_cb, + void *interrupt_arg, + uint8 *result, const char **errstr) { int password_len = strlen(password); uint32 one = pg_hton32(1); @@ -84,6 +120,20 @@ scram_SaltedPassword(const char *password, * set to a large value. */ CHECK_FOR_INTERRUPTS(); +#else + /* + * In the frontend, the iteration count is dictated by the server + * and could be set to a large value. Allow the caller to abort + * the loop via the interrupt callback. The callback owns both the + * abort policy and the error message. + */ + if (interrupt_cb != NULL && + (i % SCRAM_INTERRUPT_CHECK_INTERVAL) == 0 && + interrupt_cb(interrupt_arg, errstr)) + { + pg_hmac_free(hmac_ctx); + return -1; + } #endif if (pg_hmac_init(hmac_ctx, (const uint8 *) password, password_len) < 0 || diff --git a/src/include/common/scram-common.h b/src/include/common/scram-common.h index 27f42fdef02..e5cdb856641 100644 --- a/src/include/common/scram-common.h +++ b/src/include/common/scram-common.h @@ -49,10 +49,34 @@ */ #define SCRAM_SHA_256_DEFAULT_ITERATIONS 4096 +/* + * Optional caller-supplied interrupt check for scram_SaltedPasswordExt(). + * + * Called periodically from within the PBKDF2 iteration loop in frontend + * builds. Return true to abort, false to continue iterating. On abort, + * set *errstr to the error message to surface to the caller of + * scram_SaltedPasswordExt(). arg is opaque caller-owned state. + * + * The SCRAM code knows nothing about the policy behind the check. It + * simply offers a hook analogous to the backend's CHECK_FOR_INTERRUPTS(). + */ +typedef bool (*scram_interrupt_callback) (void *arg, const char **errstr); + +/* + * Original entry point. Equivalent to scram_SaltedPasswordExt() with + * NULL callback arguments. Kept so existing callers do not need to change. + */ extern int scram_SaltedPassword(const char *password, pg_cryptohash_type hash_type, int key_length, const uint8 *salt, int saltlen, int iterations, uint8 *result, const char **errstr); + +extern int scram_SaltedPasswordExt(const char *password, + pg_cryptohash_type hash_type, int key_length, + const uint8 *salt, int saltlen, int iterations, + scram_interrupt_callback interrupt_cb, + void *interrupt_arg, + uint8 *result, const char **errstr); extern int scram_H(const uint8 *input, pg_cryptohash_type hash_type, int key_length, uint8 *result, const char **errstr); diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe-auth-scram.c index ac7ce3c304b..540d9baf0b2 100644 --- a/src/interfaces/libpq/fe-auth-scram.c +++ b/src/interfaces/libpq/fe-auth-scram.c @@ -89,6 +89,26 @@ static bool verify_server_signature(fe_scram_state *state, bool *match, static bool calculate_client_proof(fe_scram_state *state, const char *client_final_message_without_proof, uint8 *result, const char **errstr); +static bool scram_check_connect_deadline(void *arg, const char **errstr); + +/* + * Interrupt callback passed to scram_SaltedPasswordExt(). Aborts the + * PBKDF2 loop if the in-flight connect_timeout deadline has expired and + * surfaces the failure as a libpq connection timeout. + */ +static bool +scram_check_connect_deadline(void *arg, const char **errstr) +{ + PGconn *conn = (PGconn *) arg; + + if (conn->connect_deadline > 0 && + PQgetCurrentTimeUSec() > conn->connect_deadline) + { + *errstr = libpq_gettext("connection timeout expired"); + return true; + } + return false; +} /* * Initialize SCRAM exchange status. @@ -788,14 +808,15 @@ calculate_client_proof(fe_scram_state *state, * Calculate SaltedPassword, and store it in 'state' so that we can * reuse it later in verify_server_signature. */ - if (scram_SaltedPassword(state->password, state->hash_type, - state->key_length, state->salt, state->saltlen, - state->iterations, state->SaltedPassword, - errstr) < 0 || + if (scram_SaltedPasswordExt(state->password, state->hash_type, + state->key_length, state->salt, state->saltlen, + state->iterations, + scram_check_connect_deadline, state->conn, + state->SaltedPassword, errstr) < 0 || scram_ClientKey(state->SaltedPassword, state->hash_type, state->key_length, ClientKey, errstr) < 0) { - /* errstr is already filled here */ + /* errstr is already filled in by the failing function */ pg_hmac_free(ctx); return false; } -- 2.43.0
From 473f9de901867e529d065c198195553f00e8047e Mon Sep 17 00:00:00 2001 From: Sehrope Sarkuni <[email protected]> Date: Sun, 24 May 2026 12:19:35 -0400 Subject: [PATCH 2/6] Add test_scram module for direct unit tests of SCRAM helpers Adds src/test/modules/test_scram/, mirroring the layout of the existing test_saslprep module, with one SQL-callable wrapper around scram_parse_iterations() and a regression test that exercises the parser directly rather than only through a SCRAM authentication exchange. Coverage is organized by why the input is accepted or rejected. Wires the new module into src/test/modules/Makefile and src/test/modules/meson.build so it runs under check-world. --- src/test/modules/Makefile | 1 + src/test/modules/meson.build | 1 + src/test/modules/test_scram/Makefile | 23 ++++ src/test/modules/test_scram/README | 18 +++ .../test_scram/expected/test_scram.out | 122 ++++++++++++++++++ src/test/modules/test_scram/meson.build | 33 +++++ .../modules/test_scram/sql/test_scram.sql | 31 +++++ .../modules/test_scram/test_scram--1.0.sql | 16 +++ src/test/modules/test_scram/test_scram.c | 45 +++++++ .../modules/test_scram/test_scram.control | 5 + 10 files changed, 295 insertions(+) create mode 100644 src/test/modules/test_scram/Makefile create mode 100644 src/test/modules/test_scram/README create mode 100644 src/test/modules/test_scram/expected/test_scram.out create mode 100644 src/test/modules/test_scram/meson.build create mode 100644 src/test/modules/test_scram/sql/test_scram.sql create mode 100644 src/test/modules/test_scram/test_scram--1.0.sql create mode 100644 src/test/modules/test_scram/test_scram.c create mode 100644 src/test/modules/test_scram/test_scram.control diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile index 0a74ab5c86f..dc010fb111f 100644 --- a/src/test/modules/Makefile +++ b/src/test/modules/Makefile @@ -49,6 +49,7 @@ SUBDIRS = \ test_resowner \ test_rls_hooks \ test_saslprep \ + test_scram \ test_shmem \ test_shm_mq \ test_slru \ diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build index 4bca42bb370..a8019538ef0 100644 --- a/src/test/modules/meson.build +++ b/src/test/modules/meson.build @@ -50,6 +50,7 @@ subdir('test_regex') subdir('test_resowner') subdir('test_rls_hooks') subdir('test_saslprep') +subdir('test_scram') subdir('test_shmem') subdir('test_shm_mq') subdir('test_slru') diff --git a/src/test/modules/test_scram/Makefile b/src/test/modules/test_scram/Makefile new file mode 100644 index 00000000000..b597ca5035a --- /dev/null +++ b/src/test/modules/test_scram/Makefile @@ -0,0 +1,23 @@ +# src/test/modules/test_scram/Makefile + +MODULE_big = test_scram +OBJS = \ + $(WIN32RES) \ + test_scram.o +PGFILEDESC = "test_scram - test SCRAM helpers in src/common" + +EXTENSION = test_scram +DATA = test_scram--1.0.sql + +REGRESS = test_scram + +ifdef USE_PGXS +PG_CONFIG = pg_config +PGXS := $(shell $(PG_CONFIG) --pgxs) +include $(PGXS) +else +subdir = src/test/modules/test_scram +top_builddir = ../../../.. +include $(top_builddir)/src/Makefile.global +include $(top_srcdir)/contrib/contrib-global.mk +endif diff --git a/src/test/modules/test_scram/README b/src/test/modules/test_scram/README new file mode 100644 index 00000000000..549571245e3 --- /dev/null +++ b/src/test/modules/test_scram/README @@ -0,0 +1,18 @@ +src/test/modules/test_scram + +Direct tests for SCRAM helpers +============================== + +This module provides SQL-callable wrappers around helper functions in +src/common/scram-common.c so they can be exercised in isolation by a +regression test, without going through a real SCRAM authentication +exchange. + +Currently it covers scram_parse_iterations(). + +Running the tests +================= + + make check +or + make installcheck diff --git a/src/test/modules/test_scram/expected/test_scram.out b/src/test/modules/test_scram/expected/test_scram.out new file mode 100644 index 00000000000..70a6e5bdadf --- /dev/null +++ b/src/test/modules/test_scram/expected/test_scram.out @@ -0,0 +1,122 @@ +-- +-- Direct tests for scram_parse_iterations() in src/common/scram-common.c. +-- +CREATE EXTENSION test_scram; +-- Accepted: non-empty digit-only strings whose value is in [1, INT_MAX]. +SELECT test_scram_parse_iterations('1'); + test_scram_parse_iterations +----------------------------- + 1 +(1 row) + +SELECT test_scram_parse_iterations('4096'); + test_scram_parse_iterations +----------------------------- + 4096 +(1 row) + +SELECT test_scram_parse_iterations('999999999'); + test_scram_parse_iterations +----------------------------- + 999999999 +(1 row) + +SELECT test_scram_parse_iterations('2147483647'); -- INT_MAX on every supported platform + test_scram_parse_iterations +----------------------------- + 2147483647 +(1 row) + +SELECT test_scram_parse_iterations('0042'); -- leading zeros are still digits + test_scram_parse_iterations +----------------------------- + 42 +(1 row) + +-- Rejected: value below 1 (RFC 5802 requires a positive count). +SELECT test_scram_parse_iterations('0'); + test_scram_parse_iterations +----------------------------- + invalid +(1 row) + +-- Rejected: anything that is not a pure digit string. +SELECT test_scram_parse_iterations(''); -- empty string + test_scram_parse_iterations +----------------------------- + invalid +(1 row) + +SELECT test_scram_parse_iterations('-1'); -- sign character + test_scram_parse_iterations +----------------------------- + invalid +(1 row) + +SELECT test_scram_parse_iterations('+1'); -- sign character + test_scram_parse_iterations +----------------------------- + invalid +(1 row) + +SELECT test_scram_parse_iterations(' 1'); -- leading whitespace + test_scram_parse_iterations +----------------------------- + invalid +(1 row) + +SELECT test_scram_parse_iterations('1 '); -- trailing whitespace + test_scram_parse_iterations +----------------------------- + invalid +(1 row) + +SELECT test_scram_parse_iterations('1.0'); -- decimal point + test_scram_parse_iterations +----------------------------- + invalid +(1 row) + +SELECT test_scram_parse_iterations('1a'); -- digit then letter + test_scram_parse_iterations +----------------------------- + invalid +(1 row) + +SELECT test_scram_parse_iterations('a1'); -- letter then digit + test_scram_parse_iterations +----------------------------- + invalid +(1 row) + +SELECT test_scram_parse_iterations('abc'); -- all letters + test_scram_parse_iterations +----------------------------- + invalid +(1 row) + +SELECT test_scram_parse_iterations('0x10'); -- hex prefix is not digits-only + test_scram_parse_iterations +----------------------------- + invalid +(1 row) + +-- Rejected: out-of-range values that would otherwise narrow to a bogus int. +SELECT test_scram_parse_iterations('2147483648'); -- INT_MAX + 1 + test_scram_parse_iterations +----------------------------- + invalid +(1 row) + +SELECT test_scram_parse_iterations('9999999999'); -- > INT_MAX on every supported platform + test_scram_parse_iterations +----------------------------- + invalid +(1 row) + +SELECT test_scram_parse_iterations('99999999999999999999'); -- overflows long on every supported platform + test_scram_parse_iterations +----------------------------- + invalid +(1 row) + diff --git a/src/test/modules/test_scram/meson.build b/src/test/modules/test_scram/meson.build new file mode 100644 index 00000000000..4039ae5b2af --- /dev/null +++ b/src/test/modules/test_scram/meson.build @@ -0,0 +1,33 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +test_scram_sources = files( + 'test_scram.c', +) + +if host_system == 'windows' + test_scram_sources += rc_lib_gen.process(win32ver_rc, extra_args: [ + '--NAME', 'test_scram', + '--FILEDESC', 'test_scram - test SCRAM helpers in src/common',]) +endif + +test_scram = shared_module('test_scram', + test_scram_sources, + kwargs: pg_test_mod_args, +) +test_install_libs += test_scram + +test_install_data += files( + 'test_scram.control', + 'test_scram--1.0.sql', +) + +tests += { + 'name': 'test_scram', + 'sd': meson.current_source_dir(), + 'bd': meson.current_build_dir(), + 'regress': { + 'sql': [ + 'test_scram', + ], + }, +} diff --git a/src/test/modules/test_scram/sql/test_scram.sql b/src/test/modules/test_scram/sql/test_scram.sql new file mode 100644 index 00000000000..113b92a9736 --- /dev/null +++ b/src/test/modules/test_scram/sql/test_scram.sql @@ -0,0 +1,31 @@ +-- +-- Direct tests for scram_parse_iterations() in src/common/scram-common.c. +-- +CREATE EXTENSION test_scram; + +-- Accepted: non-empty digit-only strings whose value is in [1, INT_MAX]. +SELECT test_scram_parse_iterations('1'); +SELECT test_scram_parse_iterations('4096'); +SELECT test_scram_parse_iterations('999999999'); +SELECT test_scram_parse_iterations('2147483647'); -- INT_MAX on every supported platform +SELECT test_scram_parse_iterations('0042'); -- leading zeros are still digits + +-- Rejected: value below 1 (RFC 5802 requires a positive count). +SELECT test_scram_parse_iterations('0'); + +-- Rejected: anything that is not a pure digit string. +SELECT test_scram_parse_iterations(''); -- empty string +SELECT test_scram_parse_iterations('-1'); -- sign character +SELECT test_scram_parse_iterations('+1'); -- sign character +SELECT test_scram_parse_iterations(' 1'); -- leading whitespace +SELECT test_scram_parse_iterations('1 '); -- trailing whitespace +SELECT test_scram_parse_iterations('1.0'); -- decimal point +SELECT test_scram_parse_iterations('1a'); -- digit then letter +SELECT test_scram_parse_iterations('a1'); -- letter then digit +SELECT test_scram_parse_iterations('abc'); -- all letters +SELECT test_scram_parse_iterations('0x10'); -- hex prefix is not digits-only + +-- Rejected: out-of-range values that would otherwise narrow to a bogus int. +SELECT test_scram_parse_iterations('2147483648'); -- INT_MAX + 1 +SELECT test_scram_parse_iterations('9999999999'); -- > INT_MAX on every supported platform +SELECT test_scram_parse_iterations('99999999999999999999'); -- overflows long on every supported platform diff --git a/src/test/modules/test_scram/test_scram--1.0.sql b/src/test/modules/test_scram/test_scram--1.0.sql new file mode 100644 index 00000000000..363e2a267b9 --- /dev/null +++ b/src/test/modules/test_scram/test_scram--1.0.sql @@ -0,0 +1,16 @@ +/* src/test/modules/test_scram/test_scram--1.0.sql */ + +-- complain if script is sourced in psql, rather than via CREATE EXTENSION +\echo Use "CREATE EXTENSION test_scram" to load this file. \quit + +-- +-- test_scram_parse_iterations(text) -> text +-- +-- Returns the parsed iteration count as text, or 'invalid' if the +-- parser rejects the input. Used to exercise scram_parse_iterations() +-- in src/common/scram-common.c. +-- +CREATE FUNCTION test_scram_parse_iterations(IN input text) +RETURNS text +AS 'MODULE_PATHNAME' +LANGUAGE C STRICT; diff --git a/src/test/modules/test_scram/test_scram.c b/src/test/modules/test_scram/test_scram.c new file mode 100644 index 00000000000..681536e70d3 --- /dev/null +++ b/src/test/modules/test_scram/test_scram.c @@ -0,0 +1,45 @@ +/*-------------------------------------------------------------------------- + * + * test_scram.c + * Test harness for SCRAM helpers in src/common/scram-common.c. + * + * Portions Copyright (c) 1996-2026, PostgreSQL Global Development Group + * + * IDENTIFICATION + * src/test/modules/test_scram/test_scram.c + * + * ------------------------------------------------------------------------- + */ + +#include "postgres.h" + +#include "common/scram-common.h" +#include "fmgr.h" +#include "utils/builtins.h" + +PG_MODULE_MAGIC; + +/* + * Wrapper around scram_parse_iterations() for direct testing. + * + * Returns 'invalid' when the parser rejects the input, otherwise the + * parsed integer value as text. Returning text rather than a record + * keeps the regression output one column wide and easy to read. + */ +PG_FUNCTION_INFO_V1(test_scram_parse_iterations); +Datum +test_scram_parse_iterations(PG_FUNCTION_ARGS) +{ + text *input = PG_GETARG_TEXT_PP(0); + char *cstr = text_to_cstring(input); + int iter = -1; + char buf[32]; + + if (scram_parse_iterations(cstr, &iter)) + snprintf(buf, sizeof(buf), "%d", iter); + else + snprintf(buf, sizeof(buf), "invalid"); + + pfree(cstr); + PG_RETURN_TEXT_P(cstring_to_text(buf)); +} diff --git a/src/test/modules/test_scram/test_scram.control b/src/test/modules/test_scram/test_scram.control new file mode 100644 index 00000000000..c25017b9d81 --- /dev/null +++ b/src/test/modules/test_scram/test_scram.control @@ -0,0 +1,5 @@ +# test_scram extension +comment = 'Test SCRAM helpers in src/common' +default_version = '1.0' +module_pathname = '$libdir/test_scram' +relocatable = true -- 2.43.0
From 04a3e8c116c80daf0ebb917029e7433177313be6 Mon Sep 17 00:00:00 2001 From: Sehrope Sarkuni <[email protected]> Date: Sat, 23 May 2026 09:59:45 -0400 Subject: [PATCH 3/6] libpq: expose connect deadline on PGconn Mirror the per-host connect_timeout deadline (end_time) tracked locally in pqConnectDBComplete() onto the PGconn struct as connect_deadline. Initialized to -1 (no deadline) by pqMakeEmptyPGconn(), set whenever the local end_time is computed for a new host attempt, and reset to -1 on each new connection attempt. This exposes no new behavior on its own, but lets code reached during connection establishment, such as authentication paths, consult the deadline of the in-flight attempt. --- src/interfaces/libpq/fe-connect.c | 8 ++++++++ src/interfaces/libpq/libpq-int.h | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 4272d386e64..b766013971e 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -2807,6 +2807,12 @@ pqConnectDBComplete(PGconn *conn) if (conn == NULL || conn->status == CONNECTION_BAD) return 0; + /* + * Clear any stale deadline from a previous invocation before deciding + * whether this attempt installs one. + */ + conn->connect_deadline = -1; + /* * Set up a time limit, if connect_timeout is greater than zero. */ @@ -2836,6 +2842,7 @@ pqConnectDBComplete(PGconn *conn) conn->whichaddr != last_whichaddr)) { end_time = PQgetCurrentTimeUSec() + (pg_usec_time_t) timeout * 1000000; + conn->connect_deadline = end_time; last_whichhost = conn->whichhost; last_whichaddr = conn->whichaddr; } @@ -5042,6 +5049,7 @@ pqMakeEmptyPGconn(void) conn->show_context = PQSHOW_CONTEXT_ERRORS; conn->sock = PGINVALID_SOCKET; conn->altsock = PGINVALID_SOCKET; + conn->connect_deadline = -1; conn->Pfdebug = NULL; /* diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 461b39620c3..0f8696e032d 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -484,6 +484,14 @@ struct pg_conn int whichhost; /* host we're currently trying/connected to */ pg_conn_host *connhost; /* details about each named host */ char *connip; /* IP address for current network connection */ + /* + * Deadline for the in-progress connection attempt against the current + * host, in PQgetCurrentTimeUSec() units; -1 if no connect_timeout is in + * effect. This mirrors the local end_time tracked in + * pqConnectDBComplete() so that code paths invoked during connection + * establishment (e.g. authentication) can consult the deadline. + */ + pg_usec_time_t connect_deadline; /* * The pending command queue as a singly-linked list. Head is the command -- 2.43.0
From 5a8348d63f9d5265ce9d33bb631f49c186a72f9d Mon Sep 17 00:00:00 2001 From: Sehrope Sarkuni <[email protected]> Date: Sun, 24 May 2026 10:56:12 -0400 Subject: [PATCH 1/6] Fix int overflow when parsing SCRAM iteration count parse_scram_secret() in the backend and read_server_first_message() in libpq both parsed the iteration count with strtol() into a long and then assigned the result to int *iterations. The errno check following strtol() catches ERANGE (overflow of long), but on platforms where sizeof(long) > sizeof(int) a digit string in (INT_MAX, LONG_MAX] is parsed successfully, errno is not set, and the assignment to int silently truncates the value. Practically the truncation does not cause SCRAM authentication to succeed against a verifier the client did not have the password for, since the client and server arrive at different SaltedPasswords. The authentication just fails the wrong way, with the client either running far fewer PBKDF2 iterations than the server expected or performing a different amount of work entirely. The backend variant also silently accepted zero and negative iteration counts because no range check was present at all. Add scram_parse_iterations() in src/common/scram-common.c that rejects trailing garbage, ERANGE (overflow of long), values that would narrow incorrectly to int, and values below 1 as required by RFC 5802. Replace both ad-hoc parses with a call to the helper. --- src/backend/libpq/auth-scram.c | 5 +-- src/common/scram-common.c | 53 ++++++++++++++++++++++++++++ src/include/common/scram-common.h | 2 ++ src/interfaces/libpq/fe-auth-scram.c | 4 +-- 4 files changed, 57 insertions(+), 7 deletions(-) diff --git a/src/backend/libpq/auth-scram.c b/src/backend/libpq/auth-scram.c index 4bac15fc5c1..43205b41155 100644 --- a/src/backend/libpq/auth-scram.c +++ b/src/backend/libpq/auth-scram.c @@ -600,7 +600,6 @@ parse_scram_secret(const char *secret, int *iterations, char **salt, uint8 *stored_key, uint8 *server_key) { char *v; - char *p; char *scheme_str; char *salt_str; char *iterations_str; @@ -637,9 +636,7 @@ parse_scram_secret(const char *secret, int *iterations, *hash_type = PG_SHA256; *key_length = SCRAM_SHA_256_KEY_LEN; - errno = 0; - *iterations = strtol(iterations_str, &p, 10); - if (*p || errno != 0) + if (!scram_parse_iterations(iterations_str, iterations)) goto invalid_secret; /* diff --git a/src/common/scram-common.c b/src/common/scram-common.c index 259fa5554b6..0644771b5a6 100644 --- a/src/common/scram-common.c +++ b/src/common/scram-common.c @@ -19,6 +19,8 @@ #include "postgres_fe.h" #endif +#include <limits.h> + #include "common/base64.h" #include "common/hmac.h" #include "common/scram-common.h" @@ -327,3 +329,54 @@ scram_build_secret(pg_cryptohash_type hash_type, int key_length, return result; } + +/* + * Parse a SCRAM iteration count from a null-terminated string. + * + * On success, stores the parsed value in *iterations and returns true. + * On failure, *iterations is unchanged and false is returned. + * + * The accepted input is a non-empty sequence of ASCII digits ('0'-'9'). + * Leading whitespace, a leading '+' or '-' sign, and any trailing + * non-digit characters are rejected, even though strtol() would + * otherwise accept them; the SCRAM verifier and server-first-message + * formats both define the iteration count as a sequence of digits with + * no surrounding decoration, so anything else is malformed. + * + * Beyond digit shape we also reject ERANGE (overflow of long), values + * that would silently truncate when narrowed to int on platforms where + * sizeof(long) is greater than sizeof(int), and any value below 1 + * (RFC 5802 requires a positive iteration count). + */ +bool +scram_parse_iterations(const char *str, int *iterations) +{ + const char *p; + char *endptr; + long val; + + /* Require a non-empty, pure-digit string. */ + if (*str == '\0') + return false; + for (p = str; *p != '\0'; p++) + { + if (*p < '0' || *p > '9') + return false; + } + + errno = 0; + val = strtol(str, &endptr, 10); + + /* + * The digit-only pre-scan guarantees strtol() consumes the entire + * string, so *endptr is always '\0' here. ERANGE still has to be + * checked because strtol() reports long overflow that way; the + * int-range and below-1 checks catch inputs that are valid as longs + * but unacceptable as iteration counts. + */ + Assert(*endptr == '\0'); + if (errno != 0 || val <= 0 || val > INT_MAX) + return false; + *iterations = (int) val; + return true; +} diff --git a/src/include/common/scram-common.h b/src/include/common/scram-common.h index 52545561bd3..27f42fdef02 100644 --- a/src/include/common/scram-common.h +++ b/src/include/common/scram-common.h @@ -67,4 +67,6 @@ extern char *scram_build_secret(pg_cryptohash_type hash_type, int key_length, const uint8 *salt, int saltlen, int iterations, const char *password, const char **errstr); +extern bool scram_parse_iterations(const char *str, int *iterations); + #endif /* SCRAM_COMMON_H */ diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe-auth-scram.c index 08c7c666946..ac7ce3c304b 100644 --- a/src/interfaces/libpq/fe-auth-scram.c +++ b/src/interfaces/libpq/fe-auth-scram.c @@ -608,7 +608,6 @@ read_server_first_message(fe_scram_state *state, char *input) { PGconn *conn = state->conn; char *iterations_str; - char *endptr; char *encoded_salt; char *nonce; int decoded_salt_len; @@ -673,8 +672,7 @@ read_server_first_message(fe_scram_state *state, char *input) /* read_attr_value() has appended an error string */ return false; } - state->iterations = strtol(iterations_str, &endptr, 10); - if (*endptr != '\0' || state->iterations < 1) + if (!scram_parse_iterations(iterations_str, &state->iterations)) { libpq_append_conn_error(conn, "malformed SCRAM message (invalid iteration count)"); return false; -- 2.43.0
From 0d0f3016351ba28d11bb84563591803da5b1d26d Mon Sep 17 00:00:00 2001 From: Sehrope Sarkuni <[email protected]> Date: Sat, 23 May 2026 11:16:03 -0400 Subject: [PATCH 6/6] libpq: add scram_max_iterations connection parameter Adds a client-side hard cap on the PBKDF2 iteration count advertised by the server during a SCRAM exchange. Complements connect_timeout's SCRAM iteration deadline. Whereas the deadline protects callers that set a timeout, scram_max_iterations protects all callers including ones with no timeout configured. The new parameter is a normal libpq connection option, so it is accepted via connection strings, URI parameters, the PGSCRAMMAXITERATIONS environment variable, and PQconnectdbParams() keywords. If the server-advertised iteration count exceeds the configured limit, the connection is aborted before any PBKDF2 work runs, with an error identifying both the requested and the configured value. A value of 0 disables the check, preserving existing behavior. Defaults to 100K. Includes a TAP test covering the rejection path against a doctored verifier with a large iteration count, the accept path against a normal verifier under a generous limit, and the disabled behavior. --- doc/src/sgml/libpq.sgml | 39 ++++++++++++++++++ src/interfaces/libpq/fe-auth-scram.c | 26 +++++++++++- src/interfaces/libpq/fe-connect.c | 30 ++++++++++++++ src/interfaces/libpq/libpq-int.h | 3 ++ src/test/authentication/t/001_password.pl | 48 ++++++++++++++++++++++- 5 files changed, 144 insertions(+), 2 deletions(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index af73a041762..776bdedfdc8 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -1458,6 +1458,35 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname </listitem> </varlistentry> + <varlistentry id="libpq-connect-scram-max-iterations" xreflabel="scram_max_iterations"> + <term><literal>scram_max_iterations</literal></term> + <listitem> + <para> + Maximum acceptable PBKDF2 iteration count advertised by the server + during a SCRAM authentication exchange. If the server proposes a + higher count, the connection is aborted before any PBKDF2 work is + performed. + Defaults to <literal>100000</literal>, which is above PostgreSQL's + server-side <xref linkend="guc-scram-iterations"/> default of + <literal>4096</literal> and the per-role values + <application>psql</application>'s + <command>\password</command> command produces. + Set to <literal>0</literal> to disable the check entirely and accept + any iteration count. + Negative values are rejected at connection-option time. + </para> + <para> + This caps the client-side CPU cost of SCRAM authentication + independently of <xref linkend="libpq-connect-connect-timeout"/>, + so it protects callers that connect without a timeout, and + applies on both the blocking and asynchronous connection paths. + Clients connecting to servers configured with an unusually high + <literal>scram_iterations</literal> may need to raise this + parameter, or set it to <literal>0</literal> to disable the check. + </para> + </listitem> + </varlistentry> + <varlistentry id="libpq-connect-client-encoding" xreflabel="client_encoding"> <term><literal>client_encoding</literal></term> <listitem> @@ -9406,6 +9435,16 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough) </para> </listitem> + <listitem> + <para> + <indexterm> + <primary><envar>PGSCRAMMAXITERATIONS</envar></primary> + </indexterm> + <envar>PGSCRAMMAXITERATIONS</envar> behaves the same as the <xref + linkend="libpq-connect-scram-max-iterations"/> connection parameter. + </para> + </listitem> + <listitem> <para> <indexterm> diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe-auth-scram.c index 540d9baf0b2..f63a906f571 100644 --- a/src/interfaces/libpq/fe-auth-scram.c +++ b/src/interfaces/libpq/fe-auth-scram.c @@ -698,8 +698,32 @@ read_server_first_message(fe_scram_state *state, char *input) return false; } - if (*input != '\0') + if (*input != '\0') { libpq_append_conn_error(conn, "malformed SCRAM message (garbage at end of server-first-message)"); + return false; + } + + /* + * Enforce a client-side hard cap on the server-advertised iteration + * count, if one was configured. This protects callers (including + * those with no connect_timeout set) from a misconfigured or hostile + * server forcing arbitrarily large PBKDF2 work. + */ + if (conn->scram_max_iterations != NULL) + { + int max_iterations; + + if (!pqParseIntParam(conn->scram_max_iterations, &max_iterations, conn, + "scram_max_iterations")) + return false; + if (max_iterations > 0 && state->iterations > max_iterations) + { + libpq_append_conn_error(conn, + "server requested SCRAM iteration count %d, exceeding scram_max_iterations (%d)", + state->iterations, max_iterations); + return false; + } + } return true; } diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index b766013971e..670d5abd303 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -141,6 +141,7 @@ static int ldapServiceLookup(const char *purl, PQconninfoOption *options, #else #define DefaultGSSMode "disable" #endif +#define DefaultScramMaxIterations "100000" /* ---------- * Definition of the conninfo parameters and their fallback resources. @@ -223,6 +224,10 @@ static const internalPQconninfoOption PQconninfoOptions[] = { "Channel-Binding", "", 8, /* sizeof("require") == 8 */ offsetof(struct pg_conn, channel_binding)}, + {"scram_max_iterations", "PGSCRAMMAXITERATIONS", DefaultScramMaxIterations, NULL, + "SCRAM-Max-Iterations", "", 10, /* strlen(INT32_MAX) == 10 */ + offsetof(struct pg_conn, scram_max_iterations)}, + {"connect_timeout", "PGCONNECT_TIMEOUT", NULL, NULL, "Connect-timeout", "", 10, /* strlen(INT32_MAX) == 10 */ offsetof(struct pg_conn, connect_timeout)}, @@ -1763,6 +1768,30 @@ pqConnectOptions2(PGconn *conn) goto oom_error; } + /* + * validate scram_max_iterations option + */ + if (conn->scram_max_iterations) + { + int max_iterations; + + if (!pqParseIntParam(conn->scram_max_iterations, &max_iterations, + conn, "scram_max_iterations")) + { + conn->status = CONNECTION_BAD; + return false; + } + if (max_iterations < 0) + { + conn->status = CONNECTION_BAD; + libpq_append_conn_error(conn, + "invalid %s value: \"%s\" (must be zero or positive)", + "scram_max_iterations", + conn->scram_max_iterations); + return false; + } + } + #ifndef USE_SSL /* @@ -5114,6 +5143,7 @@ freePGconn(PGconn *conn) free(conn->pghostaddr); free(conn->pgport); free(conn->connect_timeout); + free(conn->scram_max_iterations); free(conn->pgtcp_user_timeout); free(conn->client_encoding_initial); free(conn->pgoptions); diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 0f8696e032d..90991fdf3ec 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -384,6 +384,9 @@ struct pg_conn char *pgport; /* the server's communication port number, or * a comma-separated list of ports */ char *connect_timeout; /* connection timeout (numeric string) */ + char *scram_max_iterations; /* maximum acceptable server-advertised + * SCRAM iteration count (numeric + * string); 0 disables */ char *pgtcp_user_timeout; /* tcp user timeout (numeric string) */ char *client_encoding_initial; /* encoding to use */ char *pgoptions; /* options to start the backend with */ diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl index 3194d1224d0..198a4894a0d 100644 --- a/src/test/authentication/t/001_password.pl +++ b/src/test/authentication/t/001_password.pl @@ -807,11 +807,57 @@ $node->safe_psql( WHERE rolname = 'scram_slow';}); reset_pg_hba($node, 'all', 'all', 'scram-sha-256'); { + # The default scram_max_iterations cap (100000) would reject this + # doctored verifier before any PBKDF2 work begins. Explicitly + # disable the cap with scram_max_iterations=0 so the iteration + # loop is reached and connect_timeout has something to interrupt. $node->connect_fails( - "user=scram_slow connect_timeout=1", + "user=scram_slow connect_timeout=1 scram_max_iterations=0", 'connect_timeout aborts SCRAM iteration loop', expected_stderr => qr/connection timeout expired/); } + +# Test scram_max_iterations +{ + # Default rejects excessive iterations + $node->connect_fails( + "user=scram_slow", + 'scram_max_iterations default rejects oversized server iteration count', + expected_stderr => + qr/server requested SCRAM iteration count 999999999, exceeding scram_max_iterations \(100000\)/ + ); + + # Explicit setting equal to the default behaves the same way. + $node->connect_fails( + "user=scram_slow scram_max_iterations=100000", + 'scram_max_iterations rejects oversized server iteration count', + expected_stderr => + qr/server requested SCRAM iteration count 999999999, exceeding scram_max_iterations \(100000\)/ + ); + + # Accept scram iterations below scram_max_iterations. + $node->connect_ok( + "user=scram_role scram_max_iterations=100000", + 'scram_max_iterations accepts normal server iteration count'); + + # A normal verifier connects fine under the compiled-in default. + $node->connect_ok( + "user=scram_role", + 'scram_max_iterations default accepts normal verifier'); + + # Zero disables the client-side iteration-count cap. + $node->connect_ok( + "user=scram_role scram_max_iterations=0", + 'scram_max_iterations=0 accepts normal verifier'); + + # Reject negative scram_max_iterations. + $node->connect_fails( + "user=scram_role scram_max_iterations=-1", + 'scram_max_iterations rejects negative values', + expected_stderr => + qr/invalid scram_max_iterations value: "-1" \(must be zero or positive\)/ + ); +} reset_pg_hba($node, 'all', 'all', 'trust'); $node->safe_psql('postgres', 'DROP ROLE scram_slow;'); -- 2.43.0
