From f581479fd08b813e8ac5acbd96b57539f539c85d Mon Sep 17 00:00:00 2001 From: fox0430 Date: Tue, 14 Jul 2026 20:36:20 +0900 Subject: [PATCH] fix: raise SCRAM iteration cap to 10M and make it configurable --- async_postgres/pg_auth.nim | 12 ++++++++++-- async_postgres/pg_connection/dsn.nim | 9 +++++++++ async_postgres/pg_connection/lifecycle.nim | 6 ++++-- async_postgres/pg_connection/types.nim | 14 +++++++++++++- tests/test_auth.nim | 22 +++++++++++++++++++++- tests/test_dsn.nim | 20 ++++++++++++++++++++ 6 files changed, 77 insertions(+), 6 deletions(-) diff --git a/async_postgres/pg_auth.nim b/async_postgres/pg_auth.nim index e1d8db5..e6e0772 100644 --- a/async_postgres/pg_auth.nim +++ b/async_postgres/pg_auth.nim @@ -14,6 +14,11 @@ template burnStr*(s: var string) = ncutils.burnMem(addr s[0], s.len) s.setLen(0) +const DefaultMaxScramIterations* = 10_000_000 + ## Default cap on the server-requested SCRAM iteration count. PBKDF2 runs + ## synchronously on the event loop, so an unbounded count would let a + ## malicious server pin the process; 10M is far above realistic settings. + type ScramState* = object ## Intermediate state for SCRAM-SHA-256 authentication handshake. clientNonce*: string @@ -107,7 +112,10 @@ proc scramClientFirstMessage*( result = toBytes(state.gs2Header & state.clientFirstBare) proc scramClientFinalMessage*( - password: string, serverFirstData: openArray[byte], state: var ScramState + password: string, + serverFirstData: openArray[byte], + state: var ScramState, + maxIterations: int = DefaultMaxScramIterations, ): seq[byte] = ## Generate the SCRAM-SHA-256 client-final message from the server's first response. ## Computes the client proof and stores the expected server signature in `state`. @@ -146,7 +154,7 @@ proc scramClientFinalMessage*( raise newException( PgConnectionError, "SCRAM: iteration count too small: " & $iterations ) - if iterations > 600_000: + if iterations > maxIterations: raise newException( PgConnectionError, "SCRAM: iteration count too large: " & $iterations ) diff --git a/async_postgres/pg_connection/dsn.nim b/async_postgres/pg_connection/dsn.nim index 68a2b9d..8875e8a 100644 --- a/async_postgres/pg_connection/dsn.nim +++ b/async_postgres/pg_connection/dsn.nim @@ -235,6 +235,13 @@ proc applyParam*(result: var ConnConfig, key, val: string) = raise newException(PgError, "Invalid max_message_size: " & val) if result.maxMessageSize < 0: raise newException(PgError, "max_message_size must be non-negative: " & val) + of "max_scram_iterations": + try: + result.maxScramIterations = parseInt(val) + except ValueError: + raise newException(PgError, "Invalid max_scram_iterations: " & val) + if result.maxScramIterations < 0: + raise newException(PgError, "max_scram_iterations must be non-negative: " & val) else: result.extraParams.add((key, val)) @@ -476,6 +483,7 @@ proc initConnConfig*( requireAuth: set[AuthMethod] = {}, extraParams: seq[(string, string)] = @[], maxMessageSize = 0, + maxScramIterations = 0, ): ConnConfig = ## Create a connection configuration with sensible defaults. ## For DSN-based configuration, use `parseDsn` instead. @@ -501,6 +509,7 @@ proc initConnConfig*( requireAuth: requireAuth, extraParams: extraParams, maxMessageSize: maxMessageSize, + maxScramIterations: maxScramIterations, ) proc parseDsn*(dsn: string): ConnConfig = diff --git a/async_postgres/pg_connection/lifecycle.nim b/async_postgres/pg_connection/lifecycle.nim index f1f7e40..8315e54 100644 --- a/async_postgres/pg_connection/lifecycle.nim +++ b/async_postgres/pg_connection/lifecycle.nim @@ -373,8 +373,10 @@ proc connectToHost*( "server sent AuthenticationSASLContinue without a preceding " & "AuthenticationSASL (possible protocol violation or MITM)", ) - var clientFinal = - scramClientFinalMessage(config.password, msg.saslData, scramState) + var clientFinal = scramClientFinalMessage( + config.password, msg.saslData, scramState, + config.effectiveMaxScramIterations, + ) var saslMsg = encodeSASLResponse(clientFinal) ncutils.burnMem(clientFinal) try: diff --git a/async_postgres/pg_connection/types.nim b/async_postgres/pg_connection/types.nim index 2097006..5a1b498 100644 --- a/async_postgres/pg_connection/types.nim +++ b/async_postgres/pg_connection/types.nim @@ -10,7 +10,7 @@ import std/[tables, sets, deques, lists] when defined(posix): import std/posix -import ../[async_backend, pg_errors, pg_protocol, pg_types] +import ../[async_backend, pg_auth, pg_errors, pg_protocol, pg_types] when hasChronos: import chronos/streams/tlsstream @@ -149,6 +149,10 @@ type ## further recv-buffer growth, capping memory exposure to a ## misbehaving or malicious peer. ``0`` (default) selects ## `DefaultMaxBackendMessageLen` (1 GiB). + maxScramIterations*: int + ## Upper bound on the server-requested SCRAM iteration count + ## (PostgreSQL 16+ `scram_iterations`), capping CPU spent in PBKDF2. + ## ``0`` (default) selects `DefaultMaxScramIterations` (10,000,000). tracer*: PgTracer ## Optional tracer for connection-level hooks Notification* = object ## A NOTIFY message received from PostgreSQL. @@ -692,6 +696,14 @@ func effectiveMaxMessageSize*(conn: PgConnection): int {.inline.} = else: DefaultMaxBackendMessageLen +func effectiveMaxScramIterations*(config: ConnConfig): int {.inline.} = + ## Resolves the ``ConnConfig.maxScramIterations`` default (0) to + ## ``DefaultMaxScramIterations``. + if config.maxScramIterations > 0: + config.maxScramIterations + else: + DefaultMaxScramIterations + # Internal replication LSN plumbing (cross-module use within the library) # # `pg_replication` owns the typed `Lsn` API (`confirmFlushed`, diff --git a/tests/test_auth.nim b/tests/test_auth.nim index 01ba8df..c98d469 100644 --- a/tests/test_auth.nim +++ b/tests/test_auth.nim @@ -154,10 +154,30 @@ suite "SCRAM-SHA-256": test "scramClientFinalMessage rejects excessive iteration count": var state: ScramState discard scramClientFirstMessage("user", "myNonce", state) - let serverFirst = "r=myNonceServerPart,s=c2FsdA==,i=600001" + let serverFirst = "r=myNonceServerPart,s=c2FsdA==,i=10000001" expect CatchableError: discard scramClientFinalMessage("password", toBytes(serverFirst), state) + test "scramClientFinalMessage accepts iteration count above 600000": + # PG16+ scram_iterations may legitimately exceed the OWASP-recommended 600k + var state: ScramState + discard scramClientFirstMessage("user", "myNonce", state) + let serverFirst = "r=myNonceServerPart,s=W22ZaJ0SNY7soEsUEjb6gQ==,i=1000000" + discard scramClientFinalMessage("password", toBytes(serverFirst), state) + + test "scramClientFinalMessage enforces custom maxIterations": + var state: ScramState + discard scramClientFirstMessage("user", "myNonce", state) + let serverFirst = "r=myNonceServerPart,s=W22ZaJ0SNY7soEsUEjb6gQ==,i=5001" + expect CatchableError: + discard scramClientFinalMessage( + "password", toBytes(serverFirst), state, maxIterations = 5000 + ) + discard scramClientFirstMessage("user", "myNonce", state) + let atLimit = "r=myNonceServerPart,s=W22ZaJ0SNY7soEsUEjb6gQ==,i=5000" + discard + scramClientFinalMessage("password", toBytes(atLimit), state, maxIterations = 5000) + test "scramClientFinalMessage rejects invalid base64 salt": var state: ScramState discard scramClientFirstMessage("user", "myNonce", state) diff --git a/tests/test_dsn.nim b/tests/test_dsn.nim index defaf73..a26a798 100644 --- a/tests/test_dsn.nim +++ b/tests/test_dsn.nim @@ -360,6 +360,26 @@ suite "parseDsn": expect PgError: discard parseDsn("postgresql://host/db?max_message_size=-1") + test "max_scram_iterations from URI DSN": + let cfg = parseDsn("postgresql://host/db?max_scram_iterations=1000000") + check cfg.maxScramIterations == 1000000 + + test "max_scram_iterations from keyword DSN": + let cfg = parseDsn("host=h dbname=d max_scram_iterations=2000000") + check cfg.maxScramIterations == 2000000 + + test "max_scram_iterations default is zero (use library default)": + let cfg = parseDsn("postgresql://host/db") + check cfg.maxScramIterations == 0 + + test "error: invalid max_scram_iterations value": + expect PgError: + discard parseDsn("postgresql://host/db?max_scram_iterations=abc") + + test "error: negative max_scram_iterations": + expect PgError: + discard parseDsn("postgresql://host/db?max_scram_iterations=-1") + test "error: sslrootcert file not found": expect PgError: discard parseDsn("postgresql://host/db?sslrootcert=/nonexistent/file.pem")