From 375046649a87e29e862db762ec7abc7323728307 Mon Sep 17 00:00:00 2001 From: Sehrope Sarkuni 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. + + In addition to bounding network establishment, this deadline + also bounds client-side SCRAM PBKDF2 work performed during + authentication on the blocking connection path + ( and friends). Connecting to + a server that advertises a very large SCRAM iteration count under + an unusually small connect_timeout may + therefore fail with + connection timeout expired during authentication + rather than during network establishment. + 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