#1142

Periodic SecretChecker refresh updates validation secrets but never invalidates the salt-bound upstream secrets, so a password rotation strands one fresh upstream backend spawn (28P01)

Openbannoscakesopened 2mo ago

Affected version

Supavisor 2.9.7, transaction mode, auth_query tenant (a manager user is configured, so SecretChecker is running). Postgres 17.x upstream, scram-sha-256.

Summary

After ALTER ROLE <role> PASSWORD … — including when the new plaintext is byte-identical to the old one, since PostgreSQL re-salts the SCRAM verifier on every such statement — the next fresh upstream backend spawn for that user-pool fails once with

FATAL  28P01  password authentication failed for user "<role>"   (auth.c:329)

logged by Postgres against the pooler's own address, and by Supavisor as DbHandler: Auth error … "C" => "28P01". Supavisor then self-heals and the following spawn succeeds.

The interesting part is that this still happens after Supavisor's own cache has refreshed and reads as current. SecretChecker updates the validation secrets on its periodic tick but never invalidates the upstream secrets, and once the validation cache is current, the one code path that would drop the upstream secrets short of a real 28P01 stops firing.

Expected

After the credential cache has been refreshed (by whatever mechanism), a newly spawned upstream backend should authenticate on the first attempt.

Actual

One upstream spawn per rotation is rejected with 28P01, even when the refresh has already happened and even when the plaintext never changed.

Mechanism (source references, v2.9.7)

  1. lib/supavisor/client_handler.ex:782 — on a successful client SCRAM auth, the secrets cached as the upstream credential are final_secrets, which lib/supavisor/client_handler/auth_methods/scram.ex:111-118's handle_scram_final/2 builds via resolve_final_secrets/2 (:128: %{context.secret | client_key: client_key}) from a client_key recovered from that client's own SCRAM proof. This material is salt-bound.
  2. lib/supavisor/client_authentication.ex:23 — validation secrets are cached with @default_secrets_ttl :timer.hours(24).
  3. After the rotation the cached salt is superseded, but the client's handshake at the pooler still SUCCEEDS (client and pooler agree on the stale cached salt) and therefore yields a client_key derived under the old salt. Presenting that to Postgres on a fresh upstream spawn → 28P01.
  4. lib/supavisor/db_handler.ex:71 maps "28P01" => {:keep_pool, :invalidate_secrets}; the dispatch is at :359-361, and :885-889's handle_authentication_error/2 calls invalidate_global (:887) + delete_upstream_auth_secrets (:888). That is the self-heal, and it is why exactly one spawn fails.

The reported defect is the interaction with the periodic refresh:

  1. lib/supavisor/secret_checker.ex:13 — @interval :timer.seconds(15) (plus jitter, :161). :118's check_secrets/2 calls ClientAuthentication.refresh_if_changed/3 at :123 (logging at :124), which updates the validation secrets only. Nothing on this path touches the upstream secrets.
  2. delete_upstream_auth_secrets has exactly two call sites that can fire at runtime:
    • lib/supavisor/client_authentication.ex:93, inside handle_wrong_password/3 (:85) — and it is gated on refresh_if_changed(...) == :changed (:88). It is additionally rate-limited: :86's RefreshLimiter.check/1 allows 3 refreshes per pool id per minute (lib/supavisor/client_authentication/refresh_limiter.ex:8, :18) and silently returns {:error, :rate_limited} beyond that;
    • lib/supavisor/db_handler.ex:888, inside handle_authentication_error — which requires a real 28P01 to have already happened.
  3. So once SecretChecker has refreshed the validation secrets, refresh_if_changed returns :noop for every subsequent caller, handle_wrong_password skips the upstream deletion, and the stale salt-bound upstream material survives until a 28P01 forces it out. The cache reads current while the upstream credential is not.

Observed ordering (offsets relative to the rotation; environment-specific fields elided):

+0.000s  ALTER ROLE … PASSWORD  (identical plaintext)
+0.037s  [info]  Secrets changed or not present, updating cache      <- secret_checker.ex:124
+0.560s  [error] DbHandler: Auth error … "C" => "28P01"              <- fresh spawn still fails
+0.563s  [info]  Secrets changed or not present, updating cache      <- post-28P01 invalidation

Note the absence of ClientHandler: Validation secrets changed, cache updated, deleting upstream auth (client_authentication.ex:89-91) between the first refresh and the failure — that branch never runs, which is the bug.

Minimal reproducer

Transaction mode, auth_query tenant, default_pool_size = 2 (so the poolboy spec at lib/supavisor/tenant_supervisor.ex:71-79 is size: 1 (:75), max_overflow: 2 (:76), idle_timeout: 5 minutes (:78)):

  1. Drive one pooled query so the user-pool exists and its secrets are cached.
  2. ALTER ROLE <role> PASSWORD '<same value as before>' on the upstream database.
  3. Ensure the pool must create a new upstream backend — either terminate the existing pooler-owned backends, or demand more concurrency than there are live handlers. (If every handler is already warm the burst reuses them, nothing spawns, and nothing fails — this is the main reason the issue is easy to miss.)
  4. Observe one 28P01 for <role> originating from the pooler, then recovery.

Measured window

Reproduces when the fresh spawn happens within roughly 15 s of the rotation; clean by 20 s (consistent with the SecretChecker interval plus jitter). The failure is therefore intermittent in a way that tracks timing between the rotation and the next spawn, not load.

Why this is easy to miss

The 28P01 is frequently not visible to the client. Supavisor discards the failed handler, invalidates, and respawns, so a single client connection typically observes a completely successful connect and query while the 28P01 appears only in the Postgres and pooler logs. It becomes client-visible (as ECHECKOUTFAILED) when the failed spawn lands on a checkout that cannot be transparently replaced — e.g. concurrency saturating a small pool. Measured here: 5/5 single-client attempts succeeded while 28P01s were being logged.

Suggested direction

Invalidate the upstream secrets whenever the validation secrets are observed to have changed, rather than only on the handle_wrong_password path and only when that path is the one that noticed. That is, have SecretChecker's :changed outcome also call delete_upstream_auth_secrets/1 — the upstream credential is derived from the same SASL material and is stale exactly when the validation secrets were.

Activity

0

No activity yet.