| From: | Ajit Awekar <ajitpostgres(at)gmail(dot)com> |
|---|---|
| To: | Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com>, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>, Daniel Gustafsson <daniel(at)yesql(dot)se> |
| Subject: | Re: Continuous re-validation of session credentials |
| Date: | 2026-09-29 12:12:11 |
| Message-ID: | CAER375Nw65h70WW8YyGQemxnOwCDMxJUWZaimZ=zyk54U=GQ6g@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Thanks a lot Jakub for the detailed review.
1. Repeated errors resetting the timeout. Fixed: the next-validation
deadline is now tracked independently of disable_all_timeouts(); error
recovery re-arms at that same deadline instead of granting a fresh
interval.
2. Superuser silently disables validation. Fixed: the two GUCs are now
PGC_SU_BACKEND, not PGC_SUSET — settable only at session start, not via
SET afterward. Rationale documented in config.sgml
3. Logical replication skipping validation. Confirmed and fixed. The
walsender skip was unconditional; now scoped to
AmWalSenderProcess() && !am_db_walsender, so only physical replication
stays exempt. A logical walsender's streaming loop is serviced by a new
hook alongside the existing config-reload handling. New regression test
added, plus manual verification against an actively-streaming
pg_recvlogical session.
4. Idle sessions are not terminated in real time. Fixed: validation also
runs from the idle-wait point (ProcessClientReadInterrupt()), not only
at the next command boundary.
5. Test runtime (~57s/~23s). Down to ~22-30s by consolidating
independent scenarios into shared validation windows, without
shortening any timeout/interval values.
6. "CVT" unexplained. Comment added: CVT_ = CredentialValidationType.
7. Insufficient log detail. Each validator now logs specifics via
errdetail_log() (which check, which user, why).
8. Combined auth methods (clientcert=verify-full alongside
scram-sha-256). Fixed: the client cert is now explicitly re-checked
whenever the HBA line required one, regardless of the primary method's
validator.
9. CRL revocation never rechecked mid-session. Fixed for the core case
(ssl_crl_file, rechecked periodically). Two related items are
deliberately out of scope, now documented in config.sgml:
intermediate-CA revocation (the initial handshake already validates the
full chain) and ssl_crl_dir (not consulted by the periodic recheck).
10. PAM is not addressed yet proposing to keep it out of
scope as a follow-up patch
Please find the attached V3 patchset. Request a review.
Thanks & Best Regards,
Ajit
On Thu, 24 Sept 2026 at 16:32, Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com>
wrote:
> On Thu, Sep 17, 2026 at 2:02 PM Ajit Awekar <ajitpostgres(at)gmail(dot)com>
> wrote:
> >
> > Hi Hackers,
> >
> > Attached is v2 of the series, with three fixes folded into 0001 (no
> > changes to 0002-0005):
> >
> [..]
>
> Hi Ajit, I've played with the patchset a bit, but just with the 0001+0002
> parts. The basic funtionality works (as advertised :)), however to me it
> looks
> there are certain glaps that needs to be closed, see below:
>
> 1. With credential_validation_enabled=on, it looks like any ERROR resets
> the
> validation timeout. Technically the code after sigsetjmp() in postgres.c
> is used for error handling in main loop and the thing there calls
> disable_all_timeouts(), so with the patchset if one has rogue user
> that is intentionally calling something like below in constant loop:
> SELECT /* stealing */ somedata FROM somewhere;
> SELECT 0/0; -- to generate division by 0 and reset
> it won't disconnect him at and he will be able to carry on with selects
> as
> long as he wants to as those ERRORs, seem to reset the timeout
> machinery.
> If I stops erroring, then yes it will be invalidated. Perhaps some
> additional timeout of last successful validation would be needed to
> protect
> against that (?) somewhere nearby where EnableValidationTimeout() (??)
>
> 2. Dunno if that is issue or not, but superusers can disarm the credential
> invalidation mechanics completley silently via SET, because it's
> PGC_SUSET. This somewhat means that remote superuser authed by e.g.
> LDAP (and with it's cred invalidated there), could bypass this, contrary
> to what feature says and docs are silent about this too. So my question
> to wider auidence would be like this: should this be PGC_SIGHUP (so at
> least one could log disabling that GUC for everybody by superuser
> OR should that be documented some risks for superuser with this as this
> might save us from some future low-score-CVE-but-still-CVE I think).
>
> 3. 0001 says "Replication connections are out of scope for this feature"
> wihtin EnableCredentialValidationTimeout(), but I think it needs to be
> more detailed and explictly make distinction between physical rep and
> logical replication. I've tested both scenarios and for physical I think
> it is OK to be disabled (just mention it in docs explictlly?). However,
> for logical replication one can still request "replication=database"
> which is logical replication connection and still issue SQL commands
> there and cred. invalidation won't ever kick in (!!), so at least some
> comment/docs within 0001 need to be fixed or cover that scenario
> (expired
> but idle user connection with REPLICATION priv won't be ever
> invalidated).
> But I really think this escalates too quickly because if that's known
> issue, then it can be quickly abused by superusers: superuser (which e.g
> has been authed by e.g. LDAP and whom have and have this REPLICATION
> priv
> by defintiion) can simply keep logical rep connection open (psql
> replication=database) and won't be ever terminated as v2 stands today.
>
> 4. I have one doubt: all of his is implemented at the command execution
> boundary, which means idle session won't be terminated which kind of may
> look confusing to some DBAs ("I have configured cred invalidations,
> users
> are expired but they are still visible in the pg_stat_activity")
> or maybe that would be be fine, but then we should mention in docs
> enabling
> idle_session_timeout to avoid that situation. However I've noticed
> that this seems to be kind of sligthly inconsistent with how
> idle_session_timeout is implemented today: idle_session_timeout shuts
> down
> even idle sessions right on time, and not on command boundary, but
> maybe that's OK.
>
> 5. IMHO the new tests take way too much time:
> postgresql:authentication / authentication/008_continuous_validation
> ~57s
> postgresql:ssl / ssl/005_cert_continuous_validation ~23
> sadly shortening timeouts maybe have CI/buildfarm failures, dunno how to
> solve that, maybe some sleeps could be reduced, but how else we could
> test for time-based capability like that??
>
> 6. You seem to be using "CVT" shortcut in the patch, but I don't see place
> where the shortcut is explained (maybe in enum
> CredentialValidationType?)
>
> 7. Could we add more detailed logging to the server log using
> errdetail_log()
> on which exact validator failed and for which user exactly? Today with
> default logging setting I get some user killed like that we just get
> logmsg like:
> [43601] FATAL: session credentials have expired
> [43601] HINT: Please reconnect to establish a new authenticated
> session
>
> which is not helpful at all in terms of saying which user and why got
> the
> issue. Only if libpq attempts to auto-reconnect then we get:
> [43636] FATAL: password authentication failed for user "localuser"
> [43636] DETAIL: User "localuser" has an expired password.
>
> but that might be not always be true (and anyway it's hard connect those
> two PIDs are related).
>
> 8. With 0002 (TLS cert invalidation after now() being "notAfter"), I've got
> this:
> FATAL: session credentials have expired
> HINT: Please reconnect to establish a new authenticated session.
> -- new session below:
> LOG: could not accept SSL connection: certificate verify failed
> DETAIL: Client certificate verification failed at depth 0: certificate
> has expired. Failed certificate data (unverified): subject
> "/CN=localuser",
> serial number 2, issuer "/CN=myrootca".
>
> Anyway the above worked to invalidate for hostssl's auth-method "cert"
> configuration, but with with more advanced combined (two-factor) setup
> it did not:
> # TYPE DB USER ADDRESS METHOD
> # does work:
> #hostssl all all 0.0.0.0/0 cert
> # below doesn't work??? fallback to rolpasswdvalid??:
> hostssl all all 0.0.0.0/0 scram-sha-256
> clientcert=verify-full
>
> my already exisitng valid connection (at the connection time) was never
> invalidated, instead it has shown this @ ~09:05 GMT something odd from
> the start:
>
> $ psql "host=db.example.com dbname=postgres user=localuser \
> sslmode=verify-full sslcert=client.crt sslkey=client.key \
> slrootcert=root.crt"
> Password for user localuser:
> WARNING: role password will expire soon
> DETAIL: The password for role "localuser" will expire in 12 hours.
> psql (20devel)
> SSL connection (protocol: TLSv1.3, cipher: TLS_AES_256_GCM_SHA384,
> compression: off, ALPN: postgresql)
> [.. and much later @ 11:11 CEST it was still working
> postgres=> select now();
> now
> -------------------------------
> 2026-09-24 11:11:33.519307+02
> (1 row)
>
> so it has shown that rolpasswdvalid is going to expire in 12 hours, but
> the cert is going to expire much quicker, the cert was valid just till
> 11:06 CEST (just ignore +/- 2h due to TZ): Validity ()
> Not Before: Sep 24 08:54:10 2026 GMT
> Not After : Sep 24 09:06:21 2026 GMT
>
> so to me it looks like it is taking pg_authid.rolvaliduntil (based on
> uaSCRAM rather than minimum of those two (?)). Next attempt to
> re-connect after some time of course is impossible due to "SSL error:
> sslv3 alert certificate expired" (so the cert is really expired, it's
> just gap in the 2FA case where the v2-0002 fails to recheck cert
> credentials). In auth-validate at the end of CheckCredentialValidity(),
> with the below force-rechecking of CVT_CERT in case of success of
> rolpasswdvalid (e.g. SCRAM) itself:
> [..]
> + /* 2FA re-check */
> + if (result &&
> + validation_type != CVT_CERT &&
> + validators[CVT_CERT] != NULL &&
> + MyProcPort != NULL &&
> + MyProcPort->hba != NULL &&
> + MyProcPort->hba->clientcert != clientCertOff) {
> + elog(DEBUG1, "credential validation 2FA too:
> CVT_CERT");
> + result = validators[CVT_CERT] ();
> + }
> +
> return result
> }
>
> it seems to be behave OKayish with this code, but I'm not expert on auth
> on those matters. Anyway I'm just attaching steps to repro all of this
> (selfsigned CA and see next follow-up steps in #9 too).
>
> 9. TLS revocation lists (CRL) seem also not being rechecked, e.g. given a
> previous client.key would be stolen and the CA would revoke it like
> below:
>
> cat > crl.cnf << EOF
> [ca]
> default_ca = myca
> [myca]
> database = demoCA/index.txt
> default_md = sha256
> default_crl_days = 3650
> EOF
> # revoke client.crt (basically update's just CA db in mydemo/*)
> openssl ca -config crl.cnf -revoke client.crt -cert root.crt\
> -keyfile root.key
> # renerate CRL and save it root.crl
> openssl ca -config crl.cnf -gencrl -cert root.crt -keyfile root.key \
> -out root.crl
> # make it effective
> echo "ssl_crl_file = 'root.crl'" >> postgresql.auto.conf
> pg_ctl reload
>
> Now altough, the certificate has been revoked, the v2-0002 would have
> to be
> taught to be smart enough to terminate such backend(s). Reconnecting of
> course fails with: "SSL error: sslv3 alert certificate revoked", so the
> cert
> is really revoked, it's just some missing code in the patchset for
> checking
> actual cert status for not being revoked.
>
> Kind of similiar thing or open question would is if such implementation
> shouldn't verify intermediary CA revokes too (to verify full trust
> chain),
> but I'm not that deep into how x509/OpenSSL API works, so I cannot
> answer
> help with that (I don't know how heavy/inefficent that is). It feels
> kind
> of heavyweight, because be_tls_init() seems to be (re)loading in
> postmaster,
> so if the CRL is updated (with SIGHUP) that could potentially mean we
> should
> do the pricey loading of CRL every now and then, just to get up to date
> CRL data, so we revoked from actual information(???) Or maybe we should
> somehow cache it on every SIGHUP... or maybe take just document it as
> known
> issue and it doesn't work with CRL ??
>
> 10.Altough I haven't reviewed the remaining ones, qq, don't we do miss PAM
> here ? I've never used but if auth-method=pam would be deployed and the
> password would expire (PAM_ACCT_EXPIRED?) we should also invalidate the
> session, or is that out of scope for now from patch list?
>
> -J.
>
| Attachment | Content-Type | Size |
|---|---|---|
| v3-0001-Add-continuous-credential-validation-framework.patch | application/octet-stream | 56.0 KB |
| v3-0002-Add-TLS-client-certificate-expiry-and-revocation-che.patch | application/octet-stream | 28.2 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Matthias van de Meent | 2026-09-29 12:18:41 | Re: Direct TOAST v2, faster, smaller and no migration needed |
| Previous Message | Ashutosh Bapat | 2026-09-29 12:07:24 | Re: [PATCH] Two remaining shmem attachment issues in single-user mode |